Skip to content

shakmaty integration#137

Merged
thomas-mauran merged 24 commits intothomas-mauran:mainfrom
tigerros:shakmaty
Nov 23, 2025
Merged

shakmaty integration#137
thomas-mauran merged 24 commits intothomas-mauran:mainfrom
tigerros:shakmaty

Conversation

@tigerros
Copy link
Contributor

@tigerros tigerros commented Apr 26, 2025

Addresses #135. It's not complete, I'm just putting this out here as a kind of tracker (this branch will be updated further) and also so that maintainers can contribute too if they want to.

Changes:

  • Replace PieceColor with shakmaty::Color, import if there's no conflict with ratatui.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@tigerros tigerros marked this pull request as draft April 26, 2025 19:11
@tigerros
Copy link
Contributor Author

tigerros commented Apr 26, 2025

I wanted to make it incremental (one by one, works at every step), but that would waste an enormous amount of time because a ton of code is not necessary with shakmaty so I would be changing it just to remove it later. So, I identified some things that can be removed, with explanations. I've removed the tests associated with that too.

  • Coord - will be replaced with Square, undefined with Option<Square>.
  • Board - will be replaced with Chess.
  • All of pieces/ - contains logic which can be done with shakmaty, need to make a RoleExt trait to get the big string versions of pieces.
  • GameBoard::
    • fen_position - shakmaty has Fen::from_position.
    • did_piece_already_move - only used in fen_position.
    • get_piece_type and get_piece_color - shakmaty has Board::piece_at.
    • is_getting_checked - shakmaty has Chess::is_check.
    • get_king_coordinates - shakmaty has Board::king_of.
    • is_checkmate - shakmaty has Chess::is_checkmate.
    • number_of_authorized_positions - used in is_draw to check for stalemate, shakmaty has Chess::is_stalemate.
    • flip_the_board - shakmaty has Board::flip_vertical. Note that is has to be done when rendering, because Chess does not allow mutable access of the board.
    • get_authorized_positions - shakmaty has Chess::legal_moves, which can be used to look for legal squares for moves originating from a given square (which is what get_authorized_positions does).
    • is_latest_move_castling - used only in execute_move, much of that function will be removed, including the part where is_latest_move_castling.
    • get_all_protected_cells - used only in pieces/king.rs.
    • impossible_positions_king_checked - used only in pieces/.
  • utils.rs
    • clean_positions - actually not used anywhere.
    • col_to_letter, letter_to_col - shakmaty has File::char and File::from_char.
    • convert_position_into_notation, convert_notation_into_position - shakmaty has UciMove::to_string and UciMove::from_str.
    • get_int_from_char - used to aid in converting a "position" (as in convert_notation_into_position). Not necessary since we will use shakmaty's types for that.
    • is_piece_opposite_king - used only in pieces/.
    • is_cell_color_ally - used only in pieces/.
    • invert_position - shakmaty has Square::flip_vertical.

Note that I haven't implemented the replacements, so the code obviously doesn't compile. I just removed stuff I'm positive can be replaced with a much smaller, better version using shakmaty.

Removed:
- `add_piece_to_taken_pieces`: removed because shakmaty will make this trivial
- `is_latest_move_en_passant`: only used in `execute_move`, which will get revamped, and `add_piece_to_taken_piece`, which was removed
- `consecutive_non_pawn_or_capture` getter and setter. I made the field public instead, because there's no additional work being done by the getter/setter

Refactored:
- `board_history` -> `position_history` to better reflect shakmaty's naming. `board` was removed because we were already using `position_history` to store the current position, no reason to store it twice.
- merged `{white,black}_taken_pieces` to `taken_pieces: Vec<Piece>`
@thomas-mauran thomas-mauran added the refactor Refactor of the code label Apr 27, 2025
Signed-off-by: Thomas Mauran <thomasmauran@yahoo.com>
Signed-off-by: Thomas Mauran <thomasmauran@yahoo.com>
@thomas-mauran
Copy link
Owner

Hello @tigerros I am working on this pr right now and taking the time to completely refactor chess tui for a proper v2 using shakmaty, I will ping you when this will be done so we can check together the implementation and optimize it to the best

Signed-off-by: Thomas Mauran <thomasmauran@yahoo.com>
Signed-off-by: Thomas Mauran <thomasmauran@yahoo.com>
Signed-off-by: Thomas Mauran <thomasmauran@yahoo.com>
Signed-off-by: Thomas Mauran <thomasmauran@yahoo.com>
Signed-off-by: Thomas Mauran <thomasmauran@yahoo.com>
- improve the toml loading with serde
- improve the methods and remove useless mut
- remove some unwraps

Signed-off-by: Thomas Mauran <thomasmauran@yahoo.com>
Signed-off-by: Thomas Mauran <thomasmauran@yahoo.com>
Signed-off-by: Thomas Mauran <thomasmauran@yahoo.com>
Signed-off-by: Thomas Mauran <thomasmauran@yahoo.com>
@thomas-mauran
Copy link
Owner

Hello @tigerros it seems like I completed the migration !
Wondering if you wanted to review it since you are well aware about shakmaty

Signed-off-by: Thomas Mauran <thomasmauran@yahoo.com>
Signed-off-by: Thomas Mauran <thomasmauran@yahoo.com>
Signed-off-by: Thomas Mauran <thomasmauran@yahoo.com>
Signed-off-by: Thomas Mauran <thomasmauran@yahoo.com>
Signed-off-by: Thomas Mauran <thomasmauran@yahoo.com>
Signed-off-by: Thomas Mauran <thomasmauran@yahoo.com>
Signed-off-by: Thomas Mauran <thomasmauran@yahoo.com>
Signed-off-by: Thomas Mauran <thomasmauran@yahoo.com>
Signed-off-by: Thomas Mauran <thomasmauran@yahoo.com>
@thomas-mauran thomas-mauran marked this pull request as ready for review November 23, 2025 17:47
@thomas-mauran thomas-mauran merged commit 7ad817c into thomas-mauran:main Nov 23, 2025
5 checks passed
@thomas-mauran thomas-mauran mentioned this pull request Nov 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Refactor of the code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants