CottonMC / LibGui

Buttons & Co
MIT License
291 stars 49 forks source link

`IndexOutOfBoundsException` from `InventoryS2CPacket` when trying to open a HandledScreen from inside an entity #251

Closed falseresync closed 1 month ago

falseresync commented 1 month ago

I have an entity with an inventory, and I made it open a HandledScreen on interaction:

    @Override
    public ScreenHandler createMenu(int syncId, PlayerInventory playerInventory, PlayerEntity player) {
        return new AutomatonGui(syncId, playerInventory, inventory);
    }

    @Override
    protected ActionResult interactMob(PlayerEntity player, Hand hand) {
        //if (!player.getWorld().isClient) { // Tried it both ways :(
            player.openHandledScreen(this);
        //}
        return !player.getWorld().isClient ? ActionResult.CONSUME : ActionResult.SUCCESS;
    }

I have a barebones Gui from the example:

public class AutomatonGui extends SyncedGuiDescription {
    private BetterSimpleInventory inventory;

    public AutomatonGui(int syncId, PlayerInventory playerInventory) {
        super(VivatechScreenHandlers.AUTOMATON, syncId, playerInventory, new SimpleInventory(6), null);
    }

    public AutomatonGui(int syncId, PlayerInventory playerInventory, BetterSimpleInventory automatonInventory) {
        super(VivatechScreenHandlers.AUTOMATON, syncId, playerInventory, automatonInventory, null);
        inventory = automatonInventory;

        var root = new WGridPanel();
        setRootPanel(root);
        root.setSize(300, 200);
        root.setInsets(Insets.ROOT_PANEL);

        var itemSlot = WItemSlot.of(inventory, 0);
        root.add(itemSlot, 4, 1);

        root.add(this.createPlayerInventoryPanel(), 0, 3);

        root.validate(this);
    }
}
public class AutomatonScreen extends CottonInventoryScreen<AutomatonGui> {
    public AutomatonScreen(AutomatonGui description, PlayerInventory inventory, Text title) {
        super(description, inventory, title);
    }
}

Both the Gui and the Screen are registered properly

// @RegistryObject is for automatic registration in ModInitializer
    public static final @RegistryObject ScreenHandlerType<AutomatonGui> AUTOMATON =
            new ScreenHandlerType<>(AutomatonGui::new, FeatureFlags.VANILLA_FEATURES);
// Somewhere in ClientModInitializer:
HandledScreens.register(VivatechScreenHandlers.AUTOMATON, AutomatonScreen::new);

This results in an IndexOutOfBoundsException from InventoryS2CPacket handling. I have no clue what is wrong, given that vanilla screens work fine, like, for example GenericContainerScreenHandler.createGeneric9x1(syncId, playerInventory); or new HopperScreenHandler(syncId, playerInventory, inventory)

From the DEBUG log:

