Skip to main content
GameDev.net gamedev.net
🔒 Locked

STL iterator specifics

Started by CyberSlag5k Jun 4, 2006 at 1:36 PM 15 replies 1.1k views
Original Post
CyberSlag5k
CyberSlag5k
I don't have much experience with the C++ STL, so I'm incorporating it into one of my programs to learn. I'm using a vector to store a list of particles generated by a particle fountain. Each time the fountain's update function is called, the update function of each particle is called. If that function returns false (implying it is time for the particle to die), the particle should be removed from the vector: 

bool Fountain::update(const float& seconds)
{
	vector<Particle>::iterator i;
	i = particles.begin();
	while(!particles.empty() && i != particles.end())
	{
		if(!i->update(seconds))
			i = particles.erase(i);
		else
			i++;
	}
}
Rather simple stuff. I declare an iterator and have it point to the first particle. As long as the vector is not empty and i is not pointing to the end of the vector, I call the update function to the particle i is presently pointing to. If that returns false, I remove the particle from the vector and assign i to what it returns (which should be an iterator to the particle just after the one that was removed). If not, I advance the iterator to the next particle. However, something is not quite right. When it comes time for the first particle to die, the entire list gets wiped out. That really shouldn't happen, as each particle has a 3 second lifespan, and new particles are constantly being created, meaning as the first one dies, the one most recently born should have 3 more seconds of life. In doing some tests, I can see that each particle does in fact have a unique life span, and that when the first one does, the call to particles.erase(i) is made many times (implying that the update function is returning false for each element). That's a bit odd, as the update function is based entirely on the particles' lifespan, and again they all seem to have unique values for their time alive. From an STL standpoint, is what I'm doing there what I think I'm doing there, or are my assumptions about the iterator or the erase function off? Perhaps I should switch to a list? I'm not entirely convinced that the problem lies with the use of the vector there, but that's my present suspicion. Does anyone see anything wrong with this? I'll supply more code as needed.
Without order nothing can exist - without chaos nothing can evolve.
Zakwayda
Zakwayda
Strangely enough this same topic was recently discussed here. I don't know that the thread addresses the specific problem you mention, but it might be of interest to you nonetheless.
Zakwayda
Zakwayda
Looking at your code a bit closer, the method you've chosen for managing the particles is probably not the most performant (see the other thread for further discussion). At first glance though, I'm not sure why your code isn't working. I don't think the call to empty() is necessary, but it shouldn't hurt, I don't think. (The problem is probably something obvious that I'm just missing, but it's also possible that the error is elsewhere in your code.)
CyberSlag5k
CyberSlag5k
Thanks Jyk. It was good to read through that thread, gave me a few ideas.

I'm thinking now my copy constructor might be to blame, though it is very simple:

Particle::Particle(const Particle& p){	pos = p.pos;	vel = p.vel;	acc = p.acc;	rot = p.acc;	off = p.off;	offRot = p.offRot;	friction = p.friction;	life_span = p.life_span;	time_left = p.time_left;	alive = p.alive;}


The variable time_left is what determines whether or not update passes or fails. Assuming erase shifts the elements of the vector down to fill in the erased space, I would assume this is what gets called for each member. The thing is, it doesn't get much simpler than time_left = p.time_left, and so the new time_left values of the particles should still get the right values.

