Refactor a little to prepare CommandManager for API

src/main/java/net/citizensnpcs/Citizens.java

@@ -139,17 +139,11 @@ public class Citizens extends JavaPlugin implements CitizensPlugin {
     }
 
     @Override
-    public boolean onCommand(CommandSender sender, Command cmd, String cmdName, String[] args) {
+    public boolean onCommand(CommandSender sender, Command command, String cmdName, String[] args) {
         try {
-            // must put command into split.
-            String[] split = new String[args.length + 1];
-            System.arraycopy(args, 0, split, 1, args.length);
-            split[0] = cmd.getName().toLowerCase();
-
             String modifier = args.length > 0 ? args[0] : "";
-
+            if (!commands.hasCommand(command, modifier) && !modifier.isEmpty()) {
-            if (!commands.hasCommand(split[0], modifier) && !modifier.isEmpty()) {
+                return suggestClosestModifier(sender, command.getName().toLowerCase(), modifier);
-                return suggestClosestModifier(sender, split[0], modifier);
             }
 
             NPC npc = selector.getSelected(sender);
@@ -157,7 +151,7 @@ public class Citizens extends JavaPlugin implements CitizensPlugin {
             // flexibility (ie. adding more context in the future without
             // changing everything)
             try {
-                commands.execute(split, sender, sender, npc);
+                commands.execute(command, args, sender, sender, npc);
             } catch (ServerCommandException ex) {
                 Messaging.sendTr(sender, Messages.COMMAND_MUST_BE_INGAME);
             } catch (CommandUsageException ex) {
@@ -260,7 +254,7 @@ public class Citizens extends JavaPlugin implements CitizensPlugin {
 
     public void registerCommandClass(Class<?> clazz) {
         try {
-        commands.register(clazz);
+            commands.register(clazz);
         } catch (Throwable ex) {
             Messaging.logTr(Messages.CITIZENS_INVALID_COMMAND_CLASS);
             ex.printStackTrace();

src/main/java/net/citizensnpcs/EventListen.java

@@ -79,13 +79,6 @@ public class EventListen implements Listener {
         toRespawn.removeAll(coord);
     }
 
-    private void spawn(int id) {
-        NPC npc = npcRegistry.getById(id);
-        if (npc == null)
-            return;
-        npc.spawn(npc.getTrait(CurrentLocation.class).getLocation());
-    }
-
     @EventHandler(ignoreCancelled = true)
     public void onChunkUnload(ChunkUnloadEvent event) {
         ChunkCoord coord = toCoord(event.getChunk());
@@ -184,10 +177,6 @@ public class EventListen implements Listener {
         Bukkit.getPluginManager().callEvent(new EntityTargetNPCEvent(event, npc));
     }
 
-    /*
-     * Player events
-     */
-
     @EventHandler(ignoreCancelled = true)
     public void onPlayerChangedWorld(PlayerChangedWorldEvent event) {
         EntityPlayer handle = ((CraftPlayer) event.getPlayer()).getHandle();
@@ -198,6 +187,10 @@ public class EventListen implements Listener {
         // undesirable as player NPCs are not real players and confuse plugins.
     }
 
+    /*
+     * Player events
+     */
+
     @EventHandler(ignoreCancelled = true)
     public void onPlayerCreateNPC(PlayerCreateNPCEvent event) {
         if (event.getCreator().hasPermission("citizens.admin.avoid-limits"))
@@ -272,18 +265,25 @@ public class EventListen implements Listener {
         }
     }
 
+    private void spawn(int id) {
+        NPC npc = npcRegistry.getById(id);
+        if (npc == null)
+            return;
+        npc.spawn(npc.getTrait(CurrentLocation.class).getLocation());
+    }
+
     private void storeForRespawn(NPC npc) {
         toRespawn.put(toCoord(npc.getBukkitEntity().getLocation()), npc.getId());
     }
 
-    private ChunkCoord toCoord(Location loc) {
-        return new ChunkCoord(loc.getWorld().getName(), loc.getBlockX() >> 4, loc.getBlockZ() >> 4);
-    }
-
     private ChunkCoord toCoord(Chunk chunk) {
         return new ChunkCoord(chunk);
     }
 
+    private ChunkCoord toCoord(Location loc) {
+        return new ChunkCoord(loc.getWorld().getName(), loc.getBlockX() >> 4, loc.getBlockZ() >> 4);
+    }
+
     private static class ChunkCoord {
         private final String worldName;
         private final int x;

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

@@ -58,6 +58,19 @@ public class CommandManager {
 
     private final Set<Method> serverCommands = new HashSet<Method>();
 
+    // Attempt to execute a command.
+    public void execute(org.bukkit.command.Command command, String[] args, CommandSender sender,
+            Object... methodArgs) throws CommandException {
+        // must put command into split.
+        String[] newArgs = new String[args.length + 1];
+        System.arraycopy(args, 0, newArgs, 1, args.length);
+        newArgs[0] = command.getName().toLowerCase();
+
+        Object[] newMethodArgs = new Object[methodArgs.length + 1];
+        System.arraycopy(methodArgs, 0, newMethodArgs, 1, methodArgs.length);
+        executeMethod(null, newArgs, sender, newMethodArgs);
+    }
+
     /*
      * Attempt to execute a command. This version takes a separate command name
      * (for the root command) and then a list of following arguments.
@@ -73,13 +86,6 @@ public class CommandManager {
         executeMethod(null, newArgs, sender, newMethodArgs);
     }
 
-    // Attempt to execute a command.
-    public void execute(String[] args, CommandSender sender, Object... methodArgs) throws CommandException {
-        Object[] newMethodArgs = new Object[methodArgs.length + 1];
-        System.arraycopy(methodArgs, 0, newMethodArgs, 1, methodArgs.length);
-        executeMethod(null, args, sender, newMethodArgs);
-    }
-
     // Attempt to execute a command.
     private void executeMethod(Method parent, String[] args, CommandSender sender, Object[] methodArgs)
             throws CommandException {
@@ -118,48 +124,7 @@ public class CommandManager {
 
         Requirements cmdRequirements = requirements.get(method);
         if (cmdRequirements != null) {
-            NPC npc = (NPC) methodArgs[2];
+            processRequirements(sender, methodArgs, context, cmdRequirements);
-
-            // Requirements
-            if (cmdRequirements.selected()) {
-                boolean canRedefineSelected = context.hasValueFlag("id")
-                        && sender.hasPermission("npc.select");
-                String error = Messaging.tr(Messages.COMMAND_MUST_HAVE_SELECTED);
-                if (canRedefineSelected) {
-                    npc = CitizensAPI.getNPCRegistry().getById(context.getFlagInteger("id"));
-                    if (npc == null)
-                        error += ' ' + Messaging.tr(Messages.COMMAND_ID_NOT_FOUND,
-                                context.getFlagInteger("id"));
-                }
-                if (npc == null)
-                    throw new RequirementMissingException(error);
-            }
-
-            if (cmdRequirements.ownership() && npc != null && !sender.hasPermission("citizens.admin")
-                    && !npc.getTrait(Owner.class).isOwnedBy(sender))
-                throw new RequirementMissingException(Messaging.tr(Messages.COMMAND_MUST_BE_OWNER));
-
-            if (npc != null) {
-                for (Class<? extends Trait> clazz : cmdRequirements.traits()) {
-                    if (!npc.hasTrait(clazz))
-                        throw new RequirementMissingException(Messaging.tr(Messages.COMMAND_MISSING_TRAIT,
-                                clazz.getSimpleName()));
-                }
-            }
-
-            if (npc != null) {
-                Set<EntityType> types = Sets.newEnumSet(Arrays.asList(cmdRequirements.types()),
-                        EntityType.class);
-                if (types.contains(EntityType.UNKNOWN))
-                    types = EnumSet.allOf(EntityType.class);
-                types.removeAll(Sets.newHashSet(cmdRequirements.excludedTypes()));
-
-                EntityType type = npc.getTrait(MobType.class).getType();
-                if (!types.contains(type)) {
-                    throw new RequirementMissingException(Messaging.tr(
-                            Messages.COMMAND_REQUIREMENTS_INVALID_MOB_TYPE, type.getName()));
-                }
-            }
         }
 
         Object instance = instances.get(method);
@@ -232,9 +197,9 @@ public class CommandManager {
      * Checks to see whether there is a command named such at the root level.
      * This will check aliases as well.
      */
-    public boolean hasCommand(String command, String modifier) {
+    public boolean hasCommand(org.bukkit.command.Command cmd, String modifier) {
-        return commands.containsKey(command.toLowerCase() + " " + modifier.toLowerCase())
+        return commands.containsKey(cmd.getName().toLowerCase() + " " + modifier.toLowerCase())
-                || commands.containsKey(command.toLowerCase() + " *");
+                || commands.containsKey(cmd.getName().toLowerCase() + " *");
     }
 
     // Returns whether a player has permission.
@@ -252,6 +217,52 @@ public class CommandManager {
         return false;
     }
 
+    private void processRequirements(CommandSender sender, Object[] methodArgs, CommandContext context,
+            Requirements cmdRequirements) throws RequirementMissingException {
+        NPC npc = (NPC) methodArgs[2];
+
+        // Requirements
+        if (cmdRequirements.selected()) {
+            boolean canRedefineSelected = context.hasValueFlag("id")
+                    && sender.hasPermission("npc.select");
+            String error = Messaging.tr(Messages.COMMAND_MUST_HAVE_SELECTED);
+            if (canRedefineSelected) {
+                npc = CitizensAPI.getNPCRegistry().getById(context.getFlagInteger("id"));
+                if (npc == null)
+                    error += ' ' + Messaging.tr(Messages.COMMAND_ID_NOT_FOUND,
+                            context.getFlagInteger("id"));
+            }
+            if (npc == null)
+                throw new RequirementMissingException(error);
+        }
+
+        if (cmdRequirements.ownership() && npc != null && !sender.hasPermission("citizens.admin")
+                && !npc.getTrait(Owner.class).isOwnedBy(sender))
+            throw new RequirementMissingException(Messaging.tr(Messages.COMMAND_MUST_BE_OWNER));
+
+        if (npc != null) {
+            for (Class<? extends Trait> clazz : cmdRequirements.traits()) {
+                if (!npc.hasTrait(clazz))
+                    throw new RequirementMissingException(Messaging.tr(Messages.COMMAND_MISSING_TRAIT,
+                            clazz.getSimpleName()));
+            }
+        }
+
+        if (npc != null) {
+            Set<EntityType> types = Sets.newEnumSet(Arrays.asList(cmdRequirements.types()),
+                    EntityType.class);
+            if (types.contains(EntityType.UNKNOWN))
+                types = EnumSet.allOf(EntityType.class);
+            types.removeAll(Sets.newHashSet(cmdRequirements.excludedTypes()));
+
+            EntityType type = npc.getTrait(MobType.class).getType();
+            if (!types.contains(type)) {
+                throw new RequirementMissingException(Messaging.tr(
+                        Messages.COMMAND_REQUIREMENTS_INVALID_MOB_TYPE, type.getName()));
+            }
+        }
+    }
+
     /*
      * Register an class that contains commands (denoted by Command. If no
      * dependency injector is specified, then the methods of the class will be

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

@@ -45,13 +45,6 @@ public class CitizensVillagerNPC extends CitizensMobNPC {
             }
         }
 
-        @Override
-        public void bl() {
-            super.bl();
-            if (npc != null)
-                npc.update();
-        }
-
         @Override
         public boolean a(EntityHuman entityhuman) {
             if (npc == null)
@@ -59,6 +52,13 @@ public class CitizensVillagerNPC extends CitizensMobNPC {
             return false; // block trades
         }
 
+        @Override
+        public void bl() {
+            super.bl();
+            if (npc != null)
+                npc.update();
+        }
+
         @Override
         public void collide(net.minecraft.server.Entity entity) {
             // this method is called by both the entities involved - cancelling

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

@@ -13,6 +13,10 @@ public class Gravity extends Trait implements Toggleable {
         super("gravity");
     }
 
+    public void gravitate(boolean gravitate) {
+    	enabled = gravitate;
+    }
+    
     @Override
     public void run() {
         if (!npc.isSpawned() || !enabled)
@@ -22,10 +26,6 @@ public class Gravity extends Trait implements Toggleable {
         npc.getBukkitEntity().setVelocity(velocity);
     }
 
-    public void gravitate(boolean gravitate) {
-    	enabled = gravitate;
-    }
-
     @Override
     public boolean toggle() {
         return enabled = !enabled;

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

@@ -39,10 +39,6 @@ public class LookClose extends Trait implements Toggleable, CommandConfigurable
         realisticLooking = args.hasFlag('r');
     }
 
-    public void lookClose(boolean lookClose) {
-        enabled = lookClose;
-    }
-
     private void findNewTarget() {
         List<Entity> nearby = npc.getBukkitEntity().getNearbyEntities(range, range, range);
         final Location npcLocation = npc.getBukkitEntity().getLocation();
@@ -83,6 +79,10 @@ public class LookClose extends Trait implements Toggleable, CommandConfigurable
         realisticLooking = key.getBoolean("realisticlooking", key.getBoolean("realistic-looking"));
     }
 
+    public void lookClose(boolean lookClose) {
+        enabled = lookClose;
+    }
+
     @Override
     public void onDespawn() {
         lookingAt = null;

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

@@ -32,10 +32,6 @@ public class Poses extends Trait {
         return true;
     }
 
-    public void assumePose(Location location) {
-        assumePose(location.getYaw(), location.getPitch());
-    }
-
     private void assumePose(float yaw, float pitch) {
         if (!npc.isSpawned())
             npc.spawn(npc.getTrait(CurrentLocation.class).getLocation());
@@ -43,6 +39,34 @@ public class Poses extends Trait {
         Util.assumePose(npc.getBukkitEntity(), yaw, pitch);
     }
 
+    public void assumePose(Location location) {
+        assumePose(location.getYaw(), location.getPitch());
+    }
+
+    public void assumePose(String flag) {
+        Pose pose = poses.get(flag.toLowerCase());
+        assumePose(pose.getYaw(), pose.getPitch());
+    }
+
+    public void describe(CommandSender sender, int page) throws CommandException {
+        Paginator paginator = new Paginator().header("Pose");
+        paginator.addLine("<e>Key: <a>ID  <b>Name  <c>Pitch/Yaw");
+        int i = 0;
+        for (Pose pose : poses.values()) {
+            String line = "<a>" + i + "<b>  " + pose.getName() + "<c>  " + pose.getPitch() + "/"
+                    + pose.getYaw();
+            paginator.addLine(line);
+            i++;
+        }
+
+        if (!paginator.sendPage(sender, page))
+            throw new CommandException(Messages.COMMAND_PAGE_MISSING);
+    }
+
+    public boolean hasPose(String pose) {
+        return poses.containsKey(pose.toLowerCase());
+    }
+
     @Override
     public void load(DataKey key) throws NPCLoadException {
         for (DataKey sub : key.getRelative("list").getIntegerSubKeys())
@@ -64,28 +88,4 @@ public class Poses extends Trait {
         for (int i = 0; i < poses.size(); i++)
             key.setString("list." + String.valueOf(i), poses.get(i).stringValue());
     }
-
-    public void assumePose(String flag) {
-        Pose pose = poses.get(flag.toLowerCase());
-        assumePose(pose.getYaw(), pose.getPitch());
-    }
-
-    public boolean hasPose(String pose) {
-        return poses.containsKey(pose.toLowerCase());
-    }
-
-    public void describe(CommandSender sender, int page) throws CommandException {
-        Paginator paginator = new Paginator().header("Pose");
-        paginator.addLine("<e>Key: <a>ID  <b>Name  <c>Pitch/Yaw");
-        int i = 0;
-        for (Pose pose : poses.values()) {
-            String line = "<a>" + i + "<b>  " + pose.getName() + "<c>  " + pose.getPitch() + "/"
-                    + pose.getYaw();
-            paginator.addLine(line);
-            i++;
-        }
-
-        if (!paginator.sendPage(sender, page))
-            throw new CommandException(Messages.COMMAND_PAGE_MISSING);
-    }
 }

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

@@ -19,6 +19,7 @@ import net.minecraft.server.MathHelper;
 import net.minecraft.server.MobEffectList;
 import net.minecraft.server.Navigation;
 import net.minecraft.server.NetworkManager;
+import net.minecraft.server.Packet;
 import net.minecraft.server.PathfinderGoalSelector;
 import net.minecraft.server.World;
 
@@ -26,8 +27,10 @@ import org.bukkit.Location;
 import org.bukkit.Material;
 import org.bukkit.craftbukkit.CraftWorld;
 import org.bukkit.craftbukkit.entity.CraftLivingEntity;
+import org.bukkit.craftbukkit.entity.CraftPlayer;
 import org.bukkit.entity.EntityType;
 import org.bukkit.entity.LivingEntity;
+import org.bukkit.entity.Player;
 import org.bukkit.material.Stairs;
 import org.bukkit.material.Step;
 
@@ -189,6 +192,10 @@ public class NMS {
         throw new IllegalArgumentException("unable to find valid entity superclass");
     }
 
+    public static void sendPacket(Player player, Packet packet) {
+        ((CraftPlayer) player).getHandle().netServerHandler.sendPacket(packet);
+    }
+
     public static void setHeadYaw(EntityLiving handle, float yaw) {
         handle.ay = yaw;
     }

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

@@ -7,6 +7,8 @@ import net.citizensnpcs.Settings.Setting;
 import org.bukkit.ChatColor;
 
 public class StringHelper {
+    private static Pattern COLOR_MATCHER;
+
     public static String capitalize(Object string) {
         String capitalize = string.toString();
         return capitalize.replaceFirst(String.valueOf(capitalize.charAt(0)),
@@ -82,8 +84,6 @@ public class StringHelper {
         String highlight = Setting.HIGHLIGHT_COLOUR.asString();
         return highlight + "=====[ " + string.toString() + highlight + " ]=====";
     }
-
-    private static Pattern COLOR_MATCHER;
     static {
         String colors = "";
         for (ChatColor color : ChatColor.values())

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

@@ -11,9 +11,7 @@ import org.bukkit.Bukkit;
 import org.bukkit.Location;
 import org.bukkit.Material;
 import org.bukkit.World;
-import org.bukkit.block.BlockFace;
 import org.bukkit.craftbukkit.entity.CraftLivingEntity;
-import org.bukkit.craftbukkit.entity.CraftPlayer;
 import org.bukkit.entity.Entity;
 import org.bukkit.entity.EntityType;
 import org.bukkit.entity.Player;
@@ -27,9 +25,9 @@ public class Util {
     private Util() {
     }
 
-    public static void assumePose(org.bukkit.entity.Entity entity, float  yaw, float pitch) {
+    public static void assumePose(org.bukkit.entity.Entity entity, float yaw, float pitch) {
         EntityLiving handle = ((CraftLivingEntity) entity).getHandle();
-        NMS.look(handle, yaw,pitch);
+        NMS.look(handle, yaw, pitch);
     }
 
     public static void callCollisionEvent(NPC npc, net.minecraft.server.Entity entity) {
@@ -65,44 +63,6 @@ public class Util {
         NMS.look(handle, (float) yaw - 90, (float) pitch);
     }
 
-    public static BlockFace getFacingDirection(float degrees) {
-        return getFacingDirection(degrees, 10);
-    }
-
-    public static BlockFace getFacingDirection(float degrees, double leeway) {
-        while (degrees < 0D) {
-            degrees += 360D;
-        }
-        while (degrees > 360D) {
-            degrees -= 360D;
-        }
-        if (isFacingWest(degrees, leeway))
-            return BlockFace.WEST;
-        if (isFacingNorth(degrees, leeway))
-            return BlockFace.NORTH;
-        if (isFacingEast(degrees, leeway))
-            return BlockFace.EAST;
-        if (isFacingSouth(degrees, leeway))
-            return BlockFace.SOUTH;
-        return BlockFace.SELF;
-    }
-
-    private static boolean isFacingEast(double degrees, double leeway) {
-        return (135 - leeway <= degrees) && (degrees < 225 + leeway);
-    }
-
-    private static boolean isFacingNorth(double degrees, double leeway) {
-        return (45 - leeway <= degrees) && (degrees < 135 + leeway);
-    }
-
-    private static boolean isFacingSouth(double degrees, double leeway) {
-        return (225 - leeway <= degrees) && (degrees < 315 + leeway);
-    }
-
-    private static boolean isFacingWest(double degrees, double leeway) {
-        return ((0 <= degrees) && (degrees < 45 + leeway)) || ((315 - leeway <= degrees) && (degrees <= 360));
-    }
-
     public static boolean isLoaded(Location location) {
         if (location.getWorld() == null)
             return false;
@@ -159,7 +119,7 @@ public class Util {
             if (location.distanceSquared(ply.getLocation()) > radius) {
                 continue;
             }
-            ((CraftPlayer) ply).getHandle().netServerHandler.sendPacket(packet);
+            NMS.sendPacket(ply, packet);
         }
     }
 
@@ -169,7 +129,7 @@ public class Util {
             if (player == null || !player.isOnline())
                 continue;
             for (Packet packet : packets) {
-                ((CraftPlayer) player).getHandle().netServerHandler.sendPacket(packet);
+                NMS.sendPacket(player, packet);
             }
         }
     }