From edb5caec8a77a9f2dfe111264abf1aa60aa5bf8e Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Sat, 31 Oct 2026 18:44:13 +0100 Subject: [PATCH] [Refactor] Hold zone views and card view teardown outside game graphics (#7429) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Lukas Brübach --- .../src/game/player/player_event_handler.cpp | 25 ++++++------------- .../src/game/player/player_event_handler.h | 9 +++++++ cockatrice/src/game/zones/card_zone_logic.cpp | 19 ++++++-------- cockatrice/src/game/zones/card_zone_logic.h | 10 +++++--- cockatrice/src/game/zones/view_zone_logic.cpp | 5 ++++ cockatrice/src/game/zones/view_zone_logic.h | 7 ++++++ cockatrice/src/game_graphics/game_scene.cpp | 21 ++++++++++++++++ cockatrice/src/game_graphics/game_scene.h | 9 +++++++ .../src/game_graphics/zones/view_zone.cpp | 18 +++++++------ 9 files changed, 84 insertions(+), 39 deletions(-) diff --git a/cockatrice/src/game/player/player_event_handler.cpp b/cockatrice/src/game/player/player_event_handler.cpp index 55e5d9d8f..ff81498ac 100644 --- a/cockatrice/src/game/player/player_event_handler.cpp +++ b/cockatrice/src/game/player/player_event_handler.cpp @@ -1,7 +1,6 @@ #include "player_event_handler.h" #include "../../game_graphics/board/card_item.h" -#include "../../game_graphics/zones/view_zone.h" #include "../abstract_game.h" #include "../board/arrow_data.h" #include "../board/card_list.h" @@ -9,6 +8,7 @@ #include "../zones/card_zone_logic.h" #include "../zones/hand_zone_logic.h" #include "../zones/pile_zone_logic.h" +#include "../zones/view_zone_logic.h" #include "libcockatrice/card/card_info.h" #include "libcockatrice/card/printing/exact_card.h" #include "libcockatrice/protocol/pb/card_attributes.pb.h" @@ -76,12 +76,12 @@ void PlayerEventHandler::eventShuffle(const Event_Shuffle &event) } // close all views that contain shuffled cards - for (ZoneViewZone *view : zone->getViews()) { - if (view != nullptr) { - int length = view->getLogic()->getCards().length(); + for (auto *viewLogic : zone->getViews()) { + if (viewLogic != nullptr) { + int length = viewLogic->getCards().length(); // we want to close empty views as well if (length == 0 || length > absStart) { // note this assumes views always start at the top of the library - view->close(); + viewLogic->requestClose(); } } else { qWarning() << zone->getName() << "of" << player->getPlayerInfo()->getName() << "holds empty zoneview!"; @@ -345,9 +345,9 @@ void PlayerEventHandler::eventMoveCard(const Event_MoveCard &event, const GameEv if (card == nullptr) { return; } - if (startZone != targetZone) { - card->deleteCardInfoPopup(); - } + const bool zoneChanged = startZone != targetZone; + const bool ownerChanged = zoneChanged && startZone->getPlayer() != targetZone->getPlayer(); + emit cardViewRefreshRequested(card->getState(), zoneChanged, ownerChanged ? targetZone->getPlayer() : nullptr); if (event.has_card_name()) { QString name = QString::fromStdString(event.card_name()); QString providerId = @@ -361,22 +361,13 @@ void PlayerEventHandler::eventMoveCard(const Event_MoveCard &event, const GameEv parentCard->getZone()->reorganizeCards(); } - card->deleteDragItem(); - card->setId(event.new_card_id()); card->setFaceDown(event.face_down()); if (startZone != targetZone) { - card->setBeingPointedAt(false); - card->setHovered(false); - const QList &attachedCards = card->getAttachedCards(); for (auto attachedCard : attachedCards) { emit targetZone->cardAdded(attachedCard); } - - if (startZone->getPlayer() != targetZone->getPlayer()) { - card->setOwner(targetZone->getPlayer()); - } } // The log event has to be sent before the card is added to the target zone diff --git a/cockatrice/src/game/player/player_event_handler.h b/cockatrice/src/game/player/player_event_handler.h index 44b43c3f1..37215d61d 100644 --- a/cockatrice/src/game/player/player_event_handler.h +++ b/cockatrice/src/game/player/player_event_handler.h @@ -28,6 +28,7 @@ #include class CardItem; +class CardState; class CardZoneLogic; class PlayerLogic; class Event_AttachCard; @@ -268,6 +269,14 @@ signals: void cardZoneChanged(CardItem *card, bool sameZone); void requestCardMenuUpdate(CardItem *card); + /** + * @brief A card was just moved by the server and its view needs refreshing. + * + * @param card The card that moved. + * @param zoneChanged Whether the card crossed a zone boundary. + * @param newOwner The player the card now belongs to, or nullptr to leave the owner alone. + */ + void cardViewRefreshRequested(CardState *card, bool zoneChanged, PlayerLogic *newOwner); private: /** Owning player instance. */ diff --git a/cockatrice/src/game/zones/card_zone_logic.cpp b/cockatrice/src/game/zones/card_zone_logic.cpp index d997ab9ad..c4431f20a 100644 --- a/cockatrice/src/game/zones/card_zone_logic.cpp +++ b/cockatrice/src/game/zones/card_zone_logic.cpp @@ -2,9 +2,6 @@ #include "../../client/translation.h" #include "../../game_graphics/board/card_item.h" -#include "../../game_graphics/zones/card_zone.h" -#include "../../game_graphics/zones/view_zone.h" -#include "../board/card_list.h" #include "../player/player_actions.h" #include "../player/player_info.h" #include "../player/player_logic.h" @@ -47,16 +44,16 @@ CardZoneLogic::CardZoneLogic(PlayerLogic *_player, void CardZoneLogic::addCard(CardItem *card, const bool reorganize, const int x, const int y) { if (!card) { - qCWarning(CardZoneLog) << "CardZoneLogic::addCard() card is null; this shouldn't normally happen"; + qCWarning(CardZoneLogicLog) << "CardZoneLogic::addCard() card is null; this shouldn't normally happen"; return; } - for (auto *view : views) { - if (qobject_cast(view->getLogic())->prepareAddCard(x)) { + for (auto *viewLogic : views) { + if (viewLogic->prepareAddCard(x)) { auto copy = new CardItem(player, nullptr, card->getCardRef(), card->getId()); copy->setFaceDown(card->getFaceDown()); - view->getLogic()->addCard(copy, reorganize, x, y); + viewLogic->addCard(copy, reorganize, x, y); } } @@ -90,8 +87,8 @@ CardItem *CardZoneLogic::takeCard(int position, int cardId, bool toNewZone) return nullptr; } - for (auto *view : views) { - qobject_cast(view->getLogic())->removeCard(position, toNewZone); + for (auto *viewLogic : views) { + viewLogic->removeCard(position, toNewZone); } CardItem *c = cards.takeAt(position); @@ -107,7 +104,7 @@ CardItem *CardZoneLogic::getCard(int cardId) { CardItem *c = cards.findCard(cardId); if (!c) { - qCWarning(CardZoneLog) << "CardZoneLogic::getCard: card id=" << cardId << "not found"; + qCWarning(CardZoneLogicLog) << "CardZoneLogic::getCard: card id=" << cardId << "not found"; return nullptr; } // If the card's id is -1, this zone is invisible, @@ -122,7 +119,7 @@ CardItem *CardZoneLogic::getCard(int cardId) void CardZoneLogic::removeCard(CardItem *card) { if (!card) { - qCWarning(CardZoneLog) << "CardZoneLogic::removeCard: card is null, this shouldn't normally happen"; + qCWarning(CardZoneLogicLog) << "CardZoneLogic::removeCard: card is null, this shouldn't normally happen"; return; } diff --git a/cockatrice/src/game/zones/card_zone_logic.h b/cockatrice/src/game/zones/card_zone_logic.h index acd290d71..e01eec74c 100644 --- a/cockatrice/src/game/zones/card_zone_logic.h +++ b/cockatrice/src/game/zones/card_zone_logic.h @@ -21,7 +21,11 @@ class CardItem; inline Q_LOGGING_CATEGORY(CardZoneLogicLog, "card_zone_logic"); class PlayerLogic; -class ZoneViewZone; +class ZoneViewZoneLogic; +class QMenu; +class QAction; +class QPainter; +class CardDragItem; class CardZoneLogic : public QObject { @@ -79,7 +83,7 @@ public: { return cards.getContentsKnown(); } - QList &getViews() + QList &getViews() { return views; } @@ -111,7 +115,7 @@ protected: PlayerLogic *player; QString name; CardList cards; - QList views; + QList views; bool hasCardAttr; bool isShufflable; bool alwaysRevealTopCard; diff --git a/cockatrice/src/game/zones/view_zone_logic.cpp b/cockatrice/src/game/zones/view_zone_logic.cpp index b7d953549..f5e597abe 100644 --- a/cockatrice/src/game/zones/view_zone_logic.cpp +++ b/cockatrice/src/game/zones/view_zone_logic.cpp @@ -162,6 +162,11 @@ void ZoneViewZoneLogic::removeCard(int position, bool toNewZone) reorganizeCards(); } +void ZoneViewZoneLogic::requestClose() +{ + emit closeView(); +} + void ZoneViewZoneLogic::clearCards() { for (auto card : cards) { diff --git a/cockatrice/src/game/zones/view_zone_logic.h b/cockatrice/src/game/zones/view_zone_logic.h index 1abcd0516..71261e204 100644 --- a/cockatrice/src/game/zones/view_zone_logic.h +++ b/cockatrice/src/game/zones/view_zone_logic.h @@ -48,6 +48,13 @@ public: void removeCard(int position, bool toNewZone); void updateCardIds(CardAction action); + /** + * @brief Asks the graphics view showing this zone to close itself. + * + * Logic code has no handle on the view, so it goes through the logic object the view was built from. + */ + void requestClose(); + /** @brief Removes all cards from the view, without touching anything else. Used in replay rewind */ void clearCards(); diff --git a/cockatrice/src/game_graphics/game_scene.cpp b/cockatrice/src/game_graphics/game_scene.cpp index 31188293d..f54e3ae28 100644 --- a/cockatrice/src/game_graphics/game_scene.cpp +++ b/cockatrice/src/game_graphics/game_scene.cpp @@ -291,6 +291,8 @@ void GameScene::addPlayer(PlayerLogic *player) [this, id = player->getPlayerInfo()->getId()]() { clearArrowsForPlayerLocally(id); }); connect(player->getPlayerEventHandler(), &PlayerEventHandler::cardZoneChanged, this, &GameScene::onCardZoneChanged); + connect(player->getPlayerEventHandler(), &PlayerEventHandler::cardViewRefreshRequested, this, + &GameScene::onCardViewRefreshRequested); rearrange(); } @@ -650,6 +652,25 @@ void GameScene::onCardZoneChanged(CardItem *card, bool sameZone) } } +void GameScene::onCardViewRefreshRequested(CardState *card, bool zoneChanged, PlayerLogic *newOwner) +{ + auto *item = qobject_cast(card->parent()); + if (!item) { + return; + } + if (zoneChanged) { + item->deleteCardInfoPopup(); + } + item->deleteDragItem(); + if (zoneChanged) { + item->setBeingPointedAt(false); + item->setHovered(false); + } + if (newOwner) { + item->setOwner(newOwner); + } +} + void GameScene::clearArrowsForPlayer(int playerId) { for (int arrowId : arrowRegistry.idsForPlayer(playerId)) { diff --git a/cockatrice/src/game_graphics/game_scene.h b/cockatrice/src/game_graphics/game_scene.h index f0e94e14e..ca6441e91 100644 --- a/cockatrice/src/game_graphics/game_scene.h +++ b/cockatrice/src/game_graphics/game_scene.h @@ -43,6 +43,7 @@ class ZoneViewWidget; class CardZone; class AbstractCardItem; class CardItem; +class CardState; class ServerInfo_Card; class PhasesToolbar; class QBasicTimer; @@ -282,6 +283,14 @@ public slots: void requestArrowDeletion(int playerId, int arrowId); void onCardZoneChanged(CardItem *card, bool sameZone); + /** + * @brief Refreshes the view of a card that the server just moved. + * + * @param card The card that moved. + * @param zoneChanged Whether the card crossed a zone boundary. + * @param newOwner The player the card now belongs to, or nullptr to leave the owner alone. + */ + void onCardViewRefreshRequested(CardState *card, bool zoneChanged, PlayerLogic *newOwner); protected: /** @brief Handles hover updates. */ diff --git a/cockatrice/src/game_graphics/zones/view_zone.cpp b/cockatrice/src/game_graphics/zones/view_zone.cpp index 78fc662b6..7d47b3259 100644 --- a/cockatrice/src/game_graphics/zones/view_zone.cpp +++ b/cockatrice/src/game_graphics/zones/view_zone.cpp @@ -40,21 +40,23 @@ class QGraphicsItem; ZoneViewZone::ZoneViewZone(ZoneViewZoneLogic *_logic, QGraphicsItem *parent) : SelectZone(_logic, parent), bRect(QRectF()), minRows(0), groupBy(CardList::NoSort), sortBy(CardList::NoSort) { - if (!(qobject_cast(getLogic())->getRevealZone() && - !qobject_cast(getLogic())->getWriteableRevealZone())) { - qobject_cast(getLogic())->getOriginalZone()->getViews().append(this); + auto *viewLogic = qobject_cast(getLogic()); + if (!(viewLogic->getRevealZone() && !viewLogic->getWriteableRevealZone())) { + viewLogic->getOriginalZone()->getViews().append(viewLogic); } connect(_logic, &ZoneViewZoneLogic::closeView, this, &ZoneViewZone::close); } void ZoneViewZone::addToViews() { - qobject_cast(getLogic())->getOriginalZone()->getViews().append(this); + auto *viewLogic = qobject_cast(getLogic()); + viewLogic->getOriginalZone()->getViews().append(viewLogic); } void ZoneViewZone::removeFromViews() { - qobject_cast(getLogic())->getOriginalZone()->getViews().removeOne(this); + auto *viewLogic = qobject_cast(getLogic()); + viewLogic->getOriginalZone()->getViews().removeOne(viewLogic); } /** @@ -64,9 +66,9 @@ void ZoneViewZone::removeFromViews() void ZoneViewZone::close() { emit closed(); - if (!(qobject_cast(getLogic())->getRevealZone() && - !qobject_cast(getLogic())->getWriteableRevealZone())) { - qobject_cast(getLogic())->getOriginalZone()->getViews().removeOne(this); + auto *viewLogic = qobject_cast(getLogic()); + if (!(viewLogic->getRevealZone() && !viewLogic->getWriteableRevealZone())) { + viewLogic->getOriginalZone()->getViews().removeOne(viewLogic); } deleteLater(); }