Perf: Interactions, Fluid Flow, Block Deliveries

Improved performance for player interact handler, fluid flow handler,
and scheduled claim block deliveries.

src/me/ryanhamshire/GriefPrevention/BlockEventHandler.java

@@ -624,55 +624,31 @@ public class BlockEventHandler implements Listener
 		}
 	}
 	
-	//ensures fluids don't flow out of claims, unless into another claim where the owner is trusted to build
+	//ensures fluids don't flow into land claims from outside
 	private Claim lastSpreadClaim = null;
-	private Location lastSpreadSourceLocation = null;
 	@EventHandler(ignoreCancelled = true, priority = EventPriority.LOWEST)
 	public void onBlockFromTo (BlockFromToEvent spreadEvent)
 	{
-		//don't track in worlds where claims are not enabled
+	    //always allow fluids to flow straight down
+        if(spreadEvent.getFace() == BlockFace.DOWN) return;
+        
+	    //don't track in worlds where claims are not enabled
 		if(!GriefPrevention.instance.claimsEnabledForWorld(spreadEvent.getBlock().getWorld())) return;
 		
-		//always allow fluids to flow straight down
-		if(spreadEvent.getFace() == BlockFace.DOWN) return;
-		
-		//from where?
-		Block fromBlock = spreadEvent.getBlock();
-		Claim fromClaim = null;
-		if(fromBlock.getLocation().equals(lastSpreadSourceLocation))
-		{
-		    fromClaim = lastSpreadClaim;
-		}
-		else
-		{
-		    fromClaim = this.dataStore.getClaimAt(fromBlock.getLocation(), false, this.lastSpreadClaim);
-		    lastSpreadClaim = fromClaim;
-		    lastSpreadSourceLocation = fromBlock.getLocation();
-		}
-		
 		//where to?
-		Block toBlock = spreadEvent.getToBlock();
+        Block toBlock = spreadEvent.getToBlock();
+        Location toLocation = toBlock.getLocation();
+        Claim toClaim = this.dataStore.getClaimAt(toLocation, false, lastSpreadClaim);
         
-		//if from a land claim, this is easy and cheap - just don't allow for flowing out of that claim
+        //if into a land claim, it must be from the same land claim
-		if(fromClaim != null)
+        if(toClaim != null)
-		{
+        {
-		    if(!fromClaim.contains(toBlock.getLocation(), false, false))
+            this.lastSpreadClaim = toClaim;
-		    {
+            if(!toClaim.contains(spreadEvent.getBlock().getLocation(), false, false))
-		        spreadEvent.setCancelled(true);
+            {
-		    }
+                spreadEvent.setCancelled(true);
-		}
+            }
-		
+        }
-		//otherwise, just prevent spreading into a claim from outside
-        else
-		{
-    		Claim toClaim = this.dataStore.getClaimAt(toBlock.getLocation(), false, fromClaim);
-    		
-    		//if spreading into a claim
-    		if(toClaim != null)
-    		{		
-    			spreadEvent.setCancelled(true);
-    		}
-		}
 	}
 	
 	//ensures dispensers can't be used to dispense a block(like water or lava) or item across a claim boundary

src/me/ryanhamshire/GriefPrevention/DeliverClaimBlocksTask.java

