[SFML & C++] vector erase error?

Started by
9 comments, last by phil_t 8 years, 7 months ago

So I am trying to erase the bullet after some time but when I try to run my code, I get an error, the problem is something to do with projArray.erase(iter2); cant figure out why, but here is my code, any help would be great.


	projCounter = 0;

	// Iterate over the projectile
	for (iter = projArray.begin(); iter != projArray.end(); iter++){
		projArray[projCounter].rect.move(std::cos(PI * mouseAngle / 180.f) * proj.movementSpeed *-1, std::sin(PI * mouseAngle / 180.f) * proj.movementSpeed *-1);
		proj.bulletLifetime++;

		projCounter++;
	}

	projCounter = 0;

	// Delete Projectiles
	for (iter2 = projArray.begin(); iter2 != projArray.end(); iter2++){
		if (projArray[projCounter].destroy == true){
			projArray.erase(iter2);
			break;
		}

		projCounter++;
	}

	//proj.bulletLifetime++;
	if (proj.bulletLifetime >= proj.lifeTime){
		proj.destroy = true;
	}
Advertisement
It would really help if you also show the type of projArray, iter2, and explain what error you get, and when.

(does the application crash? does it give any form of indication of the error? does the compiler crash? does the compiler refuse to compile it? what is its error message?)
For some cases, operating system, and used compiler may also be useful.


Edit:
Under the assumption you're erasing from a vector: http://en.cppreference.com/w/cpp/container/vector/erase
says you cannot use the iterator anymore after an erase. Maybe that's the problem????

It would really help if you also show the type of projArray, iter2, and explain what error you get, and when.

(does the application crash? does it give any form of indication of the error? does the compiler crash? does the compiler refuse to compile it? what is its error message?)
For some cases, operating system, and used compiler may also be useful.


Edit:
Under the assumption you're erasing from a vector: http://en.cppreference.com/w/cpp/container/vector/erase
says you cannot use the iterator anymore after an erase. Maybe that's the problem????

My error: https://gyazo.com/14abfaa95c98310dabbe42750078d0e5

doesnt let me compile

std::vector<Projectile> projArray;
std::vector<Projectile>::const_iterator iter;
std::vector<Projectile>::const_iterator iter2;

Vectors require several things to be defined in order to perform these operations. I'm fairly sure it needs this in case of erase to reconstruct the array internally. That would mean that at least the assignment, '=', operator needs to be accessible, but the vector class may also need for example the copy constructor (http://stackoverflow.com/a/11309573).

Have you defined a custom assignment operator that is not public?

Not so sure about the other errors regarding a certain constructor though; have you possibly added/removed a parameter from the constructor of the ContextSettings class?

std::vector::const_iterator iter2;

This is your problem. "const_iterator" means the iterator will not change its container, which obviously is not true when erasing elements from it

The C++98/03 standard's STL implementation of vector::erase() takes only iterator; const_iterator overload has been added in the C++11 standard. However, at least MSVC's implementation has had a const_iterator overload already in VS 2008 (maybe even earlier), which has been a classic gotcha when developing cross-platform C++98/03 projects.


This is your problem. "const_iterator" means the iterator will not change its container, which obviously is not true when erasing elements from it

I thought it meant you couldn't change the value that the iterator points to. Clearly you can change the container, as vector::erase takes a const_iterator.

At any rate, the OP's error seems to indicate he doesn't have an assignment operator, which would be necessary to erase items from a vector.

This is your problem. "const_iterator" means the iterator will not change its container, which obviously is not true when erasing elements from it


I thought it meant you couldn't change the value that the iterator points to. Clearly you can change the container, as vector::erase takes a const_iterator.

At any rate, the OP's error seems to indicate he doesn't have an assignment operator, which would be necessary to erase items from a vector.

You are fully correct here, thanks for pointing that out. const_iterator means "no change through the iterator", const std::vector means "no change to the vector".
You may or may not get away with it but you should have iter2 = projArray.erase(iter2); as the iter could and should go out of scope. Wait you seem to have an iterator in both your loops and dont use it to access your vector which i dont understand as you make a variable too and access the vector in a c array way

Wait you seem to have an iterator in both your loops and dont use it to access your vector which i dont understand as you make a variable too and access the vector in a c array way

The comment above the "iter2" loop is incorrect, it removes only one projectile due to the "break".
If the OP really aims to erase one projectile only, the code is fine (although a bit clumsy with so many indices as you inidcated), only its performance is going to be horrible for more than a handful of projectiles.

you should have iter2 = projArray.erase(iter2); as the iter could and should go out of scope.

If the OP aimed to erase more than one projectile in the loop (by removing the "break" or so), the problem you mentioned pops up as you say. After "erase(iter2)", the "iter2" value has become invalid, and may not be used any more, for example "iter2++" in the posted code is undefined after an erase, and may produce nonsense.
Your suggested "iter2 = projArray.erase(iter2);" should fix the undefinedness problem, but then the "iter2++" must be moved into the "else" branch of the "if" condition as well, or you'll skip some projectiles.
Erasing more than one projectile from the vector makes the performance even more horrible.

@Elit3d:
A std::vector keeps a contiguous list of elements. That means, if you have a vector of 100 projectiles, and you erase the 5th element, 94 projectiles get copied to fill the gap (projectile 6 to 5, projectile 7 to 6, etc etc upto projectile 99 to 98). So you're not erasing one projectile, you're copying the next 94 instead.
Now imagine that in the next iteration, you find the next projectile should also be erased. You again copy everything after it, shifting again one position. Now if you had computed this before, you can save 50% cpu time, by moving all projectiles 2 places instead of 2 times 1 place. As you remove more projectiles in one sweep, this problem gets worse.

So you REALLY want to avoid erase in the middle of std::vector. There are several ways around this.

You can rebuild a vector while erasing elements from it, by moving the non-erased elements to the front of the vector. It's a fun exercise, with a few gotchas though.
In addition, when you do erase something, many elements do still get copied, which is still bad performance-wise.

Some other ideas on how to avoid copying:
  • If you don't care about the order of the projectiles, don't erase a projectile, but copy the last one in the vector to the empty space, and erase the last one from the vector
  • Use a data structure that can quickly remove elements, like std::list. However, depending on how you use the projectile vector, this may be a bad idea. For example, indexing into a list has horrible performance, so you must avoid that if you switch to a list data structure.

This topic is closed to new replies.

Advertisement