Is it an inefficient way of making collisions?

[lerio]

Let's say we have 2 rectangle shapes: shape1 and shape2, i want to test the collision between them and do different things when each side of shape1 touch the other side of shape2.

i was thinking in an easy way of doing it, and ended with something like this:

    sf::RectangleShape shape;
        sf::RectangleShape shape2;
        shape1.setSize(Vector2f(50,100));
        shape2.setSize(Vector2f(500,50));
        (. . .)

        in the game loop:

        //shapes bottom
                s1b = shape1.getPosition().y + 100;
                s2b = shape2.getPosition().y + 50;
                //shapes top
                s1t = shape1.getPosition().y;
                s2t = shape2.getPosition().y;
                //shapes right
                s1r = shape1.getPosition().x + 50;
                s2r = shape2.getPosition().x + 500;
                //shapes left
                s1l = shape1.getPosition().x;
                s2l = shape2.getPosition().x;

                //shapes are coliding?
                if(s1l <= s2r && s1r >= s2l && s1t <= s2b && s1b >= s2t){isColiding = true;}else{isColiding = false;}

                if(isColiding)
                {
                        if(s1b <= s2t + 10)//is shape1 on top of shape2?
                        {
                                spd.y = 0;
                                shape1.setPosition(shape1.getPosition().x, shape2.getPosition().y - 100);
                                std::cout<<"is on top" << std::endl;
                        }
                        else if(s1t >= s2b - 10)//is shape1 top hitting shape2 bot?
                        {
                                spd.y = speed;
                                shape1.setPosition(shape1.getPosition().x, shape2.getPosition().y + 50);
                                std::cout<<"is touching bottom" << std::endl;
                        }
                        else if(s1l >= s2r - 10)//does shape1 left collide with shape2 right?
                        {
                                spd.x = 0;
                                shape1.setPosition(shape2.getPosition().x + shape2.getSize().x, shape.getPosition().y);
                                std::cout<<"is touching right" << std::endl;
                        }
                        else if(s1r <= s2l + 10)//does shape1 right collide with shape2 left?
                        {
                                spd.x = 0;
                                shape1.setPosition(shape2.getPosition().x - shape1.getSize().x, shape1.getPosition().y);
                                std::cout<<"is touching left" << std::endl;
                        }

                }

It works, but i think that maybe this is really inefficient, because if shape1 move in a really fast speed for some reason (fps drops,etc), it will most probably miss the collisions. since it depends in a tiny 10 pixels collision check system, won't it?

[Jesper Juhl]

Problems like this is just one of the many reasons why you should use a fixed time step so that your (collision detection) code becomes independent of fps etc.

[Ixrec]

if shape1 move in a really fast speed for some reason (fps drops,etc), it will most probably miss the collisions. since it depends in a tiny 10 pixels collision check system, won't it?

1) This can happen with pretty much any collision method, and that's why people use fixed timesteps.  You can search this forum or the internet for loads of info on that.

2) But this collision code is also fairly problematic.  Miscellaneous comments to hint toward a better approach:

- First, learn all the sf::Rect methods like contains() and intersects(), and all the arithmetic/comparison operators on sf::Vector2<T>.  You can very often avoid comparing/editing individual points/components, which makes your code simpler and more readable.

- The sizes of your shapes are listed in multiple places, which is bad.  Your collision logic should be asking the shapes what their sizes are (probably with getSize()).

- You can determine what "side" of shape1 that shape2 is currently on by inspecting the coordinates of their centers.  This can be done separately from checking if they actually collide.

- It's generally best to check for collisions before actually moving things.  For instance, if the shapes are currently at (0,10) and (20,20), before moving the first shape, you compute that it wants to move to (25,20), then you check if there would be a collision if it moved there, and if not then you actually move it. If there is a collision then you do things like checking which side its on, what the final position should actually be given the obstacle and so on.

i think that maybe this is really inefficient

Efficiency is not a problem at all.  That code should be perfectly performant since only two entities are involved.  The problems it does have are of correctness, readability and maintainability.

[lerio]

Thank you guys. I will surely read Glenn Fiedler's articticle and about rect methods again.

Concerning the shape sizes listing, this was just an lazy example code that i did for this post, i'm not really using it in my game like this, i made a function and a template for that one.

About the center coordinate and the "pre-collision-check", hey, good tip, haven't tought about that, thank you!

[Ixrec]

Welcome!

The problems it does have are of correctness, readability and maintainability.

To correct myself: I forgot scalability.  If you want to test collisions between many many different entities, then things might slow down noticably.  If and when you actually see slowness, and your profiler has confirmed it's due to collision checks, then you should look up "quadtrees" and similar spatial partitioning data structures.

[Jesper Juhl]

Some more reading material for you:

http://www.koonsolo.com/news/dewitters-gameloop/

http://gameprogrammingpatterns.com/game-loop.html

http://www-cs-students.stanford.edu/~amitp/gameprog.html

You could find all these by searching the forum and/or Google, but I felt like listing them here again since they are useful and you might be lazy

[lerio]

Thank you ixrec and jesper, with your help i was able to implement the fixed timestep and it is working very well. I've made a little recording to see if it was really working =).



on the left it's rendering without fps limit and at the right, i've limited it to 10fps.