Fix NPC iteration in certain cases

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

@@ -588,8 +588,9 @@ public class NPCCommands {
         List<NPC> npcs = new ArrayList<NPC>();
 
         if (args.hasFlag('a')) {
-            for (NPC add : npcRegistry)
+            for (NPC add : npcRegistry) {
                 npcs.add(add);
+            }
         } else if (args.getValueFlags().size() == 0 && sender instanceof Player) {
             for (NPC add : npcRegistry) {
                 if (!npcs.contains(add) && add.getTrait(Owner.class).isOwnedBy(sender))

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

@@ -26,6 +26,10 @@ public class ByIdArray<T> implements Iterable<T> {
 
     public int add(T t) {
         int index = 0;
+        if (elementData[0] == null) {
+            put(index, t);
+            return index;
+        }
         while (elementData[index++] != null) {
             if (index >= elementData.length) {
                 ensureCapacity(elementData.length + 1);
@@ -81,10 +85,12 @@ public class ByIdArray<T> implements Iterable<T> {
         if (t == null)
             throw new IllegalArgumentException("can't insert a null object");
         ++modCount;
-        if (index > highest)
+        if (index > highest) {
             highest = index;
-        if (index < lowest)
+        }
+        if (index < lowest) {
             lowest = index;
+        }
 
         ensureCapacity(index + 2);
 
@@ -94,28 +100,34 @@ public class ByIdArray<T> implements Iterable<T> {
 
     private void recalcHighest() {
         highest = elementData.length - 1;
-        while (highest != 0 && elementData[highest--] == null)
+        while (highest != 0 && elementData[--highest] == null) {
             ;
         }
+    }
 
     private void recalcLowest() {
         lowest = 0;
-        while (elementData.length > lowest && elementData[lowest++] == null)
+        while (elementData.length > lowest && elementData[lowest++] == null) {
             ;
         }
+    }
 
     public T remove(int index) {
-        if (index > elementData.length || elementData[index] == null)
+        if (index > elementData.length || elementData[index] == null) {
             return null;
+        }
         @SuppressWarnings("unchecked")
         T prev = (T) elementData[index];
         elementData[index] = null;
         --size;
         ++modCount;
-        if (index >= highest)
+
+        if (index >= highest) {
             recalcHighest();
-        if (index <= lowest)
+        }
+        if (index <= lowest) {
             recalcLowest();
+        }
         return prev;
     }
 
@@ -132,13 +144,17 @@ public class ByIdArray<T> implements Iterable<T> {
     private class Itr implements Iterator<T> {
         private int expected = ByIdArray.this.modCount;
         private int idx;
-        {
+
+        private Itr() {
             if (size > 0) {
-                if (highest == Integer.MIN_VALUE || highest >= elementData.length || elementData[highest] == null)
+                if (lowest > highest || highest == Integer.MIN_VALUE || highest >= elementData.length
+                        || elementData[highest] == null) {
                     recalcHighest();
-                if (lowest >= elementData.length || elementData[lowest] == null)
+                }
+                if (lowest > highest || lowest >= elementData.length || elementData[lowest] == null) {
                     recalcLowest();
-                idx = lowest - 1;
+                }
+                idx = lowest;
             }
         }
 
@@ -147,7 +163,7 @@ public class ByIdArray<T> implements Iterable<T> {
             if (modCount != expected) {
                 throw new ConcurrentModificationException();
             }
-            return size > 0 && highest > idx;
+            return size > 0 && highest >= idx;
         }
 
         @Override
@@ -157,12 +173,12 @@ public class ByIdArray<T> implements Iterable<T> {
                 throw new ConcurrentModificationException();
             if (idx > highest)
                 throw new NoSuchElementException();
-            do
-                idx++;
-            while (idx != highest + 1 && elementData[idx] == null);
             T next = (T) elementData[idx];
             if (next == null)
                 throw new NoSuchElementException();
+            do {
+                idx++;
+            } while (idx != highest + 1 && elementData[idx] == null);
             return next;
         }
 

src/test/java/net/citizensnpcs/util/ByIdArrayTest.java

@@ -0,0 +1,66 @@
+package net.citizensnpcs.util;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+
+import org.junit.Test;
+
+import com.google.common.collect.Iterables;
+
+public class ByIdArrayTest {
+    private void assertSize(ByIdArray<?> array, int size) {
+        assertThat(array.size(), is(size));
+        assertThat(Iterables.size(array), is(size));
+    }
+
+    @Test
+    public void testBeyondCapacity() {
+        ByIdArray<Integer> array = ByIdArray.create();
+        array.put(1000, 1);
+        assertThat(array.contains(1000), is(true));
+        assertSize(array, 1);
+    }
+
+    @Test
+    public void testClear() {
+        ByIdArray<Integer> array = ByIdArray.create();
+        array.put(0, 1);
+        array.put(1, 2);
+        array.clear();
+        assertSize(array, 0);
+    }
+
+    @Test
+    public void testContains() {
+        ByIdArray<Integer> array = ByIdArray.create();
+        array.put(1, 1);
+        assertThat(array.contains(1), is(true));
+    }
+
+    @Test
+    public void testInsertion() {
+        ByIdArray<Integer> array = ByIdArray.create();
+        array.add(1);
+        array.add(2);
+        assertSize(array, 2);
+    }
+
+    @Test
+    public void testPut() {
+        ByIdArray<Integer> array = ByIdArray.create();
+        array.put(50, 1);
+        array.put(20, 2);
+        assertSize(array, 2);
+        assertThat(array.get(20), is(2));
+        assertThat(array.get(50), is(1));
+    }
+
+    @Test
+    public void testRemoval() {
+        ByIdArray<Integer> array = ByIdArray.create();
+        array.put(0, 1);
+        array.put(1, 2);
+        array.remove(1);
+        assertSize(array, 1);
+    }
+}