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

Support impossible checks when validate position #446

Conversation

Oziomajnr
Copy link
Contributor

@Oziomajnr Oziomajnr commented Jul 13, 2023

Limited the fix to standard variant for now, as I need to test for other variants too.
Fixes #403

Copy link
Member

@lenguyenthanh lenguyenthanh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, It looks correct, I have few comments about where it should be and some styles changes. Let me know if you struggle with any of these comments.

src/main/scala/Situation.scala Outdated Show resolved Hide resolved
src/main/scala/Situation.scala Outdated Show resolved Hide resolved
src/main/scala/Situation.scala Outdated Show resolved Hide resolved
src/main/scala/Situation.scala Outdated Show resolved Hide resolved
src/main/scala/Situation.scala Outdated Show resolved Hide resolved
Move implementation to standard.
Fix unsafe calls to get, head and tail.
@Oziomajnr
Copy link
Contributor Author

Oziomajnr commented Jul 15, 2023

I fixed the comments, moved the implementation to the standard variant, this made the file changes more but hopefully it's clearer. Also, checking for optional in multiple places made the look more complex than it actually is, please let me know if you have any more suggestion to improve them.

@lenguyenthanh lenguyenthanh force-pushed the Support_impossible_checks_when_validate_position branch from 868fcc8 to f1710ab Compare July 16, 2023 09:37
@lenguyenthanh
Copy link
Member

Thanks @Oziomajnr, I took the liberty to do some refactor to make the logic more clear. Please check if you think it's good. I think we can improve it's a bit further, but I think it's good for now.

@Oziomajnr
Copy link
Contributor Author

Oziomajnr commented Jul 16, 2023

>

Thanks, the changes looks good, thanks.

@lenguyenthanh
Copy link
Member

Hey @Oziomajnr, FYI, this is a breaking change, so we're discussion the best way to integrated it with lila. So this is on hold atm.

…sition

* master:
  Specialize ByRole for bitboard.Board
  Specialize ByColor for bitboard.Board
@ornicar ornicar merged commit e64ccf3 into lichess-org:master Jul 18, 2023
1 check passed
ornicar added a commit to lichess-org/lila that referenced this pull request Jul 18, 2023
ornicar added a commit that referenced this pull request Jul 19, 2023
situation.checkers == Some(0)
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.

Support impossible checks when validate position
3 participants