From f991a0077594810c47fc99a6384a84fb4b0bdd65 Mon Sep 17 00:00:00 2001 From: tastybento Date: Sat, 8 Aug 2026 12:35:11 -0700 Subject: [PATCH 1/2] fix: do not lose island progress when the server restarts 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) Claude-Session: https://claude.ai/code/session_014t1DSo2wMbTWZLcwXpwUmQ --- pom.xml | 2 +- .../world/bentobox/chunkblock/ChunkBlock.java | 5 +- .../world/bentobox/chunkblock/Settings.java | 26 ++++++++++ .../chunkblock/listeners/BlockListener.java | 21 +++++--- src/main/resources/addon.yml | 2 +- src/main/resources/config.yml | 5 ++ .../bentobox/chunkblock/SettingsTest.java | 29 +++++++++++ .../listeners/BlockListenerTest.java | 39 +++++++++++++- .../chunkblock/panels/PhasesPanelTest.java | 52 ++++++++++++------- 9 files changed, 151 insertions(+), 30 deletions(-) diff --git a/pom.xml b/pom.xml index d3265e0..3d0aeb3 100644 --- a/pom.xml +++ b/pom.xml @@ -56,7 +56,7 @@ 5.11.0 4.110.0 - 3.15.0-SNAPSHOT + 3.22.0 4.0.10 1.8.0 0.0.67 diff --git a/src/main/java/world/bentobox/chunkblock/ChunkBlock.java b/src/main/java/world/bentobox/chunkblock/ChunkBlock.java index b2fb86e..0cf8a60 100644 --- a/src/main/java/world/bentobox/chunkblock/ChunkBlock.java +++ b/src/main/java/world/bentobox/chunkblock/ChunkBlock.java @@ -277,9 +277,10 @@ public boolean loadData() { @Override public void onDisable() { - // save cache + // Save cache. This must be a direct write, not a queued one: the server disables this + // Pladdon before BentoBox, so anything queued here depends on BentoBox draining it later. if (blockListener != null) { - blockListener.saveCache(); + blockListener.saveCacheNow(); } // Stop border rendering and restore client-side blocks diff --git a/src/main/java/world/bentobox/chunkblock/Settings.java b/src/main/java/world/bentobox/chunkblock/Settings.java index f8fd0ec..1fb1c57 100644 --- a/src/main/java/world/bentobox/chunkblock/Settings.java +++ b/src/main/java/world/bentobox/chunkblock/Settings.java @@ -452,6 +452,13 @@ public class Settings implements WorldSettings { @ConfigEntry(path = "island.water-mob-protection") private boolean waterMobProtection = true; + @ConfigComment("How often island progress is written to the database, in blocks broken") + @ConfigComment("Progress is also saved whenever a phase changes, a player logs out and the server shuts down,") + @ConfigComment("so this only decides how much is lost if the server dies without shutting down cleanly.") + @ConfigComment("Lower is safer but writes more often. Minimum is 1 (save every block)") + @ConfigEntry(path = "island.save-every") + private int saveEvery = 10; + @ConfigComment("Default max team size") @ConfigComment("Permission size cannot be less than the default below. ") @ConfigEntry(path = "island.max-team-size") @@ -1933,6 +1940,25 @@ public void setMobWarning(int mobWarning) { this.mobWarning = mobWarning; } + /** + * How many blocks are broken between periodic saves of island progress. + * A value below 1 would make the modulo check throw, so it is clamped. + * @return the saveEvery value, never less than 1 + */ + public int getSaveEvery() { + if (saveEvery < 1) { + saveEvery = 1; + } + return saveEvery; + } + + /** + * @param saveEvery the saveEvery to set + */ + public void setSaveEvery(int saveEvery) { + this.saveEvery = saveEvery; + } + /** * @return the waterMobProtection */ diff --git a/src/main/java/world/bentobox/chunkblock/listeners/BlockListener.java b/src/main/java/world/bentobox/chunkblock/listeners/BlockListener.java index b12cc4e..722fdea 100644 --- a/src/main/java/world/bentobox/chunkblock/listeners/BlockListener.java +++ b/src/main/java/world/bentobox/chunkblock/listeners/BlockListener.java @@ -124,11 +124,6 @@ private record BrushSession(BukkitTask task, Block block) {} */ public static final int MAX_LOOK_AHEAD = 5; - /** - * How often island data is saved to the database (in blocks broken). - */ - public static final int SAVE_EVERY = 50; - /* * Loot tables for suspicious blocks */ @@ -161,11 +156,25 @@ public BlockListener(@NonNull ChunkBlock addon) { /** * Saves all island data from the cache to the database asynchronously. + *

