PEP 8
Constants
Tostart_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 continuedbetter? 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.
To be continued ...
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.
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:
- Only some invalid positions raise an exception. If the
yis 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 tocandidate.moves. flipped_rankis recomputed on every stringification, despite depending only upon theRANKconstant. 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}"
To be continued ...