added ExitModOrganizer(), used instead of qApp->exit()

reset geometry uses TaskDialog
moved restart code for the settings dialog to MainWindow
fixed settings sometimes not being saved when restarting
don't log "crash dumps present" every time the settings dialog is closed

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;

 }