Official Inventory System

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

 

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.

 

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]).

 

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

 

@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

 

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

 

Yes, as defined. It would be hard to support two actions with one button.

You lost me. 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.

 

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.

 

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?

 

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

 

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.

 

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

 

I'm working on this now

 

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.

 

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

 

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.

 

Is anyone working on a UI for the inventory system based on the wireframe?

 

It's a separate thread.

 

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.

 

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.

 

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

 

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.

 

@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?

 

Hey guys I've continued working on the inventory system. All details in my PR: https://github.com/UnityTechnologies/open-project-1/pull/130

 

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?

 

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.

 

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

 

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 =)