Text based chess game in Python

Question

I have reached a significant milestone since the start of my chess game project. I have implemented all basic functionality. There is no castling, en passant and pawn promotion yet but I have implemented checks and checkmate functionality.

I would like my code to be reviewed before going further in the project so that I can optimize the code beforehand.

I would appreciate any improvements and also bugs found.

Here is my code:

constants.py

WHITE = True
BLACK = False

RANK: dict[str, int] = {
    "a": 0, "b": 1, "c": 2, "d": 3,
    "e": 4, "f": 5, "g": 6, "h": 7
}

position.py

from constants import *

class Position:
    def __init__(self, y: int, x: int) -> None:
        self.y = y
        self.x = x

    def __add__(self, other):
        return Position(self.y + other.y, self.x + other.x)

    def __sub__(self, other):
        return Position(self.y - other.y, self.x - other.x)

    def __mul__(self, value: int):
        return Position(self.y * value, self.x * value)

    def __eq__(self, other) -> bool:
        return self.y == other.y and self.x == other.x

    def __repr__(self) -> str:
        return f"(y: {self.y}, x: {self.x})"

    def __str__(self) -> str:
        flipped_rank = {v: k for k, v in RANK.items()}
        return f"{flipped_rank[self.x]}{self.y + 1}"

    def abs(self):
        return Position(abs(self.y), abs(self.x))

support.py

from position import *

def is_same_color(*pieces: list[str]) -> bool:
    for i in range(len(pieces) - 1):
        if is_white(pieces[i]) == is_white(pieces[i + 1]):
            return False
    return True

def is_white(piece: str) -> bool:
    return piece.isupper()

def is_black(piece: str) -> bool:
    return piece.islower()

def is_king(piece: str) -> bool:
    return piece.lower() == "k"

def is_queen(piece: str) -> bool:
    return piece.lower() == "q"

def is_rook(piece: str) -> bool:
    return piece.lower() == "r"

def is_knight(piece: str) -> bool:
    return piece.lower() == "n"

def is_bishop(piece: str) -> bool:
    return piece.lower() == "b"

def is_pawn(piece: str) -> bool:
    return piece.lower() == "p"

def is_empty(piece: str) -> bool:
    return piece == "."

def extract_move(move: str) -> tuple[Position, Position]:
    try:
        start_pos = Position(int(move[1]) - 1, RANK[move[0]])
        end_pos = Position(int(move[3]) - 1, RANK[move[2]])
        return start_pos, end_pos
    except:
        raise ValueError(f"Invalid position {move}")

def sign(x: int | float):
    if x < 0:
        return -1
    elif x > 0:
        return 1
    elif x == 0:
        return 0

main.py

from support import *
from copy import deepcopy

start_position = [
    ["R", "N", "B", "Q", "K", "B", "N", "R"],
    ["P", "P", "P", "P", "P", "P", "P", "P"],
    [".", ".", ".", ".", ".", ".", ".", "."],
    [".", ".", ".", ".", ".", ".", ".", "."],
    [".", ".", ".", ".", ".", ".", ".", "."],
    [".", ".", ".", ".", ".", ".", ".", "."],
    ["p", "p", "p", "p", "p", "p", "p", "p"],
    ["r", "n", "b", "q", "k", "b", "n", "r"]
]

class Board:
    def __init__(self, board):
        self.board = board
        self.current_turn = WHITE
        self.legal_moves = self.generate_legal_moves()
        self.status = "RUNNING"

    def play_move(self, move):
        start_pos, end_pos = extract_move(move)
        if move in self.legal_moves:
            self[end_pos] = self[start_pos]
            self[start_pos] = "."
            self.current_turn = not self.current_turn
            self.legal_moves = self.generate_legal_moves()
            self.update_status()
        else:
            print(f"Invalid move {move}...")

    def update_status(self):
        if self.is_checkmate():
            self.status = "GAMEOVER"

    def play_moves(self, moves: str):
        for move in moves.split():
            print(self)
            self.play_move(move)

    def is_valid(self, move: str) -> bool:
        start_pos, end_pos = extract_move(move)
        largest = max(start_pos.y, start_pos.x, end_pos.y, end_pos.x)
        smallest = min(start_pos.y, start_pos.x, end_pos.y, end_pos.x)
        # Check if coordinates are out of bound
        if smallest < 0 or largest > 7:
            return False
        if start_pos == end_pos:
            return False
        piece = self[start_pos]
        if is_empty(piece):
            return False
        to_capture_piece = self[end_pos]
        if not is_empty(to_capture_piece) and not is_same_color(to_capture_piece, piece):
            return False
        delta = end_pos - start_pos
        if is_pawn(piece):
            if abs(delta.y) == 1: # 1 step forward
                if delta.x == 0 and is_empty(self[end_pos]): # No capture
                    return True
                elif abs(delta.x) == 1 and not is_empty(self[end_pos]): # Capture
                    return True
            if (abs(delta.y) == 2 and start_pos.y in (1, 6) and 
                is_empty(self[end_pos]) and is_empty(self[end_pos - Position(sign(delta.y), 0)])
            ): # 2 step forward
                return True
        elif is_bishop(piece):
            if abs(delta.y) == abs(delta.x):
                increment = Position(sign(delta.y), sign(delta.x))
                for i in range(1, abs(delta.y)):
                    if not is_empty(self[start_pos + (increment * i)]):
                        return False
                return True
        elif is_rook(piece):
            if delta.x == 0 or delta.y == 0:
                increment = Position(sign(delta.y), sign(delta.x))
                for i in range(1, max(abs(delta.y), abs(delta.x))):
                    if not is_empty(self[start_pos + (increment * i)]):
                        return False
                return True
        elif is_knight(piece):
            if delta.abs() == Position(2, 1) or delta.abs() == Position(1, 2):
                return True
        elif is_queen(piece):
            # Rook validation
            if delta.x == 0 or delta.y == 0:
                increment = Position(sign(delta.y), sign(delta.x))
                for i in range(1, max(abs(delta.y), abs(delta.x))):
                    if not is_empty(self[start_pos + (increment * i)]):
                        return False
                return True
            # Bishop validation
            if abs(delta.y) == abs(delta.x):
                increment = Position(sign(delta.y), sign(delta.x))
                for i in range(1, abs(delta.y)):
                    if not is_empty(self[start_pos + (increment * i)]):
                        return False
                return True
        elif is_king(piece):
            if abs(delta.y) in (0, 1) and abs(delta.x) in (0, 1):
                return True

        return False

    def is_check(self, move) -> bool:
        new_game = deepcopy(self)
        start_pos, end_pos = extract_move(move)
        new_game[end_pos] = new_game[start_pos]
        new_game[start_pos] = "."
        king_pos = new_game.get_king_pos(new_game.current_turn)
        for pos in new_game.get_all_pieces_pos()[not new_game.current_turn]:
            if new_game.is_valid(str(pos) + str(king_pos)):
                return True
        return False

    def is_checkmate(self) -> bool:
        return len(self.legal_moves) == 0

    def generate_legal_moves(self) -> list[str]:
        legal_moves = []
        candidate_moves = []
        pieces_pos = self.get_all_pieces_pos()[self.current_turn]
        # print([str(pos) for pos in pieces_pos])
        for pos in pieces_pos:
            piece = self[pos]
            # print("In for pos in pieces_pos:", piece, pos)
            if is_pawn(piece):
                if is_white(piece):
                    deltas = [
                        Position(1, 0), Position(2, 0),
                        Position(1, 1), Position(1, -1)
                    ]
                else:
                    deltas = [
                        Position(-1, 0), Position(-2, 0),
                        Position(-1, 1), Position(-1, -1)
                    ]
                for delta in deltas:
                    try:
                        move = str(pos) + str(pos + delta)
                        candidate_moves.append(move)
                    except KeyError:
                        pass
            elif is_knight(piece):
                deltas = [
                    Position(2, 1), Position(1, 2),
                    Position(-2, 1), Position(1, -2),
                    Position(2, -1), Position(-1, 2),
                    Position(-2, -1), Position(-1, -2)
                ]
                for delta in deltas:
                    try:
                        move = str(pos) + str(pos + delta)
                        candidate_moves.append(move)
                    except KeyError:
                        pass
            elif is_bishop(piece):
                deltas = [
                    Position(1, 1), Position(-1, -1),
                    Position(1, -1), Position(-1, 1)
                ]
                for delta in deltas:
                    for i in range(1, 8):
                        try:
                            move = str(pos) + str(pos + delta * i)
                            candidate_moves.append(move)
                        except KeyError:
                            pass
            elif is_rook(piece):
                deltas = [
                    Position(1, 0), Position(0, 1),
                    Position(-1, 0), Position(0, -1)
                ]
                for delta in deltas:
                    for i in range(1, 8):
                        try:
                            move = str(pos) + str(pos + delta * i)
                            candidate_moves.append(move)
                        except KeyError:
                            pass
            elif is_king(piece):
                deltas = [
                    Position(1, 0), Position(0, 1),
                    Position(-1, 0), Position(0, -1),
                    Position(1, 1), Position(-1, -1),
                    Position(1, -1), Position(-1, 1)
                ]
                for delta in deltas:
                    try:
                        move = str(pos) + str(pos + delta)
                        candidate_moves.append(move)
                    except KeyError:
                        pass
            elif is_queen(piece):
                deltas = [
                    # Bishop
                    Position(1, 1), Position(-1, -1),
                    Position(1, -1), Position(-1, 1),
                    # Rook
                    Position(1, 0), Position(0, 1),
                    Position(-1, 0), Position(0, -1)
                ]
                for delta in deltas:
                    for i in range(1, 8):
                        try:
                            move = str(pos) + str(pos + delta * i)
                            candidate_moves.append(move)
                        except KeyError:
                            pass

        for move in candidate_moves:
            try:
                # print(move, self.is_valid(move), not self.is_check(move))
                if self.is_valid(move) and not self.is_check(move):
                    legal_moves.append(move)
            except ValueError:
                pass
        return legal_moves

    def get_all_pieces_pos(self) -> dict[bool, list[Position]]:
        pieces_pos = {WHITE: [], BLACK: []}
        for y in range(8):
            for x in range(8):
                piece = self.board[y][x]
                if not is_empty(piece):
                    pieces_pos[is_white(piece)].append(Position(y, x))
        return pieces_pos

    def get_king_pos(self, color: bool) -> Position:
        for y in range(8):
            for x in range(8):
                if is_king(self.board[y][x]) and is_white(self.board[y][x]) == color:
                    return Position(y, x)

    def __getitem__(self, pos: Position) -> str:
        return self.board[pos.y][pos.x]

    def __setitem__(self, pos: Position, piece: str) -> None:
        self.board[pos.y][pos.x] = piece

    def __repr__(self) -> str:
        return "\n".join(
            [" ".join(rank + [str(8 - i)]) for i, rank in enumerate(self.board[::-1])] + 
            [" ".join(RANK.keys())]
        )

game = Board(start_position)

def play():
    while True:
        print(game)
        if game.status == "GAMEOVER":
            print(player, "wins!!")
            break
        player = "WHITE" if game.current_turn == WHITE else "BLACK"
        # print([str(move) for move in game.generate_legal_moves()])
        move = input(f"{player}, please enter your move:").lower().strip()
        game.play_move(move)

play()

Thank you for your time!!

Answer 1

Bug

play() gets a move string from the user, and calls game.play_move(move). which in turn calls extract_move(move), which can raise a ValueError, but that is not caught by any of the proceeding functions.

Confusing Code

                    try:
                        move = str(pos) + str(pos + delta)
                        candidate_moves.append(move)
                    except KeyError:
                        pass

Nothing about the above code looks like it can generate a KeyError. The only possibility looks to be the pos + delta, but Position.__add__ only does some math and calls the constructor which just stores the results. append on a vanilla list shouldn't fail. The only other possibility is str(...), which should always succeed:

    def __str__(self) -> str:
        flipped_rank = {v: k for k, v in RANK.items()}
        return f"{flipped_rank[self.x]}{self.y + 1}"

And here is where we find the exception: it is possible to construct an invalid Position, which cannot be stringified.

This code shows two other issues:

1. Only some invalid positions raise an exception. If the y is out of legal range, it can be stringified without issue. Only a that falls out of the valid x-range will raise an exception and be prevented from being added to candidate.moves.
2. flipped_rank is recomputed on every stringification, despite depending only upon the RANK constant. It should be computed once:

