Method for elements in my std::list not working-CodePudding

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?

CodePudding user response：

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); }
);

CodePudding user response：

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) {
      ----------^

CodePudding user response：

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.