From 78964779c62893563da678a666adeac7ce0779ae Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Sat, 31 Oct 2026 18:44:15 +0100 Subject: [PATCH] [Refactor] Move card item creation out of game logic (#7436) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [Refactor] Move card item creation out of game logic * [Game] Rename signal to createCardRequested per review * [Game] Move attachment bookkeeping onto CardState CardState::setAttachedTo now maintains the attachedCards graph, resets the grid point and emits the zone events itself, so game logic no longer needs to reach into the CardItem graphics item. CardItem::setAttachedTo delegates to the state and keeps only the view teardown (deleteLater). --------- Co-authored-by: Lukas Brübach --- cockatrice/src/game/abstract_game.h | 1 - cockatrice/src/game/board/card_state.cpp | 28 +++++++ cockatrice/src/game/board/card_state.h | 10 +-- cockatrice/src/game/player/player_actions.cpp | 1 - .../src/game/player/player_event_handler.cpp | 83 ++++++++++--------- .../src/game/player/player_event_handler.h | 7 +- cockatrice/src/game/player/player_logic.cpp | 32 +++---- cockatrice/src/game/player/player_logic.h | 4 +- cockatrice/src/game/zones/card_zone_logic.cpp | 23 +++-- cockatrice/src/game/zones/card_zone_logic.h | 5 +- cockatrice/src/game/zones/view_zone_logic.cpp | 2 +- .../src/game_graphics/board/card_item.cpp | 49 ++++------- .../src/game_graphics/board/card_item.h | 6 +- cockatrice/src/game_graphics/game_scene.cpp | 12 ++- cockatrice/src/game_graphics/game_scene.h | 2 +- .../game_graphics/player/menu/player_menu.cpp | 8 +- .../game_graphics/player/menu/player_menu.h | 4 +- .../src/game_graphics/zones/card_zone.cpp | 9 ++ .../src/game_graphics/zones/card_zone.h | 3 + .../src/game_graphics/zones/table_zone.cpp | 7 +- 20 files changed, 159 insertions(+), 137 deletions(-) diff --git a/cockatrice/src/game/abstract_game.h b/cockatrice/src/game/abstract_game.h index 4bf8c120e..00a95a3b8 100644 --- a/cockatrice/src/game/abstract_game.h +++ b/cockatrice/src/game/abstract_game.h @@ -11,7 +11,6 @@ #include #include -class CardItem; class AbstractClient; class CardState; class GameEventHandler; diff --git a/cockatrice/src/game/board/card_state.cpp b/cockatrice/src/game/board/card_state.cpp index df7963e00..7e8c079a4 100644 --- a/cockatrice/src/game/board/card_state.cpp +++ b/cockatrice/src/game/board/card_state.cpp @@ -1,5 +1,7 @@ #include "card_state.h" +#include "../zones/card_zone_logic.h" + #include #include #include @@ -206,7 +208,33 @@ void CardState::setAttachedTo(CardState *_attachedTo) if (attachedTo == _attachedTo) { return; } + + if (attachedTo != nullptr) { + attachedTo->removeAttachedCard(this); + } + + setGridPoint(QPoint(-1, getGridPoint().y())); attachedTo = _attachedTo; emit attachedToChanged(_attachedTo); emit stateChanged(); + + if (attachedTo != nullptr) { + // If the zone is being torn down, it might already be null by the time a card tries to un-attach all its + // attached cards. The graphics item that owns this state handles the teardown in that case. + CardZoneLogic *parentZone = attachedTo->getZone(); + if (parentZone == nullptr) { + return; + } + emit parentZone->cardAdded(this); + attachedTo->addAttachedCard(this); + if (getZone() != parentZone) { + parentZone->reorganizeCards(); + } + } else if (getZone() != nullptr) { + emit getZone() -> cardAdded(this); + } + + if (getZone() != nullptr) { + getZone()->reorganizeCards(); + } } diff --git a/cockatrice/src/game/board/card_state.h b/cockatrice/src/game/board/card_state.h index bce5a298d..fa1e81393 100644 --- a/cockatrice/src/game/board/card_state.h +++ b/cockatrice/src/game/board/card_state.h @@ -12,8 +12,6 @@ #include class CardZoneLogic; -class CardItem; - class CardState : public QObject { Q_OBJECT @@ -35,7 +33,7 @@ private: QString color; CardState *attachedTo = nullptr; - QList attachedCards; + QList attachedCards; CardZoneLogic *zone = nullptr; signals: @@ -226,17 +224,17 @@ public: void setAttachedTo(CardState *_attachedTo); - void addAttachedCard(CardItem *card) + void addAttachedCard(CardState *card) { attachedCards.append(card); } - void removeAttachedCard(CardItem *card) + void removeAttachedCard(CardState *card) { attachedCards.removeOne(card); } - const QList &getAttachedCards() const + const QList &getAttachedCards() const { return attachedCards; } diff --git a/cockatrice/src/game/player/player_actions.cpp b/cockatrice/src/game/player/player_actions.cpp index 4ccf25ca4..b7e59b4e3 100644 --- a/cockatrice/src/game/player/player_actions.cpp +++ b/cockatrice/src/game/player/player_actions.cpp @@ -1,7 +1,6 @@ #include "player_actions.h" #include "../../client/settings/cache_settings.h" -#include "../../game_graphics/board/card_item.h" #include "../../game_graphics/dialogs/dlg_create_token.h" #include "../abstract_game.h" #include "../board/arrow_data.h" diff --git a/cockatrice/src/game/player/player_event_handler.cpp b/cockatrice/src/game/player/player_event_handler.cpp index 8b2ea0024..6588547da 100644 --- a/cockatrice/src/game/player/player_event_handler.cpp +++ b/cockatrice/src/game/player/player_event_handler.cpp @@ -1,9 +1,9 @@ #include "player_event_handler.h" -#include "../../game_graphics/board/card_item.h" #include "../abstract_game.h" #include "../board/arrow_data.h" #include "../board/card_list.h" +#include "../board/card_state.h" #include "../board/counter_state.h" #include "../zones/card_zone_logic.h" #include "../zones/hand_zone_logic.h" @@ -27,6 +27,9 @@ #include #include #include +#include +#include +#include #include #include #include @@ -49,15 +52,11 @@ #include #include #include +#include #include #include #include -static CardItem *toCardItem(CardState *card) -{ - return card == nullptr ? nullptr : qobject_cast(card->parent()); -} - PlayerEventHandler::PlayerEventHandler(PlayerLogic *_player) : QObject(_player), player(_player) { connect(this, &PlayerEventHandler::requestCardMenuUpdate, player, &PlayerLogic::requestCardMenuUpdate); @@ -160,23 +159,29 @@ void PlayerEventHandler::eventCreateToken(const Event_CreateToken &event) } CardRef cardRef = {QString::fromStdString(event.card_name()), QString::fromStdString(event.card_provider_id())}; - CardItem *card = new CardItem(player, nullptr, cardRef, event.card_id()); // use db PT if not provided in event and not face-down - if (!QString::fromStdString(event.pt()).isEmpty()) { - card->setPT(QString::fromStdString(event.pt())); - } else if (!event.face_down()) { - ExactCard dbCard = card->getCard(); + QString pt = QString::fromStdString(event.pt()); + if (pt.isEmpty() && !event.face_down()) { + ExactCard dbCard = CardDatabaseManager::query()->getCard(cardRef); if (dbCard) { - card->setPT(dbCard.getInfo().getPowTough()); + pt = dbCard.getInfo().getPowTough(); } } - card->setColor(QString::fromStdString(event.color())); - card->setAnnotation(QString::fromStdString(event.annotation())); - card->setDestroyOnZoneChange(event.destroy_on_zone_change()); - card->setFaceDown(event.face_down()); - emit logCreateToken(player, card->getName(), card->getPT(), card->getFaceDown()); - zone->addCard(card->getState(), true, event.x(), event.y()); + emit logCreateToken(player, cardRef.name, pt, event.face_down()); + + ServerInfo_Card cardInfo; + cardInfo.set_id(event.card_id()); + cardInfo.set_name(event.card_name()); + cardInfo.set_provider_id(event.card_provider_id()); + cardInfo.set_pt(pt.toStdString()); + cardInfo.set_color(event.color()); + cardInfo.set_annotation(event.annotation()); + cardInfo.set_destroy_on_zone_change(event.destroy_on_zone_change()); + cardInfo.set_face_down(event.face_down()); + cardInfo.set_x(event.x()); + cardInfo.set_y(event.y()); + emit zone->createCardRequested(cardInfo, true); } void PlayerEventHandler::eventSetCardAttr(const Event_SetCardAttr &event, @@ -276,7 +281,7 @@ void PlayerEventHandler::eventSetCardCounter(const Event_SetCardCounter &event) int oldValue = card->getCounters().value(event.counter_id(), 0); card->setCounter(event.counter_id(), event.counter_value()); - emit requestCardMenuUpdate(toCardItem(card)); + emit requestCardMenuUpdate(card); emit logSetCardCounter(player, card->getName(), event.counter_id(), event.counter_value(), oldValue); } @@ -362,22 +367,17 @@ void PlayerEventHandler::eventMoveCard(const Event_MoveCard &event, const GameEv if (card->getAttachedTo() && (startZone != targetZone)) { CardState *parentCard = card->getAttachedTo(); - // 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); - } + // CardState::setAttachedTo removes the card from the old parent's attachedCards list and emits the zone events. + card->setAttachedTo(nullptr); parentCard->getZone()->reorganizeCards(); } card->setId(event.new_card_id()); card->setFaceDown(event.face_down()); if (startZone != targetZone) { - const QList &attachedCards = card->getAttachedCards(); + const QList &attachedCards = card->getAttachedCards(); for (auto attachedCard : attachedCards) { - emit targetZone->cardAdded(attachedCard->getState()); + emit targetZone->cardAdded(attachedCard); } } @@ -391,8 +391,8 @@ void PlayerEventHandler::eventMoveCard(const Event_MoveCard &event, const GameEv targetZone->addCard(card, true, x, y); - emit cardZoneChanged(toCardItem(card), startZone == targetZone); - emit requestCardMenuUpdate(toCardItem(card)); + emit cardZoneChanged(card, startZone == targetZone); + emit requestCardMenuUpdate(card); if (player->getPlayerActions()->isMovingCardsUntil() && startZoneString == ZoneNames::DECK && targetZone->getName() == ZoneNames::STACK) { @@ -419,7 +419,7 @@ void PlayerEventHandler::eventFlipCard(const Event_FlipCard &event) emit logFlipCard(player, card->getName(), event.face_down()); card->setFaceDown(event.face_down()); - emit requestCardMenuUpdate(toCardItem(card)); + emit requestCardMenuUpdate(card); } void PlayerEventHandler::eventDestroyCard(const Event_DestroyCard &event) @@ -434,10 +434,18 @@ void PlayerEventHandler::eventDestroyCard(const Event_DestroyCard &event) return; } - QList attachedCards = card->getAttachedCards(); + QList attachedCards = card->getAttachedCards(); // This list is always empty except for buggy server implementations. for (auto &attachedCard : attachedCards) { + card->removeAttachedCard(attachedCard); + attachedCard->setGridPoint(QPoint(-1, attachedCard->getGridPoint().y())); attachedCard->setAttachedTo(nullptr); + if (attachedCard->getZone() == nullptr) { + attachedCard->deleteView(); + } else { + emit attachedCard->getZone()->cardAdded(attachedCard); + attachedCard->getZone()->reorganizeCards(); + } } emit logDestroyCard(player, card->getName()); @@ -473,14 +481,7 @@ void PlayerEventHandler::eventAttachCard(const Event_AttachCard &event) CardState *oldParent = startCard->getAttachedTo(); - // 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); - } + startCard->setAttachedTo(targetCard); startZone->reorganizeCards(); if ((startZone != targetZone) && targetZone) { @@ -495,7 +496,7 @@ void PlayerEventHandler::eventAttachCard(const Event_AttachCard &event) } else { emit logUnattachCard(player, startCard->getName()); } - emit requestCardMenuUpdate(toCardItem(startCard)); + emit requestCardMenuUpdate(startCard); } void PlayerEventHandler::eventDrawCards(const Event_DrawCards &event) diff --git a/cockatrice/src/game/player/player_event_handler.h b/cockatrice/src/game/player/player_event_handler.h index 6b721bf61..d253deef0 100644 --- a/cockatrice/src/game/player/player_event_handler.h +++ b/cockatrice/src/game/player/player_event_handler.h @@ -27,7 +27,6 @@ #include #include -class CardItem; class CardState; class CardZoneLogic; class PlayerLogic; @@ -62,7 +61,7 @@ template class QList; * Design notes: * - All event handlers assume events are authoritative and already validated * by the server. - * - Most handlers mutate both logical state (CardItem, CardZoneLogic, counters) + * - Most handlers mutate both logical state (CardState, CardZoneLogic, counters) * and visual/UI state (views, arrows, menus). * - Logging signals are emitted *after* or *during* state mutation, depending * on whether later mutations would invalidate log data. @@ -267,8 +266,8 @@ signals: void logAlwaysLookAtTopCard(PlayerLogic *player, CardZoneLogic *zone, bool reveal); /** @} */ - void cardZoneChanged(CardItem *card, bool sameZone); - void requestCardMenuUpdate(CardItem *card); + void cardZoneChanged(CardState *card, bool sameZone); + void requestCardMenuUpdate(CardState *card); /** * @brief A card was just moved by the server and its view needs refreshing. * diff --git a/cockatrice/src/game/player/player_logic.cpp b/cockatrice/src/game/player/player_logic.cpp index f6a3a50e7..ce4e8a95e 100644 --- a/cockatrice/src/game/player/player_logic.cpp +++ b/cockatrice/src/game/player/player_logic.cpp @@ -1,8 +1,9 @@ #include "player_logic.h" -#include "../../game_graphics/board/card_item.h" #include "../abstract_game.h" #include "../board/arrow_data.h" +#include "../board/card_list.h" +#include "../board/card_state.h" #include "../board/counter_state.h" #include "../game_meta_info.h" #include "../zones/hand_zone_logic.h" @@ -33,11 +34,6 @@ #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), @@ -177,22 +173,20 @@ void PlayerLogic::processPlayerInfo(const ServerInfo_Player &info) const int cardListSize = zoneInfo.card_list_size(); if (!cardListSize) { for (int j = 0; j < zoneInfo.card_count(); ++j) { - zone->addCard((new CardItem(this))->getState(), false, -1); + emit zone->createCardRequested(ServerInfo_Card(), false); } } else { for (int j = 0; j < cardListSize; ++j) { - const ServerInfo_Card &cardInfo = zoneInfo.card_list(j); - auto *card = new CardItem(this); - card->processCardInfo(cardInfo); + ServerInfo_Card cardInfo = zoneInfo.card_list(j); // Zones without coordinates (hand, piles, stack) preserve the order // they arrive in on the server in the positions of their cards list. // The x coordinate of such cards is always 0, so inserting at it // would reverse the list on reconnect. Append instead. - if (zoneInfo.with_coords()) { - zone->addCard(card->getState(), false, cardInfo.x(), cardInfo.y()); - } else { - zone->addCard(card->getState(), false, -1); + if (!zoneInfo.with_coords()) { + cardInfo.set_x(-1); + cardInfo.set_y(-1); } + emit zone->createCardRequested(cardInfo, false); } } if (zoneInfo.has_always_reveal_top_card()) { @@ -232,13 +226,7 @@ void PlayerLogic::processCardAttachment(const ServerInfo_Player &info) continue; } - // 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); - } + startCard->setAttachedTo(targetCard); } } } @@ -250,7 +238,7 @@ void PlayerLogic::processCardAttachment(const ServerInfo_Player &info) } } -void PlayerLogic::addCard(CardItem *card) +void PlayerLogic::addCard(AbstractCardItem *card) { emit newCardAdded(card); } diff --git a/cockatrice/src/game/player/player_logic.h b/cockatrice/src/game/player/player_logic.h index ceaeff10b..196ee8ebe 100644 --- a/cockatrice/src/game/player/player_logic.h +++ b/cockatrice/src/game/player/player_logic.h @@ -73,7 +73,7 @@ signals: /** @brief Emitted when the remote playmat (card/params) is updated from player properties. */ void playmatChanged(); void newCardAdded(AbstractCardItem *card); - void requestCardMenuUpdate(CardItem *card); + void requestCardMenuUpdate(CardState *card); void counterAdded(CounterState *state); void counterRemoved(int counterId); void rearrangeCounters(); @@ -103,7 +103,7 @@ public: void processPlayerInfo(const ServerInfo_Player &info); void processCardAttachment(const ServerInfo_Player &info); - void addCard(CardItem *c); + void addCard(AbstractCardItem *c); void deleteCard(CardState *c); bool clearCardsToDelete(); diff --git a/cockatrice/src/game/zones/card_zone_logic.cpp b/cockatrice/src/game/zones/card_zone_logic.cpp index 8cce9f957..522fb5e04 100644 --- a/cockatrice/src/game/zones/card_zone_logic.cpp +++ b/cockatrice/src/game/zones/card_zone_logic.cpp @@ -1,7 +1,9 @@ #include "card_zone_logic.h" #include "../../client/translation.h" -#include "../../game_graphics/board/card_item.h" +#include "../../game_graphics/zones/card_zone.h" +#include "../board/card_list.h" +#include "../board/card_state.h" #include "../player/player_actions.h" #include "../player/player_info.h" #include "../player/player_logic.h" @@ -50,10 +52,15 @@ void CardZoneLogic::addCard(CardState *card, const bool reorganize, const int x, for (auto *viewLogic : views) { if (viewLogic->prepareAddCard(x)) { - auto copy = new CardItem(player, nullptr, card->getCardRef(), card->getId()); - copy->setFaceDown(card->getFaceDown()); - - viewLogic->addCard(copy->getState(), reorganize, x, y); + ServerInfo_Card cardInfo; + const CardRef &cardRef = card->getCardRef(); + cardInfo.set_name(cardRef.name.toStdString()); + cardInfo.set_provider_id(cardRef.providerId.toStdString()); + cardInfo.set_id(card->getId()); + cardInfo.set_face_down(card->getFaceDown()); + cardInfo.set_x(x); + cardInfo.set_y(y); + emit viewLogic->createCardRequested(cardInfo, reorganize); } } @@ -168,9 +175,9 @@ void CardZoneLogic::clearContents() for (CardState *card : toClear) { // If an incorrectly implemented server doesn't return attached cards to whom they belong before dropping a // player, we have to return them to avoid a crash. - const QList &attachedCards = card->getAttachedCards(); - for (CardItem *attachedCard : attachedCards) { - emit attachedCard->getZone()->cardAdded(attachedCard->getState()); + const QList &attachedCards = card->getAttachedCards(); + for (CardState *attachedCard : attachedCards) { + emit attachedCard->getZone()->cardAdded(attachedCard); } } diff --git a/cockatrice/src/game/zones/card_zone_logic.h b/cockatrice/src/game/zones/card_zone_logic.h index 2a6b50c5c..327b78f61 100644 --- a/cockatrice/src/game/zones/card_zone_logic.h +++ b/cockatrice/src/game/zones/card_zone_logic.h @@ -14,10 +14,9 @@ #include #include #include +#include #include -class CardItem; - inline Q_LOGGING_CATEGORY(CardZoneLogicLog, "card_zone_logic"); class PlayerLogic; @@ -33,6 +32,8 @@ class CardZoneLogic : public QObject signals: void cardAdded(CardState *addedCard); + /** @brief Asks the graphics side to build the card item for @p cardInfo and add it to this zone. */ + void createCardRequested(const ServerInfo_Card &cardInfo, bool reorganize); void cardCountChanged(); void reorganizeCards(); void updateGraphics(); diff --git a/cockatrice/src/game/zones/view_zone_logic.cpp b/cockatrice/src/game/zones/view_zone_logic.cpp index a70b8e3d1..3fcf8d075 100644 --- a/cockatrice/src/game/zones/view_zone_logic.cpp +++ b/cockatrice/src/game/zones/view_zone_logic.cpp @@ -37,7 +37,7 @@ ZoneViewZoneLogic::ZoneViewZoneLogic(PlayerLogic *_player, * Note that this method can end up modifying the cardIds despite returning false. * (for example, if the card is inserted into a hidden portion of the deck while the view is reversed) * - * Make sure to call this method once before calling addCard(), so that you skip creating a new CardItem and calling + * Make sure to call this method once before calling addCard(), so that you skip creating the card and calling * addCard() if it's not required. * * @param x The position to insert the card at. diff --git a/cockatrice/src/game_graphics/board/card_item.cpp b/cockatrice/src/game_graphics/board/card_item.cpp index e56b5848c..8030331eb 100644 --- a/cockatrice/src/game_graphics/board/card_item.cpp +++ b/cockatrice/src/game_graphics/board/card_item.cpp @@ -89,14 +89,18 @@ void CardItem::prepareDelete() owner = nullptr; } - const QList &attachedCards = state->getAttachedCards(); - while (!attachedCards.isEmpty()) { - attachedCards.first()->setZone(nullptr); // so that it won't try to call reorganizeCards() - attachedCards.first()->setAttachedTo(nullptr); + const QList &attachedStates = state->getAttachedCards(); + while (!attachedStates.isEmpty()) { + auto *attachedItem = qobject_cast(attachedStates.first()->parent()); + if (attachedItem == nullptr) { + break; + } + attachedItem->setZone(nullptr); // so that it won't try to call reorganizeCards() + attachedItem->setAttachedTo(nullptr); } if (state->getAttachedTo() != nullptr) { - state->getAttachedTo()->removeAttachedCard(this); + state->getAttachedTo()->removeAttachedCard(state); state->setAttachedTo(nullptr); } } @@ -225,36 +229,13 @@ void CardItem::setPT(const QString &_pt) void CardItem::setAttachedTo(CardItem *_attachedTo) { - if (state->getAttachedTo() != nullptr) { - state->getAttachedTo()->removeAttachedCard(this); - } + CardState *newParent = _attachedTo == nullptr ? nullptr : _attachedTo->getState(); + state->setAttachedTo(newParent); - state->setGridPoint(QPoint(-1, state->getGridPoint().y())); - state->setAttachedTo(_attachedTo == nullptr ? nullptr : _attachedTo->getState()); - if (state->getAttachedTo() != nullptr) { - // If the zone is being torn down, it might already be null by the time a card tries to un-attach all its - // attached cards - if (state->getAttachedTo()->getZone() == nullptr) { - deleteLater(); - } else { - emit state->getAttachedTo()->getZone()->cardAdded(state); - state->getAttachedTo()->addAttachedCard(this); - if (state->getZone() != state->getAttachedTo()->getZone()) { - state->getAttachedTo()->getZone()->reorganizeCards(); - } - } - } else { - // If the zone is being torn down, it might already be null by the time a card tries to un-attach all its - // attached cards - if (state->getZone() == nullptr) { - deleteLater(); - } else { - emit state->getZone()->cardAdded(state); - } - } - - if (state->getZone() != nullptr) { - state->getZone()->reorganizeCards(); + // If the zone is being torn down, it might already be null by the time a card tries to un-attach all its attached + // cards + if (newParent != nullptr ? newParent->getZone() == nullptr : state->getZone() == nullptr) { + deleteLater(); } } diff --git a/cockatrice/src/game_graphics/board/card_item.h b/cockatrice/src/game_graphics/board/card_item.h index 79c3dcdbd..706c00195 100644 --- a/cockatrice/src/game_graphics/board/card_item.h +++ b/cockatrice/src/game_graphics/board/card_item.h @@ -120,15 +120,15 @@ public: return state->getAttachedTo(); } void setAttachedTo(CardItem *_attachedTo); - void addAttachedCard(CardItem *card) + void addAttachedCard(CardState *card) { state->addAttachedCard(card); } - void removeAttachedCard(CardItem *card) + void removeAttachedCard(CardState *card) { state->removeAttachedCard(card); } - [[nodiscard]] const QList &getAttachedCards() const + [[nodiscard]] const QList &getAttachedCards() const { return state->getAttachedCards(); } diff --git a/cockatrice/src/game_graphics/game_scene.cpp b/cockatrice/src/game_graphics/game_scene.cpp index 3da4ba7cb..e7362871f 100644 --- a/cockatrice/src/game_graphics/game_scene.cpp +++ b/cockatrice/src/game_graphics/game_scene.cpp @@ -205,7 +205,7 @@ void GameScene::onCardSelectionChanged(AbstractCardItem *abstractCard, bool sele auto *owner = card->getOwner(); if (selected) { - owner->requestCardMenuUpdate(card); + owner->requestCardMenuUpdate(card->getState()); return; } @@ -231,7 +231,7 @@ void GameScene::onCardRightClicked(AbstractCardItem *abstractCard, QPoint screen setActiveCard(card); - if (auto *menu = view->getPlayerMenu()->updateCardMenu(card)) { + if (auto *menu = view->getPlayerMenu()->updateCardMenu(card->getState())) { menu->popup(screenPos); } } @@ -650,11 +650,15 @@ void GameScene::requestArrowDeletion(int playerId, int arrowId) } } -void GameScene::onCardZoneChanged(CardItem *card, bool sameZone) +void GameScene::onCardZoneChanged(CardState *card, bool sameZone) { + auto *item = qobject_cast(card->parent()); + if (!item) { + return; + } QList toDelete; for (auto *arrow : arrowRegistry.all()) { - if (arrow->getStartItem() == card || arrow->getTargetItem() == card) { + if (arrow->getStartItem() == item || arrow->getTargetItem() == item) { if (sameZone) { arrow->updatePath(); } else { diff --git a/cockatrice/src/game_graphics/game_scene.h b/cockatrice/src/game_graphics/game_scene.h index 5856d718d..3f7d40ed0 100644 --- a/cockatrice/src/game_graphics/game_scene.h +++ b/cockatrice/src/game_graphics/game_scene.h @@ -283,7 +283,7 @@ public slots: /// Queues up arrow deletion but doesn't directly modify the scene void requestArrowDeletion(int playerId, int arrowId); - void onCardZoneChanged(CardItem *card, bool sameZone); + void onCardZoneChanged(CardState *card, bool sameZone); /** * @brief Refreshes the view of a card that the server just moved. * diff --git a/cockatrice/src/game_graphics/player/menu/player_menu.cpp b/cockatrice/src/game_graphics/player/menu/player_menu.cpp index 7e69df66e..10e7a7ddc 100644 --- a/cockatrice/src/game_graphics/player/menu/player_menu.cpp +++ b/cockatrice/src/game_graphics/player/menu/player_menu.cpp @@ -87,22 +87,24 @@ void PlayerMenu::setMenusForGraphicItems() } } -QMenu *PlayerMenu::updateCardMenu(CardItem *card) +QMenu *PlayerMenu::updateCardMenu(CardState *card) { if (!card) { emit cardMenuUpdated(nullptr); return nullptr; } + auto *cardItem = qobject_cast(card->parent()); + // If is spectator (as spectators don't need card menus), return // only update the menu if the card is actually selected if ((player->getLogic()->getGame()->getPlayerManager()->isSpectator() && !player->getLogic()->getGame()->getPlayerManager()->isJudge()) || - player->getGameScene()->getActiveCard() != card) { + cardItem == nullptr || player->getGameScene()->getActiveCard() != cardItem) { return nullptr; } - CardMenu *menu = new CardMenu(player, card, shortcutsActive); + CardMenu *menu = new CardMenu(player, cardItem, shortcutsActive); connect(menu, &CardMenu::cardInfoRequested, this, &PlayerMenu::cardInfoRequested); emit cardMenuUpdated(menu); diff --git a/cockatrice/src/game_graphics/player/menu/player_menu.h b/cockatrice/src/game_graphics/player/menu/player_menu.h index 27a5efe7e..e21a689de 100644 --- a/cockatrice/src/game_graphics/player/menu/player_menu.h +++ b/cockatrice/src/game_graphics/player/menu/player_menu.h @@ -14,7 +14,7 @@ #include #include -class CardItem; +class CardState; class CardMenu; class PlayerGraphicsItem; class AbstractPlayerComponent; @@ -43,7 +43,7 @@ signals: public slots: void setMenusForGraphicItems(); - QMenu *updateCardMenu(CardItem *card); + QMenu *updateCardMenu(CardState *card); private slots: void refreshShortcuts(); diff --git a/cockatrice/src/game_graphics/zones/card_zone.cpp b/cockatrice/src/game_graphics/zones/card_zone.cpp index b29ad3666..0f91c10b8 100644 --- a/cockatrice/src/game_graphics/zones/card_zone.cpp +++ b/cockatrice/src/game_graphics/zones/card_zone.cpp @@ -24,6 +24,7 @@ CardZone::CardZone(CardZoneLogic *_logic, QGraphicsItem *parent) connect(logic, &CardZoneLogic::cardCountChanged, this, &CardZone::invalidateItems); connect(logic, &CardZoneLogic::reorganizeCards, this, &CardZone::invalidateItems); connect(logic, &CardZoneLogic::cardAdded, this, &CardZone::onCardAdded); + connect(logic, &CardZoneLogic::createCardRequested, this, &CardZone::onCreateCardRequested); connect(logic, &CardZoneLogic::setGraphicsVisibility, this, [this](bool v) { this->setVisible(v); }); connect(logic, &CardZoneLogic::updateGraphics, this, [this]() { update(); }); connect(logic, &CardZoneLogic::reorganizeCards, this, &CardZone::reorganizeCards); @@ -59,6 +60,14 @@ void CardZone::onCardAdded(CardState *addedCard) addedItem->update(); } +void CardZone::onCreateCardRequested(const ServerInfo_Card &cardInfo, bool reorganize) +{ + CardRef cardRef = {QString::fromStdString(cardInfo.name()), QString::fromStdString(cardInfo.provider_id())}; + auto *card = new CardItem(getLogic()->getPlayer(), nullptr, cardRef, cardInfo.id()); + card->processCardInfo(cardInfo); + getLogic()->addCard(card->getState(), reorganize, cardInfo.x(), cardInfo.y()); +} + void CardZone::retranslateUi() { for (auto *card : cardItems()) { diff --git a/cockatrice/src/game_graphics/zones/card_zone.h b/cockatrice/src/game_graphics/zones/card_zone.h index 7568cf25f..ea77cb057 100644 --- a/cockatrice/src/game_graphics/zones/card_zone.h +++ b/cockatrice/src/game_graphics/zones/card_zone.h @@ -22,6 +22,7 @@ class QAction; class QGraphicsItem; class QMenu; class QPoint; +class ServerInfo_Card; template class QList; class CardItem; @@ -59,6 +60,8 @@ public slots: * connection in CardZone's constructor dispatches through the vtable. */ virtual void onCardAdded(CardState *addedCard); + /** @brief Builds the card item for @p cardInfo and hands it to this zone's logic. */ + void onCreateCardRequested(const ServerInfo_Card &cardInfo, bool reorganize); public: enum diff --git a/cockatrice/src/game_graphics/zones/table_zone.cpp b/cockatrice/src/game_graphics/zones/table_zone.cpp index 2fcff1d8a..8cb209c5a 100644 --- a/cockatrice/src/game_graphics/zones/table_zone.cpp +++ b/cockatrice/src/game_graphics/zones/table_zone.cpp @@ -248,11 +248,14 @@ void TableZone::reorganizeCards() cardItems()[i]->setPos(actualX, actualY); cardItems()[i]->setRealZValue(ZValues::tableCardZValue(actualX, actualY)); - QListIterator attachedCardIterator(cardItems()[i]->getAttachedCards()); + QListIterator attachedCardIterator(cardItems()[i]->getAttachedCards()); int j = 0; while (attachedCardIterator.hasNext()) { ++j; - CardItem *attachedCard = attachedCardIterator.next(); + CardItem *attachedCard = qobject_cast(attachedCardIterator.next()->parent()); + if (attachedCard == nullptr) { + continue; + } qreal childX = actualX - j * STACKED_CARD_OFFSET_X; qreal childY = y + 5; attachedCard->setPos(childX, childY);