Tic Tac Toe Bugs/Optimization

[Pegasus1]

Hello, I am trying to produce a Tic Tac Toe problem that puts the user against the computer. This is the (probably inefficient) code that I have right now. There are two significant problems:

1. When I input field '7' it automatically says, "You win."
2. The Mersenne Twister generator that is supposed to generate a random number between 1 and 9 does not seem to work properly. It seems like the only number that it produces is the number 6. What is going on?

Please let me know what do vis-a-vis the above, and also if you have any optimization suggestions. Thank you in advance!

#include <iostream>
#include <limits>
#include <random>
#include <time.h>

using namespace std;

char grid[3][3] = {'1', '2', '3', '4', '5', '6', '7', '8', '9'};
char player = 'X';

//The following are prototypes of functions.

void Draw();

void PlayerInput();
void AIinput();
char HowToWin();
void CheckWin(int count);

int main()
{
    mt19937 mt_rand(time(0));

    int count = 0;
    cout << "Welcome to the Tic Tac Toe Game! Press enter to begin!" << endl; cout << endl;
    cin.get();

    Draw();
    while (count < 9)
    {
        ++count;
        PlayerInput();Draw();
        CheckWin(count);
        AIinput(); Draw();
        CheckWin(count);
    }
    if(count == 9)
    {
        cout << endl; cout << "Game over!" << endl;
    }
    system("Pause");
    return 0;
}


void Draw()
{
    system("CLS");                        // clears the screen

    for(int r = 0; r < 3 ; ++r)
    {
       cout << "-----------"<< endl;
       for(int c = 0; c < 3; ++c)
       {
           cout << grid[r][c] << " | ";
       }
       cout << endl;
    }
}
void GeneralInput(int n)
{
    if(n == 1)
    {
        if(grid[0][0] == '1')
        {
            grid[0][0] = player;
        }
        else
        {
            if(player = 'X')
            {
                cout << "This field has already been taken; please try again!" << endl;
                PlayerInput();
            }
            else if (player = 'O')
            {
                AIinput();
            }
        }
    }
    else if(n == 2)
    {
        if(grid[0][1] == '2')
        {
            grid[0][1] = player;
        }
        else
        {
            if(player = 'X')
            {
                cout << "This field has already been taken; please try again!" << endl;
                PlayerInput();
            }
            else if(player = 'O')
            {
                AIinput();
            }
        }
    }
    else if(n == 3)
    {
        if(grid[0][2] == '3')
        {
            grid[0][2] = player;
        }
        else
        {
            if(player = 'X')
            {
                cout << "This field has already been taken; please try again!" << endl;
                PlayerInput();
            }
            else if(player = 'O')
            {
                AIinput();
            }
        }
    }
    else if(n == 4)
    {
        if(grid[1][0] == '4')
        {
            grid[1][0] = player;
        }
        else
        {
            if(player = 'X')
            {
                cout << "This field has already been taken; please try again!" << endl;
                PlayerInput();
            }
            else if(player = 'O')
            {
                AIinput();
            }
        }
    }
    else if(n == 5)
    {
        if(grid[1][1] == '5')
        {
            grid[1][1] = player;
        }
        else
        {
            if(player = 'X')
            {
                cout << "This field has already been taken; please try again!" << endl;
                PlayerInput();
            }
            else if(player = 'O')
            {
                AIinput();
            }
        }
    }
    else if(n == 6)
    {
        if(grid[1][2] == '6')
        {
            grid[1][2] = player;
        }
        else
        {
            if(player = 'X')
            {
                cout << "This field has already been taken; please try again!" << endl;
                PlayerInput();
            }
            else if(player = 'O')
            {
                AIinput();
            }
        }
    }
    else if(n == 7)
    {
        if(grid[2][0] == '7')
        {
           grid[2][0] = player;
        }
        else
        {
            if(player = 'X')
            {
                cout << "This field has already been taken; please try again!" << endl;
                PlayerInput();
            }
            else if(player = 'O')
            {
                AIinput();
            }
        }
    }
    else if(n == 8)
    {
        if(grid[2][1] == '8')
        {
            grid[2][1] = player;
        }
        else
        {
            if(player = 'X')
            {
                cout << "This field has already been taken; please try again!" << endl;
                PlayerInput();
            }
            else if(player = 'O')
            {
                AIinput();
            }
        }
    }
    else if(n == 9)
    {
        if(grid[2][2] == '9')
        {
            grid[2][2] = player;
        }
        else
        {
            if(player = 'X')
            {
                cout << "This field has already been taken; please try again!" << endl;
                PlayerInput();
            }
            else if(player = 'O')
            {
                AIinput();
            }
        }
    }
}
void PlayerInput()
{
    player = 'X';
    cout << endl;

    cout << "Input the number of the field you would like to fill." << endl;

    int n;
    cin >> n;

    while(!cin || n < 1 || n > 9)                                       // disallows inputs that are not integers between 1 and 9.
    {
        cout << "Please enter an integer value between 1 and 9." << endl;
        cin.clear();
        cin.ignore(numeric_limits <streamsize>::max(), '\n');
        cout << "Input the number of the field you would like to fill." << endl;
        cin >> n;
    }
    GeneralInput(n);
}
void AIinput()
{
    player = 'O';

    random_device rd;                                                           // lines 258-261 used to generate a random number between x and y
    mt19937 gen(rd());
    uniform_real_distribution<> dis(1, 9);
    int n = dis(gen);

    GeneralInput(n);
}

