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>
This commit is contained in:
		| @@ -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); | ||||
|   | ||||
| @@ -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); | ||||
|   | ||||
| @@ -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. | ||||
|   | ||||
| @@ -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; | ||||
| }; | ||||
|   | ||||
| @@ -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; | ||||
|   | ||||
| @@ -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; | ||||
|   | ||||
| @@ -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; | ||||
|   | ||||
| @@ -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; | ||||
|   | ||||
| @@ -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)); | ||||
|  | ||||
|   | ||||
		Reference in New Issue
	
	Block a user