Ultimatoe — 1. GUI and general game interfaces

Question

For the community challenge, I wrote ultimatoe. For me, the most interesting part is clearly the AI, but what I did so far is just a stupid Monte Carlo tree search, which isn't ready for a review yet (but may come soon).

Before reviewing, please have a look at the README. The text is much shorter than the code.

I'd like you have a look mainly at the GUI. Especially tips making it better without much effort are welcome. It works by transforming game.asString() into graphical representation (i.e., it makes from the character 'X' a field containing "X", which is a funny (and probably stupid) idea. The string representation of the below board would be something like "XXX*-X-* \n *--O* O \n...".

I'm leaving out the imports as boring.

Game

/** An immutable representation of a game (state). */
@Immutable public interface Game<G extends Game<G>> {
    /**
     * Return the list of all players, including dummies (like e.g., a player representing "no winner").
     *
     * <p>The starting player is the first element, other real players follow, dummies are placed last.
     */
    ImmutableList<GamePlayer<G>> players();

    /** Return the zero-based turn (i.e., the number of moves already played). */
    int turn();

    /**
     * Return the player to go, unless the game has already finished.
     *
     * <p>In case {@link #isFinished()} returns true, the result is undefined.
     */
    GamePlayer<G> playerOnTurn();

    /** Return the winner. If there's none, a special dummy gets returned. */
    GamePlayer<G> winner();

    /**
     * Return the score for the initial player (the bigger the better for them).
     * The result lies between -1 and +1.
     *
     * <p>Note that there's no heuristics involved, the score reflects the rules only.
     *
     * <p>If applicable for the given game, the result is<ul>
     * <li>{@code +1.0}, when the {@link #winner()} is {@code players().get(0)}
     * <li>{@code -1.0}, when the {@link #winner()} is {@code players().get(1)}
     * <li>{@code 0.0}, otherwise.</ul>
     */
    double score();

    /** Return true if the game has been finished. */
    boolean isFinished();

    /** Return a map of all directly reachable states to the corresponding moves. */
    ImmutableBiMap<G, String> children();

    /**
     * Return a game state resulting from applying a move represented by the argument string.
     * The set of all possible moves may be obtained via {@code children().values()}.
     *
     * @throws IllegalArgumentException if the game has already finished or the move is illegal or malformed.
     */
    G play(String move);

    /**
     * Return a game state resulting from applying a randomly chosen move.
     *
     * @throws IllegalArgumentException if the game has already finished.
     */
    G play(Random random);

    /**
     * Return a unique string representation suitable for automated processing and allowing to recreate the game.
     *
     * <p>Using this method rather than {@link toString} allows the latter
     * to omit some information or add redundant text to get mare human-friendly.
     */
    String asString();
}

GamePlayer

/**
 * A representation of an abstract player (like "black" or "white" in chess).
 *
 * <p>Not to be confused with {@link GameActor}}.
 */
@Immutable public interface GamePlayer<G extends Game<G>> {
    /** Return an int uniquely identifiying {@code this}. */
    int ordinal();

    /**
     * Return true if {@code this} represents a dummy (e.g., no winner) rather than
     * a real player (e.g., "X" or "O" in tic-tac-toe).
     */
    boolean isDummy();
}

GameActor

/** Represents a real player (unlike {@link GamePlayer}) of game {@code G}, e.g., an AI player. */
public interface GameActor<G extends Game<G>> {
    /** Return the initial state of {@code G}, like e.g. an empty tic-tac-toe board. */
    G initialGame();

    /** Return the "best" move for the given game state. */
    String selectMove(G game);

    GameAIParameters parameters();
}

UltimatoeGui

Apart from two constructors and a constant, the GUI uses only interface methods (I hope so). So you should be able to find everything needed above.

/** The GUI for the {@link Ultimatoe} game. */
public final class UltimatoeGui implements GameListener<Ultimatoe> {
    private final class FieldListener implements ActionListener {
        @Override public void actionPerformed(ActionEvent e) {
            if (swingWorker!=null) return; // It's a AI turn.
            final int guiIndex = buttons.indexOf(e.getSource());
            final int guiX = guiIndex % N_OF_GUI_FIELDS;
            final int guiY = guiIndex / N_OF_GUI_FIELDS;
            // Convert the gui coordinates to game cooredinates in range 0..8.
            final int x = guiX - guiX/4;
            final int y = guiY - guiY/4;
            final String move = UltimatoeUtils.coordinatesToMoveString(x, y);
            setState(game.play(move));
        }
    }

