Inventory System

First off thanks Ciro for the merge, makes the discussion easier.

Second, I think if we just use an interface and use them as a way to refer to our objects it would be better? (more convenient at least) So if any gameobject implements an InventoryItem interface, it would be pickable, and would have certain specifications like its size and stuff. Thaat way we can avoid having to script multiple Monobehaviours for a single item. I also really want to work on a Deus Ex style inventory where you can arrange it and optimize it in fun ways, so I hope we go for that :slight_smile:

There are definitely valuable uses of interfaces. I would propose that, for now, we stick to MonoBehaviours and ScriptableObjects in order to keep the project simple. While I am certainly a fan of interfaces, we don’t have a standard contract for an item, and I’d hate to add complexity to the project (which would raise the barrier for entry for what should be a relatively beginner-friendly code base) so early into the project for no real gain. I would also like to say that there is nothing that says that our first attempt has to be perfect. There will be plenty of time to iterate on our design if we do want to take advantage of other language features later.

At least, those are my thoughts on the matter.

Thats true, although I would say something like use, drop, pick are common to all of them. So would you suggest something like we start with a basic monobehaviour type system and then refactor it into interfaces as we continue improving the code?

That is what I would recommend. Let’s keep things simple and readable for people who are coming into this as their first project (maybe having watched a few tutorials.) And then we can refactor down the road if we need to.

So to continue the design discussion, I would like to make the following proposal:

public class InventoryItem : ScriptableObject
{
  // Future fields here when scope is expanded
}

For the item scriptable object, since we don’t have any requirements, I suggest we make it a ScriptableObject named InventoryItem with no fields. The reason I suggest no fields is that the name can come from the ScriptableObject itself if we need to refer to the item by name, and there have been no other requirements. Now, if we get a requirement that items need a description, or a long and short name, we can go from there. For now, I say that there is nothing in-scope beyond having just the empty ScriptableObject itself.

I’d suggest putting InventoryItem into the CreateAssetMenu under the sub-menu “Inventory”.

Thoughts? And would someone like to propose the design of the Item Pickup MonoBehaviour?

3 Likes

@tmcdonald I think the only change I’d make to your suggestion would be to put in an enum for for an item type identifier. The card does mention a couple of different item types; healing and ingredients. The type, to me at least, reads as a separate requirement to the item name.

As for runtime pickup, regardless of whether it’s done by collisions or a pickup button I feel like that’s best done by an inventory manager script that would be attached to the player, rather than the non-scriptable object script that will be on the item. As it seems like the intent of the task is that everything to do with the data of an item is stored in the SO, I feel like it makes most sense for the “backpack” to be represented by either a List or Dictionary. The code suggestion below is from my linked post above, though I’ve not changed the variable types to match the other suggestions, and it would be pretty easy to change it to be on a button press.

public class InventoryManager : MonoBehaviour
{
    public List<InventoryItem_SO> backpack = new List<InventoryItem_SO>();
    public Dictionary<InventoryItem_SO, int> backpackDict = new Dictionary<InventoryItem_SO, int>();

    private void OnTriggerEnter(Collider other)
    {
        // If we've entered the trigger collider for a pickup item
        // add it to the backpack, and destroy the item in the world
        // Note: can refactor this to disable if we want to use object pooling
        if (other.tag == "InventoryItem")
        {
            InventoryItem pickupItem;
            if (other.gameObject.TryGetComponent<InventoryItem>(out pickupItem))
            {
                backpack.Add(pickupItem.inventoryItem);
                Destroy(other.gameObject);
            }
        }
    }
}

Well I’m not sure if I 100% get the system, but I can figure it out as I go. I could probably use some help though to get it done faster. I’ll do something for the menu arrangement bit because if we do what I suggested I predict that would take the longest to do :smile:

Wouldn’t the functionality of every item be a little different, making the whole this really difficult in the long run? Like since we’re working with food every ingredient will have a different set of properties. So your enum roster would end up being massive.

@DaSerialGenius not necessarily. The representation of a recipe for the player and how we actually store the data for it don’t have to match. To use a real world example, tomato soup is made from tomatoes, an onion, some garlic, and some stock. But all of those things are ingredients. If every item has an ID number (or UUID), then we can store the recipes separately as just a grouping of ID numbers and quantities.

