Designing Inventory managment and Inspector-driven Item definition for Crafting Game

This is for this question: http://answers.unity3d.com/questions/607749/nested-if-addition-going-wrong-somewhere.html

I wanted to write up some suggestions, but the Answers system limits how much I can write and I thought this may be interesting for others as well, so here we go:

------------
EDIT: Code here: http://dl.dropboxusercontent.com/u/30016491/ItemInspectorCode.unitypackage
------------

Hi stuart6854 ,

I noticed some Design problems beside your actual problem with adding Items.


Using a List for the ItemDictionary isn't a good idea, because that way it depends on the insertion order which position an Itemdefinition gets and you can't access it fast by it's ID and things like you did in your ChopTree method will break

Code (csharp):

1. Item newItem = PlayerInventory.ItemDictionary[0]; // This will break when moving stuff around

Also gaps can be handy if you want to do stuff like

Code (csharp):

1. woodmaterial = 100, ironmaterial = 200, axe=25, shovel=26
2. ironshovelID = ironmaterial + shovel
3.  

Your instinct was good to name that Dictionary, so also use one:

Code (csharp):

1. static Dictionary<int,Item> _ItemDictionary = new Dictionary<int,Item>();
2.     public static Dictionary<int,Item> ItemDictionary{
3.        get{return _ItemDictionary;}
4.     }
5.  

But the Problem with this is, that it doesn't work together with something else I also wanted to suggest:
Get your basic Item definitions out of the code... Isn't it a maintenance nightmare to add new Items to the code?
Just imagine adding your 50. "public Texture2D someTexture" to that PlayerInventory.

The Problem is, that when we want to be able to access those Definitions via the Inspector, we need a List<>, because Dictionaries are not supported by the Inspector, but while programming, we want to use a Dictionary for fast access...

so let's use a trick to have both - and while we're at it, the TextureDefinitions should also be static or a Singleton.

We will use a Singleton Pattern for Everything, so we can store all the Definitions in a Prefab, so we don't tie that data to a single Scene.

Let's also not mix Item Definitions with how many of the Items we have. At the Moment you grab Items from the ItemDictionary and manipulate the currentStack - this of course doesn't work if you later want to pass stacks around as you always manipulate the Definition.

So let's rethink that:

Code (csharp):

1. public enum ItemType {    None, Weapon, Tool, Food, Resource, Construction  }
2.  
3. [System.Serializable]
4. public class Item {
5.     public string Name; // put this first, so it's used as a caption in Lists in the inspector
6.     public int ID;
7.     public string Description;
8.     public int MaxStack;
9.     public ItemType Type;
10.  
11.     public GameObject ItemPrefab;
12.     public Texture2D Icon;
13. }
14.  
15. [System.Serializable]
16. public class ItemStack
17. {
18.     public int Size;
19.     public Item Item;
20. }
21.  

Now... One Prefab to rule them all - a Singleton:

Code (csharp):

1. public class Definitions : MonoBehaviour {
2.  
3.     #region Singleton Code
4.     private static Definitions instance;
5.     public static Definitions Instance
6.     {
7.         get {
8.             if (instance == null) Debug.LogError("No Instance of Definitions in the Scene!");
9.             return instance;
10.         }
11.     }
12.  
13.     void Awake()
14.     {
15.         if (instance == null)
16.         {
17.             instance = this;
18.         } else {
19.             Debug.LogError("Just one Instance of Definitions allowed!");
20.         }
21.     }
22.     #endregion
23.  
24.  
25.     public List<Item> ItemDefinitions = new List<Item>();
26.    
27.     private Dictionary<int,Item> itemDic;
28.     public Dictionary<int,Item> ItemDictionary{
29.         get{
30.             // Definitions are never changed in game, so just copy references over once:
31.             if (itemDic == null){
32.                 itemDic = new Dictionary<int, Item>();
33.                 foreach(Item item in ItemDefinitions) itemDic[item.ID] = item;
34.             }
35.             return itemDic;
36.         }
37.     }
38.  
39.  
40. }