    private final class FieldButton extends JButton {
        @Override protected void paintComponent(Graphics g) {
            super.paintComponent(g);
            final String text = getText();
            if (text.isEmpty() || text.charAt(0) != UltimatoeUtils.BORDER) return;
            setForeground(new Color(0x20FFC0C0, true));
            g.fillRect(0, 0, getWidth(), getHeight());
        }
    }

    private UltimatoeGui() {
        final JButton fasterButton = new JButton(new AbstractAction("faster") {
            @Override public void actionPerformed(ActionEvent e) {
                autoplayDelayMillis /= SPEEDUP_FACTOR;
            }
        });

        final ActionListener autoListener = new ActionListener() {
            /** Switch the AI for a player on or off. */
            @Override public void actionPerformed(ActionEvent e) {
                final JToggleButton b = (JToggleButton) e.getSource();
                final boolean isOn = b.getModel().isSelected();
                final boolean isX = b.getText().endsWith("X");
                if (isX) {
                    actors[1] = isOn ? new GameMonteCarloActor<>(INITIAL_GAME) : null;
                } else {
                    actors[0] = isOn ? new GameRandomActor<>(INITIAL_GAME) : null;
                }

                final boolean isFullAuto = actors[0]!=null && actors[1]!=null;
                fasterButton.setEnabled(isFullAuto);
                autoplayDelayMillis = MAX_AUTOPLAY_DELAY_MILLIS;
                autoplay();
            }
        };

        for (final String s : "auto-X auto-O".split(" ")) {
            final JToggleButton autoButton = new JToggleButton(s);
            autoButton.addActionListener(autoListener);
            controlPanel.add(autoButton);
        }

        fasterButton.setEnabled(false);
        controlPanel.add(fasterButton);

        mainPanel.setLayout(new GridLayout(N_OF_GUI_FIELDS, N_OF_GUI_FIELDS));
        for (int i=0; i<N_OF_GUI_FIELDS*N_OF_GUI_FIELDS; ++i) {
            final FieldButton button = new FieldButton();
            button.setPreferredSize(BUTTON_SIZE);
            button.addActionListener(listener);
            buttons.add(button);
            mainPanel.add(button);
        }

        frame.add(controlPanel, BorderLayout.NORTH);
        frame.add(mainPanel);
        frame.pack();
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

        setState(INITIAL_GAME);
        frame.setVisible(true);
        timer.start();
    }

    public static void main(String[] args) {
        new UltimatoeGui();
    }

    /** Store the new state of the game and display it. */
    @Override public void setState(final Ultimatoe game) {
        SwingUtilities.invokeLater(new Runnable() {
            @Override public void run() {
                setStateInternal(game);
                autoplay();
            }
        });
    }

    private void setStateInternal(Ultimatoe game) {
        final String newString = game.asString().replace("\n", "");
        Dout.a(game.asString());
        final String oldString = this.game==null ? null : this.game.toString().replace("\n", "");
        for (int i=0; i<buttons.size(); ++i) {
            final char c = newString.charAt(i);
            if (oldString != null && c == oldString.charAt(i)) continue;
            final JButton b = buttons.get(i);
            b.setForeground(Color.BLACK);
            b.setText("" + c);
            b.setEnabled(c == UltimatoeUtils.PLAYABLE);
            if (c == UltimatoeUtils.BORDER) b.setForeground(Color.DARK_GRAY);
        }

        this.game = game;
        frame.setTitle(title());
    }

    private String title() {
        String result;
        if (game.isFinished()) {
            result = "\"" + UltimatoeUtils.scoreToWinner(game.score()) + "\"" + " has won!";
        } else {
            result = "\"" + game.playerOnTurn().toString() + "\"" + " to go.";
        }
        result = "Turn " + game.turn() + ", " + result;
        result += " --- " + "Ultimatoe";
        return result;
    }

    private void autoplay() {
        if (game.isFinished()) return;
        autoplayInit();
        autoplayFinish();
    }

    /** Start the AI if needed. */
    private void autoplayInit() {
        if (swingWorker!=null) return;
        final GameActor<Ultimatoe> actor = actors[game.playerOnTurn().ordinal()];
        if (actor==null) return;
        swingWorker = apply(actor);
        swingWorker.execute();
    }

    /** Apply the move generated by AI if any. */
    private void autoplayFinish() {
        if (swingWorker==null) return;
        if (!swingWorker.isDone()) return;
        if (System.currentTimeMillis() < lastAutoplayMillis + autoplayDelayMillis) return;
        lastAutoplayMillis = System.currentTimeMillis();
        try {
            final String move = swingWorker.get();
            setState(game.play(move));
        } catch (InterruptedException | ExecutionException e) {
            e.printStackTrace(); //TODO
        } finally {
            swingWorker = null;
        }
    }

