Open
Conversation
Member
|
Thanks for your PR. In view of the many open feature addition PRs and the huge backlog of maintenance tasks (e.g. releases, upstream merges) I unfortunately have to say though that I am currently fairly hesitant to add features that are quite specific to rare/exotic variants, so I will defer the decision on this PR for quite some time until maintainability is improved. I hope you understand. |
cab101
reviewed
Oct 8, 2024
| inline int Position::sudoku_box_of(Square s) const { | ||
| assert(var != nullptr); | ||
| assert(sudoku_boxes()); | ||
| return rank_of(s) / var->sudokuBoxHeight * (files() / var->sudokuBoxWidth) + file_of(s) / var->sudokuBoxWidth; |
There was a problem hiding this comment.
you can use precalculated array instead of this, like sudoku_box_of[to]
Author
There was a problem hiding this comment.
Right, but it's not a heavy calculation, and it depends on variant parameters, so I prefer not to over-complicate the code
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
3 participants
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.
The PR introduces a new chess variant - "sudoku chess".
In a few words, it's normal chess, but player is not allowed to capture opponent's pieces while player's pieces break 8x8 sudoku rules (i.e. there are repeating pieces of the same type in a file, rank or 4x2 box). Full rules reference: https://gist.github.com/yusitnikov/e81a5e4df2fce6a854db8a2950fa5cfc
Benchmark results:
StateInfostruct. I guess that it could be solved by allocating the memory dynamically only when the variant is enabled, but it will add complexity to the codebase. Is it worth optimizations?Compatibility:
I personally plan using the variant only with classic setup: 8x8 board, standard initial position, normal chess rules + sudoku.
But theoretically, there's nothing in the variant implementation that prevents using it with:
LARGEBOARDScompilation option.Issues and potential improvements:
do_movere-calculates the sudoku conflicts map from the scratch. It should be possible to optimize.Sorry for a large PR! It's the smallest possible chunk of changes that will have a value for the variant support.