6
\$\begingroup\$

I was bored at school, and it was forbidden to use phones in classes; so I began writing a minesweeper game on my book and tested it on a simple compiler on my phone and copied into my computer after I got home.

I think my code is OK, but I'm afraid that I might be getting into the Dunning-Kruger effect.

#include <stdio.h>
#include <stdlib.h>
// seed functions
#include <time.h>
#include <string.h>
// timing functions
#include <sys/time.h>
// functions to catch signals
#include <signal.h>
#include <unistd.h>

// for the field itself
#define MINE 9
// for the mask
#define FLAG 9
#define EMPTY 0

#define IS_MINE(_Tile) (_Tile == MINE)

// the actual field
int **field;
// shows what the player knows
int **mask;
// sizes of the field
int height;
int width;
// mine count of the current field
int mine_cnt;

void help(char *prog, int h) {
    puts("Minestest Copyright (C) 2018 Arda Ünlü");
    puts("This program comes with ABSOLUTELY NO WARRANTY.");
    puts("This is free software, and you are welcome to redistribute it");
    puts("under certain conditions.");
    printf("Run \"%s --help\" for help.\n\n", prog);

    if(h) {
        printf("Usage: %s [difficulty | height width mine_count]\n", prog);
        puts("Difficulty can be easy, medium, or hard.");
        puts("Input format while playing:");
        puts("a b c");
        puts("a: x coordinate");
        puts("b: y coordinate");
        puts("c: 0 or 1: step on cell or flag cell (default: 0)");
    }
}

// handling signals and freeing malloc'd areas
// so we won't leak memory
// except a sigkill, then rip
void handle(int sig) {
    printf("Caught signal %d! Exiting.\n", sig);

    for(int i = 0; i < height; i++) {
        free(field[i]);
        free(mask[i]);
    }
    free(field);
    free(mask);

    exit(1);
}

// setup for the signal handler
// the actual handler is the function above, void handle(int sig)
void setupsig() {
    struct sigaction sigIntHandler;

    sigIntHandler.sa_handler = handle;
    sigemptyset(&sigIntHandler.sa_mask);
    sigIntHandler.sa_flags = 0;

    sigaction(SIGINT, &sigIntHandler, NULL);
}

void parse_args(int argc, char *argv[]) {
    if(argc > 1) {
        // predefined difficulty levels
        // and calling help
        if(argc == 2) {
            if((strcmp(argv[1], "-h") == 0)
            || (strcmp(argv[1], "--help") == 0)) {
                help(argv[0], 1);
                exit(0);
            }
            else if(strcmp(argv[1], "easy") == 0) {
                height   = 8;
                width    = 8;
                mine_cnt = 10;
            }
            else if(strcmp(argv[1], "medium") == 0) {
                height   = 16;
                width    = 16;
                mine_cnt = 40;
            }
            // "hard" lands here, as well as anything else
            else {
                height   = 16;
                width    = 30;
                mine_cnt = 99;
            }
        }
        // allow player to define their own difficulty levels
        else if(argc == 4) {
            height   = (atoi(argv[1]) ? : 16);
            width    = (atoi(argv[2]) ? : 30);
            mine_cnt = (atoi(argv[3]) ? : 99);
        }
        // arguments are invalid
        else {
            help(argv[0], 1);
            exit(1);
        }
    }
    // no arguments
    else {
        height   = 16;
        width    = 30;
        mine_cnt = 99;
    }

    // padding the field so that
    // we have zeroes all around
    height += 2;
    width  += 2;
}

void print_field() {
    char *cell[12] = {
        // cell labels, 0 to 8
        "\x1b[0m0",
        "\x1b[94m1",
        "\x1b[32m2",
        "\x1b[91m3",
        "\x1b[34m4",
        "\x1b[31m5",
        "\x1b[36m6",
        "\x1b[35m7",
        "\x1b[37m8",
        "\x1b[41;30mO\x1b[0m", //mine
        "\x1b[41;30mP\x1b[0m", //flag
    };

    printf("%d mine%s left.\n", mine_cnt, (mine_cnt == 1 ? "" : "s"));

    putchar(' '); // pad the x coordinates by 1 character

    // print x coordinates
    for(int x = 1; x < width-1; x++) {
        printf("%d", x%10);
    }
    putchar('\n');

    for(int y = 1; y < height-1; y++) {
        // reset color sequence and print the y coordinate
        printf("\x1b[0m%d", y%10);
        // print every cell on that y line
        for(int x = 1; x < width-1; x++) {
            // if the player knows anything about the cell
            // print it
            if(mask[y][x] != EMPTY) {
                printf("%s", mask[y][x] == FLAG ? cell[10] : cell[field[y][x]]);
            }
            // if they don't, just print a dot
            else {
                printf("\x1b[0m.");
            }
        }
        putchar('\n');
    }
    // reset colors after finishing printing
    printf("\x1b[0m\n");
}

int fill_field() {
    // be sure that there can be empty cells
    if(height * width < mine_cnt) {
        puts("Too many mines.");
        return 1;
    }

    // allocate our fields
    field = malloc(height * sizeof(int*));
    mask = malloc(height * sizeof(int*));

    // let's not segfault
    if(!field || !mask)
        return 1;

    // allocate every line
    for(int i = 0; i < height; i++) {
        field[i] = malloc(width * sizeof(int));
        mask[i] = malloc(width * sizeof(int));
        if(!field[i] || !mask[i])
            return 1;
    }

    // fill the mask
    for(int y = 0; y < height; y++) {
        for(int x = 0; x < width; x++) {
            mask[y][x] = !EMPTY;
        }
    }

#ifndef DEBUG
    for(int y = 1; y < height-1; y++) {
        for(int x = 1; x < width-1; x++) {
            mask[y][x] = EMPTY;
        }
    }
#endif

    // temporary variables for mine locations
    int coord_a, coord_b;

    // initialize main field
    for(int y = 0; y < height; y++) {
        for(int x = 0; x < width; x++) {
            field[y][x] = EMPTY;
        }
    }

    srand(time(NULL));

    //fill mines
    for(int i = 0; i < mine_cnt; i++) {
        coord_a = (rand() % (height-2)) + 1;
        coord_b = (rand() % (width-2))  + 1;
        //don't put mines in the same cell twice
        if(IS_MINE(field[coord_a][coord_b])) {
            i--;
            continue;
        }
        field[coord_a][coord_b] = MINE;
    }

    //fill numbers one by one
    for(int y = 1; y < height-1; y++) {
        for(int x = 1; x < width-1; x++) {
            // don't put a number if the current cell is a mine
            if(!IS_MINE(field[y][x])) {
                // looping the 3x3 adjacent cells
                for(int dy = -1; dy <= 1; dy++) {
                    for(int dx = -1; dx <= 1; dx++) {
                        // skip the current cell
                        if(dy == 0 && dx == 0) continue;
                        field[y][x] += IS_MINE(field[y + dy][x + dx]);
                    }
                }
            }
        }
    }

    return 0;
}

// this function has 2 uses that basically use the same algorithm.
// why == 1) if the cell player inputted was 0, extend that field
//           until we hit a cell which is adjacent to a mine
// why == 2) if the player inputs a cell which they already know,
//           assume they want to step on every adjacent cell
int explore_neighbors(int x, int y, int why) {
    // don't remove the flag, if there is one, if we are being
    // called as a recursion from the same function
    // if there is no flag, just remove the mask
    if(mask[y][x] != FLAG)
        mask[y][x] = !EMPTY;

    // counter for adjacent mines
    int cnt = 0;
    // loop the 3x3 adjacent cells
    for(int dy = -1; dy <= 1; dy++) {
        for(int dx = -1; dx <= 1; dx++) {
            // skip the current cell
            if(dy == 0 && dx == 0) continue;
            // why == 1) don't expand if there are mines in adjacent cells
            if(why == 1) {
                if(IS_MINE(field[y + dy][x + dx])) return 1;
            }
            // why == 2) count the mines in adjacent cells
            else
                cnt += IS_MINE(mask[y + dy][x + dx]);
        }
    }

    // if the user inputted a cell they certainly
    // know not to be empty
    if(why == 2 && cnt != 0) {
        for(int dy = -1; dy <= 1; dy++) {
            for(int dx = -1; dx <= 1; dx++) {
                // if there is a mine that was placed wrongly
                if(IS_MINE(mask[y + dy][x + dx])
                != IS_MINE(field[y + dy][x + dx])) {
                    // exit game
                    return 1;
                }
                // if not, do nothing
            }
        }
    }

    // why == 1) do this unconditionally
    // why == 2) if the number of adjacent mines correct
    if((why == 1) || (cnt == field[y][x])) {
        // loop the adjacent 3x3 block
        for(int dy = -1; dy <= 1; dy++) {
            for(int dx = -1; dx <= 1; dx++) {
                if(why == 1) {
                    // skip the current cell
                    if(dy == 0 && dx == 0) continue;
                    // expand if there are more empty areas
                    if(mask[y + dy][x + dx] == EMPTY)
                        explore_neighbors(x + dx, y + dy, 1);
                }
                // why == 2
                else {
                    // skip the current cell
                    if(dy == 0 && dx == 0) continue;
                    // unmask the adjacent cells
                    // while leaving the flags
                    if(mask[y + dy][x + dx] != FLAG)
                        mask[y + dy][x + dx] = !EMPTY;
                    // if there is a filed with no mines nearby,
                    // expand to there
                    if(field[y + dy][x + dx] == EMPTY)
                        explore_neighbors(x + dx, y + dy, 1);
                }
            }
        }
    }

    return 0;
}

