Sign in to follow this  

[SFML & C++] vector erase error?

This topic is 819 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

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;
	}

Share this post


Link to post
Share on other sites
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???? Edited by Alberth

Share this post


Link to post
Share on other sites

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;

Share this post


Link to post
Share on other sites

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?

Edited by AthosVG

Share this post


Link to post
Share on other sites

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.

Edited by Stinkfist

Share this post


Link to post
Share on other sites


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.

Share this post


Link to post
Share on other sites

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".

Share this post


Link to post
Share on other sites
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

Share this post


Link to post
Share on other sites

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.

Share this post


Link to post
Share on other sites


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.

 

The standard library has built-in functionality for this. The OP can accomplish the task of removing all projectiles which have destroy set to true using the following code:

    projArray.erase(
        std::remove_if(projArray.begin(),
        projArray.end(),
        [](const ProjArray &proj) { return proj.destroy; }
        ),
        projArray.end());

(this requires c++11 though)

 

remove_if re-arranges the items according to the predicate, then returns the iterator to the first one that needs to be removed. Then erase actually deletes them from the container.

 


Use a data structure that can quickly remove elements, like std::list.

 

I'd hesitate to recommend this by default. You'd avoid copying when deleting items, but you'll be touching non-contiguous memory as you iterate through it, so any iteration operations (such as figuring out which ones to delete) will be much slower.

Share this post


Link to post
Share on other sites

This topic is 819 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this