Pushing five object pointers in one scope, then checking if the bool of the object is false, gives an error

Question

I am working on a game for my University project and came across a problem with pointers. I have a vector of bullets that has every bullet checked for collisions. But, when I try to spawn 5 bullets at once (shotgun shot), it spawns everything correctly, then I go through the vector and check if the bullet is alive. For some reason it finds a NULL pointer or a corrupted data of a pointer. Would be really appreciated if someone knew how to fix this. Thanks!

Bullet check:

void Bullet::checkOutOfRange()
{
    if(type == "enemy_bullet")
    {
        if(body->GetPosition().x <= -10.0f)
        {
            alive = false;
        }
    }
    // checks if the body is out of the window size
    else if (body->GetPosition().x > 510.0f || body->GetPosition().x < -10.0f || body->GetPosition().y > 291.0f || body->GetPosition().y < -10)
    {
        alive = false;
    }
}

Bullet collision detection:

class MyContactListener : public b2ContactListener
{
    void BeginContact(b2Contact* contact) override
    {
        if (contact && contact->IsTouching())
        {
            Entity* A = static_cast<Entity*>(contact->GetFixtureA()->GetBody()->GetUserData());
            Entity* B = static_cast<Entity*>(contact->GetFixtureB()->GetBody()->GetUserData());


            //------
            if((A->type == "player_bullet" && B->type == "enemy") || (A->type == "enemy" && B->type == "player_bullet"))
            {
                Bullet *bullet = nullptr;
                Enemy *other = nullptr;
                if (A->type == "player_bullet" && B->type == "enemy")
                {
                    bullet = dynamic_cast<Bullet*>(A);
                    other = dynamic_cast<Enemy*>(B);
                }
                else if (A->type == "enemy" && B->type == "player_bullet")
                {
                    bullet = dynamic_cast<Bullet*>(B);
                    other = dynamic_cast<Enemy*>(A);
                }
                if ((A && B) || (B&&A)) 
                {
                    other->bodySprite.set_animation("death");
                    bullet->alive = false;
                    other->alive = false;
                }
            }
        }
    }
}

Bullet check in game:

    for (auto itr = player_bullets.begin(), itrEnd = player_bullets.end(); itr != itrEnd; ++itr)
    {
        (*itr)->checkOutOfRange(); // go to declaration to see where the bullets stop
        (*itr)->bodySprite.set_position((*itr)->body->GetPosition().x, (*itr)->body->GetPosition().y);
        (*itr)->bodySprite.set_rotation((*itr)->bulletDir * 180 / PI);
        (*itr)->moveBullet(20.0f);
        if ((*itr)->alive == false)
        {
            input_world->DestroyBody((*itr)->body);
            delete *itr;
            itr = player_bullets.erase(itr);
        }
    }

Shotgun shot spawn:

void spawnShotgunShot(float x, float y, float dir, b2World *world)
{
    // maybe bad pointer management?
    Bullet *bullet_ptr1 = new Bullet(x, y, dir, world, "player_bullet", PLAYER_BULLET, ENEMY);
    player_bullets.push_back(bullet_ptr1);
    Bullet *bullet_ptr2 = new Bullet(x, y, dir + 0.5, world, "player_bullet", PLAYER_BULLET, ENEMY);
    player_bullets.push_back(bullet_ptr2);
    Bullet *bullet_ptr3 = new Bullet(x, y, dir + 0.25, world, "player_bullet", PLAYER_BULLET, ENEMY);
    player_bullets.push_back(bullet_ptr3);
    Bullet *bullet_ptr4 = new Bullet(x, y, dir - 0.5, world, "player_bullet", PLAYER_BULLET, ENEMY);
    player_bullets.push_back(bullet_ptr4);
    Bullet *bullet_ptr5 = new Bullet(x, y, dir - 0.25, world, "player_bullet", PLAYER_BULLET, ENEMY);
    player_bullets.push_back(bullet_ptr5);
}

And here is a testbuild with the sln in it: Link to testbuild

score 0 · Answer 1 · edited May 23 '17 at 12:02

Your problem is your itrEnd variable in the loop that checks the bullets. You initialize it to player_bullets.end() at the beginning of the loop, then then check itr != itrEnd as the loop condition. But when you erase a bullet from the vector, that invalidates iterators after the one that was erased, including the end iterator. You're continuing to use the vector's original end iterator, instead of its new one based on its smaller size.

Remove the itrEnd variable and just check itr != player_bullets.end() as the loop condition. That'll ensure that you're always using the "current" end iterator, even when it changes due to elements being erased.

On a related note, when you erase an element from the vector, the iterator that you get back from the erase call is for the element after the one that was erased. At the beginning of the next iteration, the loop's ++itr will advance past that element before checking the loop condition again. That means that whenever you remove a bullet from the vector, your loop will "skip" the bullet after it. If the last bullet gets removed, the loop will "skip" the end iterator and continue iterating past the end of the vector.

See this SO question for information on how to fix that.

Pushing five object pointers in one scope, then checking if the bool of the object is false, gives an error

1 Answers1