Need help duplicating items for my inventory system. Backpacks are treated as normal items.

I have the item database code here:

using System.Collections;
using System.Collections.Generic;
using UnityEngine;

public class ItemDatabase : MonoBehaviour
{
    public List<Item> gameItems = new List<Item>();
    public List<ItemWithInventory> gameStorageItems = new List<ItemWithInventory>();

    public Item cloneItem (Item toClone)
    {
        return JsonUtility.FromJson<Item>(JsonUtility.ToJson(toClone));
    }
}

Regular items go:

using System.Collections;
using System.Collections.Generic;
using UnityEngine;

[System.Serializable]
public class Item
{
    public string itemName, itemDescription, itemID;
    public bool stackable;
    public int stackSize, maxStackSize;
}

and items with storage (such as backpacks) go:

using System.Collections;
using System.Collections.Generic;
using UnityEngine;

[System.Serializable]
public class ItemWithInventory : Item
{
    public List<Item> storedItems = new List<Item>();
}

Here is the player’s code:

using System.Collections;
using System.Collections.Generic;
using UnityEngine;
using UnityEngine.UI;

[System.Serializable]
public class PlayerEntity : MonoBehaviour
{

    public ItemDatabase itemDatabase;

    [System.Serializable]
    public class PlayerData
    {
        public List<Item> handsInventory = new List<Item>();
    }
    public PlayerData playerData;

    private void Start()
    {
        addTestItem();
    }

    private void addTestItem ()
    {
        Item testItem = itemDatabase.cloneItem(itemDatabase.gameStorageItems[0]);
        playerData.handsInventory[0] = testItem;
    }

    public void sendToInventory (Item toSend)
    {

    }
}

The item in gameStorageItems at point [0] is a backpack. However when duplicating the item when addTestItem() is called, it duplicates it as if it were a regular item and thus it has no inventory. How can I modify the code to fix this so that both regular items and backpacks can be duplicated in the same function cloneItem() in ItemDatabase?

Why do you write an item to JSON and get it from there again in order to clone it?
Why is cloneItem() part of the ItemDatabase class, not Item itself? Imho Items should know hot to clone themselves. So Item should have the cloneItem method, which you implement for Items, and then override for ItemWithInventory. You will then want to make a manual deep copy of the item and return it.
I would also consider using constructors and properties and not having all variables public, but this is just some off-topic architectural advice.

Edit:
Why would you want to work with a clone anyways? At runtime you should just return the item reference. Unless you really want to work with a second identical object. If it’s just about spawning an item from the inventory or something like that, you should rather Instantiate() the stored prefab.

I use json because if I just do:

playerData.handsInventory[0] = itemDatabase.gameStorageItems[0];

It would result in the inventory item and database item being the same instance, and when the inventory item is modified, it will also modify the original from the database, which I don’t want.

Instantiate? Items are not prefabs in this case. They are just data for items.

Also I need to know how to copy the item as a backpack and not a normal item in my code. Do you know any way to do this?

As i wrote, you will probably want to write your own functions to return a deep copy. And i would probably also make use of constructors and properties for a more robust architecture. So for example, Item:

public class Item
{
    // Since we made them private, we would use public properties to access them
    // This has the added benefit that you can forbid certain actions, like setting a different ID, but allow getting it
    private string itemName, itemDescription, itemID;
    private int stackSize, maxStackSize;

    // Constructor for constructing a new instance
    // I'm using default values for stackSize = 1. If you dont give it an optional different value, it will be 'non stackable'
    // We do not need an extra bool for this. Btw what's the difference between stackSize and maxStackSize? Can it increase?
    public Item(int itemId, string name, string description, int stackSize = 1, int maxStackSize = 1){
        itemID = itemId;
        itemName = name;
        itemDescription = description;
        this.stackSize = stackSize;
        this.maxStackSize = maxStackSize;
    }

    // Replacing your bool, we can simply check the stackSize to figure if it's stackable
    public bool IsStackable(){
        return stackSize > 1; // if its stackSize is over 1, it is stackable
    }
}

