🔒 Locked

std::list problem

Started by ninthjarl Mar 17, 2010 at 2:24 PM 4 replies 2.8k views

Original Post

Mar 17, 2010 at 2:24 PM

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]

Ohforf sake

Mar 17, 2010 at 2:45 PM

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 listif( bubbleIter != bubbles->bubbleList.end())   ++bubbleIter;//check if list is emptyif( 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.

ninthjarl

Author

ninthjarl

Mar 17, 2010 at 2:58 PM

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.

Bregma

1,022

Bregma

Mar 17, 2010 at 3:22 PM

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?

Zahlman

257

Zahlman

Mar 17, 2010 at 9:24 PM

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.

ninthjarl

Author

ninthjarl

Mar 18, 2010 at 2:00 AM

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.

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.