+ * Only safe while the server is running. On shutdown use {@link #saveCacheNow()}. */ public void saveCache() { cache.values().forEach(handler::saveObjectAsync); } + /** + * Saves all island data from the cache to the database on the calling thread. + *

+ * Used on shutdown, where an asynchronous save cannot be retried if it does not complete. + * BentoBox drains writes queued by addons as they are disabled, but this addon is a Pladdon, + * so the server disables it before BentoBox and that drain is the only thing standing between + * a queued block count and a rolled-back island. Writing directly removes the dependency. + */ + public void saveCacheNow() { + cache.values().forEach(handler::saveObjectNow); + } + // --------------------------------------------------------------------- // Section: Listeners // --------------------------------------------------------------------- @@ -448,7 +457,7 @@ private ProcessPhaseResult processPhase(Cancellable e, Island i, OneBlockIslands return new ProcessPhaseResult(phase, true, 0); } handleNewPhase(player, i, is, phase, block, prevPhaseName); - } else if (is.getBlockNumber() % SAVE_EVERY == 0) { + } else if (is.getBlockNumber() % addon.getSettings().getSaveEvery() == 0) { // Periodically save the island's progress. saveIsland(i); } diff --git a/src/main/resources/addon.yml b/src/main/resources/addon.yml index 257373a..1904e32 100755 --- a/src/main/resources/addon.yml +++ b/src/main/resources/addon.yml @@ -1,7 +1,7 @@ name: ChunkBlock main: world.bentobox.chunkblock.ChunkBlock version: ${version}${build.number} -api-version: 3.13.0 +api-version: 3.22.0 metrics: true icon: "STONE" repository: "BentoBoxWorld/ChunkBlock" diff --git a/src/main/resources/config.yml b/src/main/resources/config.yml index 3897aa6..4183285 100644 --- a/src/main/resources/config.yml +++ b/src/main/resources/config.yml @@ -446,6 +446,11 @@ island: mob-warning: 5 # Whether spawned mobs that need water to survive will spawn in a generated water block water-mob-protection: true + # How often island progress is written to the database, in blocks broken + # Progress is also saved whenever a phase changes, a player logs out and the server shuts down, + # so this only decides how much is lost if the server dies without shutting down cleanly. + # Lower is safer but writes more often. Minimum is 1 (save every block) + save-every: 10 # Default max team size # Permission size cannot be less than the default below. max-team-size: 4 diff --git a/src/test/java/world/bentobox/chunkblock/SettingsTest.java b/src/test/java/world/bentobox/chunkblock/SettingsTest.java index bc2a554..30cf8f1 100644 --- a/src/test/java/world/bentobox/chunkblock/SettingsTest.java +++ b/src/test/java/world/bentobox/chunkblock/SettingsTest.java @@ -1747,6 +1747,35 @@ void testSetHologramDuration() { s.setHologramDuration(2345); assertEquals(2345, s.getHologramDuration()); } + + /** + * Test method for {@link world.bentobox.chunkblock.Settings#getSaveEvery()}. + */ + @Test + void testGetSaveEveryDefault() { + assertEquals(10, s.getSaveEvery()); + } + + /** + * Test method for {@link world.bentobox.chunkblock.Settings#setSaveEvery(int)}. + */ + @Test + void testSetSaveEvery() { + s.setSaveEvery(25); + assertEquals(25, s.getSaveEvery()); + } + + /** + * The value is used as a modulo divisor, so anything below 1 has to be clamped or + * the block break handler would throw an ArithmeticException on every block. + */ + @Test + void testGetSaveEveryClampsZeroAndBelow() { + s.setSaveEvery(0); + assertEquals(1, s.getSaveEvery()); + s.setSaveEvery(-50); + assertEquals(1, s.getSaveEvery()); + } diff --git a/src/test/java/world/bentobox/chunkblock/listeners/BlockListenerTest.java b/src/test/java/world/bentobox/chunkblock/listeners/BlockListenerTest.java index 7e4194a..2eb6844 100644 --- a/src/test/java/world/bentobox/chunkblock/listeners/BlockListenerTest.java +++ b/src/test/java/world/bentobox/chunkblock/listeners/BlockListenerTest.java @@ -5,6 +5,8 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import java.io.File; @@ -49,6 +51,8 @@ public class BlockListenerTest extends CommonTestSetup { // Class under test private BlockListener bl; + private AbstractDatabaseHandler h; + @Mock ChunkBlock addon; @Mock @@ -78,13 +82,14 @@ public class BlockListenerTest extends CommonTestSetup { public void setUp() throws Exception { super.setUp(); // This has to be done beforeClass otherwise the tests will interfere with each other - AbstractDatabaseHandler h = mock(AbstractDatabaseHandler.class); + h = mock(AbstractDatabaseHandler.class); // Database MockedStatic mockDb = Mockito.mockStatic(DatabaseSetup.class); DatabaseSetup dbSetup = mock(DatabaseSetup.class); mockDb.when(DatabaseSetup::getDatabase).thenReturn(dbSetup); when(dbSetup.getHandler(any())).thenReturn(h); when(h.saveObject(any())).thenReturn(CompletableFuture.completedFuture(true)); + when(h.saveObjectNow(any())).thenReturn(CompletableFuture.completedFuture(true)); // Addon when(addon.getPlugin()).thenReturn(plugin); @@ -187,4 +192,36 @@ void testOnBlockFromToCenterBlock() { assertTrue(e.isCancelled()); } + /** + * Test method for {@link world.bentobox.chunkblock.listeners.BlockListener#saveCache()}. + */ + @Test + void testSaveCacheQueuesTheWrite() throws Exception { + island.setUniqueId(UUID.randomUUID().toString()); + bl.getIsland(island); + + bl.saveCache(); + + verify(h).saveObject(any()); + verify(h, never()).saveObjectNow(any()); + } + + /** + * The shutdown save has to write directly. This addon is a Pladdon, so the server disables it + * before BentoBox, and a queued write only lands if BentoBox drains the queue afterwards - + * which older BentoBox versions did not do, silently rolling islands back on every restart. + * + * Test method for {@link world.bentobox.chunkblock.listeners.BlockListener#saveCacheNow()}. + */ + @Test + void testSaveCacheNowWritesDirectly() throws Exception { + island.setUniqueId(UUID.randomUUID().toString()); + bl.getIsland(island); + + bl.saveCacheNow(); + + verify(h).saveObjectNow(any()); + verify(h, never()).saveObject(any()); + } + } diff --git a/src/test/java/world/bentobox/chunkblock/panels/PhasesPanelTest.java b/src/test/java/world/bentobox/chunkblock/panels/PhasesPanelTest.java index c417882..78ab591 100644 --- a/src/test/java/world/bentobox/chunkblock/panels/PhasesPanelTest.java +++ b/src/test/java/world/bentobox/chunkblock/panels/PhasesPanelTest.java @@ -7,6 +7,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mockStatic; import static org.mockito.Mockito.verify; @@ -71,6 +72,24 @@ class PhasesPanelTest extends CommonTestSetup { private PhasesPanel panel; + /** + * Sets what a locale reference translates to for these tests. + *

+ * The {@code user} here is a real {@link User} wrapping a mock player, not a mock, so + * {@code when(user.getTranslation(...))} does not stub anything on it - it runs the real + * method and Mockito attaches the stub to whichever mock that method happened to touch last. + * That is an implementation detail of BentoBox and moves between versions. Stub the + * {@link world.bentobox.bentobox.managers.LocalesManager} that {@code getTranslation} actually + * reads from instead, which is stable. + * + * @param reference locale key, without any addon prefix + * @param value what it should translate to + */ + private void stubTranslation(String reference, String value) { + when(lm.get(any(), eq(reference))).thenReturn(value); + when(lm.get(any(), eq("chunkblock." + reference))).thenReturn(value); + } + private void setUpAddonMocks() { when(addon.getPlugin()).thenReturn(plugin); when(addon.getOneBlockManager()).thenReturn(oneBlockManager); @@ -325,10 +344,9 @@ void testBuildBlocksText() throws Exception { OneBlockPhase phase = createTestPhase("Plains"); - when(user.getTranslation("chunkblock.gui.buttons.phase.blocks-prefix")).thenReturn("Blocks: "); - when(user.getTranslation("chunkblock.gui.buttons.phase.wrap-at")).thenReturn("50"); - when(user.getTranslation("chunkblock.gui.buttons.phase.blocks", "name", "Stone")).thenReturn("Stone, "); - when(user.getTranslation("chunkblock.gui.buttons.phase.blocks", "name", "Dirt")).thenReturn("Dirt, "); + stubTranslation("chunkblock.gui.buttons.phase.blocks-prefix", "Blocks: "); + stubTranslation("chunkblock.gui.buttons.phase.wrap-at", "50"); + stubTranslation("chunkblock.gui.buttons.phase.blocks", "[name], "); when(hooksManager.getHook("LangUtils")).thenReturn(Optional.empty()); mockedUtil.when(() -> Util.prettifyText(anyString())).thenAnswer(i -> { String arg = i.getArgument(0); @@ -827,7 +845,7 @@ void testCollectTooltipsWithRealTooltip() throws Exception { new ItemTemplateRecord.ActionRecords(ClickType.LEFT, "SELECT", "content", "tooltip.key") ); - when(user.getTranslation(world, "tooltip.key")).thenReturn("Real tooltip"); + stubTranslation("tooltip.key", "Real tooltip"); Method method = PhasesPanel.class.getDeclaredMethod("collectTooltips", List.class); method.setAccessible(true); @@ -1482,8 +1500,8 @@ void testCollectTooltipsAllBlank() throws Exception { new ItemTemplateRecord.ActionRecords(ClickType.LEFT, "VIEW", "content", "tooltip2") ); - when(user.getTranslation(world, "tooltip1")).thenReturn(" "); // Blank after translation - when(user.getTranslation(world, "tooltip2")).thenReturn(""); // Empty + stubTranslation("tooltip1", " "); // Blank after translation + stubTranslation("tooltip2", ""); // Empty Method method = PhasesPanel.class.getDeclaredMethod("collectTooltips", List.class); method.setAccessible(true); @@ -2144,8 +2162,7 @@ void testBuildDescriptionTextTemplatedWithBiome() throws Exception { try (MockedStatic ms = mockStatic(LangUtilsHook.class)) { ms.when(() -> LangUtilsHook.getBiomeName(biome, user)).thenReturn("Plains"); - when(user.getTranslationOrNothing("custom.desc", "number", "0", "[biome]", "Plains", "[bank]", "", "[economy]", "", "[level]", "", "[permission]", "", "[blocks]", "")) - .thenReturn("Plains Description"); + stubTranslation("custom.desc", "[biome] Description"); Method method = PhasesPanel.class.getDeclaredMethod("buildDescriptionText", ItemTemplateRecord.class, OneBlockPhase.class, reqTextClass, String.class); method.setAccessible(true); @@ -2179,9 +2196,8 @@ void testBuildDefaultDescription() throws Exception { reqConstructor.setAccessible(true); Object reqTexts = reqConstructor.newInstance("", "", "", ""); - when(user.getTranslationOrNothing("chunkblock.gui.buttons.phase.starting-block", "number", "0")).thenReturn("Block 0"); - when(user.getTranslationOrNothing("chunkblock.gui.buttons.phase.description", "[starting-block]", "Block 0", "[biome]", "", "[bank]", "", "[economy]", "", "[level]", "", "[permission]", "", "[blocks]", "")) - .thenReturn("Default Desc"); + stubTranslation("chunkblock.gui.buttons.phase.starting-block", "Block [number]"); + stubTranslation("chunkblock.gui.buttons.phase.description", "Default Desc [starting-block]"); Method method = PhasesPanel.class.getDeclaredMethod("buildDefaultDescription", OneBlockPhase.class, reqTextClass, String.class); method.setAccessible(true); @@ -2398,10 +2414,9 @@ void testBuildDefaultDescriptionWithBiome() throws Exception { reqConstructor.setAccessible(true); Object reqTexts = reqConstructor.newInstance("", "", "", ""); - when(user.getTranslationOrNothing("chunkblock.gui.buttons.phase.starting-block", "number", "0")).thenReturn("Block 0"); - when(user.getTranslationOrNothing("chunkblock.gui.buttons.phase.biome", "[biome]", "Plains")).thenReturn("Biome: Plains"); - when(user.getTranslationOrNothing("chunkblock.gui.buttons.phase.description", "[starting-block]", "Block 0", "[biome]", "Biome: Plains", "[bank]", "", "[economy]", "", "[level]", "", "[permission]", "", "[blocks]", "")) - .thenReturn("Description with biome"); + stubTranslation("chunkblock.gui.buttons.phase.starting-block", "Block [number]"); + stubTranslation("chunkblock.gui.buttons.phase.biome", "Biome: [biome]"); + stubTranslation("chunkblock.gui.buttons.phase.description", "Description with biome [biome]"); try (MockedStatic ms = mockStatic(LangUtilsHook.class)) { ms.when(() -> LangUtilsHook.getBiomeName(biome, user)).thenReturn("Plains"); @@ -2440,9 +2455,8 @@ void testBuildDescriptionTextNullTemplate() throws Exception { reqConstructor.setAccessible(true); Object reqTexts = reqConstructor.newInstance("", "", "", ""); - when(user.getTranslationOrNothing("chunkblock.gui.buttons.phase.starting-block", "number", "0")).thenReturn("Block 0"); - when(user.getTranslationOrNothing("chunkblock.gui.buttons.phase.description", "[starting-block]", "Block 0", "[biome]", "", "[bank]", "", "[economy]", "", "[level]", "", "[permission]", "", "[blocks]", "")) - .thenReturn("Default Description"); + stubTranslation("chunkblock.gui.buttons.phase.starting-block", "Block [number]"); + stubTranslation("chunkblock.gui.buttons.phase.description", "Default Description [starting-block]"); Method method = PhasesPanel.class.getDeclaredMethod("buildDescriptionText", ItemTemplateRecord.class, OneBlockPhase.class, reqTextClass, String.class); method.setAccessible(true); From a4a4f2d3508733e6f8ad2a6835f58b3f230867e8 Mon Sep 17 00:00:00 2001 From: tastybento Date: Sat, 8 Aug 2026 12:51:44 -0700 Subject: [PATCH 2/2] ci: build on JDK 25 so BentoBox 3.22.0 class files can be read 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 21 still emits Java 21 bytecode, which is what the addon ships. The addon's own target is unchanged: still Java 21 via 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) Claude-Session: https://claude.ai/code/session_014t1DSo2wMbTWZLcwXpwUmQ --- .github/workflows/build.yml | 12 ++++++++---- .github/workflows/modrinth-publish.yml | 7 ++++--- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 9e856bb..a1b36da 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -14,11 +14,15 @@ jobs: - uses: actions/checkout@v3 with: fetch-depth: 0 # Shallow clones should be disabled for a better relevancy of analysis - - name: Set up JDK 21 - uses: actions/setup-java@v3 + # BentoBox 3.18.0+ is compiled for Java 25 (Minecraft 26.x), so its class files + # cannot be read by a JDK 21 javac at all - the build fails with + # "class file has wrong version 69.0, should be 65.0" before reaching our code. + # The addon itself still targets 21 via in the pom. + - name: Set up JDK 25 + uses: actions/setup-java@v4 with: - distribution: 'adopt' - java-version: 21 + distribution: 'temurin' + java-version: 25 - name: Cache SonarCloud packages uses: actions/cache@v3 with: diff --git a/.github/workflows/modrinth-publish.yml b/.github/workflows/modrinth-publish.yml index 6d9a248..5574415 100644 --- a/.github/workflows/modrinth-publish.yml +++ b/.github/workflows/modrinth-publish.yml @@ -22,11 +22,12 @@ jobs: - name: Checkout repository uses: actions/checkout@v4 - # 2. Set up Java 21 (required by ChunkBlock' build) - - name: Set up Java 21 + # 2. Set up Java 25 - required to read BentoBox 3.18.0+ class files, which are + # compiled for Java 25. The addon itself still targets 21 via in the pom. + - name: Set up Java 25 uses: actions/setup-java@v4 with: - java-version: '21' + java-version: '25' distribution: 'temurin' # 3. Cache Maven dependencies to speed up builds