HandleItemOverflow BUG

P

pako

Member
Hi,

There seems to be a Bug in the parameters of HandleItemOverflow. As is, when an ItemOverflowAction is created, the "itemInfoAdded" parameter is "None". Also, both originalItemInfo and rejectedItemInfo contain the same value of the Previous Item. It's not a problem for me at this point, but maybe you could point out a use case for my future reference.

However, the itemInfoAdded parameter being empty is a problem for me, because I would like to have a reference to the new ItemInfo that was added.

I think that I have pinpointed the problem:
In ItemSlotCollection, and inside the SetItemAmount(ItemInfo itemInfo, int slotIndex, bool removePreviousItem) method (line 279), first removedItemInfoAdded is initialized to ItemInfo.None (line 300), and then a call is made on line 301 to the method HandleItemOverflow(removedItem, ref removedItemInfoAdded), which is defined in the base class.

It seems to me that removedItemInfoAdded should be replaced by the SetItemAmount method's 1st parameter: "ItemInfo itemInfo", since ItemCollection.HandleItemOverflow(ItemInfo originalItemInfo, ref ItemInfo itemInfoAdded) seems to expect the itemInfoAdded and not the removedItemInfoAdded.

As a result, if an OverflowAction gets set, the call to m_OverflowOptions.OverflowAction.HandleItemOverflow(m_Inventory, originalItemInfo, itemInfoAdded, rejectedItemInfo) will still have itemInfoAdded as initialized to ItemInfo.None.

So, am I missing something, or is this a bug?

Thank you in advance.
 
I see what you mean.
I hadn't thought of it this way. The way I saw it was independent of the item added.
But what you say does make sense and gives you more data to work with.

So here are the changes I suggest,
So you first Remove the item, Add the new one and then at the end do the overflow
what do you think?

Code: 
/// <summary>
/// Sets the amount for the specified item.
/// </summary>
/// <param name="itemInfo">The item to add.</param>
/// <param name="slotIndex">The index of the slot that the item occupies.</param>
/// <param name="removePreviousItem">If the slot is at its capacity, should the last item be replaced by the current item?</param>
/// <returns>Returns the item info that was set.</returns>
private ItemInfo? SetItemAmount(ItemInfo itemInfo, int slotIndex, bool removePreviousItem)
{
    var itemSlot = m_ItemSlotSet.GetSlot(slotIndex);
    if (!itemSlot.HasValue) {
        return null;
    }
    var currentStack = m_ItemsBySlot[slotIndex];
    var amountToAdd = itemInfo.Amount;
    var amount = itemInfo.Amount;
    ItemInfo removedItem = ItemInfo.None;
    bool overflowRemovedItem = false;
    if (currentStack != null) {
        if (itemInfo.Item.StackableEquivalentTo(currentStack.Item)) {
            // The items are the same, lets merge them.
            amount += currentStack.Amount;
        } else if (removePreviousItem) {
            removedItem = RemoveItem((ItemInfo)currentStack);
            if (removedItem.Amount > 0) {
                if (m_TryGivePreviousItemToNewItemCollection &&
                    (itemInfo.ItemCollection?.CanAddItem(removedItem).HasValue ?? false)) {
                    itemInfo.ItemCollection.AddItem(removedItem);
                } else {
                    overflowRemovedItem = true;
                }
            }
        } else {
            return null;
        }
    }
    var itemToSet = (amount, itemInfo);
    var itemInfoAdded = AddInternal(itemToSet, currentStack, false);
    
    // Set back to the orginal added item info amount.
    itemInfoAdded = new ItemInfo(itemInfo.ItemAmount, itemInfoAdded);
    m_ItemsBySlot[slotIndex] = itemInfoAdded.ItemStack;
    NotifyAdd(itemInfo, itemInfoAdded.ItemStack);
    if (overflowRemovedItem) {
        HandleItemOverflow(removedItem, ref itemInfoAdded);
    }
    return itemInfoAdded;
}



/// <summary>
/// Handle Items that do not fit in the Inventory after they are added to an item collection.
/// </summary>
/// <param name="originalItemInfo">The original Item Info that was being added.</param>
/// <param name="itemInfoAdded">The item Info Added.</param>
protected virtual void HandleItemOverflow(ItemInfo originalItemInfo, ref ItemInfo itemInfoAdded)
{
    ItemInfo rejectedItemInfo = originalItemInfo;
    if (originalItemInfo.Item == itemInfoAdded.Item) {
        rejectedItemInfo = new ItemInfo(originalItemInfo.Amount - itemInfoAdded.Amount, originalItemInfo);
    }
    
    if (m_OverflowOptions.ReturnOverflow) {
        // When the item item overflows from items being added to the same slot, the item shouldn't be returned.
        // Checking that the itemcollection isn't this, ensures the collection isn't corrupted.
        if (originalItemInfo.ItemCollection != null && originalItemInfo.ItemCollection != this) {
            var returnedItemInfo = originalItemInfo.ItemCollection.AddItem(rejectedItemInfo);
            itemInfoAdded = (returnedItemInfo.Amount, itemInfoAdded);
        }
    }
    if (m_OverflowOptions.OverflowAction != null) {
        m_OverflowOptions.OverflowAction.HandleItemOverflow(m_Inventory, originalItemInfo, itemInfoAdded, rejectedItemInfo);
    }
    if (m_OverflowOptions.InvokeRejectedEvent) {
        //Part of the item that had to be added was rejected.
        EventHandler.ExecuteEvent<ItemInfo, ItemInfo, ItemInfo>(m_Inventory,
            EventNames.c_Inventory_OnAddItemRejected_ItemInfoToAdd_ItemInfoAdded_ItemInfoRejected,
            originalItemInfo,
            itemInfoAdded,
            rejectedItemInfo
        );
        
        EventHandler.ExecuteEvent<ItemInfo>(m_Inventory, EventNames.c_Inventory_OnRejected_ItemInfo,
            rejectedItemInfo);
        
        EventHandler.ExecuteEvent<ItemInfo, ItemInfo, ItemInfo>(m_Inventory,
            EventNames.c_Inventory_OnAddItemOverflow_ItemInfoToAdd_ItemInfoAdded_ItemInfoRejected,
            originalItemInfo,
            itemInfoAdded,
            rejectedItemInfo
        );
        
        OnItemAddOverflow?.Invoke(originalItemInfo, itemInfoAdded, rejectedItemInfo);
    }
    
    
}
 
