Rework lookclose logic, Controllable is now Toggleable

src/main/java/net/citizensnpcs/command/command/NPCCommands.java

@@ -459,11 +459,10 @@ public class NPCCommands {
             max = 1,
             permission = "npc.controllable")
     public void controllable(CommandContext args, Player player, NPC npc) {
-        if (npc.hasTrait(Controllable.class)) {
+        boolean enabled = npc.getTrait(Controllable.class).toggle();
-            npc.removeTrait(Controllable.class);
+        if (enabled) {
             Messaging.send(player, StringHelper.wrap(npc.getName()) + " can no longer be controlled.");
         } else {
-            npc.addTrait(traitManager.getTrait(Controllable.class, npc));
             Messaging.send(player, StringHelper.wrap(npc.getName()) + " can now be controlled.");
         }
 

src/main/java/net/citizensnpcs/trait/Controllable.java

@@ -15,8 +15,9 @@ import org.bukkit.event.block.Action;
 import org.bukkit.event.player.PlayerInteractEvent;
 
 //TODO: reduce reliance on CitizensNPC
-public class Controllable extends Trait implements Runnable, Listener {
+public class Controllable extends Trait implements Runnable, Listener, Toggleable {
     private final CitizensNPC npc;
+    private boolean enabled;
 
     public Controllable(NPC npc) {
         this.npc = (CitizensNPC) npc;
@@ -30,6 +31,7 @@ public class Controllable extends Trait implements Runnable, Listener {
 
     @Override
     public void load(DataKey key) throws NPCLoadException {
+        enabled = key.getBoolean("enabled");
     }
 
     @EventHandler
@@ -63,7 +65,13 @@ public class Controllable extends Trait implements Runnable, Listener {
 
     @Override
     public void save(DataKey key) {
+        key.setBoolean("enabled", enabled);
     }
 
     private static final double JUMP_VELOCITY = 0.6;
+
+    @Override
+    public boolean toggle() {
+        return (enabled = !enabled);
+    }
 }

src/main/java/net/citizensnpcs/trait/LookClose.java

@@ -1,33 +1,38 @@
 package net.citizensnpcs.trait;
 
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.List;
+
 import net.citizensnpcs.Settings.Setting;
 import net.citizensnpcs.api.exception.NPCLoadException;
 import net.citizensnpcs.api.npc.NPC;
 import net.citizensnpcs.api.trait.Trait;
 import net.citizensnpcs.api.util.DataKey;
-import net.citizensnpcs.npc.CitizensNPC;
-
 import net.minecraft.server.EntityLiving;
 
 import org.bukkit.Location;
+import org.bukkit.craftbukkit.entity.CraftLivingEntity;
 import org.bukkit.entity.Entity;
+import org.bukkit.entity.Player;
 
 public class LookClose extends Trait implements Runnable, Toggleable {
-    private boolean lookClose = Setting.DEFAULT_LOOK_CLOSE.asBoolean();
+    private boolean enabled = Setting.DEFAULT_LOOK_CLOSE.asBoolean();
+    private Player lookingAt;
     private final NPC npc;
 
     public LookClose(NPC npc) {
         this.npc = npc;
     }
 
-    private void faceEntity(CitizensNPC npc, Entity target) {
+    private void faceEntity(Entity from, Entity at) {
-        if (npc.getBukkitEntity().getWorld() != target.getWorld())
+        if (from.getWorld() != at.getWorld())
             return;
-        Location loc = npc.getBukkitEntity().getLocation();
+        Location loc = from.getLocation();
 
-        double xDiff = target.getLocation().getX() - loc.getX();
+        double xDiff = at.getLocation().getX() - loc.getX();
-        double yDiff = target.getLocation().getY() - loc.getY();
+        double yDiff = at.getLocation().getY() - loc.getY();
-        double zDiff = target.getLocation().getZ() - loc.getZ();
+        double zDiff = at.getLocation().getZ() - loc.getZ();
 
         double distanceXZ = Math.sqrt(xDiff * xDiff + zDiff * zDiff);
         double distanceY = Math.sqrt(distanceXZ * distanceXZ + yDiff * yDiff);
@@ -38,38 +43,65 @@ public class LookClose extends Trait implements Runnable, Toggleable {
             yaw = yaw + (Math.abs(180 - yaw) * 2);
         }
 
-        npc.getHandle().yaw = (float) yaw - 90;
+        EntityLiving handle = ((CraftLivingEntity) from).getHandle();
-        npc.getHandle().pitch = (float) pitch;
+        handle.yaw = (float) yaw - 90;
-        npc.getHandle().X = npc.getHandle().yaw;
+        handle.pitch = (float) pitch;
+        handle.X = handle.yaw;
     }
 
     @Override
     public void load(DataKey key) throws NPCLoadException {
-        lookClose = key.getBoolean("");
+        enabled = key.getBoolean("enabled");
     }
 
     @Override
     public void run() {
-        EntityLiving search = null;
+        if (!enabled || npc.getAI().hasDestination())
-        CitizensNPC handle = (CitizensNPC) npc;
+            return;
-        if (!npc.getAI().hasDestination()
+        if (hasInvalidTarget()) {
-                && (search = handle.getHandle().world.findNearbyPlayer(handle.getHandle(), 5)) != null && lookClose)
+            findNewTarget();
-            faceEntity(handle, search.getBukkitEntity());
+        } else {
+            faceEntity(npc.getBukkitEntity(), lookingAt);
+        }
+    }
+
+    private void findNewTarget() {
+        List<Entity> nearby = npc.getBukkitEntity().getNearbyEntities(2.5, 5, 2.5);
+        Collections.sort(nearby, new Comparator<Entity>() {
+            @Override
+            public int compare(Entity o1, Entity o2) {
+                double d1 = o1.getLocation().distanceSquared(npc.getBukkitEntity().getLocation());
+                double d2 = o2.getLocation().distanceSquared(npc.getBukkitEntity().getLocation());
+                return Double.compare(d1, d2);
+            }
+        });
+        for (Entity entity : nearby) {
+            if (entity instanceof Player) {
+                lookingAt = (Player) entity;
+                return;
+            }
+        }
+        lookingAt = null;
+    }
+
+    private boolean hasInvalidTarget() {
+        return lookingAt == null || !lookingAt.isOnline()
+                || lookingAt.getLocation().distanceSquared(npc.getBukkitEntity().getLocation()) > 5;
     }
 
     @Override
     public void save(DataKey key) {
-        key.setBoolean("", lookClose);
+        key.setBoolean("enabled", enabled);
     }
 
     @Override
     public boolean toggle() {
-        lookClose = !lookClose;
+        enabled = !enabled;
-        return lookClose;
+        return enabled;
     }
 
     @Override
     public String toString() {
-        return "LookClose{" + lookClose + "}";
+        return "LookClose{" + enabled + "}";
     }
 }