Can someone code review my small SDL app? Want to make sure I didn't make any beginner mistakes

Question

In an effort to teach myself the SDL library (hence my stack overflow handle :) ) I wanted to try my hand at a side-scroller. My code is complete but I want some feedback (mostly because I have an atrocious amount of if and else statements for what seems like some simple logic).

My "program" is a c++ side-scroller where you move a single sprite across the screen. No jumping, bad guys, guns, scores, levels or anything. I wanted to use this as a base to build up upon. So I figured if my base is wrong I could end up with some pretty bad future apps. It's also multi-threaded. Next up on this I would like to make the person sprite animated (so it looks like he's walking rather than sliding) as well as make the person go faster when the arrow buttons are held down longer).

The code is kind of long but here's my main method. There's a link at the bottom for the whole program:

#include <iostream>

#include "SDL.h"

#include "game.hpp"

using std::cout;
using std::endl;

const int SCREENW = 200;
const int SCREENH = 200;
const int BPP = 32;
const int FPS = 24;

int event_loop(void* stuff);
int display_loop(void* stuff);

int main(int argc, char** argv) {
    SDL_Init(SDL_INIT_EVERYTHING | SDL_INIT_EVENTTHREAD);
    SDL_Thread* events_thurd;
    SDL_Thread* display_thurd;

    SDL_Surface* screen = SDL_SetVideoMode(SCREENW, SCREENH, BPP, SDL_SWSURFACE);
    SDL_EnableKeyRepeat(SDL_DEFAULT_REPEAT_DELAY, SDL_DEFAULT_REPEAT_INTERVAL);

    SDL_Event event;
    Game* thug_aim = new Game(&event, screen, SCREENW, SCREENH, BPP);

    events_thurd = SDL_CreateThread(event_loop, (void*)thug_aim);
    display_thurd = SDL_CreateThread(display_loop, (void*)thug_aim);

    SDL_WaitThread(events_thurd, NULL);
    SDL_KillThread(display_thurd);
    delete thug_aim;

    return 0;
}

int event_loop(void* stuff) {
    Game* gamez = (Game*)stuff;
    SDL_Event* event = gamez->get_event();

    while(1) {
        while(SDL_PollEvent(event)) {
            if(event->type == SDL_QUIT) {
                return 0;
            }
            else if(event->type == SDL_KEYDOWN) {
                if(event->key.keysym.sym == SDLK_LEFT || event->key.keysym.sym == SDLK_RIGHT) {
                    gamez->move(event->key.keysym.sym);
                }
            }
            else if(event->type == SDL_KEYUP) {
                if(event->key.keysym.sym == SDLK_LEFT || event->key.keysym.sym == SDLK_RIGHT) {
                    gamez->stop_move(event->key.keysym.sym);
                }
            }
            else {
                //not an event that concerns this game
            }
        }
    }
}

int display_loop(void* stuff) {
    Game* gamez = (Game*)stuff;
    double period = 1 / FPS * 1000;
    Uint32 milli_period = (Uint32)period;

    //get some of the attributes from gamez
    SDL_Rect* background_rect = gamez->get_background_rect();
    SDL_Rect* person_rect = gamez->get_person_rect();
    SDL_Surface* screen = gamez->get_screen();
    SDL_Surface* background = gamez->get_background();
    SDL_Surface* person = gamez->get_person();

    Uint32 start, end;
    int sleep;
    while(1) {
        start = SDL_GetTicks();

        //blit background
        SDL_BlitSurface(background, background_rect, screen, NULL);

        //blit person
        SDL_BlitSurface(person, NULL, screen, person_rect);

        end = SDL_GetTicks();

        sleep = milli_period - (end - start);
        if(sleep < 0) { sleep = 0; }
        SDL_Delay((Uint32)sleep);

        if(SDL_Flip(gamez->get_screen()) != 0) {
            cout << "error drawing to screen: " << SDL_GetError() << endl;
        }
    }
}

Here's the link to the .zip file of all my code (please ignore some of the variable names ;-) ): Anyway can you guys take a look and tell me what you think? url

Answer 1

Looks good, but some small improvements come to my mind:

• You could use switch-case instead of if-else-if
• You could create an alias for event->key.keysym.sym for readibility.