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: changing piece on the kings square with chess.put() #426

Merged
merged 2 commits into from
Jul 21, 2023

Conversation

Manukyanq
Copy link
Contributor

Hello! I want to make a chessboard, where users can manually set the position they need and thechess.put() and chess.remove() methods are superuseful for me. I found a tiny bug, which I can make work with small hack, but anyway it is confusing and will be nice to fix it.

Lets say I want manually swap white king's and white queen's positions.

const chess = new Chess()

const status1 = chess.put({ type: 'q', color: 'w' }, 'e1') // true
const status2 = chess.put({ type: 'k', color: 'w' }, 'd1') // false

but if I remove white king and only after then put queen and king it works without errors

const chess = new Chess()

chess.remove('e1')
const status1 = chess.put({ type: 'q', color: 'w' }, 'e1') // true
const status2 = chess.put({ type: 'k', color: 'w' }, 'd1') // true

I can provide more details if it is required. Thank you

@jhlywa
Copy link
Owner

jhlywa commented Jul 16, 2023

Hi @Manukyanq, thanks for the PR!

I think I'm tracking the issue. Essentially, if you overwrite the position of the king with any other piece, we need to set the _kings entry (for the overwritten king color) to EMPTY. Could you add a comment to that section just to highlight the what's happening? A one-liner is fine.

Thanks and good find!

@jhlywa jhlywa merged commit 6ff1821 into jhlywa:master Jul 21, 2023
3 checks passed
@jhlywa
Copy link
Owner

jhlywa commented Jul 21, 2023

Merged. Thanks again!

This pull request was closed.
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.

2 participants