Smart way to do this.

Started by
6 comments, last by Bob Janova 16 years ago
I currently have a very messy function that updates all my sprites. I am iterating over a vector containing all the sprites. inside the iteration, i am iterating over the same vector twice per iteration(because i have paired connected sprites to be removed). However, i sometimes ends up with an infinite loop. It basicly looks like this:
[source langg = "cpp"]
for(vector<Sprite>::iterator iter = Sprites.begin(); iter != Sprites.end();)
{
//draw and update sprite

 //should the body be killed?
  //then find conected bodies and kill them too.
 {
  for(vector<Sprite>::iterator it = Sprites.begin(); it != Sprites.end();)
  {
  //if match it=Sprites.erase
  //else ++it
  }
   for(vector<Sprite>::iterator it = Sprites.begin(); it != Sprites.end();)
  {
  //if match it=Sprites.erase
   //else ++it
  }
  iter = Sprites.begin()
 }
  //if the body (*iter) should not be killed: ++iter
}





Well. Just know that i need to iterate through Sprites twice if (*iter) has to be killed. It seems i end up with a random infinite loop through the iterations. Also, if there is a smarter way to do this, please let me know also. Thanks in advice. [Edited by - MadsGustaf on March 20, 2008 7:55:12 AM]
•°*”˜˜”*°•.˜”*°•..•°*”˜.•°*”˜˜”*°•..•°*”˜˜”*°•.˜”*°•.˜”*°•. Mads .•°*”˜.•°*”˜.•°*”˜˜”*°•.˜”*°•..•°*”˜.•°*”˜.•°*”˜ ˜”*°•.˜”*°•.˜”*°•..•°*”˜I am going to live forever... or die trying!
Advertisement
This might not answer your question but could still be causing you problems.

When you call
myVector.erase()
you need to set the iterator to be whats returned from that call. Otherwise your iterator gets out of sync with what your looping through.

Like this;

