fix annoying spawning save bug

TODO

@@ -1,5 +1,6 @@
 -Citizens2 TODO List-
 
+-Add proper color parsing for names (separate it into a trait?)
 -Properly implement commands
 -Finish saving
 -Finish pathfinding API

src/net/citizensnpcs/Citizens.java

@@ -9,6 +9,7 @@ import net.citizensnpcs.api.npc.NPC;
 import net.citizensnpcs.api.npc.trait.Character;
 import net.citizensnpcs.api.npc.trait.Trait;
 import net.citizensnpcs.api.npc.trait.trait.SpawnLocation;
+import net.citizensnpcs.npc.CitizensNPC;
 import net.citizensnpcs.npc.CitizensNPCManager;
 import net.citizensnpcs.npc.trait.CitizensCharacterManager;
 import net.citizensnpcs.npc.trait.CitizensTraitManager;
@@ -18,6 +19,7 @@ import net.citizensnpcs.util.ByIdArray;
 import net.citizensnpcs.util.Messaging;
 
 import org.bukkit.Bukkit;
+import org.bukkit.ChatColor;
 import org.bukkit.command.Command;
 import org.bukkit.command.CommandSender;
 import org.bukkit.entity.Player;
@@ -80,7 +82,7 @@ public class Citizens extends JavaPlugin {
     @Override
     public boolean onCommand(CommandSender sender, Command cmd, String cmdName, String[] args) {
         if (args[0].equals("spawn")) {
-            NPC npc = npcManager.createNPC("aPunch");
+            NPC npc = npcManager.createNPC(ChatColor.GREEN + "aPunch");
             npc.spawn(((Player) sender).getLocation());
         } else if (args[0].equals("despawn")) {
             for (NPC npc : npcManager.getSpawnedNPCs())
@@ -89,9 +91,10 @@ public class Citizens extends JavaPlugin {
         return true;
     }
 
-    // TODO possibly separate this out some more
+    // TODO separate this out some more
     private void setupNPCs() throws NPCLoadException {
         traitManager.registerTrait("location", SpawnLocation.class);
+
         for (DataKey key : saves.getKey("npc").getIntegerSubKeys()) {
             int id = Integer.parseInt(key.name());
             if (!key.keyExists("name"))
@@ -131,7 +134,8 @@ public class Citizens extends JavaPlugin {
         for (NPC npc : npcManager.getAllNPCs()) {
             DataKey root = saves.getKey("npc." + npc.getId());
             root.setString("name", npc.getFullName());
-            root.setBoolean("spawned", !npc.getBukkitEntity().isDead());
+            if (root.getBoolean("spawned"))
+                root.setBoolean("spawned", !npc.getBukkitEntity().isDead());
 
             // Save the character if it exists
             if (npc.getCharacter() != null) {
@@ -140,7 +144,7 @@ public class Citizens extends JavaPlugin {
             }
 
             // Save all existing traits
-            for (Trait trait : npc.getTraits())
+            for (Trait trait : ((CitizensNPC) npc).getTraits())
                 trait.save(root.getRelative(trait.getName()));
         }
         saves.save();

src/net/citizensnpcs/npc/CitizensNPC.java

@@ -87,11 +87,6 @@ public class CitizensNPC implements NPC {
         return t != null ? clazz.cast(t) : null;
     }
 
-    @Override
-    public Iterable<Trait> getTraits() {
-        return traits.values();
-    }
-
     @Override
     public boolean hasTrait(Class<? extends Trait> trait) {
         return traits.containsKey(trait);
@@ -153,7 +148,8 @@ public class CitizensNPC implements NPC {
         Bukkit.getPluginManager().callEvent(new NPCDespawnEvent(this));
 
         manager.despawn(this);
-        mcEntity.die();
+        getHandle().die();
+
         spawned = false;
     }
 
@@ -166,7 +162,11 @@ public class CitizensNPC implements NPC {
 
     @Override
     public Entity getBukkitEntity() {
-        return mcEntity.getBukkitEntity();
+        return getHandle().getBukkitEntity();
+    }
+
+    public Iterable<Trait> getTraits() {
+        return traits.values();
     }
 
     public CraftNPC getHandle() {

src/net/citizensnpcs/npc/trait/CitizensCharacterManager.java

@@ -1,47 +1,49 @@
 package net.citizensnpcs.npc.trait;
 
-import java.util.Collection;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.Map;
-import java.util.Set;
 
+import net.citizensnpcs.api.Factory;
 import net.citizensnpcs.api.npc.trait.Character;
 import net.citizensnpcs.api.npc.trait.CharacterManager;
 
 public class CitizensCharacterManager implements CharacterManager {
-    private final Map<String, Class<? extends Character>> registered = new HashMap<String, Class<? extends Character>>();
-    private final Set<Character> characters = new HashSet<Character>();
+    private final Map<String, Factory<? extends Character>> registered = new HashMap<String, Factory<? extends Character>>();
 
     @Override
     public Character getCharacter(String name) {
-        if (registered.get(name) == null) {
+        if (registered.get(name) == null)
             return null;
+        return registered.get(name).create();
+    }
+
+    @Override
+    public void registerCharacter(String name, Class<? extends Character> clazz) {
+        registerCharacterWithFactory(name, new DefaultCharacterFactory(clazz));
+    }
+
+    @Override
+    public void registerCharacterWithFactory(String name, Factory<? extends Character> factory) {
+        if (registered.get(name) != null)
+            throw new IllegalArgumentException("A character factory for the character '" + name
+                    + "' has already been registered.");
+        registered.put(name, factory);
+    }
+
+    private static class DefaultCharacterFactory implements Factory<Character> {
+        private final Class<? extends Character> clazz;
+
+        private DefaultCharacterFactory(Class<? extends Character> clazz) {
+            this.clazz = clazz;
         }
-        for (Character character : characters) {
-            if (character.getName().equals(name)) {
-                return character;
+
+        @Override
+        public Character create() {
+            try {
+                return clazz.newInstance();
+            } catch (Exception ex) {
+                return null;
             }
         }
-        return null;
-    }
-
-    @Override
-    public void registerCharacter(Class<? extends Character> character) {
-        if (registered.containsValue(character)) {
-            return;
-        }
-        try {
-            Character register = character.newInstance();
-            registered.put(register.getName(), character);
-            characters.add(register);
-        } catch (Exception ex) {
-            ex.printStackTrace();
-        }
-    }
-
-    @Override
-    public Collection<Character> getRegisteredCharacters() {
-        return characters;
     }
 }

src/net/citizensnpcs/npc/trait/CitizensTraitManager.java

@@ -1,15 +1,14 @@
 package net.citizensnpcs.npc.trait;
 
+import java.util.HashMap;
 import java.util.Map;
 
 import net.citizensnpcs.api.Factory;
 import net.citizensnpcs.api.npc.trait.Trait;
 import net.citizensnpcs.api.npc.trait.TraitManager;
 
-import com.google.common.collect.Maps;
-
 public class CitizensTraitManager implements TraitManager {
-    private final Map<String, Factory<? extends Trait>> registered = Maps.newHashMap();
+    private final Map<String, Factory<? extends Trait>> registered = new HashMap<String, Factory<? extends Trait>>();
 
     @Override
     public Trait getTrait(String name) {
@@ -20,20 +19,21 @@ public class CitizensTraitManager implements TraitManager {
 
     @Override
     public void registerTrait(String name, Class<? extends Trait> clazz) {
-        registerTraitWithFactory(name, new ReflectionFactory(clazz));
+        registerTraitWithFactory(name, new DefaultTraitFactory(clazz));
     }
 
     @Override
     public void registerTraitWithFactory(String name, Factory<? extends Trait> factory) {
         if (registered.get(name) != null)
-            throw new IllegalArgumentException("Trait factory already registered.");
+            throw new IllegalArgumentException("A trait factory for the trait '" + name
+                    + "' has already been registered.");
         registered.put(name, factory);
     }
 
-    private static class ReflectionFactory implements Factory<Trait> {
+    private static class DefaultTraitFactory implements Factory<Trait> {
         private final Class<? extends Trait> clazz;
 
-        private ReflectionFactory(Class<? extends Trait> clazz) {
+        private DefaultTraitFactory(Class<? extends Trait> clazz) {
             this.clazz = clazz;
         }