int play_game() {
    int ret = 0;
    // internal variables for the loop
    int flag, finished, w;
    // input variables
    int in_x, in_y, in_f;

    char input_buf[99];

    print_field();

    while(1) {
        // prompt
        printf("> ");
        fgets(input_buf, 99, stdin);
        flag = sscanf(input_buf, "%d %d %d", &in_x, &in_y, &in_f);

        // if the user didn't enter a flag
        if(flag == 2) {
            //set the internal variable to 0
            in_f = 0;
            // increment the mine count variable if the
            // current cell was flagged previously
            if(IS_MINE(mask[in_y][in_x])) {
                mine_cnt++;
            }
        }
        // if the user entered an invalid input
        // just skip it
        else if(flag != 3) {
            continue;
        }
        puts("");

        // if the input is out of bounds, skip it
        if(in_x > width - 2 || in_y > height - 2 || in_x < 1 || in_y < 1) {
            continue;
        }

        // if the inputted cell is a mine
        // and the user didn't enter a flag
        // end game
        if(IS_MINE(field[in_y][in_x]) && in_f == 0) {
            ret = 1;
            break;
        }

        // if the current cell wasn't flagged before
        // and the user flagged now,
        // decrement the mine count variable
        if(in_f && !IS_MINE(mask[in_y][in_x])) mine_cnt--;

        // the reason to call explore_neighbors
        w = 0;
        // if the current cell is 0, we want to
        // expand into the zero-mine area
        if(field[in_y][in_x] == EMPTY) w = 1;
        // if the user know the cell they entered was empty,
        // then they thnk they know all the adjacent mines.
        // they want to expand to the adjacet 3x3 area.
        else if(mask[in_y][in_x] == !EMPTY) w = 2;
        // if the variable w is set and the user didn't enter a flag,
        // call explore_neighbors with the current cell coordinates
        // and the reason why we want to explore
        if(w && in_f == 0) {
            ret = explore_neighbors(in_x, in_y, w);
        }
        if(w != 2) ret = 0;
        // if ret is set, that means the user entered
        // a known cell with wrong adjacent mines.
        // exit game.
        else if(ret) break;

        // unmask or flag the current cell according to the input
        mask[in_y][in_x] = in_f ? FLAG : !EMPTY;

        // print the field after the operation
        print_field();

        finished = 0;
        for(int y = 0; y < height; y++) {
            for(int x = 0; x < width; x++) {
                // the variable starts as 0 and increments
                // only if the current cell is masked,
                // i.e. the user doesn't know about it yey
                finished += !mask[y][x];
            }
        }
        // if that variable is still zero that means the user
        // has unmasked/flagged every cell.
        // but we still have to check if they just randomly put
        // flags everywhere.
        if(!finished && mine_cnt == 0) {
            puts("Congratulations! You've beaten the game!");
            break;
        }
    }

    // if the user failed, control flow gets here
    if(ret == 1) {
        puts("Better luck next time!");

        // unmask ever cell and print
        for(int y = 0; y < height; y++) {
            for(int x = 0; x < width; x++) {
                mask[y][x] = !EMPTY;
            }
        }
        print_field();
    }

    // hopefully we won't leak any memory
    for(int i = 0; i < height; i++) {
        free(field[i]);
        free(mask[i]);
    }
    free(field);
    free(mask);

    return ret;
}

