0
\$\begingroup\$

I am developing 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

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

std::vector::erase returns a value that points to the next value.

It returns an iterator to the next element. You should not keep a pointer to itrEnd because all iterators after the erased value pointed by the iterator are invalidated.

But...

Since erase returns an iterator to the next element, you should not use ++itr in the 3rd part of your loop definition, because, if you erase an element, you will skip that next element because itr++ is performed at the end of the loop, effectively going to the next element twice.

Instead of using a for loop, here, use a while:

auto itr = player_bullets.begin();
while (itr != player_bullets.end())
{
    // stuff
    if ((*itr)->alive == false)
    {
        input_world->DestroyBody((*itr)->body);
        delete *itr;
        itr = player_bullets.erase(itr);
    }
    else 
    {
        itr++;
    }
}

You update the value of itr yourself, whether the item needs to be erased or not.

\$\endgroup\$
2
  • \$\begingroup\$ I removed my answer as yours is more detailed and covers both the problem points, reducing confusion. \$\endgroup\$ Apr 3, 2017 at 13:35
  • \$\begingroup\$ @LarsViklund Oh, ok, I'll remove the link. \$\endgroup\$
    – Vaillancourt
    Apr 3, 2017 at 13:35

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .