8
\$\begingroup\$

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.

\$\endgroup\$

1 Answer 1

7
\$\begingroup\$

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 ;).

\$\endgroup\$
8
  • \$\begingroup\$ 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. \$\endgroup\$ Oct 25, 2014 at 5:27
  • \$\begingroup\$ @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. \$\endgroup\$
    – glampert
    Oct 25, 2014 at 13:20
  • \$\begingroup\$ @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. \$\endgroup\$
    – glampert
    Oct 25, 2014 at 13:24
  • \$\begingroup\$ 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. \$\endgroup\$ Oct 25, 2014 at 19:55
  • \$\begingroup\$ @JohnBobSmith - You mean that you want to check collision of each object against every other object? \$\endgroup\$
    – glampert
    Oct 25, 2014 at 22:02

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.