3
\$\begingroup\$

Made the game much more user-friendly than previous version - has an example grid to demonstrate input, Xs and Os are now used.

Game can now detect draw.

Changed the algorithm to detect winner, it's less buggy overall although there are still some combinations near the end of a game where it incorrectly determines a winner or loser - no idea how to fix this or what causes it.

I would appreciate some tips on how to improve structure of the game and code, and how to avoid mentioned bugs.

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

// Struct with all game state variables.
struct game_data {
    int win; // Either 0 or 1.
    int turns; // Ranges from 1 to 9(game end).
    int turn; // Either 0 or 1 where 0 is human player
    char grid[3][3];
};

//Initialising game state variables
struct game_data game = {
    0,
    1,
    0,
    { { ' ', ' ', ' ' },
      { ' ', ' ', ' ' },
      { ' ', ' ', ' ' } }
    };

void intro(void){

    printf("Welcome to NOUGHTS AND CROSSES\n\n");
    printf("The grid you will be playing on is 3x3 and your input will be determined by the co ordinates you put in, in the form 'row column'.\n\n");
    printf("For example an input of '1 1' will put a 'Z' on the first row on the first column. Like so:\n\n");

    printf(
        "+---+---+---+\n"
        "| Z |   |   |\n"
        "+---+---+---+\n"
        "|   |   |   |\n"
        " ---+---+---|\n"
        "|   |   |   |\n"
        "+---+---+---+\n"
        "\n");
}

void player_one_move(struct game_data* game)
{
    int y_val, x_val;
    printf("You are 'Crosses'. Please input co-ordinates in the form 'row column' for the 3x3 grid:\n");
    scanf(" %d %d", &y_val, &x_val);

    if (game->grid[y_val - 1][x_val - 1] == ' ') {
        game->grid[y_val - 1][x_val - 1] = 'X';
        printf("\nYour turn:\n\n");
    }
    else {
        player_one_move(game);
    }
}

void computer_move(struct game_data* game)
{
    int x_val = rand() % 3;
    int y_val = rand() % 3;

    if (game->grid[y_val][x_val] == ' ') {
        game->grid[y_val][x_val] = 'O';
        printf("\nComputer turn:\n\n");
    }
    else {
        computer_move(game);
    }
}

void update(struct game_data* game)
{

    printf(

        "+---+---+---+\n"
        "| %c | %c | %c |\n"
        "+---+---+---+\n"
        "| %c | %c | %c |\n"
        "----+---+---|\n"
        "| %c | %c | %c |\n"
        "+---+---+---+\n"
        "\n",
        game->grid[0][0], game->grid[0][1], game->grid[0][2],
        game->grid[1][0], game->grid[1][1], game->grid[1][2],
        game->grid[2][0], game->grid[2][1], game->grid[2][2]);
}

void game_event_won(struct game_data* game)
{

    int count;

    //BUGGY
    /*char current_mark;

    if (game->turn == 0) {  
        current_mark = 'X';}
    else{
        current_mark = 'O';
    }*/

    for (int y_val = 0; y_val < 3; y_val++) {
        for (int x_val = 0; x_val < 3; x_val++) {
            count = 0;
            while (game->grid[y_val][x_val] == 'X') {
                x_val++;
                count++;

            //BUGGY
            /*if (count == 3 && current_mark == 'X') {   
                game->win = 1;
                printf("You have WON\n");
            }
            if (count == 3 && current_mark == 'O') {
                game->win = 1;
                printf("You have LOST\n");
            }*/

            if (count == 3) {
                game->win = 1;
                printf("You have WON\n");
            }
          }
        }
    }
    for (int x_val = 0; x_val < 3; x_val++) {
         for (int y_val = 0; y_val < 3; y_val++) {
             count = 0;
             while (game->grid[y_val][x_val] == 'X') {
                 y_val++;
                 count++;

                 if (count == 3) {
                     game->win = 1;
                     printf("You have WON\n");
                 }
             }
         }
     }
     for (int y_val = 0; y_val < 3; y_val++) {
         count = 0;
         while (game->grid[y_val][y_val] == 'X') {
             count++;
             y_val++;

             if (count == 3) {
                  game->win = 1;
                  printf("You have won\n");
             }
         }
     }
     for (int y_val = 0; y_val < 3; y_val++) {
         count = 0;
         while (game->grid[y_val][2 - y_val] == 'X') {
             count++;
             y_val++;

             if (count == 3) {
                 game->win = 1;
                 printf("You have won\n");
             }
         }
     }
 }


 // Repetition of previous function but for 'O's. Less concise but less buggy than previous implementation.
 void game_event_lost(struct game_data* game)
 {

      int count;

      for (int y_val = 0; y_val < 3; y_val++) {
           for (int x_val = 0; x_val < 3; x_val++) {
               count = 0;
               while (game->grid[y_val][x_val] == 'O') {
                    x_val++;
                    count++;


                    if (count == 3) {
                         game->win = 1;
                         printf("You have LOST\n");
                    }
              }
         }
     }
     for (int x_val = 0; x_val < 3; x_val++) {
     for (int y_val = 0; y_val < 3; y_val++) {
        count = 0;
        while (game->grid[y_val][x_val] == 'O') {
            y_val++;
            count++;

            if (count == 3) {
                game->win = 1;
                printf("You have LOST\n");
            }
        }
    }
  }
for (int y_val = 0; y_val < 3; y_val++) {
    count = 0;
    while (game->grid[y_val][y_val] == 'O') {
        count++;
        y_val++;

        if (count == 3) {
            game->win = 1;
            printf("You have LOST\n");
        }
    }
}
for (int y_val = 0; y_val < 3; y_val++) {
    count = 0;
    while (game->grid[y_val][2 - y_val] == 'O') {
        count++;
        y_val++;

        if (count == 3) {
            game->win = 1;
            printf("You have LOST\n");
            }
        }
    }
}


