10
\$\begingroup\$

I have created a simple program that is supposed to be the core of a roguelike game. The program uses ncurses, and all that is supposed to happen is you choose a race and class, then choose an arena. The player can move around, and if the player moves onto deep water or lava, they die.

/*
 * main.c - main file for the mfs (monster fighting simulator)
 */

#include <curses.h> /* ncurses will also work */
#include <stdlib.h>

/*
 * These #define's are the curses colour pairs for different tiles.
 */
#define TILE_IWALL  1 /* indestructible wall */
#define TILE_STONEFLR   2 /* stone floor */
#define TILE_DWATER 3 /* deep water */
#define TILE_SWATER 4 /* shallow water */
#define TILE_LAVA   5 /* lava */
#define TILE_SANDFLR    6 /* sand floor */

/*
 * Declare global variables for the different arenas.
 * # = indestructible wall (impassable)
 * l = lava (impassable)
 * s = shallow water (slowing effect)
 * d = deep water (impassable)
 * . = sand floor (stumbling effect)
 * = = rock floor
 */

/* The Sandsea is basically a standard arena with nothing in it. */
char map_sandsea[256] = {
    '#','#','#','#','#','#','#','#','#','#','#','#','#','#','#','#',
    '#','.','.','.','.','.','.','.','.','.','.','.','.','.','.','#',
        '#','.','.','.','.','.','.','.','.','.','.','.','.','.','.','#',
        '#','.','.','.','.','.','.','.','.','.','.','.','.','.','.','#',
        '#','.','.','.','.','.','.','.','.','.','.','.','.','.','.','#',
        '#','.','.','.','.','.','.','.','.','.','.','.','.','.','.','#',
        '#','.','.','.','.','.','.','.','.','.','.','.','.','.','.','#',
        '#','.','.','.','.','.','.','.','.','.','.','.','.','.','.','#',
        '#','.','.','.','.','.','.','.','.','.','.','.','.','.','.','#',
        '#','.','.','.','.','.','.','.','.','.','.','.','.','.','.','#',
        '#','.','.','.','.','.','.','.','.','.','.','.','.','.','.','#',
        '#','.','.','.','.','.','.','.','.','.','.','.','.','.','.','#',
        '#','.','.','.','.','.','.','.','.','.','.','.','.','.','.','#',
        '#','.','.','.','.','.','.','.','.','.','.','.','.','.','.','#',
        '#','.','.','.','.','.','.','.','.','.','.','.','.','.','.','#',
    '#','#','#','#','#','#','#','#','#','#','#','#','#','#','#','#'
};

/* The Volcano has some lava. */
char map_volcano[256] = {
    '#','#','#','#','#','#','#','#','#','#','#','#','#','#','#','#',
    '#','#','#','#','#','#','=','=','=','=','=','#','#','#','#','#',
        '#','#','#','=','=','=','=','=','=','=','=','=','#','#','#','#',
        '#','#','l','=','=','l','l','=','l','l','=','=','=','#','#','#',
        '#','#','l','l','l','l','l','l','l','l','l','=','=','#','#','#',
        '#','#','l','l','l','l','l','l','=','=','=','=','=','=','#','#',
        '#','#','=','l','l','=','=','=','=','=','=','=','=','=','=','#',
        '#','#','=','=','=','=','=','=','=','=','=','=','=','=','=','#',
        '#','#','=','=','=','=','=','=','=','=','=','=','=','=','=','#',
        '#','#','=','=','=','=','=','l','l','=','=','=','=','=','=','#',
        '#','#','=','=','=','=','l','l','l','l','=','=','=','=','#','#',
        '#','#','=','=','=','=','=','l','l','=','=','=','=','#','#','#',
        '#','#','#','=','=','=','=','=','=','=','=','=','=','#','#','#',
        '#','#','#','#','=','=','=','=','=','=','=','=','#','#','#','#',
        '#','#','#','#','#','=','=','=','=','=','=','#','#','#','#','#',
        '#','#','#','#','#','#','#','#','#','#','#','#','#','#','#','#'
};

