From 5b11af585fdb2726f38dc98dd6d2f9d9176dbb6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Thu, 24 Sep 2026 06:55:51 +0200 Subject: [PATCH] [Game] Single chat message when a card transforms (#4764) Transform currently logs "X destroys Y." followed by "X creates token: Z." because the server implements it as DestroyCard + CreateToken events. - Add Context_TransformCard (protocol extension 1010) to the transform event batch and a unit test asserting the server attaches it - Suppress the destroy/create pair and the migration un-attach lines in the game log, printing one "X transforms Y into Z (P/T)." message --- .../game_graphics/log/message_log_widget.cpp | 34 ++++ .../game_graphics/log/message_log_widget.h | 4 +- .../remote/game/server_abstract_player.cpp | 2 + .../libcockatrice/protocol/pb/CMakeLists.txt | 1 + .../protocol/pb/context_transform_card.proto | 8 + .../protocol/pb/game_event_context.proto | 1 + tests/CMakeLists.txt | 11 ++ tests/server_transform_context_test.cpp | 169 ++++++++++++++++++ 8 files changed, 229 insertions(+), 1 deletion(-) create mode 100644 libcockatrice_protocol/libcockatrice/protocol/pb/context_transform_card.proto create mode 100644 tests/server_transform_context_test.cpp diff --git a/cockatrice/src/game_graphics/log/message_log_widget.cpp b/cockatrice/src/game_graphics/log/message_log_widget.cpp index 046a1ae90..3e4423f96 100644 --- a/cockatrice/src/game_graphics/log/message_log_widget.cpp +++ b/cockatrice/src/game_graphics/log/message_log_widget.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -120,6 +121,7 @@ MessageLogWidget::getFromStr(CardZoneLogic *zone, QString cardName, int position void MessageLogWidget::containerProcessingDone() { currentContext = MessageContext_None; + transformOldCardName.clear(); messageSuffix = messagePrefix = QString(); } @@ -129,6 +131,8 @@ void MessageLogWidget::containerProcessingStarted(const GameEventContext &contex currentContext = MessageContext_MoveCard; } else if (context.HasExtension(Context_Mulligan::ext)) { currentContext = MessageContext_Mulligan; + } else if (context.HasExtension(Context_TransformCard::ext)) { + currentContext = MessageContext_TransformCard; } } @@ -252,6 +256,27 @@ void MessageLogWidget::logCreateArrow(PlayerLogic *player, void MessageLogWidget::logCreateToken(PlayerLogic *player, QString cardName, QString pt, bool faceDown) { + if (currentContext == MessageContext_TransformCard) { + if (transformOldCardName.isEmpty()) { + appendHtmlServerMessage(tr("%1 creates token: %2%3.") + .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.") + .arg(sanitizeHtml(player->getPlayerInfo()->getName())) + .arg(cardLink(std::move(transformOldCardName))) + .arg(cardLink(std::move(cardName))) + .arg(pt.isEmpty() ? QString() : QString(" (%1)").arg(sanitizeHtml(pt)))); + } + transformOldCardName.clear(); + return; + } + if (faceDown) { appendHtmlServerMessage( tr("%1 creates a face down token.").arg(sanitizeHtml(player->getPlayerInfo()->getName()))); @@ -278,6 +303,11 @@ void MessageLogWidget::logDeckSelect(PlayerLogic *player, QString deckHash, int void MessageLogWidget::logDestroyCard(PlayerLogic *player, QString cardName) { + if (currentContext == MessageContext_TransformCard) { + transformOldCardName = cardName; + return; + } + appendHtmlServerMessage( tr("%1 destroys %2.").arg(sanitizeHtml(player->getPlayerInfo()->getName())).arg(cardLink(std::move(cardName)))); } @@ -817,6 +847,10 @@ void MessageLogWidget::logSpectatorSay(const ServerInfo_User &spectator, QString void MessageLogWidget::logUnattachCard(PlayerLogic *player, QString cardName) { + if (currentContext == MessageContext_TransformCard) { + return; + } + appendHtmlServerMessage(tr("%1 unattaches %2.") .arg(sanitizeHtml(player->getPlayerInfo()->getName())) .arg(cardLink(std::move(cardName)))); diff --git a/cockatrice/src/game_graphics/log/message_log_widget.h b/cockatrice/src/game_graphics/log/message_log_widget.h index 5642a76e9..6ae052769 100644 --- a/cockatrice/src/game_graphics/log/message_log_widget.h +++ b/cockatrice/src/game_graphics/log/message_log_widget.h @@ -34,10 +34,12 @@ private: { MessageContext_None, MessageContext_MoveCard, - MessageContext_Mulligan + MessageContext_Mulligan, + MessageContext_TransformCard }; MessageContext currentContext; + QString transformOldCardName; QString messagePrefix, messageSuffix; static QPair getFromStr(CardZoneLogic *zone, QString cardName, int position, bool ownerChange); 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 b5a620798..32104ca48 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 @@ -47,6 +47,7 @@ #include #include #include +#include #include #include #include @@ -1012,6 +1013,7 @@ Server_AbstractPlayer::cmdCreateToken(const Command_CreateToken &cmd, ResponseCo if (targetZone) { targetCard = targetZone->getCard(cmd.target_card_id()); if (targetCard && cmd.target_mode() == Command_CreateToken::TRANSFORM_INTO) { + ges.setGameEventContext(Context_TransformCard()); if (targetCard->getParentCard()) { ges.enqueueGameEvent(makeAttachCardEvent(targetCard), playerId); } diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/CMakeLists.txt b/libcockatrice_protocol/libcockatrice/protocol/pb/CMakeLists.txt index 059e83096..598fe63a8 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/CMakeLists.txt +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/CMakeLists.txt @@ -80,6 +80,7 @@ set(PROTO_FILES context_ping_changed.proto context_ready_start.proto context_set_sideboard_lock.proto + context_transform_card.proto context_undo_draw.proto developer_commands.proto event_add_to_list.proto diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/context_transform_card.proto b/libcockatrice_protocol/libcockatrice/protocol/pb/context_transform_card.proto new file mode 100644 index 000000000..f4aef7433 --- /dev/null +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/context_transform_card.proto @@ -0,0 +1,8 @@ +syntax = "proto2"; +import "game_event_context.proto"; + +message Context_TransformCard { + extend GameEventContext { + optional Context_TransformCard ext = 1010; + } +} \ No newline at end of file diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/game_event_context.proto b/libcockatrice_protocol/libcockatrice/protocol/pb/game_event_context.proto index 76dccad0d..94f8b0fa2 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/game_event_context.proto +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/game_event_context.proto @@ -11,6 +11,7 @@ message GameEventContext { CONNECTION_STATE_CHANGED = 1007; SET_SIDEBOARD_LOCK = 1008; UNCONCEDE = 1009; + TRANSFORM_CARD = 1010; } extensions 100 to max; } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 0bb9654a1..dc7a8d78f 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -328,6 +328,17 @@ cockatrice_add_test( LABELS server ) +cockatrice_add_test( + NAME + server_transform_context_test + SOURCES + server_transform_context_test.cpp + LIBS + libcockatrice_network_server_remote + libcockatrice_rng + LABELS + server +) cockatrice_add_test( NAME server_forced_password_change_test diff --git a/tests/server_transform_context_test.cpp b/tests/server_transform_context_test.cpp new file mode 100644 index 000000000..bad6622b8 --- /dev/null +++ b/tests/server_transform_context_test.cpp @@ -0,0 +1,169 @@ +/** @file server_transform_context_test.cpp + * @brief Tests that the card transform path attaches a Context_TransformCard + * to the event storage, so the client can render a single + * "Player X transforms card Y into card Z" chat message. + * @ingroup Tests + */ + +#include "game/game_config.h" +#include "game/server_abstract_player.h" +#include "game/server_card.h" +#include "game/server_cardzone.h" +#include "game/server_game.h" +#include "server_response_containers.h" +#include "server_room.h" +#include "movecard_tests/server_test_helpers.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +RNG_Abstract *rng = nullptr; // referenced by the server_remote library + +namespace +{ + +class TransformContextTest : public ::testing::Test +{ +protected: + ServerInfo_User user; + FakeServer server; + std::unique_ptr room; + std::unique_ptr game; + std::unique_ptr player; + Server_Card *targetCard = nullptr; + + void SetUp() override + { + user.set_name("test-user"); + + room = std::make_unique(0, 0, "", "", "", "", false, "", QList(), &server); + GameConfig config{.creatorInfo = user, + .gameId = 1, + .description = QString(), + .password = QString(), + .maxPlayers = 2, + .gameTypes = QList(), + .onlyBuddies = false, + .onlyRegistered = false, + .spectatorsAllowed = false, + .spectatorsNeedPassword = false, + .spectatorsCanTalk = false, + .spectatorsSeeEverything = false, + .startingLifeTotal = 20, + .shareDecklistsOnLoad = false}; + game = std::make_unique(config, room.get()); + + // Start the game with no participants so cmdCreateToken can run; the + // start is emitted via a queued connection, so process posted events. + game->startGameIfReady(true); + QCoreApplication::processEvents(); + ASSERT_TRUE(game->getGameStarted()); + + player = std::make_unique(game.get(), 1, user, false, nullptr); + auto *tableZone = new Server_CardZone(player.get(), ZoneNames::TABLE, true, ServerInfo_Zone::PublicZone); + player->addZone(tableZone); + + targetCard = new Server_Card({"Old Card", "old-card"}, player->newCardId(), 0, 0); + tableZone->insertCard(targetCard, 0, 0); + } +}; + +Server_CardZone *tableZoneOf(Server_AbstractPlayer &player) +{ + return player.getZones().value(ZoneNames::TABLE); +} + +Command_CreateToken makeTransformCommand(uint32_t targetCardId) +{ + Command_CreateToken cmd; + cmd.set_zone(ZoneNames::TABLE); + cmd.set_target_zone(ZoneNames::TABLE); + cmd.set_target_card_id(targetCardId); + cmd.set_target_mode(Command_CreateToken::TRANSFORM_INTO); + cmd.set_card_name("New Card"); + cmd.set_card_provider_id("new-card"); + cmd.set_color("m"); + cmd.set_pt("2/2"); + cmd.set_destroy_on_zone_change(true); + cmd.set_x(-1); + cmd.set_y(-1); + return cmd; +} + +TEST_F(TransformContextTest, TransformCommandSetsTransformContext) +{ + GameEventStorage ges; + ResponseContainer rc(0); + + const auto response = player->cmdCreateToken(makeTransformCommand(targetCard->getId()), rc, ges); + + EXPECT_EQ(response, Response::RespOk); + ASSERT_NE(ges.getGameEventContext(), nullptr); + EXPECT_TRUE(static_cast(ges.getGameEventContext())->HasExtension(Context_TransformCard::ext)); +} + +TEST_F(TransformContextTest, TransformEventSequenceDestroyThenCreate) +{ + GameEventStorage ges; + ResponseContainer rc(0); + + ASSERT_EQ(player->cmdCreateToken(makeTransformCommand(targetCard->getId()), rc, ges), Response::RespOk); + + QList destroyedIds; + QList createdTokens; + for (const auto *item : ges.getGameEventList()) { + const GameEvent &event = item->getGameEvent(); + if (event.HasExtension(Event_DestroyCard::ext)) { + destroyedIds.append(event.GetExtension(Event_DestroyCard::ext).card_id()); + } + if (event.HasExtension(Event_CreateToken::ext)) { + createdTokens.append(event.GetExtension(Event_CreateToken::ext)); + } + } + + // The old card is destroyed and exactly one token is created in its place. + EXPECT_EQ(destroyedIds, QList({static_cast(targetCard->getId())})); + ASSERT_EQ(createdTokens.size(), 1); + EXPECT_EQ(createdTokens.first().card_name(), "New Card"); + EXPECT_EQ(createdTokens.first().pt(), "2/2"); + EXPECT_EQ(createdTokens.first().destroy_on_zone_change(), true); + + // The old card is removed from the table and the new card replaces it. + EXPECT_EQ(tableZoneOf(*player)->getCard(targetCard->getId()), nullptr); + EXPECT_NE(tableZoneOf(*player)->getCard(createdTokens.first().card_id()), nullptr); +} + +TEST_F(TransformContextTest, PlainTokenCreateHasNoTransformContext) +{ + Command_CreateToken cmd; + cmd.set_zone(ZoneNames::TABLE); + cmd.set_card_name("Plain Token"); + cmd.set_card_provider_id("plain-token"); + cmd.set_color(""); + cmd.set_pt("1/1"); + cmd.set_destroy_on_zone_change(true); + cmd.set_x(-1); + cmd.set_y(-1); + + GameEventStorage ges; + ResponseContainer rc(0); + + EXPECT_EQ(player->cmdCreateToken(cmd, rc, ges), Response::RespOk); + EXPECT_EQ(ges.getGameEventContext(), nullptr); +} + +} // namespace + +int main(int argc, char **argv) +{ + QCoreApplication app(argc, argv); + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} \ No newline at end of file