Collision-detection for an SDL game

Question

I am working on a simple SDL game in C++. I want to make my collision detection implementation better.

Here's the complete project:

#include <SDL.h>
#include <SDL_image.h>
#include <cstdio>
#include <string>
const int SCREEN_WIDTH = 512;
const int SCREEN_HEIGHT = 512;
bool initialize_game();
bool load_game();
void close_game();
SDL_Texture* load_texture(std::string path);
bool check_collision(SDL_Rect a, SDL_Rect b);
SDL_Texture* playerOneTexture = NULL;
SDL_Window* globalWindow = NULL;
SDL_Renderer* globalRenderer = NULL;
class Timer
{
 public:
 Timer();
 void start_timer();
 void stop_timer();
 bool is_started();
 Uint32 get_ticks();
 private:
 Uint32 startTicks;
 Uint32 pausedTicks;
 bool isPaused;
 bool isStarted;
};
class Player
{
 public:
 const int PLAYER_WIDTH = 32;
 const int PLAYER_HEIGHT = 32;
 const int PLAYER_VELOCITY = 160;
 Player();
 void move_player(float timeStep, SDL_Rect objectWeHit);
 void render_player(float positionX, float positionY, SDL_Texture* texture, SDL_Rect* clip = NULL, double angle = 0.0, SDL_Point* center = NULL, SDL_RendererFlip flip = SDL_FLIP_NONE);
 void handle_player_event(SDL_Event &event);
 float positionX, positionY;
 float velocityX, velocityY;
 private:
 SDL_Rect playerCollider;
};
Timer::Timer()
{
 startTicks = 0;
 pausedTicks = 0;
 isPaused = false;
 isStarted = false;
}
void Timer::start_timer()
{
 isStarted = true;
 isPaused = false;
 startTicks = SDL_GetTicks();
 pausedTicks = 0;
}
void Timer::stop_timer()
{
 isStarted = false;
 isPaused = false;
 startTicks = 0;
 pausedTicks = 0;
}
Uint32 Timer::get_ticks()
{
 Uint32 time = 0;
 if(isStarted){
 time = SDL_GetTicks() - startTicks;
 }
 return time;
}
bool Timer::is_started()
{
 return isStarted;
}
Player::Player()
{
 positionX = 0.0;
 positionY = 0.0;
 velocityX = 0.0;
 velocityY = 0.0;
 playerCollider.w = PLAYER_WIDTH;
 playerCollider.h = PLAYER_HEIGHT;
}
void Player::move_player(float timeStep, SDL_Rect objectWeHit)
{
 positionX += velocityX * timeStep;
 playerCollider.x = positionX;
 if(positionX < 0){
 positionX = 0;
 playerCollider.x = positionX;
 }
 if(positionX > SCREEN_WIDTH - PLAYER_WIDTH){
 positionX = SCREEN_WIDTH - PLAYER_WIDTH;
 playerCollider.x = positionX;
 }
 positionY += velocityY * timeStep;
 playerCollider.y = positionY;
 if(positionY < 0){
 positionY = 0;
 playerCollider.y = positionY;
 }
 if(positionY > SCREEN_HEIGHT - PLAYER_HEIGHT){
 positionY = SCREEN_HEIGHT - PLAYER_HEIGHT;
 playerCollider.y = positionY;
 }
 if(check_collision(playerCollider, objectWeHit)){
 positionX -= (velocityX * timeStep);
 positionY -= (velocityY * timeStep);
 playerCollider.x = positionX;
 playerCollider.y = positionY;
 }
}
void Player::render_player(float positionX, float positionY, SDL_Texture* texture, SDL_Rect* clip, double angle, SDL_Point* center, SDL_RendererFlip flip)
{
 SDL_Rect renderQuad = { positionX, positionY, PLAYER_WIDTH, PLAYER_HEIGHT };
 if( clip != NULL )
 {
 renderQuad.w = clip->w;
 renderQuad.h = clip->h;
 }
 SDL_RenderCopyEx(globalRenderer, texture, clip, &renderQuad, angle, center, flip );
}
void Player::handle_player_event(SDL_Event &event)
{
 if(event.type == SDL_KEYDOWN && event.key.repeat == 0){
 switch(event.key.keysym.sym){
 case SDLK_UP:
 velocityY -= PLAYER_VELOCITY;
 break;
 case SDLK_DOWN:
 velocityY += PLAYER_VELOCITY;
 break;
 case SDLK_LEFT:
 velocityX -= PLAYER_VELOCITY;
 break;
 case SDLK_RIGHT:
 velocityX += PLAYER_VELOCITY;
 break;
 }
 }
 else if(event.type == SDL_KEYUP && event.key.repeat == 0){
 switch(event.key.keysym.sym){
 case SDLK_UP:
 velocityY += PLAYER_VELOCITY;
 break;
 case SDLK_DOWN:
 velocityY -= PLAYER_VELOCITY;
 break;
 case SDLK_LEFT:
 velocityX += PLAYER_VELOCITY;
 break;
 case SDLK_RIGHT:
 velocityX -= PLAYER_VELOCITY;
 break;
 }
 }
}
bool initialize_game()
{
 bool isSuccessful = true;
 if(SDL_Init(SDL_INIT_VIDEO) < 0){
 printf("Video failed to load... SDL Error: %s\n", SDL_GetError());
 isSuccessful = false;
 } else {
 globalWindow = SDL_CreateWindow("Robo Kill", SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED, SCREEN_WIDTH, SCREEN_HEIGHT, SDL_WINDOW_SHOWN);
 if(globalWindow == NULL){
 printf("Window failed to be created... SDL Error: %s\n", SDL_GetError());
 isSuccessful = false;
 } else {
 globalRenderer = SDL_CreateRenderer(globalWindow, -1, SDL_RENDERER_ACCELERATED | SDL_RENDERER_PRESENTVSYNC);
 if(globalRenderer == NULL){
 printf("Renderer failed to be created... SDL Error: %s\n", SDL_GetError());
 isSuccessful = false;
 } else {
 SDL_SetRenderDrawColor(globalRenderer, 0xFF, 0xFF, 0xFF, 0xFF);
 int imageFlags = IMG_INIT_PNG;
 if(!(IMG_Init(imageFlags) &imageFlags)){
 printf("SDL_image failed to load... SDL_image error: %s\n", IMG_GetError());
 isSuccessful = false;
 }
 }
 }
 }
 return isSuccessful;
}
bool load_game()
{
 bool isSuccessful = true;
 playerOneTexture = load_texture("images/player1.png");
 if(playerOneTexture == NULL){
 printf("player1.png failed to load...");
 isSuccessful = false;
 }
 return isSuccessful;
}
void close_game()
{
 SDL_DestroyTexture(playerOneTexture);
 playerOneTexture = NULL;
 SDL_DestroyRenderer(globalRenderer);
 globalRenderer = NULL;
 SDL_DestroyWindow(globalWindow);
 globalWindow = NULL;
 IMG_Quit();
 SDL_Quit();
}
SDL_Texture* load_texture(std::string path)
{
 SDL_Texture* finalTexture = NULL;
 SDL_Surface* loadedSurface = IMG_Load(path.c_str());
 if(loadedSurface == NULL){
 printf("Image %s failed to load... SDL_image error: %s\n", path.c_str(), IMG_GetError());
 } else {
 finalTexture = SDL_CreateTextureFromSurface(globalRenderer, loadedSurface);
 if(finalTexture == NULL){
 printf("Failed to create texture from surface...\n");
 }
 SDL_FreeSurface(loadedSurface);
 }
 return finalTexture;
}
bool check_collision(SDL_Rect a, SDL_Rect b)
{
 int leftA, leftB;
 int rightA, rightB;
 int topA, topB;
 int bottomA, bottomB;
 leftA = a.x;
 rightA = a.x + a.w;
 topA = a.y;
 bottomA = a.y + a.h;
 leftB = b.x;
 rightB = b.x + b.w;
 topB = b.y;
 bottomB = b.y + b.h;
 if(bottomA <= topB){
 return false;
 }
 if(topA >= bottomB){
 return false;
 }
 if(rightA <= leftB){
 return false;
 }
 if(leftA >= rightB){
 return false;
 }
 return true;
}
int main()
{
 if(!initialize_game()){
 printf("Failed to initialize...\n");
 } else {
 if(!load_game()) {
 printf("Failed to load game...\n");
 } else {
 bool isRunning = true;
 SDL_Event event;
 Player Player;
 Timer stepTimer;
 SDL_Rect wall;
 wall.x = 90;
 wall.y = 90;
 wall.w = 90;
 wall.h = 90;
 SDL_Rect wall2;
 wall2.x = 256;
 wall2.y = 256;
 wall2.w = 30;
 wall2.h = 40;
 while(isRunning) {
 while(SDL_PollEvent(&event) != 0) {
 if(event.type == SDL_QUIT){
 isRunning = false;
 }
 Player.handle_player_event(event);
 }
 float timeStep = stepTimer.get_ticks() / 1000.f;
 Player.move_player(timeStep, wall);
 stepTimer.start_timer();
 SDL_SetRenderDrawColor(globalRenderer, 0xFF, 0xFF, 0xFF, 0xFF);
 SDL_RenderClear(globalRenderer);
 SDL_SetRenderDrawColor(globalRenderer, 0x00, 0x00, 0x00, 0xFF);
 SDL_RenderDrawRect(globalRenderer, &wall);
 SDL_RenderDrawRect(globalRenderer, &wall2);
 Player.render_player(Player.positionX, Player.positionY, playerOneTexture, NULL, 0.0, NULL, SDL_FLIP_NONE);
 SDL_RenderPresent(globalRenderer);
 }
 }
 }
 close_game();
}

