[XxFran10xX]

What this changes

Performance only. Players see no difference.

The particle pass (NodeManager.particleCycle, every 5 ticks) and the upkeep total that SimpleFactions reads asked for the same things several times per node. Each is now asked once per pass or per call.

Where	Before	After
Particle pass, per claimed node	The node's guild looked up 3 times: once to see if the node is claimable, twice more inside Node.check	Looked up once and handed to Node.check(Guild)
Count at the start of the pass	Guild looked up twice per node (hasGuild, then getGuild)	Once
Node.canHold in the pass	Worked out for every node: leader's character lookup, member count, node count, capacity	Worked out once per guild per pass, because it depends only on the guild
Node.canHold, leader lookup	Always done first	Done only for a guild that is short of members or over capacity
getTotalUpkeep, getNodeAmount	Guild looked up twice per node	Once

Why

A spark profile of Main (60 players, one hour; 72,000 ms per hour is 1 ms per tick) showed:

• The particle pass: 6.5 s/h.
  • Node.check 4.1 s: canHold 3.1 s (leader lookup through Members.player into TLibs Characters.info 1.7 s, capacity 1.0 s), and isClaimable → resolveGuild 0.6 s.
  • beginCountPass 1.7 s, of which hasGuild → resolveGuild → FactionManager.getGuildByString 0.85 s.
  • isClaimable → resolveGuild called straight from the pass: 0.6 s.
