Skip to content

Estimate what a hub or installation would earn - #112

Merged
Drefvelin merged 1 commit into
mainfrom
infra-8
Oct 3, 2026
Merged

Drefvelin merged 1 commit into
mainfrom
infra-8

Conversation

@Drefvelin

Copy link
Copy Markdown
Contributor

Summary

  • Adds a daily what-if for a new supply hub: the operator's gain, each host-realm guild's change, and the hub's taxable income. Tax and fee are arithmetic on that result, so changing terms does not rerun the map.
  • Where no railway exists, the estimate uses a straight-line rail link and records that distance. The cached list is split into installations with a free slot and provinces worth a station, closest first, capped per group.
  • A realm headline turns infrastructure off for one extra run, and an installation preview adds that kind's source in one province. Both run on a province snapshot. The daily pass runs after income and the agreement tick, off the server thread, then restores live trade.

Test plan

  • JAVA_TOOL_OPTIONS="-Dorg.sqlite.tmpdir=/root/.cache/sqlite-native" mvn -o verify (2712 tests, 0 failures)
  • CodeRabbit review
  • Menus that read this cache are the next pull request, not this one

Made with Cursor

Menus can rank destinations and split tax and fee without running the map again, and the daily pass keeps that list off the server thread.

Co-authored-by: Cursor <cursoragent@cursor.com>
@coderabbitai

coderabbitai Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

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: f111d82a-7c61-46cd-9248-4864d889d000
📥 Commits

Reviewing files that changed from the base of the PR and between d97273f and 156d928.

📒 Files selected for processing (8)
  • src/main/java/net/tfminecraft/simplefactions/Cache.java
  • src/main/java/net/tfminecraft/simplefactions/guild/hub/HubEstimates.java
  • src/main/java/net/tfminecraft/simplefactions/guild/hub/SupplyHubService.java
  • src/main/java/net/tfminecraft/simplefactions/guild/income/EconomicPreview.java
  • src/main/java/net/tfminecraft/simplefactions/managers/FactionManager.java
  • src/main/java/net/tfminecraft/simplefactions/managers/ProvinceManager.java
  • src/main/resources/config.yml
  • src/test/java/net/tfminecraft/simplefactions/guild/hub/HubEstimatesTest.java

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


📝 Summary

Summary by CodeRabbit

  • New Features
    • Added daily estimates of guild supply-hub destinations, including ready installations and potential construction sites, with projected income and infrastructure value.
    • Added installation previews and estimates that account for applicable taxes and fees.
    • Added a configurable limit for the number of candidate destinations considered; the default is 24.

Walkthrough

The pull request adds daily estimates for supply hub destinations and realm infrastructure value. It ranks ready and proposed destinations, compares projected income, applies tax and fee terms, and supports installation previews.

Changes

Supply hub estimates

Layer / File(s) Summary
Projection and snapshot support
src/main/java/net/tfminecraft/simplefactions/guild/income/EconomicPreview.java, src/main/java/net/tfminecraft/simplefactions/managers/ProvinceManager.java, src/test/java/net/tfminecraft/simplefactions/guild/hub/HubEstimatesTest.java
Economic previews return projected nets for eligible guilds. Province manager snapshots can suppress infrastructure or add extra infrastructure sources. Tests cover infrastructure suppression and extra sources.
Destination selection and valuation
src/main/java/net/tfminecraft/simplefactions/Cache.java, src/main/java/net/tfminecraft/simplefactions/guild/hub/HubEstimates.java, src/main/java/net/tfminecraft/simplefactions/guild/hub/SupplyHubService.java, src/main/resources/config.yml, src/test/java/net/tfminecraft/simplefactions/guild/hub/HubEstimatesTest.java
HubEstimates ranks ready and proposed destinations, calculates projected income and infrastructure value, and applies tax and fee terms. Configuration sets the candidate limit, with a default of 24 and a minimum of 1. Tests cover selection, valuation, previews, and comparison performance.
Daily estimate rebuilding
src/main/java/net/tfminecraft/simplefactions/guild/hub/HubEstimates.java, src/main/java/net/tfminecraft/simplefactions/managers/FactionManager.java, src/test/java/net/tfminecraft/simplefactions/guild/hub/HubEstimatesTest.java
The daily rollover schedules guarded asynchronous estimate rebuilding. The rebuild replaces cached estimates and schedules a live province recalculation on completion. Tests cover scheduling without a plugin and candidate configuration defaults.

Priority: ⬇️ Low

Estimated code review effort: 4 (Complex) | ~60 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant FactionManager
  participant HubEstimates
  participant ProvinceManager
  participant EconomicPreview
  participant MainScheduler
  FactionManager->>HubEstimates: Schedule daily estimate rebuild
  HubEstimates->>ProvinceManager: Create province snapshot
  HubEstimates->>EconomicPreview: Project guild nets
  EconomicPreview-->>HubEstimates: Return projected nets
  HubEstimates->>MainScheduler: Schedule live province recalculation
  MainScheduler->>ProvinceManager: Recalculate live provinces
Loading

Suggested reviewers: ryanbarlow97

Merge Risk: 🔵 Low · up to 156d9

Daily hub estimates are rebuilt in the background. If a player upgrades a guild while that rebuild is copying province data, the refresh can fail and the previous day's estimates stay in place. Live income and trade data are not corrupted, so this is safe to merge with awareness, but capturing the province snapshot on the server thread would make the refresh reliable.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 156d9

Background estimates read data that can change during calculation, creating a risk of failed refreshes or inconsistent results across realms. Ordinary failures retain previous estimates. No new permission bypass or balance-changing capability was established, but cancellation and concurrent-state behavior remain incompletely verified.

Retained concerns

  • Low · reliability · inferred: The new background refresh crosses live-state ownership without a consistent capture boundary. Concurrent trade recalculation can interrupt copying or produce mixed-state projections. Because results are published only after the complete rebuild, one such failure can leave estimates for all participating guilds at their previous generation. Failure containment protects the existing cache but does not isolate the refresh from server-thread writers.
Security review details

Security Blast Radius

  • inferred — The demonstrated failure scope is the shared estimate refresh for participating guilds and realms in one plugin instance. Publication follows the complete rebuild, so a copying failure can preserve old estimates globally. Wider service, tenant, credential, or persistent-data exposure was not established.

Security Findings and Attack Paths

  • inferred — The supported adverse path is concurrent live trade mutation during asynchronous capture, causing stale or mixed-state estimates. The evidence does not establish permission escalation, a balance-settlement exploit, or the privileges necessary to deliberately trigger this timing condition.

Trust Boundaries and Controls

  • observed — Observed estimate callers apply hypothetical infrastructure and links to copied managers. EconomicPreview opens and finally removes a thread-local scratch context, and Guild routes trade-breakdown access to that context. This contradicts documentation suggesting that these projections write live trade breakdowns. Snapshot-only infrastructure setters rely on caller discipline rather than enforcing manager identity themselves.

Resilience and Maintainability Implications

  • inferred — Immutable cache publication and identity-checked main-thread recovery contain ordinary computation failures. They do not supply a consistent economic-state generation or prove cleanup when a queued task is cancelled before execution; those remain lifecycle assurance gaps rather than verified security failures.

Hardening Proposals

  • proposed — Capture consistent, immutable economic inputs under the live-state owner's control before dispatching background work, and reuse that captured generation for baseline and candidate calculations. This would strengthen ownership and refresh failure containment without treating estimates as authoritative transaction inputs.
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@Drefvelin
Drefvelin merged commit cc905ce into main Oct 3, 2026
2 checks passed
@Drefvelin
Drefvelin deleted the infra-8 branch October 3, 2026 18:16
@Drefvelin Drefvelin mentioned this pull request Oct 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants