7
\$\begingroup\$

Introduction


I've started to learn C programming a bit and wanted to create a simple 2D console game. Let me first introduce you to the game level/map structure:

(1)  #################      (2)  #################
     #               #           #               #
     #         v  S ^#           #      S        #
     #               #           #               #
     #  <      #######           #       v       #
     #      >  #                 #      > <      #
     #    A    #                 #       ^       #
     #         #                 #              A#
     ###########                 #      A        #
                                 #               #
                                 #################

The different symbols represent the following game objects:

  • # = wall
  • S = player
  • A = goal
  • ^,v,<,> monster looking up/down/left/right, respectively.

The goal is to get to one of the goal cells without touching a monster.

There can be only one player S, but multiple monsters and goals on the map. On each game tick, the player enters w, a, s, or d and moves up/left/down/right one cell, respectively. Then, all the monster entities move one unit in their corresponding direction.

If the player moves "into" a wall, the player's position is not updated. Monsters, however, bounce back 180 deg from the wall.

The goals (A) and walls (#) do not ever move. However, # acts as a boundary for both the player and monsters. The player can move onto the A but monsters treat the goal cells as walls as well.

One caveat is that monsters may overlap (see Level 2) such that only the monster first read from the level file is displayed in such an overlapping cell. If the player runs into a monster the monster is displayer on top and the game ends printing out a lose message. If the player reaches the goal, the goal symbol stays on top and the game ends printing out a win message.

Internally, I read in the level file into a 2D char array and save all dynamic entities (player and monsters) in a separate data structure. I then remove all dynamic entities from the 2D array to use it as a "canvas" for drawing. That way I can update all entities' locations and then decide how they're going to be painted onto the canvas, but am still able to use static elements (# and A) for collision detection.

Code


Note that I'm only allowed to use the C99 standard.

common.h

#ifndef COMMON_H
#define COMMON_H

#include <stdio.h>

typedef enum error_code {
    OK = 0,
    COULD_NOT_OPEN_FILE = 1,
    COULD_NOT_READ_FILE = 2,
    INVALID_OPTIONS = 3,
    ALLOC_FAILED = 4
} t_error_code;

typedef struct error_object {
    char msg[100];
    t_error_code error_code;
} t_error_object;

t_error_object make_error(const char *message, t_error_code error_code);
int get_file_size(FILE *f);
int get_line_count(FILE *f);
char* strdup_(const char* src);

#endif

common.c

#include "common.h"

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

t_error_object make_error(const char *message, t_error_code error_code) {
    t_error_object error_obj;

    strncpy(error_obj.msg, message, 100);
    error_obj.error_code = error_code;

    return error_obj;
}

int get_file_size(FILE *f) {
    fseek(f, 0, SEEK_END);
    int len = ftell(f);
    fseek(f, 0, SEEK_SET);

    return len;
}

char *strdup_(const char *src) {
    char *dst = malloc(strlen (src) + 1);  
    if (dst == NULL) return NULL;          
    strcpy(dst, src);                      
    return dst;                            
}

game_params.h

#ifndef GAME_PARAMS_H
#define GAME_PARAMS_H

#include <stdio.h>
#include "common.h"

typedef struct game_params {
    FILE *level_file;

    FILE *input_file;
    FILE *output_file;
} t_game_params;


void cleanup_game_files(t_game_params *params);

t_error_object open_game_files(t_game_params *params,
                     const char* level_file_name,
                     const char* input_file_name,
                     const char* output_file_name);

t_error_object parse_game_parameters(t_game_params *params_out, int argc, char **argv);

#endif

game_params.c

#include "game_params.h"
#include "common.h"

#include <getopt.h>
#include <errno.h>
#include <string.h>

void cleanup_game_files(t_game_params *params) {
    if (params->level_file != NULL)
        fclose(params->level_file);

    params->level_file = NULL;

    if (params->input_file != NULL)
        fclose(params->input_file);

    params->input_file = NULL;

    if (params->output_file != NULL)
        fclose(params->output_file);

    params->output_file = NULL;
}

t_error_object open_game_files(t_game_params *params,
                     const char *level_file_name,
                     const char *input_file_name,
                     const char *output_file_name) {
    params->input_file = stdin;
    params->output_file = stdout;

    if (input_file_name != NULL) {
        if (strstr(input_file_name, ".txt") == NULL) {
            return make_error("Eingabe-Datei kann nicht gelesen werden", COULD_NOT_READ_FILE);
        }

        params->input_file = fopen(input_file_name, "r");
        if (params->input_file == NULL) {
            return make_error("Eingabe-Datei konnte nicht geöffnet werden", COULD_NOT_OPEN_FILE);
        }
    }

    if (output_file_name != NULL) {
        params->output_file = fopen(output_file_name, "w");
        if (params->output_file == NULL) {
            return make_error("Ausgabe-Datei konnte nicht geöffnet werden", COULD_NOT_OPEN_FILE);
        }
    }
 
    if (level_file_name == NULL) {
        return make_error("Level-Datei muss angegeben werden", COULD_NOT_OPEN_FILE);
    }

    params->level_file = fopen(level_file_name, "r");

    if (params->level_file == NULL) {
        return make_error("Level-Datei konnte nicht geöffnet werden", COULD_NOT_OPEN_FILE);
    }

    return make_error("", OK);
}

t_error_object parse_game_parameters(t_game_params *params_out, int argc, char **argv) {
    char *level_file_name = NULL;
    char *input_file_name = NULL;
    char *output_file_name = NULL;

    while (optind < argc) {
        if (argv[optind][0] != '-') {
            if (level_file_name != NULL)
                return make_error("Level-Datei darf nur einmal angegeben werden", INVALID_OPTIONS);

            level_file_name = argv[optind];
            optind++;
        }
    
        int opt;
        if ((opt = getopt(argc, argv, "i:o:")) != -1) {
            switch (opt) {
                case 'i':
                    if (input_file_name != NULL)
                        return make_error("Eingabe-Datei darf nur einmal angegeben werden", INVALID_OPTIONS);
                    input_file_name = optarg;
                    break;
                case 'o':
                    if (output_file_name != NULL)
                        return make_error("Ausgabe-Datei darf nur einmal angegeben werden", INVALID_OPTIONS);
                    output_file_name = optarg;
                    break;
                default:
                    return make_error("Falsche Optionen übergeben", INVALID_OPTIONS);
            }
        }
    }

    // Öffne die Dateien zum Lesen/Schreiben und speichere File-Handles in `params`
    t_error_object ret = open_game_files(params_out, level_file_name, input_file_name, output_file_name);
    return ret;
}

entity.h

#ifndef ENTITY_H
#define ENTITY_H

#include "board.h"

typedef enum entity_type {
    PLAYER,
    MONSTER,
    NO_ENT
} t_entity_type;

typedef struct position {
    int x;
    int y;
} t_position;

typedef struct entity {
    t_position pos;
    t_direction facing_dir;
    t_entity_type type;
} t_entity;

t_entity_type get_entity_type(char c);
t_entity create_entity(t_entity_type type, int x, int y, t_direction dir);

int compare_positions(t_position *pos1, t_position* pos2);

void handle_collision(t_board *board, t_entity *entity, t_position *new_pos);
int check_wall(t_board *board, t_position *new_pos);
int check_valid_move(t_board *board, t_position *new_pos);
void move_entity(t_board *board, t_entity *entity, t_direction dir);

#endif

entity.c

#include "common.h"
#include "direction.h"
#include "entity.h"
#include "board.h"

t_entity_type get_entity_type(char c) {
    int is_player = (c == 'S');
    int is_monster = (map_char_to_direction(c) != NONE);

    if (is_player)
        return PLAYER;

    if (is_monster)
        return MONSTER;

    return NO_ENT;
}

t_entity create_entity(t_entity_type type, int x, int y, t_direction dir) {
    t_entity entity;

    t_position pos;
    pos.x = x;
    pos.y = y;

    entity.type = type;
    entity.pos = pos;
    entity.facing_dir = dir;

    return entity;
}

int check_wall(t_board *board, t_position *new_pos) {
    if (board->cells[new_pos->y][new_pos->x] == '#')
        return 1;
        
    return 0;
}

int check_valid_move(t_board *board, t_position *new_pos) {
    if (new_pos->x >= board->col_size || new_pos->x < 0)
        return 0;
    
    if (new_pos->y >= board->num_rows || new_pos->y < 0)
        return 0;

    return 1;
}

void handle_collision(t_board *board, t_entity *entity, t_position *new_pos) {
    t_position old_pos = entity->pos;

    if (!check_valid_move(board, new_pos)) {
        *new_pos = old_pos;
        return;
    }

    int collided_with_wall = check_wall(board, new_pos);

    if (entity->type == MONSTER) {
        if (collided_with_wall || get_cell_at(board, new_pos->x, new_pos->y) == 'A') {
            entity->facing_dir = get_opposite_direction(entity->facing_dir);
            *new_pos = old_pos;
            char c = map_direction_to_char(entity->facing_dir);
            set_cell_at(board, new_pos->x, new_pos->y, c);
            return;
        }
    }

    if (entity->type == PLAYER && collided_with_wall) {
        *new_pos = old_pos;
    }
}

int compare_positions(t_position *pos1, t_position* pos2) {
    if (pos1->y < pos2->y)
        return -1;

    if (pos1->y > pos2->y)
        return 2;

    if (pos1->x < pos2->x)
        return -1;

    if (pos1->x > pos2->x)
        return 1;

    return 0;
}

void move_entity(t_board *board, t_entity *entity, t_direction dir) {
    t_position old_pos;
    old_pos.x = entity->pos.x;
    old_pos.y = entity->pos.y;

    t_position new_pos = old_pos;
    t_direction new_dir = dir;

    if (entity->type == MONSTER) {
        new_dir = entity->facing_dir;
    }

    switch (new_dir) {
        case UPWARDS:
            new_pos.y--;
            break;
        case LEFT:
            new_pos.x--;
            break;
        case DOWNWARDS:
            new_pos.y++;
            break;
        case RIGHT:
            new_pos.x++;
            break;
        case NONE:
            break;
    }

    handle_collision(board, entity, &new_pos);
    
    entity->pos = new_pos;
}

board.h

#ifndef BOARD_H
#define BOARD_H

#include "game_params.h"
#include "direction.h"
#include <stdio.h>

typedef struct position t_position;
typedef struct entity t_entity;

typedef enum cell_type {
    ENTITY,
    WALL,
    EMPTY
} t_cell_type;

typedef struct board {
    int num_rows;
    int col_size;
    char **cells;
    
    int num_entities;
    int player_index;
    t_entity *entities;
    t_position *goal_positions;
} t_board;

void cleanup_board(t_board *board);

char get_cell_at(t_board *board, int x, int y);
void set_cell_at(t_board *board, int x, int y, char c);

t_cell_type get_cell_type(char c);

void clear_entities_from_board(t_board *board);
void place_entities_on_board(t_board *board);
void print_board(t_board *b, FILE *output);

void get_board_dims(char *buf, int *num_rows, int *col_size);
t_error_object fill_board(t_board *board, char *board_data, int len);
t_error_object handle_entity_alloc(t_board *board, const t_entity *entity,
                                int *actual_entity_count, int *expected_entity_count);
t_error_object set_initial_positions(t_board *board);
t_error_object initialize_board(t_board* board, const t_game_params *params);

#endif

board.c

#include "common.h"
#include "entity.h"
#include "board.h"

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

void cleanup_board(t_board *board) {
    for (int i = 0; i < board->num_rows; i++) {
        if (board->cells[i] != NULL)
            free(board->cells[i]);
    }

    if (board->cells != NULL)
        free(board->cells);

    if (board->entities)
        free(board->entities);
}

char get_cell_at(t_board* board, int x, int y) {
    return board->cells[y][x];
}

void set_cell_at(t_board *board, int x, int y, char c) {
    board->cells[y][x] = c;
}

void clear_entities_from_board(t_board *board) {
    for (int i = 0; i < board->num_entities; i++) {
        t_entity ent = board->entities[i];
        set_cell_at(board, ent.pos.x, ent.pos.y, ' ');
    }
}

void place_entities_on_board(t_board *board) {
    // First draw Player (S)
    t_entity *player = &board->entities[board->player_index];

    // 'A' always stays on top of 'S' when they overlap
    if (get_cell_at(board, player->pos.x, player->pos.y) != 'A')
        set_cell_at(board, player->pos.x, player->pos.y, 'S');

    // Then draw Monsters (M) in reverse (right-to-left)
    // to satisfy the condition that monsters seen earlier
    // should appear before monsters seen at a later point
    // in case some monsters overlap at a single position

    for (int i = board->num_entities - 1; i >= 0; i--) {
        t_entity ent = board->entities[i];

        char symbol = ' ';

        if (ent.type != MONSTER)
            continue;
        
        symbol = map_direction_to_char(ent.facing_dir);

        set_cell_at(board, ent.pos.x, ent.pos.y, symbol);
    }
}

void print_board(t_board *board, FILE *output) {
    place_entities_on_board(board);

    for (int row = 0; row < board->num_rows; row++) {
        for (int col = 0; col < board->col_size; col++) {
            char c = board->cells[row][col];

            if (c != 0)
                fputc(c, output);
        }

        fputc('\n', output);
    }

    clear_entities_from_board(board);
}

void get_board_dims(char *buf, int *num_rows, int *col_size) {
    int num_lines = 0;
    int longest_line_len = 0;

    char* buf_copy = strdup_(buf);
    char* pch = strtok(buf_copy, "\n");

    while (pch != NULL) {
        num_lines++;

        if (strlen(pch) > longest_line_len)
            longest_line_len = strlen(pch);

        pch = strtok(NULL, "\n");
    }

    free(buf_copy);
    buf_copy = NULL;

    *num_rows = num_lines;
    *col_size = longest_line_len;
}

t_error_object fill_board(t_board *board, char *board_data, int len) {
    int cur_row = 0;
    int cur_col = 0;

    char **b = calloc(board->num_rows, sizeof(char*));

    if (b == NULL) {
        return make_error("Konnte keinen Speicherplatz für das Gameboard allozieren", ALLOC_FAILED);
    }

    for (int i = 0; i < board->num_rows; i++) {
        b[i] = calloc(board->col_size, sizeof(char));

        if (b[i] == NULL) {
            return make_error("Konnte keinen Speicherplatz für das Gameboard allozieren", ALLOC_FAILED);
        }
    }

    for (int i = 0; i < len; i++) {
        if (board_data[i] == '\n') {
            cur_row++;
            cur_col = 0;
            continue;
        }

        b[cur_row][cur_col] = board_data[i];
        cur_col++;
    }

    free(board_data);

    board->cells = b;

    return make_error("", OK);
}

t_error_object handle_entity_alloc(t_board *board, const t_entity *entity,
                        int *actual_entity_count, int *expected_entity_count) {
    *actual_entity_count += 1;

    if (*actual_entity_count > *expected_entity_count) {
        *expected_entity_count = *expected_entity_count * 2 + 1;
        board->entities = realloc(board->entities, *expected_entity_count * sizeof(t_entity));
    }

    if (board->entities == NULL) {
        return make_error("Konnte keinen Speicherplatz für die Entitäten allozieren", ALLOC_FAILED);
    }

    board->entities[*actual_entity_count - 1] = *entity;

    return make_error("", OK);
}

t_cell_type get_cell_type(char c) {
    t_entity_type ent_type = get_entity_type(c);

    int is_wall = (c == '#');
    int is_empty = (c == ' ');

    if (ent_type != NO_ENT)
        return ENTITY;

    if (is_wall)
        return WALL;

    if (is_empty)
        return EMPTY;

    return EMPTY;
}

t_error_object set_initial_positions(t_board *board) {
    int expected_entity_count = 1;
    int actual_entity_count = 0;

    board->entities = calloc(expected_entity_count, sizeof(t_entity));
    
    if(board->entities == NULL) {
        return make_error("Konnte keinen Speicherplatz für die Entitäten allozieren", ALLOC_FAILED);
    }

    for (int y = 0; y < board->num_rows; y++) {
        for (int x = 0; x < board->col_size; x++) {
            int c = board->cells[y][x];
            t_cell_type type = get_cell_type(c);

            if (type != ENTITY)
                continue;

            t_entity_type ent_type = get_entity_type(c);
            t_direction ent_dir = map_char_to_direction(c);
            t_entity ent = create_entity(ent_type, x, y, ent_dir);

            t_error_object ret = handle_entity_alloc(board, &ent, &actual_entity_count, &expected_entity_count);

            if (ret.error_code != OK)
                return ret;

            if (ent_type == PLAYER)
                board->player_index = actual_entity_count - 1;
        }
    }

    board->num_entities = actual_entity_count;

    return make_error("", OK);
}

t_error_object initialize_board(t_board *board, const t_game_params *params) {
    int num_rows;
    int col_size;

    int file_size = get_file_size(params->level_file);
    char *level_data = calloc(file_size + 1, sizeof(char));

    if (level_data == NULL) {
        return make_error("Konnte keinen Speicherplatz für das Gameboard allozieren", ALLOC_FAILED);
    }

    fread(level_data, file_size, 1, params->level_file);

    if (ferror(params->level_file) != 0) {
        return make_error("Konnte Level-Datei nicht lesen", COULD_NOT_READ_FILE);
    }

    get_board_dims(level_data, &num_rows, &col_size);

    board->num_rows = num_rows;
    board->col_size = col_size;

    fill_board(board, level_data, file_size);
    set_initial_positions(board);

    return make_error("", OK);
}

direction.h

#ifndef DIRECTION_H
#define DIRECTION_H

typedef enum direction {
    UPWARDS,
    LEFT,
    DOWNWARDS,
    RIGHT,
    NONE
} t_direction;

char map_direction_to_char(t_direction dir);
t_direction map_char_to_direction(char dir);
t_direction get_opposite_direction(t_direction dir);

#endif

direction.c

#include "direction.h"

char map_direction_to_char(t_direction dir) {
    switch (dir) {
        case UPWARDS:
            return '^';
        case LEFT:
            return '<';
        case DOWNWARDS:
            return 'v';
        case RIGHT:
            return '>';
        case NONE:
            return 0;
    }

    return 0;
}

t_direction map_char_to_direction(char dir) {
    switch (dir) {
        case '^':
        case 'w':
            return UPWARDS;
        case '<':
        case 'a':
            return LEFT;
        case 'v':
        case 's':
            return DOWNWARDS;
        case '>':
        case 'd':
            return RIGHT;
    }

    return NONE;
}

t_direction get_opposite_direction(t_direction dir) {
    switch (dir) {
        case UPWARDS:
            return DOWNWARDS;
        case LEFT:
            return RIGHT;
        case DOWNWARDS:
            return UPWARDS;
        case RIGHT:
            return LEFT;
        case NONE:
            return NONE;
    }

    return NONE;
}

dungeon.h

#define DUNGEON_H

#include "game_params.h"
#include "board.h"

typedef enum game_status {
    RUNNING,
    WON,
    LOST
} t_game_status;

void cleanup(t_game_params *params, t_board *board);
t_game_status check_win_or_death(t_board *board);
void game_loop(t_board *board, t_game_params *params);
int main(int argc, char **argv);

#endif

dungeon.c

#include "common.h"
#include "direction.h"
#include "entity.h"
#include "board.h"
#include "dungeon.h"

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

void cleanup(t_game_params *params, t_board *board) {
    cleanup_game_files(params);
    cleanup_board(board);
}

t_game_status check_win_or_death(t_board *board) {
    t_entity *player = &board->entities[board->player_index];

    if (get_cell_at(board, player->pos.x, player->pos.y) == 'A')
        return WON;

    for (int i = 0; i < board->num_entities; i++) {
        t_entity *ent = &board->entities[i];

        if (ent->type == PLAYER)
            continue;

        int positions_match = compare_positions(&player->pos, &ent->pos) == 0;

        if (positions_match && ent->type == MONSTER)
            return LOST;
    }

    return RUNNING; 
}

void game_loop(t_board *board, t_game_params *params) {
    FILE *input_stream = params->input_file;
    FILE *output_stream = params->output_file;

    int step = 1;
    char command = 0;

    t_game_status game_status = RUNNING;

    while (1) {

        fprintf(output_stream, "%d ", step);
        fscanf(input_stream, " %c", &command);

        if (input_stream != stdin) {
            fprintf(output_stream, "%c", command);
            fprintf(output_stream, "\n");
        }

        t_direction dir = map_char_to_direction(command);

        for (int i = 0; i < board->num_entities; i++) {
            t_entity *ent = &board->entities[i];
            move_entity(board, ent, dir);
        }

        game_status = check_win_or_death(board);

        print_board(board, params->output_file);

        if (game_status != RUNNING)
            break;
            
        step++;
    }

    if (game_status == LOST)
        fprintf(output_stream, "Du wurdest von einem Monster gefressen.\n");
    else if (game_status == WON)
        fprintf(output_stream, "Gewonnen!\n");
}

int main(int argc, char **argv) {
    t_game_params params =  {NULL, NULL, NULL};
    t_board board =  {0, 0, NULL, 0, 0, NULL, NULL};

    t_error_object err;

    err = parse_game_parameters(&params, argc, argv);
    
    if (err.error_code != OK) {
        cleanup(&params, &board);
        fprintf(stderr, "%s, error_code: %d\n", err.msg, err.error_code);
        return err.error_code;
    }

    err = initialize_board(&board, &params);

    if (err.error_code != OK) {
        cleanup(&params, &board);
        fprintf(stderr, "%s, error_code: %d\n", err.msg, err.error_code);
        return err.error_code;
    }

    print_board(&board, params.output_file);
    game_loop(&board, &params);

    cleanup(&params, &board);

    return 0;
}

Questions

  • How can I deal with cleaning up resources in a more concise way? As of now, I'm trying to emulate exception handling by letting errors bubble up to main and doing general cleanup there. I thought about passing around a structure (allocator pattern) to error-throwing functions.
  • Better error handling
  • Instead of "abusing" the game board for both drawing and collision checking, should I wrap cells in a custom data structure?
  • I'm still working on fixing const correctness here and there.
  • Is the direction abstraction a good pattern or uselessly bloating my codebase?
  • Is there a better data structure to represent my game board and the dynamic entities?
  • Unifying collision and win checks. I use the canvas state to check for collisions and win but compare the "virtual" player and monster positions to check for a lose.
\$\endgroup\$
4
  • \$\begingroup\$ Minor: strcpy(dst, src); return dst; rewritable as return strcpy(dst, src);. \$\endgroup\$ Sep 8 at 1:27
  • \$\begingroup\$ strncpy won't terminate the string when the input is too long. You can't use it like a limiting strcpy but need to also make sure the last char is written to the destination. \$\endgroup\$
    – JDługosz
    Sep 8 at 14:52
  • \$\begingroup\$ Just out of curiosity, why are you "allowed" to only use C99? Is this for a class? Or maybe you're going to put it on a microcontroller to make a retro game? \$\endgroup\$
    – JDługosz
    Sep 8 at 21:37
  • \$\begingroup\$ P.S. see en.cppreference.com/w/c/string/byte/strncpy "If count is reached before the entire array src was copied, the resulting character array is not null-terminated. If, after copying the terminating null character from src, count is not reached, additional null characters are written to dest until the total of count characters have been written." \$\endgroup\$
    – JDługosz
    Sep 8 at 21:39
5
\$\begingroup\$

Answers to your questions

How can I deal with cleaning up resources in a more concise way? As of now, I'm trying to emulate exception handling by letting errors bubble up to main and doing general cleanup there. I thought about passing around a structure (allocator pattern) to error-throwing functions.

C++ makes this a lot easier, with RAII and language support for exceptions. In C, letting errors "bubble up" and letting main() do the cleanup only works if main() did all the allocations, or can somehow see allocations done by other functions. In larger programs, that is usually not a good strategy.

Split errors into two categories:

  1. Unrecoverable errors, like failing to allocate memory or failing to read a required file. In this case, just print the error message to stderr and call exit(EXIT_FAILURE).

  2. Recoverable errors. Use a return type that can indicate an error status, like a bool representing success or failure, an integer or enum with an error code, or if you return a pointer to an object, NULL might represent failure. Then the caller can decide how to recover from that error.

Instead of "abusing" the game board for both drawing and collision checking, should I wrap cells in a custom data structure?

Having a dedicated type for cells is indeed a good idea.

I'm still working on fixing const correctness here and there.

Yes, a lot of function arguments can be made const.

Is the direction abstraction a good pattern or uselessly bloating my codebase?

I don't think it adds that much bloat, but there are other ways it could have been handled. Consider creating a struct that stores a direction as x and y coordinates, like so:

typedef struct direction {
    int8_t dx;
    int8_t dy;
} t_direction;

Then for example in move_entity(), you no longer need the switch-statement, but can just write:

new_pos.x = old_pos.x + new_dir.dx;
new_pos.y = old_pos.y + new_dir.dy;

Is there a better data structure to represent my game board and the dynamic entities?

There are many ways you can store the board and the entities, all with their own pros and cons. Yours has the advantage that both printing the board and looking up what is at a given position is very easy. If you treat the goal as a dynamic entity, then the only static things remaining are the walls. So you could use a bit array to store it in only one eigth the amount of memory you are currently using, and still be able to fast wall collision detection. Printing the board would be a bit more complex though.

Unifying collision and win checks. I use the canvas state to check for collisions and win but compare the "virtual" player and monster positions to check for a lose.

Yes, ideally when updating the enemies and the player, set game_status if they collide with each other, or if the player collides with the goal.

Unsafe use of strncpy()

When calling make_error(), you copy the string message into the array msg[100] using strncpy(). However, if the length of message was 100 or more characters, then strncpy() will not have written a NUL-byte at the end of msg[]. Either write a NUL-byte to msg[sizeof(msg) - 1] unconditionally, or use a safer function to write into msg[], like snprintf().

Alternatively, don't make a copy at all. You are only ever calling make_error() with a string literal, so you could just store a pointer to the string in t_error_object. This would also make this object much more light-weight.

Misleading error message

If the input filename does not contain ".txt" anywhere in the filename, you return an error that translates to "input file cannot be read". However, the file might be perfectly fine. Either don't restrict the filename, or return an error message saying that the filename should end in ".txt".

Prefer bool for true/false results

A function like check_valid_move() should return a bool to indicate true or false values.

\$\endgroup\$
3
  • \$\begingroup\$ Thank you very much for the thorough answers. I have a question regarding this statement: "Unrecoverable errors, [...] print the error message to stderr and call exit(EXIT_FAILURE)." Does that mean I don't have to do any cleanup if I simply call exit? The whole point of introducing error structures was to be able to call the cleanup function without passing around a pointer to all resources to every function that may throw an error. \$\endgroup\$ Sep 8 at 14:26
  • 1
    \$\begingroup\$ Open files and allocated memory gets cleaned up automatically by the operating system (I assume even on the console you are writing your game for) when a program exits. \$\endgroup\$
    – G. Sliepen
    Sep 8 at 14:38
  • \$\begingroup\$ Ok that would make things a lot easier! \$\endgroup\$ Sep 8 at 14:55
1
\$\begingroup\$
char *strdup_(const char *src) {
    char *dst = malloc(strlen (src) + 1);  
    if (dst == NULL) return NULL;          
    strcpy(dst, src);                      
    return dst;                            
}

You are calling strlen in order to allocate memory, and then calling strcpy which works by calling strlen first! Remember the length, and use memcpy instead.

void cleanup_board(t_board *board) {
    for (int i = 0; i < board->num_rows; i++) {
        if (board->cells[i] != NULL)
            free(board->cells[i]);
    }

    if (board->cells != NULL)
        free(board->cells);

    if (board->entities)
        free(board->entities);
}

Your NULL checking everywhere is unnecessary, since free already contains such a check!

t_error_object fill_board(t_board *board, char *board_data, int len) {

This function free's board_data, which is a parameter passed in and not something it allocated. In C, you need to be careful with ownership responsibilities! This includes being neat about allocating and freeing in the same place, and using naming conventions as to when a copy is returned, when ownership is of a parameter is retained, etc. It is really the bane of existence in the C world.

It does not seem logical that this is really "fill the board, and meanwhile destroy the parameter". And what's the len for?

In this function, board->cells is overwritten, but any old value is not freed. Should it be called something like initialize_board instead, since it must only be called when the board is either freshly declared or after destroying?

return make_error("", OK);

Just pointing out that this will copy 100 '\0' characters into the structure.


You're going to a lot of trouble to measure the file size to read in the whole file, then copying the whole thing again in order to use the destructive strtok, breaking it into lines. You might be better off just using the line-at-a-time file reading functions.


You're reprinting the board at every step. How about using control codes to reposition the cursor, so you can re-draw in place each time?


Instead of your 2-D array being an array of pointers to individually allocated rows, you could use a single pointer and allocation for the entire (rows × cols) block. You already abstracted out get and set functions, so just change them to calculate row * col_count + column explicitly.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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