[16:57:38] [Server thread/DEBUG] (FabricLoader/Mixin) fabric-events-interaction-v0.mixins.json:ServerPlayNetworkHandlerMixin from mod fabric-events-interaction-v0: Class version 65 required is higher than the class version supported by the current version of Mixin (JAVA_17 supports class version 61)
[16:57:38] [Server thread/DEBUG] (FabricLoader/Mixin) fabric-events-interaction-v0.mixins.json:ServerPlayNetworkHandlerMixin from mod fabric-events-interaction-v0->@Inject::onPlayerInteractEntity(Lnet/minecraft/util/Hand;Lnet/minecraft/util/math/Vec3d;Lorg/spongepowered/asm/mixin/injection/callback/CallbackInfo;)V does use it's CallbackInfo
[16:57:38] [Server thread/DEBUG] (FabricLoader/Mixin) fabric-events-interaction-v0.mixins.json:ServerPlayNetworkHandlerMixin from mod fabric-events-interaction-v0->@Inject::onPlayerInteractEntity(Lnet/minecraft/util/Hand;Lorg/spongepowered/asm/mixin/injection/callback/CallbackInfo;)V does use it's CallbackInfo
[16:57:38] [Render thread/ERROR] (net.minecraft.client.network.ClientCommonNetworkHandler) Failed to handle packet net.minecraft.network.packet.s2c.play.InventoryS2CPacket@27dd7515
java.lang.IndexOutOfBoundsException: Index 0 out of bounds for length 0
    at java.base/jdk.internal.util.Preconditions.outOfBounds(Preconditions.java:100) ~[?:?]
    at java.base/jdk.internal.util.Preconditions.outOfBoundsCheckIndex(Preconditions.java:106) ~[?:?]
    at java.base/jdk.internal.util.Preconditions.checkIndex(Preconditions.java:302) ~[?:?]
    at java.base/java.util.Objects.checkIndex(Objects.java:385) ~[?:?]
    at java.base/java.util.ArrayList.get(ArrayList.java:427) ~[?:?]
    at net.minecraft.util.collection.DefaultedList.get(DefaultedList.java:43) ~[minecraft-merged-c458c7638f-1.21.1-net.fabricmc.yarn.1_21_1.1.21.1+build.3-v2.jar:?]
    at net.minecraft.screen.ScreenHandler.getSlot(ScreenHandler.java:502) ~[minecraft-merged-c458c7638f-1.21.1-net.fabricmc.yarn.1_21_1.1.21.1+build.3-v2.jar:?]
    at net.minecraft.screen.ScreenHandler.updateSlotStacks(ScreenHandler.java:856) ~[minecraft-merged-c458c7638f-1.21.1-net.fabricmc.yarn.1_21_1.1.21.1+build.3-v2.jar:?]
    at net.minecraft.client.network.ClientPlayNetworkHandler.onInventory(ClientPlayNetworkHandler.java:1265) ~[minecraft-merged-c458c7638f-1.21.1-net.fabricmc.yarn.1_21_1.1.21.1+build.3-v2.jar:?]
    at net.minecraft.network.packet.s2c.play.InventoryS2CPacket.apply(InventoryS2CPacket.java:59) ~[minecraft-merged-c458c7638f-1.21.1-net.fabricmc.yarn.1_21_1.1.21.1+build.3-v2.jar:?]
    at net.minecraft.network.packet.s2c.play.InventoryS2CPacket.apply(InventoryS2CPacket.java:28) ~[minecraft-merged-c458c7638f-1.21.1-net.fabricmc.yarn.1_21_1.1.21.1+build.3-v2.jar:?]
    at net.minecraft.network.NetworkThreadUtils.method_11072(NetworkThreadUtils.java:27) ~[minecraft-merged-c458c7638f-1.21.1-net.fabricmc.yarn.1_21_1.1.21.1+build.3-v2.jar:?]
    at net.minecraft.util.thread.ThreadExecutor.executeTask(ThreadExecutor.java:140) [minecraft-merged-c458c7638f-1.21.1-net.fabricmc.yarn.1_21_1.1.21.1+build.3-v2.jar:?]
    at net.minecraft.util.thread.ReentrantThreadExecutor.executeTask(ReentrantThreadExecutor.java:24) [minecraft-merged-c458c7638f-1.21.1-net.fabricmc.yarn.1_21_1.1.21.1+build.3-v2.jar:?]
    at net.minecraft.util.thread.ThreadExecutor.runTask(ThreadExecutor.java:114) [minecraft-merged-c458c7638f-1.21.1-net.fabricmc.yarn.1_21_1.1.21.1+build.3-v2.jar:?]
    at net.minecraft.util.thread.ThreadExecutor.runTasks(ThreadExecutor.java:103) [minecraft-merged-c458c7638f-1.21.1-net.fabricmc.yarn.1_21_1.1.21.1+build.3-v2.jar:?]
    at net.minecraft.client.MinecraftClient.render(MinecraftClient.java:1233) [minecraft-merged-c458c7638f-1.21.1-net.fabricmc.yarn.1_21_1.1.21.1+build.3-v2.jar:?]
    at net.minecraft.client.MinecraftClient.run(MinecraftClient.java:885) [minecraft-merged-c458c7638f-1.21.1-net.fabricmc.yarn.1_21_1.1.21.1+build.3-v2.jar:?]
    at net.minecraft.client.main.Main.main(Main.java:228) [minecraft-merged-c458c7638f-1.21.1-net.fabricmc.yarn.1_21_1.1.21.1+build.3-v2.jar:?]
    at net.fabricmc.loader.impl.game.minecraft.MinecraftGameProvider.launch(MinecraftGameProvider.java:480) [fabric-loader-0.16.5.jar:?]
    at net.fabricmc.loader.impl.launch.knot.Knot.launch(Knot.java:74) [fabric-loader-0.16.5.jar:?]
    at net.fabricmc.loader.impl.launch.knot.KnotClient.main(KnotClient.java:23) [fabric-loader-0.16.5.jar:?]
    at net.fabricmc.devlaunchinjector.Main.main(Main.java:86) [dev-launch-injector-0.2.1+build.8.jar:?]
[16:57:38] [Server thread/INFO] (net.minecraft.server.network.ServerPlayNetworkHandler) falseresync lost connection: Verbindung getrennt
[16:57:38] [Server thread/INFO] (net.minecraft.server.MinecraftServer) falseresync hat das Spiel verlassen
[16:57:38] [Server thread/INFO] (net.minecraft.server.network.ServerCommonNetworkHandler) Stopping singleplayer server as player logged out
falseresync commented 1 month ago

I assume this has something to do with the validation on slots that LibGui does, but I don't have any idea what to even search for

Juuxel commented 1 month ago

Not a bug. The vanilla Slot instances that hold the stacks on the client are created by the WItemSlot widget, but the clientside constructor in AutomatonGui doesn't add any widgets to the gui.

You probably want to make the server-side ctor a "primary" constructor and just call this() in the client-side one.

falseresync commented 1 month ago

Thanks! It toootally didn't sneak past me that all of the ScreenHandlers call this() in these scenarios xD