Use lastKeepAliveTime (NetData) in fight.godmode.

* Update lastKeepAliveTime from KeepAliveFrequency (even if that is
disabled).
* Update lastKeepAliveTime from FlyingFrequency too.
* Allow to test for feature tags efficiently.

NCPCompatProtocolLib/src/main/java/fr/neatmonster/nocheatplus/checks/net/protocollib/FlyingFrequency.java

@@ -22,7 +22,7 @@ import fr.neatmonster.nocheatplus.utilities.TrigUtil;
 
 /**
  * Prevent extremely fast ticking by just sending packets that don't do anything
- * new and also don't trigger moving events in CraftBukkit.
+ * new and also don't trigger moving events in CraftBukkit. Also update lastKeepAliveTime.
  * 
  * @author dev1mc
  *
@@ -71,6 +71,7 @@ public class FlyingFrequency extends BaseAdapter {
         counters.add(idHandled, 1);
 
         final NetData data = dataFactory.getData(player);
+        data.lastKeepAliveTime = time; // Update without much of a contract.
         
         // Counting all packets.
         // TODO: Consider using the NetStatic check.

NCPCompatProtocolLib/src/main/java/fr/neatmonster/nocheatplus/checks/net/protocollib/KeepAliveFrequency.java

@@ -12,6 +12,13 @@ import fr.neatmonster.nocheatplus.checks.CheckType;
 import fr.neatmonster.nocheatplus.checks.net.NetConfig;
 import fr.neatmonster.nocheatplus.checks.net.NetData;
 
+/**
+ * Limit keep alive packet frequency, set lastKeepAliveTime (even if disabled,
+ * in case fight.godmode is enabled).
+ * 
+ * @author asofold
+ *
+ */
 public class KeepAliveFrequency extends BaseAdapter {
 
     /** Dummy check for bypass checking and actions execution. */
@@ -30,15 +37,17 @@ public class KeepAliveFrequency extends BaseAdapter {
             event.setCancelled(true);
             return;
         }
+        // Always update last received time.
+        final NetData data = dataFactory.getData(player);
+        data.lastKeepAliveTime = time;
+        // Check activation.
         final NetConfig cc = configFactory.getConfig(player);
         if (!cc.keepAliveFrequencyActive) {
             return;
         }
-        final NetData data = dataFactory.getData(player);
         // TODO: Better modeling of actual packet sequences (flying vs. keep alive vs. request/ping).
-        // TODO: Better integration wih god-mode check / trigger reset ndt.
+        // TODO: Better integration with god-mode check / trigger reset ndt.
         data.keepAliveFreq.add(time, 1f);
-        // Use last time accepted as a hard reference.
         final float first = data.keepAliveFreq.bucketScore(0);
         if (first > 1f && !check.hasBypass(player)) {
             // Trigger a violation.

NCPCompatProtocolLib/src/main/java/fr/neatmonster/nocheatplus/checks/net/protocollib/ProtocolLibComponent.java

@@ -42,9 +42,11 @@ public class ProtocolLibComponent implements DisableListener, INotifyReload {
         StaticLog.logInfo("Adding packet level hooks for ProtocolLib (MC " + ProtocolLibrary.getProtocolManager().getMinecraftVersion().getVersion() + ")...");
         // Register Classes having a constructor with Plugin as argument.
         if (ConfigManager.isTrueForAnyConfig(ConfPaths.NET_FLYINGFREQUENCY_ACTIVE)) {
+            // (Also sets lastKeepAliveTime, if enabled.)
             register("fr.neatmonster.nocheatplus.checks.net.protocollib.FlyingFrequency", plugin);
         }
-        if (ConfigManager.isTrueForAnyConfig(ConfPaths.NET_KEEPALIVEFREQUENCY_ACTIVE)) {
+        if (ConfigManager.isTrueForAnyConfig(ConfPaths.NET_KEEPALIVEFREQUENCY_ACTIVE) || ConfigManager.isTrueForAnyConfig(ConfPaths.FIGHT_GODMODE_CHECK)) {
+            // (Set lastKeepAlive if this or fight.godmode is enabled.)
             register("fr.neatmonster.nocheatplus.checks.net.protocollib.KeepAliveFrequency", plugin);
         }
         if (ConfigManager.isTrueForAnyConfig(ConfPaths.NET_SOUNDDISTANCE_ACTIVE)) {

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

@@ -3,8 +3,10 @@ package fr.neatmonster.nocheatplus.checks.fight;
 import org.bukkit.Bukkit;
 import org.bukkit.entity.Player;
 
+import fr.neatmonster.nocheatplus.NCPAPIProvider;
 import fr.neatmonster.nocheatplus.checks.Check;
 import fr.neatmonster.nocheatplus.checks.CheckType;
+import fr.neatmonster.nocheatplus.checks.net.NetData;
 import fr.neatmonster.nocheatplus.compat.BridgeHealth;
 import fr.neatmonster.nocheatplus.utilities.CheckUtils;
 import fr.neatmonster.nocheatplus.utilities.TickTask;
@@ -20,7 +22,7 @@ public class GodMode extends Check {
     public GodMode() {
         super(CheckType.FIGHT_GODMODE);
     }
-    
+
     /**
      * New style god mode check. Much more sensitive.
      * @param player
@@ -28,140 +30,143 @@ public class GodMode extends Check {
      * @return
      */
     public boolean check(final Player player, final double damage, final FightData data){
-    	final int tick = TickTask.getTick();
+        final int tick = TickTask.getTick();
-    	
-    	final int noDamageTicks = Math.max(0, player.getNoDamageTicks());
-    	final int invulnerabilityTicks = mcAccess.getInvulnerableTicks(player);
-    	
-    	// TODO: cleanup this leugique beume...
-    	
-    	boolean legit = false; // Return, reduce vl.
-    	boolean set = false; // Set tick/ndt and return
-    	boolean resetAcc = false; // Reset acc counter.
-    	boolean resetAll = false; // Reset all and return
-    	
-    	// Check difference to expectation:
-    	final int dTick = tick - data.lastDamageTick;
-    	final int dNDT = data.lastNoDamageTicks - noDamageTicks;
-    	final int delta = dTick - dNDT;
-    	
-    	final double health = BridgeHealth.getHealth(player);
-    	
-    	// TODO: Adjust to double values.
-    	
-    	if (data.godModeHealth > health ){
-    		data.godModeHealthDecreaseTick = tick;
-    		legit = set = resetAcc = true;
-    	}
-    	
-    	// TODO: Might account for ndt/2 on regain health (!).
-    	
-    	// Invulnerable or inconsistent.
-    	// TODO: might check as well if NCP has taken over invulnerable ticks of this player.
-    	if (invulnerabilityTicks > 0 && noDamageTicks != invulnerabilityTicks || tick < data.lastDamageTick){
-    		// (Second criteria is for MCAccessBukkit.)
-    		legit = set = resetAcc = true;
-    	}
-    	
-    	// Reset accumulator.
-    	if (20 + data.godModeAcc < dTick || dTick > 40){
-    		legit = resetAcc = true;
-    		set = true; // TODO
-    	}
-    	
-    	// Check if reduced more than expected or new/count down fully.
-    	// TODO: Mostly workarounds.
-    	if (delta <= 0  || data.lastNoDamageTicks <= player.getMaximumNoDamageTicks() / 2 || dTick > data.lastNoDamageTicks || damage > BridgeHealth.getLastDamage(player)|| damage == 0.0){
-    		// Not resetting acc.
-    		legit = set = true;
-    	}
-    	
-    	if (dTick == 1 && noDamageTicks < 19){
-    		set = true;
-    	}
 
-    	if (delta == 1){
+        final int noDamageTicks = Math.max(0, player.getNoDamageTicks());
-    		// Ignore these, but keep reference value from before.
+        final int invulnerabilityTicks = mcAccess.getInvulnerableTicks(player);
-    		legit = true;
+
-    	}
+        // TODO: cleanup this leugique beume...
-    	
+
-//    	Bukkit.getServer().broadcastMessage("God " + player.getName() + " delta=" + delta + " dt=" + dTick + " dndt=" + dNDT + " acc=" + data.godModeAcc + " d=" + damage + " ndt=" + noDamageTicks + " h=" + health + " slag=" + TickTask.getLag(dTick, true));
+        boolean legit = false; // Return, reduce vl.
-    	
+        boolean set = false; // Set tick/ndt and return
-    	// TODO: might check last damage taken as well (really taken with health change)
+        boolean resetAcc = false; // Reset acc counter.
-    	
+        boolean resetAll = false; // Reset all and return
-    	// Resetting
+
-    	data.godModeHealth = health;
+        // Check difference to expectation:
-    	
+        final int dTick = tick - data.lastDamageTick;
-    	if (resetAcc || resetAll){
+        final int dNDT = data.lastNoDamageTicks - noDamageTicks;
-    		data.godModeAcc = 0;
+        final int delta = dTick - dNDT;
-    	}
+
-    	if (legit){
+        final double health = BridgeHealth.getHealth(player);
-    		data.godModeVL *= 0.97;
+
-    	}
+        // TODO: Adjust to double values.
-    	if (resetAll){
+
-    		// Reset all.
+        if (data.godModeHealth > health ){
-    		data.lastNoDamageTicks = 0;
+            data.godModeHealthDecreaseTick = tick;
-    		data.lastDamageTick = 0;
+            legit = set = resetAcc = true;
-    		return false;
+        }
-    	}
+
-    	else if (set){
+        // TODO: Might account for ndt/2 on regain health (!).
-    		// Only set the tick values.
+
-    		data.lastNoDamageTicks = noDamageTicks;
+        // Invulnerable or inconsistent.
-    		data.lastDamageTick = tick;
+        // TODO: might check as well if NCP has taken over invulnerable ticks of this player.
-    		return false;
+        if (invulnerabilityTicks > 0 && noDamageTicks != invulnerabilityTicks || tick < data.lastDamageTick){
-    	}
+            // (Second criteria is for MCAccessBukkit.)
-    	else if (legit){
+            legit = set = resetAcc = true;
-    		// Just return;
+        }
-    		return false;
+
-    	}
+        // Reset accumulator.
-    	
+        if (20 + data.godModeAcc < dTick || dTick > 40){
-    	if (tick < data.godModeHealthDecreaseTick){
+            legit = resetAcc = true;
-    		data.godModeHealthDecreaseTick = 0;
+            set = true; // TODO
-    	}
+        }
-    	else{
+
-    		final int dht = tick - data.godModeHealthDecreaseTick;
+        // Check if reduced more than expected or new/count down fully.
-    		if (dht <= 20) return false; 
+        // TODO: Mostly workarounds.
-    	}
+        if (delta <= 0  || data.lastNoDamageTicks <= player.getMaximumNoDamageTicks() / 2 || dTick > data.lastNoDamageTicks || damage > BridgeHealth.getLastDamage(player)|| damage == 0.0){
-    	
+            // Not resetting acc.
-    	final FightConfig cc = FightConfig.getConfig(player); 
+            legit = set = true;
-    	
+        }
-    	// Check for client side lag.
+
-    	final long now = System.currentTimeMillis();
+        if (dTick == 1 && noDamageTicks < 19){
-    	final long maxAge = cc.godModeLagMaxAge;
+            set = true;
-    	long keepAlive = Long.MIN_VALUE;
+        }
-    	// TODO: Get keepAlive from NetData, if available.
+
-    	if (keepAlive > now || keepAlive == Long.MIN_VALUE) {
+        if (delta == 1){
-    		keepAlive = CheckUtils.guessKeepAliveTime(player, now, maxAge);
+            // Ignore these, but keep reference value from before.
-    	}
+            legit = true;
-    	// TODO: else: still check the other time stamp ?
+        }
-    	
+
-    	if (keepAlive != Double.MIN_VALUE && now - keepAlive > cc.godModeLagMinAge && now - keepAlive < maxAge){
+        //    	Bukkit.getServer().broadcastMessage("God " + player.getName() + " delta=" + delta + " dt=" + dTick + " dndt=" + dNDT + " acc=" + data.godModeAcc + " d=" + damage + " ndt=" + noDamageTicks + " h=" + health + " slag=" + TickTask.getLag(dTick, true));
-    		// Assume lag.
+
-    		return false;
+        // TODO: might check last damage taken as well (really taken with health change)
-    	}
+
-    	
+        // Resetting
-    	// Violation probably.
+        data.godModeHealth = health;
-    	data.godModeAcc += delta;
+
-    	
+        if (resetAcc || resetAll){
-    	boolean cancel = false;
+            data.godModeAcc = 0;
-    	// TODO: bounds
+        }
-    	if (data.godModeAcc > 2){
+        if (legit){
-    		// TODO: To match with old checks vls / actions, either change actions or apply a factor.
+            data.godModeVL *= 0.97;
-    		data.godModeVL += delta;
+        }
-    		if (executeActions(player, data.godModeVL, delta, FightConfig.getConfig(player).godModeActions)){
+        if (resetAll){
-    			cancel = true;
+            // Reset all.
-    		}
+            data.lastNoDamageTicks = 0;
-    		else cancel = false;
+            data.lastDamageTick = 0;
-    	}
+            return false;
-    	else{
+        }
-    		cancel = false;
+        else if (set){
-    	}
+            // Only set the tick values.
-    	
+            data.lastNoDamageTicks = noDamageTicks;
-    	// Set tick values.
+            data.lastDamageTick = tick;
-    	data.lastNoDamageTicks = noDamageTicks;
+            return false;
-		data.lastDamageTick = tick;
+        }
-		
+        else if (legit){
-		return cancel;
+            // Just return;
+            return false;
+        }
+
+        if (tick < data.godModeHealthDecreaseTick){
+            data.godModeHealthDecreaseTick = 0;
+        }
+        else{
+            final int dht = tick - data.godModeHealthDecreaseTick;
+            if (dht <= 20) {
+                return false; 
+            }
+        }
+
+        final FightConfig cc = FightConfig.getConfig(player); 
+
+        // Check for client side lag.
+        final long now = System.currentTimeMillis();
+        final long maxAge = cc.godModeLagMaxAge;
+        long keepAlive = Long.MIN_VALUE;
+        if (NCPAPIProvider.getNoCheatPlusAPI().hasFeatureTag("checks", "KeepAliveFrequency")) {
+            keepAlive = ((NetData) (CheckType.NET_KEEPALIVEFREQUENCY.getDataFactory().getData(player))).lastKeepAliveTime;
+        }
+        keepAlive = Math.max(keepAlive, CheckUtils.guessKeepAliveTime(player, now, maxAge));
+
+        if (keepAlive != Double.MIN_VALUE && now - keepAlive > cc.godModeLagMinAge && now - keepAlive < maxAge){
+            // Assume lag.
+            return false;
+        }
+
+        // Violation probably.
+        data.godModeAcc += delta;
+
+        boolean cancel = false;
+        // TODO: bounds
+        if (data.godModeAcc > 2){
+            // TODO: To match with old checks vls / actions, either change actions or apply a factor.
+            data.godModeVL += delta;
+            if (executeActions(player, data.godModeVL, delta, FightConfig.getConfig(player).godModeActions)){
+                cancel = true;
+            }
+            else {
+                cancel = false;
+            }
+        }
+        else{
+            cancel = false;
+        }
+
+        // Set tick values.
+        data.lastNoDamageTicks = noDamageTicks;
+        data.lastDamageTick = tick;
+
+        return cancel;
     }
 
     /**
@@ -172,12 +177,12 @@ public class GodMode extends Check {
      *            the player
      */
     public void death(final Player player) {
-    	// TODO: Is this still relevant ?
+        // TODO: Is this still relevant ?
         // First check if the player is really dead (e.g. another plugin could have just fired an artificial event).
         if (BridgeHealth.getHealth(player) <= 0.0 && player.isDead()) {
-        	try {
+            try {
                 // Schedule a task to be executed in roughly 1.5 seconds.
-            	// TODO: Get plugin otherwise !?
+                // TODO: Get plugin otherwise !?
                 Bukkit.getScheduler().scheduleSyncDelayedTask(Bukkit.getPluginManager().getPlugin("NoCheatPlus"), new Runnable() {
                     @Override
                     public void run() {
@@ -185,7 +190,7 @@ public class GodMode extends Check {
                             // Check again if the player should be dead, and if the game didn't mark them as dead.
                             if (mcAccess.shouldBeZombie(player)){
                                 // Artificially "kill" them.
-                            	mcAccess.setDead(player, 19);
+                                mcAccess.setDead(player, 19);
                             }
                         } catch (final Exception e) {}
                     }

NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/net/NetData.java

@@ -29,6 +29,10 @@ public class NetData extends ACheckData {
      * time of the last event. System.currentTimeMillis() is used.
      */
     public ActionFrequency keepAliveFreq = new ActionFrequency(20, 1000);
+    
+    // Shared.
+    /** Last time some action was received (keep alive or flying). Also maintained for fight.godmode. */
+    public long lastKeepAliveTime = 0L;
 
     public NetData(final NetConfig config) {
         super(config);

NCPCore/src/main/java/fr/neatmonster/nocheatplus/components/NoCheatPlusAPI.java

@@ -52,6 +52,14 @@ public interface NoCheatPlusAPI extends ComponentRegistry<Object>, ComponentRegi
      */
     public void setFeatureTags(String key, Collection<String> featureTags);
 
+    /**
+     * Test if an entry has been made.
+     * @param key
+     * @param feature
+     * @return
+     */
+    public boolean hasFeatureTag(String key, String feature);
+
     /**
      * Get a map with all feature tags that have been set.
      * @return

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

@@ -1309,6 +1309,12 @@ public class NoCheatPlus extends JavaPlugin implements NoCheatPlusAPI {
         present.addAll(featureTags);
     }
 
+    @Override
+    public boolean hasFeatureTag(final String key, final String feature) {
+        final Collection<String>  features = this.featureTags.get(key);
+        return features == null ? false : features.contains(feature);
+    }
+
     @Override
     public Map<String, Set<String>> getAllFeatureTags() {
         final LinkedHashMap<String, Set<String>> allTags = new LinkedHashMap<String, Set<String>>();

NCPPlugin/src/test/java/fr/neatmonster/nocheatplus/PluginTests.java

@@ -134,6 +134,10 @@ public class PluginTests {
             throw new UnsupportedOperationException();
         }
 
+        @Override
+        public boolean hasFeatureTag(String key, String feature) {
+            throw new UnsupportedOperationException();
+        }
 
     }