From 879f17566d410640544786e5d75427c154b0d40d Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 10 Aug 2026 15:59:44 +0000 Subject: [PATCH] Fix Menu click handling using wrong slot offset on page 2+ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Menu.open(int page) renders display slot i from virtual slot (page * inventory.getSize() + i), but MenuManager.onInventoryClick resolved clicks via Menu.getSlot(event.getSlot()) — the raw display slot with no page offset applied. On page 1 the two happened to coincide, but on page 2+ a click was routed to whatever Slot was registered at that same grid position on page 1, silently invoking the wrong handler instead of just failing to respond. Add Menu.getSlotOnCurrentPage(int), backed by a shared Menu.toVirtualSlot(page, pageSize, displaySlot) helper, and make both the renderer (open) and MenuManager's click handler go through it so they can't drift apart again. Nav buttons (getPrevButton/ getNextButton, PreviousPageButton/NextPageButton) were unaffected since they call menu.open(page ± 1) regardless of which Slot instance triggered them, which is why this presented as "content doesn't react" rather than a routing bug. Grepped the repo for other Menu.getSlot( callers and other raw-slot resolution paths (e.g. inventory drag handling): none exist. The other getSlot(...) call sites (YAMLMenu/YAMLListMenu) resolve a different overload, getSlot(String, T, Player), and are unrelated. Fixes #156 --- .../codex/manager/api/menu/Menu.java | 25 +++++++-- .../codex/manager/api/menu/MenuManager.java | 2 +- .../codex/manager/api/menu/MenuTest.java | 52 +++++++++++++++++++ 3 files changed, 75 insertions(+), 4 deletions(-) create mode 100644 codex-api/src/test/java/studio/magemonkey/codex/manager/api/menu/MenuTest.java diff --git a/codex-api/src/main/java/studio/magemonkey/codex/manager/api/menu/Menu.java b/codex-api/src/main/java/studio/magemonkey/codex/manager/api/menu/Menu.java index d9a94a89..6baed2ff 100644 --- a/codex-api/src/main/java/studio/magemonkey/codex/manager/api/menu/Menu.java +++ b/codex-api/src/main/java/studio/magemonkey/codex/manager/api/menu/Menu.java @@ -84,6 +84,26 @@ public Slot getSlot(int i) { return slots.get(i); } + /** + * Resolves a raw display slot (0..inventory size, as seen by the player/Bukkit) + * to the {@link Slot} bound to it on the currently open page. This applies the + * same page offset that {@link #open(int)} uses to render the page, so click + * handling and rendering can't drift out of sync. + */ + @Nullable + public Slot getSlotOnCurrentPage(int displaySlot) { + return slots.get(toVirtualSlot(this.page, this.inventory.getSize(), displaySlot)); + } + + /** + * Maps a page-relative display slot to its virtual index in {@link #slots}. + * Shared by {@link #open(int)} (render) and {@link #getSlotOnCurrentPage(int)} + * (click) so the two can't disagree about what a display slot means. + */ + static int toVirtualSlot(int page, int pageSize, int displaySlot) { + return page * pageSize + displaySlot; + } + public void openSync() { new BukkitRunnable() { @Override @@ -106,8 +126,9 @@ public void open(int page) { inventory = Bukkit.createInventory(Menu.this, rows * 9, title .replace("%page%", String.valueOf(finalPage + 1)) .replace("%pages%", String.valueOf(getPages()))); + Menu.this.page = finalPage; for (int i = 0, last = Menu.this.inventory.getSize(); i < last; i++) { - Slot slot = slots.get(finalPage * Menu.this.inventory.getSize() + i); + Slot slot = getSlotOnCurrentPage(i); if (slot != null) { inventory.setItem(i, slot.getItemStack()); } @@ -115,8 +136,6 @@ public void open(int page) { Menu.this.opening = true; player.openInventory(inventory); Menu.this.opening = false; - Menu.this.page = finalPage; - } public void openSubMenu(Menu menu) { diff --git a/codex-api/src/main/java/studio/magemonkey/codex/manager/api/menu/MenuManager.java b/codex-api/src/main/java/studio/magemonkey/codex/manager/api/menu/MenuManager.java index 99d29ce4..a2101227 100644 --- a/codex-api/src/main/java/studio/magemonkey/codex/manager/api/menu/MenuManager.java +++ b/codex-api/src/main/java/studio/magemonkey/codex/manager/api/menu/MenuManager.java @@ -50,7 +50,7 @@ public void onInventoryClick(InventoryClickEvent event) { InventoryHolder holder = inventory.getHolder(); if (holder instanceof Menu) { event.setCancelled(true); - Slot slot = ((Menu) holder).getSlot(event.getSlot()); + Slot slot = ((Menu) holder).getSlotOnCurrentPage(event.getSlot()); if (slot != null) { switch (event.getClick()) { case LEFT -> slot.onLeftClick(); diff --git a/codex-api/src/test/java/studio/magemonkey/codex/manager/api/menu/MenuTest.java b/codex-api/src/test/java/studio/magemonkey/codex/manager/api/menu/MenuTest.java new file mode 100644 index 00000000..a658bf7f --- /dev/null +++ b/codex-api/src/test/java/studio/magemonkey/codex/manager/api/menu/MenuTest.java @@ -0,0 +1,52 @@ +package studio.magemonkey.codex.manager.api.menu; + +import org.junit.jupiter.api.Test; + +/** + * Covers the page-offset math shared by {@link Menu#open(int)} (render) and + * {@link Menu#getSlotOnCurrentPage(int)} (click resolution). Regression test for + * https://github.com/magemonkeystudio/codex/issues/156, where render and click + * disagreed about what a display slot meant on page 2+. + */ +class MenuTest { + + @Test + void toVirtualSlot_firstPageIsUnshifted() { + assert Menu.toVirtualSlot(0, 54, 0) == 0; + assert Menu.toVirtualSlot(0, 54, 5) == 5; + assert Menu.toVirtualSlot(0, 54, 53) == 53; + } + + @Test + void toVirtualSlot_laterPagesAreShiftedByPageSize() { + assert Menu.toVirtualSlot(1, 54, 0) == 54; + assert Menu.toVirtualSlot(1, 54, 5) == 59; + assert Menu.toVirtualSlot(2, 54, 5) == 113; + } + + @Test + void toVirtualSlot_matchesAcrossRenderAndClickForSameDisplaySlot() { + // The exact scenario from #156: a display slot on page 2 must resolve to the + // same virtual slot whether it's being rendered (open) or clicked (MenuManager). + int pageSize = 9; + int displaySlot = 3; + int page = 2; + + int renderVirtualSlot = Menu.toVirtualSlot(page, pageSize, displaySlot); + int clickVirtualSlot = Menu.toVirtualSlot(page, pageSize, displaySlot); + + assert renderVirtualSlot == clickVirtualSlot; + assert renderVirtualSlot == 21; + } + + @Test + void toVirtualSlot_differentPagesDoNotCollideForSameDisplaySlot() { + int pageSize = 9; + int displaySlot = 3; + + int page1Slot = Menu.toVirtualSlot(0, pageSize, displaySlot); + int page2Slot = Menu.toVirtualSlot(1, pageSize, displaySlot); + + assert page1Slot != page2Slot; + } +}