Weird - Collision not occuring

[Gobbles]

I noticed your using lists, I'm not overly familiar with them as I always take preferences to vectors, however back when I was learning this stuff I would do:

for(int i = 0; i < enemyList.size(); i++)
{
    sf::Sprite enemySpr = enemyList[i];
    for(int j =0; j < otherList.size(); j++)
    {
        sf::Sprite otherSpr = otherList[j];
        if(enemySpr.intersects(otherSpr))
       {
            ...//whatever you want to do on a collision
        }        
    }
}
 

Nice and simple and easy to understand. Not fast by any means, but it'll do the job of getting things to work
I should note if your going to be removing items from the lists, make sure iterate from back to front.

Edit: Formatting

[WDR]

OK... So, I tried out your method. It gave me the same error. Turns out the error had nothing to do with the type of container. More to do with incrementing the iterator. Here's the code again.

std::list<sf::Sprite>::iterator enemyit = enemy.begin(), next;
std::list<sf::Sprite>::iterator greenlaserit = greenlaser.begin(), reload;
while(enemyit != enemy.end())
{
    next = enemyit;
    next++;
    while(greenlaserit != greenlaser.end())
    {
        reload = greenlaserit;
        reload++;
        if(enemyit->getGlobalBounds().intersects(greenlaserit->getGlobalBounds()))
        {
            enemy.erase(enemyit);
            greenlaser.erase(greenlaserit);
        }
        greenlaserit = reload;
    }
    enemyit = next;
}

Here, there are two while loops and what I am doing is I am using the inner loop to check for collision and if collision occurs, I am erasing the enemy iterator. And after doing that, the inner loop will keep looping until the while condition stops being satisfied. It won't enter the outer loop until it finishes that. Problem is, the enemy iterator is being erased in the inner loop and I am not incrementing it until the program goes back to the outer loop. How do I do that so that the enemy iterator is incremented in the inner loop as soon as it is erased. I've tweaked around a lot moving code around inside and outside the loops, adding new variables, still nothing. Please help me. Thanks.

[Ixrec]

Having run into this problem in the past, my impression is that there's at least two decent ways of attacking it.

1) Use the return value of erase().  It's there precisely to help you avoid leaving your iterators invalidated.  This is probably the "right" way of doing it, since it's built into the STL and probably results in cleaner code than #2.

2) Don't do the erase()s immediately.  Instead make a vector of sprite iterators, have the collision check push the collided entities' iterators into that vector, then after all the collision checking is done call erase() on all the iterators in that vector.

[WDR]

Wow... Although the second method is pretty straightforward and what most people would opt for despite being a bit lengthy and a little messy, I am interested in experimenting with the first method. Thank you for that. You guys have been very helpful. I shall post again when I have any updates. Thanks! 

[Nexus]

It really depends on the container type. For std::deque and especially std::vector, using the return value of erase() in a loop is very inefficient. You should prefer the STL algorithms std::remove() or std::remove_if(), combined with the erase() overload for an iterator range.

WDR, keep variable declarations local and use for instead of while. Your code is very difficult to read, because your iterators are declared all over the place and have a far too wide scope. A typical iterator loop looks as follows, with the iterator declared inside the loop head:

for (auto itr = container.begin(); itr != container.end(); ++itr)

And I agree with Ixrec's second point: Your code will become much cleaner, safer and more efficient, if you simply set a flag in the objects that should be removed. If your objects have attributes like hitpoints, this can be implicitly achieved by a condition like "hitpoints <= 0". Then, all you need to do is a std::remove_if() or std::list::remove_if() at the end of each frame.

[Gobbles]

Even better then removing from the list and re-adding (well I think so), just use a flag like 'isAlive' to determine if the object should be updated or drawn. When you go to shoot another laser, just check if any in the list first are 'isAlive == false', if it is, reuse it, if there isn't any, add a new one.

[eXpl0it3r]

When you go to shoot another laser, just check if any in the list first are 'isAlive == false', if it is, reuse it, if there isn't any, add a new one.

One should never be checked against 'true' or 'false'. Simply write 'if(isNotAlive)' or 'if(!isAlive)'.

[WDR]

Guys... I tweaked around. I'm not sure, but is this a way about it?

bool alive = true;

bool isAlive() const
{
        alive = false;
}

for(std::list<sf::Sprite>::iterator enemyit = enemy.begin(); enemyit != enemy.end(); enemyit++)
{
        for(std::list<sf::Sprite>::iterator greenlaserit = greenlaser.begin(); greenlaserit != greenlaser.end(); greenlaserit++)
        {
                if(enemyit->getGlobalBounds().intersects(greenlaserit->getGlobalBounds()))
                {
                        isAlive = false;
                }
        }
}

enemy.remove_if(!isAlive);
greenlaser.remove_if(!isAlive);

Please tell me... Did I put it right? 

[Ixrec]

Did you try it?

[WDR]

Yes, I did! No collision is occurring. The lasers and enemies just pass through each other. Is there something wrong with that? Am I right?

[Ixrec]

What you're doing with alive and isAlive doesn't really make any sense.  You need to step back and rethink that stuff completely.

Hint: The biggest error is that you're trying to store the results of all the collision tests in only one bool. You're supposed to give the entities a member so there's one bool for each of them.

Also, please try to start using a debugger so you can work out this simple stuff by yourself.

[WDR]

OK... Thanks! I'll do what you said. I'll post again if I have any more queries.