[Solved], for real this time: ArrayList remove() problem

Newbie & Debugging Questions
#1

Hello JGO,

I’ve gotten back into game development, and I’ve already ran into an issue. In this game, when the player/enemy runs into the “e” square (I’ve been focusing more on the code than art, so the tile’s an E as of right now), I want it to disappear. The collision and such works fine, and one of the players deletes the tiles fine, but when I try to use the other player, it does this (Ignore all the boxes on the side, they’re for debug purposes):

Yeah. The error Eclipse gives me is this: http://pastebin.com/3vcTQZ89

Here’s the code to remove the tiles: http://pastebin.com/WPudwYY8

Am I doing something wrong? If so, what is it? This may be a very simple problem to deal with, and I’m just not looking hard enough. If the provided code is not enough, I’ll answer questions.

Thanks,
DarkCart

#2

There’s a lot potentially wrong with that code, but without more information we can’t really do much.

Do getTileX and friends return ints? Did you really mean to continue to use a specific index (i) across removals?
Which line is line 78 as mentioned in the stack trace? Is it always 78?
Less prone to cause this error directly, but still curious: did you really want remove(get(i)) and not remove(i)?

This kind of imperative code is extremely brittle.

#3

Exception in thread “AWT-EventQueue-0” java.lang.IndexOutOfBoundsException: Index: 9, Size: 9

I think you need to call “break;” to stop the For looping after removed the object, because you can’t verify the (i) if you already removed from the list.

enemyList.remove(----);
break;

#4

getTileX() and Co. return ints.
Yes, the line is always 79; here is that line (It said 79, not 78 in the stack trace).


enemyList.remove(o.getTileX());

I don’t understand what you mean by that last part.

I tried this, and it didn’t work. :frowning:

#5

The bigger issue is that when you’re removing items from a list in this manner, you’re going to end up skipping an element when an element is removed. You should walk the list backwards. This will correct the index out of bounds error.

Think about it this way. You have a list of 3 items. You set your loop counter to go from 0-2 (the size of your list) at the start. In the first pass, you remove an element. Suddenly, you’ve changed the valid index range of your list from 0-2 to 0-1. Unfortunately your counter variable in the loop doesn’t recognize this change, and will happily keep going until it hits the now invalid index of 2.

Now reverse the scenario and set your loop counter to the size of the list-1 (2) at the start. On your first pass, you’re loop removes an element from the list which leaves your valid index range at 0-1. Unlike the previous scenario, on the next pass, the loop counter will equal 1 which is still a valid index.

It’s a small gotcha that can cause quite a bit of frustration if you’ve never encountered it before. The other issue may still persist (didn’t have that much time to investigate), but it will get rid of the exception at least. :wink:

#6

Wait, I think I fixed it. Like @BurntPizza said (I think), I replaced this line:


enemyList.remove(o.getTileX1());
enemyList.remove(o.getTileY1());

with


enemyList.remove(enemyList.get(i));
enemyList.remove(enemyList.get(i));

And it started working.

#7

I meant which of the removals is problematic line (which the trace that you posted says is 78).
I’m guessing is the first of the two remove(getTileX())s.

EDIT: CodeHead got right to what I was leading up to… :point:
EDIT again: so now it appears to be working, but nobody knows why. That’s even worse.
And while I don’t know what TileX and TileY are supposed to mean, but that replacement doesn’t appear to remotely mean the same thing.

So the real question is, what are you trying to do?

#8

Never mind. Here’s what it does now:

And it still throws a exception but this time for player 1. :cranky:

#9

The exception shows the exact issue: " java.lang.IndexOutOfBoundsException: Index: 2, Size: 2". Maximum valid index for a list with a size of 2 elements is 1.

This…


enemyList.remove(enemyList.get(i));
enemyList.remove(enemyList.get(i));

…will still cause the index exception since you’re attempting to “get” an element at an invalid index.

Until you understand what’s actually causing the issue, applying fixes blindly isn’t going to help. :persecutioncomplex:

#10

You are iterating over a list and removing its elements WHILE iterating over it.


for (int i = 0; i < enemyList.size(); i++) {
                        if (enemyList.get(i).getTileX() == o.getTileX()
                                        && enemyList.get(i).getTileY() == o.getTileY()) {
                                enemyList.remove(enemyList.get(i));
...

this code will make it even worse:


 
enemyList.remove(enemyList.get(i));
enemyList.remove(enemyList.get(i));

if your enemyList has size=2, and in your i = 1, you will get


enemyList.remove(enemyList.get(1)); //ok, size=2
enemyList.remove(enemyList.get(1)); //BOOM, size=1

have a list called toRemove, and instead of removing stuff from inside your loop, add the things you want to remove to toRemove list.
AFTER you are finished iterating the enemyList, remove from enemyList all you added to toRemove list.

#11

Have you tried using an iterator? It works wonders for me, although I don’t think its the fastest way of doing this.
I also don’t know how your game even works, but hopefully the below fulfills your logic requirements.


    for (Iterator<EnemyClass> enemyIter = enemyList.iterator(); enemyIter.hasNext();) {

                                  EnemyClass enemy = enemyIter.next();

                            if (enemy.getTileX() == o.getTileX()
                                            && enemy.getTileY() == o.getTileY()) {
                                    enemyIter.remove();
                            }
     
                            if (enemy.getTileX() == o.getTileX()
                                            && enemy.getTileY() == o.getTileY()) {
                                    enemyList.remove();
                            }
     
                            if (enemy.getTileX() == o.getTileX1()
                                            && enemy.getTileY() == o.getTileY1()) {
                                    enemy.remove();
                            }
                    }
#12

Indeed, this is a valid solution so long as all removals are done using the iterator’s remove method (as illustrated in your example). As for efficiency when doing sequential accesses, iterators are generally the better choice vs index based lookups when using a list as the underlying data structure. :wink:

#13

Woah. That Iterator actually worked :o Yeah, @CodeHead was right, as long as they are all the enemyIter.remove(), it works fine. Kudos to you, @CodeHead and @MrPork.

#14

Nice, thanks for the info CodeHead ;D