Add the auxiliary OnDemandTickListener class for temporary TickListener

registration.

* OnDemandTickListener contains API for convenient on-demand
registration and use.
* TickTask recognizes these and conveniently sets them
registered/unregistered.
* TickTask was optimized to allow faster adding and removal of
TickListener instances.
* Used for delayed component registration.
* Future purpose.

NCPPlugin/src/main/java/fr/neatmonster/nocheatplus/NoCheatPlus.java

@@ -77,6 +77,7 @@ import fr.neatmonster.nocheatplus.permissions.PermissionUtil.CommandProtectionEn
 import fr.neatmonster.nocheatplus.permissions.Permissions;
 import fr.neatmonster.nocheatplus.players.DataManager;
 import fr.neatmonster.nocheatplus.utilities.BlockProperties;
+import fr.neatmonster.nocheatplus.utilities.OnDemandTickListener;
 import fr.neatmonster.nocheatplus.utilities.ReflectionUtil;
 import fr.neatmonster.nocheatplus.utilities.TickTask;
 import fr.neatmonster.nocheatplus.utilities.Updates;
@@ -340,11 +341,11 @@ public class NoCheatPlus extends JavaPlugin implements NoCheatPlusAPI {
 	protected Set<Object> allComponents = new LinkedHashSet<Object>(50);
 	
 	/** Tick listener that is only needed sometimes (component registration). */
-	protected final TickListener onDemandTickListener = new TickListener() {
+	protected final OnDemandTickListener onDemandTickListener = new OnDemandTickListener() {
 		@Override
-		public void onTick(int tick, long timeLast) {
+		public boolean delegateTick(final int tick, final long timeLast) {
 			processQueuedSubComponentHolders();
-			TickTask.removeTickListener(this);
+			return false;
 		}
 	};
 	
@@ -444,7 +445,7 @@ public class NoCheatPlus extends JavaPlugin implements NoCheatPlusAPI {
 		// Components holding more components to register later.
 		if (obj instanceof IHoldSubComponents){
 			subComponentholders.add((IHoldSubComponents) obj);
-			TickTask.addTickListener(onDemandTickListener);
+			onDemandTickListener.register();
 			added = true; // Convention.
 		}
 		

NCPPlugin/src/main/java/fr/neatmonster/nocheatplus/compat/DefaultComponentFactory.java

@@ -2,7 +2,7 @@ package fr.neatmonster.nocheatplus.compat;
 
 /**
  * Default factory for add-in components which might only be available under certain circumstances.
- * This will be called from within the plugin from within onEnable to register components in a flexible way.
+ * This will be called from within the plugin in onEnable to register components in a flexible way.
  * @author mc_dev
  *
  */

NCPPlugin/src/main/java/fr/neatmonster/nocheatplus/utilities/OnDemandTickListener.java

@@ -0,0 +1,98 @@
+package fr.neatmonster.nocheatplus.utilities;
+
+import fr.neatmonster.nocheatplus.components.TickListener;
+
+/**
+ * Auxiliary class for easier short term adding of TickListener. Override delegateTick.<br>
+ * NOTES:
+ * <li>The methods in this class are not thread-safe, despite partly delegating to thread-safe methods from TickTask.</li>
+ * <li>Registering listeners while the TickTask is locked will fail, check isRegistered after calling register if that is important. Should only be the case while NCP is not enabled or not yet enabling.</li>
+ * @author mc_dev
+ *
+ */
+public abstract class OnDemandTickListener implements TickListener{
+	
+	protected boolean isRegistered = false;
+	
+	/**
+	 * Override this to get called on a tick.
+	 * @param tick See: TickListener.onTick
+	 * @param timeLast See: TickListener.onTick
+	 * @return true to stay registered, false to unregister.
+	 */
+	public abstract boolean delegateTick(final int tick, final long timeLast);
+
+	@Override
+	public void onTick(final int tick, final long timeLast) {
+		if (!isRegistered){
+			// Could happen due to concurrency.
+			// (No extra unregister, to preserve order).
+			return;
+		}
+		else if (!delegateTick(tick, timeLast)){
+			// Remove from TickListenerS.
+			unRegister();
+		}
+	}
+	
+	/**
+	 * Register with TickTask, does check the isRegistered flag.
+	 * @return This instance for chaining.
+	 */
+	public OnDemandTickListener register(){
+		return register(false);
+	}
+	
+	/**
+	 * Register with the TickTask if force is true or if isRegistered is false.
+	 * @param force Set to true to call TickTask.addTickListener.
+	 * @return
+	 */
+	public OnDemandTickListener register(final boolean force){
+		if (force || !isRegistered){
+			// Flag is set in the TickTask.
+			TickTask.addTickListener(this);
+		}
+		return this;
+	}
+	
+	/**
+	 * Unregister from TickTask, does check the isRegistered flag.
+	 * @return This instance for chaining.
+	 */
+	public OnDemandTickListener unRegister(){
+		return unRegister(false);
+	}
+	
+	/**
+	 * Unregister from TickTask, if force is true or isRegistered is true. 
+	 * @param force
+	 * @return This instance for chaining.
+	 */
+	public OnDemandTickListener unRegister(final boolean force){
+		if (force || isRegistered){
+			// Flag is set in the TickTask.
+			TickTask.removeTickListener(this);
+		}
+		return this;
+	}
+	
+	/**
+	 * A way to set isRegistered without causing any further calls to TickTask (for call from TickTask itself).<br>
+	 * This must not cause any calls that use the TickListener registry of the TickTask (deadlocks / concurrent modification etc.).<br>
+	 * Used by the TickTask, called under lock of TickListenerS.
+	 * @param registered
+	 */
+	public void setRegistered(final boolean registered){
+		isRegistered = registered;
+	}
+	
+	/**
+	 * Test if this instance has been registered with the TickTask.
+	 * @return
+	 */
+	public boolean isRegistered(){
+		return isRegistered;
+	}
+
+}

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

@@ -59,7 +59,7 @@ public class TickTask implements Runnable {
 	private static final List<ViolationData> delayedActions = new LinkedList<ViolationData>();
 	
 	/** Tick listeners to call every tick. */
-	private static final List<TickListener> tickListeners = new ArrayList<TickListener>();
+	private static final Set<TickListener> tickListeners = new LinkedHashSet<TickListener>();
 	
 	/** Last n tick durations, measured from run to run.*/
 	private static final long[] tickDurations = new long[lagMaxTicks];
@@ -169,34 +169,69 @@ public class TickTask implements Runnable {
 	}
 	
 	/**
-	 * Add a tick listener. Should be thread safe, though... why?
+	 * Add a tick listener. Can be called during processing, but will take effect on the next tick.<br>
+	 * NOTES:
+	 * <li>Thread safe.</li>
+	 * <li>Does not work if the TickTask is locked.</li>
+	 * <li>For OnDemandTickListenerS, setRegistered(true) will get called if not locked.</li>
 	 * @param listener
 	 */
 	public static void addTickListener(TickListener listener){
 		synchronized (tickListeners) {
-			if (locked) return;
+			if (locked) return; // TODO: Boolean return value ?
 			if (!tickListeners.contains(listener)){
 				tickListeners.add(listener);
 			}
+			if (listener instanceof OnDemandTickListener){
+				((OnDemandTickListener) listener).setRegistered(true);
+			}
 		}
 	}
 	
 	/**
-	 * Remove a tick listener. Should be thread safe, though... why?
+	 * Remove a tick listener. Can be called during processing, but will take effect on the next tick.<br>
+	 * NOTES:
+	 * <li>Thread safe.</li>
+	 * <li>Always works.</li>
+	 * <li>For OnDemandTickListenerS, setRegistered(false) will get called.</li>
 	 * @param listener
 	 * @return If previously contained.
 	 */
 	public static boolean removeTickListener(TickListener listener){
 		synchronized (tickListeners) {
+			if (listener instanceof OnDemandTickListener){
+				((OnDemandTickListener) listener).setRegistered(false);
+			}
 			return tickListeners.remove(listener);
 		}
 	}
 	
 	/**
-	 * Remove all of them.
+	 * Remove all of them.<br>
+	 * Notes:
+	 * <li>Thread safe.</li>
+	 * <li>Always works.</li>
+	 * <li>For OnDemandTickListenerS, setRegistered(false) will get called.</li>
 	 */
 	public static void removeAllTickListeners() {
 		synchronized (tickListeners) {
+			// Gracefully set OnDemandTickListeners to unregistered.
+			for (final TickListener listener : tickListeners){
+				if (listener instanceof OnDemandTickListener){
+					try{
+						final OnDemandTickListener odtl = (OnDemandTickListener) listener;
+						if (odtl.isRegistered()){ // Could use the flag, but this is better.
+							odtl.setRegistered(false);
+						}
+					}
+					catch(Throwable t){
+						// Unlikely.
+						LogUtil.logWarning("[NoCheatPlus] Failed to set OnDemandTickListener to unregistered state: " + t.getClass().getSimpleName());
+						LogUtil.logWarning(t);
+					}
+				}
+			}
+			// Clean listeners.
 			tickListeners.clear();
 		}
 	}
@@ -403,15 +438,20 @@ public class TickTask implements Runnable {
 	
 	/**
 	 * 
-	 * Notify all listeners.
+	 * Notify all listeners. A copy of the listeners under lock, then processed without lock. Theoretically listeners can get processed though they have already been unregistered.
 	 * 
 	 */
 	private final void notifyListeners() {
-		final List<TickListener> copyListeners = new ArrayList<TickListener>();
+		final List<TickListener> copyListeners;
 		synchronized (tickListeners) {
-			// Synchronized to allow concurrent adding (!? why ?!).
+			// Synchronized to allow concurrent adding and removal.
 			// (Ignores the locked state while still running.)
-			copyListeners.addAll(tickListeners);
+			// TODO: Policy for locked state. Though locking should only happen during onDisable, so before / after the task is run anyway.
+			if (tickListeners.isEmpty()){
+				// Future purpose.
+				return;
+			}
+			copyListeners = new ArrayList<TickListener>(tickListeners);
 		}
 		for (final TickListener listener : copyListeners){
 			try{