1

I'm trying to write an AI that never loses at Tic Tac Toe, and I want to use the minimax algorithm to do so. However, when I try to run the program, a stack overflow appears and I can't seem to find what is the error. Could you take a look and tell me what I'm doing wrong? It doesn't go as deep in the recursion I believe, since it should only go through all the possible game outcomes, which go up to 8 moves (since the player is first to play, not the AI). It is probably me doing something wrong, but I can't find anything.

EDIT: Here's the full code, the mechanics function is the main part: EDIT2: Fixed the constructor

package Packet;
import java.util.*;
import java.util.Scanner;

public class Logic {

public static class TicTacToe{

    private int[] currentBoard = new int[9];
    private int[] availableSpots = new int [9];

    private int emptySpace = 0;
    private int playerAI = 1;
    private int playerHuman = 2;


    void TicTacToe(){
        for (int i = 0; i < 9; i++){
            this.currentBoard[i] = this.emptySpace;
        }
        for (int i = 0; i < 9; i++){
            this.availableSpots[i] = i;
        }
    }

    private int movesNumber(){
        int counter = 0;
        for (int i = 0; i < 9; i++){
            if (this.currentBoard[i] == this.emptySpace){
                counter++;
            }
        }
        return counter;
    }

    private boolean win(int[] board,int player){
        if (
        (board[0] == player && board[1] == player && board[2] == player) ||
        (board[3] == player && board[4] == player && board[5] == player) ||
        (board[6] == player && board[7] == player && board[8] == player) ||
        (board[0] == player && board[3] == player && board[6] == player) ||
        (board[1] == player && board[4] == player && board[7] == player) ||
        (board[2] == player && board[5] == player && board[8] == player) ||
        (board[0] == player && board[4] == player && board[8] == player) ||
        (board[2] == player && board[4] == player && board[6] == player) ){
            return true;
        }
        else{
            return false;
        }
    }

    private int mechanics(int[] newBoard, int player){
        if (win(newBoard,this.playerHuman)){
            return -10;
        }
        else if (win(newBoard, this.playerAI)){
            return +10;
        }
        else if (this.movesNumber() == 0){
            return 0;
        }

        ArrayList<Integer> moves = new ArrayList<Integer>();
        ArrayList<Integer> scores = new ArrayList<Integer>();


        for (int i = 0; i < this.movesNumber(); i++){
            int[] possibleBoard = new int[9];
            possibleBoard = newBoard;           

            int availableSpotNumber = i;

            int j = i;

            while (this.availableSpots[j] == 9){
                availableSpotNumber++;
                j++;
            }

            possibleBoard[availableSpotNumber] = player;

            if (player == this.playerAI){
                scores.add(this.mechanics(possibleBoard, this.playerHuman));
            }
            else{
                scores.add(this.mechanics(possibleBoard, this.playerAI));
            }
            moves.add(availableSpotNumber);

            possibleBoard[availableSpotNumber] = this.emptySpace;
        }

        int bestMove = 0;

        if (player == this.playerAI){
            int bestScore = -10000;
            for (int i = 0; i < moves.size(); i++){
                if (scores.get(i) > bestScore){
                    bestScore = scores.get(i);
                    bestMove = i;
                }
            }
        }
        else {
            int bestScore = 10000;
            for (int i = 0; i < moves.size(); i++){
                if (scores.get(i) < bestScore){
                    bestScore = scores.get(i);
                    bestMove = i;
                }
            }
        }
        return moves.get(bestMove);
    }

    public void printTable(){
        System.out.println(this.currentBoard[0] + " | " + this.currentBoard[1] + " | " + this.currentBoard[2]);
        System.out.println("-   -   -");
        System.out.println(this.currentBoard[3] + " | " + this.currentBoard[4] + " | " + this.currentBoard[5]);
        System.out.println("-   -   -");
        System.out.println(this.currentBoard[6] + " | " + this.currentBoard[7] + " | " + this.currentBoard[8]);
        System.out.println();
    }

    private void fillTable(int position,int player){
        this.currentBoard[position] = player;
        this.availableSpots[position] = 9;
    }