The above are just suggestions to improve your architecture. It’s bad design to have all public variables. A constructor also saves you some effort later on. We can now create instances of Item by writing Item(someID, someName, someDescription) with optional values for stackSize, which determines if the item is considered stackable.

To make a deep copy of the item we can now simply add the function:

public Item GetDeepCopy(){
    // Create an item with the same values as this item and return it
    Item copy = new Item(itemID, itemName, itemDescription, stackSize, maxStackSize);
    return copy;
}

Since ItemWithInventory is also an item, we need to override but reuse the GetDeepCopy functionality. We also need to fill the new list with deep copies of all the items in the inventory. We can do that like so:

public override Item GetDeepCopy(){
    item baseCopy = base.GetDeepCopy(); // this copies all the item data, but not the list
    List<Item> inventoryDeepCopy = new List<Item>();
    for(int i = 0; i < storedItems.Count; i++){
        inventoryDeepCopy.Add(storedItems[i].GetDeepCopy());
    }

    // Now set the new inventory for the deep copy. 
    // The inventory of ItemWithInventory would be a settable public property
    baseCopy.StoredItems = inventoryDeepCopy;

    // Now we return the full deep copy of the item with inventory
    return baseCopy;  
}

This works rekursively, so if there is another ItemWithInventory in your ItemWithInventory, it will also make a deep copy of that and all its items and so on, since we always call the overriden GetDeepCopy method.

Since i typed this here in the forum and not in an editor, there may be typos somewhere but you should get the idea.
Hope this helps :slight_smile:

Is there a way to make item and itemwithinventory share the same list in playerdata.handsinventory?

When I place an itemwithinventory in a List it does not use the backpack’s inventory in the inspector.

I’m not entirely sure what it is you are asking. Item and ItemWithInventory both belong to the type Item, so as you said yourself you can put an IntemWithInventory in a List. Im not quite sure what the expected behaviour is when you say:

Do you expect the backpack inventory to increase your list size in the inspector? Or do you expect the backpack to be expandable to see its items in the inspector? If it’s the latter, i’m not super familiar with how the inspector works, but i believe if it’s a type that has a list, you can expand it. However, from the perspective of the inspector, your backpack would be an Item, not an ItemWithInventory, since it’s in a List, so of course you cannot expand it, since a normal Item does not have a list to look at.
Inside of scripts there are ways to check for specialized objects like ItemWithInventory in a List of generalized objects like Item, such that you can convert them if found and check their specific contents. However, i’m not sure if there is an easy way to achieve this for the inspector. Since you can write fully customized editors, it should be possible, but i’m not sure if it’s worth the effort just to see the backpack inventory inside of the inspector. That’s for you to decide.

Yes I mean you can expand the backpack item to see its contents in the inspector. Thanks for the help, however I am working on a similar script for weapons and attachments. I might have more questions later.

Running into an error:

Specified cast is not valid

It points to this code:

    private void addTestItem ()
    {
        //Item testItem = itemDatabase.cloneItem(itemDatabase.gameStorageItems[0]);
        Item testItem = itemDatabase.gameStorageItems[0].cloneItem();
        playerData.handsInventory[0] = testItem;

        Debug.Log(JsonUtility.ToJson(((ItemWithInventory)testItem).storedItems).ToString());
    }

How to fix?

Edit:

I did some testing by adding logs to the backpack in the item database. I then tried:

    private void addTestItem ()
    {
        //Item testItem = itemDatabase.cloneItem(itemDatabase.gameStorageItems[0]);
        //Item testItem = itemDatabase.gameStorageItems[0].cloneItem();
        Item testItem = itemDatabase.gameStorageItems[0].cloneItem();
        playerData.handsInventory[0] = testItem;

        Debug.Log(JsonUtility.ToJson(((ItemWithInventory)playerData.handsInventory[0]).storedItems));
    }

I did this to see if the inventory was actually copied with the backpack, but the debug simply returned: “{}”

The inventory was not coppied over. How do I fix?

Also I cannot set the values to private because I need to be able to edit them in the inspector.

