fix: register economy command permissions and save synchronously on shutdown (1.19.2) - #63
Merged
Merged
Conversation
…hutdown Two fixes, released as 1.19.2. Economy command permissions were never declared. The commands set permissions like "invswitcher.balance", which CompositeCommand prefixes with the parent game mode's permission prefix, giving e.g. "bskyblock.invswitcher.balance". addon.yml had no permissions section, so those nodes were never registered with a default and hasPermission() was false for every non-op player - /<gm> balance and /<gm> pay failed with a permission error. Declare them in addon.yml using BentoBox's [gamemode] placeholder, which AddonsManager expands into every game mode's prefix. User commands default to true, admin eco commands to op. Shutdown saves are now synchronous. BentoBox closes its database immediately after addons are disabled and only kicks players afterwards, so the async write issued from saveOnShutdown() lost the race and was silently dropped, and the PlayerQuitEvent that would otherwise save fired after the database was closed. Everything a player did since their last world change went unsaved on server stop, and because onPlayerJoin re-applies the stored inventory, the stale snapshot then overwrote their real inventory on the next login. Route both save paths through a persist() helper that writes synchronously when shutting down. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0183f7bfWoU9rmXxRCRwvrAs
JaCoCo 0.8.12 cannot read class file major version 69, so instrumenting the classes Mockito generates at runtime failed with "Error while instrumenting ... Unsupported class file major version 69" whenever the build ran on a JDK 25 toolchain - which is what the CodeMC CI agent (OpenJDK_25) uses. 0.8.15 adds Java 25 class file support. Verified on JDK 25: mvn test now runs all 128 tests green with the agent attached, and jacoco:report analyzes the bundle and writes the report. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0183f7bfWoU9rmXxRCRwvrAs
|
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two fixes, released as 1.19.2.
Economy command permissions were never declared
Reported in testing: a normal player running
/<gamemode> balancegot a permission error for<gamemode>.invswitcher.balance, even though InvSwitcher was the economy provider.BalanceCommand.setup()callssetPermission("invswitcher.balance"). Because the command is attached to the game mode's player command,CompositeCommandprepends the parent's permission prefix, producing e.g.bskyblock.invswitcher.balance. Butaddon.ymlhad nopermissions:section at all, so that node was never registered with a default — and an undeclared node meanshasPermission()is false for everyone but ops.Fixed by declaring the nodes in
addon.ymlusing BentoBox's[gamemode]placeholder, whichAddonsManager.registerPermission()expands into every game mode's permission prefix (the same pattern Level and Bank use):[gamemode].invswitcher.balancetrue[gamemode].invswitcher.paytrue[gamemode].invswitcher.admin.eco(+.balance/.give/.take/.set)opThe commands themselves are already gated correctly —
registerEconomyCommands()only runs whenoptions.moneyis enabled and Vault is installed, and only for game modes whose overworld InvSwitcher manages. So with money off, the commands simply do not exist.Shutdown saves are now synchronous
BentoBox closes its database immediately after addons are disabled and only kicks players afterwards, so the async write issued from
saveOnShutdown()lost the race and was silently dropped. ThePlayerQuitEventthat would otherwise have saved them fired after the database was already closed.The visible effect: everything a player did since their last world change went unsaved when the server stopped, and because
PlayerListener.onPlayerJoinre-applies the stored inventory on login, the stale snapshot then overwrote their real inventory — players were rolled back to their last world change.Both save paths (including the early-returning statistics branch) now route through a
persist()helper that writes synchronously when shutting down and stays async otherwise.Testing
mvn test— 128 tests, all passing. Three newStoreTestcases cover the shutdown path: synchronous save with and without statistics enabled, and normal saves staying asynchronous.🤖 Generated with Claude Code
https://claude.ai/code/session_0183f7bfWoU9rmXxRCRwvrAs