Merge pull request #858 from isanae/proper-exit

Proper exit procedure

10 files changed, 188 insertions, 80 deletions

diff --git a/src/main.cpp b/src/main.cpp
index a6918d99..f08ba066 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -563,10 +563,12 @@ int runApplication(MOApplication &application, SingleInstance &instance,
     if (game == nullptr) {
       InstanceManager &instance = InstanceManager::instance();
       QString instanceName = instance.currentInstance();
+
       if (instanceName.compare("Portable", Qt::CaseInsensitive) != 0) {
         instance.clearCurrentInstance();
-        return INT_MAX;
+        return RestartExitCode;
       }
+
       return 1;
     }
 
@@ -710,6 +712,8 @@ int runApplication(MOApplication &application, SingleInstance &instance,
       splash.finish(&mainWindow);
 
       res = application.exec();
+      mainWindow.onBeforeClose();
+      mainWindow.close();
 
       NexusInterface::instance(&pluginContainer)
         ->getAccessManager()->setTopLevelWidget(nullptr);
@@ -867,6 +871,7 @@ int main(int argc, char *argv[])
 
   do {
     LogModel::instance().clear();
+    ResetExitFlag();
 
     // make sure the log file isn't locked in case MO was restarted and
     // the previous instance gets deleted
@@ -904,10 +909,11 @@ int main(int argc, char *argv[])
       splash = ":/MO/gui/splash";
     }
 
-    int result = runApplication(application, instance, splash);
-    if (result != INT_MAX) {
+    const int result = runApplication(application, instance, splash);
+    if (result != RestartExitCode) {
       return result;
     }
+
     argc = 1;
     moshortcut = MOShortcut("");
   } while (true);
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index 295c60a6..23733cac 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -1217,7 +1217,7 @@ void MainWindow::showEvent(QShowEvent *event)
   QMainWindow::showEvent(event);
 
   if (!m_WasVisible) {
-    readSettings(m_OrganizerCore.settings());
+    readSettings();
     refreshFilters();
 
     // this needs to be connected here instead of in the constructor because the
@@ -1267,26 +1267,44 @@ void MainWindow::showEvent(QShowEvent *event)
   }
 }
 
+void MainWindow::onBeforeClose()
+{
+  storeSettings();
+}
 
 void MainWindow::closeEvent(QCloseEvent* event)
 {
-  if (!confirmExit()) {
+  // this happens for two reasons:
+  //  1) the user requested to close the window, such as clicking the X
+  //  2) close() is called in runApplication() after application.exec()
+  //     returns, which happens when qApp->exit() is called
+  //
+  // the window must never actually close for 1), because settings haven't been
+  // saved yet: the state of many widgets is saved to the ini, which relies on
+  // the window still being onscreen (or else everything is considered hidden)
+  //
+  // for 2), the settings have been saved and the window can just close
+
+  if (ModOrganizerExiting()) {
+    // the user has confirmed if necessary and all settings have been saved,
+    // just close it
+    QMainWindow::closeEvent(event);
+  } else {
+    // never close the window because settings might need to be changed
     event->ignore();
-    return;
-  }
 
-  storeSettings(m_OrganizerCore.settings());
+    // start the process of exiting, which may require confirmation by calling
+    // canExit(), among other things
+    ExitModOrganizer();
+  }
 }
 
-bool MainWindow::confirmExit()
+bool MainWindow::canExit()
 {
-  m_closing = true;
-
   if (m_OrganizerCore.downloadManager()->downloadsInProgressNoPause()) {
     if (QMessageBox::question(this, tr("Downloads in progress"),
                           tr("There are still downloads in progress, do you really want to quit?"),
                           QMessageBox::Yes | QMessageBox::Cancel) == QMessageBox::Cancel) {
-      m_closing = false;
       return false;
     } else {
       m_OrganizerCore.downloadManager()->pauseAll();
@@ -1298,8 +1316,9 @@ bool MainWindow::confirmExit()
   HANDLE injected_process_still_running = m_OrganizerCore.findAndOpenAUSVFSProcess(hiddenList, GetCurrentProcessId());
   if (injected_process_still_running != INVALID_HANDLE_VALUE)
   {
+    m_exitAfterWait = true;
     m_OrganizerCore.waitForApplication(injected_process_still_running);
-    if (!m_closing) { // if operation cancelled
+    if (!m_exitAfterWait) { // if operation cancelled
       return false;
     }
   }
@@ -2138,20 +2157,22 @@ void MainWindow::activateProxy(bool activate)
   busyDialog.hide();
 }
 
-void MainWindow::readSettings(const Settings& settings)
+void MainWindow::readSettings()
 {
-  settings.geometry().restoreGeometry(this);
-  settings.geometry().restoreState(this);
-  settings.geometry().restoreDocks(this);
-  settings.geometry().restoreToolbars(this);
-  settings.geometry().restoreState(ui->splitter);
-  settings.geometry().restoreState(ui->categoriesSplitter);
-  settings.geometry().restoreVisibility(ui->menuBar);
-  settings.geometry().restoreVisibility(ui->statusBar);
+  const auto& s = m_OrganizerCore.settings();
+
+  s.geometry().restoreGeometry(this);
+  s.geometry().restoreState(this);
+  s.geometry().restoreDocks(this);
+  s.geometry().restoreToolbars(this);
+  s.geometry().restoreState(ui->splitter);
+  s.geometry().restoreState(ui->categoriesSplitter);
+  s.geometry().restoreVisibility(ui->menuBar);
+  s.geometry().restoreVisibility(ui->statusBar);
 
   {
     // special case in case someone puts 0 in the INI
-    auto v = settings.widgets().index(ui->executablesListBox);
+    auto v = s.widgets().index(ui->executablesListBox);
     if (!v || v == 0) {
       v = 1;
     }
@@ -2159,16 +2180,16 @@ void MainWindow::readSettings(const Settings& settings)
     ui->executablesListBox->setCurrentIndex(*v);
   }
 
-  settings.widgets().restoreIndex(ui->groupCombo);
+  s.widgets().restoreIndex(ui->groupCombo);
 
   {
-    settings.geometry().restoreVisibility(ui->categoriesGroup, false);
+    s.geometry().restoreVisibility(ui->categoriesGroup, false);
     const auto v = ui->categoriesGroup->isVisible();
     setCategoryListVisible(v);
     ui->displayCategoriesBtn->setChecked(v);
   }
 
-  if (settings.network().useProxy()) {
+  if (s.network().useProxy()) {
     activateProxy(true);
   }
 }
@@ -2215,8 +2236,10 @@ void MainWindow::processUpdates(Settings& settings) {
   }
 }
 
-void MainWindow::storeSettings(Settings& s)
+void MainWindow::storeSettings()
 {
+  auto& s = m_OrganizerCore.settings();
+
   s.geometry().saveState(this);
   s.geometry().saveGeometry(this);
   s.geometry().saveDocks(this);
@@ -2244,7 +2267,7 @@ ILockedWaitingForProcess* MainWindow::lock()
     ++m_LockCount;
     return m_LockDialog;
   }
-  if (m_closing)
+  if (m_exitAfterWait)
     m_LockDialog = new WaitingOnCloseDialog(this);
   else
     m_LockDialog = new LockedDialog(this, true);
@@ -2265,8 +2288,8 @@ void MainWindow::unlock()
   }
   --m_LockCount;
   if (m_LockCount == 0) {
-    if (m_closing && m_LockDialog->canceled())
-      m_closing = false;
+    if (m_exitAfterWait && m_LockDialog->canceled())
+      m_exitAfterWait = false;
     m_LockDialog->hide();
     m_LockDialog->deleteLater();
     m_LockDialog = nullptr;
@@ -5018,18 +5041,31 @@ void MainWindow::on_actionSettings_triggered()
   bool proxy = settings.network().useProxy();
   DownloadManager *dlManager = m_OrganizerCore.downloadManager();
   const bool oldCheckForUpdates = settings.checkForUpdates();
+  const int oldMaxDumps = settings.diagnostics().crashDumpsMax();
 
 
   SettingsDialog dialog(&m_PluginContainer, settings, this);
   dialog.exec();
 
+  auto e = dialog.exitNeeded();
+
   if (oldManagedGameDirectory != settings.game().directory()) {
-    QMessageBox::about(this, tr("Restarting MO"),
-      tr("Changing the managed game directory requires restarting MO.\n"
-         "Any pending downloads will be paused.\n\n"
-         "Click OK to restart MO now."));
-    dlManager->pauseAll();
-    qApp->exit(INT_MAX);
+    e |= Exit::Restart;
+  }
+
+  if (e.testFlag(Exit::Restart)) {
+    const auto r = MOBase::TaskDialog(this)
+      .title(tr("Restart Mod Organizer"))
+      .main("Restart Mod Organizer")
+      .content(tr("Mod Organizer must restart to finish configuration changes"))
+      .icon(QMessageBox::Question)
+      .button({tr("Restart"), QMessageBox::Yes})
+      .button({tr("Continue"), tr("Some things might be weird."), QMessageBox::No})
+      .exec();
+
+    if (r == QMessageBox::Yes) {
+      ExitModOrganizer(e);
+    }
   }
 
   InstallationManager *instManager = m_OrganizerCore.installationManager();
@@ -5092,7 +5128,10 @@ void MainWindow::on_actionSettings_triggered()
   updateDownloadView();
 
   m_OrganizerCore.setLogLevel(settings.diagnostics().logLevel());
-  m_OrganizerCore.cycleDiagnostics();
+
+  if (settings.diagnostics().crashDumpsMax() != oldMaxDumps) {
+    m_OrganizerCore.cycleDiagnostics();
+  }
 
   toggleMO2EndorseState();
 
@@ -5475,9 +5514,7 @@ void MainWindow::on_actionUpdate_triggered()
 
 void MainWindow::on_actionExit_triggered()
 {
-  if (confirmExit()) {
-    qApp->exit();
-  }
+  ExitModOrganizer();
 }
 
 void MainWindow::actionEndorseMO()
@@ -6121,7 +6158,7 @@ void MainWindow::on_actionChange_Game_triggered()
 
   if (r == QMessageBox::Yes) {
     InstanceManager::instance().clearCurrentInstance();
-    qApp->exit(INT_MAX);
+    ExitModOrganizer(Exit::Restart);
   }
 }
 
diff --git a/src/mainwindow.h b/src/mainwindow.h
index 524e2b6e..b04096be 100644
--- a/src/mainwindow.h
+++ b/src/mainwindow.h
@@ -111,7 +111,6 @@ class MainWindow : public QMainWindow, public IUserInterface
   friend class OrganizerProxy;
 
 public:
-
   explicit MainWindow(Settings &settings,
                       OrganizerCore &organizerCore, PluginContainer &pluginContainer,
                       QWidget *parent = 0);
@@ -154,7 +153,8 @@ public:
   void displayModInformation(
     ModInfo::Ptr modInfo, unsigned int modIndex, ModInfoTabIDs tabID) override;
 
-  bool confirmExit();
+  bool canExit();
+  void onBeforeClose();
 
   virtual bool closeWindow();
   virtual void setWindowEnabled(bool enabled);
@@ -383,9 +383,8 @@ private:
   LockedDialogBase *m_LockDialog { nullptr };
   uint64_t m_LockCount { 0 };
 
-  bool m_closing{ false };
-
   bool m_showArchiveData{ true };
+  bool m_exitAfterWait{ false };
 
   MOBase::DelayedFileWriter m_ArchiveListWriter;
 
@@ -672,8 +671,8 @@ private slots: // ui slots
   void on_categoriesOrBtn_toggled(bool checked);
   void on_managedArchiveLabel_linkHovered(const QString &link);
 
-  void storeSettings(Settings& settings);
-  void readSettings(const Settings& settings);
+  void storeSettings();
+  void readSettings();
   void setupModList();
 };
 
diff --git a/src/settingsdialog.cpp b/src/settingsdialog.cpp
index 8fb25b1c..daccfba9 100644
--- a/src/settingsdialog.cpp
+++ b/src/settingsdialog.cpp
@@ -33,8 +33,8 @@ SettingsDialog::SettingsDialog(PluginContainer *pluginContainer, Settings& setti
   : TutorableDialog("SettingsDialog", parent)
   , ui(new Ui::SettingsDialog)
   , m_settings(settings)
+  , m_exit(Exit::None)
   , m_pluginContainer(pluginContainer)
-  , m_restartNeeded(false)
 {
   ui->setupUi(this);
 
@@ -61,9 +61,14 @@ QWidget* SettingsDialog::parentWidgetForDialogs()
   }
 }
 
-void SettingsDialog::setRestartNeeded()
+void SettingsDialog::setExitNeeded(ExitFlags e)
 {
-  m_restartNeeded = true;
+  m_exit = e;
+}
+
+ExitFlags SettingsDialog::exitNeeded() const
+{
+  return m_exit;
 }
 
 int SettingsDialog::exec()
@@ -87,16 +92,6 @@ int SettingsDialog::exec()
     }
   }
 
