Walls are now async

src/main/java/world/bentobox/greenhouses/greenhouse/Roof.java

@@ -3,6 +3,8 @@ package world.bentobox.greenhouses.greenhouse;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
 import java.util.concurrent.CompletableFuture;
 import java.util.stream.Collectors;
 
@@ -11,6 +13,7 @@ import org.bukkit.Location;
 import org.bukkit.Material;
 import org.bukkit.World;
 import org.bukkit.util.Vector;
+import org.eclipse.jdt.annotation.NonNull;
 
 import world.bentobox.bentobox.BentoBox;
 import world.bentobox.greenhouses.Greenhouses;
@@ -22,7 +25,6 @@ import world.bentobox.greenhouses.world.AsyncWorldCache;
  *
  */
 public class Roof extends MinMaxXZ {
-    private static final BentoBox PLUGIN = Greenhouses.getInstance().getPlugin();
     private static final List<Material> ROOF_BLOCKS;
     static {
         List<Material> r = Arrays.stream(Material.values())
@@ -38,8 +40,8 @@ public class Roof extends MinMaxXZ {
      * @param m - material
      * @return true if roof material
      */
-    public static boolean roofBlocks(Material m) {
+    public static boolean roofBlocks(@NonNull Material m) {
-        return ROOF_BLOCKS.contains(m)
+        return ROOF_BLOCKS.contains(Objects.requireNonNull(m))
                 || (m.equals(Material.GLOWSTONE) && Greenhouses.getInstance().getSettings().isAllowGlowstone())
                 || (m.name().endsWith("GLASS_PANE") && Greenhouses.getInstance().getSettings().isAllowPanes());
     }
@@ -114,29 +116,32 @@ public class Roof extends MinMaxXZ {
         Vector loc = location.toVector();
         // This section tries to find a roof block
         // Try just going up - this covers every case except if the player is standing under a hole
-        Bukkit.getScheduler().runTaskAsynchronously(PLUGIN, () -> {
+        Bukkit.getScheduler().runTaskAsynchronously(BentoBox.getInstance(), () -> {
             boolean found = findRoof(loc);
-            Bukkit.getScheduler().runTask(PLUGIN, () -> r.complete(found));
+            Bukkit.getScheduler().runTask(BentoBox.getInstance(), () -> r.complete(found));
         });
         return r;
     }
 
-    private boolean findRoof(Vector loc) {
+    boolean findRoof(Vector loc) {
         // This does a ever-growing check around the player to find a wall block. It is possible for the player
         // to be outside the greenhouse in this situation, so a check is done later to make sure the player is inside
-        int roofY = loc.getBlockY();
+        int startY = loc.getBlockY();
-        for (int y = roofY; y < world.getMaxHeight(); y++) {
+        for (int y = startY; y < world.getMaxHeight(); y++) {
-            if (roofBlocks(cache.getBlockType(loc.getBlockX(),y,loc.getBlockZ()))) {
+            Vector v = new Vector(loc.getBlockX(),y,loc.getBlockZ());
+            if (roofBlocks(cache.getBlockType(v))) {
                 roofFound = true;
-                loc = new Vector(loc.getBlockX(),y,loc.getBlockZ());
+                loc = v;
                 break;
             }
         }
         // If the roof was not found start going around in circles until something is found
         // Expand in ever increasing squares around location until a wall block is found
-        spiralSearch(loc, roofY);
         if (!roofFound) {
-            return false;
+            loc = spiralSearch(loc, startY);
+            if (!roofFound) {
+                return false;
+            }
         }
         // Record the height
         this.height = loc.getBlockY();
@@ -183,47 +188,49 @@ public class Roof extends MinMaxXZ {
         return location;
     }
 
-    private void spiralSearch(Vector v, int roofY) {
+    private Vector spiralSearch(Vector v, int startY) {
-        for (int radius = 0; radius < 3 && !roofFound; radius++) {
+        for (int radius = 0; radius < 3; radius++) {
-            for (int x = v.getBlockX() - radius; x <= v.getBlockX() + radius && !roofFound; x++) {
+            for (int x = v.getBlockX() - radius; x <= v.getBlockX() + radius; x++) {
-                for (int z = v.getBlockZ() - radius; z <= v.getBlockZ() + radius && !roofFound; z++) {
+                for (int z = v.getBlockZ() - radius; z <= v.getBlockZ() + radius; z++) {
                     if (!((x > v.getBlockX() - radius && x < v.getBlockX() + radius) && (z > v.getBlockZ() - radius && z < v.getBlockZ() + radius))) {
-                        checkVertically(v, x, roofY, z);
+                        Optional<Vector> r = checkVertically(x, startY, z);
+                        if (r.isPresent()) {
+                            return r.get();
+                        }
                     }
                 }
             }
         }
-
+        return v;
     }
 
     /**
      * Get highest roof block
      * @param v - vector of search block
      * @param x - x coord of current search
-     * @param roofY - roof y coord
+     * @param startY - starting y coord
      * @param z - z coord of current search
      */
-    private void checkVertically(Vector v, final int x, final int roofY, final int z) {
+    private Optional<Vector> checkVertically(final int x, final int startY, final int z) {
-        if (!Walls.wallBlocks(cache.getBlockType(x, roofY, z))) {
+        if (!Walls.wallBlocks(cache.getBlockType(x, startY, z))) {
             // Look up
-            for (int y = roofY; y < world.getMaxHeight() && !roofFound; y++) {
+            for (int y = startY; y < world.getMaxHeight() && !roofFound; y++) {
                 if (roofBlocks(cache.getBlockType(x,y,z))) {
+
                     roofFound = true;
-                    // Move roof up because there is a higher block
+                    // Roof block found
-                    v = new Vector(x,y,z);
+                    return Optional.of(new Vector(x,y,z));
                 }
             }
         }
-
+        return Optional.empty();
     }
 
-
     @Override
     public String toString() {
-        return "Roof [" + (cache != null ? "cache=" + cache + ", " : "") + "height=" + height + ", "
+        return "Roof [height=" + height + ", roofFound=" + roofFound + ", minX=" + minX + ", maxX=" + maxX + ", minZ="
-                + (location != null ? "location=" + location + ", " : "") + "roofFound=" + roofFound + ", "
+                + minZ + ", maxZ=" + maxZ + "]";
-                + (world != null ? "world=" + world + ", " : "") + "minX=" + minX + ", maxX=" + maxX + ", minZ=" + minZ
-                + ", maxZ=" + maxZ + "]";
     }
 
+
 }

src/main/java/world/bentobox/greenhouses/greenhouse/Walls.java

@@ -6,12 +6,13 @@ import java.util.List;
 import java.util.concurrent.CompletableFuture;
 import java.util.stream.Collectors;
 
+import org.bukkit.Bukkit;
 import org.bukkit.Location;
 import org.bukkit.Material;
-import org.bukkit.World;
-import org.bukkit.block.BlockFace;
 
+import world.bentobox.bentobox.BentoBox;
 import world.bentobox.greenhouses.Greenhouses;
+import world.bentobox.greenhouses.world.AsyncWorldCache;
 
 public class Walls extends MinMaxXZ {
     private static final List<Material> WALL_BLOCKS;
@@ -28,7 +29,7 @@ public class Walls extends MinMaxXZ {
 
     private int floor;
 
-    private static final List<BlockFace> ORDINALS = Arrays.asList(BlockFace.NORTH, BlockFace.SOUTH, BlockFace.EAST, BlockFace.WEST);
+    private final AsyncWorldCache cache;
 
     class WallFinder {
         int radiusMinX;
@@ -42,6 +43,16 @@ public class Walls extends MinMaxXZ {
         boolean isSearching() {
             return !stopMinX || !stopMaxX || !stopMinZ || !stopMaxZ;
         }
+        @Override
+        public String toString() {
+            return "WallFinder [radiusMinX=" + radiusMinX + ", radiusMaxX=" + radiusMaxX + ", radiusMinZ=" + radiusMinZ
+                    + ", radiusMaxZ=" + radiusMaxZ + ", stopMinX=" + stopMinX + ", stopMaxX=" + stopMaxX + ", stopMinZ="
+                    + stopMinZ + ", stopMaxZ=" + stopMaxZ + "]";
+        }
+    }
+
+    public Walls(AsyncWorldCache cache) {
+        this.cache = cache;
     }
 
     /**
@@ -49,12 +60,17 @@ public class Walls extends MinMaxXZ {
      * @param roof - the roof
      * @return Future walls
      */
-    public CompletableFuture<Walls> findWalls(Roof roof) {
+    public CompletableFuture<Walls> findWalls(final Roof roof) {
+        CompletableFuture<Walls> r = new CompletableFuture<>();
+        Bukkit.getScheduler().runTaskAsynchronously(BentoBox.getInstance(), () -> findWalls(r, roof));
+        return r;
+    }
+
+    Walls findWalls(CompletableFuture<Walls> r, Roof roof) {
         // The player is under the roof
         // Assume the player is inside the greenhouse they are trying to create
-        Location loc = roof.getLocation();
+        final Location loc = roof.getLocation();
-        World world = loc.getWorld();
+        floor = getFloorY(roof.getHeight(), roof.getMinX(), roof.getMaxX(), roof.getMinZ(), roof.getMaxZ());
-        floor = getFloorY(world, roof.getHeight(), roof.getMinX(), roof.getMaxX(), roof.getMinZ(), roof.getMaxZ());
         // Now start with the player's x and z location
         WallFinder wf = new WallFinder();
         minX = loc.getBlockX();
@@ -70,12 +86,14 @@ public class Walls extends MinMaxXZ {
         minZ--;
         maxZ++;
         // Find the floor again, only looking within the walls
-        floor = getFloorY(world, roof.getHeight(), minX, maxX, minZ,maxZ);
+        floor = getFloorY(roof.getHeight(), minX, maxX, minZ,maxZ);
-        return CompletableFuture.completedFuture(this);
+        // Complete on main thread
+        Bukkit.getScheduler().runTask(BentoBox.getInstance(), () -> r.complete(this));
+        return this;
+
     }
 
-    void lookAround(Location loc, WallFinder wf, Roof roof) {
+    void lookAround(final Location loc, WallFinder wf, final Roof roof) {
-        World world = loc.getWorld();
         // Look around player in an ever expanding cube
         minX = loc.getBlockX() - wf.radiusMinX;
         maxX = loc.getBlockX() + wf.radiusMaxX;
@@ -87,7 +105,7 @@ public class Walls extends MinMaxXZ {
                     // Only look around outside edge
                     if (!((x > minX && x < maxX) && (z > minZ && z < maxZ))) {
                         // Look at block faces
-                        lookAtBlockFaces(wf, world, x, y, z);
+                        lookAtBlockFaces(wf, x, y, z);
                     }
                 }
             }
@@ -123,48 +141,33 @@ public class Walls extends MinMaxXZ {
         }
     }
 
-    void lookAtBlockFaces(WallFinder wf, World world, int x, int y, int z) {
+    void lookAtBlockFaces(WallFinder wf, int x, int y, int z) {
-        for (BlockFace bf: ORDINALS) {
+        // positive x
-            switch (bf) {
+        if (WALL_BLOCKS.contains(cache.getBlockType(x + 1, y, z))) {
-            case EAST:
+            wf.stopMaxX = true;
-                // positive x
+        }
-                if (WALL_BLOCKS.contains(world.getBlockAt(x, y, z).getRelative(bf).getType())) {
+        // negative x
-                    wf.stopMaxX = true;
+        if (WALL_BLOCKS.contains(cache.getBlockType(x - 1, y, z))) {
-                }
+            wf.stopMinX = true;
-                break;
+        }
-            case WEST:
+        // negative Z
-                // negative x
+        if (WALL_BLOCKS.contains(cache.getBlockType(x, y, z - 1))) {
-                if (WALL_BLOCKS.contains(world.getBlockAt(x, y, z).getRelative(bf).getType())) {
+            wf.stopMinZ = true;
-                    wf.stopMinX = true;
+        }
-                }
+        // positive Z
-                break;
+        if (WALL_BLOCKS.contains(cache.getBlockType(x, y, z + 1))) {
-            case NORTH:
+            wf.stopMaxZ = true;
-                // negative Z
-                if (WALL_BLOCKS.contains(world.getBlockAt(x, y, z).getRelative(bf).getType())) {
-                    wf.stopMinZ = true;
-                }
-                break;
-            case SOUTH:
-                // positive Z
-                if (WALL_BLOCKS.contains(world.getBlockAt(x, y, z).getRelative(bf).getType())) {
-                    wf.stopMaxZ = true;
-                }
-                break;
-            default:
-                break;
-            }
         }
-
     }
 
-    int getFloorY(World world, int y, int minX, int maxX, int minZ, int maxZ) {
+    int getFloorY(int y, int minX, int maxX, int minZ, int maxZ) {
         // Find the floor - defined as the last y under the roof where there are no wall blocks
         int wallBlockCount;
         do {
             wallBlockCount = 0;
             for (int x = minX; x <= maxX; x++) {
                 for (int z = minZ; z <= maxZ; z++) {
-                    if (WALL_BLOCKS.contains(world.getBlockAt(x, y, z).getType())) {
+                    if (WALL_BLOCKS.contains(cache.getBlockType(x, y, z))) {
                         wallBlockCount++;
                     }
                 }

src/main/java/world/bentobox/greenhouses/managers/GreenhouseFinder.java

@@ -12,7 +12,6 @@ import org.bukkit.World;
 import org.bukkit.World.Environment;
 import org.bukkit.block.Block;
 
-import world.bentobox.bentobox.BentoBox;
 import world.bentobox.greenhouses.data.Greenhouse;
 import world.bentobox.greenhouses.greenhouse.Roof;
 import world.bentobox.greenhouses.greenhouse.Walls;
@@ -65,9 +64,8 @@ public class GreenhouseFinder {
                 r.complete(result);
                 return;
             }
-            BentoBox.getInstance().logDebug(roof);
             // Find the walls
-            new Walls().findWalls(roof).thenAccept(walls -> {
+            new Walls(cache).findWalls(roof).thenAccept(walls -> {
                 // Make the initial greenhouse
                 gh = new Greenhouse(location.getWorld(), walls, roof.getHeight());
                 // Set the original biome

src/main/java/world/bentobox/greenhouses/world/AsyncWorldCache.java

@@ -11,7 +11,6 @@ import org.bukkit.Material;
 import org.bukkit.World;
 import org.bukkit.util.Vector;
 
-import world.bentobox.bentobox.BentoBox;
 import world.bentobox.bentobox.util.Pair;
 import world.bentobox.bentobox.util.Util;
 import world.bentobox.greenhouses.Greenhouses;
@@ -55,7 +54,7 @@ public class AsyncWorldCache {
      * @param z - block coord
      * @return chunk snapshot
      */
-    private ChunkSnapshot getSnap(int x, int z) {
+    private ChunkSnapshot getSnap(final int x, final int z) {
         // Convert from block to chunk coords
         Pair<Integer, Integer> key = new Pair<>((x >> 4), (z >> 4));
         // Get from cache if it is available
@@ -84,12 +83,13 @@ public class AsyncWorldCache {
      * @param z block coordinate
      * @return material type
      */
-    public Material getBlockType(int x, int y, int z) {
+    public Material getBlockType(final int x, final int y, final int z) {
         // Convert block coords to chunk coords
-        int xx = x >= 0 ? x % 16 : 15 + (x % 16);
+        // TODO: simplify this - it must be easier than this!
-        int zz = z >= 0 ? z % 16 : 15 + (z % 16);
+        int xx = x >= 0 ? x % 16 : (16 + (x % 16)) % 16;
+        int zz = z >= 0 ? z % 16 : (16 + (z % 16)) % 16;
         Material m = getSnap(x,z).getBlockType(xx, y, zz);
-        BentoBox.getInstance().logDebug(m);
+
         return m;
     }
 

src/test/java/world/bentobox/greenhouses/greenhouse/RoofTest.java

@@ -3,8 +3,8 @@ package world.bentobox.greenhouses.greenhouse;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
-import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.Mockito.when;
 
 import org.bukkit.Location;

src/test/java/world/bentobox/greenhouses/greenhouse/WallsTest.java

@@ -7,10 +7,12 @@ import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.Mockito.when;
 
+import java.util.concurrent.CompletableFuture;
+
+import org.bukkit.Bukkit;
 import org.bukkit.Location;
 import org.bukkit.Material;
 import org.bukkit.World;
-import org.bukkit.block.Block;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -20,23 +22,23 @@ import org.powermock.api.mockito.PowerMockito;
 import org.powermock.core.classloader.annotations.PrepareForTest;
 import org.powermock.modules.junit4.PowerMockRunner;
 
+import world.bentobox.bentobox.BentoBox;
 import world.bentobox.greenhouses.Greenhouses;
 import world.bentobox.greenhouses.Settings;
 import world.bentobox.greenhouses.greenhouse.Walls.WallFinder;
+import world.bentobox.greenhouses.world.AsyncWorldCache;
 
 /**
  * @author tastybento
  *
  */
 @RunWith(PowerMockRunner.class)
-@PrepareForTest(Greenhouses.class)
+@PrepareForTest({Bukkit.class, Greenhouses.class})
 public class WallsTest {
 
     @Mock
     private Roof roof;
     @Mock
-    private Block block;
-    @Mock
     private Location location;
     @Mock
     private World world;
@@ -47,6 +49,12 @@ public class WallsTest {
     @Mock
     private Greenhouses addon;
     private Settings s;
+    @Mock
+    private BentoBox plugin;
+    @Mock
+    private AsyncWorldCache cache;
+
+    private CompletableFuture<Walls> r;
 
 
     /**
@@ -54,27 +62,26 @@ public class WallsTest {
      */
     @Before
     public void setUp() throws Exception {
+        PowerMockito.mockStatic(Bukkit.class, Mockito.RETURNS_MOCKS);
         PowerMockito.mockStatic(Greenhouses.class, Mockito.RETURNS_MOCKS);
         when(Greenhouses.getInstance()).thenReturn(addon);
         s = new Settings();
         when(addon.getSettings()).thenReturn(s);
+        when(addon.getPlugin()).thenReturn(plugin);
 
-
+        walls = new Walls(cache);
-        walls = new Walls();
         when(world.getMaxHeight()).thenReturn(255);
-        when(world.getBlockAt(anyInt(), anyInt(), anyInt())).thenReturn(block);
-        when(world.getBlockAt(any(Location.class))).thenReturn(block);
         when(location.getWorld()).thenReturn(world);
         when(location.getBlockX()).thenReturn(10);
         when(location.getBlockY()).thenReturn(10);
         when(location.getBlockZ()).thenReturn(10);
-        when(location.getBlock()).thenReturn(block);
         when(location.clone()).thenReturn(location);
-        when(block.getRelative(any())).thenReturn(block);
+        when(cache.getBlockType(any())).thenReturn(Material.GLASS);
-        when(block.getType()).thenReturn(Material.GLASS);
+        when(cache.getBlockType(anyInt(),anyInt(),anyInt())).thenReturn(Material.GLASS);
         when(roof.getHeight()).thenReturn(1);
         when(roof.getLocation()).thenReturn(location);
 
+        r = new CompletableFuture<>();
     }
 
     /**
@@ -82,7 +89,7 @@ public class WallsTest {
      */
     @Test
     public void testFindWalls() {
-        walls.findWalls(roof);
+        walls.findWalls(r, roof);
         assertEquals("Walls [minX=-2, maxX=11, minZ=-2, maxZ=11, floor=0]", walls.toString());
     }
 
@@ -147,7 +154,7 @@ public class WallsTest {
     @Test
     public void testLookAtBlockFaces() {
         WallFinder wf = walls.new WallFinder();
-        walls.lookAtBlockFaces(wf, world, 0, 5, -1);
+        walls.lookAtBlockFaces(wf, 0, 5, -1);
         assertTrue(wf.stopMaxX);
         assertTrue(wf.stopMaxZ);
         assertTrue(wf.stopMinX);
@@ -159,9 +166,9 @@ public class WallsTest {
      */
     @Test
     public void testLookAtBlockFacesNoGlass() {
-        when(block.getType()).thenReturn(Material.AIR);
+        when(cache.getBlockType(anyInt(), anyInt(), anyInt())).thenReturn(Material.AIR);
         WallFinder wf = walls.new WallFinder();
-        walls.lookAtBlockFaces(wf, world, 0, 5, -1);
+        walls.lookAtBlockFaces(wf, 0, 5, -1);
         assertFalse(wf.stopMaxX);
         assertFalse(wf.stopMaxZ);
         assertFalse(wf.stopMinX);
@@ -173,7 +180,7 @@ public class WallsTest {
      */
     @Test
     public void testGetFloorYZeroY() {
-        assertEquals(0, walls.getFloorY(world, 10, 0, 1, 0, 1));
+        assertEquals(0, walls.getFloorY(10, 0, 1, 0, 1));
     }
 
     /**
@@ -181,11 +188,11 @@ public class WallsTest {
      */
     @Test
     public void testGetFloorY() {
-        when(block.getType()).thenReturn(Material.GLASS, Material.GLASS,
+        when(cache.getBlockType(anyInt(), anyInt(), anyInt())).thenReturn(Material.GLASS, Material.GLASS,
                 Material.GLASS, Material.GLASS,
                 Material.GLASS, Material.GLASS,
                 Material.AIR);
-        assertEquals(8, walls.getFloorY(world, 10, 0, 1, 0, 1));
+        assertEquals(8, walls.getFloorY(10, 0, 1, 0, 1));
     }
 
     /**