2D Collision Detection in C++

Question

I'm attempting to rewrite the classic snake game in c++. What I am inquiring about is my implementation for a 2D collision detection function. I am utilizing SFML to create the window and shapes. The two objects that I am checking are squares of the same size. I would really appreciate any feedback that is given, as I'm still a beginner when it comes to c++.

This function works as intended. I'm looking for performance improvements, and general c++ practices that I should follow. If I need to add any more code, please let me know.

Game.h

#include <SFML/Graphics.hpp>
#include <SFML/System.hpp>
#include <SFML/Window.hpp>
#include <SFML/Audio.hpp>
#include <SFML/Network.hpp>
class Game {
 // Some code omitted for brevity //
 private:
 sf::RectangleShape enemy;
 sf::RectangleShape player;
 public:
 const bool isColliding(sf::RectangleShape player, sf::RectangleShape enemy) const;
 // Some code omitted for brevity //
};

Game.cpp

/*
 Determines if the player is colliding with the enemy.
 @param sf::RectangleShape player - Player object.
 @param sf::RectangleShape enemy - Enemy object.
 @return bool - True if colliding, False otherwise.
*/
const bool Game::isColliding(sf::RectangleShape player, sf::RectangleShape enemy) const {
 sf::Vector2f playerPosition = this->player.getPosition();
 sf::Vector2f enemyPosition = this->enemy.getPosition();
 float playerWidth = this->player.getSize().x;
 float playerHeight = this->player.getSize().y;
 return (
 ((playerPosition.x + playerWidth) > enemyPosition.x && (playerPosition.x - playerWidth) < enemyPosition.x) &&
 ((playerPosition.y + playerHeight) > enemyPosition.y && (playerPosition.y - playerHeight) < enemyPosition.y)
 );
}

Answer 1

SFML implements functionalities that allow you to determine if two rectangles intersect. You can significantly shorten your current code:

const bool Game::isColliding(const sf::RectangleShape& player, const sf::RectangleShape& enemy) const 
{
 return player.getGlobalBounds().intersects(enemy.getGlobalBounds());
}

See more:

• sf::FloatRect::intersects
• sf::RectangleShape::getGlobalBounds

Answer 2

const bool Game::isColliding(...

This indicates that the returned type is const, and should not be modified by any part of your code. But it's not a reference, so it doesn't make sense. It should compile, but it's a little confusing. Removing that const is clearer.

see this SO question for more information on that.

---

Your parameters to isColliding should be const references:

bool Game::isColliding(const sf::RectangleShape& player, const sf::RectangleShape& enemy) const

because you don't modify them, and references will prevent each object form being copied.

---

You never use the player parameter that you pass to the isColliding function. this->player is not the same as player in this context. It seems like you want to check the Game's current player object against collisions with arbitrary objects. You either

1. Make the function static (so it doesn't rely on a particular Game state) and pass two arbitrary objects to be checked for collision, or
2. Change the function name to something that indicates it will only be used with the player object.

One of:

static bool Game::isColliding(const sf::RectangleShape& a, const sf::RectangleShape& b)

or

bool Game::isPlayerColliding(const sf::RectangleShape& other) const

---

These Vector2d assignments can be made into const references to prevent copy construction.

 const sf::Vector2f& playerPosition = this->player.getPosition();
 const sf::Vector2f& enemyPosition = this->enemy.getPosition();

• Ben A: I wish I could accept this answer too. I will take these into account while continuing to learn c++. Thanks for the review!

Answer 3

John's answer is correct on programming matters. I want to comment on the logic of your code.

I found the collision detection calculation difficult to interpret. Which point on the square is represented by playerPosition? The documentation for SFML says it's the upper-left corner (although in a roundabout way), but it could also reasonably be the center. I can see that playerPosition.x + playerWidth is the right edge of the player, but what does playerPosition.x - playerWidth represent? This is a random point off to the left of the player.

Giving parts of the calculation good names will make the logic much easier to understand and debug should the need arise.

float playerLeftEdge = player.getPosition().x;
float playerRightEdge = playerLeftEdge + player.getSize().x;
float playerTopEdge = player.getPosition().y;
float playerBottomEdge = playerTopEdge + player.getSize().y;
float enemyLeftEdge = enemy.getPosition().x;
float enemyRightEdge = enemyLeftEdge + enemy.getSize().x;
float enemyTopEdge = enemy.getPosition().y;
float enemyBottomEdge = enemyTopEdge + enemy.getSize().y;
return playerRightEdge > enemyLeftEdge && playerLeftEdge < enemyRightEdge &&
 playerTopEdge < enemyBottomEdge && playerBottomEdge > enemyTopEdge;

Case in point regarding debugging, according to the documentation:

In addition to the position, rotation and scale, sf::Transformable provides an "origin" component, which represents the local origin of the three other components. Let's take an example with a 10x10 pixels sprite. By default, the sprite is positioned/rotated/scaled relatively to its top-left corner, because it is the local point (0, 0). But if we change the origin to be (5, 5), the sprite will be positioned/rotated/scaled around its center instead. And if we set the origin to (10, 10), it will be transformed around its bottom-right corner.

So, the bottom edge coordinate will be greater than the top edge, requiring a reversal of the logic for y-coordinate collisions (hence my latest edit).

The logic in your code only works because the player and enemy have the same width, which may not always be true in future iterations of your game. Even if it never changes and playerWidth is always equal to enemyWidth, giving different names to conceptually different quantities makes for more understandable code.