Understanding the problem with my custom class for the Item system and game inventory

Question

Sorry, but I can not ask a question without any introduction. (If you don’t know whether to read all this, I’m still trying to ask a question, the problem is that I’m trying to change the properties of an element, does this apply to all the other same elements, how can I fix this?)

I have an Item class with some properties and variables like itemCost , durability , etc.

The Weapon class inherits Item .

I have an ItemGenerator class that initializes all items in a single array. It contains functions such as:

 private static Weapon CreateUniqueWeaponForItemList(int ID, int itemLevel, ..... string description) { Weapon item = new Weapon(); item.ID = ID; item.ItemLevel = itemLevel; ... ... item.Description = description return item; } 

something like that. This list of items is initialized at the beginning of the game. I have an array of elements containing all the elements.

 public static Item[] ItemList = new Item[200]; 

Listed below are all the unique items in the game created by the function above:

 ItemList[1] = CreateUniqueWeaponForItemList(1, 5, ....., "this is item!"); ItemList[2] = .... 

etc. At the moment this works great. When I create an item, I simply use the Item ID to indicate which item I want to create. It is also easy to save and load, just save the ID element in PlayerPrefs .

But when I start adding additional functionality (for example, updating items, updating damage, and something), I realized that this architecture is bad.

If a player has two or more of the same elements, problems begin here. If I try to change the Item properties, they apply to the ItemList[ID] , and not to the item I want.

I think they need to paste the code here to be clear. I have an inventory system, with private List<Item> _inventory = new List<Item>(); in the Player class. I have a treasure chest that gets an item from an ItemList to create

 loot.Add(ItemGenerator.CreateUniqueItem(2)); 

loot is the Item variable in the Chest class. Below are explanations of CreateUniqueItem

  public static Item CreateUniqueItem(int ID) { if (ID > ItemList.Length) { return null; } if (ItemList[ID] != null) { return ItemList[ID]; } else { return ItemList[0]; } } 

When an item is created, the player can capture it in inventory. I am just _inventory.add(item); , for example Item is equal to ItemList[2]

  Player.Insntance.Inventory.Add(chest.loot[2]); chest.loot.RemoveAt(2); 

breast .loot

 public List<Item> loot = new List<Item>(); 

which contain all the elements to capture.

I think the problem is here.

-----------------------------------------

So, here is the question itself. If I want to make an update item, I use

_inventory[0].MaxDamage++

but MaxDamage increases on all other elements of the Player’s inventory, I can’t understand why? I do not use ItemList[ID].MaxDamage++

I think that I should save all created unique elements in a file or something like that, and refer to them from the inventory, and not give a link to the inventory.

Or save in the file only the element identifier and add the int variable to the elements, like upgradeLevel , and save it. So due to upgradeLevel items can get buffs for damage.

But right? And what is the best way to do this?

-----------------------------------------