char map_beach[256] = {
    '#','#','#','#','#','#','#','#','#','#','#','#','#','#','#','#',
    '#','d','d','d','d','d','d','d','d','d','d','d','d','d','d','#',
        '#','d','d','d','d','d','d','d','d','d','d','d','d','d','d','#',
        '#','d','d','d','d','d','d','d','d','d','d','d','d','d','d','#',
        '#','s','s','d','d','d','d','d','d','d','d','d','d','s','s','#',
        '#','.','s','s','s','d','d','d','d','d','d','s','s','s','s','#',
        '#','.','.','.','s','s','s','s','s','s','s','s','.','.','.','#',
        '#','.','.','.','.','.','.','.','.','.','.','.','.','.','.','#',
        '#','.','.','.','.','.','.','.','.','.','.','.','.','.','.','#',
        '#','.','s','s','.','.','.','.','.','.','.','s','s','.','.','#',
        '#','s','s','s','s','.','.','.','.','.','s','s','s','s','.','#',
        '#','.','s','s','.','.','.','.','.','.','.','s','s','.','.','#',
        '#','.','.','.','.','.','.','.','.','.','.','.','.','.','.','#',
        '#','.','.','.','.','.','.','.','.','.','.','.','.','.','.','#',
        '#','.','.','.','.','.','.','.','.','.','.','.','.','.','.','#',
    '#','#','#','#','#','#','#','#','#','#','#','#','#','#','#','#'

};

char map_cave[256] = {
        '#','#','#','#','#','#','#','#','#','#','#','#','#','#','#','#',
        '#','#','#','#','#','#','=','=','=','=','=','=','=','=','=','#',
        '#','#','#','#','#','=','s','s','=','=','=','=','=','=','=','#',
        '#','#','#','#','=','s','s','s','s','=','=','=','=','=','=','#',
        '#','#','#','=','=','=','s','s','=','=','=','=','=','=','=','#',
        '#','#','=','=','=','=','=','=','=','=','=','=','=','=','=','#',
        '#','#','=','=','=','=','=','=','=','=','=','=','=','=','=','#',
        '#','#','=','=','=','=','=','=','=','=','=','=','=','=','=','#',
        '#','#','=','=','=','=','=','=','=','=','=','=','=','=','=','#',
        '#','#','=','=','=','s','s','s','=','=','=','=','=','=','=','#',
        '#','#','#','=','s','s','s','s','s','=','=','=','=','=','=','#',
        '#','#','#','#','s','=','=','=','s','s','=','=','=','=','=','#',
        '#','#','#','#','#','=','=','=','=','s','s','=','=','=','=','#',
        '#','#','#','#','#','#','=','=','=','s','s','=','=','=','=','#',
        '#','#','#','#','#','#','#','#','#','#','#','#','#','#','#','#',
};

/* Global variables for player position */
unsigned char px = 8;
unsigned char py = 8;

/*
 * Print out the title and the menu, ask what to do.
 */
void print_menu(void)
{
    int ch;

start:  clear();

    attron(A_REVERSE);
    printw("Monster Fighter\n");
    attroff(A_REVERSE);

    printw("[s]tart a new game\n");
    printw("e[x]it\n");

    refresh();
    ch = getch();

    switch (ch) {
    case 's':
        return;
    case 'x':
        endwin();
        exit(EXIT_SUCCESS);
    default:
        goto start;
    }
}

/*
 * Get the race of the player, for example, human, elf, dwarf...
 */
char ask_race(void)
{
    int ch;

start:  clear();

    attron(A_REVERSE);
    printw("Pick a race\n");
    attroff(A_REVERSE);

    printw("[e]lf\n");
    printw("[d]warf\n");
    printw("[h]uman\n");

    refresh();
    ch = getch();

    switch (ch) {
    case 'e':
    case 'd':
    case 'h':
        return (char)ch;
    default:
        goto start;
    }
}

/*
 * Ask player for occupation, or job, of their character.
 */
char ask_job(void)
{
    int ch;

start:  clear();

    attron(A_REVERSE);
    printw("Pick a class\n");
    attroff(A_REVERSE);

    printw("[w]arrior\n");
    printw("[r]ogue\n");

    refresh();
    ch = getch();

    switch (ch) {
    case 'w':
    case 'r':
        return (char)ch;
    default:
        goto start;
    }
}

/*
 * Ask player for map to play on
 */
char ask_map(void)
{
    int ch;

start:  clear();

    attron(A_REVERSE);
    printw("Pick a map/arena\n");
    attroff(A_REVERSE);

    printw("[s]andsea\n");
    printw("[v]olcano\n");
    printw("[b]each\n");
    printw("[c]ave\n");

    refresh();
    ch = getch();

    switch (ch) {
    case 's':
    case 'v':
    case 'b':
    case 'c':
        return (char)ch;
    default:
        goto start;
    }
}

/*
 * Return character on board, takes x and y and the map itself
 */
char get_tile(unsigned char x, unsigned char y, char map[])
{
    return map[x+(y*16)];
}

/*
 * Print out a coloured and formatted tile.
 */
