0

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"

Community
  • 1
  • 1
Sebastian Karlsson
  • 715
  • 1
  • 8
  • 19
  • yes, you cannot mutate the vector while iterating over it. But you shouldn't have such a trash field. You probably need a second vector storing the objects that should be deleted. Another option would be to operate on a copy of the vector, but I wouldn't recommend that. – torkleyy Jan 07 '17 at 12:00
  • Did you try [this](http://stackoverflow.com/questions/1604588/iterate-vector-remove-certain-items-as-i-go/1604632#1604632)? – torkleyy Jan 07 '17 at 12:03
  • @User9182736455 hmm storing objects that should be deleted sounds interesting. But let's say I am iterating over it (the "delete-vector") and deletes things from the main vector. Wouldn't iterators that are iterating over the main vector break and also cause a segmentation fault? – Sebastian Karlsson Jan 07 '17 at 12:05
  • So basically, you are creating another iterator that points to the same element, using `std::find`, erasing through that *other iterator*, hoping that the *initial iterator* will remain valid? No, it doesn't work that way. – A.S.H Jan 07 '17 at 12:05
  • @A.S.H So how would I do this correctly in a way that works? – Sebastian Karlsson Jan 07 '17 at 12:06
  • replace that whole useless loop with a `erase-remove idiom`. Plenty of examples on SO. and yes, this question *is* a duplicate. – A.S.H Jan 07 '17 at 12:07

1 Answers1

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.

Community
  • 1
  • 1
torkleyy
  • 1,137
  • 9
  • 26