Add new, efficient options for handling pistons (#933)

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

@@ -31,6 +31,7 @@ import org.bukkit.block.Block;
 import org.bukkit.block.BlockFace;
 import org.bukkit.block.BlockState;
 import org.bukkit.block.Hopper;
+import org.bukkit.block.PistonMoveReaction;
 import org.bukkit.block.data.BlockData;
 import org.bukkit.block.data.type.Dispenser;
 import org.bukkit.entity.Fireball;
@@ -49,6 +50,7 @@ import org.bukkit.event.block.BlockFromToEvent;
 import org.bukkit.event.block.BlockIgniteEvent;
 import org.bukkit.event.block.BlockIgniteEvent.IgniteCause;
 import org.bukkit.event.block.BlockMultiPlaceEvent;
+import org.bukkit.event.block.BlockPistonEvent;
 import org.bukkit.event.block.BlockPistonExtendEvent;
 import org.bukkit.event.block.BlockPistonRetractEvent;
 import org.bukkit.event.block.BlockPlaceEvent;
@@ -65,6 +67,7 @@ import org.bukkit.projectiles.ProjectileSource;
 
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.HashSet;
 import java.util.List;
 import java.util.UUID;
 
@@ -427,7 +430,7 @@ public class BlockEventHandler implements Listener
         }
 
         //warn players about disabled pistons outside of land claims
