Removing items from ArrayList upon collision (Java)?

Question

As a preface, I have searched the forums but found nothing relating to my specific situation. I just started learning Java about a week ago, and this is my first foray into object oriented programming.

I'm building a basic game (think somewhat like Space Invaders when it comes to mechanics). I have a "Projectile" class, a "FallingThings" class (which is the parent class to the classes I have for objects falling down (Money, Friend, Enemy)). Each projectile that is shot is stored in an ArrayList, and as is every instance of Money, Friend, and Enemy (each in their own ArrayList).

The problem happens when I implement collision detection. First off, it takes multiple bullets to make the collision mechanism work (I think I might have just messed up some numbers here but I'm not 100% sure). Now, sometimes after I fire multiple bullets (and they are long gone), a collision is detected without me firing another bullet and a FallingThings object disappears from the screen. Also, at other times, multiple objects disappear at once. The weirdest thing is that all this is inconsistent, and not always reproducible in the same fashion. However, my collisions with the player character work perfectly.

Anyone have some ideas as to how I can fix this? My code is below:

Method in the "FallingThings" class (the ArrayLists are defined in the StartingClass (the main class)).

   public void checkBulletCollision(Rectangle rectangle) {

    for (int j = 0; j < Character.getProjectiles().size(); j++) {
        Projectile p = (Projectile) Character.getProjectiles().get(j);
        if (p.isVisible() == true) {

            for (int i = 0; i < StartingClass.getMoneys().size(); i++) {
                Money m = (Money) StartingClass.getMoneys().get(i);
                if (m.getR().intersects(rectangle)) {
                    m.remove(i);
                    System.out.println("bullet collision");
                }
            }
            for (int i = 0; i < StartingClass.getEnemies().size(); i++) {
                Enemy e = (Enemy) StartingClass.getEnemies().get(i);
                if (e.getR().intersects(rectangle)) {
                    e.remove(i);
                }
            }
            for (int i = 0; i < StartingClass.getFriends().size(); i++) {
                Friend f = (Friend) StartingClass.getFriends().get(i);
                if (f.getR().intersects(rectangle)) {
                    f.remove(i);
                }
            }
        }
    }
}   

My update method for projectiles:

public void update() {
    y -= speedY;
    if (y < 0) {
        visible = false;
    }
    rectangle.setBounds(getRectangle());
}

I've been trying to fix this for the entire day, and still can't get a proper implementation. I have tried using ListIterator, but that caused the program to freeze and a typecasting error to be thrown.

Thank you so much for the help! =)

Answer 1

I suspect you are having problems because you are removing items from the list by index while looping over that list. Removing items causes the index counter to get out of allignment. Try use an actual Iterator. No need to even think about indexes. The Iterator has a remove method for these situations..

Iterator<Enemy> iterator = StartingClass.getEnemies().iterator();
while (iterator.hasNext()) {
    Enemy e = iterator.next();
    if (e.getR().intersects(rectangle)) {
        iterator.remove();
    }
}

Answer 2

I don't understand what m.remove does, you get the item from the list but then call "remove" on the item. do you want to delete it from the List instead?

StartingClass.getMoneys().remove(i);

Answer 3

Looks like the issue is that you are letting all types of loops to run. So even after removing a Money with your Projectile, you are on to removing and Enemy and a Friend also with the same Projectile - which is possible if that Projectile is fired from anything bigger or from a .44 magnum. Anyway, to me, looks like you need to break your iteration once you remove one item with one projectile.. so your code should be like(Use generics and enhanced for loops) :

foreachprojectile:
    for( Projectile projectile : Character.getProjectiles()){
        if (projectile.isVisible() == true) {

            Iterator<Money> iterator = StartingClass.getMoneys().iterator();
            while (iterator.hasNext()) {
                Money m = iterator.next();
                if (m.getR().intersects(rectangle)) {
                    iterator.remove();
                    projectile.setVisible(false); // or any other method that does similar
                    break foreachprojectile; //Found a hit, so do not 
                    //look for any more hits with current projectile
                }
            }
            //And so on..
        }
    }

Above code probably will get done what you are looking for; but this logic can be encapsulated better to clearly communicate the intent.

Answer 4

I figured out what was wrong and fixed the problem. It turns out that the issue was not in my implementation of the for loops (although using a listIterator is still more efficient and less error-prone, so I've still opted to change that aspect of my code), but rather in the implementation of my collision detection. I forgot that the projectile and rectangle (for the projectile) are two different objects, and as such, the rectangle was not moving with the projectile, resulting in all sorts of problems.

I determined the issue by painting the rectangle to the screen to check how it behaved, and sure enough, it just stayed in one place, not moving with the projectile. I made some changes to the projectile's update() method so that when called, the rectangle moves with the bullet. This resulted in proper collision detection and a functioning program!

Thank you so much to everyone for all the help, I learned a lot here about writing efficient code and debugging techniques! I really appreciate it! =)