Types + docs follow-up: Player/Game/GameController/Piece subclasses (#46, #47)#56
Open
dbqpdb wants to merge 1 commit into
Open
Types + docs follow-up: Player/Game/GameController/Piece subclasses (#46, #47)#56dbqpdb wants to merge 1 commit into
dbqpdb wants to merge 1 commit into
Conversation
…lasses (#46, #47) Combined follow-up pass to PR #54 (type hints, Board+Move) and PR #55 (docstrings, Board+Move). For these classes the type hints and the docstring cleanup co-locate on the same methods, so splitting into two commits would have produced artificial diff units — bundled here. Covered: - Player + RLMPlayer + HumanPlayer + NRLMPlayer - GameController - Game (with all its methods) - Piece + KQRBN_Piece - King + Queen + Rook + Bishop + Knight + Pawn Each method got: - Annotations on parameters and return type (using the SquareName / BoardIdxs / PieceChar aliases from #54 plus PEP 604 `T | None`). - A Google-style docstring trimmed to a single-line summary plus Args/Returns/Raises where they add information beyond the signature. - Inline comments restating obvious code dropped; comments explaining non-obvious *why* preserved (en-passant rank quirk, ray termination, castling check propagation, the `allow_own_king_checked` semantics). Adds `from __future__ import annotations` at the top so forward refs work (Player -> Game -> Move all reference each other). Two known bugs got one-line `# Bug #N:` markers at their sites: - #34 (castling rights on rook capture) in Game.make_move - #35 (FEN game-state ignored) in Game.__init__ Annotations only — no behavior changes. All 4 TestRLM cases pass plus test_entered_move_processing. Still out of scope: Lexicon, Loudmouth, TestRLM, top-level helpers — these are mostly commentary / test scaffolding and benefit from a different style than the core engine classes. Future pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to PR #54 (types, Board+Move) and PR #55 (docs, Board+Move). Same treatment applied to the rest of the engine's core classes:
Player,RLMPlayer,HumanPlayer,NRLMPlayerGameControllerGame(every method)Piece,KQRBN_PieceKing,Queen,Rook,Bishop,Knight,PawnWhy one combined commit (not split like #54 / #55)
For these classes the type-hint changes and docstring changes co-locate on the same methods — splitting would have produced artificial diff units. Keeping them as one commit makes each method's changes reviewable in one place.
What each method got
SquareName/BoardIdxs/PieceCharaliases introduced in Type hints for Board + Move; mypy CI workflow (advisory) (#46) #54 plus modernT | Noneunion syntax.Args:/Returns:/Raises:only where they add information.allow_own_king_checkedsemantics).Other things in here
from __future__ import annotationsat the top (forward references between Player/Game/Move all resolve cleanly).# Bug #N:markers at the sites of known bugs that are tracked separately:Game.make_moveGame.__init__Verification
All 4
TestRLMcases pass;test_entered_move_processingruns to completion and returns True. Annotations and docstring changes only — no behavior changes.Out of scope
Lexicon,Loudmouth,TestRLM, and top-level helpers (sillyDude,run_me_if_i_am_the_main_file). These are commentary / test scaffolding and benefit from a different treatment — future pass after the dead-code cleanup in #40 is resolved.Diff shape
376 insertions, 452 deletions — net negative thanks to the comment cleanup outpacing the annotation additions.
Interactions with other open PRs
from __future__ import annotationsat the top and theSquareName/BoardIdxs/PieceCharaliases. This PR adds the same lines on master. Git will either auto-resolve (identical added lines) or surface a trivial conflict for whoever rebases last.# Bug #N:comments for Bug: Move.__eq__ compares new_en_passant_square to itself #29/Bug: Board.__setitem__ constructs Exception without raising it #31/Bug: square_name_to_array_idxs uses setdefault, polluting class-level dict #33 in the Board/Move code (added in Inline docs: docstrings + comment cleanup for Board + Move (#47) #55), and adds Bug: castling rights not revoked when rook is captured on its starting square #34/Bug: FEN castling / EP / move-counter fields are discarded when loading game state #35 markers in Game. When Quick bug bag: six independent small fixes (#29, #30, #31, #32, #33, #36) #49 merges those go away in Bug: Move.__eq__ compares new_en_passant_square to itself #29/Bug: Board.__setitem__ constructs Exception without raising it #31/Bug: square_name_to_array_idxs uses setdefault, polluting class-level dict #33's sites; Bug: castling rights not revoked when rook is captured on its starting square #34/Bug: FEN castling / EP / move-counter fields are discarded when loading game state #35 stay.Closes #46 and #47 partially.
🤖 Generated with Claude Code