Skip to content

fix: stop cut-off guild capitals vouching for themselves, and add land admin commands - #199

Merged
Drefvelin merged 1 commit into
mainfrom
fix/enclave-sea-and-admin-land
Oct 20, 2026
Merged

Drefvelin merged 1 commit into
mainfrom
fix/enclave-sea-and-admin-land

Conversation

@Drefvelin

@Drefvelin Drefvelin commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Enclaves no longer keep themselves alive through their own lake. When a guild capital loses its land link to the faction capital, it is supposed to lose its seat, and the land it held drops. But the sea check behind that rule started its flood from every owned province, including the cut-off pocket. Any enclave touching water therefore counted as "reachable by sea".

On Main this is why Sunsora's The Cicatrix (province 306) kept its seat after staff unclaimed the land bridge. 306 and 310 both border water tile 791, a one-tile lake with no link to the rest of Sunsora. So the pocket 306, 310, 333, 334 and 335 stayed claimed.

Guild capitals now have to trace back to the faction capital:

  • over owned land (same as before), or
  • by water from land already traced to the capital. A traced colony can anchor the next colony, so overseas chains still work.

Sea-mode claims (founding a guild capital) now sail from that same traced land, not from the enclave. Realms with no seated faction capital keep the old rule, because they have no capital to trace back to.

I ran the old and new rules against every faction on Main. Only Sunsora changes: Cicatrix loses its seat, and 306, 310, 333, 334 and 335 drop the next time Sunsora's claims are checked.

New staff commands (simplefactions.admin):

  • /faction forceclaim <faction>: gives the province you stand in to that faction. It skips prestige, the untitled-province cap and the border rules. If another realm owns the province, it goes through the normal steal path: installations transfer, the old owner unclaims it, and the old owner's claims are rechecked. It refuses faction capitals, sea tiles and civil-war-locked realms. If the province isn't connected to the receiving realm, it warns that the land will be dropped at the next recheck.
  • /guild forceremovecapital <guild>: unseats a guild's capital, which dissolves its settlement the usual way. It then rechecks the realm's claims and lists the provinces that were cut off. It refuses the faction capital and civil-war-locked realms.

Both commands have tab completion.

Test plan

  • Full mvn verify locally. Only the two known local-only failures; jacoco shows no new fully-missed lines.
  • New ProvinceHandlerEnclaveTest: a lake enclave loses its seat and land; a sea colony keeps its seat; a colony anchors a second colony; two enclaves on one lake can't vouch for each other; sea claims can't sail from an enclave; a realm without a capital keeps the old rule; an inland guild capital still needs a land link; the preview leaves out an enclave capital.
  • New AdminLandServiceTest, plus command and tab-completion coverage.
  • Dev e2e with a bot staff character. I staged Main's situation on Dev: Sunsora also holding 306, 310, 333, 334 and 335, with The Crescent Sanctum seated at 306.
    • /faction unclaim sunsora on 293 started the recheck. 306 and 310 were then unowned: forceclaim gave them to The Village without taking them from Sunsora, with the "not connected" warning.
    • forceclaim on 241 took it from Sunsora and gave it to The Village, with no warning because it borders their land.
    • forceclaim on Sunsora's capital 226 was refused.
    • /guild forceremovecapital Knights_of_Saint_Nordbaum unseated 231. Running it again says the guild has no capital. Running it on the base guild Sunsora was refused.
    • Dev data and the 4.12.0 jar were restored afterwards.

🤖 Generated with Claude Code

…in commands

A guild capital that lost its land link to the faction capital stayed legal
whenever it touched any water, because the sea check started from every owned
province, including the cut-off pocket itself. It now has to trace back to the
faction capital: over owned land, or by water from land already traced. Sea-mode
claims set sail from that same traced land.

Adds /faction forceclaim <faction> and /guild forceremovecapital <guild> for staff.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added the size:L 100-499 effective changed lines (tests excluded in mixed PRs). label Oct 20, 2026
@coderabbitai

coderabbitai Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 886c70e8-a203-417f-8da3-e315909c77e3
📥 Commits