-  if (m_restartNeeded) {
-    if (QMessageBox::question(parentWidgetForDialogs(),
-      tr("Restart Mod Organizer?"),
-      tr("In order to finish configuration changes, MO must be restarted.\n"
-        "Restart it now?"),
-      QMessageBox::Yes | QMessageBox::No) == QMessageBox::Yes) {
-      qApp->exit(INT_MAX);
-    }
-  }
-
   return ret;
 }
 
diff --git a/src/settingsdialog.h b/src/settingsdialog.h
index e89da665..bae4a469 100644
--- a/src/settingsdialog.h
+++ b/src/settingsdialog.h
@@ -21,6 +21,7 @@ along with Mod Organizer.  If not, see <http://www.gnu.org/licenses/>.
 #define SETTINGSDIALOG_H
 
 #include "tutorabledialog.h"
+#include "util.h"
 
 class PluginContainer;
 class Settings;
@@ -74,7 +75,9 @@ public:
 
   PluginContainer* pluginContainer();
   QWidget* parentWidgetForDialogs();
-  void setRestartNeeded();
+
+  void setExitNeeded(ExitFlags e);
+  ExitFlags exitNeeded() const;
 
   int exec() override;
 
@@ -82,10 +85,10 @@ public slots:
   virtual void accept();
 
 private:
