Issues with iterating through arraylists in libgdx

Viewed 278

I'm working on a Missile Command remake in libgdx.

I have 3 array lists: bullets, missiles, and houses. I've been trying to iterate through the lists to detect collisions between missiles and houses, and missiles and bullets, but I get this very broad error message for everything I've tried so far:

Exception in thread "LWJGL Application" java.lang.IndexOutOfBoundsException: Index 0 out of bounds for length 0
    at java.base/jdk.internal.util.Preconditions.outOfBounds(Preconditions.java:64)
    at java.base/jdk.internal.util.Preconditions.outOfBoundsCheckIndex(Preconditions.java:70)
    at java.base/jdk.internal.util.Preconditions.checkIndex(Preconditions.java:248)
    at java.base/java.util.Objects.checkIndex(Objects.java:372)
    at java.base/java.util.ArrayList.get(ArrayList.java:458)
    at com.mygdx.game.Screen.MCGameScreen.render(MCGameScreen.java:112)
    at com.badlogic.gdx.Game.render(Game.java:46)
    at com.badlogic.gdx.backends.lwjgl.LwjglApplication.mainLoop(LwjglApplication.java:215)
    at com.badlogic.gdx.backends.lwjgl.LwjglApplication$1.run(LwjglApplication.java:120)

Theres been the same issue with every way I've tried to iterate through the lists that causes the (probably missiles) array list to check for something at a position that was already removed due to a collision. I do not want to "reuse" the objects in the array, and I'd much prefer just removing them.

This is the code for creating a new missile, and adding them to the missiles array, every 2 seconds a new missile is created, given a random position along the top of the screen, and added to missiles.

timer += delta;

        if(timer >= 2) {
            Sprite temp = new Sprite(missile);
            temp.setPosition((float) (Math.random() * Gdx.graphics.getWidth() - temp.getWidth()), Gdx.graphics.getHeight());
            missiles.add(temp);
            timer = 0;
        }

This is the code for creating a new bullet, and adding it to the bullets array, every time the space bar is pressed, a new bullet is created, set to the center of the tank's position, and added to bullets.

if(keycode == Input.Keys.SPACE) {
            Sprite temp = new Sprite(bullet);
            temp.setPosition(tank.getX() + tank.getWidth()/2 - temp.getWidth()/2, tank.getY() + tank.getHeight());
            bullets.add(temp);
        }

This is the initialization of the preset sprites I use when creating the temp variables to be added to the lists, as well as the arraylists themselves.

bullet = new Sprite(new Texture("bullet.png"));
bullets = new ArrayList<>();

missile = new Sprite(new Texture("missile.png"));
missiles = new ArrayList<>();

house = new Sprite(new Texture("house.png"));
houses = new ArrayList<>();

This is the first way I've tried to check for collisions where I also used the same loops to move the missiles and bullets.

        for(int i = bullets.size()-1; i >= 0; i--) {
            bullets.get(i).translateY(bulletSpeed);
            if(bullets.get(i).getY() >= Gdx.graphics.getHeight() + bullet.getHeight()) {
                bullets.remove(i);
            }
            for(int j = missiles.size()-1; j >= 0; j--) {
                if(missiles.get(j).getBoundingRectangle().overlaps(bullets.get(i).getBoundingRectangle())) {
                    missiles.remove(j);
                    bullets.remove(i);
                }
            }
        }

        for(int i = missiles.size()-1; i >= 0; i--) {
            missiles.get(i).translateY(missileSpeed);
            if(missiles.get(i).getY() <= 0) {
                missiles.remove(i);
            }
            for(int j = houses.size()-1; j >= 0; j--) {
                if(houses.get(j).getBoundingRectangle().overlaps(missiles.get(i).getBoundingRectangle())) {
                    houses.remove(j);
                    missiles.remove(i);
                }
            }
        }

This is the second method I've tried, I split up everything in to separate loops in case the index of the array was still being used in the loop even after it was removed.

        for(int i = bullets.size()-1; i >= 0; i--) {
            bullets.get(i).translateY(bulletSpeed);
            if(bullets.get(i).getY() >= Gdx.graphics.getHeight() + bullet.getHeight()) {
                bullets.remove(i);
            }
        }

        for(int i = missiles.size()-1; i >= 0; i--) {
            missiles.get(i).translateY(missileSpeed);
            if(missiles.get(i).getY() <= 0) {
                missiles.remove(i);
            }
        }

        for(int i = missiles.size()-1; i >= 0; i--) {
            for(int j = bullets.size()-1; j >= 0; j--) {
                if(missiles.get(i).getBoundingRectangle().overlaps(bullets.get(j).getBoundingRectangle())) {
                    missiles.remove(i);
                    bullets.remove(j);
                }
            }
        }

        for(int i = missiles.size()-1; i >= 0; i--) {
            for(int j = houses.size()-1; j >= 0; j--) {
                if(missiles.get(i).getBoundingRectangle().overlaps(houses.get(j).getBoundingRectangle())) {
                    missiles.remove(i);
                    houses.remove(j);
                }
            }
        }

