Skip to content

Centralize entity count persistence in BlockLimitsListener #274

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tastybento merged 1 commit into from
Jul 5, 2026
Merged

Centralize entity count persistence in BlockLimitsListener #274

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion CLAUDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<IslandBlockCount>` (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<IslandBlockCount>` (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

Expand Down
28 changes: 21 additions & 7 deletions src/main/java/world/bentobox/limits/listeners/BlockLimitsListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
}
}
32 changes: 7 additions & 25 deletions src/main/java/world/bentobox/limits/listeners/EntityLimitListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
});
}

Expand Down Expand Up @@ -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()));
}

/* =========================================================================
Expand All @@ -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());
});
}
}
Expand Down
76 changes: 76 additions & 0 deletions src/test/java/world/bentobox/limits/listeners/BlockLimitsListenerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}
}
15 changes: 15 additions & 0 deletions src/test/java/world/bentobox/limits/listeners/EntityLimitListenerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
Expand Down
Loading