Fix iterator remove() for NPCs

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

@@ -114,8 +114,9 @@ public class Citizens extends JavaPlugin implements CitizensPlugin {
             NPC npc = itr.next();
             try {
                 npc.despawn(DespawnReason.REMOVAL);
-                for (Trait trait : npc.getTraits())
+                for (Trait trait : npc.getTraits()) {
                     trait.onRemove();
+                }
             } catch (Throwable e) {
                 e.printStackTrace();
                 // ensure that all entities are despawned

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

@@ -158,6 +158,12 @@ public class ByIdArray<T> implements Iterable<T> {
             }
         }
 
+        private void advance() {
+            do {
+                idx++;
+            } while (idx != highest + 1 && elementData[idx] == null);
+        }
+
         @Override
         public boolean hasNext() {
             if (modCount != expected) {
@@ -176,9 +182,7 @@ public class ByIdArray<T> implements Iterable<T> {
             T next = (T) elementData[idx];
             if (next == null)
                 throw new NoSuchElementException();
-            do {
+            advance();
-                idx++;
-            } while (idx != highest + 1 && elementData[idx] == null);
             return next;
         }
 
@@ -186,8 +190,11 @@ public class ByIdArray<T> implements Iterable<T> {
         public void remove() {
             if (modCount != expected)
                 throw new ConcurrentModificationException();
+            if (elementData[idx] == null)
+                throw new NoSuchElementException();
             fastRemove(idx);
             expected = modCount;
+            advance();
         }
     }
 

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

@@ -3,6 +3,8 @@ package net.citizensnpcs.util;
 import static org.hamcrest.CoreMatchers.is;
 import static org.junit.Assert.assertThat;
 
+import java.util.Iterator;
+
 import org.junit.Test;
 
 import com.google.common.collect.Iterables;
@@ -45,6 +47,22 @@ public class ByIdArrayTest {
         assertSize(array, 2);
     }
 
+    @Test
+    public void testIteratorRemove() {
+        ByIdArray<Integer> array = ByIdArray.create();
+        array.put(10, 1);
+        array.put(20, 2);
+        array.put(30, 3);
+        Iterator<Integer> itr = array.iterator();
+        itr.next();
+        itr.remove();
+        itr.next();
+        assertSize(array, 2);
+        assertThat(array.contains(20), is(false));
+        assertThat(array.get(10), is(1));
+        assertThat(array.get(30), is(3));
+    }
+
     @Test
     public void testPut() {
         ByIdArray<Integer> array = ByIdArray.create();