Negamax C++ Implemention Give Wrong Result

Question

NOTE: Please comment if you think this post looks to you not having adequate details e.g. codes, results and other stuff; I will edit the post accordingly.

NOTE 2: I wrote this program by hand myself.

I have a negamax implementation the result of which looked very wrong to me i have tried many ways of debugging it but I still can't seem to get the crux of it.

First of all this is a negamax implementation for Tic Tac Toe, which has board of 3X3.

following codes are the full set in order to replicate the error I had wit this algorithm. please comment below if I missed anything.

An Example could be doing this main:

int main {

Board board;
board.startGameNage(0,0);


}

I would expect the game ended in a draw because this is computer (perfect player) vs computer (perfect player), however,Using the following set of functions I got a game ending like below:

current max move is: 0,0, current score is: -inf current max move is: 0,2, current score is: 3 current max move is: 0,1, current score is: -3 current max move is: 1,1, current score is: 3 current max move is: 2,0, current score is: -3 current max move is: 1,2, current score is: 3 current max move is: 2,1, current score is: -3 current max move is: 1,0, current score is: 3 current max move is: 1,0, current score is: -3

X X O
X O O
X X ---

the " - " there means no move is made in that cell, which looked obviously wrong.

I implemented my minimax first and this negamax was in a way evolving based on my minimax implementation, which might be the reason that I can't see my error.

I got that minimax makes moves from 2 player's perspective and evaluate scores the same as well, whereas negamax make moves from 2 player's perspective but evaluate score only from current player's perspective.

I guess this is the bit that confused me. I can't seem to see how my implementation went wrong here.

I start my game by the following function in main:

// in  main I will just give the following function a coordinate, e.g. (0,0)

void Board::startGameNega(const int & row, const int & col){

Move move(row, col);
int player = 1;
for (int depth = 0; depth < 9; depth++){
    applyMoveNega(move, player);
    Move current_move = move;
    move = negaMax(depth, player, move);
    player = -player;
    cout << "current Max move is: " << current_move.getRow()
        << " , "
        << current_move.getCol()
        << ", Current score is: "
        << current_move.getScore() << endl;
}
print(); // print the end of game board
}

here is the board.hpp:

#define LENGTH 3
#define WIDTH 3
#define CROSS 1
#define NOUGHT -1


#include <iostream>
#include <vector>
#include <array>
#include <map> 
#include "Move.hpp"

using namespace std;

#pragma once

typedef vector<Move> Moves;

struct Board {

// constructors;
Board(int width, int length) :m_width(width), m_length(width){};
Board(){};

// destructor;
~Board(){};

// negamax;
Move negaMax(const int & depth, const int & player, const Move & initialMove);
void startGameNega(const int & row, const int & col);
void applyMoveNega(const Move & move, const int & player);
bool isWon(const int & player);
bool isGameComplete();
int evaluateGameStateNega(const int & depth, const int & player);

// share;
int getOpponent(const int & player);
void deleteMove(const Move & move);
void deleteMoves(const Move & initialMove);

// utilities;
static int defaultBoard[WIDTH][LENGTH];
int getWidth() const { return m_width; }
int getLength() const { return m_length; }
void setWidth(int width){ m_width = width; }
void setLength(int length){ m_length = length; }
void print();
int getCurrentPlayer();

private:

    int m_width;
    int m_length;
    enum isWin{ yes, no, draw };
    int result;
    int m_player;
};

some key elements listed here:

print:

void Board::print(){
for (int i = 0; i < WIDTH; i++) {
    for (int j = 0; j < LENGTH; j++) {
        switch (defaultBoard[i][j]) {
        case CROSS:
            cout << "X";
            break;
        case NOUGHT:
            cout << "O";
            break;
        default:
            cout << "-";
            break;
        }
        cout << " ";
    }
    cout << endl;
}
}

generateMoves:

Moves Board::generateMoves(const int &rowIndex, const int &colIndex){

Moves Moves;

if (defaultBoard){

    for (int i = 0; i < WIDTH; i++)
    {
        for (int j = 0; j < LENGTH; j++)
        {
            if (i == rowIndex && j == colIndex)
            {
                continue;
            }
            else if (defaultBoard[i][j] == 1 || defaultBoard[i][j] == 4)
            {
                continue;
            }
            else if (defaultBoard[i][j] == 0)
            {
                Move move(i, j);
                Moves.push_back(move);
            }

        }
    }

}

return Moves;

}

applyMovesNega:

void Board::applyMoveNega(const Move & move, const int & player){

if (player == 1){
    defaultBoard[move.getRow()][move.getCol()] = CROSS;
}
else if (player == -1)
{
    defaultBoard[move.getRow()][move.getCol()] = NOUGHT;
}
}

isGameComplete:

bool Board::isGameComplete(){

if (defaultBoard[0][0] == defaultBoard[0][1] && defaultBoard[0][1] == defaultBoard[0][2] && defaultBoard[0][0] != 0 ||
    defaultBoard[1][0] == defaultBoard[1][1] && defaultBoard[1][1] == defaultBoard[1][2] && defaultBoard[1][0] != 0 ||
    defaultBoard[2][0] == defaultBoard[2][1] && defaultBoard[2][1] == defaultBoard[2][2] && defaultBoard[2][0] != 0 ||
    defaultBoard[0][0] == defaultBoard[1][0] && defaultBoard[1][0] == defaultBoard[2][0] && defaultBoard[0][0] != 0 ||
    defaultBoard[0][1] == defaultBoard[1][1] && defaultBoard[1][1] == defaultBoard[2][1] && defaultBoard[0][1] != 0 ||
    defaultBoard[0][2] == defaultBoard[1][2] && defaultBoard[1][2] == defaultBoard[2][2] && defaultBoard[0][2] != 0 ||
    defaultBoard[0][0] == defaultBoard[1][1] && defaultBoard[1][1] == defaultBoard[2][2] && defaultBoard[0][0] != 0 ||
    defaultBoard[2][0] == defaultBoard[1][1] && defaultBoard[1][1] == defaultBoard[0][2] && defaultBoard[2][0] != 0){

    return true;
}

return false;

}

evaluate the score:

int Board::evaluateGameStateNega(const int & depth, const int & player){

int new_score;
isWon(player);

if (result == isWin::yes)
    new_score = 10 - depth;
else if (result == isWin::no)
    new_score = depth - 10;
else
    new_score = 0;

return new_score;
}

deleteMove:

void Board::deleteMove(const Move & move){


defaultBoard[move.getRow()][move.getCol()] = 0;}

here's the move.hpp:

struct Move{

Move(){};
Move(const int & index) :m_rowIndex(index / 3),m_colIndex(index % 3){};
Move(const int & row, const int & col) :m_rowIndex(row), m_colIndex(col){};
Move(const int & row, const int & col, const int & score):m_rowIndex(row), m_colIndex(col), m_score(score){};

~Move(){};

//member functions;
int getRow() const { return m_rowIndex; };
int getCol() const { return m_colIndex; };
void setRow(const int & row){ m_rowIndex = row; };
void setCol(const int & col){ m_colIndex = col; };
void setScore(const int & score){ m_score = score; };
int getScore() const { return m_score; }

private:

    int m_rowIndex;
    int m_colIndex;
    int m_score;

    };

This is the actual NegaMax Function:

Move Board::negaMax(const int & depth, const int & curPlayer, const Move & initialMove){

int row = initialMove.getRow();
int col = initialMove.getCol();
int _depth = depth;
int _curplayer = curPlayer;
Moves moves = generateMoves(row, col);

Move bestMove;
Move proposedNextMove;

//change to isGameComplete as of 15/10;
if (_depth == 8 || isGameComplete())
{
    int score = evaluateGameStateNega(_depth, _curplayer);
    bestMove.setScore(score);
    bestMove.setRow(initialMove.getRow());
    bestMove.setCol(initialMove.getCol());
}
else{

    _depth += 1;
    int bestScore = -1000;

    for (auto move : moves){

        applyMoveNega(move, -_curplayer);
        proposedNextMove = negaMax(_depth, -_curplayer, move);
        int tScore = -proposedNextMove.getScore();
        proposedNextMove.setScore(tScore);

        if (proposedNextMove.getScore() > bestScore){
            bestScore = proposedNextMove.getScore();
            bestMove.setScore(bestScore);
            bestMove.setRow(move.getRow());
            bestMove.setCol(move.getCol());
        }

        deleteMove(move);
    }

}

    return bestMove;

}

I evaluate the game state using following Function:

bool Board::isWon(const int & player){


if (defaultBoard[0][0] == defaultBoard[0][1] && defaultBoard[0][1] == defaultBoard[0][2] && defaultBoard[0][0] == player ||
    defaultBoard[1][0] == defaultBoard[1][1] && defaultBoard[1][1] == defaultBoard[1][2] && defaultBoard[1][0] == player ||
    defaultBoard[2][0] == defaultBoard[2][1] && defaultBoard[2][1] == defaultBoard[2][2] && defaultBoard[2][0] == player ||
    defaultBoard[0][0] == defaultBoard[1][0] && defaultBoard[1][0] == defaultBoard[2][0] && defaultBoard[0][0] == player ||
    defaultBoard[0][1] == defaultBoard[1][1] && defaultBoard[1][1] == defaultBoard[2][1] && defaultBoard[0][1] == player ||
    defaultBoard[0][2] == defaultBoard[1][2] && defaultBoard[1][2] == defaultBoard[2][2] && defaultBoard[0][2] == player ||
    defaultBoard[0][0] == defaultBoard[1][1] && defaultBoard[1][1] == defaultBoard[2][2] && defaultBoard[0][0] == player ||
    defaultBoard[2][0] == defaultBoard[1][1] && defaultBoard[1][1] == defaultBoard[0][2] && defaultBoard[2][0] == player){

    result = isWin::yes;
    return true;
}
else if (defaultBoard[0][0] == defaultBoard[0][1] && defaultBoard[0][1] == defaultBoard[0][2] && defaultBoard[0][0] == -player ||
    defaultBoard[1][0] == defaultBoard[1][1] && defaultBoard[1][1] == defaultBoard[1][2] && defaultBoard[1][0] == -player ||
    defaultBoard[2][0] == defaultBoard[2][1] && defaultBoard[2][1] == defaultBoard[2][2] && defaultBoard[2][0] == -player ||
    defaultBoard[0][0] == defaultBoard[1][0] && defaultBoard[1][0] == defaultBoard[2][0] && defaultBoard[0][0] == -player ||
    defaultBoard[0][1] == defaultBoard[1][1] && defaultBoard[1][1] == defaultBoard[2][1] && defaultBoard[0][1] == -player ||
    defaultBoard[0][2] == defaultBoard[1][2] && defaultBoard[1][2] == defaultBoard[2][2] && defaultBoard[0][2] == -player ||
    defaultBoard[0][0] == defaultBoard[1][1] && defaultBoard[1][1] == defaultBoard[2][2] && defaultBoard[0][0] == -player ||
    defaultBoard[2][0] == defaultBoard[1][1] && defaultBoard[1][1] == defaultBoard[0][2] && defaultBoard[2][0] == -player)
{

    result = isWin::no;
    return true;

}

    result = isWin::draw;
    return false;
}

Answer 1

thanks @PaulMckenzie for pointing out some of my code issues.

But they were nothing to do with errors I made on core logic of Negamax.

One by one, i will list them all out here and hope it could also help others who want to learn Negamax as well. if I miss anything just comment, I will edit afterwards.

*

DO remember to initialise all your new fields to a value, don't leave them for the logic to decide what is the initial value. this helps in debugging and it is just a good code practice. Thanks to @PaulMcKenzie

*

• Issue 1: deleteMoveNega() && applyMoveNega()

all they do is deleting a proposed move/applying a proposed move; they don't give back/pass on the turn to current player. Because the move is proposed as the best move for the opponent of current player, once we finish calculating the score for this proposed move (A) and we want to test the next proposed move(B), we will need to delete A and give the turn back to current player. (or, to some people it's better understood as previous player.) the same applies when we apply a proposed move.

it should therefore be:

    void Board::deleteMoveNega(const Move & move){

    defaultBoard[move.getRow()][move.getCol()] = EMPTY;
    m_player = getOpponent(m_player); // give turn back to current player;
}

    void Board::applyMoveNega(const Move & move){

    defaultBoard[move.getRow()][move.getCol()] = m_player;
    m_player = getOpponent(m_player); // pass on the turn to current player;
}

this is the most important error I made because with the old codes I would just propose move and calculate scores as whoever started the game all the way through; because I manually set the player to the opponent in startGameNage(), I then played the game as opponent proposing moves and calculating scores only as the opponent all the way through (whereas I should really be switching context and be in two players' positions). And this happened in each iteration of the negamax function. this didn't enforce the concept of thinking as current player because when I am supposed to play as current player, I however played as opponent.

This is fundamentally wrong in negamax.

Once we fix this, we then don't need to manually set the turn in startGameNage() therefore:

player = -player;

should be deleted and:

int player = 1;

will be changed to:

m_player = 1;

• Issue 2: negaMax()

with deleteMove() and applyMove() sorted out, we can now have a look at our negamax engine.

applyMoveNega(move, -_curplayer);
proposedNextMove = negaMax(_depth, -_curplayer, move);

First, I don't need the current player parameter. I have private m_player which I can make use of.

Second, and more importantly, with old deleteMove() and applyMove() and setting turn manually in startGameNega(), this negation for player here (-_curplayer) is just so wrong.

for example, we apply/make a move for -_curplayer; the proposed move happens next should be for the opponent, which in our case, should be _curplayer. i am still passing -_curplayer, this will then generate moves for the wrong player right from the very beginning.

a new core negamax would be like:

    Move Board::negaMax(const int & depth, const Move & initialMove){

    int row = initialMove.getRow();
    int col = initialMove.getCol();
    int _depth = depth;

    Move bestMove;
    Move proposedNextMove;

    //change to isGameComplete as of 15/10;
    if (_depth == 8 || isGameComplete())
    {
        int score = evaluateGameStateNega(_depth);
        bestMove.setScore(score);
        bestMove.setRow(initialMove.getRow());
        bestMove.setCol(initialMove.getCol());
    }
    else{
        Moves moves = generateMoves(row, col);

        _depth += 1;
        int bestScore = -1000;

        for (auto move : moves){

            applyMoveNega(move);
            proposedNextMove = negaMax(_depth, move);
            int tScore = -proposedNextMove.getScore();
            proposedNextMove.setScore(tScore);

            if (proposedNextMove.getScore() > bestScore){
                bestScore = proposedNextMove.getScore();
                bestMove.setScore(bestScore);
                bestMove.setRow(move.getRow());
                bestMove.setCol(move.getCol());
            }

            deleteMoveNega(move);
        }
    }
    return bestMove;
}

• Issue 3: clean up a bit

Yes I just have to admit this piece of algorithm was horribly written only to rush out the logic in my head and only to be prone to errors later. As we progress, we should all be diligent enough to prevent this. However sometimes we still need to get the logic going first :)

I will post clean-ups just to make it work, but not all the clean-ups which are to make it perfect. happy to accept comment.

1. isWon()

   bool Board::isWon(const int & currentPlayer){

   if (defaultBoard[0][0] == defaultBoard[0][1] && defaultBoard[0][1] == defaultBoard[0][2] && defaultBoard[0][0] == currentPlayer ||
       defaultBoard[1][0] == defaultBoard[1][1] && defaultBoard[1][1] == defaultBoard[1][2] && defaultBoard[1][0] == currentPlayer ||
       defaultBoard[2][0] == defaultBoard[2][1] && defaultBoard[2][1] == defaultBoard[2][2] && defaultBoard[2][0] == currentPlayer ||
       defaultBoard[0][0] == defaultBoard[1][0] && defaultBoard[1][0] == defaultBoard[2][0] && defaultBoard[0][0] == currentPlayer ||
       defaultBoard[0][1] == defaultBoard[1][1] && defaultBoard[1][1] == defaultBoard[2][1] && defaultBoard[0][1] == currentPlayer ||
       defaultBoard[0][2] == defaultBoard[1][2] && defaultBoard[1][2] == defaultBoard[2][2] && defaultBoard[0][2] == currentPlayer ||
       defaultBoard[0][0] == defaultBoard[1][1] && defaultBoard[1][1] == defaultBoard[2][2] && defaultBoard[0][0] == currentPlayer ||
       defaultBoard[2][0] == defaultBoard[1][1] && defaultBoard[1][1] == defaultBoard[0][2] && defaultBoard[2][0] == currentPlayer){
   
       return true;
   }
   
   return false;
   

   }

now I realised I didn't have to check for both players; that was wrong, I shall only be checking for current player; and the code is cleaner to read with only one if statement. the result was completely unnecessary. delete them. I was just confusing myself by complicating matters.

2. evaluateGameStateNega()

following the change in isWon() we would accordingly change the implementation of evaluateGameStateNega() as well:

    int Board::evaluateGameStateNega(const int & depth){

    if (isWon(m_player))
        return 10 - depth;
    if (isWon(getOpponent(m_player)))
        return depth - 10;
    else
        return 0;
}

3. generateMoves()

Above changes suffice to make it work with all other parts being untouched. this one therefore, is to add value.

   Moves Board::generateMoves(const int &rowIndex, const int &colIndex){

Moves Moves;

if (defaultBoard){

    for (int i = 0; i < WIDTH; i++)
    {
        for (int j = 0; j < LENGTH; j++)
        {
           if (defaultBoard[i][j] == 0)
            {
                Move move(i, j);
                Moves.push_back(move);
            }

        }
    }

}

return Moves;

}

obviously I wrote redundant codes. we don't need to check if the cell was occupied or not; we just need to generate moves for all empty cells!