Method for elements in my std::list not working

Question

I am making a very simple clicker game, and whenever the user clicks, a ball will fall in the background from the top of the screen to the bottom. I store these falling balls (type FallingBall, which has an x, y, radius, and speed, which is set to 1.0f by default) in an std::list. When I iterate over these falling balls to move them, they simply just don't move.

FallingBall move method (returns true if the ball needs to be removed):

bool move(float frameTime) {
    pos.y += speed * frameTime;
    speed *= 1.98f;

    if (pos.y - radius > GetScreenHeight()) {
        return true;
    }
        
    return false;
}

Instantiating and moving falling balls:

if (IsMouseButtonPressed(0)) {

    if (CheckCollisionPointCircle(mousePos, clicker.getPos(), clicker.getRadius())) {
        clicker.click();
        FallingBall newFallingBall = FallingBall();
        newFallingBall.setX(GetRandomValue(newFallingBall.getRadius(), GetScreenWidth() - newFallingBall.getRadius()));
        fallingBalls.push_back(newFallingBall);
        std::cout << fallingBalls.size() << std::endl;
    }
}

for (FallingBall fallingBall : fallingBalls) {
            
    if (fallingBall.move(frameTime)) {
        fallingBalls.pop_back();
    }
}

The balls are definitely in the list and being instantiated as they appear on screen and the length of the list increments by one every time I click on the clicker. The balls simply just don't move. I assume it has something to do with the way I'm accessing the FallingBall objects?

Daniel Langr · Answer 1 · 2022-02-28T09:15:04.820

You cannot generally remove elements from a list with pop_back() while iterating over its elements (moreover, you would likely remove another element than the moved one). Once the removed element is processed in an actual iteration, iterators to that element are invalidated. You should switch to an iterator-based loop and remove elements with erase().

Relevant question: Can you remove elements from a std::list while iterating through it?.

You can try something as:

auto it = fallingBalls.begin();
while (it != fallingBalls.end()) {
  if (it->move(frameTime)) 
    it = fallingBalls.erase(it);
  else
    ++it;
}

Alternatively, you can use remove_if member function, which I would prefer. C++14 version with a generic lambda:

fallingBalls.remove_if(
  [](auto& fallingBall){ return fallingBall.move(frameTime); }
);

score 0 · Answer 2 · answered Feb 28 '22 at 08:51

0

The for-loop

for (FallingBall fallingBall : fallingBalls) {

creates copies of the list members, and only the copies are updated...

Try using a reference instead

for (FallingBall& fallingBall : fallingBalls) {
      ----------^

answered Feb 28 '22 at 08:51

BoP

2,310
1
15
24

I tried this because it was my first thought, and it technically does move the ball, but it just slams it straight into the ground at an insane speed and then throws an exception: `Exception thrown: read access violation. this was 0xFFFFFFFFFFFFFFF3.` – TheAngriestCrusader Feb 28 '22 at 08:53
This exception is raised on the `pos.y += speed * frameTime;` line in the method. – TheAngriestCrusader Feb 28 '22 at 08:53
Actually, if I make the speed multiplier REALLY slow, it works fine but the error is still thrown when the ball goes off the screen. I'm gonna try a different method of removing the falling balls because I think it's the deletion of the balls that's raising the error. – TheAngriestCrusader Feb 28 '22 at 08:56
This is exactly what I'm implementing now and exactly the question I looked up to get the correct algorithm, I'll let you know how it goes. – TheAngriestCrusader Feb 28 '22 at 09:02

TheAngriestCrusader · Answer 3 · 2022-02-28T14:15:39.020

Using an answer from this question, I managed to create this working solution:

std::list<FallingBall>::iterator i = fallingBalls.begin();
        
while (i != fallingBalls.end()) {
            
    if (i -> move(frameTime)) {
        fallingBalls.erase(i++);
    }
    else {
        i++;
    }
}

Edit: Someone else has posted this answer before I saw it and also provided a more modern approach to the situation, I will mark their answer as the one that answered my question.

Method for elements in my std::list not working

3 Answers3