From acb11c881b42017fdda432bbdbbf1487575d1b6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vasco=20Guerreiro=20Vint=C3=A9m=20Morais?= Date: Wed, 1 Apr 2026 20:59:31 +0100 Subject: [PATCH] refactor: new way to check if a move is from the bottom of the deck --- cockatrice/src/game/player/player_actions.cpp | 2 -- .../remote/game/server_abstract_player.cpp | 24 +++++++++++++++---- .../remote/game/server_abstract_player.h | 3 +-- .../protocol/pb/command_move_card.proto | 3 --- .../movecard_tests/reverse_card_move_test.cpp | 2 +- 5 files changed, 21 insertions(+), 13 deletions(-) diff --git a/cockatrice/src/game/player/player_actions.cpp b/cockatrice/src/game/player/player_actions.cpp index ffab32190..287231402 100644 --- a/cockatrice/src/game/player/player_actions.cpp +++ b/cockatrice/src/game/player/player_actions.cpp @@ -664,7 +664,6 @@ void PlayerActions::moveBottomCardsTo(const QString &targetZone, const QString & cmd.set_target_zone(targetZone.toStdString()); cmd.set_x(0); cmd.set_y(0); - cmd.set_is_from_bottom(true); for (int i = maxCards - number; i < maxCards; ++i) { auto card = cmd.mutable_cards_to_move()->add_card(); @@ -786,7 +785,6 @@ void PlayerActions::actDrawBottomCards() cmd.set_target_zone(ZoneNames::HAND); cmd.set_x(0); cmd.set_y(0); - cmd.set_is_from_bottom(true); for (int i = maxCards - number; i < maxCards; ++i) { cmd.mutable_cards_to_move()->add_card()->set_card_id(i); diff --git a/libcockatrice_network/libcockatrice/network/server/remote/game/server_abstract_player.cpp b/libcockatrice_network/libcockatrice/network/server/remote/game/server_abstract_player.cpp index 2bf144086..aa12f1a80 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/game/server_abstract_player.cpp +++ b/libcockatrice_network/libcockatrice/network/server/remote/game/server_abstract_player.cpp @@ -237,8 +237,7 @@ Response::ResponseCode Server_AbstractPlayer::moveCard(GameEventStorage &ges, int yCoord, bool fixFreeSpaces, bool undoingDraw, - bool isReversed, - bool isFromBottom) + bool isReversed) { // Disallow controller change to other zones than the table. if (((targetzone->getType() != ServerInfo_Zone::PublicZone) || !targetzone->hasCoords()) && @@ -290,7 +289,23 @@ Response::ResponseCode Server_AbstractPlayer::moveCard(GameEventStorage &ges, bool revealTopStart = false; bool revealTopTarget = false; - // If the cards are to be taken from the bottom of the deck, reverse the order in which they are taken. + bool isFromBottom = false; + if (startzone->getName() == ZoneNames::DECK) { + int movedCount = static_cast(cardsToMove.size()); + int tailStart = startzone->getCards().size() - movedCount; + if (tailStart >= 0) { + isFromBottom = true; + int expectedPosition = tailStart; + for (const auto &card : cardsToMove) { + if (card.position != expectedPosition) { + isFromBottom = false; + break; + } + ++expectedPosition; + } + } + } + if (isFromBottom) { std::ranges::reverse_view reversedCardsToMove{cardsToMove}; for (auto card : reversedCardsToMove) { @@ -772,8 +787,7 @@ Server_AbstractPlayer::cmdMoveCard(const Command_MoveCard &cmd, ResponseContaine cardsToMove.append(&cmd.cards_to_move().card(i)); } - return moveCard(ges, startZone, cardsToMove, targetZone, cmd.x(), cmd.y(), true, false, cmd.is_reversed(), - cmd.is_from_bottom()); + return moveCard(ges, startZone, cardsToMove, targetZone, cmd.x(), cmd.y(), true, false, cmd.is_reversed()); } Response::ResponseCode diff --git a/libcockatrice_network/libcockatrice/network/server/remote/game/server_abstract_player.h b/libcockatrice_network/libcockatrice/network/server/remote/game/server_abstract_player.h index a8e88e8b3..9d9809298 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/game/server_abstract_player.h +++ b/libcockatrice_network/libcockatrice/network/server/remote/game/server_abstract_player.h @@ -92,8 +92,7 @@ public: int yCoord, bool fixFreeSpaces = true, bool undoingDraw = false, - bool isReversed = false, - bool isFromBottom = false); + bool isReversed = false); void processMoveCard(GameEventStorage &ges, Server_CardZone *startzone, diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/command_move_card.proto b/libcockatrice_protocol/libcockatrice/protocol/pb/command_move_card.proto index 8151d1343..a5b96da2e 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/command_move_card.proto +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/command_move_card.proto @@ -52,7 +52,4 @@ message Command_MoveCard { // Inverts the x coordinate to apply from the end of the target zone instead of the start optional bool is_reversed = 8 [default = false]; - - // Inverts the order in which cards are moved (if false, from top to bottom) - optional bool is_from_bottom = 9 [default = false]; } diff --git a/tests/movecard_tests/reverse_card_move_test.cpp b/tests/movecard_tests/reverse_card_move_test.cpp index b5c3a91d5..f0fab71db 100644 --- a/tests/movecard_tests/reverse_card_move_test.cpp +++ b/tests/movecard_tests/reverse_card_move_test.cpp @@ -57,7 +57,7 @@ TEST(ReverseCardMoveTest, MoveCardFromBottomTest) QList cardsToMove = {&moveA, &moveB, &moveC, &moveD}; GameEventStorage ges; - const auto response = player.moveCard(ges, &deckZone, cardsToMove, &exileZone, 0, 0, false, false, false, true); + const auto response = player.moveCard(ges, &deckZone, cardsToMove, &exileZone, 0, 0, false, false, false); EXPECT_EQ(response, Response::RespOk);