Merge remote-tracking branch 'origin/master' into collapsible-separators

3 files changed, 140 insertions, 85 deletions

diff --git a/src/commandline.cpp b/src/commandline.cpp
index bb9ff9b8..8c722990 100644
--- a/src/commandline.cpp
+++ b/src/commandline.cpp
@@ -206,9 +206,11 @@ std::optional<int> CommandLine::setupCore(OrganizerCore& organizer) const
   if (m_shortcut.isValid()) {
     if (m_shortcut.hasExecutable()) {
       try {
+        // make sure MO doesn't exit even if locking is disabled, ForceWait and
+        // PreventExit will do that
         organizer.processRunner()
           .setFromShortcut(m_shortcut)
-          .setWaitForCompletion()
+          .setWaitForCompletion(ProcessRunner::ForceWait, UILocker::PreventExit)
           .run();
 
         return 0;
@@ -229,9 +231,12 @@ std::optional<int> CommandLine::setupCore(OrganizerCore& organizer) const
     try
     {
       // pass the remaining parameters to the binary
+      //
+      // make sure MO doesn't exit even if locking is disabled, ForceWait and
+      // PreventExit will do that
       organizer.processRunner()
         .setFromFileOrExecutable(exeName, m_untouched)
-        .setWaitForCompletion()
+        .setWaitForCompletion(ProcessRunner::ForceWait, UILocker::PreventExit)
         .run();
 
       return 0;
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index ee25fc61..f761a5f0 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -1506,67 +1506,91 @@ void MainWindow::activateSelectedProfile()
 
 void MainWindow::on_profileBox_currentIndexChanged(int index)
 {
-  if (ui->profileBox->isEnabled()) {
-    int previousIndex = m_OldProfileIndex;
-    m_OldProfileIndex = index;
+  if (!ui->profileBox->isEnabled()) {
+    return;
+  }
+
+  int previousIndex = m_OldProfileIndex;
+  m_OldProfileIndex = index;
 
-    if ((previousIndex != -1) &&
+  // select has changed, save stuff
+  if ((previousIndex != -1) &&
       (m_OrganizerCore.currentProfile() != nullptr) &&
       m_OrganizerCore.currentProfile()->exists()) {
-      m_OrganizerCore.saveCurrentLists();
-    }
+    m_OrganizerCore.saveCurrentLists();
+  }
 
-    // Avoid doing any refresh if currentProfile is already set but previous index was -1
-    // as it means that this is happening during initialization so everything has already been set.
-    if (previousIndex == -1
+  // Avoid doing any refresh if currentProfile is already set but previous
+  // index was -1 as it means that this is happening during initialization so
+  // everything has already been set.
+  if (previousIndex == -1
       && m_OrganizerCore.currentProfile() != nullptr
       && m_OrganizerCore.currentProfile()->exists()
-      && ui->profileBox->currentText() == m_OrganizerCore.currentProfile()->name()) {
-      return;
-    }
+      && ui->profileBox->currentText() == m_OrganizerCore.currentProfile()->name()){
+    return;
+  }
 
-    // ensure the new index is valid
-    if (index < 0 || index >= ui->profileBox->count()) {
-      log::debug("invalid profile index, using last profile");
-      ui->profileBox->setCurrentIndex(ui->profileBox->count() - 1);
-    }
+  // ensure the new index is valid
+  if (index < 0 || index >= ui->profileBox->count()) {
+    log::debug("invalid profile index, using last profile");
+    ui->profileBox->setCurrentIndex(ui->profileBox->count() - 1);
+  }
 
-    if (ui->profileBox->currentIndex() == 0) {
-      ui->profileBox->setCurrentIndex(previousIndex);
 
-      std::optional<QString> newSelection;
+  // handle <manage> item
+  if (ui->profileBox->currentIndex() == 0) {
+    // remember the profile name that was selected before, previousIndex can't
+    // be used again because adding/deleting profiles will change the order
+    // in the list
+    const QString previousName = ui->profileBox->itemText(previousIndex);
 
-      ProfilesDialog dlg(ui->profileBox->currentText(), m_OrganizerCore, this);
-      dlg.exec();
-      newSelection = dlg.selectedProfile();
+    // show the dialog
+    ProfilesDialog dlg(previousName, m_OrganizerCore, this);
+    dlg.exec();
 
-      while (!refreshProfiles()) {
-        ProfilesDialog dlg(ui->profileBox->currentText(), m_OrganizerCore, this);
-        dlg.exec();
-        newSelection = dlg.selectedProfile();
-      }
+    // check if the user clicked 'select' to select another profile
+    std::optional<QString> newSelection = dlg.selectedProfile();
 
-      if (newSelection) {
-        ui->profileBox->setCurrentText(*newSelection);
-        activateSelectedProfile();
-      }
+    // refresh the profile box; this loops until there is at least one profile
+    // available, which shouldn't really happen because the dialog won't allow
+    // it
+    //
+    // the `false` to refreshProfiles() is so it doesn't try to select the
+    // profile in the list because 1) it's done just below, and 2) it might be
+    // wrong profile if there's something in newSelection
+    while (!refreshProfiles(false)) {
+      ProfilesDialog dlg(previousName, m_OrganizerCore, this);
+      dlg.exec();
+      newSelection = dlg.selectedProfile();
     }
-    else {
-      activateSelectedProfile();
+
+    // note that setCurrentText() is recursive, it will re-execute this function
+    if (newSelection) {
+      ui->profileBox->setCurrentText(*newSelection);
+    } else {
+      ui->profileBox->setCurrentText(previousName);
     }
 
-    LocalSavegames* saveGames = m_OrganizerCore.managedGame()->feature<LocalSavegames>();
-    if (saveGames != nullptr) {
-      if (saveGames->prepareProfile(m_OrganizerCore.currentProfile())) {
-        m_SavesTab->refreshSaveList();
-      }
+    // nothing else to do because setCurrentText() is recursive and will
+    // have re-executed on_profileBox_currentIndexChanged() again, doing all
+    // the stuff below for the new selection
+    return;
+  }
+
+
+  activateSelectedProfile();
+
+  LocalSavegames *saveGames = m_OrganizerCore.managedGame()->feature<LocalSavegames>();
+  if (saveGames != nullptr) {
+    if (saveGames->prepareProfile(m_OrganizerCore.currentProfile())) {
+      m_SavesTab->refreshSaveList();
     }
+  }
 
-    BSAInvalidation* invalidation = m_OrganizerCore.managedGame()->feature<BSAInvalidation>();
-    if (invalidation != nullptr) {
-      if (invalidation->prepareProfile(m_OrganizerCore.currentProfile())) {
-        QTimer::singleShot(5, &m_OrganizerCore, SLOT(profileRefresh()));
-      }
+  BSAInvalidation *invalidation = m_OrganizerCore.managedGame()->feature<BSAInvalidation>();
+  if (invalidation != nullptr) {
+    if (invalidation->prepareProfile(m_OrganizerCore.currentProfile())) {
+      QTimer::singleShot(5, &m_OrganizerCore, SLOT(profileRefresh()));
     }
   }
 }
diff --git a/src/processrunner.cpp b/src/processrunner.cpp
index bc4e6227..cdcbfa16 100644
--- a/src/processrunner.cpp
+++ b/src/processrunner.cpp
@@ -228,7 +228,7 @@ const std::chrono::milliseconds Infinite(-1);
 // waits for completion, times out after `wait` if not Infinite
 //
 std::optional<ProcessRunner::Results> timedWait(
-  HANDLE handle, DWORD pid, UILocker::Session& ls,
+  HANDLE handle, DWORD pid, UILocker::Session* ls,
   std::chrono::milliseconds wait, std::atomic<bool>& interrupt)
 {
   using namespace std::chrono;
@@ -249,35 +249,38 @@ std::optional<ProcessRunner::Results> timedWait(
 
     // the process is still running
 
-    // check the lock widget
-    switch (ls.result())
-    {
-      case UILocker::StillLocked:
+    // check the lock widget; the session can be null when running shortcuts
+    // with locking disabled, in which case the user cannot force unlock
+    if (ls) {
+      switch (ls->result())
       {
-        break;
-      }
+        case UILocker::StillLocked:
+        {
+          break;
+        }
 
-      case UILocker::ForceUnlocked:
-      {
-        log::debug("waiting for {} force unlocked by user", pid);
-        return ProcessRunner::ForceUnlocked;
-      }
+        case UILocker::ForceUnlocked:
+        {
+          log::debug("waiting for {} force unlocked by user", pid);
+          return ProcessRunner::ForceUnlocked;
+        }
 
-      case UILocker::Cancelled:
-      {
-        log::debug("waiting for {} cancelled by user", pid);
-        return ProcessRunner::Cancelled;
-      }
+        case UILocker::Cancelled:
+        {
+          log::debug("waiting for {} cancelled by user", pid);
+          return ProcessRunner::Cancelled;
+        }
 
-      case UILocker::NoResult:  // fall-through
-      default:
-      {
-        // shouldn't happen
-        log::debug(
-          "unexpected result {} while waiting for {}",
-          static_cast<int>(ls.result()), pid);
+        case UILocker::NoResult:  // fall-through
+        default:
+        {
+          // shouldn't happen
+          log::debug(
+            "unexpected result {} while waiting for {}",
+            static_cast<int>(ls->result()), pid);
 
-        return ProcessRunner::Error;
+          return ProcessRunner::Error;
+        }
       }
     }
 
@@ -296,7 +299,7 @@ std::optional<ProcessRunner::Results> timedWait(
 }
 
 ProcessRunner::Results waitForProcessesThreadImpl(
-  HANDLE job, UILocker::Session& ls, std::atomic<bool>& interrupt)
+  HANDLE job, UILocker::Session* ls, std::atomic<bool>& interrupt)
 {
   using namespace std::chrono;
 
@@ -315,8 +318,11 @@ ProcessRunner::Results waitForProcessesThreadImpl(
       return ProcessRunner::Completed;
     }
 
-    // update the lock widget
-    ls.setInfo(ip.p.pid(), ip.p.name());
+    // update the lock widget; the session can be null when running shortcuts
+    // with locking disabled
+    if (ls) {
+      ls->setInfo(ip.p.pid(), ip.p.name());
+    }
 
     if (ip.p.pid() != currentPID) {
       // log any change in the process being waited for
@@ -354,15 +360,19 @@ ProcessRunner::Results waitForProcessesThreadImpl(
 }
 
 void waitForProcessesThread(
-  ProcessRunner::Results& result, HANDLE job, UILocker::Session& ls,
+  ProcessRunner::Results& result, HANDLE job, UILocker::Session* ls,
   std::atomic<bool>& interrupt)
 {
   result = waitForProcessesThreadImpl(job, ls, interrupt);
-  ls.unlock();
+
+  // the session can be null when running shortcuts with locking disabled
+  if (ls) {
+    ls->unlock();
+  }
 }
 
 ProcessRunner::Results waitForProcesses(
-  const std::vector<HANDLE>& initialProcesses, UILocker::Session& ls)
+  const std::vector<HANDLE>& initialProcesses, UILocker::Session* ls)
 {
   if (initialProcesses.empty()) {
     // nothing to wait for
@@ -415,7 +425,7 @@ ProcessRunner::Results waitForProcesses(
 
   auto* t = QThread::create(
     waitForProcessesThread,
-    std::ref(results), monitor, std::ref(ls), std::ref(interrupt));
+    std::ref(results), monitor, ls, std::ref(interrupt));
 
   QEventLoop events;
   QObject::connect(t, &QThread::finished, [&]{
@@ -436,7 +446,7 @@ ProcessRunner::Results waitForProcesses(
 }
 
 ProcessRunner::Results waitForProcess(
-  HANDLE initialProcess, LPDWORD exitCode, UILocker::Session& ls)
+  HANDLE initialProcess, LPDWORD exitCode, UILocker::Session* ls)
 {
   std::vector<HANDLE> processes = {initialProcess};
 
@@ -862,8 +872,10 @@ ProcessRunner::Results ProcessRunner::postRun()
     m_lockReason = UILocker::LockUI;
   }
 
+  const bool lockEnabled = m_core.settings().interface().lockGUI();
+
   if (mustWait) {
-    if (!m_core.settings().interface().lockGUI()) {
+    if (!lockEnabled) {
       // at least tell the user what's going on
       log::debug(
         "locking is disabled, but the output of the application is required; "
@@ -877,7 +889,7 @@ ProcessRunner::Results ProcessRunner::postRun()
       return Running;
     }
 
-    if (!m_core.settings().interface().lockGUI()) {
+    if (!lockEnabled) {
       // disabling locking is like clicking on unlock immediately
       log::debug("not waiting for process because locking is disabled");
       return ForceUnlocked;
@@ -886,9 +898,23 @@ ProcessRunner::Results ProcessRunner::postRun()
 
   auto r = Error;
 
-  withLock([&](auto& ls) {
-    r = waitForProcess(m_handle.get(), &m_exitCode, ls);
-  });
+  if (mustWait && m_lockReason == UILocker::PreventExit && !lockEnabled) {
+    // this happens when running shortcuts and locking is disabled
+    //
+    // MO must stay alive until all processes are dead or child processes
+    // may not get hooked properly, but the user has disabled locking the ui
+    //
+    // this is a bit of an edge case, but that means the user wants to run
+    // shortcuts without seeing the lock dialog, so allow them to do that
+    //
+    // MO will be running in the background with no visual feedback, but that's
+    // how it is
+    r = waitForProcess(m_handle.get(), &m_exitCode, nullptr);
+  } else {
+    withLock([&](auto& ls) {
+      r = waitForProcess(m_handle.get(), &m_exitCode, &ls);
+    });
+  }
 
   if (shouldRefresh(r)) {
     m_core.afterRun(m_sp.binary, m_exitCode);
@@ -940,7 +966,7 @@ ProcessRunner::Results ProcessRunner::waitForAllUSVFSProcessesWithLock(
         return;
       }
 
-      r = waitForProcesses(processes, ls);
+      r = waitForProcesses(processes, &ls);
 
       if (r != Completed) {
         // error, cancelled, or unlocked