How do I get a destructor on an object in a vector not to throw a failed assertion?

Question

I'm programming a Breakout game in C++. I'm having a HUGE problem that's preventing me from giving the game multi-ball functionality. I think it has something to do with the destructor. Have a look:

for loop for the balls (Driver.cpp):

for (Ball& b : balls) { // Loops over all balls
    (...)
    // Collision for when you miss
    if (b.getYPos() > HEIGHT) { // If the ball falls below the defined height of the screen
    balls.erase(balls.begin() + b.getID()); // Wipe the ball out of memory to make room (Troublesome line)
    Ball::addToID(-1); // Shift the ball ID to assign to the next ball back one
    (...)
}

And I get this error:

Debug Error!
Program: Breakout.exe
abort() has been called
(Press Retry to debug the application)

Do you know why this mysterious crash is happening? Or more importantly, a fix for it?

Here's a replicable piece of code to help:

Driver.cpp:

#include <vector>
#include <allegro5\allegro.h>
#include "Ball.h"

using namespace std;

vector<Ball> balls(0); // Pay attention to this line

const POS WIDTH = 1600, HEIGHT = 900;


int main {
    while (running) {
        if (al_key_down(&key, ALLEGRO_KEY_SPACE)) { // Spawn the ball
                balls.push_back(Ball(WIDTH / 2, 500, 10, 10)); // Spawn the ball
                balls[Ball::getIDtoAssign()].setYSpeed(5);
            }

            for (Ball& b : balls) { // Pay attention to this loop
                b.draw(); // This line is what's crashing.
                b.move();

                (...)
                // Collision for when you miss
                balls.erase(
                    remove_if(balls.begin(), balls.end(),
                        [=](Ball& b) {
                            // Collision for when you miss
                            return b.getYPos() > HEIGHT; // If the ball falls below the defined height of the screen, wipe the ball out of memory to make room
                        }
                    ),
                    balls.end()
                            );
                }
            }
        }
    }
    return 0;
}

Ball.h:

#pragma once
#include <allegro5\allegro_primitives.h>

using namespace std;

class Ball {
public:
    Ball();
    Ball(float x, float y, float w, float h);
    ~Ball();
    void draw();
    void move();
    float getYPos();
    void setYSpeed(float set);
private:
    float xPos; // Horizontal position
    float yPos; // Vertical position (upside down)
    float width; // Sprite width
    float height; // Sprite height
    float xSpeed; // Horizontal speed
    float ySpeed; // Vertical speed (inverted)
}

Ball.cpp:

#include "Ball.h"

short Ball::ballIDtoAssign = 0;

Ball::Ball() {
    this->xPos = 0;
    this->yPos = 0;
    this->width = 0;
    this->height = 0;
    this->xSpeed = 0;
    this->ySpeed = 0;
}

Ball::Ball(float x, float y, float w, float h) {
    this->xPos = x;
    this->yPos = y;
    this->width = w;
    this->height = h;
    this->xSpeed = 0;
    this->ySpeed = 0;
}

Ball::~Ball() {
    // Destructor
}

void Ball::draw() {
    al_draw_filled_rectangle(xPos, yPos, xPos + width, yPos + height, al_map_rgb(0xFF, 0xFF, 0xFF));
}

void Ball::move() {
    xPos += xSpeed;
    yPos += ySpeed;
}

float Ball::getYPos() {
    return yPos;
}

void Ball::setYSpeed(float set) {
    ySpeed = set;
}

Answer 1

You cannot modify a container while you are iterating through it with a range-for loop. You don't have access to the iterator that the loop uses internally, and erase() will invalidate that iterator.

You can use the container's iterators manually, paying attention to the new iterator that erase() returns, eg:

for(auto iter = balls.begin(); iter != balls.end(); ) { // Loops over all balls
    Ball& b = *iter: 
    ...
    // Collision for when you miss
    if (b.getYPos() > HEIGHT) { // If the ball falls below the defined height of the screen
        ...
        iter = balls.erase(iter); // Wipe the ball out of memory to make room
    }
    else {
        ++iter;
    }
}

Alternatively, use the erase-remove idiom via std::remove_if() instead:

balls.erase(
    std::remove_if(balls.begin(), balls.end(),
        [=](Ball &b){
            // Collision for when you miss
            return b.getYPos() > HEIGHT; // If the ball falls below the defined height of the screen, wipe the ball out of memory to make room
        }
    ),
    balls.end()
);

---

UPDATE: now that you have posted more of your code, it is clear to see that you are trying to use ID numbers as indexes into the vector, but you are not implementing those IDs correctly, and they are completely unnecessary and should be eliminated.

The Ball::ballID member is never being assigned any value, so in this statement:

balls.erase(balls.begin() + b.getID()); // The troublesome line

Trying to erase() the result of balls.begin() + b.getID() causes undefined behavior since the iterator has an indeterminate value, thus you can end up trying to erase the wrong Ball object, or even an invalid Ball object (which is likely the root cause of your runtime crash).

Also, in this section of code:

balls.push_back(Ball(WIDTH / 2, 500, 10, 10)); // Spawn the ball
balls[Ball::getIDtoAssign()].setYSpeed(5);
Ball::addToID(1);

Since you want to access the Ball object you just pushed, that code can be simplified to this:

balls.back().setYSpeed(5);

And I already gave you code further above to show you how to remove balls from the vector without using IDs.

So, there is need for an ID system at all.

With that said, try something more like this:

Driver.cpp:

#include <vector>
...
#include "Ball.h"
using namespace std;

vector<Ball> balls;
const POS WIDTH = 1600, HEIGHT = 900;

int main {
    ...

    while (running) {
        ...
        
        if (input.type == ALLEGRO_EVENT_TIMER) { // Runs at 60FPS
            ...

            if (al_key_down(&key, ALLEGRO_KEY_SPACE)) { // Spawn the ball
                balls.push_back(Ball(WIDTH / 2, 500, 10, 10)); // Spawn the ball
                balls.back().setYSpeed(5);
            }

            for (auto iter = balls.begin(); iter != balls.end(); ) {
                Ball &b = *iter;
                ...

                if (b.getYPos() > HEIGHT) { // Collision for when you miss
                    iter = balls.erase(iter);
                }
                else {
                    ++iter;
                }
            }

            /* alternatively:

            for (Ball& b : balls) {
                b.draw();
                b.move();
            }

            balls.erase(
                std::remove_if(balls.begin(), balls.end(),
                    [=](Ball &b){
                        // Collision for when you miss
                        return b.getYPos() > HEIGHT; // If the ball falls below the defined height of the screen, wipe the ball out of memory to make room
                    }
                ),
                balls.end()
            );
            */
        }
    }
    return 0;
}

Ball.h:

#pragma once
...

class Ball {
public:
    ...
    // NO ID METHODS!

private:
    ...
    // NO ID MEMBERS!
}

Ball.cpp:

#include "Ball.h"

...

// NO ID MEMBER/METHODS!

Answer 2

OK, so I managed to figure out why the program crashes. It was because I had the erase-remove inside the for loop which can cause all sorts of problems.