Ncurses Snake Game using a Linked List in C

Question

This is a lightweight text based version of the popular game Snake written entirely in C. In order to build it, ncurses needs to be installed on the system. Navigation is done using the vim-keys and you can quit with 'q'. The bones of the snake are stored in a linked list. This is one of my personal projects and I would love to recieve expert advice.

In snake.c I have:

#define _POSIX_C_SOURCE 199309L

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

#include "linked_list.h"

enum dir {
    LEFT, DOWN, UP, RIGHT
};

/* global variables */
int width;
int height;
struct node *snk;
int len;
enum dir dir;
int food_x;
int food_y;
bool gameover;

/* initial setup */
void setup(void)
{
    setlocale(LC_ALL, "");
    initscr(); cbreak(); noecho();
    nonl();
    intrflush(stdscr, FALSE);
    keypad(stdscr, TRUE);
    timeout(0);
    curs_set(0);

    getmaxyx(stdscr, height, width);
    width = (width + 1) / 2;

    snk = new_node(width / 2, 0, NULL);
    len = 1;
    dir = DOWN;

    do {
        food_x = rand() % width;
        food_y = rand() % height;
    } while (snk->x == food_x && snk->y == food_y);

    gameover = false;

    srand(time(0));
}

/* listen for key press */
void input(void)
{
    char c = getch();
    if (c == 'h' && (dir != RIGHT || len == 1))
        dir = LEFT;
    else if (c == 'j' && (dir != UP || len == 1))
        dir = DOWN;
    else if (c == 'k' && (dir != DOWN || len == 1))
        dir = UP;
    else if (c == 'l' && (dir != LEFT || len == 1))
        dir = RIGHT;
    else if (c == 'q')
        gameover = true;
}

/* move snake, check for collision, handle food */
void logic(void)
{
    switch (dir) {
    case LEFT:
        snk = new_node(snk->x - 1, snk->y, snk);
        break;
    case DOWN:
        snk = new_node(snk->x, snk->y + 1, snk);
        break;
    case UP:
        snk = new_node(snk->x, snk->y - 1, snk);
        break;
    case RIGHT:
        snk = new_node(snk->x + 1, snk->y, snk);
        break;
    default:
        break;
    }

    if (snk->x < 0)
        snk->x = width - 1;
    else if (snk->x >= width)
        snk->x = 0;
    else if (snk->y < 0)
        snk->y = height - 1;
    else if (snk->y >= height)
        snk->y = 0;

    if (node_exists(snk->next, snk->x, snk->y))
        gameover = true;

    if (snk->x == food_x && snk->y == food_y) {
        do {
            food_x = rand() % width;
            food_y = rand() % height;
        } while (node_exists(snk, food_x, food_y));
        ++len;
    } else {
        delete_last(snk);
    }
}

/* draw the snake and the food */
void draw(void)
{
    clear();
    struct node *cursor = snk;
    while (cursor != NULL) {
        mvprintw(cursor->y, cursor->x * 2, "o");
        cursor = cursor->next;
    }
    mvprintw(food_y, food_x * 2, "+");
    refresh();
}

int main(void)
{
    setup();

    while (!gameover) {
        input();
        logic();
        draw();

        struct timespec ts;
        ts.tv_sec = 0;
        ts.tv_nsec = 100000000;
        nanosleep(&ts, &ts);
    }

    endwin();
    return EXIT_SUCCESS;
}

In linked_list.c I have:

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

#include "linked_list.h"

/* create new node */
struct node *new_node(int x, int y, struct node *next)
{
    struct node *new = malloc(sizeof *new);
    if (!new) {
        fprintf(stderr, "Error: memory allocation failed\n");
        exit(EXIT_FAILURE);
    }
    new->x = x;
    new->y = y;
    new->next = next;
    return new;
}

/* delete last node */
void delete_last(struct node *head)
{
    if (head == NULL) {
        fprintf(stderr, "Error: linked list underflow\n");
        exit(EXIT_FAILURE);
    }
    struct node **cursor = &head;
    while ((*cursor)->next != NULL)
        cursor = &(*cursor)->next;
    *cursor = NULL;
    free(*cursor);
}

/* check if node exists */
bool node_exists(struct node *head, int x, int y)
{
    struct node *cursor = head;
    while (cursor != NULL) {
        if (cursor->x == x && cursor->y == y)
            return true;
        cursor = cursor->next;
    }
    return false;
}

In linked_list.h I have:

#ifndef LINKED_LIST_H
#define LINKED_LIST_H

struct node {
    int x;
    int y;
    struct node *next;
};

/* create new node */
struct node *new_node(int x, int y, struct node *next);

/* delete last node */
void delete_last(struct node *head);

/* check if node exists */
bool node_exists(struct node *head, int x, int y);

#endif /* LINKED_LIST_H */

Answer 1

Overall this is quite nice code, it is split nicely into functions that each have their own responsibility. Some minor improvements are possible:

Avoid global variables

While there are no problems with the global variables in this program, the use of global variables is problematic if you work on larger projects. It is therefore good practice to try to remove global variables even in a small program like this one. The typical way to remove them is to create a struct that holds them:

struct state {
     int width;
     int height;
     ...
};

And in main() you can declare a variable of this type, and pass a pointer to it to any functions that need to access the state, like so:

void setup(struct state *state) {
    ...
    getmaxyx(stdscr, state->height, state->width);
    state->width = ...;
    ...
}

...

int main(void) {
    struct state state;
    setup(&state);
    
    while (!state->gameover) {
        ...
    }
    
    ...
}

Clean up properly on exit

When the game is over, you immediately exit the game, but you should try to clean up any resources used first. You should delete any memory allocated for the linked list. While it doesn't seem to matter (memory automatically gets freed upon program exit), consider that if you ever want to expand the game, maybe by allowing starting over after a game ends, you want to ensure the old linked list is freed before creating a new snake. Also, by properly cleaning up memory before exit, you can use tools like Valgrind to find real memory leaks without getting false positives.

Since you put all the setup code into the function setup(), consider moving any cleanup code in a function of its own, for example named cleanup().

Consider using an array instead of a linked list

Using a linked list seems very appropriate for the snake's body, as you can easily add on one end and remove from the other end. However, a linked list has some performance issues. This is mainly caused by each node being allocated separately, so they are not consecutive in memory, and the need to follow pointers to iterate through the list. It would be more efficient to store the snake's body positions in a dynamically allocated array, that you reallocate in case you need to grow it. You can treat this array as a circular buffer to ensure adding a new head and removing the tail is efficient.

Consider supporting WASD and cursor keys as well

Not everyone is used to vi-style navigation, in fact as a long-time vim-user myself I haven't even bothered to learn navigation using HJKL, since my keyboard has proper cursor keys. Consider supporting the cursor keys themselves, they are defined in ncurses as KEY_UP, KEY_DOWN, KEY_LEFT and KEY_RIGHT. Also popular in many games nowadays is to use the WASD keys for movement.