0

I am a beginner at Java and game programming. I am working on a Java 2D game and adding an inventory and looting system. Basically, when my player picks up an item, I want to remove the physical object from the world. However, I am getting a ConcurrentModificationException error. Here is a part of my code (which I think may be part of the problem so I will include the code):

for (GameObject obj : goManager.gameObjects) {
    if (obj instanceof ItemObject itemObj) {
        if (bounds().intersects(itemObj.getBounds())) {
            collidingItem = itemObj;
        } else {
            collidingItem = null;
        }
    }
}

And this is the code where the error happens:

inventory.add(collidingItem.getItem());
goManager.gameObjects.remove(collidingItem); // Error happens here
collidingItem = null;

The output:

Exception in thread "Thread-0" java.util.ConcurrentModificationException
    at java.base/java.util.ArrayList$Itr.checkForComodification(ArrayList.java:1013)
    at java.base/java.util.ArrayList$Itr.next(ArrayList.java:967)
    at com.yiwuen.PixelBlock.entity.GameObjectManager.render(GameObjectManager.java:12)
    at com.yiwuen.PixelBlock.Game.render(Game.java:146)
    at com.yiwuen.PixelBlock.Game.run(Game.java:103)
    at java.base/java.lang.Thread.run(Thread.java:833)

I'm not sure how to remove the colliding item from the ArrayList because when I do so, that error occurs. Any thoughts?

I've also tried to just set the colliding item to null but that obviously didn't work. I researched but I didn't understand any of the solutions.

yiwuen
  • 11
  • 2
  • The only safe way to remove items from a list _during_ a loop is if you're looping back-to-front, so your removal doesn't affect anything about the (sub)list you haven't iterated over yet. Consider iterating over [0,1,2], we start at iteration 0, and and we remove that element. Now the list is only [1,2], where in the list are we? where do we iterate to? You modified the list concurrent to iterating over it, you basically guaranteed bugs, so java goes "no". If you want to remove items while iterating, use a `for` loop that runs from `.size()-1` to zero, or use an Iterator. – Mike 'Pomax' Kamermans Aug 21 '23 at 00:46
  • See [Why is a ConcurrentModificationException thrown and how to debug it](https://stackoverflow.com/questions/602636/why-is-a-concurrentmodificationexception-thrown-and-how-to-debug-it) – Mike 'Pomax' Kamermans Aug 21 '23 at 00:47
  • There’s probably a more general structural problem with the way concurrency is handled in this game. You really don’t want to be modifying the game state concurrently while rendering, this will result in rendering an inconsistent state. You’ll need to make sure you’re synchronizing appropriately, or perhaps even change the threading model more significantly. You might not want to have state updates and rendering on different threads at all. – Tim Moore Aug 21 '23 at 00:52

2 Answers2

1

If you are trying to remove an element from the list inside the loop, do it using an Iterator.

for (Iterator<GameObject> it = goManager.gameObjects.iterator(); it.hasNext(); ) {
    GameObject obj = it.next();
    // method logic 

    // remove using Iterator.
    it.remove()
}
hermit
  • 1,048
  • 1
  • 6
  • 16
0

In java ,there has a counter-intuitive method ,is that we can not delete item from list by iterator. If the object named gameObjects defined by you ,i suggest you convert the data type to CopyOnWriteArrayList If not ,you could use the following code

public static void main(String[] args) {
        ArrayList<String> strings = new ArrayList<>();
        strings.add("a");
        strings.add("b");
        strings.add("c");
        strings.add("d");
        strings.add("e");

        for (int i = 0; i < strings.size(); i++) {
            if("c".equalsIgnoreCase(strings.get(i))){
                strings.remove(i);
                // important 
                i--;
            }
        }

        System.out.println("strings = " + strings);

    }

Above are a simple instance,you could believe this is a good practice.

ash
  • 26
  • 3
  • Don't do it like this. Just use an iterator obtained from `List.iterator()`. – WJS Aug 21 '23 at 02:55
  • This isn't a bad approach, except your _for-loop_ relies on _strings.size()_. You'll need to adjust this value whenever you decrement _i_. – Reilas Aug 21 '23 at 21:21