int main(void)
{

    srand((unsigned)time(0));

    intro();

    while (game.win == 0 ) {
        if (game.turn == 0) {
            player_one_move(&game);
            game.turns++;
            game.turn = 1;
        }

        else {
            game.turn = 0;
            computer_move(&game);
            game.turns++;
        }
        if (game.turns == 9 && game.win == 0){
            game.win = 1;
            printf("You have drawn\n");
            break;
        }
        update(&game);


        game_event_won(&game);
        game_event_lost(&game);

    }

    return 0;
}

EDIT:

I've changed the game_event_won function. Much simpler and bug free as far I know, even if it's a little uglier. Could probably make it shorter though.

 void game_event_won(struct game_data* game)
    {

      if( ((game->grid[0][0] == game->grid[0][1]) && (game->grid[0][1] == game->grid[0][2]) && (game->grid[0][2] == 'X')) ||
          ((game->grid[1][0] == game->grid[1][1]) && (game->grid[1][1] == game->grid[1][2]) && (game->grid[1][2] == 'X')) ||
          ((game->grid[2][0] == game->grid[2][1]) && (game->grid[2][1] == game->grid[2][2]) && (game->grid[2][2] == 'X')) ||
          ((game->grid[0][0] == game->grid[1][0]) && (game->grid[1][0] == game->grid[2][0]) && (game->grid[2][0] == 'X')) ||
          ((game->grid[0][1] == game->grid[1][1]) && (game->grid[1][1] == game->grid[2][1]) && (game->grid[2][1] == 'X')) ||
          ((game->grid[0][2] == game->grid[1][2]) && (game->grid[1][2] == game->grid[2][2]) && (game->grid[2][2] == 'X')) ||
          ((game->grid[0][0] == game->grid[1][1]) && (game->grid[1][1] == game->grid[2][2]) && (game->grid[2][2] == 'X')) ||
          ((game->grid[0][2] == game->grid[1][1]) && (game->grid[1][1] == game->grid[2][0]) && (game->grid[2][0] == 'X')) ){
             game->win = 1;
             printf("You have WON\n");
       }

       if( ((game->grid[0][0] == game->grid[0][1]) && (game->grid[0][1] == game->grid[0][2]) && (game->grid[0][2] == 'O')) ||
           ((game->grid[1][0] == game->grid[1][1]) && (game->grid[1][1] == game->grid[1][2]) && (game->grid[1][2] == 'O')) ||
           ((game->grid[2][0] == game->grid[2][1]) && (game->grid[2][1] == game->grid[2][2]) && (game->grid[2][2] == 'O')) ||
           ((game->grid[0][0] == game->grid[1][0]) && (game->grid[1][0] == game->grid[2][0]) && (game->grid[2][0] == 'O')) ||
           ((game->grid[0][1] == game->grid[1][1]) && (game->grid[1][1] == game->grid[2][1]) && (game->grid[2][1] == 'O')) ||
           ((game->grid[0][2] == game->grid[1][2]) && (game->grid[1][2] == game->grid[2][2]) && (game->grid[2][2] == 'O')) ||
           ((game->grid[0][0] == game->grid[1][1]) && (game->grid[1][1] == game->grid[2][2]) && (game->grid[2][2] == 'O')) ||
           ((game->grid[0][2] == game->grid[1][1]) && (game->grid[1][1] == game->grid[2][0]) && (game->grid[2][0] == 'O')) ){
             game->win = 1;
             printf("You have LOST\n");
       }

  }