FLIPPED_RANK = {v: k for k, v in RANK.items()}

class Position:
    def __init__(self, y: int, x: int):
        if x not in range(8) or y not in range(8):
            raise ValueError("Illegal position coordinate")
        self.y = y
        self.x = x

    def __str__(self) -> str:
        return f"{FLIPPED_RANK[self.x]}{self.y + 1}"

PEP 8

Constants

start_position is a constant. As such, PEP 8: The Style Guide for Python Code recommends using UPPER_CASE for the identifier's name. It terms of names, it isn't really just one position. It is the starting position of all the pieces (plural), so perhaps STARTING_POSITIONS would be better? Or maybe even STARTING_BOARD, since it gets assigned to the .board member?

Blank Space

is_valid(...) and generate_legal_moves(...) are LONG functions. They could use additional blank lines to break the code into logical sections ... or be separated into more functions.

Type Hints

You've added type hints in many places, but omitted them in several. Eg, Position.abs() needs a return type, In Board, the __init__(), play_move(), lacks parameter type hints, and many methods could be declared to return -> None.

def is_same_color(*pieces: list[str]) -> bool means the function accepts a variable number of lists of strings. I'm certain you just want the function to accept a variable number of strings, which would be written as: *pieces: str.

Types

def get_king_pos(self, color: bool) is an indication you need some better types, since a colour is not a boolean. You have two sides/players. They are not the True player and the False player. Let's create an enumeration for the the player:

from enum import Enum

class Player(Enum):
    WHITE = "White"
    BLACK = "Black"

You can use Player as a type. If the player variable holds the current player, you can now write f"{player.value}, please enter your move:" to create the input() prompt.

Separate Logic and I/O

You have a game system where "K" represents the white king, and "r" represents the black rook. Unicode characters can convey this information much more directly: ♜♞♝♛♚♟︎ ♖♘♗♕♔♙. You could change the implementation to use these characters instead of upper & lower case letters, but if you wanted to build a graphical version of the game, or a 3d animated battle-chess game, you'd again find the unicode chess piece characters annoying to work with.

Really, what you want is for the game logic to use an "abstract" internal representation, and have the UI layer convert to the appropriate user representation. So maybe an enumeration for piece types:

class PieceType:
    PAWN = 1
    ROOK = 2
    BISHOP = 3
    KNIGHT = 4
    QUEEN = 5
    KING = 6

Now the black king could be represented as (PieceType.KING, Player.BLACK), and a dictionary could map that value to 'k', and later changing that to use '♚' would be a trivial modification.

But that still leaves a long string of if is_pawn(piece) type tests. It would be even better to have a type hierarchy which represents the chess pieces, and the specific types would know how they can move:

class ChessPiece:
    ...

class Pawn(ChessPiece):
    ...

class Rook(ChessPiece):
    ...

...

class King(ChessPiece):
    ...

With appropriate member functions, you could ask the piece which side it belongs to, where it is on the board, and what its valid moves are.

Answer 2

Just one thing I wanted to point out regarding magic methods.

Position.abs

This method can be replaced with the __abs__ magic method, allowing you to call the abs function on the object, instead of calling its method.

class Position:
    ...
    def __abs__(self):
        return Position(abs(self.y), abs(self.x))

# Example Usage
p = Position(2, -3)
p2 = abs(p)