Basically, I have my move_player function check for collisions by calling check_collision. Right now, check_collision needs two arguments: the playerCollider, and one other object to collide with.

Note that, in the inside of main, I have two wall rects. Right now, I can only collide with one of the rects, and not the other. I need to be able to collide with both. I'm wondering what the best approach would be for detecting multiple different collisions. Say I have 50 walls in my map, I don't want to have to say "collide with wall X" 50 times. I hope that makes sense.

Feel free to suggest any other improvements to the code too. Also, I based this code heavily off of lazyfoo.net tutorials, so credit goes to them for teaching me what I know.

Answer 1

You have implemented your own rectangle collision check routine when you really didn't have to. SDL provides the function SDL_IntersectRect() which tests two SDL_Rects for intersection and also returns the intersection rect of A and B.

---

Now lets talk about coding practices a little bit.

It looks like you are an adept of the single return statement. In the initialize_game() function, for instance, it has lead to a lot of juggling and flag checking to ensure the single return. In that case, it only made the code more complex. You should have just done it like so:

if(SDL_Init(SDL_INIT_VIDEO) < 0) {
 return false;
}
... stuff ...
if(globalWindow == NULL) {
 return false;
}

Prefer to return early and avoid deep nesting. It makes logic easier to follow. The single return technique is hardly applicable to modern C++. The RAII idiom covers dynamic resource management inside functions.

