removed some redundant functions in InstanceManager, made them all non-static

documentation

6 files changed, 250 insertions, 76 deletions

diff --git a/src/createinstancedialog.cpp b/src/createinstancedialog.cpp
index 2ff8adf3..13dd200c 100644
--- a/src/createinstancedialog.cpp
+++ b/src/createinstancedialog.cpp
@@ -443,7 +443,7 @@ QString CreateInstanceDialog::dataPath() const
   QString s;
 
   if (instanceType() == Portable) {
-    s = QDir(InstanceManager::portablePath()).absolutePath();
+    s = QDir(InstanceManager::singleton().portablePath()).absolutePath();
   } else {
     s = InstanceManager::singleton().instancePath(instanceName());
   }
diff --git a/src/createinstancedialogpages.cpp b/src/createinstancedialogpages.cpp
index 26f2f61d..f2a7ab36 100644
--- a/src/createinstancedialogpages.cpp
+++ b/src/createinstancedialogpages.cpp
@@ -136,11 +136,11 @@ TypePage::TypePage(CreateInstanceDialog& dlg)
 {
   ui->createGlobal->setDescription(
     ui->createGlobal->description()
-      .arg(InstanceManager::singleton().instancesPath()));
+      .arg(InstanceManager::singleton().globalInstancesRootPath()));
 
   ui->createPortable->setDescription(
     ui->createPortable->description()
-      .arg(InstanceManager::portablePath()));
+      .arg(InstanceManager::singleton().portablePath()));
 
   if (InstanceManager::singleton().portableInstanceExists()) {
     ui->createPortable->setEnabled(false);
@@ -748,7 +748,7 @@ void NamePage::onChanged()
 
 void NamePage::updateWarnings()
 {
-  const auto root = InstanceManager::singleton().instancesPath();
+  const auto root = InstanceManager::singleton().globalInstancesRootPath();
 
   m_okay = checkName(root, ui->instanceName->text());
   updateNavigation();
@@ -899,9 +899,9 @@ void PathsPage::setPaths(const QString& name, bool force)
   QString path;
 
   if (m_dlg.instanceType() == CreateInstanceDialog::Portable) {
-    path = InstanceManager::portablePath();
+    path = InstanceManager::singleton().portablePath();
   } else {
-    const auto root = InstanceManager::singleton().instancesPath();
+    const auto root = InstanceManager::singleton().globalInstancesRootPath();
     path = root + "/" + name;
   }
 
diff --git a/src/instancemanager.cpp b/src/instancemanager.cpp
index 8163149b..e7c2a611 100644
--- a/src/instancemanager.cpp
+++ b/src/instancemanager.cpp
@@ -79,7 +79,7 @@ QString Instance::profileName() const
 
 QString Instance::iniPath() const
 {
-  return InstanceManager::iniPath(m_dir);
+  return InstanceManager::singleton().iniPath(m_dir);
 }
 
 bool Instance::isPortable() const
@@ -96,6 +96,7 @@ Instance::SetupResults Instance::setup(PluginContainer& plugins)
     return SetupResults::BadIni;
   }
 
+  // game name and directory are from ini unless overridden by setGame()
   if (m_gameName.isEmpty()) {
     if (auto v=s.game().name())
       m_gameName = *v;
@@ -106,11 +107,13 @@ Instance::SetupResults Instance::setup(PluginContainer& plugins)
       m_gameDir = *v;
   }
 
+  // getting game plugin
   const auto r = getGamePlugin(plugins);
   if (r != SetupResults::Ok) {
     return r;
   }
 
+  // getting game variant, error if it's missing and required by the plugin
   if (m_gameVariant.isEmpty()) {
     if (auto v=s.game().edition()) {
       m_gameVariant = *v;
@@ -123,15 +126,22 @@ Instance::SetupResults Instance::setup(PluginContainer& plugins)
     m_plugin->setGameVariant(m_gameVariant);
   }
 
+  // figuring out profile from ini if it's missing
   getProfile(s);
 
+  // updating the settings since some of these values might have been missing
   s.game().setName(m_gameName);
   s.game().setDirectory(m_gameDir);
   s.game().setSelectedProfileName(m_profile);
 
-  if (!m_gameVariant.isEmpty())
+  if (!m_gameVariant.isEmpty()) {
+    // don't write a variant to the ini if the plugin doesn't require one
     s.game().setEdition(m_gameVariant);
+  }
 
+  // the game directory may be different than what the plugin detected, the user
+  // can change it in the settings and might have multiple versions of the game
+  // installed
   m_plugin->setGamePath(m_gameDir);
 
   return SetupResults::Ok;
@@ -299,31 +309,27 @@ InstanceManager &InstanceManager::singleton()
 void InstanceManager::overrideInstance(const QString& instanceName)
 {
   m_overrideInstanceName = instanceName;
-  m_overrideInstance = true;
 }
 
 void InstanceManager::overrideProfile(const QString& profileName)
 {
   m_overrideProfileName = profileName;
-  m_overrideProfile = true;
 }
 
 std::optional<Instance> InstanceManager::currentInstance() const
 {
-  const QString profile = m_overrideProfile ? m_overrideProfileName : "";
+  const QString profile = m_overrideProfileName ?
+    *m_overrideProfileName : "";
+
+  const QString name = m_overrideInstanceName ?
+    *m_overrideInstanceName : GlobalSettings::currentInstance();
 
-  if (portableInstallIsLocked()) {
+
+  if (!allowedToChangeInstance()) {
     // force portable instance
     return Instance(QDir(portablePath()), true, profile);
   }
 
-  QString name;
-
-  if (m_overrideInstance)
-    name = m_overrideInstanceName;
-  else
-    name = GlobalSettings::currentInstance();
-
   if (name.isEmpty()) {
     if (portableInstanceExists()) {
       // use portable
@@ -340,7 +346,7 @@ std::optional<Instance> InstanceManager::currentInstance() const
 void InstanceManager::clearCurrentInstance()
 {
   setCurrentInstance("");
-  m_overrideInstance = false;
+  m_overrideInstanceName = {};
 }
 
 void InstanceManager::setCurrentInstance(const QString &name)
@@ -350,27 +356,27 @@ void InstanceManager::setCurrentInstance(const QString &name)
 
 QString InstanceManager::instancePath(const QString& instanceName) const
 {
-  return QDir::fromNativeSeparators(instancesPath() + "/" + instanceName);
+  return QDir::fromNativeSeparators(globalInstancesRootPath() + "/" + instanceName);
 }
 
-QString InstanceManager::instancesPath() const
+QString InstanceManager::globalInstancesRootPath() const
 {
   return QDir::fromNativeSeparators(
         QStandardPaths::writableLocation(QStandardPaths::DataLocation));
 }
 
-QString InstanceManager::iniPath(const QDir& instanceDir)
+QString InstanceManager::iniPath(const QDir& instanceDir) const
 {
   return instanceDir.filePath(QString::fromStdWString(AppConfig::iniFileName()));
 }
 
-std::vector<QDir> InstanceManager::instancePaths() const
+std::vector<QDir> InstanceManager::globalInstancePaths() const
 {
   const std::set<QString> ignore = {
     "cache", "qtwebengine",
   };
 
-  const QDir root(instancesPath());
+  const QDir root(globalInstancesRootPath());
   const auto dirs = root.entryList(QDir::Dirs | QDir::NoDotAndDotDot);
 
   std::vector<QDir> list;
@@ -384,24 +390,12 @@ std::vector<QDir> InstanceManager::instancePaths() const
   return list;
 }
 
-QStringList InstanceManager::instanceNames() const
-{
-  QStringList list;
-
-  for (auto&& d : instancePaths()) {
-    list.push_back(d.dirName());
-  }
-
-  return list;
-}
-
-
-bool InstanceManager::isPortablePath(const QString& dataPath)
+bool InstanceManager::hasAnyInstances() const
 {
-  return (dataPath == portablePath());
+  return portableInstanceExists() || !globalInstancePaths().empty();
 }
 
-QString InstanceManager::portablePath()
+QString InstanceManager::portablePath() const
 {
   return qApp->applicationDirPath();
 }
@@ -412,16 +406,13 @@ bool InstanceManager::portableInstanceExists() const
                        QString::fromStdWString(AppConfig::iniFileName()));
 }
 
-
-bool InstanceManager::portableInstallIsLocked() const
-{
-  return QFile::exists(qApp->applicationDirPath() + "/" +
-                       QString::fromStdWString(AppConfig::portableLockFileName()));
-}
-
 bool InstanceManager::allowedToChangeInstance() const
 {
-  return !portableInstallIsLocked();
+  const auto lockFile =
+    qApp->applicationDirPath() + "/" +
+    QString::fromStdWString(AppConfig::portableLockFileName());
+
+  return !QFile::exists(lockFile);
 }
 
 const MOBase::IPluginGame* InstanceManager::gamePluginForDirectory(
@@ -429,6 +420,7 @@ const MOBase::IPluginGame* InstanceManager::gamePluginForDirectory(
 {
   const QString ini = iniPath(instanceDir);
 
+  // reading ini
   Settings s(ini);
 
   if (s.iniStatus() != QSettings::NoError)
@@ -437,6 +429,7 @@ const MOBase::IPluginGame* InstanceManager::gamePluginForDirectory(
     return nullptr;
   }
 
+  // using game name from ini, if available
   const auto instanceGameName = s.game().name();
 
   if (instanceGameName && !instanceGameName->isEmpty())
@@ -448,7 +441,7 @@ const MOBase::IPluginGame* InstanceManager::gamePluginForDirectory(
     }
 
     log::error(
-      "no plugin reports game name '{}' found in ini {}",
+      "no plugin has game name '{}' that was found in ini {}",
       *instanceGameName, ini);
   }
   else
@@ -457,8 +450,7 @@ const MOBase::IPluginGame* InstanceManager::gamePluginForDirectory(
   }
 
 
-  log::error("falling back on looksValid check");
-
+  // using game directory from ini, if available
   const auto gameDir = s.game().directory();
 
   if (gameDir && !gameDir->isEmpty())
@@ -470,7 +462,7 @@ const MOBase::IPluginGame* InstanceManager::gamePluginForDirectory(
     }
 
     log::error(
-      "no plugins appear to support game directory '{}' from ini {}",
+      "no plugin appears to support game directory '{}' from ini {}",
       *gameDir, ini);
   }
   else
@@ -478,6 +470,17 @@ const MOBase::IPluginGame* InstanceManager::gamePluginForDirectory(
     log::error("no game directory found in ini {}", ini);
   }
 
+
+  // looking for a plugin that can handle the directory
+  log::debug("falling back on looksValid check");
+
+  for (const IPluginGame* game : plugins.plugins<IPluginGame>()) {
+    if (game->looksValid(instanceDir)) {
+      return game;
+    }
+  }
+
+
   return nullptr;
 }
 
@@ -485,6 +488,7 @@ QString InstanceManager::makeUniqueName(const QString& instanceName) const
 {
   const QString sanitized = sanitizeInstanceName(instanceName);
 
+  // trying "name (N)"
   QString name = sanitized;
   for (int i=2; i<100; ++i) {
     if (!instanceExists(name)) {
@@ -499,7 +503,7 @@ QString InstanceManager::makeUniqueName(const QString& instanceName) const
 
 bool InstanceManager::instanceExists(const QString& instanceName) const
 {
-  const QDir root = instancesPath();
+  const QDir root = globalInstancesRootPath();
   return root.exists(instanceName);
 }
 
diff --git a/src/instancemanager.h b/src/instancemanager.h
index 79f1f30b..e2ceb743 100644
--- a/src/instancemanager.h
+++ b/src/instancemanager.h
@@ -10,33 +10,122 @@ class Settings;
 class PluginContainer;
 
 
+// represents an instance, either global or portable
+//
+// if setup() is not called, the game plugin is not available and the INI is
+// not processed at all, so name(), directory() and isPortable() really are the
+// only meaningful functions
+//
+// setup() must be called when MO wants to use the instance, it will read the
+// INI, figure out the game plugin to use and set it up by calling
+// setGameVaraint(), setGamePath(), etc. on it
+//
+// when setup() fails because the game name/directory or variant are missing,
+// setGame() and setVariant() can be called before retrying setup(); this
+// happens on startup if that information is missing
+//
 class Instance
 {
 public:
+  // returned by setup()
+  //
   enum class SetupResults
   {
+    // instance is ready to be used
     Ok,
+
+    // error while reading the INI
     BadIni,
+
+    // both the game name and directory are missing from the ini; setup() will
+    // attempt to recover if either are missing, but not when both are
     IniMissingGame,
+
+    // either:
+    // 1) there is no plugin with the given name, or
+    // 2) if the name is missing, no plugin can handle the game directory
     PluginGone,
+
+    // the selected plugin does not consider the game directory as being valid
     GameGone,
+
+    // there is no game variant specified in the INI, but the plugin requires
+    // one
     MissingVariant
   };
 
+
+  // an instance that lives in the given directory; `portable` must be `true`
+  // if this is a portable instance
+  //
+  // `profileName` can be given to override what's in the INI; this typically
+  // happens when the profile is overriden on the command line
+  //
   Instance(QDir dir, bool portable, QString profileName={});
 
+  // finds the appropriate game plugin and sets it up so MO can use it
+  //
+  // setup() tries to recover from some errors, but can fail for a variety of
+  // reasons, see SetupResults
+  //
   SetupResults setup(PluginContainer& plugins);
 
+
+  // overrides the game name and directory
+  //
   void setGame(const QString& name, const QString& dir);
+
+  // overrides the game variant
+  //
   void setVariant(const QString& name);
 
+
+  // returns the instance name; this is the directory name or "Portable" for
+  // portable instances
+  //
+  // can be called without setup()
+  //
   QString name() const;
+
+  // returns either:
+  // 1) the game name from the INI,
+  // 2) gameName() from the game plugin if it was missing, or
+  // 3) whatever was given in setGame()
+  //
   QString gameName() const;
+
+  // returns either:
+  // 1) the game directory from the INI,
+  // 2) gameDirectory() from the game plugin if it was missing, or
+  // 3) whatever was given in setGame()
+  //
   QString gameDirectory() const;
+
+  // returns the instance directory; can be called without setup()
+  //
   QDir directory() const;
+
+  // returns the selected game plugin; will return null if setup() hasn't been
+  // called, or if it failed
+  //
   MOBase::IPluginGame* gamePlugin() const;
+
+  // returns either:
+  // 1) the profile name given in the constructor,
+  // 2) the profile name from the INI, or
+  // 3) the default profile name if it's missing (see
+  //    AppConfig::defaultProfileName())
+  //
   QString profileName() const;
+
+  // returns the path to the INI file for this instance; the file may not
+  // exist
+  //
   QString iniPath() const;
+
+  // returns whether this is a portable instance; this is the flag given in the
+  // constructor
+  //
   bool isPortable() const;
 
 private:
@@ -46,51 +135,132 @@ private:
   MOBase::IPluginGame* m_plugin;
   QString m_profile;
 
+  // figures out the game plugin for this instance
+  //
   SetupResults getGamePlugin(PluginContainer& plugins);
+
+  // figures out the profile name for this instance
+  //
   void getProfile(const Settings& s);
 };
 
 
+// manages global and portable instances
+//
 class InstanceManager
 {
 public:
+  // there is only one manager; this isn't called instance() because it's hella
+  // confusing
+  //
   static InstanceManager& singleton();
 
+  // overrides instance name found in registry
+  //
   void overrideInstance(const QString& instanceName);
+
+  // overrides profile name from INI for currentInstance()
+  //
   void overrideProfile(const QString& profileName);
 
+  // returns a game plugin that considers the given directory valid
+  //
+  // this will check for an INI file in the directory and use its game name
+  // and directory if available
+  //
+  // if there is no INI, if it's missing these values or if there are no game
+  // plugins that can handle these values, this returns the first plugin that
+  // considers the given directory valid
+  //
+  // returns null if all of this fails
+  //
   const MOBase::IPluginGame* gamePluginForDirectory(
     const QDir& dir, const PluginContainer& plugins) const;
 
+  // clears the instance name from the registry; on restart, this will make MO
+  // either select the portable instance if it exists, or display the instance
+  // selection/creation dialog
+  //
   void clearCurrentInstance();
+
+  // returns the current instance from the registry; this may be empty if the
+  // instance name in the registry is empty or non-existent and there is no
+  // portable instance set up
+  //
   std::optional<Instance> currentInstance() const;
+
+  // sets the instance name in the registry so the same instance is opened next
+  // time MO runs
+  //
   void setCurrentInstance(const QString &name);
 
+  // whether MO should allow the user to change the current instance from the
+  // user interface
+  //
   bool allowedToChangeInstance() const;
-  static bool isPortablePath(const QString& dataPath);
-  static QString portablePath();
+
+  // whether a portable instance exists; this basically checks for an INI in
+  // the application directory
+  //
   bool portableInstanceExists() const;
 
-  QString instancesPath() const;
-  QStringList instanceNames() const;
-  std::vector<QDir> instancePaths() const;
+  // whether any instance exists, whether global or portable
+  //
+  bool hasAnyInstances() const;
+
+  // returns the absolute path to the portable instance, regardless of whether
+  // one exists
+  //
+  QString portablePath() const;
 
+  // returns the absolute path to the directory that contains global instances
+  // (typically AppData/Local/ModOrganizer)
+  //
+  QString globalInstancesRootPath() const;
+
+  // returns the list of absolute path to all existing global instances; this
+  // does not include the portable instance
+  //
+  std::vector<QDir> globalInstancePaths() const;
+
+  // returns `name` modified so that it is a valid instance name
+  //
   QString sanitizeInstanceName(const QString &name) const;
+
+  // sanitizes the given instance name and either
+  // 1) returns it if there is no instance with this name
+  // 2) tries to add " (N)" at the end until it works
+  //
+  // may return an empty string if no unique name can be found
+  //
   QString makeUniqueName(const QString& instanceName) const;
+
+  // returns whether a global instance with this name already exists
+  //
   bool instanceExists(const QString& instanceName) const;
+
+  // returns whether the given instance name would be a valid name; this does
+  // not check whether the instance already exists, it's basiscally just a check
+  // against what sanitizeInstanceName() returns
+  //
   bool validInstanceName(const QString& instanceName) const;
+
+  // returns the absolute path of a global instance with the given name; this
+  // does not check if the name is valid or if exists
+  //
   QString instancePath(const QString& instanceName) const;
-  static QString iniPath(const QDir& instanceDir);
+
+  // returns the absolute path to the INI file for the given instance directory;
+  // the file may not exist
+  //
+  QString iniPath(const QDir& instanceDir) const;
 
 private:
   InstanceManager();
-  bool portableInstallIsLocked() const;
 
 private:
-  bool m_overrideInstance{false};
-  QString m_overrideInstanceName;
-  bool m_overrideProfile{false};
-  QString m_overrideProfileName;
+  std::optional<QString> m_overrideInstanceName;
+  std::optional<QString> m_overrideProfileName;
 };
 
 #endif  // MODORGANIZER_INSTANCEMANAGER_INCLUDED
diff --git a/src/instancemanagerdialog.cpp b/src/instancemanagerdialog.cpp
index 3b6daeb8..c2108f9d 100644
--- a/src/instancemanagerdialog.cpp
+++ b/src/instancemanagerdialog.cpp
@@ -48,7 +48,7 @@ public:
   void setDir(const QDir& dir)
   {
     m_dir = dir;
-    m_settings.reset(new Settings(InstanceManager::iniPath(dir)));
+    m_settings.reset(new Settings(InstanceManager::singleton().iniPath(dir)));
   }
 
   QString name() const
@@ -96,7 +96,7 @@ public:
 
   QString iniFile() const
   {
-    return InstanceManager::iniPath(m_dir);
+    return InstanceManager::singleton().iniPath(m_dir);
   }
 
   QIcon icon(const PluginContainer& plugins) const
@@ -351,7 +351,7 @@ void InstanceManagerDialog::updateInstances()
 
   m_instances.clear();
 
-  for (auto&& d : m.instancePaths()) {
+  for (auto&& d : m.globalInstancePaths()) {
     m_instances.push_back(std::make_unique<InstanceInfo>(d, false));
   }
 
diff --git a/src/main.cpp b/src/main.cpp
index e56197c9..7f0886f6 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -280,7 +280,7 @@ std::optional<Instance> selectInstance()
   PluginContainer pc(nullptr);
   pc.loadPlugins();
 
-  if (m.instancePaths().empty() && !m.portableInstanceExists()) {
+  if (!m.hasAnyInstances()) {
     // no instances configured
     CreateInstanceDialog dlg(pc, nullptr);
     if (dlg.exec() != QDialog::Accepted) {
@@ -454,10 +454,6 @@ int runApplication(
 
   log::info("data path: {}", dataPath);
 
-  if (InstanceManager::isPortablePath(dataPath)) {
-    log::debug("this is a portable instance");
-  }
-
   log::info("working directory: {}", QDir::currentPath());
 
   if (!instance.secondary()) {
@@ -529,6 +525,10 @@ int runApplication(
       }
     }
 
+    if (currentInstance.isPortable()) {
+      log::debug("this is a portable instance");
+    }
+
     checkPathsForSanity(*currentInstance.gamePlugin(), settings);
 
     organizer.setManagedGame(currentInstance.gamePlugin());
@@ -665,13 +665,13 @@ int doOneRun(
     if (!currentInstance->directory().exists()) {
       // the previously used instance doesn't exist anymore
 
-      if (m.instanceNames().empty() && !m.portableInstanceExists()) {
+      if (m.hasAnyInstances()) {
         criticalOnTop(QObject::tr(
-          "Instance at '%1' not found. You must create a new instance")
-            .arg(currentInstance->directory().absolutePath()));
+          "Instance at '%1' not found. Select another instance.")
+          .arg(currentInstance->directory().absolutePath()));
       } else {
         criticalOnTop(QObject::tr(
-          "Instance at '%1' not found. Select another instance.")
+          "Instance at '%1' not found. You must create a new instance")
             .arg(currentInstance->directory().absolutePath()));
       }