@@ -26,53 +26,77 @@ import org.bukkit.entity.Player;
 //runs every 5 minutes in the main thread, grants blocks per hour / 12 to each online player who appears to be actively playing
 class DeliverClaimBlocksTask implements Runnable 
 {	
+    private Player player;
+    
+    public DeliverClaimBlocksTask(Player player)
+    {
+        this.player = player;
+    }
+    
 	@Override
 	public void run()
 	{
-		Player [] players = GriefPrevention.instance.getServer().getOnlinePlayers();
+		//if no player specified, this task will create a player-specific task for each online player, scheduled one tick apart
-		
+	    if(this.player == null)
-		//ensure players get at least 1 block (if accrual is totally disabled, this task won't even be scheduled)
-		int accruedBlocks = GriefPrevention.instance.config_claims_blocksAccruedPerHour / 12;
-		if(accruedBlocks < 0) accruedBlocks = 1;
-		
-		//for each online player
-		for(int i = 0; i < players.length; i++)
 		{
-			Player player = players[i];
+	        Player [] players = GriefPrevention.instance.getServer().getOnlinePlayers();
-			DataStore dataStore = GriefPrevention.instance.dataStore;
-			PlayerData playerData = dataStore.getPlayerData(player.getUniqueId());
 	        
-			Location lastLocation = playerData.lastAfkCheckLocation;
+	        long i = 0;
-			try  //distance squared will throw an exception if the player has changed worlds
+	        for(Player onlinePlayer : players)
-			{
+	        {
-				//if he's not in a vehicle and has moved at least three blocks since the last check
+	            DeliverClaimBlocksTask newTask = new DeliverClaimBlocksTask(onlinePlayer);
-				//and he's not being pushed around by fluids
+	            GriefPrevention.instance.getServer().getScheduler().scheduleSyncDelayedTask(GriefPrevention.instance, newTask, i++);
-				if(!player.isInsideVehicle() && 
+	        }
-				   (lastLocation == null || lastLocation.distanceSquared(player.getLocation()) >= 9) &&
-				   !player.getLocation().getBlock().isLiquid())
-				{					
-					//if player is over accrued limit, accrued limit was probably reduced in config file AFTER he accrued
-					//in that case, leave his blocks where they are
-					if(playerData.getAccruedClaimBlocks() > GriefPrevention.instance.config_claims_maxAccruedBlocks) continue;
-					
-					//add blocks
-					playerData.setAccruedClaimBlocks(playerData.getAccruedClaimBlocks() + accruedBlocks);
-					
-					//respect limits
-					if(playerData.getAccruedClaimBlocks() > GriefPrevention.instance.config_claims_maxAccruedBlocks)
-					{
-						playerData.setAccruedClaimBlocks(GriefPrevention.instance.config_claims_maxAccruedBlocks); 
-					}
-					
-					//intentionally NOT saving data here to reduce overall secondary storage access frequency
-					//many other operations will cause this players data to save, including his eventual logout
-					//dataStore.savePlayerData(player.getName(), playerData);
-				}
-			}
-			catch(Exception e) { }
-			
-			//remember current location for next time
-			playerData.lastAfkCheckLocation = player.getLocation();
 		}
+	    
+	    //otherwise, deliver claim blocks to the specified player
+	    else
+	    {
+	        int accruedBlocks = GriefPrevention.instance.config_claims_blocksAccruedPerHour / 12;
+	        if(accruedBlocks < 0) accruedBlocks = 1;
+	        
+	        DataStore dataStore = GriefPrevention.instance.dataStore;
+            PlayerData playerData = dataStore.getPlayerData(player.getUniqueId());
+            
+            Location lastLocation = playerData.lastAfkCheckLocation;
+            try  //distance squared will throw an exception if the player has changed worlds
+            {
+                //if he's not in a vehicle and has moved at least three blocks since the last check
+                //and he's not being pushed around by fluids
+                if(!player.isInsideVehicle() && 
+                   (lastLocation == null || lastLocation.distanceSquared(player.getLocation()) >= 9) &&
+                   !player.getLocation().getBlock().isLiquid())
+                {                   
+                    //if player is over accrued limit, accrued limit was probably reduced in config file AFTER he accrued
+                    //in that case, leave his blocks where they are
+                    if(playerData.getAccruedClaimBlocks() > GriefPrevention.instance.config_claims_maxAccruedBlocks) return;
+                    
+                    //add blocks
+                    playerData.setAccruedClaimBlocks(playerData.getAccruedClaimBlocks() + accruedBlocks);
+                    
+                    //respect limits
+                    if(playerData.getAccruedClaimBlocks() > GriefPrevention.instance.config_claims_maxAccruedBlocks)
+                    {
+                        playerData.setAccruedClaimBlocks(GriefPrevention.instance.config_claims_maxAccruedBlocks); 
+                    }
+                    
+                    //intentionally NOT saving data here to reduce overall secondary storage access frequency
+                    //many other operations will cause this players data to save, including his eventual logout
+                    //dataStore.savePlayerData(player.getName(), playerData);
+                }
+            }
+            catch(IllegalArgumentException e)  //can't measure distance when to/from are different worlds
+            {
+                
+            }
+            catch(Exception e)
+            {
+                GriefPrevention.AddLogEntry("Problem delivering claim blocks to player " + player.getName() + ":");
+                e.printStackTrace();
+            }
+            
+            //remember current location for next time
+            playerData.lastAfkCheckLocation = player.getLocation();
+	    }
 	}
 }

src/me/ryanhamshire/GriefPrevention/GriefPrevention.java

@@ -238,7 +238,7 @@ public class GriefPrevention extends JavaPlugin
 		//20L ~ 1 second
 		if(this.config_claims_blocksAccruedPerHour > 0)
 		{
-			DeliverClaimBlocksTask task = new DeliverClaimBlocksTask();
+			DeliverClaimBlocksTask task = new DeliverClaimBlocksTask(null);
 			this.getServer().getScheduler().scheduleSyncRepeatingTask(this, task, 20L * 60 * 5, 20L * 60 * 5);
 		}
 		

