4
\$\begingroup\$

This is my attempt at making a basic TicTacToe game to play against another human player or a computer controlled opponent. The project was a lot harder for me than I initially thought it would be!

I have the feeling I've over complicated some things and think that especially the 'AI' part is not very elegant/efficient. For now tho this is the best I could do.

Would appreciate feedback on anything that comes to mind (program structure, optimization, bad practices etc.)

/*
 * tictactoe.c
 * 
 * Copyright 2024 Reslashd
 * 
 * A simple program to play Tic Tac Toe versus a friend or a
 * (very basic) computer controlled opponent.
 * 
 */

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

bool checkMove(char grid[][3], int move, bool move_ok);
int checkThree(char grid[][3], int three_row);
int checkTwo(char grid[][3], int move);
void clearScreen(void);
int cpuMove(char grid[][3], int move, int turn, int difficulty);
void drawGrid(char grid[][3]);
int gameLoop(char grid[][3], int choice, int difficulty, int three_row);
int getMove(char grid[][3], int move, char mark, int choice, int turns, int difficulty);
void makeMove(char grid[][3], int move, char mark);
bool needMove(bool need_move, int nX, int nO, int result);
int pickCpuMove(int move, int location);
void pressToContinue(void);
void resetGrid(char grid[][3]);
void resetVars(int *nX_ptr, int *nO_ptr, int *result_ptr);
int setDifficulty(int difficulty);
int showMenu(int choice);
void showScores(int n_owins, int n_xwins, int n_draws);
char startingMark(char mark, int choice);
char switchMark(char mark);

int main(void){
    char grid[3][3] = {{'1','2','3'}, {'4','5','6'}, {'7','8','9'}};
    int choice = 0;
    int difficulty = '1';
    int n_draws = 0;
    int n_owins = 0;
    int n_xwins = 0; 
    int three_row = 0; 
    srand((unsigned int)time(NULL));

    
    while ((choice = showMenu(choice)) != '5'){     
        if (choice == '1' || choice == '2'){
            three_row = gameLoop(grid, choice, difficulty, three_row);
            clearScreen();
            drawGrid(grid);
            if(three_row == 1){
                puts("\nThree in a row!");
                puts("O wins!");
                n_owins++;
            }
            else if (three_row == 2){
                puts("\nThree in a row!"); 
                puts("X wins!");
                n_xwins++;
            }   
            else{
                puts("\nIts a draw!");
                n_draws++;
            }   
            resetGrid(grid);
            three_row = 0;
            pressToContinue();
        } else if (choice == '3') { 
            showScores(n_owins, n_xwins, n_draws);
        } else if (choice == '4') {
            difficulty = setDifficulty(difficulty);     
        }       
    }
    return EXIT_SUCCESS;
}

bool checkMove(char grid[][3], int move, bool move_ok){
    size_t column = 0;
    size_t row = 0;
    
    if (move >= '1' && move <= '9'){            
        for (row = 0 ; row < 3; row++){
                for(column = 0; column < 3 ; column++){
                    if (grid[row][column] == move){ 
                        move_ok = 1;    
                        goto end_checkmove;                     
                    }   
                    else{
                        move_ok = 0;
                    }
                }               
        }       
    } else {
        move_ok = 0;
    }

    end_checkmove:
    return move_ok;
}

int checkThree(char grid[][3], int three_row){
    /*
     * This functions checks for 'three in a row' by checking the 
     * total decimal number in a row, column and diagonal.
     * 3 times decimal 79 (OOO) = 237
     * 3 times decimal 88 (XXX) = 264 
     * Returns 0 = draw, 1 = O wins, 2 = X wins
     */
     
    size_t column = 0;
    size_t row = 0;
    int result = 0; 

    // check diagonal left to right
    for (row = 0 ; row < 3; row++){
        result = result + grid[row][row];
        if (result == 237 || result == 264){
            if (result == 237) {
                three_row = 1;              
            } else{
                three_row = 2;
            }           
            goto return_result;
        }   
    }
    result = 0;
    
    // check diagonal right to left
    for (row = 0 ; row < 3; row++){
        result = result + grid[row][3 - row - 1];
        if (result == 237 || result == 264){
            if (result == 237) {
                three_row = 1;              
            } else{
                three_row = 2;
            }           
            goto return_result;
        }   
    }
    result = 0;

    // check rows
    for (row = 0 ; row < 3; row++){
        result = grid[row][0]+grid[row][1]+grid[row][2];    
        if (result == 237 || result == 264){
            if (result == 237) {
                three_row = 1;              
            } else{
                three_row = 2;
            }           
            goto return_result;
        }   
    }
    result = 0;
    
    // check columns
    for (column = 0 ; column < 3; column++){
        result = grid[0][column]+grid[1][column]+grid[2][column];   
        if (result == 237 || result == 264){
            if (result == 237) {
                three_row = 1;
            } else{
                three_row = 2;
            }
        }   
    }   

    return_result:  
    return three_row;
}

