Tic Tac Toe in C w/ ncurses Revision
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 immitate 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;
}
beginner c tic-tac-toe
New contributor
add a comment |
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 immitate 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;
}
beginner c tic-tac-toe
New contributor
add a comment |
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 immitate 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;
}
beginner c tic-tac-toe
New contributor
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 immitate 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;
}
beginner c tic-tac-toe
beginner c tic-tac-toe
New contributor
New contributor
New contributor
asked 3 hours ago
some_guy632
865
865
New contributor
New contributor
add a comment |
add a comment |
3 Answers
3
active
oldest
votes
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 case
s in check_for_winning_move
and check_for_block
.
"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.
– some_guy632
1 hour ago
1
Re. static functions - If you don't understand this concept, do some reading here: en.wikipedia.org/wiki/Static_(keyword)#Static_function and here: geeksforgeeks.org/what-are-static-functions-in-c
– Reinderien
1 hour ago
1
"Is it bad practice to use 1 and 0?" Here, yes. If the language gives you a header for standard booleans, use it. "What about using #defines?" If you mean defining your owntrue
andfalse
, don't do this -stdbool.h
does that for you.
– Reinderien
1 hour ago
1
"return a variety of possible numbers if true" - This is a different creature. This isn't bad practice, but the way you're doing it is bad practice. You can have internal signalling integers, but you shouldtypedef
anenum
for this purpose so that you can create named constants to indicate what the function return value actually means.
– Reinderien
1 hour ago
2
Let us continue this discussion in chat.
– Reinderien
1 hour ago
|
show 2 more comments
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, #define
s, struct
s 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 #include
s 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.
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!
– some_guy632
22 mins ago
1
You have to forward declare because your main uses those functions. without a forward declaration your code would not compile. but if you put themain()
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 includingstdlib.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 hassize_t
which is ubiquitous in C and C++) And good inclination to get your includes reviewed.
– bruglesco
16 mins ago
add a comment |
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.
For my part, I would probably loop over the given indices and avoid all the repetition involved with comparing different indices of playable_spaces
to the same letter. But this is probably less efficient, and you would also need to store the winning line indexes in their own arrays. If there's a better way to handle this, hopefully someone will edit it in.
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?
– some_guy632
1 hour ago
1
@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.
– Graham
1 hour ago
add a comment |
Your Answer
StackExchange.ifUsing("editor", function () {
return StackExchange.using("mathjaxEditing", function () {
StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix) {
StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
});
});
}, "mathjax-editing");
StackExchange.ifUsing("editor", function () {
StackExchange.using("externalEditor", function () {
StackExchange.using("snippets", function () {
StackExchange.snippets.init();
});
});
}, "code-snippets");
StackExchange.ready(function() {
var channelOptions = {
tags: "".split(" "),
id: "196"
};
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function() {
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled) {
StackExchange.using("snippets", function() {
createEditor();
});
}
else {
createEditor();
}
});
function createEditor() {
StackExchange.prepareEditor({
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader: {
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
},
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
});
}
});
some_guy632 is a new contributor. Be nice, and check out our Code of Conduct.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f210577%2ftic-tac-toe-in-c-w-ncurses-revision%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
3 Answers
3
active
oldest
votes
3 Answers
3
active
oldest
votes
active
oldest
votes
active
oldest
votes
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 case
s in check_for_winning_move
and check_for_block
.
"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.
– some_guy632
1 hour ago
1
Re. static functions - If you don't understand this concept, do some reading here: en.wikipedia.org/wiki/Static_(keyword)#Static_function and here: geeksforgeeks.org/what-are-static-functions-in-c
– Reinderien
1 hour ago
1
"Is it bad practice to use 1 and 0?" Here, yes. If the language gives you a header for standard booleans, use it. "What about using #defines?" If you mean defining your owntrue
andfalse
, don't do this -stdbool.h
does that for you.
– Reinderien
1 hour ago
1
"return a variety of possible numbers if true" - This is a different creature. This isn't bad practice, but the way you're doing it is bad practice. You can have internal signalling integers, but you shouldtypedef
anenum
for this purpose so that you can create named constants to indicate what the function return value actually means.
– Reinderien
1 hour ago
2
Let us continue this discussion in chat.
– Reinderien
1 hour ago
|
show 2 more comments
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 case
s in check_for_winning_move
and check_for_block
.
"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.
– some_guy632
1 hour ago
1
Re. static functions - If you don't understand this concept, do some reading here: en.wikipedia.org/wiki/Static_(keyword)#Static_function and here: geeksforgeeks.org/what-are-static-functions-in-c
– Reinderien
1 hour ago
1
"Is it bad practice to use 1 and 0?" Here, yes. If the language gives you a header for standard booleans, use it. "What about using #defines?" If you mean defining your owntrue
andfalse
, don't do this -stdbool.h
does that for you.
– Reinderien
1 hour ago
1
"return a variety of possible numbers if true" - This is a different creature. This isn't bad practice, but the way you're doing it is bad practice. You can have internal signalling integers, but you shouldtypedef
anenum
for this purpose so that you can create named constants to indicate what the function return value actually means.
– Reinderien
1 hour ago
2
Let us continue this discussion in chat.
– Reinderien
1 hour ago
|
show 2 more comments
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 case
s in check_for_winning_move
and check_for_block
.
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 case
s in check_for_winning_move
and check_for_block
.
edited 48 mins ago
answered 1 hour ago
Reinderien
3,189720
3,189720
"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.
– some_guy632
1 hour ago
1
Re. static functions - If you don't understand this concept, do some reading here: en.wikipedia.org/wiki/Static_(keyword)#Static_function and here: geeksforgeeks.org/what-are-static-functions-in-c
– Reinderien
1 hour ago
1
"Is it bad practice to use 1 and 0?" Here, yes. If the language gives you a header for standard booleans, use it. "What about using #defines?" If you mean defining your owntrue
andfalse
, don't do this -stdbool.h
does that for you.
– Reinderien
1 hour ago
1
"return a variety of possible numbers if true" - This is a different creature. This isn't bad practice, but the way you're doing it is bad practice. You can have internal signalling integers, but you shouldtypedef
anenum
for this purpose so that you can create named constants to indicate what the function return value actually means.
– Reinderien
1 hour ago
2
Let us continue this discussion in chat.
– Reinderien
1 hour ago
|
show 2 more comments
"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.
– some_guy632
1 hour ago
1
Re. static functions - If you don't understand this concept, do some reading here: en.wikipedia.org/wiki/Static_(keyword)#Static_function and here: geeksforgeeks.org/what-are-static-functions-in-c
– Reinderien
1 hour ago
1
"Is it bad practice to use 1 and 0?" Here, yes. If the language gives you a header for standard booleans, use it. "What about using #defines?" If you mean defining your owntrue
andfalse
, don't do this -stdbool.h
does that for you.
– Reinderien
1 hour ago
1
"return a variety of possible numbers if true" - This is a different creature. This isn't bad practice, but the way you're doing it is bad practice. You can have internal signalling integers, but you shouldtypedef
anenum
for this purpose so that you can create named constants to indicate what the function return value actually means.
– Reinderien
1 hour ago
2
Let us continue this discussion in chat.
– Reinderien
1 hour ago
"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.
– some_guy632
1 hour ago
"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.
– some_guy632
1 hour ago
1
1
Re. static functions - If you don't understand this concept, do some reading here: en.wikipedia.org/wiki/Static_(keyword)#Static_function and here: geeksforgeeks.org/what-are-static-functions-in-c
– Reinderien
1 hour ago
Re. static functions - If you don't understand this concept, do some reading here: en.wikipedia.org/wiki/Static_(keyword)#Static_function and here: geeksforgeeks.org/what-are-static-functions-in-c
– Reinderien
1 hour ago
1
1
"Is it bad practice to use 1 and 0?" Here, yes. If the language gives you a header for standard booleans, use it. "What about using #defines?" If you mean defining your own
true
and false
, don't do this - stdbool.h
does that for you.– Reinderien
1 hour ago
"Is it bad practice to use 1 and 0?" Here, yes. If the language gives you a header for standard booleans, use it. "What about using #defines?" If you mean defining your own
true
and false
, don't do this - stdbool.h
does that for you.– Reinderien
1 hour ago
1
1
"return a variety of possible numbers if true" - This is a different creature. This isn't bad practice, but the way you're doing it is bad practice. You can have internal signalling integers, but you should
typedef
an enum
for this purpose so that you can create named constants to indicate what the function return value actually means.– Reinderien
1 hour ago
"return a variety of possible numbers if true" - This is a different creature. This isn't bad practice, but the way you're doing it is bad practice. You can have internal signalling integers, but you should
typedef
an enum
for this purpose so that you can create named constants to indicate what the function return value actually means.– Reinderien
1 hour ago
2
2
Let us continue this discussion in chat.
– Reinderien
1 hour ago
Let us continue this discussion in chat.
– Reinderien
1 hour ago
|
show 2 more comments
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, #define
s, struct
s 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 #include
s 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.
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!
– some_guy632
22 mins ago
1
You have to forward declare because your main uses those functions. without a forward declaration your code would not compile. but if you put themain()
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 includingstdlib.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 hassize_t
which is ubiquitous in C and C++) And good inclination to get your includes reviewed.
– bruglesco
16 mins ago
add a comment |
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, #define
s, struct
s 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 #include
s 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.
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!
– some_guy632
22 mins ago
1
You have to forward declare because your main uses those functions. without a forward declaration your code would not compile. but if you put themain()
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 includingstdlib.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 hassize_t
which is ubiquitous in C and C++) And good inclination to get your includes reviewed.
– bruglesco
16 mins ago
add a comment |
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, #define
s, struct
s 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 #include
s 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.
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, #define
s, struct
s 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 #include
s 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.
edited 14 mins ago
answered 33 mins ago
bruglesco
1,3432521
1,3432521
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!
– some_guy632
22 mins ago
1
You have to forward declare because your main uses those functions. without a forward declaration your code would not compile. but if you put themain()
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 includingstdlib.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 hassize_t
which is ubiquitous in C and C++) And good inclination to get your includes reviewed.
– bruglesco
16 mins ago
add a comment |
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!
– some_guy632
22 mins ago
1
You have to forward declare because your main uses those functions. without a forward declaration your code would not compile. but if you put themain()
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 includingstdlib.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 hassize_t
which is ubiquitous in C and C++) And good inclination to get your includes reviewed.
– bruglesco
16 mins ago
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!
– some_guy632
22 mins ago
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!
– some_guy632
22 mins ago
1
1
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.– bruglesco
16 mins ago
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.– bruglesco
16 mins ago
add a comment |
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.
For my part, I would probably loop over the given indices and avoid all the repetition involved with comparing different indices of playable_spaces
to the same letter. But this is probably less efficient, and you would also need to store the winning line indexes in their own arrays. If there's a better way to handle this, hopefully someone will edit it in.
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?
– some_guy632
1 hour ago
1
@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.
– Graham
1 hour ago
add a comment |
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.
For my part, I would probably loop over the given indices and avoid all the repetition involved with comparing different indices of playable_spaces
to the same letter. But this is probably less efficient, and you would also need to store the winning line indexes in their own arrays. If there's a better way to handle this, hopefully someone will edit it in.
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?
– some_guy632
1 hour ago
1
@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.
– Graham
1 hour ago
add a comment |
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.
For my part, I would probably loop over the given indices and avoid all the repetition involved with comparing different indices of playable_spaces
to the same letter. But this is probably less efficient, and you would also need to store the winning line indexes in their own arrays. If there's a better way to handle this, hopefully someone will edit it in.
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.
For my part, I would probably loop over the given indices and avoid all the repetition involved with comparing different indices of playable_spaces
to the same letter. But this is probably less efficient, and you would also need to store the winning line indexes in their own arrays. If there's a better way to handle this, hopefully someone will edit it in.
answered 1 hour ago
community wiki
Graham
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?
– some_guy632
1 hour ago
1
@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.
– Graham
1 hour ago
add a comment |
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?
– some_guy632
1 hour ago
1
@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.
– Graham
1 hour ago
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?
– some_guy632
1 hour ago
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?
– some_guy632
1 hour ago
1
1
@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.
– Graham
1 hour ago
@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.
– Graham
1 hour ago
add a comment |
some_guy632 is a new contributor. Be nice, and check out our Code of Conduct.
some_guy632 is a new contributor. Be nice, and check out our Code of Conduct.
some_guy632 is a new contributor. Be nice, and check out our Code of Conduct.
some_guy632 is a new contributor. Be nice, and check out our Code of Conduct.
Thanks for contributing an answer to Code Review Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
Use MathJax to format equations. MathJax reference.
To learn more, see our tips on writing great answers.
Some of your past answers have not been well-received, and you're in danger of being blocked from answering.
Please pay close attention to the following guidance:
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f210577%2ftic-tac-toe-in-c-w-ncurses-revision%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown