CODEWITHSUNDEEP
c++
thomas1413
thomas1413

Reputation: 67

Nested loops, deleting from vectors

I am trying to make my simple game and I came across small issue. So I have 2 vectors of pointers. One is for bullets and one is for enemy units. First I tried to iterate through all of my bullets and then in second loop iterate through my enemies and when I find collision I erase enemy unit and bullet, but erasing bullets is crashing my game, so I figured out that I shouldnt erase from vector while its still iterating in first loop.

My second ide was something like this:

std::vector<Bullet*>::iterator remove = std::remove_if(bullets.begin(), bullets.end(),
        [&](Bullet* x)
    {
        for (std::vector<EnemyUnit*>::iterator it = enemyUnits.begin(); it != enemyUnits.end(); ++it)
        {
            if (x->collision->CheckCollision((*it)->collision))
            {
                enemyUnits.erase(it);

                return true;
                
            }
            else
            {
                return false;
            }
        }
    });
    bullets.erase(remove, bullets.end());

So it seems to work in a way, but my bullets can only collide with one enemy at a time and it seems like its not checking for all enemies. For example I shoot 5 bullets and firstly they can only collide with one enemy and after one bullet kills enemy other bullets will be able to collide with second enemy and so on . It seems like

for (std::vector<EnemyUnit*>::iterator it = enemyUnits.begin(); it != enemyUnits.end(); ++it)

Doesnt take place and it only gets one enemy? Is it wrong way to do it? I need to use pointers and vectors, because its for my project.

Upvotes: 2

Views: 199

Answers (2)

Mark Taylor
Mark Taylor

Reputation: 1851

Answer advising to remove the return statements is correct. The return statements are exiting the inside loop early, but that isn't the only problem. Removing those returns will reveal the next problem. You need:

it = enemyUnits.erase(it);

to correctly set the iterator after an erase to prepare for the next pass through the loop.

Upvotes: 1

Olaf Dietsche
Olaf Dietsche

Reputation: 74018

The predicate for std::remove_if returns after the first check, no matter what.

To fix this, the predicate can return on a collision, or at the end (after the loop) on no collision, e.g.

for (auto it = enemyUnits.begin(); it != enemyUnits.end(); ++it)
{
    if (x->collision->CheckCollision((*it)->collision))
    {
        enemyUnits.erase(it);
        return true;
    }
}

return false;

This way, if a collision is detected, it will exit early and return true. If there's no collision with any enemy, it will stop after the loop and return false.

Upvotes: 3

Related Questions