Game Development Community

Need help from a C++ guru please

by Scott Wilson-Billing · in Torque Game Builder · 05/08/2010 (4:34 pm) · 9 replies

I have some of my code in Torque script (well most of it) and some of it in C++ for performance reasons. I should say that C++ is not my strongest language (much more comfortable with Java).

I'm having some random bugs which i've tracked down to my use of SimSet and iterators.

The basic problem is:

1. In C++ I'm iterating over a SimSet.
2. I call back into script to perform a script function on one of the iterated objects.
3. This call back results in the script side removing the object from the SimSet.
4. All is fine UNLESS the object which got removed happened to be at the END of the SimSet, then all hell breaks lose and TGBGame exits.

What I've found is that my iterator end pointer is pointing to the object removed from the SimSet and therefore the exit test in the loop never occurs.

I've fixed it (I hope) with a simple test at the end of the iterator loop but I thought that an iterator over a collection in C++ was a static collection and didn't change if the referenced objects were not longer in the collection?

This is my code with the fix:

Con::printf("********* START OF UPDATE *************");
		for(SimSet::iterator i = invaders->begin(); i != invaders->end(); ++i) {
			Con::printf("invaders end=%u", invaders->end());
			t2dSceneObject *invader = (t2dSceneObject *)(*i);
			if (invader != NULL) {
				bool isDead = dAtoi(invader->getDataField(StringTable->insert("isDead"), NULL));
				Con::printf("update invader i=%u, invader id=%u", i, invader->getId());					
				if (invader != NULL && Sim::findObject(invader->getId()) != NULL) {
					Con::printf("About to call updateInvader in TS");
					Con::executef(invader, 2, "updateInvader");
				}
				if (i == invaders->end()) {
					Con::printf("********* FOUND END - EXITING LOOP *************");
					break;
				}
				
			} // END if (invader != NULL)
		} // END for(SimSet::iterator i = invaders->begin(); i != invaders->end(); i++)
		
		Con::printf("********* END OF UPDATE *************");

Is there a more elegant way of dealing with this or am I barking up the wrong tree?
Cheers

#1
05/09/2010 (6:30 am)
Are you deleting invaders inside this function?
Con::executef(invader, 2, "updateInvader");
If so, that may be your problem. It's usually not a good idea to delete objects from a container while you are iterating over it... That leads to major headaches ;-). Instead just flag them for deletion and delete them after the loop. Or you could just hide your invaders after they get killed and bring them back once the level restarts...this way you save all that nasty alloc/dealloc time.

Alain
BTW,
bool isDead = dAtoi(invader->getDataField(StringTable->insert("isDead"), NULL));
does not seem to do anything.
#2
05/09/2010 (7:46 am)
Thanks Alain, I'm not deleting the invader, but just removing it from the simset (referenced by that iterator).

I guess once removed the iterator is no longer stable?

Cheers
#3
05/09/2010 (11:07 am)
If you remove items from the simset while iterating over it then the FOR loop's end condition starts changing inbetween iterations... not a good thing! :-(

Alain
#4
05/09/2010 (11:10 am)
Thanks for clarifying Alain, it kind of makes sense and it was always when I remove the last invader in the sim set that caused the crash - hence the "end" condition going of of scope.

I've fixed them all now to exit if they detect that they are the last item in the iterator - no longer relying on the loop end condition.
Cheers
#5
05/09/2010 (6:49 pm)
If the item is definitely going to be removed from the SimSet during the callback, you could just do a while loop, like this:
while( invaders->size() > 0 )
{
    ...
}
#6
05/10/2010 (12:23 am)
Thanks Phillip, unfortunately it wont always be the case the item will be removed, so I am stuck with the iterator.
Cheers
#7
05/12/2010 (3:49 pm)
how about not letting the invader kill himself in the script but add himself to a "kill me SimSet" through which you go after the loop up there and kill them? So kind of "pseudo schedule" their removal
#8
05/12/2010 (4:17 pm)
Marc, that is one possibility to consider - I have the isDead flag so I don't evan need the other sim set, just set the isDead flag and sweep up after the loop.
Thanks
#9
05/12/2010 (4:44 pm)
thats a way too as long as the simset is not too large or alike.

The general idea is just to decouple the removal from the updating so it does not interfer the consistency of the set. I guess there are even other ways. For example safeDelete theoretically should work too depending on the case but normally one would want to recycle most stuff so a solution that works with the pooling might be favorable