Sign in to follow this  
ninthjarl

std::list problem

Recommended Posts

This is my first post on gamedev.net Right I am new to game programming and I am stuck with this problem since more than two hours now. Basically it is a small 2D game with bubbles and spiky things. I am stuck with the part where I need to check the collisions. I am getting the dreaded iterator cannot be referenced error. Moreover, its not crashing at the beginning or end of the loop its pretty random and I am not able to figure out if I am doing something wrong while erasing the element from list. in the beginning of the program I have instantiated objects of these classes. Bubbles* bubbles; Spikies* spikies; Both these classes have std::list of bubble instances( bubbleList)and spiky instances (spikyList) respectively
BubbleList::iterator bubbleIter;
SpikyList::iterator spikyIter;
bool isTouching = false;

bubbleIter = bubbles->bubbleList.begin();
if(!bubbles->bubbleList.empty())
{
	while( bubbleIter != bubbles->bubbleList.end() )
	{
		isTouching = false;
		for(spikyIter = spikies->spikyList.begin(); spikyIter != spikies->spikyList.end(); spikyIter++)
		{

			if( (*spikyIter)->mState == ALIVE)
				isTouching = (*bubbleIter)->boundingCircle->Intersects((*spikyIter)->boundingCircle);

			if(isTouching)
			{
				SAFE_DELETE(*bubbleIter);
				
				bubbleIter = bubbles->bubbleList.erase(bubbleIter);
				(*spikyIter)->mState = DEAD;
				
			}
		}
		//checking if we are not at the end of the list
		if( bubbleIter != bubbles->bubbleList.end())
			++bubbleIter;
		//check if list is empty
		if( bubbles->bubbleList.empty() )
			break;
	}
}

Any input would be much appreciated. Thanks. [Edited by - ninthjarl on March 17, 2010 2:54:58 PM]

Share this post


Link to post
Share on other sites
If you indent your code it becomes obvious:


for(spikyIter = spikies->spikyList.begin(); spikyIter != spikies->spikyList.end(); pikyIter++)
{
if( (*spikyIter)->mState == ALIVE)
isTouching = (*bubbleIter)->boundingCircle->Intersects((*spikyIter)->boundingCircle);

if(isTouching)
{
SAFE_DELETE(*bubbleIter);

bubbleIter = bubbles->bubbleList.erase(bubbleIter);
(*spikyIter)->mState = DEAD;


}
}

//checking if we are not at the end of the list
if( bubbleIter != bubbles->bubbleList.end())
++bubbleIter;
//check if list is empty
if( bubbles->bubbleList.empty() )
break;





The "checking if we are not at the end of the list" is performed after the for loop, not in it. There might still be spikes in the spikelist that get tested against the already invalid bubble iterator.

Edit: Besides, even if it doesn't crash, the immediate next bubble after a bubble that gets destroyed will be omitted. You should play through various situations on a piece of paper and refactor those two loops afterwards.

Share this post


Link to post
Share on other sites
[quote]Original post by Ohforf sake
Hey sorry as I said this is my first post so didn't know how to paste the code. I have modified it now so it should be more clear now.
I am checking if I am at the end of list outside the for loop not inside it.

Share this post


Link to post
Share on other sites
Also, why do you mark spiky things as "dead" after they collide with a bubble?

Also, pay close attention to how the 'isTouching' variable is treated. What happens if one spiky is alive and the next is dead? The "if alive, change isTouching" part is skipped, and resetting the variable is also skipped (because it's not in the innermost loop), so the dead spiky still gets a chance to collide.

If you scope your variables properly you avoid these problems. Better yet, don't even bother using one where you don't need it: you can use the result of the Intersects() check directly to control the if-block.

Anyway, the sane way to handle this sort of thing is to mark everything that you want to remove first, and then remove it after you've done all the marking. There are standard library algorithms you can use to help with this.

Finally, why do you have pointers everywhere? You do know that you can put objects directly into the standard library containers, yes? You would only want to indirect them through a pointer in order to "share" them (in which case you would have to think much, much harder about your deletion logic) or to get polymorphic behaviour. And either way you are better off using a smart pointer class, e.g. boost::shared_ptr, rather than raw pointers.

Share this post


Link to post
Share on other sites
Thanks everyone for the replies and help. I did post the same question on another forum and Avi Berger helped me out with code example. Here is the updated code.

BubbleList::iterator bubbleIter;
SpikyList::iterator spikyIter;
bool isTouching = false;
bubbleIter = bubbles->bubbleList.begin();
while( bubbleIter != bubbles->bubbleList.end() )
{
isTouching = false;
for( spikyIter = spikies->spikyList.begin();
spikyIter != spikies->spikyList.end();
spikyIter++ )
{
if( (*spikyIter)->mState == ALIVE &&
(*bubbleIter)->boundingCircle->Intersects((*spikyIter)->boundingCircle) )
{
isTouching = true;
(*spikyIter)->mState = DEAD;
break;
}
}
if( isTouching )
{
SAFE_DELETE(*bubbleIter);
bubbleIter = bubbles->bubbleList.erase(bubbleIter);
}
else
++bubbleIter;
}


Quote:
Original post by Bregma
bubbleIter = bubbles->bubbleList.erase(bubbleIter);

What does this line do? What happens if you erase the last bubble in the list but are not done the spikies processing?


This line erases the object in the list that the iterator is pointing at and returns the pointer to the next item in the list. So we wont be in the middle of invalid memory.

Zahlman,
Quote:
Original post by Zahlman
Anyway, the sane way to handle this sort of thing is to mark everything that you want to remove first, and then remove it after you've done all the marking. There are standard library algorithms you can use to help with this.


I was actually thinking of implementing something like this but now that the problem is resolved. I will try to implement it in future.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this