extract cardMenu from CardItem

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) {
+        if (owner->getGame()->getActiveCard() == this) {
-            owner->setCardMenu(nullptr);
+            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,10 +407,14 @@ void CardItem::handleClickedToPlay(bool shiftHeld)
 void CardItem::mouseReleaseEvent(QGraphicsSceneMouseEvent *event)
 {
     if (event->button() == Qt::RightButton) {
-        if (cardMenu != nullptr && !cardMenu->isEmpty() && owner != nullptr) {
+
+        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())) {
         handleClickedToPlay(event->modifiers().testFlag(Qt::ShiftModifier));
@@ -477,11 +466,8 @@ QVariant CardItem::itemChange(GraphicsItemChange change, const QVariant &value)
 {
     if ((change == ItemSelectedHasChanged) && owner != nullptr) {
         if (value == true) {
-            owner->setCardMenu(cardMenu);
+            owner->updateCardMenu(this);
             owner->getGame()->setActiveCard(this);
-        } else if (owner->getCardMenu() == cardMenu) {
-            owner->setCardMenu(nullptr);
-            owner->getGame()->setActiveCard(nullptr);
         }
     }
     return AbstractCardItem::itemChange(change, value);

cockatrice/src/game/board/card_item.h

@@ -33,8 +33,6 @@ private:
     CardItem *attachedTo;
     QList<CardItem *> 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();

cockatrice/src/game/player/player.cpp

@@ -3897,20 +3897,14 @@ 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;
+        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<ZoneViewZone *>(card->getZone())) {
@@ -3923,6 +3917,8 @@ void Player::updateCardMenu(const CardItem *card)
         }
     }
 
+    QMenu *cardMenu = new QMenu;
+
     if (revealedCard) {
         cardMenu->addAction(aHide);
         cardMenu->addAction(aClone);
@@ -3933,18 +3929,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
@@ -3960,23 +3944,7 @@ void Player::updateCardMenu(const CardItem *card)
                     cardMenu->addSeparator();
                     cardMenu->addAction(aSelectAll);
                     cardMenu->addAction(aSelectRow);
-                    return;
+                    return cardMenu;
-                }
-
-                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);
                 }
 
                 cardMenu->addAction(aTap);
@@ -3996,11 +3964,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);
@@ -4024,7 +3992,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 {
@@ -4045,7 +4013,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);
@@ -4075,7 +4043,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") {
@@ -4096,7 +4064,7 @@ void Player::updateCardMenu(const CardItem *card)
                 }
             }
         } else {
-            cardMenu->addMenu(moveMenu);
+            cardMenu->addMenu(createMoveMenu());
         }
     } else {
         if (card->getZone() && card->getZone()->getName() != "hand") {
@@ -4110,6 +4078,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)
@@ -4219,14 +4223,22 @@ 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.
+ *
+ * @param card The card to create the menu for. Pass nullptr to disable the card menu.
+ * @return The new card menu
+ */
+QMenu *Player::updateCardMenu(const CardItem *card)
 {
-    if (aCardMenu != nullptr) {
+    QMenu *menu = createCardMenu(card);
+
     aCardMenu->setEnabled(menu != nullptr);
     if (menu) {
         aCardMenu->setMenu(menu);
     }
-    }
+
+    return menu;
 }
 
 QMenu *Player::getCardMenu() const

cockatrice/src/game/player/player.h

@@ -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,9 @@ public:
     {
         return arrows;
     }
-    void setCardMenu(QMenu *menu);
+    QMenu *updateCardMenu(const CardItem *card);
     QMenu *getCardMenu() const;
-    void updateCardMenu(const CardItem *card);
+    QMenu *createCardMenu(const CardItem *card);
     bool getActive() const
     {
         return active;