From dfa82fa0674918460edbfb4f1c44b94eabc2edae Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Wed, 11 Nov 2026 12:52:15 +0100 Subject: [PATCH 1/3] fix: stop a broken station's site choice from starting a build Breaking an idle station while choosing a building site left the placement active. The next left click then took the materials and started the build on the removed station: it never ticked, never refunded, and left a "Constructing" text floating where it stood. - Removing a station drops the site choices made for it. - The build-site click and the blueprint menu check that the station is still registered. - Left-clicking the station itself now cancels the site choice. Co-Authored-By: Claude Opus 5.5 --- .../vfbuilders/managers/StationManager.java | 31 ++++- .../tfminecraft/vfbuilders/ManagerTest.java | 116 ++++++++++++++++++ 2 files changed, 146 insertions(+), 1 deletion(-) diff --git a/src/main/java/net/tfminecraft/vfbuilders/managers/StationManager.java b/src/main/java/net/tfminecraft/vfbuilders/managers/StationManager.java index 775ca21..ed148d2 100644 --- a/src/main/java/net/tfminecraft/vfbuilders/managers/StationManager.java +++ b/src/main/java/net/tfminecraft/vfbuilders/managers/StationManager.java @@ -240,10 +240,14 @@ public void inventoryClick(InventoryClickEvent e) { return; } p.closeInventory(); + if (!isRegistered(h.getStation())) { + p.sendMessage("§cThe station is gone, so nothing can be built there."); + return; + } ActivePlacement placement = new ActivePlacement(p, h.getStation(), b); activePlacements.put(p.getUniqueId(), placement); - p.sendMessage("§aStand where it should be built, within "+Cache.constructionDistance+" blocks of the station, and left-click."); + p.sendMessage("§aStand where it should be built, within "+Cache.constructionDistance+" blocks of the station, and left-click. Left-click the station to cancel."); startParticleTrail(placement); // Method defined below } } @@ -263,6 +267,21 @@ public void onPlayerLeftClick(PlayerInteractEvent e) { Location stationLoc = placement.getStation().getLocation(); Location clickLoc = p.getLocation(); + if (!isRegistered(placement.getStation())) { + activePlacements.remove(uuid); + p.sendMessage("§cThe station is gone, so no building site was chosen."); + return; + } + + // Hitting the station itself cancels the choice instead of building or breaking. + Block clicked = e.getClickedBlock(); + if (clicked != null && clicked.getLocation().equals(stationLoc)) { + activePlacements.remove(uuid); + e.setCancelled(true); + p.sendMessage("§eBuilding site choice cancelled."); + return; + } + if (!java.util.Objects.equals(clickLoc.getWorld(), stationLoc.getWorld()) || clickLoc.distanceSquared(stationLoc) > Math.pow(Cache.constructionDistance, 2)) { p.sendMessage("§cToo far from the station! (max "+Cache.constructionDistance+" blocks)"); @@ -430,7 +449,17 @@ private boolean remove(Location loc) { stations.put(loc, station); return false; } + // A site choice for the removed station must not start a build on it later. + activePlacements.values().removeIf(placement -> { + if (placement.getStation() != station) return false; + placement.getPlayer().sendMessage("§cThe station is gone, so no building site was chosen."); + return true; + }); return true; } + private boolean isRegistered(ActiveStation station) { + return stations.get(station.getLocation()) == station; + } + } diff --git a/src/test/java/net/tfminecraft/vfbuilders/ManagerTest.java b/src/test/java/net/tfminecraft/vfbuilders/ManagerTest.java index 1f40bf0..79d744a 100644 --- a/src/test/java/net/tfminecraft/vfbuilders/ManagerTest.java +++ b/src/test/java/net/tfminecraft/vfbuilders/ManagerTest.java @@ -504,4 +504,120 @@ void particleTrailStopsAfterCommitOrAnotherConstructionAndIgnoresOtherWorlds() t manager.onBlockBreak(new BlockBreakEvent(block, player)); verify(b).drop(any()); } + + String drainMessagesUntil(String fragment) { + var mock = (org.mockbukkit.mockbukkit.entity.PlayerMock) player; + for (String m = mock.nextMessage(); m != null; m = mock.nextMessage()) { + if (m.contains(fragment)) return m; + } + return null; + } + + PlayerInteractEvent leftClickBlock(Block clicked) { + return new PlayerInteractEvent( + player, Action.LEFT_CLICK_BLOCK, null, clicked, org.bukkit.block.BlockFace.UP); + } + + @Test + void breakingAnIdleStationDuringSiteChoiceCannotStartABuildOnIt() throws Exception { + var s = manager.getStation(block); + var b = mock(Blueprint.class); + when(b.hasInputs(player)).thenReturn(true); + BlueprintLoader.get().put("windrider", b); + player.teleport(block.getLocation()); + manager.inventoryClick( + click(s, tagged("vfb_blueprint_id", "windrider"), net.tfminecraft.vfbuilders.enums.VFBGUI.BLUEPRINT)); + assertNotNull(drainMessagesUntil("Left-click the station to cancel")); + + manager.onBlockBreak(new BlockBreakEvent(block, player)); + + assertTrue(placements().isEmpty()); + assertNotNull(drainMessagesUntil("no building site was chosen")); + manager.onPlayerLeftClick(left()); + verify(b, never()).takeInputs(player); + assertFalse(s.hasBlueprint()); + assertNull(s.getSpawnLocation()); + } + + @Test + void stalePlacementOnARemovedStationIsDroppedEvenIfAStationReturnsThere() throws Exception { + var old = manager.getStation(block); + var b = mock(Blueprint.class); + when(b.hasInputs(player)).thenReturn(true); + manager.onBlockBreak(new BlockBreakEvent(block, player)); + var replacement = manager.getStation(block); + assertNotSame(old, replacement); + player.teleport(block.getLocation()); + placements().put(player.getUniqueId(), new ActivePlacement(player, old, b)); + + manager.onPlayerLeftClick(left()); + + assertTrue(placements().isEmpty()); + verify(b, never()).takeInputs(player); + assertFalse(old.hasBlueprint()); + assertFalse(replacement.hasBlueprint()); + assertNotNull(drainMessagesUntil("no building site was chosen")); + } + + @Test + void blueprintMenuLeftOpenOnARemovedStationStartsNoSiteChoice() throws Exception { + var s = manager.getStation(block); + var b = mock(Blueprint.class); + when(b.hasInputs(player)).thenReturn(true); + BlueprintLoader.get().put("boat", b); + manager.onBlockBreak(new BlockBreakEvent(block, player)); + + manager.inventoryClick( + click(s, tagged("vfb_blueprint_id", "boat"), net.tfminecraft.vfbuilders.enums.VFBGUI.BLUEPRINT)); + + assertTrue(placements().isEmpty()); + assertNotNull(drainMessagesUntil("nothing can be built there")); + } + + @Test + void leftClickingTheStationCancelsTheSiteChoiceAndOtherBlocksStillConfirm() throws Exception { + var s = manager.getStation(block); + var b = mock(Blueprint.class); + var vehicle = mock(net.tfminecraft.vehicleframework.vehicles.Vehicle.class); + when(vehicle.getName()).thenReturn("Windrider"); + when(b.getVehicle()).thenReturn(vehicle); + when(b.hasInputs(player)).thenReturn(true); + player.teleport(block.getLocation()); + placements().put(player.getUniqueId(), new ActivePlacement(player, s, b)); + + var onStation = leftClickBlock(block); + manager.onPlayerLeftClick(onStation); + + assertTrue(onStation.isCancelled()); + assertTrue(placements().isEmpty()); + verify(b, never()).takeInputs(player); + assertFalse(s.hasBlueprint()); + assertNotNull(drainMessagesUntil("Building site choice cancelled")); + + placements().put(player.getUniqueId(), new ActivePlacement(player, s, b)); + var elsewhere = leftClickBlock(player.getWorld().getBlockAt(3, 64, 1)); + manager.onPlayerLeftClick(elsewhere); + + assertFalse(elsewhere.isCancelled()); + verify(b).takeInputs(player); + assertSame(b, s.getBlueprint()); + } + + @Test + void removingAStationKeepsSiteChoicesForOtherStations() throws Exception { + var other = player.getWorld().getBlockAt(2, 64, 1); + when(api.getChecker().checkBlock(other, "v.stone")).thenReturn(true); + var s = manager.getStation(block); + var otherStation = manager.getStation(other); + var second = server.addPlayer(); + var mine = new ActivePlacement(player, s, mock(Blueprint.class)); + var theirs = new ActivePlacement(second, otherStation, mock(Blueprint.class)); + placements().put(player.getUniqueId(), mine); + placements().put(second.getUniqueId(), theirs); + + manager.onBlockBreak(new BlockBreakEvent(block, player)); + + assertNull(placements().get(player.getUniqueId())); + assertSame(theirs, placements().get(second.getUniqueId())); + } } From 95f3ff5a1d2200fedf0fdc36d203333bbde50b05 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Wed, 11 Nov 2026 12:59:22 +0100 Subject: [PATCH 2/3] Word the cancelled site choice like the other cancel and cover a failed removal save Co-Authored-By: Claude Opus 5.5 --- .../vfbuilders/managers/StationManager.java | 6 +++--- .../tfminecraft/vfbuilders/ManagerTest.java | 20 +++++++++++++++++-- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/main/java/net/tfminecraft/vfbuilders/managers/StationManager.java b/src/main/java/net/tfminecraft/vfbuilders/managers/StationManager.java index ed148d2..166ea52 100644 --- a/src/main/java/net/tfminecraft/vfbuilders/managers/StationManager.java +++ b/src/main/java/net/tfminecraft/vfbuilders/managers/StationManager.java @@ -241,7 +241,7 @@ public void inventoryClick(InventoryClickEvent e) { } p.closeInventory(); if (!isRegistered(h.getStation())) { - p.sendMessage("§cThe station is gone, so nothing can be built there."); + p.sendMessage("§cThe station was removed, so nothing can be built there."); return; } @@ -269,7 +269,7 @@ public void onPlayerLeftClick(PlayerInteractEvent e) { if (!isRegistered(placement.getStation())) { activePlacements.remove(uuid); - p.sendMessage("§cThe station is gone, so no building site was chosen."); + p.sendMessage("§cBuilding site choice cancelled: the station was removed."); return; } @@ -452,7 +452,7 @@ private boolean remove(Location loc) { // A site choice for the removed station must not start a build on it later. activePlacements.values().removeIf(placement -> { if (placement.getStation() != station) return false; - placement.getPlayer().sendMessage("§cThe station is gone, so no building site was chosen."); + placement.getPlayer().sendMessage("§cBuilding site choice cancelled: the station was removed."); return true; }); return true; diff --git a/src/test/java/net/tfminecraft/vfbuilders/ManagerTest.java b/src/test/java/net/tfminecraft/vfbuilders/ManagerTest.java index 79d744a..0c36645 100644 --- a/src/test/java/net/tfminecraft/vfbuilders/ManagerTest.java +++ b/src/test/java/net/tfminecraft/vfbuilders/ManagerTest.java @@ -532,7 +532,7 @@ void breakingAnIdleStationDuringSiteChoiceCannotStartABuildOnIt() throws Excepti manager.onBlockBreak(new BlockBreakEvent(block, player)); assertTrue(placements().isEmpty()); - assertNotNull(drainMessagesUntil("no building site was chosen")); + assertNotNull(drainMessagesUntil("the station was removed")); manager.onPlayerLeftClick(left()); verify(b, never()).takeInputs(player); assertFalse(s.hasBlueprint()); @@ -556,7 +556,7 @@ void stalePlacementOnARemovedStationIsDroppedEvenIfAStationReturnsThere() throws verify(b, never()).takeInputs(player); assertFalse(old.hasBlueprint()); assertFalse(replacement.hasBlueprint()); - assertNotNull(drainMessagesUntil("no building site was chosen")); + assertNotNull(drainMessagesUntil("the station was removed")); } @Test @@ -566,6 +566,7 @@ void blueprintMenuLeftOpenOnARemovedStationStartsNoSiteChoice() throws Exception when(b.hasInputs(player)).thenReturn(true); BlueprintLoader.get().put("boat", b); manager.onBlockBreak(new BlockBreakEvent(block, player)); + assertNotSame(s, manager.getStation(block)); // a new station now stands on the same spot manager.inventoryClick( click(s, tagged("vfb_blueprint_id", "boat"), net.tfminecraft.vfbuilders.enums.VFBGUI.BLUEPRINT)); @@ -620,4 +621,19 @@ void removingAStationKeepsSiteChoicesForOtherStations() throws Exception { assertNull(placements().get(player.getUniqueId())); assertSame(theirs, placements().get(second.getUniqueId())); } + + @Test + void failedRemovalSaveKeepsTheSiteChoiceForTheRestoredStation() throws Exception { + var s = manager.getStation(block); + var mine = new ActivePlacement(player, s, mock(Blueprint.class)); + placements().put(player.getUniqueId(), mine); + database.when(() -> Database.saveStationsNow(anyMap())).thenReturn(false); + var event = new BlockBreakEvent(block, player); + + manager.onBlockBreak(event); + + assertTrue(event.isCancelled()); + assertSame(s, manager.getStation(block)); + assertSame(mine, placements().get(player.getUniqueId())); + } } From 19adfee5b18e75e5b3cea4daf1f953419eaffccb Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Wed, 11 Nov 2026 13:19:18 +0100 Subject: [PATCH 3/3] Drop the left-click-the-station cancel: survival clicks never reach it ItemsAdder runs the mining of its custom-block stations, so a survival player's left click on the station never fires PlayerInteractEvent. The prompt promised a cancel that only worked in creative. Breaking the station still cancels the choice with a message. Co-Authored-By: Claude Opus 5.5 --- .../vfbuilders/managers/StationManager.java | 11 +----- .../tfminecraft/vfbuilders/ManagerTest.java | 36 +------------------ 2 files changed, 2 insertions(+), 45 deletions(-) diff --git a/src/main/java/net/tfminecraft/vfbuilders/managers/StationManager.java b/src/main/java/net/tfminecraft/vfbuilders/managers/StationManager.java index 166ea52..840ad63 100644 --- a/src/main/java/net/tfminecraft/vfbuilders/managers/StationManager.java +++ b/src/main/java/net/tfminecraft/vfbuilders/managers/StationManager.java @@ -247,7 +247,7 @@ public void inventoryClick(InventoryClickEvent e) { ActivePlacement placement = new ActivePlacement(p, h.getStation(), b); activePlacements.put(p.getUniqueId(), placement); - p.sendMessage("§aStand where it should be built, within "+Cache.constructionDistance+" blocks of the station, and left-click. Left-click the station to cancel."); + p.sendMessage("§aStand where it should be built, within "+Cache.constructionDistance+" blocks of the station, and left-click."); startParticleTrail(placement); // Method defined below } } @@ -273,15 +273,6 @@ public void onPlayerLeftClick(PlayerInteractEvent e) { return; } - // Hitting the station itself cancels the choice instead of building or breaking. - Block clicked = e.getClickedBlock(); - if (clicked != null && clicked.getLocation().equals(stationLoc)) { - activePlacements.remove(uuid); - e.setCancelled(true); - p.sendMessage("§eBuilding site choice cancelled."); - return; - } - if (!java.util.Objects.equals(clickLoc.getWorld(), stationLoc.getWorld()) || clickLoc.distanceSquared(stationLoc) > Math.pow(Cache.constructionDistance, 2)) { p.sendMessage("§cToo far from the station! (max "+Cache.constructionDistance+" blocks)"); diff --git a/src/test/java/net/tfminecraft/vfbuilders/ManagerTest.java b/src/test/java/net/tfminecraft/vfbuilders/ManagerTest.java index 0c36645..d183961 100644 --- a/src/test/java/net/tfminecraft/vfbuilders/ManagerTest.java +++ b/src/test/java/net/tfminecraft/vfbuilders/ManagerTest.java @@ -513,11 +513,6 @@ String drainMessagesUntil(String fragment) { return null; } - PlayerInteractEvent leftClickBlock(Block clicked) { - return new PlayerInteractEvent( - player, Action.LEFT_CLICK_BLOCK, null, clicked, org.bukkit.block.BlockFace.UP); - } - @Test void breakingAnIdleStationDuringSiteChoiceCannotStartABuildOnIt() throws Exception { var s = manager.getStation(block); @@ -527,7 +522,7 @@ void breakingAnIdleStationDuringSiteChoiceCannotStartABuildOnIt() throws Excepti player.teleport(block.getLocation()); manager.inventoryClick( click(s, tagged("vfb_blueprint_id", "windrider"), net.tfminecraft.vfbuilders.enums.VFBGUI.BLUEPRINT)); - assertNotNull(drainMessagesUntil("Left-click the station to cancel")); + assertNotNull(drainMessagesUntil("and left-click")); manager.onBlockBreak(new BlockBreakEvent(block, player)); @@ -575,35 +570,6 @@ void blueprintMenuLeftOpenOnARemovedStationStartsNoSiteChoice() throws Exception assertNotNull(drainMessagesUntil("nothing can be built there")); } - @Test - void leftClickingTheStationCancelsTheSiteChoiceAndOtherBlocksStillConfirm() throws Exception { - var s = manager.getStation(block); - var b = mock(Blueprint.class); - var vehicle = mock(net.tfminecraft.vehicleframework.vehicles.Vehicle.class); - when(vehicle.getName()).thenReturn("Windrider"); - when(b.getVehicle()).thenReturn(vehicle); - when(b.hasInputs(player)).thenReturn(true); - player.teleport(block.getLocation()); - placements().put(player.getUniqueId(), new ActivePlacement(player, s, b)); - - var onStation = leftClickBlock(block); - manager.onPlayerLeftClick(onStation); - - assertTrue(onStation.isCancelled()); - assertTrue(placements().isEmpty()); - verify(b, never()).takeInputs(player); - assertFalse(s.hasBlueprint()); - assertNotNull(drainMessagesUntil("Building site choice cancelled")); - - placements().put(player.getUniqueId(), new ActivePlacement(player, s, b)); - var elsewhere = leftClickBlock(player.getWorld().getBlockAt(3, 64, 1)); - manager.onPlayerLeftClick(elsewhere); - - assertFalse(elsewhere.isCancelled()); - verify(b).takeInputs(player); - assertSame(b, s.getBlueprint()); - } - @Test void removingAStationKeepsSiteChoicesForOtherStations() throws Exception { var other = player.getWorld().getBlockAt(2, 64, 1);