Refactor code structure for improved readability and maintainability

miskibin · miskibin · commit 5a2af9a02667 · 2026-01-06T17:49:15.000+01:00
diff --git a/draughts/boards/american.py b/draughts/boards/american.py
@@ -5,7 +5,7 @@
 import numpy as np
 
 from draughts.boards.base import BaseBoard
-from draughts.models import Color, Figure
+from draughts.models import Color, Figure, EMPTY, MAN, KING
 from draughts.move import Move
 
 # fmt: off
@@ -46,58 +46,77 @@ def is_draw(self) -> bool:
 
     @property
     def legal_moves(self) -> Generator[Move, None, None]:
-        if self.turn == Color.BLACK:
-            squares_list = np.transpose(np.nonzero(self._pos > 0))
-        else:
-            squares_list = np.transpose(np.nonzero(self._pos < 0))
-        for square in squares_list.flatten():
+        # Use flatnonzero for faster piece lookup
+        turn_val = self.turn.value
+        squares_list = np.flatnonzero(self._pos * turn_val > 0)
+        for square in squares_list:
             moves = self._legal_moves_from(square)
             for move in moves:
                 yield move
 
     def _legal_moves_from(
         self, square: int, is_after_capture=False
-    ) -> Generator[Move, None, None]:
-        row = self.ROW_IDX[square]
+    ) -> list[Move]:
+        """
+        Generate legal moves using pre-computed attack tables.
+        American checkers: men can only capture forward, kings can capture in all directions.
+        """
         moves = []
-        odd = bool(row % 2 != 0 and self.turn == Color.BLACK) or (
-            row % 2 == 0 and self.turn == Color.WHITE
-        )
-        is_king = bool(self[square] == self.turn.value * Figure.KING)
-        # is_king = False  # DEBUG
-        for mv_offset, cap_offset, dir in [
-            (4 - odd, 7, self.turn.value),
-            (5 - odd, 9, self.turn.value),
-        ] + is_king * [
-            (4 - (not odd), 7, -self.turn.value),
-            (5 - (not odd), 9, -self.turn.value),
-        ]:
-            move_sq = square + mv_offset * (dir)
-            capture_sq = square + cap_offset * (dir)
-
-            if (
-                0 <= move_sq < len(self._pos)
-                and row + 1 * (dir) == self.ROW_IDX[move_sq]
-                and self[move_sq] == 0
-                and not is_after_capture
-            ):
-                moves.append(Move([square, move_sq]))
-            elif (
-                0 <= capture_sq < len(self._pos)
-                and row + 2 * (dir) == self.ROW_IDX[capture_sq]
-                and self[capture_sq] == 0
-                and self[move_sq] * self.turn.value < 0
-            ):
-                move = Move(
-                    [square, capture_sq],
-                    captured_list=[move_sq],
-                    captured_entities=[self[move_sq]],
-                )
-                moves.append(move)
-                self.push(move, False)
-                moves += [move + m for m in self._legal_moves_from(capture_sq, True)]
-                self.pop(False)
-
+        pos = self._pos
+        turn_val = self.turn.value
+        piece_val = abs(pos[square])
+        is_king = piece_val == KING
+        
+        # Use pre-computed attack tables
+        attack_table = self.WHITE_MAN_ATTACKS if turn_val < 0 else self.BLACK_MAN_ATTACKS
+        
+        if is_king:
+            # Kings can move and capture in all directions using KING_DIAGONALS
+            for direction in self.KING_DIAGONALS[square]:
+                if len(direction) >= 1:
+                    target = direction[0]
+                    # Regular move (not after capture)
+                    if pos[target] == EMPTY and not is_after_capture:
+                        moves.append(Move([square, target]))
+                    # Capture
+                    if len(direction) >= 2:
+                        jump_over = direction[0]
+                        land_on = direction[1]
+                        if pos[jump_over] * turn_val < 0 and pos[land_on] == EMPTY:
+                            move = Move(
+                                [square, land_on],
+                                captured_list=[jump_over],
+                                captured_entities=[pos[jump_over]],
+                            )
+                            moves.append(move)
+                            self.push(move, False)
+                            moves += [move + m for m in self._legal_moves_from(land_on, True)]
+                            self.pop(False)
+        else:
+            # Men use attack table (forward moves only, but can capture forward)
+            for entry in attack_table[square]:
+                target = entry.target
+                jump_over = entry.jump_over
+                land_on = entry.land_on
+                
+                # Regular move (only forward, not after capture)
+                if target >= 0 and not is_after_capture:
+                    if pos[target] == EMPTY:
+                        moves.append(Move([square, target]))
+                
+                # Capture move (men can only capture forward in American checkers)
+                if jump_over >= 0 and land_on >= 0:
+                    if pos[jump_over] * turn_val < 0 and pos[land_on] == EMPTY:
+                        move = Move(
+                            [square, land_on],
+                            captured_list=[jump_over],
+                            captured_entities=[pos[jump_over]],
+                        )
+                        moves.append(move)
+                        self.push(move, False)
+                        moves += [move + m for m in self._legal_moves_from(land_on, True)]
+                        self.pop(False)
+        
         return moves
 
 
