[Game] Log transformed card revert on zone change as a single message

cockatrice/src/game_graphics/log/message_log_widget.cpp

@@ -118,8 +118,30 @@ MessageLogWidget::getFromStr(CardZoneLogic *zone, QString cardName, int position
     return {cardName, fromStr};
 }
 
+void MessageLogWidget::logTransformIntoCard(const QString &playerName,
+                                            QString oldCardName,
+                                            QString newCardName,
+                                            QString pt,
+                                            bool faceDown)
+{
+    if (faceDown) {
+        appendHtmlServerMessage(
+            tr("%1 transforms %2 into a face down card.").arg(playerName).arg(cardLink(std::move(oldCardName))));
+    } else {
+        appendHtmlServerMessage(tr("%1 transforms %2 into %3%4.")
+                                    .arg(playerName)
+                                    .arg(cardLink(std::move(oldCardName)))
+                                    .arg(cardLink(std::move(newCardName)))
+                                    .arg(pt.isEmpty() ? QString() : QString(" (%1)").arg(sanitizeHtml(pt))));
+    }
+}
+
 void MessageLogWidget::containerProcessingDone()
 {
+    for (const auto &[playerName, cardName] : deferredDestroyCardNames) {
+        appendHtmlServerMessage(tr("%1 destroys %2.").arg(playerName).arg(cardLink(cardName)));
+    }
+    deferredDestroyCardNames.clear();
     currentContext = MessageContext_None;
     transformOldCardName.clear();
     messageSuffix = messagePrefix = QString();
@@ -262,21 +284,21 @@ void MessageLogWidget::logCreateToken(PlayerLogic *player, QString cardName, QSt
                                         .arg(sanitizeHtml(player->getPlayerInfo()->getName()))
                                         .arg(cardLink(std::move(cardName)))
                                         .arg(pt.isEmpty() ? QString() : QString(" (%1)").arg(sanitizeHtml(pt))));
-        } else if (faceDown) {
-            appendHtmlServerMessage(tr("%1 transforms %2 into a face down card.")
-                                        .arg(sanitizeHtml(player->getPlayerInfo()->getName()))
-                                        .arg(cardLink(std::move(transformOldCardName))));
         } else {
-            appendHtmlServerMessage(tr("%1 transforms %2 into %3%4.")
+            logTransformIntoCard(sanitizeHtml(player->getPlayerInfo()->getName()), std::move(transformOldCardName),
-                                        .arg(sanitizeHtml(player->getPlayerInfo()->getName()))
+                                 std::move(cardName), pt, faceDown);
-                                        .arg(cardLink(std::move(transformOldCardName)))
-                                        .arg(cardLink(std::move(cardName)))
-                                        .arg(pt.isEmpty() ? QString() : QString(" (%1)").arg(sanitizeHtml(pt))));
         }
         transformOldCardName.clear();
         return;
     }
 
+    if (currentContext == MessageContext_MoveCard && !deferredDestroyCardNames.isEmpty()) {
+        const auto deferredDestroy = deferredDestroyCardNames.takeLast();
+        logTransformIntoCard(deferredDestroy.first, std::move(deferredDestroy.second), std::move(cardName), pt,
+                             faceDown);
+        return;
+    }
+
     if (faceDown) {
         appendHtmlServerMessage(
             tr("%1 creates a face down token.").arg(sanitizeHtml(player->getPlayerInfo()->getName())));
@@ -308,6 +330,11 @@ void MessageLogWidget::logDestroyCard(PlayerLogic *player, QString cardName)
         return;
     }
 
+    if (currentContext == MessageContext_MoveCard) {
+        deferredDestroyCardNames.append({sanitizeHtml(player->getPlayerInfo()->getName()), std::move(cardName)});
+        return;
+    }
+
     appendHtmlServerMessage(
         tr("%1 destroys %2.").arg(sanitizeHtml(player->getPlayerInfo()->getName())).arg(cardLink(std::move(cardName))));
 }
@@ -319,6 +346,14 @@ void MessageLogWidget::logMoveCard(PlayerLogic *player,
                                    CardZoneLogic *targetZone,
                                    int newX)
 {
+    // A destroyed card that was not replaced by a re-created card has no
+    // following move event, so flush any pending destroy logs before logging
+    // the next move within this batch.
+    for (const auto &[playerName, cardName] : deferredDestroyCardNames) {
+        appendHtmlServerMessage(tr("%1 destroys %2.").arg(playerName).arg(cardLink(cardName)));
+    }
+    deferredDestroyCardNames.clear();
+
     if (currentContext == MessageContext_Mulligan) {
         return;
     }

cockatrice/src/game_graphics/log/message_log_widget.h

@@ -9,6 +9,7 @@
 
 #include "../../interface/widgets/server/chat_view/chat_view.h"
 
+#include <QList>
 #include <QPair>
 #include <QString>
 #include <qtmetamacros.h>
@@ -40,9 +41,19 @@ private:
 
     MessageContext currentContext;
     QString transformOldCardName;
+    // During a move, a destroyed token may be replaced by a re-created stashed
+    // card in the start zone. The destroy event name is buffered here so the
+    // matching create-token event can render "transforms X into Y" instead of
+    // a destroy + create token pair. Entries are {playerName, cardName}.
+    QList<QPair<QString, QString>> deferredDestroyCardNames;
     QString messagePrefix, messageSuffix;
 
     static QPair<QString, QString> getFromStr(CardZoneLogic *zone, QString cardName, int position, bool ownerChange);
+    void logTransformIntoCard(const QString &playerName,
+                              QString oldCardName,
+                              QString newCardName,
+                              QString pt,
+                              bool faceDown);
 
 public:
     void connectToPlayerEventHandler(PlayerEventHandler *player);

tests/server_transform_context_test.cpp

@@ -17,9 +17,12 @@
 #include <QCoreApplication>
 #include <gtest/gtest.h>
 #include <libcockatrice/protocol/pb/command_create_token.pb.h>
+#include <libcockatrice/protocol/pb/command_move_card.pb.h>
+#include <libcockatrice/protocol/pb/context_move_card.pb.h>
 #include <libcockatrice/protocol/pb/context_transform_card.pb.h>
 #include <libcockatrice/protocol/pb/event_create_token.pb.h>
 #include <libcockatrice/protocol/pb/event_destroy_card.pb.h>
+#include <libcockatrice/protocol/pb/event_move_card.pb.h>
 #include <libcockatrice/protocol/pb/serverinfo_user.pb.h>
 #include <libcockatrice/rng/rng_abstract.h>
 #include <libcockatrice/utility/zone_names.h>
@@ -159,6 +162,82 @@ TEST_F(TransformContextTest, PlainTokenCreateHasNoTransformContext)
     EXPECT_EQ(ges.getGameEventContext(), nullptr);
 }
 
