diff --git a/src/main/java/net/tfminecraft/dowsing/managers/NodeManager.java b/src/main/java/net/tfminecraft/dowsing/managers/NodeManager.java index 78717b9..0ffa8cd 100644 --- a/src/main/java/net/tfminecraft/dowsing/managers/NodeManager.java +++ b/src/main/java/net/tfminecraft/dowsing/managers/NodeManager.java @@ -162,8 +162,10 @@ public static double getTotalUpkeep(Guild g) { int[] amount = {-1}; java.util.function.IntSupplier count = () -> amount[0] >= 0 ? amount[0] : (amount[0] = getNodeAmount(g)); for (Node n : nodes) { - if (!n.getIsActive() || !n.hasGuild()) continue; - if (!n.getGuild().getId().equalsIgnoreCase(g.getId())) continue; + if (!n.getIsActive()) continue; + // One lookup of the node's guild answers both whether it has one and which it is. + Guild owner = n.getGuild(); + if (owner == null || !owner.getId().equalsIgnoreCase(g.getId())) continue; total += n.getDailyUpkeep(count); } return total; @@ -174,9 +176,10 @@ public static Integer getNodeAmount(Guild g) { if(g == null) return i; if(passCounts != null) return g.getId() == null ? i : passCounts.getOrDefault(g.getId(), i); for(Node n : nodes) { - if(!n.hasGuild()) continue; + Guild owner = n.getGuild(); + if(owner == null) continue; if(n.getBlock().isSpecial()) continue; - if(n.getGuild().getId().equalsIgnoreCase(g.getId())) i++; + if(owner.getId().equalsIgnoreCase(g.getId())) i++; } return i; } @@ -186,28 +189,44 @@ public static Integer getNodeAmount(Guild g) { * nodes x nodes guild lookups. Counted once per pass instead, and kept right by {@link #released}. */ private static Map passCounts; + /** + * Whether each guild checked so far in one particle pass can hold its nodes. {@link Node#canHold} + * asks only about the guild, so all its nodes get the same answer until one is let go; that + * changes the count and resyncs realms, so {@link #released} forgets the answers. + */ + private static Map passHolds; static void beginCountPass() { Map counts = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); for(Node n : nodes) { - if(!n.hasGuild()) continue; + Guild owner = n.getGuild(); + if(owner == null) continue; if(n.getBlock().isSpecial()) continue; - String id = n.getGuild().getId(); + String id = owner.getId(); if(id != null) counts.merge(id, 1, Integer::sum); } passCounts = counts; + passHolds = new IdentityHashMap<>(); } static void endCountPass() { passCounts = null; + passHolds = null; } /** A node of {@code g} was just let go during a particle pass, so its guild holds one fewer. */ public static void released(Guild g, Node n) { + if(passHolds != null) passHolds.clear(); if(passCounts == null || g == null || g.getId() == null || n.getBlock().isSpecial()) return; passCounts.computeIfPresent(g.getId(), (id, count) -> count - 1); } + /** {@link Node#canHold} for a node's check: worked out once per guild during a particle pass. */ + public static boolean canHold(Node n, Guild g) { + if(passHolds == null) return n.canHold(g); + return passHolds.computeIfAbsent(g, n::canHold); + } + /** Room for new nodes, from the guild's active members. */ public static Integer getNodeCapacity(Guild g) { return capacityFor(g, net.tfminecraft.dowsing.utils.ActiveMembers.count(g)); @@ -324,11 +343,15 @@ public void run() beginCountPass(); try { for(Node n : nodes){ - if(n.isClaimable()){ + if(!SimpleFactionsApi.ready()) continue; + // Looked up once here and handed to the check, not once to see if the + // node is claimable and twice more inside the check. + Guild g = n.getGuild(); + if(g == null){ Location loc = n.getLoc().clone().add(0.5, 1, 0.5); loc.getWorld().spawnParticle(Particle.HAPPY_VILLAGER, loc, 10); } else { - n.check(); + n.check(g); } } } finally { diff --git a/src/main/java/net/tfminecraft/dowsing/objects/Node.java b/src/main/java/net/tfminecraft/dowsing/objects/Node.java index c16bdbd..fd2ceea 100644 --- a/src/main/java/net/tfminecraft/dowsing/objects/Node.java +++ b/src/main/java/net/tfminecraft/dowsing/objects/Node.java @@ -305,12 +305,13 @@ Guild resolveGuild() { public boolean canHold(Guild g){ if(g == null) return false; if(held(g)) return true; - Player p = Members.player(g.getLeader()); - if(p != null && p.isOnline() && Permissions.isAdmin(p)) return true; // Holding counts every member, active or not; only new claims need active ones. - if(g.getMembers().size() < Cache.minMembersForNode) return false; - if(NodeManager.getNodeAmount(g)-NodeManager.getHeldNodeCapacity(g) > 0) return false; - return true; + if(g.getMembers().size() >= Cache.minMembersForNode + && NodeManager.getNodeAmount(g)-NodeManager.getHeldNodeCapacity(g) <= 0) return true; + // Short of members or over capacity, only an online admin leader keeps the nodes. Asked last: + // it reads the leader's character, which a guild within its limits never needs. + Player p = Members.player(g.getLeader()); + return p != null && p.isOnline() && Permissions.isAdmin(p); } /** @@ -413,10 +414,16 @@ Integer getNaturalYieldFromChunk(NodeBlock b, Location l) throws NumberFormatExc public void check() { if (!SimpleFactionsApi.ready()) return; - if(isClaimable()) return; - Guild g = resolveGuild(); + check(resolveGuild()); + } + + /** + * {@link #check()} for a caller that has just resolved this node's guild, so the particle pass + * looks each node's guild up once instead of three times. Null is an unclaimed node. + */ + public void check(Guild g) { if(g == null) return; - if(canHold(g)) return; + if(NodeManager.canHold(this, g)) return; Player p = Members.player(g.getLeader()); if(p != null && p.isOnline()) { p.sendMessage("§cYou lost control of the "+block.getResource()+" Node §c!"); diff --git a/src/test/java/net/tfminecraft/dowsing/managers/NodeManagerCyclesTest.java b/src/test/java/net/tfminecraft/dowsing/managers/NodeManagerCyclesTest.java index 79f0dc8..88ed2a1 100644 --- a/src/test/java/net/tfminecraft/dowsing/managers/NodeManagerCyclesTest.java +++ b/src/test/java/net/tfminecraft/dowsing/managers/NodeManagerCyclesTest.java @@ -137,7 +137,7 @@ void countsCapacityAndUpkeepRespectOwnershipSpecialNodesAndPurchasedLimits() { assertEquals(1, NodeManager.getNodeCapacity(null)); assertEquals(1, NodeManager.getNodeCapacity(f.guild)); NodeManager.nodes.add(f.node); when(f.node.getIsActive()).thenReturn(true); when(f.node.getDailyUpkeep(any(java.util.function.IntSupplier.class))).thenReturn(2.25); Node inactive = f.nodeAt(f.location), unowned = f.nodeAt(f.location), other = f.nodeAt(f.location); - when(unowned.hasGuild()).thenReturn(false); when(unowned.getIsActive()).thenReturn(true); + when(unowned.getGuild()).thenReturn(null); when(unowned.getIsActive()).thenReturn(true); Guild otherGuild = mock(Guild.class); when(otherGuild.getId()).thenReturn("other"); when(other.getGuild()).thenReturn(otherGuild); when(other.getIsActive()).thenReturn(true); NodeManager.nodes.addAll(List.of(inactive, unowned, other)); assertEquals(2, NodeManager.getNodeAmount(f.guild)); assertEquals(2.25, NodeManager.getTotalUpkeep(f.guild)); @@ -222,7 +222,7 @@ void cachingReloadsBlockTypeAndPreviouslySelectedProductionMethods() { @Test void aParticlePassCountsEachGuildOnceAndTracksNodesLetGo() { Node second = f.nodeAt(f.location), unowned = f.nodeAt(f.location), other = f.nodeAt(f.location), special = f.nodeAt(f.location), noId = f.nodeAt(f.location); - when(unowned.hasGuild()).thenReturn(false); + when(unowned.getGuild()).thenReturn(null); Guild otherGuild = mock(Guild.class); when(otherGuild.getId()).thenReturn("OTHER"); when(other.getGuild()).thenReturn(otherGuild); NodeBlock specialBlock = mock(NodeBlock.class); when(specialBlock.isSpecial()).thenReturn(true); when(special.getBlock()).thenReturn(specialBlock); Guild idless = mock(Guild.class); when(noId.getGuild()).thenReturn(idless); @@ -233,7 +233,7 @@ void aParticlePassCountsEachGuildOnceAndTracksNodesLetGo() { try { assertEquals(2, NodeManager.getNodeAmount(f.guild)); assertEquals(1, NodeManager.getNodeAmount(otherGuild)); assertEquals(0, NodeManager.getNodeAmount(idless)); assertEquals(0, NodeManager.getNodeAmount(null)); - when(f.node.hasGuild()).thenReturn(false); + when(f.node.getGuild()).thenReturn(null); assertEquals(2, NodeManager.getNodeAmount(f.guild), "counted once at the start of the pass"); NodeManager.released(f.guild, f.node); assertEquals(1, NodeManager.getNodeAmount(f.guild)); NodeManager.released(f.guild, special); NodeManager.released(null, f.node); NodeManager.released(idless, f.node); @@ -266,15 +266,68 @@ void aRealCheckThatLetsANodeGoLowersItsGuildsCountForTheRestOfThePass() { } } + @Test + void aGuildsRightToItsNodesIsWorkedOutOncePerPassAndAgainAfterOneIsLetGo() { + Cache.extraCapacity = false; NodeManager.extraCapacityByGuild.put("guild", 1); + Node first = new Node(UUID.randomUUID(), f.block, f.location, f.guild, true, 1, 0, f.type, 60, 0, 25); + Node second = new Node(UUID.randomUUID(), f.block, f.location, f.guild, true, 1, 0, f.type, 60, 0, 25); + Node third = new Node(UUID.randomUUID(), f.block, f.location, f.guild, true, 1, 0, f.type, 60, 0, 25); + NodeManager.nodes.addAll(List.of(first, second)); + clearInvocations(f.guild); + first.check(); second.check(); + verify(f.guild, times(2)).isHeld(); + NodeManager.beginCountPass(); + try { + first.check(); second.check(); + verify(f.guild, times(3)).isHeld(); + } finally { + NodeManager.endCountPass(); + } + NodeManager.nodes.add(third); + NodeManager.beginCountPass(); + try { + first.check(); second.check(); third.check(); + // Over capacity: the first node goes, then the guild is judged afresh, once for the other two. + verify(f.guild, times(5)).isHeld(); + assertTrue(first.isClaimable()); assertFalse(second.isClaimable()); assertFalse(third.isClaimable()); + } finally { + NodeManager.endCountPass(); + } + second.check(); + verify(f.guild, times(6)).isHeld(); + } + + @Test + void aParticlePassLooksUpEachNodesGuildOnceToCountAndOnceToCheck() { + Cache.extraCapacity = false; NodeManager.extraCapacityByGuild.put("guild", 1); + Node first = new Node(UUID.randomUUID(), f.block, f.location, f.guild, true, 1, 0, f.type, 60, 0, 25); + Node second = new Node(UUID.randomUUID(), f.block, f.location, f.guild, true, 1, 0, f.type, 60, 0, 25); + Node unclaimed = new Node(UUID.randomUUID(), f.block, f.location, null, false, 1, 0, f.type, 60, 0, 25); + NodeManager.nodes.addAll(List.of(first, second, unclaimed)); + try (Cycles cycles = new Cycles()) { + f.manager.start(); + f.factions.clearInvocations(); clearInvocations(f.guild); + cycles.tasks.get(5L).run(); + f.factions.verify(() -> FactionManager.getGuildByString("guild"), times(4)); + verify(f.guild, times(1)).isHeld(); + verify(f.world, times(1)).spawnParticle(Particle.HAPPY_VILLAGER, new Location(f.world, 2.5, 65, 3.5), 10); + assertFalse(first.isClaimable()); assertFalse(second.isClaimable()); + } + } + @Test void timersUseTheirIntendedCadenceAndRunParticlesChecksAndEfficiencyGrowth() { Node claimed = f.node, unclaimed = f.nodeAt(f.location); - when(unclaimed.isClaimable()).thenReturn(true); NodeManager.nodes.addAll(List.of(claimed, unclaimed)); + when(unclaimed.getGuild()).thenReturn(null); NodeManager.nodes.addAll(List.of(claimed, unclaimed)); try (Cycles cycles = new Cycles()) { f.manager.start(); assertEquals(java.util.Set.of(5L, 600L, 1200L, 72000L), cycles.tasks.keySet()); - cycles.tasks.get(5L).run(); verify(claimed).check(); + cycles.tasks.get(5L).run(); verify(claimed).check(f.guild); verify(unclaimed, never()).check(any()); verify(f.world).spawnParticle(Particle.HAPPY_VILLAGER, new Location(f.world, 2.5, 65, 3.5), 10); cycles.tasks.get(72000L).run(); verify(claimed).growEfficiency(); verify(unclaimed).growEfficiency(); + // While SimpleFactions is not ready a pass neither checks nodes nor marks them claimable. + net.tfminecraft.dowsing.testsupport.FactionFixture.api.when(net.tfminecraft.simplefactions.api.SimpleFactionsApi::ready).thenReturn(false); + cycles.tasks.get(5L).run(); verify(claimed, times(1)).check(f.guild); + verify(f.world, times(1)).spawnParticle(Particle.HAPPY_VILLAGER, new Location(f.world, 2.5, 65, 3.5), 10); } } diff --git a/src/test/java/net/tfminecraft/dowsing/objects/NodeBehaviorTest.java b/src/test/java/net/tfminecraft/dowsing/objects/NodeBehaviorTest.java index d9e6511..26cd07e 100644 --- a/src/test/java/net/tfminecraft/dowsing/objects/NodeBehaviorTest.java +++ b/src/test/java/net/tfminecraft/dowsing/objects/NodeBehaviorTest.java @@ -206,10 +206,20 @@ void guildNoticesReachOnlyOnlineMembersOfTheOwningGuild() { } @Test - void deletedOwnerDuringResolutionLeavesTheNodeClaimable() { + void aCheckLooksItsOwnerUpOnceAndADeletedOwnerLeavesTheNodeClaimable() { Node node = f.owned(); f.factions.when(() -> FactionManager.getGuildByString("guild")).thenReturn(f.guild).thenReturn(null); - node.check(); assertTrue(node.isClaimable()); + node.check(); assertSame(f.guild, node.guild, "one lookup per check"); + node.check(); assertNull(node.guild); assertTrue(node.isClaimable()); + } + + @Test + void aGuildWithinItsLimitsKeepsItsNodesWithoutItsLeaderBeingLookedUp() { + Node node = f.owned(); + clearInvocations(f.guild); + assertTrue(node.canHold(f.guild)); verify(f.guild, never()).getLeader(); + Cache.minMembersForNode = 3; + assertFalse(node.canHold(f.guild)); verify(f.guild).getLeader(); } @Test