1 Like

Well, the ScriptableObject for the item is also the type of item. There may be a need in the future to create an enum, but since the list of items reads to me more like suggestions than intentional requirements. Additionally, there are most likely going to be functional difference between “ingredients” and “potions,” which would most likely have to be differentiated in code beyond just an enum. I’d hate to put a requirement on an “item type” enum at the start of the project until we’re ready to actually decide how an ingredient and other items behave differently. You know?

I like the Dictionary you posted. I think that fits - a very simple collection of items with their quantities.

1 Like

Not sure how this is going to pan out.
but there are many ways of doing things.

For this time i thing im going to vote to go with the SO path, seems interesting and more options for people, new people included to learn about SO a lot better.
I think its going to be a lot more easily extensible and easy to work with for this simple project.

we could probably use inheritance to dictate different types of items but i dont know how thats going to work out.

For Later on - for pickups. Walking over things should be for mass items like coins or ammo. tap or hold to pickup i think design wise is more interesting and engaging. We can do both for different things.

For the moment, getting the basic skeletal framework done and ready to extend is probably important so we can see how things are going to expand outwards. A lot of items are dependant on other cards it seems so we cant see too far into the future, we have to work with a framework and intention to extend or change it.

If we like the idea of treating the examples of “ingredients” and “healing items” as requirements rather than as suggestions, I suggest that we create an abstract base class and then a ScriptableObject for Ingredient and another for UsableItem:

public abstract class InventoryItem : ScriptableObject
{
  // Future common fields
}

public class Ingredient : InventoryItem
{
  // Future fields specific to ingredients
}

public class UsableItem : InventoryItem
{
  // Future fields specific to usable items
}

However, I don’t know that it’s necessary.

3 Likes

I feel like we’re slowly coming back to my interface idea

public Interface InventoryItem
{
    int count;
    void Use();
    // Any more as required
}

public class Ingredient : MonoBehaviour, InventoryItem
{
    void Use()
    {
     // implementations in whatever way
    }
}

Although it could be argued that something like count wouldnt really matter if we were doing a single holdable item, but I guess that could easily be added later

EDIT: an abstract class would also allow us to have stuff like rotating items and all that. (not sure I’ll have to test it out)

I’m not necessarily fully in camp “abstract class” but it does help a bit with keeping shared data in one place. I would say that one of the requirements of the ticket is that the items are implemented via ScriptableObjects - we could also keep track of the quantity held using the ScriptableObjects, I’m just not sure how that will interact with the saving/loading system.

A little JSON could help, since they are all implementing the same base class we just need to have a system to find what object it is.

//THIS WOULD BE IN FILE.JSON
{
    item_name: "Onion",
    count: 1
}

then maybe load the onion object using:

Instantiate (Resources.Load ("Items/" + item_name) as GameObject);

I would prefer not to use ScriptableObjects for the inventory system.
More on this here:

But, since this is a simple project with a simple inventory system, then @tmcdonald 's approach looks the best.

Some thoughts:

  1. There is no need to specify that this is an ‘InventoryItem’. Just ‘Item’ would be enough.
  2. Inventory can be a ScriptableObject too! That way we can easily create new inventories for NPCs or chests.
public class Inventory : ScriptableObject
{
    public void Add(Item item, int count = 1);

    public void Remove(Item item, int count = 1);

    public bool Contains(Item item, int count = 1);
}
  1. MonoBehaviour part of Item I suggest to name ItemInstance
public class ItemInstance : MonoBehaviour
{
    [SerializeField] private Item _item;

    public Item item => _item;
}
2 Likes

Now we’re talking. So far we’ve got the following:

Item, an abstract ScriptableObject

public abstract class Item : ScriptableObject
{
  // Future fields here
}

Ingredient, inherits Item and represents a collectible ingredient

public class Ingredient : Item
{
  // Future fields here specific to ingredients
}

(I’m skipping UsableItem because we don’t have information on how it works right now, we can leave that for future design when we have more requirements. Conversely, we can just make Ingredients Items and not bother with the abstract, or use an interface as suggested by @DaSerialGenius )

