Search Unity

  1. Click here to see what's on sale for the "Best of Super Sale" on the Asset Store
    Dismiss Notice
  2. Good news ✨ We have more Unite Now videos available for you to watch on-demand! Come check them out and ask our experts any questions!
    Dismiss Notice

Official Inventory System

Discussion in 'Open Projects' started by JakHussain, Sep 30, 2020.

  1. JakHussain

    JakHussain

    Joined:
    Oct 20, 2016
    Posts:
    318
    Hi, I'd like to request to take on the Inventory System ticket if it's still available.

    If you're happy for me to work on it, could you please share with me any additional details about the scope of the feature and how it may potentially expand in the future such as:

    • Can all items be "used"?
    • Would all items require a sprite icon?
    • Are there different types of items that may have additional functions?
    • Will items be collectable in the 3D game environment or will they only act as data that's represented in the UI?
    My github profile is at: https://github.com/pablothedolphin
     
  2. SideSwipe9th

    SideSwipe9th

    Joined:
    Jan 10, 2019
    Posts:
    24
    Hey there,

    I would like to try and make an attempt at the inventory system. I see on the card for it that there's some basic ideas for items like health potions, crafting ingredients. I've got an idea for a scriptable object class that could do this, and I was wondering if there was a more complete list of what item types we might need?
     
  3. Caffeen

    Caffeen

    Joined:
    Dec 25, 2013
    Posts:
    29
    I'd love to create the Inventory system. I have lots of experience creating fully-featured ScriptableObject-based inventory systems. I also have lots of time right now so I can get it done fast.

    Here's an example of one I made:
     
  4. krisslj

    krisslj

    Joined:
    Feb 24, 2019
    Posts:
    2
    Hi! I would like to make the inventory system ( https://open.codecks.io/unity-open-project-1/decks/15/card/15o-inventory-system ).
    I have made an inventory system with scriptable objects before, and could alter it to fit into this project! I would need some information about if it implies an equipment system as well, and if so, what kind of items can be equipped and where. I assume it means UI too, but please let me know if it doesn't.
    Hope I can be to help :)
     
  5. SideSwipe9th

    SideSwipe9th

    Joined:
    Jan 10, 2019
    Posts:
    24
    I've made a fork with my rough idea for an inventory system and manager. The main code commit is here. And a couple of changes I forgot to commit the first time are here.

    The InventoryItem_SO contains the basic info on the item; an enum representing its type, an int representing a value that should be increased when picked up or consumed, and a GameObject that is the prefab of the item that you are picking up which can be placed again in the world if we are allowing for items to be dropped by the player. We can also add any other fields here, for example Inventory UI Sprites as needed.

    The InventoryItem script is pretty barebones, and just contains a reference to the scriptable object which has all the real data.

    The InventoryManager is where most of the logic happens. It contains a List of InventoryItem_SO representing the backpack, though this could also be an array if we want to limit the backpack size. Runtime pickup of an item is handled by a trigger collider on the item being picked up, if the item we're colliding with is tagged "InventoryItem" we add the SO for it to our backpack, and then delete the GameObject from the world. This can be refactored pretty easily if we instead want to use object pooling, and hide the item GameObjects offscreen if we don't want to instantiate them afresh if we allow for items to be dropped.

    The InventoryManager is also where consuming of consumables will be processed. I've not added any code for this yet, but it would be a simple matter of adding a ProcessItem function which would check the enum on the SO before adding the relevant value to the health or money, or other system. Such processing could be done immediately on pickup, or delayed until the player selects the item from their inventory to consume.
     
  6. SideSwipe9th

    SideSwipe9th

    Joined:
    Jan 10, 2019
    Posts:
    24
    And of course I'm happy to take feedback here or on GitHub if anyone has any questions about why I made these choices, or if you think they may cause issues in the future :)
     
  7. AnisimovArthur

    AnisimovArthur

    Joined:
    Mar 4, 2013
    Posts:
    19
    Hey,

    I noticed that you are using enum in your SO to identify an item type. I want to say that you can identify the item by SO itself.
    So in the future you can create a new SO if you will need a new item type without extending enum.

    If you are worry about identifying it in the code later, you can create Heal, Money... classes inherited from abstract Item class and add ItemPrefab property to the SO.

    Hope it helps you and helps your code get in the project!
     
  8. StigiBengBeng

    StigiBengBeng

    Joined:
    Sep 22, 2013
    Posts:
    2
    Hello,
    I would like to start working on simple inventory system. I was thinking it could be build in phases, and my focus would be to make it easy to build upon. I have some experience with Unity, but actually it would be my first inventory system ;)

    I have a question though, couldn't find it mentioned anywhere:
    Have you already planned how items will drop and the way the character will be picking them up?
    From concept art and the general art style, in my opinion items should drop directly on the ground (Zelda: BotW style) and picked one by one via keybind action.
    Hmm now that I think about it, it might be too early to ask this, and it's not really a necessary information to create this system.
     
  9. SilverScales

    SilverScales

    Joined:
    Jun 13, 2018
    Posts:
    1
    Hahaha dang, i was just about to sign up for it. Already begun building one... Anyways, I'll just pick something else.
     
  10. robizeta2005

    robizeta2005

    Joined:
    Jan 28, 2020
    Posts:
    1
    I would like you give me some directives about the parameters of the scriptableobject,
    otherwise I will write the scriptable object in a reasonable and exstensible way, I hope :)

    I will even implement it a few different ways, so you can choose the better one for you, or switch them during the development without effort
     
    Last edited: Sep 30, 2020
  11. StigiBengBeng

    StigiBengBeng

    Joined:
    Sep 22, 2013
    Posts:
    2
    There are few other threads already and they haven't picked anyone yet, so you might as well post your thread too, I think.
    On the other hand they might as well have a look at few different implementations I guess ;)
     
  12. SideSwipe9th

    SideSwipe9th

    Joined:
    Jan 10, 2019
    Posts:
    24
    So the reason I wanted an explicit enum in the SO is so that all the information for an inventory item is in one place; the SO. This makes it simpler for the inventory manager to manage the whole inventory, as it can be represented as a List of type InventoryItem_SO. And because instances of a SO are assets, this also allows you to easily create multiple items of the same type, but with different properties; for example a small, medium, and large heal potion that heals 10, 50, and 100 hit points.

    If I'm understanding your suggestion correctly, this would complicate inventory management, as instead of extending the enum in the InventoryItem_SO.cs file, you'd be creating entirely new _SO.cs files for each item type; HealItem_SO.cs, CraftingItem_SO.cs, etc. Then when it comes to inventory management, instead of maintaining a single list, you'll have separate lists for each SO type. And every time you want to add a new item type, you'll need to add more code to your inventory manager to handle it.

    One problem with adding separate heal or money classes on the item GameObject is that, if at pickup you destroy the item from the world space, you will also delete everything attached to it. You can work around this by having a physical inventory storage area someplace else in the scene that the player can never see, however that comes with its own tradeoffs for memory usage and active objects in a scene.

    If I misunderstood your suggestion, please let me know :)
     
  13. KingAustin

    KingAustin

    Joined:
    Jan 12, 2016
    Posts:
    12
    I know there are already 5 other topics asking to do inventory system, but this is different. I want to look at all the options for what kind of inventory systems that could be implemented and push forward my suggestions.

    Constraints or goals:
    What do we want our inventory system to do?
    1) For specific items.

    if we only have a few milestone items that need to be collected a limited sidebar inventory would suffice or a pre-listed collection inventory with ghost outlines to show how far a player has progressed though the game.
    Technically speaking a milestone inventory dose not count as a standard inventory, the items cant be consumed or interacted with in any way other than viewing it. An example would be other story games where you are given a quest item thats not actually in your inventory.

    a simplistic limited slot inventory is also an option, it would have <8 slots to carry quest items or other kind of items and they would not stack, this would save work and effort for it.

    2) for mass items.
    this would involve stacking items and many different items types with many different uses. The items can be used up in different ways and there may equipment items too.

    both are viable options, 1 for milestone item collection or unlocking item collection and 1 for normal inventory described hence forth.

    clarifications?
    Looking at the cards in the roadmap there seems to be an energy feature, i am unsure if it applies to the player but it seems the characters can pickup something that recovers energy for them in all ways it appears to be an HP system it has no mention that attacking loses energy for them. The something that can be picked up is called an item.
    Would other characters also be required to have an inventory?
    would it be the same as the player or less detailed and simplistic, would the player be able to view the enemies inventory and loot them?

    What items should be included in the game?
    currently the only mechanic i know about from the cards is the energy mechanic. The way forward for the inventory system would describe how it interacts with energy.
    Will the player consume an item to recover energy?
    to prevent infinite lifespan eating, would energy items be limited in anyway such as preventing the player from quickly eating in battle to cheat death?
    What would the stack size for certain items be?
    what will the energy pickup (items) be and what will they look like?

    and most importantly what other items are needed in the game and what will they be and what will they look like, this also includes items for other mechanics.

    while the main inventory system is code based, The items, pickups, milestones and anything else will need 2D icons as well as the main inventory layout and backbone. The 3D world items will also need to be created and these can be assigned alongside this after its been decided whats being included in the game or what the inventory system will be like.

    if there are other ways to do inventory systems or styles, needs, limitations and more please do say.
    i dont know what kind of inventory system this game needs because the cards dont describe many mechanics.
     
    Semetre and Neonage like this.
  14. tmcdonald

    tmcdonald

    Joined:
    Mar 20, 2016
    Posts:
    131
  15. JakHussain

    JakHussain

    Joined:
    Oct 20, 2016
    Posts:
    318
    Bumping my request
     
  16. AnisimovArthur

    AnisimovArthur

    Joined:
    Mar 4, 2013
    Posts:
    19
    Not certainly in that way. I mean you can create extra SO per item (not .cs, exactly SO).

    And then you have the "gameWorldRuntimePrefab" variable in your SO. Instead of GameObject type you can use your InventoryItem.cs:
    Code (CSharp):
    1. /// <summary>
    2. /// A reference to the prefab that can be spawned in the world if the player drops the item.
    3. /// </summary>
    4. public InventoryItem gameWorldRuntimePrefab;
    And then you can inherit Health.cs from InventoryItem.cs and do Health's logic there.

    But basically it would be better to separate Pickup and Item logic.
     
    JohanAcosta likes this.
  17. DaSerialGenius

    DaSerialGenius

    Joined:
    Jun 27, 2017
    Posts:
    17
    So I was looking at the codedeck page and saw that the Inventory system is unassigned. I'm thinking something with a with a pickable script with an overridable use/throw/etc functions kinda like in Deus Ex PC.

    I'll use some placeholder art, mostly squares lol

    I think keeping inventory like in Deus Ex might be fun, a grid and you can place objects based on its size. Adds a bit of complexity to the game!

    Let me know if it gets assigned to me so that I can get started!
     
    theabhinv likes this.
  18. cirocontinisio

    cirocontinisio

    Unity Technologies

    Joined:
    Jun 20, 2016
    Posts:
    402
    This is the official thread to discuss the Inventory System (roadmap card).

    Please start with simple implementations that generally stick to what the card on the roadmap is saying. There will be time to expand the functionality.
     
    tmcdonald likes this.
  19. tmcdonald

    tmcdonald

    Joined:
    Mar 20, 2016
    Posts:
    131
    Thanks for the merge!
     
    cirocontinisio likes this.
  20. tmcdonald

    tmcdonald

    Joined:
    Mar 20, 2016
    Posts:
    131
    I canceled my PR (it was a fun diversion and I was hoping to spark some conversation but I'm definitely not interested in co-opting everyone else's fun) and prefer to discuss the design here, now that there is a single thread and not just a bunch of disparate threads.

    This task seems relatively light on detail. In order to meet the requirements, I think we only need the following components:

    • A ScriptableObject that represents a type of item. This should have the bare minimum on it for now, in order to prevent expanding the scope for no reason.
    • A MonoBehaviour that represents a pickup in the world. This should have a reference to the item type.
    • A MonoBehaviour that allows a character to interact with a pickup. For now, it could just have an OnTriggerEnter.
    • A MonoBehaviour that represents the character's current inventory. To be as simple as possible, it could just be a dictionary of item and the carried quantity.
    There is a lot of talk about UI design and whatnot here. I believe these topics are outside of the scope.

    I know there are a lot of parties interested. I think it would be valuable for us to agree on a design ahead of time here that doesn't exceed the scope (remember, to meet the requirements of the card, we only need: item scriptable object, the ability to pick up an item, the ability for a character to remember what items they picked up.)

    Now, before I keep rambling, I'd like to leave the floor open for people to discuss whether or not they think these 4 components meet the requirements of the ticket (without blowing up the scope of the story.) If so, we can discuss naming convention, and pseudo-code them out. Once this is done, someone can just throw it into a repo and add a comment crediting the forum members discussing it here (if desired.)

    Thoughts?
     
    ChemaDmk and davejrodriguez like this.
  21. DaSerialGenius

    DaSerialGenius

    Joined:
    Jun 27, 2017
    Posts:
    17
    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 :)
     
  22. tmcdonald

    tmcdonald

    Joined:
    Mar 20, 2016
    Posts:
    131
    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.
     
  23. DaSerialGenius

    DaSerialGenius

    Joined:
    Jun 27, 2017
    Posts:
    17
    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?
     
  24. tmcdonald

    tmcdonald

    Joined:
    Mar 20, 2016
    Posts:
    131
    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:

    Code (CSharp):
    1. public class InventoryItem : ScriptableObject
    2. {
    3.   // Future fields here when scope is expanded
    4. }
    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?
     
  25. SideSwipe9th

    SideSwipe9th

    Joined:
    Jan 10, 2019
    Posts:
    24
    @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<T> 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.


    Code (CSharp):
    1. public class InventoryManager : MonoBehaviour
    2. {
    3.     public List<InventoryItem_SO> backpack = new List<InventoryItem_SO>();
    4.     public Dictionary<InventoryItem_SO, int> backpackDict = new Dictionary<InventoryItem_SO, int>();
    5.  
    6.     private void OnTriggerEnter(Collider other)
    7.     {
    8.         // If we've entered the trigger collider for a pickup item
    9.         // add it to the backpack, and destroy the item in the world
    10.         // Note: can refactor this to disable if we want to use object pooling
    11.         if (other.tag == "InventoryItem")
    12.         {
    13.             InventoryItem pickupItem;
    14.             if (other.gameObject.TryGetComponent<InventoryItem>(out pickupItem))
    15.             {
    16.                 backpack.Add(pickupItem.inventoryItem);
    17.                 Destroy(other.gameObject);
    18.             }
    19.         }
    20.     }
    21. }
     
  26. DaSerialGenius

    DaSerialGenius

    Joined:
    Jun 27, 2017
    Posts:
    17
    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 :D
     
  27. DaSerialGenius

    DaSerialGenius

    Joined:
    Jun 27, 2017
    Posts:
    17
    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.
     
  28. SideSwipe9th

    SideSwipe9th

    Joined:
    Jan 10, 2019
    Posts:
    24
    @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.
     
    ChemaDmk likes this.
  29. tmcdonald

    tmcdonald

    Joined:
    Mar 20, 2016
    Posts:
    131
    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.
     
    davejrodriguez likes this.
  30. KingAustin

    KingAustin

    Joined:
    Jan 12, 2016
    Posts:
    12
    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.
     
  31. tmcdonald

    tmcdonald

    Joined:
    Mar 20, 2016
    Posts:
    131
    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:

    Code (CSharp):
    1. public abstract class InventoryItem : ScriptableObject
    2. {
    3.   // Future common fields
    4. }
    5.  
    6. public class Ingredient : InventoryItem
    7. {
    8.   // Future fields specific to ingredients
    9. }
    10.  
    11. public class UsableItem : InventoryItem
    12. {
    13.   // Future fields specific to usable items
    14. }
    However, I don't know that it's necessary.
     
    ChemaDmk, MUGIK and KingAustin like this.
  32. DaSerialGenius

    DaSerialGenius

    Joined:
    Jun 27, 2017
    Posts:
    17
    I feel like we're slowly coming back to my interface idea
    Code (CSharp):
    1.  
    2. public Interface InventoryItem
    3. {
    4.     int count;
    5.     void Use();
    6.     // Any more as required
    7. }
    8.  
    9. public class Ingredient : MonoBehaviour, InventoryItem
    10. {
    11.     void Use()
    12.     {
    13.      // implementations in whatever way
    14.     }
    15. }
    16.  
    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)
     
  33. tmcdonald

    tmcdonald

    Joined:
    Mar 20, 2016
    Posts:
    131
    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.
     
  34. DaSerialGenius

    DaSerialGenius

    Joined:
    Jun 27, 2017
    Posts:
    17
    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.

    Code (CSharp):
    1.  
    2. //THIS WOULD BE IN FILE.JSON
    3. {
    4.     item_name: "Onion",
    5.     count: 1
    6. }
    then maybe load the onion object using:

    Code (CSharp):
    1. Instantiate (Resources.Load ("Items/" + item_name) as GameObject);
     
  35. MUGIK

    MUGIK

    Joined:
    Jul 2, 2015
    Posts:
    198
    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.
    Code (CSharp):
    1. public class Inventory : ScriptableObject
    2. {
    3.     public void Add(Item item, int count = 1);
    4.  
    5.     public void Remove(Item item, int count = 1);
    6.  
    7.     public bool Contains(Item item, int count = 1);
    8. }
    3. MonoBehaviour part of Item I suggest to name ItemInstance
    Code (CSharp):
    1. public class ItemInstance : MonoBehaviour
    2. {
    3.     [SerializeField] private Item _item;
    4.  
    5.     public Item item => _item;
    6. }
     
    KingAustin and tmcdonald like this.
  36. tmcdonald

    tmcdonald

    Joined:
    Mar 20, 2016
    Posts:
    131
    Now we're talking. So far we've got the following:

    Item, an abstract ScriptableObject
    Code (CSharp):
    1. public abstract class Item : ScriptableObject
    2. {
    3.   // Future fields here
    4. }
    Ingredient, inherits Item and represents a collectible ingredient
    Code (CSharp):
    1. public class Ingredient : Item
    2. {
    3.   // Future fields here specific to ingredients
    4. }
    (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 ?)
    Code (CSharp):
    1. public class ItemInstance : MonoBehaviour
    2. {
    3.   [SerializeField] private Item _item;
    4.  
    5.   public Item Item => _item;
    6. }
    Inventory, a ScriptableObject that represents a collection of items belonging to an entity.
    Code (CSharp):
    1. public class Inventory : ScriptableObject
    2. {
    3.    [SerializeField] private Dictionary<Item, int> _items;
    4.  
    5.     public void Add(Item item, int count = 1)
    6.     {
    7.         // todo: code
    8.     }
    9.     public void Remove(Item item, int count = 1)
    10.     {
    11.         // todo: code
    12.     }
    13.     public bool Contains(Item item)
    14.     {
    15.         // todo: code
    16.     }
    17. }
    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)
    Code (CSharp):
    1. public class InventoryController : MonoBehaviour
    2. {
    3.     [SerializeField] private Inventory _inventory;
    4.  
    5.     private void OnTriggerEnter(Collider other)
    6.     {
    7.       // Check to see if it was an ItemInstance, if so add it and destroy
    8.     }
    9. }
     
    Last edited: Oct 1, 2020
  37. KingAustin

    KingAustin

    Joined:
    Jan 12, 2016
    Posts:
    12
    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.
     
  38. SideSwipe9th

    SideSwipe9th

    Joined:
    Jan 10, 2019
    Posts:
    24
    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:

    Code (CSharp):
    1. public Dictionary<Item, int> GetBackpackItemsOfType(InventoryItem_SO.eType typeToFind){
    2.     Dictionary<Item, int> filteredList = new Dictionary<Item, int>();
    3.     foreach (KeyValuePair<Item, int> entry in backpack)
    4.     {
    5.         if (entry.Key.type == typeToFind)
    6.         {
    7.             filteredList.Add (entry.Key, entry.Value);
    8.         }
    9.     }
    10.     return filteredList;
    11. }
     
  39. SideSwipe9th

    SideSwipe9th

    Joined:
    Jan 10, 2019
    Posts:
    24
    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.
     
    Last edited: Oct 1, 2020
    KingAustin likes this.
  40. lmbarns

    lmbarns

    Joined:
    Jul 14, 2011
    Posts:
    1,483
    How many monobehaviours flying around are reasonable? If you have 100+ items in the world, do you really need 100+ monobehaviours?

    Code (CSharp):
    1.  
    2. public class Item : ScriptableObject
    3. {
    4.     public int count=0;
    5.     public void Increment(){ count++; }
    6.     public void Decrement(){ if (count > 0){ count--; }
    7.     }
    8. }
    Code (CSharp):
    1. public class InventoryManager : MonoBehaviour
    2. {
    3.     Dictionary<string, Item> PossibleItems = new Dictionary<string, Item>();
    4.  
    5.     void Start()
    6.     {
    7.         PossibleItems.Add("Heal Potion", new Item { count=0 });
    8.         PossibleItems.Add("Sandwich", new Item { count = 0 });
    9.     }
    10.     void HandleItemPickup(string name)
    11.     {
    12.         Item temp;
    13.         PossibleItems.TryGetValue(name, out temp);
    14.         temp.Increment();  
    15.     }
    16.     //could just get items with a count > 0 from possibleitems
    17.     void GetItemsCollected()
    18.     {
    19.         var keys = PossibleItems.Where(item => item.Value.count > 0)
    20.                 .Select(item => item.Key);
    21.  
    22.         //populate visual items for things we have collected from these keys
    23.     }
    24.     private void OnTriggerEnter(Collider other)
    25.     {
    26.         if (other.CompareTag("InventoryItem"))
    27.         {
    28.             HandleItemPickup(other.name);
    29.             Destroy(other.gameObject);
    30.         }
    31.     }
    32. }
    33.  
    1 monobehaviour, items in the world would just have a collider, renderer, and be named by what they are.
     
  41. SideSwipe9th

    SideSwipe9th

    Joined:
    Jan 10, 2019
    Posts:
    24
    I'm not sure why the number of MonoBehaviours is a concern? You cannot attach a Scriptable Object to a GameObject directly. You can only do that as a public/private variable/property on a MonoBehaviour.

    MonoBehaviour overhead isn't huge when you remove empty Start and Update methods. Using a MonoBehaviour that only contains public/private variables and properties is an accepted game design pattern. If we're at the stage where there's too many MonoBehaviours in a scene, we're also at a stage where there's too many GameObjects in a scene. And the solution for that is splitting into subscenes and loading them additively.
     
  42. CireSur

    CireSur

    Joined:
    Dec 1, 2017
    Posts:
    2
    I would suggest the Inventory derive from MonoBehaviour rather than ScriptableObject. The add and remove methods will change the values directly to the reference rather than an instance.

    No, we can use Item and do checks if Item is of a specific type when we need to. As long as all item types derive from Item, we can use a single dictionary for all of it.
     
  43. Neonage

    Neonage

    Joined:
    May 22, 2020
    Posts:
    190
    I didn't get whats the problem here, it's just an example how NOT to use prefabs.
    You can easily group your ScriptableObjects together and create unique combinations with a few drag and drops.
    It supports full Json serialisation with polymorphism.
    They we're created as pure shared data containers with scriptable logic.They're perfect for the inventory
     
    davejrodriguez likes this.
  44. SideSwipe9th

    SideSwipe9th

    Joined:
    Jan 10, 2019
    Posts:
    24
    My understanding is you cannot do that in C#? If you have a class
    Item
    , and an inherited class
    ItemVariant
    , you cannot safely add an instance of
    ItemVariant
    to a
    List<Item>
    or
    Dictionary<Item, int>
    ? Can you show me a code example of how this would work?
     
  45. LaneFox

    LaneFox

    Joined:
    Jun 29, 2011
    Posts:
    6,699
    This works, you just have to cast it to ItemVariant if you don't want it back as the list's type.
     
  46. SideSwipe9th

    SideSwipe9th

    Joined:
    Jan 10, 2019
    Posts:
    24
    If we're casting to and from the base and subclasses whenever we're manipulating (adding, editing, retrieving, or deleting), surely we will also be boxing because the subclasses will have values that are unique to them?

    This is an issue because boxing has garbage collection implications, which will impact upon performance. I'm sure we could write a variant of Dictionary<Tkey, Tvalue> which has caching to minimise the impact of boxing, but we wouldn't be able to eliminate it entirely if we went down that path.
     
  47. tmcdonald

    tmcdonald

    Joined:
    Mar 20, 2016
    Posts:
    131
    Hey guys, I went ahead and created the repo (not a pull request) of the basic ideas we scaffolded out here.

    https://github.com/trevorjmcdonald/open-project-1/tree/trevorjmcdonald/inventory-system-base

    Please feel free to submit changes to it (or create your own if you wish.)

    @ImBarns The ItemInstance is basically a script on items in the game world, not items in the inventory. And it shouldn't be a big deal to have hundreds in the game world - they're not running updates or anything. Plus, premature optimization is the death of projects!

    @cirocontinisio would it be possible to get some feedback or direction?
     
    AnisimovArthur likes this.
  48. Neonage

    Neonage

    Joined:
    May 22, 2020
    Posts:
    190
    Boxing doesn't matter in that case at all, it would be only 5 times slower, 5 ticks instead of 1.
    And item stats update rate is super low - only when inspecting items in UI.
     
    kirbygc00 likes this.
  49. Neonage

    Neonage

    Joined:
    May 22, 2020
    Posts:
    190
    For the sake of god, please, don't use prefixes for any member!
     
  50. tmcdonald

    tmcdonald

    Joined:
    Mar 20, 2016
    Posts:
    131
    Feel free to vote in the Coding Standards poll!

    https://forum.unity.com/threads/vote-for-coding-standards.980337/#post-6371562

    EDIT: I apologize if this came off as dismissive btw, I meant that we should use that thread/poll to establish the coding standards. I don't really have strong preferences other than I don't like having public fields if I can help it. I actually only started doing the underscore prefix on private members fairly recently, because I got some reasonably compelling arguments in favor of them.
     
    Last edited: Oct 1, 2020
    MUGIK likes this.
unityunity