Understanding the design decision on Item system

C

chrisk

Active member
Hi, Justin, I'm having little hard time understanding how Item systems works. Having understanding why you designed it this way will help set my mind around it.

First of all, the term Item is both used as Inventory Items and general English terms in the Class name. This creates big confusion that Item is not really an Inventory Items and what does what. I'll have to go back to the code remind myself. I hope you use Item as just Inventory Items when naming Classes. Yeah, good naming is one of the most difficult tasks in programming. ^^

And second of all, Item.cs itself does quite a lot of things, handing pickups, drops, store information about UI, handing animations and audio, having direct access to players and so on. It seems like a required component for all Items. If it's a required component, shouldn't it be part of all Items by making a parent Class? (I'm the using Items to mean Inventory Items here, e.g., Shootable Weapon. I can't help but it sounds bit strange.) And it looks like some of Item.cs can be abstracted as Abilities to make it simpler.

I think the whole confusion started because the name Item and overall, naming are too general. I would get rid of Item class by folding into UsableItem or something and save the term Item to mean Inventory Item. Right now, the functionalities are split into Item.cs and other Inventory Items classes and make it little confusing what does what.

Maybe there is something I don't understand why you designed it that way, hence I make this post.
I hope it will be clear for the many other users who are looking into extending the code.

Thank you very much for your answer.

Cheers!
 
chrisk said:
First of all, the term Item is both used as Inventory Items and general English terms in the Class name. This creates big confusion that Item is not really an Inventory Items and what does what. I'll have to go back to the code remind myself. I hope you use Item as just Inventory Items when naming Classes. Yeah, good naming is one of the most difficult tasks in programming. ^^
Click to expand...
I'm not completely following - can you give an example?

chrisk said:
And second of all, Item.cs itself does quite a lot of things, handing pickups, drops, store information about UI, handing animations and audio, having direct access to players and so on. It seems like a required component for all Items. If it's a required component, shouldn't it be part of all Items by making a parent Class? (I'm the using Items to mean Inventory Items here, e.g., Shootable Weapon. I can't help but it sounds bit strange.) And it looks like some of Item.cs can be abstracted as Abilities to make it simpler.

I think the whole confusion started because the name Item and overall, naming are too general. I would get rid of Item class by folding into UsableItem or something and save the term Item to mean Inventory Item. Right now, the functionalities are split into Item.cs and other Inventory Items classes and make it little confusing what does what.
Click to expand...
You're right - Item.cs is required for all items. In version 1 of TPC ShootableWeapon used to inherit Item, but now if version 2 you can have multiple ShootableWeapons on the same Item so that's why it's separate. Not all Items are Usable (such as a static item) so that's why it's the generic Item name.
 
Justin said:
I'm not completely following - can you give an example?
Click to expand...

The Item Class itself and PerspectiveItem Class. PerspectiveItem is not an inventory Item but just PerspectiveVisual standin. The Item Class is the source of the bigger confusion because it's more like a ItemHandler that handles everything else that Inventory Item doesn't. It's more like a utility bag. ^^


You're right - Item.cs is required for all items. In version 1 of TPC ShootableWeapon used to inherit Item, but now if version 2 you can have multiple ShootableWeapons on the same Item so that's why it's separate. Not all Items are Usable (such as a static item) so that's why it's the generic Item name.
Click to expand...

I guess I can understand why you separated them out if that was the only way. But I'm still not clear the responsibilities of the Item.cs. When you say multiple ShootableWeapons, you mean left and right pistol? or what's the use case for having multiple ShootableWeapons that needs to have the same item type?

Yeah, not all items are Usable and Static Items should inherit from lower than Usable. If you fold Item.cs into Item Class, Usable can inherit from Item.cs and Static Item can inherit from Item.cs

Thanks.
 
Regarding, multiple ShootableWeaons on the same Item type, I see that the demo AssaultRile has both ShootableWeapon and MeleeWeapon sharing the same Item.cs. Is that what you mean?

If so, I think I can finally understand what's going on. I got everything backward then. Item.cs is the TRUE Inventory Item and everything else, based on UsableItem are not inventory item but they are the FUNCTIONAL part of Item.cs. Likewise, PerspectiveItem is not really Item but VISUAL part of Item.cs

It is a very different design from the traditional sense. I'll have to think about it a little to digest. When I saw the name Item on UsableItem, I assumed that it was the Inventory Item
Cheers!
 
I love UFPS 2.0 but I must say I really am not a fan of how the 'item'/'inventory' system works. Definitely, in my opinion, the biggest flaw to the asset. But by no means does it ruin the asset, I love UFPS 2.0.
 
chrisk said:
Item.cs is the TRUE Inventory Item and everything else, based on UsableItem are not inventory item but they are the FUNCTIONAL part of Item.cs. Likewise, PerspectiveItem is not really Item but VISUAL part of Item.cs
Click to expand...
That is correct :) PerspectiveItem isn't always visual (in the case of the body item) but yes, in most cases it controls the visuals. I agree there are a lot of "item" names and that part can be confusing. I tried to explain it on this page but I probably need to add a diagram so it's not just a wall of text:

https://opsive.com/support/documentation/ultimate-character-controller/component-overview/
 
Thanks Justin, it makes much better sense now. I was reading the code first and glanced over the documentations. Code is the best doc in my book. ^^ Naming is really the hardest thing in programming and I think there still are rooms for improvement. I hope the things are not written on stones. Thanks for being patient and I hope you are not the only one who writes codes and support users. I've been there and it was very very difficult working so hard but being underappreciated by players. Wish you the very best. ^^
 
Nothing is set in stone - once you've played with everything some more and if you have some alternate names we can discuss it then :)
 
Hallo, i think, that my queation is common with TS.

