Fix reload removing NPCs. This fixes CITIZENS-13.

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

@@ -36,7 +36,6 @@ import net.citizensnpcs.util.Messaging;
 import net.citizensnpcs.util.Metrics;
 import net.citizensnpcs.util.StringHelper;
 
-import org.bukkit.Bukkit;
 import org.bukkit.ChatColor;
 import org.bukkit.command.Command;
 import org.bukkit.command.CommandSender;
@@ -170,13 +169,13 @@ public class Citizens extends JavaPlugin {
         // Register commands
         registerCommands();
 
-        Bukkit.getScheduler().scheduleSyncRepeatingTask(this, new NPCUpdater(npcManager), 0, 1);
+        getServer().getScheduler().scheduleSyncRepeatingTask(this, new NPCUpdater(npcManager), 0, 1);
 
         Messaging.log("v" + getDescription().getVersion() + " enabled.");
 
         // Setup NPCs after all plugins have been enabled (allows for multiworld
         // support and for NPCs to properly register external settings)
-        if (Bukkit.getScheduler().scheduleSyncDelayedTask(this, new Runnable() {
+        if (getServer().getScheduler().scheduleSyncDelayedTask(this, new Runnable() {
             @Override
             public void run() {
                 try {
@@ -228,8 +227,7 @@ public class Citizens extends JavaPlugin {
     public void reload() throws NPCLoadException {
         Editor.leaveAll();
         config.load();
-        saves.load();
+        npcManager.safeRemove();
-        npcManager.removeAll();
         setupNPCs();
 
         getServer().getPluginManager().callEvent(new CitizensReloadEvent());
@@ -237,13 +235,14 @@ public class Citizens extends JavaPlugin {
 
     public void save() {
         config.save();
-        for (NPC npc : npcManager) {
+        for (NPC npc : npcManager)
             ((CitizensNPC) npc).save(saves.getKey("npc." + npc.getId()));
-        }
+
         saves.save();
     }
 
     private void setupNPCs() throws NPCLoadException {
+        saves.load();
         int created = 0, spawned = 0;
         for (DataKey key : saves.getKey("npc").getIntegerSubKeys()) {
             int id = Integer.parseInt(key.name());

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

@@ -46,6 +46,7 @@ public class AdminCommands {
             plugin.reload();
             Messaging.send(sender, "<e>Citizens reloaded.");
         } catch (NPCLoadException ex) {
+            ex.printStackTrace();
             throw new CommandException("Error occured while reloading, see console.");
         }
     }

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

@@ -40,12 +40,12 @@ public class NPCCommands {
     }
 
     @Command(
-            aliases = { "npc" },
+             aliases = { "npc" },
-            usage = "character [character]",
+             usage = "character [character]",
-            desc = "Set the character of an NPC",
+             desc = "Set the character of an NPC",
-            modifiers = { "character" },
+             modifiers = { "character" },
-            min = 2,
+             min = 2,
-            max = 2)
+             max = 2)
     public void character(CommandContext args, Player player, NPC npc) throws CommandException {
         String name = args.getString(1).toLowerCase();
         Character character = characterManager.getCharacter(name);
@@ -56,19 +56,19 @@ public class NPCCommands {
         if (!player.hasPermission("citizens.npc.character." + character.getName())
                 && !player.hasPermission("citizens.npc.character.*") && !player.hasPermission("citizens.admin"))
             throw new NoPermissionsException();
-        Messaging.send(player,
+        Messaging.send(player, StringHelper.wrap(npc.getName() + "'s") + " character is now '"
-                StringHelper.wrap(npc.getName() + "'s") + " character is now '" + StringHelper.wrap(name) + "'.");
+                + StringHelper.wrap(name) + "'.");
         npc.setCharacter(character);
     }
 
     @Command(
-            aliases = { "npc" },
+             aliases = { "npc" },
-            usage = "create [name] (--type (type) --char (char))",
+             usage = "create [name] (--type (type) --char (char))",
-            desc = "Create a new NPC",
+             desc = "Create a new NPC",
-            modifiers = { "create" },
+             modifiers = { "create" },
-            min = 2,
+             min = 2,
-            max = 5,
+             max = 5,
-            permission = "npc.create")
+             permission = "npc.create")
     @Requirements
     public void create(CommandContext args, Player player, NPC npc) {
         String name = args.getString(1);
@@ -117,13 +117,13 @@ public class NPCCommands {
     }
 
     @Command(
-            aliases = { "npc" },
+             aliases = { "npc" },
-            usage = "despawn",
+             usage = "despawn",
-            desc = "Despawn an NPC",
+             desc = "Despawn an NPC",
-            modifiers = { "despawn" },
+             modifiers = { "despawn" },
-            min = 1,
+             min = 1,
-            max = 1,
+             max = 1,
-            permission = "npc.despawn")
+             permission = "npc.despawn")
     public void despawn(CommandContext args, Player player, NPC npc) {
         npc.getTrait(Spawned.class).setSpawned(false);
         npc.despawn();
@@ -131,14 +131,14 @@ public class NPCCommands {
     }
 
     @Command(
-            aliases = { "npc" },
+             aliases = { "npc" },
-            usage = "list (page) ((-a) --owner (owner) --type (type) --char (char))",
+             usage = "list (page) ((-a) --owner (owner) --type (type) --char (char))",
-            desc = "List NPCs",
+             desc = "List NPCs",
-            flags = "a",
+             flags = "a",
-            modifiers = { "list" },
+             modifiers = { "list" },
-            min = 1,
+             min = 1,
-            max = 2,
+             max = 2,
-            permission = "npc.list")
+             permission = "npc.list")
     @Requirements
     public void list(CommandContext args, Player player, NPC npc) throws CommandException {
         List<NPC> npcs = new ArrayList<NPC>();
@@ -203,13 +203,13 @@ public class NPCCommands {
     }
 
     @Command(
-            aliases = { "npc" },
+             aliases = { "npc" },
-            usage = "lookclose",
+             usage = "lookclose",
-            desc = "Toggle whether an NPC will look when a player is near",
+             desc = "Toggle whether an NPC will look when a player is near",
-            modifiers = { "lookclose", "look", "rotate" },
+             modifiers = { "lookclose", "look", "rotate" },
-            min = 1,
+             min = 1,
-            max = 1,
+             max = 1,
-            permission = "npc.lookclose")
+             permission = "npc.lookclose")
     public void lookClose(CommandContext args, Player player, NPC npc) {
         String msg = StringHelper.wrap(npc.getName()) + " will "
                 + (npc.getTrait(LookClose.class).toggle() ? "now rotate" : "no longer rotate");
@@ -226,13 +226,13 @@ public class NPCCommands {
     }
 
     @Command(
-            aliases = { "npc" },
+             aliases = { "npc" },
-            usage = "owner [name]",
+             usage = "owner [name]",
-            desc = "Set the owner of an NPC",
+             desc = "Set the owner of an NPC",
-            modifiers = { "owner" },
+             modifiers = { "owner" },
-            min = 2,
+             min = 2,
-            max = 2,
+             max = 2,
-            permission = "npc.owner")
+             permission = "npc.owner")
     public void owner(CommandContext args, Player player, NPC npc) throws CommandException {
         String name = args.getString(1);
         if (npc.getTrait(Owner.class).getOwner().equals(name))
@@ -243,12 +243,12 @@ public class NPCCommands {
     }
 
     @Command(
-            aliases = { "npc" },
+             aliases = { "npc" },
-            usage = "remove (all)",
+             usage = "remove (all)",
-            desc = "Remove an NPC",
+             desc = "Remove an NPC",
-            modifiers = { "remove" },
+             modifiers = { "remove" },
-            min = 1,
+             min = 1,
-            max = 2)
+             max = 2)
     @Requirements
     public void remove(CommandContext args, Player player, NPC npc) throws CommandException {
         if (args.argsLength() == 2) {
@@ -271,13 +271,13 @@ public class NPCCommands {
     }
 
     @Command(
-            aliases = { "npc" },
+             aliases = { "npc" },
-            usage = "rename [name]",
+             usage = "rename [name]",
-            desc = "Rename an NPC",
+             desc = "Rename an NPC",
-            modifiers = { "rename" },
+             modifiers = { "rename" },
-            min = 2,
+             min = 2,
-            max = 2,
+             max = 2,
-            permission = "npc.rename")
+             permission = "npc.rename")
     public void rename(CommandContext args, Player player, NPC npc) {
         String oldName = npc.getName();
         String newName = args.getString(1);
@@ -286,19 +286,18 @@ public class NPCCommands {
             newName = newName.substring(0, 15);
         }
         npc.setName(newName);
-        Messaging.send(player,
+        Messaging.send(player, ChatColor.GREEN + "You renamed " + StringHelper.wrap(oldName) + " to "
-                ChatColor.GREEN + "You renamed " + StringHelper.wrap(oldName) + " to " + StringHelper.wrap(newName)
+                + StringHelper.wrap(newName) + ".");
-                        + ".");
     }
 
     @Command(
-            aliases = { "npc" },
+             aliases = { "npc" },
-            usage = "select [id]",
+             usage = "select [id]",
-            desc = "Select an NPC with the given ID",
+             desc = "Select an NPC with the given ID",
-            modifiers = { "select" },
+             modifiers = { "select" },
-            min = 2,
+             min = 2,
-            max = 2,
+             max = 2,
-            permission = "npc.select")
+             permission = "npc.select")
     @Requirements(ownership = true)
     public void select(CommandContext args, Player player, NPC npc) throws CommandException {
         NPC toSelect = npcManager.getNPC(args.getInteger(1));
@@ -311,13 +310,13 @@ public class NPCCommands {
     }
 
     @Command(
-            aliases = { "npc" },
+             aliases = { "npc" },
-            usage = "spawn [id]",
+             usage = "spawn [id]",
-            desc = "Spawn an existing NPC",
+             desc = "Spawn an existing NPC",
-            modifiers = { "spawn" },
+             modifiers = { "spawn" },
-            min = 2,
+             min = 2,
-            max = 2,
+             max = 2,
-            permission = "npc.spawn")
+             permission = "npc.spawn")
     @Requirements
     public void spawn(CommandContext args, Player player, NPC npc) throws CommandException {
         NPC respawn = npcManager.getNPC(args.getInteger(1));
@@ -337,13 +336,13 @@ public class NPCCommands {
     }
 
     @Command(
-            aliases = { "npc" },
+             aliases = { "npc" },
-            usage = "tp",
+             usage = "tp",
-            desc = "Teleport to an NPC",
+             desc = "Teleport to an NPC",
-            modifiers = { "tp", "teleport" },
+             modifiers = { "tp", "teleport" },
-            min = 1,
+             min = 1,
-            max = 1,
+             max = 1,
-            permission = "npc.tp")
+             permission = "npc.tp")
     public void tp(CommandContext args, Player player, NPC npc) {
         // Spawn the NPC if it isn't spawned to prevent NPEs
         if (!npc.isSpawned())
@@ -353,13 +352,13 @@ public class NPCCommands {
     }
 
     @Command(
-            aliases = { "npc" },
+             aliases = { "npc" },
-            usage = "tphere",
+             usage = "tphere",
-            desc = "Teleport an NPC to your location",
+             desc = "Teleport an NPC to your location",
-            modifiers = { "tphere" },
+             modifiers = { "tphere" },
-            min = 1,
+             min = 1,
-            max = 1,
+             max = 1,
-            permission = "npc.tphere")
+             permission = "npc.tphere")
     public void tphere(CommandContext args, Player player, NPC npc) {
         // Spawn the NPC if it isn't spawned to prevent NPEs
         if (!npc.isSpawned())

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

@@ -7,6 +7,7 @@ import java.util.List;
 
 import net.citizensnpcs.Citizens;
 import net.citizensnpcs.api.event.NPCSelectEvent;
+import net.citizensnpcs.api.exception.NPCLoadException;
 import net.citizensnpcs.api.npc.NPC;
 import net.citizensnpcs.api.npc.NPCManager;
 import net.citizensnpcs.api.npc.character.Character;
@@ -51,9 +52,9 @@ public class CitizensNPCManager implements NPCManager {
         return createNPC(type, generateUniqueId(), name, character);
     }
 
-    public void despawn(NPC npc, boolean deselect) {
+    public void despawn(NPC npc, boolean keepSelected) {
         npc.getTrait(SpawnLocation.class).setLocation(npc.getBukkitEntity().getLocation());
-        if (!deselect)
+        if (!keepSelected)
             npc.removeMetadata("selectors", plugin);
         npc.getBukkitEntity().remove();
     }
@@ -100,13 +101,16 @@ public class CitizensNPCManager implements NPCManager {
     public void remove(NPC npc) {
         npcs.remove(npc.getId());
         saves.getKey("npc").removeKey(String.valueOf(npc.getId()));
+        removeMetadata(npc);
+    }
 
-        // Remove metadata from selectors
+    public void safeRemove() throws NPCLoadException {
-        if (npc.hasMetadata("selectors")) {
+        // Destroy all NPCs everywhere besides storage
-            for (MetadataValue value : npc.getMetadata("selectors"))
+        while (iterator().hasNext()) {
-                if (Bukkit.getPlayer(value.asString()) != null)
+            NPC npc = iterator().next();
-                    Bukkit.getPlayer(value.asString()).removeMetadata("selected", plugin);
+            removeMetadata(npc);
-            npc.removeMetadata("selectors", plugin);
+            npc.despawn();
+            iterator().remove();
         }
     }
 
@@ -115,7 +119,6 @@ public class CitizensNPCManager implements NPCManager {
             iterator().next().remove();
     }
 
-    // TODO: remove
     public void selectNPC(Player player, NPC npc) {
         // Remove existing selection if any
         if (player.hasMetadata("selected"))
@@ -130,4 +133,14 @@ public class CitizensNPCManager implements NPCManager {
         // Call selection event
         player.getServer().getPluginManager().callEvent(new NPCSelectEvent(npc, player));
     }
+
+    private void removeMetadata(NPC npc) {
+        // Remove metadata from selectors
+        if (npc.hasMetadata("selectors")) {
+            for (MetadataValue value : npc.getMetadata("selectors"))
+                if (Bukkit.getPlayer(value.asString()) != null)
+                    Bukkit.getPlayer(value.asString()).removeMetadata("selected", plugin);
+            npc.removeMetadata("selectors", plugin);
+        }
+    }
 }