fix(ManagedPackPage): only update the current instance exactly

Also carry on the original ID to avoid updating the wrong instance.

Signed-off-by: flow <flowlnlnln@gmail.com>

9 files changed, 82 insertions, 32 deletions

diff --git a/launcher/InstanceImportTask.cpp b/launcher/InstanceImportTask.cpp
index 834e9320..b97870da 100644
--- a/launcher/InstanceImportTask.cpp
+++ b/launcher/InstanceImportTask.cpp
@@ -265,7 +265,12 @@ void InstanceImportTask::processFlame()
     Q_ASSERT(pack_version_id_it != m_extra_info.constEnd());
     auto pack_version_id = pack_version_id_it.value();
 
-    auto* inst_creation_task = new FlameCreationTask(m_stagingPath, m_globalSettings, m_parent, pack_id, pack_version_id);
+    QString original_instance_id;
+    auto original_instance_id_it = m_extra_info.constFind("original_instance_id");
+    if (original_instance_id_it != m_extra_info.constEnd())
+        original_instance_id = original_instance_id_it.value();
+
+    auto* inst_creation_task = new FlameCreationTask(m_stagingPath, m_globalSettings, m_parent, pack_id, pack_version_id, original_instance_id);
 
     inst_creation_task->setName(*this);
     inst_creation_task->setIcon(m_instIcon);
@@ -273,7 +278,7 @@ void InstanceImportTask::processFlame()
     inst_creation_task->setConfirmUpdate(shouldConfirmUpdate());
     
     connect(inst_creation_task, &Task::succeeded, this, [this, inst_creation_task] {
-        setOverride(inst_creation_task->shouldOverride());
+        setOverride(inst_creation_task->shouldOverride(), inst_creation_task->originalInstanceID());
         emitSucceeded();
     });
     connect(inst_creation_task, &Task::failed, this, &InstanceImportTask::emitFailed);
@@ -339,7 +344,12 @@ void InstanceImportTask::processModrinth()
     if (pack_version_id_it != m_extra_info.constEnd())
         pack_version_id = pack_version_id_it.value();
 
-    auto* inst_creation_task = new ModrinthCreationTask(m_stagingPath, m_globalSettings, m_parent, pack_id, pack_version_id);
+    QString original_instance_id;
+    auto original_instance_id_it = m_extra_info.constFind("original_instance_id");
+    if (original_instance_id_it != m_extra_info.constEnd())
+        original_instance_id = original_instance_id_it.value();
+
+    auto* inst_creation_task = new ModrinthCreationTask(m_stagingPath, m_globalSettings, m_parent, pack_id, pack_version_id, original_instance_id);
 
     inst_creation_task->setName(*this);
     inst_creation_task->setIcon(m_instIcon);
@@ -347,7 +357,7 @@ void InstanceImportTask::processModrinth()
     inst_creation_task->setConfirmUpdate(shouldConfirmUpdate());
     
     connect(inst_creation_task, &Task::succeeded, this, [this, inst_creation_task] {
-        setOverride(inst_creation_task->shouldOverride());
+        setOverride(inst_creation_task->shouldOverride(), inst_creation_task->originalInstanceID());
         emitSucceeded();
     });
     connect(inst_creation_task, &Task::failed, this, &InstanceImportTask::emitFailed);
