Fixes, flyable NPCs

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

@@ -204,15 +204,18 @@ public class NPCCommands {
         }
 
         if (args.hasValueFlag("trait")) {
-            msg += " with traits ";
             Iterable<String> parts = Splitter.on(",").trimResults().split(args.getFlag("trait"));
+            StringBuilder builder = new StringBuilder();
             for (String tr : parts) {
-                Class<? extends Trait> clazz = CitizensAPI.getTraitFactory().getTraitClass(tr);
+                Trait trait = CitizensAPI.getTraitFactory().getTrait(tr);
-                if (clazz == null)
+                if (trait == null)
                     continue;
-                npc.addTrait(clazz);
+                npc.addTrait(trait);
-                msg += StringHelper.wrap(tr) + ", ";
+                builder.append(StringHelper.wrap(tr) + ", ");
             }
+            if (builder.length() > 0)
+                builder.delete(builder.length() - 2, builder.length());
+            msg += " with traits " + builder.toString();
         }
 
         if (args.hasValueFlag("b")) {
@@ -338,6 +341,25 @@ public class NPCCommands {
         Messaging.send(sender, msg + " when a player is nearby.");
     }
 
+    @Command(
+            aliases = { "npc" },
+            usage = "mount",
+            desc = "Mounts a controllable NPC",
+            modifiers = { "mount" },
+            min = 1,
+            max = 1,
+            permission = "npc.controllable")
+    public void mount(CommandContext args, Player player, NPC npc) {
+        boolean enabled = npc.hasTrait(Controllable.class) && npc.getTrait(Controllable.class).isEnabled();
+        if (!enabled) {
+            Messaging.send(player, StringHelper.wrap(npc.getName()) + " is not controllable.");
+            return;
+        }
+        boolean success = npc.getTrait(Controllable.class).mount(player);
+        if (!success)
+            Messaging.sendF(player, ChatColor.GREEN + "Couldn't mount %s.", StringHelper.wrap(npc.getName()));
+    }
+
     @Command(
             aliases = { "npc" },
             usage = "moveto",
@@ -554,7 +576,8 @@ public class NPCCommands {
         float newSpeed = (float) args.getDouble(1);
         npc.getNavigator().getDefaultParameters().speedModifier(newSpeed);
 
-        Messaging.sendF(sender, ChatColor.GREEN + "NPC speed modifier set to %f.", newSpeed);
+        Messaging.sendF(sender, ChatColor.GREEN + "NPC speed modifier set to %s.",
+                StringHelper.wrap(newSpeed));
     }
 
     @Command(

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

@@ -54,8 +54,7 @@ public class TraitCommands {
             added.add(StringHelper.wrap(traitName));
         }
         if (added.size() > 0)
-            Messaging
+            Messaging.sendF(sender, ChatColor.GREEN + "Added %s successfully.", Joiner.on(", ").join(added));
-                    .sendF(sender, ChatColor.GREEN + "Added %s successfully.", Joiner.on(", ").join(removed));
         if (removed.size() > 0)
             Messaging.sendF(sender, ChatColor.GREEN + "Removed %s successfully.",
                     Joiner.on(", ").join(removed));

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

@@ -77,7 +77,7 @@ public class EntityHumanNPC extends EntityPlayer implements NPCHolder {
         if (npc == null)
             return;
         Navigation navigation = getNavigation();
-        if (!navigation.f()) {
+        if (!navigation.f() || npc.getNavigator().isNavigating()) {
             navigation.e();
             moveOnCurrentHeading();
         } else if (motX != 0 || motZ != 0 || motY != 0) {

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

@@ -1,5 +1,8 @@
 package net.citizensnpcs.trait;
 
+import java.lang.reflect.Constructor;
+import java.util.Map;
+
 import net.citizensnpcs.api.event.NPCRightClickEvent;
 import net.citizensnpcs.api.exception.NPCLoadException;
 import net.citizensnpcs.api.trait.Trait;
@@ -9,28 +12,40 @@ import net.minecraft.server.EntityPlayer;
 
 import org.bukkit.craftbukkit.entity.CraftLivingEntity;
 import org.bukkit.craftbukkit.entity.CraftPlayer;
+import org.bukkit.entity.EntityType;
+import org.bukkit.entity.Player;
 import org.bukkit.event.EventHandler;
 import org.bukkit.event.block.Action;
 import org.bukkit.event.player.PlayerInteractEvent;
+import org.bukkit.util.Vector;
+
+import com.google.common.collect.Maps;
 
 //TODO: reduce reliance on CitizensNPC
 public class Controllable extends Trait implements Toggleable {
+    private Controller controller = new GroundController();
     private boolean enabled;
 
     public Controllable() {
         super("controllable");
     }
 
+    private void enterOrLeaveVehicle(Player player) {
+        EntityPlayer handle = ((CraftPlayer) player).getHandle();
+        if (getHandle().passenger != null) {
+            if (getHandle().passenger == handle)
+                player.leaveVehicle();
+            return;
+        }
+        handle.setPassengerOf(getHandle());
+    }
+
     private EntityLiving getHandle() {
         return ((CraftLivingEntity) npc.getBukkitEntity()).getHandle();
     }
 
-    private void jump() {
+    public boolean isEnabled() {
-        boolean allowed = getHandle().onGround;
+        return enabled;
-        if (!allowed)
-            return;
-        getHandle().motY = JUMP_VELOCITY;
-        // TODO: make jumping work in liquid or make liquids float the npc
     }
 
     @Override
@@ -38,16 +53,32 @@ public class Controllable extends Trait implements Toggleable {
         enabled = key.getBoolean("");
     }
 
+    public boolean mount(Player toMount) {
+        if (npc.getBukkitEntity().getPassenger() != null)
+            return false;
+        ((CraftPlayer) toMount).getHandle().setPassengerOf(getHandle());
+        return true;
+    }
+
     @EventHandler
     public void onPlayerInteract(PlayerInteractEvent event) {
         if (!npc.isSpawned() || !enabled)
             return;
         EntityPlayer handle = ((CraftPlayer) event.getPlayer()).getHandle();
         Action performed = event.getAction();
-        if (performed == Action.PHYSICAL || !handle.equals(getHandle().passenger))
+        if (!handle.equals(getHandle().passenger))
             return;
-        if (performed == Action.LEFT_CLICK_AIR || performed == Action.LEFT_CLICK_BLOCK) {
+        switch (performed) {
-            jump();
+            case RIGHT_CLICK_BLOCK:
+            case RIGHT_CLICK_AIR:
+                controller.rightClick(event);
+                break;
+            case LEFT_CLICK_BLOCK:
+            case LEFT_CLICK_AIR:
+                controller.leftClick(event);
+                break;
+            default:
+                break;
         }
     }
 
@@ -55,26 +86,34 @@ public class Controllable extends Trait implements Toggleable {
     public void onRightClick(NPCRightClickEvent event) {
         if (!enabled || !npc.isSpawned() || !event.getNPC().equals(npc))
             return;
-        EntityPlayer handle = ((CraftPlayer) event.getClicker()).getHandle();
+        controller.rightClickEntity(event);
-        if (getHandle().passenger != null) {
+    }
-            if (getHandle().passenger == handle)
+
-                event.getClicker().leaveVehicle();
+    @Override
+    public void onSpawn() {
+        EntityType type = npc.getBukkitEntity().getType();
+        Class<? extends Controller> clazz = controllerTypes.get(type);
+        if (clazz == null) {
+            controller = new GroundController();
             return;
         }
-        handle.setPassengerOf(getHandle());
+        try {
+            Constructor<? extends Controller> innerConstructor = clazz.getConstructor(Controllable.class);
+            if (innerConstructor == null) {
+                controller = clazz.newInstance();
+            } else
+                controller = innerConstructor.newInstance(this);
+        } catch (Exception e) {
+            controller = new GroundController();
+        }
     }
 
     @Override
     public void run() {
         if (!enabled || !npc.isSpawned() || getHandle().passenger == null)
             return;
-        EntityLiving handle = getHandle();
+        controller.run((Player) getHandle().passenger.getBukkitEntity());
-        boolean onGround = handle.onGround;
-        handle.motX += handle.passenger.motX * (onGround ? GROUND_SPEED : AIR_SPEED);
-        handle.motZ += handle.passenger.motZ * (onGround ? GROUND_SPEED : AIR_SPEED);
-        handle.e(npc.getNavigator().getDefaultParameters().speed());
     }
-
     @Override
     public void save(DataKey key) {
         key.setBoolean("enabled", enabled);
@@ -88,8 +127,91 @@ public class Controllable extends Trait implements Toggleable {
         return enabled;
     }
 
-    private static final double AIR_SPEED = 1.5;
+    private class AirController implements Controller {
-    private static final double GROUND_SPEED = 4;
+        boolean paused = false;
 
-    private static final double JUMP_VELOCITY = 0.6;
+        @Override
+        public void leftClick(PlayerInteractEvent event) {
+            paused = !paused;
+        }
+
+        @Override
+        public void rightClick(PlayerInteractEvent event) {
+            paused = !paused;
+        }
+
+        @Override
+        public void rightClickEntity(NPCRightClickEvent event) {
+            enterOrLeaveVehicle(event.getClicker());
+        }
+
+        @Override
+        public void run(Player rider) {
+            if (paused)
+                return;
+            Vector dir = rider.getEyeLocation().getDirection();
+            double y = dir.getY();
+            dir.multiply(npc.getNavigator().getDefaultParameters().speedModifier()).setY(y);
+            EntityLiving handle = getHandle();
+            handle.motX += dir.getX();
+            handle.motY += dir.getY();
+            handle.motZ += dir.getZ();
+        }
+    }
+
+    private static interface Controller {
+        void leftClick(PlayerInteractEvent event);
+
+        void rightClick(PlayerInteractEvent event);
+
+        void rightClickEntity(NPCRightClickEvent event);
+
+        void run(Player rider);
+    }
+
+    private class GroundController implements Controller {
+        private void jump() {
+            boolean allowed = getHandle().onGround;
+            if (!allowed)
+                return;
+            getHandle().motY = JUMP_VELOCITY;
+        }
+        @Override
+        public void leftClick(PlayerInteractEvent event) {
+            jump();
+        }
+        @Override
+        public void rightClick(PlayerInteractEvent event) {
+        }
+
+        @Override
+        public void rightClickEntity(NPCRightClickEvent event) {
+            enterOrLeaveVehicle(event.getClicker());
+        }
+
+        @Override
+        public void run(Player rider) {
+            EntityLiving handle = getHandle();
+            boolean onGround = handle.onGround;
+            float speedMod = npc.getNavigator().getDefaultParameters()
+                    .modifiedSpeed((onGround ? GROUND_SPEED : AIR_SPEED));
+            handle.motX += handle.passenger.motX * speedMod;
+            handle.motZ += handle.passenger.motZ * speedMod;
+        }
+
+        private static final float AIR_SPEED = 1.5F;
+
+        private static final float GROUND_SPEED = 4F;
+
+        private static final float JUMP_VELOCITY = 0.6F;
+    }
+
+    private static final Map<EntityType, Class<? extends Controller>> controllerTypes = Maps
+            .newEnumMap(EntityType.class);
+
+    static {
+        controllerTypes.put(EntityType.BLAZE, AirController.class);
+        controllerTypes.put(EntityType.ENDER_DRAGON, AirController.class);
+        controllerTypes.put(EntityType.GHAST, AirController.class);
+    }
 }

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

@@ -26,6 +26,7 @@ public class NMS {
     private static Map<Class<? extends Entity>, Integer> ENTITY_CLASS_TO_INT;
     private static Map<Integer, Class<? extends Entity>> ENTITY_INT_TO_CLASS;
     private static Field GOAL_FIELD;
+    private static Field LAND_SPEED_MODIFIER_FIELD;
     private static final Map<EntityType, Float> MOVEMENT_SPEEDS = Maps.newEnumMap(EntityType.class);
     private static Field PATHFINDING_RANGE;
 
@@ -89,6 +90,16 @@ public class NMS {
         throw new IllegalArgumentException("unable to find valid entity superclass");
     }
 
+    public static void setLandSpeedModifier(EntityLiving handle, float speed) {
+        if (LAND_SPEED_MODIFIER_FIELD == null)
+            return;
+        try {
+            LAND_SPEED_MODIFIER_FIELD.setFloat(handle, speed);
+        } catch (IllegalArgumentException e) {
+        } catch (IllegalAccessException e) {
+        }
+    }
+
     public static void stopNetworkThreads(NetworkManager manager) {
         if (THREAD_STOPPER == null)
             return;
@@ -133,6 +144,7 @@ public class NMS {
         MOVEMENT_SPEEDS.put(EntityType.PLAYER, 1F);
         MOVEMENT_SPEEDS.put(EntityType.VILLAGER, 0.3F);
 
+        LAND_SPEED_MODIFIER_FIELD = getField(EntityLiving.class, "bB");
         SPEED_FIELD = getField(EntityLiving.class, "bw");
         PATHFINDING_RANGE = getField(Navigation.class, "e");
         GOAL_FIELD = getField(PathfinderGoalSelector.class, "a");