URGENT: shift-clicking any stack in a custom Container causes the game to freeze without crashing
LemmaEOF opened this issue ยท 4 comments
Container#transferSlot
and Container#insertItem
both use while(true)
loops, and one or both of them cause infinite loops in custom containers if any item is shift-clicked. This doesn't leave a stacktrace, and I haven't opened a profiler yet to inspect where things go wrong, but we may need to make some changes to either of those methods. This could come in the form of avoiding the loops entirely, or by adding extra breakpoints that modded containers are more likely to run into. In either case, this feels like something that Fabric API should absolutely deal with to make custom containers feasible.
It doesn't help that (in my decompile) transferSlot
is 277 lines, and insertItem
is 123. The while(true)
loops are in the middle of each, too, so we can't just inject, fix the loop, and cancel, and overwriting is a bad idea too.
I've fixed this crash/hang on my end by overriding Container#transferSlot with a copy-pasted implementation from HopperContainer. This one works for all the inventories I have registered (with the exception of armor slots, but thats something i have to do).
@Override
public ItemStack transferSlot(PlayerEntity playerEntity_1, int int_1) {
ItemStack itemStack_1 = ItemStack.EMPTY;
Slot slot_1 = this.slotList.get(int_1);
if (slot_1 != null && slot_1.hasStack()) {
ItemStack itemStack_2 = slot_1.getStack();
itemStack_1 = itemStack_2.copy();
if (int_1 < this.inventory.getInvSize()) {
if (!this.insertItem(itemStack_2, this.inventory.getInvSize(), this.slotList.size(), true)) {
return ItemStack.EMPTY;
}
} else if (!this.insertItem(itemStack_2, 0, this.inventory.getInvSize(), false)) {
return ItemStack.EMPTY;
}
if (itemStack_2.isEmpty()) {
slot_1.setStack(ItemStack.EMPTY);
} else {
slot_1.markDirty();
}
}
return itemStack_1;
}
However, if theres anything fabric can do to have this work OOTB, that'd be great.
We've talked in the Discord about blocks extending GenericContainer
, but I feel that it might be a nice idea to create a FabricContainer
so we already have it there to add hooks in the future if needed.
Almost all containers override the transferSlot
method, because you need to implement the logic of "from which slots to which slots you can shift-click" there, and that is quite specific to each container. I don't deny there is some common logic that could be abstracted, if you make some assumptions about the container (I have my own generic container class).
And the insertItem
should not get stuck in the infinite loop if your slots don't have any strange logic and work as expected by the minecraft code. Do you have any custom slots?