    private SwingWorker<String, Void> apply(final GameActor<Ultimatoe> actor) {
        return new SwingWorker<String, Void>() {
            @Override protected String doInBackground() throws Exception {
                return actor.selectMove(game);
            }
        };
    }

    private static final int N_OF_GUI_FIELDS = 11;
    private static final Dimension BUTTON_SIZE = new Dimension(50, 50);
    private static final int MAX_AUTOPLAY_DELAY_MILLIS = 2000;
    private static final double SPEEDUP_FACTOR = 1.2;

    private static final Ultimatoe INITIAL_GAME = Ultimatoe.INITIAL_GAME;

    private final JFrame frame = new JFrame();
    private final JPanel controlPanel = new JPanel();
    private final JPanel mainPanel = new JPanel();
    private final FieldListener listener = new FieldListener();
    private final List<JButton> buttons = Lists.newArrayList();

    private final Timer timer = new Timer(10, new ActionListener() {
        @Override public void actionPerformed(ActionEvent e) {
            autoplay();
        }
    });
    private long lastAutoplayMillis;
    private long autoplayDelayMillis;

    @NonNull private Ultimatoe game;

    /** The worker currently running the AI computing the move, or null. */
    @Nullable private SwingWorker<String, Void> swingWorker;

    @SuppressWarnings("unchecked")
    private final GameActor<Ultimatoe>[] actors = (GameActor<Ultimatoe>[]) new GameActor<?>[2];
}

I'd like to make the code better, but the price (length and effort) should be rather low as for me the interesting part is the AI.

---

There's a followup question showing the game engine.

Answer 1

Normally, X is the player to go first in Tic-Tac-Toe games. In your implementation, O goes first. This is a bit unusual.

---

A lot of your interfaces are using generics, but is that really necessary? I find that sometimes adding generics for these kinds of games is a case of YAGNI (You Ain't Gonna Need It). Will you ever be passing another type as the generic parameter?

---

You seem to rely quite a lot on String objects (in play(String move) for example). A problem with this is that you only want a very very small subset of all possible strings. It is very hard to document how the String should be formatted to be accepted. I believe that it would be a good idea to use a custom GameMove class for this, that will make the usage so much clearer.

---

In your FieldListener you do a bunch of mathematics with the x and y pos to retrieve the actual button that was clicked. I am not a fan of this. You're using a FieldButton class, you might as well give that a x and y property and then you can retrieve the x and y much easier.

Alternatively, you could create a specific instance of an ActionListener to it and pass x and y values along there.

mainPanel.setLayout(new GridLayout(N_OF_GUI_FIELDS, N_OF_GUI_FIELDS));
for (int i=0; i<N_OF_GUI_FIELDS*N_OF_GUI_FIELDS; ++i) {
    final FieldButton button = new FieldButton();
    button.setPreferredSize(BUTTON_SIZE);
    button.addActionListener(listener);
    buttons.add(button);
    mainPanel.add(button);
}

Here you could easily do that mathematics and call a method that creates a specific listener for that specific button, with its associated x and y values.

You might want to read some previous answers of mine about how to create multiple instances of an ActionListener that shares code.

---

Speaking of x and y values, I noticed that you don't really use the fact that this game is two-dimensional. Most loops that loop over all buttons is only using one variable, i. I'd recommend making more use of x and y.

---

Your GUI does not allow switching which AI controls which player at runtime. As you work on the AI aspect, I expect there to be more than two AIs, and the more you add the more of a mess you will have. Deal with this potential mess as soon as possible. Implementing this part in a clean way will allow players to play a bit against your Random AI, a bit against your MonteCarlo AI, and maybe even ask an AI for advice (without having them making a move). That last part is achievable in your code without too much effort as you have the GameActor return a move, instead of making the move (this is good).

---

boolean isX = b.getText().endsWith("X")

Not a fan of that. I'd perhaps create two instances of ActionListeners here as well.

---

for (final String s : "auto-X auto-O".split(" ")) {

Looping over an array of String by creating a string and splitting it is.... not ideal.

You could do:

for (final String s : new String[]{ "auto-X", "auto-O" }) {

Or you could extract a method and call it twice.

---

Your setStateInternal method reads the String representation of a board, and then transforms it into representations for the buttons. Why transform a 2d array to a String only to transform it back into a 2d representation again? (in the form of a value for each button). I'd skip this String entirely here and read the values directly from the game model

---

Your apply method would be much better named as createWorkerForActor or similar.

---

I recommend adhering to the Java conventions when it comes to where to place constants and fields. (At the top of the file)