Drag that Behaviour on an Empty GameObject called "Singletons" and create a Prefab from it.
Never edit that GameObject in the Hierarchy, but always by selecting the Prefab in the Project View and editing it there. Now move all your Definitions to the inspector and we get:



Now let's also move the Craftable Definitions in here and at the same time improve them by allowing for Recipes with variable Number of Ingredients without the hack of setting the ResourceID to 99999.

We also rename this to ItemRecipe from CraftableItem, as technically it's not an Item - for that it would have to derive from Item:

Code (csharp):

1. [System.Serializable]
2. public class ItemRecipe {
3.     public string Comment; // just for the Designer, not shown in game
4.  
5.     public int CraftID;
6.     public int ItemID;
7.  
8.     public List<Ingredient> Ingredients = new List<Ingredient>();
9.  
10.     public Item Item{
11.         get {
12.             return Definitions.Instance.ItemDictionary[ItemID];
13.         }
14.     }
15. }
16.  
17.  
18. [System.Serializable]
19. public class Ingredient {
20.     public int ItemID;
21.     public int Amount;
22. }
23.  

And add the following to Definitions

Code (csharp):

1.     public List<ItemRecipe> Recipes = new List<ItemRecipe>();
2.    
3.     private Dictionary<int,ItemRecipe> recDic;
4.     public Dictionary<int,ItemRecipe> RecipeDictionary{
5.         get{
6.             // Definitions are never changed in game, so just copy references over once:
7.             if (recDic == null){
8.                 recDic = new Dictionary<int, ItemRecipe>();
9.                 foreach(ItemRecipe rec in Recipes) recDic[rec.CraftID] = rec;
10.             }
11.             return recDic;
12.         }
13.     }
14.  

And once we move the Recipes into the Prefab, we get this:



Now before we look into the Inventory, one word of caution:
At the moment you don't stack consumables like your Axe - I assume it breakes after a while?
If you will have consumables that stack later, you need to check their consumed status and only stack things that have the same damage.

Alright - this is where you had problems to begin with and that's because your ChopTree method wasn't creating new Items when it said it would, but just referenced the ones in the Dictionary, so when you wanted to stack them, it could be that you would stack the same Item instance with itself, but we're looking into that later... for now let's just assume that we get new individual instances and want to add them to our Inventory.

Code (csharp):

1. public class Inventory : MonoBehaviour {
2.     // This can also be used for chests etc.
3.  
4.     public int Size = 8; // Number of item stacks allowed in here
5.  
6.     void Awake()
7.     {
8.         stacks = new List<ItemStack>(Size);
9.         while (stacks.Count<Size) stacks.Add(null); // initialize, so List can be used like an Array later
10.         readonlyStacks = stacks.AsReadOnly();
11.     }
12.  
13.     // We don't expose this in the Inspector, because we don't want Items created that aren't a copy of our Definitions.
14.     // This should only be used during runtime, not to give the player starting Items. This would have to be done in Code
15.     private List<ItemStack> stacks;
16.     private ReadOnlyCollection<ItemStack> readonlyStacks;
17.  
18.     // we only return a readonly Version, because the Inventory methods should be used, not the List methods
19.     public ReadOnlyCollection<ItemStack> Stacks
20.     {
21.         get { return readonlyStacks; }
22.     }
23.  
24.     // Removes the specified ItemStack from the inventory and returns it.
25.     public ItemStack Remove(int pos)
26.     {
27.         ItemStack stack = stacks[pos];
28.         stacks[pos] = null;
29.         return stack;
30.     }
31.  
32.     // Removes one Item from the specified ItemStack and returns it.
33.     public ItemStack RemoveOne(int pos)
34.     {
35.         ItemStack stack = stacks[pos];
36.         if (stack == null) return null; // nothing there
37.         stack.Size--; // take one
38.         ItemStack newSt = new ItemStack(){ Size=1, Item=stack.Item };
39.         if (stack.Size==0) stacks[pos] = null; // nothing left: clear slot
40.         return newSt;
41.     }
42.  
43.     // Adds the specified stack to the inventory, returning the remaining stack that didn't fit or null if all fit
44.     public ItemStack AddStack(ItemStack stack)
45.     {
46.         if (stack.Size<1 || stack.Item==null) Debug.LogError("Trying to add empty stack to inventory");
47.  
48.         // First Run: Try to add stack to stacks already in the inventory
49.         foreach(ItemStack st in stacks)
50.         {
51.             if (st != null  st.Item.ID==stack.Item.ID) while(st.Size<st.Item.MaxStack  stack.Size>0)
52.             {
53.                 stack.Size--; // move one...
54.                 st.Size++; // ...over
55.             }
56.         }
57.  
58.         // stack already empty?
59.         if(stack.Size==0) return null;
60.  
61.         // Second Run: Try to put the rest in an empty slot
62.         for(int i = 0; i<stacks.Count; i++)
63.         {
64.             if (stacks[i]==null){ // found a free slot
65.                 stacks[i] = stack;
66.                 return null;
67.             }
68.         }
69.  
70.         return stack; // return what's left
71.     }
72.  
73. }

Now, how do we test this? I assume you will alter the state of Items, so we are not allowed to use the ones in the Dictionary, but need to make copies, so we can alter the damage etc. without changing the Originals.

For this, let's add some code to Item - Mono already has a method, so we don't need to copy ourselves:

Code (csharp):

1.     public Item Clone()
2.     {
3.         // return a shallow copy, as we don't need to clone the prefab and icon
4.         return (Item)this.MemberwiseClone();
5.     }

But be aware that referenced objects are not cloned - which is good for the prefab and icon, but if you reference something that also needs to be cloned, you need to call MemberwiseClone() on that object manually;

Also, let's add some convenience methods to Definitions:

Code (csharp):

1.     public static Item ItemClone(int id)
2.     {
3.         return instance.ItemDictionary[id].Clone();
4.     }
5.  
6.     // used internally, doesn't check for valid stack sizes
7.      public static ItemStack StackClone(int itemID, int size)
8.     {
9.         return new ItemStack(){ Size = size, Item = ItemClone(itemID) };
10.     }
11.  

So we can finally do some Inventory tests:

Code (csharp):

1. public class TestStuff : MonoBehaviour {
2.  
3.     void Start () {
4.         Inventory inv = GetComponent<Inventory>();
5.  
6.         for(int i = 1; i<=4; i++)
7.         {
8.             Debug.Log("### Round " + i);
9.             TryAddStack(inv, Definitions.StackClone(3, 12)); // 12xChicken (maxStack:25)
10.             TryAddStack(inv, Definitions.StackClone(4, 3)); // 3xWoodWall (maxStack:5);
11.             TryAddStack(inv, Definitions.StackClone(2, 1)); // 1xAxe (maxStack:1)
12.         }
13.         Debug.Log("---");
14.         printInventory(inv);
15.     }
16.  
17.     void TryAddStack(Inventory inv, ItemStack st)
18.     {
19.         ItemStack rest = inv.AddStack(st);
20.         if (rest != null) Debug.Log("Couldn't fit " + rest.Size + " x " + rest.Item.Name);
21.     }
22.  
23.     void printInventory(Inventory inv)
24.     {
25.         foreach(ItemStack st in inv.Stacks)
26.         {
27.             if (st!=null) { Debug.Log(st.Size + " piece(s) of " + st.Item.Name); }
28.         }
29.     }
30. }

Which gives this output:

And now your ChopTree method can also be tidier:

Code (csharp):