ItemInstance, the game world representation of an item (sound right @MUGIK ?)

public class ItemInstance : MonoBehaviour
{
  [SerializeField] private Item _item;

  public Item Item => _item;
}

Inventory, a ScriptableObject that represents a collection of items belonging to an entity.

public class Inventory : ScriptableObject
{
   [SerializeField] private Dictionary<Item, int> _items;

    public void Add(Item item, int count = 1)
    {
        // todo: code
    }
    public void Remove(Item item, int count = 1)
    {
        // todo: code
    }
    public bool Contains(Item item)
    {
        // todo: code
    }
}

InventoryController (my naming, I’m definitely amenable to changing it - @SideSwipe9th recommended InventoryManager but I was thinking that Manager is more of a global object, whereas Controller is more of an entity-level object, I am down with whatever y’all prefer though), MonoBehaviour that allows an entity to have access to an inventory (and temporarily allows us to pick it up via a TriggerEnter)

public class InventoryController : MonoBehaviour
{
    [SerializeField] private Inventory _inventory;

    private void OnTriggerEnter(Collider other)
    {
      // Check to see if it was an ItemInstance, if so add it and destroy
    }
}
5 Likes

seems good,
not sure how this works but i think you can create a PR and see if they accept it. According to the contributions topic page. You have already done the code work, and creating a PR should help get it noticed more.

its a good starting point and any changes or more can be discussed here and issues raised in the repository so other people can create their own PRs etc.

I have a concern with using inheritance. Will we not need to maintain separate dictionaries for each subclass we define off of Item? Because Ingredient may have fields that the base class won’t, you can’t maintain a single Dictionary of type Item. If the type of the item is instead an enum on the Item SO, then we only need to maintain a single Dictionary representing the backpack in our inventory controller.

As for filtering that list to get only items of a certain type, we can easily do that with a helper function in the inventory controller like:

public Dictionary<Item, int> GetBackpackItemsOfType(InventoryItem_SO.eType typeToFind){
    Dictionary<Item, int> filteredList = new Dictionary<Item, int>();
    foreach (KeyValuePair<Item, int> entry in backpack)
    { 
        if (entry.Key.type == typeToFind)
        {
            filteredList.Add (entry.Key, entry.Value);
        }
    }
    return filteredList;
}

Unfortunately using a ScriptableObject is one of the requirements from the card on Codecks.

I would not recommend this. Scriptable objects are assets in their own right. If you make a change to the value of a SO, that change applies to all instances of it in the game. If we tracked the quantity of how many of each item the player has within the SO, we’d lose the easy ability to have multiple saves.

JSON only really makes sense for saving player data in a save system. We can easily mark our inventory controller and any other classes with the [Serializable] tag, and feed those classes into the JsonUtility.ToJson for saving to file. But that’s a separate system from this task entirely right now.

1 Like

How many monobehaviours flying around are reasonable? If you have 100+ items in the world, do you really need 100+ monobehaviours?

public class Item : ScriptableObject
{
    public int count=0;
    public void Increment(){ count++; }
    public void Decrement(){ if (count > 0){ count--; }
    }
}
public class InventoryManager : MonoBehaviour
{
    Dictionary<string, Item> PossibleItems = new Dictionary<string, Item>(); 

    void Start()
    {
        PossibleItems.Add("Heal Potion", new Item { count=0 });
        PossibleItems.Add("Sandwich", new Item { count = 0 });
    }
    void HandleItemPickup(string name)
    {
        Item temp;
        PossibleItems.TryGetValue(name, out temp);
        temp.Increment();   
    }
    //could just get items with a count > 0 from possibleitems
    void GetItemsCollected()
    {
        var keys = PossibleItems.Where(item => item.Value.count > 0)
                .Select(item => item.Key);

        //populate visual items for things we have collected from these keys
    }
    private void OnTriggerEnter(Collider other)
    {
        if (other.CompareTag("InventoryItem"))
        {
            HandleItemPickup(other.name);
            Destroy(other.gameObject);
        }
    }
}

1 monobehaviour, items in the world would just have a collider, renderer, and be named by what they are.