I've also changed the main loop to fit it, preventing false draws:

    int main(void)
    {

        srand((unsigned)time(0));

        intro();

        while (game.win == 0 ) {
        if (game.turn == 0) {
            player_one_move(&game);
            game.turns++;
            game.turn = 1;
        }

        else {
            game.turn = 0;
            computer_move(&game);
            game.turns++;
        }

        update(&game);

        game_event_won(&game);

        if (game.turns == 10 && game.win == 0){
            game.win = 1;
            printf("You have DRAWN\n");
            break;
        }


    }



    return 0;
}
\$\endgroup\$
1
\$\begingroup\$

Input validation

User input is evil - be it nefarious or just wrong. Spend some effort to validate it.

printf("You are 'Crosses'. Please input co-ordinates ...\n");
// scanf(" %d %d", &y_val, &x_val);
if (scanf(" %d %d", &y_val, &x_val) != 2) {
  puts("Invalid input");
  exit(EXIT_FAILURE);
}
if (y_val < 1 || y_val > 3 || x_val < 1 || x_val > 3) {
  puts("Out of range input");
  exit(EXIT_FAILURE);
}

Better code would selectively allow re-inputting.

Remove //BUGGY comments

Post working code here. For bugs you have trouble solving, consider Stack overflow.

Use const

Functions that do not change their pointed-to data are better coded with const. That allows for clearer function interface, wider application, more compiler time checks and potential better optimization.

// void update(struct game_data* game)
void update(const struct game_data* game)

Re-use code

Rather than 8 lines of if( ((game->grid[0][0] == game->grid[0][1]) ... with 'X' and then 8 more lines of similar code with 'O', form a helper function.

 char three_in_row(const char g[][3]) {
   if(g[0][0] != ' ' && g[0][0] == g[0][1] && g[0][1] == g[0][2]) return g[0][0];
   if(g[1][0] != ' ' && g[1][0] == g[1][1] && g[1][1] == g[1][2]) return g[1][0];
   if(g[2][0] != ' ' && g[2][0] == g[2][1] && g[2][1] == g[2][2]) return g[2][0];
   if(g[0][0] != ' ' && g[0][0] == g[1][0] && g[1][0] == g[2][0]) return g[0][0];
   if(g[0][1] != ' ' && g[0][1] == g[1][1] && g[1][1] == g[2][1]) return g[0][1];
   if(g[0][2] != ' ' && g[0][2] == g[1][2] && g[1][2] == g[2][2]) return g[0][2];
   if(g[0][0] != ' ' && g[0][0] == g[1][1] && g[1][1] == g[2][2]) return g[0][0];
   if(g[0][2] != ' ' && g[0][2] == g[1][1] && g[1][1] == g[2][0]) return g[0][2];
   return ' ';
 }

void game_event_won(struct game_data* game) {
  switch (three_in_row(game->grid)) {
    case 'X': game->win = 1; printf("You have WON\n"); break;
    case 'O': game->win = 1; printf("You have LOST\n"); break;
  }
}

State assessment

Rather than game_event_won(), consider a function TTT_rate(game) that rates the board from -100 to 100.

0: tie
100: X won
-100: O won
1...99: X winning
-1...-99: O winning

No need for recursion

Use a loop in computer_move(struct game_data* game)

Comment and code doe not jibe

Looks like the range is [1...10].

int turns; // Ranges from 1 to 9(game end).
if (game.turns == 10 && game.win == 0){

Format

I'd expect the while block to be indented.

Tip: Use an auto formatter. Manually formatting is unproductive.

    while (game.win == 0 ) {
    // if (game.turn == 0) {
      if (game.turn == 0) {
      ...

No need for global data

Move struct game_data game from global to inside main().

Why should user always go first?

Consider allowing user or computer to go first.

Advanced

Call a function for X and O moves. The function pair could be from user_move(), computer_move(), smarter_computer_move1(), smarter_computer_move2(), etc. This would allow 2 to play, computer algorithm 1 vs algorithm 2, user vs various levels of play, etc.

\$\endgroup\$
  • \$\begingroup\$ Would you be able to explain more about the state assessment? What would you judge the rating of the board on? \$\endgroup\$ – Abdi Mo Feb 16 at 20:28
  • 1
    \$\begingroup\$ @AbdiMo Of course 3-in-a row returns 100,-100. The number of 2 of 3 in a row possibilities that could complete worth 12 each. The number of 1 of 3 in a row possibilities that could complete worth 2 each. The user whose turn it is next rates a point. Or another approach, just rate each square. The key is that this is a heuristic evaluational. You could try 2 different approaches and play them off against each other. Baby AI steps. \$\endgroup\$ – chux - Reinstate Monica Feb 16 at 20:35

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.