I want to make in my game several different resources, like gold, metal, food etc…
Is it a correct idea to implement this idea with UCC items and inventory? How should I organize in this case configuration of item categories, item types, items and inventory system?
These items are not carryable. So I need no opportunity to display them at the camera view. I want to use inventory only to handle the amount of resources, that character has.

Thanks.
 
You could, in this case you wouldn't need any items but just use the ItemType. You'd have to create a new script that calls Inventory.PickupItemType when the character enters the trigger so the inventory can then manage it.
 
Thank you, Justin.
It works properly!

But now I need an oppotunity not only to add, but and to remove some count (float) of an ItemType and if the remain count (float) of an ItemType < 1f and this ItemType is equipped in any SlotId, unequip it.

This oppotunity is required for example for a trading.

As I have understood, current implementation of RemoveItem(ItemType itemType, int slotID, bool drop) removes total count of an itemType.

It seems to me that UseItem(ItemType itemType, float count) is a good choice, but in the case when itemType should be unequipped , UseItem do not make it.
 
Last edited:
UseItem is the correct method. You could check to see if the item is empty and if it is then call RemoveItem.
 
I have extended inventory class and add this code.
Please, review :)
It seems to me that it works fine!


C#: 
public void UseItemAndRemove(ItemType itemType, float count)
        {
            var existingAmount = 0f;
            if (!m_ItemTypeCount.TryGetValue(itemType, out existingAmount))
            {
                Debug.LogError("Error: Trying to UseAndRemove item " + itemType.name + " when the ItemType doesn't exist.");
                return;
            }


            int equippedAmount = GetITemTypeEquippedAmount(itemType);


         
            float notEquippedAmount = existingAmount - equippedAmount;


            if (notEquippedAmount >= count || equippedAmount == 0)
            {
                UseItem(itemType, count);
            }
            else
            {
                UseItem(itemType, notEquippedAmount);
                RemoveItemTypeCount(itemType, Mathf.RoundToInt(count - notEquippedAmount));
            }
        }



        private int GetITemTypeEquippedAmount(ItemType itemType)
        {
            int equippedAmount = 0;

            for (int i = 0; i < SlotCount; i++)
            {
                var currentItem = GetItem(i);
                if (currentItem != null)
                {
                    if (currentItem.ItemType == itemType)
                    {
                        equippedAmount += 1;
                    }
                }
            }

            return equippedAmount;
        }


        private void RemoveItemTypeCount(ItemType itemType, int count)
        {
            int removedAmount = 0;

            for (int i = SlotCount - 1; i >= 0; i--)
            {
                var currentItem = GetItem(i);
                if (currentItem != null)
                {
                    if (currentItem.ItemType == itemType)
                    {
                        RemoveItem(itemType, i, false);
                        removedAmount += 1;

                        if (removedAmount >= count) return;
                    }
                }
            }
        }
 
Glad it works. I would make it into a c# extension so you don't have to replace the inventory file every time.
 
Thank you!


I have else some ideas how to extend inventory class.


I also propose you:

1. make these field protected to allow people extend your class

C#: 
        protected Dictionary<ItemType, Item>[] m_ItemTypeItemMap;
        protected Dictionary<ItemType, float> m_ItemTypeCount = new Dictionary<ItemType, float>();
        protected Item[] m_ActiveItem;


2. add in inventory class bool property "doNotUseItemCapacities", because of people can use this class not only on the player, but on the store (trader) for example, which can have unlimited count of an itemTypes.

C#: 
     protected override bool PickupItemTypeInternal(ItemType itemType, float count)
        {
            var existingAmount = 0f;
            if (!m_ItemTypeCount.TryGetValue(itemType, out existingAmount)) {
                m_ItemTypeCount.Add(itemType, Mathf.Min(count, _doNotUseItemsCapacities ? float.MaxValue : itemType.Capacity));
            } else {
                // The ItemType was not picked up if it is already at capacity.
                if (!_doNotUseItemsCapacities && existingAmount == itemType.Capacity) {
                    return false;
                }
                m_ItemTypeCount[itemType] = Mathf.Clamp(existingAmount + count, 0, _doNotUseItemsCapacities ? float.MaxValue : itemType.Capacity);
            }
            return true;
        }




        protected override void UseItemInternal(ItemType itemType, float count)
        {
            var existingAmount = 0f;
            if (!m_ItemTypeCount.TryGetValue(itemType, out existingAmount)) {
                Debug.LogError("Error: Trying to use item " + itemType.name + " when the ItemType doesn't exist.");
                return;
            }
            m_ItemTypeCount[itemType] = Mathf.Clamp(existingAmount - count, 0, _doNotUseItemsCapacities ? float.MaxValue : itemType.Capacity);
        }



        protected override Item RemoveItemTypeInternal(ItemType itemType, int slotID)
        {
            var existingAmount = 0f;
            if (!m_ItemTypeCount.TryGetValue(itemType, out existingAmount)) {
                return null;
            }

            Item item;
            if (!m_ItemTypeItemMap[slotID].TryGetValue(itemType, out item)) {
                // Remove the ItemType. This ItemType does not correspond to an item so it should be completely removed.
                m_ItemTypeCount[itemType] = 0;
                return null;
            }
            // Remove a single Item. The character may be carrying multiple of the same ItemType in the case of dual wielding.
            m_ItemTypeCount[itemType] = Mathf.Clamp(existingAmount - 1, 0, _doNotUseItemsCapacities ? float.MaxValue : itemType.Capacity);

            if (slotID == -1) {
                return null;
            }

            // The item should no longer be equipped.
            if (m_ActiveItem[slotID] != null && m_ActiveItem[slotID].ItemType == itemType) {
                m_ActiveItem[slotID] = null;
            }

            return item;
        }
 
Last edited:
You must log in or register to reply here.
Top