diff --git a/launcher/InstanceList.cpp b/launcher/InstanceList.cpp
index cebd70d7..68e3e92c 100644
--- a/launcher/InstanceList.cpp
+++ b/launcher/InstanceList.cpp
@@ -816,7 +816,7 @@ class InstanceStaging : public Task {
     void childSucceded()
     {
         unsigned sleepTime = backoff();
-        if (m_parent->commitStagedInstance(m_stagingPath, m_instance_name, m_groupName, m_child->shouldOverride()))
+        if (m_parent->commitStagedInstance(m_stagingPath, m_instance_name, m_groupName, *m_child.get()))
         {
             emitSucceeded();
             return;
@@ -880,25 +880,22 @@ QString InstanceList::getStagedInstancePath()
     return path;
 }
 
-bool InstanceList::commitStagedInstance(const QString& path, InstanceName const& instanceName, const QString& groupName, bool should_override)
+bool InstanceList::commitStagedInstance(const QString& path, InstanceName const& instanceName, const QString& groupName, InstanceTask const& commiting)
 {
     QDir dir;
     QString instID;
     InstancePtr inst;
 
+    auto should_override = commiting.shouldOverride();
+
     if (should_override) {
-        // This is to avoid problems when the instance folder gets manually renamed
-        if ((inst = getInstanceByManagedName(instanceName.originalName()))) {
-            instID = QFileInfo(inst->instanceRoot()).fileName();
-        } else if ((inst = getInstanceByManagedName(instanceName.modifiedName()))) {
-            instID = QFileInfo(inst->instanceRoot()).fileName();
-        } else {
-            instID = FS::RemoveInvalidFilenameChars(instanceName.modifiedName(), '-');
-        }
+        instID = commiting.originalInstanceID();
     } else {
         instID = FS::DirNameFromString(instanceName.modifiedName(), m_instDir);
     }
 
+    Q_ASSERT(!instID.isEmpty());
+
     {
         WatchLock lock(m_watcher, m_instDir);
         QString destination = FS::PathCombine(m_instDir, instID);
diff --git a/launcher/InstanceList.h b/launcher/InstanceList.h
index 3673298f..edacba3c 100644
--- a/launcher/InstanceList.h
+++ b/launcher/InstanceList.h
@@ -133,7 +133,7 @@ public:
      * should_override is used when another similar instance already exists, and we want to override it
      * - for instance, when updating it.
      */
-    bool commitStagedInstance(const QString& keyPath, const InstanceName& instanceName, const QString& groupName, bool should_override);
+    bool commitStagedInstance(const QString& keyPath, const InstanceName& instanceName, const QString& groupName, const InstanceTask&);
 
     /**
      * Destroy a previously created staging area given by @keyPath - used when creation fails.
diff --git a/launcher/InstanceTask.h b/launcher/InstanceTask.h
index e481354c..7c02160a 100644
--- a/launcher/InstanceTask.h
+++ b/launcher/InstanceTask.h
@@ -49,8 +49,15 @@ class InstanceTask : public Task, public InstanceName {
 
     bool shouldOverride() const { return m_override_existing; }
 
+    [[nodiscard]] QString originalInstanceID() const { return m_original_instance_id; };
+
    protected:
-    void setOverride(bool override) { m_override_existing = override; }
+    void setOverride(bool override, QString instance_id_to_override = {})
+    {
+        m_override_existing = override;
+        if (!instance_id_to_override.isEmpty())
+            m_original_instance_id = instance_id_to_override;
+    }
 
    protected: /* data */
     SettingsObjectPtr m_globalSettings;
@@ -60,4 +67,6 @@ class InstanceTask : public Task, public InstanceName {
 
     bool m_override_existing = false;
     bool m_confirm_update = true;
+
+    QString m_original_instance_id;
 };
diff --git a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp
index ad50597e..1e76f252 100644
--- a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp
+++ b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp
@@ -81,13 +81,19 @@ bool FlameCreationTask::updateInstance()
     auto instance_list = APPLICATION->instances();
 
     // FIXME: How to handle situations when there's more than one install already for a given modpack?
-    auto inst = instance_list->getInstanceByManagedName(originalName());
+    InstancePtr inst;
+    if (auto original_id = originalInstanceID(); !original_id.isEmpty()) {
+        inst = instance_list->getInstanceById(original_id);
+        Q_ASSERT(inst);
+    } else {
+        inst = instance_list->getInstanceByManagedName(originalName());
 
-    if (!inst) {
-        inst = instance_list->getInstanceById(originalName());
+        if (!inst) {
+            inst = instance_list->getInstanceById(originalName());
 
-        if (!inst)
-            return false;
+            if (!inst)
+                return false;
+        }
     }
 
     QString index_path(FS::PathCombine(m_stagingPath, "manifest.json"));
@@ -233,7 +239,7 @@ bool FlameCreationTask::updateInstance()
         }
     }
 
-    setOverride(true);
+    setOverride(true, inst->id());
     qDebug() << "Will override instance!";
 
     m_instance = inst;
diff --git a/launcher/modplatform/flame/FlameInstanceCreationTask.h b/launcher/modplatform/flame/FlameInstanceCreationTask.h
index 2a513602..3a1c729f 100644
--- a/launcher/modplatform/flame/FlameInstanceCreationTask.h
+++ b/launcher/modplatform/flame/FlameInstanceCreationTask.h
@@ -51,11 +51,21 @@ class FlameCreationTask final : public InstanceCreationTask {
     Q_OBJECT
 
    public:
-    FlameCreationTask(const QString& staging_path, SettingsObjectPtr global_settings, QWidget* parent, QString id, QString version_id)
-        : InstanceCreationTask(), m_parent(parent), m_managed_id(std::move(id)), m_managed_version_id(std::move(version_id))
+    FlameCreationTask(const QString& staging_path,
+                      SettingsObjectPtr global_settings,
+                      QWidget* parent,
+                      QString id,
+                      QString version_id,
+                      QString original_instance_id = {})
+        : InstanceCreationTask()
+        , m_parent(parent)
+        , m_managed_id(std::move(id))
+        , m_managed_version_id(std::move(version_id))
     {
         setStagingPath(staging_path);
         setParentSettings(global_settings);
+
+        m_original_instance_id = std::move(original_instance_id);
     }
 
     bool abort() override;
diff --git a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
index c043a2e3..1c0e8979 100644
--- a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
+++ b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
@@ -33,13 +33,19 @@ bool ModrinthCreationTask::updateInstance()
     auto instance_list = APPLICATION->instances();
 
     // FIXME: How to handle situations when there's more than one install already for a given modpack?
-    auto inst = instance_list->getInstanceByManagedName(originalName());
+    InstancePtr inst;
+    if (auto original_id = originalInstanceID(); !original_id.isEmpty()) {
+        inst = instance_list->getInstanceById(original_id);
+        Q_ASSERT(inst);
+    } else {
+        inst = instance_list->getInstanceByManagedName(originalName());
 
-    if (!inst) {
-        inst = instance_list->getInstanceById(originalName());
+        if (!inst) {
+            inst = instance_list->getInstanceById(originalName());
 
-        if (!inst)
-            return false;
+            if (!inst)
+                return false;
+        }
     }
 
     QString index_path = FS::PathCombine(m_stagingPath, "modrinth.index.json");
@@ -138,7 +144,7 @@ bool ModrinthCreationTask::updateInstance()
     }
 
 
-    setOverride(true);
+    setOverride(true, inst->id());
     qDebug() << "Will override instance!";
 
     m_instance = inst;
diff --git a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.h b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.h
index 551674d2..122fc5ce 100644
--- a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.h
+++ b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.h
@@ -14,11 +14,21 @@ class ModrinthCreationTask final : public InstanceCreationTask {
     Q_OBJECT
 
    public:
-    ModrinthCreationTask(QString staging_path, SettingsObjectPtr global_settings, QWidget* parent, QString id, QString version_id = {})
-        : InstanceCreationTask(), m_parent(parent), m_managed_id(std::move(id)), m_managed_version_id(std::move(version_id))
+    ModrinthCreationTask(QString staging_path,
+                         SettingsObjectPtr global_settings,
+                         QWidget* parent,
+                         QString id,
+                         QString version_id = {},
+                         QString original_instance_id = {})
+        : InstanceCreationTask()
+        , m_parent(parent)
+        , m_managed_id(std::move(id))
+        , m_managed_version_id(std::move(version_id))
     {
         setStagingPath(staging_path);
         setParentSettings(global_settings);
+
+        m_original_instance_id = std::move(original_instance_id);
     }
 
     bool abort() override;
diff --git a/launcher/ui/pages/instance/ManagedPackPage.cpp b/launcher/ui/pages/instance/ManagedPackPage.cpp
index 0be27ffc..c70d70ab 100644
--- a/launcher/ui/pages/instance/ManagedPackPage.cpp
+++ b/launcher/ui/pages/instance/ManagedPackPage.cpp
@@ -275,6 +275,7 @@ void ModrinthManagedPackPage::update()
     // NOTE: Don't use 'm_pack.id' here, since we didn't completely parse all the metadata for the pack, including this field.
     extra_info.insert("pack_id", m_inst->getManagedPackID());
     extra_info.insert("pack_version_id", version.id);
+    extra_info.insert("original_instance_id", m_inst->id());
 
     auto extracted = new InstanceImportTask(version.download_url, this, std::move(extra_info));
 
@@ -413,6 +414,7 @@ void FlameManagedPackPage::update()
     QMap<QString, QString> extra_info;
     extra_info.insert("pack_id", m_inst->getManagedPackID());
     extra_info.insert("pack_version_id", QString::number(version.fileId));
+    extra_info.insert("original_instance_id", m_inst->id());
 
     auto extracted = new InstanceImportTask(version.downloadUrl, this, std::move(extra_info));