+  Ui::SettingsDialog* ui;
   Settings& m_settings;
   std::vector<std::unique_ptr<SettingsTab>> m_tabs;
-  Ui::SettingsDialog* ui;
-  bool m_restartNeeded;
+  ExitFlags m_exit;
   PluginContainer* m_pluginContainer;
 };
 
diff --git a/src/settingsdialognexus.cpp b/src/settingsdialognexus.cpp
index 2dd8b998..d49e0a33 100644
--- a/src/settingsdialognexus.cpp
+++ b/src/settingsdialognexus.cpp
@@ -312,7 +312,7 @@ void NexusSettingsTab::addNexusLog(const QString& s)
 
 bool NexusSettingsTab::setKey(const QString& key)
 {
-  dialog().setRestartNeeded();
+  dialog().setExitNeeded(Exit::Restart);
   const bool ret = settings().nexus().setApiKey(key);
   updateNexusState();
   return ret;
@@ -320,7 +320,7 @@ bool NexusSettingsTab::setKey(const QString& key)
 
 bool NexusSettingsTab::clearKey()
 {
-  dialog().setRestartNeeded();
+  dialog().setExitNeeded(Exit::Restart);
   const auto ret = settings().nexus().clearApiKey();
 
   NexusInterface::instance(dialog().pluginContainer())->getAccessManager()->clearApiKey();
diff --git a/src/settingsdialogworkarounds.cpp b/src/settingsdialogworkarounds.cpp
index 0e31fc4b..1c5fbe26 100644
--- a/src/settingsdialogworkarounds.cpp
+++ b/src/settingsdialogworkarounds.cpp
@@ -2,6 +2,7 @@
 #include "ui_settingsdialog.h"
 #include "spawn.h"
 #include "settings.h"
+#include <report.h>
 #include <iplugingame.h>
 
 WorkaroundsSettingsTab::WorkaroundsSettingsTab(Settings& s, SettingsDialog& d)
@@ -118,16 +119,18 @@ void WorkaroundsSettingsTab::on_bsaDateBtn_clicked()
 
 void WorkaroundsSettingsTab::on_resetGeometryBtn_clicked()
 {
-  const auto caption = QObject::tr("Restart Mod Organizer?");
-  const auto text = QObject::tr(
-    "In order to reset the geometry, Mod Organizer must be restarted.\n"
-    "Restart now?");
+  const auto r = MOBase::TaskDialog(parentWidget())
+    .title(QObject::tr("Restart Mod Organizer"))
+    .main(QObject::tr("Restart Mod Organizer"))
+    .content(QObject::tr("Geometries will be reset to their default values."))
+    .icon(QMessageBox::Question)
+    .button({QObject::tr("Restart Mod Organizer"), QMessageBox::Ok})
+    .button({QObject::tr("Cancel"), QMessageBox::Cancel})
+    .exec();
 
-  const auto res = QMessageBox::question(
-    parentWidget(), caption, text, QMessageBox::Yes | QMessageBox::Cancel);
-
-  if (res == QMessageBox::Yes) {
+  if (r == QMessageBox::Ok) {
     settings().geometry().requestReset();
-    qApp->exit(INT_MAX);
+    ExitModOrganizer(Exit::Restart);
+    dialog().close();
   }
 }
diff --git a/src/shared/util.cpp b/src/shared/util.cpp
index 07983e12..58f4eb2b 100644
--- a/src/shared/util.cpp
+++ b/src/shared/util.cpp
@@ -19,6 +19,7 @@ along with Mod Organizer.  If not, see <http://www.gnu.org/licenses/>.
 

 #include "util.h"

 #include "windows_error.h"

+#include "mainwindow.h"

 

 namespace MOShared

 {

@@ -244,3 +245,49 @@ MOBase::VersionInfo createVersionInfo()
 }

 

 } // namespace MOShared

+

+

+static bool g_exiting = false;

+

+MainWindow* findMainWindow()

+{

+  for (auto* tl : qApp->topLevelWidgets()) {

+    if (auto* mw=dynamic_cast<MainWindow*>(tl)) {

+      return mw;

+    }

+  }

+

+  return nullptr;

+}

+

+bool ExitModOrganizer(ExitFlags e)

+{

+  if (g_exiting) {

+    return true;

+  }

+

+  if (!e.testFlag(Exit::Force)) {

+    if (auto* mw=findMainWindow()) {

+      if (!mw->canExit()) {

+        return false;

+      }

+    }

+  }

+

+  g_exiting = true;

+

+  const int code = (e.testFlag(Exit::Restart) ? RestartExitCode : 0);

+  qApp->exit(code);

+

+  return true;

+}

+

+bool ModOrganizerExiting()

+{

+  return g_exiting;

+}

+

+void ResetExitFlag()

+{

+  g_exiting = false;

+}

diff --git a/src/shared/util.h b/src/shared/util.h
index 7bae96f2..aea6f200 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -48,4 +48,22 @@ MOBase::VersionInfo createVersionInfo();
 

 } // namespace MOShared

 

+

+enum class Exit

+{

+  None    = 0x00,

+  Normal  = 0x01,

+  Restart = 0x02,

+  Force   = 0x04

+};

+

+const int RestartExitCode = INT_MAX;

+

+using ExitFlags = QFlags<Exit>;

+Q_DECLARE_OPERATORS_FOR_FLAGS(ExitFlags);

+

+bool ExitModOrganizer(ExitFlags e=Exit::Normal);

+bool ModOrganizerExiting();

+void ResetExitFlag();

+

 #endif // UTIL_H

diff --git a/src/spawn.cpp b/src/spawn.cpp
index 079677f4..a34230b2 100644
--- a/src/spawn.cpp
+++ b/src/spawn.cpp
@@ -186,12 +186,12 @@ QMessageBox::StandardButton badSteamReg(
     .details(details)

     .icon(QMessageBox::Critical)

     .button({

-    QObject::tr("Continue without starting Steam"),

-    QObject::tr("The program may fail to launch."),

-    QMessageBox::Yes})

+      QObject::tr("Continue without starting Steam"),

+      QObject::tr("The program may fail to launch."),

+      QMessageBox::Yes})

     .button({

-    QObject::tr("Cancel"),

-    QMessageBox::Cancel})

+      QObject::tr("Cancel"),

+      QMessageBox::Cancel})

     .exec();

 }

 

@@ -517,7 +517,7 @@ bool restartAsAdmin(QWidget* parent)
   }

 

   log::debug("exiting MO");

-  qApp->exit(0);

+  ExitModOrganizer(Exit::Force);

 

   return true;

 }