int checkTwo(char grid[][3], int move){
/*  
 *  This function checks if there are 2 XX's or OO's in a row,
 *  column or one of the diagonals and if a certain counter move by 
 *  the CPU is needed.  
 * 
 *  If needed it picks (randomly until the empty spot is found) a  
 *  counter move from a set of possible moves to either prevent a loss   
 *  or make a move to win the game.
 *  
 */
    size_t column = 0;
    size_t row = 0;
    
    bool need_move = 0;
    int nO = 0;
    int nX = 0;
    int result = 0;
        
    int *nX_ptr = &nX;
    int *nO_ptr = &nO;
    int *result_ptr = &result;  

    // check diagonal left to right 
    for (row = 0 ; row < 3; row++){
        result = result + grid[row][row];
        if(grid[row][row] == 'X'){
            nX++;
        }
        else if (grid[row][row] == 'O'){
            nO++;
        }   
    }   
    need_move = needMove(need_move, nX, nO, result);
    if (need_move == 1){
        move = pickCpuMove(move, 1);        
        goto return_move;
    }   

    resetVars(nX_ptr, nO_ptr, result_ptr);
        
    // check diagonal right to left 
    for (row = 0 ; row < 3; row++){
        result = result + grid[row][3 - row - 1]; 
        if(grid[row][3 - row - 1] == 'X'){
            nX++;
        }
        else if (grid[row][3 - row - 1] == 'O'){
            nO++;
        }
    }
    need_move = needMove(need_move, nX, nO, result);
    if (need_move == 1){
        move = pickCpuMove(move, 2);        
        goto return_move;
    }        

    resetVars(nX_ptr, nO_ptr, result_ptr); 

    // check rows 
    for (row = 0 ; row < 3; row++){ 
        resetVars(nX_ptr, nO_ptr, result_ptr);
        for(column = 0 ; column < 3; column++){
            if(grid[row][column] == 'X'){
                nX++;
            }
            else if(grid[row][column] == 'O'){
                nO++;               
            }
            result = grid[row][0]+grid[row][1]+grid[row][2];    
            need_move = needMove(need_move, nX, nO, result);
            if (need_move == 1){
                move = pickCpuMove(move, (int)row + 3); // location 3, 4, 5
                goto return_move;
            }   
        }   
    }   

    resetVars(nX_ptr, nO_ptr, result_ptr);
    
    // check columns 
    for (column = 0 ; column < 3; column++){    
        resetVars(nX_ptr, nO_ptr, result_ptr);
        for(row = 0 ; row < 3; row++){
            if(grid[row][column] == 'X'){
                nX++;
            }
            else if(grid[row][column] == 'O'){
                nO++;               
            }
            result = grid[0][column]+grid[1][column]+grid[2][column];   
            need_move = needMove(need_move, nX, nO, result);
            if (need_move == 1){
                move = pickCpuMove(move, (int)column + 6); // location 6, 7, 8
                goto return_move;
            }   
        }   
    }       

    move = (rand() % 9) + 49; // Failsafe return random move.
    return_move:    
    return move;    
}

void clearScreen(void){
    #if defined(__linux__) || defined(__unix__) || defined(__APPLE__)
        system("clear");
    #endif

    #if defined(_WIN32) || defined(_WIN64)
        system("cls");
    #endif
}

