4
\$\begingroup\$

This Connect 4 game will be used for implementing game-playing AI. Sample players are supplied. One takes user input, and the other plays randomly. Right now it's set for a human player to play against a randomly playing agent, but in the future I plan to try out algorithms such as Minimax and have AI agents play against themselves.

Some more features (such as notifying the players when the game is over, instead of just printing to stdout) need to be added for later work, but I'm showing this simply as a working base. I'm not so much looking for feature suggestions, since those will come later. I will be adding docstrings and comments, but for now it should be straight forward.

I'm looking for tips on: program correctness, code structure, and best practices (whether specific to Python or not).

(Python 3.2.3)

#!/usr/bin/env python

import numpy as np
from copy import copy
from abc import ABCMeta, abstractmethod

class Connect4Game():
    def __init__(self, player1, player2):
        self.player1 = player1
        self.player2 = player2
        self.player_names = {player1 : "Player1", player2 : "Player2"}
        self.board = np.zeros((6, 7), dtype=np.int8)
        self.turn = player1
        self.last_move = ()
        self.winner = None

    def gameover(self):
        if self.last_move == ():
            return False

        rind, cind = self.last_move

        row = self.board[rind, :]
        col = self.board[:, cind]
        diag1 = np.diagonal(self.board, cind - rind)
        diag2 = np.diagonal(np.fliplr(self.board), -(cind + rind - 6))

        for line in [row, col, diag1, diag2]:
            if line.shape[0] < 4:
                continue

            for four in [line[i:i+4] for i in range(len(line)-3)]:
                if sum(four == 1) == 4:
                    self.winner = self.player1
                    return True
                elif sum(four == 2) == 4:
                    self.winner = self.player2
                    return True

        if sum(self.board[0] == 0) == 0:
            return True

        return False

    def move(self, cind):
        free_index = sum(self.board[:, cind] == 0) - 1
        if free_index == -1:
            return False

        if self.turn is self.player1:
            self.board[free_index, cind] = 1
            self.turn = self.player2
        else:
            self.board[free_index, cind] = 2
            self.turn = self.player1

        self.last_move = (free_index, cind)
        return True

    def play(self):
        while not self.gameover():
            print(self.board)
            print(self.player_names[self.turn]+"'s turn.")

            self.move(self.turn.get_move(copy(self.board)))

        print(self.board)

        if self.winner is self.player1:
            print("Winner: player1")
        elif self.winner is self.player2:
            print("Winner: player2")
        else:
            print("Tie")

class BasePlayer(metaclass=ABCMeta):
    @abstractmethod
    def __init__(self, empty, me, opponent):
        pass

    @abstractmethod
    def get_move(self, board):
        pass

class HumanPlayer(BasePlayer):
    def __init__(self, empty, me, opponent):
        pass

    def get_move(self, board):
        col = input("Your move (0-6): ")
        return int(col)

class RandomPlayer(BasePlayer):
    def __init__(self, empty, me, opponent):
        pass

    def get_move(self, board):
        free_columns = np.where(board[0] == 0)[0]
        return np.random.choice(free_columns)

def main():
    player1 = HumanPlayer(0, 1, 2)
    player2 = RandomPlayer(0, 2, 1)

    game = Connect4Game(player1, player2)
    game.play()

if __name__ == "__main__":
    main()
\$\endgroup\$
5
\$\begingroup\$

I would advise using slightly longer names (rind should be row_ind, etc.), adding comments (while you are writing the code, not after) and avoiding counterintuitive variable names such as four. It is quite confusing when you are writing statements such as four == 2.

In addition, I would replace

self.last_move == () 

with

not self.last_move

to future-proof your code in case you switch to using lists or something else entirely.

Also, to simplify your structure, I would suggest splitting up your longer functions into other smaller functions, to make your code more digestible and reusable.

Also, instead of initializing players with tuples of seemingly random numbers, add some constants in your code for further increased readability.

| improve this answer | |
\$\endgroup\$
1
\$\begingroup\$
  • Having self.player1 and self.player2 is an early indication of a problem. Programmers can't count to two; we know just 'zero', 'one' and 'many'. Anything appearing more than once shall be packed into a container (think of adapting the same code to more than 2 players). A list would do well.

  • An if/else' clause inmove` seems unnecessary:

    Having a player list it is just

    self.board[free_index, cind] = self.turn.me
    self.turn = self.player[self.player.next]
    

    with no ifs whatsoever (in your case next means opponent). Again, a multiple-player game is not that different from a two-player one.

  • In a turn-oriented games, a standard practice is to explicitly validate moves: provide a game.get_move(player) method to repeatedly prompt player for a move until a valid one is submitted. This also implies separate move validation and move execution methods.

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.