From f8b06f28d19906cdd74ee97bc5f91bab1a241209 Mon Sep 17 00:00:00 2001 From: DawnFire42 Date: Mon, 16 Mar 2026 20:00:57 -0400 Subject: [PATCH] refactor: extract AbstractPlayerComponent interface for polymorphic player component management. Non-QObject polymorphic interface with setShortcutsActive(), setShortcutsInactive(), and retranslateUi(). Uses regular multiple inheritance to avoid diamond inheritance with Qt's MOC. All zone menus, SayMenu, and AbstractCounter implement this interface. PlayerMenu manages them via a managedComponents list with two template helpers (addManagedMenu/registerManagedComponent), replacing individual if-guarded lifecycle calls with a single polymorphic loop. SayMenu now owns its shortcut and translation lifecycle instead of having PlayerMenu manage its title and shortcuts externally. Counters are iterated via Player::getCounters() rather than managedComponents to avoid duplicating the authoritative owner's map. --- cockatrice/src/game/board/abstract_counter.h | 9 +- .../player/menu/abstract_player_component.h | 32 +++++++ .../src/game/player/menu/custom_zone_menu.h | 12 ++- cockatrice/src/game/player/menu/grave_menu.h | 9 +- cockatrice/src/game/player/menu/hand_menu.h | 9 +- .../src/game/player/menu/library_menu.h | 9 +- .../src/game/player/menu/player_menu.cpp | 89 ++++--------------- cockatrice/src/game/player/menu/player_menu.h | 29 ++++-- cockatrice/src/game/player/menu/rfg_menu.h | 11 ++- cockatrice/src/game/player/menu/say_menu.cpp | 34 ++++++- cockatrice/src/game/player/menu/say_menu.h | 11 ++- .../src/game/player/menu/sideboard_menu.h | 10 ++- .../src/game/player/menu/utility_menu.h | 10 ++- 13 files changed, 164 insertions(+), 110 deletions(-) create mode 100644 cockatrice/src/game/player/menu/abstract_player_component.h diff --git a/cockatrice/src/game/board/abstract_counter.h b/cockatrice/src/game/board/abstract_counter.h index ea13cb00f..074650d54 100644 --- a/cockatrice/src/game/board/abstract_counter.h +++ b/cockatrice/src/game/board/abstract_counter.h @@ -8,6 +8,7 @@ #define COUNTER_H #include "../../interface/widgets/menus/tearoff_menu.h" +#include "../player/menu/abstract_player_component.h" #include #include @@ -18,7 +19,7 @@ class QKeyEvent; class QMenu; class QString; -class AbstractCounter : public QObject, public QGraphicsItem +class AbstractCounter : public QObject, public QGraphicsItem, public AbstractPlayerComponent { Q_OBJECT Q_INTERFACES(QGraphicsItem) @@ -56,10 +57,10 @@ public: QGraphicsItem *parent = nullptr); ~AbstractCounter() override; - void retranslateUi(); + void retranslateUi() override; void setValue(int _value); - void setShortcutsActive(); - void setShortcutsInactive(); + void setShortcutsActive() override; + void setShortcutsInactive() override; void delCounter(); QMenu *getMenu() const diff --git a/cockatrice/src/game/player/menu/abstract_player_component.h b/cockatrice/src/game/player/menu/abstract_player_component.h new file mode 100644 index 000000000..989300d41 --- /dev/null +++ b/cockatrice/src/game/player/menu/abstract_player_component.h @@ -0,0 +1,32 @@ +/** + * @file abstract_player_component.h + * @ingroup GameMenusPlayers + * @brief Polymorphic interface for player-bound UI components managed by PlayerMenu. + */ + +#ifndef COCKATRICE_ABSTRACT_PLAYER_COMPONENT_H +#define COCKATRICE_ABSTRACT_PLAYER_COMPONENT_H + +/** + * @brief Interface for player-bound UI components that need shortcut and translation lifecycle management. + * + * Not a QObject — avoids diamond inheritance with Qt's MOC. Each concrete component + * inherits QObject through its Qt base class (QMenu, TearOffMenu, QGraphicsItem, etc.) + * and this interface through regular multiple inheritance. + */ +class AbstractPlayerComponent +{ +public: + virtual ~AbstractPlayerComponent() = default; + + /// Bind keyboard shortcuts. Called when this player gains focus. + virtual void setShortcutsActive() = 0; + + /// Unbind keyboard shortcuts. Called when this player loses focus. + virtual void setShortcutsInactive() = 0; + + /// Retranslate all user-visible strings. Called on language change. + virtual void retranslateUi() = 0; +}; + +#endif // COCKATRICE_ABSTRACT_PLAYER_COMPONENT_H diff --git a/cockatrice/src/game/player/menu/custom_zone_menu.h b/cockatrice/src/game/player/menu/custom_zone_menu.h index 0944029f4..c4e66754e 100644 --- a/cockatrice/src/game/player/menu/custom_zone_menu.h +++ b/cockatrice/src/game/player/menu/custom_zone_menu.h @@ -7,15 +7,23 @@ #ifndef COCKATRICE_CUSTOM_ZONE_MENU_H #define COCKATRICE_CUSTOM_ZONE_MENU_H +#include "abstract_player_component.h" + #include class Player; -class CustomZoneMenu : public QMenu +class CustomZoneMenu : public QMenu, public AbstractPlayerComponent { Q_OBJECT public: explicit CustomZoneMenu(Player *player); - void retranslateUi(); + void retranslateUi() override; + void setShortcutsActive() override + { + } + void setShortcutsInactive() override + { + } private: Player *player; diff --git a/cockatrice/src/game/player/menu/grave_menu.h b/cockatrice/src/game/player/menu/grave_menu.h index faaf497b6..429173afa 100644 --- a/cockatrice/src/game/player/menu/grave_menu.h +++ b/cockatrice/src/game/player/menu/grave_menu.h @@ -8,12 +8,13 @@ #define COCKATRICE_GRAVE_MENU_H #include "../../../interface/widgets/menus/tearoff_menu.h" +#include "abstract_player_component.h" #include #include class Player; -class GraveyardMenu : public TearOffMenu +class GraveyardMenu : public TearOffMenu, public AbstractPlayerComponent { Q_OBJECT signals: @@ -25,9 +26,9 @@ public: void createViewActions(); void populateRevealRandomMenuWithActivePlayers(); void onRevealRandomTriggered(); - void retranslateUi(); - void setShortcutsActive(); - void setShortcutsInactive(); + void retranslateUi() override; + void setShortcutsActive() override; + void setShortcutsInactive() override; QMenu *mRevealRandomGraveyardCard = nullptr; QMenu *moveGraveMenu = nullptr; diff --git a/cockatrice/src/game/player/menu/hand_menu.h b/cockatrice/src/game/player/menu/hand_menu.h index 51e071a62..76434cc98 100644 --- a/cockatrice/src/game/player/menu/hand_menu.h +++ b/cockatrice/src/game/player/menu/hand_menu.h @@ -8,6 +8,7 @@ #define COCKATRICE_HAND_MENU_H #include "../../../interface/widgets/menus/tearoff_menu.h" +#include "abstract_player_component.h" #include #include @@ -15,7 +16,7 @@ class Player; class PlayerActions; -class HandMenu : public TearOffMenu +class HandMenu : public TearOffMenu, public AbstractPlayerComponent { Q_OBJECT @@ -31,9 +32,9 @@ public: return mRevealRandomHandCard; } - void retranslateUi(); - void setShortcutsActive(); - void setShortcutsInactive(); + void retranslateUi() override; + void setShortcutsActive() override; + void setShortcutsInactive() override; private slots: void populateRevealHandMenuWithActivePlayers(); diff --git a/cockatrice/src/game/player/menu/library_menu.h b/cockatrice/src/game/player/menu/library_menu.h index c0883107c..444e8f516 100644 --- a/cockatrice/src/game/player/menu/library_menu.h +++ b/cockatrice/src/game/player/menu/library_menu.h @@ -8,6 +8,7 @@ #define COCKATRICE_LIBRARY_MENU_H #include "../../../interface/widgets/menus/tearoff_menu.h" +#include "abstract_player_component.h" #include #include @@ -15,7 +16,7 @@ class Player; class PlayerActions; -class LibraryMenu : public TearOffMenu +class LibraryMenu : public TearOffMenu, public AbstractPlayerComponent { Q_OBJECT public slots: @@ -28,15 +29,15 @@ public: void createShuffleActions(); void createMoveActions(); void createViewActions(); - void retranslateUi(); + void retranslateUi() override; void populateRevealLibraryMenuWithActivePlayers(); void populateLendLibraryMenuWithActivePlayers(); void populateRevealTopCardMenuWithActivePlayers(); void onRevealLibraryTriggered(); void onLendLibraryTriggered(); void onRevealTopCardTriggered(); - void setShortcutsActive(); - void setShortcutsInactive(); + void setShortcutsActive() override; + void setShortcutsInactive() override; [[nodiscard]] bool isAlwaysRevealTopCardChecked() const { diff --git a/cockatrice/src/game/player/menu/player_menu.cpp b/cockatrice/src/game/player/menu/player_menu.cpp index 3016a727f..7786ec3fc 100644 --- a/cockatrice/src/game/player/menu/player_menu.cpp +++ b/cockatrice/src/game/player/menu/player_menu.cpp @@ -15,33 +15,24 @@ PlayerMenu::PlayerMenu(Player *_player) : player(_player) playerMenu = new TearOffMenu(); if (player->getPlayerInfo()->getLocalOrJudge()) { - handMenu = new HandMenu(player, player->getPlayerActions(), playerMenu); - playerMenu->addMenu(handMenu); - - libraryMenu = new LibraryMenu(player, playerMenu); - playerMenu->addMenu(libraryMenu); + handMenu = addManagedMenu(player, player->getPlayerActions(), playerMenu); + libraryMenu = addManagedMenu(player, playerMenu); } else { handMenu = nullptr; libraryMenu = nullptr; } - graveMenu = new GraveyardMenu(player, playerMenu); - playerMenu->addMenu(graveMenu); - - rfgMenu = new RfgMenu(player, playerMenu); - playerMenu->addMenu(rfgMenu); + graveMenu = addManagedMenu(player, playerMenu); + rfgMenu = addManagedMenu(player, playerMenu); if (player->getPlayerInfo()->getLocalOrJudge()) { - sideboardMenu = new SideboardMenu(player, playerMenu); - playerMenu->addMenu(sideboardMenu); - - customZonesMenu = new CustomZoneMenu(player); - playerMenu->addMenu(customZonesMenu); + sideboardMenu = addManagedMenu(player, playerMenu); + customZonesMenu = addManagedMenu(player); playerMenu->addSeparator(); countersMenu = playerMenu->addMenu(QString()); - utilityMenu = new UtilityMenu(player, playerMenu); + utilityMenu = createManagedComponent(player, playerMenu); } else { sideboardMenu = nullptr; customZonesMenu = nullptr; @@ -50,8 +41,7 @@ PlayerMenu::PlayerMenu(Player *_player) : player(_player) } if (player->getPlayerInfo()->getLocal()) { - sayMenu = new SayMenu(player); - playerMenu->addMenu(sayMenu); + sayMenu = addManagedMenu(player); } else { sayMenu = nullptr; } @@ -99,40 +89,18 @@ void PlayerMenu::retranslateUi() { playerMenu->setTitle(tr("Player \"%1\"").arg(player->getPlayerInfo()->getName())); - if (handMenu) { - handMenu->retranslateUi(); - } - if (libraryMenu) { - libraryMenu->retranslateUi(); - } - - graveMenu->retranslateUi(); - rfgMenu->retranslateUi(); - - if (sideboardMenu) { - sideboardMenu->retranslateUi(); + for (auto *component : managedComponents) { + component->retranslateUi(); } if (countersMenu) { countersMenu->setTitle(tr("&Counters")); } - if (customZonesMenu) { - customZonesMenu->retranslateUi(); - } - QMapIterator counterIterator(player->getCounters()); while (counterIterator.hasNext()) { counterIterator.next().value()->retranslateUi(); } - - if (utilityMenu) { - utilityMenu->retranslateUi(); - } - - if (sayMenu) { - sayMenu->setTitle(tr("S&ay")); - } } void PlayerMenu::refreshShortcuts() @@ -153,52 +121,29 @@ void PlayerMenu::setShortcutsActive() { shortcutsActive = true; - if (handMenu) { - handMenu->setShortcutsActive(); - } - if (libraryMenu) { - libraryMenu->setShortcutsActive(); - } - graveMenu->setShortcutsActive(); - // No shortcuts for RfgMenu yet - - if (sideboardMenu) { - sideboardMenu->setShortcutsActive(); + for (auto *component : managedComponents) { + component->setShortcutsActive(); } + // Counters implement AbstractPlayerComponent but are iterated via Player::counters + // (the authoritative source) rather than managedComponents to avoid a redundant + // list that must stay in sync with the map. QMapIterator counterIterator(player->getCounters()); while (counterIterator.hasNext()) { counterIterator.next().value()->setShortcutsActive(); } - - if (utilityMenu) { - utilityMenu->setShortcutsActive(); - } } void PlayerMenu::setShortcutsInactive() { shortcutsActive = false; - if (handMenu) { - handMenu->setShortcutsInactive(); - } - if (libraryMenu) { - libraryMenu->setShortcutsInactive(); - } - graveMenu->setShortcutsInactive(); - // No shortcuts for RfgMenu yet - - if (sideboardMenu) { - sideboardMenu->setShortcutsInactive(); + for (auto *component : managedComponents) { + component->setShortcutsInactive(); } QMapIterator counterIterator(player->getCounters()); while (counterIterator.hasNext()) { counterIterator.next().value()->setShortcutsInactive(); } - - if (utilityMenu) { - utilityMenu->setShortcutsInactive(); - } } \ No newline at end of file diff --git a/cockatrice/src/game/player/menu/player_menu.h b/cockatrice/src/game/player/menu/player_menu.h index 882bfedc5..d97bd2f96 100644 --- a/cockatrice/src/game/player/menu/player_menu.h +++ b/cockatrice/src/game/player/menu/player_menu.h @@ -1,7 +1,7 @@ /** * @file player_menu.h * @ingroup GameMenusPlayers - * @brief TODO: Document this. + * @brief Orchestrates lifecycle management for all player-bound UI components. */ #ifndef COCKATRICE_PLAYER_MENU_H @@ -18,6 +18,7 @@ #include "sideboard_menu.h" #include "utility_menu.h" +#include #include #include @@ -37,6 +38,7 @@ private slots: public: PlayerMenu(Player *player); + // Lifecycle methods: delegate to all managedComponents, plus counters separately via player->getCounters(). void retranslateUi(); QMenu *updateCardMenu(const CardItem *card); @@ -66,8 +68,8 @@ public: return shortcutsActive; } - void setShortcutsActive(); - void setShortcutsInactive(); + void setShortcutsActive(); // Delegates to all managedComponents, plus counters separately. + void setShortcutsInactive(); // Delegates to all managedComponents, plus counters separately. private: Player *player; @@ -82,9 +84,26 @@ private: SayMenu *sayMenu; CustomZoneMenu *customZonesMenu; - bool shortcutsActive; + // Drives AbstractPlayerComponent lifecycle delegation. Counters are iterated separately via player->getCounters(). + QList managedComponents; + bool shortcutsActive = false; - void initSayMenu(); + // Creates component, adds it as a submenu of playerMenu, and registers in managedComponents. + template MenuT *addManagedMenu(Args &&...args) + { + auto *menu = new MenuT(std::forward(args)...); + playerMenu->addMenu(menu); + managedComponents.append(menu); + return menu; + } + + // Creates component and registers in managedComponents, but does NOT add it as a submenu. + template ComponentT *createManagedComponent(Args &&...args) + { + auto *component = new ComponentT(std::forward(args)...); + managedComponents.append(component); + return component; + } }; #endif // COCKATRICE_PLAYER_MENU_H diff --git a/cockatrice/src/game/player/menu/rfg_menu.h b/cockatrice/src/game/player/menu/rfg_menu.h index 0b4623d2a..8f79b2f4a 100644 --- a/cockatrice/src/game/player/menu/rfg_menu.h +++ b/cockatrice/src/game/player/menu/rfg_menu.h @@ -8,19 +8,26 @@ #define COCKATRICE_RFG_MENU_H #include "../../../interface/widgets/menus/tearoff_menu.h" +#include "abstract_player_component.h" #include #include class Player; -class RfgMenu : public TearOffMenu +class RfgMenu : public TearOffMenu, public AbstractPlayerComponent { Q_OBJECT public: explicit RfgMenu(Player *player, QWidget *parent = nullptr); void createMoveActions(); void createViewActions(); - void retranslateUi(); + void retranslateUi() override; + void setShortcutsActive() override + { + } + void setShortcutsInactive() override + { + } QMenu *moveRfgMenu = nullptr; diff --git a/cockatrice/src/game/player/menu/say_menu.cpp b/cockatrice/src/game/player/menu/say_menu.cpp index 3c4802aa5..116fba49a 100644 --- a/cockatrice/src/game/player/menu/say_menu.cpp +++ b/cockatrice/src/game/player/menu/say_menu.cpp @@ -8,6 +8,31 @@ SayMenu::SayMenu(Player *_player) : player(_player) { connect(&SettingsCache::instance().messages(), &MessageSettings::messageMacrosChanged, this, &SayMenu::initSayMenu); initSayMenu(); + retranslateUi(); +} + +void SayMenu::retranslateUi() +{ + setTitle(tr("S&ay")); +} + +void SayMenu::setShortcutsActive() +{ + shortcutsActive = true; + + const auto menuActions = actions(); + for (int i = 0; i < menuActions.size() && i < 10; ++i) { + menuActions[i]->setShortcut(QKeySequence("Ctrl+" + QString::number((i + 1) % 10))); + } +} + +void SayMenu::setShortcutsInactive() +{ + shortcutsActive = false; + + for (auto *action : actions()) { + action->setShortcut(QKeySequence()); + } } void SayMenu::initSayMenu() @@ -19,10 +44,11 @@ void SayMenu::initSayMenu() for (int i = 0; i < count; ++i) { auto *newAction = new QAction(SettingsCache::instance().messages().getMessageAt(i), this); - if (i < 10) { - newAction->setShortcut(QKeySequence("Ctrl+" + QString::number((i + 1) % 10))); - } connect(newAction, &QAction::triggered, player->getPlayerActions(), &PlayerActions::actSayMessage); addAction(newAction); } -} \ No newline at end of file + + if (shortcutsActive) { + setShortcutsActive(); + } +} diff --git a/cockatrice/src/game/player/menu/say_menu.h b/cockatrice/src/game/player/menu/say_menu.h index 5dbde2277..fadf5f368 100644 --- a/cockatrice/src/game/player/menu/say_menu.h +++ b/cockatrice/src/game/player/menu/say_menu.h @@ -7,18 +7,27 @@ #ifndef COCKATRICE_SAY_MENU_H #define COCKATRICE_SAY_MENU_H +#include "abstract_player_component.h" + #include class Player; -class SayMenu : public QMenu +class SayMenu : public QMenu, public AbstractPlayerComponent { Q_OBJECT public: explicit SayMenu(Player *player); + + void retranslateUi() override; + void setShortcutsActive() override; + void setShortcutsInactive() override; + +private slots: void initSayMenu(); private: Player *player; + bool shortcutsActive = false; }; #endif // COCKATRICE_SAY_MENU_H diff --git a/cockatrice/src/game/player/menu/sideboard_menu.h b/cockatrice/src/game/player/menu/sideboard_menu.h index 22d5a2d69..4a77d1b52 100644 --- a/cockatrice/src/game/player/menu/sideboard_menu.h +++ b/cockatrice/src/game/player/menu/sideboard_menu.h @@ -7,18 +7,20 @@ #ifndef COCKATRICE_SIDEBOARD_MENU_H #define COCKATRICE_SIDEBOARD_MENU_H +#include "abstract_player_component.h" + #include class Player; -class SideboardMenu : public QMenu +class SideboardMenu : public QMenu, public AbstractPlayerComponent { Q_OBJECT public: explicit SideboardMenu(Player *player, QMenu *playerMenu); - void retranslateUi(); - void setShortcutsActive(); - void setShortcutsInactive(); + void retranslateUi() override; + void setShortcutsActive() override; + void setShortcutsInactive() override; private: Player *player; diff --git a/cockatrice/src/game/player/menu/utility_menu.h b/cockatrice/src/game/player/menu/utility_menu.h index ff57e7252..f6577d7d1 100644 --- a/cockatrice/src/game/player/menu/utility_menu.h +++ b/cockatrice/src/game/player/menu/utility_menu.h @@ -7,17 +7,19 @@ #ifndef COCKATRICE_UTILITY_MENU_H #define COCKATRICE_UTILITY_MENU_H +#include "abstract_player_component.h" + #include class Player; -class UtilityMenu : public QMenu +class UtilityMenu : public QMenu, public AbstractPlayerComponent { Q_OBJECT public slots: void populatePredefinedTokensMenu(); - void retranslateUi(); - void setShortcutsActive(); - void setShortcutsInactive(); + void retranslateUi() override; + void setShortcutsActive() override; + void setShortcutsInactive() override; public: explicit UtilityMenu(Player *player, QMenu *playerMenu);