int cpuMove(char grid[][3], int move, int turn, int difficulty){
    char first_turn[5] = {'1','3', '5', '7', '9'};
    char second_turn[4] = {'1','3', '7', '9'};
    int n = 0;  
    
    if(turn == 1){
        // First turn strategy - take center or a corner
            n = rand() % 5;
            move = first_turn[n];   
    } else if (turn == 2){
        // Second turn strategy - take center if possible else a corner
        if(grid[1][1] != 'X' && grid[1][1] != 'O'){
            move = '5';
        }
        else if(grid[1][1] == 'X' || grid[1][1] == 'O'){
            n = rand() % 4;
            move = second_turn[n];
        }   
    } else if (turn == 3){
        // Third turn strategy - take center if possible else random move
        if(grid[1][1] != 'X' && grid[1][1] != 'O'){
            move = '5';
        }
        else {
            move = (rand() % 9) + 49; 
        }
        // Remaining turns
    } else {    
        //easy 
        if (difficulty == '1'){
            move = (rand() % 9) + 49;  // Random move '1' - '9'
        }
        // hard
        else if (difficulty == '2'){
            move = checkTwo(grid, move); // Analyze grid and make move
        }           
    }   
    return move;
}

void drawGrid(char grid[][3]){
    printf("\n%c | %c | %c\n", grid[0][0], grid[0][1], grid[0][2]);
    puts("=========");
    printf("%c | %c | %c\n", grid[1][0], grid[1][1], grid[1][2]);
    puts("=========");
    printf("%c | %c | %c\n", grid[2][0], grid[2][1], grid[2][2]);   
}

int gameLoop(char grid[][3], int choice, int difficulty, int three_row){
    int move = 0;
    int turn = 1;
    char mark = 'X';
    
    mark = startingMark(mark, choice);
    clearScreen();  
    while(turn != 10) {
        clearScreen();
        drawGrid(grid);
        move = getMove(grid, move, mark, choice, turn, difficulty);
        makeMove(grid, move, mark); 
        mark = switchMark(mark);
        turn++;
        three_row = checkThree(grid, three_row);
        if(three_row == 1 || three_row == 2){
            break;
        }       
    }
    return three_row;
}

void makeMove(char grid[][3], int move, char mark){ 
    size_t column = 0;
    size_t row = 0;
    
    for (row = 0 ; row < 3; row++){
        for(column = 0; column < 3 ; column++){
            if (grid[row][column] == move){ 
                grid[row][column] = mark;                       
            }   
        }               
    }
}

bool needMove(bool need_move, int nX, int nO, int result){
    if (nX == 2 || nO == 2){
        if (result < 237){
            need_move = 1;
        }        
    } 
    else{
        need_move = 0;
    }
    return need_move;
}

int pickCpuMove(int move, int location){
/*  
 *  Return a (random until empty spot is found) move from a 
 *  selection of moves based on where the sequence XX or OO was 
 *  found on the grid.
*/
    int n = 0;
    char diag_LR[3] = {'1', '5', '9'}; // location 1 
    char diag_RL[3] = {'3', '5', '7'}; // location 2
    char row_One[3] = {'1', '2', '3'}; // location 3
    char row_Two[3] = {'4', '5', '6'}; // location 4
    char row_Three[3] = {'7', '8', '9'}; // location 5
    char col_One[3] = {'1', '4', '7'}; // location 6
    char col_Two[3] = {'2', '5', '8'}; // location 7
    char col_Three[3] = {'3', '6', '9'}; // location 8
    n = rand() % 3;
    
    if(location == 1){
        move = diag_LR[n];
    } else if(location == 2){
        move = diag_RL[n];
    } else if(location == 3){
        move = row_One[n];
    } else if (location == 4){
        move = row_Two[n];
    } else if (location == 5){
        move = row_Three[n];
    } else if(location == 6){
        move = col_One[n];
    } else if (location == 7){
        move = col_Two[n];
    } else if (location == 8){
        move = col_Three[n];
    }
    return move;
}

void pressToContinue(void){
    puts("\nPress ENTER to continue");
    getchar();
    getchar();  
}

void resetGrid(char grid[][3]){
    size_t column = 0;
    size_t row = 0; 
    char first = '1';
    
    for (row = 0 ; row < 3; row++){
        for(column = 0; column < 3 ; column++){
            grid[row][column] = first;
            first++;        
        }               
    }   
    
}
void resetVars(int *nX_ptr, int *nO_ptr, int *result_ptr){
    *nX_ptr = 0;
    *nO_ptr = 0;
    *result_ptr = 0;    
}

