From 5431ad53ebde930e16700da37886c72b6323b824 Mon Sep 17 00:00:00 2001 From: Face o' Cat Date: Mon, 2 Nov 2026 20:51:43 +0200 Subject: [PATCH] fix(gui): predict slot rules on the client's machine menu The client's menu backed its slots with a blank ItemStackHandler, which accepts anything up to a full stack. So it predicted placing items in output slots, past slot limits and against filters, until the server corrected it. ClientMenuItems keeps the same contents, filled by vanilla's menu sync, but takes isItemValid and getSlotLimit from the client block entity's own screenItems(), which has the same roles, filters and limits on any ScreenHost. Its contents aren't read or changed. The server still decides, so a filter that depends on server-only state can still mispredict. screenItems() must now work on the client, which ScreenHost and the docs now say. The default screenDestinations already relied on it. The test machine gets an output slot, a filter and a limit, and the GUI smoke test checks them through a real client menu. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_018zpP6ZrFjaGe477FvWAZKo --- docs/gui-screens.md | 5 ++ .../mc/lib/gui/runtime/ClientMenuItems.java | 36 ++++++++++++++ .../mc/lib/gui/runtime/MachineMenu.java | 9 ++-- .../moddev/mc/lib/gui/runtime/ScreenHost.java | 3 +- .../mc/lib/smoke/ClientMenuItemsSmoke.java | 48 +++++++++++++++++++ .../moddev/mc/lib/smoke/GuiClientSmoke.java | 18 +++++++ .../moddev/mc/lib/smoke/MMDLibGameTests.java | 6 +++ .../moddev/mc/lib/smoke/MachineSmoke.java | 5 +- 8 files changed, 124 insertions(+), 6 deletions(-) create mode 100644 src/main/java/zone/moddev/mc/lib/gui/runtime/ClientMenuItems.java create mode 100644 src/runtimeTest/java/zone/moddev/mc/lib/smoke/ClientMenuItemsSmoke.java diff --git a/docs/gui-screens.md b/docs/gui-screens.md index cf989d7..e152a6f 100644 --- a/docs/gui-screens.md +++ b/docs/gui-screens.md @@ -57,6 +57,11 @@ stacks to one item. Sparse machine indexes remain reserved and inactive. } ``` +`screenItems()` must also work on the client. The client's menu keeps its own copy of the slot +contents, and asks the client block entity's `screenItems()` what each slot accepts and how much it +holds, so a click is predicted the way the server will answer it. It's only asked about rules, so its +contents don't need to be synced. + `screenDestinations` controls shift-click routing into the machine; its default tries all storage indexes, subject to their filters. `screenRecipeSlots` explicitly names recipe inputs in recipe order; its default disables JEI transfer. Both methods must work on the client as well as the diff --git a/src/main/java/zone/moddev/mc/lib/gui/runtime/ClientMenuItems.java b/src/main/java/zone/moddev/mc/lib/gui/runtime/ClientMenuItems.java new file mode 100644 index 0000000..5a9a207 --- /dev/null +++ b/src/main/java/zone/moddev/mc/lib/gui/runtime/ClientMenuItems.java @@ -0,0 +1,36 @@ +package zone.moddev.mc.lib.gui.runtime; + +import net.minecraft.world.item.ItemStack; + +import net.neoforged.neoforge.items.IItemHandler; +import net.neoforged.neoforge.items.ItemStackHandler; + +/** + * The client's copy of the slots behind a {@link MachineMenu}. Vanilla's menu sync fills it; what + * each slot accepts and how much it holds come from the client block entity's own {@link + * ScreenHost#screenItems()}, so a click is predicted the way the server will answer it. The server + * still decides: a filter that depends on server-only state can mispredict, and the server's answer + * replaces the prediction. + */ +public final class ClientMenuItems extends ItemStackHandler { + private final IItemHandler rules; + + /** + * @param size the menu's slot count, which ghost slots can take past the end of {@code rules} + * @param rules asked what each slot accepts; its contents aren't read or changed + */ + public ClientMenuItems(int size, IItemHandler rules) { + super(size); + this.rules = rules; + } + + @Override + public boolean isItemValid(int slot, ItemStack stack) { + return slot < rules.getSlots() && rules.isItemValid(slot, stack); + } + + @Override + public int getSlotLimit(int slot) { + return slot < rules.getSlots() ? rules.getSlotLimit(slot) : 0; + } +} diff --git a/src/main/java/zone/moddev/mc/lib/gui/runtime/MachineMenu.java b/src/main/java/zone/moddev/mc/lib/gui/runtime/MachineMenu.java index d27cc28..bf8692f 100644 --- a/src/main/java/zone/moddev/mc/lib/gui/runtime/MachineMenu.java +++ b/src/main/java/zone/moddev/mc/lib/gui/runtime/MachineMenu.java @@ -76,10 +76,11 @@ public MachineMenu(int id, Inventory inventory, BlockEntity entity) { .mapToInt(e -> ((Element.Slot) e).index() + 1) .max() .orElse(0); - IItemHandler items = - owner.level().isClientSide - ? new ItemStackHandler(count) - : host == null ? new ItemStackHandler(0) : host.screenItems(); + IItemHandler items; + if (owner.level().isClientSide) + items = + host == null ? new ItemStackHandler(count) : new ClientMenuItems(count, host.screenItems()); + else items = host == null ? new ItemStackHandler(0) : host.screenItems(); machineSlots = count; var layout = ScreenLayout.of( diff --git a/src/main/java/zone/moddev/mc/lib/gui/runtime/ScreenHost.java b/src/main/java/zone/moddev/mc/lib/gui/runtime/ScreenHost.java index b50087a..3dac75b 100644 --- a/src/main/java/zone/moddev/mc/lib/gui/runtime/ScreenHost.java +++ b/src/main/java/zone/moddev/mc/lib/gui/runtime/ScreenHost.java @@ -13,7 +13,8 @@ public interface ScreenHost { /** * The inventory behind the screen's {@code } elements, numbered as in the XML; - * returning an {@code ItemStorage} keeps its output restrictions and filters for players. + * returning an {@code ItemStorage} keeps its output restrictions and filters for players. It must + * work on the client too, where the menu asks it what each slot accepts to predict clicks. */ IItemHandlerModifiable screenItems(); diff --git a/src/runtimeTest/java/zone/moddev/mc/lib/smoke/ClientMenuItemsSmoke.java b/src/runtimeTest/java/zone/moddev/mc/lib/smoke/ClientMenuItemsSmoke.java new file mode 100644 index 0000000..98639ac --- /dev/null +++ b/src/runtimeTest/java/zone/moddev/mc/lib/smoke/ClientMenuItemsSmoke.java @@ -0,0 +1,48 @@ +package zone.moddev.mc.lib.smoke; + +import net.minecraft.world.item.ItemStack; +import net.minecraft.world.item.Items; + +import zone.moddev.mc.lib.api.capabilities.ItemStorage; +import zone.moddev.mc.lib.api.capabilities.Role; +import zone.moddev.mc.lib.gui.runtime.ClientMenuItems; + +/** + * Checks the client's slot handler against a real storage's rules. The GUI smoke test checks it + * behind a real client menu. + */ +final class ClientMenuItemsSmoke { + private ClientMenuItemsSmoke() {} + + static void check() { + var changes = new int[1]; + var rules = + new ItemStorage(2) + .onChange(() -> changes[0]++) + .role(Role.OUTPUT, 1) + .filter(stack -> !stack.is(Items.DIRT), 0) + .limit(16, 0); + var client = new ClientMenuItems(3, rules); + var iron = new ItemStack(Items.IRON_INGOT); + + require(client.isItemValid(0, iron), "the filter accepts iron"); + require(!client.isItemValid(0, new ItemStack(Items.DIRT)), "the filter refuses dirt"); + require(!client.isItemValid(1, iron), "an output slot refuses players"); + require(client.getSlotLimit(0) == 16, "the slot limit, got " + client.getSlotLimit(0)); + require(client.getSlotLimit(1) == rules.getSlotLimit(1), "an unlimited slot"); + + // Past the storage's end, as ghost slots can be: refused, not thrown. + require(!client.isItemValid(2, iron), "no rules for a slot past the storage"); + require(client.getSlotLimit(2) == 0, "no room in a slot past the storage"); + + // Contents are the menu's: the server may put anything anywhere, and rules stay untouched. + client.setStackInSlot(1, new ItemStack(Items.GOLD_INGOT, 3)); + require(client.getStackInSlot(1).is(Items.GOLD_INGOT), "sync fills an output slot"); + require(rules.getStackInSlot(1).isEmpty(), "the rules' storage is left alone"); + require(changes[0] == 0, "the rules' storage saw no change"); + } + + private static void require(boolean condition, String what) { + if (!condition) throw new AssertionError("Client menu items: " + what); + } +} diff --git a/src/runtimeTest/java/zone/moddev/mc/lib/smoke/GuiClientSmoke.java b/src/runtimeTest/java/zone/moddev/mc/lib/smoke/GuiClientSmoke.java index d78e549..240c44c 100644 --- a/src/runtimeTest/java/zone/moddev/mc/lib/smoke/GuiClientSmoke.java +++ b/src/runtimeTest/java/zone/moddev/mc/lib/smoke/GuiClientSmoke.java @@ -174,6 +174,15 @@ public static void frame(RenderFrameEvent.Post event) { "MMDLIB_GUI_JEI_SERVER_PASS: recipe ingredient moved to input and ghost filter synced" + " from server"); } + var input = screen.getMenu().slots.get(0); + var ingot = new net.minecraft.world.item.ItemStack(net.minecraft.world.item.Items.IRON_INGOT); + if (input.getMaxStackSize(ingot) != 16 + || !input.mayPlace(ingot) + || input.mayPlace( + new net.minecraft.world.item.ItemStack(net.minecraft.world.item.Items.DIRT))) { + fail(new AssertionError("Client menu ignored the input slot's limit or filter")); + return; + } GuiGaugeSmoke.check(screen); Screenshot.grab( mc.gameDirectory, @@ -193,6 +202,15 @@ public static void frame(RenderFrameEvent.Post event) { fail(new AssertionError("Rendered context visibility incorrect")); return; } + if (screen + .getMenu() + .slots + .get(1) + .mayPlace( + new net.minecraft.world.item.ItemStack(net.minecraft.world.item.Items.IRON_INGOT))) { + fail(new AssertionError("Client menu predicted placing an item in an output slot")); + return; + } var config = screen.getMenu().bound.screen().find("config").orElseThrow(); var r = screen.bounds(config); screen.mouseClicked(r.x() + 32, r.y() + 52, 0); diff --git a/src/runtimeTest/java/zone/moddev/mc/lib/smoke/MMDLibGameTests.java b/src/runtimeTest/java/zone/moddev/mc/lib/smoke/MMDLibGameTests.java index 3349b0d..84acd33 100644 --- a/src/runtimeTest/java/zone/moddev/mc/lib/smoke/MMDLibGameTests.java +++ b/src/runtimeTest/java/zone/moddev/mc/lib/smoke/MMDLibGameTests.java @@ -70,6 +70,12 @@ public static void standaloneMachine(GameTestHelper helper) { helper.succeed(); } + @GameTest(template = EMPTY) + public static void clientMenuItems(GameTestHelper helper) { + ClientMenuItemsSmoke.check(); + helper.succeed(); + } + @GameTest(template = EMPTY) public static void machinePartsLoad(GameTestHelper helper) { MachinePartsLoaderSmoke.check(); diff --git a/src/runtimeTest/java/zone/moddev/mc/lib/smoke/MachineSmoke.java b/src/runtimeTest/java/zone/moddev/mc/lib/smoke/MachineSmoke.java index 65c7214..24251c7 100644 --- a/src/runtimeTest/java/zone/moddev/mc/lib/smoke/MachineSmoke.java +++ b/src/runtimeTest/java/zone/moddev/mc/lib/smoke/MachineSmoke.java @@ -134,7 +134,10 @@ public static final class Counter extends BlockEntity implements zone.moddev.mc.lib.gui.runtime.ScreenHost { public int clicks, guiReads; public final zone.moddev.mc.lib.api.capabilities.ItemStorage items = - new zone.moddev.mc.lib.api.capabilities.ItemStorage(4); + new zone.moddev.mc.lib.api.capabilities.ItemStorage(4) + .role(zone.moddev.mc.lib.api.capabilities.Role.OUTPUT, 1) + .filter(stack -> !stack.is(net.minecraft.world.item.Items.DIRT), 0) + .limit(16, 0); public final java.util.Map sides = java.util.Map.of( "input",