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\$
2
\$\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\$ – Lars Viklund Apr 3 '17 at 13:35
  • \$\begingroup\$ @LarsViklund Oh, ok, I'll remove the link. \$\endgroup\$ – Vaillancourt Apr 3 '17 at 13:35

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.