Refactor: Simplify closeRequest and remove closed signal (#6062)

* Refactor: simplify closeRequest and remove closed signal

* clean up closeRequest usages

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()) {
+    if (!confirmClose()) {
-        return;
+        return false;
     }
 
-    emit deckEditorClosing(this);
+    return close();
-    close();
 }

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;

cockatrice/src/client/tabs/tab.cpp

@@ -43,16 +43,7 @@ void Tab::deleteCardInfoPopup(const QString &cardName)
     }
 }
 
-/**
+bool Tab::closeRequest()
- * Overrides the closeEvent in order to emit a close signal
- */
-void Tab::closeEvent(QCloseEvent *event)
 {
-    emit closed();
+    return close();
-    event->accept();
-}
-
-void Tab::closeRequest(bool /*forced*/)
-{
-    close();
 }

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.
+     * Nicely asks to close the tab.
-     * Signals for cleanup should be emitted from this method instead of the destructor.
+     * 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()
     {
     }

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()) {
+    if (!leaveGame()) {
-        return;
+        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);

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<AbstractClient *> &_clients,
@@ -212,7 +215,7 @@ public:
     ~TabGame() override;
     void retranslateUi() override;
     void updatePlayerListDockTitle();
-    void closeRequest(bool forced = false) override;
+    bool closeRequest() override;
     const QMap<int, Player *> &getPlayers() const
     {
         return players;

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()

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;

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()

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

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);
     });