From 46a97bf39eafe3ea0741502ff083d3cdaef6ad0f Mon Sep 17 00:00:00 2001 From: DawnFire42 Date: Mon, 10 Aug 2026 17:18:13 -0400 Subject: [PATCH] Delete the replaced card menu in TabGame to fix menu rebuild leak PlayerMenu creates a new CardMenu on every right-click and TabGame installs it with QAction::setMenu(), which does not take ownership, so the previous menu was never freed. TabGame now deletes the menu it replaces and reuses a single empty placeholder menu instead of allocating one per update. The installed menu is also released when the tab is destroyed. --- .../src/interface/widgets/tabs/tab_game.cpp | 24 ++++++++++++++----- .../src/interface/widgets/tabs/tab_game.h | 2 ++ 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/cockatrice/src/interface/widgets/tabs/tab_game.cpp b/cockatrice/src/interface/widgets/tabs/tab_game.cpp index 01d021907..055ab321c 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_game.cpp +++ b/cockatrice/src/interface/widgets/tabs/tab_game.cpp @@ -330,6 +330,9 @@ void TabGame::emitUserEvent() TabGame::~TabGame() { + if (aCardMenu && aCardMenu->menu() && aCardMenu->menu() != emptyCardMenu) { + delete aCardMenu->menu(); + } for (auto &player : game->getPlayerManager()->getPlayers()) { player->clear(); } @@ -1158,10 +1161,16 @@ QString TabGame::getTabText() const /** * @param menu The menu to set. Pass in nullptr to set the menu to empty. + * + * The previous card menu is deleted here: QAction::setMenu() does not take ownership, so whoever installs the menu + * (this widget) has to dispose of the one it replaces. The empty placeholder menu is reused instead of deleted. */ void TabGame::setCardMenu(CardMenu *menu) { - if (!aCardMenu) { + if (!aCardMenu || !emptyCardMenu) { + if (menu) { + menu->deleteLater(); + } return; } @@ -1171,10 +1180,11 @@ void TabGame::setCardMenu(CardMenu *menu) gameView->viewport()->update(); } } - if (menu) { - aCardMenu->setMenu(menu); - } else { - aCardMenu->setMenu(new QMenu); + + QMenu *previousMenu = aCardMenu->menu(); + aCardMenu->setMenu(menu ? menu : emptyCardMenu); + if (previousMenu && previousMenu != emptyCardMenu && previousMenu != menu) { + previousMenu->deleteLater(); } } @@ -1258,7 +1268,8 @@ void TabGame::createMenuItems() gameMenu->addSeparator(); - aCardMenu = gameMenu->addMenu(new QMenu(this)); + emptyCardMenu = new QMenu(this); + aCardMenu = gameMenu->addMenu(emptyCardMenu); // Sync the new action with the same state the dock button already shows. updateInviteButtonState(); @@ -1290,6 +1301,7 @@ void TabGame::createReplayMenuItems() gameMenu->addAction(aLeaveGame); aCardMenu = nullptr; + emptyCardMenu = nullptr; addTabMenu(gameMenu); } diff --git a/cockatrice/src/interface/widgets/tabs/tab_game.h b/cockatrice/src/interface/widgets/tabs/tab_game.h index 142c02999..5c2af82b4 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_game.h +++ b/cockatrice/src/interface/widgets/tabs/tab_game.h @@ -101,6 +101,8 @@ private: QAction *aInviteToGame = nullptr; QList phaseActions; QAction *aCardMenu; + /** @brief Placeholder submenu shown when no card is selected; reused instead of reallocated. */ + QMenu *emptyCardMenu = nullptr; /** * @brief The actions associated with managing a QDockWidget