Minesweeper in C

Question

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;
}

• betseg: 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).
• Snowbody: 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).
• karolba: @Snowbody Wouldn't dual licensing be at play here?

Answer 1

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);

• Zeta: Other topics I might add later: superfluous function calls, non-standard ternary operator, lengthy functions.
• chux: 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.
• Zeta: @chux Yeah, string concatenation comes to mind. I'll changed the wording to "Prefer".
• chux: 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.
• Zeta: @chux good detail for your "malloc() style" section. Thanks

Answer 2

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)");

• betseg: About the functional error, I noticed it too. Also, what does the b in b_mine stand for?
• chux: @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.