Reviewing files that changed from the base of the PR and between af268f6 and d699d1e.

📒 Files selected for processing: 8
  • src/main/java/net/tfminecraft/simplefactions/managers/CommandManager.java
  • src/main/java/net/tfminecraft/simplefactions/map/AdminLandService.java
  • src/main/java/net/tfminecraft/simplefactions/objects/handler/ProvinceHandler.java
  • src/main/java/net/tfminecraft/simplefactions/utils/TabCompletion.java
  • src/test/java/net/tfminecraft/simplefactions/managers/CommandManagerCoverageTest.java
  • src/test/java/net/tfminecraft/simplefactions/managers/TabCompletionCoverageTest.java
  • src/test/java/net/tfminecraft/simplefactions/map/AdminLandServiceTest.java
  • src/test/java/net/tfminecraft/simplefactions/objects/handler/ProvinceHandlerEnclaveTest.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.

📝 Summary

Summary by CodeRabbit

  • New Features

    • Added administrator commands to force faction province claims and remove guild capitals.
    • Added validation and feedback for invalid provinces, factions, guilds, capitals, sea regions and civil-war restrictions.
    • Improved province claim legality using capital-based land and sea connectivity.
    • Updated claim previews and revalidation to reflect connected territories.
    • Added tab-completion suggestions for the new administrator commands.
  • Tests

    • Expanded coverage for administrative actions, claim validation, capital removal, connectivity and enclave scenarios.

Province claim legality now uses connected capital and sea routes. The change adds administrator commands to force faction claims and remove guild capitals, with tab-completion support and tests for the new behaviours.

Changes: Province land management

Layer / File(s) Summary
Capital connectivity and claim legality
src/main/java/net/tfminecraft/simplefactions/objects/handler/ProvinceHandler.java, src/test/java/net/tfminecraft/simplefactions/objects/handler/ProvinceHandlerEnclaveTest.java
Claim checks, legal-province calculations, and previews use shared guild-capital and sea-source calculations. Tests cover enclave and colony capital validation, sea connectivity, and preview filtering.
Forced claims and capital removal
src/main/java/net/tfminecraft/simplefactions/map/AdminLandService.java, src/test/java/net/tfminecraft/simplefactions/map/AdminLandServiceTest.java
AdminLandService adds forced province claims and guild-capital removal, including validation, ownership changes, and claim revalidation. Tests cover rejection conditions and reported outcomes.
Administrator commands and suggestions
src/main/java/net/tfminecraft/simplefactions/managers/CommandManager.java, src/main/java/net/tfminecraft/simplefactions/utils/TabCompletion.java, src/test/java/net/tfminecraft/simplefactions/managers/CommandManagerCoverageTest.java, src/test/java/net/tfminecraft/simplefactions/managers/TabCompletionCoverageTest.java
CommandManager adds /faction forceclaim and /guild forceremovecapital. Tab completion suggests the commands and eligible guild IDs. Tests cover access checks, delegation, and suggestion filtering.

Priority: ➖ Normal

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

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Administrator
  participant CommandManager
  participant AdminLandService
  Administrator->>CommandManager: Run forceclaim or forceremovecapital
  CommandManager->>AdminLandService: Delegate validated command
  AdminLandService-->>CommandManager: Return service messages
  CommandManager-->>Administrator: Send messages
Loading

Suggested reviewers: ryanbarlow97

Merge Risk: ⚪ Minimal · up to d699d

Guild capitals must now connect to the faction capital over owned land or through water reached from already-connected land. Two new admin commands let staff force a province claim for a faction and remove a guild capital. No concrete defects were found in the claim checks, the forced-claim and capital-removal logic, or the command and tab-completion wiring. The change appears ready to merge after normal checks. The reported in-game end-to-end test on the dev server remains outstanding.

- [ ] 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

Comment @coderabbitai help to get the list of available commands.

@Drefvelin
Drefvelin merged commit 07ef4fa into main Oct 20, 2026
3 checks passed
@Drefvelin
Drefvelin deleted the fix/enclave-sea-and-admin-land branch October 20, 2026 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 effective changed lines (tests excluded in mixed PRs).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant