0

I am working on a small game and came across a big problem with lists.

Here's my code:

void cCollisionManager::checkCollision(cPlayer * pPlayer, std::list<cAsteroid*> *asteroidList, std::list<cShot*> *ShotList)
{       
    sf::FloatRect PlayerBox = pPlayer->getSprite()->getGlobalBounds();

    for (auto it : *asteroidList) {
        for (auto es : *ShotList) {
            sf::FloatRect asteroidboundingBox = it->getSprite()->getGlobalBounds();
            sf::FloatRect ShotBox = es->getSprite().getGlobalBounds();

            if (asteroidboundingBox.intersects(ShotBox)) {          

                it = asteroidList->erase(it);

                *pPlayer->pPunkte += 1;
                std::cout << *pPlayer->pPunkte << std::endl;
            }

            if (asteroidboundingBox.intersects(PlayerBox)) {
                if (*pPlayer->phealth >= 0.f)
                    *pPlayer->phealth -= 0.5f;
            }  
        }
    }
}

I used SFML and basically everything works. But if I want to delete the colliding asteroid and the shot, the programs exits with an error. In the if loop I tried to erase the object, but the compiler also gives an error saying that the argument type is not the same as the object type I am giving to it.

EDIT I had another look at the other question, you recommended to me, but still I haven't found out how to solve that problem. So if I changed my code to a while loop, the game couldn't handle it, because the Collision Manager is actually called in every single Call of the SFML main loop. So it would just get stuck in my collision loop. So I changed my code a bit, but still, things are not working.

Arthus V
  • 21
  • 4
  • 7
    [Don't modify sequences that are being enumerated with range-for.](https://stackoverflow.com/questions/10360461/removing-item-from-vector-while-in-c11-range-for-loop) Use iterators and the appropriate result of an `erase`. – WhozCraig Jun 04 '17 at 21:31
  • 1
    If you have trouble removing an element from a list, try to write a simple program that does that and nothing nothing else. When you have found your error and mastered the technique, *then* you can use it in the middle of a complex program. – Beta Jun 04 '17 at 21:35
  • 1
    Even after heeding to @WhozCraig's comment, there's a chance that you may use the `it` when an `end()` iterator is returned - in your inner loop along the lines `sf::FloatRect asteroidboundingBox = it->getSprite()....` – WhiZTiM Jun 04 '17 at 21:35
  • 2
    Possible duplicate of [Can you remove elements from a std::list while iterating through it?](https://stackoverflow.com/questions/596162/can-you-remove-elements-from-a-stdlist-while-iterating-through-it) – zett42 Jun 04 '17 at 21:35
  • `for (auto it : *asteroidList)` ... you seem to confuse what the `for` loop does. You have propably named the variable `it` in assumption that it is an iterator, but actually it is the *value* (`cAsteroid*`) of each element of the list. That's why `erase(it)` does not compile. – zett42 Jun 04 '17 at 21:41
  • @zett42 okay I get your point, but what should I do then to get things done?^^ – Arthus V Jun 04 '17 at 21:49
  • Look at the question I linked above. – zett42 Jun 04 '17 at 21:50
  • @zett42 I saw this post but I couldn't really help me :( Searched the whole day for a answer to my problem – Arthus V Jun 04 '17 at 21:52
  • @zett42 ... but still I'll have another look at it. Thanks for your help – Arthus V Jun 04 '17 at 21:54
  • Try to do what commenter @Beta suggested. If you still can't get it done, edit code in your question to include that simple program (also called [mcve]). – zett42 Jun 04 '17 at 21:57
  • @zett42 I'll do that! – Arthus V Jun 04 '17 at 22:03
  • And I guess you are leaking memory here when erasing the pointer from the list but not deleting the instance itself. – kreuzerkrieg Jun 05 '17 at 10:14
  • 1
    @kreuzerkrieg Yeah I recognized that. The problem is now solved - I used a while loop and did some minor asjustments and my problems where solved - literally. The Asteroid class actually hash a destructor so I just forgot about calling it - my RAM will thank you!! – Arthus V Jun 05 '17 at 20:25

1 Answers1

1

Don't modify sequences that are being enumerated with range-for. Use iterators and the appropriate result of an erase. – WhozCraig

This is actually the answer to it. I did the mistake - using a for loop and not a while loop and so I had some big issues and bad construction ideas for my code - luckily everything now works! Here is my final code:

    auto it = asteroidList->begin();
auto es = ShotList->begin();

while (it != asteroidList->end()) {

    sf::FloatRect PlayerBox = pPlayer->getSprite()->getGlobalBounds();
    sf::FloatRect asteroidboundingBox = (*it)->getSprite()->getGlobalBounds();

    while (es != ShotList->end()) 
    {
        sf::FloatRect ShotBox = (*es)->getSprite().getGlobalBounds();

            if (asteroidboundingBox.intersects(ShotBox)) {
                it = asteroidList->erase(it);   
                es = ShotList->erase(es);   

                std::cout << "Asteroid destroyed" << std::endl;
                *pPlayer->pPunkte += 1;
                std::cout << *pPlayer->pPunkte << std::endl;
                }


        if (es != ShotList->end())
            es++;   

    }
        if (asteroidboundingBox.intersects(PlayerBox)) 
        {
            if (*pPlayer->phealth > 3.f) {
                *pPlayer->phealth -= 5.f;
                it = asteroidList->erase(it);   
            }   
            else
                *pPlayer->pBStateAlive = false;
        }


    if (it != asteroidList->end()) {
        it++;   
        es = ShotList->begin();     

    }


}

}

Arthus V
  • 21
  • 4