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 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);