+TEST_F(TransformContextTest, MovingTransformedTokenRevertsToOriginalCard)
+{
+    GameEventStorage transformGes;
+    ResponseContainer rc(0);
+    ASSERT_EQ(player->cmdCreateToken(makeTransformCommand(targetCard->getId()), rc, transformGes), Response::RespOk);
+
+    uint32_t transformedCardId = 0;
+    for (const auto *item : transformGes.getGameEventList()) {
+        const GameEvent &event = item->getGameEvent();
+        if (event.HasExtension(Event_CreateToken::ext)) {
+            transformedCardId = event.GetExtension(Event_CreateToken::ext).card_id();
+        }
+    }
+    ASSERT_NE(transformedCardId, 0u);
+    ASSERT_NE(tableZoneOf(*player)->getCard(transformedCardId), nullptr);
+
+    auto *graveZone = new Server_CardZone(player.get(), ZoneNames::GRAVE, false, ServerInfo_Zone::HiddenZone);
+    player->addZone(graveZone);
+
+    CardToMove cardToMove;
+    cardToMove.set_card_id(transformedCardId);
+    QList<const CardToMove *> cardsToMove = {&cardToMove};
+
+    GameEventStorage moveGes;
+    ASSERT_EQ(player->moveCard(moveGes, tableZoneOf(*player), cardsToMove, graveZone, 0, 0, false, false, false),
+              Response::RespOk);
+
+    // The batch context is MoveCard so the client associates this sequence with
+    // the revert of a transformed token.
+    ASSERT_NE(moveGes.getGameEventContext(), nullptr);
+    EXPECT_TRUE(static_cast<GameEventContext *>(moveGes.getGameEventContext())->HasExtension(Context_MoveCard::ext));
+
+    int destroyIndex = -1;
+    int createIndex = -1;
+    int moveIndex = -1;
+    Event_CreateToken resurrectEvent;
+    QList<const GameEvent *> events;
+    for (const auto *item : moveGes.getGameEventList()) {
+        events.append(&item->getGameEvent());
+    }
+    for (int i = 0; i < events.size(); ++i) {
+        const GameEvent &event = *events.at(i);
+        if (event.HasExtension(Event_DestroyCard::ext)) {
+            const auto &destroyEvent = event.GetExtension(Event_DestroyCard::ext);
+            EXPECT_EQ(destroyEvent.card_id(), transformedCardId);
+            EXPECT_EQ(QString::fromStdString(destroyEvent.zone_name()), ZoneNames::TABLE);
+            destroyIndex = i;
+        } else if (event.HasExtension(Event_CreateToken::ext)) {
+            resurrectEvent = event.GetExtension(Event_CreateToken::ext);
+            createIndex = i;
+        } else if (event.HasExtension(Event_MoveCard::ext)) {
+            moveIndex = i;
+        }
+    }
+
+    // The transformed token is destroyed, the stashed original is re-created in
+    // the start zone, and the original card continues into the target zone.
+    EXPECT_NE(destroyIndex, -1);
+    ASSERT_NE(createIndex, -1);
+    EXPECT_NE(moveIndex, -1);
+    EXPECT_LT(destroyIndex, createIndex);
+    EXPECT_LT(createIndex, moveIndex);
+
+    EXPECT_EQ(resurrectEvent.card_name(), "Old Card");
+    EXPECT_EQ(QString::fromStdString(resurrectEvent.zone_name()), ZoneNames::TABLE);
+    ASSERT_TRUE(resurrectEvent.has_card_id());
+
+    // The transformed card is gone from the table and the original card moved
+    // into the graveyard.
+    EXPECT_EQ(tableZoneOf(*player)->getCard(transformedCardId), nullptr);
+    const auto &graveCards = graveZone->getCards();
+    ASSERT_EQ(graveCards.size(), 1);
+    EXPECT_EQ(graveCards.first()->getId(), resurrectEvent.card_id());
+    EXPECT_EQ(graveCards.first()->getName(), "Old Card");
+}
+
 } // namespace
 
 int main(int argc, char **argv)