I'm in the process of making a simple game via SFML and have encountered the following error:
Debug assertation failed! Expression: vector subscript out of range
This error only appears when the 'rocket' makes contact with the first 'enemy' whilst there are 3 enemies on screen at the same time. I understand that it has to do with the fact that I'm deleting elements from a vector in a for loop, but can I get a clear answer as to why this error only occurs when there a 3 or more enemies on screen. Also, how could I rewrite my code so that I can remove the 'enemy' elements from the enemies vector without causing an error? Thanks in advance
for (int i = 0; i < rockets.size(); i++) //This is where the error points to
{
for (int j = 0; j < enemies.size(); j++)
{
Rocket *rocket = rockets[i];
Enemy *enemy = enemies[j];
if (checkCollision(rocket->getSprite(), enemy->getSprite()))
{
//hitSound.play();
score++;
std::string finalScore = "Score: " + std::to_string(score);
scoreText.setString(finalScore);
sf::FloatRect scoreBounds = scoreText.getGlobalBounds();
scoreText.setOrigin(scoreBounds.width / 2, scoreBounds.height / 2);
scoreText.setPosition(viewSize.x * 0.5f, viewSize.y * 0.10f);
rockets.erase(rockets.begin() + i);
enemies.erase(enemies.begin() + j);
delete(rocket);
delete(enemy);
if (score % 5 == 0) levelUp();
printf("rocket intersects with enemy \n");
}
}
}
I have stored the enemy and rocket objects in vectors:
std::vector <Enemy*> enemies;
std::vector <Rocket*> rockets;
Here's my entire update function if that helps:
void update(float dt)
{
hero.update(dt);
currentTime += dt;
if (currentTime >= prevTime + 1.125f)
{
spawnEnemy();
prevTime = currentTime;
}
for (int i = 0; i < enemies.size(); i++)
{
Enemy *enemy = enemies[i];
enemy->update(dt);
if (enemy->getSprite().getPosition().x < 0)
{
enemies.erase(enemies.begin() + i);
delete(enemy);
gameOver = true;
gameOverSound.play();
}
}
for (int i = 0; i < rockets.size(); i++)
{
Rocket *rocket = rockets[i];
rocket->update(dt);
if (rocket->getSprite().getPosition().x > viewSize.x)
{
rockets.erase(rockets.begin() + i);
delete(rocket);
}
}
for (int i = 0; i < rockets.size(); i++)
{
for (int j = 0; j < enemies.size(); j++)
{
Rocket *rocket = rockets[i];
Enemy *enemy = enemies[j];
if (checkCollision(rocket->getSprite(), enemy->getSprite()))
{
//hitSound.play();
score++;
std::string finalScore = "Score: " + std::to_string(score);
scoreText.setString(finalScore);
sf::FloatRect scoreBounds = scoreText.getGlobalBounds();
scoreText.setOrigin(scoreBounds.width / 2, scoreBounds.height / 2);
scoreText.setPosition(viewSize.x * 0.5f, viewSize.y * 0.10f);
rockets.erase(rockets.begin() + i);
enemies.erase(enemies.begin() + j);
delete(rocket);
delete(enemy);
if (score % 5 == 0) levelUp();
printf("rocket intersects with enemy \n");
}
}
}
for (int i = 0; i < enemies.size(); i++)
{
Enemy *enemy = enemies[i];
if (checkCollision(enemy->getSprite(), hero.getSprite()))
{
gameOver = true;
gameOverSound.play();
}
}
}
Also, how could I rewrite my code so that I can remove the 'enemy' elements from the enemies vector without causing an error?
If you are allowed to change Rocket and Enemy, the simplest change would be to add a bool member to denote if the object is alive or dead. Then it's just a matter of marking the object as dead without actually erasing the object from the container. Then after the loop(s), remove the dead objects.
The change required for Enemy and Rocket classes would be to add a bool member that denotes whether the object needs to be erased:
class Enemy
{
bool destroyed;
//...
public:
Enemy() : destroyed(false) {}
bool is_destroyed() const { return destroyed; }
void set_destroyed() { destroyed = true; }
};
class Rocket
{
bool destroyed;
//...
public:
Rocket() : destroyed(false) {}
bool is_destroyed() const { return destroyed; }
void set_destroyed() { destroyed = true; }
};
Once that change is done, here is a probable code change for the main loop:
#include <algorithm>
//...
for (int i = 0; i < rockets.size(); i++)
{
if ( rockets[i]->is_destroyed() ) // skip if this was destroyed
continue;
for (int j = 0; j < enemies.size(); j++)
{
if (enemies[j]->is_destroyed()) // skip if this was destroyed
continue;
Rocket *rocket = rockets[i];
Enemy *enemy = enemies[j];
if (checkCollision(rocket->getSprite(), enemy->getSprite()))
{
//hitSound.play();
score++;
std::string finalScore = "Score: " + std::to_string(score);
scoreText.setString(finalScore);
sf::FloatRect scoreBounds = scoreText.getGlobalBounds();
scoreText.setOrigin(scoreBounds.width / 2, scoreBounds.height / 2);
scoreText.setPosition(viewSize.x * 0.5f, viewSize.y * 0.10f);
// "destroy" these items
rocket->set_destroyed();
enemy->set_destroyed();
if (score % 5 == 0) levelUp();
printf("rocket intersects with enemy \n");
break; // since the rocket is destroyed,
}
}
}
// remove the dead items
// first partition the dead items off from the live items
auto iter = std::stable_partition(rockets.begin(), rockets.end(), [](rocket *r)
{return r->is_destroyed();});
// issue a delete call on these items.
std::for_each(rockets.begin(), iter, [](rocket *r) { delete r; });
// now erase from the container
rockets.erase(rockets.begin(), iter);
// do similar for the enemies container
auto iter2 = std::stable_partition(enemies.begin(), enemies.end(), [](enemy *e)
{return e->is_destroyed();});
std::for_each(enemies.begin(), iter2, [](enemy *e) { delete e; });
enemies.erase(enemies.begin(), iter2);
Basically we mark the items to destroy, and then we deal with them after the loop is completed. The way we deal with them is to partition the dead items using std::stable_partition, call delete on each of the items on the left of the partition, and then finally erase them from the container.
The reason I didn't simply use std::remove_if is that the remove_if algorithm invalidates the items that are to be removed, thus attempting to issue a delete call will result in undefined behavior. So partitioning off the "dead" items first, delete-ing them second, and then finally erasing them IMO is safer.