Package refactor, getNPC lookup optimisation, fix ByIdArray iterator remove

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

@@ -8,7 +8,6 @@ import java.util.logging.Level;
 import net.citizensnpcs.Settings.Setting;
 import net.citizensnpcs.api.CitizensAPI;
 import net.citizensnpcs.api.event.CitizensReloadEvent;
-import net.citizensnpcs.api.exception.NPCException;
 import net.citizensnpcs.api.exception.NPCLoadException;
 import net.citizensnpcs.api.npc.NPC;
 import net.citizensnpcs.api.scripting.EventRegistrar;
@@ -124,10 +123,7 @@ public class Citizens extends JavaPlugin {
         // Don't bother with this part if MC versions are not compatible
         if (compatible) {
             save();
-            while (npcManager.iterator().hasNext()) {
-                npcManager.iterator().next().despawn();
-                npcManager.iterator().remove();
-            } // TODO: clean up
+            npcManager.safeRemove();
             npcManager = null;
             getServer().getScheduler().cancelTasks(this);
         }
@@ -292,11 +288,7 @@ public class Citizens extends JavaPlugin {
                 }
             }
             NPC npc = npcManager.createNPC(type, id, key.getString("name"), null);
-            try {
-                ((CitizensNPC) npc).load(key);
-            } catch (NPCException ex) {
-                Messaging.log(ex.getMessage());
-            }
+            ((CitizensNPC) npc).load(key);
 
             ++created;
             if (npc.isSpawned())

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

@@ -132,30 +132,34 @@ public abstract class CitizensNPC extends AbstractNPC {
         return getHandle() != null;
     }
 
