Simple snake game with C and raylib

Question

Made a simple snake game using C and raylib for graphics, Would like to be critiqued on the clearity and efficiency of the code.

#include <raylib.h>

#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <stdbool.h>

#define DIRECTION_UP (Vec2i){0, -1}
#define DIRECTION_DOWN (Vec2i){0, 1}
#define DIRECTION_LEFT (Vec2i){1, 0}
#define DIRECTION_RIGHT (Vec2i){-1, 0}

enum GameState
{ 
    GAME_START,
    GAME_RUNNING,
    GAME_OVER 
};

enum CellState
{
    CELL_EMPTY,
    CELL_SNAKE,
    CELL_APPLE
};

typedef unsigned uint;

typedef struct
{
    int x, y;
} Vec2i;

typedef struct
{
    uint *cells;
    uint height, width;
} Board;

typedef struct
{
    bool collided;
    uint tailLength;
    Vec2i headPosition;
    Vec2i headDirection;
    Vec2i *tail;
} Snake;

static uint gameState = GAME_START;
static uint gameScore = 0;

static Board board;
static Snake snake;
static Vec2i applePosition;

void board_set(Vec2i v, uint c)
{
#ifdef DEBUG
    if (v.x > board.width || v.y > board.height)
    {
        fprintf(stderr, "Can not get board at (%d,%d).\n", v.x, v.y);
        exit(1);
    }
#endif

    *(board.cells + v.x * board.width + v.y) = c;
}

uint board_get(Vec2i v)
{
#ifdef DEBUG
    if (v.x > board.width || v.y > board.height)
    {
        fprintf(stderr, "Can not set board at (%d,%d).\n", v.x, v.y);
        exit(1);
    }
#endif
    return *(board.cells + v.x * board.width + v.y);
}

void apple_spawn()
{
    while (true)
    {
        //Spawn within bounds of the board
        int x = (rand() % (board.width - 2)) + 1;
        int y = (rand() % (board.height - 2)) + 1;
        //Do not spawn on snake cells
        if (board_get((Vec2i){x, y}) != CELL_SNAKE)
        {
            applePosition.x = x;
            applePosition.y = y;

            break;
        }
    }
}

void snake_tail_grow()
{
    snake.tailLength++;
    if (snake.tailLength == 1)
    {
        snake.tail[snake.tailLength].x = snake.headPosition.x ;
        snake.tail[snake.tailLength].y = snake.headPosition.y ;
        return;
    }

    //Previous tail 
    snake.tail[snake.tailLength] = snake.tail[snake.tailLength - 1];
}

bool apple_eaten()
{
    //Check if snake head is on the same cell as the apple
    if (snake.headPosition.x == applePosition.x && snake.headPosition.y == applePosition.y)
        return true;

    return false;
}
void game_create(uint w, uint h)
{
    board.cells = calloc(h * w, sizeof *(board.cells));
    snake.tail = calloc(h * w, sizeof (Vec2i));//Allocate for longest tail length possible on the board
    if (board.cells == NULL || snake.tail == NULL)
    {
        fprintf(stderr, "Failed to allocate memory.\n");
        exit(1);
    }

    board.width = w;
    board.height = h;

    snake.tailLength = 0;

    snake.headDirection.x = 0;
    snake.headDirection.y = 0;

    //Spawn snake at the center of the board
    uint centerX = board.width / 2;
    uint centerY = board.height / 2;
    snake.headPosition.x = centerX;
    snake.headPosition.y = centerY;
}

void game_destroy()
{
    free(snake.tail);
    free(board.cells);
}

void game_update()
{
    //Clear board
    memset(board.cells, CELL_EMPTY, board.height * board.width * sizeof *(board.cells)); 

    //Update cells to be on the new positions
    board_set(snake.headPosition, CELL_SNAKE);
    board_set(applePosition, CELL_APPLE);
    for (uint i = 0; i < snake.tailLength; i++)
        board_set(snake.tail[i], CELL_SNAKE);

    //Snake collision with board boundary
    if (snake.headPosition.x >= board.width || snake.headPosition.x <= 0)
        snake.collided = true;
    if (snake.headPosition.y >= board.height || snake.headPosition.y <= 0)
        snake.collided = true;

    //Shift to last to make space for new tail
    memmove(snake.tail + 1, snake.tail, snake.tailLength * sizeof *(snake.tail));
    snake.tail[0] = snake.headPosition;

    //Move snake in the direction
    snake.headPosition.x += snake.headDirection.x;
    snake.headPosition.y += snake.headDirection.y;

    if (apple_eaten())
    {
        gameScore++;
        snake_tail_grow();
        apple_spawn();
    }

    if (snake.collided)
        gameState = GAME_OVER;
}

void game_input()
{
    if (IsKeyDown(KEY_W))
        snake.headDirection = DIRECTION_UP;
    if (IsKeyDown(KEY_S))
        snake.headDirection = DIRECTION_DOWN;
    if (IsKeyDown(KEY_D))
        snake.headDirection = DIRECTION_LEFT;
    if (IsKeyDown(KEY_A))
        snake.headDirection = DIRECTION_RIGHT;
}

void game_draw()
{
    uint widthRatio = GetScreenWidth() / board.width;
    uint heightRatio = GetScreenHeight() / board.height;

    for (uint x = 0; x < board.width; x++)
        for (uint y = 0; y < board.height; y++)
        {
            switch (board_get((Vec2i){x, y}))
            {
                case CELL_SNAKE :
                    DrawRectangle(x * widthRatio, y * heightRatio, widthRatio, heightRatio, GREEN);
                    break;
                case CELL_APPLE :
                    DrawRectangle(x * widthRatio, y * heightRatio, widthRatio, heightRatio, RED);
                    break;
            }
        }
    //Draw score
    DrawText(TextFormat("Score : %u", gameScore), 0, 0, 20, WHITE);
}

