Rework actions execution, simplify + optimize, add metrics count

asynchronously.

src/fr/neatmonster/nocheatplus/checks/Check.java

@@ -9,6 +9,7 @@ import fr.neatmonster.nocheatplus.actions.ParameterName;
 import fr.neatmonster.nocheatplus.actions.types.ActionList;
 import fr.neatmonster.nocheatplus.hooks.NCPExemptionManager;
 import fr.neatmonster.nocheatplus.hooks.NCPHookManager;
+import fr.neatmonster.nocheatplus.metrics.MetricsData;
 import fr.neatmonster.nocheatplus.players.ExecutionHistory;
 import fr.neatmonster.nocheatplus.utilities.TickTask;
 
@@ -119,16 +120,20 @@ public abstract class Check {
             // One of the hooks has decided to cancel the VL processing, return false.
             return false;
         
-        final DelayedActionsExecution delayedActions = new DelayedActionsExecution(violationData);
+        // Add this failed check to the Metrics data (async!).
+        MetricsData.addFailed(violationData.check.type);
    
-        if (isMainThread) return delayedActions.execute();
+        final boolean hasCancel = violationData.hasCancel(); 
-        else{
+    	
-        	TickTask.requestActionsExecution(delayedActions);
+        if (isMainThread) return violationData.executeActions();
-        	return delayedActions.hasCancel();
+        else if (violationData.applicableActions.length > 0){
+        	// Only schedule if there is something to schedule.
+        	if (!hasCancel || violationData.applicableActions.length > 1)
+        		TickTask.requestActionsExecution(violationData);
         }
         
     	// (Design change: Permission checks are moved to cached permissions, lazily updated.)
-    	
+    	return hasCancel;
     }
 
     /**

src/fr/neatmonster/nocheatplus/checks/DelayedActionsExecution.java

@@ -1,63 +0,0 @@
-package fr.neatmonster.nocheatplus.checks;
-
-import fr.neatmonster.nocheatplus.actions.Action;
-import fr.neatmonster.nocheatplus.actions.types.CancelAction;
-import fr.neatmonster.nocheatplus.metrics.MetricsData;
-
-/**
- * For scheduling actions execution. This does not check the NCPHookManager.
- * <hr>
- * Not put to ViolationData itself for the possibility of adding other data (might be considered though).
- * @author mc_dev
- *
- */
-public class DelayedActionsExecution {
-
-	protected final ViolationData violationData;
-	protected final Action[] actions;
-
-	public DelayedActionsExecution(final ViolationData violationData) {
-		this.violationData = violationData;
-		actions = violationData.getActions();
-	}
-	
-	/**
-	 * Execute actions and return if cancel.
-	 * @return
-	 */
-	public boolean execute(){
-		try {
-
-       	   ViolationHistory.getHistory(violationData.player).log(violationData.check.getClass().getName(), violationData.addedVL);
-
-           // Add this failed check to the Metrics data.
-           MetricsData.addFailed(violationData.check.type);
-
-           // TODO: the time is taken here, which makes sense for delay, but otherwise ?
-           final long time = System.currentTimeMillis() / 1000L;
-           boolean cancel = false;
-           for (final Action action : actions)
-               if (Check.getHistory(violationData.player).executeAction(violationData, action, time))
-                   // The execution history said it really is time to execute the action, find out what it is and do
-                   // what is needed.
-                   if (action.execute(violationData)) cancel = true;
-
-           return cancel;
-       } catch (final Exception e) {
-           e.printStackTrace();
-           // On exceptions cancel events.
-           return true;
-       }
-	}
-	
-	/**
-	 * Check if the actions contain a cancel. 
-	 * @return
-	 */
-	public boolean hasCancel(){
-		for (final Action action : actions){
-			if (action instanceof CancelAction) return true;
-		}
-		return false;
-	}
-}

src/fr/neatmonster/nocheatplus/checks/ViolationData.java

@@ -4,6 +4,7 @@ import org.bukkit.entity.Player;
 
 import fr.neatmonster.nocheatplus.actions.Action;
 import fr.neatmonster.nocheatplus.actions.types.ActionList;
+import fr.neatmonster.nocheatplus.actions.types.CancelAction;
 
 /*
  * M""MMMMM""M oo          dP            dP   oo                   M""""""'YMM            dP            
@@ -25,6 +26,9 @@ public class ViolationData {
     /** The actions to be executed. */
     public final ActionList actions;
     
+    /** The actions applicable for the violation level. */
+    public final Action[] applicableActions;
+
     /** The violation level added. */
     public final double     addedVL;
 
@@ -58,6 +62,7 @@ public class ViolationData {
         this.vL = vL;
         this.addedVL = addedVL;
         this.actions = actions;
+        this.applicableActions = actions.getActions(vL);
     }
 
     /**
@@ -66,6 +71,44 @@ public class ViolationData {
      * @return the actions
      */
     public Action[] getActions() {
-        return actions.getActions(vL);
+        return applicableActions;
     }
+    
+	/**
+	 * Execute actions and return if cancel. Does add it to history.
+	 * @return
+	 */
+	public boolean executeActions(){
+		try {
+
+       	   ViolationHistory.getHistory(player).log(check.getClass().getName(), addedVL);
+
+           // TODO: the time is taken here, which makes sense for delay, but otherwise ?
+           final long time = System.currentTimeMillis() / 1000L;
+           boolean cancel = false;
+           for (final Action action : getActions())
+               if (Check.getHistory(player).executeAction(this, action, time))
+                   // The execution history said it really is time to execute the action, find out what it is and do
+                   // what is needed.
+                   if (action.execute(this)) cancel = true;
+
+           return cancel;
+       } catch (final Exception e) {
+           e.printStackTrace();
+           // On exceptions cancel events.
+           return true;
+       }
+	}
+    
+	/**
+	 * Check if the actions contain a cancel. 
+	 * @return
+	 */
+	public boolean hasCancel(){
+		for (final Action action : applicableActions){
+			if (action instanceof CancelAction) return true;
+		}
+		return false;
+	}
+   
 }