-    public void load(DataKey root) throws NPCLoadException {
+    public void load(DataKey root) {
         Character character = CitizensAPI.getCharacterManager().getCharacter(root.getString("character"));
 
         // Load the character if it exists
         if (character != null) {
-            character.load(root.getRelative("characters." + character.getName()));
+            try {
+                character.load(root.getRelative("characters." + character.getName()));
+            } catch (NPCLoadException e) {
+                Messaging.severe("Unable to load character " + character.getName() + ".");
+                e.printStackTrace();
+            }
             setCharacter(character);
         }
 
         // Load traits
         for (DataKey traitKey : root.getRelative("traits").getSubKeys()) {
             Trait trait = traitManager.getTrait(traitKey.name(), this);
-            if (trait == null)
-                throw new NPCLoadException("No trait with the name '" + traitKey.name()
-                        + "' exists. Was it registered properly?");
+            if (trait == null) {
+                Messaging.severe("Skipping trait with name '" + traitKey.name() + "' while loading '" + getId()
+                        + "': trait does not exist. Was it registered properly or has the name changed?");
+                continue;
+            }
             addTrait(trait);
             try {
                 getTrait(trait.getClass()).load(traitKey);
             } catch (Exception ex) {
-                Bukkit.getLogger().log(
-                        Level.SEVERE,
-                        "[Citizens] The trait '" + traitKey.name()
-                                + "' failed to load properly for the NPC with the ID '" + getId() + "'. "
-                                + ex.getMessage());
+                Messaging.log("[Citizens] The trait '" + traitKey.name()
+                        + "' failed to load properly for the NPC with the ID '" + getId() + "'. " + ex.getMessage());
                 ex.printStackTrace();
             }
         }

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

@@ -7,7 +7,6 @@ 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;
@@ -77,11 +76,6 @@ public class CitizensNPCManager implements NPCManager {
         net.minecraft.server.Entity handle = ((CraftEntity) entity).getHandle();
         if (handle instanceof NPCHandle)
             return ((NPCHandle) handle).getNPC();
-        for (NPC npc : npcs) { // fall back to linear search
-            if (npc.isSpawned() && npc.getBukkitEntity().getEntityId() == entity.getEntityId()) {
-                return npc;
-            }
-        }
         return null;
     }
 
@@ -117,8 +111,9 @@ public class CitizensNPCManager implements NPCManager {
     }
 
     public void removeAll() {
-        while (iterator().hasNext())
-            iterator().next().remove();
+        Iterator<NPC> itr = iterator();
+        while (itr.hasNext())
+            itr.next().remove();
     }
 
     private void removeMetadata(NPC npc) {
@@ -131,13 +126,14 @@ public class CitizensNPCManager implements NPCManager {
         }
     }
 
-    public void safeRemove() throws NPCLoadException {
+    public void safeRemove() {
         // Destroy all NPCs everywhere besides storage
-        while (iterator().hasNext()) {
-            NPC npc = iterator().next();
+        Iterator<NPC> itr = this.iterator();
+        while (itr.hasNext()) {
+            NPC npc = itr.next();
+            itr.remove();
             removeMetadata(npc);
             npc.despawn();
-            iterator().remove();
         }
     }
 

src/main/java/net/citizensnpcs/trait/text/PageChangePrompt.java

@@ -1,6 +1,5 @@
-package net.citizensnpcs.trait.text.prompt;
+package net.citizensnpcs.trait.text;
 
-import net.citizensnpcs.trait.text.Text;
 import net.citizensnpcs.util.Messaging;
 import net.citizensnpcs.util.StringHelper;
 

src/main/java/net/citizensnpcs/trait/text/StartPrompt.java

@@ -1,6 +1,5 @@
-package net.citizensnpcs.trait.text.prompt;
+package net.citizensnpcs.trait.text;
 
-import net.citizensnpcs.trait.text.Text;
 import net.citizensnpcs.util.Messaging;
 import net.citizensnpcs.util.StringHelper;
 

src/main/java/net/citizensnpcs/trait/text/Text.java

@@ -15,7 +15,6 @@ import net.citizensnpcs.api.util.DataKey;
 import net.citizensnpcs.editor.Editor;
 import net.citizensnpcs.npc.CitizensNPC;
 import net.citizensnpcs.trait.Toggleable;
-import net.citizensnpcs.trait.text.prompt.StartPrompt;
 import net.citizensnpcs.util.Messaging;
 import net.citizensnpcs.util.Paginator;
 import net.minecraft.server.EntityHuman;

src/main/java/net/citizensnpcs/trait/text/TextAddPrompt.java

@@ -1,6 +1,5 @@
-package net.citizensnpcs.trait.text.prompt;
+package net.citizensnpcs.trait.text;
 
-import net.citizensnpcs.trait.text.Text;
 import net.citizensnpcs.util.Messaging;
 import net.citizensnpcs.util.StringHelper;
 

src/main/java/net/citizensnpcs/trait/text/TextEditPrompt.java

@@ -1,6 +1,5 @@
-package net.citizensnpcs.trait.text.prompt;
+package net.citizensnpcs.trait.text;
 
-import net.citizensnpcs.trait.text.Text;
 import net.citizensnpcs.util.Messaging;
 
 import org.bukkit.ChatColor;

src/main/java/net/citizensnpcs/trait/text/TextEditStartPrompt.java

@@ -1,6 +1,5 @@
-package net.citizensnpcs.trait.text.prompt;
+package net.citizensnpcs.trait.text;
 
-import net.citizensnpcs.trait.text.Text;
 import net.citizensnpcs.util.Messaging;
 import net.citizensnpcs.util.StringHelper;
 

src/main/java/net/citizensnpcs/trait/text/TextRemovePrompt.java

@@ -1,6 +1,5 @@
-package net.citizensnpcs.trait.text.prompt;
+package net.citizensnpcs.trait.text;
 
-import net.citizensnpcs.trait.text.Text;
 import net.citizensnpcs.util.Messaging;
 import net.citizensnpcs.util.StringHelper;
 

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

@@ -56,10 +56,11 @@ public class ByIdArray<T> implements Iterable<T> {
     }
 
     private void fastRemove(int index) {
-        if (index == lowest)
-            recalcLowest();
+        ++modCount;
         if (index == highest)
             recalcHighest();
+        if (index == lowest)
+            recalcLowest();
         elementData[index] = null;
         --size;
     }
@@ -99,7 +100,7 @@ public class ByIdArray<T> implements Iterable<T> {
 
     private void recalcLowest() {
         lowest = 0;
-        while (elementData.length > lowest && highest > lowest && elementData[lowest++] == null)
+        while (elementData.length > lowest && elementData[lowest++] == null)
             ;
     }
 
@@ -107,10 +108,10 @@ public class ByIdArray<T> implements Iterable<T> {
         if (index > elementData.length || elementData[index] == null)
             return null;
         ++modCount;
-        if (index == lowest)
-            recalcLowest();
         if (index == highest)
             recalcHighest();
+        if (index == lowest)
+            recalcLowest();
         @SuppressWarnings("unchecked")
         T prev = (T) elementData[index];
         elementData[index] = null;
@@ -139,7 +140,11 @@ public class ByIdArray<T> implements Iterable<T> {
                     recalcLowest();
                 idx = lowest;
                 if (elementData[lowest] == null) {
-                    Messaging.log("lowest is still null!");
+                    idx = 0;
+                    while (elementData.length > idx && elementData[idx++] == null)
+                        ;
+                    if (elementData[idx] == null)
+                        Messaging.debug("idx is still null!");
                 }
             }
         }
@@ -171,6 +176,11 @@ public class ByIdArray<T> implements Iterable<T> {
                 throw new ConcurrentModificationException();
             fastRemove(idx);
             expected = modCount;
+            if (hasNext()) {
+                do
+                    idx++;
+                while (idx != highest + 1 && elementData[idx] == null);
+            }
         }
     }
 

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

@@ -26,7 +26,7 @@ public class Messaging {
     }
 
     public static void log(Object... msg) {
-        log(Level.INFO, SPACE.join(msg));
+        log(Level.INFO, msg);
     }
 
     public static void send(CommandSender sender, Object msg) {
@@ -51,4 +51,8 @@ public class Messaging {
 
         send(sender, send);
     }
+
+    public static void severe(Object... messages) {
+        log(Level.SEVERE, messages);
+    }
 }