mirror of
https://github.com/Cockatrice/Cockatrice.git
synced 2026-11-06 18:13:29 -08:00
[CardMenu] Free the card menus and submenus rebuilt on each right-click (#7093)
* Parent card menu submenus to the menu that owns them QMenu::addMenu(QMenu *) does not take ownership of the submenu, and these submenus were created without a parent. Since CardMenu is rebuilt from scratch on every right-click, each rebuild leaked the MoveMenu, PtMenu, card counters and related-cards submenus. Pass the owning CardMenu as the QWidget parent so Qt's parent-child mechanism frees them with it. * 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.
This commit is contained in:
parent
daa8671b4d
commit
17f583013b
7 changed files with 32 additions and 18 deletions
|
|
@ -123,7 +123,7 @@ CardMenu::CardMenu(PlayerGraphicsItem *_player, CardItem *_card, bool _shortcuts
|
|||
|
||||
aRevealToAll = new QAction(this);
|
||||
|
||||
mCardCounters = new QMenu;
|
||||
mCardCounters = new QMenu(this);
|
||||
|
||||
// Card counters
|
||||
for (int i = 0; i < 6; ++i) {
|
||||
|
|
@ -225,7 +225,7 @@ void CardMenu::createTableMenu(bool canModifyCard)
|
|||
}
|
||||
addSeparator();
|
||||
addAction(aClone);
|
||||
addMenu(new MoveMenu(player));
|
||||
addMenu(new MoveMenu(player, this));
|
||||
addSeparator();
|
||||
addAction(aAttach);
|
||||
if (card->getAttachedTo()) {
|
||||
|
|
@ -233,7 +233,7 @@ void CardMenu::createTableMenu(bool canModifyCard)
|
|||
}
|
||||
addAction(aDrawArrow);
|
||||
addSeparator();
|
||||
addMenu(new PtMenu(player));
|
||||
addMenu(new PtMenu(player, this));
|
||||
addAction(aSetAnnotation);
|
||||
addSeparator();
|
||||
addAction(aReduceLifeByPower);
|
||||
|
|
@ -275,7 +275,7 @@ void CardMenu::createStackMenu(bool canModifyCard)
|
|||
addAction(aPlayFacedown);
|
||||
addSeparator();
|
||||
addAction(aClone);
|
||||
addMenu(new MoveMenu(player));
|
||||
addMenu(new MoveMenu(player, this));
|
||||
addSeparator();
|
||||
addAction(aAttach);
|
||||
addAction(aDrawArrow);
|
||||
|
|
@ -304,7 +304,7 @@ void CardMenu::createGraveyardOrExileMenu(bool canModifyCard)
|
|||
addAction(aPlayFacedown);
|
||||
addSeparator();
|
||||
addAction(aClone);
|
||||
addMenu(new MoveMenu(player));
|
||||
addMenu(new MoveMenu(player, this));
|
||||
addSeparator();
|
||||
addAction(aAttach);
|
||||
addAction(aDrawArrow);
|
||||
|
|
@ -342,7 +342,7 @@ void CardMenu::createHandOrCustomZoneMenu(bool canModifyCard)
|
|||
|
||||
addSeparator();
|
||||
addAction(aClone);
|
||||
addMenu(new MoveMenu(player));
|
||||
addMenu(new MoveMenu(player, this));
|
||||
|
||||
// actions that are really wonky when done from deck or sideboard
|
||||
if (card->getZone()->getName() == ZoneNames::HAND) {
|
||||
|
|
@ -366,7 +366,7 @@ void CardMenu::createHandOrCustomZoneMenu(bool canModifyCard)
|
|||
void CardMenu::createZonelessMenu(bool canModifyCard)
|
||||
{
|
||||
if (canModifyCard) {
|
||||
addMenu(new MoveMenu(player));
|
||||
addMenu(new MoveMenu(player, this));
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -415,7 +415,7 @@ void CardMenu::addRelatedCardView()
|
|||
}
|
||||
|
||||
addSeparator();
|
||||
auto viewRelatedCards = new QMenu(tr("View related cards"));
|
||||
auto viewRelatedCards = new QMenu(tr("View related cards"), this);
|
||||
addMenu(viewRelatedCards);
|
||||
for (const CardRelation *relatedCard : relatedCards) {
|
||||
QString relatedCardName = relatedCard->getName();
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@
|
|||
#include <libcockatrice/game/player/player_actions.h>
|
||||
#include <libcockatrice/game/player/player_logic.h>
|
||||
|
||||
MoveMenu::MoveMenu(PlayerGraphicsItem *player) : QMenu(tr("Move to"))
|
||||
MoveMenu::MoveMenu(PlayerGraphicsItem *player, QWidget *parent) : QMenu(tr("Move to"), parent)
|
||||
{
|
||||
aMoveToTopLibrary = new QAction(this);
|
||||
aMoveToTopLibrary->setData(cmMoveToTopLibrary);
|
||||
|
|
|
|||
|
|
@ -17,7 +17,7 @@ class MoveMenu : public QMenu
|
|||
Q_OBJECT
|
||||
|
||||
public:
|
||||
explicit MoveMenu(PlayerGraphicsItem *player);
|
||||
explicit MoveMenu(PlayerGraphicsItem *player, QWidget *parent);
|
||||
void setShortcutsActive();
|
||||
void retranslateUi();
|
||||
|
||||
|
|
|
|||
|
|
@ -9,7 +9,7 @@
|
|||
#include <libcockatrice/game/player/player_actions.h>
|
||||
#include <libcockatrice/game/player/player_logic.h>
|
||||
|
||||
PtMenu::PtMenu(PlayerGraphicsItem *player) : QMenu(tr("Power / toughness"))
|
||||
PtMenu::PtMenu(PlayerGraphicsItem *player, QWidget *parent) : QMenu(tr("Power / toughness"), parent)
|
||||
{
|
||||
PlayerActions *playerActions = player->getLogic()->getPlayerActions();
|
||||
|
||||
|
|
|
|||
|
|
@ -18,7 +18,7 @@ class PtMenu : public QMenu
|
|||
Q_OBJECT
|
||||
|
||||
public:
|
||||
explicit PtMenu(PlayerGraphicsItem *player);
|
||||
explicit PtMenu(PlayerGraphicsItem *player, QWidget *parent);
|
||||
void retranslateUi();
|
||||
void setShortcutsActive();
|
||||
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -101,6 +101,8 @@ private:
|
|||
QAction *aInviteToGame = nullptr;
|
||||
QList<QAction *> 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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue