Dictionary<List<Interface?>, int> Help?

I'm trying to build my inventory system, and I thought I had it working... until I tried to add two different items to the inventory. Because they both inherit from BaseItem, it thought I was trying to add a duplicate object.

Currently, I'm using Dictionary<BaseItem, int>. The int is for quantity. Now, this won't work, because I can only have one BaseItem in the Dictionary. So! I'm kicking around a couple options:

A: Should I create a new GameObject whenever a new item is added to the inventory, attach the class script to that and then store the GameObject instance in the dictionary?

B: I considered (as in the title of the post) storing a list in the dictionary which would hold an interface for all of the variables of the items. My brain isn't fully wrapped around that one yet, but isn't that just the long way to get to the same problem? Wouldn't it see the interface as a duplicate entry?

C: And this is probably the ugliest solution (but one that would work). I could just store the String name of the item, and when I need to get its variables, I would iterate through all items in the game looking for a matching name and grab the variables then.

Thoughts? Help? Thanks!

P.S. Someone should really make a video tutorial on how to create an old-school JRPG inventory system with Unity5... just sayin.

 

You should be able to use different instances of base item as different keys.

Another way is to use a unique ID as a key, rather then the actual item itself.

 

Wouldn't that be similar to my ugly "name look-up" idea? All of the individual items in the game will have a unique name, so it's basically the same as an ID.

And to create instances, wouldn't I need to use empty game objects? I use "new BaseItem" when I create an item, but that doesn't seem to create a separate instance.

 

You only need GameObjects if your class is a MonoBehaviour/Component that needs to be attached to a GameObject.

If you call 'new SomeClass', you're creating an instance of that class.

Code (csharp):

1.  
2. var obj1 = new BaseItem();
3. var obj2 = new BaseItem();
4. Debug.Log(object.ReferenceEquals(obj1, obj2)); //will output false
5.  

 

int comparisons are much faster then string comparisons. You would also calculate the whole set programmatically, so there is no chance of typos.

However different instances should be able to be used as different keys, as indicated in @lordofduct's post

 

You can definitely use different instances of the same class as different keys in a dictionary. However, there are several subtleties here that could be tripping you up.

First, I presume BaseItem is a class? That means it's a reference type, which means when you create an entry in the dictionary using a BaseItem as a key, the dictionary stores a reference to that item (not a copy). If you subsequently change that item, you are also changing the dictionary's key. Basically, once you've written something like myDictionary[myItem] = value, you shouldn't ever change myItem again--if you want a different item after that point, you need to use new.

Secondly, when you try to retrieve a value from your dictionary based on a key, the dictionary has to check whether the key you're looking up is equal to one of the keys it has stored. Again, assuming BaseItem is a class, the default equality comparision is reference equality--meaning that two items will only compare as equal if they are exactly the same instance. If you say "new BaseItem()", then that new item will not compare as equal to any item you have ever used before, even if it contains exactly the same data.

If you want some other way of testing for equality, you can override the Equals method on your class. (If you do, you should probably also override GetHashCode).

But back up for a minute. You said that the purpose of this dictionary is to keep track of the quantity of each item in your inventory. That means that, conceptually, the key shouldn't be an item, it should be a kind of item.

Have you considered using, say, an enum?

 

That is a lot of info...
Ok, so yes, BaseItem is a class, and no, it's never actually stored in the inventory itself. I just have the inventory storing BaseItem "types". I have things like "Hunting Rifle" which is a type of Weapon, which inherits from BaseItem.

Maybe I'm not creating my items correctly?

I have this in a static class called ItemManager

Code (CSharp):

1. public BaseWeapon huntingRifle = new BaseWeapon("Hunting Rifle", "Shoot, shoot shoot. Bullet, bullet, gun.", BaseWeapon.WeaponTypes.RIFLE, 5, 10);

Then I have this function run when the game starts:

Code (CSharp):

1. InventoryController.thisInventoryController.AddItemToInventory(ItemManager.thisIM.huntingRifle, 1);

And that obviously calls the function on my InventoryController...which is currently under construction because I tore it apart trying to fix it

How would I use an enum in this case?

 

And what precisely is the unexpected behavior that you've noticed?

 

I'm also trying to figure out why your system doesn't work. It's ugly, and certainly not how I would approach it. But it doesn't appear fundamentally broken.

 

Ugly and inefficient is kinda my trademark. It's the result of a non-programmer getting sick of unreliable programmers and deciding to do it himself.

I'm having a tough time explaining this because my code is all over the place, across several different scripts. (That's me, trying to keep things clean...and failing, by breaking everything into individual scripts.)

How would you guys approach this? Any pseudo-code suggestions?

(I'm in the process of dismantling my code and forcing it to work. I should be stopped. If you thought the above examples were ugly, wait until you see this.)

 

Speaking off the top of my head, and without testing here are some ideas.

I'd implement a custom Inventory collection. This would expose the methods AddItem, RemoveItem and the like.

Inside the Inventory collection I would have a List<InventoryItems>.

The InventoryItem class would expose fields for baseItem, noInStock,.

AddItem would iterate through the List until it found an item with the same unique identifier for baseItem. Then it would increment that item. Or it would not find the baseItem and add it to the end of the list.

Removing items would work in much the same way.

Yeah, I've been meaning to do this for ages. Plenty of people asking for it. Maybe tonight is the night.

 

I'm doing something like this, using unique, integer ids for the items and then storing those in the inventory list. Then, when I need to access the variables for a particular item, I wrote a function "GetItemByID" which takes an integer ID from the inventory list and returns the associated BaseItem.

That, however, creates a new problem. Because I'm doing it this way, my GetItemByID function iterates through all items in the game in order to find one that matches the ID. That means I have to store all items in the game in a List (or something else that I can iterate through).

I have a script that creates this list when the game starts. However! I can't reference those items until after the game has started. So, my AddItem(rifle, etc, etc) doesn't know what I'm talking about. DAMMIT! I suck

 

That means you have O(n) insertion and deletion, because you have to scan the entire list one item at a time. You should probably be using a SortedList or a Dictionary here.

If khanstruct is actually using each instance of the BaseItem class to represent a type of item and hold that item's stats, I think his plan of using a Dictionary<BaseItem, int> is pretty reasonable (if any of us could figure out why it's not working...)

 

Yeah. For an inventory script for an RPG I don't see the O(n) as being that critical. But you are right, with a very large inventory this would cause you problems. In my head for inventory items the order matters, which is why I would use a list. If order genuinely doesn't matter then a dictionary is better.

Is it possible that you are attempting to add the same item twice? Your AddItem method should have a check to see if the item already exists in the collection before a new item is added.

 

For my game, I'm using the following general approach to maintain a library of kinds of things:

First, define an enum that simply lists all the kinds, something like this:

Code (CSharp):

1. public enum UnitType
2. {
3.     Pawn,
4.     Knight,
5.     Bishop,
6.     Rook,
7.     Queen,
8.     King
9. }

Then I define a class that holds information about various unit types. Each instance of this class contains the info for one type, but the class also contains a static dictionary (initialized in the static constructor) that contains everything. Like this:

Code (CSharp):

1. using System.Collections.Generic;
2.  
3. public class UnitInfo
4. {
5.     public string code;
6.     public string name;
7.     public string description;
8.     public int pointCost;
9.  
10.  
11.     public UnitInfo(string code, string name, int pointCost)
12.     {
13.         this.code = code;
14.         this.name = name;
15.         description = "";
16.         this.pointCost = pointCost;
17.     }
18.  
19.  
20.     public static readonly SortedDictionary<UnitType, UnitInfo> info;        // Main lookup table
21.     static UnitInfo()
22.     {
23.         info = new SortedDictionary<UnitType, UnitInfo>();
24.  
25.         info [UnitType.Pawn]    = new UnitInfo ("P",    "pawn",    1);
26.         info [UnitType.Knight]    = new UnitInfo ("N",    "knight",    3);
27.         info [UnitType.Bishop]    = new UnitInfo ("B",    "bishop",    3);
28.         info [UnitType.Rook]    = new UnitInfo ("R",    "rook",    5);
29.         info [UnitType.Queen]    = new UnitInfo ("Q",    "queen",    9);
30.         info [UnitType.King]    = new UnitInfo ("K",    "king",    7);
31.  
32.  
33.         // Unit descriptions
34.         info [UnitType.Pawn].description =
35.             "<b>March:</b> Forward 1 (or 2, from the first or second Rank)\n" +
36.             "<b>Attack:</b> Diagonally Forward 1";
37.         info [UnitType.Knight].description =
38.             "<b>March/Attack:</b> Leaping (1,2)";
39.         info [UnitType.Bishop].description =
40.             "<b>March/Attack:</b> Riding Diagonally";
41.         info [UnitType.Rook].description =
42.             "<b>March/Attack:</b> Riding Straight";
43.         info [UnitType.Queen].description =
44.             "<b>March/Attack:</b> Riding Straight or Diagonally";
45.         info [UnitType.King].description =
46.             "<b>March/Attack:</b> 1 in any direction";
47.     }
48. }

Then, when you just care about what kind of unit something is, all you need to pass around is a UnitType, but any time you need to look up the additional information, you can say UnitInfo.info[unitType] and access it all easily. You can also iterate over all types in the game by saying

Code (CSharp):

1. foreach (UnitType utype in Enum.GetValues(typeof(UnitType)))

 

It did. If it found a duplicate, it would just increase the quantity. If it didn't then it would add a new item to the Dictionary.

I tested this by adding three things. First, I'd add 1 of Item A. Then, I'd add 5 of Item A. Then I'd add 1 of Item B.

The first time I did this, it ended up just adding a total of 7 Item A's. I tried fixing it, and it added 6 of Item A, then it crashed and said "InventoryList already contains the Key, BaseItem". Grrrr!

I'm sorry I can't be much help. I've torn so many pieces of my code out and rewritten them, that it would take me forever to put them back the way they were when I had that functionality working. Of course, if I can't get my current approach to work, I may just delete the whole inventory system and start over.

 

Sometimes this is the best way.

 

You may want to consider using some form of version control. It's not super-important if you're the only person working on the project, but it would let you "go back in time" to see how things used to be, and also acts as a backup in case you accidentally break your code somehow.

 

Alright, even with my ugly, bloated, hard-coded approach, it still didn't work. Time to start over.

 

Before even writing a line of code, try to write down all the things you want the system to do, and the functions and attributes you need to make it happen. That often helps me when I am stuck, and reduces the time I need to think about how to implement it.

 

why dont you use an enum as your weapon type? without knowing exactly what youre trying to do hard to provide best solution, but something like this might fit?

Code (csharp):

1.  
2.  
3. enum WeaponType
4. {
5.   HuntingRifle,
6.   Shotgun,
7.   PlasmaGun
8. };
9.  
10. public class InventoryQuantity
11. {
12. public int Quantity;
13. public IBaseWeapon baseWeapon;
14. }
15.  
16. Interface IBaseWeapon
17. {
18.   WeaponType WeaponType
19.   string Name;
20.   //whatever
21.  
22.   void Fire();
23. }
24.  
25. public class HuntingRifle : IBaseWeapon
26. {
27.    public void Fire()
28.    {
29.      //pew pew
30.    }
31.  
32. }
33.  
34. Dictionary<int, InventoryQuantity> Inventory;
35.  
36. void yolo()
37. {
38.   HuntingRifle r = new HuntingRifle();
39.   r.WeaponType = WeaponType.HuntingRifle;
40.  
41.   InventoryQuantity iq = new InventoryQuantity();
42.   iq.Quantity = 1;
43.   iq.baseWeapon = r;
44.  
45.   Inventory.Add(r.WeaponType, iq);
46.  
47. }
48.  
49. void fomo()
50. {
51.   if(Inventory.ContainsKey(WeaponType.HuntingRifle))
52.   {
53.      (Inventory[WeaponType.HuntingRifle] as IBaseWeapon).Fire();
54.  
55.   }
56. }
57.  
58.  

 

Ok, I've got it mostly working, using the Dictionary<BaseItem, int>.

I am getting an "out of sync" error when I first run the game. I'm assuming that due to me changing the values in the Dictionary at runtime.

Currently, I'm having an odd problem that shouldn't even exist. I have a bunch of public Sprite variables that are the icons for each item type. I have a really simple switch case function that gets the WeaponType and assigns the appropriate icon. But I'm getting a NullReferenceException

Code (CSharp):

1. public BaseWeapon(int ID, string ItemName, string ItemDescription, WeaponTypes WeaponType, int Range, int BaseDamage)
2.     {
3.  
4.         itemName = ItemName;
5.         itemDescription = ItemDescription;
6.         weaponType = WeaponType;
7.         range = Range;
8.         baseDamage = BaseDamage;
9.         itemIcon = SetIcon(WeaponType);
10.     }
11.  
12.     public Sprite SetIcon(WeaponTypes wType)
13.     {
14.         switch (wType)
15.         {
16.             case WeaponTypes.BAND:
17.                 return ItemManager.thisItemManager.bandIcon;
18.             case WeaponTypes.BATON:
19.                 return ItemManager.thisItemManager.batonIcon;
20.             case WeaponTypes.CANNON:
21.                 return ItemManager.thisItemManager.cannonIcon;
22.             case WeaponTypes.DAGGER:
23.                 return ItemManager.thisItemManager.daggerIcon;
24.             case WeaponTypes.DISK:
25.                 return ItemManager.thisItemManager.diskIcon;
26.             case WeaponTypes.GRENADE:
27.                 return ItemManager.thisItemManager.grenadeIcon;
28.             case WeaponTypes.HAMMER:
29.                 return ItemManager.thisItemManager.hammerIcon;
30.             case WeaponTypes.PISTOL:
31.                 return ItemManager.thisItemManager.pistolIcon;
32.             case WeaponTypes.RIFLE:
33.                 return ItemManager.thisItemManager.rifleIcon;
34.             case WeaponTypes.SWORD:
35.                 return ItemManager.thisItemManager.swordIcon;//Null Reference Here?
36.             default:
37.                 return ItemManager.thisItemManager.swordIcon;
38.         }
39.     }

And yes, I've assigned sprites to them in the inspector.

