Skip to content

refactor!: remove obsolete account-addressed economy APIs - #40

Merged
ryanbarlow97 merged 1 commit into
mainfrom
stage7/account-api
Oct 14, 2026
Merged

ryanbarlow97 merged 1 commit into
mainfrom
stage7/account-api

Conversation

@ryanbarlow97

@ryanbarlow97 ryanbarlow97 commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Account-addressed compatibility methods resolve whichever character is active when a callback runs. Remove the 16 obsolete balance/mutation/accessor signatures and LegacyCalls after auditing their consumers; callers use Denars with a captured CharacterRef instead.

Keep OfflineModifier name lookup, the join listener, Database.hasPlayerData readiness, ledger totals/rankings, legacy PlayerData/Account readers, imports, heirs and receipts. The retained readiness method now uses the same Bukkit player lookup and Denars.active check directly. No persisted format changes.

Removed APIs: OfflineModifier.apply/change/balance; MoneyManager.doTaxes/addMoney/getBalance/changeBal/addMoneyToAccount; PlayerManager.get(Player/UUID); DenarEconomy.getPlayerManager; Database.savePlayerData/loadPlayerData/getPlayerBalance; LegacyCalls. This is an intentional breaking public API change for the next minor release.

Validation:

  • Java 21 tfmc-mvn -o -B clean verify: 265 tests, no failures/errors/skips; existing 100% line/branch/instruction checks pass.
  • Refreshed all 41 active source repositories. No external production or obsolete reflection callers. The one obsolete negative SF mock verification is removed in the coordinated SF change.
  • Constant-pool/reflection/config census covered 89 distinct proposed/installed/private JARs with no external DE wrapper consumers.
  • Actual published RPCharacters 2.20.0 wallet binds this candidate; captured-character debit/refund after A-to-B switch, duplicate IDs and ledger reopen preserve balances and money totals.

Paired Docs PR: TF-Minecraft/Docs#240

Rollout remains gated on verified 6c Dev activation and the reviewed stopped cohort. Publish merged numeric releases after review; preserve the complete displaced 6c rollback set and all migration/history data. Main is excluded.

Coordinated SF consumer/test change: TF-Minecraft/SimpleFactions#166

@github-actions github-actions Bot added the size:L 100-499 effective changed lines (tests excluded in mixed PRs). label Oct 14, 2026
@coderabbitai

coderabbitai Bot commented Oct 14, 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: f950d155-02c0-436f-b2cd-5f841cec43b1
📥 Commits

Reviewing files that changed from the base of the PR and between c6cdc86 and e4b3305.

📒 Files selected for processing: 9
  • src/main/java/net/tfminecraft/denareconomy/DenarEconomy.java
  • src/main/java/net/tfminecraft/denareconomy/accounts/LegacyCalls.java
  • src/main/java/net/tfminecraft/denareconomy/accounts/OfflineModifier.java
  • src/main/java/net/tfminecraft/denareconomy/database/Database.java
  • src/main/java/net/tfminecraft/denareconomy/managers/MoneyManager.java
  • src/main/java/net/tfminecraft/denareconomy/managers/PlayerManager.java
  • src/test/java/net/tfminecraft/denareconomy/accounts/LegacyCallsTest.java
  • src/test/java/net/tfminecraft/denareconomy/accounts/OfflineModifierTest.java
  • src/test/java/net/tfminecraft/denareconomy/database/DatabaseTest.java
💤 Files with no reviewable changes: 6
  • src/test/java/net/tfminecraft/denareconomy/accounts/LegacyCallsTest.java
  • src/test/java/net/tfminecraft/denareconomy/accounts/OfflineModifierTest.java
  • src/main/java/net/tfminecraft/denareconomy/managers/MoneyManager.java
  • src/main/java/net/tfminecraft/denareconomy/DenarEconomy.java
  • src/main/java/net/tfminecraft/denareconomy/managers/PlayerManager.java
  • src/main/java/net/tfminecraft/denareconomy/accounts/LegacyCalls.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

  • Bug Fixes

    • Player-data readiness now requires the player to be online and have an active character.
  • Refactor

    • Removed legacy account balance, payment and player-data access methods. Account operations now rely on the current character-based system.
  • Tests

    • Added coverage for player-data readiness, character-bound payments, account totals and rankings, and remembered player names.

The change removes deprecated compatibility accessors and account operations. It updates the player-data readiness check and adds database tests for readiness, character-bound payments, ledger totals, rankings, and join-time name remembering.

Changes: Account API and database behaviour

Layer / File(s) Summary
Remove legacy account APIs
src/main/java/net/tfminecraft/denareconomy/DenarEconomy.java, src/main/java/net/tfminecraft/denareconomy/accounts/LegacyCalls.java, src/main/java/net/tfminecraft/denareconomy/accounts/OfflineModifier.java, src/main/java/net/tfminecraft/denareconomy/managers/MoneyManager.java, src/main/java/net/tfminecraft/denareconomy/managers/PlayerManager.java, src/test/java/net/tfminecraft/denareconomy/accounts/*
Removes deprecated account operations and snapshot accessors. Remembered-name lookup remains. Removes the legacy account tests and narrows the OfflineModifier tests to name persistence and lookup.
Check online character readiness
src/main/java/net/tfminecraft/denareconomy/database/Database.java, src/test/java/net/tfminecraft/denareconomy/database/DatabaseTest.java
Database.hasPlayerData returns true only when the UUID resolves to an online player with an active character. Tests cover readiness for null, unknown, offline, and characterless players, plus test setup and teardown.
Test payments and ledger results
src/test/java/net/tfminecraft/denareconomy/database/DatabaseTest.java
Tests cover payments using captured characters, ledger-based server totals, top-balance results, and name remembering on join.

Priority: ➖ Normal

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

Change: Refactor

Merge Risk: ⚪ Minimal · up to e4b33

No actionable merge-blocking issue is established; the change is mergeable after normal checks.

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

@ryanbarlow97
ryanbarlow97 merged commit b4771ad into main Oct 14, 2026
3 checks passed
@ryanbarlow97
ryanbarlow97 deleted the stage7/account-api branch October 14, 2026 09:07
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