[SOLVED] Checking for collisions in vector of shapes is bugged

[jilda]

edit: added SOLVED
I just started coding in SFML so I am not sure if there is already some class for the following thing:

I have RectangleShapes in my game. I need to check if they are colliding between each other so I can adjust their position based on that. So if I hit into another RectangleShape it will stop etc...


Person is a class which holds the rectangle shape and other info, like speed etc.
I made a global vector:

std::vector<Person*>& fuckingPerson;

And I am pushing newly created items into it:

fuckingPerson.push_back(* new Someone(sf::Vector2f(300, 211), sf::Vector2f(0, 0), fuckingPerson));
fuckingPerson.push_back(*new Someone(sf::Vector2f(300, 311), sf::Vector2f(0, 0), fuckingPerson));
MainPlayer ply(sf::Vector2f(250, 100), sf::Vector2f(0, 0), window, mainView, fuckingPerson);
fuckingPerson.push_back(&ply);
 

I have an update function in the Person class (MainPlayer and Someone are inherited from Person)

void Person::update()
{
        curPos = m_recShShape->getPosition();
        m_recShShape->move(m_vfSpeed);
        if (m_vfSpeed.x > 0) m_vfSpeed.x -= m_fFriction;
        if (m_vfSpeed.x < 0) m_vfSpeed.x += m_fFriction;
        if (m_vfSpeed.y > 0) m_vfSpeed.y -= m_fFriction;
        if (m_vfSpeed.y < 0) m_vfSpeed.y += m_fFriction;
       
        if (m_vfSpeed.x < m_fFriction && m_vfSpeed.x > 0) m_vfSpeed.x = 0;
        if (m_vfSpeed.y < m_fFriction && m_vfSpeed.y > 0) m_vfSpeed.y = 0;

        if (collidesWithWall() || collidesWithSomeone())
        {
                m_recShShape->setPosition(curPos);
                m_vfSpeed.x = 0;
                m_vfSpeed.y = 0;
        }
}

My main problem lies probably in the collidesWithSomeone function:

bool Person::collidesWithSomeone()
{
        sf::FloatRect boundingBox = m_recShShape->getGlobalBounds();
        std::vector<Person*>::iterator j;
        for (j = fuckingPerson.begin(); j != fuckingPerson.end(); j++)
        {
               
                if (this != (*j))
                {
                        if (boundingBox.intersects((*j)->getPersonShape().getGlobalBounds()))
                        {
                                return  true;
                        }
                        else
                        {
                                return false;
                        }
                }
        }
}

My problem is that the collision detection works between the first inserted Person and MainPlayer. Other inserted Persons are ignored. Examples in images below. I have been trying to solve this problem for about 4 hours now and I just can't find the damn bug.

Blue ones are "Someone".
Green one is "MainPlayer". Global bounds box is drawn for each shape.
Detects collision properly on the first inserted:


Another doesn't:

[zsbzsb]

First off, stop the swearing - in your code and forum post. There is no need for it and it won't help get you anywhere no matter how "cool" you may think it is. People browse these forums from work and other public places and there is no need for them or people around them to be exposed to this kind of thing.

Second, read this link and update your post. Pay attention to the bullet points. Also learn to use your debugger, this forum will not act as a debugger for you.

Third, here are some general tips about your code.

• Don't use manual memory management. Instead use RAII so you avoid memory leaks.
• Don't use global variables, they can and will cause issues - not to mention it is a sign of bad code design.
• Learn to use else if statements, it makes your code easier to read and will possibly execute faster.
• Also learn to use clamp functions, then you don't need another set of conditional statements.
• You can return the value of a function directly without needing additional additional return [true|false]; statements.

[Arcade]

You guessed right, the collidesWithSomeone function has a logic error. Look at it a little longer and you'll see why it only works with one object 

Specifically

for (j = fuckingPerson.begin(); j != fuckingPerson.end(); j++)
{
       
    if (this != (*j))
    {
        if (boundingBox.intersects((*j)->getPersonShape().getGlobalBounds()))
        {
            return  true;
        }
        else
        {
            return false;
        }
    }
}
 

Thank about how many times this is going to loop. The answer is not very many because you're returning after the first check.

I would also recommend looking into some of the things zsbzsb pointed out even though they aren't directly related to your problem.

[jilda]

Thanks, I got it working.
Thanks for the advices, but I not a newbie programmer, I have been programming in various languages (starting with c) for over 6 years now.

zsbzsb:
-1){swearing} I was frustrated with looking for the bug. When I am frustrated I am not very creative, so variables get first name which comes to mind. Also, this is my personal project, so I don't really care. You should meet my friend, one of the best programmers I know, and he puts swearing into his code all the time. Really, it doesn't matter if it won't be public.
0) I use the debugger
1) okay, I will look into unique pointers, never worked with them
2) I don't use them, this was just a temporary solution, because a static declaration of the vector didn't work properly for some reason
3) I know how and when to use else if, guess I was just too tired when writing that code
4) googled clamp, found some one line functions in boost. Is that supposed to be it?
5) Yes I know, I just prefer readability

Arcade:
I fixed the code with:

for (j = fuckingPerson.begin(); j != fuckingPerson.end(); j++)
{
        if (this != (*j) && boundingBox.intersects((*j)->getPersonShape().getGlobalBounds())) { k++; }
}
return (k > 0) ? true : false;

[Laurent]

This is very inefficient, because you continue the test even after finding a collision. When you only need the first collision you usually do:

for (each entity)
{
    if (collides_with(entity))
        return true;
}

return false;

And no need to write "cond ? true : false", since "cond" is already a boolean that holds true or false.

[jilda]

Oh god, at first look I thought "wait, this is the same way I did it in the first post". But then I realized how stupid I am.

Thanks