7
\$\begingroup\$

Someone in this thread said I could ask for a review of my revision: Ncurses Tic Tac Toe with simplistic AI

I re-wrote it from scratch using the suggestions from the answers. In particular I did the following:

  1. I got rid of Global Variables. For the few global constants I used #defines instead.

  2. I used arrays instead of lists of numbered variables, as suggested.

  3. Although it was not suggested I do so, I did improve the AI. This is the main reason that it is not shorter I think.

There is one set of functions in particular that I think I could have combined into one function with a switch, during the AI Logic phase. I commented them so in the code and that's something I'd definitely like some feedback on how to do.

I'd like to thank everyone in the previous thread for their tips and suggestions. There are some I did not follow, and for different reasons:

  1. I did not stop using rand() because it seems random enough for my purposes. I did a test on my own using 10,000 attempts and checked how often it picked each number between 0 and 99 and it was plenty random enough. So for this project I just kept using it. For those who still suggest I use a different random function, though, I am open to suggestions on how to best do that. I wouldn't mind making my OWN random function and would be stoked on tips for doing that. It's hard to imagine how I could improve even on rand() though.

  2. I wanted to use Structs to imitate OOP for the tile spaces but could not quite wrap my head around how to do so. In Python that kind of thing would be trivial but in C it's not easy for me to understand. Tips on how I could integrate Structs into this to get some kind of OOP would be really welcome.

Note on AI: I made it much tougher this time. It will beat a human player most of the time. It would beat a human every single time (ot tie) if I did not intentionally build in a "fart" function that causes it to fail a certain percentage of the time.

Note on the code itself: Anyone who wants to use this for their own purposes is more than welcome. Although I am super proud of it I recognize that it is amateur stuff. If anyone wants to play some tic tac toe or can think of a use for this themselves they are more than welcome to it. It should compile pretty easily on most linux systems if you have ncurses.

Note on commenting: My comment style was a cause for concern from some of the reviewers in the other thread so I changed it up a bit. I fear it might be too verbose but I wanted it to be easy for people who aren't too familiar with ncurses to follow along.

Note on Length: This is the real failing here. Although I implemented many of the suggestions from the other thread my code is actually longer... not shorter. There's at least one set of functions I can probably combine, but how else can I shorten it up?

As before, any and all comments and suggestions are welcome. I want to nail down a solid coding style with simple projects like this before moving on to more complex projects in this language. Thank you! Without further ado, here is the code:

// tic tac toe v2 using suggestions from Stack Exchange for better style
// Minus the struct stuff which I don't quite understand just yet.

// ncurses for, well, ncurses
#include <ncurses.h>
// time for the random seed
#include <time.h>
// string.h for strlen() 
#include <string.h>
// stdlib and stdio because why not
#include <stdlib.h>
#include <stdio.h>
// ctype.h for toupper()
#include <ctype.h>

// #define's for the COLOR_PAIRs
#define X_COLOR 1
#define O_COLOR 2
#define BG_COLOR 3

// #defines used as a global constant
#define num_spaces 9

// Function Declarations
void init_spaces(char *space_ptr);
void paint_board(char playable_spaces[num_spaces]);
void take_turn(char side, char *space_ptr, char playable_spaces[num_spaces]);
void victory_splash(int game_over_state);
void paint_background();
void player_turn(char *space_ptr, char playable_spaces[num_spaces], char side);
void ai_turn(char *space_ptr, char playable_spaces[num_spaces], char side);
void set_color_ai_side(char ai_side);
void set_color_side(char side);
int main_menu();
int evaluate_board(char playable_spaces[num_spaces]);
int spaces_left(char playable_spaces[num_spaces]);
int ai_fart(const int chance_to_fart);
int pick_random_space(char playable_spaces[num_spaces]);
int check_for_winning_move(char playable_spaces[num_spaces], char ai_side);
int check_for_block(char playable_spaces[num_spaces], char side);
int check_for_2_space_path(char playable_spaces[num_spaces], char ai_side);
char pick_side();

int main(){
    // To-Do: Try the time(NULL) method for srand initialization and see if it works the same
    time_t t;
    srand((unsigned) time(&t));
    char playable_spaces[num_spaces] = {'X', 'X', 'X', 'X', 'X', 'X', 'X', 'X', 'X'};
    char *space_ptr = &playable_spaces[0];

    // Game over splash
    char game_over_str[] =  " Game Over! Any key to continue... ";
    char go_padding[] = "                                   ";
    int game_over_len = strlen(game_over_str);
    int row, col, x, y;

    //curses init
    initscr();
    cbreak();
    keypad(stdscr, 1);
    curs_set(0);
    start_color();
    init_pair(X_COLOR, COLOR_CYAN, COLOR_BLACK);
    init_pair(O_COLOR, COLOR_GREEN, COLOR_BLACK);
    init_pair(BG_COLOR, COLOR_YELLOW, COLOR_BLACK);
    noecho();

    // Main Menu outer loop
    int running = 1;
    while(running){
        curs_set(0);
        // Main menu function quits or continues
        running = main_menu();
        // In-Game inner loop
        if(running == 0){
            break;
        }
        int playing = 1;
        while(playing){
            // Init all spaces to blank
            init_spaces(space_ptr);
            // Player picks their side.
            char side = pick_side();
            // The inner, inner turn loop
            int turning = 1;
            while(turning){
                int game_over = 0;
                // Paint the board state as it is that turn
                paint_board(playable_spaces);
                // Function that governs the turn cycle
                take_turn(side, space_ptr, playable_spaces);
                // Evaluate the board for game over state
                game_over = evaluate_board(playable_spaces);
                if(game_over > 0){
                    // paint the board with a splash on game over
                    // so the player can evaluate the board for a moment
                    paint_board(playable_spaces);
                    getmaxyx(stdscr, row, col);
                    y = row / 2 + 6;
                    x = col / 2 - game_over_len / 2;
                    attron(COLOR_PAIR(BG_COLOR));
                    mvprintw(y++, x, go_padding);
                    mvprintw(y++, x, game_over_str);
                    mvprintw(y, x, go_padding);
                    refresh();
                    getch();
                    // call victory_splash with int game_over as a parameter
                    // 1 = X wins, 2 = O wins, 3 = Tie
                    victory_splash(game_over);
                    // Reset the turning and playing loops to effectively start over
                    turning = 0;
                    playing = 0;
                }
            }
        }
    }

    // end curses
    endwin();

    return 0;
}


void init_spaces(char *space_ptr){ 
    // init all the spaces to ' ';
    int i;
    for(i = 0; i < 9; i++){
        *space_ptr = ' ';
        space_ptr++;
    }
}

void paint_board(char playable_spaces[num_spaces]){
    // paint the board and the playable spaces
    clear();
    paint_background();
    char break_lines[] = " ------- ";
    char play_lines[] =  " | | | | ";
    char padding[] =     "         ";
    int row, col, x, y;
    getmaxyx(stdscr, row, col);
    y = row / 2 - 4;
    int len;
    len = strlen(padding);
    x = col / 2 - len / 2;
    int k;
    const int num_lines = 9;
    attron(COLOR_PAIR(BG_COLOR));
    for(k = 0; k < num_lines; k++){
        // Paint the board itself without the pieces
        if(k == 0 || k == num_lines - 1){
            mvprintw(y + k, x, padding);
        }else{
            if(k % 2 == 0){
                mvprintw(y + k, x, play_lines);
            }else{
                mvprintw(y + k, x, break_lines);
            }
        }
    }
    attroff(COLOR_PAIR(BG_COLOR));
    // insert Xs and Os:
    // First set the dynamic x and y coordinates based on terminal size
    int playable_x[num_spaces] = {x+2, x+4, x+6, x+2, x+4, x+6, x+2, x+4, x+6};
    int playable_y[num_spaces] = {y+2, y+2, y+2, y+4, y+4, y+4, y+6, y+6, y+6};
    for(k = 0; k < num_spaces; k++){
        // For each of the playable spaces, first set the color
        if(playable_spaces[k] == 'O'){
            attron(COLOR_PAIR(O_COLOR));        
        }else if(playable_spaces[k] == 'X'){
            attron(COLOR_PAIR(X_COLOR));
        }else{
            attron(COLOR_PAIR(BG_COLOR));
        }
        // then insert the char for that space into the proper spot on the terminal
        mvaddch(playable_y[k], playable_x[k], playable_spaces[k]);
    }
    // refresh the screen
    refresh();
}

void take_turn(char side, char *space_ptr, char playable_spaces[num_spaces]){
    // using "side" to determine the order, call the functions to play a whole turn
    if(side == 'X'){
        player_turn(space_ptr, playable_spaces, side);
        paint_board(playable_spaces);
        if(spaces_left(playable_spaces)){
            if(!(evaluate_board(playable_spaces))){
                ai_turn(space_ptr, playable_spaces, side);
                paint_board(playable_spaces);
            }
        }
    }else if(side == 'O'){
        ai_turn(space_ptr, playable_spaces, side);
        paint_board(playable_spaces);
        if(spaces_left(playable_spaces)){
            if(!(evaluate_board(playable_spaces))){
                player_turn(space_ptr, playable_spaces, side);
                paint_board(playable_spaces);
            }
        }
    }
}

int main_menu(){
    clear();
    // Takes user input and returns an int that quits or starts a game
    int row, col, x, y;
    char error_string[] = " Invalid Input! Any key to try again... ";
    int error_str_len = strlen(error_string);
    char str1[] =      " NCURSES TIC TAC TOE (v2) ";
    char padding[] =   "                          ";
    char str2[] =      "    (P)lay or (Q)uit?     ";
    int len = strlen(str1);
    paint_background();
    getmaxyx(stdscr, row, col);
    y = row / 2 - 2;
    x = col / 2 - len / 2;
    mvprintw(y++, x, padding);
    mvprintw(y++, x, str1);
    mvprintw(y++, x, padding);
    mvprintw(y++, x, str2);
    mvprintw(y++, x, padding);
    int input;
    refresh();
    // get user input and return it
    input = toupper(getch());
    if(input == 'P'){
        return 1;
    }else if(input == 'Q'){
        return 0;
    }else{
        // call the function again if the input is bad
        x = col / 2 - error_str_len / 2;
        mvprintw(++y, x, error_string);
        getch();
        main_menu();
    }
}