int setDifficulty(int difficulty){
    int input = 0;
    clearScreen();
    puts("\n***** SET DIFFICULTY *****\n");
    if (difficulty == '1'){
        puts("Current difficulty is EASY\n");
    } else if (difficulty == '2'){
        puts("Current difficulty is HARD\n");
    }
    puts("1 = Easy");
    puts("2 = Hard");
    fputs("\nChoose your difficulty: ", stdout);
    getchar();
    input = getchar();
    
    if (input == '1'){              
        difficulty = '1';               
        puts("Difficulty set to EASY");
    } else if (input == '2'){
        difficulty = '2';   
        puts("Difficulty set to HARD");
    }
    else{
        puts("\nInvalid choice!");
    }   
    pressToContinue();  
    return difficulty;
}

int showMenu(int choice){
    clearScreen();
    puts("\nTIC TAC TOE v1.0 by Reslashd");
    puts("\n******* MENU *******\n");
    puts("1 = 1 Player vs CPU");
    puts("2 = 2 Players");
    puts("3 = View scores");
    puts("4 = Set difficulty"); 
    puts("5 = Exit");
    choice = getchar();
    return choice;
}

void showScores(int n_owins, int n_xwins, int n_draws){
    clearScreen();
    puts("\n***** SCORES *****");
    printf("\nO has %d wins", n_owins);
    printf("\nX has %d wins", n_xwins); 
    printf("\nThere are %d draws\n", n_draws);
    pressToContinue();
}

char startingMark(char mark, int choice){
    int start_mark = 0;
        
    puts("\nPlease choose starting player");
    if (choice == '1'){
        puts("1 = O (CPU)");
    } else if (choice == '2'){
        puts("1 = O");
    }
    puts("2 = X");
    getchar();
    start_mark = getchar();
    if (start_mark == '1'){
        mark = 'O';
    } else if (start_mark == '2'){
        mark = 'X';
    }
    return mark;
}

char switchMark(char mark){
    if (mark == 'X'){
        mark = 'O';
    } else if (mark == 'O') {
        mark = 'X';
    }   
    return mark;
}

int getMove(char grid[][3], int move, char mark, int choice, int turn, int difficulty){ 
    bool move_ok = 0;
    
    // get Human player move 
    if(choice == '2' || (choice == '1' && mark == 'X')){    
        while(move_ok != 1){                
            printf("\nIt is %c's turn, place your mark: ", mark);
            while((move = getchar()) == '\n');  
        
            move_ok = checkMove(grid, move, move_ok);
            if (move_ok != 1){
                puts("Invalid move please try again...");
                pressToContinue();
                clearScreen();
                drawGrid(grid);
            } 
        }
    }
    // get CPU move
    else if (choice == '1' && mark == 'O'){     
        while (move_ok != 1){   
            move = cpuMove(grid, move, turn, difficulty);
            move_ok = checkMove(grid, move, move_ok);
        }                                       
    }
    return move;    
}
\$\endgroup\$
8
  • 1
    \$\begingroup\$ Why magic numbers 49, 79, 88? If code needs the value of the character 'O', Why not use 'O' instead of 79 or 'O' + 'O' + 'O' instead of 237? \$\endgroup\$ Feb 8 at 20:14
  • 1
    \$\begingroup\$ What happens if you Ctrl+D (or Ctrl+Z on Windows/DOS) while at the menu? For me, it was an endless loop because EOF is not handled. \$\endgroup\$
    – aghast
    Feb 9 at 0:05
  • \$\begingroup\$ @chux-ReinstateMonica, you know they have a shorthand notation for repeated addition, now, don't you? Not sure if the Stack sites have got the upgraded version of Unicode installed, but try 'O' \x00D7 3... \$\endgroup\$
    – aghast
    Feb 9 at 0:07
  • \$\begingroup\$ @aghast Source coding outside the allowed subset of ASCII leads to its own troubles. Let's go with 3* 'O'. \$\endgroup\$ Feb 9 at 1:22
  • 1
    \$\begingroup\$ @Reslashd When you are taking input from a stream, one possibility is that you will encounter "end of file". Depending on what function you call, it gets handled differently. It may just be the end of a string, or it may cause a return of NULL or a return of EOF. In cases where you are reading characters, EOF is returned. This is an integer value outside the range of a char, so you have to special-case it. If you ignore EOF and ask for another character of input, you get... EOF! It's like a horror movie villain, it just keeps coming back. You need to detect it and ... whatever. \$\endgroup\$
    – aghast
    Feb 12 at 22:09

