From 03bebbe4c2c7118334dbbc532c9b40a1b296569b Mon Sep 17 00:00:00 2001 From: RickyRister <42636155+RickyRister@users.noreply.github.com> Date: Fri, 15 Aug 2025 02:13:28 -0300 Subject: [PATCH] Rework card menu handling (#6069) * extract cardMenu from CardItem * move cardMenu saving to TabGame * delete TabGame::updateCardMenu * move checking to updateCardMenu * unset activeCard when all cards are unselected --- cockatrice/src/client/tabs/tab_game.cpp | 29 ++--- cockatrice/src/client/tabs/tab_game.h | 3 +- cockatrice/src/game/board/card_item.cpp | 35 +++--- cockatrice/src/game/board/card_item.h | 17 +-- cockatrice/src/game/player/player.cpp | 136 ++++++++++++------------ cockatrice/src/game/player/player.h | 9 +- 6 files changed, 106 insertions(+), 123 deletions(-) diff --git a/cockatrice/src/client/tabs/tab_game.cpp b/cockatrice/src/client/tabs/tab_game.cpp index bfe9f5244..d639c242c 100644 --- a/cockatrice/src/client/tabs/tab_game.cpp +++ b/cockatrice/src/client/tabs/tab_game.cpp @@ -261,6 +261,10 @@ void TabGame::retranslateUi() sayLabel->setText(tr("&Say:")); } + if (aCardMenu) { + aCardMenu->setText(tr("Selected cards")); + } + viewMenu->setTitle(tr("&View")); cardInfoDockMenu->setTitle(tr("Card Info")); messageLayoutDockMenu->setTitle(tr("Messages")); @@ -576,6 +580,7 @@ Player *TabGame::addPlayer(int playerId, const ServerInfo_User &info) scene->addPlayer(newPlayer); connect(newPlayer, &Player::newCardAdded, this, &TabGame::newCardAdded); + connect(newPlayer, &Player::cardMenuUpdated, this, &TabGame::setCardMenu); messageLog->connectToPlayer(newPlayer); if (local && !spectator) { @@ -1213,22 +1218,17 @@ Player *TabGame::getActiveLocalPlayer() const void TabGame::setActiveCard(CardItem *card) { activeCard = card; - updateCardMenu(card); } -void TabGame::updateCardMenu(AbstractCardItem *card) +/** + * @param menu The menu to set. Pass in nullptr to set the menu to empty. + */ +void TabGame::setCardMenu(QMenu *menu) { - if (card == nullptr) { - return; - } - Player *player; - if ((clients.size() > 1) || !players.contains(localPlayerId)) { - player = card->getOwner(); + if (menu) { + aCardMenu->setMenu(menu); } else { - player = players.value(localPlayerId); - } - if (player != nullptr) { - player->updateCardMenu(static_cast(card)); + aCardMenu->setMenu(new QMenu); } } @@ -1285,6 +1285,11 @@ void TabGame::createMenuItems() gameMenu->addAction(aConcede); gameMenu->addAction(aFocusChat); gameMenu->addAction(aLeaveGame); + + gameMenu->addSeparator(); + + aCardMenu = gameMenu->addMenu(new QMenu(this)); + addTabMenu(gameMenu); } diff --git a/cockatrice/src/client/tabs/tab_game.h b/cockatrice/src/client/tabs/tab_game.h index 7927d44a6..96e902c2e 100644 --- a/cockatrice/src/client/tabs/tab_game.h +++ b/cockatrice/src/client/tabs/tab_game.h @@ -118,6 +118,7 @@ private: *aPlayerListDockVisible, *aPlayerListDockFloating, *aReplayDockVisible, *aReplayDockFloating; QAction *aFocusChat; QList phaseActions; + QAction *aCardMenu; Player *addPlayer(int playerId, const ServerInfo_User &info); @@ -171,7 +172,7 @@ private slots: void incrementGameTime(); void adminLockChanged(bool lock); void newCardAdded(AbstractCardItem *card); - void updateCardMenu(AbstractCardItem *card); + void setCardMenu(QMenu *menu); void actGameInfo(); void actConcede(); diff --git a/cockatrice/src/game/board/card_item.cpp b/cockatrice/src/game/board/card_item.cpp index c1d50b2ba..d0336e4e5 100644 --- a/cockatrice/src/game/board/card_item.cpp +++ b/cockatrice/src/game/board/card_item.cpp @@ -24,30 +24,17 @@ CardItem::CardItem(Player *_owner, QGraphicsItem *parent, const CardRef &cardRef { owner->addCard(this); - cardMenu = new QMenu; - ptMenu = new QMenu; - moveMenu = new QMenu; - connect(&SettingsCache::instance().cardCounters(), &CardCounterSettings::colorChanged, this, [this](int counterId) { if (counters.contains(counterId)) update(); }); - - retranslateUi(); -} - -CardItem::~CardItem() -{ - delete cardMenu; - delete ptMenu; - delete moveMenu; } void CardItem::prepareDelete() { if (owner != nullptr) { - if (owner->getCardMenu() == cardMenu) { - owner->setCardMenu(nullptr); + if (owner->getGame()->getActiveCard() == this) { + owner->updateCardMenu(nullptr); owner->getGame()->setActiveCard(nullptr); } owner = nullptr; @@ -79,8 +66,6 @@ void CardItem::setZone(CardZone *_zone) void CardItem::retranslateUi() { - moveMenu->setTitle(tr("&Move to")); - ptMenu->setTitle(tr("&Power / toughness")); } void CardItem::paint(QPainter *painter, const QStyleOptionGraphicsItem *option, QWidget *widget) @@ -422,9 +407,13 @@ void CardItem::handleClickedToPlay(bool shiftHeld) void CardItem::mouseReleaseEvent(QGraphicsSceneMouseEvent *event) { if (event->button() == Qt::RightButton) { - if (cardMenu != nullptr && !cardMenu->isEmpty() && owner != nullptr) { - cardMenu->popup(event->screenPos()); - return; + + if (owner != nullptr) { + owner->getGame()->setActiveCard(this); + if (QMenu *cardMenu = owner->updateCardMenu(this)) { + cardMenu->popup(event->screenPos()); + return; + } } } else if ((event->modifiers() != Qt::AltModifier) && (event->button() == Qt::LeftButton) && (!SettingsCache::instance().getDoubleClickToPlay())) { @@ -477,11 +466,11 @@ QVariant CardItem::itemChange(GraphicsItemChange change, const QVariant &value) { if ((change == ItemSelectedHasChanged) && owner != nullptr) { if (value == true) { - owner->setCardMenu(cardMenu); owner->getGame()->setActiveCard(this); - } else if (owner->getCardMenu() == cardMenu) { - owner->setCardMenu(nullptr); + owner->updateCardMenu(this); + } else if (owner->scene()->selectedItems().isEmpty()) { owner->getGame()->setActiveCard(nullptr); + owner->updateCardMenu(nullptr); } } return AbstractCardItem::itemChange(change, value); diff --git a/cockatrice/src/game/board/card_item.h b/cockatrice/src/game/board/card_item.h index 565c51fbc..569079092 100644 --- a/cockatrice/src/game/board/card_item.h +++ b/cockatrice/src/game/board/card_item.h @@ -33,8 +33,6 @@ private: CardItem *attachedTo; QList attachedCards; - QMenu *cardMenu, *ptMenu, *moveMenu; - void prepareDelete(); void handleClickedToPlay(bool shiftHeld); public slots: @@ -54,7 +52,7 @@ public: const CardRef &cardRef = {}, int _cardid = -1, CardZone *_zone = nullptr); - ~CardItem() override; + void retranslateUi(); CardZone *getZone() const { @@ -135,19 +133,6 @@ public: void resetState(bool keepAnnotations = false); void processCardInfo(const ServerInfo_Card &_info); - QMenu *getCardMenu() const - { - return cardMenu; - } - QMenu *getPTMenu() const - { - return ptMenu; - } - QMenu *getMoveMenu() const - { - return moveMenu; - } - bool animationEvent(); CardDragItem *createDragItem(int _id, const QPointF &_pos, const QPointF &_scenePos, bool faceDown); void deleteDragItem(); diff --git a/cockatrice/src/game/player/player.cpp b/cockatrice/src/game/player/player.cpp index 776acfbf7..008d13e40 100644 --- a/cockatrice/src/game/player/player.cpp +++ b/cockatrice/src/game/player/player.cpp @@ -449,11 +449,6 @@ Player::Player(const ServerInfo_User &info, int _id, bool _local, bool _judge, T initSayMenu(); } - aCardMenu = new QAction(this); - aCardMenu->setEnabled(false); - playerMenu->addSeparator(); - playerMenu->addAction(aCardMenu); - if (local || judge) { for (auto &playerList : playerLists) { @@ -870,8 +865,6 @@ void Player::retranslateUi() } } - aCardMenu->setText(tr("Selec&ted cards")); - if (local) { sayMenu->setTitle(tr("S&ay")); } @@ -3904,20 +3897,12 @@ void Player::refreshShortcuts() } } -void Player::updateCardMenu(const CardItem *card) +QMenu *Player::createCardMenu(const CardItem *card) { - // If bad card OR is a spectator (as spectators don't need card menus), return - // only update the menu if the card is actually selected - if (card == nullptr || (game->isSpectator() && !judge) || game->getActiveCard() != card) { - return; + if (card == nullptr) { + return nullptr; } - QMenu *cardMenu = card->getCardMenu(); - QMenu *ptMenu = card->getPTMenu(); - QMenu *moveMenu = card->getMoveMenu(); - - cardMenu->clear(); - bool revealedCard = false; bool writeableCard = getLocalOrJudge(); if (auto *view = qobject_cast(card->getZone())) { @@ -3930,6 +3915,8 @@ void Player::updateCardMenu(const CardItem *card) } } + QMenu *cardMenu = new QMenu; + if (revealedCard) { cardMenu->addAction(aHide); cardMenu->addAction(aClone); @@ -3940,18 +3927,6 @@ void Player::updateCardMenu(const CardItem *card) } else if (writeableCard) { bool canModifyCard = judge || card->getOwner() == this; - if (moveMenu->isEmpty() && canModifyCard) { - moveMenu->addAction(aMoveToTopLibrary); - moveMenu->addAction(aMoveToXfromTopOfLibrary); - moveMenu->addAction(aMoveToBottomLibrary); - moveMenu->addSeparator(); - moveMenu->addAction(aMoveToHand); - moveMenu->addSeparator(); - moveMenu->addAction(aMoveToGraveyard); - moveMenu->addSeparator(); - moveMenu->addAction(aMoveToExile); - } - if (card->getZone()) { if (card->getZone()->getName() == "table") { // Card is on the battlefield @@ -3967,23 +3942,7 @@ void Player::updateCardMenu(const CardItem *card) cardMenu->addSeparator(); cardMenu->addAction(aSelectAll); cardMenu->addAction(aSelectRow); - return; - } - - if (ptMenu->isEmpty()) { - ptMenu->addAction(aIncP); - ptMenu->addAction(aDecP); - ptMenu->addAction(aFlowP); - ptMenu->addSeparator(); - ptMenu->addAction(aIncT); - ptMenu->addAction(aDecT); - ptMenu->addAction(aFlowT); - ptMenu->addSeparator(); - ptMenu->addAction(aIncPT); - ptMenu->addAction(aDecPT); - ptMenu->addSeparator(); - ptMenu->addAction(aSetPT); - ptMenu->addAction(aResetPT); + return cardMenu; } cardMenu->addAction(aTap); @@ -4003,11 +3962,11 @@ void Player::updateCardMenu(const CardItem *card) } cardMenu->addAction(aDrawArrow); cardMenu->addSeparator(); - cardMenu->addMenu(ptMenu); + cardMenu->addMenu(createPtMenu()); cardMenu->addAction(aSetAnnotation); cardMenu->addSeparator(); cardMenu->addAction(aClone); - cardMenu->addMenu(moveMenu); + cardMenu->addMenu(createMoveMenu()); cardMenu->addSeparator(); cardMenu->addAction(aSelectAll); cardMenu->addAction(aSelectRow); @@ -4031,7 +3990,7 @@ void Player::updateCardMenu(const CardItem *card) cardMenu->addAction(aDrawArrow); cardMenu->addSeparator(); cardMenu->addAction(aClone); - cardMenu->addMenu(moveMenu); + cardMenu->addMenu(createMoveMenu()); cardMenu->addSeparator(); cardMenu->addAction(aSelectAll); } else { @@ -4052,7 +4011,7 @@ void Player::updateCardMenu(const CardItem *card) cardMenu->addSeparator(); cardMenu->addAction(aClone); - cardMenu->addMenu(moveMenu); + cardMenu->addMenu(createMoveMenu()); cardMenu->addSeparator(); cardMenu->addAction(aSelectAll); cardMenu->addAction(aSelectColumn); @@ -4082,7 +4041,7 @@ void Player::updateCardMenu(const CardItem *card) cardMenu->addSeparator(); cardMenu->addAction(aClone); - cardMenu->addMenu(moveMenu); + cardMenu->addMenu(createMoveMenu()); // actions that are really wonky when done from deck or sideboard if (card->getZone()->getName() == "hand") { @@ -4103,7 +4062,7 @@ void Player::updateCardMenu(const CardItem *card) } } } else { - cardMenu->addMenu(moveMenu); + cardMenu->addMenu(createMoveMenu()); } } else { if (card->getZone() && card->getZone()->getName() != "hand") { @@ -4117,6 +4076,42 @@ void Player::updateCardMenu(const CardItem *card) cardMenu->addAction(aSelectAll); } } + + return cardMenu; +} + +QMenu *Player::createMoveMenu() const +{ + QMenu *moveMenu = new QMenu("Move to"); + moveMenu->addAction(aMoveToTopLibrary); + moveMenu->addAction(aMoveToXfromTopOfLibrary); + moveMenu->addAction(aMoveToBottomLibrary); + moveMenu->addSeparator(); + moveMenu->addAction(aMoveToHand); + moveMenu->addSeparator(); + moveMenu->addAction(aMoveToGraveyard); + moveMenu->addSeparator(); + moveMenu->addAction(aMoveToExile); + return moveMenu; +} + +QMenu *Player::createPtMenu() const +{ + QMenu *ptMenu = new QMenu("Power / toughness"); + ptMenu->addAction(aIncP); + ptMenu->addAction(aDecP); + ptMenu->addAction(aFlowP); + ptMenu->addSeparator(); + ptMenu->addAction(aIncT); + ptMenu->addAction(aDecT); + ptMenu->addAction(aFlowT); + ptMenu->addSeparator(); + ptMenu->addAction(aIncPT); + ptMenu->addAction(aDecPT); + ptMenu->addSeparator(); + ptMenu->addAction(aSetPT); + ptMenu->addAction(aResetPT); + return ptMenu; } void Player::addRelatedCardView(const CardItem *card, QMenu *cardMenu) @@ -4226,23 +4221,30 @@ void Player::addRelatedCardActions(const CardItem *card, QMenu *cardMenu) } } -void Player::setCardMenu(QMenu *menu) +/** + * Creates a card menu from the given card and sets it as the currently active card menu. + * Will first check if the card should have a card menu, and no-ops if not. + * + * @param card The card to create the menu for. Pass nullptr to disable the card menu. + * @return The new card menu, or nullptr if failed. + */ +QMenu *Player::updateCardMenu(const CardItem *card) { - if (aCardMenu != nullptr) { - aCardMenu->setEnabled(menu != nullptr); - if (menu) { - aCardMenu->setMenu(menu); - } - } -} - -QMenu *Player::getCardMenu() const -{ - if (aCardMenu != nullptr) { - return aCardMenu->menu(); - } else { + if (!card) { + emit cardMenuUpdated(nullptr); return nullptr; } + + // If is spectator (as spectators don't need card menus), return + // only update the menu if the card is actually selected + if ((game->isSpectator() && !judge) || game->getActiveCard() != card) { + return nullptr; + } + + QMenu *menu = createCardMenu(card); + emit cardMenuUpdated(menu); + + return menu; } QString Player::getName() const diff --git a/cockatrice/src/game/player/player.h b/cockatrice/src/game/player/player.h index 3efad6b77..b4f901329 100644 --- a/cockatrice/src/game/player/player.h +++ b/cockatrice/src/game/player/player.h @@ -156,6 +156,7 @@ signals: void sizeChanged(); void playerCountChanged(); + void cardMenuUpdated(QMenu *cardMenu); public slots: void actUntapAll(); void actRollDie(); @@ -271,7 +272,6 @@ private: *aMoveBottomToPlayFaceDown, *aMoveBottomCardToTop, *aMoveBottomCardToGraveyard, *aMoveBottomCardToExile, *aMoveBottomCardsToGraveyard, *aMoveBottomCardsToExile, *aDrawBottomCard, *aDrawBottomCards; - QAction *aCardMenu; QList aAddCounter, aSetCounter, aRemoveCounter; QAction *aPlay, *aPlayFacedown, *aHide, *aTap, *aDoesntUntap, *aAttach, *aUnattach, *aDrawArrow, *aSetPT, *aResetPT, *aIncP, *aDecP, *aIncT, *aDecT, *aIncPT, *aDecPT, *aFlowP, *aFlowT, *aSetAnnotation, *aFlip, *aPeek, *aClone, @@ -326,6 +326,8 @@ private: const QString &avalue, bool allCards, EventProcessingOptions options); + QMenu *createMoveMenu() const; + QMenu *createPtMenu() const; void addRelatedCardActions(const CardItem *card, QMenu *cardMenu); void addRelatedCardView(const CardItem *card, QMenu *cardMenu); void createCard(const CardItem *sourceCard, @@ -484,9 +486,8 @@ public: { return arrows; } - void setCardMenu(QMenu *menu); - QMenu *getCardMenu() const; - void updateCardMenu(const CardItem *card); + QMenu *updateCardMenu(const CardItem *card); + QMenu *createCardMenu(const CardItem *card); bool getActive() const { return active;