Some in depth documentation

src/cc/co/evenprime/bukkit/nocheat/Colors.java

@@ -26,7 +26,7 @@ public class Colors {
     }
     
     /** 
-     * Remove instances of &X with a color
+     * Remove instances of &X
      * 
      * @param text
      * @return

src/cc/co/evenprime/bukkit/nocheat/EventManager.java

@@ -6,5 +6,11 @@ import cc.co.evenprime.bukkit.nocheat.config.ConfigurationCacheStore;
 
 public interface EventManager extends Listener {
 
+    /**
+     * Used for debug output, if checks are activated for the world-specific
+     * config that is given as a parameter
+     * @param cc The config
+     * @return A list of active/enabled checks
+     */
     public List<String> getActiveChecks(ConfigurationCacheStore cc);
 }

src/cc/co/evenprime/bukkit/nocheat/actions/Action.java

@@ -2,25 +2,28 @@ package cc.co.evenprime.bukkit.nocheat.actions;
 
 /**
  * An action gets executed as the result of a failed check. If it 'really' gets
- * executed depends on how many executions have occured within the last 60
+ * executed depends on how many executions have occurred within the last 60
  * seconds and how much time was between this and the previous execution
  * 
  */
 public abstract class Action {
 
     /**
-     * Delay in violations (only do if there were more than "delay" violations
+     * Delay in violations. An "ExecutionHistory" will use this info to make
-     * in last 60 seconds)
+     * sure that there were at least "delay" attempts to execute this action
+     * before it really gets executed.
      */
     public final int    delay;
 
     /**
-     * Repeat only every "repeat" seconds
+     * Repeat only every "repeat" seconds. An "ExecutionHistory" will use this 
+     * info to make sure that there were at least "repeat" seconds between the
+     * last execution of this action and this execution. 
      */
     public final int    repeat;
 
     /**
-     * The name of the action, to identify it in the config file
+     * The name of the action, to identify it, e.g. in the config file
      */
     public final String name;
 

src/cc/co/evenprime/bukkit/nocheat/actions/ParameterName.java

@@ -2,7 +2,6 @@ package cc.co.evenprime.bukkit.nocheat.actions;
 
 /**
  * Some wildcards that are used in commands and log messages
- *
  */
 public enum ParameterName {
     PLAYER("player"), LOCATION("location"), WORLD("world"),

src/cc/co/evenprime/bukkit/nocheat/actions/types/ActionList.java

@@ -8,28 +8,38 @@ import java.util.Map;
 import cc.co.evenprime.bukkit.nocheat.actions.Action;
 
 /**
- * A list of actions, that associates actions and a treshold. It allows to
+ * A list of actions, that associates actions to tresholds. It allows to
- * retrieve all actions that match a certain treshold.
+ * retrieve all actions that match a certain treshold
  * 
  */
 public class ActionList {
 
+    // This is a very bad design decision, but it's also really
+    // convenient to define this here
+    public final String permissionSilent;
+
     public ActionList(String permission) {
         this.permissionSilent = permission + ".silent";
     }
 
+    // If there are no actions registered, we still return an Array. It's
+    // just empty/size=0
     private final static Action[]        emptyArray = new Action[0];
 
+    // The actions of this ActionList, "bundled" by treshold (violation level)
     private final Map<Integer, Action[]> actions    = new HashMap<Integer, Action[]>();
+
+    // The tresholds of this list
     private final List<Integer>          tresholds  = new ArrayList<Integer>();
-    public final String                  permissionSilent;
 
     /**
      * Add an entry to this actionList. The list will be sorted by tresholds
      * automatically after the insertion.
      * 
-     * @param treshold
+     * @param treshold The minimum violation level a player needs to have
-     * @param actionNames
+     * to be suspected to the given actions
+     * @param actions The actions that will be used if the player reached the
+     * accompanying treshold/violation level
      */
     public void setActions(Integer treshold, Action[] actions) {
 
@@ -43,11 +53,11 @@ public class ActionList {
 
     /**
      * Get a list of actions that match the violation level.
-     * The only method that has to be called by a check, besides
+     * The only method that has to be called by a check
-     * a call to Action
      * 
-     * @param violationLevel
+     * @param violationLevel The violation level that should be matched.
-     * @return
+     * @return The array of actions whose treshold was closest to the 
+     * violationLevel but not bigger
      */
     public Action[] getActions(double violationLevel) {
 
@@ -65,6 +75,11 @@ public class ActionList {
             return emptyArray;
     }
 
+    /**
+     * Get a sorted list of the tresholds/violation levels that were used
+     * in this list
+     * @return The sorted list of tresholds
+     */
     public List<Integer> getTresholds() {
         return tresholds;
     }

src/cc/co/evenprime/bukkit/nocheat/actions/types/ActionWithParameters.java

@@ -6,6 +6,10 @@ import cc.co.evenprime.bukkit.nocheat.actions.Action;
 import cc.co.evenprime.bukkit.nocheat.actions.ParameterName;
 import cc.co.evenprime.bukkit.nocheat.checks.Check;
 
+/**
+ * Action with parameters is used to
+ *
+ */
 public abstract class ActionWithParameters extends Action {
 
     private final ArrayList<Object> messageParts;

src/cc/co/evenprime/bukkit/nocheat/actions/types/ConsolecommandAction.java

@@ -15,10 +15,23 @@ public class ConsolecommandAction extends ActionWithParameters {
         super(name, delay, repeat, command);
     }
 
+    /**
+     * Fill in the placeholders ( stuff that looks like '[something]') with
+     * information, make a nice String out of it that can be directly used
+     * as a command in the console.
+     * 
+     * @param player The player that is used to fill in missing data
+     * @param check The check that is used to fill in missing data
+     * @return The complete, ready to use, command
+     */
     public String getCommand(NoCheatPlayer player, Check check) {
         return super.getMessage(player, check);
     }
 
+    /**
+     * Convert the commands data into a string that can be used in the config
+     * files
+     */
     public String toString() {
         return "cmd:" + name + ":" + delay + ":" + repeat;
     }

src/cc/co/evenprime/bukkit/nocheat/actions/types/DummyAction.java

@@ -4,11 +4,12 @@ import cc.co.evenprime.bukkit.nocheat.actions.Action;
 
 /**
  * If an action can't be parsed correctly, at least keep it
- * stored in this form
+ * stored in this form to not lose it when loading/storing the config file
  *
  */
 public class DummyAction extends Action {
 
+    // The original string used for this action definition
     private final String def;
 
     public DummyAction(String def) {

src/cc/co/evenprime/bukkit/nocheat/actions/types/LogAction.java

@@ -9,34 +9,57 @@ import cc.co.evenprime.bukkit.nocheat.checks.Check;
  */
 public class LogAction extends ActionWithParameters {
 
+    // Some flags to decide where the log message should show up, based on
+    // the config file
     private final boolean toChat;
     private final boolean toConsole;
     private final boolean toFile;
 
     public LogAction(String name, int delay, int repeat, boolean toChat, boolean toConsole, boolean toFile, String message) {
-        // Log messages may have color codes now
         super(name, delay, repeat, message);
         this.toChat = toChat;
         this.toConsole = toConsole;
         this.toFile = toFile;
     }
 
+    /**
+     * Parse the final log message out of various data from the player and
+     * check that triggered the action.
+     * @param player The player that is used as a source for the log message
+     * @param check The check that is used as a source for the log message
+     * @return
+     */
     public String getLogMessage(NoCheatPlayer player, Check check) {
         return super.getMessage(player, check);
     }
 
+    /**
+     * Should the message be shown in chat?
+     * @return true, if yes
+     */
     public boolean toChat() {
         return toChat;
     }
 
+    /**
+     * Should the message be shown in the console?
+     * @return true, if yes
+     */
     public boolean toConsole() {
         return toConsole;
     }
 
+    /**
+     * Should the message be written to the logfile?
+     * @return true, if yes
+     */
     public boolean toFile() {
         return toFile;
     }
 
+    /**
+     * Create the string that's used to define the action in the logfile
+     */
     public String toString() {
         return "log:" + name + ":" + delay + ":" + repeat + ":" + (toConsole ? "c" : "") + (toChat ? "i" : "") + (toFile ? "f" : "");
     }

src/cc/co/evenprime/bukkit/nocheat/checks/blockbreak/BlockBreakCheck.java

@@ -1,10 +1,16 @@
 package cc.co.evenprime.bukkit.nocheat.checks.blockbreak;
 
 import cc.co.evenprime.bukkit.nocheat.NoCheat;
+import cc.co.evenprime.bukkit.nocheat.NoCheatPlayer;
 import cc.co.evenprime.bukkit.nocheat.checks.Check;
 import cc.co.evenprime.bukkit.nocheat.config.ConfigurationCacheStore;
 import cc.co.evenprime.bukkit.nocheat.data.DataStore;
 
+/**
+ * Abstract base class for BlockBreakChecks. Provides some static convenience
+ * methods for retrieving data and config objects for players
+ *
+ */
 public abstract class BlockBreakCheck extends Check {
 
     private static final String id = "blockbreak";
@@ -13,7 +19,15 @@ public abstract class BlockBreakCheck extends Check {
         super(plugin, id, name);
     }
 
-    public static BlockBreakData getData(DataStore base) {
+    /**
+     * Get the "BlockBreakData" object that belongs to the player. Will ensure
+     * that such a object exists and if not, create one
+     * 
+     * @param player
+     * @return
+     */
+    public static BlockBreakData getData(NoCheatPlayer player) {
+        DataStore base = player.getDataStore();
         BlockBreakData data = base.get(id);
         if(data == null) {
             data = new BlockBreakData();
@@ -22,6 +36,17 @@ public abstract class BlockBreakCheck extends Check {
         return data;
     }
 
+    /**
+     * Get the BlockBreakConfig object that belongs to the world that the player
+     * currently resides in.
+     * 
+     * @param player
+     * @return
+     */
+    public static BlockBreakConfig getConfig(NoCheatPlayer player) {
+        return getConfig(player.getConfigurationStore());
+    }
+
     public static BlockBreakConfig getConfig(ConfigurationCacheStore cache) {
         BlockBreakConfig config = cache.get(id);
         if(config == null) {

src/cc/co/evenprime/bukkit/nocheat/checks/blockbreak/BlockBreakCheckListener.java

@@ -36,6 +36,10 @@ public class BlockBreakCheckListener implements Listener, EventManager {
         this.plugin = plugin;
     }
 
+    /**
+     * We listen to blockBreak events for obvious reasons
+     * @param event The blockbreak event
+     */
     @EventHandler(priority = EventPriority.LOWEST)
     public void blockBreak(final BlockBreakEvent event) {
 
@@ -45,13 +49,13 @@ public class BlockBreakCheckListener implements Listener, EventManager {
         boolean cancelled = false;
 
         final NoCheatPlayer player = plugin.getPlayer(event.getPlayer());
-        final BlockBreakConfig cc = BlockBreakCheck.getConfig(player.getConfigurationStore());
+        final BlockBreakConfig cc = BlockBreakCheck.getConfig(player);
-
+        final BlockBreakData data = BlockBreakCheck.getData(player);
-        final BlockBreakData data = BlockBreakCheck.getData(player.getDataStore());
 
+        // Remember the location of the block that will be broken
         data.brokenBlockLocation.set(event.getBlock());
 
-        // Only if the block got damaged before, do the check(s)
+        // Only if the block got damaged directly before, do the check(s)
         if(!data.brokenBlockLocation.equals(data.lastDamagedBlock)) {
             // Something caused a blockbreak event that's not from the player
             // Don't check it at all
@@ -59,21 +63,37 @@ public class BlockBreakCheckListener implements Listener, EventManager {
             return;
         }
 
-        // Now do the actual checks, if still needed
+        // Now do the actual checks, if still needed. It's a good idea to make
+        // computationally cheap checks first, because it may save us from
+        // doing the computationally expensive checks.
+
+        // First NoSwing: Did the arm of the player move before breaking this
+        // block?
         if(cc.noswingCheck && !player.hasPermission(Permissions.BLOCKBREAK_NOSWING)) {
             cancelled = noswingCheck.check(player, data, cc);
         }
+
+        // Second Reach: Is the block really in reach distance
         if(!cancelled && cc.reachCheck && !player.hasPermission(Permissions.BLOCKBREAK_REACH)) {
             cancelled = reachCheck.check(player, data, cc);
         }
+
+        // Third Direction: Did the player look at the block at all
         if(!cancelled && cc.directionCheck && !player.hasPermission(Permissions.BLOCKBREAK_DIRECTION)) {
             cancelled = directionCheck.check(player, data, cc);
         }
 
+        // At least one check failed and demanded to cancel the event
         if(cancelled)
             event.setCancelled(cancelled);
     }
 
+    /**
+     * We listen to BlockDamage events to grab the information if it has been
+     * an "insta-break". That info may come in handy later.
+     * 
+     * @param event The BlockDamage event
+     */
     @EventHandler(priority = EventPriority.MONITOR)
     public void blockHit(final BlockDamageEvent event) {
 
@@ -81,17 +101,24 @@ public class BlockBreakCheckListener implements Listener, EventManager {
             return;
 
         NoCheatPlayer player = plugin.getPlayer(event.getPlayer());
-        BlockBreakData data = BlockBreakCheck.getData(player.getDataStore());
+        BlockBreakData data = BlockBreakCheck.getData(player);
 
         // Only interested in insta-break events here
         if(event.getInstaBreak()) {
-            // Remember this location. We ignore block breaks in the block-break
+            // Remember this location. We handle insta-breaks slightly
-            // direction check that are insta-breaks
+            // different in some of the blockbreak checks.
             data.instaBrokenBlockLocation.set(event.getBlock());
         }
 
     }
 
+    /**
+     * We listen to BlockInteract events to be (at least in many cases) able
+     * to distinguish between blockbreak events that were triggered by players
+     * actually digging and events that were artificially created by plugins.
+     * 
+     * @param event
+     */
     @EventHandler(priority = EventPriority.MONITOR)
     public void blockInteract(final PlayerInteractEvent event) {
 
@@ -99,15 +126,23 @@ public class BlockBreakCheckListener implements Listener, EventManager {
             return;
 
         NoCheatPlayer player = plugin.getPlayer(event.getPlayer());
-        BlockBreakData data = BlockBreakCheck.getData(player.getDataStore());
+        BlockBreakData data = BlockBreakCheck.getData(player);
-        // Remember this location. Only blockbreakevents for this specific block
+        // Remember this location. Only blockbreakevents for this specific 
-        // will be handled at all
+        // block will be handled at all
         data.lastDamagedBlock.set(event.getClickedBlock());
     }
 
+    /**
+     * We listen to PlayerAnimationEvent because it is (currently) equivalent
+     * to "player swings arm" and we want to check if he did that between
+     * blockbreaks.
+     * 
+     * @param event The PlayerAnimation Event
+     */
     @EventHandler(priority = EventPriority.MONITOR)
     public void armSwing(final PlayerAnimationEvent event) {
-        BlockBreakCheck.getData(plugin.getPlayer(event.getPlayer()).getDataStore()).armswung = true;
+        // Just set a flag to true when the arm was swung
+        BlockBreakCheck.getData(plugin.getPlayer(event.getPlayer())).armswung = true;
     }
 
     public List<String> getActiveChecks(ConfigurationCacheStore cc) {

src/cc/co/evenprime/bukkit/nocheat/checks/blockbreak/BlockBreakConfig.java

@@ -8,7 +8,8 @@ import cc.co.evenprime.bukkit.nocheat.config.Permissions;
 
 /**
  * Configurations specific for the "BlockBreak" checks
- * Every world gets one of these assigned to it.
+ * Every world gets one of these assigned to it, or if a world doesn't get
+ * it's own, it will use the "global" version
  * 
  */
 public class BlockBreakConfig implements ConfigItem {
@@ -16,10 +17,12 @@ public class BlockBreakConfig implements ConfigItem {
     public final boolean    reachCheck;
     public final double     reachDistance;
     public final ActionList reachActions;
+
     public final boolean    directionCheck;
     public final ActionList directionActions;
     public final double     directionPrecision;
     public final long       directionPenaltyTime;
+
     public final boolean    noswingCheck;
     public final ActionList noswingActions;
 

src/cc/co/evenprime/bukkit/nocheat/checks/blockbreak/BlockBreakData.java

@@ -4,20 +4,30 @@ import cc.co.evenprime.bukkit.nocheat.DataItem;
 import cc.co.evenprime.bukkit.nocheat.data.SimpleLocation;
 
 /**
- * Playerspecific data for the blockbreak check group
+ * Playerspecific data for the blockbreak checks
  * 
  */
 public class BlockBreakData implements DataItem {
 
-    public double                 reachVL                    = 0.0D;
+    // Keep track of violation levels for the three checks
-    public double                 directionVL                = 0.0D;
+    public double               reachVL                    = 0.0D;
-    public double                 noswingVL                  = 0.0D;
+    public double               directionVL                = 0.0D;
+    public double               noswingVL                  = 0.0D;
 
-    public long                   directionLastViolationTime = 0;
+    // Used for the penalty time feature of the direction check
-    public final SimpleLocation   instaBrokenBlockLocation   = new SimpleLocation();
+    public long                 directionLastViolationTime = 0;
-    public final SimpleLocation   brokenBlockLocation        = new SimpleLocation();
+
+    // Have a nicer/simpler way to work with block locations instead of
+    // Bukkits own "Location" class
+    public final SimpleLocation instaBrokenBlockLocation   = new SimpleLocation();
+    public final SimpleLocation brokenBlockLocation        = new SimpleLocation();
+    public final SimpleLocation lastDamagedBlock           = new SimpleLocation();
+
+    // indicate if the player swung his arm since he got checked last time
+    public boolean              armswung                   = true;
+
+    // For logging, remember the reachDistance that was calculated in the 
+    // reach check
+    public double               reachDistance;
 
-    public double                 reachDistance;
-    public boolean                armswung                   = true;
-    public final SimpleLocation   lastDamagedBlock           = new SimpleLocation();
 }

src/cc/co/evenprime/bukkit/nocheat/checks/blockbreak/DirectionCheck.java

@@ -22,52 +22,64 @@ public class DirectionCheck extends BlockBreakCheck {
     public boolean check(final NoCheatPlayer player, final BlockBreakData data, final BlockBreakConfig ccblockbreak) {
 
         final SimpleLocation brokenBlock = data.brokenBlockLocation;
-        final boolean isInstaBreak = data.instaBrokenBlockLocation.equals(brokenBlock);
         boolean cancel = false;
 
+        // How far "off" is the player with his aim. We calculate from the
+        // players eye location and view direction to the center of the target
+        // block. If the line of sight is more too far off, "off" will be
+        // bigger than 0
         double off = CheckUtil.directionCheck(player, brokenBlock.x + 0.5D, brokenBlock.y + 0.5D, brokenBlock.z + 0.5D, 1D, 1D, ccblockbreak.directionPrecision);
 
         final long time = System.currentTimeMillis();
 
         if(off < 0.1D) {
-            // Player did nothing wrong
+            // Player did likely nothing wrong
-            // reduce violation counter
+            // reduce violation counter to reward him
             data.directionVL *= 0.9D;
         } else {
             // Player failed the check
             // Increment violation counter
-            if(isInstaBreak) {
+            if(data.instaBrokenBlockLocation.equals(brokenBlock)) {
                 // Instabreak block failures are very common, so don't be as
                 // hard on people failing them
                 off /= 5;
             }
+
+            // Add to the overall violation level of the check and add to
+            // statistics
             data.directionVL += off;
             incrementStatistics(player, Id.BB_DIRECTION, off);
 
+            // Execute whatever actions are associated with this check and the
+            // violation level and find out if we should cancel the event
             cancel = executeActions(player, ccblockbreak.directionActions, data.directionVL);
 
             if(cancel) {
-                // Needed to calculate penalty times
+                // if we should cancel, remember the current time too
                 data.directionLastViolationTime = time;
             }
         }
 
         // If the player is still in penalty time, cancel the event anyway
         if(data.directionLastViolationTime + ccblockbreak.directionPenaltyTime > time) {
+            // A saveguard to avoid people getting stuck in penalty time
+            // indefinitely in case the system time of the server gets changed
             if(data.directionLastViolationTime > time) {
-                System.out.println("Nocheat noted that your time ran backwards for " + (data.directionLastViolationTime - time) + " ms");
                 data.directionLastViolationTime = 0;
             }
+
+            // He is in penalty time, therefore request cancelling of the event
             return true;
         }
 
         return cancel;
     }
 
+    @Override
     public String getParameter(ParameterName wildcard, NoCheatPlayer player) {
 
         if(wildcard == ParameterName.VIOLATIONS)
-            return String.format(Locale.US, "%d", (int) getData(player.getDataStore()).directionVL);
+            return String.format(Locale.US, "%d", (int) getData(player).directionVL);
         else
             return super.getParameter(wildcard, player);
     }

src/cc/co/evenprime/bukkit/nocheat/checks/blockbreak/NoswingCheck.java

@@ -6,6 +6,11 @@ import cc.co.evenprime.bukkit.nocheat.NoCheatPlayer;
 import cc.co.evenprime.bukkit.nocheat.actions.ParameterName;
 import cc.co.evenprime.bukkit.nocheat.data.Statistics.Id;
 
+/**
+ * We require that the player moves his arm between blockbreaks, this is
+ * what gets checked here.
+ *
+ */
 public class NoswingCheck extends BlockBreakCheck {
 
     public NoswingCheck(NoCheat plugin) {
@@ -16,24 +21,30 @@ public class NoswingCheck extends BlockBreakCheck {
 
         boolean cancel = false;
 
-        // did he swing his arm before?
+        // did he swing his arm before
         if(data.armswung) {
+            // "consume" the flag
             data.armswung = false;
+            // reward with lowering of the violation level
             data.noswingVL *= 0.90D;
         } else {
+            // he failed, increase vl and statistics
             data.noswingVL += 1;
             incrementStatistics(player, Id.BB_NOSWING, 1);
 
+            // Execute whatever actions are associated with this check and the
+            // violation level and find out if we should cancel the event
             cancel = executeActions(player, cc.noswingActions, data.noswingVL);
         }
 
         return cancel;
     }
 
+    @Override
     public String getParameter(ParameterName wildcard, NoCheatPlayer player) {
 
         if(wildcard == ParameterName.VIOLATIONS)
-            return String.format(Locale.US, "%d", (int) getData(player.getDataStore()).noswingVL);
+            return String.format(Locale.US, "%d", (int) getData(player).noswingVL);
         else
             return super.getParameter(wildcard, player);
     }

src/cc/co/evenprime/bukkit/nocheat/checks/blockbreak/ReachCheck.java

@@ -9,8 +9,8 @@ import cc.co.evenprime.bukkit.nocheat.data.SimpleLocation;
 import cc.co.evenprime.bukkit.nocheat.data.Statistics.Id;
 
 /**
- * The reach check will find out if a player interacts with something that's too
+ * The reach check will find out if a player interacts with something that's
- * far away
+ * too far away
  * 
  */
 public class ReachCheck extends BlockBreakCheck {
@@ -25,30 +25,38 @@ public class ReachCheck extends BlockBreakCheck {
 
         final SimpleLocation brokenBlock = data.brokenBlockLocation;
 
+        // Distance is calculated from eye location to center of targeted block
+        // If the player is further away from his target than allowed, the
+        // difference will be assigned to "distance"
         final double distance = CheckUtil.reachCheck(player, brokenBlock.x + 0.5D, brokenBlock.y + 0.5D, brokenBlock.z + 0.5D, player.isCreative() ? cc.reachDistance + 2 : cc.reachDistance);
 
-        if(distance > 0D) {
+        if(distance <= 0D) {
-            // Player failed the check
+            // Player passed the check, reward him
-
+            data.reachVL *= 0.9D;
-            // Increment violation counter
+        } else {
+            // He failed, increment violation level and statistics
             data.reachVL += distance;
             incrementStatistics(player, Id.BB_REACH, distance);
+
+            // Remember how much further than allowed he tried to reach for
+            // logging, if necessary
             data.reachDistance = distance;
 
+            // Execute whatever actions are associated with this check and the
+            // violation level and find out if we should cancel the event
             cancel = executeActions(player, cc.reachActions, data.reachVL);
-        } else {
-            data.reachVL *= 0.9D;
         }
 
         return cancel;
     }
 
+    @Override
     public String getParameter(ParameterName wildcard, NoCheatPlayer player) {
 
         if(wildcard == ParameterName.VIOLATIONS)
-            return String.format(Locale.US, "%d", (int) getData(player.getDataStore()).reachVL);
+            return String.format(Locale.US, "%d", (int) getData(player).reachVL);
         else if(wildcard == ParameterName.REACHDISTANCE)
-            return String.format(Locale.US, "%.2f", getData(player.getDataStore()).reachDistance);
+            return String.format(Locale.US, "%.2f", getData(player).reachDistance);
         else
             return super.getParameter(wildcard, player);
     }