diff --git a/pom.xml b/pom.xml index 1f8e911..81eba42 100644 --- a/pom.xml +++ b/pom.xml @@ -62,7 +62,7 @@ -LOCAL - 1.28.4 + 1.29.0 BentoBoxWorld_Limits bentobox-world https://sonarcloud.io diff --git a/src/main/java/world/bentobox/limits/Settings.java b/src/main/java/world/bentobox/limits/Settings.java index 4148501..8e65c74 100644 --- a/src/main/java/world/bentobox/limits/Settings.java +++ b/src/main/java/world/bentobox/limits/Settings.java @@ -100,7 +100,7 @@ public Settings(Limits addon) { loadEntityLimits(addon, "entitylimits-end", List.of(Environment.THE_END)); // Log limits on join - logLimitsOnJoin = addon.getConfig().getBoolean("log-limits-on-join", true); + logLimitsOnJoin = addon.getConfig().getBoolean("log-limits-on-join", false); // Async Golums asyncGolums = addon.getConfig().getBoolean("async-golums", true); // Show or suppress the "hit the limit" player notifications diff --git a/src/main/java/world/bentobox/limits/listeners/BlockLimitsListener.java b/src/main/java/world/bentobox/limits/listeners/BlockLimitsListener.java index 9635288..101a9a1 100644 --- a/src/main/java/world/bentobox/limits/listeners/BlockLimitsListener.java +++ b/src/main/java/world/bentobox/limits/listeners/BlockLimitsListener.java @@ -472,6 +472,22 @@ && isSamePlant(b.getRelative(BlockFace.DOWN).getType(), fixMaterial(blockData))) * * @return limit amount if at/over the limit (nothing was counted), or -1 on success */ + /** + * True when the config ignores the island's center block and {@code loc} is that + * block. Compares block coordinates so entity locations (fractional coordinates, + * yaw/pitch) are matched correctly. + */ + private boolean isIgnoredCenterBlock(Island island, Location loc) { + if (!addon.getConfig().getBoolean("ignore-center-block", true)) { + return false; + } + Location center = island.getCenter(); + return center != null && Objects.equals(center.getWorld(), loc.getWorld()) + && center.getBlockX() == loc.getBlockX() + && center.getBlockY() == loc.getBlockY() + && center.getBlockZ() == loc.getBlockZ(); + } + public int processKey(World world, Location location, NamespacedKey key, boolean add) { if (!addon.inGameModeWorld(world)) { return -1; @@ -483,8 +499,7 @@ public int processKey(World world, Location location, NamespacedKey key, boolean if (gameMode.isEmpty()) { return -1; } - if (addon.getConfig().getBoolean("ignore-center-block", true) - && i.getCenter().equals(location)) { + if (isIgnoredCenterBlock(i, location)) { return -1; } islandCountMap.putIfAbsent(id, new IslandBlockCount(id, gameMode)); @@ -524,6 +539,64 @@ private void updateSaveMap(String id) { } } + /** + * The canonical copper chest material, or {@code null} on servers older than 1.21.9 + * where copper chests (and copper golems) do not exist. + */ + @Nullable + public Material getCopperChestMaterial() { + return COPPER_CHEST; + } + + /** + * Read-only check of whether counting one more block of {@code key} on the island at + * {@code loc} would exceed the active limit. Used for blocks that appear without a + * place event — e.g. the copper chest a copper golem creates from a copper block, which + * fires no {@link BlockPlaceEvent} and so escapes the normal check (see + * issue #276). + * + * @return the limit value if already at/over it, otherwise -1 + */ + public int checkBlockLimit(Location loc, NamespacedKey key) { + World w = loc.getWorld(); + if (w == null || !addon.inGameModeWorld(w)) { + return -1; + } + Environment env = envOf(w); + return addon.getIslands().getIslandAt(loc).map(i -> { + String id = i.getUniqueId(); + String gameMode = addon.getGameModeName(w); + if (gameMode.isEmpty() || isIgnoredCenterBlock(i, loc)) { + return -1; + } + islandCountMap.putIfAbsent(id, new IslandBlockCount(id, gameMode)); + return checkLimit(w, env, key, id); + }).orElse(-1); + } + + /** + * Unconditionally count one block of {@code key} against the island at {@code loc}. + * Mirror of {@link #removeBlock(Block)} for blocks that are created without a place + * event; the block really exists, so not counting it would let the count drift below + * reality (see issue #276). + */ + public void addBlockCount(Location loc, NamespacedKey key) { + World w = loc.getWorld(); + if (w == null || !addon.inGameModeWorld(w)) { + return; + } + addon.getIslands().getIslandAt(loc).ifPresent(i -> { + String id = i.getUniqueId(); + String gameMode = addon.getGameModeName(w); + if (gameMode.isEmpty() || isIgnoredCenterBlock(i, loc)) { + return; + } + Environment env = envOf(w); + islandCountMap.computeIfAbsent(id, k -> new IslandBlockCount(id, gameMode)).add(env, key); + updateSaveMap(id); + }); + } + /** * Resolve the active limit for this material in this world for this island. * Priority: island-env limit, world-named limit, env-default, none. diff --git a/src/main/java/world/bentobox/limits/listeners/EntityLimitListener.java b/src/main/java/world/bentobox/limits/listeners/EntityLimitListener.java index 7226811..2a64f88 100644 --- a/src/main/java/world/bentobox/limits/listeners/EntityLimitListener.java +++ b/src/main/java/world/bentobox/limits/listeners/EntityLimitListener.java @@ -52,11 +52,13 @@ public class EntityLimitListener implements Listener { private static final String ENTITY_LIMIT_HIT = "entity-limits.hit-limit"; /** Notification placeholder for the entity name. */ private static final String ENTITY_PLACEHOLDER = "[entity]"; + /** Locale reference for the block limit notification. */ + private static final String BLOCK_LIMIT_HIT = "block-limits.hit-limit"; + /** Notification placeholder for the block material name. */ + private static final String BLOCK_PLACEHOLDER = "[material]"; private final Limits addon; /** Entity UUIDs that have just spawned to prevent double-processing. */ private final List justSpawned = new ArrayList<>(); - /** Entity UUIDs that are currently portaling to prevent double-decrement on cross-world removal. */ - private final List justPortaled = new ArrayList<>(); /** Maps entity UUID to island ID so decrement works even when the entity dies off-island. */ private final Map entityIslandMap = new HashMap<>(); /** Cardinal directions used for block structure detection. */ @@ -124,6 +126,12 @@ public void onCreatureSpawn(final CreatureSpawnEvent creatureSpawnEvent) { && creatureSpawnEvent.getSpawnReason().equals(SpawnReason.BREEDING))) { return; } + // A copper golem turns its copper block into a copper chest with no place event, + // bypassing the COPPER_CHEST block limit (#276). Cancel the build if at that limit. + if (isBuildCopperGolem(creatureSpawnEvent.getSpawnReason()) + && checkCopperChestLimit(creatureSpawnEvent)) { + return; + } if (creatureSpawnEvent.getSpawnReason().equals(SpawnReason.BUILD_SNOWMAN) || creatureSpawnEvent.getSpawnReason().equals(SpawnReason.BUILD_IRONGOLEM)) { checkLimit(creatureSpawnEvent, creatureSpawnEvent.getEntity(), creatureSpawnEvent.getSpawnReason(), @@ -154,11 +162,11 @@ public void onBlock(HangingPlaceEvent hangingPlaceEvent) { } User u = User.getInstance(player); if (res.getTypelimit() != null) { - u.notify("block-limits.hit-limit", "[material]", + u.notify(BLOCK_LIMIT_HIT, BLOCK_PLACEHOLDER, DisplayNames.entity(u, hangingPlaceEvent.getEntity().getType()), TextVariables.NUMBER, String.valueOf(res.getTypelimit().getValue())); } else { - u.notify("block-limits.hit-limit", "[material]", + u.notify(BLOCK_LIMIT_HIT, BLOCK_PLACEHOLDER, res.getGrouplimit().getKey().getName() + " (" + res.getGrouplimit().getKey().getTypes().stream() .map(x -> DisplayNames.entity(u, x)) @@ -260,6 +268,14 @@ private void notifyEntityLimit(Player player, EntityType type, AtLimitResult res @EventHandler(priority = EventPriority.MONITOR, ignoreCancelled = true) public void onCreatureSpawnTrack(final CreatureSpawnEvent e) { trackSpawn(e.getEntity()); + // The copper block became a copper chest as part of the (uncancelled) build; count it + // so the COPPER_CHEST limit is enforceable and the count stays accurate (#276). + if (isBuildCopperGolem(e.getSpawnReason())) { + Material chest = addon.getBlockLimitListener().getCopperChestMaterial(); + if (chest != null) { + addon.getBlockLimitListener().addBlockCount(e.getLocation(), chest.getKey()); + } + } } @EventHandler(priority = EventPriority.MONITOR, ignoreCancelled = true) @@ -326,9 +342,6 @@ public void onEntityRemove(EntityRemoveEvent e) { World w = entity.getWorld(); if (!addon.inGameModeWorld(w)) return; - // Entity is being portaled — count transfer was already handled by onEntityPortal. - if (justPortaled.remove(entity.getUniqueId())) return; - String islandId = entityIslandMap.remove(entity.getUniqueId()); if (islandId != null) { addon.getBlockLimitListener().decrementEntity(islandId, envOf(w), entity.getType()); @@ -356,8 +369,9 @@ public void onEntityPortal(EntityPortalEvent e) { if (fromEnv == toEnv) return; if (!addon.inGameModeWorld(fromWorld) && !addon.inGameModeWorld(toWorld)) return; - // Prevent onEntityRemove from double-decrementing for the source-world removal. - justPortaled.add(entity.getUniqueId()); + // No EntityRemoveEvent guard is needed here: Paper suppresses the event for + // dimension changes (RemovalReason.CHANGED_DIMENSION carries a null Bukkit cause), + // so the source-world removal never reaches onEntityRemove. // Decrement at source if on a tracked island if (addon.inGameModeWorld(fromWorld)) { @@ -382,6 +396,61 @@ public void onEntityPortal(EntityPortalEvent e) { * Limit-checking core * ========================================================================= */ + /** + * Whether the spawn reason is a copper golem build. Matched by name so the addon links + * and runs on servers older than 1.21.9, where {@code BUILD_COPPERGOLEM} is absent. + */ + private static boolean isBuildCopperGolem(SpawnReason reason) { + return reason.name().equals("BUILD_COPPERGOLEM"); + } + + /** + * When a copper golem is built its copper block is replaced by a copper chest without a + * {@link org.bukkit.event.block.BlockPlaceEvent}, so the {@code COPPER_CHEST} block limit + * is never checked. Cancel the build if the island is already at that limit (#276). + * + * @return true if the spawn was cancelled because the copper chest limit was hit + */ + private boolean checkCopperChestLimit(CreatureSpawnEvent e) { + Material chest = addon.getBlockLimitListener().getCopperChestMaterial(); + if (chest == null) { + return false; + } + Location loc = e.getLocation(); + int limit = addon.getBlockLimitListener().checkBlockLimit(loc, chest.getKey()); + if (limit < 0) { + return false; + } + e.setCancelled(true); + tellPlayersBlockLimit(loc, chest, limit); + return true; + } + + /** + * Notify players near {@code location} that a block limit was hit, mirroring the block + * listener's own {@code block-limits.hit-limit} message. + */ + private void tellPlayersBlockLimit(Location location, Material material, int limit) { + if (!addon.getSettings().isShowLimitMessages()) { + return; + } + World w = location.getWorld(); + if (w == null) { + return; + } + Bukkit.getScheduler().runTask(addon.getPlugin(), () -> { + for (Entity ent : w.getNearbyEntities(location, 5, 5, 5)) { + if (ent instanceof Player p) { + p.updateInventory(); + User u = User.getInstance(p); + u.notify(BLOCK_LIMIT_HIT, BLOCK_PLACEHOLDER, + DisplayNames.material(u, material.getKey()), + TextVariables.NUMBER, String.valueOf(limit)); + } + } + }); + } + private boolean checkLimit(Cancellable cancelableEvent, LivingEntity livingEntity, SpawnReason spawnReason, boolean runAsync) { Location l = livingEntity.getLocation(); diff --git a/src/main/java/world/bentobox/limits/listeners/JoinListener.java b/src/main/java/world/bentobox/limits/listeners/JoinListener.java index 1ec5384..6e2b6b3 100644 --- a/src/main/java/world/bentobox/limits/listeners/JoinListener.java +++ b/src/main/java/world/bentobox/limits/listeners/JoinListener.java @@ -81,10 +81,15 @@ public void checkPerms(Player player, String permissionPrefix, String islandId, */ public void mergePerms(Player player, String permissionPrefix, String islandId, String gameMode) { IslandBlockCount islandBlockCount = addon.getBlockLimitListener().getIsland(islandId); + boolean bannerLogged = false; for (PermissionAttachmentInfo permissionInfo : player.getEffectivePermissions()) { if (!permissionInfo.getValue() || !permissionInfo.getPermission().startsWith(permissionPrefix)) { continue; } + if (!bannerLogged) { + logIfEnabled("Setting login limit via perm for " + player.getName() + "..."); + bannerLogged = true; + } islandBlockCount = applyOnePerm(player, permissionInfo, permissionPrefix, islandId, gameMode, islandBlockCount); } @@ -110,7 +115,6 @@ private IslandBlockCount applyOnePerm(Player player, PermissionAttachmentInfo pe } IslandBlockCount ibc = current != null ? current : new IslandBlockCount(islandId, gameMode); - logIfEnabled("Setting login limit via perm for " + player.getName() + "..."); LimitsPermCheckEvent limitsPermCheckEvent = new LimitsPermCheckEvent(player, islandId, ibc, entityGroup, entityType, material, parsed.value); diff --git a/src/main/resources/config.yml b/src/main/resources/config.yml index 642feaa..74591b8 100644 --- a/src/main/resources/config.yml +++ b/src/main/resources/config.yml @@ -24,8 +24,9 @@ ignore-center-block: true # Cooldown for player recount command in seconds cooldown: 120 -# Log limits on join (to console) -log-limits-on-join: true +# Log limits on join (to console). Off by default - enable only for debugging as it +# can be noisy in console logs for servers with many permission-based limits. +log-limits-on-join: false # Use async checks for snowmen and iron golums. Set to false if you see problems. async-golums: true diff --git a/src/test/java/world/bentobox/limits/SettingsTest.java b/src/test/java/world/bentobox/limits/SettingsTest.java index c900e07..59087d8 100644 --- a/src/test/java/world/bentobox/limits/SettingsTest.java +++ b/src/test/java/world/bentobox/limits/SettingsTest.java @@ -94,8 +94,8 @@ void testGetGroupLimitDefinitions() { } @Test - void testLogLimitsOnJoinDefaultsTrue() { - assertTrue(settings.isLogLimitsOnJoin()); + void testLogLimitsOnJoinDefaultsFalse() { + assertFalse(settings.isLogLimitsOnJoin()); } @Test diff --git a/src/test/java/world/bentobox/limits/listeners/BlockLimitsListenerTest.java b/src/test/java/world/bentobox/limits/listeners/BlockLimitsListenerTest.java index f6c1941..7e7591f 100644 --- a/src/test/java/world/bentobox/limits/listeners/BlockLimitsListenerTest.java +++ b/src/test/java/world/bentobox/limits/listeners/BlockLimitsListenerTest.java @@ -1097,6 +1097,35 @@ void testIndividualLimitStillAppliesInsideGroup() { assertTrue(event.isCancelled()); } + // --- Center-block exclusion for the no-place-event helpers (#276 review) --- + + @Test + void testCheckBlockLimitIgnoresCenterBlock() { + // At the limit everywhere else, but the location is the island center block + IslandBlockCount ibc = new IslandBlockCount("test-island-id", "BSkyBlock"); + ibc.setBlockLimit(Environment.NORMAL, Material.CHEST.getKey(), 0); + listener.setIsland("test-island-id", ibc); + + // Entity-style location inside the center block (fractional coordinates) + Location entityLoc = new Location(world, 0.5, 65.0, 0.3); + assertEquals(-1, listener.checkBlockLimit(entityLoc, Material.CHEST.getKey())); + // Away from the center the limit applies + assertEquals(0, listener.checkBlockLimit(blockLocation, Material.CHEST.getKey())); + } + + @Test + void testAddBlockCountIgnoresCenterBlock() { + IslandBlockCount ibc = new IslandBlockCount("test-island-id", "BSkyBlock"); + listener.setIsland("test-island-id", ibc); + + Location entityLoc = new Location(world, 0.5, 65.0, 0.3); + listener.addBlockCount(entityLoc, Material.CHEST.getKey()); + assertEquals(0, listener.getIsland("test-island-id").getBlockCount(Material.CHEST.getKey())); + + listener.addBlockCount(blockLocation, Material.CHEST.getKey()); + assertEquals(1, listener.getIsland("test-island-id").getBlockCount(Material.CHEST.getKey())); + } + // --- Custom block keys (#176) --- @Test diff --git a/src/test/java/world/bentobox/limits/listeners/EntityLimitListenerTest.java b/src/test/java/world/bentobox/limits/listeners/EntityLimitListenerTest.java index 6d51b18..71921e1 100644 --- a/src/test/java/world/bentobox/limits/listeners/EntityLimitListenerTest.java +++ b/src/test/java/world/bentobox/limits/listeners/EntityLimitListenerTest.java @@ -37,6 +37,7 @@ import org.bukkit.event.entity.CreatureSpawnEvent; import org.bukkit.event.entity.CreatureSpawnEvent.SpawnReason; import org.bukkit.event.entity.EntityBreedEvent; +import org.bukkit.event.entity.EntityPortalEvent; import org.bukkit.event.entity.EntityRemoveEvent; import org.bukkit.event.Event; import org.bukkit.event.block.Action; @@ -433,6 +434,51 @@ void testCreatureSpawnDebounceSkipsSecond() throws Exception { assertFalse(event.isCancelled()); } + // --- Copper golem / copper chest limit tests (#276) --- + + @Test + void testCopperGolemAtChestLimitCancels() { + // Building a copper golem turns its copper block into a copper chest with no place + // event; the spawn must be cancelled when the COPPER_CHEST block limit is reached. + when(bll.getCopperChestMaterial()).thenReturn(Material.COPPER_CHEST); + when(bll.checkBlockLimit(any(Location.class), any())).thenReturn(1); + LivingEntity golem = mockEntity(EntityType.COPPER_GOLEM, location); + + CreatureSpawnEvent event = new CreatureSpawnEvent(golem, SpawnReason.BUILD_COPPERGOLEM); + + ell.onCreatureSpawn(event); + + assertTrue(event.isCancelled()); + verify(bll).checkBlockLimit(location, Material.COPPER_CHEST.getKey()); + } + + @Test + void testCopperGolemUnderChestLimitNotCancelled() { + when(bll.getCopperChestMaterial()).thenReturn(Material.COPPER_CHEST); + when(bll.checkBlockLimit(any(Location.class), any())).thenReturn(-1); + LivingEntity golem = mockEntity(EntityType.COPPER_GOLEM, location); + + CreatureSpawnEvent event = new CreatureSpawnEvent(golem, SpawnReason.BUILD_COPPERGOLEM); + + ell.onCreatureSpawn(event); + + assertFalse(event.isCancelled()); + } + + @Test + void testCopperGolemTrackCountsChest() { + // On a successful build the created copper chest must be counted so the limit is + // enforceable and the count does not drift below reality. + when(bll.getCopperChestMaterial()).thenReturn(Material.COPPER_CHEST); + LivingEntity golem = mockEntity(EntityType.COPPER_GOLEM, location); + + CreatureSpawnEvent event = new CreatureSpawnEvent(golem, SpawnReason.BUILD_COPPERGOLEM); + + ell.onCreatureSpawnTrack(event); + + verify(bll).addBlockCount(location, Material.COPPER_CHEST.getKey()); + } + // --- VehicleCreateEvent tests --- @Test @@ -758,6 +804,74 @@ void testReloadedEntityDecrementsWhenItDiesOffIsland() throws Exception { assertFalse(entityIslandMap().containsKey(enderman.getUniqueId())); } + // --- EntityPortalEvent / phantom-count tests --- + + @Test + void testEntityPortalTransfersCountBetweenEnvironments() throws Exception { + Location netherLoc = mockNetherLocation(); + LivingEntity chicken = mockEntity(EntityType.CHICKEN, location); + ibc.incrementEntity(Environment.NORMAL, EntityType.CHICKEN); + + ell.onEntityPortal(new EntityPortalEvent(chicken, location, netherLoc)); + + assertEquals(0, ibc.getEntityCount(Environment.NORMAL, EntityType.CHICKEN)); + assertEquals(1, ibc.getEntityCount(Environment.NETHER, EntityType.CHICKEN)); + assertEquals("test-island-id", entityIslandMap().get(chicken.getUniqueId())); + } + + @Test + void testPortaledEntityDeathStillDecrements() throws Exception { + // Regression: Paper never fires an EntityRemoveEvent for the dimension change itself + // (RemovalReason.CHANGED_DIMENSION has a null Bukkit cause), so the old justPortaled + // guard was never consumed and instead swallowed the entity's real death decrement, + // leaving a phantom count in the destination environment. + Location netherLoc = mockNetherLocation(); + World nether = netherLoc.getWorld(); + LivingEntity chicken = mockEntity(EntityType.CHICKEN, location); + ibc.incrementEntity(Environment.NORMAL, EntityType.CHICKEN); + ell.onEntityPortal(new EntityPortalEvent(chicken, location, netherLoc)); + assertEquals(1, ibc.getEntityCount(Environment.NETHER, EntityType.CHICKEN)); + + // The entity now lives in the nether and dies there. + when(chicken.getWorld()).thenReturn(nether); + when(chicken.getLocation()).thenReturn(netherLoc); + ell.onEntityRemove(new EntityRemoveEvent(chicken, EntityRemoveEvent.Cause.DEATH)); + + assertEquals(0, ibc.getEntityCount(Environment.NETHER, EntityType.CHICKEN)); + assertFalse(entityIslandMap().containsKey(chicken.getUniqueId())); + } + + @Test + void testRoundTripPortalThenDeathLeavesNoPhantomCount() throws Exception { + // The reported symptom: a mob portals to the nether, comes back, and dies in the + // overworld — the overworld count must return to zero, not stick at a phantom 1. + Location netherLoc = mockNetherLocation(); + World nether = netherLoc.getWorld(); + LivingEntity chicken = mockEntity(EntityType.CHICKEN, location); + ibc.incrementEntity(Environment.NORMAL, EntityType.CHICKEN); + + ell.onEntityPortal(new EntityPortalEvent(chicken, location, netherLoc)); + when(chicken.getWorld()).thenReturn(nether); + when(chicken.getLocation()).thenReturn(netherLoc); + ell.onEntityPortal(new EntityPortalEvent(chicken, netherLoc, location)); + when(chicken.getWorld()).thenReturn(world); + when(chicken.getLocation()).thenReturn(location); + + ell.onEntityRemove(new EntityRemoveEvent(chicken, EntityRemoveEvent.Cause.DEATH)); + + assertEquals(0, ibc.getEntityCount(Environment.NORMAL, EntityType.CHICKEN)); + assertEquals(0, ibc.getEntityCount(Environment.NETHER, EntityType.CHICKEN)); + } + + private Location mockNetherLocation() { + World nether = mock(World.class); + when(nether.getEnvironment()).thenReturn(Environment.NETHER); + when(addon.inGameModeWorld(nether)).thenReturn(true); + Location netherLoc = mock(Location.class); + when(netherLoc.getWorld()).thenReturn(nether); + return netherLoc; + } + @SuppressWarnings("unchecked") private Map entityIslandMap() throws Exception { Field f = EntityLimitListener.class.getDeclaredField("entityIslandMap");