-
-
Notifications
You must be signed in to change notification settings - Fork 437
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
Allow placement below top X cards #2666
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an exciting change!
cockatrice/src/player.cpp
Outdated
@@ -723,6 +725,7 @@ void Player::retranslateUi() | |||
} | |||
|
|||
aMoveToTopLibrary->setText(tr("&Top of library")); | |||
aMoveToXfromTopOfLibrary->setText(tr("Below X cards...")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"X cards from the top..."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed
cmd->mutable_cards_to_move()->CopyFrom(idList); | ||
cmd->set_target_player_id(getId()); | ||
cmd->set_target_zone("deck"); | ||
cmd->set_x(number); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if I set number = 100
in the UI but I only have 60 cards in the deck?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if number > deck.size() { number = deck.size() }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the wording is fine now
Thanks guys! This is awesome. |
void Player::actMoveCardXCardsFromTop() | ||
{ | ||
bool ok; | ||
int number = QInputDialog::getInt(0, tr("Place card X cards from top library"), tr("How many cards from the top of the deck should this card be placed:"), defaultNumberTopCardsToPlaceBelow, 1, 2000000000, 1, &ok); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this X cards ABOVE our card or X-1? Unexpectedly Absent says "just beneath the top X cards of your library" whereas Oust says second from the top
which is clarified as 6/15/2010: If the targeted creature’s owner has one or more cards in his or her library, that creature is put into that library directly under the top card.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2nd from top = means put 2 in input.
Beneath top X cards = X+1 is your input
void Player::actMoveCardXCardsFromTop() | ||
{ | ||
bool ok; | ||
int number = QInputDialog::getInt(0, tr("Place card X cards from top library"), tr("How many cards from the top of the deck should this card be placed:"), defaultNumberTopCardsToPlaceBelow, 1, 2000000000, 1, &ok); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be top of library
not top library
Related Ticket(s)
NOTE: This PR also addresses an option in the server that if we sent it (-inf, -2] as X, it would crash the user. This just prevents a crash and is backwards compatible with prior client/servers.
Short roundup of the initial problem
Couldn't place a card X cards from top easily into the deck.
Screenshots