From 956c12eb32fc2c3d79c430a7fbe803dfc5005437 Mon Sep 17 00:00:00 2001
From: RickyRister <42636155+RickyRister@users.noreply.github.com>
Date: Thu, 26 Dec 2024 13:49:17 -0800
Subject: [PATCH] remove shortcut workaround; always add card menu to player
 (#5337)

* remove workaround

* make aCardMenu less innocuous

* make card menus active for all players
---
 cockatrice/src/game/player/player.cpp | 26 ++++++++------------------
 cockatrice/src/game/player/player.h   |  3 ++-
 2 files changed, 10 insertions(+), 19 deletions(-)

diff --git a/cockatrice/src/game/player/player.cpp b/cockatrice/src/game/player/player.cpp
index 286eb7c5b..7fb36c3f3 100644
--- a/cockatrice/src/game/player/player.cpp
+++ b/cockatrice/src/game/player/player.cpp
@@ -426,14 +426,10 @@ Player::Player(const ServerInfo_User &info, int _id, bool _local, bool _judge, T
         initSayMenu();
     }
 
-    if (local || judge) {
-        aCardMenu = new QAction(this);
-        aCardMenu->setEnabled(false);
-        playerMenu->addSeparator();
-        playerMenu->addAction(aCardMenu);
-    } else {
-        aCardMenu = nullptr;
-    }
+    aCardMenu = new QAction(this);
+    aCardMenu->setEnabled(false);
+    playerMenu->addSeparator();
+    playerMenu->addAction(aCardMenu);
 
     if (local || judge) {
 
@@ -833,13 +829,13 @@ void Player::retranslateUi()
             counterIterator.next().value()->retranslateUi();
         }
 
-        aCardMenu->setText(tr("Selec&ted cards"));
-
         for (auto &allPlayersAction : allPlayersActions) {
             allPlayersAction->setText(tr("&All players"));
         }
     }
 
+    aCardMenu->setText(tr("Selec&ted cards"));
+
     if (local) {
         sayMenu->setTitle(tr("S&ay"));
     }
@@ -1005,15 +1001,9 @@ void Player::setShortcutsActive()
     aPlayFacedown->setShortcut(shortcuts.getSingleShortcut("Player/aPlayFacedown"));
     aPlay->setShortcut(shortcuts.getSingleShortcut("Player/aPlay"));
 
+    // Don't enable always-active shortcuts in local games, since it causes keyboard shortcuts to work inconsistently
+    // when there are more than 1 player.
     if (!game->getIsLocalGame()) {
-        /* Attach all card menu actions that also work on the opponent's cards to the TabGame object so that those
-         * shortcuts will be active regardless of whether your card menu currently exists.
-         * Added as a workaround to get the clone keyboard shortcut to work on opponent's cards.
-         */
-        game->addAction(aClone);
-        game->addAction(aDrawArrow);
-        game->addAction(aSelectAll);
-
         // unattach action is only active in card menu if the active card is attached.
         // make unattach shortcut always active so that it consistently works when multiple cards are selected.
         game->addAction(aUnattach);
diff --git a/cockatrice/src/game/player/player.h b/cockatrice/src/game/player/player.h
index fdd491d83..8ef06ca30 100644
--- a/cockatrice/src/game/player/player.h
+++ b/cockatrice/src/game/player/player.h
@@ -260,10 +260,11 @@ private:
         *aMoveTopCardToExile, *aMoveTopCardsToGraveyard, *aMoveTopCardsToExile, *aMoveTopCardsUntil,
         *aMoveTopCardToBottom, *aViewGraveyard, *aViewRfg, *aViewSideboard, *aDrawCard, *aDrawCards, *aUndoDraw,
         *aMulligan, *aShuffle, *aMoveTopToPlay, *aMoveTopToPlayFaceDown, *aUntapAll, *aRollDie, *aCreateToken,
-        *aCreateAnotherToken, *aCardMenu, *aMoveBottomToPlay, *aMoveBottomToPlayFaceDown, *aMoveBottomCardToTop,
+        *aCreateAnotherToken, *aMoveBottomToPlay, *aMoveBottomToPlayFaceDown, *aMoveBottomCardToTop,
         *aMoveBottomCardToGraveyard, *aMoveBottomCardToExile, *aMoveBottomCardsToGraveyard, *aMoveBottomCardsToExile,
         *aDrawBottomCard, *aDrawBottomCards;
 
+    QAction *aCardMenu;
     QList<QAction *> 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,