Remove explicit @ServerCommand, remove unnecessary instanceof in CitizensNPCRegistry

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

@@ -40,7 +40,7 @@ import com.google.common.collect.ListMultimap;
 import com.google.gson.internal.Pair;
 
 public class EventListen implements Listener {
-    private final NPCRegistry npcManager = CitizensAPI.getNPCRegistry();
+    private final NPCRegistry npcRegistry = CitizensAPI.getNPCRegistry();
     private final ListMultimap<Pair<Integer, Integer>, Integer> toRespawn = ArrayListMultimap.create();
 
     /*
@@ -52,7 +52,7 @@ public class EventListen implements Listener {
         if (!toRespawn.containsKey(coord))
             return;
         for (int id : toRespawn.get(coord)) {
-            NPC npc = npcManager.getNPC(id);
+            NPC npc = npcRegistry.getNPC(id);
             npc.spawn(npc.getTrait(CurrentLocation.class).getLocation());
         }
         toRespawn.removeAll(coord);
@@ -64,7 +64,7 @@ public class EventListen implements Listener {
             return;
 
         Pair<Integer, Integer> coord = toIntPair(event.getChunk());
-        for (NPC npc : npcManager) {
+        for (NPC npc : npcRegistry) {
             if (!npc.isSpawned())
                 continue;
             Location loc = npc.getBukkitEntity().getLocation();
@@ -81,10 +81,10 @@ public class EventListen implements Listener {
      */
     @EventHandler
     public void onEntityDamage(EntityDamageEvent event) {
-        if (!npcManager.isNPC(event.getEntity()))
+        if (!npcRegistry.isNPC(event.getEntity()))
             return;
 
-        NPC npc = npcManager.getNPC(event.getEntity());
+        NPC npc = npcRegistry.getNPC(event.getEntity());
         if (event instanceof EntityDamageByEntityEvent) {
             NPCDamageByEntityEvent damageEvent = new NPCDamageByEntityEvent(npc, (EntityDamageByEntityEvent) event);
             Bukkit.getPluginManager().callEvent(damageEvent);
@@ -108,18 +108,18 @@ public class EventListen implements Listener {
 
     @EventHandler
     public void onEntityDeath(EntityDeathEvent event) {
-        if (!npcManager.isNPC(event.getEntity()))
+        if (!npcRegistry.isNPC(event.getEntity()))
             return;
-        NPC npc = npcManager.getNPC(event.getEntity());
+        NPC npc = npcRegistry.getNPC(event.getEntity());
         npc.despawn();
     }
 
     @EventHandler
     public void onEntityTarget(EntityTargetEvent event) {
-        if (event.isCancelled() || !npcManager.isNPC(event.getEntity()) || !(event.getTarget() instanceof Player))
+        if (event.isCancelled() || !npcRegistry.isNPC(event.getEntity()) || !(event.getTarget() instanceof Player))
             return;
 
-        NPC npc = npcManager.getNPC(event.getEntity());
+        NPC npc = npcRegistry.getNPC(event.getEntity());
         Player player = (Player) event.getTarget();
 
         // Call right-click event
@@ -149,7 +149,7 @@ public class EventListen implements Listener {
 
     @EventHandler
     public void onPlayerInteractEntity(PlayerInteractEntityEvent event) {
-        if (!npcManager.isNPC(event.getRightClicked()))
+        if (!npcRegistry.isNPC(event.getRightClicked()))
             return;
 
         // Call target event for NPCs
@@ -171,7 +171,7 @@ public class EventListen implements Listener {
             if (!event.getWorld().isChunkLoaded(chunk.first, chunk.second))
                 continue;
             for (int id : toRespawn.get(chunk)) {
-                NPC npc = npcManager.getNPC(id);
+                NPC npc = npcRegistry.getNPC(id);
                 npc.spawn(npc.getTrait(CurrentLocation.class).getLocation());
             }
             toRespawn.removeAll(chunk);
@@ -183,7 +183,7 @@ public class EventListen implements Listener {
         if (event.isCancelled())
             return;
 
-        for (NPC npc : npcManager) {
+        for (NPC npc : npcRegistry) {
             if (!npc.isSpawned() || !npc.getBukkitEntity().getWorld().equals(event.getWorld()))
                 continue;
 

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

@@ -51,7 +51,7 @@ public class CommandManager {
 
     private final Map<Method, Requirements> requirements = new HashMap<Method, Requirements>();
 
-    private final Map<Method, ServerCommand> serverCommands = new HashMap<Method, ServerCommand>();
+    private final Set<Method> serverCommands = new HashSet<Method>();
 
     private final Map<String, List<Command>> subCommands = new HashMap<String, List<Command>>();
 
@@ -86,14 +86,13 @@ public class CommandManager {
         if (method == null)
             method = commands.get(cmdName.toLowerCase() + " *");
 
-        if (method != null && methodArgs != null && serverCommands.get(method) == null
-                && methodArgs[1] instanceof ConsoleCommandSender)
-            throw new ServerCommandException();
-
         if (method == null && parent == null)
             throw new UnhandledCommandException();
 
-        if (methodArgs[1] instanceof Player && !hasPermission(method, sender))
+        if (!serverCommands.contains(method) && methodArgs[1] instanceof ConsoleCommandSender)
+            throw new ServerCommandException();
+
+        if (!hasPermission(method, sender) && methodArgs[1] instanceof Player)
             throw new NoPermissionsException();
 
         Requirements cmdRequirements = requirements.get(method);
@@ -258,12 +257,9 @@ public class CommandManager {
             if (requirements != null)
                 requirements.put(method, cmdRequirements);
 
-            ServerCommand serverCommand = null;
+            Class<?> senderClass = method.getParameterTypes()[1];
-            if (method.isAnnotationPresent(ServerCommand.class))
+            if (senderClass == CommandSender.class)
-                serverCommand = method.getAnnotation(ServerCommand.class);
+                serverCommands.add(method);
-
-            if (serverCommand != null)
-                serverCommands.put(method, serverCommand);
 
             // We want to be able invoke with an instance
             if (!isStatic) {

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

@@ -1,8 +0,0 @@
-package net.citizensnpcs.command;
-
-import java.lang.annotation.Retention;
-import java.lang.annotation.RetentionPolicy;
-
-@Retention(RetentionPolicy.RUNTIME)
-public @interface ServerCommand {
-}

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

@@ -6,7 +6,6 @@ import net.citizensnpcs.api.npc.NPC;
 import net.citizensnpcs.command.Command;
 import net.citizensnpcs.command.CommandContext;
 import net.citizensnpcs.command.Requirements;
-import net.citizensnpcs.command.ServerCommand;
 import net.citizensnpcs.command.exception.CommandException;
 import net.citizensnpcs.util.Messaging;
 import net.citizensnpcs.util.StringHelper;
@@ -22,7 +21,6 @@ public class AdminCommands {
     }
 
     @Command(aliases = { "citizens" }, desc = "Show basic plugin information", max = 0, permission = "admin")
-    @ServerCommand
     public void citizens(CommandContext args, CommandSender player, NPC npc) {
         Messaging.send(player,
                 "          " + StringHelper.wrapHeader("<e>Citizens v" + plugin.getDescription().getVersion()));
@@ -39,7 +37,6 @@ public class AdminCommands {
             min = 1,
             max = 1,
             permission = "admin")
-    @ServerCommand
     public void reload(CommandContext args, CommandSender sender, NPC npc) throws CommandException {
         Messaging.send(sender, "<e>Reloading Citizens...");
         try {
@@ -59,7 +56,6 @@ public class AdminCommands {
             min = 1,
             max = 1,
             permission = "admin")
-    @ServerCommand
     public void save(CommandContext args, CommandSender sender, NPC npc) {
         Messaging.send(sender, "<e>Saving Citizens...");
         plugin.save();

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

@@ -10,7 +10,6 @@ import net.citizensnpcs.api.npc.NPC;
 import net.citizensnpcs.command.Command;
 import net.citizensnpcs.command.CommandContext;
 import net.citizensnpcs.command.Requirements;
-import net.citizensnpcs.command.ServerCommand;
 import net.citizensnpcs.command.exception.CommandException;
 import net.citizensnpcs.util.Paginator;
 
@@ -33,7 +32,6 @@ public class HelpCommands {
             max = 2,
             permission = "help")
     @Requirements
-    @ServerCommand
     public void citizensHelp(CommandContext args, CommandSender sender, NPC npc) throws CommandException {
         int page = args.argsLength() == 2 ? args.getInteger(1) : 1;
         Paginator paginator = new Paginator().header("Citizens Help");
@@ -70,7 +68,6 @@ public class HelpCommands {
             max = 2,
             permission = "script.help")
     @Requirements
-    @ServerCommand
     public void scriptHelp(CommandContext args, CommandSender sender, NPC npc) throws CommandException {
         int page = args.argsLength() == 2 ? args.getInteger(1) : 1;
         Paginator paginator = new Paginator().header("Script Help");
@@ -89,7 +86,6 @@ public class HelpCommands {
             max = 2,
             permission = "npc.help")
     @Requirements
-    @ServerCommand
     public void npcHelp(CommandContext args, CommandSender sender, NPC npc) throws CommandException {
         int page = args.argsLength() == 2 ? args.getInteger(1) : 1;
         Paginator paginator = new Paginator().header("NPC Help");

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

@@ -16,7 +16,6 @@ import net.citizensnpcs.api.trait.trait.Spawned;
 import net.citizensnpcs.command.Command;
 import net.citizensnpcs.command.CommandContext;
 import net.citizensnpcs.command.Requirements;
-import net.citizensnpcs.command.ServerCommand;
 import net.citizensnpcs.command.exception.CommandException;
 import net.citizensnpcs.command.exception.NoPermissionsException;
 import net.citizensnpcs.npc.NPCSelector;
@@ -65,7 +64,7 @@ public class NPCCommands {
             permission = "npc.age")
     @Requirements(selected = true, ownership = true, types = { EntityType.CHICKEN, EntityType.COW, EntityType.OCELOT,
             EntityType.PIG, EntityType.SHEEP, EntityType.VILLAGER, EntityType.WOLF })
-    public void age(CommandContext args, Player player, NPC npc) throws CommandException {
+    public void age(CommandContext args, CommandSender sender, NPC npc) throws CommandException {
         Age trait = npc.getTrait(Age.class);
 
         if (args.argsLength() > 1) {
@@ -85,19 +84,19 @@ public class NPCCommands {
             }
 
             trait.setAge(age);
-            Messaging.send(player, StringHelper.wrap(npc.getName()) + " is now " + ageStr + ".");
+            Messaging.send(sender, StringHelper.wrap(npc.getName()) + " is now " + ageStr + ".");
         }
 
         if (args.hasFlag('l'))
-            Messaging.send(player, "<a>Age " + (trait.toggle() ? "locked" : "unlocked") + ".");
+            Messaging.send(sender, "<a>Age " + (trait.toggle() ? "locked" : "unlocked") + ".");
     }
 
     @Command(aliases = { "npc" }, usage = "behaviour [scripts]", desc = "Sets the behaviour of a NPC", modifiers = {
             "behaviour", "ai" }, min = 2, max = -1)
-    public void behaviour(CommandContext args, Player player, NPC npc) throws CommandException {
+    public void behaviour(CommandContext args, CommandSender sender, NPC npc) throws CommandException {
         Iterable<String> files = Splitter.on(',').split(args.getJoinedStrings(1, ','));
         npc.getTrait(Behaviour.class).addScripts(files);
-        player.sendMessage(ChatColor.GREEN + "Behaviours added.");
+        sender.sendMessage(ChatColor.GREEN + "Behaviours added.");
     }
 
     @Command(
@@ -107,7 +106,7 @@ public class NPCCommands {
             modifiers = { "character" },
             min = 2,
             max = 2)
-    public void character(CommandContext args, Player player, NPC npc) throws CommandException {
+    public void character(CommandContext args, CommandSender sender, NPC npc) throws CommandException {
         String name = args.getString(1).toLowerCase();
         Character character = characterManager.getCharacter(name);
 
@@ -115,16 +114,16 @@ public class NPCCommands {
             throw new CommandException("The character '" + args.getString(1) + "' does not exist.");
         if (npc.getCharacter() != null && npc.getCharacter().getName().equalsIgnoreCase(character.getName()))
             throw new CommandException("The NPC already has the character '" + name + "'.");
-        if (!player.hasPermission("citizens.npc.character." + character.getName())
+        if (!sender.hasPermission("citizens.npc.character." + character.getName())
-                && !player.hasPermission("citizens.npc.character.*") && !player.hasPermission("citizens.admin"))
+                && !sender.hasPermission("citizens.npc.character.*") && !sender.hasPermission("citizens.admin"))
             throw new NoPermissionsException();
 
         EntityType type = EntityType.valueOf(npc.getTrait(MobType.class).getType());
         if (!character.getValidTypes().isEmpty() && !character.getValidTypes().contains(type)) {
-            Messaging.sendError(player, "This NPC cannot be given the character '" + character.getName() + "'.");
+            Messaging.sendError(sender, "This NPC cannot be given the character '" + character.getName() + "'.");
             return;
         }
-        Messaging.send(player, StringHelper.wrap(npc.getName() + "'s") + " character is now " + StringHelper.wrap(name)
+        Messaging.send(sender, StringHelper.wrap(npc.getName() + "'s") + " character is now " + StringHelper.wrap(name)
                 + ".");
         npc.setCharacter(character);
     }
@@ -219,10 +218,10 @@ public class NPCCommands {
             min = 1,
             max = 1,
             permission = "npc.despawn")
-    public void despawn(CommandContext args, Player player, NPC npc) {
+    public void despawn(CommandContext args, CommandSender sender, NPC npc) {
         npc.getTrait(Spawned.class).setSpawned(false);
         npc.despawn();
-        Messaging.send(player, ChatColor.GREEN + "You despawned " + StringHelper.wrap(npc.getName()) + ".");
+        Messaging.send(sender, ChatColor.GREEN + "You despawned " + StringHelper.wrap(npc.getName()) + ".");
     }
 
     @Command(
@@ -235,7 +234,6 @@ public class NPCCommands {
             max = 2,
             permission = "npc.list")
     @Requirements
-    @ServerCommand
     public void list(CommandContext args, CommandSender sender, NPC npc) throws CommandException {
         List<NPC> npcs = new ArrayList<NPC>();
 
@@ -302,19 +300,19 @@ public class NPCCommands {
             min = 1,
             max = 1,
             permission = "npc.lookclose")
-    public void lookClose(CommandContext args, Player player, NPC npc) {
+    public void lookClose(CommandContext args, CommandSender sender, NPC npc) {
         String msg = StringHelper.wrap(npc.getName()) + " will "
                 + (npc.getTrait(LookClose.class).toggle() ? "now rotate" : "no longer rotate");
-        Messaging.send(player, msg + " when a player is nearby.");
+        Messaging.send(sender, msg + " when a player is nearby.");
     }
 
     @Command(aliases = { "npc" }, desc = "Show basic NPC information", max = 0)
-    public void npc(CommandContext args, Player player, NPC npc) {
+    public void npc(CommandContext args, CommandSender sender, NPC npc) {
-        Messaging.send(player, StringHelper.wrapHeader(npc.getName()));
+        Messaging.send(sender, StringHelper.wrapHeader(npc.getName()));
-        Messaging.send(player, "    <a>ID: <e>" + npc.getId());
+        Messaging.send(sender, "    <a>ID: <e>" + npc.getId());
-        Messaging.send(player, "    <a>Character: <e>"
+        Messaging.send(sender, "    <a>Character: <e>"
                 + (npc.getCharacter() != null ? npc.getCharacter().getName() : "None"));
-        Messaging.send(player, "    <a>Type: <e>" + npc.getTrait(MobType.class).getType());
+        Messaging.send(sender, "    <a>Type: <e>" + npc.getTrait(MobType.class).getType());
     }
 
     @Command(
@@ -325,7 +323,6 @@ public class NPCCommands {
             min = 1,
             max = 2,
             permission = "npc.owner")
-    @ServerCommand
     public void owner(CommandContext args, CommandSender sender, NPC npc) throws CommandException {
         if (args.argsLength() == 1) {
             Messaging.send(sender, StringHelper.wrap(npc.getName() + "'s Owner: ")
@@ -348,7 +345,6 @@ public class NPCCommands {
             min = 1,
             max = 1,
             permission = "npc.power")
-    @ServerCommand
     @Requirements(selected = true, ownership = true, types = { EntityType.CREEPER })
     public void power(CommandContext args, CommandSender sender, NPC npc) {
         String msg = StringHelper.wrap(npc.getName()) + " will "
@@ -364,7 +360,6 @@ public class NPCCommands {
             min = 2,
             max = 2,
             permission = "npc.profession")
-    @ServerCommand
     @Requirements(selected = true, ownership = true, types = { EntityType.VILLAGER })
     public void profession(CommandContext args, CommandSender sender, NPC npc) throws CommandException {
         String profession = args.getString(1);
@@ -387,7 +382,6 @@ public class NPCCommands {
             min = 1,
             max = 2)
     @Requirements
-    @ServerCommand
     public void remove(CommandContext args, CommandSender sender, NPC npc) throws CommandException {
         if (args.argsLength() == 2) {
             if (!args.getString(1).equalsIgnoreCase("all"))
@@ -419,7 +413,6 @@ public class NPCCommands {
             min = 2,
             max = 2,
             permission = "npc.rename")
-    @ServerCommand
     public void rename(CommandContext args, CommandSender sender, NPC npc) {
         String oldName = npc.getName();
         String newName = args.getString(1);
@@ -441,7 +434,6 @@ public class NPCCommands {
             max = 2,
             permission = "npc.select")
     @Requirements(ownership = true)
-    @ServerCommand
     public void select(CommandContext args, CommandSender sender, NPC npc) throws CommandException {
         NPC toSelect = npcRegistry.getNPC(args.getInteger(1));
         if (toSelect == null || !toSelect.getTrait(Spawned.class).shouldSpawn())
@@ -486,7 +478,6 @@ public class NPCCommands {
             min = 1,
             max = 1,
             permission = "npc.controllable")
-    @ServerCommand
     public void controllable(CommandContext args, CommandSender sender, NPC npc) {
         boolean enabled = npc.getTrait(Controllable.class).toggle();
         if (enabled) {
@@ -494,7 +485,6 @@ public class NPCCommands {
         } else {
             Messaging.send(sender, StringHelper.wrap(npc.getName()) + " can no longer be controlled.");
         }
-
     }
 
     @Command(

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

@@ -10,7 +10,6 @@ import net.citizensnpcs.api.scripting.Script;
 import net.citizensnpcs.api.scripting.ScriptFactory;
 import net.citizensnpcs.command.Command;
 import net.citizensnpcs.command.CommandContext;
-import net.citizensnpcs.command.ServerCommand;
 import net.citizensnpcs.command.exception.CommandException;
 import net.citizensnpcs.util.Messaging;
 
@@ -33,7 +32,6 @@ public class ScriptCommands {
             min = 2,
             max = 2,
             permission = "script.compile")
-    @ServerCommand
     public void runScript(final CommandContext args, final CommandSender sender, NPC npc) throws CommandException {
         File file = new File(plugin.getDataFolder(), args.getString(1));
         if (!file.exists())
@@ -42,8 +40,8 @@ public class ScriptCommands {
             @Override
             public void onScriptCompiled(ScriptFactory script) {
                 Script s = script.newInstance();
-                if (args.hasValueFlag("i")) {
+                if (args.hasValueFlag("methods")) {
-                    for (String m : Splitter.on(',').split(args.getFlag("i"))) {
+                    for (String m : Splitter.on(',').split(args.getFlag("methods"))) {
                         s.invoke(m, new Object[] {});
                     }
                 }

src/main/java/net/citizensnpcs/npc/CitizensNPC.java

@@ -31,6 +31,7 @@ public abstract class CitizensNPC extends AbstractNPC {
     protected CitizensNPC(int id, String name) {
         super(id, name);
         traitManager = (CitizensTraitManager) CitizensAPI.getTraitManager();
+        // TODO: remove this dependency
     }
 
     @Override

src/main/java/net/citizensnpcs/npc/CitizensNPCRegistry.java

@@ -40,10 +40,10 @@ import net.citizensnpcs.npc.entity.CitizensWolfNPC;
 import net.citizensnpcs.npc.entity.CitizensZombieNPC;
 import net.citizensnpcs.util.ByIdArray;
 
+import org.apache.commons.lang.Validate;
 import org.bukkit.craftbukkit.entity.CraftEntity;
 import org.bukkit.entity.Entity;
 import org.bukkit.entity.EntityType;
-import org.bukkit.entity.LivingEntity;
 
 public class CitizensNPCRegistry implements NPCRegistry {
     private final ByIdArray<NPC> npcs = new ByIdArray<NPC>();
@@ -113,12 +113,9 @@ public class CitizensNPCRegistry implements NPCRegistry {
 
     @Override
     public NPC getNPC(Entity entity) {
-        if (!(entity instanceof LivingEntity))
+        Validate.notNull(entity);
-            return null;
         net.minecraft.server.Entity handle = ((CraftEntity) entity).getHandle();
-        if (handle instanceof NPCHandle)
+        return handle instanceof NPCHandle ? ((NPCHandle) handle).getNPC() : null;
-            return ((NPCHandle) handle).getNPC();
-        return null;
     }
 
     @Override

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

@@ -18,7 +18,7 @@ public class Age extends Trait implements Runnable, Toggleable {
 
     @Override
     public void load(DataKey key) throws NPCLoadException {
-        if (!(npc.getBukkitEntity() instanceof Ageable))
+        if (npc.isSpawned() && !(npc.getBukkitEntity() instanceof Ageable))
             throw new NPCLoadException("NPC must be ageable");
         age = key.getInt("age");
         locked = key.getBoolean("locked");

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

@@ -17,7 +17,7 @@ public class Powered extends Trait implements Toggleable {
 
     @Override
     public void load(DataKey key) throws NPCLoadException {
-        if (!(npc.getBukkitEntity() instanceof Creeper))
+        if (npc.isSpawned() && !(npc.getBukkitEntity() instanceof Creeper))
             throw new NPCLoadException("NPC must be a creeper");
         powered = key.getBoolean("");
     }

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

@@ -21,7 +21,7 @@ public class Saddle extends Trait implements Toggleable, Listener {
 
     @Override
     public void load(DataKey key) throws NPCLoadException {
-        if (!(npc.getBukkitEntity() instanceof Pig))
+        if (npc.isSpawned() && !(npc.getBukkitEntity() instanceof Pig))
             throw new NPCLoadException("NPC must be a pig to have this trait");
         saddle = key.getBoolean("");
     }

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

@@ -22,7 +22,7 @@ public class WoolColor extends Trait implements Listener {
 
     @Override
     public void load(DataKey key) throws NPCLoadException {
-        if (!(npc.getBukkitEntity() instanceof Sheep))
+        if (npc.isSpawned() && !(npc.getBukkitEntity() instanceof Sheep))
             throw new NPCLoadException("NPC must be a sheep");
         try {
             color = DyeColor.valueOf(key.getString(""));