From 50a588460634e5c2494a0f29727e51c7d4d705fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Sun, 11 Oct 2026 14:57:43 +0200 Subject: [PATCH] [Game] Restore attach bookkeeping via CardItem and repaint on state change --- .../src/game/player/player_event_handler.cpp | 17 +++++++++++++++-- cockatrice/src/game/player/player_logic.cpp | 13 ++++++++++++- .../src/game_graphics/board/card_item.cpp | 4 ++++ 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/cockatrice/src/game/player/player_event_handler.cpp b/cockatrice/src/game/player/player_event_handler.cpp index a51cf8665..4698aeced 100644 --- a/cockatrice/src/game/player/player_event_handler.cpp +++ b/cockatrice/src/game/player/player_event_handler.cpp @@ -362,7 +362,13 @@ void PlayerEventHandler::eventMoveCard(const Event_MoveCard &event, const GameEv if (card->getAttachedTo() && (startZone != targetZone)) { CardState *parentCard = card->getAttachedTo(); - card->setAttachedTo(nullptr); + // Route through CardItem so the old parent's attachedCards list is cleaned up: CardState::setAttachedTo + // only repoints the pointer, leaving a dangling entry that reorganizeCards/prepareDelete would dereference. + if (CardItem *item = toCardItem(card)) { + item->setAttachedTo(nullptr); + } else { + card->setAttachedTo(nullptr); + } parentCard->getZone()->reorganizeCards(); } @@ -467,7 +473,14 @@ void PlayerEventHandler::eventAttachCard(const Event_AttachCard &event) CardState *oldParent = startCard->getAttachedTo(); - startCard->setAttachedTo(targetCard); + // Route through CardItem::setAttachedTo to keep the attachedCards graph, grid-x reset and zone emits intact: + // CardState::setAttachedTo alone only repoints the pointer, so the target would never list the card and the old + // parent would keep a dangling pointer. + if (CardItem *startItem = toCardItem(startCard)) { + startItem->setAttachedTo(toCardItem(targetCard)); + } else { + startCard->setAttachedTo(targetCard); + } startZone->reorganizeCards(); if ((startZone != targetZone) && targetZone) { diff --git a/cockatrice/src/game/player/player_logic.cpp b/cockatrice/src/game/player/player_logic.cpp index 4348a3756..f6a3a50e7 100644 --- a/cockatrice/src/game/player/player_logic.cpp +++ b/cockatrice/src/game/player/player_logic.cpp @@ -33,6 +33,11 @@ #include #include +static CardItem *toCardItem(CardState *card) +{ + return card == nullptr ? nullptr : qobject_cast(card->parent()); +} + PlayerLogic::PlayerLogic(const ServerInfo_User &info, int _id, bool _local, bool _judge, AbstractGame *_parent) : QObject(_parent), game(_parent), playerInfo(new PlayerInfo(info, _id, _local, _judge)), playerEventHandler(new PlayerEventHandler(this)), playerActions(new PlayerActions(this)), active(false), @@ -227,7 +232,13 @@ void PlayerLogic::processCardAttachment(const ServerInfo_Player &info) continue; } - startCard->setAttachedTo(targetCard); + // Route through CardItem::setAttachedTo to maintain the attachedCards graph and reparenting; + // CardState::setAttachedTo alone only repoints the pointer. + if (CardItem *startItem = toCardItem(startCard)) { + startItem->setAttachedTo(toCardItem(targetCard)); + } else { + startCard->setAttachedTo(targetCard); + } } } } diff --git a/cockatrice/src/game_graphics/board/card_item.cpp b/cockatrice/src/game_graphics/board/card_item.cpp index 4b8bbd3df..9a308ba51 100644 --- a/cockatrice/src/game_graphics/board/card_item.cpp +++ b/cockatrice/src/game_graphics/board/card_item.cpp @@ -71,6 +71,10 @@ CardItem::CardItem(PlayerLogic *_owner, update(); }); + // The retyped logic layer drives CardState setters directly (bypassing the CardItem wrappers that called + // update()), so repaint on every state change: attacking, annotation, doesntUntap, P/T and counters. + connect(state, &CardState::stateChanged, this, [this] { update(); }); + connect(state, &CardState::viewDeleteRequested, this, &CardItem::deleteLater); }