void print_formatted_tile(char tile)
{
    if (has_colors() == TRUE) {
        switch (tile) {
        case '#':
            attron(COLOR_PAIR(TILE_IWALL));
            printw("%c", tile);
            attroff(COLOR_PAIR(TILE_IWALL));
            return;
        case '.':
            attron(COLOR_PAIR(TILE_SANDFLR));
            printw("%c", tile);
            attroff(COLOR_PAIR(TILE_SANDFLR));
            return;
        case '=':
            attron(COLOR_PAIR(TILE_STONEFLR));
            printw("%c", tile);
            attroff(COLOR_PAIR(TILE_STONEFLR));
            return;
        case 'l':
            attron(COLOR_PAIR(TILE_LAVA));
            printw("^");
            attroff(COLOR_PAIR(TILE_LAVA));
            return;
        case 's':
            attron(COLOR_PAIR(TILE_SWATER));
            printw("~");
            attroff(COLOR_PAIR(TILE_SWATER));
            return;
        case 'd':
            attron(COLOR_PAIR(TILE_DWATER));
            printw("~");
            attroff(COLOR_PAIR(TILE_DWATER));
            return;
        }
    } else {
        switch (tile) {
        case 'l':
            printw("^");
            return;
        case 's':
            printw("~");
            return;
        case 'd':
            printw("~");
            return;
        default:
            printw("%c", tile);
            return;
        }
    }
}

/*
 * Print out a map onto the screen along with current hp, etc
 */
void render_screen(
    char map[],
    unsigned char max_hp,
    unsigned char cur_hp,
    unsigned char dexterity,
    unsigned char max_dex,
    unsigned char stealth,
    char race,
    char job)
{
    unsigned char x;
    unsigned char y;

    /* print some information, e.g. hp */
    clear();
    attron(A_REVERSE);
    printw( "Hp: %d/%d  Dex: %d/%d  Slth: %d  Race: %c  Job: %c  Pos: %d,%d\n",
        cur_hp,
        max_hp,
        dexterity,
        max_dex,
        stealth,
        race,
        job,
        px,
        py);
    attroff(A_REVERSE);

    /* render the map */
    for (y = 0; y < 16; y++) {
        for (x = 0; x < 16; x++) {
            printw(" ");
            if (x == px && y == py) { /* player */
                attron(A_BOLD);
                printw("@");
                attroff(A_BOLD);
            } else {
                print_formatted_tile(get_tile(x, y, map));
            }
        }

        printw("\n");
    }

    /* controls */
    attron(A_REVERSE);
    printw("q: quit  hjkl: left down up right  yubn: diagonals\n");
    attroff(A_REVERSE);

    refresh();
}

/*
 * Do stuff based on what the user inputs.
 */
void handle_input(char ch, char map[]) {
    switch (ch) {
    case 'q': /* exit */
        endwin();
        exit(EXIT_SUCCESS);
    case 'h':
        if (get_tile(px-1, py, map) != '#') {
            px -= 1;
        }
        return;
    case 'j':
        if (get_tile(px, py+1, map) != '#') {
            py += 1;
        }
        return;
    case 'k':
        if (get_tile(px, py-1, map) != '#') {
            py -= 1;
        }
        return;
    case 'l':
        if (get_tile(px+1, py, map) != '#') {
            px += 1;
        }
        return;
    case 'y':
        if (get_tile(px-1, py-1, map) != '#') {
            py -= 1;
            px -= 1;
        }
        return;
    case 'u':
        if (get_tile(px+1, py-1, map) != '#') {
            py -= 1;
            px += 1;
        }
        return;
    case 'b':
        if (get_tile(px-1, py+1, map) != '#') {
            py += 1;
            px -= 1;
        }
        return;
    case 'n':
        if (get_tile(px-1, py-1, map) != '#') {
            py -= 1;
            px -= 1;
        }
        return;
    default:
        return;
    }
}

/*
 * Die! And the reason of death.
 */
void tombstone(void)
{
    clear();

    attron(A_REVERSE);
    printw("You have died a horrible death\n\n");
    attroff(A_REVERSE);

    printw("This thing all things devour,\n");
    printw("Birds, beasts, trees, flowers.\n");
    printw("Gnaws iron, bites steel,\n");
    printw("Grinds hard stones to meal.\n");
    printw("Slays king, ruins town,\n");
    printw("And beats high mountain down.\n");
    printw("-- J. R. R. Tolkien's ");

    attron(A_BOLD);
    printw("The Hobbit\n");
    attroff(A_BOLD);

    printw("\n");
    printw("Farewell, adventurer.\n");

    getch();
}