2 Answers 2

4
\$\begingroup\$

First thing that comes to mind is to remove goto (see SO question):

  • checkMove you only need to check a single cell not up to 9 with loops
bool checkMove(char grid[][3], int move) { // wouldn't pass move_ok
  bool move_ok;
  if (move >= '1' && move <= '9') {
    // convert from ascii to int, then to position
    int row = (move - 49) / 3;
    int column = (move - 49) % 3;
    if(grid[row][column] == move) {
      move_ok = 1;
    } else {
      move_ok = 0;
    }
  } else {
    move_ok = 0;
  }
  return move_ok;
}
  • makeMove can be very similar to checkMove (don't need loop)
// convert from ascii to int, then to position
int row = (move - 49) / 3;
int column = (move - 49) % 3;
grid[row][column] = mark
  • checkThree you could easily check cell equality in a single line, and then return immediately instead of using goto. Also use bool
bool checkThree(char grid[][3]) {//don't need three_row
// left to right diagonal
if (grid[0][0] == grid[1][1] && grid[1][1] == grid[2][2]) {
  return true;
}
...
return false; // you don't treat 1 or 2 differently from this function

this would require slightly different validating

three_row = checkThree(grid);//don't need to pass three_row
if (three_row){
  if(mark == 'O') {
    return 1;
  } else {
    return 2;
  }
}
...
  • checkTwo could be very similar to checkThree, just expand the check (I think you can remove needMove, pickCpuMove, or resetVars)
move = '0'
// left to right diagonal
if (grid[0][0] == grid[1][1] && grid[1][1] != grid[2][2]) {
  // know 2 are the same, but not the last
  return '9';
} else if (grid[0][0] != grid[1][1] && grid[1][1] == grid[2][2]) {
  // know last are the same, but not the first
  return '1';
} else if (grid[0][0] == grid[2][2]) {
  // know ends are the same, but not the mid
  return '5'
}
...

Simplify:

  • if (grid[1][1] != 'X' && grid[1][1] != 'O') can be if (grid[1][1] != '5')
  • } else if (grid[1][1] != 'X' || grid[1][1] == 'O') { can be } else {
  • } else if (mark == 'O') { can be } else {
  • don't need to pass so many arguments
    • getMove doesn't need move, it should return move (cpuMove, pickCpuMove (unless you remove it as noted above) also don't need it, same reason)
    • IMO using function parameters as variables is confusing. Generally I treat function parameters as read-only (there are exceptions to this (see pass by reference), but when I read myVar = myFunc(myVar) I don't expect myVar to change inside myFunc). In setDifficulty I would do something like:
  if (input == '1') {
    puts("Difficulty set to EASY");
    return '1';
  } else if (input == '2') {
    puts("Difficulty set to HARD");
    return '2';
  } else {
    puts("\nInvalid choice! Difficulty will remain the same");
    return difficulty
  }

EDIT Based on https://github.com/Reslashd/tictactoe/blob/main/tictactoe.c

Variable/parameter clean up

  • clear names: Reading a variable should let future you/devs know what is stored in it.
  • don't pass basic variables as parameters to edit
    • in main, three_row has several issues. It isn't very clear what it stores. I'd change it winner and remove it as a parameter to gameLoop. gameLoop without the parameter could just put the check in the if: if(checkThree(grid)) and the final return would be return 0 (no winner)
    • switchMark is assigning to mark, instead it should just return. Shorter and clearer code. Don't treat parameters as editable variables, generally should be read only
    • in gameLoop just initialize mark without passing mark: char mark = startingMark(choice) (might rename to getStartingMark). Inside startingMark change the parameter name to be something like numPlayers instead of choice (clear what is being passed in). The logic of startingMark isn't very clear. It initializes start_mark to an int later it's a char (technically stored same, but to be clear to developer distinguish the type better).
    • showMenu doesn't need choice, it returns the player choice, initialize choice like char choice = getchar();
  • change parameter names to be different than what you pass. This might help separate use/difference
    • setDifficulty the parameter could be currentDifficulty. Side note, inside this function input could be initialized in on line with correct type char input = getchar();
    • switchMark the param could be currentMark

Simplify Again

  • 99% of the time if you see a pattern it can be simplified
    • checkTwo and checkThree row/col check could be put in a for loop. checkTwo's fail safe return should validate the random choice is a valid move like all the rest (as cpuMove also has a rand choice, you make a rand choice function, then you won't have to check it later):
  char randMove;
  do{
    randMove = (rand() % 9) + 49;   
  }while(!checkMove(grid, randMove));
  return randMove; // Failsafe return random move.

All these together: https://gist.github.com/depperm/b28798d3730c0f8394fd1ed380fa95e5 (almost 100 lines shorter)

\$\endgroup\$
7
  • \$\begingroup\$ Thanks a lot for your feedback. I will go over it in detail this weekend and see if I understand everything and if I can implement some improvements. Will probably get back to you with some additional questions. Question about the GOTO; I can understand in this case it is poor design (I think I can implement your better alternative solution). I am wondering tho: besides being a poor design choice could the way I've used them in this program (break out of a nested loop) also cause unforeseen problems or bugs I might not be aware of? \$\endgroup\$
    – Reslashd
    Feb 1 at 18:25
  • \$\begingroup\$ @Reslashd maybe, this is a fairly simple program, its more of good practice not to use \$\endgroup\$
    – depperm
    Feb 1 at 22:23
  • \$\begingroup\$ I had to make an addition to your suggested improvement for checkTwo or else the program gets stuck in an infinite loop. If for example the first row = "O X O" it would return '2' while there already is an X in that position on the grid and thats not possible. So to fix this I've added a checkMove before returning a move i.e. if(checkMove('9')) before returning '9'. I feel the function still is a bit bloated tho but you were right; I could get rid of the mentioned functions which is great! I wonder if there is a way to further optimize the checkTwo function? \$\endgroup\$
    – Reslashd
    Feb 7 at 18:58
  • \$\begingroup\$ The simplifying suggestions also make sense though these too had to be modified a bit i.e. if (grid[1][1] == '5') (take center position on the grid if no X or O there). Question about the returning of values by a function: If I understand correctly this immediately also ends the function? What is the advantage of returning a value directly instead of a variable (like in the setDifficulty function? link to updated code. \$\endgroup\$
    – Reslashd
    Feb 7 at 19:07
  • \$\begingroup\$ @Reslashd advantage is run less logic: faster, less chance of edge case/error. IMO variables come in handy if you are performing multiple computations. I can add further edit in a few minutes \$\endgroup\$
    – depperm
    Feb 8 at 17:36
3
\$\begingroup\$

We can easily change the order of functions so we don't need to forward-declare so many (possibly none at all). That's why C programmers expect to find main() last in the file.


It looks like you're assuming your code will run in an ASCII locale - the computations are completely broken where 'X', 'O' and/or ' ' are encoded differently.

\$\endgroup\$
4
  • \$\begingroup\$ Thanks for your input. I understand I can avoid the forward declaring but other than style/preference is there a specific reason not to? About the ASCII locale; I'll be honest I hadn't thought about that... \$\endgroup\$
    – Reslashd
    Feb 1 at 18:28
  • 1
    \$\begingroup\$ The main reason not to forward-declare is that it's extra effort you probably don't need. It's clearly easier if any changes can be made in a single place, rather than needing two edits. \$\endgroup\$ Feb 1 at 19:13
  • \$\begingroup\$ You are right that it is a bit of a pain to change it at two places in the code. As I've understood it tho it can help the compiler spot some subtle errors link. Since I'm a beginner I think it's a good idea for now to keep doing it this way. Or are there any other downsides that I've perhaps missed or not understood correctly? \$\endgroup\$
    – Reslashd
    Feb 7 at 19:15
  • 1
    \$\begingroup\$ Your link is to something different, mostly referencing the old style of declaring functions with unspecified arguments (()). Every function should have a prototype, but it's fine for the prototype to also be the definition. \$\endgroup\$ Feb 10 at 15:54

Your Answer

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

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