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

Priority drops #805

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

inversed-ru
Copy link

Implementation of priority drops, a feature I needed for my chess variant. Pieces from priorityDropTypes must be dropped before dropping any other pieces. In my variant, I use this feature to control the early game setup, even though I don't technically impose a separate setup phase: stronger pieces can be dropped only after all the weaker ones have been dropped.

@ianfab ianfab added enhancement New feature or request and removed enhancement New feature or request labels Jul 4, 2024
@@ -1086,6 +1086,8 @@ bool Position::legal(Move m) const {
if (popcount((DarkSquares & to ? DarkSquares : ~DarkSquares) & pieces(us, BISHOP)) + 1 > (count_with_hand(us, BISHOP) + 1) / 2)
return false;
}
if (type_of(m) == DROP && (!var->isPriorityDrop[type_of(moved_piece(m))]) && priorityDropCountInHand[us] > 0)
Copy link

Choose a reason for hiding this comment

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

For efficiency, I think these two new conditions should be checked in reverse order.

Copy link
Member

@ianfab ianfab 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 your PR and sorry for the delay. I added a few comments.

@@ -90,6 +90,7 @@ struct Variant {
bool mustCapture = false;
bool mustDrop = false;
PieceType mustDropType = ALL_PIECES;
bool isPriorityDrop[PIECE_TYPE_NB] = {};
Copy link
Member

@ianfab ianfab Jul 25, 2024

Choose a reason for hiding this comment

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

It would be more consistent and storage efficient to use a PieceSet here, similar as for other properties. This would also simplify the parsing, since it can reuse existing logic. It could perhaps also already be made color-specific, similar as promotionPieceTypes or the like.

@@ -1086,6 +1086,8 @@ bool Position::legal(Move m) const {
if (popcount((DarkSquares & to ? DarkSquares : ~DarkSquares) & pieces(us, BISHOP)) + 1 > (count_with_hand(us, BISHOP) + 1) / 2)
return false;
}
if (type_of(m) == DROP && (!var->isPriorityDrop[type_of(moved_piece(m))]) && priorityDropCountInHand[us] > 0)
Copy link
Member

Choose a reason for hiding this comment

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

Since this is fairly trivial logic and does not depend on other moves, it should be more suitable to validate this in move generation and pseudo-legal validation instead of in legal validation.

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.

3 participants