Search code examples
javasystem-error

Removing an object is causing freezes


[For clarification, 'monsters' is a list of NPCs that when shot by an object 'bullet' are removed from the list. ]

Error at start:

W/System.err: java.util.ConcurrentModificationException
W/System.err:     at java.util.ArrayList$Itr.next(ArrayList.java:831)
W/System.err:     at com.companyname.product.GameplayScene.update(GameplayScene.java:113)
//...

I believe this means that it is trying to perform an action on something in the list that isn't there. So my question is could I please get some help identifying the point where an object is being called that isn't there, as I can't seem to find it. I know which class it is in as the first line references it for each error.

        for (Bullet bullet : player.getList())
            if (npcManager.bulletCollide(bullet)) { //checks for a collision between all npcs and all bullets
                 player.getList().remove(bullet);//npc is removed in method above

        }

       if (!gameOver) {
        npcManager.update(); //updates movement of npcs
        if (npcManager.playerCollideNPC(player)) {
            //end game
        }
    }

    for (RectNPC NPC : npcManager.getList())
        if (obstacleManager.NPCCollide(NPC)) { //checks collision between npcs and other objects
            //
        }

        int i = 0;
        //checks collisions between NPCs themselves
        while (i < (Constants.NUMBER_ENEMIES - 1)) {
            if (npcManager.getList().size() <= 1)
                return;
            if (Rect.intersects(npcManager.getList().get(i).getRectangle(), npcManager.getList().get(i + 1).getRectangle()))
                npcManager.getList().get(i).setRectangle(200);
            i += 1;
        }
    }

(Above) I imagine the issue will be with calling something in the update (shown above) where the object is no longer available, is there an issue with any of how I am removing the NPCs?

 public boolean bulletCollide(Bullet bullet) {
    for(RectNPC npc : monsters) {
        if(npc.bulletCollideNPC(bullet)) {
            monsters.remove(npc);
            monsters.add(0, new RectNPC(new Rect(100, 100, 200, 200), Color.rgb(255, 0, 0), 25));
            return true;
        }
    }
    return false;
}

(Above) Code where I am removing the NPC, I've included incase it is helpful - however it does do what it's meant to so I don't believe this is the problem.


My main question is, is there anything here that would be causing this error (the game stopping for a few frames and giving the error at the top) - or is there something/somewhere else in specific I should be looking at? Also I know some of my syntax isn't great, I apologise it's my first Java project.


Current Iterators:

for(Iterator<RectNPC> iterator = monsters.iterator(); iterator.hasNext();) {
        if(iterator.next().bulletCollideNPC(bullet)) {
            iterator.remove();
            monsters.add(0, new RectNPC(new Rect(100, 100, 200, 200), Color.rgb(255, 0, 0), 25));
            return true;
        }
    }

and

for (Iterator<Bullet> iterator = player.getList().iterator(); iterator.hasNext(); ) {
            if (npcManager.bulletCollide(iterator.next())) {
                iterator.remove();
                //
            }
        }

Solution

  • In java, you cannot do for( T variable : collection ) { collection.remove( variable ); } In other words, you cannot be removing items from a collection which is being iterated with a for loop. It will result in a ConcurrentModificationException. Google for "java fail-fast iterator" for more information about why and how.

    You have two options:

    • Collect in a temporary list all the monsters that are to be removed, then once you are done iterating over the main list, iterate once more through the temporary list to remove them from the main list. The point is that you are not going to be removing from the same list that you are iterating.

    • Instead of a "foreach" (for) loop, use an actual iterator, and when you want to remove an item, remove it by invoking the iterator's remove() method, not the containing collection's remove() method. The iterator's remove() method does the necessary tricks to prevent a ConcurrentModificationException from being thrown.