---

Other minor things:

• NULL is not C++! NULL is a C library macro. Modern C++ should use nullptr.

• Did you code everything up in the same file? If so, it is probably time to move each class to it's own module.

• Give std::cout a try. You might end up liking it ;).

• JohnBobSmith: Okay, that sounds good. Is it possible to pass multiple arguments to SDL_IntersectRect() ? Also, to use nullptr, would I just replace all instances of NULL with nullptr? Or is it more complex than that? Still learning pointers here. I will do some digging and hopefully find a solution.
• glampert: @JohnBobSmith - SDL_IntersectRect takes 3 rects, a, b and c. c is filled with the intersection of a and b if there is any. Check the documentation in the link.
• glampert: @JohnBobSmith - Just replacing NULL with nullptr should work fine. Your compiler will need C++11 support to be enabled (this is probably the default on VS, GCC has to be enabled by you). If you get a compilation error after that, then it probably indicates that you had a bogus conversion somewhere, which is one of the problems of NULL. That is easy to fix. But shouldn't happen in your program, I'd guess.
• JohnBobSmith: Okay, my problem that I still have is that I'd have to call SDL_IntersectRect for every rectangle in the map. I want to be able to collide with all objects, not just one. I hope that makes sense. Still looking for a solution to this problem... If a code sample would help, let me know and I will pastebin one.
• glampert: @JohnBobSmith - You mean that you want to check collision of each object against every other object?