1.     void TryToChop(){
2.         int amountGiven;
3.         int itemID = -1;
4.  
5.         Ray ray = Camera.main.ScreenPointToRay(new Vector3(Screen.width / 2, Screen.height / 2));
6.         RaycastHit hit;
7.  
8.         if (Physics.Raycast(ray, out hit)){
9.             string hitTag = hit.transform.tag;
10.             if (hitTag == "Tree"){
11.                 amountGiven = Random.Range(3, 7);
12.                 itemID = 0; // Wood
13.             } else if (hitTag == "Rock"){
14.                 amountGiven = Random.Range(1, 4);
15.                 itemID = 1;  // Stone
16.             }
17.  
18.             if (itemID >= 0) // hit something good
19.             {
20.                 Inventory inv = GetComponent<Inventory>();
21.                 ItemStack choppedStuff = Definitions.StackClone(itemID, amountGiven);
22.                 ItemStack rest = inv.AddStack(choppedStuff); // watch out, choppedStuff.Size will change!
23.                 if (rest != null) Debug.Log("Dropped " + rest.Size + " x " + rest.Item.Name + " on the floor");
24.                 StartCoroutine(DisplayGUI(amountGiven, choppedStuff.Item.Name));
25.             }
26.         }
27.     }
28.  

 

How could you do something similar with Inherited items?

Like when you have an Item class, and inherited Weapons, Keys, Consumables, etc.
I'm looking for a nice way to build definitions for these variety of objects within the Inspector.

I hope the basic idea of storing definitions and editing the values in the inspector could also work with inherited types.
Or how would you make the behaviour of using a Consumable and a Weapon differ in your scenario?

By the way this is a very good resource you put on here, so thanks!
Hope you could elaborate.

 

Make a base class Item with everything you need to display it, pick it up, etc.

Create derived class such as consumable with a virtual function for 'OnUse'. That way you can just derive each item like the following.

Code (csharp):

1.  
2. class Potion : Consumable
3. {
4.     public float HealingAmount;
5.     public override void OnUse(Player user, Player target)
6.     {
7.        target.health += HealingAmount;
8.        base.OnUse(user,target); //I'd have base.OnUse handle removal of a consumable from the inventory
9.     }
10. }
11.  

Then a small, medium, and large potion can be prefabbed and artwork just switched.
Weapons become something similar.

Code (csharp):

1.  
2. class Weapon : Item
3. {
4.    public PlayerStats StatModifiers; //Assume this displays all stats in inspector. You can use a PropertyDrawer to do that
5.    public virtual void OnUse(Player user);
6.    public virtual void OnHit(Player user,Player target)
7.    {
8.         target.buffs.AddStatusModifier("Poison"); //Simple example of what you can do with functions like this
9.   }
10. }
11.  

 

How would you then add that new weapon or consumable to the dictionary or inventory or reference it in a recipe in the editor, then? Don't their lists only take elements of our regular "item" class type?

I think that was the point BigRoy and I are trying to get at...:

...rather than the more general question of "how do I inherit from a base class?"

 

I'd also like to know if this can somehow be used with inherited items. It also might be problematic to have to remember arbitrary indexes for each item, but I can't think of a solution for that.

 

I like the idea, but I don't feel a nested list on a MonoBehaviour is very maintainable. For example it's going to almost be impossible to find anything if you have 100+ items. Not to mention it's difficult for designers and artists to search for item content in the Unity project.

I think it's better to place all the item definitions into a folder in Resources, then dynamically catalogue them at run time. Other than sucking up a tiny bit of memory and a second of the user's time when the game boots, this will have no performance impact. And it allows you to scale your inventory items by adding new MonoBehaviours if you so desire.

 

Slightly off-topic, but I prefer using ScriptableObject databases with a custom editor. It suffers from a lot of same "hundreds of items aren't easily maintainable" arguments around here, but organization is a matter of implementation not method.

The following screenshot shows my sub-categorization (using inheritance, no less), and this occurs automatically as more sub-type databases are created. Make a new Item subclass, add that subclass type to a list, and everything else is automatically generated- takes 10 seconds and the categories and items themselves are collapsible.

 

@Lysander Thanks for sharing this. We're doing something extremely similar with class inheritance. Except we decided to use Unity's built-in inspector instead of a custom one. Thought about using scriptable objects, but decided to use MonoBehaviors loaded from a designated Resources folder. A pinch more overhead at initial load, but requires a lot less dev work by avoiding the custom inspector.