2D SDL Asteroids-like game

Question

I am writing an Asteroids clone in SDL2 using in C. I have written a game loop function that seems to work with no stutter, however I would just like to ensure my game loop will be fast and efficient before I go any further with this program. I'm very new to SDL; not so much C. I've tried to make my code as modular as possible.

game/game.c (where the rendering loop is)

/* Calls the appropriate functions and contains the game loop
 * game/game.c */

#include <stdio.h>
#include <stdlib.h>
#include <time.h>

#include "../global.h"
#include "../asteroid/asteroid.h"
#include "../drawing/drawing.h"

bool init_sdl();
void free_sdl();

private void game_loop();
int total_ast_count = 0;

/* Function main
 * Entry point, sets up window etc. */

int main(void) {
    if (!init_sdl()) {
        fprintf(stderr, "Error: cannot initialise SDL. Message: %s\n", SDL_GetError());
        free_sdl();
        return EXIT_FAILURE;
    }

    // Everything but the background in this game is white
    if (!set_col(0xFF, 0xFF, 0xFF)) {
        fprintf(stderr, "Error: cannot set the colour. Message: %s\n", SDL_GetError());
        free_sdl();
        return EXIT_FAILURE;
    }

    srand(time(NULL));

    // Testing: reg and draw a sin
    for (int i = 0; i < 4; i++) {
        ast_t* ast = NULL;

        if (!(ast = reg_asteroid(0, 0, 0, AST_BIG))) {
            fprintf(stderr, "Error: cannot register the requested astroid. Message: %s\n", SDL_GetError());
            free_sdl(); // asteroid.c/reg_asteroid()'s malloc can never have worked here, so we don't need to free() anything
            return EXIT_FAILURE;
        }

        total_ast_count++;
    }

    if (!clear_renderer()) {
        fprintf(stderr, "Error: cannot clear the renderer. Message: %s\n", SDL_GetError());
        free_sdl();
        return EXIT_FAILURE;
    }

    game_loop();
    // Testing: now free...
    autofree_asteroids();
    free_sdl();

    return EXIT_SUCCESS;
}

/* Function game_loop
 * The main game loop - picks up user events and performs the appropriate function call */

private void game_loop() {
    SDL_Event e;
    bool running = true;

    while (running) {
        while (SDL_PollEvent(&e) != 0) {
            switch (e.type) {
                case SDL_QUIT:
                    running = false;
                    break;
            }
        }

        set_col(0x00, 0x00, 0x00);
        clear_renderer();
        SDL_Delay(5);
        set_col(0xFF, 0xFF, 0xFF);

        for (int i = 0; i < total_ast_count; i++) {
            ast_t* ast = asteroid_list[i];

            ast->x += 1;
            ast->y += 1;

            if (!draw_asteroid(ast)) {
                fprintf(stderr, "Error: cannot draw the requested asteroid. Message: %s\n", SDL_GetError());
                autofree_asteroids();
                free_sdl();
            }
        }

        render();
    }
}

global.h (stuff needed by (nearly) every file)

/* Main header file for every file
 * global.h */

#ifndef GLOBAL_H
#define GLOBAL_H

#include <SDL2/SDL.h>

// Boolean
#define true (1)
#define false (0)
typedef unsigned char bool;

// Window width/height
#define SCREEN_WIDTH 1680
#define SCREEN_HEIGHT 1050

// Since static has multiple meanings, use this for functions
#define private static

#endif

---

drawing/drawing.c (the file which directly interacts with the SDL_Renderer and SDL_Window with native SDL functions)

/* Functions for managing SDL (drawing to screen etc.)
 * drawing.c */

#include "../drawing/drawing.h"

SDL_Window* window = NULL;
SDL_Renderer* renderer = NULL;

/* Function init_sdl()
 * Initialise SDL, the window and the renderer. Returns the success. */

bool init_sdl() {
    // Initialise SDL
    if (SDL_Init(SDL_INIT_VIDEO))
        return false;

    // Set texture filtering to linear
    if (!SDL_SetHint(SDL_HINT_RENDER_SCALE_QUALITY, "1"))
        return false;

    // Create window    
    window = SDL_CreateWindow(
        "ODEF Asteroids",
        SDL_WINDOWPOS_UNDEFINED,
        SDL_WINDOWPOS_UNDEFINED,
        SCREEN_WIDTH,
        SCREEN_HEIGHT,
        SDL_WINDOW_SHOWN
    );

    if (window == NULL)
        return false;

    // Create the renderer
    renderer = SDL_CreateRenderer(window, -1, SDL_RENDERER_ACCELERATED);

    if (renderer == NULL)
        return false;

    // Renderer colour (black background)
    if (!set_col(0x00, 0x00, 0x00))
        return false;

    return true;
}

/* Function free_sdl
 * Frees memory and closes the SDL subsystems. */

