From 04be0fe6342ab6ccdca7eeb788be67c3f926b2fd Mon Sep 17 00:00:00 2001 From: RickyRister <42636155+RickyRister@users.noreply.github.com> Date: Fri, 1 Aug 2025 19:45:54 -0700 Subject: [PATCH] Refactor: Simplify closeRequest and remove closed signal (#6062) * Refactor: simplify closeRequest and remove closed signal * clean up closeRequest usages --- .../client/tabs/abstract_tab_deck_editor.cpp | 15 +++++++---- .../client/tabs/abstract_tab_deck_editor.h | 3 ++- cockatrice/src/client/tabs/tab.cpp | 13 ++-------- cockatrice/src/client/tabs/tab.h | 18 +++++-------- cockatrice/src/client/tabs/tab_game.cpp | 18 ++++++++----- cockatrice/src/client/tabs/tab_game.h | 5 +++- cockatrice/src/client/tabs/tab_message.cpp | 6 ++--- cockatrice/src/client/tabs/tab_message.h | 4 ++- cockatrice/src/client/tabs/tab_room.cpp | 6 ++--- cockatrice/src/client/tabs/tab_room.h | 4 ++- cockatrice/src/client/tabs/tab_supervisor.cpp | 26 +++++++++---------- 11 files changed, 61 insertions(+), 57 deletions(-) diff --git a/cockatrice/src/client/tabs/abstract_tab_deck_editor.cpp b/cockatrice/src/client/tabs/abstract_tab_deck_editor.cpp index b6908ce94..68de8bdac 100644 --- a/cockatrice/src/client/tabs/abstract_tab_deck_editor.cpp +++ b/cockatrice/src/client/tabs/abstract_tab_deck_editor.cpp @@ -549,6 +549,12 @@ void AbstractTabDeckEditor::filterTreeChanged(FilterTree *filterTree) databaseDisplayDockWidget->setFilterTree(filterTree); } +void AbstractTabDeckEditor::closeEvent(QCloseEvent *event) +{ + emit deckEditorClosing(this); + event->accept(); +} + // Method uses to sync docks state with menu items state bool AbstractTabDeckEditor::eventFilter(QObject *o, QEvent *e) { @@ -592,12 +598,11 @@ bool AbstractTabDeckEditor::confirmClose() return true; } -void AbstractTabDeckEditor::closeRequest(bool forced) +bool AbstractTabDeckEditor::closeRequest() { - if (!forced && !confirmClose()) { - return; + if (!confirmClose()) { + return false; } - emit deckEditorClosing(this); - close(); + return close(); } \ No newline at end of file diff --git a/cockatrice/src/client/tabs/abstract_tab_deck_editor.h b/cockatrice/src/client/tabs/abstract_tab_deck_editor.h index 7b52ff3b6..dd85028dc 100644 --- a/cockatrice/src/client/tabs/abstract_tab_deck_editor.h +++ b/cockatrice/src/client/tabs/abstract_tab_deck_editor.h @@ -78,7 +78,7 @@ public slots: void actDecrementCardFromSideboard(const ExactCard &card); void actOpenRecent(const QString &fileName); void filterTreeChanged(FilterTree *filterTree); - void closeRequest(bool forced = false) override; + bool closeRequest() override; virtual void showPrintingSelector() = 0; virtual void dockTopLevelChanged(bool topLevel) = 0; @@ -117,6 +117,7 @@ protected slots: virtual void freeDocksSize() = 0; virtual void refreshShortcuts() = 0; + void closeEvent(QCloseEvent *event) override; bool eventFilter(QObject *o, QEvent *e) override; virtual void dockVisibleTriggered() = 0; virtual void dockFloatingTriggered() = 0; diff --git a/cockatrice/src/client/tabs/tab.cpp b/cockatrice/src/client/tabs/tab.cpp index dcc1ae0c2..8b098c280 100644 --- a/cockatrice/src/client/tabs/tab.cpp +++ b/cockatrice/src/client/tabs/tab.cpp @@ -43,16 +43,7 @@ void Tab::deleteCardInfoPopup(const QString &cardName) } } -/** - * Overrides the closeEvent in order to emit a close signal - */ -void Tab::closeEvent(QCloseEvent *event) +bool Tab::closeRequest() { - emit closed(); - event->accept(); -} - -void Tab::closeRequest(bool /*forced*/) -{ - close(); + return close(); } \ No newline at end of file diff --git a/cockatrice/src/client/tabs/tab.h b/cockatrice/src/client/tabs/tab.h index 68a1882a7..fc97585c3 100644 --- a/cockatrice/src/client/tabs/tab.h +++ b/cockatrice/src/client/tabs/tab.h @@ -15,12 +15,6 @@ class Tab : public QMainWindow signals: void userEvent(bool globalEvent = true); void tabTextChanged(Tab *tab, const QString &newTabText); - /** - * Emitted when the tab is closed (because Qt doesn't provide a built-in close signal) - * This signal is emitted from this class's overridden Tab::closeEvent method. - * Make sure any subclasses that override closeEvent still emit this signal from there. - */ - void closed(); protected: TabSupervisor *tabSupervisor; @@ -31,7 +25,6 @@ protected: protected slots: void showCardInfoPopup(const QPoint &pos, const CardRef &cardRef); void deleteCardInfoPopup(const QString &cardName); - void closeEvent(QCloseEvent *event) override; private: CardRef currentCard; @@ -59,13 +52,16 @@ public: } virtual QString getTabText() const = 0; virtual void retranslateUi() = 0; + /** - * Sends a request to close the tab. - * Signals for cleanup should be emitted from this method instead of the destructor. + * Nicely asks to close the tab. + * Override this method to do checks or ask for confirmation before closing the tab. + * If you need to force close the tab, just call close() instead. * - * @param forced whether this close request was initiated by the user or forced by the server. + * @return True if the tab is successfully closed. */ - virtual void closeRequest(bool forced = false); + virtual bool closeRequest(); + virtual void tabActivated() { } diff --git a/cockatrice/src/client/tabs/tab_game.cpp b/cockatrice/src/client/tabs/tab_game.cpp index c4a119961..f96949074 100644 --- a/cockatrice/src/client/tabs/tab_game.cpp +++ b/cockatrice/src/client/tabs/tab_game.cpp @@ -376,15 +376,19 @@ void TabGame::refreshShortcuts() } } -void TabGame::closeRequest(bool forced) +bool TabGame::closeRequest() { - if (!forced && !leaveGame()) { - return; + if (!leaveGame()) { + return false; } - emit gameClosing(this); + return close(); +} - close(); +void TabGame::closeEvent(QCloseEvent *event) +{ + emit gameClosing(this); + event->accept(); } void TabGame::incrementGameTime() @@ -1249,7 +1253,7 @@ void TabGame::createMenuItems() aConcede = new QAction(this); connect(aConcede, &QAction::triggered, this, &TabGame::actConcede); aLeaveGame = new QAction(this); - connect(aLeaveGame, &QAction::triggered, this, [this] { closeRequest(); }); + connect(aLeaveGame, &QAction::triggered, this, &TabGame::closeRequest); aFocusChat = new QAction(this); connect(aFocusChat, &QAction::triggered, sayEdit, qOverload<>(&LineEditCompleter::setFocus)); aCloseReplay = nullptr; @@ -1299,7 +1303,7 @@ void TabGame::createReplayMenuItems() aFocusChat = nullptr; aLeaveGame = nullptr; aCloseReplay = new QAction(this); - connect(aCloseReplay, &QAction::triggered, this, [this] { closeRequest(); }); + connect(aCloseReplay, &QAction::triggered, this, &TabGame::closeRequest); phasesMenu = nullptr; gameMenu = new QMenu(this); diff --git a/cockatrice/src/client/tabs/tab_game.h b/cockatrice/src/client/tabs/tab_game.h index e00a76dc8..7927d44a6 100644 --- a/cockatrice/src/client/tabs/tab_game.h +++ b/cockatrice/src/client/tabs/tab_game.h @@ -202,6 +202,9 @@ private slots: void dockFloatingTriggered(); void dockTopLevelChanged(bool topLevel); +protected slots: + void closeEvent(QCloseEvent *event) override; + public: TabGame(TabSupervisor *_tabSupervisor, QList &_clients, @@ -212,7 +215,7 @@ public: ~TabGame() override; void retranslateUi() override; void updatePlayerListDockTitle(); - void closeRequest(bool forced = false) override; + bool closeRequest() override; const QMap &getPlayers() const { return players; diff --git a/cockatrice/src/client/tabs/tab_message.cpp b/cockatrice/src/client/tabs/tab_message.cpp index 9dc53156d..06700d4e3 100644 --- a/cockatrice/src/client/tabs/tab_message.cpp +++ b/cockatrice/src/client/tabs/tab_message.cpp @@ -39,7 +39,7 @@ TabMessage::TabMessage(TabSupervisor *_tabSupervisor, vbox->addWidget(sayEdit); aLeave = new QAction(this); - connect(aLeave, &QAction::triggered, this, [this] { closeRequest(); }); + connect(aLeave, &QAction::triggered, this, &TabMessage::closeRequest); messageMenu = new QMenu(this); messageMenu->addAction(aLeave); @@ -86,10 +86,10 @@ QString TabMessage::getTabText() const return tr("%1 - Private chat").arg(QString::fromStdString(otherUserInfo->name())); } -void TabMessage::closeRequest(bool /*forced*/) +void TabMessage::closeEvent(QCloseEvent *event) { emit talkClosing(this); - close(); + event->accept(); } void TabMessage::sendMessage() diff --git a/cockatrice/src/client/tabs/tab_message.h b/cockatrice/src/client/tabs/tab_message.h index dd7424e5d..659d0b804 100644 --- a/cockatrice/src/client/tabs/tab_message.h +++ b/cockatrice/src/client/tabs/tab_message.h @@ -37,6 +37,9 @@ private slots: void addMentionTag(QString mentionTag); void messageClicked(); +protected slots: + void closeEvent(QCloseEvent *event) override; + public: TabMessage(TabSupervisor *_tabSupervisor, AbstractClient *_client, @@ -44,7 +47,6 @@ public: const ServerInfo_User &_otherUserInfo); ~TabMessage() override; void retranslateUi() override; - void closeRequest(bool forced = false) override; void tabActivated() override; QString getUserName() const; QString getTabText() const override; diff --git a/cockatrice/src/client/tabs/tab_room.cpp b/cockatrice/src/client/tabs/tab_room.cpp index 1eb648f48..569a88094 100644 --- a/cockatrice/src/client/tabs/tab_room.cpp +++ b/cockatrice/src/client/tabs/tab_room.cpp @@ -103,7 +103,7 @@ TabRoom::TabRoom(TabSupervisor *_tabSupervisor, hbox->addWidget(userList, 1); aLeaveRoom = new QAction(this); - connect(aLeaveRoom, &QAction::triggered, this, [this] { closeRequest(); }); + connect(aLeaveRoom, &QAction::triggered, this, &TabRoom::closeRequest); roomMenu = new QMenu(this); roomMenu->addAction(aLeaveRoom); @@ -173,11 +173,11 @@ void TabRoom::actShowPopup(const QString &message) } } -void TabRoom::closeRequest(bool /*forced*/) +void TabRoom::closeEvent(QCloseEvent *event) { sendRoomCommand(prepareRoomCommand(Command_LeaveRoom())); emit roomClosing(this); - close(); + event->accept(); } void TabRoom::tabActivated() diff --git a/cockatrice/src/client/tabs/tab_room.h b/cockatrice/src/client/tabs/tab_room.h index ca88a256f..4c0c6c18f 100644 --- a/cockatrice/src/client/tabs/tab_room.h +++ b/cockatrice/src/client/tabs/tab_room.h @@ -88,13 +88,15 @@ private slots: void processRemoveMessagesEvent(const Event_RemoveMessages &event); void refreshShortcuts(); +protected slots: + void closeEvent(QCloseEvent *event) override; + public: TabRoom(TabSupervisor *_tabSupervisor, AbstractClient *_client, ServerInfo_User *_ownUser, const ServerInfo_Room &info); void retranslateUi() override; - void closeRequest(bool forced = false) override; void tabActivated() override; void processRoomEvent(const RoomEvent &event); int getRoomId() const diff --git a/cockatrice/src/client/tabs/tab_supervisor.cpp b/cockatrice/src/client/tabs/tab_supervisor.cpp index e75a5ca9c..d87cb4744 100644 --- a/cockatrice/src/client/tabs/tab_supervisor.cpp +++ b/cockatrice/src/client/tabs/tab_supervisor.cpp @@ -364,7 +364,7 @@ void TabSupervisor::addCloseButtonToTab(Tab *tab, int tabIndex, QAction *manager // If managed, all close requests should go through the menu action connect(closeButton, &CloseButton::clicked, this, [manager] { checkAndTrigger(manager, false); }); } else { - connect(closeButton, &CloseButton::clicked, tab, [tab] { tab->closeRequest(); }); + connect(closeButton, &CloseButton::clicked, tab, &Tab::closeRequest); } tabBar()->setTabButton(tabIndex, closeSide, closeButton); } @@ -458,16 +458,16 @@ void TabSupervisor::stop() emit localGameEnded(); } else { if (tabAccount) { - tabAccount->closeRequest(true); + tabAccount->close(); } if (tabServer) { - tabServer->closeRequest(true); + tabServer->close(); } if (tabAdmin) { - tabAdmin->closeRequest(true); + tabAdmin->close(); } if (tabLog) { - tabLog->closeRequest(true); + tabLog->close(); } } @@ -486,7 +486,7 @@ void TabSupervisor::stop() } for (const auto tab : tabsToDelete) { - tab->closeRequest(true); + tab->close(); } userListManager->handleDisconnect(); @@ -510,7 +510,7 @@ void TabSupervisor::openTabVisualDeckStorage() { tabVisualDeckStorage = new TabDeckStorageVisual(this); myAddTab(tabVisualDeckStorage, aTabVisualDeckStorage); - connect(tabVisualDeckStorage, &Tab::closed, this, [this] { + connect(tabVisualDeckStorage, &QObject::destroyed, this, [this] { tabVisualDeckStorage = nullptr; aTabVisualDeckStorage->setChecked(false); }); @@ -533,7 +533,7 @@ void TabSupervisor::openTabServer() tabServer = new TabServer(this, client); connect(tabServer, &TabServer::roomJoined, this, &TabSupervisor::addRoomTab); myAddTab(tabServer, aTabServer); - connect(tabServer, &Tab::closed, this, [this] { + connect(tabServer, &QObject::destroyed, this, [this] { tabServer = nullptr; aTabServer->setChecked(false); }); @@ -558,7 +558,7 @@ void TabSupervisor::openTabAccount() connect(tabAccount, &TabAccount::userJoined, this, &TabSupervisor::processUserJoined); connect(tabAccount, &TabAccount::userLeft, this, &TabSupervisor::processUserLeft); myAddTab(tabAccount, aTabAccount); - connect(tabAccount, &Tab::closed, this, [this] { + connect(tabAccount, &QObject::destroyed, this, [this] { tabAccount = nullptr; aTabAccount->setChecked(false); }); @@ -581,7 +581,7 @@ void TabSupervisor::openTabDeckStorage() tabDeckStorage = new TabDeckStorage(this, client, userInfo); connect(tabDeckStorage, &TabDeckStorage::openDeckEditor, this, &TabSupervisor::openDeckInNewTab); myAddTab(tabDeckStorage, aTabDeckStorage); - connect(tabDeckStorage, &Tab::closed, this, [this] { + connect(tabDeckStorage, &QObject::destroyed, this, [this] { tabDeckStorage = nullptr; aTabDeckStorage->setChecked(false); }); @@ -604,7 +604,7 @@ void TabSupervisor::openTabReplays() tabReplays = new TabReplays(this, client, userInfo); connect(tabReplays, &TabReplays::openReplay, this, &TabSupervisor::openReplay); myAddTab(tabReplays, aTabReplays); - connect(tabReplays, &Tab::closed, this, [this] { + connect(tabReplays, &QObject::destroyed, this, [this] { tabReplays = nullptr; aTabReplays->setChecked(false); }); @@ -627,7 +627,7 @@ void TabSupervisor::openTabAdmin() tabAdmin = new TabAdmin(this, client, (userInfo->user_level() & ServerInfo_User::IsAdmin)); connect(tabAdmin, &TabAdmin::adminLockChanged, this, &TabSupervisor::adminLockChanged); myAddTab(tabAdmin, aTabAdmin); - connect(tabAdmin, &Tab::closed, this, [this] { + connect(tabAdmin, &QObject::destroyed, this, [this] { tabAdmin = nullptr; aTabAdmin->setChecked(false); }); @@ -649,7 +649,7 @@ void TabSupervisor::openTabLog() { tabLog = new TabLog(this, client); myAddTab(tabLog, aTabLog); - connect(tabLog, &Tab::closed, this, [this] { + connect(tabLog, &QObject::destroyed, this, [this] { tabLog = nullptr; aTabAdmin->setChecked(false); });