This is veering off subject, but I think it's become more of an all-around "Inventory System" post...

 

My guess would be that your constructor is running before the Unity engine has finished initializing all your game objects.

If this class is a MonoBehavior, try moving the initialization to Start() instead of the constructor.

If not, I'd do a "lazy initialization" where you don't actually compute the value of itemIcon until someone asks for it. I'm guessing it's currently a public field? Try making a public property instead (backed by a private field), and write the get method to initialize the private field if it's currently null.

Alternately...do you really need to store itemIcon as an explicit field? Maybe you could just call SetIcon(weaponType) whenever you need to know the icon. If that seems too verbose to you, you could even create a get-only property called "itemIcon" that just says "return SetIcon(weaponType);"

 

Done! It's all working perfectly! Thanks!

 

I still don't get why you use a Dictionary<BaseItem,int>. It just doesn't make sense if two items of the same type are exactly the same at all times. You should use a Dictionary<ItemType,int>.

 

To make your inventory even more flexible, instead BaseItem class you could create a InventoryItemBehaviour that you can give to any gameObject you want. That way you could do some funny things, like picking up enemies into your inventory and later releasing them; any game object as long it has the component script.

 

BaseItem is the root class for all objects that can be stored in the inventory. I then have classes that inherit from it, such as BaseWeapon, BaseArmor, BaseAccessory, etc. The individual items are then instances of those classes.

So, my inventory is set to store the broadest and most basic type of item (BaseItem).

 

Sure, but I think too many subclassing is a bad design for games, especially when Unity does a great job with its component based system.

Let's say your boss comes and says, this Armor of yours needs to have an attack on use. I guess taking your BaseWeapon code and putting it all the way back into BaseItem is a bad idea. Rather, use components so you maintain flexibility what your items can do. You could create weapons that have no attack, or accessories that have armour functionality. Sure maybe it's not needed right now, but by creating small classes with various functionality can help you create interesting items.

EDIT: Fixed typo.

 

I don't think you got my point.

I'm assuming that all your items of the same type are equal. For example, a Hammer is exactly equal to another Hammer. No two instances of Hammer are different, aside from being separate objects.

Using that assumption, it does not make any sense to store your inventory in a Dictionary<BaseItem,int> when you should really use something like Dictionary<ItemType,int>. To use BaseItem would be the same as saying, "of this exact Hammer object, I have three." This does not make sense. Instead, you would say, "of Hammers - which is an item type - I have three."

 

I'm not sure we're understanding each other. Or, if we are, I'm failing to see the difference between what I've done and what you're explaining... except that you call it ItemType and I call it BaseItem.

 

I get your point, but I don't have a boss, and I know how the game will function, so this approach is easier to manage. My armor will never attack anyone.

EDIT: Your approach would mean creating all of my items and attaching components to them individually. That's a headache that just isn't worth it in this case.

 

I was under the impression that an instance of a subclass of BaseItem is an actual object? Such as, if you had a Hammer class that is a subclass of BaseItem, a Hammer object would be an actual object?

 

Each item is an instance that is created once at runtime. Here is my hunting rifle:

Code (CSharp):

1. public BaseWeapon huntingRifle = new BaseWeapon("Hunting Rifle", "Shoot, shoot shoot. Bullet, bullet, gun.", BaseWeapon.WeaponTypes.RIFLE, 5, 10);

Then, when I want to give the player a hunting rifle, I just say:

Code (CSharp):

1. AddItemToInventory(huntingRifle, 1);

which first looks for the hunting rifle in the inventory. If it finds it, it increments its value. If not, it adds it as a new entry.

EDIT: The only time these appear as objects is in the inventory screen, when I instantiate a single button for each BaseItem in the list.

 

Ah, I see. That makes BaseWeapon and BaseItem an item type rather than an actual item, of course.

 

So, I take it you were asking for suggestions in the OP. My recommendation would be to rename BaseItem to something like BaseItemType to better represent what it actually is. (see my confusion for reference )

 

I suppose, but then I also have an enum on it called ItemType, which determines whether it's a weapon, armor, consumable, etc.
EDIT: ...which, in hindsight, may be redundant, since, if it's a weapon, I can just check to see if it's a BaseWeapon... ah well

 

Exactly

 

Yeah, it can be a headache to manually create prefabs for each individual item, but then you can create a factory class that parses your spreadsheet database and creates gameObjects and attaches all components you assigned in db together. In the end, it all comes down to what approach someone likes the most

 

Taxonomies can get complicated. I would tend to call a weapon/armor/consumable division a "category" rather than a "type", but I don't think there are any widespread standards about that sort of thing, so it's mostly a question of choosing a scheme that you (and anyone else on your dev team) can remember.

 

Yes, "category" works, too. I was just trying to point out that naming it "...Item" is quite misleading.