Cleanup, add() method in ByIdArray

src/net/citizensnpcs/Citizens.java

@@ -39,6 +39,18 @@ public class Citizens extends JavaPlugin {
         CitizensAPI.setTraitManager(traitManager);
     }
 
+    @Override
+    public boolean onCommand(CommandSender sender, Command cmd, String cmdName, String[] args) {
+        if (args[0].equals("spawn")) {
+            NPC npc = npcManager.createNPC(ChatColor.GREEN + "aPunch");
+            npc.spawn(((Player) sender).getLocation());
+        } else if (args[0].equals("despawn")) {
+            for (NPC npc : npcManager.getSpawnedNPCs())
+                npc.despawn();
+        }
+        return true;
+    }
+
     @Override
     public void onDisable() {
         config.save();
@@ -79,16 +91,24 @@ public class Citizens extends JavaPlugin {
         }
     }
 
-    @Override
+    private void saveNPCs() {
-    public boolean onCommand(CommandSender sender, Command cmd, String cmdName, String[] args) {
+        for (NPC npc : npcManager.getAllNPCs()) {
-        if (args[0].equals("spawn")) {
+            DataKey root = saves.getKey("npc." + npc.getId());
-            NPC npc = npcManager.createNPC(ChatColor.GREEN + "aPunch");
+            root.setString("name", npc.getFullName());
-            npc.spawn(((Player) sender).getLocation());
+            if (root.getBoolean("spawned"))
-        } else if (args[0].equals("despawn")) {
+                root.setBoolean("spawned", !npc.getBukkitEntity().isDead());
-            for (NPC npc : npcManager.getSpawnedNPCs())
+
-                npc.despawn();
+            // Save the character if it exists
+            if (npc.getCharacter() != null) {
+                root.setString("character", npc.getCharacter().getName());
+                npc.getCharacter().save(root.getRelative(npc.getCharacter().getName()));
+            }
+
+            // Save all existing traits
+            for (Trait trait : ((CitizensNPC) npc).getTraits())
+                trait.save(root.getRelative(trait.getName()));
         }
-        return true;
+        saves.save();
     }
 
     // TODO separate this out some more
@@ -129,24 +149,4 @@ public class Citizens extends JavaPlugin {
         Messaging.log("Loaded " + ((ByIdArray<NPC>) npcManager.getAllNPCs()).size() + " NPCs ("
                 + ((ByIdArray<NPC>) npcManager.getSpawnedNPCs()).size() + " spawned).");
     }
-
-    private void saveNPCs() {
-        for (NPC npc : npcManager.getAllNPCs()) {
-            DataKey root = saves.getKey("npc." + npc.getId());
-            root.setString("name", npc.getFullName());
-            if (root.getBoolean("spawned"))
-                root.setBoolean("spawned", !npc.getBukkitEntity().isDead());
-
-            // Save the character if it exists
-            if (npc.getCharacter() != null) {
-                root.setString("character", npc.getCharacter().getName());
-                npc.getCharacter().save(root.getRelative(npc.getCharacter().getName()));
-            }
-
-            // Save all existing traits
-            for (Trait trait : ((CitizensNPC) npc).getTraits())
-                trait.save(root.getRelative(trait.getName()));
-        }
-        saves.save();
-    }
 }

src/net/citizensnpcs/EventListen.java

@@ -26,35 +26,6 @@ public class EventListen implements Listener {
         this.manager = manager;
     }
 
-    /*
-     * Entity events
-     */
-    @EventHandler
-    public void onEntityDamage(EntityDamageEvent event) {
-        if (!manager.isNPC(event.getEntity()))
-            return;
-
-        event.setCancelled(true); // TODO: implement damage handlers
-        if (event instanceof EntityDamageByEntityEvent) {
-            EntityDamageByEntityEvent e = (EntityDamageByEntityEvent) event;
-            if (e.getDamager() instanceof Player) {
-                NPC npc = manager.getNPC(event.getEntity());
-                if (npc.getCharacter() != null)
-                    npc.getCharacter().onLeftClick(npc, (Player) e.getDamager());
-            }
-        }
-    }
-
-    @EventHandler
-    public void onEntityTarget(EntityTargetEvent event) {
-        if (event.isCancelled() || !manager.isNPC(event.getEntity()) || !(event.getTarget() instanceof Player))
-            return;
-
-        NPC npc = manager.getNPC(event.getEntity());
-        if (npc.getCharacter() != null)
-            npc.getCharacter().onRightClick(npc, (Player) event.getTarget());
-    }
-
     /*
      * World events
      */
@@ -83,4 +54,33 @@ public class EventListen implements Listener {
             }
         }
     }
