diff --git a/.pr-review.json b/.pr-review.json new file mode 100644 index 0000000..f5f311c --- /dev/null +++ b/.pr-review.json @@ -0,0 +1,21 @@ +{ + "languages": [ + "java", + "yaml", + "markdown" + ], + "focus": [ + "thread-safety", + "paper-chunk-ticketing", + "item-loss-prevention", + "resource-cleanup", + "performance-and-chunk-loading", + "null-safety-and-unit-tests" + ], + "exclude_paths": [ + "target/**", + "*.bak*", + "docs/**" + ], + "instructions": "Canalhandia is a Minecraft Paper 1.21.x server plugin written in modern Java 25. Enforce these core invariants:\n1. Thread Safety: Bukkit API, World, Entity, and Inventory mutations MUST run on the main server thread. Async threads only do pure calculation or async file I/O.\n2. Item & Inventory Safety: Never discard player items. Always handle full-inventory overflow by dropping excess items at the player's location. On multi-step container placement (e.g. chests), roll back blocks if not all items fit.\n3. Lifecycle & Cleanup: All registered chunk tickets, recipes, schedulers, and I/O executors must be cleanly flushed and unloaded in onDisable() and module toggles.\n4. Chunk Loading: Never trigger synchronous chunk generation or loading inside event handlers. Always check world.isChunkLoaded() before querying blocks.\n5. Test Coverage: All domain logic, coordinates math, parsers, and pure helpers must have corresponding JUnit tests in src/test/java." +} diff --git a/src/main/java/dev/marcospaulo/canalhandia/ChunkLoaderListener.java b/src/main/java/dev/marcospaulo/canalhandia/ChunkLoaderListener.java index dcfb959..bb2e30f 100644 --- a/src/main/java/dev/marcospaulo/canalhandia/ChunkLoaderListener.java +++ b/src/main/java/dev/marcospaulo/canalhandia/ChunkLoaderListener.java @@ -125,11 +125,14 @@ final class ChunkLoaderListener implements Listener { event.setDropItems(false); event.setExpToDrop(0); - if (player.getGameMode() != GameMode.CREATIVE) { + if (player.getGameMode() == GameMode.CREATIVE) { + Msg.ok(player, "Âncora de Chunk #" + loader.id() + " desativada (Modo Criativo)."); + } else { Map overflow = player.getInventory().addItem(ChunkAnchorItem.create(plugin, 1)); for (ItemStack drop : overflow.values()) { player.getWorld().dropItemNaturally(player.getLocation(), drop); } + Msg.ok(player, "Âncora de Chunk #" + loader.id() + " desativada e recolhida para o seu inventário."); } try { @@ -139,11 +142,13 @@ final class ChunkLoaderListener implements Listener { } plugin.blueMap().syncChunkLoaders(); - Msg.ok(player, "Âncora de Chunk #" + loader.id() + " desativada e recolhida para o seu inventário."); } @EventHandler(priority = EventPriority.HIGH, ignoreCancelled = true) public void onPlayerInteract(PlayerInteractEvent event) { + if (!plugin.settings().moduleEnabled(Module.CHUNKLOADER)) { + return; + } if (event.getAction() != Action.RIGHT_CLICK_BLOCK) { return; } diff --git a/src/main/java/dev/marcospaulo/canalhandia/VoidProtection.java b/src/main/java/dev/marcospaulo/canalhandia/VoidProtection.java index a70bb14..4cccc36 100644 --- a/src/main/java/dev/marcospaulo/canalhandia/VoidProtection.java +++ b/src/main/java/dev/marcospaulo/canalhandia/VoidProtection.java @@ -1,7 +1,5 @@ package dev.marcospaulo.canalhandia; -import net.kyori.adventure.text.Component; -import net.kyori.adventure.text.format.NamedTextColor; import org.bukkit.HeightMap; import org.bukkit.Location; import org.bukkit.Material; @@ -9,15 +7,13 @@ import org.bukkit.World; import org.bukkit.block.Block; import org.bukkit.block.BlockState; import org.bukkit.block.Chest; -import org.bukkit.block.DoubleChest; -import org.bukkit.entity.Player; import org.bukkit.event.entity.EntityDamageEvent; import org.bukkit.inventory.Inventory; import org.bukkit.inventory.ItemStack; import java.util.ArrayList; -import java.util.Collections; import java.util.List; +import java.util.Map; /** * Rescues player items on void death, either into a safe nearby chest @@ -25,6 +21,8 @@ import java.util.List; */ public final class VoidProtection { + private static final int MAX_CHECKED_COLUMNS = 120; + private VoidProtection() { } @@ -40,7 +38,7 @@ public final class VoidProtection { /** * Finds the nearest safe solid block with air space above it within the given horizontal radius. - * Searches in an expanding spiral from the death coordinate. + * Only checks already-loaded chunks and caps checked columns to prevent server hitches. */ public static Location findSafeChestLocation(World world, Location deathLoc, int radius) { if (world == null || deathLoc == null || radius <= 0) { @@ -49,19 +47,29 @@ public final class VoidProtection { int centerX = deathLoc.getBlockX(); int centerZ = deathLoc.getBlockZ(); int minHeight = world.getMinHeight(); + int checked = 0; - Location best = null; - double bestDistSq = Double.MAX_VALUE; + for (int r = 0; r <= radius; r += (r > 8 ? 2 : 1)) { + Location ringBest = null; + double ringBestDistSq = Double.MAX_VALUE; - for (int r = 0; r <= radius; r++) { - for (int dx = -r; dx <= r; dx++) { - for (int dz = -r; dz <= r; dz++) { - if (Math.abs(dx) != r && Math.abs(dz) != r) { - continue; // Only check perimeter of current radius + for (int dx = -r; dx <= r; dx += (r > 8 ? 2 : 1)) { + for (int dz = -r; dz <= r; dz += (r > 8 ? 2 : 1)) { + if (r > 0 && Math.abs(dx) != r && Math.abs(dz) != r) { + continue; } + if (++checked > MAX_CHECKED_COLUMNS) { + return ringBest; + } + int x = centerX + dx; int z = centerZ + dz; + // Do not load or generate new chunks synchronously on death + if (!world.isChunkLoaded(x >> 4, z >> 4)) { + continue; + } + int topY; try { topY = world.getHighestBlockYAt(x, z, HeightMap.MOTION_BLOCKING_NO_LEAVES); @@ -78,24 +86,21 @@ public final class VoidProtection { if (isSafeGround(ground) && isReplaceable(space)) { double distSq = (dx * dx) + (dz * dz); - if (distSq < bestDistSq) { - bestDistSq = distSq; - best = space.getLocation(); - if (r == 0) { - return best; - } + if (distSq < ringBestDistSq) { + ringBestDistSq = distSq; + ringBest = space.getLocation(); } } } } - if (best != null) { - return best; + if (ringBest != null) { + return ringBest; } } - return best; + return null; } - private static boolean isSafeGround(Block block) { + static boolean isSafeGround(Block block) { if (block == null) { return false; } @@ -103,13 +108,12 @@ public final class VoidProtection { if (mat.isAir() || !mat.isSolid()) { return false; } - // Avoid placing on hazardous blocks return mat != Material.LAVA && mat != Material.FIRE && mat != Material.SOUL_FIRE && mat != Material.CACTUS && mat != Material.MAGMA_BLOCK && mat != Material.SWEET_BERRY_BUSH && mat != Material.WITHER_ROSE && mat != Material.POWDER_SNOW; } - private static boolean isReplaceable(Block block) { + static boolean isReplaceable(Block block) { if (block == null) { return false; } @@ -120,18 +124,24 @@ public final class VoidProtection { /** * Stores items into a chest (and an adjacent chest if needed) at the target location. - * Returns true if items were successfully stored. + * All items must be stored without overflow; on any failure, blocks are rolled back + * and false is returned so caller can safely fall back to keepInventory. */ public static boolean rescueToChest(List items, Location chestLoc) { if (items == null || items.isEmpty() || chestLoc == null) { return false; } - try { - Block chestBlock = chestLoc.getBlock(); - chestBlock.setType(Material.CHEST, false); + Block chestBlock = chestLoc.getBlock(); + Material orig1 = chestBlock.getType(); + Block adjacent = null; + Material orig2 = null; + + try { + chestBlock.setType(Material.CHEST, false); BlockState state = chestBlock.getState(); if (!(state instanceof Chest chest)) { + chestBlock.setType(orig1, false); return false; } @@ -140,27 +150,62 @@ public final class VoidProtection { for (ItemStack item : items) { if (item != null && !item.getType().isAir()) { - var leftover = inv.addItem(item.clone()); + Map leftover = inv.addItem(item.clone()); remaining.addAll(leftover.values()); } } - // If items didn't fit in a single chest (27 slots), try expanding to an adjacent double chest if (!remaining.isEmpty()) { - Block adjacent = findAdjacentSpace(chestLoc); - if (adjacent != null) { - adjacent.setType(Material.CHEST, false); - BlockState adjState = adjacent.getState(); - if (adjState instanceof Chest adjChest) { - Inventory adjInv = adjChest.getInventory(); - for (ItemStack rem : remaining) { - adjInv.addItem(rem); - } - } + adjacent = findAdjacentSpace(chestLoc); + if (adjacent == null) { + // Cannot fit all items and no space for second chest -> rollback + inv.clear(); + chestBlock.setType(orig1, false); + return false; + } + + orig2 = adjacent.getType(); + adjacent.setType(Material.CHEST, false); + BlockState adjState = adjacent.getState(); + if (!(adjState instanceof Chest adjChest)) { + inv.clear(); + chestBlock.setType(orig1, false); + adjacent.setType(orig2, false); + return false; + } + + Inventory adjInv = adjChest.getInventory(); + List secondLeftover = new ArrayList<>(); + for (ItemStack rem : remaining) { + Map leftover = adjInv.addItem(rem); + secondLeftover.addAll(leftover.values()); + } + + if (!secondLeftover.isEmpty()) { + // Still overflowed double chest -> rollback everything + inv.clear(); + adjInv.clear(); + chestBlock.setType(orig1, false); + adjacent.setType(orig2, false); + return false; } } + return true; } catch (Exception e) { + try { + if (chestBlock.getType() == Material.CHEST) { + BlockState s = chestBlock.getState(); + if (s instanceof Chest c) c.getInventory().clear(); + chestBlock.setType(orig1, false); + } + if (adjacent != null && adjacent.getType() == Material.CHEST) { + BlockState s2 = adjacent.getState(); + if (s2 instanceof Chest c2) c2.getInventory().clear(); + adjacent.setType(orig2 != null ? orig2 : Material.AIR, false); + } + } catch (Exception ignored) { + } return false; } } diff --git a/src/test/java/dev/marcospaulo/canalhandia/VoidProtectionTest.java b/src/test/java/dev/marcospaulo/canalhandia/VoidProtectionTest.java index 320825d..2c0bfdb 100644 --- a/src/test/java/dev/marcospaulo/canalhandia/VoidProtectionTest.java +++ b/src/test/java/dev/marcospaulo/canalhandia/VoidProtectionTest.java @@ -1,8 +1,11 @@ package dev.marcospaulo.canalhandia; import org.bukkit.event.entity.EntityDamageEvent; +import org.bukkit.inventory.ItemStack; import org.junit.jupiter.api.Test; +import java.util.List; + import static org.junit.jupiter.api.Assertions.*; class VoidProtectionTest { @@ -24,6 +27,14 @@ class VoidProtectionTest { @Test void handlesNullWorldOrLocationsGracefully() { assertNull(VoidProtection.findSafeChestLocation(null, null, 10)); + assertNull(VoidProtection.findSafeChestLocation(null, null, 0)); assertFalse(VoidProtection.rescueToChest(null, null)); + assertFalse(VoidProtection.rescueToChest(List.of(), null)); + } + + @Test + void nullBlocksAreNeitherSafeNorReplaceable() { + assertFalse(VoidProtection.isSafeGround(null)); + assertFalse(VoidProtection.isReplaceable(null)); } }