Still, switching things over to a list (who's behavior would differ in that it would simply re-link, rather than shift, I would imagine), appears to work properly. That's good enough for my program, but if anyone has any further insight as to why the vector version (and thus my copy constructor, I'd imagine) was in error, I would be very interested to know.

Thanks again, Jyk!
Without order nothing can exist - without chaos nothing can evolve.
Zakwayda
Zakwayda
Still don't see what might have caused the particular problem you mentioned, but I did notice this copy-and-paste error:
rot = p.acc;
Also, from what you posted at least it looks like you could ditch your copy constructor and just use the default (which would eliminate future such errors). That is of course assuming that a shallow copy is appropriate, but that's likely to be the case for a particle class.
CyberSlag5k
CyberSlag5k
A shallow copy is fine, though I was under the impression I had to define my own for whatever reason. Now that I think about it, though, I see no reason why the wouldnt do.

Thanks again, Jyk!

EDIT:
Same problem using the default copy constructor and vectors, but fine with lists. The mystery continues, while I have fun with my oh-so-sexy, working, particle fountains.

[Edited by - CyberSlag5k on June 4, 2006 3:53:55 PM]
Without order nothing can exist - without chaos nothing can evolve.
CyberSlag5k
CyberSlag5k
Nope.
Without order nothing can exist - without chaos nothing can evolve.
mikeman
mikeman
This is weird. If the particular piece of code that handles the particles is not very big, just post the whole thing. The fact that lists work and vectors don't means that maybe there is still a small bug in your program somewhere.
CyberSlag5k
CyberSlag5k
Could be, though honestly I'm not doing a whole lot of dynamic allocation. I have a dynamic matrix of floats for my map (which I'm very careful with), a couple of pointers for a couple of objects (less than 5), and that's about it. I only have 2 static arrays, one for standard key input and one for special key input. So as far as memory collisions go, I'd be pretty amazed if I have one.

Still, ya never know. It could be something entirely different. It's not all that large, but I tore it apart somewhat getting these silly particle fountains working. I'm going to go through and clean it up a bit later tonight, I'll let you guys know if I find anything.

Thanks again for all if your suggestions and help, guys!
Without order nothing can exist - without chaos nothing can evolve.
Fruny
Fruny
Quote:
!particles.empty() && i != particles.end()


The first part of the test is redundant. When the container is empty, begin() == end() and thus i == end().


That aside, your code here is entirely correct, so I guess you should show us the contents of your update() function
"Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it." — Brian W. Kernighan
CyberSlag5k
CyberSlag5k
You'll never make me talk!

Fine...

bool Particle::update(const float& seconds){	Object::update(seconds);	time_left -= seconds;	if(time_left <= 0.0f)		return false;	return true;}


As you can see, there's not really much to see. The object update has nothing to do with time_left, it merely positions the particle and applies velocity and acceleration.

I didn't really know what i would point to if I deleted the last item of the vector, so I thought I'd use the particles.empty() as an added precaution. Is it that both the iterator and particles.end() would point to null if particles were empty, then?
Without order nothing can exist - without chaos nothing can evolve.
mikeman
mikeman
Actually, I am more interested in the code that emits the particles and puts them in the container. You say that, when the first one dies, they all dissapear, even though new particles are created all the time. That means that, after 3 seconds, particles are created and die instantly,ie they are initialized with time_left=0.0 for some reason.
CyberSlag5k
CyberSlag5k
Correct. I was suspicious of that as well, but it too isn't anything special:

void Fountain::addParticle(){// velocity randomization code removed	Particle temp2(pos, part_vel, 20.0f);	temp2.setRot(rot);	temp2.setOff(off);	temp2.setOffRot(offRot);	temp2.setAcc(part_acc);	particles.push_back(temp2);	count++;}


The third parameter in the particle's constructor is the particle's lifespan:

Particle::Particle(const Vector3& p, const Vector3& v, const float& l){	pos = p;	vel = v;	acc = Vector3::Vector3(0.0f, -1.0f, 0.0f);	friction = 1.0f;	life_span = l;	time_left = life_span;	alive = true;}
Without order nothing can exist - without chaos nothing can evolve.
mikeman
mikeman
Man, this is really, really weird. I can't possibly imagine what could be wrong, but obviously something is. It's like a freaking murder mystery! If you feel like it, just post the entire source so we can compile it and see it running :)
Fruny
Fruny
Quote:
Original post by CyberSlag5k
I didn't really know what i would point to if I deleted the last item of the vector, so I thought I'd use the particles.empty() as an added precaution. Is it that both the iterator and particles.end() would point to null if particles were empty, then?


When a container is empty, begin() == end(). What end() points to is always undefined. You cannot assume it is NULL or anything like that.
"Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it." — Brian W. Kernighan
CyberSlag5k
CyberSlag5k
And so i ends up pointing to end as well, because if there were only a single element it would be at position end()-1, which then references the next item, end(), upon erasure...yes?
Without order nothing can exist - without chaos nothing can evolve.

Topic Locked

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

Sign in to reply to this topic.