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. ChemaDmk

    ChemaDmk

    Unity Technologies

    Joined:
    Jan 14, 2020
    Posts:
    22
    Yes ! I didn't add this in my description but Items have quantities in the inventory.

    Not sure what a complex dish would be, but for now : dishes are a combination of ingredients + Utensils.
    I'll be checking the Cooking thread next.

    In a separate topic : There will be a recipe list with the possible combinations of ingredients and utensils to use to create a dish, but it's not in the inventory
     
  2. tmcdonald

    tmcdonald

    Joined:
    Mar 20, 2016
    Posts:
    131
    It depends on if there is a requirement to support having multiple stacks of the same kind of item.

    EDIT:

    Do you foresee a need to have multiple stacks of the same kind of item? For example, if there is an ingredient, "Carrot", should there be one stack of "Carrot" in the inventory, or should we support having multiple stacks? (If we want to settle on minimum scope, we can do the former, and just make sure it won't be an issue breaking it out later.)
     
  3. tmcdonald

    tmcdonald

    Joined:
    Mar 20, 2016
    Posts:
    131
    Hey I saw you added a PR to the existing branch, I went ahead and completed it. I had a comment - I think we settled on ItemType being solved via inheritance. I went ahead and accepted the PR but would you mind removing the enum for now? Or we can discuss it here if you want.
     
    Last edited: Oct 22, 2020
  4. davejrodriguez

    davejrodriguez

    Joined:
    Feb 5, 2013
    Posts:
    62
    Ah, gotcha. Can't keep track of all the boards.

    Strictly the editor. Data structure would remain the same.

    When product of one recipe gets used in another. (Eg. [Cocoa + Milk -> Milk Chocolate] [Milk Chocolate + Flour + Butter + Sugar -> Chocolate Chip Cookies]).
     
    kirbygc00, ChemaDmk and tmcdonald like this.
  5. tmcdonald

    tmcdonald

    Joined:
    Mar 20, 2016
    Posts:
    131
    I don't think it would be too hard to implement in this system (once we have the Recipe system, which isn't part of this I think.) You would combine two ingredients to create a new ingredient (instead of a Dish.) The only thing would be, in your example, if Milk Chocolate was also a Dish in addition to being an ingredient. I don't think it would work with their existing UI design, since Dishes and Ingredients have separate actions (and there is no obvious way to support an item being both.)
     
  6. davejrodriguez

    davejrodriguez

    Joined:
    Feb 5, 2013
    Posts:
    62
    @tmcdonald Right, I made a proposal in the crafting/cooking thread that supports "complex" dishes. But like you said it would require multiple item actions which would need to be supported by the UI. I disagree that it would be hard to support it. Can't say I'm a fan of user interface directing game mechanics. But simple is good and I am fine with simple dishes :)
     
    tmcdonald likes this.
  7. tmcdonald

    tmcdonald

    Joined:
    Mar 20, 2016
    Posts:
    131
    Apologies, I meant that it would be hard for the UI (as defined) to support it. As for our purposes (the Inventory System "backend") the biggest hurdle would just be that we couldn't use inheritance to solve our issues and would have to probably make ItemType a ScriptableObject (or Enum if it didn't have fields/properties).
     
  8. davejrodriguez

    davejrodriguez

    Joined:
    Feb 5, 2013
    Posts:
    62
    Yes, as defined. It would be hard to support two actions with one button.

    You lost me. :confused: I don't know why we'd need an ItemType at all. Just let the type tell you what it can do.

    Anyway, I don't want to derail this thread with discussion about something we're not doing. Here is my post in the cooking crafting thread. A lot of overlapping ideas with current Inventory discussion.
     
  9. tmcdonald

    tmcdonald

    Joined:
    Mar 20, 2016
    Posts:
    131
    If we wanted to support an item having multiple types (being an Ingredient and a Dish.)

    EDIT: And I agree about derailing. I'm trying to keep the scope of this PR as small as possible.
     
    ChemaDmk likes this.
  10. shuttle127

    shuttle127

    Joined:
    Oct 1, 2020
    Posts:
    142
    Isn’t that what the current wireframe is suggesting though, change the text based on what item type is visible/selected and thus trigger a different action? I’m not sure if it’s how this PR is written, but wouldn’t it just be a generic action attached to the item that is triggered by the button click, and the action itself knows what to do?
     
  11. tmcdonald

    tmcdonald

    Joined:
    Mar 20, 2016
    Posts:
    131
    He's referring to having one item having multiple actions ("Milk Chocolate" being both an Ingredient and Dish, with two possible actions showing in the UI.)
     
    davejrodriguez likes this.
  12. shuttle127

    shuttle127

    Joined:
    Oct 1, 2020
    Posts:
    142
    Ah, I see.

    I would think it possible to prevent that from happening with a different tab in the UI for each type, so the button’s action would be based on the tab instead of the item. Basically, viewing “Milk Chocolate” from the ingredient tab would treat it like an ingredient (e.g. trigger a UseAction passing the item), and via the dish tab like a dish (e.g. trigger a DishAction passing the item). Does that make sense?

    EDIT: I’ll stop asking questions until we know if an item having multiple types is a requirement, apologies for continuing the derail.
     
    Last edited: Oct 22, 2020
    tmcdonald likes this.
  13. tmcdonald

    tmcdonald

    Joined:
    Mar 20, 2016
    Posts:
    131
    I posted this on the PR:

    So I don't know if you guys are following the thread, but we wanted to change the Inventory from a Dictionary<Item, int> to a List or Array. So we probably want a new C# class that has an Item and Quantity. And then the List (or Array) should be a collection of that class. I originally suggested "ItemStack" for the class name, but you can use whatever name you like that makes sense. If we make this change, the CustomEditor for Inventory can probably be deleted. Would anyone like to make this change? If no takers, I will probably be able to work on it tonight.

    (If anyone takes this, please let us know.)
     
  14. Fhenix

    Fhenix

    Joined:
    Jan 21, 2017
    Posts:
    1
    I'm working on this now
     
    tmcdonald likes this.
  15. tmcdonald

    tmcdonald

    Joined:
    Mar 20, 2016
    Posts:
    131
    Would anyone like to brainstorm with me on handling item selection? Or do we want to handle this in the UI side? If so, we might be ready to just close this PR. @ChemaDmk

    EDIT: Actually I'm going to review this against the acceptance criteria. Will get back tomorrow once I figure out if we're missing anything.
     
    Last edited: Oct 23, 2020
  16. NoDumbQuestion

    NoDumbQuestion

    Joined:
    Nov 10, 2017
    Posts:
    181
    @tmcdonald
    I have do this once. See if this implemention help.

    Techinically, you would have Inventory only hold index ID of item and nothing else. By making sure Inventory is only Data and no logic. It force implement other System to depend only on Data input.
    All Bag/backpack only hold List<int> ItemIds.

    And one SOItemCenterDatabase to link the index to ItemScriptableObject contains basic info for minimum UI:
    - Name
    - Desc
    - Icon
    - GameObjectPrefab (First Component implement Interface that can be executable)
    - no Enum, No tag (just use GetComponent<interface> from Gameobject and let it fail based on other system implementation)
    - No Index, or ID here to prevent wrong implementation

    The GUI will take indexID and scan Database for reference to spawn Sprite in UI.
    The Action/Event or Craft, use Item system will simply check the ItemGameObject that have IConsumableItem,/INonsumableItem or custom infertace implemented.


    The Cons:
    - You have to make Global Static Editor to add new ScriptableObject to CenterDatabase. And Editor preview Item from index in Editor (also allow drop any ItemSO to simplify for Designer) because all Bags, Treasure box only have item index to prevent non register Item to popup randomly.
    - It is slower and scale with system complexiplity as more Interface popup need to do if/else check.

    The Pros:
    - Force everything rely on GameObject implement the right component reduce error to one guy. Other system only have to check if ItemGameobject is setup correctly before call their function.
    - All item are just ScriptableObject link to a prefab. All method, execution is based on Gameobject. You can change gameobject implemation in PlayMode since everything just rely on Data.
     
  17. NoDumbQuestion

    NoDumbQuestion

    Joined:
    Nov 10, 2017
    Posts:
    181
    I have been looking through your PR code.
    Your inventory system is far different from my generic approach.

    New inheritance class foreach type of new Item would be good as foresight but it would become cumbersome in long run.

    Because if Designer decide to have more new items it would be a mess to maintain, update other system to answer new ItemClassType. Since all item class is locked/predefined, it just hard to refactor everything.
    In my experience, it would be just along list of SwitchCase to handle everyitem type for every class exist in game without breaking existing Item.

    Since this game design is simple enough, do your way is fine. Still, I would bump this up as discussion since it would make implmentation of newer system like crafting,equipting item harder to do if and only if someone need redesign system.
     
  18. Soundguy

    Soundguy

    Joined:
    Oct 30, 2009
    Posts:
    45
    Is anyone working on a UI for the inventory system based on the wireframe?
     
  19. tmcdonald

    tmcdonald

    Joined:
    Mar 20, 2016
    Posts:
    131
    It's a separate thread.
     
  20. tmcdonald

    tmcdonald

    Joined:
    Mar 20, 2016
    Posts:
    131
    I would offer a correction: it isn't "my" PR. It is a collective effort from several people designing in this thread. I'm considering seeing if people want to make "ItemType" a ScriptableObject itself (rather than rely on inheritance) to make it easier to switch items to a different type.
     
  21. shuttle127

    shuttle127

    Joined:
    Oct 1, 2020
    Posts:
    142
    Would that introduce extra conditional logic based on which SO it is? Also, is there any other information specific to type, or just its name? If there's more than just name it might make sense to make that change?

    ========

    While not directly an “ItemType” issue, there's a new proposal in the cooking/crafting thread to add item quality to improve the cooking aspect of the game. Since Chema said she was already going to review that thread, can wait for her perspective before trying to add it, just wanted to bring it up for those that aren't hopping around the forums like I am.
     
  22. tmcdonald

    tmcdonald

    Joined:
    Mar 20, 2016
    Posts:
    131
    Right now, the only thing associated with ItemType is the name of the action. I suspect that the "Action Name" we are using right now is very much not the way we want to go moving forward. But I think the context of "Action Name" (and the like) needs to be established as part of the UI. So I'm not sure. There needs to be an Item Selection system and a Recipe system (that interact with each other and with the inventory.)
     
  23. NoDumbQuestion

    NoDumbQuestion

    Joined:
    Nov 10, 2017
    Posts:
    181
    I made different inventory system with code demo along with it.
    This might fix issues with ItemType class. I change it all to interfaces in GameObject.

    Other system only need have access to prefab, and have to manually check for their GameObject have correct interface.

    https://github.com/UnityTechnologies/open-project-1/pull/105


    Now we just need discuss specific what kind of items interaction we need to put into game.
     
    Last edited: Oct 25, 2020
    kirbygc00 likes this.
  24. cirocontinisio

    cirocontinisio

    Unity Technologies

    Joined:
    Jun 20, 2016
    Posts:
    422
    Just reviewed the code in PR #100. Great work @tmcdonald et al.! I love that you started simple, thanks for that! Sorry for not taking a look earlier.

    @NoDumbQuestion yours looks good too, although at first glance it looks like you're introducing some unneeded complexity by creating a system that is ready to handle future functionality that might never arrive to the game. But I want to take a better look so allow me to do that tomorrow! The Inspector work looks great though!
     
    kirbygc00 likes this.
  25. tmcdonald

    tmcdonald

    Joined:
    Mar 20, 2016
    Posts:
    131
    @cirocontinisio thanks for the feedback. @Soundguy submitted a PR into the branch that took care of your comments. I pushed a change as well this morning for consistency. I also ended up getting rid of the inheritance model, ItemType is now a ScriptableObject itself (I'm still not a big fan of ActionName being part of it, but it is what it is until we actually implement the action system.) As part of this initial push, do you want items to already have the "Action" commands implemented, or are you fine with that being a different work item?
     
  26. cirocontinisio

    cirocontinisio

    Unity Technologies

    Joined:
    Jun 20, 2016
    Posts:
    422
    Thanks everyone, I merged the version that @tmcdonald and the others were working on. Let's call it a v1, and we can keep iterating on it - and use this thread as a base for discussion.
     
    tmcdonald likes this.
  27. circa94

    circa94

    Joined:
    Aug 26, 2014
    Posts:
    4
  28. circa94

    circa94

    Joined:
    Aug 26, 2014
    Posts:
    4
    My idea is to create some events for example when an item was collected or has been used. Based on the event system with scriptable objects introduced by @Amel-Unity
    With this we can raise an event and another system can listen to them and play for example a sound when an item was collected. What you think about that?
     
  29. shuttle127

    shuttle127

    Joined:
    Oct 1, 2020
    Posts:
    142
    Hopefully the people that posted in the item pickup thread are following this thread now that I suggested to check it out. I'd recommend coordinating with them to get some ideas too. This could also overlap with the UI a bit because we were talking in the wireframing thread about popups on the screen when the player acquires an item, so love to hear other suggestions.
     
    ChemaDmk likes this.
  30. frankitox16

    frankitox16

    Joined:
    Sep 1, 2015
    Posts:
    2
    Looking at the inventory system, do we want a ItemStack for each kind of Item, or do we want a stack for everything that was looted? Like, maybe a boss drops a bag with a lot of spices or something, maybe we'd want to support having different items in the stack instead of multiple stacks
     
  31. kirbygc00

    kirbygc00

    Joined:
    Apr 4, 2016
    Posts:
    45
    Hi all,

    good work so far everyone! Do we have any higher-priority task to do for this feature?

    Some minor things:
    • I see that InventoryController.cs both interacts with items on trigger enter and destroys items.
      • I would probably want to separate out that logic... have some kind of method you call on the item instance which can do whatever it needs to do... e.g. turnip.pickup() => destroys GO, wheat.pickup()=> removes 1 item stack, when item stacks hit 0 the game object is destroyed, etc...
      • Would have some kind of List<ItemInstances> held on the InventoryController that is updated on trigger enter / exit. this would represent all interactables (items) in range. We would wait for a button press before we tried to interact with the item.
    • I think it would be nice for add and remove in Inventory.cs to both point to a modify function which does the actual modifying.
      • Is there a cap for the item stack size? I see we just add or remove as long as the count is above 0.
    • Is there some reason the items are not stored as a hashmap? does it have to deal with having multiple stacks of the same item type? is that a requirement?
    • Maybe too early, but I think it would be nice to use some more interfaces here, e.g. from the @tmcdonald PR:
      • Interfaces would definitely handle this and similar scenarios quite well.
    Anyways, some minor things. Good work so far everyone. Looks good =)
     
unityunity