char HowToWin()
{
    // Checking to see if we have three 'X's in a row

    if(grid[0][0] == 'X' && grid[0][1] == 'X' && grid[0][2] == 'X')
    {
        return 'X';
    }
    if(grid[1][0] == 'X' && grid[1][1] == 'X' && grid[1][2] == 'X')
    {
        return 'X';
    }
    if(grid[2][0] == 'X' && grid[2][1] == 'X' && grid[2][2] == 'X')
    {
        return 'X';
    }

    //Checking to see if we have three 'X's in a column

    if(grid[0][0] == 'X' && grid[1][0] == 'X' && grid[2][0] == 'X')
    {
        return 'X';
    }
    if(grid[0][1] == 'X' && grid[1][1] == 'X' && grid[2][1] == 'X')
    {
        return 'X';
    }
    if(grid[0][2] == 'X' && grid[1][2] == 'X' && grid[2][2] == 'X')
    {
        return 'X';
    }

    //Checking to see if we have three 'X's in a diagonal

    if(grid[0][0] == 'X' && grid[1][1] == 'X' && grid[2][2] == 'X')
    {
        return 'X';
    }
    if(grid[2][0] == 'X' && grid[1][1] == 'X' && grid[0][2] == 'X')
    {
        return 'X';
    }

    //Checking to see if we have three 'O's in a row

    if(grid[0][0] == 'O' && grid[0][1] == 'O' && grid[0][2] == 'O')
    {
        return 'O';
    }
    if(grid[1][0] == 'O' && grid[1][1] == 'O' && grid[1][2] == 'O')
    {
        return 'O';
    }
    if(grid[2][0] == 'O' && grid[2][1] == 'O' && grid[2][2] == 'O')
    {
        return 'O';
    }

    //Checking to see if we have three 'O's in a column

    if(grid[0][0] == 'O' && grid[1][0] == 'O' && grid[2][0] == 'O')
    {
        return 'O';
    }
    if(grid[0][1] == 'O' && grid[1][1] == 'O' && grid[2][1] == 'O')
    {
        return 'O';
    }
    if(grid[0][2] == 'O' && grid[1][2] == 'O' && grid[2][2] == 'O')
    {
        return 'O';
    }

    //Checking to see if we have three 'O's in a diagonal

    if(grid[0][0] == 'O' && grid[1][1] == 'O' && grid[2][2] == 'O')
    {
        return 'O';
    }
    if(grid[2][0] == 'O' && grid[1][1] == 'O' && grid[0][2] == 'O')
    {
        return 'O';
    }
}
void CheckWin(int count)
{
        if (HowToWin() == 'X')
        {
            cout << "You win!" << endl;
        }
        else if (HowToWin() == '0')
        {
            cout << "The computer wins!" << endl;
        }
        else if (HowToWin() != 'X' && HowToWin() != 'X' && count == 9)
        {
            cout << "You drew with the computer!" << endl;
        }
}

[Pegasus1]

wrt 1: The problem seems to be somewhere in HowToWin(), but I don't see exactly what.
wrt 2: The random number generator is found in AIinput();

[Pegasus1]

UPDATE: I fixed Problem 1 by adding "return '/';" right before the termination of HowtoWin(). Very strange!

[Pegasus1]

Fixed the random number generator problem!

[Chervil]

There are very many compiler warnings issued.
In many places, e.g. line 70, you have the assignment operator = rather than the comparison operator ==

if (player = 'X')

should be

if (player == 'X')

[TheIdeasMan]

Hi,

Just an idea not related to your questions :+)

When one represents a 2d array as a 1d array, there is a formula: row * NumCols + col. So this works when rows and cols are numbered in the normal C/C++ fashion: 0 to 2. In the 1d array runs from 0 to 8. You could adjust your numbering system to suit this, or subtract 1 from the user input. So one can do the reverse of this - work out the row and col given a number from 0 to 8. Use integer division and the modulus. And the point of all this is so you can have one function instead of 9 repetitions of lines 62 to 80 :+)