int evaluate_board(char playable_spaces[num_spaces]){
    // Evaluates the state of the playable spaces and either does nothing
    // or ends the game.
    // Check all the possible winning combinations:
    if(playable_spaces[0] == 'X' && playable_spaces[1] == 'X' && playable_spaces[2] == 'X'){
        return 1;
    }else if(playable_spaces[3] == 'X' && playable_spaces[4] == 'X' && playable_spaces[5] == 'X'){
        return 1;
    }else if(playable_spaces[6] == 'X' && playable_spaces[7] == 'X' && playable_spaces[8] == 'X'){
        return 1;
    }else if(playable_spaces[0] == 'X' && playable_spaces[3] == 'X' && playable_spaces[6] == 'X'){
        return 1;
    }else if(playable_spaces[1] == 'X' && playable_spaces[4] == 'X' && playable_spaces[7] == 'X'){
        return 1;
    }else if(playable_spaces[2] == 'X' && playable_spaces[5] == 'X' && playable_spaces[8] == 'X'){
        return 1;
    }else if(playable_spaces[0] == 'X' && playable_spaces[4] == 'X' && playable_spaces[8] == 'X'){
        return 1;
    }else if(playable_spaces[2] == 'X' && playable_spaces[4] == 'X' && playable_spaces[6] == 'X'){
        return 1;
    }else if(playable_spaces[0] == 'O' && playable_spaces[1] == 'O' && playable_spaces[2] == 'O'){
        return 2;
    }else if(playable_spaces[3] == 'O' && playable_spaces[4] == 'O' && playable_spaces[5] == 'O'){
        return 2;
    }else if(playable_spaces[6] == 'O' && playable_spaces[7] == 'O' && playable_spaces[8] == 'O'){
        return 2;
    }else if(playable_spaces[0] == 'O' && playable_spaces[3] == 'O' && playable_spaces[6] == 'O'){
        return 2;
    }else if(playable_spaces[1] == 'O' && playable_spaces[4] == 'O' && playable_spaces[7] == 'O'){
        return 2;
    }else if(playable_spaces[2] == 'O' && playable_spaces[5] == 'O' && playable_spaces[8] == 'O'){
        return 2;
    }else if(playable_spaces[0] == 'O' && playable_spaces[4] == 'O' && playable_spaces[8] == 'O'){
        return 2;
    }else if(playable_spaces[2] == 'O' && playable_spaces[4] == 'O' && playable_spaces[6] == 'O'){
        return 2;
    }else{
        // Check all spaces for a tie
        int hits = 0;
        int i;
        for(i = 0; i < num_spaces; i++){
            if(playable_spaces[i] != ' '){
                hits++;
            }
        }
        if(hits >= num_spaces){
            return 3;
        }else{
            return 0;
        }
    }
}

char pick_side(){
    // Takes user input and returns the chosen side
    clear();
    paint_background();
    int row, col, x, y;
    char str1[] =    " Press 'X' for X, 'O' for O, or 'R' for random! ";
    char str2[] =    "        Good choice! Any key to continue...     ";
    char padding[] = "                                                ";
    char err_str[] = "      Invalid input! Any key to continue...     ";
    int len = strlen(str1);
    getmaxyx(stdscr, row, col);
    y = row / 2 - 2;
    x = col / 2 - len / 2;
    mvprintw(y++, x, padding);
    mvprintw(y++, x, str1);
    mvprintw(y++, x, padding);
    int input;
    int pick;
    refresh();
    // Get user input for picking a side. 'R' is random.
    input = toupper(getch());
    if(input == 'X' || input == 'O'){
        mvprintw(y, x, str2);
        refresh();
        getch();
        return (char) input;
    }else if(input == 'R'){
        pick = rand() % 2;
        if(pick == 0){
            input = 'X';
        }else if(pick == 1){
            input = 'O';
        }
        mvprintw(y, x, str2);
        refresh();
        getch();
        return (char) input;
    }else{
        // Call the function again on bad input
        mvprintw(y, x, err_str);
        refresh();
        getch();
        pick_side();
    }
}

void victory_splash(int game_over_state){
    // Takes the game over state and creates a victory splash
    char padding[] = "                                   ";
    char *str1 =     "              X Wins!              ";
    char *str2 =     "              O Wins!              ";
    char str3[] =    "         any key to continue...    ";
    char *str4 =     "             A tie game!           ";
    int len = strlen(padding);
    char *vic_pointer = NULL;
    // To avoid code duplication, use a pointer to pick the right string
    if(game_over_state == 1){
        vic_pointer = str1;
    }else if(game_over_state == 2){
        vic_pointer = str2;
    }else if(game_over_state == 3){
        vic_pointer = str4;
    }
    clear();
    paint_background();
    int row, col, x, y;
    getmaxyx(stdscr, row, col);
    y = row / 2 - 2;
    x = col / 2 - len / 2;
    mvprintw(y++, x, padding);
    mvprintw(y++, x, vic_pointer);
    mvprintw(y++, x, padding);
    mvprintw(y, x, str3);
    refresh();
    getch();
}

void paint_background(){
    // Paints an elaborate flashy background
    int row, col, x, y;
    int pick;
    getmaxyx(stdscr, row, col);
    for(y = 0; y <= row; y++){
        for(x = 0; x <= col; x++){
            pick = rand() % 3;
            if(pick == 0){
                attron(COLOR_PAIR(X_COLOR));
                mvprintw(y, x, "X");
                attroff(COLOR_PAIR(X_COLOR));
            }else if(pick == 1){
                attron(COLOR_PAIR(O_COLOR));
                mvprintw(y, x, "O");
                attroff(COLOR_PAIR(O_COLOR));
            }else if(pick == 2){
                attron(COLOR_PAIR(BG_COLOR));
                mvprintw(y, x, " ");
                attroff(COLOR_PAIR(BG_COLOR));
            }
        }
    }
    refresh();
}

void player_turn(char *space_ptr, char playable_spaces[num_spaces], char side){
    // Function for the player turn
    char padding[] =  "                                                ";
    char str1[] =     "    Use arrow keys to move and 'P' to place!    ";
    char str2[] =     "                   Good move!                   ";
    char str3[] =     "                 Invalid input!                 ";
    char str4[] =     "             You can't move that way!           ";
    char str5[] =     "              Space already occupied!           ";
    int len = strlen(padding);
    int row, col, x, y;
    getmaxyx(stdscr, row, col);
    const int board_line_len = 9;
    const int board_lines = 9;
    y = row / 2 - board_line_len / 2;
    x = col / 2 - board_line_len / 2;
    // Use the same method of dynamically measuring where the spaces are at using
    // terminal size as in the paint_board() function.
    int playable_x[num_spaces] = {x+2, x+4, x+6, x+2, x+4, x+6, x+2, x+4, x+6};
    int playable_y[num_spaces] = {y+2, y+2, y+2, y+4, y+4, y+4, y+6, y+6, y+6};
    // The variables and mvprintw functions for the "info line"
    const int info_line_y = (row / 2 - board_lines / 2) + 10;
    const int info_line_x = col / 2 - len / 2;
    mvprintw(info_line_y - 1, info_line_x, padding);
    mvprintw(info_line_y, info_line_x, str1);
    mvprintw(info_line_y + 1, info_line_x, padding);
    // Using a loop and pointers to collect user input
    int moving = 1;
    int input;
    int *pos_x = &playable_x[0];
    int *pos_y = &playable_y[0];
    move(*pos_y, *pos_x);
    curs_set(1);
    refresh();
    while(moving){
        // For each movement key, if the move is valid, use pointer
        // arithmetic to mov pos_x and pos_y around.
        input = toupper(getch());
        if(input == KEY_UP){
            if(*pos_y != playable_y[0]){
                pos_y -= 3;
                move(*pos_y, *pos_x);
                refresh();
            }else{
                mvprintw(info_line_y, info_line_x, str4);
                move(*pos_y, *pos_x);
                refresh();
            }
        }else if(input == KEY_DOWN){
            if(*pos_y != playable_y[6]){
                pos_y += 3;
                move(*pos_y, *pos_x);
                refresh();
            }else{
                mvprintw(info_line_y, info_line_x, str4);
                move(*pos_y, *pos_x);
                refresh();
            }
        }else if(input == KEY_LEFT){
            if(*pos_x != playable_x[0]){
                pos_x -= 1;
                move(*pos_y, *pos_x);
                refresh();
            }else{
                mvprintw(info_line_y, info_line_x, str4);
                move(*pos_y, *pos_x);
                refresh();
            }
        }else if(input == KEY_RIGHT){
            if(*pos_x != playable_x[2]){
                pos_x += 1;
                move(*pos_y, *pos_x);
                refresh();
            }else{
                mvprintw(info_line_y, info_line_x, str4);
                move(*pos_y, *pos_x);
                refresh();
            }
        }else if(input == 'P'){
            // I wanted to use KEY_ENTER instead of 'P' but it would not work
            // for some reason. When the user presses 'P' it checks where the
            // cursor is and sets the space_ptr to the appropriate index in the
            // playable_spaces array.
            if(*pos_y == playable_y[0] && *pos_x == playable_x[0]){
                space_ptr = &playable_spaces[0];                
            }else if(*pos_y == playable_y[1] && *pos_x == playable_x[1]){
                space_ptr = &playable_spaces[1];
            }else if(*pos_y == playable_y[2] && *pos_x == playable_x[2]){
                space_ptr = &playable_spaces[2];
            }else if(*pos_y == playable_y[3] && *pos_x == playable_x[3]){
                space_ptr = &playable_spaces[3];
            }else if(*pos_y == playable_y[4] && *pos_x == playable_x[4]){
                space_ptr = &playable_spaces[4];
            }else if(*pos_y == playable_y[5] && *pos_x == playable_x[5]){
                space_ptr = &playable_spaces[5];
            }else if(*pos_y == playable_y[6] && *pos_x == playable_x[6]){
                space_ptr = &playable_spaces[6];
            }else if(*pos_y == playable_y[7] && *pos_x == playable_x[7]){
                space_ptr = &playable_spaces[7];
            }else if(*pos_y == playable_y[8] && *pos_x == playable_x[8]){
                space_ptr = &playable_spaces[8];
            }
            // Then checks to see if that space is empty.
            // If so it sets the color properly and then places the piece.
            if(*space_ptr == ' '){
                if(side == 'X'){
                    attron(COLOR_PAIR(X_COLOR));
                    mvaddch(*pos_y, *pos_x, 'X');
                    attron(COLOR_PAIR(BG_COLOR));
                    *space_ptr = 'X';
                }else if(side == 'O'){
                    attron(COLOR_PAIR(O_COLOR));
                    mvaddch(*pos_y, *pos_x, 'O');
                    attron(COLOR_PAIR(BG_COLOR));
                    *space_ptr = 'O';
                }
                refresh();
                moving = 0;
            }else{
                mvprintw(info_line_y, info_line_x, str5);
                move(*pos_y, *pos_x);
                refresh();
            }
        }else{
            mvprintw(info_line_y, info_line_x, str3);
            move(*pos_y, *pos_x);
            refresh();
        }
    }
}

//////////////////////////////////////////////////////////////////////////////////////
// Begin AI Logic ////////////////////////////////////////////////////////////////////
//////////////////////////////////////////////////////////////////////////////////////

void ai_turn(char *space_ptr, char playable_spaces[num_spaces], char side){
    // wrapper for the AI turn
    /*
        Note: Since it is easy to accidentally create an unbeatable AI for tic tac toe
              I am building into the AI the chance for it to not make the optimal move.
                    This intentional fuzziness will be built into the functions that check for
                    avaialable spaces. When they find an optimal move they may just decide
                    to return 0 anyway.

        P-Code:
        if center square not taken, take center square 70% of the time;
        else:
            if opponent about to win, block them 90% of the time;
            elif self about to win take winning spot 90% of the time;
            else pick a random open spot;
    */
    // The chances for the AI to blow a move
    const int chance_to_fart_big_move = 10;
    const int chance_to_fart_center = 30;
    // Picking the character for the AI to use in its calculations
    char ai_side;
    if(side == 'X'){
        ai_side = 'O';
    }else if(side == 'O'){
        ai_side = 'X';
    }
    // Check the board state with a few functions.
    // These all return 0 if FALSE and the number of a valid
    // index to move into if TRUE
    int can_block_opponent = check_for_block(playable_spaces, side);
    int can_winning_move = check_for_winning_move(playable_spaces, ai_side);
    // Flow through the decision making logic applying the functions and checking for a fart
    int thinking = 1;
    int picked_space;
    while(thinking){
        if(playable_spaces[4] == ' '){
            if(!(ai_fart(chance_to_fart_center))){
                picked_space = 4;
                thinking = 0;
                break;
            }
        }
        if(can_winning_move){
            if(!(ai_fart(chance_to_fart_big_move))){
                picked_space = can_winning_move;
                thinking = 0;
            }else{
                picked_space = pick_random_space(playable_spaces);
                thinking = 0;
            }
        }else if(can_block_opponent){
            if(!(ai_fart(chance_to_fart_big_move))){
                picked_space = can_block_opponent;
                thinking = 0;
            }else{
                picked_space = pick_random_space(playable_spaces);
                thinking = 0;
            }
        }else{
            picked_space = pick_random_space(playable_spaces);
            thinking = 0;
        }
    }
    space_ptr = &playable_spaces[picked_space];
    if(ai_side == 'X'){
        attron(COLOR_PAIR(X_COLOR));
    }else if(ai_side == 'O'){
        attron(COLOR_PAIR(O_COLOR));
    }
    *space_ptr = ai_side;
    attron(COLOR_PAIR(BG_COLOR));
}


int ai_fart(const int chance_to_fart){
    // Takes the fart chance and returns 1 if the AI blows the move, 0 otherwise
    int roll;
    roll = rand() % 100 + 1;
    if(roll < chance_to_fart){
        return 1;
    }else{
        return 0;
    }
}

int pick_random_space(char playable_spaces[num_spaces]){
    // Returns a random open space on the board
    int roll;
    int rolling = 1;
    int pick;
    while(rolling){
        roll = rand() % num_spaces;
        if(playable_spaces[roll] == ' '){
            pick = roll;
            rolling = 0;
        }else{
            continue;
        }
    }
    return pick;
}

int check_for_winning_move(char playable_spaces[num_spaces], char ai_side){
    // Checks to see if the AI can win the game with a final move and returns the
    // index of the valid move if TRUE, returns 0 if FALSE
    int space;
    int pick;
    int picked = 0;
    for(space = 0; space < num_spaces; space++){
        // For each space: Check to see if it is a potential winning space and if so
        // switch "picked" to 1 and set "pick" to the winning index
        switch(space){
            case(0):
                if(playable_spaces[space] == ' '){
                    if(playable_spaces[1] == ai_side && playable_spaces[2] == ai_side){
                        pick = space;
                        picked = 1;
                    }else if(playable_spaces[3] == ai_side && playable_spaces[6] == ai_side){
                        pick = space;
                        picked = 1;
                    }else if(playable_spaces[4] == ai_side && playable_spaces[8] == ai_side){
                        pick = space;
                        picked = 1;
                    }
                }
                break;
            case(1):
                if(playable_spaces[space] == ' '){
                    if(playable_spaces[0] == ai_side && playable_spaces[2] == ai_side){
                        pick = space;
                        picked = 1;
                    }else if(playable_spaces[4] == ai_side && playable_spaces[7] == ai_side){
                        pick = space;
                        picked = 1;
                    }
                }
                break;
            case(2):
                if(playable_spaces[space] == ' '){
                    if(playable_spaces[1] == ai_side && playable_spaces[0] == ai_side){
                        pick = space;
                        picked = 1;
                    }else if(playable_spaces[4] == ai_side && playable_spaces[6] == ai_side){
                        pick = space;
                        picked = 1;
                    }else if(playable_spaces[5] == ai_side && playable_spaces[8] == ai_side){
                        pick = space;
                        picked = 1;
                    }
                }
                break;
            case(3):
                if(playable_spaces[space] == ' '){
                    if(playable_spaces[4] == ai_side && playable_spaces[5] == ai_side){
                        pick = space;
                        picked = 1;
                    }else if(playable_spaces[0] == ai_side && playable_spaces[6] == ai_side){
                        pick = space;
                        picked = 1;
                    }
                }
                break;
            case(4):
                if(playable_spaces[space] == ' '){
                    if(playable_spaces[1] == ai_side && playable_spaces[7] == ai_side){
                        pick = space;
                        picked = 1;
                    }else if(playable_spaces[3] == ai_side && playable_spaces[5] == ai_side){
                        pick = space;
                        picked = 1;
                    }else if(playable_spaces[0] == ai_side && playable_spaces[8] == ai_side){
                        pick = space;
                        picked = 1;
                    }else if(playable_spaces[6] == ai_side && playable_spaces[2] == ai_side){
                        pick = space;
                        picked = 1;
                    }
                }
                break;
            case(5):
                if(playable_spaces[space] == ' '){
                    if(playable_spaces[8] == ai_side && playable_spaces[2] == ai_side){
                        pick = space;
                        picked = 1;
                    }else if(playable_spaces[3] == ai_side && playable_spaces[4] == ai_side){
                        pick = space;
                        picked = 1;
                    }
                }
                break;
            case(6):
                if(playable_spaces[space] == ' '){
                    if(playable_spaces[4] == ai_side && playable_spaces[2] == ai_side){
                        pick = space;
                        picked = 1;
                    }else if(playable_spaces[7] == ai_side && playable_spaces[8] == ai_side){
                        pick = space;
                        picked = 1;
                    }else if(playable_spaces[3] == ai_side && playable_spaces[0] == ai_side){
                        pick = space;
                        picked = 1;
                    }
                }
                break;
            case(7):
                if(playable_spaces[space] == ' '){
                    if(playable_spaces[6] == ai_side && playable_spaces[8] == ai_side){
                        pick = space;
                        picked = 1;
                    }else if(playable_spaces[4] == ai_side && playable_spaces[1] == ai_side){
                        pick = space;
                        picked = 1;
                    }
                }
                break;
            case(8):
                if(playable_spaces[space] == ' '){
                    if(playable_spaces[5] == ai_side && playable_spaces[2] == ai_side){
                        pick = space;
                        picked = 1;
                    }else if(playable_spaces[4] == ai_side && playable_spaces[0] == ai_side){
                        pick = space;
                        picked = 1;
                    }else if(playable_spaces[7] == ai_side && playable_spaces[6] == ai_side){
                        pick = space;
                        picked = 1;
                    }
                }
                break;
        }
    }
    // return winning index if any
    if(picked){
        return pick;
    }else{
        return 0;
    }
}

int check_for_block(char playable_spaces[num_spaces], char side){
    // Checks to see if the AI can block the player from winning the game with a final move
    // and returns the index of the valid move if TRUE, returns 0 if FALSE
    // Note: I am sure there is a way to combine this this function with the
    //  check_for_winning_move() function in order to avoid code duplication, probably using
    //  one more parameter as a switch of some kind. I'd be open to examples of how to do that.
    int space;
    int pick;
    int picked = 0;
    for(space = 0; space < num_spaces; space++){
        // For each space: Check to see if it is a potential winning space and if so
        // switch "picked" to 1 and set "pick" to the winning index
        switch(space){
            case(0):
                if(playable_spaces[space] == ' '){
                    if(playable_spaces[1] == side && playable_spaces[2] == side){
                        pick = space;
                        picked = 1;
                    }else if(playable_spaces[3] == side && playable_spaces[6] == side){
                        pick = space;
                        picked = 1;
                    }else if(playable_spaces[4] == side && playable_spaces[8] == side){
                        pick = space;
                        picked = 1;
                    }
                }
                break;
            case(1):
                if(playable_spaces[space] == ' '){
                    if(playable_spaces[0] == side && playable_spaces[2] == side){
                        pick = space;
                        picked = 1;
                    }else if(playable_spaces[4] == side && playable_spaces[7] == side){
                        pick = space;
                        picked = 1;
                    }
                }
                break;
            case(2):
                if(playable_spaces[space] == ' '){
                    if(playable_spaces[1] == side && playable_spaces[0] == side){
                        pick = space;
                        picked = 1;
                    }else if(playable_spaces[4] == side && playable_spaces[6] == side){
                        pick = space;
                        picked = 1;
                    }else if(playable_spaces[5] == side && playable_spaces[8] == side){
                        pick = space;
                        picked = 1;
                    }
                }
                break;
            case(3):
                if(playable_spaces[space] == ' '){
                    if(playable_spaces[4] == side && playable_spaces[5] == side){
                        pick = space;
                        picked = 1;
                    }else if(playable_spaces[0] == side && playable_spaces[6] == side){
                        pick = space;
                        picked = 1;
                    }
                }
                break;
            case(4):
                if(playable_spaces[space] == ' '){
                    if(playable_spaces[1] == side && playable_spaces[7] == side){
                        pick = space;
                        picked = 1;
                    }else if(playable_spaces[3] == side && playable_spaces[5] == side){
                        pick = space;
                        picked = 1;
                    }else if(playable_spaces[0] == side && playable_spaces[8] == side){
                        pick = space;
                        picked = 1;
                    }else if(playable_spaces[6] == side && playable_spaces[2] == side){
                        pick = space;
                        picked = 1;
                    }
                }
                break;
            case(5):
                if(playable_spaces[space] == ' '){
                    if(playable_spaces[8] == side && playable_spaces[2] == side){
                        pick = space;
                        picked = 1;
                    }else if(playable_spaces[3] == side && playable_spaces[4] == side){
                        pick = space;
                        picked = 1;
                    }
                }
                break;
            case(6):
                if(playable_spaces[space] == ' '){
                    if(playable_spaces[4] == side && playable_spaces[2] == side){
                        pick = space;
                        picked = 1;
                    }else if(playable_spaces[7] == side && playable_spaces[8] == side){
                        pick = space;
                        picked = 1;
                    }else if(playable_spaces[3] == side && playable_spaces[0] == side){
                        pick = space;
                        picked = 1;
                    }
                }
                break;
            case(7):
                if(playable_spaces[space] == ' '){
                    if(playable_spaces[6] == side && playable_spaces[8] == side){
                        pick = space;
                        picked = 1;
                    }else if(playable_spaces[4] == side && playable_spaces[1] == side){
                        pick = space;
                        picked = 1;
                    }
                }
                break;
            case(8):
                if(playable_spaces[space] == ' '){
                    if(playable_spaces[5] == side && playable_spaces[2] == side){
                        pick = space;
                        picked = 1;
                    }else if(playable_spaces[4] == side && playable_spaces[0] == side){
                        pick = space;
                        picked = 1;
                    }else if(playable_spaces[7] == side && playable_spaces[6] == side){
                        pick = space;
                        picked = 1;
                    }
                }
                break;
        }
    }
    // return winning index if any
    if(picked){
        return pick;
    }else{
        return 0;
    }
}

