Struggling with collision detection. How do I detect the collision correctly?

Question

class ShapePlanet
{
public:
    sf::CircleShape shape;
    sf::Vector2f planetposition;
    int radius = 45;
    ShapePlanet()
    {
        float x = rand() % 1020; //invece che 1280
        float y = rand() % 650; //invece che 720
        this->shape.setFillColor(sf::Color::Green);
        this->planetposition.x = x;
        this->planetposition.y = y;
        this->shape.setPosition(x, y);
    }
};

struct Node_Planet
{
    ShapePlanet Planet;
    Node_Planet* next;
};
typedef Node_Planet* ptr_list;

class Planet
{
private:
    ptr_list head;
public:
    ptr_list create(int n)
    {
        ptr_list tmp = NULL;
        for (int i = 0; i < n; i = i + 1)
        {
            {
                tmp = new Node_Planet;
                tmp->Planet = ShapePlanet();
                tmp->next = head;
                head = tmp;
            }
            return this->head;
        }


        void draw(sf::RenderWindow& window)
        {
            ptr_list p = this->head;
            while (p != NULL)
            {
                window.draw(p->Planet.shape);
                p = p->next;
            }
        }

        void IsItOverLapped()
        {
            ptr_list p1 = this->head;
            ptr_list p2 = this->head;
            // bool collision = false;
            sf::FloatRect shape1;
            sf::FloatRect shape2;
            float distance;
            float dx;
            float dy;
            //float distance = std::sqrt((dx * dx) + (dy * dy));

            while ((p1 != NULL))
            {
                //float distance = std::sqrt((dx * dx) + (dy * dy));
                while (p2 != NULL)
                {
                    shape1 = p1->Planet.shape.getGlobalBounds();
                    shape2 = p2->Planet.shape.getLocalBounds();
                    dx = (p1->Planet.shape.getPosition().x + (shape1.width / 2)) - (p2->Planet.shape.getPosition().x + (shape2.width / 2));
                    dy = (p1->Planet.shape.getPosition().y + (shape1.height / 2)) - (p2->Planet.shape.getPosition().y + (shape2.height / 2));
                    distance = std::sqrt((dx * dx) + (dy * dy));
                    if (distance <= (shape1.width / 2) + (shape2.width / 2))
                    //if (p2->Planet.shape.getGlobalBounds().intersects(p1->Planet.shape.getLocalBounds()))
                    {
                        std::cout << "Collision Detected" << std::endl;
                        //p2 = p2->next;
                        //collision = true;
                    }
                    else
                    {
                        std::cout << "No Collision Detected" << std::endl;
                        //collision = false;
                    }
                    /*if (p2->Planet.shape.getLocalBounds().intersects(p1->Planet.shape.getLocalBounds()))
                        std::cout << "collision";
                        std::cout <<'\n'; */
                    p2 = p2->next;
                }
                p1 = p1->next;
            }
            //return collision;
        }
    }
};

So, I would like to be able to draw the shapes on the screen. The problem is that I struggle with the detection of collision of the shapes. If I am able to detect the collision, then I can change the position of the overlapped shape. But I am struggling with it a lot. I have a list of objects. And they have some charateristics as the shape, the position etc. I have tried also to use the if(object1.getLocalBounds.intersects(object2.getLocalbounds()) {then do something} but seems the code doesn't go in the if side. I have tried a lot of times and I really don't know how to handle this thing anymore.

Answer 1

p1 = head;
p2 = head;

while (p1)
{
    while (p2)
    {
        // ...
        p2 = p2->next;
    }
    (*)
    p1 = p1->next;
}

When the code reaches (*) the first time, p2 was set to nullptr in the inner loop; but as you don't reset it within inner loop, it will remain there for all other loop runs, thus comparing second planet in the list against none of the others any more.

Additionally, as you initially set both pointers to head, you'll compare very first planet against itself, naturally resulting in a collision.

The fix for both issues:

p1 = head;
while (p1)
{
    p2 = p1->next;
    //       ^ avoid self-comparison
    while (p2)
    {
        // ...
        p2 = p2->next;
    }
    p1 = p1->next;
}

Answer 2

First, you should not build your own data structures (such as ptr_list) unless you have a very good reason to (or, if you're writing the code explicitly to learn about pointers and data structures, but then maybe don't mix it with collision). Instead, use the STL -- already one thing less you have to worry about. Also avoid new and delete.

Next, make sure you understand the SFML functions you're using. getLocalRect() and getGlobalRect() return you rectangles in two different coordinate systems, you can't just add/subtract them and expect a meaningful result. Consult the SFML documentation for more clarity. Plus, you should use directly sf::Rect::intersects() instead of building your own.

What you need to consider is that SFML will do an intersection check of axis-aligned bounding boxes, and not check if the actual rectangles intersect. This is simpler and faster than correct rectangle check, however less accurate. I would start with axis-aligned rectangles and then take it from there.

What could also help is to separate the code that checks for overlaps (collision detection) and the code that reacts to it (collision response). The first one would be a simple bool function that doesn't modify any state, and the second then performs the necessary actions.