Adjust isEnabled and hasBypass use.

Refactor
* Move hasBypass code to CheckUtils.

Efficiency
* Alter/add methods for testing with with optional check data/config.
* Use more efficient calls in several places (unfinished).

Consistency
* Log an error, if calling hasBypass off main thread unexpectedly.
* (Mostly irrelevant null checks.)

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

@@ -8,12 +8,14 @@ import org.bukkit.entity.Player;
 
 import fr.neatmonster.nocheatplus.NCPAPIProvider;
 import fr.neatmonster.nocheatplus.actions.ActionList;
+import fr.neatmonster.nocheatplus.checks.access.ICheckConfig;
+import fr.neatmonster.nocheatplus.checks.access.ICheckData;
 import fr.neatmonster.nocheatplus.compat.MCAccess;
 import fr.neatmonster.nocheatplus.components.MCAccessHolder;
-import fr.neatmonster.nocheatplus.hooks.NCPExemptionManager;
 import fr.neatmonster.nocheatplus.hooks.NCPHookManager;
 import fr.neatmonster.nocheatplus.players.DataManager;
 import fr.neatmonster.nocheatplus.players.ExecutionHistory;
+import fr.neatmonster.nocheatplus.utilities.CheckUtils;
 import fr.neatmonster.nocheatplus.utilities.TickTask;
 
 /**
@@ -113,6 +115,31 @@ public abstract class Check implements MCAccessHolder {
         return type;
     }
 
+    /**
+     * Checks both configuration flags and if the player is exempted from this
+     * check (hasBypass). Intended for higher efficiency with multiple calls.
+     * 
+     * @param player
+     * @param data
+     * @param cc
+     * @return
+     */
+    public boolean isEnabled(final Player player, final ICheckData data, final ICheckConfig cc) {
+        return cc.isEnabled(type) && !CheckUtils.hasBypass(type, player, data);
+    }
+
+    /**
+     * Checks both configuration flags and if the player is exempted from this
+     * check (hasBypass). Intended for higher efficiency with multiple calls.
+     * 
+     * @param player
+     * @param cc
+     * @return
+     */
+    public boolean isEnabled(final Player player, final ICheckConfig cc) {
+        return cc.isEnabled(type) && !CheckUtils.hasBypass(type, player, null);
+    }
+
     /**
      * Checks both configuration flags and if the player is exempted from this
      * check (hasBypass).
@@ -122,33 +149,18 @@ public abstract class Check implements MCAccessHolder {
      * @return true, if the check is enabled
      */
     public boolean isEnabled(final Player player) {
-        if (!type.isEnabled(player)) {
-            return false;
-        }
-        return !hasBypass(player);
+        return type.isEnabled(player) && !CheckUtils.hasBypass(type, player, null);
     }
 
     /**
-     * Check if the player is exempted by permissions or otherwise.
+     * Check if the player is exempted by permissions or otherwise.<br>
+     * 
      * 
      * @param player
      * @return
      */
     public boolean hasBypass(final Player player) {
-        // TODO: Checking for the thread might be a temporary measure.
-        if (Bukkit.isPrimaryThread()) {
-            // Check permissions directly.
-            final String permission = type.getPermission();
-            if (permission != null && player.hasPermission(permission)) {
-                return true;
-            }
-        }
-        else if (type.hasCachedPermission(player)) {
-            // Assume asynchronously running check.
-            return true;
-        }
-        // TODO: ExemptionManager relies on initial setup (problematic).
-        return NCPExemptionManager.isExempted(player, type);
+        return CheckUtils.hasBypass(type, player, null);
     }
 
     @Override

NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/CheckType.java

@@ -99,6 +99,7 @@ public enum CheckType {
     MOVING_SURVIVALFLY(MOVING, Permissions.MOVING_SURVIVALFLY),
 
     NET(new NetConfigCache(), new NetDataFactory(), Permissions.NET),
+    NET_ATTACKFREQUENCY(NET, Permissions.NET_ATTACKFREQUENCY),
     NET_FLYINGFREQUENCY(NET, Permissions.NET_FLYINGFREQUENCY),
     NET_KEEPALIVEFREQUENCY(NET, Permissions.NET_KEEPALIVEFREQUENCY),
     NET_SOUNDDISTANCE(NET), // Can not exempt players from this one.

NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/moving/MovingListener.java

@@ -244,13 +244,12 @@ public class MovingListener extends CheckListener implements TickListener, IRemo
             priority = EventPriority.MONITOR)
     public void onPlayerBedLeave(final PlayerBedLeaveEvent event) {
         final Player player = event.getPlayer();
-
         if (bedLeave.isEnabled(player) && bedLeave.checkBed(player)) {
+            final MovingConfig cc = MovingConfig.getConfig(player);
             // Check if the player has to be reset.
             // To "cancel" the event, we teleport the player.
             final Location loc = player.getLocation(useLoc);
             final MovingData data = MovingData.getData(player);
-            final MovingConfig cc = MovingConfig.getConfig(player); 
             Location target = null;
             final boolean sfCheck = MovingUtil.shouldCheckSurvivalFly(player, data, cc);
             if (sfCheck) {
@@ -260,7 +259,7 @@ public class MovingListener extends CheckListener implements TickListener, IRemo
                 // TODO: Add something to guess the best set back location (possibly data.guessSetBack(Location)).
                 target = LocUtil.clone(loc);
             }
-            if (sfCheck && cc.sfSetBackPolicyFallDamage && noFall.isEnabled(player)) {
+            if (sfCheck && cc.sfSetBackPolicyFallDamage && noFall.isEnabled(player, cc)) {
                 // Check if to deal damage.
                 double y = loc.getY();
                 if (data.hasSetBack()) {
@@ -1276,7 +1275,7 @@ public class MovingListener extends CheckListener implements TickListener, IRemo
             DebugUtil.outputDebugVehicleMove(player, vehicle, from, to, fake);
         }
 
-        if (morePacketsVehicle.isEnabled(player)) {
+        if (morePacketsVehicle.isEnabled(player, data, cc)) {
             // If the player is handled by the more packets vehicle check, execute it.
             newTo = morePacketsVehicle.check(player, from, to, data, cc);
         }
@@ -1314,7 +1313,7 @@ public class MovingListener extends CheckListener implements TickListener, IRemo
             return;
         }
         final MovingConfig cc = MovingConfig.getConfig(player);
-        if (event.isCancelled() || !MovingUtil.shouldCheckSurvivalFly(player, data, cc) || !noFall.isEnabled(player)) {
+        if (event.isCancelled() || !MovingUtil.shouldCheckSurvivalFly(player, data, cc) || !noFall.isEnabled(player, cc)) {
             data.clearNoFallData();
             return;
         }
@@ -1564,8 +1563,9 @@ public class MovingListener extends CheckListener implements TickListener, IRemo
         final Entity attacker = event.getAttacker();
         if (attacker instanceof Player && attacker.equals(event.getVehicle().getPassenger())) {
             final Player player = (Player) attacker;
-            if (survivalFly.isEnabled(player) || creativeFly.isEnabled(player)) {
-                if (MovingConfig.getConfig(player).vehiclePreventDestroyOwn) {
+            final MovingConfig cc = MovingConfig.getConfig(player);
+            if (survivalFly.isEnabled(player, cc) || creativeFly.isEnabled(player, cc)) {
+                if (cc.vehiclePreventDestroyOwn) {
                     event.setCancelled(true);
                     player.sendMessage(ChatColor.DARK_RED + "Destroying your own vehicle is disabled.");
                 }
@@ -1835,7 +1835,7 @@ public class MovingListener extends CheckListener implements TickListener, IRemo
 
     private void handleHoverViolation(final Player player, final Location loc, final MovingConfig cc, final MovingData data) {
         // Check nofall damage (!).
-        if (cc.sfHoverFallDamage && noFall.isEnabled(player)) {
+        if (cc.sfHoverFallDamage && noFall.isEnabled(player, cc)) {
             // Consider adding 3/3.5 to fall distance if fall distance > 0?
             noFall.checkDamage(player, data, loc.getY());
         }

NCPCore/src/main/java/fr/neatmonster/nocheatplus/hooks/APIUtils.java

@@ -116,9 +116,10 @@ public class APIUtils {
     }
 
     /**
-     * Return if the check type requires synchronization.
+     * Return if the check type requires synchronization. This indicates, if a
+     * check can be called off primary thread at all.
      * <hr>
-     * The should be chat checks, currently.
+     * That should be CHAT and NET checks, currently.
      * 
      * @param type
      *            the check type

NCPCore/src/main/java/fr/neatmonster/nocheatplus/utilities/CheckUtils.java

@@ -1,13 +1,21 @@
 package fr.neatmonster.nocheatplus.utilities;
 
+import org.bukkit.Bukkit;
 import org.bukkit.entity.Entity;
 import org.bukkit.entity.Player;
 
+import fr.neatmonster.nocheatplus.NCPAPIProvider;
+import fr.neatmonster.nocheatplus.checks.CheckType;
+import fr.neatmonster.nocheatplus.checks.access.ICheckConfig;
+import fr.neatmonster.nocheatplus.checks.access.ICheckData;
 import fr.neatmonster.nocheatplus.checks.blockbreak.BlockBreakData;
 import fr.neatmonster.nocheatplus.checks.combined.CombinedData;
 import fr.neatmonster.nocheatplus.checks.fight.FightData;
 import fr.neatmonster.nocheatplus.checks.inventory.InventoryData;
+import fr.neatmonster.nocheatplus.hooks.APIUtils;
+import fr.neatmonster.nocheatplus.hooks.NCPExemptionManager;
 import fr.neatmonster.nocheatplus.logging.StaticLog;
+import fr.neatmonster.nocheatplus.logging.Streams;
 
 /**
  * Random auxiliary gear, some might have general quality. Contents are likely to get moved to other classes.
@@ -117,4 +125,65 @@ public class CheckUtils {
         return false;
     }
 
+    /**
+     * Check for config flag and exemption (hasBypass). Meant thread-safe.
+     * 
+     * @param checkType
+     * @param player
+     * @param data
+     *            If data is null, the data factory will be used for the given
+     *            check type.
+     * @param cc
+     *            If config is null, the config factory will be used for the
+     *            given check type.
+     * @return
+     */
+    public static boolean isEnabled(final CheckType checkType, final Player player, final ICheckData data, final ICheckConfig cc) {
+        if (cc == null) {
+            if (!checkType.isEnabled(player)) {
+                return false;
+            }
+        }
+        else if (!cc.isEnabled(checkType)) {
+            return false;
+        }
+        return !hasBypass(checkType, player, data);
+    }
+
+    /**
+     * Check for exemption by permissions, API access, possibly other. Meant
+     * thread-safe.
+     * 
+     * @param player
+     * @param checkType
+     * @param data
+     *            If data is null, the data factory will be used for the given
+     *            check type.
+     * @return
+     */
+    public static boolean hasBypass(final CheckType checkType, final Player player, final ICheckData data) {
+        // TODO: Checking for the thread might be a temporary measure.
+        final String permission =  checkType.getPermission();
+        if (Bukkit.isPrimaryThread()) {
+            if (permission != null && player.hasPermission(permission)) {
+                return true;
+            }
+        } else if (permission != null) {
+            if (data == null) {
+                if (checkType.hasCachedPermission(player, permission)) {
+                    return true;
+                }
+            }
+            else if (data.hasCachedPermission(permission)) {
+                return true;
+            }
+            if (!APIUtils.needsSynchronization(checkType)) {
+                // Checking for exemption can cause harm now.
+                NCPAPIProvider.getNoCheatPlusAPI().getLogManager().severe(Streams.STATUS, "Off primary thread call to hasByPass for " + checkType + ".");
+            }
+        }
+        // TODO: ExemptionManager relies on the initial definition for which type can be checked off main thread.
+        return NCPExemptionManager.isExempted(player, checkType);
+    }
+
 }