vector<Sprite>::iterator iter;for( iter = Sprites.begin(); iter != Sprites.end(); ++iter){  if ( /*Your Condition*/ )  {      iter = Sprites.erase(); //Get the return value  }}


Also note that i've moved the increment out of the loop. I think this makes it a bit clearer.
Yea, but then iter will skip the element after the deleted element, as erase() returns an iterator to the element after.
•°*”˜˜”*°•.˜”*°•..•°*”˜.•°*”˜˜”*°•..•°*”˜˜”*°•.˜”*°•.˜”*°•. Mads .•°*”˜.•°*”˜.•°*”˜˜”*°•.˜”*°•..•°*”˜.•°*”˜.•°*”˜ ˜”*°•.˜”*°•.˜”*°•..•°*”˜I am going to live forever... or die trying!
Quote:Original post by MadsGustaf
Yea, but then iter will skip the element after the deleted element, as erase() returns an iterator to the element after.


Not that i'm aware of. Erase() removes the element from the vector and then points to the next valid element. So erase will remove the element your looking at and effectivly then 'bump' all of the elements after it down to fill the gap. The return iterator will point the same place you were looking at before the erase, only now it will have the next element in it.

What you said above is actually whats happening in your original code. You call erase and everthing moves down, then you increment the

See here.

Quote:Return value

A random access iterator pointing to the new location of the element that followed the last element erased by the function call, which is the vector end if the operation erased the last element in the sequence.


Hope that helps.

EDIT: Perhaps i'm gettin confused is the following line a misprint?

for(vector<Sprite>::iterator iter = Sprites.begin(); it != Sprites.end();)

What is 'it'? Maybe its unclear from the bits you've commented out. Could you post an expanded version?
Sorry it is my bad. it is supposed to be:
for(vector<Sprite>::iterator it = Sprites.begin(); it != Sprites.end();)

the problem is that i can remove either none or one or two sprites at random places in the Sprite vector during each iteration. Using the approach in my first post it causes an infinite loop in the middle of the game, i cannot seem to figure out where..

[Edited by - MadsGustaf on March 20, 2008 8:51:40 AM]
•°*”˜˜”*°•.˜”*°•..•°*”˜.•°*”˜˜”*°•..•°*”˜˜”*°•.˜”*°•.˜”*°•. Mads .•°*”˜.•°*”˜.•°*”˜˜”*°•.˜”*°•..•°*”˜.•°*”˜.•°*”˜ ˜”*°•.˜”*°•.˜”*°•..•°*”˜I am going to live forever... or die trying!
It would seem to me that if you have multiple sprites connected, and if you destory one then you destroy the other one. The best way would be to wrap them up into say a SpriteGroup. Have the SpriteGroup have another list (or even an array if its fixed number) of sprites. Then destory the group when you need to. There must be some reason they are connected so why not make this connection in the code?

Alternatively i think its the erasing thats causing the infinite loops, although without the full source its hard to tell. Could you not use your current loops to 'flag' sprites up for deletion. Then at the end of it all, use ONE loop to iterate through and remove the appropriate elements.

If the above is not relevent can you explain more why you need to iterate your vector, inside a loop that already doing it?
Heres the whole code. Some of it is probably not that relevant

[source langg = "cpp"]vector<b2Body*> bodies;     for(vector<Sprite>::iterator iter = Sprites.begin(); iter != Sprites.end();)  	..................cout << "Drawings is done.. Should the sprite be killed?" << endl;	    if((*iter).body->GetUserData() == "kill")        {         		  cout << "1.About to enter first loop to destroy bodies.. 1/5" << endl;		  		  for(b2JointNode* j = (*iter).body->GetJointList(); j;  j =j->next)		  {		   cout << "1.First loop: destroying bodies? 2/5" << endl;		   if(j->joint->GetBody1()->GetUserData() == "kill" || j->joint->GetBody1()->GetUserData() == "isconnected" || j->joint->GetBody1()->GetUserData() == "emo")		   {		    cout << "1.body should be killed 3/5" << endl;		    b2Body* b =	j->joint->GetBody1();			for(vector<Sprite>::iterator it = Sprites.begin(); it != Sprites.end();)			{			 cout << "1.Seaching for the sprite corresponding to the body.. 4/5" << endl;			 if( &(*(*it).body) == (&(*b)))			 {			  cout << "1.Match was found. deleting body + sprite 5/5" << endl; 			  bodies.push_back(b);			  it = Sprites.erase(it);			 			 }			 else			 ++it; 			}		   }		   cout << "2.About to enter second loop to destroy bodies.. 1/4" << endl;		   if(j->joint->GetBody2()->GetUserData() == "kill" || j->joint->GetBody2()->GetUserData() == "isconnected" || j->joint->GetBody2()->GetUserData() == "emo")		   {		    cout << "2.Second loop: destroying bodies 2/4" << endl;			   b2Body* b = j->joint->GetBody2(); 		     for(vector<Sprite>::iterator it = Sprites.begin(); it != Sprites.end();)			{		    cout << "2.Seaching for the sprite corresponding to the body.. 3/4" << endl;				 if( &(*(*it).body) == (&(*b)))			 {			  cout << "2.Match was found. deleting body + sprite  4/4" << endl;  			  bodies.push_back(b);			  it = Sprites.erase(it);			 			 }			 else			 ++it; 			}		   		   }		  		  }		  		 cout << "Setting :iter = Sprites.begin(); " << endl; 				           		iter = Sprites.begin();              }       else       {       cout << "incrementing iter" << endl;       iter++;       }   	       }		 unique(bodies.begin(),bodies.end());	  for_each(bodies.begin(),bodies.end(),worlddestroy);


I am usiing the b2Body as an identifier for the appropriate sprites, so i cannot delete the bodies before i have deleted all the sprites. Basicly what the two for loops are doing is finding.

When the infinite loop is reached, the console keeps spamming these 3 lines of outputs:

Quote:
Drawings is done.. Should the sprite be killed?
1.About to enter first loop to destroy bodies.. 1/5
Setting :iter = Sprites.begin();


I would had thought that, in this line

  for(b2JointNode* j = (*iter).body->GetJointList(); j;  j =j->next)

that there would be no joints, forcing nothing below that to be executed, but the joint list etc. is handled by the physics engine, and i am only setting (*iter).body->GetUserData() == "kill" if there actually is a joint.
•°*”˜˜”*°•.˜”*°•..•°*”˜.•°*”˜˜”*°•..•°*”˜˜”*°•.˜”*°•.˜”*°•. Mads .•°*”˜.•°*”˜.•°*”˜˜”*°•.˜”*°•..•°*”˜.•°*”˜.•°*”˜ ˜”*°•.˜”*°•.˜”*°•..•°*”˜I am going to live forever... or die trying!
Here's your problem.

Quote:I would had thought that, in this line ... that there would be no joints

That would appear to be the case from your output. So the whole 'j' loop is not running, and your code is basically this:
    for(vector<Sprite>::iterator iter = Sprites.begin(); iter != Sprites.end();)  	..................cout << "Drawings is done.. Should the sprite be killed?" << endl;	    if((*iter).body->GetUserData() == "kill")        {         		  cout << "1.About to enter first loop to destroy bodies.. 1/5" << endl;	// ... (for loop here that doesn't run)	 cout << "Setting :iter = Sprites.begin(); " << endl; 				           		iter = Sprites.begin();              }       else       {       cout << "incrementing iter" << endl;       iter++;       }   	       }	

Or, in pseudo-code:
iterate through vector of sprites {  if(to be killed){   do nothing   go back to start  } else move on}


Not surprisingly, you keep going back to the start and then trying to kill the same entry over and over. Where I've bolded 'do nothing' you should actually be removing the sprite in question from the iteration, and proceeding (I don't see why you go back to the start each time?).

This topic is closed to new replies.

Advertisement