+
+    /*
+     * Entity events
+     */
+    @EventHandler
+    public void onEntityDamage(EntityDamageEvent event) {
+        if (!manager.isNPC(event.getEntity()))
+            return;
+
+        event.setCancelled(true); // TODO: implement damage handlers
+        if (event instanceof EntityDamageByEntityEvent) {
+            EntityDamageByEntityEvent e = (EntityDamageByEntityEvent) event;
+            if (e.getDamager() instanceof Player) {
+                NPC npc = manager.getNPC(event.getEntity());
+                if (npc.getCharacter() != null)
+                    npc.getCharacter().onLeftClick(npc, (Player) e.getDamager());
+            }
+        }
+    }
+
+    @EventHandler
+    public void onEntityTarget(EntityTargetEvent event) {
+        if (event.isCancelled() || !manager.isNPC(event.getEntity()) || !(event.getTarget() instanceof Player))
+            return;
+
+        NPC npc = manager.getNPC(event.getEntity());
+        if (npc.getCharacter() != null)
+            npc.getCharacter().onRightClick(npc, (Player) event.getTarget());
+    }
 }

src/net/citizensnpcs/Settings.java

@@ -7,53 +7,9 @@ import net.citizensnpcs.storage.flatfile.YamlStorage;
 import net.citizensnpcs.util.Messaging;
 
 public class Settings {
-    public enum Setting {
-        DEBUG_MODE("general.debug-mode", false);
-
-        private String path;
-        private Object value;
-
-        Setting(String path, Object value) {
-            this.path = path;
-            this.value = value;
-        }
-
-        public String getPath() {
-            return path;
-        }
-
-        public int getInt() {
-            return (Integer) value;
-        }
-
-        public double getDouble() {
-            return (Double) value;
-        }
-
-        public long getLong() {
-            return (Long) value;
-        }
-
-        public boolean getBoolean() {
-            return (Boolean) value;
-        }
-
-        public String getString() {
-            return value.toString();
-        }
-
-        private Object get() {
-            return value;
-        }
-
-        private void set(Object value) {
-            this.value = value;
-        }
-    }
-
     private final YamlStorage config;
-    private final DataKey root;
 
+    private final DataKey root;
     public Settings(Citizens plugin) {
         config = new YamlStorage(plugin.getDataFolder() + File.separator + "config.yml");
         root = config.getKey("");
@@ -73,4 +29,48 @@ public class Settings {
     public void save() {
         config.save();
     }
+
+    public enum Setting {
+        DEBUG_MODE("general.debug-mode", false);
+
+        private String path;
+        private Object value;
+
+        Setting(String path, Object value) {
+            this.path = path;
+            this.value = value;
+        }
+
+        private Object get() {
+            return value;
+        }
+
+        public boolean getBoolean() {
+            return (Boolean) value;
+        }
+
+        public double getDouble() {
+            return (Double) value;
+        }
+
+        public int getInt() {
+            return (Integer) value;
+        }
+
+        public long getLong() {
+            return (Long) value;
+        }
+
+        public String getPath() {
+            return path;
+        }
+
+        public String getString() {
+            return value.toString();
+        }
+
+        private void set(Object value) {
+            this.value = value;
+        }
+    }
 }

src/net/citizensnpcs/npc/CitizensNPC.java

@@ -28,13 +28,6 @@ public class CitizensNPC implements NPC {
     private boolean spawned;
     private final CitizensNPCManager manager;
 
-    public CitizensNPC(String name, Character character) {
-        this.name = name;
-        this.character = character;
-        manager = (CitizensNPCManager) CitizensAPI.getNPCManager();
-        id = manager.getUniqueID();
-    }
-
     public CitizensNPC(int id, String name, Character character) {
         this.name = name;
         this.character = character;
@@ -42,19 +35,11 @@ public class CitizensNPC implements NPC {
         this.id = id;
     }
 
-    @Override
+    public CitizensNPC(String name, Character character) {
-    public String getFullName() {
-        return name;
-    }
-
-    @Override
-    public String getName() {
-        return ChatColor.stripColor(name);
-    }
-
-    @Override
-    public void setName(String name) {
         this.name = name;
+        this.character = character;
+        manager = (CitizensNPCManager) CitizensAPI.getNPCManager();
+        id = manager.getUniqueID();
     }
 
     @Override
@@ -65,16 +50,50 @@ public class CitizensNPC implements NPC {
             Messaging.debug("The NPC already has the trait '" + getTrait(trait.getClass()).getName() + "'.");
     }
 
+    @Override
+    public void despawn() {
+        if (!isSpawned()) {
+            Messaging.debug("The NPC is already despawned.");
+            return;
+        }
+
+        Bukkit.getPluginManager().callEvent(new NPCDespawnEvent(this));
+
+        manager.despawn(this);
+        getHandle().die();
+
+        spawned = false;
+    }
+
+    @Override
+    public Entity getBukkitEntity() {
+        return getHandle().getBukkitEntity();
+    }
+
     @Override
     public Character getCharacter() {
         return character;
     }
 
+    @Override
+    public String getFullName() {
+        return name;
+    }
+
+    public CraftNPC getHandle() {
+        return mcEntity;
+    }
+
     @Override
     public int getId() {
         return id;
     }
 
+    @Override
+    public String getName() {
+        return ChatColor.stripColor(name);
+    }
+
     @Override
     public Navigator getNavigator() {
         // TODO add default navigator
@@ -87,11 +106,28 @@ 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);
     }
 
+    @Override
+    public boolean isSpawned() {
+        return spawned;
+    }
+
+    @Override
+    public void remove() {
+        if (isSpawned())
+            despawn();
+        manager.remove(this);
+    }
+
     @Override
     public void removeTrait(Class<? extends Trait> trait) {
         if (!hasTrait(trait)) {
@@ -111,8 +147,8 @@ public class CitizensNPC implements NPC {
     }
 
     @Override
-    public boolean isSpawned() {
+    public void setName(String name) {
-        return spawned;
+        this.name = name;
     }
 
     @Override
@@ -137,40 +173,4 @@ public class CitizensNPC implements NPC {
 
         spawned = true;
     }
-
-    @Override
-    public void despawn() {
-        if (!isSpawned()) {
-            Messaging.debug("The NPC is already despawned.");
-            return;
-        }
-
-        Bukkit.getPluginManager().callEvent(new NPCDespawnEvent(this));
-
-        manager.despawn(this);
-        getHandle().die();
-
-        spawned = false;
-    }
-
-    @Override
-    public void remove() {
-        if (isSpawned())
-            despawn();
-        manager.remove(this);
-    }
-
-    @Override
-    public Entity getBukkitEntity() {
-        return getHandle().getBukkitEntity();
-    }
-
-    @Override
-    public Iterable<Trait> getTraits() {
-        return traits.values();
-    }
-
-    public CraftNPC getHandle() {
-        return mcEntity;
-    }
 }

src/net/citizensnpcs/npc/CitizensNPCManager.java

@@ -42,9 +42,25 @@ public class CitizensNPCManager implements NPCManager {
         return npc;
     }
 
+    public void despawn(NPC npc) {
+        CraftNPC mcEntity = ((CitizensNPC) npc).getHandle();
+        for (Player player : Bukkit.getOnlinePlayers()) {
+            ((CraftPlayer) player).getHandle().netServerHandler.sendPacket(new Packet29DestroyEntity(mcEntity.id));
+        }
+        Location loc = npc.getBukkitEntity().getLocation();
+        getWorldServer(loc.getWorld()).removeEntity(mcEntity);
+        npc.getTrait(SpawnLocation.class).setLocation(loc);
+
+        spawned.remove(mcEntity.getPlayer().getEntityId());
+    }
+
     @Override
-    public NPC getNPC(int id) {
+    public Iterable<NPC> getAllNPCs() {
-        return byID.get(id);
+        return byID;
+    }
+
+    private MinecraftServer getMinecraftServer(Server server) {
+        return ((CraftServer) server).getServer();
     }
 
     @Override
@@ -53,13 +69,8 @@ public class CitizensNPCManager implements NPCManager {
     }
 
     @Override
-    public Iterable<NPC> getAllNPCs() {
+    public NPC getNPC(int id) {
-        return byID;
+        return byID.get(id);
-    }
-
-    @Override
-    public Iterable<NPC> getSpawnedNPCs() {
-        return spawned;
     }
 
     @Override
@@ -73,8 +84,8 @@ public class CitizensNPCManager implements NPCManager {
     }
 
     @Override
-    public boolean isNPC(Entity entity) {
+    public Iterable<NPC> getSpawnedNPCs() {
-        return spawned.contains(entity.getEntityId());
+        return spawned;
     }
 
     public int getUniqueID() {
@@ -87,6 +98,21 @@ public class CitizensNPCManager implements NPCManager {
         return count;
     }
 
+    private WorldServer getWorldServer(World world) {
+        return ((CraftWorld) world).getHandle();
+    }
+
+    @Override
+    public boolean isNPC(Entity entity) {
+        return spawned.contains(entity.getEntityId());
+    }
+
+    public void remove(NPC npc) {
+        if (spawned.contains(npc.getBukkitEntity().getEntityId()))
+            despawn(npc);
+        byID.remove(npc.getId());
+    }
+
     public CraftNPC spawn(NPC npc, Location loc) {
         WorldServer ws = getWorldServer(loc.getWorld());
         CraftNPC mcEntity = new CraftNPC(getMinecraftServer(ws.getServer()), ws, npc.getFullName(),
@@ -99,30 +125,4 @@ public class CitizensNPCManager implements NPCManager {
         spawned.put(mcEntity.getPlayer().getEntityId(), npc);
         return mcEntity;
     }
-
-    public void despawn(NPC npc) {
-        CraftNPC mcEntity = ((CitizensNPC) npc).getHandle();
-        for (Player player : Bukkit.getOnlinePlayers()) {
-            ((CraftPlayer) player).getHandle().netServerHandler.sendPacket(new Packet29DestroyEntity(mcEntity.id));
-        }
-        Location loc = npc.getBukkitEntity().getLocation();
-        getWorldServer(loc.getWorld()).removeEntity(mcEntity);
-        npc.getTrait(SpawnLocation.class).setLocation(loc);
-
-        spawned.remove(mcEntity.getPlayer().getEntityId());
-    }
-
-    public void remove(NPC npc) {
-        if (spawned.contains(npc.getBukkitEntity().getEntityId()))
-            despawn(npc);
-        byID.remove(npc.getId());
-    }
-
-    private WorldServer getWorldServer(World world) {
-        return ((CraftWorld) world).getHandle();
-    }
-
-    private MinecraftServer getMinecraftServer(Server server) {
-        return ((CraftServer) server).getServer();
-    }
 }

src/net/citizensnpcs/resources/lib/NPCNetHandler.java

@@ -28,50 +28,10 @@ public class NPCNetHandler extends NetServerHandler {
     public void a() {
     }
 
-    @Override
-    public void sendMessage(String string) {
-    }
-
-    @Override
-    public void a(String string, Object[] objects) {
-    }
-
     @Override
     public void a(Packet packet) {
     }
 
-    @Override
-    public void sendPacket(Packet packet) {
-    }
-
-    @Override
-    public void a(Packet3Chat packet) {
-    }
-
-    @Override
-    public void a(Packet10Flying packet) {
-    }
-
-    @Override
-    public void a(Packet14BlockDig packet) {
-    }
-
-    @Override
-    public void a(Packet15Place packet) {
-    }
-
-    @Override
-    public void a(Packet16BlockItemSwitch packet) {
-    }
-
-    @Override
-    public void a(Packet28EntityVelocity packet) {
-    }
-
-    @Override
-    public void a(Packet51MapChunk packet) {
-    }
-
     @Override
     public void a(Packet101CloseWindow packet) {
     }
@@ -85,10 +45,50 @@ public class NPCNetHandler extends NetServerHandler {
     }
 
     @Override
-    public void a(Packet255KickDisconnect packet) {
+    public void a(Packet10Flying packet) {
     }
 
     @Override
     public void a(Packet130UpdateSign packet) {
     }
+
+    @Override
+    public void a(Packet14BlockDig packet) {
+    }
+
+    @Override
+    public void a(Packet15Place packet) {
+    }
+
+    @Override
+    public void a(Packet16BlockItemSwitch packet) {
+    }
+
+    @Override
+    public void a(Packet255KickDisconnect packet) {
+    }
+
+    @Override
+    public void a(Packet28EntityVelocity packet) {
+    }
+
+    @Override
+    public void a(Packet3Chat packet) {
+    }
+
+    @Override
+    public void a(Packet51MapChunk packet) {
+    }
+
+    @Override
+    public void a(String string, Object[] objects) {
+    }
+
+    @Override
+    public void sendMessage(String string) {
+    }
+
+    @Override
+    public void sendPacket(Packet packet) {
+    }
 }

src/net/citizensnpcs/resources/lib/NPCNetworkManager.java

@@ -21,22 +21,18 @@ public class NPCNetworkManager extends NetworkManager {
         }
     }
 
+    @Override
+    public void a() {
+    }
+
     @Override
     public void a(NetHandler netHandler) {
     }
 
-    @Override
-    public void queue(Packet packet) {
-    }
-
     @Override
     public void a(String s, Object... objects) {
     }
 
-    @Override
-    public void a() {
-    }
-
     @Override
     public void b() {
     }
@@ -49,4 +45,8 @@ public class NPCNetworkManager extends NetworkManager {
     public int e() {
         return 0;
     }
+
+    @Override
+    public void queue(Packet packet) {
+    }
 }

src/net/citizensnpcs/storage/Storage.java

@@ -4,9 +4,9 @@ import net.citizensnpcs.api.DataKey;
 
 public interface Storage {
 
+    public DataKey getKey(String root);
+
     public void load();
 
     public void save();
-
-    public DataKey getKey(String root);
 }

src/net/citizensnpcs/storage/database/DatabaseStorage.java

@@ -7,6 +7,12 @@ import net.citizensnpcs.storage.Storage;
 
 public class DatabaseStorage implements Storage {
 
+    @Override
+    public DataKey getKey(String root) {
+        // TODO Auto-generated method stub
+        return null;
+    }
+
     @Override
     public void load() {
         // TODO Auto-generated method stub
@@ -19,12 +25,6 @@ public class DatabaseStorage implements Storage {
 
     }
 
-    @Override
-    public DataKey getKey(String root) {
-        // TODO Auto-generated method stub
-        return null;
-    }
-
     public class DatabaseKey extends DataKey {
 
         @Override
@@ -63,6 +63,12 @@ public class DatabaseStorage implements Storage {
             return 0;
         }
 
+        @Override
+        public Object getRaw(String key) {
+            // TODO Auto-generated method stub
+            return null;
+        }
+
         @Override
         public DataKey getRelative(String relative) {
             // TODO Auto-generated method stub
@@ -123,22 +129,16 @@ public class DatabaseStorage implements Storage {
 
         }
 
-        @Override
-        public void setString(String key, String value) {
-            // TODO Auto-generated method stub
-
-        }
-
-        @Override
-        public Object getRaw(String key) {
-            // TODO Auto-generated method stub
-            return null;
-        }
-
         @Override
         public void setRaw(String path, Object value) {
             // TODO Auto-generated method stub
 
         }
+
+        @Override
+        public void setString(String key, String value) {
+            // TODO Auto-generated method stub
+
+        }
     }
 }

src/net/citizensnpcs/test/ByIdArrayTest.java

@@ -17,6 +17,8 @@ public class ByIdArrayTest {
         test.remove(1000); // try illegal remove
         test.clear();
         assert (test.size() == 0);
+        test.add("one");
+        assert (test.get(0).equals("one"));
     }
 
     @Test

src/net/citizensnpcs/util/ByIdArray.java

@@ -6,6 +6,7 @@ import java.util.NoSuchElementException;
 
 public class ByIdArray<T> implements Iterable<T> {
     private Object[] elementData;
+    private final int capacity;
     private int size;
     private int modCount;
     private int highest = Integer.MIN_VALUE;
@@ -18,15 +19,40 @@ public class ByIdArray<T> implements Iterable<T> {
     public ByIdArray(int capacity) {
         if (capacity < 0)
             throw new IllegalArgumentException("illegal capacity");
+        this.capacity = capacity;
         elementData = new Object[capacity];
     }
 
+    /**
+     * Uses a linear search to insert an object at the first available space.
+     * 
+     * @param t
+     *            The object to add
+     * @return The index the object was added at
+     */
+    public int add(T t) {
+        int index = 0;
+        while (elementData[index++] != null) {
+            if (index >= elementData.length) {
+                System.out.println(elementData.length + " " + index);
+                ensureCapacity(elementData.length + 1);
+                index = elementData.length - 1;
+            }
+        }
+        put(index, t);
+        return index;
+    }
+
     public void clear() {
         modCount = highest = size = lowest = 0;
-        elementData = new Object[50];
+        elementData = new Object[capacity];
     }
 
-    private void ensureCapacity(int minCapacity) { // from ArrayList
+    public boolean contains(int index) {
+        return elementData.length > index && elementData[index] != null;
+    }
+
+    public void ensureCapacity(int minCapacity) { // from ArrayList
         int oldCapacity = elementData.length;
         if (minCapacity > oldCapacity) {
             int newCapacity = (oldCapacity * 3) / 2 + 1;
@@ -45,11 +71,6 @@ public class ByIdArray<T> implements Iterable<T> {
         --size;
     }
 
-    private void recalcLowest() {
-        while (elementData.length > lowest && highest > lowest && elementData[lowest++] == null)
-            ;
-    }
-
     @SuppressWarnings("unchecked")
     public T get(int index) {
         if (index > elementData.length)
@@ -88,7 +109,7 @@ public class ByIdArray<T> implements Iterable<T> {
 
     public void put(int index, T t) {
         if (t == null)
-            throw new IllegalArgumentException("The variable 't' cannot be null.");
+            throw new IllegalArgumentException("can't insert a null object");
         ++modCount;
         if (index > highest)
             highest = index;
@@ -106,6 +127,11 @@ public class ByIdArray<T> implements Iterable<T> {
             ;
     }
 
+    private void recalcLowest() {
+        while (elementData.length > lowest && highest > lowest && elementData[lowest++] == null)
+            ;
+    }
+
     public T remove(int index) {
         if (index > elementData.length || elementData[index] == null)
             return null;
@@ -130,8 +156,4 @@ public class ByIdArray<T> implements Iterable<T> {
         if (elementData.length > highest)
             elementData = Arrays.copyOf(elementData, highest + 1);
     }
-
-    public boolean contains(int index) {
-        return elementData.length > index && elementData[index] != null;
-    }
 }

src/net/citizensnpcs/util/Messaging.java

@@ -8,6 +8,11 @@ import org.bukkit.Bukkit;
 
 public class Messaging {
 
+    public static void debug(Object msg) {
+        if (Setting.DEBUG_MODE.getBoolean())
+            log(msg);
+    }
+
     public static void log(Level level, Object msg) {
         Bukkit.getLogger().log(level, "[Citizens] " + msg);
     }
@@ -15,9 +20,4 @@ public class Messaging {
     public static void log(Object msg) {
         log(Level.INFO, msg);
     }
-
-    public static void debug(Object msg) {
-        if (Setting.DEBUG_MODE.getBoolean())
-            log(msg);
-    }
 }