int main(int argc, char *argv[]) {
    // set up the signal handling code
    setupsig();

    int ret = 0;

    parse_args(argc, argv);

    help(argv[0], 0);

    // exit if an error occures while trying to set up the field
    if((ret = fill_field()) != 0) {
        return ret;
    }

    struct timeval begin, end;
    // the time when the game actually starts
    gettimeofday(&begin, NULL);
    ret = play_game();
    // the time after the game ends
    gettimeofday(&end, NULL);

    printf("Game lasted %.2f seconds.\n",
           (double) (end.tv_usec - begin.tv_usec) / 1000000
         + (double) (end.tv_sec  - begin.tv_sec));

    return ret;
}
\$\endgroup\$
  • 2
    \$\begingroup\$ Oops, I've noticed a bug but I don't want to edit the question. if(height * width < mine_cnt) should be if((height-2) * (width-2) < mine_cnt). \$\endgroup\$ – betseg Feb 21 '18 at 11:23
  • 1
    \$\begingroup\$ Do note that your inclusion of the GPL has no effect; when you signed up you agreed that all code posted on CodeReview would be licensed under Creative Commons Sharealike (CC BY-SA 3.0). \$\endgroup\$ – Snowbody Feb 22 '18 at 5:39
  • \$\begingroup\$ @Snowbody Wouldn't dual licensing be at play here? \$\endgroup\$ – iokanuon Sep 16 '18 at 14:17
8
\$\begingroup\$

Use proper functions instead of macros

Macros can get surprisingly hard to handle. For example IS_MINE(MINE ^ MINE) returns true, whereas IS_MINE((MINE ^ MINE)) returns false.

Since you use C99, prefer inline functions instead:

inline int IS_MINE(int tile) { return tile == MINE; }

Prefer proper (constant) variables instead of macros

static const int MINE = 9;
static const int FLAG = 9;
static const int EMPTY = 0;

That's important as soon as you change int to another type, as the compiler can now issue warnings.

Make function-specific read-only data static and const

The cell in print_field never gets changed, and only needs to get initialized once. We do not need to conjure a new cell every time.

Also, we have to make sure that we don't change the contents of cell, so we should make it const:

static const char *cell[12] = {
    ....
};

Keep the global variables to a minimum

Yes, it's a game, but a proper struct that contains your current state makes sure that you a) don't accidentally change a global variable where you didn't intend to and b) don't forget any game variable.

Use enumerations for variables that contain only some distinct values

The neighbours can get explored in two circumstances, therefore either why = 1 or why = 2 in explore_neighbors.

But those are magic numbers. We can enumerate those reasons and use the enumeration instead:

enum exploration {
    EXPLORE_EMPTY_CELLS,      //!< explores all surrounding non-mine cells
    REVEAL_SURROUNDING_CELLS  //!< explores all cells around the current cell
};

It's a lot easier to read later in your code. Compare

explore_neighbors(x, y, EXPLORE_EMPTY_CELLS);

to

explore_neighbors(x, y, 1);

Use descriptive variable names

i for iteration is fine, but w for "exploration reason why" isn't.

Keep the scope of your variables short

You already use C99, so keep the scope of your variables to a minimum. You never use flag outside of your while(1) in play_game, so move it into the loop, for example.

Use sizeof instead of magic numbers on arrays with static size

In play_game, you use 99 twice:

char input_buf[99];

print_field();