• NodeManager.getTotalUpkeep (SimpleFactions' Ledger.liveNodeUpkeep): 2.7 s/h, mostly hasGuild / getGuild → resolveGuild. getNodeAmount has the same pattern, about 1.1 s/h.

The profile is from before #31, so its capacity line was getNodeCapacity; the same check now uses getHeldNodeCapacity.

Why the result is the same

• Nothing is remembered between passes. Both pass memos are created in beginCountPass and dropped in endCountPass, so a deleted or renamed guild and a node that changes owner are seen on the next pass exactly as before.
• The lookups that were merged ran back to back with nothing in between that could change the answer.
• canHold reads only the guild: held realm, leader, member count, node count, capacity. Within one pass these change only when a node is let go, which lowers the count and resyncs realms. released clears the memo at that point, so the next node of that guild is judged afresh, as before.
• The canHold reorder returns the same value: held, or within member and capacity limits, or led by an online admin. Only the order of the last two changed, and both are plain reads.
• While SimpleFactions is not ready the pass still does nothing for a node, as before.

Left as it is

• A pass still looks each claimed node's guild up twice: once for the count and once for the check. Reusing the first for the second would need its own "a node was let go" invalidation, for about 0.4 s/h that the SimpleFactions-side change to getGuildByString removes anyway.
• getTotalUpkeep(g) still looks up the guild of every active node, once each. Skipping the other guilds' nodes without a lookup would rely on how SimpleFactions matches ids.
• For the few nodes that do belong to the guild, getDailyUpkeep still resolves the guild and its capacity per node.
• The 30 s realm check and the one-minute production loop use the same hasGuild then getGuild pattern, but they were not in the profile.

How it was tested

New and adjusted unit tests:

• A pass over real nodes makes 2 guild lookups per claimed node and works canHold out once for a guild with two nodes.
• Within a pass canHold is worked out once per guild, again after a node is let go, and nothing is kept once the pass ends.
• A guild within its limits keeps its nodes without its leader being looked up; a guild short of members does have it looked up.
• While SimpleFactions is not ready a pass neither checks nodes nor shows the claimable particle.
• Node.check makes one guild lookup.
• Existing tests that stubbed hasGuild() on mock nodes now stub getGuild(), which the counting code reads.

mvn -B clean verify on Windows with JDK 21: 258 tests, 250 pass, 8 fail. The same 8 fail on origin/main on this PC and are not touched by this change. They write files and fail with AccessDeniedException in DurableFiles.write:

• DatabaseTest: jsonAccessorsAndSavePreserveTypedDefaultsAndExistingValues, snapshotSavesBeforePruningAndPreservesUnknownFiles
• NodePaymentsTest: conflictingTargetStateAndInvalidJournalNeverPost, conflictsAndChangedOwnersRemainBlockedForStaff, crashAfterPostingReappliesTheSavedTargetAndNeverIncrementsAgain, failedCapacitySaveIsReplayedWithSamePayment, refusalLeavesStateWhileUnavailableBlocksNewIntentAndRecoversSameId, successfulChangesPersistExactLevelCapacityAndRefund

Because of them the local JaCoCo gate reports 30 missed lines, all in NodePayments.java, Database.java and DurableFiles.java. Node.java and NodeManager.java have no missed lines. The Linux CI run is the real check of the 100% gate.

Not measured on a live server. The saving is in lookup counts; a new spark profile after the next release will show the time.

🤖 Generated with Claude Code

[coderabbitai]

No actionable comments were generated in the recent review.

Summary

Summary by CodeRabbit

• Bug Fixes
  • Improved node ownership checks so guild membership and node capacity are considered before leadership status.
  • Nodes without a guild remain available to be claimed, and unowned nodes are no longer checked for loss of control.
  • Guild ownership is re-evaluated when a node is released, helping capacity changes take effect promptly.
• Visual Updates
  • Unowned nodes now display happy-villager particles during the particle cycle.

Node eligibility checks now accept a guild value, and particle passes reuse guild lookups and held-state results. The particle cycle handles unowned nodes directly and skips checks when SimpleFactionsApi is unavailable.

Changes: Guild node eligibility and particle cycles

Layer / File(s)	Summary
Guild eligibility and node checks src/main/java/net/tfminecraft/dowsing/objects/Node.java, src/test/java/net/tfminecraft/dowsing/objects/NodeBehaviorTest.java	Node.canHold applies member and capacity checks before checking for an online admin leader. Node.check() delegates to an overload that accepts a guild. Tests cover guild resolution and leader lookups.
Node counts and held-state caching src/main/java/net/tfminecraft/dowsing/managers/NodeManager.java, src/test/java/net/tfminecraft/dowsing/managers/NodeManagerCyclesTest.java	getTotalUpkeep and getNodeAmount use one guild lookup per node. Particle passes cache held-state results by guild identity and clear the cache when a node is released or the pass ends. Tests cover cache reuse and re-evaluation.
Particle-cycle guild handling src/main/java/net/tfminecraft/dowsing/managers/NodeManager.java, src/test/java/net/tfminecraft/dowsing/managers/NodeManagerCyclesTest.java	The particle cycle skips work when SimpleFactionsApi is unavailable, emits particles for nodes without a guild, and passes the retrieved guild to Node.check(g). Tests cover lookup counts and readiness handling.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Refactor

Possibly related PRs

• TF-Minecraft/Dowsing#28: Both changes update the per-pass cache lifecycle and clear state when a node is released.
• TF-Minecraft/Dowsing#25: Both changes modify the held-guild eligibility decision used by the per-pass cache.
• TF-Minecraft/Dowsing#31: Both changes use held-node capacity in the node-retention check.

Suggested reviewers: ryanbarlow97, drefvelin

Merge Risk: Minimal · up to 2d807

This change reduces repeated guild and capacity lookups in the particle pass and upkeep calculations without intended behavior changes, and no actionable merge-blocking risk was identified. The author reports pre-existing local test failures, so a Linux CI run remains the normal check.

- [ ] Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 2bc855f5-873c-4963-8fb9-721610c624af

Commits

Reviewing files that changed from the base of the PR and between 695d64e and 2d80759.

Files selected for processing: 4

• src/main/java/net/tfminecraft/dowsing/managers/NodeManager.java
• src/main/java/net/tfminecraft/dowsing/objects/Node.java
• src/test/java/net/tfminecraft/dowsing/managers/NodeManagerCyclesTest.java
• src/test/java/net/tfminecraft/dowsing/objects/NodeBehaviorTest.java

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

[XxFran10xX]

@coderabbitai review

[coderabbitai]

Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.