Refactor pistons to use new bounding box (#1139)

src/main/java/me/ryanhamshire/GriefPrevention/BlockEventHandler.java

@@ -74,6 +74,7 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Objects;
 import java.util.UUID;
+import java.util.function.BiPredicate;
 
 //event handlers related to blocks
 public class BlockEventHandler implements Listener
@@ -516,7 +517,8 @@ public class BlockEventHandler implements Listener
 
         BlockFace direction = event.getDirection();
         Block pistonBlock = event.getBlock();
-        Claim pistonClaim = this.dataStore.getClaimAt(pistonBlock.getLocation(), false, null);
+        Claim pistonClaim = this.dataStore.getClaimAt(pistonBlock.getLocation(), false,
+                pistonMode != PistonMode.CLAIMS_ONLY, null);
 
         // A claim is required, but the piston is not inside a claim.
         if (pistonClaim == null && pistonMode == PistonMode.CLAIMS_ONLY)
@@ -532,7 +534,8 @@ public class BlockEventHandler implements Listener
             if (isRetract) return;
 
             Block invadedBlock = pistonBlock.getRelative(direction);
-            Claim invadedClaim = this.dataStore.getClaimAt(invadedBlock.getLocation(), false, pistonClaim);
+            Claim invadedClaim = this.dataStore.getClaimAt(invadedBlock.getLocation(), false,
+                    pistonMode != PistonMode.CLAIMS_ONLY, pistonClaim);
             if (invadedClaim != null && (pistonClaim == null || !Objects.equals(pistonClaim.getOwnerID(), invadedClaim.getOwnerID())))
             {
                 event.setCancelled(true);
@@ -546,23 +549,24 @@ public class BlockEventHandler implements Listener
         // Expand to include invaded zone.
         movedBlocks.resize(direction, 1);
 
-        /*
+        if (pistonClaim != null)
-         * Claims-only mode. All moved blocks must be inside of the owning claim.
-         * From BigScary:
-         *  - Could push into another land claim, don't want to spend CPU checking for that
-         *  - Push ice out, place torch, get water outside the claim
-         */
-        if (pistonMode == PistonMode.CLAIMS_ONLY)
         {
-            if (!new BoundingBox(pistonClaim).contains(movedBlocks))
+            // If blocks are all inside the same claim as the piston, allow.
+            if (new BoundingBox(pistonClaim).contains(movedBlocks)) return;
+
+            /*
+             * In claims-only mode, all moved blocks must be inside of the owning claim.
+             * From BigScary:
+             *  - Could push into another land claim, don't want to spend CPU checking for that
+             *  - Push ice out, place torch, get water outside the claim
+             */
+            if (pistonMode == PistonMode.CLAIMS_ONLY)
+            {
                 event.setCancelled(true);
-
+                return;
-            return;
+            }
         }
 
-        // Ensure we have top level claim - piston ownership is only checked based on claim owner in everywhere mode.
-        while (pistonClaim != null && pistonClaim.parent != null) pistonClaim = pistonClaim.parent;
-
         // Check if blocks are in line vertically.
         if (movedBlocks.getLength() == 1 && movedBlocks.getWidth() == 1)
         {
@@ -573,94 +577,89 @@ public class BlockEventHandler implements Listener
             if (!isRetract && direction == BlockFace.DOWN) return;
         }
 
-        // Fast mode: Use the intersection of a cuboid containing all blocks instead of individual locations.
+        // Assemble list of potentially intersecting claims from chunks interacted with.
-        if (pistonMode == PistonMode.EVERYWHERE_SIMPLE)
+        ArrayList<Claim> intersectable = new ArrayList<>();
+        int chunkXMax = movedBlocks.getMaxX() >> 4;
+        int chunkZMax = movedBlocks.getMaxZ() >> 4;
+
+        for (int chunkX = movedBlocks.getMinX() >> 4; chunkX <= chunkXMax; ++chunkX)
         {
-            ArrayList<Claim> intersectable = new ArrayList<>();
+            for (int chunkZ = movedBlocks.getMinZ() >> 4; chunkZ <= chunkZMax; ++chunkZ)
-            int chunkXMax = movedBlocks.getMaxX() >> 4;
-            int chunkZMax = movedBlocks.getMaxZ() >> 4;
-
-            for (int chunkX = movedBlocks.getMinX() >> 4; chunkX <= chunkXMax; ++chunkX)
             {
-                for (int chunkZ = movedBlocks.getMinZ() >> 4; chunkZ <= chunkZMax; ++chunkZ)
+                ArrayList<Claim> chunkClaims = dataStore.chunksToClaimsMap.get(DataStore.getChunkHash(chunkX, chunkZ));
-                {
+                if (chunkClaims == null) continue;
-                    ArrayList<Claim> chunkClaims = dataStore.chunksToClaimsMap.get(DataStore.getChunkHash(chunkX, chunkZ));
-                    if (chunkClaims == null) continue;
 
-                    for (Claim claim : chunkClaims)
+                for (Claim claim : chunkClaims)
-                    {
+                {
-                        if (pistonBlock.getWorld().equals(claim.getLesserBoundaryCorner().getWorld()))
+                    // Ensure claim is not piston claim and is in same world.
-                            intersectable.add(claim);
+                    if (pistonClaim != claim && pistonBlock.getWorld().equals(claim.getLesserBoundaryCorner().getWorld()))
-                    }
+                        intersectable.add(claim);
                 }
             }
-
-            for (Claim claim : intersectable)
-            {
-                if (claim == pistonClaim) continue;
-
-                // Ensure claim intersects with bounding box.
-                if (!new BoundingBox(claim).intersects(movedBlocks)) continue;
-
-                // If owners are different, cancel.
-                if (pistonClaim == null || !Objects.equals(pistonClaim.getOwnerID(), claim.getOwnerID()))
-                {
-                    event.setCancelled(true);
-                    return;
-                }
-            }
-
-            return;
         }
 
-        // Precise mode: Each block must be considered individually.
+        BiPredicate<Claim, BoundingBox> intersectionHandler;
-        Claim lastClaim = pistonClaim;
+        final Claim finalPistonClaim = pistonClaim;
-        HashSet<Block> checkBlocks = new HashSet<>(blocks);
 
-        // Add all blocks that will be occupied after the shift.
+        // Fast mode: Bounding box intersection always causes a conflict, even if blocks do not conflict.
-        for (Block block : blocks)
+        if (pistonMode == PistonMode.EVERYWHERE_SIMPLE)
-            if (block.getPistonMoveReaction() != PistonMoveReaction.BREAK)
-                checkBlocks.add(block.getRelative(direction));
-
-        for (Block block : checkBlocks)
         {
-            // Reimplement DataStore#getClaimAt to ignore subclaims to maximize performance.
+            intersectionHandler = (claim, claimBoundingBox) ->
-            Location location = block.getLocation();
-            Claim claim = null;
-            if (lastClaim != null && lastClaim.inDataStore && lastClaim.contains(location, false, true))
-                claim = lastClaim;
-            else
             {
-                ArrayList<Claim> chunkClaims = dataStore.chunksToClaimsMap.get(DataStore.getChunkHash(location));
+                // If owners are different, cancel.
-                if (chunkClaims != null)
+                if (finalPistonClaim == null || !Objects.equals(finalPistonClaim.getOwnerID(), claim.getOwnerID()))
                 {
-                    for (Claim chunkClaim : chunkClaims)
+                    event.setCancelled(true);
+                    return true;
+                }
+
+                // Otherwise, proceed to next claim.
+                return false;
+            };
+        }
+        // Precise mode: Bounding box intersection may not yield a conflict. Individual blocks must be considered.
+        else
+        {
+            // Set up list of affected blocks.
+            HashSet<Block> checkBlocks = new HashSet<>(blocks);
+
+            // Add all blocks that will be occupied after the shift.
+            for (Block block : blocks)
+                if (block.getPistonMoveReaction() != PistonMoveReaction.BREAK)
+                    checkBlocks.add(block.getRelative(direction));
+
+            intersectionHandler = (claim, claimBoundingBox) ->
+            {
+                // Ensure that the claim contains an affected block.
+                if (checkBlocks.stream().noneMatch(claimBoundingBox::contains)) return false;
+
+                // If pushing this block will change ownership, cancel the event and take away the piston (for performance reasons).
+                if (finalPistonClaim == null || !Objects.equals(finalPistonClaim.getOwnerID(), claim.getOwnerID()))
+                {
+                    event.setCancelled(true);
+                    if (GriefPrevention.instance.config_pistonExplosionSound)
                     {
-                        if (chunkClaim.contains(location, false, true))
+                        pistonBlock.getWorld().createExplosion(pistonBlock.getLocation(), 0);
-                        {
-                            claim = chunkClaim;
-                            break;
-                        }
                     }
+                    pistonBlock.getWorld().dropItem(pistonBlock.getLocation(), new ItemStack(event.isSticky() ? Material.STICKY_PISTON : Material.PISTON));
+                    pistonBlock.setType(Material.AIR);
+                    return true;
                 }
-            }
 
-            if (claim == null) continue;
+                // Otherwise, proceed to next claim.
+                return false;
+            };
+        }
 
-            lastClaim = claim;
+        for (Claim claim : intersectable)
+        {
+            BoundingBox claimBoundingBox = new BoundingBox(claim);
 
-            // If pushing this block will change ownership, cancel the event and take away the piston (for performance reasons).
+            // Ensure claim intersects with block bounding box.
-            if (pistonClaim == null || !Objects.equals(pistonClaim.getOwnerID(), claim.getOwnerID()))
+            if (!claimBoundingBox.intersects(movedBlocks)) continue;
-            {
+
-                event.setCancelled(true);
+            // Do additional mode-based handling.
-                if (GriefPrevention.instance.config_pistonExplosionSound)
+            if (intersectionHandler.test(claim, claimBoundingBox)) return;
-                {
-                    pistonBlock.getWorld().createExplosion(pistonBlock.getLocation(), 0);
-                }
-                pistonBlock.getWorld().dropItem(pistonBlock.getLocation(), new ItemStack(event.isSticky() ? Material.STICKY_PISTON : Material.PISTON));
-                pistonBlock.setType(Material.AIR);
-                return;
-            }
         }
     }
 

src/main/java/me/ryanhamshire/GriefPrevention/DataStore.java

@@ -726,9 +726,26 @@ public abstract class DataStore
     //ignoreHeight = TRUE means that a location UNDER an existing claim will return the claim
     //cachedClaim can be NULL, but will help performance if you have a reasonable guess about which claim the location is in
     synchronized public Claim getClaimAt(Location location, boolean ignoreHeight, Claim cachedClaim)
+    {
+        return getClaimAt(location, ignoreHeight, false, cachedClaim);
+    }
+
+    /**
+     * Get the claim at a specific location.
+     *
+     * <p>The cached claim may be null, but will increase performance if you have a reasonable idea
+     * of which claim is correct.
+     *
+     * @param location the location
+     * @param ignoreHeight whether or not to check containment vertically
+     * @param ignoreSubclaims whether or not subclaims should be returned over claims
+     * @param cachedClaim the cached claim, if any
+     * @return the claim containing the location or null if no claim exists there
+     */
+    synchronized public Claim getClaimAt(Location location, boolean ignoreHeight, boolean ignoreSubclaims, Claim cachedClaim)
     {
         //check cachedClaim guess first.  if it's in the datastore and the location is inside it, we're done
-        if (cachedClaim != null && cachedClaim.inDataStore && cachedClaim.contains(location, ignoreHeight, true))
+        if (cachedClaim != null && cachedClaim.inDataStore && cachedClaim.contains(location, ignoreHeight, !ignoreSubclaims))
             return cachedClaim;
 
         //find a top level claim
@@ -740,6 +757,9 @@ public abstract class DataStore
         {
             if (claim.inDataStore && claim.contains(location, ignoreHeight, false))
             {
+                // If ignoring subclaims, claim is a match.
+                if (ignoreSubclaims) return claim;
+
                 //when we find a top level claim, if the location is in one of its subdivisions,
                 //return the SUBDIVISION, not the top level claim
                 for (int j = 0; j < claim.children.size(); j++)