added selection command, bug fixes

src/net/citizensnpcs/Citizens.java

@@ -191,11 +191,15 @@ public class Citizens extends JavaPlugin {
     }
 
     private void registerPermissions() {
+        // TODO There has to be a better way than this (maybe use Permission
+        // annotation to register permissions?)
         Map<String, Boolean> children = new HashMap<String, Boolean>();
         children.put("citizens.npc.create", true);
         children.put("citizens.npc.spawn", true);
         children.put("citizens.npc.despawn", true);
         children.put("citizens.npc.select", true);
+        children.put("citizens.npc.tp", true);
+        children.put("citizens.npc.tphere", true);
 
         Permission perm = new Permission("citizens.*", PermissionDefault.OP, children);
         getServer().getPluginManager().addPermission(perm);

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

@@ -1,5 +1,6 @@
 package net.citizensnpcs.command.command;
 
+import net.citizensnpcs.Settings.Setting;
 import net.citizensnpcs.api.npc.NPC;
 import net.citizensnpcs.api.npc.trait.Character;
 import net.citizensnpcs.api.npc.trait.DefaultInstanceFactory;
@@ -63,6 +64,19 @@ public class NPCCommands {
             Messaging.send(player, successMsg);
     }
 
+    @Command(
+             aliases = { "npc" },
+             usage = "despawn",
+             desc = "Despawn an NPC",
+             modifiers = { "despawn" },
+             min = 1,
+             max = 1)
+    @Permission("npc.despawn")
+    public void despawnNPC(CommandContext args, Player player, NPC npc) {
+        npc.despawn();
+        Messaging.send(player, ChatColor.GREEN + "You despawned " + StringHelper.wrap(npc.getName()) + ".");
+    }
+
     @Command(
              aliases = { "npc" },
              usage = "spawn [id]",
@@ -71,7 +85,6 @@ public class NPCCommands {
              min = 2,
              max = 2)
     @Permission("npc.spawn")
-    @Requirements
     public void spawnNPC(CommandContext args, Player player, NPC npc) {
         CitizensNPC respawn = (CitizensNPC) npcManager.getNPC(args.getInteger(1));
         if (respawn == null) {
@@ -89,34 +102,34 @@ public class NPCCommands {
             Messaging.send(player, ChatColor.GREEN + "You respawned " + StringHelper.wrap(respawn.getName())
                     + " at your location.");
         } else
-            Messaging.sendError(player, respawn.getName()
+            Messaging
-                    + " is already spawned at another location. Use '/npc move' to teleport the NPC to your location.");
+                    .sendError(
+                            player,
+                            respawn.getName()
+                                    + " is already spawned at another location. Use '/npc tphere' to teleport the NPC to your location.");
     }
 
     @Command(
              aliases = { "npc" },
-             usage = "despawn",
+             usage = "select [id]",
-             desc = "Despawn an NPC",
+             desc = "Select an NPC",
-             modifiers = { "despawn" },
+             modifiers = { "select" },
-             min = 1,
+             min = 2,
-             max = 1)
+             max = 2)
-    @Permission("npc.despawn")
+    @Permission("npc.select")
-    public void despawnNPC(CommandContext args, Player player, NPC npc) {
+    @Requirements(ownership = true)
-        npc.despawn();
+    public void selectNPC(CommandContext args, Player player, NPC npc) {
-        Messaging.send(player, ChatColor.GREEN + "You despawned " + StringHelper.wrap(npc.getName()) + ".");
+        NPC toSelect = npcManager.getNPC(args.getInteger(1));
-    }
+        if (toSelect == null) {
-
+            Messaging.sendError(player, "No NPC with the ID '" + args.getInteger(1) + "' exists.");
-    @Command(
+            return;
-             aliases = { "npc" },
+        }
-             usage = "tp",
+        if (npc != null && toSelect.getId() == npc.getId()) {
-             desc = "Teleport to an NPC",
+            Messaging.sendError(player, "You already have that NPC selected.");
-             modifiers = { "tp", "teleport" },
+            return;
-             min = 1,
+        }
-             max = 1)
+        npcManager.selectNPC(player, toSelect);
-    @Permission("npc.tp")
+        Messaging.sendWithNPC(player, Setting.SELECTION_MESSAGE.getString(), toSelect);
-    public void teleportToNPC(CommandContext args, Player player, NPC npc) {
-        player.teleport(npc.getBukkitEntity(), TeleportCause.COMMAND);
-        Messaging.send(player, ChatColor.GREEN + "You teleported to " + StringHelper.wrap(npc.getName()) + ".");
     }
 
     @Command(
@@ -131,4 +144,17 @@ public class NPCCommands {
         npc.getBukkitEntity().teleport(player, TeleportCause.COMMAND);
         Messaging.send(player, StringHelper.wrap(npc.getName()) + " was teleported to your location.");
     }
+
+    @Command(
+             aliases = { "npc" },
+             usage = "tp",
+             desc = "Teleport to an NPC",
+             modifiers = { "tp", "teleport" },
+             min = 1,
+             max = 1)
+    @Permission("npc.tp")
+    public void teleportToNPC(CommandContext args, Player player, NPC npc) {
+        player.teleport(npc.getBukkitEntity(), TeleportCause.COMMAND);
+        Messaging.send(player, ChatColor.GREEN + "You teleported to " + StringHelper.wrap(npc.getName()) + ".");
+    }
 }

src/net/citizensnpcs/npc/CitizensNPCManager.java

@@ -31,9 +31,7 @@ import com.google.common.collect.HashMultimap;
 import com.google.common.collect.SetMultimap;
 
 public class CitizensNPCManager implements NPCManager {
-    // TODO: merge spawned and byID
+    private final ByIdArray<NPC> npcs = new ByIdArray<NPC>();
-    private final ByIdArray<NPC> spawned = new ByIdArray<NPC>();
-    private final ByIdArray<NPC> byID = new ByIdArray<NPC>();
     private final SetMultimap<Integer, String> selected = HashMultimap.create();
     private final Storage saves;
 
@@ -52,11 +50,12 @@ public class CitizensNPCManager implements NPCManager {
     }
 
     public NPC createNPC(int id, String name, Character character) {
-        if (byID.contains(id))
+        if (npcs.contains(id))
-            throw new IllegalArgumentException("id already taken");
+            throw new IllegalArgumentException("An NPC already has the ID '" + id + "'.");
+
         CitizensNPC npc = new CitizensNPC(this, id, name);
         npc.setCharacter(character);
-        byID.put(npc.getId(), npc);
+        npcs.put(npc.getId(), npc);
         return npc;
     }
 
@@ -66,7 +65,6 @@ public class CitizensNPCManager implements NPCManager {
         npc.getTrait(SpawnLocation.class).setLocation(loc);
 
         selected.removeAll(npc.getId());
-        spawned.remove(mcEntity.getPlayer().getEntityId());
         for (Player player : Bukkit.getOnlinePlayers())
             ((CraftPlayer) player).getHandle().netServerHandler.sendPacket(new Packet29DestroyEntity(mcEntity.id));
         mcEntity.die();
@@ -74,7 +72,7 @@ public class CitizensNPCManager implements NPCManager {
 
     @Override
     public Iterator<NPC> iterator() {
-        return byID.iterator();
+        return npcs.iterator();
     }
 
     private MinecraftServer getMinecraftServer(Server server) {
@@ -83,22 +81,23 @@ public class CitizensNPCManager implements NPCManager {
 
     @Override
     public NPC getNPC(Entity entity) {
-        return spawned.get(entity.getEntityId());
+        for (NPC npc : npcs)
+            if (npc.isSpawned() && npc.getBukkitEntity().getEntityId() == entity.getEntityId())
+                return npc;
+        return null;
     }
 
     @Override
     public NPC getNPC(int id) {
-        return byID.get(id);
+        return npcs.get(id);
     }
 
     @Override
     public Collection<NPC> getNPCs(Class<? extends Character> character) {
         List<NPC> npcs = new ArrayList<NPC>();
-        for (NPC npc : this) {
+        for (NPC npc : this)
-            if (npc.getCharacter() != null && npc.getCharacter().getClass().equals(character)) {
+            if (npc.getCharacter() != null && npc.getCharacter().getClass().equals(character))
                 npcs.add(npc);
-            }
-        }
         return npcs;
     }
 
@@ -115,13 +114,13 @@ public class CitizensNPCManager implements NPCManager {
 
     @Override
     public boolean isNPC(Entity entity) {
-        return spawned.contains(entity.getEntityId());
+        return getNPC(entity) != null;
     }
 
     public void remove(NPC npc) {
-        if (spawned.contains(npc.getBukkitEntity().getEntityId()))
+        if (npc.isSpawned())
             despawn(npc);
-        byID.remove(npc.getId());
+        npcs.remove(npc.getId());
         saves.getKey("npc").removeKey("" + npc.getId());
         selected.removeAll(npc.getId());
     }
@@ -134,16 +133,14 @@ public class CitizensNPCManager implements NPCManager {
         mcEntity.setPositionRotation(loc.getX(), loc.getY(), loc.getZ(), loc.getYaw(), loc.getPitch());
         ws.addEntity(mcEntity);
         ws.players.remove(mcEntity);
-
-        spawned.put(mcEntity.getPlayer().getEntityId(), npc);
         return mcEntity;
     }
 
     public void selectNPC(Player player, NPC npc) {
         // Remove existing selection if any
-        NPC select = getSelectedNPC(player);
+        NPC existing = getSelectedNPC(player);
-        if (select != null)
+        if (existing != null)
-            selected.get(select.getId()).remove(player.getName());
+            selected.get(existing.getId()).remove(player.getName());
         selected.put(npc.getId(), player.getName());
     }