inventory class: singleton or static?

[Epsilone]

Hi guys
I have a problem.

Short version:
I have an inventory and don’t know if I should use the singleton pattern or a class with only static members

Long version:
I’m currently making a game with an inventory class where you can add items and take items. the inventory is unique and contain
-a dictionaly (with the items and their quantities)
-methods to take and put items in the inventory
-methods to check if we have specific items

I created my inventory class as a classic class, but I rapidly found myself transorming the methods into static methods and variable into static variable. and in the end I only have static field.

Afraid that it maight cause some problems I looked on the web if all statics class were bad, and found this response (for java but that sould not change anything):
“Static classes are OK, just don’t forget to add a private constructor. Ah, and mind multi-threading accesses.”
“Make sure it doesn’t have any state. This is, there’s no field in the class unless it’s declared static final”

For the first one I guess the private constructor is to prevent ourself to instanciate it by mistake. I don’t think we can make the constructor private in unity but it’s a small project so I will just be carefull
The second statement is more problematic: my dictionary will have many state as the inventory content change all the time. However I don’t understant why it might be a problem as long as I deal with the concurential access.

I have many states, so I’m wondering if I should rather use the singleton pattern. I have heard many thing about the singleton patter. First that it was a normal and usefull pattern, then that it was an awefull pattern to avoid at all cost, and finaly that it is a good pattern as long as it is used correcly. But I have no idea if a unity inventory would be a good case for the singleton partern.

So yes… I don’t know if I should do a full static class. I don’t know if I should use the singleton patern. I don’t know if I should use something else but these king of classes are current in a game project, so I’m sure I’m not the first one to hesitate between the 2.

Do you guys have advice?

[bgulanowski]

A question like this risks bringing out the ideologues, but I guess ideology and game development rarely go together.

End of day, as an old college prof used to say, “Working code is king!”

In general, you are creating a global manager object. Implicit in your game design, you only need one. But do you need the same one the entire time the game is alive? Or do you need to create a new one sometimes? What if you load a saved game? Or quit to main menu?

If you don’t need the inventory manager to always be alive, you should consider a normal class and a normal object. It can be a property of some other top-level object that is responsible for being the root object in the object graph.

If you need to access this object from everywhere in your code, that might make it hard to change the inventory manager. But that’s speculation. Generally, other objects that need to talk to the inventory manager are either also managers, or have shorter life spans, and can be provided a reference to the inventory manager when instantiated.

Static classes are best used as containers for stateless functions. Keeping static, mutable state around is considered bad form because it is error-prone to clean up. Whereas with normal objects, releasing/disposing the object cleans them up. Create a new one to start fresh. No old data hanging around. Can’t do that with a static class.

This is an old topic in Object-Oriented programming. Although it’s relevant in all kinds of programming. Avoid global state, avoid persistent state, avoid mutable state, when possible. But solve your real problem first, then come back to design improvements later.

[palex-nx]

It depends on the nature of your game. If only player have inventory, then it makes no difference how you implement it with static class or single instance. If it is possible others will have inventory, something like NPC or chest may be inventory container for instance, then it is better to use singleton for character because you can easily create instances for other uses. If it is possible there will be more than one character with inventory, like in multiplayer or multi-character (lost vikings, goblins), then you should consider instantiating an inventory object for every character and can’t use statics or singletons.

[lordofduct]

I would suggest singleton.

Well really I would suggest neither singleton nor static… for scalability.

BUT, if you can’t, or don’t want to. Go with a singleton (or service locator), this way the inventory has object identity (you can pass references to it around). That way if you decided to scale to a multi-inventory system you can do so with less work than if it were static.

[Epsilone]

Thank you both for your responses

@palex-nx : Only player have inventory. Actualy there is no player, it’s more an inventory for a colony but it doesn’t change the fact that the inventory is unique.

@lordofduct : what do you mean you suggest neither for scalability? It’s certain the game will keep only one slall inventory but I’m curious what problem could arise if stuff scale up and what you would recommend instead of these 2 solution?

[lordofduct]

What I mean about scalability is in case you ever decide to need more than 1 inventory. I don’t know your game, so I don’t know your actual needs.

You say something about it being the inventory for the ‘colony’… what if the scope of the game changes and there’s more than 1 colony?

What if there are some other NPC that has an inventory… like say a tradesman (or… uhh, a foreign colony that does maritime trade??? I don’t know your game, so yeah). How is their inventory going to be represented? Are you going to have distinct classes for every potential inventory that might come up?

This is what I mean about scalability. By selecting either singleton or static you’re restricting yourself to a single inventory ever. You can’t represent anything but a single inventory with your singleton.

…

What my suggestion would otherwise be is ‘Inventory’ is just a component/class that a ‘player/colony/etc’ can have. It’s composited as part of what makes a ‘player/colony/etc’. So this way if you have a reference to a ‘player/colony/etc’ you also have a reference to its ‘inventory’.

In simple terms you might just say:

var inv = player.GetComponent<Inventory>();

Personally I try to keep the number of singletons/statics/services I have down to a minimum. If we’re talking about stuff that makes up the player I might have a static location to easily grab a reference to the player (like a player locator service), and then I get its components off of the player itself. Health? It’s attached to the player (colony?). Inventory? Also attached to the player.

