[MEANWHILE] Refactor penalty time handling + add one for "item change".

Refactored penalty time handling to use a PenaltyTime object, taking
into account time running backwards, also unify attack (close combat)
penalties to a generic attack penalty. Combined.yawrate still keeps the
timeFreeze penalty, due to also cancelling other actions than melee,
still changed to a PenaltyTime object.

Changing the item in hand now leads to an attack penalty (that also goes 
for not changing the item, but changing the slot).

"Quick" addition, not much testing, except few unit tests.

Note that this could change false detection behavior of other sub-checks
of fight, because the penalty time is checked last. Previously checks
like direction or reach would have cancelled already if the player was 
within their penalty time. Hard to say if this creates new false
positives, but it will be more strict on continuous violations.

NCPCommons/src/main/java/fr/neatmonster/nocheatplus/utilities/PenaltyTime.java

@@ -0,0 +1,67 @@
+package fr.neatmonster.nocheatplus.utilities;
+
+/**
+ * Simple penalty duration checker for "current ms", taking into account clocks running backwards.
+ * @author mc_dev
+ *
+ */
+public class PenaltyTime {
+	
+    /** Last time a penalty was dealt (for consistency). */
+    private long   penaltyLast = 0;
+    
+    /** Time when the penalty ends. Penalty ends with hitting the penaltyEnd time (equality).*/
+    private long   penaltyEnd  = 0;
+    
+	/**
+	 * Merges new penalty time.
+	 * @param now Current ms time.
+	 */
+	public void applyPenalty(long duration) {
+		applyPenalty(System.currentTimeMillis(), duration);
+	}
+	
+	/**
+	 * Merges new penalty time.
+	 * @param now Current ms time.
+	 * @param duration Penalty duration in ms.
+	 */
+	public void applyPenalty(long now, long duration) {
+		penaltyLast = now;
+		if (now < penaltyLast) {
+			penaltyEnd = now + duration;
+		} else {
+			penaltyEnd = Math.max(now + duration, penaltyEnd);
+		}
+	}
+	
+	/**
+	 * Test if a penalty applies right now.
+	 * @return
+	 */
+	public boolean isPenalty() {
+		return isPenalty(System.currentTimeMillis());
+	}
+	
+	/**
+	 * Test if a penalty applies at the given time. Penalty ends with hitting the penaltyEnd time (equality).
+	 * @param now Current time in ms.
+	 * @return
+	 */
+	public boolean isPenalty(long now) {
+		if (now < penaltyLast) {
+			resetPenalty();
+			return false;
+		} else {
+			return now < penaltyEnd;
+		}
+	}
+	
+	/**
+	 * Reset the penalty.
+	 */
+	public void resetPenalty() {
+		penaltyLast = 0;
+		penaltyEnd = 0;
+	}
+}

NCPCommons/src/test/java/fr/neatmonster/nocheatplus/test/TestPenaltyTime.java

@@ -0,0 +1,60 @@
+package fr.neatmonster.nocheatplus.test;
+
+import static org.junit.Assert.fail;
+
+import org.junit.Test;
+
+import fr.neatmonster.nocheatplus.utilities.PenaltyTime;
+
+
+public class TestPenaltyTime {
+	
+	@Test
+	public void testZeroSequence() {
+		long now = System.currentTimeMillis();
+		PenaltyTime pt = new PenaltyTime();
+		pt.applyPenalty(now, 0);
+		if (pt.isPenalty(now )) {
+			fail("Expect no penalty with duration 0.");
+		}
+	}
+	
+	@Test
+	public void testSequence() {
+		long now = System.currentTimeMillis();
+		PenaltyTime pt = new PenaltyTime();
+		for (long i = 0; i < 10000; i ++) {
+			long j = i % 100;
+			if (j == 0) {
+				if (pt.isPenalty(now + i)) {
+					fail("Expect no penalty at i=" + i);
+				}
+				pt.applyPenalty(now + i, 50);
+			} else if (j < 50) {
+				if (!pt.isPenalty(now + i)) {
+					fail("Expect penalty at i=" + i);
+				}
+			} else {
+				if (pt.isPenalty(now + i)) {
+					fail("Expect no penalty at i=" + i);
+				}
+			}
+			
+			
+		}
+	}
+	
+	@Test
+	public void testReset() {
+		long now = System.currentTimeMillis();
+		PenaltyTime pt = new PenaltyTime();
+		pt.applyPenalty(now, 73);
+		if (pt.isPenalty(now - 1)) {
+			fail("isPenalty should return false on past time.");
+		}
+		pt.applyPenalty(now - 1, 73);
+		if (pt.isPenalty(now + 72)) {
+			fail("isPenalty should not return edge time after reset.");
+		}
+	}
+}

NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/combined/Combined.java

@@ -147,12 +147,12 @@ public class Combined {
 		if (total > threshold){
 			// Add time 
 			final float amount = ((total - threshold) / threshold * 1000f);
-			data.timeFreeze = Math.max(data.timeFreeze, now + (long) Math.min(Math.max(cc.yawRatePenaltyFactor * amount ,  cc.yawRatePenaltyMin), cc.yawRatePenaltyMax));
+			data.timeFreeze.applyPenalty(now, (long) Math.min(Math.max(cc.yawRatePenaltyFactor * amount ,  cc.yawRatePenaltyMin), cc.yawRatePenaltyMax));
 			// TODO: balance (100 ... 200 ) ?
 			if (cc.yawRateImprobable && Improbable.check(player, amount / 100f, now, "combined.yawrate"))
 				cancel = true;
 		}
-		if (now < data.timeFreeze){
+		if (data.timeFreeze.isPenalty()){
 		    cancel = true;
 		}
 		return cancel;

NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/combined/CombinedData.java

@@ -9,6 +9,7 @@ import fr.neatmonster.nocheatplus.checks.access.ACheckData;
 import fr.neatmonster.nocheatplus.checks.access.CheckDataFactory;
 import fr.neatmonster.nocheatplus.checks.access.ICheckData;
 import fr.neatmonster.nocheatplus.utilities.ActionFrequency;
+import fr.neatmonster.nocheatplus.utilities.PenaltyTime;
 
 public class CombinedData extends ACheckData {
 	
@@ -65,8 +66,8 @@ public class CombinedData extends ACheckData {
 	public float sumYaw;
 	public final ActionFrequency yawFreq = new ActionFrequency(3, 333);
 
-	// General penalty time (used for fighting mainly, set by yawrate check).
+	// General penalty time. Used for fighting mainly, but not only close combat (!), set by yawrate check.
-	public long timeFreeze = 0;
+	public final PenaltyTime timeFreeze = new PenaltyTime();
 	
 	// Bedleave check
 	public boolean wasInBed = false;

NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/combined/CombinedListener.java

@@ -16,7 +16,10 @@ import fr.neatmonster.nocheatplus.checks.CheckType;
 import fr.neatmonster.nocheatplus.utilities.TickTask;
 
 /**
- * GarbageCollectior class to combine some things, make available for other checks, or just because they don't fit into another section.
+ * Class to combine some things, make available for other checks, or just because they don't fit into another section.<br>
+ * This is registered before the FightListener.
+ * Do note the registration order in fr.neatmonster.nocheatplus.NoCheatPlus.onEnable (within NCPPlugin).
+ * 
  * @author mc_dev
  *
  */

NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/fight/Direction.java

@@ -86,25 +86,14 @@ public class Direction extends Check {
             // cancel the event.
             cancel = executeActions(player, data.directionVL, distance, cc.directionActions);
 
-            if (cancel)
+            if (cancel) {
-                // If we should cancel, remember the current time too.
+            	// Deal an attack penalty time.
-                data.directionLastViolationTime = System.currentTimeMillis();
+            	data.attackPenalty.applyPenalty(cc.directionPenalty);
+            }
         } else
             // Reward the player by lowering their violation level.
             data.directionVL *= 0.8D;
 
-        // If the player is still in penalty time, cancel the event anyway.
-        if (data.directionLastViolationTime + cc.directionPenalty > System.currentTimeMillis()) {
-            // A safeguard to avoid people getting stuck in penalty time indefinitely in case the system time of the
-            // server gets changed.
-        	// TODO: Change this to a general attack penalty time.
-            if (data.directionLastViolationTime > System.currentTimeMillis())
-                data.directionLastViolationTime = 0;
-
-            // They are in penalty time, therefore request cancelling of the event.
-            return true;
-        }
-
         return cancel;
     }
 }

NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/fight/FightConfig.java

@@ -67,6 +67,8 @@ public class FightConfig extends ACheckConfig {
     public final int        angleThreshold;
     public final ActionList angleActions;
     
+    public final long		toolChangeAttackPenalty;
+
     public final boolean    criticalCheck;
     public final double     criticalFallDistance;
     public final double     criticalVelocity;
@@ -133,6 +135,8 @@ public class FightConfig extends ACheckConfig {
         angleThreshold = data.getInt(ConfPaths.FIGHT_ANGLE_THRESHOLD);
         angleActions = data.getOptimizedActionList(ConfPaths.FIGHT_ANGLE_ACTIONS, Permissions.FIGHT_ANGLE);
         
+        toolChangeAttackPenalty = data.getLong(ConfPaths.FIGHT_TOOLCHANGEPENALTY);
+
         criticalCheck = data.getBoolean(ConfPaths.FIGHT_CRITICAL_CHECK);
         criticalFallDistance = data.getDouble(ConfPaths.FIGHT_CRITICAL_FALLDISTANCE);
         criticalVelocity = data.getDouble(ConfPaths.FIGHT_CRITICAL_VELOCITY);

NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/fight/FightData.java

@@ -15,6 +15,7 @@ import fr.neatmonster.nocheatplus.checks.access.ICheckData;
 import fr.neatmonster.nocheatplus.checks.access.SubCheckDataFactory;
 import fr.neatmonster.nocheatplus.hooks.APIUtils;
 import fr.neatmonster.nocheatplus.utilities.ActionFrequency;
+import fr.neatmonster.nocheatplus.utilities.PenaltyTime;
 
 /*
  * MM""""""""`M oo          dP         dP   M""""""'YMM            dP            
@@ -144,6 +145,9 @@ public class FightData extends ACheckData {
     public double lastAttackedY;
     public double lastAttackedZ;
     
+    /** Attack penalty (close combat, ENTITY_ATTACK). */
+    public final PenaltyTime attackPenalty = new PenaltyTime();
+    
     /** The entity id  which might get counter-attacked. */
     public int thornsId = Integer.MIN_VALUE;
     
@@ -155,9 +159,6 @@ public class FightData extends ACheckData {
     // Data of the angle check.
     public TreeMap<Long, Location> angleHits = new TreeMap<Long, Location>();
     
-    // Data of the direction check.
-    public long                    directionLastViolationTime = 0;
-    
     // FastHeal
     public long					   fastHealRefTime = 0;
     /** Buffer has to be initialized in constructor. */
@@ -183,7 +184,6 @@ public class FightData extends ACheckData {
     public boolean                 noSwingArmSwung;
 
     // Data of the reach check.
-    public long                    reachLastViolationTime;
 	public double                  reachMod = 1;
     
     // Data of the SelfHit check.

NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/fight/FightListener.java

@@ -15,6 +15,7 @@ import org.bukkit.event.entity.EntityRegainHealthEvent;
 import org.bukkit.event.entity.EntityRegainHealthEvent.RegainReason;
 import org.bukkit.event.player.PlayerAnimationEvent;
 import org.bukkit.event.player.PlayerChangedWorldEvent;
+import org.bukkit.event.player.PlayerItemHeldEvent;
 import org.bukkit.event.player.PlayerToggleSprintEvent;
 import org.bukkit.inventory.ItemStack;
 import org.bukkit.inventory.PlayerInventory;
@@ -46,7 +47,8 @@ import fr.neatmonster.nocheatplus.utilities.build.BuildParameters;
  *                  d8888P                                                                                     
  */
 /**
- * Central location to listen to events that are relevant for the fight checks.
+ * Central location to listen to events that are relevant for the fight checks.<br>
+ * This listener is registered after the CombinedListener.
  * 
  * @see FightEvent
  */
@@ -248,6 +250,15 @@ public class FightListener extends CheckListener implements JoinLeaveListener{
     		}
     	}
     	
+    	// Generic attacking penalty.
+    	// (Cancel after sprinting hacks, because of potential fp).
+        if (!cancelled && data.attackPenalty.isPenalty(now)) {
+        	cancelled = true;
+        	if (cc.debug) {
+        		System.out.println(player.getName() + " ~ attack penalty.");
+        	}
+        }
+    	
         return cancelled;
     }
     
@@ -475,4 +486,14 @@ public class FightListener extends CheckListener implements JoinLeaveListener{
 		final FightData data = FightData.getData(event.getPlayer());
 		data.angleHits.clear();
 	}
+	
+	@EventHandler(ignoreCancelled = false, priority = EventPriority.MONITOR)
+    public void onItemHeld(final PlayerItemHeldEvent event) {
+		final Player player = event.getPlayer();
+		final long penalty = FightConfig.getConfig(player).toolChangeAttackPenalty;
+		if (penalty > 0 ) {
+			FightData.getData(player).attackPenalty.applyPenalty(penalty);
+		}
+	}
+
 }

NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/fight/Reach.java

@@ -112,14 +112,16 @@ public class Reach extends Check {
             if (Improbable.check(player, (float) violation / 2f, System.currentTimeMillis(), "fight.reach")){
             	cancel = true;
             }
-            if (cancel){
+            if (cancel && cc.reachPenalty > 0){
-                // If we should cancel, remember the current time too.
+                // Apply an attack penalty time.
-                data.reachLastViolationTime = System.currentTimeMillis();
+                data.attackPenalty.applyPenalty(cc.reachPenalty);
             }
         }
         else if (lenpRel - distanceLimit * reachMod > 0){
         	// Silent cancel.
-            data.reachLastViolationTime = Math.max(data.reachLastViolationTime, System.currentTimeMillis() - cc.reachPenalty / 2);
+        	if (cc.reachPenalty > 0) {
+        		data.attackPenalty.applyPenalty(cc.reachPenalty / 2);
+        	}
             cancel = true;
             Improbable.feed(player, (float) (lenpRel - distanceLimit * reachMod) / 4f, System.currentTimeMillis());
         }
@@ -138,25 +140,9 @@ public class Reach extends Check {
         else{
         	data.reachMod = Math.min(1.0, data.reachMod + DYNAMIC_STEP);
         }
-        final boolean cancelByPenalty;
-        // If the player is still in penalty time, cancel the event anyway.
-        if (data.reachLastViolationTime + cc.reachPenalty > System.currentTimeMillis()) {
-            // A safeguard to avoid people getting stuck in penalty time indefinitely in case the system time of the
-            // server gets changed.
-            if (data.reachLastViolationTime > System.currentTimeMillis()){
-            	data.reachLastViolationTime = 0;
-            }
-
-            // They are in penalty time, therefore request cancelling of the event.
-            cancelByPenalty = !cancel;
-            cancel = true;
-        }
-        else{
-        	cancelByPenalty = false;
-        }
         
         if (cc.debug && player.hasPermission(Permissions.ADMINISTRATION_DEBUG)){
-            player.sendMessage("NC+: Attack " + (cancel ? (cancelByPenalty ? "(cancel/penalty) ":"(cancel/reach) ") : "") + damaged.getType()+ " height="+ StringUtil.fdec3.format(height) + " dist=" + StringUtil.fdec3.format(lenpRel) +" @" + StringUtil.fdec3.format(reachMod));
+            player.sendMessage("NC+: Attack/reach " + damaged.getType()+ " height="+ StringUtil.fdec3.format(height) + " dist=" + StringUtil.fdec3.format(lenpRel) +" @" + StringUtil.fdec3.format(reachMod));
         }
 
         return cancel;

NCPCore/src/main/java/fr/neatmonster/nocheatplus/config/ConfPaths.java

@@ -430,6 +430,7 @@ public abstract class ConfPaths {
     public static final String  FIGHT                                    = CHECKS + "fight.";
     
 	public static final String  FIGHT_CANCELDEAD                         = FIGHT + "canceldead";
+	public static final  String FIGHT_TOOLCHANGEPENALTY		 			 = FIGHT + "toolchangepenalty";
 
     private static final String FIGHT_ANGLE                              = FIGHT + "angle.";
     public static final String  FIGHT_ANGLE_CHECK                        = FIGHT_ANGLE + "active";

NCPCore/src/main/java/fr/neatmonster/nocheatplus/config/DefaultConfig.java

@@ -318,6 +318,7 @@ public class DefaultConfig extends ConfigFile {
          */
         set(ConfPaths.FIGHT_CANCELDEAD, true);
         set(ConfPaths.FIGHT_YAWRATE_CHECK, true);
+        set(ConfPaths.FIGHT_TOOLCHANGEPENALTY, 500L);
         
         set(ConfPaths.FIGHT_ANGLE_CHECK, true);
         set(ConfPaths.FIGHT_ANGLE_THRESHOLD, 50);