int main()
{
    InitWindow(640, 640, "Snake");
    SetTargetFPS(10);

    game_create(20, 20);
    apple_spawn();

    while (!WindowShouldClose())
    {
        BeginDrawing();
        ClearBackground(BLACK);
        switch (gameState)
        {
            case GAME_START :

                DrawText("Press space to start.", 0, 0, 40, WHITE);
                DrawText("W,A,S,D keys for movement.", 0, 40, 25, WHITE);
                if (IsKeyPressed(KEY_SPACE))
                    gameState = GAME_RUNNING;

                break;

            case GAME_RUNNING :

                game_input();
                game_update();
                game_draw();

                break;

            case GAME_OVER :
                DrawText("Game Over :(", 0, 0, 40, RED);
                DrawText(TextFormat("Score : %u", gameScore), 0, 40, 40, WHITE);
                DrawText("Press escape to exit.", 0, 80, 40, WHITE);
        }

        EndDrawing();
    }

    game_destroy();
    CloseWindow();
    return 0;
}

How did I do?

Answer 1

Since there's only one translation unit here, mark your functions static similar to what you've already done with your global variables.

I don't see a lot of value in typedef unsigned uint; - I'd delete that.

You follow what I consider to be a reasonable pattern for struct declaration - no tags, all typedefs. You do not do the same with your enum but for consistency's sake you should.

Your code is non-reentrant due to the reliance on global state. This isn't the end of the world, but a proper refactor would involve moving all of those variables to a game state structure and operating only on instances of that structure instead of globals. You ask:

should the game state structure be static as well?, also would you mind elaborating the global state part a bit more?

No, your game state data should not be made static: once it's removed from the global namespace, it will be passed around in parameters and return values. For an example of what this could look like:

typedef struct {
    unsigned score;
    // ...
} GameState;



static void game_update(GameState *game) {
    // ...
    if (apple_eaten())
    {
        game->score++;
    }

You need more const arguments, particularly for struct arguments.

Why should your if (v.x > board.width || v.y > board.height) check be surrounded in an #ifdef DEBUG? This is surely not performance-impactful so should be left in release builds.

*(board.cells + v.x * board.width + v.y) is slightly awkward. You can instead use an index-indirection like

board.cells[v.x * board.width + v.y] = c;

This while(true):

while (true)
{
    //Spawn within bounds of the board
    int x = (rand() % (board.width - 2)) + 1;
    int y = (rand() % (board.height - 2)) + 1;
    //Do not spawn on snake cells
    if (board_get((Vec2i){x, y}) != CELL_SNAKE)
    {
        applePosition.x = x;
        applePosition.y = y;

        break;
    }
}

can more legibly capture the termination condition as

int x, y;

do {
    // Spawn within bounds of the board
    x = (rand() % (board.width - 2)) + 1;
    y = (rand() % (board.height - 2)) + 1;
    // Do not spawn on snake cells
} while (board_get((Vec2i){x, y}) == CELL_SNAKE);

applePosition.x = x;
applePosition.y = y;

I consider the early-return in snake_tail_grow to be more legibly replaced by an else:

static void snake_tail_grow()
{
    snake.tailLength++;
    if (snake.tailLength == 1)
    {
        snake.tail[snake.tailLength].x = snake.headPosition.x;
        snake.tail[snake.tailLength].y = snake.headPosition.y;
    }
    else
    {
        //Previous tail 
        snake.tail[snake.tailLength] = snake.tail[snake.tailLength - 1];
    }
}

The boolean expression in apple_eaten should be returned directly rather than returning literals conditionally:

return snake.headPosition.x == applePosition.x && snake.headPosition.y == applePosition.y;

Since the results of the conditionals in game_input are mutually exclusive, you should use else on all but the first if.

Why are your widthRatio and heightRatio uint? Why should they not be promoted to floats?

Answer 2

In addition to @Reinderien good review.

Allocate consistently

Consider:

//                          Size of refenced object 
board.cells = calloc(h * w, sizeof *(board.cells));

//                         Size of refenced type.  
snake.tail = calloc(h * w, sizeof (Vec2i));

Instead consider a uniform style. I recommend the size of refenced object one. Also check return values.

// Example
snake.tail = calloc(h * w, sizeof snake.tail[0]);
if (snake.tail == NULL) {
  Handle_OutOfMemory();
}

Simplify code

    // snake.tail[snake.tailLength].x = snake.headPosition.x ;
    // snake.tail[snake.tailLength].y = snake.headPosition.y ;

    snake.tail[snake.tailLength] = snake.headPosition;

Think big (for the future)

Use size_t math rather than uint math for array index calculation.

// h * w
// ...
// *(board.cells + v.x * board.width + v.y) = c;

(size_t)h * w
...
*(board.cells + (size_t) v.x * board.width + v.y) = c;

Or simply use size_t for .x, .y, game_create(), ... rather than unsigned.

rand() % (board.width - 2) is insufficient/unbalanced when board.width on the order of RAND_MAX and RAND_MAX may be as small as 32767. Consider an assertion.

assert(board.width > RAND_MAX/4);

Avoid overflow. Perform math with the wider of size_t, unsigned, rather than unsigned.

//                                 unsigned     * unsigned
// memset(board.cells, CELL_EMPTY, board.height * board.width * sizeof *(board.cells)); 
//                              size_t                * unsigned
memset(board.cells, CELL_EMPTY, sizeof board.cells[0] * board.height * board.width);