I've also tried this variant, using enhanced for loops.

        for(Sprite missile: missiles) {
            missile.translateY(missileSpeed);
            if(missile.getY() <= 0) {
                missiles.remove(missiles.indexOf(missile));
            }
        }

        for(Sprite bullet: bullets) {
            bullet.translateY(bulletSpeed);
            if(bullet.getY() >= Gdx.graphics.getHeight() + bullet.getHeight()) {
                bullets.remove(bullets.indexOf(bullet));
            }
        }

        for(Sprite missile: missiles) {
            for(Sprite bullet: bullets) {
                if(missile.getBoundingRectangle().overlaps(bullet.getBoundingRectangle())) {
                    missiles.remove(missiles.indexOf(missile));
                    bullets.remove(bullets.indexOf(bullet));
                }
            }
        }

Please let me know if you need any details that I left out!

3 Answers

I went back to my answer where I split up each operation in to it's own for loop:

        for(int i = bullets.size()-1; i >= 0; i--) {
            bullets.get(i).translateY(bulletSpeed);
        }

        for(int i = bullets.size()-1; i >= 0; i--) {
            if(bullets.get(i).getY() >= Gdx.graphics.getHeight() + bullet.getHeight()) {
                bullets.remove(i);
            }
        }

        for(int i = missiles.size()-1; i >= 0; i--) {
            missiles.get(i).translateY(missileSpeed);
        }

        for(int i = missiles.size()-1; i >= 0; i--) {
            if(missiles.get(i).getY() <= 0) {
                missiles.remove(i);
            }
        }

        for(int i = missiles.size()-1; i >= 0; i--) {
            for(int j = bullets.size()-1; j >= 0; j--) {
                if(missiles.get(i).getBoundingRectangle().overlaps(bullets.get(j).getBoundingRectangle())) {
                    bullets.remove(j);
                    missiles.remove(i);
                    break;
                }
            }
        }

        for(int i = missiles.size()-1; i >= 0; i--) {
            for(int j = houses.size()-1; j >= 0; j--) {
                if(missiles.get(i).getBoundingRectangle().overlaps(houses.get(j).getBoundingRectangle())) {
                    houses.remove(j);
                    missiles.remove(i);
                    break;
                }
            }
        }

I added a break; after I removed an item from the array, as that index would no longer be in use.

So one thing right off the bat that I notice and would be concerned about. While your iterating through the list's, your mutating them. You make a call to

List<E>.remove(i);

and this is highly regarded as a less than safe operation.

If you really want to mutate a list while iterating, the standard and safe way to do it is make a call to the lists'

`iterator()` 

function and then calling remove() on the iterator itself.

Not 100% sure if this is the solution to your original problem but this easily could be what is giving you trouble

Here are the problems with each of your approaches:

First way:

    for(int i = bullets.size()-1; i >= 0; i--) {
        bullets.get(i).translateY(bulletSpeed);
        if(bullets.get(i).getY() >= Gdx.graphics.getHeight() + bullet.getHeight()) {
            bullets.remove(i);
        }
        for(int j = missiles.size()-1; j >= 0; j--) {
            if(missiles.get(j).getBoundingRectangle().overlaps(bullets.get(i).getBoundingRectangle())) {
                missiles.remove(j);
                bullets.remove(i);
            }
        }
    }

You are iterating through the bullets list backwards, which would normally be OK, but then you continue to try to access the bullet in the loop after it's been removed. When a bullet reaches the top of the screen, you remove it. Then you you call bullets.get(i) again in your inner missiles loop, but that bullet is already gone from the list so you get IndexOutOfBoundsException if it was the last bullet in the list.

Second way:

    for(int i = missiles.size()-1; i >= 0; i--) {
        for(int j = bullets.size()-1; j >= 0; j--) {
            if(missiles.get(i).getBoundingRectangle().overlaps(bullets.get(j).getBoundingRectangle())) {
                missiles.remove(i);
                bullets.remove(j);
            }
        }
    }

Here, when a collision is found, you remove the missile, but then you keep iterating through the bullets, so the call to missiles.get(i) will fail with IndexOutOfBoundsException if it was the last one in the list. You can fix this by adding a break statement after removing the missile, since there's no need to keep checking the removed missile against the rest of the bullets:

for(int i = missiles.size()-1; i >= 0; i--) {
    for(int j = bullets.size()-1; j >= 0; j--) {
        if(missiles.get(i).getBoundingRectangle().overlaps(bullets.get(j).getBoundingRectangle())) {
            missiles.remove(i);
            bullets.remove(j);
            break;
        }
    }
}

You would want to do the same thing to your loop with the houses.

Third way:

You cannot safely remove items when using enhanced iteration syntax (because it's iterating from low to high instead of backwards). What you can do is specifically use an Iterator and call remove on the iterator:

    for(Iterator<Sprite> iterator = missiles.iterator(); iterator.hasNext();) {
        Sprite missile = iterator.next()
        missile.translateY(missileSpeed);
        if(missile.getY() <= 0) {
            iterator.remove();
        }
    }
Related