src/fr/neatmonster/nocheatplus/utilities/TickTask.java

@@ -11,8 +11,8 @@ import org.bukkit.entity.Player;
 
 import fr.neatmonster.nocheatplus.NoCheatPlus;
 import fr.neatmonster.nocheatplus.checks.CheckType;
-import fr.neatmonster.nocheatplus.checks.DelayedActionsExecution;
 import fr.neatmonster.nocheatplus.checks.ICheckData;
+import fr.neatmonster.nocheatplus.checks.ViolationData;
 
 /**
  * Task to run every tick, to update permissions and execute actions, and maybe later for extended lag measurement.
@@ -46,7 +46,7 @@ public class TickTask implements Runnable {
 	private static final Set<PermissionUpdateEntry> permissionUpdates = Collections.synchronizedSet(new HashSet<PermissionUpdateEntry>(50));
 	
 	/** Actions to execute. */
-	public static final List<DelayedActionsExecution> delayedActions = Collections.synchronizedList(new LinkedList<DelayedActionsExecution>());
+	public static final List<ViolationData> delayedActions = Collections.synchronizedList(new LinkedList<ViolationData>());
 	
 	/** Task id of the running TickTask */
 	protected static int taskId = -1;
@@ -67,17 +67,17 @@ public class TickTask implements Runnable {
 	}
 	
 	private void executeActions() {
-		final List<DelayedActionsExecution> copyActions = new LinkedList<DelayedActionsExecution>();
+		final List<ViolationData> copyActions = new LinkedList<ViolationData>();
 		synchronized (delayedActions) {
 			copyActions.addAll(delayedActions);
 			delayedActions.clear();
 		}
-		for (final DelayedActionsExecution actions : copyActions){
+		for (final ViolationData violationData : copyActions){
-			actions.execute();
+			violationData.executeActions();
 		}
 	}
 	
-	public static void requestActionsExecution(final DelayedActionsExecution actions) {
+	public static void requestActionsExecution(final ViolationData actions) {
 		delayedActions.add(actions);
 	}