[Scripting] Removing an single inventory item via runtimeInventory.Remove removes all of them

NeatWolf

Hi there,

there's something I can't really explain, since it feels very weird that this went unnoticed for so much time.

I even debugged it and there isn't anything else acting on the inventory.

basically, I'm using the unhandled inventory interaction to drop a single vase (inventory item) on a pedestal (a socket/monobehaviour storing just a single inventory ID, and some way to show the appropriate item put on it).

The unhandled inventory interaction sends a unique message "DroppedInventoryID" to the component, and it processes if the socket can accept it, and whether to just drop the inventory item on the pedestal, or swap it with what's already on the pedestal.

The code is pretty straightforward:

so just a single item should be removed from the inventory (the vases are stackable).

Yet, on the console, I read this:

Which means that before calling Remove, the player has 2 items of that type. After removing one, it has 0 of them.

The "OnPlaceAction.Execute(this);" has been added later, the issue was already there. I can remove that line and the behaviour stays the same

The inventory UI reflects this situation correctly: The item completely disappears from the player inventory, regardless of having 2 at first.

Could you please check it out?
Am I doing something wrong?

Those are the only messages on the console, so it's not happening twice.

It makes no sense to me, so I must have run into some corner case scenario.

EDIT:
Trying to Step into the code via debugger.
Parameters get correctly passed to the function (in green)

Correctly skips the "other players" branch:

This is the item that's about to be affected:

NeatWolf

Oh.

I read the "setAmount" optional parameter meaning.

so, both writing

KickStarter.runtimeInventory.Remove(invItemID);

or

KickStarter.runtimeInventory.Remove(invItemID, 1);

will result in all invItemID to be removed.

isn't it a bit counter-intuitive?

To remove a single item I have to write:

KickStarter.runtimeInventory.Remove(invItemID, 1, true);

that's... a kind of unusual and verbose syntax.

Isn't there any chance to have a RemoveAll method and a Remove(id, amount) one?

That would break some code but having to specify either 0 or 2 optional parameters makes little sense to me.

I mean, is there any reason a person should write:

KickStarter.runtimeInventory.Remove(invItemID, 5); ?

that number makes no sense unless you also add the third parameter.

What about passing setting the default value of the setAmount to true, and have a different, shortest method that calls SetAmount(id, anyAmount, true) called RemoveAll?

I thought it was a bug.

Or, am I missing something?

It feels it's like that by design or a leftover from previous implementations (left like that to avoid breaking code), to avoid newbie coders to "forget" to remove all the items, so it's a shortcut to make sure every item gets removed, but still... I never found a method that needs 3 parameters to remove a single item from a list, and the second one is completely useless unless you also specify a third one, which in my opinion shouldn't be there at all if not with the opposite default value.

That was... very confusing to me, I triple checked everything and... well, I will never forget about that syntax anymore, that's for sure

Apologies for the extra long post, I was really convinced something weird was going on.

ChrisIceBox

Fair point, will consider.