That’s what [SerializeField] exists for.

When you run into an error, always post exactly what line throws exactly what error (and which line in the example the line in the error refers to). Since that’s the only location you are casting, i’m assuming your Debug.Log() line throws the error. Never really used JsonUtility. Cant tell you why the cast wouldnt be allowed at that location either (but i may with the actual error).

To test if the item was cloned you should rather check the actual item, not a debug based on some return from JsonUtility.

ItemWithInventory testItem = (ItemWithInventory)itemDatabase.gameStorageItems[0].cloneItem(); // assuming you make sure [0] is an ItemWithInventory
Debug.Log("Copy Item Count: " + testItem.storedItems.Count);

If this returns 0, then the copy is indeed empty. The code i posted above should work tho, unless i had a slip somewhere.

Ok so I tried:

using System.Collections;
using System.Collections.Generic;
using UnityEngine;

[System.Serializable]
public class ItemWithInventory : Item
{
    public List<Item> storedItems = new List<Item>();

    public override Item getDeepCopy ()
    {
        Item baseCopy = base.getDeepCopy();
        List<Item> inventoryDeepCopy = new List<Item>();
        for (int i = 0; i < storedItems.Count; i++)
        {
            inventoryDeepCopy.Add(storedItems[i].getDeepCopy());
        }
        ((ItemWithInventory)baseCopy).storedItems = inventoryDeepCopy;
        return baseCopy;
    }
}

but I get error:

How to fix?

EDIT:

Also, I cannot replace the stackable bool, as what if the item is supposed to be stackable (such as a pile of logs), but you only have 1 log in your inventory?

I guess you took my advice for giving Item a constructor? If so, ItemWithInventory also needs one, where you pass all these values down to Item. Currently the compiler complains since it tries to create an item when you create an ItemWithInventory, but it does not get the required parameters.
Consider reading up on constructors, and possibly inheritance as a whole since this seems to give you trouble.

Imagine we have a base class:

public class Person{
    private string name;
    public Person(string name){
        this.name = name;
    }
}

So in order to create a person, we must give this person a name. We cannot create a person without a name.
Now we create a specialized subclass of person, let’s say Programmer. A Programmer is also a Person, who also has… an age. I’m not very creative ok? :smile:
So how do we create a Programmer? It is a Person, so it also requires a name to be created. The rest does not matter as far as the compiler is concerned, but it needs this name. So what we need to do is create a constructor and pass the required parameters down to our base class. We can do this like so:

public class Programmer : Person{
    private int age;
    public Programmer(string name, int age) : base(name){
        this.age = age;
    }
}

So now we provide a name to our Person constructor and the compiler is happy. Afterall, the system basically exists to make sure that no Person without a name can exist, so it makes sense that we cannot create a Programmer, which is a Person, without a name. The same applies to your items. There are no items without, say, an ID (or at least there shouldnt). So with the constructor we make sure that all items must have an ID, because otherwise we cannot create them. However, this also means that all ItemWithInventorys’ need to have an ID and so on.

So TL: DR; do what i did in the Person example to pass the parameters of the ItemWithInventory constructor down to your base class Item and this particular error should disappear. However, i would strongly recommend you to look up some basic tutorials on inheritance, casting, object oriented programming and other basics, since you still seem to have some troubles with these, which also slows down how fast you will arrive at your goals.

Anyways, hope this clears up some of your confusion :slight_smile:

I am getting another error:

When I do:

using System.Collections;
using System.Collections.Generic;
using UnityEngine;

[System.Serializable]
public class ItemWithInventory : Item
{
    public List<Item> storedItems = new List<Item>();

    public ItemWithInventory (string _itemName, string _itemDescription, string _itemID, bool _stackable, int _stackSize, int _maxStackSize, List<Item> _storedItems) : base(_itemName, _itemDescription, _itemID, _stackable, _stackSize, _maxStackSize)
    {
        this.storedItems = _storedItems;
    }