Sangemdoko said:
I see what you mean.
I hadn't thought of it this way. The way I saw it was independent of the item added.
But what you say does make sense and gives you more data to work with.

So here are the changes I suggest,
So you first Remove the item, Add the new one and then at the end do the overflow
what do you think?

Code: 
/// <summary>
/// Sets the amount for the specified item.
/// </summary>
/// <param name="itemInfo">The item to add.</param>
/// <param name="slotIndex">The index of the slot that the item occupies.</param>
/// <param name="removePreviousItem">If the slot is at its capacity, should the last item be replaced by the current item?</param>
/// <returns>Returns the item info that was set.</returns>
private ItemInfo? SetItemAmount(ItemInfo itemInfo, int slotIndex, bool removePreviousItem)
{
    var itemSlot = m_ItemSlotSet.GetSlot(slotIndex);
    if (!itemSlot.HasValue) {
        return null;
    }
    var currentStack = m_ItemsBySlot[slotIndex];
    var amountToAdd = itemInfo.Amount;
    var amount = itemInfo.Amount;
    ItemInfo removedItem = ItemInfo.None;
    bool overflowRemovedItem = false;
    if (currentStack != null) {
        if (itemInfo.Item.StackableEquivalentTo(currentStack.Item)) {
            // The items are the same, lets merge them.
            amount += currentStack.Amount;
        } else if (removePreviousItem) {
            removedItem = RemoveItem((ItemInfo)currentStack);
            if (removedItem.Amount > 0) {
                if (m_TryGivePreviousItemToNewItemCollection &&
                    (itemInfo.ItemCollection?.CanAddItem(removedItem).HasValue ?? false)) {
                    itemInfo.ItemCollection.AddItem(removedItem);
                } else {
                    overflowRemovedItem = true;
                }
            }
        } else {
            return null;
        }
    }
    var itemToSet = (amount, itemInfo);
    var itemInfoAdded = AddInternal(itemToSet, currentStack, false);
   
    // Set back to the orginal added item info amount.
    itemInfoAdded = new ItemInfo(itemInfo.ItemAmount, itemInfoAdded);
    m_ItemsBySlot[slotIndex] = itemInfoAdded.ItemStack;
    NotifyAdd(itemInfo, itemInfoAdded.ItemStack);
    if (overflowRemovedItem) {
        HandleItemOverflow(removedItem, ref itemInfoAdded);
    }
    return itemInfoAdded;
}



/// <summary>
/// Handle Items that do not fit in the Inventory after they are added to an item collection.
/// </summary>
/// <param name="originalItemInfo">The original Item Info that was being added.</param>
/// <param name="itemInfoAdded">The item Info Added.</param>
protected virtual void HandleItemOverflow(ItemInfo originalItemInfo, ref ItemInfo itemInfoAdded)
{
    ItemInfo rejectedItemInfo = originalItemInfo;
    if (originalItemInfo.Item == itemInfoAdded.Item) {
        rejectedItemInfo = new ItemInfo(originalItemInfo.Amount - itemInfoAdded.Amount, originalItemInfo);
    }
   
    if (m_OverflowOptions.ReturnOverflow) {
        // When the item item overflows from items being added to the same slot, the item shouldn't be returned.
        // Checking that the itemcollection isn't this, ensures the collection isn't corrupted.
        if (originalItemInfo.ItemCollection != null && originalItemInfo.ItemCollection != this) {
            var returnedItemInfo = originalItemInfo.ItemCollection.AddItem(rejectedItemInfo);
            itemInfoAdded = (returnedItemInfo.Amount, itemInfoAdded);
        }
    }
    if (m_OverflowOptions.OverflowAction != null) {
        m_OverflowOptions.OverflowAction.HandleItemOverflow(m_Inventory, originalItemInfo, itemInfoAdded, rejectedItemInfo);
    }
    if (m_OverflowOptions.InvokeRejectedEvent) {
        //Part of the item that had to be added was rejected.
        EventHandler.ExecuteEvent<ItemInfo, ItemInfo, ItemInfo>(m_Inventory,
            EventNames.c_Inventory_OnAddItemRejected_ItemInfoToAdd_ItemInfoAdded_ItemInfoRejected,
            originalItemInfo,
            itemInfoAdded,
            rejectedItemInfo
        );
       
        EventHandler.ExecuteEvent<ItemInfo>(m_Inventory, EventNames.c_Inventory_OnRejected_ItemInfo,
            rejectedItemInfo);
       
        EventHandler.ExecuteEvent<ItemInfo, ItemInfo, ItemInfo>(m_Inventory,
            EventNames.c_Inventory_OnAddItemOverflow_ItemInfoToAdd_ItemInfoAdded_ItemInfoRejected,
            originalItemInfo,
            itemInfoAdded,
            rejectedItemInfo
        );
       
        OnItemAddOverflow?.Invoke(originalItemInfo, itemInfoAdded, rejectedItemInfo);
    }
   
   
}
Click to expand...

This works very well, thank you!!!
 
You must log in or register to reply here.
Back
Top