Remove pointless abstraction in WaypointGoal, made it a bit faster when only one waypoint

src/main/java/net/citizensnpcs/npc/ai/CitizensNavigator.java

@@ -4,6 +4,7 @@ import net.citizensnpcs.Settings.Setting;
 import net.citizensnpcs.api.ai.EntityTarget;
 import net.citizensnpcs.api.ai.Navigator;
 import net.citizensnpcs.api.ai.TargetType;
+import net.citizensnpcs.api.ai.event.CancelReason;
 import net.citizensnpcs.api.ai.event.NavigationBeginEvent;
 import net.citizensnpcs.api.ai.event.NavigationCancelEvent;
 import net.citizensnpcs.api.ai.event.NavigationCompleteEvent;
@@ -29,7 +30,7 @@ public class CitizensNavigator implements Navigator {
     @Override
     public void cancelNavigation() {
         if (executing != null) {
-            Bukkit.getPluginManager().callEvent(new NavigationCancelEvent(this));
+            Bukkit.getPluginManager().callEvent(new NavigationCancelEvent(this, CancelReason.PLUGIN));
         }
         executing = null;
     }

src/main/java/net/citizensnpcs/npc/entity/EntityHumanNPC.java

@@ -103,11 +103,11 @@ public class EntityHumanNPC extends EntityPlayer implements NPCHolder {
 
         // taken from EntityLiving
         if (bu) {
-            boolean inLiquid = G() || H();
+            boolean inLiquid = H() || J();
             if (inLiquid) {
                 motY += 0.04;
             } else if (onGround && bE == 0) {
-                // this.ac(); - this doesn't jump high enough
+                // this.aZ(); - this doesn't jump high enough
                 motY = 0.6;
                 bE = 10;
             }
@@ -116,8 +116,8 @@ public class EntityHumanNPC extends EntityPlayer implements NPCHolder {
         }
 
         bs *= 0.98F;
+        aG *= bs();
         e(br, bs);
         as = yaw;
     }
-
 }

src/main/java/net/citizensnpcs/trait/waypoint/LinearWaypointProvider.java

@@ -1,10 +1,16 @@
 package net.citizensnpcs.trait.waypoint;
 
+import java.util.Iterator;
 import java.util.List;
 
 import javax.annotation.Nullable;
 
+import net.citizensnpcs.Settings.Setting;
 import net.citizensnpcs.api.CitizensAPI;
+import net.citizensnpcs.api.ai.Goal;
+import net.citizensnpcs.api.ai.GoalSelector;
+import net.citizensnpcs.api.ai.Navigator;
+import net.citizensnpcs.api.ai.event.NavigationCompleteEvent;
 import net.citizensnpcs.api.event.NPCDespawnEvent;
 import net.citizensnpcs.api.npc.NPC;
 import net.citizensnpcs.api.util.DataKey;
@@ -22,11 +28,11 @@ import org.bukkit.event.player.PlayerInteractEvent;
 import org.bukkit.event.player.PlayerItemHeldEvent;
 
 import com.google.common.base.Function;
-import com.google.common.collect.Iterables;
+import com.google.common.collect.Iterators;
 import com.google.common.collect.Lists;
 
 public class LinearWaypointProvider implements WaypointProvider {
-    private WaypointGoal currentGoal;
+    private LinearWaypointGoal currentGoal;
     private NPC npc;
     private final List<Waypoint> waypoints = Lists.newArrayList();
 
@@ -51,6 +57,15 @@ public class LinearWaypointProvider implements WaypointProvider {
                         location.getBlockY(), location.getBlockZ());
             }
 
+            private Location getPreviousWaypoint(int fromSlot) {
+                if (waypoints.size() <= 1)
+                    return null;
+                fromSlot--;
+                if (fromSlot < 0)
+                    fromSlot = waypoints.size() - 1;
+                return waypoints.get(fromSlot).getLocation();
+            }
+
             @EventHandler
             public void onNPCDespawn(NPCDespawnEvent event) {
                 if (event.getNPC().equals(npc))
@@ -61,11 +76,28 @@ public class LinearWaypointProvider implements WaypointProvider {
             public void onPlayerInteract(PlayerInteractEvent event) {
                 if (!event.getPlayer().equals(player) || event.getAction() == Action.PHYSICAL)
                     return;
+                if (event.getPlayer().getWorld() != npc.getBukkitEntity().getWorld())
+                    return;
                 if (event.getAction() == Action.LEFT_CLICK_BLOCK
                         || event.getAction() == Action.LEFT_CLICK_AIR) {
                     if (event.getClickedBlock() == null)
                         return;
                     Location at = event.getClickedBlock().getLocation();
+                    Location prev = getPreviousWaypoint(editingSlot);
+
+                    if (prev != null) {
+                        double distance = at.distanceSquared(prev);
+                        double maxDistance = Setting.DEFAULT_PATHFINDING_RANGE.asDouble();
+                        maxDistance = Math.pow(maxDistance, 2);
+                        if (distance > maxDistance) {
+                            Messaging.sendF(player, ChatColor.RED
+                                    + "Previous waypoint is %d blocks away but the distance limit is %d.",
+                                    StringHelper.wrap(distance, ChatColor.RED),
+                                    StringHelper.wrap(maxDistance, ChatColor.RED));
+                            return;
+                        }
+                    }
+
                     waypoints.add(Math.max(0, editingSlot), new Waypoint(at));
                     editingSlot = Math.min(editingSlot + 1, waypoints.size());
                     Messaging.send(
@@ -130,8 +162,7 @@ public class LinearWaypointProvider implements WaypointProvider {
     public void onSpawn(NPC npc) {
         this.npc = npc;
         if (currentGoal == null) {
-            Iterable<Location> provider = Iterables.transform(waypoints, WAYPOINT_TRANSFORMER);
+            currentGoal = new LinearWaypointGoal();
-            currentGoal = new WaypointGoal(provider, npc.getNavigator());
             CitizensAPI.registerEvents(currentGoal);
         }
         npc.getDefaultGoalController().addGoal(currentGoal, 1);
@@ -158,6 +189,77 @@ public class LinearWaypointProvider implements WaypointProvider {
         currentGoal.setPaused(paused);
     }
 
+    private class LinearWaypointGoal implements Goal {
+        private Location currentDestination;
+        private Iterator<Location> itr;
+        private boolean paused;
+        private GoalSelector selector;
+
+        private void ensureItr() {
+            if (itr == null || !itr.hasNext())
+                itr = Iterators.transform(waypoints.iterator(), WAYPOINT_TRANSFORMER);
+        }
+
+        private Navigator getNavigator() {
+            return npc.getNavigator();
+        }
+
+        public boolean isPaused() {
+            return paused;
+        }
+
+        @EventHandler
+        public void onNavigationComplete(NavigationCompleteEvent event) {
+            if (currentDestination == null || !event.getNavigator().equals(getNavigator()))
+                return;
+            if (currentDestination.equals(event.getNavigator().getTargetAsLocation()))
+                selector.finish();
+        }
+
+        public void onProviderChanged() {
+            itr = Iterators.transform(waypoints.iterator(), WAYPOINT_TRANSFORMER);
+            if (currentDestination != null)
+                selector.finish();
+        }
+
+        @Override
+        public void reset() {
+            currentDestination = null;
+            selector = null;
+        }
+
+        @Override
+        public void run() {
+        }
+
+        public void setPaused(boolean paused) {
+            if (paused && currentDestination != null)
+                selector.finish();
+            this.paused = paused;
+        }
+
+        @Override
+        public boolean shouldExecute(GoalSelector selector) {
+            if (paused || currentDestination != null || waypoints.size() == 0)
+                return false;
+            if (waypoints.size() == 1) {
+                // avoid repeatedly pathing to the same point and wasting
+                // memory.
+                Location dest = npc.getBukkitEntity().getLocation();
+                if (waypoints.get(0).getLocation().distanceSquared(dest) < 1)
+                    return false;
+            }
+            ensureItr();
+            boolean shouldExecute = itr.hasNext();
+            if (shouldExecute) {
+                this.selector = selector;
+                currentDestination = itr.next();
+                getNavigator().setTarget(currentDestination);
+            }
+            return shouldExecute;
+        }
+    }
+
     private static final Function<Waypoint, Location> WAYPOINT_TRANSFORMER = new Function<Waypoint, Location>() {
         @Override
         public Location apply(@Nullable Waypoint input) {

src/main/java/net/citizensnpcs/trait/waypoint/RandomPointFinder.java

@@ -6,6 +6,10 @@ import org.bukkit.Location;
 
 public class RandomPointFinder implements Iterator<Location> {
 
+    public Location find() {
+        return null;
+    }
+
     @Override
     public boolean hasNext() {
         return true;
@@ -13,8 +17,7 @@ public class RandomPointFinder implements Iterator<Location> {
 
     @Override
     public Location next() {
-        // TODO Auto-generated method stub
+        return find();
-        return null;
     }
 
     @Override

src/main/java/net/citizensnpcs/trait/waypoint/WanderingWaypointProvider.java

@@ -2,7 +2,6 @@ package net.citizensnpcs.trait.waypoint;
 
 import java.util.Iterator;
 
-import net.citizensnpcs.api.CitizensAPI;
 import net.citizensnpcs.api.npc.NPC;
 import net.citizensnpcs.api.util.DataKey;
 import net.citizensnpcs.editor.Editor;
@@ -11,9 +10,9 @@ import org.bukkit.Location;
 import org.bukkit.entity.Player;
 
 public class WanderingWaypointProvider implements WaypointProvider, Iterable<Location> {
-    private WaypointGoal currentGoal;
     private final Iterator<Location> iterator = new RandomPointFinder();
     private NPC npc;
+    private boolean paused;
 
     @Override
     public Editor createEditor(Player player) {
@@ -34,7 +33,7 @@ public class WanderingWaypointProvider implements WaypointProvider, Iterable<Loc
 
     @Override
     public boolean isPaused() {
-        return currentGoal.isPaused();
+        return paused;
     }
 
     @Override
@@ -49,11 +48,12 @@ public class WanderingWaypointProvider implements WaypointProvider, Iterable<Loc
     @Override
     public void onSpawn(NPC npc) {
         this.npc = npc;
+        /*
         if (currentGoal == null) {
             currentGoal = new WaypointGoal(this, npc.getNavigator());
             CitizensAPI.registerEvents(currentGoal);
         }
-        npc.getDefaultGoalController().addGoal(currentGoal, 1);
+        npc.getDefaultGoalController().addGoal(currentGoal, 1);TODO*/
     }
 
     @Override
@@ -62,6 +62,6 @@ public class WanderingWaypointProvider implements WaypointProvider, Iterable<Loc
 
     @Override
     public void setPaused(boolean paused) {
-        currentGoal.setPaused(paused);
+        // TODO
     }
 }

src/main/java/net/citizensnpcs/trait/waypoint/WaypointGoal.java

@@ -1,88 +0,0 @@
-package net.citizensnpcs.trait.waypoint;
-
-import java.util.Iterator;
-
-import net.citizensnpcs.api.ai.Goal;
-import net.citizensnpcs.api.ai.GoalSelector;
-import net.citizensnpcs.api.ai.Navigator;
-import net.citizensnpcs.api.ai.event.NavigationCancelEvent;
-import net.citizensnpcs.api.ai.event.NavigationCompleteEvent;
-
-import org.bukkit.Location;
-import org.bukkit.event.EventHandler;
-
-public class WaypointGoal implements Goal {
-    private Location currentDestination;
-    private Iterator<Location> itr;
-    private final Navigator navigator;
-    private boolean paused;
-    private final Iterable<Location> provider;
-    private GoalSelector selector;
-
-    public WaypointGoal(Iterable<Location> provider, Navigator navigator) {
-        this.provider = provider;
-        this.navigator = navigator;
-    }
-
-    private void ensureItr() {
-        if (itr == null || !itr.hasNext()) {
-            itr = provider.iterator();
-        }
-    }
-
-    public boolean isPaused() {
-        return paused;
-    }
-
-    @EventHandler
-    public void onNavigationCancel(NavigationCancelEvent event) {
-        if (currentDestination == null || !event.getNavigator().equals(navigator))
-            return;
-        if (currentDestination.equals(event.getNavigator().getTargetAsLocation()))
-            selector.finish();
-    }
-
-    @EventHandler
-    public void onNavigationComplete(NavigationCompleteEvent event) {
-        if (currentDestination == null || !event.getNavigator().equals(navigator))
-            return;
-        if (currentDestination.equals(event.getNavigator().getTargetAsLocation()))
-            selector.finish();
-    }
-
-    public void onProviderChanged() {
-        itr = provider.iterator();
-        if (currentDestination != null)
-            selector.finish();
-    }
-
-    @Override
-    public void reset() {
-        currentDestination = null;
-        selector = null;
-    }
-
-    @Override
-    public void run() {
-    }
-
-    public void setPaused(boolean paused) {
-        if (paused && currentDestination != null)
-            selector.finish();
-        this.paused = paused;
-    }
-
-    @Override
-    public boolean shouldExecute(GoalSelector selector) {
-        if (paused || currentDestination != null)
-            return false;
-        ensureItr();
-        boolean shouldExecute = itr.hasNext();
-        if (shouldExecute) {
-            this.selector = selector;
-            currentDestination = itr.next();
-            navigator.setTarget(currentDestination);
-        }
-        return shouldExecute;
-    }
-}

src/main/java/net/citizensnpcs/util/StringHelper.java

@@ -69,7 +69,11 @@ public class StringHelper {
     }
 
     public static String wrap(Object string) {
-        return ChatColor.YELLOW + string.toString() + ChatColor.GREEN;
+        return wrap(string, ChatColor.GREEN);
+    }
+
+    public static String wrap(Object string, ChatColor colour) {
+        return ChatColor.YELLOW + string.toString() + colour;
     }
 
     public static String wrapHeader(Object string) {