C++ Segmentation fault when erasing from vector

Question

NOT A DUPLICATE OF iterate vector, remove certain items as I go , since I have tried the solution used in that issue!

So I am making a game engine where I have a class called "Scene" which has a vector of game objects.

The scene has a function called "destantiate" which should be used to "delete" game objects.

When I call this function, I get Segmentation fault when trying to use the std::vector::erase function.

I think it might have to do with another loop iterating the vector and it is trying to access a pointer in the vector which has been erased?

Take a look at the code below:

#include "Scene.h"
#include "Instance.h"


Scene::Scene() {
    this->camera = new Camera(0, 0);
}

void Scene::instantiate(Instance *instance) {
    this->instances->push_back(instance);
}

void Scene::destantiate(Instance &instance) {
    instance.trash = true;
}

void Scene::tick(float delta) {
    this->camera->tick(delta);
    for(std::vector<Instance*>::iterator it = this->instances->begin(); it != this->instances->end(); ++it) {
        if ((*it)->trash) {
            std::vector<Instance*>::iterator position = std::find(
            this->instances->begin(),
            this->instances->end(),
            (*it)
            );

            if (position != this->instances->end()) {
                this->instances->erase(position);
            }

            continue;
        }
        (*it)->collisionBox->width = (*it)->sprite->getCurrentImage()->getWidth();
        (*it)->collisionBox->height = (*it)->sprite->getCurrentImage()->getHeight();
        (*it)->tick(delta);
    }
}

void Scene::draw(float delta) {
    this->camera->draw(delta);
    for(std::vector<Instance*>::iterator it = this->instances->begin(); it != this->instances->end(); ++it) {
        if ((*it)->trash) { continue; }
        glPushMatrix();

        if ((*it)->centeredOrigo) {
            glTranslatef((*it)->x - (*it)->sprite->getCurrentImage()->getWidth()/2, (*it)->y - (*it)->sprite->getCurrentImage()->getHeight()/2, 0.0f);
        } else {
            glTranslatef((*it)->x, (*it)->y, 0.0f); 
        }

        if ((*it)->centeredOrigo) {
            glTranslatef(((*it)->sprite->getCurrentImage()->getWidth()/2), ((*it)->sprite->getCurrentImage()->getHeight()/2), 0);
        }

        glRotatef((*it)->rotation, 0.0f, 0.0f, 1.0f);

        if ((*it)->centeredOrigo) {
            glTranslatef(-((*it)->sprite->getCurrentImage()->getWidth()/2), -((*it)->sprite->getCurrentImage()->getHeight()/2), 0);
        }

        (*it)->draw(delta);

        glPopMatrix();
    }
}

The std::vector::erase function is being called in the "tick" function where it checks if an object has the "trash" flag.

This is where the "destantiate" function is being called in runtime, inside of a game object class:

#include "SDLOpenGL.h"
#include "TestObj.h"


TestObj::TestObj(float x, float y) : Instance(x, y) {
    this->sprite->addImage(game.loader->load("assets/card.png"));
    this->centeredOrigo = true;
}

void TestObj::tick(float delta) {
    if (this->trash) { return; }
    //this->x = game.getMousePosition().x;
    //this->y = game.getMousePosition().y;
    this->rotation += 2.0f;

    if (game.keyboardDown(SDL_SCANCODE_LEFT)) {
        this->x -= 9.5f;
    }
    if (game.keyboardDown(SDL_SCANCODE_RIGHT)) {
        this->x += 9.5f;
    }
    if (game.keyboardDown(SDL_SCANCODE_UP)) {
        this->y -= 9.5f;
    }
    if (game.keyboardDown(SDL_SCANCODE_DOWN)) {
        this->y += 9.5f;

        //Segmentation fault
        game.getCurrentScene()->destantiate(*this);
    }
}

void TestObj::draw(float delta) {
    if (this->trash) { return; }

    this->sprite->draw(delta);
    this->collisionBox->draw(delta);
}

Output:

Segmentation fault: 11

And valgrind says something about "use of uninitialized pointer"

Answer 1

I think you misunderstood what I meant. I'll try to explain with code:

void deinstantiate(GameObject go) {
    flaggedForDeletion.push_back(go);
}

void tick(float delta) {
    for(auto it = gameObjects.begin(); it != gameObjects.end(); ++it) {
        it->tick(delta);
    }

    for(auto it = flaggedForDeletion.begin(); it != flaggedForDeletion.end(); ++it) {
        std::remove(vec.begin(), vec.end(), *it);
    }
}

So you just store which objects you want to remove and remove them afterwards. You cannot remove them while iterating over them because it would invalidate your iterator.

Except this solution works for you.