Prevent a disabled "runfly" check from crippling the "morepackets"

check, handle teleportations better, prevent exploiting
the relation between "runfly" and "morepackets" check

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

@@ -9,7 +9,5 @@ import java.util.Map;
  */
 public interface DataItem {
 
-    public void clearCriticalData();
-
     public abstract void collectData(Map<String, Object> map);
 }

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

@@ -126,10 +126,6 @@ public class NoCheat extends JavaPlugin implements Listener {
             return conf.getConfigurationCacheForWorld(null);
     }
 
-    public void clearCriticalData(String playerName) {
-        players.clearCriticalData(playerName);
-    }
-
     @Override
     public boolean onCommand(CommandSender sender, Command command, String label, String[] args) {
         boolean result = CommandHandler.handleCommand(this, sender, command, label, args);
@@ -159,7 +155,6 @@ public class NoCheat extends JavaPlugin implements Listener {
         conf.cleanup();
         this.conf = new ConfigurationManager(this, this.getDataFolder());
         players.cleanDataMap();
-        players.clearCriticalData();
     }
 
     /**

src/cc/co/evenprime/bukkit/nocheat/checks/WorkaroundsListener.java

@@ -2,15 +2,10 @@ package cc.co.evenprime.bukkit.nocheat.checks;
 
 import java.util.Collections;
 import java.util.List;
-import org.bukkit.Location;
-import org.bukkit.entity.Player;
 import org.bukkit.event.EventHandler;
 import org.bukkit.event.EventPriority;
 import org.bukkit.event.Listener;
 import org.bukkit.event.player.PlayerMoveEvent;
-import org.bukkit.event.player.PlayerPortalEvent;
-import org.bukkit.event.player.PlayerRespawnEvent;
-import org.bukkit.event.player.PlayerTeleportEvent;
 import org.bukkit.event.player.PlayerToggleSprintEvent;
 import cc.co.evenprime.bukkit.nocheat.EventManager;
 import cc.co.evenprime.bukkit.nocheat.NoCheat;
@@ -30,33 +25,12 @@ public class WorkaroundsListener implements Listener, EventManager {
         this.plugin = plugin;
     }
 
-    @EventHandler(priority = EventPriority.MONITOR)
-    public void teleport(final PlayerTeleportEvent event) {
-        if(event.isCancelled())
-            return;
-        handleTeleportation(event.getPlayer(), event.getTo());
-    }
-
-    @EventHandler(priority = EventPriority.MONITOR)
-    public void portal(final PlayerPortalEvent event) {
-        if(event.isCancelled())
-            return;
-        handleTeleportation(event.getPlayer(), event.getTo());
-    }
-
-    @EventHandler(priority = EventPriority.MONITOR)
-    public void respawn(final PlayerRespawnEvent event) {
-        handleTeleportation(event.getPlayer(), event.getRespawnLocation());
-    }
-
     @EventHandler(priority = EventPriority.HIGHEST)
     public void playerMove(final PlayerMoveEvent event) {
         // No typo here. I really only handle cancelled events and ignore others
         if(!event.isCancelled())
             return;
 
-        handleTeleportation(event.getPlayer(), event.getTo());
-
         // Fix a common mistake that other developers make (cancelling move
         // events is crazy, rather set the target location to the from location)
         event.setCancelled(false);
@@ -70,10 +44,6 @@ public class WorkaroundsListener implements Listener, EventManager {
         }
     }
 
-    private void handleTeleportation(final Player player, final Location to) {
-        plugin.clearCriticalData(player.getName());
-    }
-
     @Override
     public List<String> getActiveChecks(ConfigurationCacheStore cc) {
         return Collections.emptyList();

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

@@ -28,14 +28,6 @@ public class BlockBreakData implements DataItem {
     public boolean                armswung                   = true;
     public final SimpleLocation   lastDamagedBlock           = new SimpleLocation();
 
-    @Override
-    public void clearCriticalData() {
-        instaBrokenBlockLocation.reset();
-        brokenBlockLocation.reset();
-        directionLastViolationTime = 0;
-        armswung = true;
-    }
-
     @Override
     public void collectData(Map<String, Object> map) {
         map.put("blockbreak.reach.vl", (int) reachTotalVL);

src/cc/co/evenprime/bukkit/nocheat/checks/chat/ChatData.java

@@ -27,9 +27,4 @@ public class ChatData implements DataItem {
         map.put("chat.spam.failed", spamFailed);
         map.put("chat.color.failed", colorFailed);
     }
-
-    @Override
-    public void clearCriticalData() {
-
-    }
 }

src/cc/co/evenprime/bukkit/nocheat/checks/fight/FightData.java

@@ -38,9 +38,4 @@ public class FightData implements DataItem {
         map.put("fight.noswing.failed", noswingFailed);
         map.put("fight.reach.failed", (int) reachFailed);
     }
-
-    @Override
-    public void clearCriticalData() {
-
-    }
 }

src/cc/co/evenprime/bukkit/nocheat/checks/inventory/InventoryData.java

@@ -16,9 +16,4 @@ public class InventoryData implements DataItem {
         map.put("inventory.drop.vl", (int) dropTotalVL);
         map.put("inventory.drop.failed", (int) dropFailed);
     }
-
-    @Override
-    public void clearCriticalData() {
-
-    }
 }

src/cc/co/evenprime/bukkit/nocheat/checks/moving/MovingCheckListener.java

@@ -10,6 +10,8 @@ import org.bukkit.event.EventPriority;
 import org.bukkit.event.Listener;
 import org.bukkit.event.block.BlockPlaceEvent;
 import org.bukkit.event.player.PlayerMoveEvent;
+import org.bukkit.event.player.PlayerPortalEvent;
+import org.bukkit.event.player.PlayerRespawnEvent;
 import org.bukkit.event.player.PlayerTeleportEvent;
 import org.bukkit.event.player.PlayerVelocityEvent;
 import org.bukkit.util.Vector;
@@ -77,7 +79,7 @@ public class MovingCheckListener implements Listener, EventManager {
     }
 
     @EventHandler(priority = EventPriority.HIGHEST)
-    protected void teleport(final PlayerTeleportEvent event) {
+    public void teleport(final PlayerTeleportEvent event) {
 
         // No typo here, I really want to only handle cancelled events
         if(!event.isCancelled())
@@ -86,15 +88,37 @@ public class MovingCheckListener implements Listener, EventManager {
         NoCheatPlayer player = plugin.getPlayer(event.getPlayer());
         final MovingData data = MovingCheck.getData(player.getDataStore());
 
+        // If it was a teleport initialized by NoCheat, do it anyway
         if(data.teleportTo.isSet() && data.teleportTo.equals(event.getTo())) {
             event.setCancelled(false);
+        } else {
+            // Only if it wasn't NoCheat, drop data from morepackets check
+            data.clearMorePacketsData();
         }
-        return;
 
+        // Always forget runfly specific data
+        data.teleportTo.reset();
+        data.clearRunFlyData();
+
+        return;
+    }
+
+    @EventHandler(priority = EventPriority.MONITOR)
+    public void portal(final PlayerPortalEvent event) {
+        final MovingData data = MovingCheck.getData(plugin.getPlayer(event.getPlayer()).getDataStore());
+        data.clearMorePacketsData();
+        data.clearRunFlyData();
+    }
+
+    @EventHandler(priority = EventPriority.MONITOR)
+    public void portal(final PlayerRespawnEvent event) {
+        final MovingData data = MovingCheck.getData(plugin.getPlayer(event.getPlayer()).getDataStore());
+        data.clearMorePacketsData();
+        data.clearRunFlyData();
     }
 
     @EventHandler(priority = EventPriority.LOWEST)
-    protected void move(final PlayerMoveEvent event) {
+    public void move(final PlayerMoveEvent event) {
 
         if(event.isCancelled())
             return;
@@ -107,7 +131,6 @@ public class MovingCheckListener implements Listener, EventManager {
         }
 
         final MovingConfig cc = MovingCheck.getConfig(player.getConfigurationStore());
-
         final MovingData data = MovingCheck.getData(player.getDataStore());
 
         // Various calculations related to velocity estimates
@@ -116,7 +139,8 @@ public class MovingCheckListener implements Listener, EventManager {
         if(!cc.check || player.hasPermission(Permissions.MOVING)) {
             // Just because he is allowed now, doesn't mean he will always
             // be. So forget data about the player related to moving
-            player.getDataStore().get("moving").clearCriticalData();
+            data.clearRunFlyData();
+            data.clearMorePacketsData();
             return;
         }
 
@@ -171,7 +195,7 @@ public class MovingCheckListener implements Listener, EventManager {
     }
 
     @EventHandler(priority = EventPriority.MONITOR)
-    protected void velocity(final PlayerVelocityEvent event) {
+    public void velocity(final PlayerVelocityEvent event) {
         if(event.isCancelled())
             return;
         final MovingData data = MovingCheck.getData(plugin.getPlayer(event.getPlayer()).getDataStore());

src/cc/co/evenprime/bukkit/nocheat/checks/moving/MovingData.java

@@ -48,7 +48,7 @@ public class MovingData implements DataItem {
     public int                   bunnyhopdelay;
 
     public int                   morePacketsCounter;
-    public int                   morePacketsBuffer       = 50;
+    public int                   morePacketsBuffer       = 30;
     public int                   packets;
 
     public final PreciseLocation morePacketsSetbackPoint = new PreciseLocation();
@@ -67,16 +67,17 @@ public class MovingData implements DataItem {
 
     public int                   onIce                   = 0;
 
-    @Override
+    public void clearRunFlyData() {
-    public void clearCriticalData() {
-        teleportTo.reset();
-        jumpPhase = 0;
         runflySetBackPoint.reset();
+        jumpPhase = 0;
         fallDistance = 0;
         lastAddedFallDistance = 0;
         bunnyhopdelay = 0;
-        morePacketsBuffer = 50;
+    }
+
+    public void clearMorePacketsData() {
         morePacketsSetbackPoint.reset();
+        morePacketsBuffer = 30;
         lastElapsedIngameSeconds = 0;
         morePacketsCounter = 0;
     }

src/cc/co/evenprime/bukkit/nocheat/checks/moving/RunflyCheck.java

@@ -26,7 +26,7 @@ public class RunflyCheck extends MovingCheck {
         if(player.hasPermission(Permissions.MOVING_RUNFLY)) {
             // If the player doesn't get checked for movement
             // reset his critical data
-            data.clearCriticalData();
+            data.clearRunFlyData();
             return null;
         }
         final boolean flyAllowed = cc.allowFlying || player.hasPermission(Permissions.MOVING_FLYING) || (player.isCreative() && cc.identifyCreativeMode);

src/cc/co/evenprime/bukkit/nocheat/data/DataStore.java

@@ -2,7 +2,6 @@ package cc.co.evenprime.bukkit.nocheat.data;
 
 import java.util.HashMap;
 import java.util.Map;
-
 import cc.co.evenprime.bukkit.nocheat.DataItem;
 
 public class DataStore {
@@ -20,12 +19,6 @@ public class DataStore {
         dataMap.put(id, data);
     }
 
-    public void clearCriticalData() {
-        for(DataItem data : dataMap.values()) {
-            data.clearCriticalData();
-        }
-    }
-
     public void collectData(Map<String, Object> map) {
         for(DataItem data : dataMap.values()) {
             data.collectData(map);

src/cc/co/evenprime/bukkit/nocheat/data/PlayerManager.java

@@ -5,9 +5,7 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
-
 import org.bukkit.entity.Player;
-
 import cc.co.evenprime.bukkit.nocheat.NoCheat;
 import cc.co.evenprime.bukkit.nocheat.NoCheatPlayer;
 import cc.co.evenprime.bukkit.nocheat.player.NoCheatPlayerImpl;
@@ -46,23 +44,6 @@ public class PlayerManager {
         return p;
     }
 
-    /**
-     * Reset data that may cause problems after e.g. changing the config
-     * 
-     */
-    public void clearCriticalData() {
-        for(NoCheatPlayer b : this.players.values()) {
-            b.getDataStore().clearCriticalData();
-        }
-    }
-
-    public void clearCriticalData(String playerName) {
-        NoCheatPlayer p = this.players.get(playerName.toLowerCase());
-        if(p != null) {
-            p.getDataStore().clearCriticalData();
-        }
-    }
-
     public void cleanDataMap() {
         long time = System.currentTimeMillis();
         List<String> removals = new ArrayList<String>(5);