///////////////////////////////////////////////////////////////////////////////////
// End AI Logic ///////////////////////////////////////////////////////////////////
///////////////////////////////////////////////////////////////////////////////////

int spaces_left(char playable_spaces[num_spaces]){
    // Returns 0 if no spaces left
    int hits = 0;
    int k;
    for(k = 0; k < num_spaces; k++){
        if(playable_spaces[k] == ' '){
        hits++;
        }
    }
    return hits;
}

Edit: Thanks guys! So many good tips. We are getting closer to a good style! I will re-write this again for sure. My goal is to develop a style with this project that I can carry forward into more complicated ones and the critique I have received is invaluable. Don't stop it coming just because I marked an answer! I've already begun on Tic Tac Toe v3 using many of these suggestions.

If I may ask for something specific I would ask that someone go in to detail about how I can use STRUCTS to emulate OOP for my tile spaces. I larger projects (say, a Roguelike) I would want to use Objects to represent the tiles. How would I go about doing that in C, with Structs?

Edit 2: One of the people who gave an answer asked me to put this up on GitHub so here you go: https://github.com/JanitorsBucket/tic_tac_toe_NCURSES_v2

\$\endgroup\$
  • 2
    \$\begingroup\$ I finally decided to compile and run this. Nice background! I might submit a separate answer with a "suggested implementation". \$\endgroup\$ – Reinderien Dec 30 '18 at 16:04
7
\$\begingroup\$

Capitalize your defines

#define num_spaces 9

should have NUM_SPACES instead of num_spaces.

Identify your local functions

Tell the compiler when you don't intend to export functions to a different translation unit. Mark all of your functions static except main.

Const integral args aren't needed

This:

int ai_fart(const int chance_to_fart);

doesn't really need its argument made const. const is most often applied to pointer-type arguments to indicate that the value won't be changed, but for integral types, the caller's copy cannot be changed, so doing this has little value.

Use real booleans

There are several variables, including running and turning, that are int but should be boolean. Include stdbool.h for this purpose.

Use enums for integer signalling values

In at least one spot (the return value of evaluate_board), you're returning an integer that has special meaning. This is the perfect situation to change the return type from int to an enum type that you typedef, and have both the caller and callee use the enum's named constants to make the code clearer, more meaningful, and (to a certain extent) more able to be statically checked by the compiler.

Don't separate declaration from initialization

You do this in several places:

int i;
for(i = 0; i < 9; i++){
    *space_ptr = ' ';
    space_ptr++;
}
// ...
int len;
len = strlen(padding);
// ...
int input;
input = toupper(getch());

Don't do this. Just use the syntax int input = toupper(getch());

Remove redundant else

Whenever you return in an if, the following code doesn't need an else. So delete else from code like this:

if(input == 'Q'){
    return 0;
}else{
// ...

This occurs many times throughout your code.

Use more loops

Computers are good at repetition. This:

int playable_x[num_spaces] = {x+2, x+4, x+6, x+2, x+4, x+6, x+2, x+4, x+6};

should not be initialized in a literal. Initialize it in a loop. The compiler will make the (usually right) decision as to whether the loop should be unrolled.

Similarly, in your evaluate_board function, you have highly repeated code that should be refactored into loops to check playable_spaces.

More code that needs to be a loop -

        if(*pos_y == playable_y[0] && *pos_x == playable_x[0]){
            space_ptr = &playable_spaces[0];    
        // ...            

Choose better variable names

Particularly for str1, str2, etc.

Don't repeat yourself

This block of code:

    if(input == KEY_UP){
        if(*pos_y != playable_y[0]){
            pos_y -= 3;
            move(*pos_y, *pos_x);
            refresh();
        }else{
            mvprintw(info_line_y, info_line_x, str4);
            move(*pos_y, *pos_x);
            refresh();
        }

is repeated nearly verbatim four times. You should put this into a function. The same goes for your cases in check_for_winning_move and check_for_block.

Don't hard-code things that should be computed

For instance, don't store or hard-code "9" anywhere. "9" is just the game grid width squared, so define the game grid width (3), and then make a convenience #define equal to the game grid width squared.

Make symbols to explain magic numbers

For instance, you're using 0 as "invisible cursor". It's the fault of ncurses for not giving you a symbol to use, but you can still fix this in your code by doing something like

#define INVISIBLE_CUR 0

Pass arrays as const where appropriate

Many of your functions accept arrays but don't modify them:

  • spaces_left
  • pick_random_space
  • check_for_winning_move
  • check_for_block

The array arguments there should be declared const.

Use a standard main signature

i.e.

int main(int argc, char **argv)

Don't abuse loops

Several of your loops need to die, specifically:

while(playing){

That only gets executed once, so kill it.

for(i = 0; i < 9; i++){
    *space_ptr = ' ';
    space_ptr++;
}

This shouldn't exist at all. Call memset instead.

while(thinking)

This seems to just be a way to hack in the equivalent of a goto; the loop only executes once. Kill the loop and move its contents to a separate function, so that you can use return for early termination.

Use boolean expressions directly

This:

if(roll < chance_to_fart){
    return 1;
}else{
    return 0;
}

is an anti-pattern; you can just do:

return roll < chance_to_fart;

Use more switches

The following should all be converted to switches:

if(playable_spaces[k] == 'O'){

and

input = toupper(getch());
if(input == 'P'){

and

input = toupper(getch());
if(input == 'X' || input == 'O'){

and

if(game_over_state == 1){
    vic_pointer = str1;

and

        pick = rand() % 3;
        if(pick == 0){

UI bug

If the user tries to navigate off of the grid, you show the message You can't move that way, which is good - but that message obscures the prompt, and doesn't go away even when the user navigates to a different, valid cell. There are several ways to fix this. The easiest one is to have the error message be output on a different line from the prompt, and erase the error message as soon as the user has entered valid input.

Sample refactored code

This is nowhere near complete.

// Refer to https://codereview.stackexchange.com/questions/210577/tic-tac-toe-in-c-w-ncurses-revision

// tic tac toe v2 using suggestions from Stack Exchange for better style
// Minus the struct stuff which I don't quite understand just yet.

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

// #define's for the COLOR_PAIRs
#define X_COLOR 1
#define O_COLOR 2
#define BG_COLOR 3

#define SQSIZE 3
#define NUM_SPACES (SQSIZE*SQSIZE)

#define INVISIBLE_CURSOR 0

// Function Declarations
static void init_spaces(char *space_ptr);
static void paint_board(char playable_spaces[NUM_SPACES]);
static void take_turn(char side, char *space_ptr,
                      char playable_spaces[NUM_SPACES]);
static void victory_splash(int game_over_state);
static void paint_background();
static void player_turn(char *space_ptr, char playable_spaces[NUM_SPACES],
                        char side);
static void ai_turn(char *space_ptr, char playable_spaces[NUM_SPACES],
                    char side);
static int pick_ai_space(const char playable_spaces[NUM_SPACES],
                         int chance_to_fart_big_move,
                         int chance_to_fart_center,
                         char side, char ai_side);
static bool main_menu();
static int evaluate_board(char playable_spaces[NUM_SPACES]);
static int spaces_left(const char playable_spaces[NUM_SPACES]);
static bool ai_fart(int chance_to_fart);
static int pick_random_space(const char playable_spaces[NUM_SPACES]);
static int check_for_winning_move(const char playable_spaces[NUM_SPACES],
                                  char ai_side);
static int check_for_block(const char playable_spaces[NUM_SPACES], char side);
static char pick_side();


int main(int argc, char **argv) {
    // To-Do: Try the time(NULL) method for srand initialization and see if it
    // works the same
    time_t t;
    srand((unsigned)time(&t));
    char playable_spaces[NUM_SPACES] = "XXX"
                                       "XXX"
                                       "XXX";
    char *space_ptr = playable_spaces;

    // Game over splash
    const char *game_over_str = " Game Over! Any key to continue... ",
               *go_padding    = "                                   ";
    int game_over_len = strlen(game_over_str);

    //curses init
    initscr();
    cbreak();
    keypad(stdscr, 1);
    curs_set(INVISIBLE_CURSOR);
    start_color();
    init_pair(X_COLOR, COLOR_CYAN, COLOR_BLACK);
    init_pair(O_COLOR, COLOR_GREEN, COLOR_BLACK);
    init_pair(BG_COLOR, COLOR_YELLOW, COLOR_BLACK);
    noecho();

    // Main Menu outer loop
    // Main menu function quits or continues
    while (main_menu()) {
        // Init all spaces to blank
        init_spaces(space_ptr);
        // Player picks their side.
        char side = pick_side();
        // The inner, inner turn loop
        int game_over;
        do {
            // Paint the board state as it is that turn
            paint_board(playable_spaces);
            // Function that governs the turn cycle
            take_turn(side, space_ptr, playable_spaces);
            // Evaluate the board for game over state
            game_over = evaluate_board(playable_spaces);
        } while (!game_over);
        // paint the board with a splash on game over
        // so the player can evaluate the board for a moment
        paint_board(playable_spaces);
        int row, col;
        getmaxyx(stdscr, row, col);
        int y = row/2 + 6,
            x = col/2 - game_over_len/2;
        attron(COLOR_PAIR(BG_COLOR));
        mvprintw(y++, x, go_padding);
        mvprintw(y++, x, game_over_str);
        mvprintw(y, x, go_padding);
        refresh();
        getch();
        // call victory_splash with int game_over as a parameter
        // 1 = X wins, 2 = O wins, 3 = Tie
        victory_splash(game_over);
    }

    // end curses
    endwin();

    return 0;
}

static void init_spaces(char *space_ptr) {
    // init all the spaces to ' '
    memset(space_ptr, ' ', NUM_SPACES);
}

static void paint_board(char playable_spaces[NUM_SPACES]) {
    // paint the board and the playable spaces
    clear();
    paint_background();
    const char *break_lines = " ------- ",
               *play_lines  = " | | | | ",
               *padding     = "         ";
    int row, col;
    getmaxyx(stdscr, row, col);
    int y = row/2 - 4,
        len = strlen(padding),
        x = col/2 - len/2;
    attron(COLOR_PAIR(BG_COLOR));
    for (int k = 0; k < NUM_SPACES; k++) {
        // Paint the board itself without the pieces
        if (k == 0 || k == NUM_SPACES - 1)
            mvprintw(y + k, x, padding);
        else if (k%2 == 0)
            mvprintw(y + k, x, play_lines);
        else
            mvprintw(y + k, x, break_lines);
    }
    attroff(COLOR_PAIR(BG_COLOR));

    // insert Xs and Os:
    // First set the dynamic x and y coordinates based on terminal size
    int playable_x[NUM_SPACES], playable_y[NUM_SPACES];
    for (int i = 0; i < SQSIZE; i++) {
        int ycoord = y + 2*(i + 1);
        for (int j = 0; j < SQSIZE; j++) {
            int idx = SQSIZE*i + j;
            playable_x[idx] = x + 2*(j + 1);
            playable_y[idx] = ycoord;
        }
    }

    for (int k = 0; k < NUM_SPACES; k++) {
        // For each of the playable spaces, first set the color
        int color;
        switch (playable_spaces[k]) {
            case 'O':
                color = O_COLOR;
                break;
            case 'X':
                color = X_COLOR;
                break;
            default:
                color = BG_COLOR;
        }
        attron(COLOR_PAIR(color));
        // then insert the char for that space into the proper spot on the terminal
        mvaddch(playable_y[k], playable_x[k], playable_spaces[k]);
    }
    // refresh the screen
    refresh();
}

static void take_turn(char side, char *space_ptr,
                      char playable_spaces[NUM_SPACES]) {
    // using "side" to determine the order, call the functions to play a whole turn
    if (side == 'X') {
        player_turn(space_ptr, playable_spaces, side);
        paint_board(playable_spaces);
        if (spaces_left(playable_spaces)) {
            if (!(evaluate_board(playable_spaces))) {
                ai_turn(space_ptr, playable_spaces, side);
                paint_board(playable_spaces);
            }
        }
    }
    else if (side == 'O') {
        ai_turn(space_ptr, playable_spaces, side);
        paint_board(playable_spaces);
        if (spaces_left(playable_spaces)) {
            if (!(evaluate_board(playable_spaces))) {
                player_turn(space_ptr, playable_spaces, side);
                paint_board(playable_spaces);
            }
        }
    }
}

static bool main_menu() {
    const char *error_string = " Invalid Input! Any key to try again... ",
               *str1         = " NCURSES TIC TAC TOE (v2) ",
               *padding      = "                          ",
               *str2         = "    (P)lay or (Q)uit?     ";
    int len = strlen(str1),
        error_str_len = strlen(error_string);
    for (;;) {
        clear();
        // Takes user input and returns an int that quits or starts a game
        int row, col;

        paint_background();
        getmaxyx(stdscr, row, col);
        int y = row/2 - 2,
            x = col/2 - len/2;
        mvprintw(y++, x, padding);
        mvprintw(y++, x, str1);
        mvprintw(y++, x, padding);
        mvprintw(y++, x, str2);
        mvprintw(y++, x, padding);
        refresh();
        // get user input and return it
        switch (toupper(getch())) {
            case 'P':
                return true;
            case 'Q':
                return false;
            default:
                // call the function again if the input is bad
                x = col/2 - error_str_len/2;
                mvprintw(++y, x, error_string);
                getch();
        }
    }
}

static int evaluate_board(char playable_spaces[NUM_SPACES]) {
    // Evaluates the state of the playable spaces and either does nothing
    // or ends the game.
    // Check all the possible winning combinations:
    if (playable_spaces[0] == 'X' && playable_spaces[1] == 'X' && playable_spaces[2] == 'X') {
        return 1;
    }
    if (playable_spaces[3] == 'X' && playable_spaces[4] == 'X' && playable_spaces[5] == 'X') {
        return 1;
    }
    if (playable_spaces[6] == 'X' && playable_spaces[7] == 'X' && playable_spaces[8] == 'X') {
        return 1;
    }
    if (playable_spaces[0] == 'X' && playable_spaces[3] == 'X' && playable_spaces[6] == 'X') {
        return 1;
    }
    if (playable_spaces[1] == 'X' && playable_spaces[4] == 'X' && playable_spaces[7] == 'X') {
        return 1;
    }
    if (playable_spaces[2] == 'X' && playable_spaces[5] == 'X' && playable_spaces[8] == 'X') {
        return 1;
    }
    if (playable_spaces[0] == 'X' && playable_spaces[4] == 'X' && playable_spaces[8] == 'X') {
        return 1;
    }
    if (playable_spaces[2] == 'X' && playable_spaces[4] == 'X' && playable_spaces[6] == 'X') {
        return 1;
    }
    if (playable_spaces[0] == 'O' && playable_spaces[1] == 'O' && playable_spaces[2] == 'O') {
        return 2;
    }
    if (playable_spaces[3] == 'O' && playable_spaces[4] == 'O' && playable_spaces[5] == 'O') {
        return 2;
    }
    if (playable_spaces[6] == 'O' && playable_spaces[7] == 'O' && playable_spaces[8] == 'O') {
        return 2;
    }
    if (playable_spaces[0] == 'O' && playable_spaces[3] == 'O' && playable_spaces[6] == 'O') {
        return 2;
    }
    if (playable_spaces[1] == 'O' && playable_spaces[4] == 'O' && playable_spaces[7] == 'O') {
        return 2;
    }
    if (playable_spaces[2] == 'O' && playable_spaces[5] == 'O' && playable_spaces[8] == 'O') {
        return 2;
    }
    else if (playable_spaces[0] == 'O' && playable_spaces[4] == 'O' && playable_spaces[8] == 'O') {
        return 2;
    }
    else if (playable_spaces[2] == 'O' && playable_spaces[4] == 'O' && playable_spaces[6] == 'O') {
        return 2;
    }

    // Check all spaces for a tie
    int hits = 0;
    for (int i = 0; i < NUM_SPACES; i++)
        if (playable_spaces[i] != ' ')
            hits++;

    if (hits >= NUM_SPACES)
        return 3;

    return 0;
}

char pick_side() {
    const char *str1    = " Press 'X' for X, 'O' for O, or 'R' for random! ",
               *str2    = "        Good choice! Any key to continue...     ",
               *padding = "                                                ",
               *err_str = "      Invalid input! Any key to continue...     ";
    int len = strlen(str1);

    for (;;) {
        // Takes user input and returns the chosen side
        clear();
        paint_background();
        int row, col;
        getmaxyx(stdscr, row, col);
        int y = row / 2 - 2,
            x = col / 2 - len / 2;
        mvprintw(y++, x, padding);
        mvprintw(y++, x, str1);
        mvprintw(y++, x, padding);
        refresh();
        // Get user input for picking a side. 'R' is random.
        char input = toupper(getch());
        switch (input) {
            case 'X':
            case 'O': {
                mvprintw(y, x, str2);
                refresh();
                getch();
                return input;
            }
            case 'R': {
                bool pick = rand() % 2;
                if (pick)
                    input = 'X';
                else
                    input = 'O';
                mvprintw(y, x, str2);
                refresh();
                getch();
                return input;
            }
            default: {
                // Call the function again on bad input
                mvprintw(y, x, err_str);
                refresh();
                getch();
            }
        }
    }
}

static void victory_splash(int game_over_state) {
    // Takes the game over state and creates a victory splash
    const char *padding = "                                   ",
               *str1    = "              X Wins!              ",
               *str2    = "              O Wins!              ",
               *str3    = "         any key to continue...    ",
               *str4    = "             A tie game!           ";
    int len = strlen(padding);
    const char *vic_pointer;
    // To avoid code duplication, use a pointer to pick the right string
    switch (game_over_state) {
    case 1:
        vic_pointer = str1;
    case 2:
        vic_pointer = str2;
    case 3:
        vic_pointer = str4;
    }
    clear();
    paint_background();
    int row, col;
    getmaxyx(stdscr, row, col);
    int y = row/2 - 2,
        x = col/2 - len/2;
    mvprintw(y++, x, padding);
    mvprintw(y++, x, vic_pointer);
    mvprintw(y++, x, padding);
    mvprintw(y, x, str3);
    refresh();
    getch();
}

static void paint_background() {
    // Paints an elaborate flashy background
    int row, col;
    getmaxyx(stdscr, row, col);
    for (int y = 0; y <= row; y++) {
        for (int x = 0; x <= col; x++) {
            int color;
            char draw;
            switch (rand() % 3) {
                case 0:
                    color = X_COLOR;
                    draw = 'X';
                    break;
                case 1:
                    color = O_COLOR;
                    draw = 'O';
                    break;
                case 2:
                    color = BG_COLOR;
                    draw = ' ';
                    break;
            }
            attron(COLOR_PAIR(color));
            char draw_str[] = {draw, '\0'};
            mvprintw(y, x, draw_str);
            attroff(COLOR_PAIR(color));
        }
    }
    refresh();
}

static void player_turn(char *space_ptr, char playable_spaces[NUM_SPACES], char side) {
    // Function for the player turn
    char padding[] =  "                                                ";
    char str1[] =     "    Use arrow keys to move and 'P' to place!    ";
    char str3[] =     "                 Invalid input!                 ";
    char str4[] =     "             You can't move that way!           ";
    char str5[] =     "              Space already occupied!           ";
    int len = strlen(padding);
    int row, col, x, y;
    getmaxyx(stdscr, row, col);
    const int board_line_len = 9;
    const int board_lines = 9;
    y = row / 2 - board_line_len / 2;
    x = col / 2 - board_line_len / 2;
    // Use the same method of dynamically measuring where the spaces are at using
    // terminal size as in the paint_board() function.
    int playable_x[NUM_SPACES] = {x+2, x+4, x+6, x+2, x+4, x+6, x+2, x+4, x+6};
    int playable_y[NUM_SPACES] = {y+2, y+2, y+2, y+4, y+4, y+4, y+6, y+6, y+6};
    // The variables and mvprintw functions for the "info line"
    const int info_line_y = (row / 2 - board_lines / 2) + 10;
    const int info_line_x = col / 2 - len / 2;
    mvprintw(info_line_y - 1, info_line_x, padding);
    mvprintw(info_line_y, info_line_x, str1);
    mvprintw(info_line_y + 1, info_line_x, padding);
    // Using a loop and pointers to collect user input
    int moving = 1;
    int input;
    int *pos_x = &playable_x[0];
    int *pos_y = &playable_y[0];
    move(*pos_y, *pos_x);
    curs_set(1);
    refresh();
    while(moving) {
        // For each movement key, if the move is valid, use pointer
        // arithmetic to mov pos_x and pos_y around.
        input = toupper(getch());
        if (input == KEY_UP) {
            if (*pos_y != playable_y[0]) {
                pos_y -= 3;
                move(*pos_y, *pos_x);
                refresh();
            }
            else{
                mvprintw(info_line_y, info_line_x, str4);
                move(*pos_y, *pos_x);
                refresh();
            }
        }
        else if (input == KEY_DOWN) {
            if (*pos_y != playable_y[6]) {
                pos_y += 3;
                move(*pos_y, *pos_x);
                refresh();
            }
            else{
                mvprintw(info_line_y, info_line_x, str4);
                move(*pos_y, *pos_x);
                refresh();
            }
        }
        else if (input == KEY_LEFT) {
            if (*pos_x != playable_x[0]) {
                pos_x -= 1;
                move(*pos_y, *pos_x);
                refresh();
            }
            else{
                mvprintw(info_line_y, info_line_x, str4);
                move(*pos_y, *pos_x);
                refresh();
            }
        }
        else if (input == KEY_RIGHT) {
            if (*pos_x != playable_x[2]) {
                pos_x += 1;
                move(*pos_y, *pos_x);
                refresh();
            }
            else{
                mvprintw(info_line_y, info_line_x, str4);
                move(*pos_y, *pos_x);
                refresh();
            }
        }
        else if (input == 'P') {
            // I wanted to use KEY_ENTER instead of 'P' but it would not work
            // for some reason. When the user presses 'P' it checks where the
            // cursor is and sets the space_ptr to the appropriate index in the
            // playable_spaces array.
            if (*pos_y == playable_y[0] && *pos_x == playable_x[0]) {
                space_ptr = &playable_spaces[0];                
            }
            else if (*pos_y == playable_y[1] && *pos_x == playable_x[1]) {
                space_ptr = &playable_spaces[1];
            }
            else if (*pos_y == playable_y[2] && *pos_x == playable_x[2]) {
                space_ptr = &playable_spaces[2];
            }
            else if (*pos_y == playable_y[3] && *pos_x == playable_x[3]) {
                space_ptr = &playable_spaces[3];
            }
            else if (*pos_y == playable_y[4] && *pos_x == playable_x[4]) {
                space_ptr = &playable_spaces[4];
            }
            else if (*pos_y == playable_y[5] && *pos_x == playable_x[5]) {
                space_ptr = &playable_spaces[5];
            }
            else if (*pos_y == playable_y[6] && *pos_x == playable_x[6]) {
                space_ptr = &playable_spaces[6];
            }
            else if (*pos_y == playable_y[7] && *pos_x == playable_x[7]) {
                space_ptr = &playable_spaces[7];
            }
            else if (*pos_y == playable_y[8] && *pos_x == playable_x[8]) {
                space_ptr = &playable_spaces[8];
            }
            // Then checks to see if that space is empty.
            // If so it sets the color properly and then places the piece.
            if (*space_ptr == ' ') {
                if (side == 'X') {
                    attron(COLOR_PAIR(X_COLOR));
                    mvaddch(*pos_y, *pos_x, 'X');
                    attron(COLOR_PAIR(BG_COLOR));
                    *space_ptr = 'X';
                }
                else if (side == 'O') {
                    attron(COLOR_PAIR(O_COLOR));
                    mvaddch(*pos_y, *pos_x, 'O');
                    attron(COLOR_PAIR(BG_COLOR));
                    *space_ptr = 'O';
                }
                refresh();
                moving = 0;
            }
            else{
                mvprintw(info_line_y, info_line_x, str5);
                move(*pos_y, *pos_x);
                refresh();
            }
        }
        else{
            mvprintw(info_line_y, info_line_x, str3);
            move(*pos_y, *pos_x);
            refresh();
        }
    }
}

//////////////////////////////////////////////////////////////////////////////////////
// Begin AI Logic ////////////////////////////////////////////////////////////////////
//////////////////////////////////////////////////////////////////////////////////////

static void ai_turn(char *space_ptr, char playable_spaces[NUM_SPACES], char side) {
    // wrapper for the AI turn
    /*
        Note: Since it is easy to accidentally create an unbeatable AI for tic tac toe
              I am building into the AI the chance for it to not make the optimal move.
                    This intentional fuzziness will be built into the functions that check for
                    avaialable spaces. When they find an optimal move they may just decide
                    to return 0 anyway.

        P-Code:
        if center square not taken, take center square 70% of the time;
        else:
            if opponent about to win, block them 90% of the time;
            elif self about to win take winning spot 90% of the time;
            else pick a random open spot;
    */
    // Picking the character for the AI to use in its calculations
    char ai_side;
    if (side == 'X')
        ai_side = 'O';
    else
        ai_side = 'X';

    // Check the board state with a few functions.
    int picked_space = pick_ai_space(playable_spaces, 10, 30, side, ai_side);
    space_ptr = &playable_spaces[picked_space];
    if (ai_side == 'X')
        attron(COLOR_PAIR(X_COLOR));
    else
        attron(COLOR_PAIR(O_COLOR));
    *space_ptr = ai_side;
    attron(COLOR_PAIR(BG_COLOR));
}

static int pick_ai_space(const char playable_spaces[NUM_SPACES],
                         int chance_to_fart_big_move,
                         int chance_to_fart_center,
                         char side, char ai_side) {
    int can_block_opponent = check_for_block(playable_spaces, side),
        can_winning_move = check_for_winning_move(playable_spaces, ai_side);

    // Flow through the decision making logic applying the functions and
    // checking for a fart
    if (playable_spaces[4] == ' ' &&
        !(ai_fart(chance_to_fart_center)))
        return 4;
    if (can_winning_move) {
        if (!(ai_fart(chance_to_fart_big_move)))
            return can_winning_move;
        return pick_random_space(playable_spaces);
    }
    if (can_block_opponent) {
        if (!(ai_fart(chance_to_fart_big_move)))
            return can_block_opponent;
        return pick_random_space(playable_spaces);
    }
    return pick_random_space(playable_spaces);
}

static bool ai_fart(int chance_to_fart) {
    // Takes the fart chance and returns 1 if the AI blows the move, 0 otherwise
    int roll = rand() % 100 + 1;
    return roll < chance_to_fart;
}

static int pick_random_space(const char playable_spaces[NUM_SPACES]) {
    // Returns a random open space on the board
    for (;;) {
        int roll = rand() % NUM_SPACES;
        if (playable_spaces[roll] == ' ')
            return roll;
    }
}

static int check_for_winning_move(const char playable_spaces[NUM_SPACES], char ai_side) {
    // Checks to see if the AI can win the game with a final move and returns the
    // index of the valid move if TRUE, returns 0 if FALSE
    int pick;
    bool picked = false;
    for (int space = 0; space < NUM_SPACES; space++) {
        // For each space: Check to see if it is a potential winning space and if so
        // switch "picked" to 1 and set "pick" to the winning index
        switch (space) {
            case 0:
                if (playable_spaces[space] == ' ') {
                    if (playable_spaces[1] == ai_side && playable_spaces[2] == ai_side) {
                        pick = space;
                        picked = true;
                    }
                    else if (playable_spaces[3] == ai_side && playable_spaces[6] == ai_side) {
                        pick = space;
                        picked = true;
                    }
                    else if (playable_spaces[4] == ai_side && playable_spaces[8] == ai_side) {
                        pick = space;
                        picked = true;
                    }
                }
                break;
            case 1:
                if (playable_spaces[space] == ' ') {
                    if (playable_spaces[0] == ai_side && playable_spaces[2] == ai_side) {
                        pick = space;
                        picked = true;
                    }
                    else if (playable_spaces[4] == ai_side && playable_spaces[7] == ai_side) {
                        pick = space;
                        picked = true;
                    }
                }
                break;
            case 2:
                if (playable_spaces[space] == ' ') {
                    if (playable_spaces[1] == ai_side && playable_spaces[0] == ai_side) {
                        pick = space;
                        picked = true;
                    }
                    else if (playable_spaces[4] == ai_side && playable_spaces[6] == ai_side) {
                        pick = space;
                        picked = true;
                    }
                    else if (playable_spaces[5] == ai_side && playable_spaces[8] == ai_side) {
                        pick = space;
                        picked = true;
                    }
                }
                break;
            case 3:
                if (playable_spaces[space] == ' ') {
                    if (playable_spaces[4] == ai_side && playable_spaces[5] == ai_side) {
                        pick = space;
                        picked = true;
                    }
                    else if (playable_spaces[0] == ai_side && playable_spaces[6] == ai_side) {
                        pick = space;
                        picked = true;
                    }
                }
                break;
            case 4:
                if (playable_spaces[space] == ' ') {
                    if (playable_spaces[1] == ai_side && playable_spaces[7] == ai_side) {
                        pick = space;
                        picked = true;
                    }
                    else if (playable_spaces[3] == ai_side && playable_spaces[5] == ai_side) {
                        pick = space;
                        picked = true;
                    }
                    else if (playable_spaces[0] == ai_side && playable_spaces[8] == ai_side) {
                        pick = space;
                        picked = true;
                    }
                    else if (playable_spaces[6] == ai_side && playable_spaces[2] == ai_side) {
                        pick = space;
                        picked = true;
                    }
                }
                break;
            case 5:
                if (playable_spaces[space] == ' ') {
                    if (playable_spaces[8] == ai_side && playable_spaces[2] == ai_side) {
                        pick = space;
                        picked = true;
                    }
                    else if (playable_spaces[3] == ai_side && playable_spaces[4] == ai_side) {
                        pick = space;
                        picked = true;
                    }
                }
                break;
            case 6:
                if (playable_spaces[space] == ' ') {
                    if (playable_spaces[4] == ai_side && playable_spaces[2] == ai_side) {
                        pick = space;
                        picked = true;
                    }
                    else if (playable_spaces[7] == ai_side && playable_spaces[8] == ai_side) {
                        pick = space;
                        picked = true;
                    }
                    else if (playable_spaces[3] == ai_side && playable_spaces[0] == ai_side) {
                        pick = space;
                        picked = true;
                    }
                }
                break;
            case 7:
                if (playable_spaces[space] == ' ') {
                    if (playable_spaces[6] == ai_side && playable_spaces[8] == ai_side) {
                        pick = space;
                        picked = true;
                    }
                    else if (playable_spaces[4] == ai_side && playable_spaces[1] == ai_side) {
                        pick = space;
                        picked = true;
                    }
                }
                break;
            case 8:
                if (playable_spaces[space] == ' ') {
                    if (playable_spaces[5] == ai_side && playable_spaces[2] == ai_side) {
                        pick = space;
                        picked = true;
                    }
                    else if (playable_spaces[4] == ai_side && playable_spaces[0] == ai_side) {
                        pick = space;
                        picked = true;
                    }
                    else if (playable_spaces[7] == ai_side && playable_spaces[6] == ai_side) {
                        pick = space;
                        picked = true;
                    }
                }
                break;
        }
    }
    // return winning index if any
    if (picked)
        return pick;
    return 0;
}

static int check_for_block(const char playable_spaces[NUM_SPACES], char side) {
    // Checks to see if the AI can block the player from winning the game with a final move
    // and returns the index of the valid move if TRUE, returns 0 if FALSE
    // Note: I am sure there is a way to combine this this function with the
    //  check_for_winning_move() function in order to avoid code duplication, probably using
    //  one more parameter as a switch of some kind. I'd be open to examples of how to do that.
    int pick;
    bool picked = false;
    for (int space = 0; space < NUM_SPACES; space++) {
        // For each space: Check to see if it is a potential winning space and if so
        // switch "picked" to 1 and set "pick" to the winning index
        switch (space) {
            case 0:
                if (playable_spaces[space] == ' ') {
                    if (playable_spaces[1] == side && playable_spaces[2] == side) {
                        pick = space;
                        picked = true;
                    }
                    else if (playable_spaces[3] == side && playable_spaces[6] == side) {
                        pick = space;
                        picked = true;
                    }
                    else if (playable_spaces[4] == side && playable_spaces[8] == side) {
                        pick = space;
                        picked = true;
                    }
                }
                break;
            case 1:
                if (playable_spaces[space] == ' ') {
                    if (playable_spaces[0] == side && playable_spaces[2] == side) {
                        pick = space;
                        picked = true;
                    }
                    else if (playable_spaces[4] == side && playable_spaces[7] == side) {
                        pick = space;
                        picked = true;
                    }
                }
                break;
            case 2:
                if (playable_spaces[space] == ' ') {
                    if (playable_spaces[1] == side && playable_spaces[0] == side) {
                        pick = space;
                        picked = true;
                    }
                    else if (playable_spaces[4] == side && playable_spaces[6] == side) {
                        pick = space;
                        picked = true;
                    }
                    else if (playable_spaces[5] == side && playable_spaces[8] == side) {
                        pick = space;
                        picked = true;
                    }
                }
                break;
            case 3:
                if (playable_spaces[space] == ' ') {
                    if (playable_spaces[4] == side && playable_spaces[5] == side) {
                        pick = space;
                        picked = true;
                    }
                    else if (playable_spaces[0] == side && playable_spaces[6] == side) {
                        pick = space;
                        picked = true;
                    }
                }
                break;
            case 4:
                if (playable_spaces[space] == ' ') {
                    if (playable_spaces[1] == side && playable_spaces[7] == side) {
                        pick = space;
                        picked = true;
                    }
                    else if (playable_spaces[3] == side && playable_spaces[5] == side) {
                        pick = space;
                        picked = true;
                    }
                    else if (playable_spaces[0] == side && playable_spaces[8] == side) {
                        pick = space;
                        picked = true;
                    }
                    else if (playable_spaces[6] == side && playable_spaces[2] == side) {
                        pick = space;
                        picked = true;
                    }
                }
                break;
            case 5:
                if (playable_spaces[space] == ' ') {
                    if (playable_spaces[8] == side && playable_spaces[2] == side) {
                        pick = space;
                        picked = true;
                    }
                    else if (playable_spaces[3] == side && playable_spaces[4] == side) {
                        pick = space;
                        picked = true;
                    }
                }
                break;
            case 6:
                if (playable_spaces[space] == ' ') {
                    if (playable_spaces[4] == side && playable_spaces[2] == side) {
                        pick = space;
                        picked = true;
                    }
                    else if (playable_spaces[7] == side && playable_spaces[8] == side) {
                        pick = space;
                        picked = true;
                    }
                    else if (playable_spaces[3] == side && playable_spaces[0] == side) {
                        pick = space;
                        picked = true;
                    }
                }
                break;
            case 7:
                if (playable_spaces[space] == ' ') {
                    if (playable_spaces[6] == side && playable_spaces[8] == side) {
                        pick = space;
                        picked = true;
                    }
                    else if (playable_spaces[4] == side && playable_spaces[1] == side) {
                        pick = space;
                        picked = true;
                    }
                }
                break;
            case 8:
                if (playable_spaces[space] == ' ') {
                    if (playable_spaces[5] == side && playable_spaces[2] == side) {
                        pick = space;
                        picked = true;
                    }
                    else if (playable_spaces[4] == side && playable_spaces[0] == side) {
                        pick = space;
                        picked = true;
                    }
                    else if (playable_spaces[7] == side && playable_spaces[6] == side) {
                        pick = space;
                        picked = true;
                    }
                }
                break;
        }
    }
    // return winning index if any
    if (picked)
        return pick;
    return 0;
}

///////////////////////////////////////////////////////////////////////////////////
// End AI Logic ///////////////////////////////////////////////////////////////////
///////////////////////////////////////////////////////////////////////////////////

static int spaces_left(const char playable_spaces[NUM_SPACES]) {
    // Returns 0 if no spaces left
    int hits = 0;
    for (int k = 0; k < NUM_SPACES; k++)
        if (playable_spaces[k] == ' ')
            hits++;
    return hits;
}
\$\endgroup\$
  • \$\begingroup\$ "Tell the compiler when you don't intend to export functions to a different translation unit. Mark all of your functions static except main." Why? "There are several variables, including running and turning, that are int but should be boolean. Include stdbool.h for this purpose." I did this for a reason though. If they return 0 then they are passed over, otherwise they return integers that do different things. Is that bad practice? Thanks for your tips! I'll keep working at it. C sure is harder than Python in a lot of ways. \$\endgroup\$ – some_guy632 Dec 30 '18 at 4:15
  • \$\begingroup\$ I mis-spoke in my above comment but ran out of time to edit. Running and turning are just booleans but is it bad practice to use 1 and 0? What about using #defines? I have a lot of functions that I use in testing if statements that return 0 if false and return a variety of possible numbers if true, doing different things. Is that also bad practice? \$\endgroup\$ – some_guy632 Dec 30 '18 at 4:22
  • \$\begingroup\$ Thanks! Can I get you to expand on why this is? What other functionality does stdbool.h offer to make it superior? \$\endgroup\$ – some_guy632 Dec 30 '18 at 4:25
  • 2
    \$\begingroup\$ Let us continue this discussion in chat. \$\endgroup\$ – Reinderien Dec 30 '18 at 4:27
  • \$\begingroup\$ Hey I have one more for you: since ncurses.h defines its own TRUE and FALSE will I run into conflicts using stdbool.h? \$\endgroup\$ – some_guy632 Dec 30 '18 at 6:27
7
\$\begingroup\$

A small review

I noticed in both iterations of your code that you have to forward declare every single function. This is not advisable. It is better to declare main() last, thereby eliminating your need to forward declare. After all just imagine the forward declarations required for a project of significant size.

After that you might also want to start separating your logic into modular files that you can then #include. Small logically connected groups of functions, #defines, structs and the like.


also don't do this:

// ncurses for, well, ncurses
#include <ncurses.h>
// time for the random seed
#include <time.h>
// string.h for strlen() 
#include <string.h>
// stdlib and stdio because why not
#include <stdlib.h>
#include <stdio.h>
// ctype.h for toupper()
#include <ctype.h>

If you want to comment what each include is for (which I find unnecessary but I could be wrong) at least do it off to the side so scanning the #includes is readable. Like so:

#include <ncurses.h> // for, well, ncurses
#include <time.h> // for random seed
#include <string.h> // for strlen()
#include <stdlib.h> // why not?
#include <stdio.h> // why not?
#include <ctype.h> // for toupper()

And never, ever, ever include headers you don't need. (I didn't read enough of the code but the comment "why not?" makes me think you didn't know if you were going to need stdlib or stdio and that's a bad reason to include something.) Just add the header when you need it.

\$\endgroup\$
  • \$\begingroup\$ Does forward declaring serve a purpose? I did it because it seems like the formal way to do it but coming from Python I'm cool not doing it! I commented the #includes hoping specifically that I'd get feedback on my choice of includes. I've often heard you should always include stdlib.h unless you're hurting for resources. Is that true? Thank you for the review! \$\endgroup\$ – some_guy632 Dec 30 '18 at 5:08
  • 3
    \$\begingroup\$ You have to forward declare because your main uses those functions. without a forward declaration your code would not compile. but if you put the main() at the end then its a non-issue. Anytime you use a function in another one though it needs to be declared or defined prior to its use. As for including stdlib.h I can't say for sure as I don't actually write in C but I imagine it's far more likely you were taught that because almost everything you write will use it. (After all it has size_t which is ubiquitous in C and C++) And good inclination to get your includes reviewed. \$\endgroup\$ – AGirlHasNoName Dec 30 '18 at 5:15
  • 1
    \$\begingroup\$ Re. forward declaration. It's important to note that for a "project of significant size", your functions would be spread across several translation units and you'd have headers, making forward declaration mandatory. \$\endgroup\$ – Reinderien Dec 30 '18 at 14:44
  • \$\begingroup\$ @Reinderien but you won't be forward declaring. The computer will. I sorta touched on that. I'll rephrase when I get a chance. \$\endgroup\$ – AGirlHasNoName Dec 30 '18 at 15:32
  • 1
    \$\begingroup\$ For clarity, I agree about code should avoid adding non-standard .h files. To include only just the right standard header files is work with little benefit. In my experience, better to include unneeded standard header files than to miss one. IMO, I'd like to see a tool that auto checked code for standard identifiers and the auto included those header. Save human programing time for the larger issues. \$\endgroup\$ – chux Dec 31 '18 at 4:45
7
\$\begingroup\$

I've made this Community Wiki because my answer revolves around 1 example that those more knowledgeable about C could probably improve and expand. Improvements are welcome!

Practice DRY code

There's one piece of feedback you received a bit in your previous review, and I think taking it to heart would improve your coding style tremendously: practice DRY code. You should never have code that could be basically copy-pasted with only a few value changes. Learning how to avoid this will make your code shorter, easier to read, and easier to maintain. You can often avoid repetition by modularizing your code further, usually by making more functions.

To demonstrate what I mean, let's take a section of your code as a case study:

Case study: win-checking conditionals

    if(playable_spaces[0] == 'X' && playable_spaces[1] == 'X' && playable_spaces[2] == 'X'){
        return 1;
    }else if(playable_spaces[3] == 'X' && playable_spaces[4] == 'X' && playable_spaces[5] == 'X'){
        return 1;
    }else if(playable_spaces[6] == 'X' && playable_spaces[7] == 'X' && playable_spaces[8] == 'X'){
        return 1;
    }else if(playable_spaces[0] == 'X' && playable_spaces[3] == 'X' && playable_spaces[6] == 'X'){
        return 1;
    }else if(playable_spaces[1] == 'X' && playable_spaces[4] == 'X' && playable_spaces[7] == 'X'){
        return 1;
    }else if(playable_spaces[2] == 'X' && playable_spaces[5] == 'X' && playable_spaces[8] == 'X'){
        return 1;
    }else if(playable_spaces[0] == 'X' && playable_spaces[4] == 'X' && playable_spaces[8] == 'X'){
        return 1;
    }else if(playable_spaces[2] == 'X' && playable_spaces[4] == 'X' && playable_spaces[6] == 'X'){
        return 1;
    }else if(playable_spaces[0] == 'O' && playable_spaces[1] == 'O' && playable_spaces[2] == 'O'){
        return 2;
    }else if(playable_spaces[3] == 'O' && playable_spaces[4] == 'O' && playable_spaces[5] == 'O'){
        return 2;
    }else if(playable_spaces[6] == 'O' && playable_spaces[7] == 'O' && playable_spaces[8] == 'O'){
        return 2;
    }else if(playable_spaces[0] == 'O' && playable_spaces[3] == 'O' && playable_spaces[6] == 'O'){
        return 2;
    }else if(playable_spaces[1] == 'O' && playable_spaces[4] == 'O' && playable_spaces[7] == 'O'){
        return 2;
    }else if(playable_spaces[2] == 'O' && playable_spaces[5] == 'O' && playable_spaces[8] == 'O'){
        return 2;
    }else if(playable_spaces[0] == 'O' && playable_spaces[4] == 'O' && playable_spaces[8] == 'O'){
        return 2;
    }else if(playable_spaces[2] == 'O' && playable_spaces[4] == 'O' && playable_spaces[6] == 'O'){
        return 2;
    }

Without even comprehending what this code does, I can tell it's not quite right because it has too much repetition.

If you have multiple conditional branches that return the same thing, you can combine them with the || or operator (only the 'X' code is shown below):

    if((playable_spaces[0] == 'X' && playable_spaces[1] == 'X' && playable_spaces[2] == 'X')
    || (playable_spaces[3] == 'X' && playable_spaces[4] == 'X' && playable_spaces[5] == 'X')
    || (playable_spaces[6] == 'X' && playable_spaces[7] == 'X' && playable_spaces[8] == 'X')
    || (playable_spaces[0] == 'X' && playable_spaces[3] == 'X' && playable_spaces[6] == 'X')
    || (playable_spaces[1] == 'X' && playable_spaces[4] == 'X' && playable_spaces[7] == 'X')
    || (playable_spaces[2] == 'X' && playable_spaces[5] == 'X' && playable_spaces[8] == 'X')
    || (playable_spaces[0] == 'X' && playable_spaces[4] == 'X' && playable_spaces[8] == 'X')
    || (playable_spaces[2] == 'X' && playable_spaces[4] == 'X' && playable_spaces[6] == 'X')) {
        return 1;
    }

I've skipped reproducing the 'O' code because it's identical to the 'X' code aside from a change from 'O'. This is also unnecessary repetition. You could make a function that takes 'X' and 'O' as char arguments, and returns the above || sequence.

A simpler way

First, let's assume we have some constants:

#define SIDE_LEN 3
#define NUM_SQUARES (SIDE_LEN*SIDE_LEN)

Now let's create a Board structure that is defined like this:

typedef struct board {
    char spaces[NUM_SQUARES];
    int available;
} Board;

The spaces contain the tokens for either X or 0 or empty. The value available simply holds the number of empty squares. Initialization of this structure is obvious and should be done before each game.

Now we can easily create a helper function:

char board_gettoken(const Board *b, int row, int col) {
    return b->spaces[row + col * SIDE_LEN];
}

If we were describing how to win to a child, we might say that anywhere we get three in a row, a column or along the diagonal, it's a win. If all of the squares are filled (that is, available == 0) and there isn't a winner, it's a tie. Finally, note that only the player who just made a move can possibly win. We can use these simple facts to write a very simple board_evaluate function:

// given that `token` just moved, return 
//  0 if no winner
//  1 if token just won
//  2 if tie
int board_evaluate(const Board *b, char token) {
    bool diag_winner = true;
    bool rev_diag_winner = true;
    for (int i=0; i < SIDE_LEN; ++i) {
        diag_winner &= board_gettoken(b, i, i) == token;
        rev_diag_winner &= board_gettoken(b, i, SIDE_LEN-1-i) == token;
    }
    if (diag_winner || rev_diag_winner) {
        return 1;
    }
    for (int i=0; i < SIDE_LEN; ++i) {
        bool row_winner = true;
        bool col_winner = true;
        for (int j=0; j < SIDE_LEN; ++j) {
            col_winner &= board_gettoken(b, i, j) == token;
            row_winner &= board_gettoken(b, j, i) == token;
        }
        if (row_winner || col_winner) {
            return 1;
        }
    }
    // must be a non-win or tie
    return b->available == 0 ? 2 : 0;
}

Finally, note that if we wanted to create 4x4 or 9x9 version, all that would need to be changed is the SIDE_LEN constant value. It's also handy to prefix functions that deal with a board with board_ so it's easy to see that they are related. In object-oriented language such as C++, these would likely be member functions.

Using DRY code for a better AI

One can use the function above to write a faster, better, shorter AI player as well. First, we define another helper function:

int board_makemove(Board *b, int i, int j, char token) {
    board_settoken(b, i, j, token);
    --b->available;
    return board_evaluate(b, token);
}

When we've made a final selection, this function uses the board_settoken to update the data structure, decrements the available square count and then returns the result of the board evaluation. There's a reason we don't decrement the available count in the board_settoken routine as we'll see in the following function:

int ai_turn(Board *b, char token) {
    const char antitoken = token == 'X' ? 'O' : 'X';
    // first look for a move that would win
    for (int i=0; i < SIDE_LEN; ++i) {
        for (int j=0; j < SIDE_LEN; ++j) {
            if (board_gettoken(b, i, j) == ' ') {
                board_settoken(b, i, j, token);
                int status = board_evaluate(b, token);
                if (status) {
                    --b->available;
                    return status;
                } else {
                    board_settoken(b, i, j, ' ');  // undo move
                }
            }
        }
    }
    // next look for a move that would block
    for (int i=0; i < SIDE_LEN; ++i) {
        for (int j=0; j < SIDE_LEN; ++j) {
            if (board_gettoken(b, i, j) == ' ') {
                board_settoken(b, i, j, antitoken);
                if (board_evaluate(b, antitoken) == 1) {
                    return board_makemove(b, i, j, token);
                } else {
                    board_settoken(b, i, j, ' ');  // undo move
                }
            }
        }
    }
    // look for center
    { 
        int i = SIDE_LEN/2;
        if (board_gettoken(b, i, i) == ' ') {
            return board_makemove(b, i, i, token);
        }
    }
    // look for corner 
    for (int i=0; i < SIDE_LEN; i += (SIDE_LEN-1)) {  
        for (int j=0; j < SIDE_LEN; j += (SIDE_LEN-1)) {  
            if (board_gettoken(b, i, j) == ' ') {
                return board_makemove(b, i, j, token);
            }
        }
    }
    // choose first available
    for (int i=0; i < SIDE_LEN; ++i) {
        for (int j=0; j < SIDE_LEN; ++j) {
            if (board_gettoken(b, i, j) == ' ') {
                return board_makemove(b, i, j, token);
            }
        }
    }
    return 0;
}

This is a very simple bit of code, and easy to follow, but it's very competent at playing the game. There are still more opportunities to DRY the code above.

\$\endgroup\$
  • \$\begingroup\$ I felt that even as I wrote that. The evaluate_board() function was the only one I had to come back and bug fix multiple times. I agree that a loop is probably the way to do it and I'd be interested in some examples. For games with bigger boards I won't ever be able to do it like that. In python I'd just create objects for the tiles and loop over them one by one, calling functions to check nearby spaces. How can I better do that in C? \$\endgroup\$ – some_guy632 Dec 30 '18 at 4:07
  • 1
    \$\begingroup\$ @some_guy632 I'm unfortunately probably not the best person to ask this, because I'm more of a Python programmer than a C programmer, hence the community wiki. I am familiar enough with C syntax to be able to edit in an example of doing it with a loop, so I will do that. \$\endgroup\$ – Graham Dec 30 '18 at 4:21
  • \$\begingroup\$ The more I think about this one the more I want to stick with forward declarations. What if you have functiones that depend on other functions but you coded them in the wrong order? Is there any good reason NOT to forward declare? \$\endgroup\$ – some_guy632 Dec 30 '18 at 9:11
  • 1
    \$\begingroup\$ @some_guy632 It's probably better to ask Reinderien, who's actually coded in C. For my part, for code that's restricted to your own file, it's probably better to put a bit of care into ordering your functions so there's no conflicts, before resorting to forward declarations. The reason for this is DRY: copying function declarations twice creates another vector for continuity error, where the forward declaration may go out of sync with the forward declaration. In either case, AFAIK, the worst thing that can happen is a compilation error, so I'm not sure if verbose forward declaring is that bad. \$\endgroup\$ – Graham Dec 30 '18 at 14:41
  • 2
    \$\begingroup\$ @some_guy632 I think you meant that comment on my answer. You are more than welcome to continue using forward decisions. If you code in the wrong ordwr your compiler will warn you. The more functions you have the more important organizing your code becomes. And forward declarations don't organize things for you they just make it so you don't have to. They have their uses but in this case you are using them as a messy workaround. And header files aren't the same thing as forward declarations. \$\endgroup\$ – AGirlHasNoName Dec 30 '18 at 15:55
6
\$\begingroup\$

Compiling using clang with warnings turned on reveals a number of issues:

$ clang -Wall -lcurses -o ttt ttt.c
ttt.c:240:1: warning: control may reach end of non-void function [-Wreturn-type]
}
^

Why is main_menu() calling itself recursively? If you want a loop, write a loop.

ttt.c:339:1: warning: control may reach end of non-void function [-Wreturn-type]
}
^

Same issue with pick_side(). It should be a loop.


ttt.c:402:10: warning: unused variable 'str2' [-Wunused-variable]
    char str2[] =     "                   Good move!                   ";
         ^

Self-explanatory.


ttt.c:555:14: warning: variable 'ai_side' is used uninitialized whenever 'if'
      condition is false [-Wsometimes-uninitialized]
    }else if(side == 'O'){
             ^~~~~~~~~~~
ttt.c:562:68: note: uninitialized use occurs here
    int can_winning_move = check_for_winning_move(playable_spaces, ai_side);
                                                                   ^~~~~~~
ttt.c:555:11: note: remove the 'if' if its condition is always true
    }else if(side == 'O'){
          ^~~~~~~~~~~~~~~
ttt.c:552:17: note: initialize the variable 'ai_side' to silence this warning
    char ai_side;
                ^
                 = '\0'
4 warnings generated.

That if-elseif would be better written as char ai_side = (side == 'X') ? 'O' : 'X';, which would have avoided all of those warnings.

\$\endgroup\$
  • 4
    \$\begingroup\$ On the last point, better still would to have an array of function pointers and call either human_turn or ai_turn. All other turn processing is identical and that entire function simply goes away. \$\endgroup\$ – Edward Dec 30 '18 at 22:27
3
\$\begingroup\$

Based on the length of my original answer, the nature of my new advice and this guidance, I'm submitting another answer here.

You need to have a fundamental re-think about the nature of your data. This is not C-specific; the same thing would apply in Python (or whatever other language you use). You're very often carrying around display information (the letter 'X' and 'O', for instance, or coordinates on the screen) and using those to affect your logic. Your logical data should be pure, in the sense that:

  • Your logic should not have anything to do with screen coordinates
  • Your logic should not ever refer to 'X' and 'O', but rather to player numbers.
  • You should never be reading the contents of the screen, or any display buffers, back into the logic to decide what to do next.

You have another habit - hard-coding things that should be computed. This especially applies to manipulating coordinates of the game grid. The contents of the game grid should not be 'X' and 'O', but rather an enum with three values - blank, player1, or player2. The game grid should be represented as a two-dimensional, and not one-dimensional, array. Manipulating this grid should be done with two-dimensional coordinates, and not a one-dimensional "flattened" index. Rewriting your code like this will make it easier to write loops and functions, and significantly DRY up your code.

\$\endgroup\$
  • \$\begingroup\$ "You should never be reading the contents of the screen, or any display buffers, back into the logic to decide what to do next." Can you expand on this? The nature of ncurses makes it very easy to conflate the two compared to, say, Pygame. \$\endgroup\$ – some_guy632 Dec 31 '18 at 0:06
  • \$\begingroup\$ @some_guy632 Let's discuss this in the same chat - chat.stackexchange.com/rooms/87664/… \$\endgroup\$ – Reinderien Dec 31 '18 at 0:15

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.