From 49a2fe4e2bf0b5fa4f4973a26734a6639e90a96d Mon Sep 17 00:00:00 2001 From: tastybento Date: Fri, 9 Oct 2026 17:21:03 -0700 Subject: [PATCH] Let players use their own inventory while a panel is open #2767 tried this by only cancelling clicks in the panel itself, but that stopped icon pickers seeing clicks in the player's inventory (#2776) and let shift-clicks and drags move items into panels, where they were lost. It was reverted in 3.11.1. This version keeps routing player-inventory clicks to the panel and only lets them through when: - the action cannot move an item into the panel (pickup, place, swap, hotbar swap, drop, bundles; not shift-click or double-click), and - the panel has no listener, or its listener opts in via the new PanelListener#allowsPlayerInventoryClicks() (default false, so addon icon pickers are unaffected). TabbedPanel and AbstractRefreshingPanel opt in. A held item can never be placed in a panel, even if a click handler uncancels the click, and drags that touch panel slots are cancelled. Co-Authored-By: Claude Opus 5.5 --- .../bentobox/api/panels/PanelListener.java | 15 +++ .../bentobox/api/panels/TabbedPanel.java | 8 ++ .../listeners/PanelListenerManager.java | 60 +++++++++- .../customizable/AbstractRefreshingPanel.java | 8 ++ .../listeners/PanelListenerManagerTest.java | 109 ++++++++++++++++++ 5 files changed, 199 insertions(+), 1 deletion(-) diff --git a/src/main/java/world/bentobox/bentobox/api/panels/PanelListener.java b/src/main/java/world/bentobox/bentobox/api/panels/PanelListener.java index f1f898237..9d19744b3 100644 --- a/src/main/java/world/bentobox/bentobox/api/panels/PanelListener.java +++ b/src/main/java/world/bentobox/bentobox/api/panels/PanelListener.java @@ -53,4 +53,19 @@ default boolean hasClickCooldown() { default boolean isActionableSlot(int rawSlot) { return true; } + + /** + * Whether the player may use their own inventory while this panel is open: pick up, place, + * swap and drop items, and use bundles. When {@code true}, such clicks are left to vanilla and + * are not passed to {@link #onInventoryClick(User, InventoryClickEvent)}. Clicks that + * could move an item into the panel, such as shift-clicks, are always blocked. + *

