I just spent hours tracking down a weird bug in a custom Container and eventually realised it was to do with retrySlotClick. (I have a set of slots which automatically sort themselves when items are added or removed, and shift-clicking resulted in the whole set of slots being transferred if the items were the same, because retrySlotClick was called after the sorting). I've experimentally overridden retrySlotClick to do nothing, and it seems to have fixed the problem. So what is the point of that method in the first place? It seems to be only called in one place in vanilla and I don't really understand why. Except for an unusual modded case (like my self-sorting Container), when will a slot click fail the first time but succeed the second time? Doesn't the method just introduce a big risk of stack overflow if the slot click fails repeatedly?

Edited February 16, 20178 yr by Jay Avery

Inside Container#slotClick, if it's called with ClickType.QUICK_MOVE (shift-click), it calls Container#transferStackIntoSlot. This will try to move the ItemStack to other stacks in the inventory. If the stack after the move isn't empty, it retries to combine the ItemStack into other stacks by calling Container#retrySlotClick, which in turn calls Container#slotClick with the ClickType always being ClickType.QUICK_MOVE until the clicked Slot is empty.

 

It is basically a "brute-force" way of shift-clicking an ItemStack to other stacks the inventory.

Edited February 16, 20178 yr by larsgerrits

18 minutes ago, larsgerrits said:

Inside Container#slotClick, if it's called with ClickType.QUICK_MOVE (shift-click), it calls Container#transferStackIntoSlot. This will try to move the ItemStack to other stacks in the inventory. If the stack after the move isn't empty, it retries to combine the ItemStack into other stacks by calling Container#retrySlotClick, which in turn calls Container#slotClick with the ClickType always being ClickType.QUICK_MOVE until the clicked Slot is empty.

 

It is basically a "brute-force" way of shift-clicking an ItemStack to other stacks the inventory.

Hmm, I see. It seems strange because mergeItemStack (as called by most transferStackInSlot implementations) has a loop in itself, and keeps trying until the stack is empty.

 

Is it probably safe to override it to do nothing, as long as I'm aware of that? It seems that the worst possible outcome is that a stack might sometimes not fully merge when shift-clicked, if it needs to spread between multiple slots in order to transfer - unless I'm missing some other worse implication/s?

Edited February 16, 20178 yr by Jay Avery
formatting

I'm pretty sure it's safe to override it to do nothing, as I can't see any other issues which may be caused by overriding it.