    public override Item getDeepCopy ()
    {
        Item baseCopy = base.getDeepCopy();
        List<Item> inventoryDeepCopy = new List<Item>();
        for (int i = 0; i < storedItems.Count; i++)
        {
            inventoryDeepCopy.Add(storedItems[i].getDeepCopy());
        }
        ((ItemWithInventory)baseCopy).storedItems = inventoryDeepCopy;
        return baseCopy;
    }
}

Line 23 is giving me:

What is going on now?

Oh right, i overlooked that. We can not cast Item to ItemWithInventory there. Similar to how we cannot cast a Person to be a Programmer, unless the Person was already a Programmer (which was just saved as a Person) to begin with. Or at least i believe that’s the explanation, since i last properly worked with inheritance structures in Java haha.

Instead, let’s get the inventoryDeepCopy first, then create a new ItemWithInventory, where we use our own item values and this new inventoryDeepCopy as parameters for the constructor. So it should look something like this:

public override Item getDeepCopy ()
    {
        List<Item> inventoryDeepCopy = new List<Item>();
        for (int i = 0; i < storedItems.Count; i++)
        {
            inventoryDeepCopy.Add(storedItems[i].getDeepCopy());
        }
        ItemWithInventory copy = new ItemWithInventory(itemName, itemDescription, itemID, stackable, stackSize, maxStackSize, inventoryDeepCopy);
        return copy;
    }

Now we actually create an ItemWithInventory, which thus can also store the inventory. We return it as an Item, because that’s what the base class method is supposed to do, but that’s fine.

Btw, your code is a bit irritating to read sometimes, since you are not following most common C# naming conventions. Method, class and property names should start with a capital letter. Variable names use the standard camelCase. Some people like to indicate member variables with an _underScore, but i have never seen anybody use the underscore for parameters. If this is your first programming language, please consider looking up naming conventions and doing it ‘right’ from the beginning, before you learn some bad habits which make your code harder to read. If you come from another programming language to C# i guess it’s your decision if you wanna unlearn old naming conventions or not.

Hope this helps :slight_smile:

I mostly use what they call camel case. Where they start with a lowercase, then (with no spaces) type a capital letter for the first letter of each following first word. EX:

itemWithInventory

I am sure this is one of the standard naming conventions.

Edit, it is still returning a backpack with empty inventory:

    private void addTestItem ()
    {
        //Item testItem = itemDatabase.cloneItem(itemDatabase.gameStorageItems[0]);
        //Item testItem = itemDatabase.gameStorageItems[0].cloneItem();
        //Item testItem = itemDatabase.gameStorageItems[0].cloneItem();
        //playerData.handsInventory[0] = testItem;

        //Debug.Log(JsonUtility.ToJson(((ItemWithInventory)playerData.handsInventory[0]).storedItems));
        Item testItem = itemDatabase.gameStorageItems[0].getDeepCopy();
        playerData.handsInventory[0] = testItem;
        Debug.Log(JsonUtility.ToJson(((ItemWithInventory)playerData.handsInventory[0]).storedItems));
    }

Edit Again:

I think I might be more able to accomplish what I want if I find a tutorial on how to make a DAYZ style inventory system but my search has returned nothing relevant. Know any tutorials?

