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.