diff --git a/draughts/boards/base.py b/draughts/boards/base.py
@@ -12,6 +12,8 @@
 from draughts.utils import (
     get_diagonal_moves,
     get_short_diagonal_moves,
+    generate_man_attack_tables,
+    generate_king_attack_tables,
 )
 
 # fmt: off
@@ -97,6 +99,16 @@ class BaseBoard(ABC):
     Despite the name, this contains LONG moves (all squares on diagonal) for kings.
     (one for move and one for capture)
     """
+    
+    # Pre-computed attack tables for optimized move generation
+    WHITE_MAN_ATTACKS = ...
+    """Pre-computed attack table for white men: square -> list of (target, jump_over, land_on)"""
+    
+    BLACK_MAN_ATTACKS = ...
+    """Pre-computed attack table for black men: square -> list of (target, jump_over, land_on)"""
+    
+    KING_DIAGONALS = ...
+    """Pre-computed diagonal lines for kings: square -> [up-right, up-left, down-right, down-left]"""
 
     def __init_subclass__(cls, **kwargs):
         parent_class = cls.__bases__[0]
@@ -110,6 +122,10 @@ def __init_subclass__(cls, **kwargs):
         # (first 2 squares) which is sufficient for men who only move/capture one square at a time
         cls.DIAGONAL_SHORT_MOVES = get_diagonal_moves(len(cls.STARTING_POSITION))
         cls.DIAGONAL_LONG_MOVES = get_short_diagonal_moves(len(cls.STARTING_POSITION))
+        
+        # Generate pre-computed attack tables for optimized move generation
+        cls.WHITE_MAN_ATTACKS, cls.BLACK_MAN_ATTACKS = generate_man_attack_tables(len(cls.STARTING_POSITION))
+        cls.KING_DIAGONALS = generate_king_attack_tables(len(cls.STARTING_POSITION))
 
     def __init__(
         self,
diff --git a/draughts/boards/standard.py b/draughts/boards/standard.py
@@ -6,10 +6,6 @@
 from draughts.boards.base import BaseBoard
 from draughts.models import Color, Figure, EMPTY, MAN, KING
 from draughts.move import Move
-from draughts.utils import (
-    get_diagonal_moves,
-    get_short_diagonal_moves,
-)
 
 # fmt: off
 SQUARES=  [ B10, D10, F10, H10, J10,
@@ -141,38 +137,41 @@ def _get_man_legal_moves_from(
         moves = []
         pos = self._pos  # Local reference for faster access
         turn_val = self.turn.value
-        # white can move only on even directions
-        for idx, direction in enumerate(self.DIAGONAL_LONG_MOVES[square]):
-            if (
-                len(direction) > 0
-                and (turn_val + idx)
-                in [-1, 0, 3, 4]  # TERRIBLE HACK get only directions for given piece
-                and pos[direction[0]] == EMPTY
-                and not is_capture_mandatory
-            ):
-                moves.append(Move([square, direction[0]]))
-            elif (
-                len(direction) > 1
-                and pos[direction[0]] * turn_val < 0
-                and pos[direction[1]] == EMPTY
-            ):
-                move = Move(
-                    [square, direction[1]], [direction[0]], [pos[direction[0]]]
-                )
-                # moves.append(move)
-                self.push(move, False)
-                new_moves = [
-                    move + m for m in self._get_man_legal_moves_from(direction[1], True)
-                ]
-                moves += [move] if len(new_moves) == 0 else new_moves
-                self.pop(False)
+        
+        # Use pre-computed attack tables based on turn
+        attack_table = self.WHITE_MAN_ATTACKS if turn_val < 0 else self.BLACK_MAN_ATTACKS
+        
+        for entry in attack_table[square]:
+            target = entry.target
+            jump_over = entry.jump_over
+            land_on = entry.land_on
+            
+            # Regular move (only if target is valid and not in capture mode)
+            if target >= 0 and not is_capture_mandatory:
+                if pos[target] == EMPTY:
+                    moves.append(Move([square, target]))
+            
+            # Capture move (jump over enemy, land on empty)
+            if jump_over >= 0 and land_on >= 0:
+                if pos[jump_over] * turn_val < 0 and pos[land_on] == EMPTY:
+                    move = Move(
+                        [square, land_on], [jump_over], [pos[jump_over]]
+                    )
+                    self.push(move, False)
+                    new_moves = [
+                        move + m for m in self._get_man_legal_moves_from(land_on, True)
+                    ]
+                    moves += [move] if len(new_moves) == 0 else new_moves
+                    self.pop(False)
+        
         return moves
 
     def _get_king_legal_moves_from(
         self, square: int, is_capture_mandatory: bool, forbidden_squares: set[int] | None = None
     ) -> list[Move]:
         """
         Generate legal moves for a king from a given square.
