Found (and fixed) minor bug in the Inventory system

[Seth_Durham]

Steps to reproduce:

1. In inventory, have a stackable item (I tested with both the mana potions and the scrolls) and at least one empty inventory slot.

2. In equipment slots, have a slot that contains an item (hat, dagger, hammer, doesn’t matter).

3. Drag the stackable item from the inventory slot over to the equipment slot that has an item in it.

Expected result: The stackable item returns to inventory, the equipment item remains equipped.

Actual result: The stackable item returns to inventory, but the equipped piece of gear is unequipped and placed in inventory.

Solution:
In DragItem.cs --> AttemptSwap(), a couple of additional checks are needed…

            // Abort if we can't do a successful swap
            if (source.MaxAcceptable(removedDestinationItem) < removedDestinationNumber ||
                destination.MaxAcceptable(removedSourceItem) < removedSourceNumber ||
                destination.MaxAcceptable(removedSourceItem) == 0 || //added check
                source.MaxAcceptable(removedDestinationItem) == 0    //added check
            )

Result: attempting to drop a stackable item onto an equipment slot no longer unequips the item.

[Seth_Durham]

Well, this fixed the issue with stackable items, but introduced a new display bug:

Steps to reproduce:

1. After fix was applied, drag a non-stackable, equipable item (I used a hat) into an occupied equipment slot not meant for drag item (e.g. I had a weapon equipped, and dragged hat over weapon slot).

Expected result: drag item returns to inventory, equipped item remains equipped.

Actual result: drag item returns to inventory, equipped item remains equipped, but an additional inventory slot is supplied with an icon of the drag item (though that slot does not actually contain the item).

After running the debugger and adding some breakpoints, I discovered that after the swap is aborted under these conditions, when source.AddItems(removedSourceItem, removedSourceNumber) is called the value of removedSourceNumber is 0 … so we’re telling the InventorySlotUI to add 0 of an item into inventory.

Solution:
In InventorySlotUI.cs --> AddItems() added a check that the qty we are trying to add is > 0.

        public void AddItems(InventoryItem item, int number)
        {
            if (number > 0)
            {
                inventory.AddItemToSlot(index, item, number);
            }
        }

and hopefully that’s that … haven’t come across any other issues thus far.

[Brian_Trotter]

Well done, I just confirmed the initial bug, made the changes, confirmed the 2nd bug (good luck doing anything with those 0 qty items, they’re just inventory hogs) and made the changes. Bugs squashed.

[sampattuzzi]

I believe that this fix is equivalent and for me but better states the reason we are aborting. Because ultimately we don’t want to do the swap if we aren’t transferring anything into the destination slot.

            if (source.MaxAcceptable(removedDestinationItem) < removedDestinationNumber ||
                destination.MaxAcceptable(removedSourceItem) < removedSourceNumber ||
                removedSourceNumber == 0)
            {
                if (removedDestinationNumber > 0)
                {
                    destination.AddItems(removedDestinationItem, removedDestinationNumber);
                }
                if (removedSourceNumber > 0)
                {
                    source.AddItems(removedSourceItem, removedSourceNumber);
                }
                return;
            }

Surely source.MaxAcceptible being zero isn’t an issue if you aren’t transferring anything back into it so the first part of the condition should handle that.

I also wrapped the AddItems in if statements as below to ensure we are never calling them with 0 as I think that doesn’t make sense.

Let me know if this patch works for you @Brian_Trotter and I will include it in the course.

[Brian_Trotter]

@sampattuzzi This patch did not appear to stop the bug…

[sampattuzzi]

Really? That exact same scenario works for me. This is my diff:

[Brian_Trotter]

LOL, @sampattuzzi some TA I am, I reversed the source and destination when I patched my copy… patch confirmed.

[sampattuzzi]

I’ve updated just the Inventory.zip asset. I think it’s not such a serious bug that it needs fixing in all the asset packs. Sorry for the long delay here.