void free_sdl() {
    SDL_DestroyRenderer(renderer);
    renderer = NULL;
    SDL_DestroyWindow(window);
    window = NULL;

    SDL_Quit();
}

/* Function set_col
 * Attempts to set the colour of the renderer. */

bool set_col(uint8_t r, uint8_t g, uint8_t b) {
    if (SDL_SetRenderDrawColor(renderer, r, g, b, 0xFF) < 0)
        return false;
    return true;
}

/* Function draw_line
 * Draws a line on the screen */

bool draw_line(int start_x, int start_y, int end_x, int end_y) {
    if (SDL_RenderDrawLine(renderer, start_x, start_y, end_x, end_y) < 0)
        return false;
    return true;
}

/* Function render
 * Updates the screen */

void render() { 
    SDL_RenderPresent(renderer);
}

/* Function clear_renderer
 * Clears the renderer and returns the success. */

bool clear_renderer() {
    return (SDL_RenderClear(renderer) < 0) ? false : true;
}

drawing/drawing.h (function signatures for drawing.c)

/* Custom SDL drawing functions for things implemented in drawing.c
 * drawing.h */

#include "../global.h"

void render();
bool clear_renderer();
bool draw_line(int, int, int, int);
bool set_col(uint8_t, uint8_t, uint8_t);

asteroid/asteroid.c (functions related to asteroids)

/* Functions for managing asteroids
 * asteroid/asteroid.c */

#include <time.h>
#include <stdlib.h>

#include "../asteroid/asteroid.h"
#include "../drawing/drawing.h"

ast_t* asteroid_list[AST_MAX_COUNT];

/* Function reg_asteroid
 * Register an asteroid, this _must_ be done before drawing one. */

ast_t* reg_asteroid(int x, int y, int direction, unsigned char state) {
    for (int i = 0; i < AST_MAX_COUNT; i++) {
        if (asteroid_list[i] == NULL) {
            if ((asteroid_list[i] = malloc(sizeof(ast_t))) == NULL) {
                SDL_SetError("Malloc failed (your system is broken, buy a new PC... now)");
                return NULL;
            }

            asteroid_list[i]-> x = (x == -1) ? rand() % SCREEN_WIDTH : x;
            asteroid_list[i]-> y = (y == -1) ? rand() % SCREEN_HEIGHT : y;
            asteroid_list[i]-> direction = 0; // todo
            asteroid_list[i]-> state = state;
            asteroid_list[i]-> index = i;

            return asteroid_list[i];
        }
    }

    SDL_SetError("There were no free slots in the asteroid list!");
    return NULL;
}

/* Function dereg_asteroid
 * Remove an asteroid's pointer from the array and free it's memory. */

void dereg_asteroid(ast_t* ast) {
    asteroid_list[ast->index] = NULL;
    free(ast);
}

/* Function autofree_asteroids
 * Runs dereg_asteroid across all registered asteroids */

void autofree_asteroids() {
    for (int i = 0; i < AST_MAX_COUNT; i++) {
        if (asteroid_list[i] != NULL)
            dereg_asteroid(asteroid_list[i]);
    }
}

/* Function draw_asteroid
 * Takes an ast_t* and draws an asteroid (make sure it has been registered via reg_asteroid first). */

bool draw_asteroid(ast_t* ast) {
    /*   __A
          B /  \ F
          C \__/ E
         D      */

    // Set up sizes

    const int start_x = ast->x, start_y = ast->y;
    short
        ast_slant_width, // the width of B, C, E or F
        ast_slant_height, // the height of B, C, E or F
        ast_straight_width; // the width of A or D

    switch (ast->state) {
        case AST_BIG:
            ast_slant_width = 30;
            ast_slant_height = 45;
            ast_straight_width = 50;

            break;

        case AST_MED:
            ast_slant_width = 15;
            ast_slant_height = 22;
            ast_straight_width = 25;

            break;

        case AST_SML:
            ast_slant_width = 8;
            ast_slant_height = 15;
            ast_straight_width = 18;

            break;

        default:
            SDL_SetError("The asteroid state of %d is invalid", (int)ast->state);
            return false;
    }

    bool result = true;

    // A
    result = draw_line(
        start_x + ast_slant_width,
        start_y,
        start_x + ast_slant_width + ast_straight_width,
        start_y
    );

    if (!result)
        return false;

    // B
    result = draw_line(
        start_x + ast_slant_width,
        start_y,
        start_x,
        start_y + ast_slant_height
    );

    if (!result)
        return false;

    // C
    result = draw_line(
        start_x,
        start_y + ast_slant_height,
        start_x + ast_slant_width,
        start_y + (ast_slant_height * 2)
    );

    if (!result)
        return false;

    // D
    result = draw_line(
        start_x + ast_slant_width,
        start_y + (ast_slant_height * 2),
        start_x + ast_slant_width + ast_straight_width,
        start_y + (ast_slant_height * 2)
    );

    if (!result)
        return false;

    // E
    result = draw_line(
        start_x + ast_slant_width + ast_straight_width,
        start_y + (ast_slant_height * 2),
        start_x + (ast_slant_width * 2) + ast_straight_width,
        start_y + ast_slant_height
    );

    if (!result)
        return false;

    // F
    result = draw_line(
        start_x + (ast_slant_width * 2) + ast_straight_width,
        start_y + ast_slant_height,
        start_x + ast_slant_width + ast_straight_width,
        start_y
    );

    if (!result)
        return false;

    render();
    return true;
}

