Tic Tac Toe Comp Move help

Hey guys

I am doing well making a tic tac toe program and so far I have created the board and am able to place the "O" character as a human move in any of the nine slots. Also, I have a function that checks if the move is legal before it allows this to happen.

The problem comes when I try to ask the computer to make a move. I keeps saying "cannot move to this location" even when the square is empty. All the game does so far is call the computer move then call the player move so I can test to see if its working ok before I go through all the winning moves etc.

Here is the code that organises the computer movement each turn

Code:

---

// function to determine a computer move
int TicTacToe::makeComputerMove ( char m_Board[][ 3 ] )
{
        int square = 0;

        int xCorr = rand() % 8 + 0;

        if (( xCorr < 0 ) || ( xCorr > 8 ))
        {
                return 0;
        }

        int yCorr = rand() % 8 + 0;

        if (( yCorr < 0 ) || ( yCorr > 8 ))
        {
                return 0;
        }

        if (( xCorr == 0 ) && ( yCorr == 0 )) {  m_Board[ xCorr ][ yCorr ]; square = 1; }
        else if (( xCorr == 0 ) && ( yCorr == 1 )) { m_Board[ xCorr ][ yCorr ]; square = 2; }
        else if (( xCorr == 0 ) && ( yCorr == 2 )) { m_Board[ xCorr ][ yCorr ]; square = 3; }
        else if (( xCorr == 1 ) && ( yCorr == 0 )) { m_Board[ xCorr ][ yCorr ]; square = 4; }
        else if (( xCorr == 1 ) && ( yCorr == 1 )) { m_Board[ xCorr ][ yCorr ]; square = 5; }
        else if (( xCorr == 1 ) && ( yCorr == 2 )) { m_Board[ xCorr ][ yCorr ]; square = 6; }
        else if (( xCorr == 2 ) && ( yCorr == 0 )) { m_Board[ xCorr ][ yCorr ]; square = 7; }
        else if (( xCorr == 2 ) && ( yCorr == 1 )) { m_Board[ xCorr ][ yCorr ]; square = 8; }
        else if (( xCorr == 2 ) && ( yCorr == 2 )) { m_Board[ xCorr ][ yCorr ]; square = 9; }

        isLegalComp ( m_Board, xCorr, yCorr, square );

        return 0;
}

---

This function checks if the move is legal

Code:

---

// function to check that the move the computer
// makes is a legal one
int TicTacToe::isLegalComp ( char m_Board [][ 3 ], int &x, int &y, int &square )
{       
        // first row first column
        if  (( square == 1 ) && ( m_SquareTaken[ 0 ] == 0 ))
        {
                m_Board[ x ][ y ] = 'X';
               
                m_SquareTaken[ 0 ]++;
                return 0;
        }

        // first row second column
        if (( square == 2 ) && ( m_SquareTaken[ 1 ] == 0 ))
        {
                m_Board[ x ][ y ] = 'X';       
       
                m_SquareTaken[ 1 ]++;
                return 0;
        }
       
        // first row third column
        if (( square == 3 ) && ( m_SquareTaken[ 2 ] == 0 ))
        {
                m_Board[ x ][ y ] = 'X';
               
                m_SquareTaken[ 2 ]++;
                return 0;
        }
       
        // second row first column
        if (( square == 4 ) && ( m_SquareTaken[ 3 ] == 0 ))
        {
                m_Board[ x ][ y ] = 'X';
               
                m_SquareTaken[ 3 ]++;
                return 0;
        }
       
        // second row second column
        if (( square == 5 ) && ( m_SquareTaken[ 4 ] == 0 ))
        {
                m_Board[ x ][ y ] = 'X';
               
                m_SquareTaken[ 4 ]++;
                return 0;
        }
       
        // second row third column
        if (( square == 6 ) && ( m_SquareTaken[ 5 ] == 0 ))
        {
                m_Board[ x ][ y ] = 'X';
               
                m_SquareTaken[ 5 ]++;
                return 0;
        }
       
        // third row first column
        if (( square == 7 ) && ( m_SquareTaken[ 6 ] == 0 ))
        {
                m_Board[ x ][ y ] = 'X';
               
                m_SquareTaken[ 6 ]++;
                return 0;
        }
       
        // third row second column
        if (( square == 8 ) && ( m_SquareTaken[ 7 ] == 0 ))
        {
                m_Board[ x ][ y ] = 'X';
               
                m_SquareTaken[ 7 ]++;
                return 0;
        }
       
        // third row third column
        if (( square == 9 ) && ( m_SquareTaken[ 8 ] == 0 ))
        {
                m_Board[ x ][ y ] = 'X';

                m_SquareTaken[ 8 ]++;
                return 0;
        }

        // move is illegal
        else
        {
                std::cout << "\nCannot move to this location!\n";
                return 1;
        }

        return 0;
}

---

The human move functions are almost identical to these and they are working perfectly, I do not understand why the computer move is not working.

