More to vehicle internals.

Set-back handling:
* Make the VehicleSetBack a class in the moving package.
* For set-backs use the debug flag from the time of checking.

Debugging:
* Alter debug output for moves slightly (differing locations).
* Add debug output for vehicle moves with players involved.

Note on 1.6:
* Not even PlayerMove is fired for pigs and horses, so no tracking.

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

@@ -11,7 +11,7 @@ public class LocUtil {
 	 * @param loc
 	 * @return
 	 */
-	static final Location clone(final Location loc){
+	public static final Location clone(final Location loc){
 		return new Location(loc.getWorld(), loc.getX(), loc.getY(), loc.getZ(), loc.getYaw(), loc.getPitch());
 	}
 
@@ -22,7 +22,7 @@ public class LocUtil {
 	 * @param pitch
 	 * @return
 	 */
-	static final Location clone(final Location loc, final float yaw, final float pitch){
+	public static final Location clone(final Location loc, final float yaw, final float pitch){
 		return new Location(loc.getWorld(), loc.getX(), loc.getY(), loc.getZ(), yaw, pitch);
 	}
 
@@ -32,7 +32,7 @@ public class LocUtil {
 	 * @param ref
 	 * @return
 	 */
-	static final Location clone(final Location setBack, final Location ref) {
+	public static final Location clone(final Location setBack, final Location ref) {
 		if (setBack == null){
 			return clone(ref);
 		}
@@ -41,7 +41,7 @@ public class LocUtil {
 		}
 	}
 
-	static final Location clone(final Location setBack, final PlayerLocation ref) {
+	public static final Location clone(final Location setBack, final PlayerLocation ref) {
 		if (setBack == null) return ref.getLocation();
 		else{
 			return clone(setBack, ref.getYaw(), ref.getPitch());
@@ -53,7 +53,7 @@ public class LocUtil {
 	 * @param setBack
 	 * @param loc
 	 */
-	static final void set(final Location setBack, final Location loc) {
+	public static final void set(final Location setBack, final Location loc) {
 		setBack.setWorld(loc.getWorld());
 		setBack.setX(loc.getX());
 		setBack.setY(loc.getY());
@@ -62,7 +62,7 @@ public class LocUtil {
 		setBack.setPitch(loc.getPitch());
 	}
 
-	static final void set(final Location setBack, final PlayerLocation loc) {
+	public static final void set(final Location setBack, final PlayerLocation loc) {
 		setBack.setWorld(loc.getWorld());
 		setBack.setX(loc.getX());
 		setBack.setY(loc.getY());

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

@@ -71,7 +71,10 @@ public class MorePacketsVehicle extends Check {
         if (!data.hasMorePacketsVehicleSetBack()){
         	// TODO: Check if other set-back is appropriate or if to set on other events.
         	data.setMorePacketsVehicleSetBack(from);
-        	if (data.morePacketsVehicleTaskId != -1) Bukkit.getScheduler().cancelTask(data.morePacketsVehicleTaskId);
+        	if (data.morePacketsVehicleTaskId != -1) {
+        		// TODO: Set back outdated or not?
+        		Bukkit.getScheduler().cancelTask(data.morePacketsVehicleTaskId);
+        	}
         }
 
         // Take a packet from the buffer.

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

@@ -69,7 +69,6 @@ import fr.neatmonster.nocheatplus.utilities.BlockProperties;
 import fr.neatmonster.nocheatplus.utilities.CheckUtils;
 import fr.neatmonster.nocheatplus.utilities.PlayerLocation;
 import fr.neatmonster.nocheatplus.utilities.StringUtil;
-import fr.neatmonster.nocheatplus.utilities.TeleportUtil;
 import fr.neatmonster.nocheatplus.utilities.TickTask;
 import fr.neatmonster.nocheatplus.utilities.TrigUtil;
 import fr.neatmonster.nocheatplus.utilities.build.BuildParameters;
@@ -418,10 +417,9 @@ public class MovingListener extends CheckListener implements TickListener, IRemo
 		
 		// Ignore players in vehicles.
 		if (player.isInsideVehicle()){
-			// Workaround for pigs and other!
+			// Workaround for pigs and other (1.5.x and before)!
-			
+			// Note that with 1.6 not even PlayerMove fires for horses and pigs.
 			// (isInsideVehicle is the faster check without object creation, do re-check though, if it changes to only check for Vehicle instances.)
-			
 			final Entity vehicle = CheckUtils.getLastNonPlayerVehicle(player);
 			data.wasInVehicle = true;
 			data.sfHoverTicks = -1;
@@ -1099,6 +1097,11 @@ public class MovingListener extends CheckListener implements TickListener, IRemo
     		data.clearNoFallData();
         }
         
+        if (cc.debug) {
+        	// Log move.
+        	DebugUtil.outputDebugVehicleMove(player, vehicle, from, to, fake);
+        }
+        
         if (morePacketsVehicle.isEnabled(player)){
             // If the player is handled by the more packets vehicle check, execute it.
             newTo = morePacketsVehicle.check(player, from, to, data, cc);
@@ -1108,36 +1111,15 @@ public class MovingListener extends CheckListener implements TickListener, IRemo
             data.clearMorePacketsData();
         }
         
-        // Did one of the checks decide we need a new "to"-location?
+        // Schedule a set-back?
         if (newTo != null && data.morePacketsVehicleTaskId == -1){
             // Schedule a delayed task to teleport back the vehicle with the player.
         	// (Only schedule if not already scheduled.)
-        	data.morePacketsVehicleTaskId = Bukkit.getScheduler().scheduleSyncDelayedTask(plugin, new Runnable() {
+        	// TODO: Might log debug if skipping.
-                private Entity  vehicle;
+            // TODO: Problem: scheduling allows a lot of things to happen until the task is run. Thus control about some things might be necessary.
-                private Player player;
+        	// TODO: Reset on world changes or not?
-                private Location location;
+        	data.morePacketsVehicleTaskId = Bukkit.getScheduler().scheduleSyncDelayedTask(plugin, new VehicleSetBack(vehicle, player, newTo, cc.debug));
-
-                @Override
-                public void run() {
-                	data.morePacketsVehicleTaskId = -1;
-                    try{
-                		MovingData.getData(player).setTeleported(location);
-                    	TeleportUtil.teleport(vehicle, player, location, MovingConfig.getConfig(player).debug);
         }
-                    catch(Throwable t){
-                    	LogUtil.logSevere(t);
-                    }
-                }
-
-                public Runnable set(final Entity vehicle, final Player player, final Location location) {
-                    this.vehicle = vehicle;
-                    this.player = player;
-                    this.location = location;
-                    return this;
-                }
-            }.set(vehicle, player, newTo), 1L);
-        }
-        // TODO: Log this one too if debug set.
     }
     
     @EventHandler(priority = EventPriority.LOWEST, ignoreCancelled = false)

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

@@ -0,0 +1,41 @@
+package fr.neatmonster.nocheatplus.checks.moving;
+
+import org.bukkit.Location;
+import org.bukkit.entity.Entity;
+import org.bukkit.entity.Player;
+
+import fr.neatmonster.nocheatplus.logging.LogUtil;
+import fr.neatmonster.nocheatplus.utilities.TeleportUtil;
+
+/**
+ * Task for scheduling a vehicle set back. Resets the morePacketsVehicleTaskId in the MovingData for the player.
+ * @author mc_dev
+ *
+ */
+public class VehicleSetBack implements Runnable{
+    private final Entity  vehicle;
+    private final Player player;
+    private final Location location;
+    private final boolean debug;
+    
+	public VehicleSetBack(Entity vehicle, Player player, Location location, boolean debug) {
+		this.vehicle = vehicle;
+        this.player = player;
+        this.location = location;
+        this.debug = debug;
+	}
+
+    @Override
+    public void run() {
+    	final MovingData data = MovingData.getData(player);
+    	data.morePacketsVehicleTaskId = -1;
+        try{
+    		data.setTeleported(location);
+        	TeleportUtil.teleport(vehicle, player, location, debug);
+        }
+        catch(Throwable t){
+        	LogUtil.logSevere(t);
+        }
+    }
+    
+}

NCPCore/src/main/java/fr/neatmonster/nocheatplus/logging/DebugUtil.java

@@ -3,6 +3,7 @@ package fr.neatmonster.nocheatplus.logging;
 import java.util.Arrays;
 
 import org.bukkit.Location;
+import org.bukkit.entity.Entity;
 import org.bukkit.entity.Player;
 
 import fr.neatmonster.nocheatplus.compat.MCAccess;
@@ -18,19 +19,87 @@ import fr.neatmonster.nocheatplus.utilities.build.BuildParameters;
  */
 public class DebugUtil {
 	
+	public static boolean isSamePos(final double x1, final double y1, final double z1, final double x2, final double y2, final double z2){
+		return x1 == x2 && y1 == y2 && z1 == z2;
+	}
+	
+	public static boolean isSamePos(final Location loc1, final Location loc2){
+		return isSamePos(loc1.getX(), loc1.getY(), loc1.getZ(), loc2.getX(), loc2.getY(), loc2.getZ());
+	}
+	
+	public static void addLocation(final double x, final double y, final double z, final StringBuilder builder){
+		builder.append(x + ", " + y + ", " + z);
+	}
+	
+	public static void addLocation(final Location loc, final StringBuilder builder){
+		addLocation(loc.getX(), loc.getY(), loc.getZ(), builder);
+	}
+	
+	public static void addLocation(final PlayerLocation loc, final StringBuilder builder){
+		addLocation(loc.getX(), loc.getY(), loc.getZ(), builder);
+	}
+	
+	public static void addFormattedLocation(final double x, final double y, final double z, final StringBuilder builder){
+		builder.append(StringUtil.fdec3.format(x) + ", " + StringUtil.fdec3.format(y) + ", " + StringUtil.fdec3.format(z));
+	}
+	
+	public static void addFormattedLocation(final Location loc, final StringBuilder builder){
+		addFormattedLocation(loc.getX(), loc.getY(), loc.getZ(), builder);
+	}
+	
+	public static void addFormattedLocation(final PlayerLocation loc, final StringBuilder builder){
+		addFormattedLocation(loc.getX(), loc.getY(), loc.getZ(), builder);
+	}
+
+	
+	/**
+	 * With line break between from and to.
+	 * @param fromX
+	 * @param fromY
+	 * @param fromZ
+	 * @param toX
+	 * @param toY
+	 * @param toZ
+	 * @param builder
+	 */
+	public static void addMove(final double fromX, final double fromY, final double fromZ, final double toX, final double toY, final double toZ, final StringBuilder builder){
+		builder.append("from: ");
+		addLocation(fromX, fromY, fromZ, builder);
+		builder.append("\nto: ");
+		addLocation(toX, toY, toZ, builder);
+	}
+	
+	/**
+	 * No line breaks, max. 3 digits after comma.
+	 * @param fromX
+	 * @param fromY
+	 * @param fromZ
+	 * @param toX
+	 * @param toY
+	 * @param toZ
+	 * @param builder
+	 */
+	public static void addFormattedMove(final double fromX, final double fromY, final double fromZ, final double toX, final double toY, final double toZ, final StringBuilder builder){
+		addFormattedLocation(fromX, fromY, fromZ, builder);
+		builder.append(" -> ");
+		addFormattedLocation(toX, toY, toZ, builder);
+	}
+
 	/**
 	 * 3 decimal digits after comma (StringUtil.fdec3). No leading new line.
 	 * @param from
 	 * @param to
-	 * @param loc Reference location for from, usually Player.getLocation().
+	 * @param loc Reference location for from, usually Player.getLocation(). May be null.
 	 * @param builder
 	 * @return
 	 */
 	public static void addFormattedMove(final PlayerLocation from, final PlayerLocation to, final Location loc, final StringBuilder builder){
-		builder.append(StringUtil.fdec3.format(from.getX()) + (from.getX() == loc.getX() ? "" : ("(" + StringUtil.fdec3.format(loc.getX()) + ")")));
+		if (loc != null && !from.isSamePos(loc)){
-		builder.append(", " + StringUtil.fdec3.format(from.getY()) + (from.getY() == loc.getY() ? "" : ("(" + StringUtil.fdec3.format(loc.getY()) + ")")));
+			builder.append("(");
-		builder.append(", " + StringUtil.fdec3.format(from.getZ()) + (from.getZ() == loc.getZ() ? "" : ("(" + StringUtil.fdec3.format(loc.getZ()) + ")")));
+			addFormattedLocation(loc, builder);
-		builder.append(" -> " + StringUtil.fdec3.format(to.getX()) + ", " + StringUtil.fdec3.format(to.getY()) + ", " + StringUtil.fdec3.format(to.getZ()));
+			builder.append(") ");
+		}
+		addFormattedMove(from.getX(), from.getY(), from.getZ(), to.getX(), to.getY(), to.getZ(), builder);	
 	}
 	
 	/**
@@ -41,10 +110,44 @@ public class DebugUtil {
 	 * @param builder
 	 */
 	public static void addMove(final PlayerLocation from, final PlayerLocation to, final Location loc, final StringBuilder builder){
-		builder.append("from: " + from.getX() + (from.getX() == loc.getX() ? "" : ("(" + loc.getX() + ")")));
+		if (loc != null && !from.isSamePos(loc)){
-		builder.append(", " + from.getY() + (from.getY() == loc.getY() ? "" : ("(" + loc.getY() + ")")));
+			builder.append("Location: ");
-		builder.append(", " + from.getZ() + (from.getZ() == loc.getZ() ? "" : ("(" + loc.getZ() + ")")));
+			addLocation(loc, builder);
-		builder.append("\nto: " + to.getX() + ", " + to.getY() + ", " + to.getZ());
+			builder.append("\n");
+		}
+		addMove(from.getX(), from.getY(), from.getZ(), to.getX(), to.getY(), to.getZ(), builder);	
+	}
+	
+	/**
+	 * 3 decimal digits after comma (StringUtil.fdec3). No leading new line.
+	 * @param from
+	 * @param to
+	 * @param loc Reference location for from, usually Player.getLocation().
+	 * @param builder
+	 * @return
+	 */
+	public static void addFormattedMove(final Location from, final Location to, final Location loc, final StringBuilder builder){
+		if (loc != null && !isSamePos(from, loc)){
+			builder.append("(");
+			addFormattedLocation(loc, builder);
+			builder.append(") ");
+		}
+		addFormattedMove(from.getX(), from.getY(), from.getZ(), to.getX(), to.getY(), to.getZ(), builder);		}
+	
+	/**
+	 * Add exact coordinates, multiple lines. No leading new line.
+	 * @param from
+	 * @param to
+	 * @param loc Reference location for from, usually Player.getLocation().
+	 * @param builder
+	 */
+	public static void addMove(final Location from, final Location to, final Location loc, final StringBuilder builder){
+		if (loc != null && !isSamePos(from, loc)){
+			builder.append("Location: ");
+			addLocation(loc, builder);
+			builder.append("\n");
+		}
+		addMove(from.getX(), from.getY(), from.getZ(), to.getX(), to.getY(), to.getZ(), builder);
 	}
 
 	/**
@@ -111,4 +214,34 @@ public class DebugUtil {
 		builder.append(tag + " id=" + loc.getTypeId() + " data=" + loc.getData() + " shape=" + Arrays.toString(loc.getBlockCache().getBounds(loc.getBlockX(), loc.getBlockY(), loc.getBlockZ())));
 	}
 
+	public static void outputDebugVehicleMove(final Player player, final Entity vehicle, final Location from, final Location to, final boolean fake) {
+		// TODO: log move + player name, vehicle type, if vehicle matches the players vehicle / inconsistencies.
+		final StringBuilder builder = new StringBuilder(250);
+		final Location vLoc = vehicle.getLocation();
+		final Location loc = player.getLocation();
+		// TODO: Differentiate debug levels (needs setting up some policy + document in BuildParamteres)?
+		// TODO: Might use more appropriate methods (distinct "player location: ...")
+		final Entity actualVehicle = player.getVehicle();
+		final boolean wrongVehicle = actualVehicle == null || actualVehicle.getEntityId() != vehicle.getEntityId();
+		if (BuildParameters.debugLevel > 0) {
+			builder.append("\n-------------- VEHICLE MOVE " + (fake ? "(fake)" : "") + "--------------\n");
+			builder.append(player.getName() + " " + from.getWorld().getName() + ":\n");
+			addMove(from, to, null, builder);
+			builder.append("\n Vehicle: ");
+			addLocation(vLoc, builder);
+			builder.append("\n Player: ");
+			addLocation(loc, builder);
+		}
+		else {
+			builder.append(player.getName() + " " + from.getWorld().getName() + "veh." + (fake ? "(fake)" : "") + " ");
+			addFormattedMove(from, to, null, builder);
+			builder.append("\n Vehicle: ");
+			addFormattedLocation(vLoc, builder);
+			builder.append(" Player: ");
+			addFormattedLocation(loc, builder);
+		}
+		builder.append("\n Vehicle type: " + vehicle.getType() + (wrongVehicle ? (actualVehicle == null ? " (exited?)" : " actual: " + actualVehicle.getType()) : ""));
+		System.out.print(builder.toString());
+	}
+
 }

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

@@ -15,7 +15,7 @@ public class TeleportUtil {
 	 */
 	public static void teleport(final Entity vehicle, final Player player, final Location location, final boolean debug) {
 		// TODO: This handling could conflict with WorldGuard region flags.
-		// TODO: Account for nested passengers.
+		// TODO: Account for nested passengers and inconsistencies.
 		final Entity passenger = vehicle.getPassenger();
 		final boolean vehicleTeleported;
 		final boolean playerIsPassenger = player.equals(passenger);