Crash when move Shulker while it is opened
CristianH96 opened this issue ยท 3 comments
If you open a shulkerbox from the inventory with right click and, while the shulkerbox is open, if you try to move it through the inventory, the game (and the server) crashes.
1.17.1? It shouldn't crash the client though, it just disconnects. Immediately after you left/right click the opened shulkerbox. Also occurs if you try to drop the shulker box with (default) Q. Can you confirm?
In 1.17.1, the constructor for ScreenHandlerSlotUpdateS2CPacket
got updated, but the mod still uses the old parameters and that leads to the crash.
This issue seems to come from ContainerMixin (link to file)
Specifically, the onSlotClick
method mixin. I've included the method definition below, and added comments describing a potential fix:
@Inject(method = "onSlotClick", at = @At("HEAD"), cancellable = true)
public void QS$onClick(int slotId, int button, SlotActionType actionType, PlayerEntity player, CallbackInfo ci) {
if (slotId > 0 && slotId < slots.size()) {
Slot slot = this.slots.get(slotId);
if (slot != null && slot.inventory instanceof PlayerInventory)
if (hasItem()) {
if (Util.areItemsEqual(slot.getStack(), getOpenedItem())) {
ci.cancel();
// The code below seems to cause the crash (on 1.17.1)
// I tried just removing the whole if block
// And it looks like just calling ci.cancel() is enough for the client to get the slot update, at least from the little bit of testing I've done
// I had no issues with the shulker box disappearing when clicked on. It acts as if I'm supposed to be unable to click on the slot with the opened shulker box, matching the behavior in 1.17.
// Additional testing is probably required though, as there has to be a reason for this block of code to exist
if (player instanceof ServerPlayerEntity) {
ServerPlayerEntity sPlayer = (ServerPlayerEntity) player;
sPlayer.networkHandler.sendPacket(new ScreenHandlerSlotUpdateS2CPacket(syncId, slotId, slot.getStack()));
}
}
}
}
}
I've also attached a JAR file with the potential fix. It also includes a potential fix for #26, I did that by commenting out the if condition in the setOpenedItem function (in the same file)
@Unique
@Override
public void setOpenedItem(ItemStack openedItem) {
// Commented out the if statement below, the this.openededStack = openedItem statement is intact.
// Again, there must be a reason for this line of code to exist, but from the limited testing I did, I was unable to reproduce the issue with the ender chest disappearing when clicked on.
if (!Util.isEnderChest(openedItem))
this.openededStack = openedItem;
}
I hope distributing the JAR file here is fine.
(Zipped because uploading the JAR directly seems to be unsupported)
quickshulker-1.17-issue-26-27-potential-fix.zip
Thank you for the testing and providing fixes!
I do not recall the exact use case for ignoring ender chests when setting the opened item, but it appears to not be necessarily anymore. Seems to function just fine without, and does fix #26.
As for the first issue, I believe this was a fix for vanilla clients(Yes fully vanilla clients can connect and use some of the features!). But also does appear to be obsolete. I haven't noticed any issues with it removed either.