int main(void)
{
    char race; /* e for elf, d for dwarf, etc */
    char job; /* w for warrior, r for rogue */
    char arena; /* which arena to fight on */

    unsigned char max_hp; /* maximum hit points */
    unsigned char dexterity; /* dodging, etc */
    unsigned char max_dex; /* max dexterity */
    unsigned char stealth; /* stealth */
    unsigned char cur_hp; /* current hp */

    unsigned short int i; /* counter variable */
    char map[256];
    int ch; /* input temporary variable */

    max_hp = 0;
    dexterity = 0;
    max_dex = 0;
    stealth = 0;
    cur_hp = 0;

    /* Initialise curses */
    initscr();
    raw();
    noecho();
    keypad(stdscr, TRUE);
    start_color();

    init_pair(1, COLOR_GREEN, COLOR_BLACK); /* indestructible wall */
    init_pair(2, COLOR_WHITE, COLOR_BLACK); /* stone floor */
    init_pair(3, COLOR_BLUE, COLOR_BLACK); /* deep water */
    init_pair(4, COLOR_CYAN, COLOR_BLACK); /* shallow water */
    init_pair(5, COLOR_RED, COLOR_BLACK); /* lava */
    init_pair(6, COLOR_YELLOW, COLOR_BLACK); /* sand floor */

    /* Clear the screen if not already cleared */
    clear();

    /* ask the user for information */
    print_menu();
    race = ask_race();
    job = ask_job();
    arena = ask_map();

    /* Set character stats based on race */
    switch (race) {
    case 'e':
        max_hp += 20;
        max_dex += 10;
        stealth += 10;
        break;
    case 'd':
        max_hp += 20;
        max_dex += 5;
        stealth += 5;
        break;
    case 'h':
        max_hp += 10;
        max_dex += 5;
        stealth += 7;
        break;
    }

    /* Set character stats based on job */
    switch (job) {
    case 'w':
        max_hp += 5;
        stealth -= 5;
        break;
    case 'r':
        max_dex += 2;
        stealth += 5;
        break;
    }

    /* Set map */
    switch (arena) {
    case 's':
        for (i = 0; i < 256; i++) {
            map[i] = map_sandsea[i];
        }
        break;
    case 'v':
        for (i = 0; i < 256; i++) {
            map[i] = map_volcano[i];
        }
        break;
    case 'b':
        for (i = 0; i < 256; i++) {
            map[i] = map_beach[i];
        }
        break;
    case 'c':
        for (i = 0; i < 256; i++) {
            map[i] = map_cave[i];
        }
        break;
    }

    for (;;) {
        render_screen(map, max_hp, cur_hp, dexterity, max_dex, stealth, race, job);
        ch = getch();
        handle_input((char)ch, map);

        if (get_tile(px, py, map) == 's') {
            dexterity = max_dex - 3;
        } else {
            dexterity = max_dex;
        }

        if (get_tile(px, py, map) == 'd' || get_tile(px, py, map) == 'l') {
            tombstone();
            break;
        }
    }

    endwin();
    return 0;
}

Compile command: cc -O2 -fomit-frame-pointer -pipe -march=native -s -o monfight main.c -lcurses

Specific things that I would like to improve the most are speed improvements, because on old computers this program will lag.

Improve this question
\$\endgroup\$

2 Answers 2

Reset to default
5
\$\begingroup\$

Here are some things that may help you improve your code.

Use const where practical

The render_screen routine does not alter the passed map, as expected, so it should be passed as const char map[] instead.

Eliminate global variables where practical

Having routines dependent on global variables makes it that much more difficult to understand the logic and introduces many opportunities for error. Eliminating global variables where practical is always a good idea, whether programming for desktop machines or for embedded systems. For global variables such as px and py, consider wrapping them in a structure and passing a pointer to the structure where needed. For the maps, I would recommend making them all const and then creating a static array to hold all maps.

Eliminate goto

It's almost always possible and almost always desirable to eliminate goto in C code. Doing so allows the control flow to be more easily understood and often eliminates classes of errors such as relying on uninitialized variables. See the code below for a way to do that in your code.

Avoid repeating code

Each of the ask_ functions does something nearly identical. They present a set of choices and allow the user to select one. Rather than having individual but largely repetitive functions for these, it would make more sense to create a generic menu function:

char menu(const char *title, const char *choices[], int numchoices, const char letters[])
{
    int ch;

    for (ch = '0'; NULL == strchr(letters, ch); ch = getch()) {
        clear();
        attron(A_REVERSE);
        printw(title);
        attroff(A_REVERSE);
        for (int i=0; i < numchoices; ++i) {
            printw(choices[i]);
        }
        refresh();
    }
    return ch;
}

Then call the menu as follows:

char ask_job(void)
{
    const char *title = "Pick a class\n";
    const char *choices[] = { "[w]arrior\n", "[r]ogue\n"};
    int numchoices = 2;
    const char *letters="wr";

    return menu(title, choices, numchoices, letters);
}

Alternatively, create a struct for the menu and have menu take a pointer to such a structure. For that you could either have a maximum allowable number of items (so that the choices member would be of fixed size) or use a gcc extension for statically initializing a struct with a flexible array member.

Eliminate "magic numbers"

This code has "magic numbers," that is, unnamed constants such as 16, 256, etc. Generally it's better to avoid that and give such constants meaningful names. That way, if anything ever needs to be changed, you won't have to go hunting through the code for all instances of "16" and then trying to determine if this particular 16 means the width of the playing field or some other constant that happens to have the same value.

Separate related but different functions

The print_formatted_tile has two distinct parts -- one of printing in color and the other not in color. Instead of having to decide which half of the function to use every time a tile is printed, what would make more sense is to divide this into two separate functions, decide once at the beginning of the program which to use and just call that.

Use library functions where appropriate

The switch (arena) in main uses a for loop to copy the arena. Instead, use memcpy which is often optimized for speed and makes your code both shorter and faster.

Eliminate return 0 at the end of main

When a C program reaches the end of main the compiler will automatically generate code to return 0, so there is no reason to put return 0; explicitly at the end of main.

Improve this answer
\$\endgroup\$
2
\$\begingroup\$

Defining maps

Your current map definitions are hard to read:

char map_volcano[256] = {
    '#','#','#','#','#','#','#','#','#','#','#','#','#','#','#','#',
    '#','#','#','#','#','#','=','=','=','=','=','#','#','#','#','#',
        '#','#','#','=','=','=','=','=','=','=','=','=','#','#','#','#',
        '#','#','l','=','=','l','l','=','l','l','=','=','=','#','#','#',
        '#','#','l','l','l','l','l','l','l','l','l','=','=','#','#','#',
        '#','#','l','l','l','l','l','l','=','=','=','=','=','=','#','#',
        '#','#','=','l','l','=','=','=','=','=','=','=','=','=','=','#',
        '#','#','=','=','=','=','=','=','=','=','=','=','=','=','=','#',
        '#','#','=','=','=','=','=','=','=','=','=','=','=','=','=','#',
        '#','#','=','=','=','=','=','l','l','=','=','=','=','=','=','#',
        '#','#','=','=','=','=','l','l','l','l','=','=','=','=','#','#',
        '#','#','=','=','=','=','=','l','l','=','=','=','=','#','#','#',
        '#','#','#','=','=','=','=','=','=','=','=','=','=','#','#','#',
        '#','#','#','#','=','=','=','=','=','=','=','=','#','#','#','#',
        '#','#','#','#','#','=','=','=','=','=','=','#','#','#','#','#',
        '#','#','#','#','#','#','#','#','#','#','#','#','#','#','#','#'
};

You can use a string initializer to get rid of all the ' and , characters:

char map_volcano[256] =
    "################"
    "######=====#####"
    "###=========####"
    "##l==ll=ll===###"
    "##lllllllll==###"
    "##llllll======##"
    "##=ll==========#"
    "##=============#"
    "##=============#"
    "##=====ll======#"
    "##====llll====##"
    "##=====ll====###"
    "###==========###"
    "####========####"
    "#####======#####"
    "################";

Notice that you can continue a string constant across lines by just closing the string on one line and opening a new string on another line. In the future, you may want to add a function that will read a map from a file. That way you can add maps without recompiling the program.

Repetitive code

In handle_input() there is a lot of repeated code:

case 'h':
    if (get_tile(px-1, py, map) != '#') {
        px -= 1;
    }
    return;
case 'j':
    if (get_tile(px, py+1, map) != '#') {
        py += 1;
    }
    return;

You can simplify the function to avoid repetition like this:

void handle_input(char ch, char map[])
{
    int dx = 0;
    int dy = 0;
    switch (ch) {
        case 'q': /* exit */
            endwin();
            exit(EXIT_SUCCESS);
        case 'h': dx = -1; break;
        case 'j': dy =  1; break;
        case 'k': dy = -1; break;
        case 'l': dx =  1; break;
        case 'y': dx = -1; dy = -1; break;
        case 'u': dx =  1; dy = -1; break;
        case 'b': dx = -1; dy =  1; break;
        case 'n': dx =  1; dy =  1; break;
        default:
            return;
    }
    if (get_tile(px+dx, py+dy, map) != '#') {
        px += dx;
        py += dy;
    }
}
Improve this answer
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.