From 521b4feb25bf6c3989439f9baf391b1b44a9a014 Mon Sep 17 00:00:00 2001 From: asofold Date: Thu, 15 May 2014 10:06:47 +0200 Subject: [PATCH] Re-check use of "useLoc" once more. Fixes: * On passable-violations useLoc might have been passed as newTo. Reduce potential for accidental future misuse: * Call useLoc.setWorld(null) as late as possible. * Do not reset world of MoveInfo.useLoc in MoveInfo.set. --- .../checks/blockplace/BlockPlaceListener.java | 3 +- .../nocheatplus/checks/fight/Angle.java | 9 ++++-- .../nocheatplus/checks/moving/MoveInfo.java | 22 ++++++++----- .../checks/moving/MovingListener.java | 31 ++++++++++++++----- .../nocheatplus/checks/moving/Passable.java | 5 +-- 5 files changed, 48 insertions(+), 22 deletions(-) diff --git a/NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/blockplace/BlockPlaceListener.java b/NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/blockplace/BlockPlaceListener.java index 5f51e8d1..c9ca254d 100644 --- a/NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/blockplace/BlockPlaceListener.java +++ b/NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/blockplace/BlockPlaceListener.java @@ -134,7 +134,6 @@ public class BlockPlaceListener extends CheckListener { if (!cancelled && reach.isEnabled(player) && reach.check(player, block, data, cc)) { cancelled = true; } - useLoc.setWorld(null); // Direction check. if (!cancelled && direction.isEnabled(player) && direction.check(player, block, blockAgainst, data, cc)) { @@ -150,6 +149,8 @@ public class BlockPlaceListener extends CheckListener { if (cancelled) { event.setCancelled(cancelled); } + // Cleanup + // Reminder(currently unused): useLoc.setWorld(null); } @EventHandler(priority = EventPriority.LOWEST, ignoreCancelled = true) diff --git a/NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/fight/Angle.java b/NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/fight/Angle.java index 28ab00c0..d2639368 100644 --- a/NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/fight/Angle.java +++ b/NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/fight/Angle.java @@ -42,11 +42,14 @@ public class Angle extends Check { boolean cancel = false; // Remove the old locations from the map. - for (final long time : new TreeMap(data.angleHits).navigableKeySet()) - if (System.currentTimeMillis() - time > 1000L) - data.angleHits.remove(time); + for (final long time : new TreeMap(data.angleHits).navigableKeySet()) { + if (System.currentTimeMillis() - time > 1000L) { + data.angleHits.remove(time); + } + } // Add the new location to the map. + // TODO: Alter method to store something less fat. data.angleHits.put(System.currentTimeMillis(), player.getLocation()); // This needs to be a copy at present. // Not enough data to calculate deltas. diff --git a/NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/moving/MoveInfo.java b/NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/moving/MoveInfo.java index 0590f525..3d4df336 100644 --- a/NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/moving/MoveInfo.java +++ b/NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/moving/MoveInfo.java @@ -13,11 +13,12 @@ import fr.neatmonster.nocheatplus.utilities.PlayerLocation; * */ public class MoveInfo { + + /** For temporary use. Might need cloning for passing to external API. Only use after calling MoveInfo.set! */ + public final Location useLoc = new Location(null, 0, 0, 0); public final BlockCache cache; public final PlayerLocation from; public final PlayerLocation to; - /** For temporary use. Might need cloning for passing to external API. */ - public final Location useLoc = new Location(null, 0, 0, 0); public MoveInfo(final MCAccess mcAccess){ cache = mcAccess.getBlockCache(null); @@ -26,26 +27,31 @@ public class MoveInfo { } /** - * Demands at least setting from. + * Initialize from, and if given to. Note that useLoc is left untouched (!). * @param player - * @param from - * @param to + * @param from Must not be null. + * @param to Can be null. * @param yOnGround */ public final void set(final Player player, final Location from, final Location to, final double yOnGround){ + this.cache.setAccess(from.getWorld()); this.from.set(from, player, yOnGround); - this.cache.setAccess(from.getWorld()); this.from.setBlockCache(cache); if (to != null){ this.to.set(to, player, yOnGround); this.to.setBlockCache(cache); } - useLoc.setWorld(null); // Just in case of repeated setting. + // Note: using set to reset to by passing null won't work. } + + /** + * Clear caches and remove World references and such. Also resets the world of useLoc. + */ public final void cleanup(){ + useLoc.setWorld(null); from.cleanup(); to.cleanup(); cache.cleanup(); - useLoc.setWorld(null); } + } diff --git a/NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/moving/MovingListener.java b/NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/moving/MovingListener.java index ca6238e6..4fc90f37 100644 --- a/NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/moving/MovingListener.java +++ b/NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/moving/MovingListener.java @@ -281,13 +281,14 @@ 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); } - useLoc.setWorld(null); if (sfCheck && cc.sfFallDamage && noFall.isEnabled(player)) { // Check if to deal damage. double y = loc.getY(); if (data.hasSetBack()) y = Math.min(y, data.getSetBackY()); noFall.checkDamage(player, data, y); } + // Cleanup + useLoc.setWorld(null); // Teleport. data.prepareSetBack(target); // Should be enough. | new Location(target.getWorld(), target.getX(), target.getY(), target.getZ(), target.getYaw(), target.getPitch()); player.teleport(target, TeleportCause.PLUGIN);// TODO: schedule / other measures ? @@ -418,8 +419,12 @@ public class MovingListener extends CheckListener implements TickListener, IRemo // TODO: Order this to above "early return"? // Set up data / caching. final MoveInfo moveInfo; - if (parkedInfo.isEmpty()) moveInfo = new MoveInfo(mcAccess); - else moveInfo = parkedInfo.remove(parkedInfo.size() - 1); + if (parkedInfo.isEmpty()) { + moveInfo = new MoveInfo(mcAccess); + } + else { + moveInfo = parkedInfo.remove(parkedInfo.size() - 1); + } final MovingConfig cc = MovingConfig.getConfig(player); moveInfo.set(player, from, to, cc.yOnGround); // TODO: Data resetting above ? @@ -1123,8 +1128,12 @@ public class MovingListener extends CheckListener implements TickListener, IRemo boolean allowReset = true; if (!data.noFallSkipAirCheck) { final MoveInfo moveInfo; - if (parkedInfo.isEmpty()) moveInfo = new MoveInfo(mcAccess); - else moveInfo = parkedInfo.remove(parkedInfo.size() - 1); + if (parkedInfo.isEmpty()) { + moveInfo = new MoveInfo(mcAccess); + } + else { + moveInfo = parkedInfo.remove(parkedInfo.size() - 1); + } moveInfo.set(player, loc, null, cc.noFallyOnGround); // NOTE: No isIllegal check here. final PlayerLocation pLoc = moveInfo.from; @@ -1150,7 +1159,6 @@ public class MovingListener extends CheckListener implements TickListener, IRemo moveInfo.cleanup(); parkedInfo.add(moveInfo); } - useLoc.setWorld(null); final float fallDistance = player.getFallDistance(); final double damage = BridgeHealth.getDamage(event); final float yDiff = (float) (data.noFallMaxY - loc.getY()); @@ -1183,6 +1191,8 @@ public class MovingListener extends CheckListener implements TickListener, IRemo } } // Entity fall-distance should be reset elsewhere. + // Cleanup. + useLoc.setWorld(null); } /** @@ -1468,8 +1478,12 @@ public class MovingListener extends CheckListener implements TickListener, IRemo return; } final MoveInfo info; - if (parkedInfo.isEmpty()) info = new MoveInfo(mcAccess); - else info = parkedInfo.remove(parkedInfo.size() - 1); + if (parkedInfo.isEmpty()) { + info = new MoveInfo(mcAccess); + } + else { + info = parkedInfo.remove(parkedInfo.size() - 1); + } for (final String playerName : hoverTicks) { // TODO: put players into the set (+- one tick would not matter ?) // TODO: might add an online flag to data ! @@ -1546,6 +1560,7 @@ public class MovingListener extends CheckListener implements TickListener, IRemo // Check if player is on ground. final Location loc = player.getLocation(useLoc); // useLoc.setWorld(null) is done in onTick. info.set(player, loc, null, cc.yOnGround); + // (Could use useLoc of MoveInfo here. Note orderm though.) final boolean res; // TODO: Collect flags, more margin ? final int loaded = info.from.ensureChunksLoaded(); diff --git a/NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/moving/Passable.java b/NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/moving/Passable.java index 9e2bb85d..ab15d757 100644 --- a/NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/moving/Passable.java +++ b/NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/moving/Passable.java @@ -136,7 +136,7 @@ public class Passable extends Check { System.out.println(player.getName() + " Using set-back location for passable."); } } else if (cc.debug) { - System.out.println(player.getName() + " Ignorng set-back for passable."); + System.out.println(player.getName() + " Ignoring set-back for passable."); } } @@ -155,7 +155,8 @@ public class Passable extends Check { // TODO: Consider another set back position for this, also keeping track of players moving around in blocks. final Location newTo; if (loc != null) { - newTo = loc; + // Ensure the given location is cloned. + newTo = LocUtil.clone(loc); } else { newTo = from.getLocation(); if (cc.debug) {