Here is a short insertion of the Item class:

 public class Item { private int _ID private int _itemLevel; ... ... public Item(){ _ID = 0; _itemLevel = 0; ... ... } public ID{ get { return _ID; } set { _ID = value; } public int ItemLevel { get { return _itemLevel; } set { _itemLevel = value; } } ... ... ... } 

The weapon class is the same, but there are additional variables, such as damage.

 public class Weapon : Item { private int _maxDamage; public Weapon() { _maxDamage = 0; } public int MaxDamage { get { return _maxDamage; } set { _maxDamage = value; } } } 

I can provide a complete list of code on GitHub if necessary. But I think the code I have inserted is higher than enough. I hope I don’t forget anything.

I will not be surprised if the problem is trivial, but for some reason I cannot understand it and it causes a headache.

I apologize if there is too much text, but I could not shorten it.

Also sorry for my poor English spelling.

Thanks in advance.

Answer 1

You store links in your array, and therefore there is only one element. There are several ways to solve this. In fact, your “list of items” is almost like a list of templates. A bit like Plato's theory of perfect form. Whenever you bring an item to the game in the game (whether in the chest or player’s inventory), you want to clone the item.

Think of your array of sword objects as a “sword concept,” and whenever there is one in the chest, we “clone” this template. The chest now contains a duplicate of the template. Obviously, when a player takes a sword, we simply transfer it from one container to another (we do not leave the sword in the chest, it is transferred to the player’s inventory).

So how can we do this? Well, we can use cloning. Therefore, the base class of the element may look like this:

 // Not tying ourselves just to weapons here... what about food, clothes, etc..? public abstract class Item { public int ID { get; set; } public string Name { get; set; } // Let have a copy constructor public Item(Item other) { ID = other.ID; Name = other.Name; } // This part is important! public abstract Item Clone(); } 

Good. We have an item. And it has some basic properties. Let them make weapons.

 public class Weapon : Item { public int Damage { get; set; } // also has a copy constructor public Weapon(Weapon other) : base(other) // Item copies it stuff! { Damage = other.Damage; } // Need to implement this: public override Item Clone() { return new Weapon(this); } } 

Now you can get a whole bunch of other things (food, clothes, dirty magazines, works on Plato, etc.).

Now you can have an array of these Item instances, and whenever you want to put it in a chest in a game, you just go:

 Item newItem = itemList[index].Clone(); 

This effectively creates a new instance of any item. Food will be cloned properly. So, if a player has a cloned sword, now you can clone it and increase its damage - because it is a different sword (but based on the original sword of Plato!).

This is not the only way to solve this problem, and inheritance trees can become quite messy when elements have several different properties, and there are potentially hundreds of small variations. I support component design in these cases, but this is a bit beyond the scope of this answer.

Answer 2

You may find that this problem becomes easier if you retrieve items from ScriptableObject .

A ScriptableObject (e.g. GameObject) must be created explicitly, so you can understand it when dealing with an instance and when dealing with a ScriptableObject resource. Another nice thing is that you can create your own template assets ("sword + 1", "invisiblity cloak", etc.) and edit them manually (setting individual element properties, etc.) and check them. In addition, all the boring things have been created for you, such as managing instance IDs and manual serialization.

Answer 3

Consider using only one element class. Switch the element type using enumerations, in my opinion, this is more flexible. I use something like the following code because using inheritance is a messy thing ... sooo ^^

ehm ... this example is for unity users;)

 using UnityEngine; using System.Collections.Generic; using System.Collections; public class Player : MonoBehaviour { public List<Item> Items = new List<Item>(); void Start() { Test(); AddItem( new ItemGenerator().GetRandomItem()); AddItem( new ItemGenerator(ItemType.Weapon, "New Cool Weapon Item", UnityEngine.Random.Range(1,100), UnityEngine.Random.Range(1,100))); Item item = new Item(); item.ItemType = ItemType.Quest; AddItem(item); AddItem(new Item()); AddItem( new ItemGenerator().GetRandomItem("Random Test Item 1 2 3")); AddItem( item.Clone()); } public void AddItem(Item item) { Items.Add(item); } void Test() { Debug.Log("Test starts"); // example flexibility Item item = new Item(); item.ItemType = Itemtype.Weapon; if(item.ItemType == Itemtype.Weapon) Debug.Log(item.WeaponProperties.GetDps(item)); item.ItemType = Itemtype.Armor; if(item.ItemType == Itemtype.Armor) Debug.Log(item.ArmorProperties.Defense); switch(item.ItemType) { case ItemType.Weapon: Debug.Log("Do Weapon Stuff - instantiate at weapon spawnpoint"); break; case ItemType.Armor: Debug.Log("Do Armor Stuff - instantiate at spawnpoint"); break; default: Debug.Log("what ever..."); break; } // example item generator Item item2 = new ItemGenerator(ItemType.Weapon); Debug.Log(item2.Name); Item item3 = new ItemGenerator(ItemType.Armor, "New Armor Item"); Debug.Log(item3.Name); item3.ItemType = ItemType.Weapon; item3.Name = "Ultra Sword"; Debug.Log(item3.Name); Item item4 = new ItemGenerator(ItemType.Weapon, "New Weapon Item", UnityEngine.Random.Range(1,100), UnityEngine.Random.Range(1,100)); Debug.Log(item3.Name); Item item5 = new ItemGenerator().GetRandomItem(); Debug.Log(item2.Name); Debug.Log(item2.ItemType); // clone item Item item6 = item5.Clone(); Debug.Log(item2.Name); Debug.Log("Test ends"); } } public enum ItemType { Weapon, Armor, Consumable, Quest, Key //... } public class Item // :ScriptableObject { public string Name = "FooItem"; public ItemType Itemtype; public Attributes Attributes = new Attributes(); public WeaponProperties WeaponProperties = new WeaponProperties(); public ArmorProperties ArmorProperties = new ArmorProperties(); public Item Clone() { return (Item)MemberwiseClone(); } } [System.Serializable] public class WeaponProperties { public int MinimalDamage = 10; public int MaximalDamage= 20; public float Speed = 1.5f; public static float GetDps(Item weapon) { return Mathf.RoundToInt(((weapon.WeaponProperties.MinimalDamage + weapon.WeaponProperties.MaximalDamage) * 0.5f) / weapon.WeaponProperties.Speed); } } [System.Serializable] public class ArmorProperties { public int Defense = 25; } [System.Serializable] public class Attributes { public int Strength = 25; public int Stamina = 20; } public class ItemGenerator : Item { public ItemGenerator(ItemType itemType) : this(itemType, "NewNamelessItem") { } public ItemGenerator(ItemType itemType, string name)this(itemType, name, 0, 0) { } public ItemGenerator(ItemType itemType, string name, int strength, int stamina) : this(itemType, name, strength, stamina, 0) { } public ItemGenerator(ItemType itemType, string name, int strength, int stamina, int defense) { Name = name; Attributes.Strength = strength; Attributes.Stamina = stamina; switch(item.ItemType) { case ItemType.Weapon: break; case ItemType.Armor: ArmorProperties.Defense = defense; break; default: Debug.Log("what ever..."); break; } } public Item GetRandomItem() { return GetRandomItem( "New Random Item"); } public Item GetRandomItem(string name) { Name = name; Attributes.Strength = Random.Range(0,100); Attributes.Stamina = Random.Range(0,100); var values = Enum.GetValues(typeof(ItemType)); ItemType = (ItemType)values.GetValue(Random.Range(0, values.Length)); switch(item.ItemType) { case ItemType.Weapon: break; case ItemType.Armor: ArmorProperties.Defense = Random.Range(0,100); break; default: Debug.Log("what ever..."); break; } return this; } } 

Finally, this means that each element can be changed to a completely different element, and you have complete control over all the variables.