Vector iterator incompatible after push_back() of a new object

Question

I have an Entity class that all objects (Missiles, Spacesship, Stars, Explosions) inherit from and whenever instances of these classes are created, Entity's constructor adds 'this' to its vector list of all instances, just like this:

std::vector<Entity*> Entity::instances;
Entity::Entity()
{
    instances.push_back(this);
}

This worked fine, because I could just loop through every instance and execute its method via e->Render() or e->Update() etc.

What is weird is, it works everywhere else but in one specific place. It works here: Core.cpp

for (int i = 0; i < 128; i++)
{
    stars[i] = new Star();
    stars[i]->position.x = rand() % (screenWidth + 1);
    stars[i]->position.y = rand() % (screenHeight + 1);
    stars[i]->angle = rand() % 360;
    stars[i]->boundingbox.w = 2 + rand() % 7;
    stars[i]->boundingbox.h = stars[i]->boundingbox.w;
}

for (int i = 0; i < 16; i++)
{
    meteorites[i] = new Meteorite();
    meteorites[i]->Spawn();
}

BUT here .. it pops 'vector iterators incompatible'

Meteorite.cpp

void Meteorite::TakeDamage()
{
    health -= 5;
    if (health <= 0)
    {
        Missile* explo = new Missile();
        explo->position.x = position.x;
        explo->position.y = position.y;

        Spawn();
    }
}

I'm completely clueless - it's not as if I was creating these elements in a different than the usual way.

#EDIT I think this is also important, Meteorite::Update() that detects collision and runs TakeDamage() where the instance is created:

for each (Entity* e in Entity::instances)
{
    if (e == this)
    {
        continue;
    }

    if (e->entClass == "ENTITY_MISSILE")
    {
        Missile* m = (Missile*)e;
        if (abs(position.x - m->position.x) * 2 < (boundingbox.w + m-
                >boundingbox.w))
        {
            if (abs(position.y - m->position.y) * 2 < (boundingbox.h + m-
                    >boundingbox.h))
            {
                TakeDamage();
                break;
            }
        }
    }
}

Answer 1

Note that when you call push_back on a std::vector, then iterators get invalidated (emphasis mine):

If the new size() is greater than capacity() then all iterators and references (including the past-the-end iterator) are invalidated. Otherwise only the past-the-end iterator is invalidated.

So in your for loop when TakeDamage gets called, and you construct a Missile (which subsequently calls Entity::Entity() and instances.push_back(this)), you are invalidating your iterators.

You either need to loop via indices instead of iterators, or keep track of which objects need TakeDamage() called on them and call them after you're finished iterating, or create a copy of instances that you iterate on.

Note that I've never seen the syntax for each (... in ...) but presumably it's using iterators.

---

You mention that the code in the question is ran within Meteorite::Update, and you also mention that

This worked fine, because I could just loop through every instance and execute its method via e->Render() or e->Update() etc.

It's VERY likely you're iterating over all instances, calling Update, and within that calling TakeDamage which is invalidating your instances. Calling push_back within TakeDamage while iterating over instances is definitely your issue.

Answer 2

try: for(Entity* e : Entity::instances) instead of your for each thing