fix: do not lose island progress when the server restarts - #18
Merged
Conversation
Ported from AOneBlock, which this addon is forked from and shares the bug with verbatim - same code, same line numbers. See BentoBoxWorld/AOneBlock#550. A player breaking blocks and then sitting through a restart would come back to their block count rolled back to the last checkpoint - up to 49 blocks of progress gone, repeatedly, on every restart. The shutdown save was queued, not written. onDisable() called saveCache(), which uses saveObjectAsync(), and this addon is a Pladdon: the server disables it before BentoBox, so the write landed in a queue that BentoBox had to drain on its way out. BentoBox 3.22.0 added that drain (flushAll), but every earlier version discarded it silently. Write directly on shutdown instead, via a new saveCacheNow() using saveObjectNow(). That removes the dependency on core behaviour entirely rather than relying on a specific BentoBox version getting it right. saveObjectNow() is BentoBox 3.22.0 API, so bump the dependency and raise api-version to match. Without the api-version bump the addon would still load on an older core and throw NoSuchMethodError at shutdown, which is worse than the bug being fixed. It now refuses to load with "Please update BentoBox". Also make the periodic save interval configurable as island.save-every, defaulting to 10 rather than the hardcoded 50. Nothing helps if the server is SIGKILLed, but this caps what an unclean kill can cost at 9 blocks. Test notes: the BentoBox bump broke the same 5 PhasesPanelTest tests it broke in AOneBlock, at the same line numbers. None were regressions - all called when(user.getTranslation(...)) on a real User rather than a mock, which stubs nothing on the User and instead attaches to whichever mock the real method last touched. 3.22.0's getTranslation(World, ...) calls getIWM().getAddon() first, moving the target. Ported the stubTranslation() helper that stubs the LocalesManager these actually read from. The same pattern remains elsewhere in that class and is worth a follow-up sweep. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014t1DSo2wMbTWZLcwXpwUmQ
BentoBox 3.18.0 onwards is compiled for Java 25 (Minecraft 26.x), so its class files are version 69. A JDK 21 javac cannot parse those at all, and the build dies with "class file has wrong version 69.0, should be 65.0" against every BentoBox type before it reaches any of our code. This only surfaces once the dependency moves to 3.22.0, as it does in this branch. It did not show up locally because the dev machine is already on JDK 25 - the compiler reads the newer class files happily and <release>21</release> still emits Java 21 bytecode, which is what the addon ships. The addon's own target is unchanged: still Java 21 via <release> in the pom. Only the JDK doing the compiling moves. Also moves setup-java to v4 and the 'adopt' distribution to 'temurin', since neither v3 nor adopt offers a Java 25 build. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014t1DSo2wMbTWZLcwXpwUmQ
|
Merged
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.
Ported from AOneBlock, which this addon is forked from. ChunkBlock carries the bug verbatim — same code, same line numbers. See BentoBoxWorld/AOneBlock#550 for the original report.
The bug
Reported on Discord (against AOneBlock): a player breaks 30 blocks, the server restarts while they are still online, and they come back with those 30 blocks un-broken. Every restart, repeatedly.
The shutdown save was queued rather than written.
ChunkBlock.onDisable()calledsaveCache(), which usessaveObjectAsync(). This addon is a Pladdon, so the server disables it before BentoBox — the write went into a queue that BentoBox had to drain on its way out. BentoBox 3.22.0 added that drain (AbstractDatabaseHandler.flushAll()), but every earlier version discarded it silently.With the shutdown save lost, the count fell back to the last periodic checkpoint, which was a hardcoded every-50-blocks. Break fewer than 50 between restarts and nothing persists at all.
The fix
Write directly on shutdown. New
BlockListener.saveCacheNow()usingsaveObjectNow(), called fromonDisable(). This removes the dependency on core behaviour rather than relying on a particular BentoBox version getting it right.saveCache()is unchanged and still used foronReload(), where async is correct.island.save-every, default 10 (was a hardcodedSAVE_EVERY = 50). Nothing helps if the server isSIGKILLed — there is no shutdown path to run — but this caps what an unclean kill can cost at 9 blocks instead of 49.getSaveEvery()clamps to ≥1 since it is a modulo divisor.saveObjectNow()is 3.22.0 API, sobentobox.versionis bumped andaddon.ymlapi-versiongoes 3.13.0 → 3.22.0.The api-version bump is required, not cosmetic. Without it the addon would still load on an older core and throw
NoSuchMethodErrorat shutdown — worse than the bug being fixed. With it, BentoBox refuses to load the addon and saysNOTE: Please update BentoBox.Verification
Full suite: 620 tests, 0 failures.
Scope caveat: unlike the AOneBlock PR, this port was not boot-tested on a real server. The equivalent AOneBlock change was verified on two (BentoBox 3.15.1 refuses to load it with a clear message and no linkage error; 3.22.1-SNAPSHOT enables, runs and disables cleanly), and the code here is identical, but ChunkBlock itself has only been checked against its test suite.
Test fallout
The BentoBox bump broke the same 5
PhasesPanelTesttests it broke in AOneBlock, at the same line numbers. None were real regressions. All calledwhen(user.getTranslation(...))on a realUser(fromUser.getInstance(mockPlayer)), which stubs nothing on the User — Mockito attaches the stub to whichever mock the real method last touched. 3.22.0'sgetTranslation(World, ...)callsgetIWM().getAddon(world)first, moving the target.Ported the
stubTranslation()helper that stubs theLocalesManagerthese actually read from.Follow-up worth doing: roughly a dozen more
when(user.getTranslation(...))calls remain in that class, passing today by the same accident. They will break on some future BentoBox internal change.🤖 Generated with Claude Code
https://claude.ai/code/session_014t1DSo2wMbTWZLcwXpwUmQ