+        Uses pre-computed KING_DIAGONALS for optimized diagonal traversal.
         
         Args:
             square: The current square of the king
@@ -184,7 +183,8 @@ def _get_king_legal_moves_from(
         turn_val = self.turn.value
         max_len = 0  # Track max length across ALL directions
         
-        for direction in self.DIAGONAL_SHORT_MOVES[square]:
+        # Use pre-computed king diagonal table
+        for direction in self.KING_DIAGONALS[square]:
             dir_len = len(direction)
             for idx, target in enumerate(direction):
                 # Check if path crosses a forbidden square (previously captured piece)
diff --git a/draughts/engines/hub.py b/draughts/engines/hub.py
@@ -349,7 +349,7 @@ def __init__(
         self,
         path: str | Path,
         time_limit: float = 1.0,
-        depth_limit: int = 6,
+        depth_limit: Optional[int] = None,
         init_timeout: float = 10.0,
     ):
         """
@@ -358,7 +358,7 @@ def __init__(
         Args:
             path: Path to the engine executable
             time_limit: Time limit per move in seconds (default 1.0)
-            depth_limit: Maximum search depth
+            depth_limit: Maximum search depth (None for no limit)
             init_timeout: Timeout for engine initialization in seconds
         """
         self.path = Path(path)
diff --git a/draughts/utils.py b/draughts/utils.py
@@ -1,5 +1,79 @@
 import numpy as np
 from collections import defaultdict
+from typing import NamedTuple
+
+
+class AttackEntry(NamedTuple):
+    """Pre-computed attack info for a square in a direction."""
+    target: int       # Move target (adjacent empty square)
+    jump_over: int    # Square to jump over for capture
+    land_on: int      # Landing square after capture
+
+
+def generate_man_attack_tables(position_length: int) -> tuple[dict, dict]:
+    """
+    Generate pre-computed attack tables for men pieces.
+    
+    Returns:
+        white_attacks: dict[square, list[AttackEntry]] - for white men (moving up)
+        black_attacks: dict[square, list[AttackEntry]] - for black men (moving down)
+    """
+    diagonal_moves = get_short_diagonal_moves(position_length)
+    
+    white_attacks: dict[int, list[AttackEntry]] = {}
+    black_attacks: dict[int, list[AttackEntry]] = {}
+    
+    for square in range(position_length):
+        directions = diagonal_moves[square]
+        # directions: [up-right, up-left, down-right, down-left]
+        
+        white_attacks[square] = []
+        black_attacks[square] = []
+        
+        # White moves up (directions 0, 1)
+        for dir_idx in [0, 1]:  # up-right, up-left
+            d = directions[dir_idx]
+            if len(d) >= 1:
+                target = d[0]
+                jump_over = d[0] if len(d) >= 2 else -1
+                land_on = d[1] if len(d) >= 2 else -1
+                white_attacks[square].append(AttackEntry(target, jump_over, land_on))
+        
+        # Black moves down (directions 2, 3)
+        for dir_idx in [2, 3]:  # down-right, down-left
+            d = directions[dir_idx]
+            if len(d) >= 1:
+                target = d[0]
+                jump_over = d[0] if len(d) >= 2 else -1
+                land_on = d[1] if len(d) >= 2 else -1
+                black_attacks[square].append(AttackEntry(target, jump_over, land_on))
+        
+        # Both colors can capture in all 4 directions
+        for dir_idx in range(4):
+            d = directions[dir_idx]
+            if len(d) >= 2:
+                # Add capture-only entries for backward captures
+                if dir_idx in [2, 3]:  # backward for white
+                    jump_over = d[0]
+                    land_on = d[1]
+                    white_attacks[square].append(AttackEntry(-1, jump_over, land_on))
+                if dir_idx in [0, 1]:  # backward for black
+                    jump_over = d[0]
+                    land_on = d[1]
+                    black_attacks[square].append(AttackEntry(-1, jump_over, land_on))
+    
+    return white_attacks, black_attacks
+
+
+def generate_king_attack_tables(position_length: int) -> dict[int, list[list[int]]]:
+    """
+    Generate pre-computed diagonal lines for kings.
+    Kings can move along entire diagonals, so we store the full diagonal for each direction.
+    
+    Returns:
+        dict[square, list[list[int]]] - for each square, 4 lists of squares in each diagonal direction
+    """
+    return get_diagonal_moves(position_length)
 
 
 def _get_all_squares_at_the_diagonal(square: int, position_length: int) -> list[list[int]]:
diff --git a/tools/compare_versions.py b/tools/compare_versions.py
@@ -228,12 +228,22 @@ def start(self):
             stderr=subprocess.PIPE,
             text=True,
             bufsize=1,  # Line buffered
+            env={**__import__("os").environ, "PYTHONUNBUFFERED": "1"},
         )
         # Wait for ready signal
         assert self.process.stdout is not None
         ready = self.process.stdout.readline()
-        if not ready or "ready" not in ready:
-            raise RuntimeError(f"Worker failed to start: {ready}")
+        if not ready:
+            # Try to get stderr for better error message
+            assert self.process.stderr is not None
+            stderr = self.process.stderr.read()
+            raise RuntimeError(f"Worker failed to start (no output). stderr: {stderr}")
+        try:
+            data = json.loads(ready)
+            if data.get("status") != "ready":
+                raise RuntimeError(f"Worker failed to start: {ready}")
+        except json.JSONDecodeError:
+            raise RuntimeError(f"Worker failed to start (invalid JSON): {ready}")
     
     def get_move(self, fen: str | None, depth: int) -> dict:
         """Get engine move from persistent worker."""
diff --git a/uv.lock b/uv.lock