Yes it is. You would use camelCase for normal things like variables, then use UpperCamelCase for class, method and property names. You are, however, writing method names also in camelCase, and use underscore for parameters (which i personally never saw anybody do). If i ever see people use underscores (in C#, it’s rather popular in C++ i believe), they mostly use it to indicate member variables (example a private variable which has a public property with the same name, so they can be easily differentiated), constants, or in shader languages.
Just thought i’d mention this, before you learn bad habits. Do with this information what you want^^

Did you test the inventory WITHOUT your Json line? I already mentioned this above, but i have no idea how JsonUtility works, and saying X does not work, because GetFromJson(Json(X)) does not work is not exactly a scientific approach. Create an ItemWithInventory test = someItem.GetCopy(), where someItem is of type ItemWithInventory. Then see if test.storedItems.Length is 0. I can not imagine why it would be, because we are literally putting a copy of all the inventory items into a new list and then set this list to be our inventory. Maybe you are calling Item.GetCopy instead of ItemWithInventory.GetCopy. Did you use the virtual keyword for Item.GetCopy()?

Never played DayZ, but may be able to help with a different approach if you explained the type of inventory you want. I found it a bit weird that you seemingly ‘need to’ make DeepCopies of Items, since normally you would want to pass a reference from inventory to inventory. Maybe there is a deeper problem with your concept / approach.

Edit: This didnt let go of me.
So i took the time to create a working example. It’s basically exactly what i posted you minus some typos. I dont know where your error was (or if Json is the problem), but i guess now you have something that works to compare to:
Script

using System.Collections;
using System.Collections.Generic;
using UnityEngine;

public class ItemExample : MonoBehaviour
{
    void Start()
    {
        // Creating some example items
        Item a = new Item(0, "a", "someA");
        Item b = new Item(1, "b", "someB");
        Item c = new Item(2, "c", "someC");
        Item d = new Item(3, "d", "someD");

        // Creating an example backpack and putting items
        ItemWithInventory backpack = new ItemWithInventory(4, "Backpack", "Packs Items");
        backpack.Inventory.Add(a);
        backpack.Inventory.Add(b);

        // Checking that the backpack contains items
        Debug.Log("First item of backpack: " + backpack.Inventory[0].ToString()); // prints a

        // Getting a deep copy of backpack
        Item backpackCopy = backpack.GetDeepCopy();

        // Checking that copy of backpack contains items
        Debug.Log("First item of backpackCopy: " + ((ItemWithInventory)backpackCopy).Inventory[0].ToString()); // prints a
    }
}

public class Item
{
    // Since we made them private, we would use public properties to access them
    // This has the added benefit that you can forbid certain actions, like setting a different ID, but allow getting it
    protected int itemID;
    protected string itemName, itemDescription;
    protected int stackSize, maxStackSize;

    // Constructor for constructing a new instance
    // I'm using default values for stackSize = 1. If you dont give it an optional different value, it will be 'non stackable'
    // We do not need an extra bool for this. Btw what's the difference between stackSize and maxStackSize? Can it increase?
    public Item(int itemId, string name, string description, int stackSize = 1, int maxStackSize = 1)
    {
        itemID = itemId;
        itemName = name;
        itemDescription = description;
        this.stackSize = stackSize;
        this.maxStackSize = maxStackSize;
    }

    public override string ToString()
    {
        return itemName;
    }

    public virtual Item GetDeepCopy()
    {
        // Create an item with the same values as this item and return it
        Item copy = new Item(itemID, itemName, itemDescription, stackSize, maxStackSize);
        return copy;
    }

    // Replacing your bool, we can simply check the stackSize to figure if it's stackable
    public bool IsStackable()
    {
        return stackSize > 1; // if its stackSize is over 1, it is stackable
    }
}

public class ItemWithInventory : Item
{
    private List<Item> inventory = null;
    public List<Item> Inventory
    {
        get // currently only alloweing getting inventory
        {
            return inventory;
        }
    }

    public ItemWithInventory(int itemId, string name, string description, List<Item> inventory = null, int stackSize = 1, int maxStackSize = 1) : base(itemId, name, description, stackSize, maxStackSize)
    {
        if (inventory != null)
        {
            this.inventory = inventory;
        }
        else
        {
            this.inventory = new List<Item>();
        }
    }

    public override Item GetDeepCopy()
    {
        List<Item> inventoryDeepCopy = new List<Item>();
        for (int i = 0; i < Inventory.Count; i++)
        {
            inventoryDeepCopy.Add(Inventory[i].GetDeepCopy());
        }
        ItemWithInventory copy = new ItemWithInventory(itemID, itemName, itemDescription, inventoryDeepCopy, stackSize, maxStackSize);
        return copy;
    }
}

It also works if we saved the first backpack directly as an Item instead of ItemWithInventory (tested this too). I posted the example with ItemWithInventory, since putting items in an Item always required a cast, so it’s a bit unconvenient to write and less readable to read.