0

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.

Ausche
  • 139
  • 2
  • 12
  • 5
    Whether this is the problem or not, trust me you don't want to `erase` while iterating with a ranged `for`. – user4581301 Nov 12 '16 at 18:16
  • Possible duplicate of [Erasing elements from a vector](http://stackoverflow.com/questions/347441/erasing-elements-from-a-vector) – LogicStuff Nov 12 '16 at 18:17
  • So I looked at the thread @LogicStuff posted and I'm attempting to put it in a while loop, however I'm having trouble referencing an element in the `enemies` vector using the iterator For instance: `playerAction->intersects(*iter->getBox())` gives me an error saying `"std::unique_ptr>" has no member "getBox"` – Ausche Nov 12 '16 at 18:51
  • You need `(*iter)->getBox()` – Igor Tandetnik Nov 12 '16 at 19:04

1 Answers1

3

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().

Constantinos Glynos
  • 2,952
  • 2
  • 14
  • 32