    public void startGame(){
        while(true){
            this.printTable();
            Scanner ulaz = new Scanner(System.in);
            fillTable(ulaz.nextInt(), this.playerHuman);
            this.printTable();
            fillTable(this.mechanics(this.currentBoard, this.playerAI), this.playerAI);
            ulaz.close();
        }
    }

    public void resetGame(){
        for (int i = 0; i < 9; i++){
            this.currentBoard[i] = this.emptySpace;
        }
        for (int i = 0; i < 9; i++){
            this.availableSpots[i] = i;
        }
    }
}

public static void main(String[] args){
    TicTacToe game = new TicTacToe();
    game.startGame();
}
}

Also, here's the exact errors I get:

Exception in thread "main" java.lang.StackOverflowError
    at Packet.Logic$TicTacToe.mechanics(Logic.java:54)
    at Packet.Logic$TicTacToe.mechanics(Logic.java:84)
    at Packet.Logic$TicTacToe.mechanics(Logic.java:87)
    at Packet.Logic$TicTacToe.mechanics(Logic.java:84)
    at Packet.Logic$TicTacToe.mechanics(Logic.java:87)
    at Packet.Logic$TicTacToe.mechanics(Logic.java:84)
    at Packet.Logic$TicTacToe.mechanics(Logic.java:87)

After this part, these parts appear a bunch of times (at least 50)

at Packet.Logic$TicTacToe.mechanics(Logic.java:84)
at Packet.Logic$TicTacToe.mechanics(Logic.java:87) 

Line 54:

if (win(newBoard,this.playerHuman)){

Line 84:

scores.add(this.mechanics(possibleBoard, this.playerHuman));

Line 87:

scores.add(this.mechanics(possibleBoard, this.playerAI));
  • 1
    And what is the `win` method? – Elliott Frisch Nov 14 '17 at 17:07
  • Unreadable, incomplete code. Your mechanics method has a call to itself inside. When I see this error in recursive code, it tells me that the developer didn't think carefully enough about their stopping condition. – duffymo Nov 14 '17 at 17:07
  • @duffymo: "Your mechanics method has a call to itself inside" That's like the definition of what recursion means. – Piotr Wilkin Nov 14 '17 at 17:19
  • Yes. What’s your point? – duffymo Nov 14 '17 at 17:20
  • The stopping condition is the problem. – duffymo Nov 14 '17 at 17:21
  • You basically just said that the problem in recursive code is recursive code. I haven't analyzed the code in detail, but the only case in which you might have a point is a case in which the code would contain trivially identical recursive calls, i.e. `function(param1, param2)` would call `return function(param1, param2)`. That does not happen here, as far as I can tell. – Piotr Wilkin Nov 14 '17 at 17:25
  • 1
    Asking for a code review? This is probably not the right forum for that. – scottb Nov 14 '17 at 17:27
  • Also the function `void Logic()` is never called in your code. I suggest refactoring the large code block into even smaller functions. – jontro Nov 14 '17 at 17:29
  • Oh god, that was supposed to be a constructor in the class, I've edited it, but still didn't resolve the problem. – EdgyParalelogram Nov 14 '17 at 17:31
  • Did you try running your code with an increased stack size (for example -Xss128m) to see if the problem is with lack of tail recursion or a logic problem within your code? – Piotr Wilkin Nov 14 '17 at 17:33
  • I did it with the increased stack size, it actually just made the program run a bit longer, but the same error happens, only without the first error line from the previous iteration. – EdgyParalelogram Nov 14 '17 at 17:38
  • Okay, two hints on how to debug this: (a) try to increase the stack size even more and see if on a huge stack it manages to finish (b) make sure that between the recursive calls, the state of the board actually changes (i.e. that you are actually recording a move and not passing the same board over and over due to a bug). Since your code has branching recursion, it doesn't really need a large depth to overflow the stack limit. You might also try a simplified version of the game to see if it converges. – Piotr Wilkin Nov 14 '17 at 17:49
  • Also, one thing I see now is your use of internal data inside a recursive call - that's a big no-no. If you are using recursive calls, you should always only use data passed in the parameters, unless it's for memoization. In this case, you're using `availableSpots`, which you keep as a field and you're not passing as a recursive argument. Also, see the copying remark @xs0 made. – Piotr Wilkin Nov 14 '17 at 17:52
  • The same thing (probably even more relevant) applies to `movesNumber`. Your algorithm might never stop because (a) tic-tac-toe has no winning strategy and (b) you are not correctly accounting for the fact that there might not be any moves left to make. – Piotr Wilkin Nov 14 '17 at 17:54
  • I think I've got to the core of the problem. Added lines `this.availableSpots[j] = 9;` and `this.availableSpots[j] = j;` before and after the recursion part. – EdgyParalelogram Nov 14 '17 at 18:05
  • If you still get stack overflow , you can limit the depth of the recursion and do it in several intervals: see [1](https://stackoverflow.com/a/46022542/3992939) and [2](https://stackoverflow.com/a/46128171/3992939) – c0der Nov 14 '17 at 18:09
  • @EdgyParalelogram a better approach (instead of setting and resetting a field) would be to just pass `availableSpots` as an argument to your recursive call. – Piotr Wilkin Nov 14 '17 at 18:10
  • Also you're right, it does seem that the `movesNumber` is the problem, since I don't update it with the hypothetical boards, but with the actual board, since I've fixed it, it seems to be working, but the result isn't what I want, so it's the logic of the problem now, thanks @PiotrWilkin, my main problem has been solved – EdgyParalelogram Nov 14 '17 at 18:13

2 Answers2

0

There's a couple of problems, but here's how you can debug this:

add a StringBuilder debug = new StringBuilder(); field to your class, then change the main loop like this:

int debugLen = debug.length();
debug.append("\nSetting ").append(availableSpotNumber).append(" to ").append(player);
possibleBoard[availableSpotNumber] = player;

try {
    if (player == this.playerAI) {
        scores.add(this.mechanics(possibleBoard, this.playerHuman));
    } else {
        scores.add(this.mechanics(possibleBoard, this.playerAI));
    }
    moves.add(availableSpotNumber);
} catch (StackOverflowError error) {
     throw new StackOverflowError(debug.toString());
}

debug.setLength(debugLen);
possibleBoard[availableSpotNumber] = this.emptySpace;

Then you will see what is happening, which will give you a clue what to fix next. For example, the current version is doing this, for initial human move 1:

Setting 0 to 1
Setting 0 to 2
Setting 0 to 1
Setting 0 to 2
etc..

But, if you're too lazy, you can find a fixed version here.

xs0
  • 2,990
  • 17
  • 25
0

This might or might not be a code issue, as Java is not a fully functional language with it comes to recursion, you might want to see this answer: Does Java 8 have tail call optimization?

Basically, a language that allows for unlimited-depth recursion has to have tail recursion optimization. With tail call optimization, if the return value is the result of exactly the same function with different parameters, the stack will get replaced instead of the new call being added to the stack.

If a language does not have tail call optimization, then you're limited by stack size in how deep your recursion goes, even if the recursive calls have correct termination conditions (note: I haven't analyzed the code in depth, so obviously there might be a problem in the recursive logic itself). If you want to tweak the stack size, use the -Xss Java runtime parameter. Generally, increasing the stack size is a good heuristic (although not a fool-proof method) of checking whether the fault is with the language or with your algorithm.

Piotr Wilkin
  • 3,446
  • 10
  • 18
  • @scottb: he asks why his code is throwing a `StackOverflow` error. I'm explaning why perfectly valid recursive code with correct stopping conditions can still fail to execute in Java due to stack limitations. I fail to see how that is irrelevant. – Piotr Wilkin Nov 14 '17 at 17:27
  • @scottb: "and its answer does not depend upon tail recursion". How do you know that? Did you analyze the code and find the bug that causes an infinite recursion? If yes, please point it to us. If no, please stop making comments that do not contribute to the issue at all and contain false claims (about how lack of tail recursion in Java is irrelevant to `StackOverflow` errors). – Piotr Wilkin Nov 14 '17 at 17:29
  • Again, the question is asking how to solve the problem in Java. Your answer begins with a comparison to other functional languages which is outside the solution space for this question. – scottb Nov 14 '17 at 17:30
  • I actually also said how to solve the problem in Java (which is "increase the stack size by using `-Xss`, assuming that the recursive calls are correct"). You dismissed that by saying that "the question can be answered without JVM flags". – Piotr Wilkin Nov 14 '17 at 17:32