Iterating over and removing elements from a Vector

Question

I'm trying to iterate over a vector of stored std::unique_ptr<Enemy>'s to check if they're colliding with the player's attack. If they are, I want to delete the Enemy from the vector. However when I do this, I get the error:

File: c:\program files (x86)\microsoft visual studio 14.0\vc\include\vector Line: 102 Expression: vector iterator not incrementable

Here's the code:

if(actionCnt > 0)
{
    sf::FloatRect* playerAction = player->action(actionCnt);

    if (!enemies.empty()) {
        for (auto&i : enemies) {
            if (playerAction->intersects(i->getBox())) {
                i->kill();
                enemies.erase(std::remove(enemies.begin(), enemies.end(), i));
            }
        }
    }

    delete playerAction;
    playerAction = NULL;
}

So essentially, if the player triggers an attack, actionCnt goes to 1 and this segment gets executed.

playerAction is a rectangle that gets moved to simulate a swing, so here I'm checking to see if the rectangle collides with an enemy which would kill it if so.

I'm using SFML libraries, but it should be obvious what the classes do.

Right now I only have 1 enemy on the field for testing this stuff. So when I hit it, the enemies vector should be empty.

Answer 1

The auto in your code will return the type inside the vector container. It will not give you the iterator. Instead of a range-based for loop, you can use a classic for loop and erase your element with the iterator.

if (!enemies.empty())
{
    for (auto itr = std::begin(enemies); itr!=std::end(enemies); ++itr)
    {
       if ((*itr)->hit)
       {
          (*itr)->kill();
          enemies.erase(itr);
       }
    }
}

Or you can a lambda function inside the std::remove_if and do the whole thing in one line. This is actually known as the erase-remove idiom (more here). In a nutshell, the remove_if shifts (moves) all the elements in the container that return false from the lambda function in a preserve-the-ordering fashion, and returns an iterator to the one past the last element. The elements that returned true from the lambda function are moved to the end of the vector and are still dereferencable, yet undefined. To erase the elements completely, the vector's erase function is called to delete all the elements from the logical end (iterator from std::remove_if) to the physical end (vector.end()) of the container.

if (!enemies.empty())
   enemies.erase(std::remove_if(enemies.begin(),enemies.end(),[](const auto &itr){return itr->hit;}),std::end(enemies)); 

Furthermore, the ranged-based for loop is used for when you wan to perform an action to your elements without mutating the container itself. For a safer code, you can use a while-loop instead of a range-based for-loop.

if (!enemies.empty())
{
    auto itr = std::begin(enemies); 
    while (itr != std::end(enemies) )
    {
        if ((*itr)->hit)
        {
           (*itr)->kill();
           enemies.erase(itr);
        }
        ++itr;
    } 
}

You can have a look at the code here: http://rextester.com/EHLB11709 I tried to imitate your game logic focusing only on the delete part. From the output you will see that the 6th element has been deleted.

Furthermore, if you want to remove everything from your vector, you should use clear().