while(1) {
    // prompt
    printf("> ");
    fgets(input_buf, 99, stdin);

That's error prone. You might change 99, get a phone call or get called into a meeting, and then you forgot to change the other. Instead, use sizeof or a compile-time constant instead:

print_field();

while(1) {
    char input_buf[99];
    // prompt
    printf("> ");
    fgets(input_buf, sizeof(input_buf), stdin);

Use sizeof(array)/sizeof(array[0]) if you don't use char in a similar situation.

Prefer a single allocation for a field

We don't need to call malloc so often. A single call is fine if we access the cells like

field[x + y * width];

or similar. Less allocations means less possible errors. You can use

// If you keep `width' and `height' global, the function
// will be index_ptr(int x, int y, int * memory)

inline int* index_ptr(int width, int height, int x, int y, int * memory) {
    assert(0 <= x && x < width);
    assert(0 <= y && y < height);

    return memory + x + (y * width);
}

inline int index(int width, int height, int x, int y, int * memory) {
    return *index_ptr(width, height, x, y, memory);
}

if you don't want to remember the formula. Note that this has the nice side-effect that we now can add additional checks for our accesses.

Our allocation for field and mask gets now easy as well:

field = malloc(sizeof(*field) * width * height);
mask  = malloc(sizeof(*mask) * width * height);
\$\endgroup\$
  • \$\begingroup\$ Other topics I might add later: superfluous function calls, non-standard ternary operator, lengthy functions. \$\endgroup\$ – Zeta Feb 21 '18 at 17:16
  • \$\begingroup\$ Mostly agree aside from wide application of "Use proper (constant) variables instead of macros". In this case, using a const object rather than a constant, as you suggest, can be done. Yet this advice, which is often good, is not a truism. There are cases where a constant is needed. \$\endgroup\$ – chux Feb 21 '18 at 17:34
  • \$\begingroup\$ @chux Yeah, string concatenation comes to mind. I'll changed the wording to "Prefer". \$\endgroup\$ – Zeta Feb 21 '18 at 18:47
  • \$\begingroup\$ Aside: for large allocations (not applicable here) , consider the difference between malloc(width * height * sizeof(*mask)); and malloc(sizeof(*mask) * width * height);. Given int width, height the first may overflow multiplications that that 2nd one does not due to wider size_t math. \$\endgroup\$ – chux Feb 21 '18 at 19:06
  • 1
    \$\begingroup\$ @chux good detail for your "malloc() style" section. Thanks \$\endgroup\$ – Zeta Feb 21 '18 at 19:20
5
\$\begingroup\$

Good stuff

Impressive early use of signal handling.

Good formating.

Good comment level.

Functional error?

Something thing is amiss concerning height and width. I'd expect height possible values for coord_a, not height-2.

// ??
coord_a = (rand() % (height-2)) + 1;

With using a sub-range of height-2, height * width < mine_cnt then does not look like a proper test.

I'd recommend a review and tolerate mine fields as small as 1x1.

Ah, I now see height += 2; width += 2; which handle most of this concern.

As height is no longer the mine field height, use of a different variable name would add clarity. height_p2 maybe?

Either way, if(height * width < mine_cnt) is likely incorrect. Perhaps if((height - 2) * (width - 2) < mine_cnt)

Non-standard code

My compile reported: "ISO C forbids omitting the middle term of a ?: expression".

// height   = (atoi(argv[1]) ? : 16);
// Perhaps?
int i = atoi(argv[1]);
height = i > 0 ? i : 16;  // also perform sign check

// Later code depends on `height > 2` so maybe
height = i > 2 ? i : 16; 
// Also suggest an sane upper bound check

Avoid naked magic numbers

Either add a comment to detail the screen escape sequences or use some constant

    "\x1b[94m1",  // Bright Blue
    "\x1b[32m2",  // Green
    // or
    #define GREEN "\x1b[32m2"

Let code compute

// fgets(input_buf, 99, stdin);
fgets(input_buf, sizeof input_buf, stdin);

malloc() style

Rather than ptr = malloc(n * sizeof (some_type)), consider ptr = malloc(sizeof *ptr * n). It is easier to code right, review and maintain. Leading with sizeof() insures correct math for more complicated computations.

// field = malloc(height * sizeof(int*));
field = malloc(sizeof *field * height);

Debug idea

Allow a select seed, via command options, for srand(time(NULL));. Then one can use a desired set-up.


Minor stuff follows


Global variables

I am neither a fan nor a opponent of global variables. Either approach has it merits, yet in this case, consider a structure storing the puzzle data and pass the b_mine *state through funcitons. This approach allows for future growth and, in my experience, easier to debug.

// int **field;
//... 
// int mine_cnt;

typedef struct {
  int **field;
  //... 
  int mine_cnt;
} b_mine;

Why #define?

Although #define IS_MINE(_Tile) (_Tile == MINE) is certainly a benign and speedy use of a macro for an otherwise is_mine() function, be wary of using define when a function will suffice.

Concatenate literals

An alternative to many puts()

    puts(
      "Input format while playing:\n"
      "a b c\n"
      "a: x coordinate\n";
      ...
      "c: 0 or 1: step on cell or flag cell (default: 0)");
\$\endgroup\$
  • \$\begingroup\$ About the functional error, I noticed it too. Also, what does the b in b_mine stand for? \$\endgroup\$ – betseg Feb 21 '18 at 18:08
  • \$\begingroup\$ @betseg mine seemed too short for a distinctive type name, 'b' is the first character of betseg. Had code used width_p2 or the like, I think neither of us would have made that mistake. \$\endgroup\$ – chux Feb 21 '18 at 18:39

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.