var player = Services.Get<IPlayerLocator>().GetPlayer(...);
var healthmeter = player.GetComponent<Health>();
var inventory = player.GetComponent<Inventory>();

//extended example of how we can have more than 1 inventory
var merchant = *some merchant in our scenario*;
var merchInventory = merchant.GetComponent<Inventory>();

PerformTrade(inventory, merchantInventory);

[Epsilone]

@bgulanowski : Sorry I didn’t see your post :o
Mmmh you are right, the inventory will need to be saved and load when we quit and launch the game… I will have a clear impact on the architecture choice.

Concerning the acces to the inventory, the classes that can modify it are limited (only 2 classes) but a lot of classes need to acces the inventory only to read informations and act acordingly, if it’s a simple object, it will quickly become messy to pass a reference to half of the classes of my project.

You say “mutable state around is considered bad form because it is error-prone to clean up.” but I don’t get why. You mean if I want to restart the game and empty the inventory, just emptying the dictionary containing the items will lead to errors?

“But solve your real problem first, then come back to design improvements later.”
I should do a poster with that sentence, so much wisdom… I will keep that in mind

[Epsilone]

@lordofduct ok I understand better what you mean, thank you

[bgulanowski]

@Epsilone why is it “bad form”, or why is it “error-prone”?

A single dictionary stored in a single static property may be easy to handle, but it’s not about one property. It’s about what happens when they increase in number. The complexity, and associated risks, tend to accelerate at geometric and even factorial rates. One or two, even three, is OK, but beyond that, the risk line goes straight up.

“Bad form” simply because choosing a risky option is irrational when there are alternatives.

Developing an intuition for what is or isn’t risky, in the land of complexity (Extremistan), is not easy. It also takes a lot of expertise, in the right kind of math, to be able to make sound empirical arguments about risk. But, with experience, we learn which choices have unintended negative consequences, and to avoid them.

For me, static (global) mutable state, classes being used like normal objects, and unnecessary singletons are all things I have seen cause problems. I can’t even remember the exact problems anymore. (But for one thing, they are more annoying to test.) That doesn’t mean they are always bad, or even usually bad. Just sometimes bad in serious ways.

This is a general quality of many kinds of expedient choices. But in a world of high uncertainty, sometimes expediency saves time. If time is the most scarce asset, it is often more rational to be greedy (algorithmically): to save time now, and reconsider when you have more information.

[lordofduct]

In my day job I’m currently converting a huge code base from vb6 to C#.

In it they rely heavily on global static variables to store junk. Variables up in the air like “gVehNo” and “CustListReturn”.

There’s thousands upon thousands of these just sitting out there.

Why do they exist? Oh well, Form A opens Form B which opens Form C and Form C asks the user to input some information… that information is used to query the database and get some results (say a list of customers, or a vehicle number). Once done Form C closes, Form B closes and Form A is back at work. But Form A needed those results from Form C.

How does Form A get a reference to those results?

Easy! Just create a global static variable, say a list or a dict or whatever, and toss the results in there and than Form A can just look in that global for the results.

…

Do this 1000+ times… come back 6 months later… and now I ask you “What is CustListReturn” for?

Uhhhhhhhhhh

I search the project for uses of it and notice Form C references it and Form A… but so does Form X and Form Y. Turns out X and Y also needed to do a similar situation of returning customers from Y to X and and decided to do the same global fix… but why add ANOTHER ‘CustListReturn2’ list… lets be crafty and recycle the first list.

Also, what does A have to do with C? A doesn’t reference C ever… yet A reads from CustListReturn, and C writes to it. What’s going on here? Oh… I have to dig in and find out that A opens B opens C. But what about N which ALSO opens C because it happened to also need it?

Oh and lets not forget that there’s another global variable called ‘CMode’ which dictates how Form C behaves. If it’s in mode 1 it edits customers, in mode 2 it adds customers, and mode 0 means someone clicked the the cancel button on Form’s C,D,E, or F.

Form C only writes to CustListReturn if it’s in mode 2, which is set by Form A, and that’s how I know Form A relies on Form C.

But then one day Rick from accounting says they want Form Y to be used during a workflow they do from A-B-C… Form Y should open up so they can pick customers whose birthdays are shared with Einstein cause they get a tax credit. A developer goes in and adds this functionality but forgets that Form Y modifies ‘CustListReturn’ so now when Form C edits it, the Form Y goes and edits it, and we return to Form A, CustListReturn is in an unexpected state and Form A doesn’t behave correctly.

…

And I slowly strangle myself in the corner.

…

Mutable global/static fields are terrible because they create winding messes of spaghetti code that are a headache to maintain.

This isn’t to say NEVER use them… but rather to say use them sparingly and intelligently.

But at the end of the day if your static isn’t something that can be described as a “utility”… you’r probably doing it wrong. (I glare at a large portion of the Unity API like Random, Time, and more).

What does any of that even mean???

[lordofduct]

TLDR;

Having static/globals for mutable state is like organizing your house by just laying everything out on the ground.

“I store my keys in the middle of the living room floor” is absurd.

And yet in the programming world you can tell an entire team of developers that’s insane for years on end… and they’ll continue doing it over and over again.