for (auto enemy : this->enemies)
{
    if (enemy->getHP() <= 0)
    {
        enemies.erase(enemy);
    }
}

I have a vector enemies containing multiple of Enemy* elements and i want to erase an enemy if their hp is 0 or below

I write the code above and it gave me this error message:

No instance of overloaded function "std::vector<_Ty, _Alloc>::erase [with _Ty=Enemy *, _Alloc=std::allocator<Enemy *>]" matches the argument list
    argument types are: (Enemy*)
    object type is: std::vector<Enemy*,std::allocator<Enemy*>>

I assume that is not the right way to do it, so how?

Im new in stackoverflow and im still learning english so sorry if i made mistakes

EDIT:

It's my almost complete code:

struct enemyType
{
public:
    int type;
    sf::Vector2f pos;
}
std::vector<std::vector<enemyType>> enemyList = {
    {
        { trashMonster, sf::Vector2f(5.f * 16, 18.f * 16) }
    }
}
std::vector<Enemy*> enemies;
std::vector<Enemy*>* GetEnemy(int level)
{
    for (int i = 0; i < enemyList[level].size(); i  )
    {
        switch (enemyList[level][i].type)
        {
        case trashMonster:
            n_TrashMonster->setPosition(enemyList[level][i].pos);
            enemies.emplace_back(n_TrashMonster);
            break;
        default:
            std::cout << "Error to get an enemy\n";
            break;
        }
    }
    return &enemies;
}
//Code in different file
std::vector<Enemy*> enemies;
this->enemies = *GetEnemy(lvl);
for (auto enemy : this->enemies)
{
    enemy->update(player->getCollisionBox());
    //collision enemies to tilemap
    collision::MapCollision(*this->map.getTilesCol(), *enemy);
    if (enemy->getHP() <= 0)
    {
        enemies.erase(enemy);
    }
}

Didn't include that because my code is a complete mess so I was afraid people won't get the point of my question and it's my first question here

CodePudding user response：

The std::vector<T>::erase function does not have a erase(T a) overload. And if you want to remove elemennts from a vector you can't iterate over them like that. I suggest a convencional loop.

for (size_t i=0; i<this->enemies.size();  i){
    if (this->enemies[i]->getHP()){
        std::swap(enemies[i],enemies::back());
        delete enemies::back();//Only if you don't free the space elsewere
        enemies.pop_back();
    }
}

Edit: This will mess up the order of the vector. If you don't want that you can use erase(enemies.begin() i) iinstead of swaping it back and removeing it

CodePudding user response：

When using STL containers usually you don't even need an (explicit) for loop. Use std::remove_if like this

#include <vector>
#include <algorithm>
#include <iostream>

class Enemy
{
public:
    // not explicit on purpose, so I can initalize vector more quickly (in real code you should have explicit constructors if they have one argument of a different type)
    Enemy(int hp) : 
        m_hp{ hp }
    {
    };

    int getHP() const noexcept
    {
        return m_hp;
    }

    int m_hp;
};

int main()
{
    // create a test vector with enemies with given hitpoints
    std::vector<Enemy> enemies{ 1,2,0,4,5,6,0,7,8 };

    // boolean lambda function that determines if an enemy is dead.
    auto enemy_is_dead = [](const Enemy& enemy) { return enemy.getHP() <= 0; };

    // use remove_if (this will move all the items to be removed to the end
    auto remove_from = std::remove_if(enemies.begin(), enemies.end(), enemy_is_dead );

    // then shrink the vector
    enemies.erase(remove_from, enemies.end());
    
    for (const auto& enemy : enemies)
    {
        std::cout << enemy.getHP() << " ";
    }

    return 0;
}