A bug that I couldn't find

I have been trying to create an inventory system but having a problem. I have a list called 'items' and here is the code of the function that manages items that are to be added to inventory.
The error i am encountering with this code is that whenever i try to add a non existing item to inventory, it works perfectly, but whenever i try to add the same item in inventory one more time, independent of the amount i write, it increments stackSize by 1 and never adds any more. I have tried it with arrays too but it's still not working, there were also some other weird stuff happening wtih the project but i would like to get this one resolved first. Thanks!

public void AddToInventory(Item _item, int amount = 1)
{

   foreach(Item item in items)
    {
        if(item.Name == _item.Name)
        {
            item.stackSize += amount;
            FlushList();
            return;
        }
    }

    Item item2 = new Item();
    item2 = _item;
    item2.stackSize = amount;

    items.Add(item2);
}

This is how I test it:

private void Awake()
{
    items = new List<Item>();
    itemsInstantiated = new List<GameObject>();

    AddToInventory(itemDb.GetItem("Material"));
    AddToInventory(itemDb.GetItem("Material"));
    AddToInventory(itemDb.GetItem("Material"));
    AddToInventory(itemDb.GetItem("Material"));
    // Stacksize of materials after this code is 2.
    // Doesn't matter how many times i call the function above.

    AddToInventory(itemDb.GetItem("Water Bottle(Full)"), 21);
    AddToInventory(itemDb.GetItem("Apple"));
    FlushList();
}

FlushList() method:

 public void FlushList()
{
    // Visual stuff.
    // There is a more efficent way but it's just a project to pass time so 
    // didn't bother.
    foreach(GameObject go in itemsInstantiated)
    {
        Destroy(go);
    }

    foreach(Item item in items)
    {
        GameObject inst = Instantiate(ItemPrefab, ListView.transform);
        itemsInstantiated.Add(inst);

        inst.transform.GetChild(0).GetComponent<TextMeshProUGUI>().text = item.Name;
        inst.transform.GetChild(1).GetComponent<TextMeshProUGUI>().text = item.Description;
        inst.transform.GetChild(2).GetComponent<TextMeshProUGUI>().text = item.stackSize.ToString();

        if(item.isConsumeable)
        {
            inst.transform.GetChild(3).gameObject.SetActive(true);
            inst.transform.GetChild(3).GetComponent<Button>().onClick.AddListener(() => { ConsumeItem(item); });
        }
    }
}

ConsumeItem() method:

public void ConsumeItem(Item item)
{
    item.consumeAction?.Invoke();

    PlayerNeeds pn = GetComponent<PlayerNeeds>();
    // Increase methods don't touch the list.
    pn.IncreaseHunger(item.hungerIncrease);
    pn.IncreaseThirst(item.thirstIncrease);
    pn.IncreaseSleepiness(item.thirstIncrease);

    RemoveItem(item);
}

Item Database : Item database is where i store all items. Items there only have names and descriptions.

public Item GetItem(string itemName, int amount = 1)
{
    // Amount here is used by other functions such as crafting recipes.
    // I feel like this is where the problem is.
    foreach(Item item in AllItems)
    {
        if(item.Name == itemName)
        {
            Item newItem = item;
            newItem.stackSize = amount;
            return newItem;
        }
    }
    print("Item: " + itemName + " could not be found!");
    return null;
}

Item class:

public class Item
{
  public string Name;
  public string Description;
  public int categoryIndex = 1;
  public bool isConsumeable;

  public int stackSize = 1;

  public int hungerIncrease;
  public int thirstIncrease;
  public int sleepIncrease;
  public Action consumeAction;
}

_item is an item from item database and it's stacksize is always 1, i am asking for 'Item' input because if that item doesn't exist in the inventory of player, i will add that item instead.

Some examples: * AddToInventory(item A, 3)
Result: 3 item A in my inventory(works perfectly).

*AddToInventory(item A, 3)
AddToInventory(item A, 3)
Result: 4 item A.

**AddToInventory(item A, 45)
AddToInventory(item A, 57)
Result: 46 item A.

Upvotes: 0

Views: 101

Answers (2)

St. Pat
St. Pat

Reputation: 749

The problem lies in the GetItem method, and is related to working with references.

In the GetItem method, you create a "copy" of the item in the AllItems list, but in fact, you are merely making a new reference to the same object. You then assign a value to stackSize (in your example, it's always the default value of 1.

The sequence of events here is as follows:

  1. You get the item from the AllItems list, and update its stackSize with the value provided to the function (or default 1)
  2. You add this item to the inventory
  3. You get the same item from the AllItems list, and again update the stackSize to 1. The problem occurs here. Since you copy a reference to the same object, the object in the inventory gets updated as well when you change the stack size of the object from whatever it is, back to 1.
  4. You add the Item to your inventory, but the stack size of this one and only object was just reset to 1, so the size is incremented to 2 every time.

This can be resolved by either taking out the line in GetItem that changes stackSize, since it doesn't seem to be used in your example, or you can add a method to the Item class to manually copy each field to a new object.

The copy method would look something like this.

public Item CopyItem()
{
    var newItem = new Item();

    newItem.Name = this.Name;
    newItem.Description = this.Description;
    newItem.categoryIndex = this.categoryIndex;
    newItem.isConsumeable = this.isConsumeable;
    newItem.stackSize = this.stackSize;
    newItem.hungerIncrease = this.hungerIncrease;
    newItem.thirstIncrease = this.thirstIncrease;
    newItem.sleepIncrease = this.sleepIncrease;
    newItem.consumeAction = this.consumeAction;

    return newItem;
}

and would be called like so:

Item newItem = item.CopyItem();

Upvotes: 1

Martin Verjans
Martin Verjans

Reputation: 4796

First of all, by doing item2 = _item, you are erasing the new object you just created one line up. Then, the consequences of doing this is that your items in your list are not just copies of the original items but they are the items. If you modify one in this function (by increment the stacksize), then they will by modified everywhere they are used. Maybe it is what you want, maybe not.

Your function can actually be rewritten like this, no feature added or removed :

public void AddToInventory(Item _item, int amount = 1)
{
    if (!items.Contains(_item))
    {
        items.Add(_item);
        _item.stackSize = amount;
    }
    else
    {
        _item.stackSize += amount;
       FlushList(); //I don't know what this function is supposed to be doing.
    }
}

If that is not your goal, maybe you need to clarify your question.

Maybe if you can show the Item class we would understand your issue better.

Upvotes: 0

Related Questions