Any help appreiciated - I know its alot to read but the program is quite large.

Code:

---

        if  (( square == 1 ) && ( m_SquareTaken[ 0 ] == 0 ))
        {
                m_Board[ x ][ y ] = 'X';
               
                m_SquareTaken[ 0 ]++;
                return 0;
        }

---

can be written as:

Code:

---

      if (square >= 1 && square <= 9)
      {
          if (m_SquareTaken[square-1] == 0)
          {
                m_Board[ x ][ y ] = 'X';
                m_SquareTaken[square-1] = 1;
                return 0;
          }
      }
      // otherwise, not a legal move.

---

Have you filled m_Board with zero's?

Edit: Also, I would separate the "make move" (that is putting an X or O in the m_Board cell) and the checking if it's a legal move.

Edit2:

Code:

---

        int xCorr = rand() % 8 + 0;

        if (( xCorr < 0 ) || ( xCorr > 8 ))
        {
                return 0;
        }

        int yCorr = rand() % 8 + 0;

        if (( yCorr < 0 ) || ( yCorr > 8 ))
        {
                return 0;
        }

        if (( xCorr == 0 ) && ( yCorr == 0 )) {  m_Board[ xCorr ][ yCorr ]; square = 1; }
        else if (( xCorr == 0 ) && ( yCorr == 1 )) { m_Board[ xCorr ][ yCorr ]; square = 2; }
        else if (( xCorr == 0 ) && ( yCorr == 2 )) { m_Board[ xCorr ][ yCorr ]; square = 3; }
        else if (( xCorr == 1 ) && ( yCorr == 0 )) { m_Board[ xCorr ][ yCorr ]; square = 4; }
        else if (( xCorr == 1 ) && ( yCorr == 1 )) { m_Board[ xCorr ][ yCorr ]; square = 5; }
        else if (( xCorr == 1 ) && ( yCorr == 2 )) { m_Board[ xCorr ][ yCorr ]; square = 6; }
        else if (( xCorr == 2 ) && ( yCorr == 0 )) { m_Board[ xCorr ][ yCorr ]; square = 7; }
        else if (( xCorr == 2 ) && ( yCorr == 1 )) { m_Board[ xCorr ][ yCorr ]; square = 8; }
        else if (( xCorr == 2 ) && ( yCorr == 2 )) { m_Board[ xCorr ][ yCorr ]; square = 9; }
        else // invalid xCorr or yCorr.

---

The blue section is meaningless - you should get a warning "code has no effect" or some such for that bit.

The range you generate xCorr and yCorr [should that be xCoord, yCoord?] is much large than what you need, so you may end up with "unknown" value of the square.
(and square = (xCorr * 3 + yCorr)+1, so you don't need so many if-statements, really).

--
Mats

You're generating two numbers between zero and seven, inclusive, and using that to pick the spot on the board. I can't understand why you'd be doing that. It looks like you mean to be picking numbers between zero and two, inclusive.

You're then checking if the random values are less than zero (which it can never be) or greater than eight (which it can never be).

I would suggest simply generating one random number between zero and eight, using rand() % 9, and square is then one plus that, xCorr is that divided by 3, and yCorr is simply that mod 3.
That removes every single if-statement from makeComputerMove.

Note that if it returns the same thing in every case then it may as well just return void.

Perhaps you intend to use a do .. while loop that repeats while the move is not legal? At the moment you are ignoring the return value.

There isn't any advantage to passing x and y by reference. Passing by value for built-in types is preferred unless you have a need to do otherwise.

If you want computer to pick a completely random move you shouldn't need anything more than this:

Code:

---

void pick_random_move(char board[][3])
{
    int x, y;
    do {
        x = rand() % 3;
        y = rand() % 3;
    } while (board[x][y] != '\0');
    board[x][y] = 'X';
}

---

That assumes that the board is initialized to all zeros, and that there is at least one unmarked square.

I also don't see much point in storing whether a square is taken or not in a separate array (especially since it is indexed differently). If a square is taken, you'd have an 'X' or 'O' there, otherwise it would be zero.

Neither do I understand why your functions have an int return type and why they all return 0. Only main requires this "meaningless" return value which might be used by the process that started your program.

Thanks alot for the advice guys sorry for the late reply.

Great tips and I never knew I was doing it so badly either, the sense you spoke in the replies made perfect logic and I should be able to solve the problem now.

I also changed all the int functions to void, where needed, oh and as a side note, its ok for to mave all my functions bar one private in the class? I mean I only do that cause there is no need for them to be public, since only the class is using them.

[QUOTE=swgh;789988I also changed all the int functions to void, where needed, oh and as a side note, its ok for to mave all my functions bar one private in the class? I mean I only do that cause there is no need for them to be public, since only the class is using them.[/QUOTE]

That is perfectly fine - if there is no use of a function outside of the class, then it can be made private.

--
Mats