-        if (GriefPrevention.instance.config_pistonsInClaimsOnly &&
+        if (GriefPrevention.instance.config_pistonMovement == PistonMode.CLAIMS_ONLY &&
                 (block.getType() == Material.PISTON || block.getType() == Material.STICKY_PISTON) &&
                 claim == null)
         {
@@ -463,33 +466,47 @@ public class BlockEventHandler implements Listener
         return false;
     }
 
-    //blocks "pushing" other players' blocks around (pistons)
+    // Prevent pistons pushing blocks into or out of claims.
     @EventHandler(ignoreCancelled = true, priority = EventPriority.LOWEST)
     public void onBlockPistonExtend(BlockPistonExtendEvent event)
     {
-        //return if piston checks are not enabled
+        onPistonEvent(event, event.getBlocks());
-        if (!GriefPrevention.instance.config_checkPistonMovement) return;
+    }
 
-        //pushing down is ALWAYS safe
+    // Prevent pistons pulling blocks into or out of claims.
-        if (event.getDirection() == BlockFace.DOWN) return;
+    @EventHandler(ignoreCancelled = true, priority = EventPriority.LOWEST)
+    public void onBlockPistonRetract(BlockPistonRetractEvent event)
+    {
+        onPistonEvent(event, event.getBlocks());
+    }
 
-        //don't track in worlds where claims are not enabled
+    // Handle piston push and pulls.
+    private void onPistonEvent(BlockPistonEvent event, List<Block> blocks)
+    {
+        PistonMode pistonMode = GriefPrevention.instance.config_pistonMovement;
+        // Return if piston movements are ignored.
+        if (pistonMode == PistonMode.IGNORED) return;
+
+        // Don't check in worlds where claims are not enabled.
         if (!GriefPrevention.instance.claimsEnabledForWorld(event.getBlock().getWorld())) return;
 
+        BlockFace direction = event.getDirection();
         Block pistonBlock = event.getBlock();
-        List<Block> blocks = event.getBlocks();
+        Claim pistonClaim = this.dataStore.getClaimAt(pistonBlock.getLocation(), false, null);
 
-        //if no blocks moving, then only check to make sure we're not pushing into a claim from outside
+        // A claim is required, but the piston is not inside a claim.
-        //this avoids pistons breaking non-solids just inside a claim, like torches, doors, and touchplates
+        if (pistonClaim == null && pistonMode == PistonMode.CLAIMS_ONLY)
-        if (blocks.size() == 0)
         {
-            Block invadedBlock = pistonBlock.getRelative(event.getDirection());
+            event.setCancelled(true);
+            return;
+        }
 
-            //pushing "air" is harmless
+        // If no blocks are moving, quickly check if another claim's boundaries are violated.
-            if (invadedBlock.getType() == Material.AIR) return;
+        if (blocks.isEmpty())
-
+        {
-            if (this.dataStore.getClaimAt(pistonBlock.getLocation(), false, null) == null &&
+            Block invadedBlock = pistonBlock.getRelative(direction);
-                    this.dataStore.getClaimAt(invadedBlock.getLocation(), false, null) != null)
+            Claim invadedClaim = this.dataStore.getClaimAt(invadedBlock.getLocation(), false, pistonClaim);
+            if (invadedClaim != null && (pistonClaim == null || !pistonClaim.ownerID.equals(invadedClaim.ownerID)))
             {
                 event.setCancelled(true);
             }
@@ -497,167 +514,138 @@ public class BlockEventHandler implements Listener
             return;
         }
 
-        //who owns the piston, if anyone?
+        int minX, maxX, minY, maxY, minZ, maxZ;
-        String pistonClaimOwnerName = "_";
+        minX = maxX = pistonBlock.getX();
-        Claim claim = this.dataStore.getClaimAt(event.getBlock().getLocation(), false, null);
+        minY = maxY = pistonBlock.getY();
-        if (claim != null) pistonClaimOwnerName = claim.getOwnerName();
+        minZ = maxZ = pistonBlock.getZ();
 
-        //if pistons are limited to same-claim block movement
+        // Find min and max values for faster claim lookups and bounding box-based fast mode.
-        if (GriefPrevention.instance.config_pistonsInClaimsOnly)
+        for (Block block : blocks)
         {
-            //if piston is not in a land claim, cancel event
+            minX = Math.min(minX, block.getX());
-            if (claim == null)
+            minY = Math.min(minY, block.getY());
-            {
+            minZ = Math.min(minZ, block.getZ());
-                event.setCancelled(true);
+            maxX = Math.max(maxX, block.getX());
-                return;
+            maxY = Math.max(maxY, block.getY());
-            }
+            maxZ = Math.max(maxZ, block.getZ());
-
-            for (Block pushedBlock : event.getBlocks())
-            {
-                //if pushing blocks located outside the land claim it lives in, cancel the event
-                if (!claim.contains(pushedBlock.getLocation(), false, false))
-                {
-                    event.setCancelled(true);
-                    return;
-                }
-
-                //if pushing a block inside the claim out of the claim, cancel the event
-                //reason: could push into another land claim, don't want to spend CPU checking for that
-                //reason: push ice out, place torch, get water outside the claim
-                if (!claim.contains(pushedBlock.getRelative(event.getDirection()).getLocation(), false, false))
-                {
-                    event.setCancelled(true);
-                    return;
-                }
-            }
         }
 
-        //otherwise, consider ownership of piston and EACH pushed block
+        // Add direction to include invaded zone.
+        if (direction.getModX() > 0)
+            maxX += direction.getModX();
         else
+            minX += direction.getModX();
+        if (direction.getModY() > 0)
+            maxY += direction.getModY();
+        if (direction.getModZ() > 0)
+            maxZ += direction.getModZ();
+        else
+            minZ += direction.getModZ();
+
+        /*
+         * 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)
         {
-            //which blocks are being pushed?
+            Location minLoc = pistonClaim.getLesserBoundaryCorner();
-            Claim cachedClaim = claim;
+            Location maxLoc = pistonClaim.getGreaterBoundaryCorner();
-            for (int i = 0; i < blocks.size(); i++)
-            {
-                //if ANY of the pushed blocks are owned by someone other than the piston owner, cancel the event
-                Block block = blocks.get(i);
-                claim = this.dataStore.getClaimAt(block.getLocation(), false, cachedClaim);
-                if (claim != null)
-                {
-                    cachedClaim = claim;
-                    if (!claim.getOwnerName().equals(pistonClaimOwnerName))
-                    {
-                        event.setCancelled(true);
-                        pistonBlock.getWorld().createExplosion(pistonBlock.getLocation(), 0);
-                        pistonBlock.getWorld().dropItem(pistonBlock.getLocation(), new ItemStack(pistonBlock.getType()));
-                        pistonBlock.setType(Material.AIR);
-                        return;
-                    }
-                }
-            }
 
-            //if any of the blocks are being pushed into a claim from outside, cancel the event
+            if (minY < minLoc.getY() || minX < minLoc.getBlockX() || maxX > maxLoc.getBlockX() || minZ < minLoc.getBlockZ() || maxZ > maxLoc.getBlockZ())
-            for (int i = 0; i < blocks.size(); i++)
+                event.setCancelled(true);
-            {
-                Block block = blocks.get(i);
-                Claim originalClaim = this.dataStore.getClaimAt(block.getLocation(), false, cachedClaim);
-                String originalOwnerName = "";
-                if (originalClaim != null)
-                {
-                    cachedClaim = originalClaim;
-                    originalOwnerName = originalClaim.getOwnerName();
-                }
 
-                Claim newClaim = this.dataStore.getClaimAt(block.getRelative(event.getDirection()).getLocation(), false, cachedClaim);
+            return;
-                String newOwnerName = "";
-                if (newClaim != null)
-                {
-                    newOwnerName = newClaim.getOwnerName();
-                }
-
-                //if pushing this block will change ownership, cancel the event and take away the piston (for performance reasons)
-                if (!newOwnerName.equals(originalOwnerName) && !newOwnerName.isEmpty())
-                {
-                    event.setCancelled(true);
-                    pistonBlock.getWorld().createExplosion(pistonBlock.getLocation(), 0);
-                    pistonBlock.getWorld().dropItem(pistonBlock.getLocation(), new ItemStack(pistonBlock.getType()));
-                    pistonBlock.setType(Material.AIR);
-                    return;
-                }
-            }
         }
-    }
 
-    //blocks theft by pulling blocks out of a claim (again pistons)
+        // Pushing down or pulling up is safe if all blocks are in line with the piston.
-    @EventHandler(ignoreCancelled = true, priority = EventPriority.LOWEST)
+        if (minX == maxX && minZ == maxZ && direction == (event instanceof BlockPistonExtendEvent ? BlockFace.DOWN : BlockFace.UP)) return;
-    public void onBlockPistonRetract(BlockPistonRetractEvent event)
-    {
-        //return if piston checks are not enabled
-        if (!GriefPrevention.instance.config_checkPistonMovement) return;
 
-        //pulling up is always safe
+        // Fast mode: Use the intersection of a cuboid containing all blocks instead of individual locations.
-        if (event.getDirection() == BlockFace.UP) return;
+        if (pistonMode == PistonMode.EVERYWHERE_SIMPLE)
-
-        try
         {
-            //don't track in worlds where claims are not enabled
+            ArrayList<Claim> intersectable = new ArrayList<>();
-            if (!GriefPrevention.instance.claimsEnabledForWorld(event.getBlock().getWorld())) return;
+            int chunkXMax = maxX >> 4;
+            int chunkZMax = maxZ >> 4;
 
-            //if pistons limited to only pulling blocks which are in the same claim the piston is in
+            for (int chunkX = minX >> 4; chunkX <= chunkXMax; ++chunkX)
-            if (GriefPrevention.instance.config_pistonsInClaimsOnly)
             {
-                //if piston not in a land claim, cancel event
+                for (int chunkZ = minZ >> 4; chunkZ <= chunkZMax; ++chunkZ)
-                Claim pistonClaim = this.dataStore.getClaimAt(event.getBlock().getLocation(), false, null);
+                {
-                if (pistonClaim == null && !event.getBlocks().isEmpty())
+                    ArrayList<Claim> chunkClaims = dataStore.chunksToClaimsMap.get(DataStore.getChunkHash(chunkX, chunkZ));
+                    if (chunkClaims != null)
+                        intersectable.addAll(chunkClaims);
+                }
+            }
+
+            for (Claim claim : intersectable)
+            {
+                if (claim == pistonClaim) continue;
+
+                Location minLoc = claim.getLesserBoundaryCorner();
+                Location maxLoc = claim.getGreaterBoundaryCorner();
+
+                // Ensure claim intersects with bounding box.
+                if (maxY < minLoc.getBlockY() || minX > maxLoc.getBlockX() || maxX < minLoc.getBlockX() || minZ > maxLoc.getBlockZ() || maxZ < minLoc.getBlockZ())
+                    continue;
+
+                // If owners are different, cancel.
+                if (pistonClaim == null || !pistonClaim.ownerID.equals(claim.ownerID))
                 {
                     event.setCancelled(true);
                     return;
                 }
-
-                for (Block movedBlock : event.getBlocks())
-                {
-                    //if pulled block isn't in the same land claim, cancel the event
-                    if (!pistonClaim.contains(movedBlock.getLocation(), false, false))
-                    {
-                        event.setCancelled(true);
-                        return;
-                    }
-                }
             }
 
-            //otherwise, consider ownership of both piston and block
+            return;
+        }
+
+        // Precise mode: Each block must be considered individually.
+        Claim lastClaim = pistonClaim;
+        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));
+
+        for (Block block : checkBlocks)
+        {
+            // Reimplement DataStore#getClaimAt to ignore subclaims to maximize performance.
+            Location location = block.getLocation();
+            Claim claim = null;
+            if (lastClaim != null && lastClaim.inDataStore && lastClaim.contains(location, false, true))
+                claim = lastClaim;
             else
             {
-                //who owns the piston, if anyone?
+                ArrayList<Claim> chunkClaims = dataStore.chunksToClaimsMap.get(DataStore.getChunkHash(location));
-                String pistonOwnerName = "_";
+                if (chunkClaims != null)
-                Block block = event.getBlock();
-                Location pistonLocation = block.getLocation();
-                Claim pistonClaim = this.dataStore.getClaimAt(pistonLocation, false, null);
-                if (pistonClaim != null) pistonOwnerName = pistonClaim.getOwnerName();
-
-                String movingBlockOwnerName = "_";
-                for (Block movedBlock : event.getBlocks())
                 {
-                    //who owns the moving block, if anyone?
+                    for (Claim chunkClaim : chunkClaims)
-                    Claim movingBlockClaim = this.dataStore.getClaimAt(movedBlock.getLocation(), false, pistonClaim);
-                    if (movingBlockClaim != null) movingBlockOwnerName = movingBlockClaim.getOwnerName();
-
-                    //if there are owners for the blocks, they must be the same player
-                    //otherwise cancel the event
-                    if (!pistonOwnerName.equals(movingBlockOwnerName))
                     {
-                        event.setCancelled(true);
+                        if (chunkClaim.contains(location, false, true))
-                        block.getWorld().createExplosion(block.getLocation(), 0);
+                        {
-                        block.getWorld().dropItem(block.getLocation(), new ItemStack(Material.STICKY_PISTON));
+                            claim = chunkClaim;
-                        block.setType(Material.AIR);
+                            break;
-                        return;
+                        }
                     }
                 }
             }
-        }
+
-        catch (NoSuchMethodError exception)
+            if (claim == null) continue;
-        {
+
-            GriefPrevention.AddLogEntry("Your server is running an outdated version of 1.8 which has a griefing vulnerability.  Update your server (reruns buildtools.jar to get an updated server JAR file) to ensure players can't steal claimed blocks using pistons.");
+            lastClaim = claim;
+
+            // If pushing this block will change ownership, cancel the event and take away the piston (for performance reasons).
+            if (pistonClaim == null || !pistonClaim.ownerID.equals(claim.ownerID))
+            {
+                event.setCancelled(true);
+                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/GriefPrevention.java

@@ -205,8 +205,7 @@ public class GriefPrevention extends JavaPlugin
     public HashMap<String, Integer> config_seaLevelOverride;        //override for sea level, because bukkit doesn't report the right value for all situations
 
     public boolean config_limitTreeGrowth;                          //whether trees should be prevented from growing into a claim from outside
-    public boolean config_checkPistonMovement;                      //whether to check piston movement
+    public PistonMode config_pistonMovement;                            //Setting for piston check options
-    public boolean config_pistonsInClaimsOnly;                      //whether pistons are limited to only move blocks located within the piston's land claim
 
     public boolean config_advanced_fixNegativeClaimblockAmounts;    //whether to attempt to fix negative claim block amounts (some addons cause/assume players can go into negative amounts)
     public int config_advanced_claim_expiration_check_rate;            //How often GP should check for expired claims, amount in seconds
@@ -625,12 +624,11 @@ public class GriefPrevention extends JavaPlugin
         this.config_blockSurfaceOtherExplosions = config.getBoolean("GriefPrevention.BlockSurfaceOtherExplosions", true);
         this.config_blockSkyTrees = config.getBoolean("GriefPrevention.LimitSkyTrees", true);
         this.config_limitTreeGrowth = config.getBoolean("GriefPrevention.LimitTreeGrowth", false);
-        this.config_checkPistonMovement = config.getBoolean("GriefPrevention.CheckPistonMovement", true);
+        this.config_pistonMovement = PistonMode.of(config.getString("GriefPrevention.PistonMovement", "CLAIMS_ONLY"));
-        this.config_pistonsInClaimsOnly = config.getBoolean("GriefPrevention.LimitPistonsToLandClaims", true);
+        if (config.isBoolean("GriefPrevention.LimitPistonsToLandClaims") && !config.isBoolean("GriefPrevention.LimitPistonsToLandClaims"))
-        if (!this.config_checkPistonMovement && this.config_pistonsInClaimsOnly) {
+            this.config_pistonMovement = PistonMode.EVERYWHERE_SIMPLE;
-            AddLogEntry("Error: You have enabled LimitPistonsToLandClaims, but CheckPistonMovement is off!");
+        if (config.isBoolean("GriefPrevention.CheckPistonMovement") && !config.getBoolean("GriefPrevention.CheckPistonMovement"))
-            this.config_pistonsInClaimsOnly = false;
+            this.config_pistonMovement = PistonMode.IGNORED;
-        }
 
         this.config_fireSpreads = config.getBoolean("GriefPrevention.FireSpreads", false);
         this.config_fireDestroys = config.getBoolean("GriefPrevention.FireDestroys", false);
@@ -897,8 +895,9 @@ public class GriefPrevention extends JavaPlugin
         outConfig.set("GriefPrevention.BlockSurfaceOtherExplosions", this.config_blockSurfaceOtherExplosions);
         outConfig.set("GriefPrevention.LimitSkyTrees", this.config_blockSkyTrees);
         outConfig.set("GriefPrevention.LimitTreeGrowth", this.config_limitTreeGrowth);
-        outConfig.set("GriefPrevention.CheckPistonMovement", this.config_checkPistonMovement);
+        outConfig.set("GriefPrevention.PistonMovement", this.config_pistonMovement.name());
-        outConfig.set("GriefPrevention.LimitPistonsToLandClaims", this.config_pistonsInClaimsOnly);
+        outConfig.set("GriefPrevention.CheckPistonMovement", null);
+        outConfig.set("GriefPrevention.LimitPistonsToLandClaims", null);
 
         outConfig.set("GriefPrevention.FireSpreads", this.config_fireSpreads);
         outConfig.set("GriefPrevention.FireDestroys", this.config_fireDestroys);

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

@@ -0,0 +1,25 @@
+package me.ryanhamshire.GriefPrevention;
+
+public enum PistonMode {
+
+    EVERYWHERE,
+    EVERYWHERE_SIMPLE,
+    CLAIMS_ONLY,
+    IGNORED;
+
+    public static PistonMode of(String value)
+    {
+        if (value == null) {
+            return CLAIMS_ONLY;
+        }
+        try
+        {
+            return valueOf(value.toUpperCase());
+        }
+        catch (IllegalArgumentException e)
+        {
+            return CLAIMS_ONLY;
+        }
+    }
+
+}

src/main/java/me/ryanhamshire/GriefPrevention/metrics/MetricsHandler.java

@@ -93,7 +93,7 @@ public class MetricsHandler
         addSimplePie("block_sky_trees", plugin.config_blockSkyTrees);
         addSimplePie("limit_tree_growth", plugin.config_limitTreeGrowth);
 
-        addSimplePie("pistons_only_work_in_claims", plugin.config_pistonsInClaimsOnly);
+        addSimplePie("pistons_only_work_in_claims", plugin.config_pistonMovement.name().toLowerCase().replace('_', ' '));
         addSimplePie("creatures_trample_crops", plugin.config_creaturesTrampleCrops);
 
         addSimplePie("claim_tool", plugin.config_claims_modificationTool.name());