From ac1bd9ad08158bce372d0036a8955095eb16f536 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Sat, 26 Sep 2026 21:31:37 +0200 Subject: [PATCH] fix: bind research menus to their own station A player who owned more than one research station could act on the wrong one. Opening the scrap confirmation fired an inventory close for the station menu, which cleared the player's open-station entry; clicks then fell back to the first station the player owned, so confirming a scrap could delete a different station's project. Each station and scrap-confirm menu now carries its lectern location in an InventoryHolder, and clicks resolve the station from the menu that is open. The per-player open-station map and its first-owned-station fallback are removed. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../research/manager/InventoryManager.java | 10 ++- .../research/manager/ResearchManager.java | 66 ++++++++-------- .../research/manager/StationMenuHolder.java | 32 ++++++++ .../manager/MultipleStationsTest.java | 75 +++++++++++++++++++ .../research/manager/ResearchRefundTest.java | 7 +- 5 files changed, 148 insertions(+), 42 deletions(-) create mode 100644 src/main/java/net/tfminecraft/research/manager/StationMenuHolder.java create mode 100644 src/test/java/net/tfminecraft/research/manager/MultipleStationsTest.java diff --git a/src/main/java/net/tfminecraft/research/manager/InventoryManager.java b/src/main/java/net/tfminecraft/research/manager/InventoryManager.java index c07dbed..cb96ac2 100644 --- a/src/main/java/net/tfminecraft/research/manager/InventoryManager.java +++ b/src/main/java/net/tfminecraft/research/manager/InventoryManager.java @@ -66,7 +66,9 @@ public InventoryManager(PlayerManager playerManager) { } public void openMain(Player player, ResearchStation station) { - Inventory inventory = Research.plugin.getServer().createInventory(null, 54, mainInventoryTitle()); + StationMenuHolder holder = new StationMenuHolder(station.getLocation()); + Inventory inventory = Research.plugin.getServer().createInventory(holder, 54, mainInventoryTitle()); + holder.setInventory(inventory); populateMain(player, station, inventory); player.openInventory(inventory); } @@ -368,8 +370,10 @@ public void updateExperimentPreview(Inventory inventory, ItemStack experimentSta buildPreviewAspectItem(match.getSecondaryAspect(), match.getSecondaryPoints(), project)); } - public void openScrapConfirm(Player player) { - Inventory inventory = Research.plugin.getServer().createInventory(null, 9, scrapConfirmTitle()); + public void openScrapConfirm(Player player, ResearchStation station) { + StationMenuHolder holder = new StationMenuHolder(station.getLocation()); + Inventory inventory = Research.plugin.getServer().createInventory(holder, 9, scrapConfirmTitle()); + holder.setInventory(inventory); inventory.setItem(CONFIRM_SCRAP_YES, buildButton(GuiCache.confirmButton, GuiText.label(GuiCache.scrapConfirmYesLabel))); inventory.setItem(CONFIRM_SCRAP_NO, buildButton(GuiCache.cancelButton, GuiText.label(GuiCache.scrapConfirmNoLabel))); player.openInventory(inventory); diff --git a/src/main/java/net/tfminecraft/research/manager/ResearchManager.java b/src/main/java/net/tfminecraft/research/manager/ResearchManager.java index d18e778..0c2d8fb 100644 --- a/src/main/java/net/tfminecraft/research/manager/ResearchManager.java +++ b/src/main/java/net/tfminecraft/research/manager/ResearchManager.java @@ -1,7 +1,6 @@ package net.tfminecraft.research.manager; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.UUID; @@ -21,6 +20,7 @@ import org.bukkit.event.player.PlayerInteractEvent; import org.bukkit.inventory.EquipmentSlot; import org.bukkit.inventory.Inventory; +import org.bukkit.inventory.InventoryView; import org.bukkit.inventory.ItemStack; import net.tfminecraft.research.Cache; @@ -62,7 +62,6 @@ public final class ResearchManager implements Listener { private final InventoryManager inventoryManager; private final List stations = new ArrayList<>(); - private final Map openGui = new HashMap<>(); public ResearchManager(PlayerManager playerManager) { this.playerManager = playerManager; @@ -76,14 +75,12 @@ public static ResearchManager getInstance() { public void start() { stations.clear(); - openGui.clear(); stations.addAll(StationStore.loadAll()); } public void unloadAll() { StationStore.saveAll(stations); stations.clear(); - openGui.clear(); } public ResearchStation getStationAt(Location location) { @@ -181,7 +178,7 @@ public void onInventoryClick(InventoryClickEvent event) { event.setCancelled(true); - ResearchStation station = resolveStationForPlayerGui(player); + ResearchStation station = resolveStationForMenu(player, event.getView()); if (station == null) { return; } @@ -236,13 +233,11 @@ public void onInventoryClose(InventoryCloseEvent event) { event.getView().getTopInventory().setItem(GridLayout.SLOT_EXPERIMENT, null); returnItemToPlayer(player, experiment); } - openGui.remove(player.getUniqueId()); } private void handleMainClick(Player player, ResearchStation station, int slot, Inventory inventory) { if (slot == GridLayout.SLOT_SCRAP) { - openGui.put(player.getUniqueId(), blockLocation(station.getLocation())); - inventoryManager.openScrapConfirm(player); + inventoryManager.openScrapConfirm(player, station); } else if (slot == GridLayout.SLOT_CONFIRM_EXPERIMENT) { confirmExperiment(player, station, inventory); } @@ -538,7 +533,6 @@ private void finishCompletedStation(Player player, ResearchStation station, Inve Location loc = blockLocation(station.getLocation()); StationCompleteEffects.play(player, loc); stations.remove(station); - openGui.remove(player.getUniqueId()); StationStore.deleteStation(loc); inventory.clear(); player.closeInventory(); @@ -638,17 +632,14 @@ private void scrapStation(ResearchStation station, String ownerMessage) { Player owner = Bukkit.getPlayer(ownerUuid); if (owner != null && owner.isOnline()) { - String openTitle = owner.getOpenInventory().getTitle(); - if (openTitle.equals(InventoryManager.mainInventoryTitle())) { - Location openLoc = openGui.get(ownerUuid); - if (openLoc != null && station.isAt(openLoc)) { - // The close handler owns returning the experiment item. - owner.closeInventory(); - } - } else if (openTitle.equals(InventoryManager.scrapConfirmTitle())) { + InventoryView openView = owner.getOpenInventory(); + String openTitle = openView.getTitle(); + if ((openTitle.equals(InventoryManager.mainInventoryTitle()) + || openTitle.equals(InventoryManager.scrapConfirmTitle())) + && isMenuFor(openView, station)) { + // The close handler owns returning the experiment item. owner.closeInventory(); } - openGui.remove(ownerUuid); if (ownerMessage != null && !ownerMessage.isBlank()) { owner.sendMessage(ownerMessage); } @@ -659,27 +650,36 @@ private void scrapStation(ResearchStation station, String ownerMessage) { } private void openMainGui(Player player, ResearchStation station) { - openGui.put(player.getUniqueId(), blockLocation(station.getLocation())); inventoryManager.openMain(player, station); } /** - * Resolves the station for an open research GUI from the lectern location in {@link #openGui}, - * or the player's owned station if that session map was lost (e.g. after reload). + * Resolves the station a research menu was opened for. Each menu carries its lectern location, + * so a player with several stations only ever acts on the one whose menu is open. */ - private ResearchStation resolveStationForPlayerGui(Player player) { - Location openLoc = openGui.get(player.getUniqueId()); - if (openLoc != null) { - ResearchStation atOpen = getStationAt(openLoc); - if (atOpen != null && atOpen.getOwnerUuid().equals(player.getUniqueId())) { - return atOpen; - } + private ResearchStation resolveStationForMenu(Player player, InventoryView view) { + Location menuLoc = menuStationLocation(view); + if (menuLoc == null) { + return null; } - for (ResearchStation station : stations) { - if (station.getOwnerUuid().equals(player.getUniqueId())) { - openGui.put(player.getUniqueId(), blockLocation(station.getLocation())); - return station; - } + ResearchStation station = getStationAt(menuLoc); + if (station == null || !station.getOwnerUuid().equals(player.getUniqueId())) { + return null; + } + return station; + } + + private boolean isMenuFor(InventoryView view, ResearchStation station) { + Location menuLoc = menuStationLocation(view); + return menuLoc != null && station.isAt(menuLoc); + } + + private Location menuStationLocation(InventoryView view) { + if (view == null || view.getTopInventory() == null) { + return null; + } + if (view.getTopInventory().getHolder() instanceof StationMenuHolder holder) { + return holder.getStationLocation(); } return null; } diff --git a/src/main/java/net/tfminecraft/research/manager/StationMenuHolder.java b/src/main/java/net/tfminecraft/research/manager/StationMenuHolder.java new file mode 100644 index 0000000..4648a2d --- /dev/null +++ b/src/main/java/net/tfminecraft/research/manager/StationMenuHolder.java @@ -0,0 +1,32 @@ +package net.tfminecraft.research.manager; + +import org.bukkit.Location; +import org.bukkit.inventory.Inventory; +import org.bukkit.inventory.InventoryHolder; + +/** + * Ties a research menu to the station it was opened for, so a player with several stations + * always acts on the lectern whose menu is open. + */ +public final class StationMenuHolder implements InventoryHolder { + + private final Location stationLocation; + private Inventory inventory; + + public StationMenuHolder(Location stationLocation) { + this.stationLocation = stationLocation; + } + + public Location getStationLocation() { + return stationLocation; + } + + void setInventory(Inventory inventory) { + this.inventory = inventory; + } + + @Override + public Inventory getInventory() { + return inventory; + } +} diff --git a/src/test/java/net/tfminecraft/research/manager/MultipleStationsTest.java b/src/test/java/net/tfminecraft/research/manager/MultipleStationsTest.java new file mode 100644 index 0000000..57cf550 --- /dev/null +++ b/src/test/java/net/tfminecraft/research/manager/MultipleStationsTest.java @@ -0,0 +1,75 @@ +package net.tfminecraft.research.manager; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.*; + +import java.util.List; +import java.util.UUID; + +import org.bukkit.Bukkit; +import org.bukkit.Location; +import org.bukkit.World; +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.InventoryType.SlotType; +import org.bukkit.inventory.Inventory; +import org.bukkit.inventory.InventoryView; +import org.bukkit.inventory.ItemStack; +import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; + +import net.tfminecraft.research.Messages; +import net.tfminecraft.research.database.StationStore; +import net.tfminecraft.research.model.ResearchStation; + +class MultipleStationsTest { + + @Test + void scrapConfirmActsOnTheStationWhoseMenuIsOpen() { + UUID ownerId = UUID.randomUUID(); + Player owner = mock(Player.class); + World world = mock(World.class); + Location first = new Location(world, 10, 64, 10); + Location second = new Location(world, 20, 64, 20); + ResearchStation firstStation = new ResearchStation(first, ownerId, null); + ResearchStation secondStation = new ResearchStation(second, ownerId, null); + + Inventory top = mock(Inventory.class); + InventoryView view = mock(InventoryView.class); + when(owner.getUniqueId()).thenReturn(ownerId); + when(owner.isOnline()).thenReturn(true); + when(owner.getOpenInventory()).thenReturn(view); + when(view.getTitle()).thenReturn("Confirm Scrap"); + when(view.getTopInventory()).thenReturn(top); + when(view.getPlayer()).thenReturn(owner); + when(view.convertSlot(InventoryManager.CONFIRM_SCRAP_YES)).thenReturn(InventoryManager.CONFIRM_SCRAP_YES); + when(view.getItem(InventoryManager.CONFIRM_SCRAP_YES)).thenReturn(mock(ItemStack.class)); + when(top.getHolder()).thenReturn(new StationMenuHolder(second)); + + try (MockedStatic bukkit = mockStatic(Bukkit.class); + MockedStatic store = mockStatic(StationStore.class); + MockedStatic menus = mockStatic(InventoryManager.class); + MockedStatic messages = mockStatic(Messages.class)) { + bukkit.when(() -> Bukkit.getPlayer(ownerId)).thenReturn(owner); + store.when(StationStore::loadAll).thenReturn(List.of(firstStation, secondStation)); + menus.when(InventoryManager::mainInventoryTitle).thenReturn("Research Station"); + menus.when(InventoryManager::scrapConfirmTitle).thenReturn("Confirm Scrap"); + ResearchManager manager = new ResearchManager(null); + manager.start(); + + InventoryClickEvent click = new InventoryClickEvent(view, SlotType.CONTAINER, + InventoryManager.CONFIRM_SCRAP_YES, ClickType.LEFT, InventoryAction.PICKUP_ALL); + manager.onInventoryClick(click); + + assertTrue(click.isCancelled()); + assertNotNull(manager.getStationAt(first), "The other station must keep its project"); + assertNull(manager.getStationAt(second), "The station whose menu was open must be scrapped"); + store.verify(() -> StationStore.deleteStation(second)); + store.verify(() -> StationStore.deleteStation(first), never()); + } + } +} diff --git a/src/test/java/net/tfminecraft/research/manager/ResearchRefundTest.java b/src/test/java/net/tfminecraft/research/manager/ResearchRefundTest.java index b19ad20..8cf970a 100644 --- a/src/test/java/net/tfminecraft/research/manager/ResearchRefundTest.java +++ b/src/test/java/net/tfminecraft/research/manager/ResearchRefundTest.java @@ -5,7 +5,6 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.*; -import java.lang.reflect.Field; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -68,6 +67,7 @@ private void exerciseRefund(boolean breakStation, boolean fullInventory) throws when(view.getTitle()).thenReturn("Research Station"); when(view.getTopInventory()).thenReturn(top); when(view.getPlayer()).thenReturn(owner); + when(top.getHolder()).thenReturn(new StationMenuHolder(location)); when(top.getItem(GridLayout.SLOT_EXPERIMENT)).thenAnswer(call -> slot.get()); doAnswer(call -> { slot.set(call.getArgument(1)); @@ -86,11 +86,6 @@ private void exerciseRefund(boolean breakStation, boolean fullInventory) throws menus.when(InventoryManager::mainInventoryTitle).thenReturn("Research Station"); ResearchManager manager = new ResearchManager(null); manager.start(); - Field field = ResearchManager.class.getDeclaredField("openGui"); - field.setAccessible(true); - @SuppressWarnings("unchecked") - Map openGui = (Map) field.get(manager); - openGui.put(ownerId, location); // Bukkit dispatches InventoryCloseEvent synchronously from closeInventory(). doAnswer(call -> {