diff --git a/CLAUDE.md b/CLAUDE.md index fbf4940..4bc4c69 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -112,7 +112,9 @@ Two cancellable events are fired so other plugins can intercept limit applicatio ### Data Persistence -`IslandBlockCount` is persisted via BentoBox's `Database` (JSON flat-file by default, configurable in BentoBox). Saves are batched: every 10 block changes triggers an async save (`CHANGE_LIMIT = 9`). A full save of all changed islands occurs on addon disable. +`IslandBlockCount` is persisted via BentoBox's `Database` (JSON flat-file by default, configurable in BentoBox). Saves are batched: every 10 changes to an island (block or entity) triggers an async save (`CHANGE_LIMIT = 9`). A full save of all changed islands occurs on addon disable. + +Entity count mutations must go through `BlockLimitsListener.incrementEntity(Island, Environment, EntityType)` / `decrementEntity(String, Environment, EntityType)` — these mutate the count *and* enroll the change in the batch-save cycle. Never call `IslandBlockCount.incrementEntity`/`decrementEntity` directly from a listener, or the change is only persisted on addon disable and is lost on a crash. (`RecountCalculator` is the exception: it rebuilds a fresh `IslandBlockCount` and saves it wholesale via `setIsland`.) ### Testing diff --git a/src/main/java/world/bentobox/limits/listeners/BlockLimitsListener.java b/src/main/java/world/bentobox/limits/listeners/BlockLimitsListener.java index b5b3a06..3cf800a 100644 --- a/src/main/java/world/bentobox/limits/listeners/BlockLimitsListener.java +++ b/src/main/java/world/bentobox/limits/listeners/BlockLimitsListener.java @@ -37,6 +37,7 @@ import org.bukkit.event.block.BlockSpreadEvent; import org.bukkit.event.block.EntityBlockFormEvent; import org.bukkit.event.block.LeavesDecayEvent; +import org.bukkit.entity.EntityType; import org.bukkit.event.entity.EntityChangeBlockEvent; import org.bukkit.event.entity.EntityExplodeEvent; import org.bukkit.event.player.PlayerInteractEvent; @@ -472,7 +473,6 @@ public void removeBlock(Block b) { } private void updateSaveMap(String id) { - saveMap.putIfAbsent(id, 0); if (saveMap.merge(id, 1, Integer::sum) > CHANGE_LIMIT) { handler.saveObjectAsync(islandCountMap.get(id)); saveMap.remove(id); @@ -568,12 +568,26 @@ public IslandBlockCount getIsland(Island island) { } /** - * Notify that the island's data has changed so it gets batched for a save. - * Called from {@link EntityLimitListener} when entity counts are incremented - * or decremented — block changes go through {@link #process} which already - * calls this internally. + * Increment the island's entity count for this environment and batch the change + * for a save. Creates the island's count record if it does not exist yet. + * Entity mutations must go through this or {@link #decrementEntity} so they join + * the batch-save cycle — block changes go through {@link #process} which handles + * this internally. */ - public void markChanged(String islandId) { - updateSaveMap(islandId); + public void incrementEntity(Island island, Environment env, EntityType type) { + getIsland(island).incrementEntity(env, type); + updateSaveMap(island.getUniqueId()); + } + + /** + * Decrement the island's entity count for this environment and batch the change + * for a save. No-op if the island has no count record. + */ + public void decrementEntity(String islandId, Environment env, EntityType type) { + IslandBlockCount ibc = getIsland(islandId); + if (ibc != null) { + ibc.decrementEntity(env, type); + updateSaveMap(islandId); + } } } diff --git a/src/main/java/world/bentobox/limits/listeners/EntityLimitListener.java b/src/main/java/world/bentobox/limits/listeners/EntityLimitListener.java index eff8f9c..54e3c8c 100644 --- a/src/main/java/world/bentobox/limits/listeners/EntityLimitListener.java +++ b/src/main/java/world/bentobox/limits/listeners/EntityLimitListener.java @@ -270,10 +270,8 @@ private void trackSpawn(Entity entity) { addon.getIslands().getIslandAt(entity.getLocation()) .filter(island -> !island.isSpawn()) .ifPresent(island -> { - IslandBlockCount ibc = addon.getBlockLimitListener().getIsland(island); - ibc.incrementEntity(envOf(w), entity.getType()); + addon.getBlockLimitListener().incrementEntity(island, envOf(w), entity.getType()); entityIslandMap.put(entity.getUniqueId(), island.getUniqueId()); - addon.getBlockLimitListener().markChanged(island.getUniqueId()); }); } @@ -324,23 +322,14 @@ public void onEntityRemove(EntityRemoveEvent e) { String islandId = entityIslandMap.remove(entity.getUniqueId()); if (islandId != null) { - IslandBlockCount ibc = addon.getBlockLimitListener().getIsland(islandId); - if (ibc != null) { - ibc.decrementEntity(envOf(w), entity.getType()); - addon.getBlockLimitListener().markChanged(islandId); - } + addon.getBlockLimitListener().decrementEntity(islandId, envOf(w), entity.getType()); return; } addon.getIslands().getIslandAt(entity.getLocation()) .filter(island -> !island.isSpawn()) - .ifPresent(island -> { - IslandBlockCount ibc = addon.getBlockLimitListener().getIsland(island.getUniqueId()); - if (ibc != null) { - ibc.decrementEntity(envOf(w), entity.getType()); - addon.getBlockLimitListener().markChanged(island.getUniqueId()); - } - }); + .ifPresent(island -> addon.getBlockLimitListener().decrementEntity(island.getUniqueId(), envOf(w), + entity.getType())); } /* ========================================================================= @@ -366,23 +355,16 @@ public void onEntityPortal(EntityPortalEvent e) { entityIslandMap.remove(entity.getUniqueId()); addon.getIslands().getIslandAt(entity.getLocation()) .filter(island -> !island.isSpawn()) - .ifPresent(island -> { - IslandBlockCount ibc = addon.getBlockLimitListener().getIsland(island.getUniqueId()); - if (ibc != null) { - ibc.decrementEntity(fromEnv, entity.getType()); - addon.getBlockLimitListener().markChanged(island.getUniqueId()); - } - }); + .ifPresent(island -> addon.getBlockLimitListener().decrementEntity(island.getUniqueId(), fromEnv, + entity.getType())); } // Increment at destination if on a tracked island if (addon.inGameModeWorld(toWorld)) { addon.getIslands().getIslandAt(e.getTo()) .filter(island -> !island.isSpawn()) .ifPresent(island -> { - IslandBlockCount ibc = addon.getBlockLimitListener().getIsland(island); - ibc.incrementEntity(toEnv, entity.getType()); + addon.getBlockLimitListener().incrementEntity(island, toEnv, entity.getType()); entityIslandMap.put(entity.getUniqueId(), island.getUniqueId()); - addon.getBlockLimitListener().markChanged(island.getUniqueId()); }); } } diff --git a/src/test/java/world/bentobox/limits/listeners/BlockLimitsListenerTest.java b/src/test/java/world/bentobox/limits/listeners/BlockLimitsListenerTest.java index 3331193..033b4c4 100644 --- a/src/test/java/world/bentobox/limits/listeners/BlockLimitsListenerTest.java +++ b/src/test/java/world/bentobox/limits/listeners/BlockLimitsListenerTest.java @@ -35,6 +35,7 @@ import org.bukkit.block.data.type.TechnicalPiston; import org.bukkit.configuration.file.FileConfiguration; import org.bukkit.configuration.file.YamlConfiguration; +import org.bukkit.entity.EntityType; import org.bukkit.entity.Player; import org.bukkit.ExplosionResult; import org.bukkit.entity.Entity; @@ -1308,4 +1309,79 @@ void testSaveSavesChangedIslands() { // saveObjectAsync is called once by setIsland and once by save verify(dbMock, atLeastOnce()).saveObjectAsync(any()); } + + // --- Entity count persistence tests --- + + @Test + void testIncrementEntityCreatesRecordAndCounts() { + when(island.getGameMode()).thenReturn("BSkyBlock"); + + listener.incrementEntity(island, Environment.NORMAL, EntityType.CHICKEN); + + IslandBlockCount ibc = listener.getIsland("test-island-id"); + assertNotNull(ibc); + assertEquals(1, ibc.getEntityCount(Environment.NORMAL, EntityType.CHICKEN)); + } + + @Test + void testDecrementEntityLowersCount() { + when(island.getGameMode()).thenReturn("BSkyBlock"); + listener.incrementEntity(island, Environment.NORMAL, EntityType.CHICKEN); + listener.incrementEntity(island, Environment.NORMAL, EntityType.CHICKEN); + + listener.decrementEntity("test-island-id", Environment.NORMAL, EntityType.CHICKEN); + + assertEquals(1, listener.getIsland("test-island-id").getEntityCount(Environment.NORMAL, EntityType.CHICKEN)); + } + + @Test + void testDecrementEntityWithoutRecordIsNoOp() { + // No record exists and repeated no-op decrements must not count toward the batch save + for (int i = 0; i < 20; i++) { + listener.decrementEntity("unknown-island", Environment.NORMAL, EntityType.CHICKEN); + } + + assertNull(listener.getIsland("unknown-island")); + Database dbMock = mockedDb.constructed().get(0); + verify(dbMock, never()).saveObjectAsync(any()); + } + + @Test + void testEntityChangesBatchSaveAfterThreshold() { + when(island.getGameMode()).thenReturn("BSkyBlock"); + // CHANGE_LIMIT = 9, so the 10th change triggers a save — entity changes must + // join the same batch-save cycle as block changes (they used to be persisted + // only on addon disable, losing counts on a crash) + for (int i = 0; i < 10; i++) { + listener.incrementEntity(island, Environment.NORMAL, EntityType.CHICKEN); + } + + Database dbMock = mockedDb.constructed().get(0); + verify(dbMock, atLeastOnce()).saveObjectAsync(any()); + } + + @Test + void testEntityChangesNoSaveBeforeThreshold() { + when(island.getGameMode()).thenReturn("BSkyBlock"); + for (int i = 0; i < 9; i++) { + listener.incrementEntity(island, Environment.NORMAL, EntityType.CHICKEN); + } + + Database dbMock = mockedDb.constructed().get(0); + verify(dbMock, never()).saveObjectAsync(any()); + } + + @Test + void testMixedEntityIncrementAndDecrementCountTowardBatchSave() { + when(island.getGameMode()).thenReturn("BSkyBlock"); + for (int i = 0; i < 5; i++) { + listener.incrementEntity(island, Environment.NORMAL, EntityType.CHICKEN); + } + for (int i = 0; i < 5; i++) { + listener.decrementEntity("test-island-id", Environment.NORMAL, EntityType.CHICKEN); + } + + Database dbMock = mockedDb.constructed().get(0); + verify(dbMock, atLeastOnce()).saveObjectAsync(any()); + } } diff --git a/src/test/java/world/bentobox/limits/listeners/EntityLimitListenerTest.java b/src/test/java/world/bentobox/limits/listeners/EntityLimitListenerTest.java index 7ed2db5..65735aa 100644 --- a/src/test/java/world/bentobox/limits/listeners/EntityLimitListenerTest.java +++ b/src/test/java/world/bentobox/limits/listeners/EntityLimitListenerTest.java @@ -5,6 +5,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.Mockito.doAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; @@ -116,6 +117,20 @@ void setUp() throws Exception { ibc.incrementEntity(Environment.NORMAL, EntityType.ENDERMAN); when(bll.getIsland(anyString())).thenReturn(ibc); when(addon.getBlockLimitListener()).thenReturn(bll); + // Delegate the count-mutation wrappers to the real IslandBlockCount so tests + // can assert on counts. Mirrors BlockLimitsListener's contract: increment + // creates the record, decrement is a no-op when there is none. + doAnswer(inv -> { + ibc.incrementEntity(inv.getArgument(1), inv.getArgument(2)); + return null; + }).when(bll).incrementEntity(any(Island.class), any(Environment.class), any(EntityType.class)); + doAnswer(inv -> { + IslandBlockCount target = bll.getIsland((String) inv.getArgument(0)); + if (target != null) { + target.decrementEntity(inv.getArgument(1), inv.getArgument(2)); + } + return null; + }).when(bll).decrementEntity(anyString(), any(Environment.class), any(EntityType.class)); FileConfiguration config = new YamlConfiguration(); config.load("src/main/resources/config.yml");