Gate commander tax increment on server-accepted move

cockatrice/src/game/player/player_actions.cpp

@@ -29,6 +29,7 @@
 #include <libcockatrice/protocol/pb/command_shuffle.pb.h>
 #include <libcockatrice/protocol/pb/command_undo_draw.pb.h>
 #include <libcockatrice/protocol/pb/context_move_card.pb.h>
+#include <libcockatrice/protocol/pending_command.h>
 #include <libcockatrice/settings/card_override_settings.h>
 #include <libcockatrice/settings/interface_settings.h>
 #include <libcockatrice/utility/clamped_arithmetic.h>
@@ -51,10 +52,10 @@ PlayerActions::PlayerActions(PlayerLogic *_player)
     connect(moveTopCardTimer, &QTimer::timeout, [this]() { actMoveTopCardToPlay(); });
 }
 
-void PlayerActions::playCard(CardItem *card, bool faceDown)
+PendingCommand *PlayerActions::prepareCardMove(CardItem *card, bool faceDown)
 {
     if (card == nullptr) {
-        return;
+        return nullptr;
     }
 
     Command_MoveCard cmd;
@@ -66,7 +67,7 @@ void PlayerActions::playCard(CardItem *card, bool faceDown)
 
     ExactCard exactCard = card->getCard();
     if (!exactCard) {
-        return;
+        return nullptr;
     }
 
     const CardInfo &info = exactCard.getInfo();
@@ -97,7 +98,14 @@ void PlayerActions::playCard(CardItem *card, bool faceDown)
         cmd.set_x(gridPoint.x());
         cmd.set_y(gridPoint.y());
     }
-    sendGameCommand(cmd);
+    return prepareGameCommand(cmd);
+}
+
+void PlayerActions::playCard(CardItem *card, bool faceDown)
+{
+    if (PendingCommand *pend = prepareCardMove(card, faceDown)) {
+        sendGameCommand(pend);
+    }
 }
 
 /**
@@ -1638,9 +1646,10 @@ void PlayerActions::playSelectedCards(QList<CardItem *> selectedCards, const boo
     playSelectedCardsImpl(selectedCards, faceDown, nullptr);
 }
 
-void PlayerActions::playSelectedCardsImpl(QList<CardItem *> selectedCards,
+void PlayerActions::playSelectedCardsImpl(
-                                          bool faceDown,
+    QList<CardItem *> selectedCards,
-                                          const std::function<void(CardItem *, const QString &)> &postPlayCallback)
+    bool faceDown,
+    const std::function<void(PendingCommand *, const QString &)> &postPlayCallback)
 {
     // CardIds will get shuffled downwards when cards leave the deck.
     // We need to iterate through the cards in reverse order so cardIds don't get changed out from under us as we play
@@ -1651,10 +1660,16 @@ void PlayerActions::playSelectedCardsImpl(QList<CardItem *> selectedCards,
     for (auto &card : selectedCards) {
         if (card && !isUnwritableRevealZone(card->getZone()) && card->getZone()->getName() != ZoneNames::TABLE) {
             const QString originalZone = card->getZone()->getName();
-            playCard(card, faceDown);
+            PendingCommand *pend = prepareCardMove(card, faceDown);
-            if (postPlayCallback) {
+            if (pend == nullptr) {
-                postPlayCallback(card, originalZone);
+                continue;
             }
+            // Connect before send: a local game processes the command synchronously inside
+            // sendGameCommand, firing the pend's finished signal before this call returns.
+            if (postPlayCallback) {
+                postPlayCallback(pend, originalZone);
+            }
+            sendGameCommand(pend);
         }
     }
 }
@@ -1671,13 +1686,22 @@ void PlayerActions::actPlayAndIncreasePartnerTax(QList<CardItem *> selectedCards
 
 void PlayerActions::playAndIncreaseTax(QList<CardItem *> selectedCards, int counterId)
 {
-    playSelectedCardsImpl(selectedCards, false, [this, counterId](CardItem * /*card*/, const QString &originalZone) {
+    playSelectedCardsImpl(selectedCards, false, [this, counterId](PendingCommand *pend, const QString &originalZone) {
-        if (originalZone == ZoneNames::COMMAND) {
+        if (originalZone != ZoneNames::COMMAND || pend == nullptr) {
-            CounterState *state = player->getCounters().value(counterId, nullptr);
+            return;
-            if (state && state->isActive()) {
-                sendIncCounter(counterId, 1);
-            }
         }
+        // Gate the tax increment on the server accepting the move, so a rejected move
+        // (the card couldn't legally leave the command zone) never inflates the tax.
+        connect(pend, &PendingCommand::finished, this,
+                [this, counterId](const Response &response, const CommandContainer &, const QVariant &) {
+                    if (response.response_code() != Response::RespOk) {
+                        return;
+                    }
+                    CounterState *state = player->getCounters().value(counterId, nullptr);
+                    if (state && state->isActive()) {
+                        sendIncCounter(counterId, 1);
+                    }
+                });
     });
 }
 

cockatrice/src/game/player/player_actions.h

@@ -257,23 +257,29 @@ private:
     void playSelectedCards(QList<CardItem *> selectedCards, bool faceDown = false);
 
     /**
-     * @brief Shared implementation for playing selected cards with an optional post-play callback.
+     * @brief Builds the move command for playing a card, returning the prepared (unsent) PendingCommand.
-     * @param selectedCards Cards to play
+     * @return The prepared command, or nullptr if the card cannot be played.
-     * @param faceDown Whether to play cards face-down
-     * @param postPlayCallback Called after each card is played, receiving the card and its *original* zone name
-     *        (captured before playCard, since playCard sends a move command that may change the card's zone).
      */
-    void playSelectedCardsImpl(QList<CardItem *> selectedCards,
+    PendingCommand *prepareCardMove(CardItem *card, bool faceDown);
-                               bool faceDown,
-                               const std::function<void(CardItem *, const QString &)> &postPlayCallback = nullptr);
 
     /**
-     * @brief Plays the selected cards and, for each that came from the command zone,
+     * @brief Shared implementation for playing selected cards with an optional per-card callback.
-     *        increments the given (active) tax counter by one.
+     * @param selectedCards Cards to play
+     * @param faceDown Whether to play cards face-down
+     * @param postPlayCallback Called for each card once its move is prepared but before it is sent,
+     *        receiving the move's PendingCommand and the card's *original* zone name (the move may
+     *        change the card's zone). Connect response handling here so it is attached before send.
+     */
+    void
+    playSelectedCardsImpl(QList<CardItem *> selectedCards,
+                          bool faceDown,
+                          const std::function<void(PendingCommand *, const QString &)> &postPlayCallback = nullptr);
+
+    /**
+     * @brief Plays the selected cards and, for each that came from the command zone and whose move
+     *        the server accepts, increments the given (active) tax counter by one.
      * @param selectedCards Cards to play
      * @param counterId The tax counter to increment (CounterIds::CommanderTax or PartnerTax)
-     * @note The increment is sent optimistically, not gated on the move succeeding; a rejected
-     *       move can leave the tax incremented without a card being cast.
      */
     void playAndIncreaseTax(QList<CardItem *> selectedCards, int counterId);