To check for a win, consider maintaining a sum of each column, row and both diagonals. If the sum of a row equals 3 * the value for an X then that is a win. So then you could have:

// check rows
if (SumRow[0] == 3 || SumRow[1] == 3 || SumRow[2] == 3) {
    WinX = true;
   return;
}

// check columns and diagonals  

Checking for a win only needs to happens after 5 turns (Player 1 could win on third turn).


I might not reply for awhile - it's late at my end, but hopefully this helps you out a bit :+)

[jlb]

Perhaps you should increase your compiler warning level, or fix the warnings your compiler is generating?

In function 'void GeneralInput(int)': 70:28: warning: suggest parentheses around assignment used as truth value [-Wparentheses] 75:34: warning: suggest parentheses around assignment used as truth value [-Wparentheses] 89:28: warning: suggest parentheses around assignment used as truth value [-Wparentheses] 94:33: warning: suggest parentheses around assignment used as truth value [-Wparentheses] 108:28: warning: suggest parentheses around assignment used as truth value [-Wparentheses] 113:33: warning: suggest parentheses around assignment used as truth value [-Wparentheses] 127:28: warning: suggest parentheses around assignment used as truth value [-Wparentheses] 132:33: warning: suggest parentheses around assignment used as truth value [-Wparentheses] 146:28: warning: suggest parentheses around assignment used as truth value [-Wparentheses] 151:33: warning: suggest parentheses around assignment used as truth value [-Wparentheses] 165:28: warning: suggest parentheses around assignment used as truth value [-Wparentheses] 170:33: warning: suggest parentheses around assignment used as truth value [-Wparentheses] 184:28: warning: suggest parentheses around assignment used as truth value [-Wparentheses] 189:33: warning: suggest parentheses around assignment used as truth value [-Wparentheses] 203:28: warning: suggest parentheses around assignment used as truth value [-Wparentheses] 208:33: warning: suggest parentheses around assignment used as truth value [-Wparentheses] 222:28: warning: suggest parentheses around assignment used as truth value [-Wparentheses] 227:33: warning: suggest parentheses around assignment used as truth value [-Wparentheses] In function 'char HowToWin()': 349:1: warning: control reaches end of non-void function [-Wreturn-type]

I fixed Problem 1 by adding "return '/';" right before the termination of HowtoWin(). Very strange!

Why are you returning '/'? That doesn't look like a valid value for the logic in your CheckWin() function. By the way it's not strange, you should always insure that a function that returns a value actually returns a value.

This looks like you have some logic holes in that function, perhaps you should try to simplify the logic. You have what appears to be a lot of duplicate code, you may want to investigate using some loops.

[Pegasus1]

Thanks for the replies everyone. I am currently thinking through them. My code at the moment is as follows:

http://paste.ofcode.org/fKVfEcb2fppKujSf7NPnGY

The "only" problem now (other than obvious optimization issues) is that the Mersenne Twister random number generator in the AIinput() sometimes returns values for fields that are already taken. I tried to get around this in GeneralInput() by looping back to AIinput().

Once again, I very much appreciate all the feedback!

[TheIdeasMan]

Hi,

Just had another thought. If you made a vector of the all sums, with the first 8 for the O's (3 rows, 3 cols, 2 diag), then the next 8 for the X's , then you could use the find algorithm to search for a winning value. That way, to determine a win could be only 1 statement. Note that the values in this vector are only incremented once a O or X is placed.

Also, just to reiterate about compiler warning levels, they are your friend - they tell where problems are, so set the warning level at it's highest (or at least turn on as many warnings as are practicable). From my point of view, if I have warnings, then I am not finished.

The other concept is of code repetition, if you find yourself repeating code, or doing code that is very similar, then there is almost always a better way :+)

Good Luck !!

[Chervil]

The "only" problem now (other than obvious optimization issues) is that the Mersenne Twister random number generator in the AIinput() sometimes returns values for fields that are already taken. I tried to get around this in GeneralInput() by looping back to AIinput().

There is a problem with the way you are generating random numbers.

void AIinput()
{
    player = 'O';

    mt19937 mt_rand(time(0)); // seed the generator
    uniform_int_distribution<> distr(1, 9); // define the range
    int n = (int)distr(mt_rand);

    GeneralInput(n);
}

Above, at line 255, the generator is reseeded using the current time on every call. The generator should be seeded just once at the start of the program, and then subsequent values are generated by the mt19937 itself.

It may not seem to matter much here, because the user input may be sufficiently slow as to allow the time to change on each move. But for example if you were to get the computer to play both X and O sides of the game, the speed of play would be fast enough to cause it to be trapped by the re-seeding problem, every move would be identical - at least the first several million iterations.