From eee8709636994dd9113866db2e107d16f3dea69c Mon Sep 17 00:00:00 2001 From: tastybento Date: Wed, 22 Apr 2020 18:25:10 -0700 Subject: [PATCH] Combined safe spot checking into one place. We had two sets of checking with different criteria. This consolidates them. Fixes https://github.com/BentoBoxWorld/BentoBox/issues/1296 --- .../bentobox/managers/IslandsManager.java | 83 ++++++++++++------- .../util/teleport/SafeSpotTeleport.java | 47 ++--------- .../bentobox/managers/IslandsManagerTest.java | 44 ++++------ 3 files changed, 80 insertions(+), 94 deletions(-) diff --git a/src/main/java/world/bentobox/bentobox/managers/IslandsManager.java b/src/main/java/world/bentobox/bentobox/managers/IslandsManager.java index d8ea533fc..ce0ddd991 100644 --- a/src/main/java/world/bentobox/bentobox/managers/IslandsManager.java +++ b/src/main/java/world/bentobox/bentobox/managers/IslandsManager.java @@ -23,8 +23,6 @@ import org.bukkit.World; import org.bukkit.attribute.Attribute; import org.bukkit.block.Block; import org.bukkit.block.BlockFace; -import org.bukkit.block.data.BlockData; -import org.bukkit.block.data.Openable; import org.bukkit.entity.Boat; import org.bukkit.entity.Entity; import org.bukkit.entity.Player; @@ -212,47 +210,76 @@ public class IslandsManager { * @return true if safe, otherwise false */ public boolean isSafeLocation(@NonNull Location l) { - if (l.getWorld() == null) { - return false; - } Block ground = l.getBlock().getRelative(BlockFace.DOWN); Block space1 = l.getBlock(); Block space2 = l.getBlock().getRelative(BlockFace.UP); + return checkIfSafe(l.getWorld(), ground.getType(), space1.getType(), space2.getType()); + } - // Ground must be solid - if (!ground.getType().isSolid()) { + /** + * Check if a location is safe for teleporting + * @param world - world + * @param material - + * @param material2 + * @param material3 + * @return + */ + public boolean checkIfSafe(@Nullable World world, @NonNull Material ground, @NonNull Material space1, @NonNull Material space2) { + // Ground must be solid, space 1 and 2 must not be solid + if (world == null || !ground.isSolid() + || (space1.isSolid() && !space1.name().contains("SIGN")) + || (space2.isSolid() && !space2.name().contains("SIGN"))) { return false; } // Cannot be submerged or water cannot be dangerous - if (space1.isLiquid() && (space2.isLiquid() || plugin.getIWM().isWaterNotSafe(l.getWorld()))) { + if (space1.equals(Material.WATER) && (space2.equals(Material.WATER) || plugin.getIWM().isWaterNotSafe(world))) { return false; } - - // Portals are not "safe" - if (space1.getType() == Material.NETHER_PORTAL || ground.getType() == Material.NETHER_PORTAL || space2.getType() == Material.NETHER_PORTAL - || space1.getType() == Material.END_PORTAL || ground.getType() == Material.END_PORTAL || space2.getType() == Material.END_PORTAL) { + // Lava + if (ground.equals(Material.LAVA) + || space1.equals(Material.LAVA) + || space2.equals(Material.LAVA)) { return false; } - if (ground.getType().equals(Material.LAVA) - || space1.getType().equals(Material.LAVA) - || space2.getType().equals(Material.LAVA)) { + // Unsafe types + if (((space1.equals(Material.AIR) && space2.equals(Material.AIR)) + || (space1.equals(Material.NETHER_PORTAL) && space2.equals(Material.NETHER_PORTAL))) + && (ground.name().contains("FENCE") + || ground.name().contains("DOOR") + || ground.name().contains("GATE") + || ground.name().contains("PLATE") + || ground.name().contains("SIGN") + || ground.name().contains("BANNER") + || ground.name().contains("BUTTON") + || ground.name().contains("BOAT"))) { return false; } - - // Check for trapdoors - BlockData bd = ground.getBlockData(); - if (bd instanceof Openable) { - return !((Openable)bd).isOpen(); - } - - if (ground.getType().equals(Material.CACTUS) || ground.getType().toString().contains("BOAT") || ground.getType().toString().contains("FENCE") - || ground.getType().toString().contains("SIGN")) { + // Known unsafe blocks + switch (ground) { + // Unsafe + case ANVIL: + case BARRIER: + case CACTUS: + case END_PORTAL: + case END_ROD: + case FIRE: + case FLOWER_POT: + case LADDER: + case LEVER: + case TALL_GRASS: + case PISTON_HEAD: + case MOVING_PISTON: + case TORCH: + case WALL_TORCH: + case TRIPWIRE: + case WATER: + case COBWEB: + case NETHER_PORTAL: + case MAGMA_BLOCK: return false; + default: + return true; } - // Check that the space is not solid - // The isSolid function is not fully accurate (yet) so we have to check a few other items - // isSolid thinks that PLATEs and SIGNS are solid, but they are not - return (!space1.getType().isSolid() || space1.getType().toString().contains("SIGN")) && (!space2.getType().isSolid() || space2.getType().toString().contains("SIGN")); } /** diff --git a/src/main/java/world/bentobox/bentobox/util/teleport/SafeSpotTeleport.java b/src/main/java/world/bentobox/bentobox/util/teleport/SafeSpotTeleport.java index d26b18c41..78726ce33 100644 --- a/src/main/java/world/bentobox/bentobox/util/teleport/SafeSpotTeleport.java +++ b/src/main/java/world/bentobox/bentobox/util/teleport/SafeSpotTeleport.java @@ -245,46 +245,17 @@ public class SafeSpotTeleport { * @param z - z coordinate * @return true if this is a safe spot, false if this is a portal scan */ - private boolean checkBlock(ChunkSnapshot chunk, int x, int y, int z) { + boolean checkBlock(ChunkSnapshot chunk, int x, int y, int z) { World world = location.getWorld(); Material type = chunk.getBlockType(x, y, z); - if (!type.equals(Material.AIR)) { // AIR - Material space1 = chunk.getBlockType(x, Math.min(y + 1, SafeSpotTeleport.MAX_HEIGHT), z); - Material space2 = chunk.getBlockType(x, Math.min(y + 2, SafeSpotTeleport.MAX_HEIGHT), z); - if ((space1.equals(Material.AIR) && space2.equals(Material.AIR)) || (space1.equals(Material.NETHER_PORTAL) && space2.equals(Material.NETHER_PORTAL)) - && (!type.toString().contains("FENCE") && !type.toString().contains("DOOR") && !type.toString().contains("GATE") && !type.toString().contains("PLATE") - && !type.toString().contains("SIGN"))) { - switch (type) { - // Unsafe - case ANVIL: - case BARRIER: - case CACTUS: - case END_PORTAL: - case FIRE: - case FLOWER_POT: - case LADDER: - case LAVA: - case LEVER: - case TALL_GRASS: - case PISTON_HEAD: - case MOVING_PISTON: - case STONE_BUTTON: - case TORCH: - case TRIPWIRE: - case WATER: - case COBWEB: - //Block is dangerous - break; - case NETHER_PORTAL: - if (portal) { - // A portal has been found, switch to non-portal mode now - portal = false; - } - break; - default: - return safe(chunk, x, y, z, world); - } - } + Material space1 = chunk.getBlockType(x, Math.min(y + 1, SafeSpotTeleport.MAX_HEIGHT), z); + Material space2 = chunk.getBlockType(x, Math.min(y + 2, SafeSpotTeleport.MAX_HEIGHT), z); + if (space1.equals(Material.NETHER_PORTAL) || space2.equals(Material.NETHER_PORTAL)) { + // A portal has been found, switch to non-portal mode now + portal = false; + } + if (plugin.getIslands().checkIfSafe(world, type, space1, space2)) { + return safe(chunk, x, y, z, world); } return false; } diff --git a/src/test/java/world/bentobox/bentobox/managers/IslandsManagerTest.java b/src/test/java/world/bentobox/bentobox/managers/IslandsManagerTest.java index f906b3951..bd5dcf0f6 100644 --- a/src/test/java/world/bentobox/bentobox/managers/IslandsManagerTest.java +++ b/src/test/java/world/bentobox/bentobox/managers/IslandsManagerTest.java @@ -38,7 +38,6 @@ import org.bukkit.block.Block; import org.bukkit.block.BlockFace; import org.bukkit.block.BlockState; import org.bukkit.block.data.BlockData; -import org.bukkit.block.data.Openable; import org.bukkit.entity.Cow; import org.bukkit.entity.Creeper; import org.bukkit.entity.Entity; @@ -202,6 +201,7 @@ public class IslandsManagerTest { when(location.getWorld()).thenReturn(world); when(location.getBlock()).thenReturn(space1); when(location.getWorld()).thenReturn(world); + when(location.clone()).thenReturn(location); Chunk chunk = mock(Chunk.class); when(location.getChunk()).thenReturn(chunk); when(space1.getRelative(BlockFace.DOWN)).thenReturn(ground); @@ -360,8 +360,8 @@ public class IslandsManagerTest { @Test public void testIsSafeLocationSubmerged() { when(ground.getType()).thenReturn(Material.STONE); - when(space1.isLiquid()).thenReturn(true); - when(space2.isLiquid()).thenReturn(true); + when(space1.getType()).thenReturn(Material.WATER); + when(space2.getType()).thenReturn(Material.WATER); assertFalse(manager.isSafeLocation(location)); } @@ -373,19 +373,19 @@ public class IslandsManagerTest { when(ground.getType()).thenReturn(Material.STONE); when(space1.getType()).thenReturn(Material.AIR); when(space2.getType()).thenReturn(Material.NETHER_PORTAL); - assertFalse(manager.isSafeLocation(location)); + assertTrue(manager.isSafeLocation(location)); when(ground.getType()).thenReturn(Material.STONE); when(space1.getType()).thenReturn(Material.AIR); when(space2.getType()).thenReturn(Material.END_PORTAL); - assertFalse(manager.isSafeLocation(location)); + assertTrue(manager.isSafeLocation(location)); when(ground.getType()).thenReturn(Material.STONE); when(space1.getType()).thenReturn(Material.NETHER_PORTAL); when(space2.getType()).thenReturn(Material.AIR); - assertFalse(manager.isSafeLocation(location)); + assertTrue(manager.isSafeLocation(location)); when(ground.getType()).thenReturn(Material.STONE); when(space1.getType()).thenReturn(Material.END_PORTAL); when(space2.getType()).thenReturn(Material.AIR); - assertFalse(manager.isSafeLocation(location)); + assertTrue(manager.isSafeLocation(location)); when(ground.getType()).thenReturn(Material.NETHER_PORTAL); when(space1.getType()).thenReturn(Material.AIR); when(space2.getType()).thenReturn(Material.AIR); @@ -421,20 +421,8 @@ public class IslandsManagerTest { @Test public void testTrapDoor() { when(ground.getType()).thenReturn(Material.OAK_TRAPDOOR); - - // Open trapdoor - Openable trapDoor = mock(Openable.class); - when(trapDoor.isOpen()).thenReturn(true); - when(ground.getBlockData()).thenReturn(trapDoor); - assertFalse("Open trapdoor", manager.isSafeLocation(location)); - - when(trapDoor.isOpen()).thenReturn(false); - assertTrue("Closed trapdoor", manager.isSafeLocation(location)); - when(ground.getType()).thenReturn(Material.IRON_TRAPDOOR); - assertTrue("Closed iron trapdoor", manager.isSafeLocation(location)); - when(trapDoor.isOpen()).thenReturn(true); assertFalse("Open iron trapdoor", manager.isSafeLocation(location)); } @@ -1138,7 +1126,7 @@ public class IslandsManagerTest { IslandsManager im = new IslandsManager(plugin); assertFalse(im.fixIslandCenter(island)); } - + /** * Test method for {@link world.bentobox.bentobox.managers.IslandsManager#fixIslandCenter(Island)}. */ @@ -1171,9 +1159,9 @@ public class IslandsManagerTest { assertEquals(0, captor.getValue().getBlockX()); assertEquals(120, captor.getValue().getBlockY()); assertEquals(0, captor.getValue().getBlockZ()); - + } - + /** * Test method for {@link world.bentobox.bentobox.managers.IslandsManager#fixIslandCenter(Island)}. */ @@ -1206,9 +1194,9 @@ public class IslandsManagerTest { assertEquals(100000, captor.getValue().getBlockX()); assertEquals(120, captor.getValue().getBlockY()); assertEquals(8765, captor.getValue().getBlockZ()); - + } - + /** * Test method for {@link world.bentobox.bentobox.managers.IslandsManager#fixIslandCenter(Island)}. */ @@ -1235,7 +1223,7 @@ public class IslandsManagerTest { IslandsManager im = new IslandsManager(plugin); assertFalse(im.fixIslandCenter(island)); } - + /** * Test method for {@link world.bentobox.bentobox.managers.IslandsManager#fixIslandCenter(Island)}. */ @@ -1262,7 +1250,7 @@ public class IslandsManagerTest { IslandsManager im = new IslandsManager(plugin); assertFalse(im.fixIslandCenter(island)); } - + /** * Test method for {@link world.bentobox.bentobox.managers.IslandsManager#fixIslandCenter(Island)}. */ @@ -1295,7 +1283,7 @@ public class IslandsManagerTest { assertEquals(100050, captor.getValue().getBlockX()); assertEquals(120, captor.getValue().getBlockY()); assertEquals(8815, captor.getValue().getBlockZ()); - + } /** * Test method for {@link world.bentobox.bentobox.managers.IslandsManager#fixIslandCenter(Island)}. @@ -1314,5 +1302,5 @@ public class IslandsManagerTest { when(iwm.inWorld(eq(world))).thenReturn(false); assertFalse(im.fixIslandCenter(island)); } - + }