src/me/ryanhamshire/GriefPrevention/PlayerEventHandler.java

@@ -1115,15 +1115,16 @@ class PlayerEventHandler implements Listener
         }
         
         //don't care about left-clicking on most blocks, this is probably a break action
-        PlayerData playerData = this.dataStore.getPlayerData(player.getUniqueId());
+        PlayerData playerData = null;
         if(action == Action.LEFT_CLICK_BLOCK && clickedBlock != null)
         {
             //exception for blocks on a specific watch list
             if(!this.onLeftClickWatchList(clickedBlockType) && !GriefPrevention.instance.config_mods_accessTrustIds.Contains(new MaterialInfo(clickedBlock.getTypeId(), clickedBlock.getData(), null)))
             {
                 //and an exception for putting our fires
-                if(event.getClickedBlock() != null && event.getClickedBlock().getRelative(event.getBlockFace()).getType() == Material.FIRE)
+                if(clickedBlockType == Material.NETHERRACK && event.getClickedBlock() != null && event.getClickedBlock().getRelative(event.getBlockFace()).getType() == Material.FIRE)
                 {
+                    if(playerData == null) playerData = this.dataStore.getPlayerData(player.getUniqueId());
                     Claim claim = this.dataStore.getClaimAt(clickedBlock.getLocation(), false, playerData.lastClaim);
                     if(claim != null)
                     {
@@ -1146,11 +1147,13 @@ class PlayerEventHandler implements Listener
 		//apply rules for containers and crafting blocks
 		if(	clickedBlock != null && GriefPrevention.instance.config_claims_preventTheft && (
 						event.getAction() == Action.RIGHT_CLICK_BLOCK && (
-						clickedBlock.getState() instanceof InventoryHolder ||
+						this.isInventoryHolder(clickedBlock) ||
 						clickedBlockType == Material.ANVIL ||
 						GriefPrevention.instance.config_mods_containerTrustIds.Contains(new MaterialInfo(clickedBlock.getTypeId(), clickedBlock.getData(), null)))))
 		{			
-			//block container use while under siege, so players can't hide items from attackers
+		    if(playerData == null) playerData = this.dataStore.getPlayerData(player.getUniqueId());
+		    
+		    //block container use while under siege, so players can't hide items from attackers
 			if(playerData.siegeData != null)
 			{
 				GriefPrevention.sendMessage(player, TextMode.Err, Messages.SiegeNoContainers);
@@ -1196,7 +1199,8 @@ class PlayerEventHandler implements Listener
 				(GriefPrevention.instance.config_claims_lockTrapDoors && clickedBlockType == Material.TRAP_DOOR) ||
 				(GriefPrevention.instance.config_claims_lockFenceGates && clickedBlockType == Material.FENCE_GATE))
 		{
-			Claim claim = this.dataStore.getClaimAt(clickedBlock.getLocation(), false, playerData.lastClaim);
+		    if(playerData == null) playerData = this.dataStore.getPlayerData(player.getUniqueId());
+		    Claim claim = this.dataStore.getClaimAt(clickedBlock.getLocation(), false, playerData.lastClaim);
 			if(claim != null)
 			{
 				playerData.lastClaim = claim;
@@ -1214,10 +1218,12 @@ class PlayerEventHandler implements Listener
 		//otherwise apply rules for buttons and switches
 		else if(clickedBlock != null && GriefPrevention.instance.config_claims_preventButtonsSwitches && (clickedBlockType == null || clickedBlockType == Material.STONE_BUTTON || clickedBlockType == Material.WOOD_BUTTON || clickedBlockType == Material.LEVER || GriefPrevention.instance.config_mods_accessTrustIds.Contains(new MaterialInfo(clickedBlock.getTypeId(), clickedBlock.getData(), null))))
 		{
-			Claim claim = this.dataStore.getClaimAt(clickedBlock.getLocation(), false, playerData.lastClaim);
+		    if(playerData == null) playerData = this.dataStore.getPlayerData(player.getUniqueId());
+		    Claim claim = this.dataStore.getClaimAt(clickedBlock.getLocation(), false, playerData.lastClaim);
 			if(claim != null)
 			{
-				playerData.lastClaim = claim;
+			    if(playerData == null) playerData = this.dataStore.getPlayerData(player.getUniqueId());
+			    playerData.lastClaim = claim;
 				
 				String noAccessReason = claim.allowAccess(player);
 				if(noAccessReason != null)
@@ -1232,7 +1238,8 @@ class PlayerEventHandler implements Listener
 		//apply rule for note blocks and repeaters
 		else if(clickedBlock != null && clickedBlockType == Material.NOTE_BLOCK || clickedBlockType == Material.DIODE_BLOCK_ON || clickedBlockType == Material.DIODE_BLOCK_OFF)
 		{
-			Claim claim = this.dataStore.getClaimAt(clickedBlock.getLocation(), false, playerData.lastClaim);
+		    if(playerData == null) playerData = this.dataStore.getPlayerData(player.getUniqueId());
+		    Claim claim = this.dataStore.getClaimAt(clickedBlock.getLocation(), false, playerData.lastClaim);
 			if(claim != null)
 			{
 				String noBuildReason = claim.allowBuild(player, clickedBlockType);
@@ -1269,7 +1276,8 @@ class PlayerEventHandler implements Listener
 			
 			else if(clickedBlock != null && materialInHand ==  Material.BOAT)
 			{
-				Claim claim = this.dataStore.getClaimAt(clickedBlock.getLocation(), false, playerData.lastClaim);
+			    if(playerData == null) playerData = this.dataStore.getPlayerData(player.getUniqueId());
+			    Claim claim = this.dataStore.getClaimAt(clickedBlock.getLocation(), false, playerData.lastClaim);
 				if(claim != null)
 				{
 					String noAccessReason = claim.allowAccess(player);
@@ -1296,6 +1304,7 @@ class PlayerEventHandler implements Listener
 				}
 			
 				//enforce limit on total number of entities in this claim
+				if(playerData == null) playerData = this.dataStore.getPlayerData(player.getUniqueId());
 				Claim claim = this.dataStore.getClaimAt(clickedBlock.getLocation(), false, playerData.lastClaim);
 				if(claim == null) return;
 				
@@ -1335,6 +1344,7 @@ class PlayerEventHandler implements Listener
 					return;
 				}
 				
+				if(playerData == null) playerData = this.dataStore.getPlayerData(player.getUniqueId());
 				Claim claim = this.dataStore.getClaimAt(clickedBlock.getLocation(), false /*ignore height*/, playerData.lastClaim);
 				
 				//no claim case
@@ -1387,6 +1397,7 @@ class PlayerEventHandler implements Listener
 			else if(materialInHand != GriefPrevention.instance.config_claims_modificationTool) return;
 			
 			//disable golden shovel while under siege
+			if(playerData == null) playerData = this.dataStore.getPlayerData(player.getUniqueId());
 			if(playerData.siegeData != null)
 			{
 				GriefPrevention.sendMessage(player, TextMode.Err, Messages.SiegeNoShovel);
@@ -1588,6 +1599,7 @@ class PlayerEventHandler implements Listener
 			}
 			
 			//if he's resizing a claim and that claim hasn't been deleted since he started resizing it
+			if(playerData == null) playerData = this.dataStore.getPlayerData(player.getUniqueId());
 			if(playerData.claimResizing != null && playerData.claimResizing.inDataStore)
 			{
 				if(clickedBlock.getLocation().equals(playerData.lastShovelLocation)) return;
@@ -1724,6 +1736,7 @@ class PlayerEventHandler implements Listener
 			}
 			
 			//otherwise, since not currently resizing a claim, must be starting a resize, creating a new claim, or creating a subdivision
+			if(playerData == null) playerData = this.dataStore.getPlayerData(player.getUniqueId());
 			Claim claim = this.dataStore.getClaimAt(clickedBlock.getLocation(), true /*ignore height*/, playerData.lastClaim);			
 			
 			//if within an existing claim, he's not creating a new one
@@ -1919,7 +1932,25 @@ class PlayerEventHandler implements Listener
 		}
 	}
 	
-	private boolean onLeftClickWatchList(Material material)
+	//determines whether a block type is an inventory holder.  uses a caching strategy to save cpu time
+	private ConcurrentHashMap<Integer, Boolean> inventoryHolderCache = new ConcurrentHashMap<Integer, Boolean>();
+	private boolean isInventoryHolder(Block clickedBlock)
+	{
+	    Integer cacheKey = clickedBlock.getTypeId();
+	    Boolean cachedValue = this.inventoryHolderCache.get(cacheKey);
+	    if(cachedValue != null)
+	    {
+	        return cachedValue.booleanValue();
+	    }
+	    else
+	    {
+	        boolean isHolder = clickedBlock.getState() instanceof InventoryHolder;
+	        this.inventoryHolderCache.put(cacheKey, isHolder);
+	        return isHolder;
+	    }
+    }
+
+    private boolean onLeftClickWatchList(Material material)
 	{
 	    switch(material)
         {