fix: do not lose island progress when the server restarts

.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 <release> 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:

.github/workflows/modrinth-publish.yml

@@ -22,11 +22,12 @@ jobs:
       - name: Checkout repository
         uses: actions/checkout@v4
 
-      # 2. Set up Java 21 (required by AOneBlock' 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 <release> 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

pom.xml

@@ -56,7 +56,7 @@
         <mockito.version>5.11.0</mockito.version>
         <mock-bukkit.version>4.110.0</mock-bukkit.version>
         <!-- More visible way how to change dependency versions -->
-        <bentobox.version>3.15.0-SNAPSHOT</bentobox.version>
+        <bentobox.version>3.22.0</bentobox.version>
         <items-adder.version>4.0.10</items-adder.version>
         <nexo.version>1.8.0</nexo.version>
         <craftengine.version>0.0.67</craftengine.version>

src/main/java/world/bentobox/aoneblock/AOneBlock.java

@@ -237,9 +237,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();
         }
 
         // Clear holograms

src/main/java/world/bentobox/aoneblock/Settings.java

@@ -400,6 +400,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")
@@ -1865,6 +1872,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
      */

src/main/java/world/bentobox/aoneblock/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 AOneBlock addon) {
 
     /**
      * Saves all island data from the cache to the database asynchronously.
+     * <p>
+     * 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.
+     * <p>
+     * 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);
         }

src/main/resources/addon.yml

@@ -1,7 +1,7 @@
 name: AOneBlock
 main: world.bentobox.aoneblock.AOneBlock
 version: ${version}${build.number}
-api-version: 3.13.0
+api-version: 3.22.0
 metrics: true
 icon: "STONE"
 repository: "BentoBoxWorld/AOneBlock"

src/main/resources/config.yml

@@ -404,6 +404,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

src/test/java/world/bentobox/aoneblock/SettingsTest.java

@@ -1732,6 +1732,35 @@ void testSetHologramDuration() {
         s.setHologramDuration(2345);
         assertEquals(2345, s.getHologramDuration());
     }
+
+    /**
+     * Test method for {@link world.bentobox.aoneblock.Settings#getSaveEvery()}.
+     */
+    @Test
+    void testGetSaveEveryDefault() {
+        assertEquals(10, s.getSaveEvery());
+    }
+
+    /**
+     * Test method for {@link world.bentobox.aoneblock.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());
+    }
 
 
 

src/test/java/world/bentobox/aoneblock/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<Object> h;
+
     @Mock
     AOneBlock 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<Object> h = mock(AbstractDatabaseHandler.class);
+        h = mock(AbstractDatabaseHandler.class);
         // Database
         MockedStatic<DatabaseSetup> 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.aoneblock.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.aoneblock.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());
+    }
+
 }

src/test/java/world/bentobox/aoneblock/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.
+     * <p>
+     * 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("aoneblock." + 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("aoneblock.gui.buttons.phase.blocks-prefix")).thenReturn("Blocks: ");
-        when(user.getTranslation("aoneblock.gui.buttons.phase.wrap-at")).thenReturn("50");
-        when(user.getTranslation("aoneblock.gui.buttons.phase.blocks", "name", "Stone")).thenReturn("Stone, ");
-        when(user.getTranslation("aoneblock.gui.buttons.phase.blocks", "name", "Dirt")).thenReturn("Dirt, ");
+        stubTranslation("aoneblock.gui.buttons.phase.blocks-prefix", "Blocks: ");
+        stubTranslation("aoneblock.gui.buttons.phase.wrap-at", "50");
+        stubTranslation("aoneblock.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<LangUtilsHook> 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("aoneblock.gui.buttons.phase.starting-block", "number", "0")).thenReturn("Block 0");
-        when(user.getTranslationOrNothing("aoneblock.gui.buttons.phase.description", "[starting-block]", "Block 0", "[biome]", "", "[bank]", "", "[economy]", "", "[level]", "", "[permission]", "", "[blocks]", ""))
-            .thenReturn("Default Desc");
+        stubTranslation("aoneblock.gui.buttons.phase.starting-block", "Block [number]");
+        stubTranslation("aoneblock.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("aoneblock.gui.buttons.phase.starting-block", "number", "0")).thenReturn("Block 0");
-        when(user.getTranslationOrNothing("aoneblock.gui.buttons.phase.biome", "[biome]", "Plains")).thenReturn("Biome: Plains");
-        when(user.getTranslationOrNothing("aoneblock.gui.buttons.phase.description", "[starting-block]", "Block 0", "[biome]", "Biome: Plains", "[bank]", "", "[economy]", "", "[level]", "", "[permission]", "", "[blocks]", ""))
-            .thenReturn("Description with biome");
+        stubTranslation("aoneblock.gui.buttons.phase.starting-block", "Block [number]");
+        stubTranslation("aoneblock.gui.buttons.phase.biome", "Biome: [biome]");
+        stubTranslation("aoneblock.gui.buttons.phase.description", "Description with biome [biome]");
 
         try (MockedStatic<LangUtilsHook> 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("aoneblock.gui.buttons.phase.starting-block", "number", "0")).thenReturn("Block 0");
-        when(user.getTranslationOrNothing("aoneblock.gui.buttons.phase.description", "[starting-block]", "Block 0", "[biome]", "", "[bank]", "", "[economy]", "", "[level]", "", "[permission]", "", "[blocks]", ""))
-            .thenReturn("Default Description");
+        stubTranslation("aoneblock.gui.buttons.phase.starting-block", "Block [number]");
+        stubTranslation("aoneblock.gui.buttons.phase.description", "Default Description [starting-block]");
 
         Method method = PhasesPanel.class.getDeclaredMethod("buildDescriptionText", ItemTemplateRecord.class, OneBlockPhase.class, reqTextClass, String.class);
         method.setAccessible(true);