asteroids/asteroids.h (function signatures and definitions for asteroids)

/* Header file for anything to do with drawing asteroids
 * asteroid.h */

#ifndef ASTEROID_H
#define ASTEROID_H

#include "../global.h"

#define AST_MAX_COUNT 60 // maximum amount of asteroids that can ever be on the screen

// Asteroid sizes
#define AST_BIG 0
#define AST_MED 1
#define AST_SML 2

// Asteroid type
typedef struct {
    unsigned int x, y, index;
    int direction;
    unsigned char state;
} ast_t;

// Managing the asteroid array
extern ast_t* asteroid_list[AST_MAX_COUNT];
ast_t* reg_asteroid(int, int, int, unsigned char);
void dereg_asteroid();
void autofree_asteroids();

bool draw_asteroid(ast_t*);

#endif

After some further experimentation with multiple asteroids, it seems there is some minor flickering, leading me to believe there certainly is a better way to do this.

Answer 1

I used to love me some asteroids! This is a fun project to work on. You've separated the functionality pretty well. Nice work!

Simplify

I see a few things that are more complicated than they need to be. For example, why dynamically allocate the asteroids? They're fairly small (approximately 20 bytes), and AST_MAX_COUNT is only 60. That's only 60 * 20 = 1200 bytes or about 1.2 kilobytes. Just make an array of them! Then you eliminate the possibility of not being able to allocate one, and you never have to free them.

Another thing that could be simplified is your drawing functions. For example, draw_line is essentially a 1-line function that just calls SDL_RenderDrawLine(). It could be rewritten as:

bool draw_line(int start_x, int start_y, int end_x, int end_y)
{
    return SDL_RenderDrawLine(renderer, start_x, start_y, end_x, end_y) < 0;
}

Which means it could be eliminated. The only thing it provides is the renderer. If the renderer was not a global (see below for more on that), but just passed around, you'd have access to it and not need the wrappers.

Additionally, it looks like your draw_asteroid() function could benefit by using table look-ups. Rather than a switch statement, you could make an array with the values you want in it. Something like this:

typedef struct ast_sizes_t {
    short slant_width;
    short slant_height;
    short straight_width;
} ast_size_t;

const ast_sizes_t asteroid_sizes[AST_MAX_SIZES] = {
    { 30, 45, 50 }, // AST_BIG
    { 15, 22, 25 }, // AST_MED
    {  8, 15, 18 }  // AST_SML
};

And then the switch statement becomes just a single set of assignments:

ast_slant_width = asteroid_sizes [ ast->state ].slant_width;
ast_slant_height = asteroid_sizes [ ast->state ].slant_height;
ast_straight_width = asteroid_sizes [ ast->state ].straight_width;

Avoid Globals

You have several global variables. This is a bad idea because it leaves those variables open to being modified anywhere by anyone. This makes it really difficult to debug problems and to reason about your program. I recommend at the very least marking your globals as static so that they can only be modified within the source file they are in. (It looks like you may have been planning to do this with your private macro.)

Better yet, don't use globals at all and pass around the values you need. It looks like there are some groupings you make into structs which would reduce the number of arguments you'd need to pass to various functions. For example, the window and renderer variables could be put into a struct together, and could be passed to all of the functions in drawing.c. Likewise, the asteroid list and the total_ast_count seem related and could probably be put into a struct together.

Errors

It looks like there might be an error in your game_loop() function. In the for loop it checks to see if drawing an asteroid was successful. (Consequently, what are the circumstances where drawing lines can fail? If it's only when the renderer is NULL, then perhaps you don't need any checks once you've verified that you have a valid renderer at setup?) If drawing an asteroid fails, you free all the asteroids, free SDL, and then call render() and continue to loop. You'll hit the inner while loop at the top and SDL is no longer initialized. It seems like that's bad. If you set running to false after freeing SDL, or even just return, it should fix the problem. In fact, if you just set it to false you'll still execute render() which you probably don't want.

Answer 2

when writing function prototypes for functions that have no parameters do not use:

function();

rather use:

function( void );

Do not write the prototype different than the signature on the actual function.

I.E. this function prototype:

void dereg_asteroid();

does not match the actual function:

void dereg_asteroid(ast_t* ast)