+ * Defaults to {@code false} because some panels read clicks in the player's inventory, for + * example to let them pick an icon. Panels without a listener always allow it. + * @return {@code true} to let the player use their own inventory while the panel is open + * @since 3.23.4 + */ + default boolean allowsPlayerInventoryClicks() { + return false; + } } diff --git a/src/main/java/world/bentobox/bentobox/api/panels/TabbedPanel.java b/src/main/java/world/bentobox/bentobox/api/panels/TabbedPanel.java index a8e3ad98b..23b22ce9f 100644 --- a/src/main/java/world/bentobox/bentobox/api/panels/TabbedPanel.java +++ b/src/main/java/world/bentobox/bentobox/api/panels/TabbedPanel.java @@ -207,6 +207,14 @@ public void onInventoryClose(InventoryCloseEvent event) { } } + /** + * Tabs and flag buttons are all in the panel, so the player may use their own inventory. + */ + @Override + public boolean allowsPlayerInventoryClicks() { + return true; + } + @Override public void onInventoryClick(User user, InventoryClickEvent event) { // Note: click cooldown is now applied in PanelListenerManager before handlers run. diff --git a/src/main/java/world/bentobox/bentobox/listeners/PanelListenerManager.java b/src/main/java/world/bentobox/bentobox/listeners/PanelListenerManager.java index cd9d79991..32ebfabc8 100644 --- a/src/main/java/world/bentobox/bentobox/listeners/PanelListenerManager.java +++ b/src/main/java/world/bentobox/bentobox/listeners/PanelListenerManager.java @@ -1,6 +1,7 @@ package world.bentobox.bentobox.listeners; import java.util.ArrayList; +import java.util.EnumSet; import java.util.HashMap; import java.util.Map; import java.util.Optional; @@ -10,8 +11,10 @@ import org.bukkit.event.EventHandler; import org.bukkit.event.EventPriority; import org.bukkit.event.Listener; +import org.bukkit.event.inventory.InventoryAction; import org.bukkit.event.inventory.InventoryClickEvent; import org.bukkit.event.inventory.InventoryCloseEvent; +import org.bukkit.event.inventory.InventoryDragEvent; import org.bukkit.event.inventory.InventoryType.SlotType; import org.bukkit.event.player.PlayerQuitEvent; import org.bukkit.event.server.PluginDisableEvent; @@ -28,6 +31,22 @@ public class PanelListenerManager implements Listener { private static final Map openPanels = new HashMap<>(); + /** + * Actions that, when performed on a slot of the player's own inventory, only move items + * within that inventory, to the cursor, or out of the world via a drop. None of them can put + * an item into the panel. Shift-click ({@link InventoryAction#MOVE_TO_OTHER_INVENTORY}) and + * double-click ({@link InventoryAction#COLLECT_TO_CURSOR}, which also gathers matching panel + * icons) are deliberately absent. + */ + private static final EnumSet PLAYER_INVENTORY_ACTIONS = EnumSet.of( + InventoryAction.PICKUP_ALL, InventoryAction.PICKUP_SOME, InventoryAction.PICKUP_HALF, + InventoryAction.PICKUP_ONE, InventoryAction.PLACE_ALL, InventoryAction.PLACE_SOME, + InventoryAction.PLACE_ONE, InventoryAction.SWAP_WITH_CURSOR, InventoryAction.DROP_ALL_SLOT, + InventoryAction.DROP_ONE_SLOT, InventoryAction.HOTBAR_SWAP, + InventoryAction.PICKUP_FROM_BUNDLE, InventoryAction.PICKUP_ALL_INTO_BUNDLE, + InventoryAction.PICKUP_SOME_INTO_BUNDLE, InventoryAction.PLACE_FROM_BUNDLE, + InventoryAction.PLACE_ALL_INTO_BUNDLE, InventoryAction.PLACE_SOME_INTO_BUNDLE); + @EventHandler(priority = EventPriority.LOW) public void onInventoryClick(InventoryClickEvent event) { User user = User.getInstance(event.getWhoClicked()); // The player that clicked the item @@ -43,6 +62,13 @@ public void onInventoryClick(InventoryClickEvent event) { // Get the panel itself Panel panel = openPanels.get(user.getUniqueId()); + // Let the player use their own inventory while the panel is open, unless the panel + // reads clicks there (e.g. "click an item in your inventory" icon pickers). + if (isPlayerInventoryClickAllowed(panel, event)) { + event.setCancelled(false); + return; + } + // Apply the click cooldown for throttled panels (tabbed and settings panels) first, // before any other (potentially expensive) work. A rejected spam click must do the // absolute minimum: without this early return, every click - even the throttled ones - @@ -74,7 +100,9 @@ public void onInventoryClick(InventoryClickEvent event) { if (pi != null) { pi.getClickHandler().ifPresent(handler -> // Execute the handler's onClick method and optionally cancel the event if the handler returns true - event.setCancelled(handler.onClick(panel, user, event.getClick(), event.getSlot()))); + event.setCancelled(handler.onClick(panel, user, event.getClick(), event.getSlot()) + // An item picked up from the player's inventory must never be placed in the panel + || hasCursorItem(event))); } // If there is a listener, then run it and refresh the panel panel.getListener().ifPresent(l -> { @@ -92,6 +120,36 @@ public void onInventoryClick(InventoryClickEvent event) { } } + /** + * Whether a click can be left to vanilla: it is in the player's own (bottom) inventory, it + * cannot move an item into the panel, and the panel does not want to see it. + */ + private boolean isPlayerInventoryClickAllowed(Panel panel, InventoryClickEvent event) { + return event.getClickedInventory() != null + && event.getClickedInventory().equals(event.getView().getBottomInventory()) + && PLAYER_INVENTORY_ACTIONS.contains(event.getAction()) + && panel.getListener().map(PanelListener::allowsPlayerInventoryClicks).orElse(true); + } + + private boolean hasCursorItem(InventoryClickEvent event) { + return event.getCursor() != null && !event.getCursor().getType().isAir(); + } + + /** + * Stops items being dragged into a panel. Panels never accept items, and anything that got + * into one would be lost when it closes. Drags confined to the player's own inventory are fine. + */ + @EventHandler(priority = EventPriority.LOW) + public void onInventoryDrag(InventoryDragEvent event) { + Panel panel = openPanels.get(event.getWhoClicked().getUniqueId()); + if (panel != null && panel.getInventory().equals(event.getInventory())) { + int topSize = event.getView().getTopInventory().getSize(); + if (event.getRawSlots().stream().anyMatch(slot -> slot < topSize)) { + event.setCancelled(true); + } + } + } + @EventHandler(priority = EventPriority.LOWEST) public void onInventoryClose(InventoryCloseEvent event) { if (openPanels.containsKey(event.getPlayer().getUniqueId())) { diff --git a/src/main/java/world/bentobox/bentobox/panels/customizable/AbstractRefreshingPanel.java b/src/main/java/world/bentobox/bentobox/panels/customizable/AbstractRefreshingPanel.java index 62957374c..7e66e3b8c 100644 --- a/src/main/java/world/bentobox/bentobox/panels/customizable/AbstractRefreshingPanel.java +++ b/src/main/java/world/bentobox/bentobox/panels/customizable/AbstractRefreshingPanel.java @@ -90,6 +90,14 @@ public void onInventoryClick(User user, InventoryClickEvent event) { // Clicks are handled by the buttons } + /** + * All buttons are in the panel, so the player may use their own inventory. + */ + @Override + public boolean allowsPlayerInventoryClicks() { + return true; + } + @Override public void refreshPanel() { if (closed || panel == null) { diff --git a/src/test/java/world/bentobox/bentobox/listeners/PanelListenerManagerTest.java b/src/test/java/world/bentobox/bentobox/listeners/PanelListenerManagerTest.java index 404f57fd7..9875ed368 100644 --- a/src/test/java/world/bentobox/bentobox/listeners/PanelListenerManagerTest.java +++ b/src/test/java/world/bentobox/bentobox/listeners/PanelListenerManagerTest.java @@ -14,16 +14,19 @@ import java.util.HashMap; import java.util.Map; +import java.util.Set; import java.util.Optional; import java.util.UUID; import org.bukkit.ChatColor; +import org.bukkit.Material; import org.bukkit.entity.HumanEntity; import org.bukkit.entity.Player; import org.bukkit.event.inventory.ClickType; import org.bukkit.event.inventory.InventoryAction; import org.bukkit.event.inventory.InventoryClickEvent; import org.bukkit.event.inventory.InventoryCloseEvent; +import org.bukkit.event.inventory.InventoryDragEvent; import org.bukkit.event.inventory.InventoryType; import org.bukkit.event.inventory.InventoryType.SlotType; import org.bukkit.event.player.PlayerQuitEvent; @@ -31,6 +34,7 @@ import org.bukkit.inventory.InventoryView; import org.bukkit.inventory.ItemStack; import org.bukkit.inventory.MenuType; +import org.bukkit.inventory.PlayerInventory; import org.jetbrains.annotations.Nullable; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -341,6 +345,111 @@ void testOnInventoryClickOpenPanelsRightPanelRightSlot() { verify(pl).onInventoryClick(any(), any()); } + /** + * A view whose raw slots 0-8 are the panel and 9 onwards are the player's inventory. + */ + class SplitView extends MyView { + + private final Inventory bottom = mock(PlayerInventory.class); + private ItemStack cursor; + + SplitView() { + super(ChatColor.RED + PANEL_NAME, panel.getInventory()); + } + + @Override + public Inventory getBottomInventory() { + return bottom; + } + + @Override + public Inventory getInventory(int rawSlot) { + return rawSlot < 9 ? getTopInventory() : bottom; + } + + @Override + public ItemStack getCursor() { + return cursor; + } + + @Override + public void setCursor(ItemStack item) { + cursor = item; + } + } + + private InventoryClickEvent clickPlayerInventory(InventoryAction action) { + PanelListenerManager.getOpenPanels().put(uuid, panel); + InventoryClickEvent e = new InventoryClickEvent(new SplitView(), type, 20, click, action); + plm.onInventoryClick(e); + return e; + } + + @Test + void testPlayerInventoryClickRoutedToListenerByDefault() { + // A listener that has not opted in (e.g. an icon picker) still gets the click, cancelled + InventoryClickEvent e = clickPlayerInventory(InventoryAction.PICKUP_ALL); + assertTrue(e.isCancelled()); + verify(pl).onInventoryClick(any(), eq(e)); + } + + @Test + void testPlayerInventoryClickAllowedWhenListenerOptsIn() { + when(pl.allowsPlayerInventoryClicks()).thenReturn(true); + InventoryClickEvent e = clickPlayerInventory(InventoryAction.PICKUP_ALL); + assertFalse(e.isCancelled()); + verify(pl, never()).onInventoryClick(any(), any()); + verify(pl, never()).refreshPanel(); + } + + @Test + void testPlayerInventoryClickAllowedWithoutListener() { + when(panel.getListener()).thenReturn(Optional.empty()); + assertFalse(clickPlayerInventory(InventoryAction.DROP_ONE_SLOT).isCancelled()); + assertFalse(clickPlayerInventory(InventoryAction.PICKUP_FROM_BUNDLE).isCancelled()); + } + + @Test + void testPlayerInventoryShiftAndDoubleClickBlocked() { + when(panel.getListener()).thenReturn(Optional.empty()); + assertTrue(clickPlayerInventory(InventoryAction.MOVE_TO_OTHER_INVENTORY).isCancelled()); + assertTrue(clickPlayerInventory(InventoryAction.COLLECT_TO_CURSOR).isCancelled()); + } + + @Test + void testPanelClickWithCursorItemStaysCancelled() { + PanelListenerManager.getOpenPanels().put(uuid, panel); + // The handler asks for the click not to be cancelled + when(ch.onClick(any(), any(), any(), any(Integer.class))).thenReturn(false); + SplitView splitView = new SplitView(); + ItemStack held = mock(ItemStack.class); + when(held.getType()).thenReturn(Material.STONE); + splitView.setCursor(held); + InventoryClickEvent e = new InventoryClickEvent(splitView, type, 0, click, InventoryAction.PLACE_ALL); + plm.onInventoryClick(e); + verify(ch).onClick(eq(panel), any(User.class), eq(click), eq(0)); + assertTrue(e.isCancelled()); + } + + @Test + void testDragIntoPanelCancelled() { + PanelListenerManager.getOpenPanels().put(uuid, panel); + InventoryDragEvent e = new InventoryDragEvent(new SplitView(), null, mock(ItemStack.class), false, + Map.of(3, mock(ItemStack.class), 20, mock(ItemStack.class))); + plm.onInventoryDrag(e); + assertTrue(e.isCancelled()); + } + + @Test + void testDragWithinPlayerInventoryAllowed() { + PanelListenerManager.getOpenPanels().put(uuid, panel); + InventoryDragEvent e = new InventoryDragEvent(new SplitView(), null, mock(ItemStack.class), false, + Map.of(20, mock(ItemStack.class), 21, mock(ItemStack.class))); + plm.onInventoryDrag(e); + assertFalse(e.isCancelled()); + assertEquals(Set.of(20, 21), e.getRawSlots()); + } + /** * Test method for {@link world.bentobox.bentobox.listeners.PanelListenerManager#onInventoryClose(org.bukkit.event.inventory.InventoryCloseEvent)}. */