From 8bd858496437e91a35815cb5640682653113514e Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Thu, 15 Oct 2026 21:14:31 +0000 Subject: [PATCH 1/6] fix: deliver node output into the hopper above the node Output previously fell as loose items onto the hopper, so a block above it trapped the items where they despawned while the menu still reported a successful cycle. Insert the output into the hopper inventory as one transaction; a full hopper restores its contents and holds the cycle for retry. The loose drop remains when no hopper is present. --- .../dowsing/utils/ItemDropper.java | 46 +++++++++++++-- .../dowsing/utils/ItemDropperTest.java | 57 +++++++++++++++++++ 2 files changed, 97 insertions(+), 6 deletions(-) diff --git a/src/main/java/net/tfminecraft/dowsing/utils/ItemDropper.java b/src/main/java/net/tfminecraft/dowsing/utils/ItemDropper.java index ed6e2cb..3d17780 100644 --- a/src/main/java/net/tfminecraft/dowsing/utils/ItemDropper.java +++ b/src/main/java/net/tfminecraft/dowsing/utils/ItemDropper.java @@ -1,5 +1,6 @@ package net.tfminecraft.dowsing.utils; +import java.util.Arrays; import java.util.HashMap; import java.util.Map; import java.util.Set; @@ -8,8 +9,12 @@ import java.util.function.Consumer; import org.bukkit.Location; +import org.bukkit.Material; import org.bukkit.Sound; +import org.bukkit.block.Block; +import org.bukkit.block.Hopper; import org.bukkit.entity.Item; +import org.bukkit.inventory.Inventory; import org.bukkit.inventory.ItemStack; import org.bukkit.util.Vector; @@ -29,7 +34,12 @@ public ItemDropper() { this.warning = warning; } - /** Returns false when a configured reward cannot be created, so the cycle can retry. */ + /** + * Delivers output into the hopper above the node when one is present, so blocks + * above the hopper cannot trap and despawn it. Returns false when a configured + * reward cannot be created or the hopper cannot hold the output, so the cycle + * can retry instead of losing items. + */ public boolean dropItems(Node n) { Map items = new HashMap<>(); for(String key : n.getCompleteDrop().keySet()) { @@ -74,16 +84,40 @@ public boolean dropItems(Node n) { } n.update(); Location loc = n.getLoc().clone().add(0.5, 2, 0.5); - for(Map.Entry entry : result.entrySet()) { - for(int count = 0; count < entry.getValue(); count++) { - Item e = loc.getWorld().dropItem(loc, items.get(entry.getKey()).clone()); - e.setVelocity(new Vector()); - loc.getWorld().playSound(loc, Sound.ENTITY_ITEM_PICKUP, 0.5f, 1f); + Block above = n.getLoc().clone().add(0, 1, 0).getBlock(); + if(above != null && above.getType() == Material.HOPPER && above.getState() instanceof Hopper hopper) { + if(!deliverToHopper(n, hopper.getInventory(), items, result, loc)) return false; + } else { + for(Map.Entry entry : result.entrySet()) { + for(int count = 0; count < entry.getValue(); count++) { + Item e = loc.getWorld().dropItem(loc, items.get(entry.getKey()).clone()); + e.setVelocity(new Vector()); + loc.getWorld().playSound(loc, Sound.ENTITY_ITEM_PICKUP, 0.5f, 1f); + } } } n.setLastResult(result); return true; } + /** One transaction: a full hopper keeps its contents and the pending output for a later retry. */ + private boolean deliverToHopper(Node n, Inventory inventory, Map items, Map result, Location loc) { + ItemStack[] before = Arrays.stream(inventory.getContents()) + .map(item -> item == null ? null : item.clone()).toArray(ItemStack[]::new); + for(Map.Entry entry : result.entrySet()) { + ItemStack stack = items.get(entry.getKey()).clone(); + stack.setAmount(entry.getValue()); + if(!inventory.addItem(stack).isEmpty()) { + inventory.setContents(before); + if(unavailableRewardsLogged.add("blocked:" + n.getId())) { + warning.accept("[Dowsing] Cannot complete node " + n.getId() + ": output hopper is full"); + } + return false; + } + loc.getWorld().playSound(loc, Sound.ENTITY_ITEM_PICKUP, 0.5f, 1f); + } + unavailableRewardsLogged.remove("blocked:" + n.getId()); + return true; + } public void dropItem(Location loc, String path, Boolean noV) { ItemCreator ic = new ItemCreator(); ItemStack item = ic.getItemFromPath(path); diff --git a/src/test/java/net/tfminecraft/dowsing/utils/ItemDropperTest.java b/src/test/java/net/tfminecraft/dowsing/utils/ItemDropperTest.java index c589e6b..05de004 100644 --- a/src/test/java/net/tfminecraft/dowsing/utils/ItemDropperTest.java +++ b/src/test/java/net/tfminecraft/dowsing/utils/ItemDropperTest.java @@ -18,9 +18,13 @@ import org.bukkit.Bukkit; import org.bukkit.Location; +import org.bukkit.Material; import org.bukkit.Sound; import org.bukkit.World; +import org.bukkit.block.Block; +import org.bukkit.block.Hopper; import org.bukkit.entity.Item; +import org.bukkit.inventory.Inventory; import org.bukkit.inventory.ItemStack; import org.bukkit.util.Vector; import org.junit.jupiter.api.AfterEach; @@ -145,6 +149,59 @@ void directDropsSkipMissingItemsAndOptionallyClearVelocity() { } } + @Test + void outputGoesToTheHopperAboveTheNodeInsteadOfTheGround() { + World world = mock(World.class); + Location origin = new Location(world, 4, 60, 7); + Node node = nodeAt(origin, Map.of("v.STONE", 1.0), 3); + Inventory inventory = hopperAbove(world); + ItemStack existing = mock(ItemStack.class); + when(existing.clone()).thenReturn(mock(ItemStack.class)); + when(inventory.getContents()).thenReturn(new ItemStack[] {existing, null, null, null, null}); + when(inventory.addItem(any(ItemStack.class))).thenReturn(new HashMap<>()); + ItemStack template = mock(ItemStack.class); + ItemStack delivered = mock(ItemStack.class); + when(template.clone()).thenReturn(delivered); + assertTrue(new ItemDropper(path -> template, message -> fail(message)).dropItems(node)); + verify(delivered).setAmount(3); + verify(inventory).addItem(delivered); + verify(world, never()).dropItem(any(Location.class), any(ItemStack.class)); + verify(world).playSound(new Location(world, 4.5, 62, 7.5), Sound.ENTITY_ITEM_PICKUP, 0.5f, 1f); + verify(node).setLastResult(Map.of("v.STONE", 3)); verify(node).update(); + assertEquals(new Location(world, 4, 60, 7), origin); + } + + @Test + void fullHopperHoldsTheCycleAndRestoresItsContents() { + World world = mock(World.class); + Node node = nodeAt(new Location(world, 4, 60, 7), Map.of("v.STONE", 1.0), 3); + Inventory inventory = hopperAbove(world); + ItemStack[] before = new ItemStack[5]; + when(inventory.getContents()).thenReturn(before); + ItemStack template = mock(ItemStack.class); + ItemStack delivered = mock(ItemStack.class); + when(template.clone()).thenReturn(delivered); + when(inventory.addItem(any(ItemStack.class))).thenReturn(new HashMap<>(Map.of(0, delivered))); + List warnings = new ArrayList<>(); + ItemDropper dropper = new ItemDropper(path -> template, warnings::add); + assertFalse(dropper.dropItems(node)); assertFalse(dropper.dropItems(node)); + verify(inventory, times(2)).setContents(before); + verify(node, never()).setLastResult(any()); + verify(world, never()).dropItem(any(Location.class), any(ItemStack.class)); + assertEquals(1, warnings.size()); assertTrue(warnings.getFirst().contains("output hopper is full")); + } + + private Inventory hopperAbove(World world) { + Block block = mock(Block.class); + Hopper hopper = mock(Hopper.class); + Inventory inventory = mock(Inventory.class); + when(world.getBlockAt(any(Location.class))).thenReturn(block); + when(block.getType()).thenReturn(Material.HOPPER); + when(block.getState()).thenReturn(hopper); + when(hopper.getInventory()).thenReturn(inventory); + return inventory; + } + private Node nodeAt(Location location, Map drops, int yield) { Node node = mock(Node.class); when(node.getId()).thenReturn(UUID.randomUUID()); when(node.getLoc()).thenReturn(location); when(node.getCompleteDrop()).thenReturn(drops); when(node.getYield()).thenReturn(yield); From 617a8a7236dc5a8994cfca9267d4d8c1b4547ae2 Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Thu, 15 Oct 2026 21:22:09 +0000 Subject: [PATCH 2/6] fix: hold rolled node output until the hopper accepts it A full hopper previously left the cycle to be rerolled on each retry, so blocked output could complete empty or skewed toward whatever fitted. Store the rolled result on the node and in its saved JSON until delivery succeeds, and play the delivery sound only after the whole output is inserted. --- .../net/tfminecraft/dowsing/objects/Node.java | 8 +++ .../tfminecraft/dowsing/utils/Database.java | 7 +++ .../dowsing/utils/ItemDropper.java | 60 +++++++++++-------- .../dowsing/objects/NodeBehaviorTest.java | 3 + .../dowsing/utils/DatabaseTest.java | 2 + .../dowsing/utils/ItemDropperTest.java | 60 ++++++++++++++++++- 6 files changed, 114 insertions(+), 26 deletions(-) diff --git a/src/main/java/net/tfminecraft/dowsing/objects/Node.java b/src/main/java/net/tfminecraft/dowsing/objects/Node.java index ed29310..8fad58b 100644 --- a/src/main/java/net/tfminecraft/dowsing/objects/Node.java +++ b/src/main/java/net/tfminecraft/dowsing/objects/Node.java @@ -54,6 +54,7 @@ public class Node { Integer multiplier; Double costIncrease; Map lastResult = new HashMap<>(); + Map pendingResult; List addedDrops = new ArrayList(); List errors = new ArrayList(); Map completeDrop = new HashMap<>(); @@ -151,6 +152,13 @@ public Map getLastResult() { public void setLastResult(Map lastResult) { this.lastResult = lastResult; } + /** Rolled output that could not be delivered yet; null when no cycle output is waiting. */ + public Map getPendingResult() { + return pendingResult; + } + public void setPendingResult(Map pendingResult) { + this.pendingResult = pendingResult; + } public NodeBlock getBlock() { return block; } diff --git a/src/main/java/net/tfminecraft/dowsing/utils/Database.java b/src/main/java/net/tfminecraft/dowsing/utils/Database.java index dbee916..4fa0bba 100644 --- a/src/main/java/net/tfminecraft/dowsing/utils/Database.java +++ b/src/main/java/net/tfminecraft/dowsing/utils/Database.java @@ -8,6 +8,7 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.TreeMap; import java.util.UUID; import java.util.logging.Level; @@ -38,6 +39,7 @@ @SuppressWarnings("deprecation") // Stored configuration text uses Bukkit legacy color codes. public class Database { + static final String PENDING_RESULT_KEY = "pending result"; private JSONObject json = new JSONObject(); // org.json.simple JSONParser parser = new JSONParser(); private File[] listFiles(File directory) { @@ -116,6 +118,10 @@ public void loadNodes() { if(json.containsKey(LastCycleResult.KEY)) { n.setLastResult(LastCycleResult.fromJson(json.get(LastCycleResult.KEY))); } + if(json.containsKey(PENDING_RESULT_KEY)) { + Map pendingResult = LastCycleResult.fromJson(json.get(PENDING_RESULT_KEY)); + if(!pendingResult.isEmpty()) n.setPendingResult(pendingResult); + } NodeManager.nodes.add(n); } catch (Exception ex) { ex.printStackTrace(); @@ -248,6 +254,7 @@ public boolean saveNode(Node n) { defaults.put("active pms", pmArray); defaults.put("efficiency", n.getEfficiency()); defaults.put(LastCycleResult.KEY, LastCycleResult.toJson(n.getLastResult())); + defaults.put(PENDING_RESULT_KEY, LastCycleResult.toJson(n.getPendingResult())); return save(file, defaults); } catch (Throwable ex) { ex.printStackTrace(); diff --git a/src/main/java/net/tfminecraft/dowsing/utils/ItemDropper.java b/src/main/java/net/tfminecraft/dowsing/utils/ItemDropper.java index 3d17780..84609bc 100644 --- a/src/main/java/net/tfminecraft/dowsing/utils/ItemDropper.java +++ b/src/main/java/net/tfminecraft/dowsing/utils/ItemDropper.java @@ -2,6 +2,7 @@ import java.util.Arrays; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.Map; import java.util.Set; import java.util.HashSet; @@ -41,8 +42,10 @@ public ItemDropper() { * can retry instead of losing items. */ public boolean dropItems(Node n) { + Map pending = n.getPendingResult(); + boolean retained = pending != null && !pending.isEmpty(); Map items = new HashMap<>(); - for(String key : n.getCompleteDrop().keySet()) { + for(String key : retained ? pending.keySet() : n.getCompleteDrop().keySet()) { if(key.equalsIgnoreCase("nothing")) continue; ItemStack item = itemFactory.apply(key); if(item == null) { @@ -56,37 +59,45 @@ public boolean dropItems(Node n) { items.put(key, item); } Map result = new HashMap<>(); - Double maxWeight = 0.0; - for(String key : n.getCompleteDrop().keySet()) { - maxWeight = maxWeight+n.getCompleteDrop().get(key); - } - int safety = 0; - Integer dropped = 0; - while(dropped < n.getYield() && safety < n.getYield() * 10) { - safety++; - Double random = Math.random(); - Double previous = 0.0; + if(retained) { + // A held cycle keeps its rolled output; retries must not reroll it. + result = new LinkedHashMap<>(pending); + } else { + Double maxWeight = 0.0; for(String key : n.getCompleteDrop().keySet()) { - if(maxWeight <= 0) { - maxWeight = 1.0; - } - Double chance = n.getCompleteDrop().get(key) / maxWeight; - Double max = previous+chance; - if(random <= max && random > previous) { - dropped++; - if(!key.equalsIgnoreCase("nothing")) { - result.merge(key, 1, Integer::sum); + maxWeight = maxWeight+n.getCompleteDrop().get(key); + } + int safety = 0; + Integer dropped = 0; + while(dropped < n.getYield() && safety < n.getYield() * 10) { + safety++; + Double random = Math.random(); + Double previous = 0.0; + for(String key : n.getCompleteDrop().keySet()) { + if(maxWeight <= 0) { + maxWeight = 1.0; } - break; + Double chance = n.getCompleteDrop().get(key) / maxWeight; + Double max = previous+chance; + if(random <= max && random > previous) { + dropped++; + if(!key.equalsIgnoreCase("nothing")) { + result.merge(key, 1, Integer::sum); + } + break; + } + previous = max; } - previous = max; } } n.update(); Location loc = n.getLoc().clone().add(0.5, 2, 0.5); Block above = n.getLoc().clone().add(0, 1, 0).getBlock(); if(above != null && above.getType() == Material.HOPPER && above.getState() instanceof Hopper hopper) { - if(!deliverToHopper(n, hopper.getInventory(), items, result, loc)) return false; + if(!deliverToHopper(n, hopper.getInventory(), items, result, loc)) { + n.setPendingResult(result); + return false; + } } else { for(Map.Entry entry : result.entrySet()) { for(int count = 0; count < entry.getValue(); count++) { @@ -96,6 +107,7 @@ public boolean dropItems(Node n) { } } } + n.setPendingResult(null); n.setLastResult(result); return true; } @@ -113,9 +125,9 @@ private boolean deliverToHopper(Node n, Inventory inventory, Map(List.of("v.STONE(1)"))); node.setErrors(new ArrayList<>(List.of("broken"))); node.setLastResult(new HashMap<>(Map.of("v.STONE", 2))); + node.setPendingResult(new HashMap<>(Map.of("v.DIRT", 5))); assertEquals(replacement, node.getId()); assertSame(replacementBlock, node.getBlock()); assertSame(moved, node.getLoc()); assertSame(replacementType, node.getCurrentType()); assertSame(f.guild, node.getGuild()); assertFalse(node.getIsActive()); assertEquals(3, node.getLevel()); @@ -78,6 +80,7 @@ void persistedStateAndAccessorsRetainTheRequestedValues() { assertEquals(3, node.getMultiplier()); assertEquals(1.5, node.getCostIncrease()); assertEquals(List.of("v.STONE(1)"), node.getAddedDrops()); assertEquals(List.of("broken"), node.getErrors()); assertEquals(Map.of("v.STONE", 2), node.getLastResult()); + assertEquals(Map.of("v.DIRT", 5), node.getPendingResult()); } @Test diff --git a/src/test/java/net/tfminecraft/dowsing/utils/DatabaseTest.java b/src/test/java/net/tfminecraft/dowsing/utils/DatabaseTest.java index c2c4d4d..ef7ceec 100644 --- a/src/test/java/net/tfminecraft/dowsing/utils/DatabaseTest.java +++ b/src/test/java/net/tfminecraft/dowsing/utils/DatabaseTest.java @@ -118,6 +118,7 @@ Node node(World world, boolean owned) { when(n.getIsActive()).thenReturn(true); when(n.getLevel()).thenReturn(2); when(n.getCycleTime()).thenReturn(3); when(n.getTimeLeft()).thenReturn(4); when(n.getInputCounter()).thenReturn(5); when(n.getEfficiency()).thenReturn(80.0); when(n.getLastResult()).thenReturn(Map.of("v.STONE",4)); when(n.hasGuild()).thenReturn(owned); + when(n.getPendingResult()).thenReturn(Map.of("v.DIRT",2)); Guild guild = mock(Guild.class); when(guild.getId()).thenReturn("guild"); when(n.getGuild()).thenReturn(guild); return n; } @@ -144,6 +145,7 @@ Node node(World world, boolean owned) { types.when(() -> TypeLoader.getByString("type")).thenReturn(loadedType); pms.when(() -> PMLoader.getByString("pm")).thenReturn(pm); db.loadNodes(); assertEquals(1,NodeManager.nodes.size()); verify(slot).setActivePm(pm); verify(NodeManager.nodes.getFirst()).setLastResult(Map.of("v.STONE",4)); + verify(NodeManager.nodes.getFirst()).setPendingResult(Map.of("v.DIRT",2)); NodeManager.nodes.clear(); json.remove("efficiency"); json.remove(LastCycleResult.KEY); json.put("active pms",new JSONArray()); Files.writeString(path,json.toJSONString()); db.loadNodes(); assertEquals(1,NodeManager.nodes.size()); types.when(() -> TypeLoader.getByString("type")).thenReturn(null); NodeManager.nodes.clear(); db.loadNodes(); assertTrue(NodeManager.nodes.isEmpty()); diff --git a/src/test/java/net/tfminecraft/dowsing/utils/ItemDropperTest.java b/src/test/java/net/tfminecraft/dowsing/utils/ItemDropperTest.java index 05de004..316b0d3 100644 --- a/src/test/java/net/tfminecraft/dowsing/utils/ItemDropperTest.java +++ b/src/test/java/net/tfminecraft/dowsing/utils/ItemDropperTest.java @@ -174,23 +174,79 @@ void outputGoesToTheHopperAboveTheNodeInsteadOfTheGround() { @Test void fullHopperHoldsTheCycleAndRestoresItsContents() { World world = mock(World.class); - Node node = nodeAt(new Location(world, 4, 60, 7), Map.of("v.STONE", 1.0), 3); + Node node = nodeAt(new Location(world, 4, 60, 7), Map.of("v.STONE", 1.0, "v.DIRT", 1.0), 3); + Map pending = new LinkedHashMap<>(Map.of("v.STONE", 1, "v.DIRT", 2)); + when(node.getPendingResult()).thenReturn(pending); Inventory inventory = hopperAbove(world); ItemStack[] before = new ItemStack[5]; when(inventory.getContents()).thenReturn(before); ItemStack template = mock(ItemStack.class); ItemStack delivered = mock(ItemStack.class); when(template.clone()).thenReturn(delivered); - when(inventory.addItem(any(ItemStack.class))).thenReturn(new HashMap<>(Map.of(0, delivered))); + // The first reward fits and the second does not, on both attempts. + when(inventory.addItem(any(ItemStack.class))).thenReturn(new HashMap<>(), new HashMap<>(Map.of(0, delivered)), + new HashMap<>(), new HashMap<>(Map.of(0, delivered))); List warnings = new ArrayList<>(); ItemDropper dropper = new ItemDropper(path -> template, warnings::add); assertFalse(dropper.dropItems(node)); assertFalse(dropper.dropItems(node)); verify(inventory, times(2)).setContents(before); + verify(node, times(2)).setPendingResult(pending); + verify(node, never()).setPendingResult(null); verify(node, never()).setLastResult(any()); verify(world, never()).dropItem(any(Location.class), any(ItemStack.class)); + verify(world, never()).playSound(any(Location.class), any(Sound.class), anyFloat(), anyFloat()); assertEquals(1, warnings.size()); assertTrue(warnings.getFirst().contains("output hopper is full")); } + @Test + void blockedOutputIsDeliveredAsRolledOnceTheHopperHasRoom() { + World world = mock(World.class); + Node node = nodeAt(new Location(world, 4, 60, 7), Map.of("v.STONE", 1.0), 3); + when(node.getPendingResult()).thenReturn(null, Map.of("v.STONE", 3)); + Inventory inventory = hopperAbove(world); + when(inventory.getContents()).thenReturn(new ItemStack[5]); + ItemStack template = mock(ItemStack.class); + ItemStack delivered = mock(ItemStack.class); + when(template.clone()).thenReturn(delivered); + when(inventory.addItem(any(ItemStack.class))).thenReturn(new HashMap<>(Map.of(0, delivered)), new HashMap<>()); + ItemDropper dropper = new ItemDropper(path -> template, message -> {}); + assertFalse(dropper.dropItems(node)); + verify(node).setPendingResult(Map.of("v.STONE", 3)); + // A reroll at the new yield would deliver 99 instead of the held 3. + when(node.getYield()).thenReturn(99); + assertTrue(dropper.dropItems(node)); + verify(delivered, times(2)).setAmount(3); verify(delivered, never()).setAmount(99); + verify(node).setPendingResult(null); verify(node).setLastResult(Map.of("v.STONE", 3)); + } + + @Test + void nothingOnlyOutputCompletesWithoutTouchingTheHopper() { + World world = mock(World.class); + Node node = nodeAt(new Location(world, 4, 60, 7), Map.of("nothing", 1.0), 3); + Inventory inventory = hopperAbove(world); + when(inventory.getContents()).thenReturn(new ItemStack[5]); + assertTrue(new ItemDropper(path -> { fail("Nothing must not be resolved"); return null; }, message -> fail(message)).dropItems(node)); + verify(inventory, never()).addItem(any(ItemStack.class)); + verify(world, never()).playSound(any(Location.class), any(Sound.class), anyFloat(), anyFloat()); + verify(node).setPendingResult(null); verify(node).setLastResult(Map.of()); + } + + @Test + void pendingAmountsAboveOneStackAreDeliveredAsOneTransaction() { + World world = mock(World.class); + Node node = nodeAt(new Location(world, 4, 60, 7), Map.of("v.STONE", 1.0), 3); + when(node.getPendingResult()).thenReturn(Map.of("v.STONE", 130)); + Inventory inventory = hopperAbove(world); + when(inventory.getContents()).thenReturn(new ItemStack[5]); + when(inventory.addItem(any(ItemStack.class))).thenReturn(new HashMap<>()); + ItemStack template = mock(ItemStack.class); + ItemStack delivered = mock(ItemStack.class); + when(template.clone()).thenReturn(delivered); + assertTrue(new ItemDropper(path -> template, message -> fail(message)).dropItems(node)); + verify(delivered).setAmount(130); verify(inventory).addItem(delivered); + verify(node).setPendingResult(null); verify(node).setLastResult(Map.of("v.STONE", 130)); + } + private Inventory hopperAbove(World world) { Block block = mock(Block.class); Hopper hopper = mock(Hopper.class); From e99449b3458701fb24e72b88201beed086d9e372 Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Thu, 15 Oct 2026 21:25:33 +0000 Subject: [PATCH 3/6] fix: cancel held node output when its inputs are refunded Deactivation returns the current cycle's inputs, so a held output kept from that cycle would be delivered for free after reactivation. Clear it once refunds drain the pending inputs; incomplete refunds keep it for the retry, and no refund means the output stays paid for. --- .../net/tfminecraft/dowsing/objects/Node.java | 3 ++ .../dowsing/objects/NodeDeactivationTest.java | 29 +++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/src/main/java/net/tfminecraft/dowsing/objects/Node.java b/src/main/java/net/tfminecraft/dowsing/objects/Node.java index 8fad58b..e3f6fab 100644 --- a/src/main/java/net/tfminecraft/dowsing/objects/Node.java +++ b/src/main/java/net/tfminecraft/dowsing/objects/Node.java @@ -503,12 +503,15 @@ public void deActivate() { this.isActive = false; if(isClaimable()) return; if(this.cycleTime >= 0 && this.cycleTime < Cache.cycleLength) { + boolean refunded = this.getInputCounter() > 0; while(this.getInputCounter() > 0) { int pendingInputs = this.getInputCounter(); refund(); // A missing barrel or hopper prevents refunds. Keep the pending inputs for retry. if(this.getInputCounter() >= pendingInputs) break; } + // Returning this cycle's inputs cancels its held output; incomplete refunds keep it. + if(refunded && this.getInputCounter() == 0) setPendingResult(null); } NodeManager.requestNodeBenefitSync(); } diff --git a/src/test/java/net/tfminecraft/dowsing/objects/NodeDeactivationTest.java b/src/test/java/net/tfminecraft/dowsing/objects/NodeDeactivationTest.java index d1ec256..e4f8d68 100644 --- a/src/test/java/net/tfminecraft/dowsing/objects/NodeDeactivationTest.java +++ b/src/test/java/net/tfminecraft/dowsing/objects/NodeDeactivationTest.java @@ -2,8 +2,10 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import java.util.Map; import java.util.UUID; import org.junit.jupiter.api.AfterEach; @@ -101,6 +103,33 @@ void successfulRefundsDrainPendingInputs(int cycleTime) { assertEquals(0, node.getInputCounter()); } + @Test + void completedRefundsCancelHeldOutputButIncompleteRefundsKeepIt() { + RefundNode node = new RefundNode(2, 1); + node.setPendingResult(Map.of("v.STONE", 3)); + + node.deActivate(); + + assertEquals(1, node.getInputCounter()); + assertEquals(Map.of("v.STONE", 3), node.getPendingResult()); + + node.successfulRefundsRemaining = 1; + node.deActivate(); + + assertEquals(0, node.getInputCounter()); + assertNull(node.getPendingResult()); + } + + @Test + void heldOutputSurvivesDeactivationWhenNoInputsAreRefunded() { + RefundNode node = new RefundNode(0, 0); + node.setPendingResult(Map.of("v.STONE", 3)); + + node.deActivate(); + + assertEquals(Map.of("v.STONE", 3), node.getPendingResult()); + } + @Test void noPendingInputsSkipsRefunds() { RefundNode node = new RefundNode(0, 0); From a8d56a096921018ac87951edf06d90673f7a3254 Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Thu, 15 Oct 2026 21:31:25 +0000 Subject: [PATCH 4/6] fix: settle held node output before new cycles and on removal Activation now delivers held output before taking new inputs, so a later deactivation refund cannot cancel output earned by an earlier cycle; a hopper that still cannot hold it blocks activation with an error. Breaking a node drops its held output with the node item, so deletion and guild-loss cleanup return the items instead of discarding them. --- .../net/tfminecraft/dowsing/objects/Node.java | 15 +++++++++ .../dowsing/objects/NodeProductionTest.java | 32 +++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/src/main/java/net/tfminecraft/dowsing/objects/Node.java b/src/main/java/net/tfminecraft/dowsing/objects/Node.java index e3f6fab..ecd5692 100644 --- a/src/main/java/net/tfminecraft/dowsing/objects/Node.java +++ b/src/main/java/net/tfminecraft/dowsing/objects/Node.java @@ -488,6 +488,13 @@ public void activate() { if(failed) { return; } + if(this.pendingResult != null && !this.pendingResult.isEmpty()) { + // Settle held output before new inputs are taken, so a later refund cannot cancel it. + if(!new ItemDropper().dropItems(this)) { + this.errors.add("§7Held output does not fit in the hopper"); + return; + } + } this.isActive = true; this.timeLeft = this.modifiedTime; this.cycleTime = 0; @@ -691,6 +698,14 @@ public void breakNode() { Location loc = new Location(this.getLoc().getWorld(), this.getLoc().getX(), this.getLoc().getY(), this.getLoc().getZ()); loc.add(0.5,0,0.5); ItemDropper dropper = new ItemDropper(); + if(this.pendingResult != null) { + // Removing the node returns its held output instead of discarding it. + for(Map.Entry entry : this.pendingResult.entrySet()) { + for(int count = 0; count < entry.getValue(); count++) { + dropper.dropItem(loc, entry.getKey(), false); + } + } + } dropper.dropItem(loc, path, false); loc.getWorld().playSound(loc, Sound.ENTITY_GLOW_ITEM_FRAME_REMOVE_ITEM, 0.5f, 1f); NodeManager.requestNodeBenefitSync(); diff --git a/src/test/java/net/tfminecraft/dowsing/objects/NodeProductionTest.java b/src/test/java/net/tfminecraft/dowsing/objects/NodeProductionTest.java index 67f8daf..df90586 100644 --- a/src/test/java/net/tfminecraft/dowsing/objects/NodeProductionTest.java +++ b/src/test/java/net/tfminecraft/dowsing/objects/NodeProductionTest.java @@ -5,6 +5,7 @@ import static org.mockito.Mockito.*; import java.util.List; +import java.util.Map; import org.bukkit.Location; import org.bukkit.Material; @@ -102,6 +103,27 @@ void successfulActivationResetsTheCycleConsumesInputsAndClearsOldErrors() { } } + @Test + void activationSettlesHeldOutputBeforeTakingNewInputs() { + Node node = f.owned(); node.setPendingResult(Map.of("v.STONE", 3)); + boolean[] blocked = {true}; + try (var engines = engines(true, true, true, true, true); + var droppers = mockConstruction(ItemDropper.class, (dropper, context) -> when(dropper.dropItems(node)) + .thenAnswer(inv -> { boolean wasBlocked = blocked[0]; blocked[0] = false; return !wasBlocked; }))) { + node.activate(); + assertFalse(node.getIsActive()); + assertEquals(List.of("§7Held output does not fit in the hopper"), node.getErrors()); + assertEquals(Map.of("v.STONE", 3), node.getPendingResult()); + for (NodeEngine engine : engines.constructed()) verify(engine, never()).takeInputs(any()); + node.activate(); + assertTrue(node.getIsActive(), node.getErrors().toString()); + assertEquals(1, node.getInputCounter()); + assertEquals(2, droppers.constructed().size()); + for (ItemDropper dropper : droppers.constructed()) verify(dropper).dropItems(node); + verify(engines.constructed().getLast()).takeInputs(node); + } + } + @Test void unclaimedNodesCannotStartAndPendingRefundsAreRetriedBeforeStarting() { Node node = f.node(); @@ -188,6 +210,16 @@ void breakingAVanillaNodeClearsTheBlockAndReturnsItsItemAtTheBlockCenter() { } } + @Test + void breakingANodeReturnsItsHeldOutputItems() { + Node node = f.node(); node.setPendingResult(Map.of("v.STONE", 2)); + try (var droppers = mockConstruction(ItemDropper.class)) { + node.breakNode(); + verify(droppers.constructed().getFirst(), times(2)).dropItem(new Location(f.world, 2.5, 64, 3.5), "v.STONE", false); + verify(droppers.constructed().getFirst()).dropItem(new Location(f.world, 2.5, 64, 3.5), "v.IRON_BLOCK", false); + } + } + @Test void furnitureRemovalSkipsUnrelatedEntitiesAndHandlesItemsAdderLookupFailures() { Node node = f.node(); when(f.block.getBlock()).thenReturn("ia.nodes:iron"); From 8c0833c9c160b58775de27a8c7b2295080860333 Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Thu, 15 Oct 2026 21:34:30 +0000 Subject: [PATCH 5/6] fix: break each node only once Two stale menus could both run the removal path, dropping the node item and its held output twice. Mark the node broken on the first break and consume the held output as it is dropped, so repeated removal from any caller is a no-op. --- .../java/net/tfminecraft/dowsing/objects/Node.java | 10 ++++++++-- .../dowsing/objects/NodeProductionTest.java | 6 +++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/main/java/net/tfminecraft/dowsing/objects/Node.java b/src/main/java/net/tfminecraft/dowsing/objects/Node.java index ecd5692..c2a9725 100644 --- a/src/main/java/net/tfminecraft/dowsing/objects/Node.java +++ b/src/main/java/net/tfminecraft/dowsing/objects/Node.java @@ -55,6 +55,7 @@ public class Node { Double costIncrease; Map lastResult = new HashMap<>(); Map pendingResult; + boolean broken; List addedDrops = new ArrayList(); List errors = new ArrayList(); Map completeDrop = new HashMap<>(); @@ -685,6 +686,9 @@ public Integer getNaturalYieldFromChunk(String resource, Location l) throws IOEx return 0; } public void breakNode() { + // Stale menus can repeat removal; the node item and held output must drop only once. + if(this.broken) return; + this.broken = true; String path = this.getBlock().getBlock(); String type = path.split("\\.")[0]; if(type.equalsIgnoreCase("v")) { @@ -698,9 +702,11 @@ public void breakNode() { Location loc = new Location(this.getLoc().getWorld(), this.getLoc().getX(), this.getLoc().getY(), this.getLoc().getZ()); loc.add(0.5,0,0.5); ItemDropper dropper = new ItemDropper(); - if(this.pendingResult != null) { + Map heldOutput = this.pendingResult; + setPendingResult(null); + if(heldOutput != null) { // Removing the node returns its held output instead of discarding it. - for(Map.Entry entry : this.pendingResult.entrySet()) { + for(Map.Entry entry : heldOutput.entrySet()) { for(int count = 0; count < entry.getValue(); count++) { dropper.dropItem(loc, entry.getKey(), false); } diff --git a/src/test/java/net/tfminecraft/dowsing/objects/NodeProductionTest.java b/src/test/java/net/tfminecraft/dowsing/objects/NodeProductionTest.java index df90586..1358e4a 100644 --- a/src/test/java/net/tfminecraft/dowsing/objects/NodeProductionTest.java +++ b/src/test/java/net/tfminecraft/dowsing/objects/NodeProductionTest.java @@ -211,12 +211,16 @@ void breakingAVanillaNodeClearsTheBlockAndReturnsItsItemAtTheBlockCenter() { } @Test - void breakingANodeReturnsItsHeldOutputItems() { + void breakingANodeReturnsItsHeldOutputItemsOnlyOnce() { Node node = f.node(); node.setPendingResult(Map.of("v.STONE", 2)); try (var droppers = mockConstruction(ItemDropper.class)) { + node.breakNode(); + assertNull(node.getPendingResult()); + // A stale second menu repeating the removal must not drop anything again. node.breakNode(); verify(droppers.constructed().getFirst(), times(2)).dropItem(new Location(f.world, 2.5, 64, 3.5), "v.STONE", false); verify(droppers.constructed().getFirst()).dropItem(new Location(f.world, 2.5, 64, 3.5), "v.IRON_BLOCK", false); + assertEquals(1, droppers.constructed().size()); } } From 311a7b58f29d18772153e556f59575452b38ad0e Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Thu, 15 Oct 2026 21:41:49 +0000 Subject: [PATCH 6/6] fix: make broken nodes terminal for stale menus A node removed by validation could still be deactivated from a stale menu, refunding its inputs after its held output had already dropped. Broken nodes now reject activation, deactivation and refunds, and inventory clicks for nodes no longer in the registry close the stale menu without acting. Menu test fixtures now register their node, matching the production invariant. --- .../dowsing/managers/NodeManager.java | 6 ++++++ .../net/tfminecraft/dowsing/objects/Node.java | 4 ++++ .../dowsing/managers/NodeManagerEventsTest.java | 16 ++++++++++++++-- .../dowsing/managers/NodeRefundMenuTest.java | 7 +++++++ .../dowsing/objects/NodeProductionTest.java | 16 ++++++++++++++++ 5 files changed, 47 insertions(+), 2 deletions(-) diff --git a/src/main/java/net/tfminecraft/dowsing/managers/NodeManager.java b/src/main/java/net/tfminecraft/dowsing/managers/NodeManager.java index c6f9cc5..d306a58 100644 --- a/src/main/java/net/tfminecraft/dowsing/managers/NodeManager.java +++ b/src/main/java/net/tfminecraft/dowsing/managers/NodeManager.java @@ -429,6 +429,12 @@ public void invenClick(InventoryClickEvent e) { Player p = (Player) e.getWhoClicked(); if(!currentNode.containsKey(p)) return; Node n = currentNode.get(p); + if(!nodes.contains(n)) { + // The node was already removed; stale menus must not act on it. + currentNode.remove(p); + p.closeInventory(); + return; + } InventoryManager inv = new InventoryManager(); if(e.getView().getTitle().equalsIgnoreCase("§7"+n.getBlock().getResource()+ " Node")) { e.setCancelled(true); diff --git a/src/main/java/net/tfminecraft/dowsing/objects/Node.java b/src/main/java/net/tfminecraft/dowsing/objects/Node.java index c2a9725..a2cada5 100644 --- a/src/main/java/net/tfminecraft/dowsing/objects/Node.java +++ b/src/main/java/net/tfminecraft/dowsing/objects/Node.java @@ -418,6 +418,7 @@ public void input() { ng.takeInputs(this); } public void refund() { + if(this.broken) return; NodeEngine ng = new NodeEngine(); Boolean failed = false; this.errors.clear(); @@ -440,6 +441,7 @@ public void refund() { ng.refund(this);; } public void activate() { + if(this.broken) return; if(hasPendingRefund()) { deActivate(); return; @@ -509,6 +511,8 @@ public boolean hasPendingRefund() { } public void deActivate() { this.isActive = false; + // A broken node is terminal: stale menus must not refund or restart it. + if(this.broken) return; if(isClaimable()) return; if(this.cycleTime >= 0 && this.cycleTime < Cache.cycleLength) { boolean refunded = this.getInputCounter() > 0; diff --git a/src/test/java/net/tfminecraft/dowsing/managers/NodeManagerEventsTest.java b/src/test/java/net/tfminecraft/dowsing/managers/NodeManagerEventsTest.java index 27a9154..4dbfa0e 100644 --- a/src/test/java/net/tfminecraft/dowsing/managers/NodeManagerEventsTest.java +++ b/src/test/java/net/tfminecraft/dowsing/managers/NodeManagerEventsTest.java @@ -193,13 +193,24 @@ void unrelatedInventoryClicksAreIgnored() { verify(event, never()).setCancelled(true); } + @Test + void staleMenusCannotActOnRemovedNodes() { + InventoryClickEvent event = f.click("§7Confirm Action", 11); + f.manager.confirm.put(f.player, ConfirmType.DEACTIVATE); + NodeManager.nodes.remove(f.node); + f.manager.invenClick(event); + verify(f.player).closeInventory(); + assertFalse(f.manager.currentNode.containsKey(f.player)); + verify(f.node, never()).deActivate(); verify(f.node, never()).breakNode(); + } + @ParameterizedTest @ValueSource(strings = {"§7Iron Node", "§7Iron Node: Fuel", "§7Iron Node: Type", "§7Confirm Action"}) void everyManagedMenuCancelsBottomInventoryClicksAndRemovesOrphanNodes(String title) { f.manager.currentSlot.put(f.player, f.slot); InventoryClickEvent event = f.click(title, 0); when(event.getClickedInventory()).thenReturn(mock(Inventory.class)); f.manager.invenClick(event); verify(event).setCancelled(true); verify(f.node, never()).breakNode(); - when(f.node.hasGuild()).thenReturn(false); NodeManager.nodes.add(f.node); + when(f.node.hasGuild()).thenReturn(false); event = f.click(title, 0); f.manager.invenClick(event); verify(f.node).breakNode(); verify(f.player).closeInventory(); assertTrue(NodeManager.nodes.isEmpty()); } @@ -361,7 +372,7 @@ void confirmationMenusSupportCancelAcceptAndAbsentActions() { f.manager.confirm.put(f.player, ConfirmType.DELETE_NODE); f.manager.invenClick(f.click("§7Confirm Action", 15)); verify(f.lastInventory()).nodeView(f.player, f.node); f.manager.invenClick(f.click("§7Confirm Action", 0)); verify(f.node, never()).breakNode(); - NodeManager.nodes.add(f.node); f.manager.invenClick(f.click("§7Confirm Action", 11)); + f.manager.invenClick(f.click("§7Confirm Action", 11)); verify(f.node).breakNode(); assertTrue(NodeManager.nodes.isEmpty()); assertFalse(f.manager.confirm.containsKey(f.player)); } @@ -567,6 +578,7 @@ PlayerInteractEvent interact(Action action) { } InventoryClickEvent click(String title, int index) { manager.currentNode.put(player, node); + if (!NodeManager.nodes.contains(node)) NodeManager.nodes.add(node); InventoryClickEvent event = mock(InventoryClickEvent.class); InventoryView view = mock(InventoryView.class); when(event.getWhoClicked()).thenReturn(player); when(event.getView()).thenReturn(view); when(view.getTitle()).thenReturn(title); when(view.getTopInventory()).thenReturn(inventory); when(event.getClickedInventory()).thenReturn(inventory); when(event.getSlot()).thenReturn(index); return event; diff --git a/src/test/java/net/tfminecraft/dowsing/managers/NodeRefundMenuTest.java b/src/test/java/net/tfminecraft/dowsing/managers/NodeRefundMenuTest.java index 5a963b0..ae2bb81 100644 --- a/src/test/java/net/tfminecraft/dowsing/managers/NodeRefundMenuTest.java +++ b/src/test/java/net/tfminecraft/dowsing/managers/NodeRefundMenuTest.java @@ -15,6 +15,7 @@ import org.bukkit.event.inventory.InventoryClickEvent; import org.bukkit.inventory.Inventory; import org.bukkit.inventory.InventoryView; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -54,6 +55,12 @@ void setUp() { when(click.getClickedInventory()).thenReturn(top); when(view.getTopInventory()).thenReturn(top); manager.currentNode.put(player, node); + NodeManager.nodes.add(node); + } + + @AfterEach + void tearDown() { + NodeManager.nodes.remove(node); } @ParameterizedTest diff --git a/src/test/java/net/tfminecraft/dowsing/objects/NodeProductionTest.java b/src/test/java/net/tfminecraft/dowsing/objects/NodeProductionTest.java index 1358e4a..9759761 100644 --- a/src/test/java/net/tfminecraft/dowsing/objects/NodeProductionTest.java +++ b/src/test/java/net/tfminecraft/dowsing/objects/NodeProductionTest.java @@ -224,6 +224,22 @@ void breakingANodeReturnsItsHeldOutputItemsOnlyOnce() { } } + @Test + void brokenNodesCannotActivateDeactivateOrRefund() { + Node node = f.owned(); node.setIsActive(true); node.setInputCounter(2); + node.setPendingResult(Map.of("v.STONE", 1)); + try (var droppers = mockConstruction(ItemDropper.class)) { + node.breakNode(); + } + try (var engines = engines(true, true, true, true, true)) { + node.activate(); + node.deActivate(); + node.refund(); + assertFalse(node.getIsActive()); assertEquals(2, node.getInputCounter()); + assertTrue(engines.constructed().isEmpty()); + } + } + @Test void furnitureRemovalSkipsUnrelatedEntitiesAndHandlesItemsAdderLookupFailures() { Node node = f.node(); when(f.block.getBlock()).thenReturn("ia.nodes:iron");