Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Correct Move Notation When Playing as Black #124

Closed
wants to merge 21 commits into from

Conversation

TomPlanche
Copy link
Contributor

@TomPlanche TomPlanche commented Jan 26, 2025

Description

This PR fixes an issue where chess moves were being recorded incorrectly in the game history when playing as black. The visual board flip for black's perspective was causing moves to be recorded with incorrect coordinates, leading to confusing move history and potential move repetition issues.

The fix involves properly translating coordinates back to standard chess notation before recording moves in the history, while maintaining the visual board flip for gameplay purposes.

Fixes #123

How Has This Been Tested?

The following test scenarios were executed to verify the fix:

  • Test A: Basic move recording

    • Play as white: e2-e4
    • Play as black: d7-d5
    • Verify moves are recorded correctly in history
    • Verify visual board state matches moves
  • Test B: Multiple move sequences

    • Play complete opening sequences
    • Verify all moves are recorded correctly
    • Check both white and black move notations
  • Test C: Edge cases

    • Verify bot games aren't affected
    • Verify multiplayer games work correctly
    • Test castling notation
    • Test capture notation
  • Test D: Integration testing

    • Verify move history display
    • Check game state persistence
    • Verify move playback

Checklist:

  • My code follows the style guidelines of this project
    - Follows Rust formatting guidelines
    - Consistent with existing codebase style

  • I have performed a self-review of my code
    - Checked coordinate transformation logic
    - Verified edge cases
    - Reviewed error handling

  • I have commented my code, particularly in hard-to-understand areas
    - Added explanatory comments for coordinate transformation
    - Documented special cases for bot/multiplayer

  • I have made corresponding changes to the documentation
    - No user-facing documentation changes needed
    - Code documentation updated

  • My changes generate no new warnings
    - Clean compilation
    - No clippy warnings

  • I have added tests that prove my fix is effective
    - Added test cases for move recording
    - Included both black and white perspective tests

  • New and existing unit tests pass locally with my changes
    - All tests passing
    - No regressions

  • Any dependent changes have been merged and published
    - No dependencies affected
    - Self-contained fix

@thomas-mauran
Copy link
Owner

Hello @TomPlanche you are right thank you very much for the contribution, the moves in the history should be independant from the notation. Therefore I see a little but in your implementation, when the black pawns in multiplayer it seems like the move is not visible on the white host board, I don't know if you checked that case ?

image

@TomPlanche
Copy link
Contributor Author

Hi @thomas-mauran, since I'm on MacOs, the multiplayer does not work, I cannot test it.
Trying to fix the problem.

@thomas-mauran
Copy link
Owner

Hi @thomas-mauran, since I'm on MacOs, the multiplayer does not work, I cannot test it.

Trying to fix the problem.

Oh really ? I am on Mac OS and it seems like the multiplayer is working for me, do you have an error message so I can help you out ?

@TomPlanche TomPlanche closed this by deleting the head repository Feb 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

History Notation Error
2 participants