From 4441b373385f9b7f77deed2a27751337951f38f6 Mon Sep 17 00:00:00 2001
From: flow <flowlnlnln@gmail.com>
Date: Thu, 7 Jul 2022 21:10:41 -0300
Subject: refactor: move modrinth modpack import to separate file

Signed-off-by: flow <flowlnlnln@gmail.com>
---
 .../modrinth/ModrinthInstanceCreationTask.cpp      | 225 +++++++++++++++++++++
 1 file changed, 225 insertions(+)
 create mode 100644 launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp

(limited to 'launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp')

diff --git a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
new file mode 100644
index 00000000..7eb6cc8f
--- /dev/null
+++ b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
@@ -0,0 +1,225 @@
+#include "ModrinthInstanceCreationTask.h"
+
+#include "Application.h"
+#include "FileSystem.h"
+#include "Json.h"
+
+#include "minecraft/MinecraftInstance.h"
+#include "minecraft/PackProfile.h"
+
+#include "net/NetJob.h"
+#include "net/ChecksumValidator.h"
+
+#include "settings/INISettingsObject.h"
+
+#include "ui/dialogs/CustomMessageBox.h"
+
+bool ModrinthCreationTask::createInstance()
+{
+    QEventLoop loop;
+
+    if (m_files.empty() && !parseManifest())
+        return false;
+
+    auto mcPath = FS::PathCombine(m_stagingPath, ".minecraft");
+
+    auto override_path = FS::PathCombine(m_stagingPath, "overrides");
+    if (QFile::exists(override_path)) {
+        if (!QFile::rename(override_path, mcPath)) {
+            setError(tr("Could not rename the overrides folder:\n") + "overrides");
+            return false;
+        }
+    }
+
+    // Do client overrides
+    auto client_override_path = FS::PathCombine(m_stagingPath, "client-overrides");
+    if (QFile::exists(client_override_path)) {
+        if (!FS::overrideFolder(mcPath, client_override_path)) {
+            setError(tr("Could not rename the client overrides folder:\n") + "client overrides");
+            return false;
+        }
+    }
+
+    QString configPath = FS::PathCombine(m_stagingPath, "instance.cfg");
+    auto instanceSettings = std::make_shared<INISettingsObject>(configPath);
+    MinecraftInstance instance(m_globalSettings, instanceSettings, m_stagingPath);
+    auto components = instance.getPackProfile();
+    components->buildingFromScratch();
+    components->setComponentVersion("net.minecraft", minecraftVersion, true);
+
+    if (!fabricVersion.isEmpty())
+        components->setComponentVersion("net.fabricmc.fabric-loader", fabricVersion);
+    if (!quiltVersion.isEmpty())
+        components->setComponentVersion("org.quiltmc.quilt-loader", quiltVersion);
+    if (!forgeVersion.isEmpty())
+        components->setComponentVersion("net.minecraftforge", forgeVersion);
+    if (m_instIcon != "default") {
+        instance.setIconKey(m_instIcon);
+    } else {
+        instance.setIconKey("modrinth");
+    }
+    instance.setName(m_instName);
+    instance.setManagedPack("modrinth", getManagedPackID(), m_managed_name, m_managed_id, {});
+    instance.saveNow();
+
+    m_files_job = new NetJob(tr("Mod download"), APPLICATION->network());
+
+    for (auto file : m_files) {
+        auto path = FS::PathCombine(m_stagingPath, ".minecraft", file.path);
+        qDebug() << "Will try to download" << file.downloads.front() << "to" << path;
+        auto dl = Net::Download::makeFile(file.downloads.dequeue(), path);
+        dl->addValidator(new Net::ChecksumValidator(file.hashAlgorithm, file.hash));
+        m_files_job->addNetAction(dl);
+
+        if (file.downloads.size() > 0) {
+            // FIXME: This really needs to be put into a ConcurrentTask of
+            // MultipleOptionsTask's , once those exist :)
+            connect(dl.get(), &NetAction::failed, [this, &file, path, dl] {
+                auto dl = Net::Download::makeFile(file.downloads.dequeue(), path);
+                dl->addValidator(new Net::ChecksumValidator(file.hashAlgorithm, file.hash));
+                m_files_job->addNetAction(dl);
+                dl->succeeded();
+            });
+        }
+    }
+
+    bool ended_well = false;
+
+    connect(m_files_job.get(), &NetJob::succeeded, this, [&]() { ended_well = true; });
+    connect(m_files_job.get(), &NetJob::failed, [&](const QString& reason) {
+        ended_well = false;
+        setError(reason);
+    });
+    connect(m_files_job.get(), &NetJob::finished, &loop, &QEventLoop::quit);
+    connect(m_files_job.get(), &NetJob::progress, [&](qint64 current, qint64 total) { setProgress(current, total); });
+
+    setStatus(tr("Downloading mods..."));
+    m_files_job->start();
+
+    loop.exec();
+
+    return ended_well;
+}
+
+bool ModrinthCreationTask::parseManifest()
+{
+    try {
+        QString indexPath = FS::PathCombine(m_stagingPath, "modrinth.index.json");
+        auto doc = Json::requireDocument(indexPath);
+        auto obj = Json::requireObject(doc, "modrinth.index.json");
+        int formatVersion = Json::requireInteger(obj, "formatVersion", "modrinth.index.json");
+        if (formatVersion == 1) {
+            auto game = Json::requireString(obj, "game", "modrinth.index.json");
+            if (game != "minecraft") {
+                throw JSONValidationError("Unknown game: " + game);
+            }
+
+            m_managed_version_id = Json::ensureString(obj, "versionId", "Managed ID");
+            m_managed_name = Json::ensureString(obj, "name", "Managed Name");
+
+            auto jsonFiles = Json::requireIsArrayOf<QJsonObject>(obj, "files", "modrinth.index.json");
+            bool had_optional = false;
+            for (auto modInfo : jsonFiles) {
+                Modrinth::File file;
+                file.path = Json::requireString(modInfo, "path");
+
+                auto env = Json::ensureObject(modInfo, "env");
+                // 'env' field is optional
+                if (!env.isEmpty()) {
+                    QString support = Json::ensureString(env, "client", "unsupported");
+                    if (support == "unsupported") {
+                        continue;
+                    } else if (support == "optional") {
+                        // TODO: Make a review dialog for choosing which ones the user wants!
+                        if (!had_optional) {
+                            had_optional = true;
+                            auto info = CustomMessageBox::selectable(
+                                m_parent, tr("Optional mod detected!"),
+                                tr("One or more mods from this modpack are optional. They will be downloaded, but disabled by default!"),
+                                QMessageBox::Information);
+                            info->exec();
+                        }
+
+                        if (file.path.endsWith(".jar"))
+                            file.path += ".disabled";
+                    }
+                }
+
+                QJsonObject hashes = Json::requireObject(modInfo, "hashes");
+                QString hash;
+                QCryptographicHash::Algorithm hashAlgorithm;
+                hash = Json::ensureString(hashes, "sha1");
+                hashAlgorithm = QCryptographicHash::Sha1;
+                if (hash.isEmpty()) {
+                    hash = Json::ensureString(hashes, "sha512");
+                    hashAlgorithm = QCryptographicHash::Sha512;
+                    if (hash.isEmpty()) {
+                        hash = Json::ensureString(hashes, "sha256");
+                        hashAlgorithm = QCryptographicHash::Sha256;
+                        if (hash.isEmpty()) {
+                            throw JSONValidationError("No hash found for: " + file.path);
+                        }
+                    }
+                }
+                file.hash = QByteArray::fromHex(hash.toLatin1());
+                file.hashAlgorithm = hashAlgorithm;
+
+                // Do not use requireUrl, which uses StrictMode, instead use QUrl's default TolerantMode
+                // (as Modrinth seems to incorrectly handle spaces)
+
+                auto download_arr = Json::ensureArray(modInfo, "downloads");
+                for (auto download : download_arr) {
+                    qWarning() << download.toString();
+                    bool is_last = download.toString() == download_arr.last().toString();
+
+                    auto download_url = QUrl(download.toString());
+
+                    if (!download_url.isValid()) {
+                        qDebug()
+                            << QString("Download URL (%1) for %2 is not a correctly formatted URL").arg(download_url.toString(), file.path);
+                        if (is_last && file.downloads.isEmpty())
+                            throw JSONValidationError(tr("Download URL for %1 is not a correctly formatted URL").arg(file.path));
+                    } else {
+                        file.downloads.push_back(download_url);
+                    }
+                }
+
+                m_files.push_back(file);
+            }
+
+            auto dependencies = Json::requireObject(obj, "dependencies", "modrinth.index.json");
+            for (auto it = dependencies.begin(), end = dependencies.end(); it != end; ++it) {
+                QString name = it.key();
+                if (name == "minecraft") {
+                    minecraftVersion = Json::requireString(*it, "Minecraft version");
+                } else if (name == "fabric-loader") {
+                    fabricVersion = Json::requireString(*it, "Fabric Loader version");
+                } else if (name == "quilt-loader") {
+                    quiltVersion = Json::requireString(*it, "Quilt Loader version");
+                } else if (name == "forge") {
+                    forgeVersion = Json::requireString(*it, "Forge version");
+                } else {
+                    throw JSONValidationError("Unknown dependency type: " + name);
+                }
+            }
+        } else {
+            throw JSONValidationError(QStringLiteral("Unknown format version: %s").arg(formatVersion));
+        }
+        QFile::remove(indexPath);
+    } catch (const JSONValidationError& e) {
+        setError(tr("Could not understand pack index:\n") + e.cause());
+        return false;
+    }
+
+    return true;
+}
+
+QString ModrinthCreationTask::getManagedPackID() const
+{
+    if (!m_source_url.isEmpty()) {
+        QRegularExpression regex(R"(data\/(.*)\/versions)");
+        return regex.match(m_source_url).captured(0);
+    }
+
+    return {};
+}
-- 
cgit 


From 208ed73e59c46ee7966f463558c07805a9b541e6 Mon Sep 17 00:00:00 2001
From: flow <flowlnlnln@gmail.com>
Date: Fri, 8 Jul 2022 13:00:44 -0300
Subject: feat: add early modrinth pack updating

Still some FIXMEs and TODOs to consider, but the general thing is here!

Signed-off-by: flow <flowlnlnln@gmail.com>
---
 launcher/InstanceImportTask.cpp                    |  11 +-
 launcher/InstanceList.cpp                          |  66 +++++++++---
 launcher/InstanceList.h                            |   7 +-
 launcher/InstanceTask.h                            |   7 ++
 .../modrinth/ModrinthInstanceCreationTask.cpp      | 118 +++++++++++++++++++--
 .../modrinth/ModrinthInstanceCreationTask.h        |   2 +-
 6 files changed, 185 insertions(+), 26 deletions(-)

(limited to 'launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp')

diff --git a/launcher/InstanceImportTask.cpp b/launcher/InstanceImportTask.cpp
index b19b5fa6..4bdf9cd2 100644
--- a/launcher/InstanceImportTask.cpp
+++ b/launcher/InstanceImportTask.cpp
@@ -593,15 +593,16 @@ void InstanceImportTask::processModrinth()
     inst_creation_task->setIcon(m_instIcon);
     inst_creation_task->setGroup(m_instGroup);
     
-    connect(inst_creation_task, &Task::succeeded, this, &InstanceImportTask::emitSucceeded);
+    connect(inst_creation_task, &Task::succeeded, this, [this, inst_creation_task] {
+        setOverride(inst_creation_task->shouldOverride());
+        emitSucceeded();
+    });
     connect(inst_creation_task, &Task::failed, this, &InstanceImportTask::emitFailed);
     connect(inst_creation_task, &Task::progress, this, &InstanceImportTask::setProgress);
     connect(inst_creation_task, &Task::status, this, &InstanceImportTask::setStatus);
-    connect(inst_creation_task, &Task::finished, this, [inst_creation_task]{ inst_creation_task->deleteLater(); });
+    connect(inst_creation_task, &Task::finished, inst_creation_task, &InstanceCreationTask::deleteLater);
 
-    connect(this, &Task::aborted, inst_creation_task, [inst_creation_task] {
-        inst_creation_task->abort();
-    });
+    connect(this, &Task::aborted, inst_creation_task, &InstanceCreationTask::abort);
 
     inst_creation_task->start();
 }
diff --git a/launcher/InstanceList.cpp b/launcher/InstanceList.cpp
index 4447a17c..698aa24e 100644
--- a/launcher/InstanceList.cpp
+++ b/launcher/InstanceList.cpp
@@ -535,7 +535,20 @@ InstancePtr InstanceList::getInstanceById(QString instId) const
     return InstancePtr();
 }
 
-QModelIndex InstanceList::getInstanceIndexById(const QString& id) const
+InstancePtr InstanceList::getInstanceByManagedName(QString managed_name) const
+{
+    if (managed_name.isEmpty())
+        return {};
+
+    for (auto instance : m_instances) {
+        if (instance->getManagedPackName() == managed_name)
+            return instance;
+    }
+
+    return {};
+}
+
+QModelIndex InstanceList::getInstanceIndexById(const QString &id) const
 {
     return index(getInstIndex(getInstanceById(id).get()));
 }
@@ -764,9 +777,8 @@ class InstanceStaging : public Task {
     Q_OBJECT
     const unsigned minBackoff = 1;
     const unsigned maxBackoff = 16;
-
    public:
-    InstanceStaging(InstanceList* parent, Task* child, const QString& stagingPath, const QString& instanceName, const QString& groupName)
+    InstanceStaging(InstanceList* parent, InstanceTask* child, const QString& stagingPath, const QString& instanceName, const QString& groupName)
         : backoff(minBackoff, maxBackoff)
     {
         m_parent = parent;
@@ -808,7 +820,8 @@ class InstanceStaging : public Task {
     void childSucceded()
     {
         unsigned sleepTime = backoff();
-        if (m_parent->commitStagedInstance(m_stagingPath, m_instanceName, m_groupName)) {
+        if (m_parent->commitStagedInstance(m_stagingPath, m_instanceName, m_groupName, m_child->shouldOverride()))
+        {
             emitSucceeded();
             return;
         }
@@ -834,8 +847,8 @@ class InstanceStaging : public Task {
      */
     ExponentialSeries backoff;
     QString m_stagingPath;
-    InstanceList* m_parent;
-    unique_qobject_ptr<Task> m_child;
+    InstanceList * m_parent;
+    unique_qobject_ptr<InstanceTask> m_child;
     QString m_instanceName;
     QString m_groupName;
     QTimer m_backoffTimer;
@@ -866,23 +879,52 @@ QString InstanceList::getStagedInstancePath()
     return path;
 }
 
-bool InstanceList::commitStagedInstance(const QString& path, const QString& instanceName, const QString& groupName)
+bool InstanceList::commitStagedInstance(const QString& path, const QString& instanceName, const QString& groupName, bool should_override)
 {
     QDir dir;
-    QString instID = FS::DirNameFromString(instanceName, m_instDir);
+    QString instID;
+    InstancePtr inst;
+
+    QString raw_inst_name = instanceName.section(' ', 0, -2);
+    if (should_override) {
+        // This is to avoid problems when the instance folder gets manually renamed
+        if ((inst = getInstanceByManagedName(raw_inst_name))) {
+            instID = QFileInfo(inst->instanceRoot()).fileName();
+        } else {
+            instID = FS::RemoveInvalidFilenameChars(raw_inst_name, '-');
+        }
+    } else {
+        instID = FS::DirNameFromString(raw_inst_name, m_instDir);
+    }
+
     {
         WatchLock lock(m_watcher, m_instDir);
         QString destination = FS::PathCombine(m_instDir, instID);
-        if (!dir.rename(path, destination)) {
-            qWarning() << "Failed to move" << path << "to" << destination;
-            return false;
+
+        if (should_override) {
+            if (!FS::overrideFolder(destination, path)) {
+                qWarning() << "Failed to override" << path << "to" << destination;
+                return false;
+            }
+
+            if (!inst)
+                inst = getInstanceById(instID);
+            if (inst)
+                inst->setName(instanceName);
+        } else {
+            if (!dir.rename(path, destination)) {
+                qWarning() << "Failed to move" << path << "to" << destination;
+                return false;
+            }
         }
         m_instanceGroupIndex[instID] = groupName;
-        instanceSet.insert(instID);
         m_groupNameCache.insert(groupName);
+        instanceSet.insert(instID);
+
         emit instancesChanged();
         emit instanceSelectRequest(instID);
     }
+
     saveGroupList();
     return true;
 }
diff --git a/launcher/InstanceList.h b/launcher/InstanceList.h
index 62282f04..6b4dcfa4 100644
--- a/launcher/InstanceList.h
+++ b/launcher/InstanceList.h
@@ -101,7 +101,10 @@ public:
     InstListError loadList();
     void saveNow();
 
+    /* O(n) */
     InstancePtr getInstanceById(QString id) const;
+    /* O(n) */
+    InstancePtr getInstanceByManagedName(QString managed_name) const;
     QModelIndex getInstanceIndexById(const QString &id) const;
     QStringList getGroups();
     bool isGroupCollapsed(const QString &groupName);
@@ -127,8 +130,10 @@ public:
     /**
      * Commit the staging area given by @keyPath to the provider - used when creation succeeds.
      * Used by instance manipulation tasks.
+     * 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 QString& instanceName, const QString & groupName);
+    bool commitStagedInstance(const QString & keyPath, const QString& instanceName, const QString & groupName, bool should_override);
 
     /**
      * Destroy a previously created staging area given by @keyPath - used when creation fails.
diff --git a/launcher/InstanceTask.h b/launcher/InstanceTask.h
index 82e23f11..02810a52 100644
--- a/launcher/InstanceTask.h
+++ b/launcher/InstanceTask.h
@@ -43,10 +43,17 @@ public:
         return m_instGroup;
     }
 
+    bool shouldOverride() const { return m_override_existing; }
+
+protected:
+    void setOverride(bool override) { m_override_existing = override; }
+
 protected: /* data */
     SettingsObjectPtr m_globalSettings;
     QString m_instName;
     QString m_instIcon;
     QString m_instGroup;
     QString m_stagingPath;
+
+    bool m_override_existing = false;
 };
diff --git a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
index 7eb6cc8f..efb8c99b 100644
--- a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
+++ b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
@@ -2,25 +2,128 @@
 
 #include "Application.h"
 #include "FileSystem.h"
+#include "InstanceList.h"
 #include "Json.h"
 
 #include "minecraft/MinecraftInstance.h"
 #include "minecraft/PackProfile.h"
 
-#include "net/NetJob.h"
+#include "modplatform/ModIndex.h"
+
 #include "net/ChecksumValidator.h"
 
 #include "settings/INISettingsObject.h"
 
 #include "ui/dialogs/CustomMessageBox.h"
 
+#include <QAbstractButton>
+
+bool ModrinthCreationTask::abort()
+{
+    if (m_files_job)
+        return m_files_job->abort();
+    return true;
+}
+
+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?
+    // Based on the way we create the instance name (name + " " + version). Is there a better way?
+    auto inst = instance_list->getInstanceByManagedName(m_instName.section(' ', 0, -2));
+
+    if (!inst) {
+        inst = instance_list->getInstanceById(m_instName);
+
+        if (!inst)
+            return false;
+    }
+
+    QString index_path = FS::PathCombine(m_stagingPath, "modrinth.index.json");
+    if (!parseManifest(index_path, m_files))
+        return false;
+
+    auto version_id = inst->getManagedPackVersionID();
+    auto version_str = !version_id.isEmpty() ? tr(" (version %1)").arg(version_id) : "";
+
+    auto info = CustomMessageBox::selectable(m_parent, tr("Similar modpack was found!"),
+                                             tr("One or more of your instances are from this same modpack%1. Do you want to create a "
+                                                "separate instance, or update the existing one?")
+                                                 .arg(version_str),
+                                             QMessageBox::Information, QMessageBox::Ok | QMessageBox::Abort);
+    info->setButtonText(QMessageBox::Ok, tr("Update existing instance"));
+    info->setButtonText(QMessageBox::Abort, tr("Create new instance"));
+
+    if (info->exec() && info->clickedButton() == info->button(QMessageBox::Abort))
+        return false;
+
+    // Remove repeated files, we don't need to download them!
+    QDir old_inst_dir(inst->instanceRoot());
+
+    QString old_index_path(FS::PathCombine(old_inst_dir.absolutePath(), "mrpack", "modrinth.index.json"));
+    QFileInfo old_index_file(old_index_path);
+    if (old_index_file.exists()) {
+        std::vector<Modrinth::File> old_files;
+        parseManifest(old_index_path, old_files);
+
+        // Let's remove all duplicated, identical resources!
+        auto files_iterator = m_files.begin();
+begin:
+        while (files_iterator != m_files.end()) {
+            auto const& file = *files_iterator;
+
+            auto old_files_iterator = old_files.begin();
+            while (old_files_iterator != old_files.end()) {
+                auto const& old_file = *old_files_iterator;
+
+                if (old_file.hash == file.hash) {
+                    qDebug() << "Removed file at" << file.path << "from list of downloads";
+                    files_iterator = m_files.erase(files_iterator);
+                    old_files_iterator = old_files.erase(old_files_iterator);
+                    goto begin; // Sorry :c
+                }
+
+                old_files_iterator++;
+            }
+
+            files_iterator++;
+        }
+
+        // Some files were removed from the old version, and some will be downloaded in an updated version,
+        // so we're fine removing them!
+        if (!old_files.empty()) {
+            QDir old_minecraft_dir(inst->gameRoot());
+            for (auto const& file : old_files) {
+                qWarning() << "Removing" << file.path;
+                old_minecraft_dir.remove(file.path);
+            }
+        }
+    }
+
+    // TODO: Currently 'overrides' will always override the stuff on update. How do we preserve unchanged overrides?
+
+    setOverride(true);
+    qDebug() << "Will override instance!";
+
+    // We let it go through the createInstance() stage, just with a couple modifications for updating
+    return false;
+}
+
+// https://docs.modrinth.com/docs/modpacks/format_definition/
 bool ModrinthCreationTask::createInstance()
 {
     QEventLoop loop;
 
-    if (m_files.empty() && !parseManifest())
+    QString index_path = FS::PathCombine(m_stagingPath, "modrinth.index.json");
+    if (m_files.empty() && !parseManifest(index_path, m_files))
         return false;
 
+    // Keep index file in case we need it some other time (like when changing versions)
+    QString new_index_place(FS::PathCombine(m_stagingPath, "mrpack", "modrinth.index.json"));
+    FS::ensureFilePathExists(new_index_place);
+    QFile::rename(index_path, new_index_place);
+
     auto mcPath = FS::PathCombine(m_stagingPath, ".minecraft");
 
     auto override_path = FS::PathCombine(m_stagingPath, "overrides");
@@ -43,6 +146,7 @@ bool ModrinthCreationTask::createInstance()
     QString configPath = FS::PathCombine(m_stagingPath, "instance.cfg");
     auto instanceSettings = std::make_shared<INISettingsObject>(configPath);
     MinecraftInstance instance(m_globalSettings, instanceSettings, m_stagingPath);
+
     auto components = instance.getPackProfile();
     components->buildingFromScratch();
     components->setComponentVersion("net.minecraft", minecraftVersion, true);
@@ -53,6 +157,7 @@ bool ModrinthCreationTask::createInstance()
         components->setComponentVersion("org.quiltmc.quilt-loader", quiltVersion);
     if (!forgeVersion.isEmpty())
         components->setComponentVersion("net.minecraftforge", forgeVersion);
+
     if (m_instIcon != "default") {
         instance.setIconKey(m_instIcon);
     } else {
@@ -101,11 +206,10 @@ bool ModrinthCreationTask::createInstance()
     return ended_well;
 }
 
-bool ModrinthCreationTask::parseManifest()
+bool ModrinthCreationTask::parseManifest(QString index_path, std::vector<Modrinth::File>& files)
 {
     try {
-        QString indexPath = FS::PathCombine(m_stagingPath, "modrinth.index.json");
-        auto doc = Json::requireDocument(indexPath);
+        auto doc = Json::requireDocument(index_path);
         auto obj = Json::requireObject(doc, "modrinth.index.json");
         int formatVersion = Json::requireInteger(obj, "formatVersion", "modrinth.index.json");
         if (formatVersion == 1) {
@@ -184,7 +288,7 @@ bool ModrinthCreationTask::parseManifest()
                     }
                 }
 
-                m_files.push_back(file);
+                files.push_back(file);
             }
 
             auto dependencies = Json::requireObject(obj, "dependencies", "modrinth.index.json");
@@ -205,7 +309,7 @@ bool ModrinthCreationTask::parseManifest()
         } else {
             throw JSONValidationError(QStringLiteral("Unknown format version: %s").arg(formatVersion));
         }
-        QFile::remove(indexPath);
+
     } catch (const JSONValidationError& e) {
         setError(tr("Could not understand pack index:\n") + e.cause());
         return false;
diff --git a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.h b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.h
index 61f7dd5c..4e804e58 100644
--- a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.h
+++ b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.h
@@ -24,7 +24,7 @@ class ModrinthCreationTask final : public InstanceCreationTask {
     bool createInstance() override;
 
    private:
-    bool parseManifest();
+    bool parseManifest(QString, std::vector<Modrinth::File>&);
     QString getManagedPackID() const;
 
    private:
-- 
cgit 


From 6131346e2f80c5ce4377fcc608be4f3929f43f91 Mon Sep 17 00:00:00 2001
From: flow <flowlnlnln@gmail.com>
Date: Thu, 14 Jul 2022 16:13:23 -0300
Subject: refactor: change the way instance names are handled

While working on pack updating, instance naming always gets in the way,
since we need both way of respecting the user's name choice, and a
standarized way of getting the original pack name / version.

This tries to circunvent such problems by abstracting away the naming
schema into it's own struct, holding both the original name / version,
and the user-defined name, so that everyone can be happy and world peace
can be achieved! (at least that's what i'd hope :c).

Signed-off-by: flow <flowlnlnln@gmail.com>
---
 launcher/InstanceCopyTask.cpp                      |  2 +-
 launcher/InstanceImportTask.cpp                    |  8 +--
 launcher/InstanceList.cpp                          | 32 ++++-----
 launcher/InstanceList.h                            |  6 +-
 launcher/InstanceTask.cpp                          | 29 +++++++-
 launcher/InstanceTask.h                            | 78 ++++++++++------------
 launcher/minecraft/VanillaInstanceCreationTask.cpp |  4 +-
 .../modplatform/atlauncher/ATLPackInstallTask.cpp  |  2 +-
 .../flame/FlameInstanceCreationTask.cpp            |  4 +-
 .../modplatform/legacy_ftb/PackInstallTask.cpp     |  2 +-
 .../modplatform/modpacksch/FTBPackInstallTask.cpp  |  2 +-
 .../modrinth/ModrinthInstanceCreationTask.cpp      | 10 ++-
 .../technic/SingleZipPackInstallTask.cpp           |  2 +-
 .../modplatform/technic/SolderPackInstallTask.cpp  |  2 +-
 launcher/ui/dialogs/NewInstanceDialog.cpp          | 29 ++++++--
 launcher/ui/dialogs/NewInstanceDialog.h            |  6 +-
 .../ui/pages/modplatform/atlauncher/AtlPage.cpp    |  2 +-
 launcher/ui/pages/modplatform/ftb/FtbPage.cpp      |  2 +-
 launcher/ui/pages/modplatform/legacy_ftb/Page.cpp  |  2 +-
 .../ui/pages/modplatform/modrinth/ModrinthPage.cpp |  2 +-
 .../ui/pages/modplatform/technic/TechnicPage.cpp   |  4 +-
 21 files changed, 134 insertions(+), 96 deletions(-)

(limited to 'launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp')

diff --git a/launcher/InstanceCopyTask.cpp b/launcher/InstanceCopyTask.cpp
index c2bfe839..b1e33884 100644
--- a/launcher/InstanceCopyTask.cpp
+++ b/launcher/InstanceCopyTask.cpp
@@ -44,7 +44,7 @@ void InstanceCopyTask::copyFinished()
     auto instanceSettings = std::make_shared<INISettingsObject>(FS::PathCombine(m_stagingPath, "instance.cfg"));
 
     InstancePtr inst(new NullInstance(m_globalSettings, instanceSettings, m_stagingPath));
-    inst->setName(m_instName);
+    inst->setName(name());
     inst->setIconKey(m_instIcon);
     if(!m_keepPlaytime) {
         inst->resetTimePlayed();
diff --git a/launcher/InstanceImportTask.cpp b/launcher/InstanceImportTask.cpp
index da57ddeb..09e65064 100644
--- a/launcher/InstanceImportTask.cpp
+++ b/launcher/InstanceImportTask.cpp
@@ -255,7 +255,7 @@ void InstanceImportTask::processFlame()
 {
     auto* inst_creation_task = new FlameCreationTask(m_stagingPath, m_globalSettings, m_parent);
 
-    inst_creation_task->setName(m_instName);
+    inst_creation_task->setName(*this);
     inst_creation_task->setIcon(m_instIcon);
     inst_creation_task->setGroup(m_instGroup);
     
@@ -278,7 +278,7 @@ void InstanceImportTask::processTechnic()
     shared_qobject_ptr<Technic::TechnicPackProcessor> packProcessor = new Technic::TechnicPackProcessor();
     connect(packProcessor.get(), &Technic::TechnicPackProcessor::succeeded, this, &InstanceImportTask::emitSucceeded);
     connect(packProcessor.get(), &Technic::TechnicPackProcessor::failed, this, &InstanceImportTask::emitFailed);
-    packProcessor->run(m_globalSettings, m_instName, m_instIcon, m_stagingPath);
+    packProcessor->run(m_globalSettings, name(), m_instIcon, m_stagingPath);
 }
 
 void InstanceImportTask::processMultiMC()
@@ -292,7 +292,7 @@ void InstanceImportTask::processMultiMC()
     instance.resetTimePlayed();
 
     // set a new nice name
-    instance.setName(m_instName);
+    instance.setName(name());
 
     // if the icon was specified by user, use that. otherwise pull icon from the pack
     if (m_instIcon != "default") {
@@ -317,7 +317,7 @@ void InstanceImportTask::processModrinth()
 {
     auto* inst_creation_task = new ModrinthCreationTask(m_stagingPath, m_globalSettings, m_parent, m_sourceUrl.toString());
 
-    inst_creation_task->setName(m_instName);
+    inst_creation_task->setName(*this);
     inst_creation_task->setIcon(m_instIcon);
     inst_creation_task->setGroup(m_instGroup);
     
diff --git a/launcher/InstanceList.cpp b/launcher/InstanceList.cpp
index 698aa24e..0e59150e 100644
--- a/launcher/InstanceList.cpp
+++ b/launcher/InstanceList.cpp
@@ -778,19 +778,14 @@ class InstanceStaging : public Task {
     const unsigned minBackoff = 1;
     const unsigned maxBackoff = 16;
    public:
-    InstanceStaging(InstanceList* parent, InstanceTask* child, const QString& stagingPath, const QString& instanceName, const QString& groupName)
-        : backoff(minBackoff, maxBackoff)
+    InstanceStaging(InstanceList* parent, InstanceTask* child, QString stagingPath, InstanceName const& instanceName, QString groupName)
+        : m_parent(parent), backoff(minBackoff, maxBackoff), m_stagingPath(std::move(stagingPath)), m_instance_name(std::move(instanceName)), m_groupName(std::move(groupName))
     {
-        m_parent = parent;
         m_child.reset(child);
         connect(child, &Task::succeeded, this, &InstanceStaging::childSucceded);
         connect(child, &Task::failed, this, &InstanceStaging::childFailed);
         connect(child, &Task::status, this, &InstanceStaging::setStatus);
         connect(child, &Task::progress, this, &InstanceStaging::setProgress);
-        m_instanceName = instanceName;
-        m_groupName = groupName;
-        m_stagingPath = stagingPath;
-        m_backoffTimer.setSingleShot(true);
         connect(&m_backoffTimer, &QTimer::timeout, this, &InstanceStaging::childSucceded);
     }
 
@@ -820,7 +815,7 @@ class InstanceStaging : public Task {
     void childSucceded()
     {
         unsigned sleepTime = backoff();
-        if (m_parent->commitStagedInstance(m_stagingPath, m_instanceName, m_groupName, m_child->shouldOverride()))
+        if (m_parent->commitStagedInstance(m_stagingPath, m_instance_name, m_groupName, m_child->shouldOverride()))
         {
             emitSucceeded();
             return;
@@ -830,7 +825,7 @@ class InstanceStaging : public Task {
             emitFailed(tr("Failed to commit instance, even after multiple retries. It is being blocked by something."));
             return;
         }
-        qDebug() << "Failed to commit instance" << m_instanceName << "Initiating backoff:" << sleepTime;
+        qDebug() << "Failed to commit instance" << m_instance_name.name() << "Initiating backoff:" << sleepTime;
         m_backoffTimer.start(sleepTime * 500);
     }
     void childFailed(const QString& reason)
@@ -840,6 +835,7 @@ class InstanceStaging : public Task {
     }
 
    private:
+    InstanceList * m_parent;
     /*
      * WHY: the whole reason why this uses an exponential backoff retry scheme is antivirus on Windows.
      * Basically, it starts messing things up while the launcher is extracting/creating instances
@@ -847,9 +843,8 @@ class InstanceStaging : public Task {
      */
     ExponentialSeries backoff;
     QString m_stagingPath;
-    InstanceList * m_parent;
     unique_qobject_ptr<InstanceTask> m_child;
-    QString m_instanceName;
+    InstanceName m_instance_name;
     QString m_groupName;
     QTimer m_backoffTimer;
 };
@@ -859,7 +854,7 @@ Task* InstanceList::wrapInstanceTask(InstanceTask* task)
     auto stagingPath = getStagedInstancePath();
     task->setStagingPath(stagingPath);
     task->setParentSettings(m_globalSettings);
-    return new InstanceStaging(this, task, stagingPath, task->name(), task->group());
+    return new InstanceStaging(this, task, stagingPath, *task, task->group());
 }
 
 QString InstanceList::getStagedInstancePath()
@@ -879,22 +874,23 @@ QString InstanceList::getStagedInstancePath()
     return path;
 }
 
-bool InstanceList::commitStagedInstance(const QString& path, const QString& instanceName, const QString& groupName, bool should_override)
+bool InstanceList::commitStagedInstance(QString path, InstanceName const& instanceName, QString groupName, bool should_override)
 {
     QDir dir;
     QString instID;
     InstancePtr inst;
 
-    QString raw_inst_name = instanceName.section(' ', 0, -2);
     if (should_override) {
         // This is to avoid problems when the instance folder gets manually renamed
-        if ((inst = getInstanceByManagedName(raw_inst_name))) {
+        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(raw_inst_name, '-');
+            instID = FS::RemoveInvalidFilenameChars(instanceName.modifiedName(), '-');
         }
     } else {
-        instID = FS::DirNameFromString(raw_inst_name, m_instDir);
+        instID = FS::DirNameFromString(instanceName.modifiedName(), m_instDir);
     }
 
     {
@@ -910,7 +906,7 @@ bool InstanceList::commitStagedInstance(const QString& path, const QString& inst
             if (!inst)
                 inst = getInstanceById(instID);
             if (inst)
-                inst->setName(instanceName);
+                inst->setName(instanceName.name());
         } else {
             if (!dir.rename(path, destination)) {
                 qWarning() << "Failed to move" << path << "to" << destination;
diff --git a/launcher/InstanceList.h b/launcher/InstanceList.h
index 6b4dcfa4..df11b55a 100644
--- a/launcher/InstanceList.h
+++ b/launcher/InstanceList.h
@@ -24,10 +24,10 @@
 
 #include "BaseInstance.h"
 
-#include "QObjectPtr.h"
-
 class QFileSystemWatcher;
 class InstanceTask;
+struct InstanceName;
+
 using InstanceId = QString;
 using GroupId = QString;
 using InstanceLocator = std::pair<InstancePtr, int>;
@@ -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 QString& instanceName, const QString & groupName, bool should_override);
+    bool commitStagedInstance(QString keyPath, const InstanceName& instanceName, QString groupName, bool should_override);
 
     /**
      * Destroy a previously created staging area given by @keyPath - used when creation fails.
diff --git a/launcher/InstanceTask.cpp b/launcher/InstanceTask.cpp
index dd132877..43a0b947 100644
--- a/launcher/InstanceTask.cpp
+++ b/launcher/InstanceTask.cpp
@@ -1,9 +1,34 @@
 #include "InstanceTask.h"
 
-InstanceTask::InstanceTask()
+QString InstanceName::name() const
 {
+    if (!m_modified_name.isEmpty())
+        return modifiedName();
+    return QString("%1 %2").arg(m_original_name, m_original_version);
 }
 
-InstanceTask::~InstanceTask()
+QString InstanceName::originalName() const
 {
+    return m_original_name;
 }
+
+QString InstanceName::modifiedName() const
+{
+    if (!m_modified_name.isEmpty())
+        return m_modified_name;
+    return m_original_name;
+}
+
+QString InstanceName::version() const
+{
+    return m_original_version;
+}
+
+void InstanceName::setName(InstanceName& other)
+{
+   m_original_name = other.m_original_name; 
+   m_original_version = other.m_original_version; 
+   m_modified_name = other.m_modified_name;
+}
+
+InstanceTask::InstanceTask() : Task(), InstanceName() {}
diff --git a/launcher/InstanceTask.h b/launcher/InstanceTask.h
index 02810a52..5d67a2f0 100644
--- a/launcher/InstanceTask.h
+++ b/launcher/InstanceTask.h
@@ -1,56 +1,50 @@
 #pragma once
 
-#include "tasks/Task.h"
 #include "settings/SettingsObject.h"
+#include "tasks/Task.h"
+
+struct InstanceName {
+   public:
+    InstanceName() = default;
+    InstanceName(QString name, QString version) : m_original_name(std::move(name)), m_original_version(std::move(version)) {}
+
+    [[nodiscard]] QString modifiedName() const;
+    [[nodiscard]] QString originalName() const;
+    [[nodiscard]] QString name() const;
+    [[nodiscard]] QString version() const;
+
+    void setName(QString name) { m_modified_name = name; }
+    void setName(InstanceName& other);
+
+   protected:
+    QString m_original_name;
+    QString m_original_version;
 
-class InstanceTask : public Task
-{
+    QString m_modified_name;
+};
+
+class InstanceTask : public Task, public InstanceName {
     Q_OBJECT
-public:
-    explicit InstanceTask();
-    virtual ~InstanceTask();
-
-    void setParentSettings(SettingsObjectPtr settings)
-    {
-        m_globalSettings = settings;
-    }
-
-    void setStagingPath(const QString &stagingPath)
-    {
-        m_stagingPath = stagingPath;
-    }
-
-    void setName(const QString &name)
-    {
-        m_instName = name;
-    }
-    QString name() const
-    {
-        return m_instName;
-    }
-
-    void setIcon(const QString &icon)
-    {
-        m_instIcon = icon;
-    }
-
-    void setGroup(const QString &group)
-    {
-        m_instGroup = group;
-    }
-    QString group() const
-    {
-        return m_instGroup;
-    }
+   public:
+    InstanceTask();
+    ~InstanceTask() override = default;
+
+    void setParentSettings(SettingsObjectPtr settings) { m_globalSettings = settings; }
+
+    void setStagingPath(const QString& stagingPath) { m_stagingPath = stagingPath; }
+
+    void setIcon(const QString& icon) { m_instIcon = icon; }
+
+    void setGroup(const QString& group) { m_instGroup = group; }
+    QString group() const { return m_instGroup; }
 
     bool shouldOverride() const { return m_override_existing; }
 
-protected:
+   protected:
     void setOverride(bool override) { m_override_existing = override; }
 
-protected: /* data */
+   protected: /* data */
     SettingsObjectPtr m_globalSettings;
-    QString m_instName;
     QString m_instIcon;
     QString m_instGroup;
     QString m_stagingPath;
diff --git a/launcher/minecraft/VanillaInstanceCreationTask.cpp b/launcher/minecraft/VanillaInstanceCreationTask.cpp
index 2d1593b6..fb11379c 100644
--- a/launcher/minecraft/VanillaInstanceCreationTask.cpp
+++ b/launcher/minecraft/VanillaInstanceCreationTask.cpp
@@ -1,9 +1,9 @@
 #include "VanillaInstanceCreationTask.h"
 
 #include "FileSystem.h"
-#include "settings/INISettingsObject.h"
 #include "minecraft/MinecraftInstance.h"
 #include "minecraft/PackProfile.h"
+#include "settings/INISettingsObject.h"
 
 VanillaCreationTask::VanillaCreationTask(BaseVersionPtr version, QString loader, BaseVersionPtr loader_version)
     : InstanceCreationTask(), m_version(version), m_using_loader(true), m_loader(loader), m_loader_version(loader_version)
@@ -23,7 +23,7 @@ bool VanillaCreationTask::createInstance()
         if(m_using_loader)
             components->setComponentVersion(m_loader, m_loader_version->descriptor());
 
-        inst.setName(m_instName);
+        inst.setName(name());
         inst.setIconKey(m_instIcon);
     }
     instance_settings->resumeSave();
diff --git a/launcher/modplatform/atlauncher/ATLPackInstallTask.cpp b/launcher/modplatform/atlauncher/ATLPackInstallTask.cpp
index 70a35395..13cab256 100644
--- a/launcher/modplatform/atlauncher/ATLPackInstallTask.cpp
+++ b/launcher/modplatform/atlauncher/ATLPackInstallTask.cpp
@@ -1005,7 +1005,7 @@ void PackInstallTask::install()
 
     components->saveNow();
 
-    instance.setName(m_instName);
+    instance.setName(name());
     instance.setIconKey(m_instIcon);
     instance.setManagedPack("atlauncher", m_pack_safe_name, m_pack_name, m_version_name, m_version_name);
     instanceSettings->resumeSave();
diff --git a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp
index f0f02bc8..202caa28 100644
--- a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp
+++ b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp
@@ -5,6 +5,7 @@
 
 #include "Application.h"
 #include "FileSystem.h"
+#include "InstanceList.h"
 #include "Json.h"
 
 #include "minecraft/MinecraftInstance.h"
@@ -288,7 +289,8 @@ bool FlameCreationTask::createInstance()
         FS::deletePath(jarmodsPath);
     }
 
-    instance.setName(m_instName);
+    instance.setManagedPack("flame", {}, m_pack.name, {}, m_pack.version);
+    instance.setName(name());
 
     m_mod_id_resolver = new Flame::FileResolvingTask(APPLICATION->network(), m_pack);
     connect(m_mod_id_resolver.get(), &Flame::FileResolvingTask::succeeded, this, [this, &loop] { idResolverSucceeded(loop); });
diff --git a/launcher/modplatform/legacy_ftb/PackInstallTask.cpp b/launcher/modplatform/legacy_ftb/PackInstallTask.cpp
index 83e14969..e2e61a7c 100644
--- a/launcher/modplatform/legacy_ftb/PackInstallTask.cpp
+++ b/launcher/modplatform/legacy_ftb/PackInstallTask.cpp
@@ -228,7 +228,7 @@ void PackInstallTask::install()
 
     progress(4, 4);
 
-    instance.setName(m_instName);
+    instance.setName(name());
     if(m_instIcon == "default")
     {
         m_instIcon = "ftb_logo";
diff --git a/launcher/modplatform/modpacksch/FTBPackInstallTask.cpp b/launcher/modplatform/modpacksch/FTBPackInstallTask.cpp
index 3c15667c..f17a311a 100644
--- a/launcher/modplatform/modpacksch/FTBPackInstallTask.cpp
+++ b/launcher/modplatform/modpacksch/FTBPackInstallTask.cpp
@@ -335,7 +335,7 @@ void PackInstallTask::install()
 
     components->saveNow();
 
-    instance.setName(m_instName);
+    instance.setName(name());
     instance.setIconKey(m_instIcon);
     instance.setManagedPack("modpacksch", QString::number(m_pack.id), m_pack.name, QString::number(m_version.id), m_version.name);
     instanceSettings->resumeSave();
diff --git a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
index efb8c99b..06ac29c3 100644
--- a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
+++ b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
@@ -8,8 +8,6 @@
 #include "minecraft/MinecraftInstance.h"
 #include "minecraft/PackProfile.h"
 
-#include "modplatform/ModIndex.h"
-
 #include "net/ChecksumValidator.h"
 
 #include "settings/INISettingsObject.h"
@@ -30,11 +28,10 @@ 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?
-    // Based on the way we create the instance name (name + " " + version). Is there a better way?
-    auto inst = instance_list->getInstanceByManagedName(m_instName.section(' ', 0, -2));
+    auto inst = instance_list->getInstanceByManagedName(originalName());
 
     if (!inst) {
-        inst = instance_list->getInstanceById(m_instName);
+        inst = instance_list->getInstanceById(originalName());
 
         if (!inst)
             return false;
@@ -163,8 +160,9 @@ bool ModrinthCreationTask::createInstance()
     } else {
         instance.setIconKey("modrinth");
     }
-    instance.setName(m_instName);
+
     instance.setManagedPack("modrinth", getManagedPackID(), m_managed_name, m_managed_id, {});
+    instance.setName(name());
     instance.saveNow();
 
     m_files_job = new NetJob(tr("Mod download"), APPLICATION->network());
diff --git a/launcher/modplatform/technic/SingleZipPackInstallTask.cpp b/launcher/modplatform/technic/SingleZipPackInstallTask.cpp
index 9093b245..6438d9ef 100644
--- a/launcher/modplatform/technic/SingleZipPackInstallTask.cpp
+++ b/launcher/modplatform/technic/SingleZipPackInstallTask.cpp
@@ -133,7 +133,7 @@ void Technic::SingleZipPackInstallTask::extractFinished()
     shared_qobject_ptr<Technic::TechnicPackProcessor> packProcessor = new Technic::TechnicPackProcessor();
     connect(packProcessor.get(), &Technic::TechnicPackProcessor::succeeded, this, &Technic::SingleZipPackInstallTask::emitSucceeded);
     connect(packProcessor.get(), &Technic::TechnicPackProcessor::failed, this, &Technic::SingleZipPackInstallTask::emitFailed);
-    packProcessor->run(m_globalSettings, m_instName, m_instIcon, m_stagingPath, m_minecraftVersion);
+    packProcessor->run(m_globalSettings, name(), m_instIcon, m_stagingPath, m_minecraftVersion);
 }
 
 void Technic::SingleZipPackInstallTask::extractAborted()
diff --git a/launcher/modplatform/technic/SolderPackInstallTask.cpp b/launcher/modplatform/technic/SolderPackInstallTask.cpp
index 89dbf4ca..65b6ca51 100644
--- a/launcher/modplatform/technic/SolderPackInstallTask.cpp
+++ b/launcher/modplatform/technic/SolderPackInstallTask.cpp
@@ -214,7 +214,7 @@ void Technic::SolderPackInstallTask::extractFinished()
     shared_qobject_ptr<Technic::TechnicPackProcessor> packProcessor = new Technic::TechnicPackProcessor();
     connect(packProcessor.get(), &Technic::TechnicPackProcessor::succeeded, this, &Technic::SolderPackInstallTask::emitSucceeded);
     connect(packProcessor.get(), &Technic::TechnicPackProcessor::failed, this, &Technic::SolderPackInstallTask::emitFailed);
-    packProcessor->run(m_globalSettings, m_instName, m_instIcon, m_stagingPath, m_minecraftVersion, true);
+    packProcessor->run(m_globalSettings, name(), m_instIcon, m_stagingPath, m_minecraftVersion, true);
 }
 
 void Technic::SolderPackInstallTask::extractAborted()
diff --git a/launcher/ui/dialogs/NewInstanceDialog.cpp b/launcher/ui/dialogs/NewInstanceDialog.cpp
index 675f8b15..04fecbde 100644
--- a/launcher/ui/dialogs/NewInstanceDialog.cpp
+++ b/launcher/ui/dialogs/NewInstanceDialog.cpp
@@ -177,13 +177,30 @@ NewInstanceDialog::~NewInstanceDialog()
     delete ui;
 }
 
-void NewInstanceDialog::setSuggestedPack(const QString& name, InstanceTask* task)
+void NewInstanceDialog::setSuggestedPack(QString name, InstanceTask* task)
 {
     creationTask.reset(task);
+
     ui->instNameTextBox->setPlaceholderText(name);
+    importVersion.clear();
 
-    if(!task)
-    {
+    if (!task) {
+        ui->iconButton->setIcon(APPLICATION->icons()->getIcon("default"));
+        importIcon = false;
+    }
+
+    auto allowOK = task && !instName().isEmpty();
+    m_buttons->button(QDialogButtonBox::Ok)->setEnabled(allowOK);
+}
+
+void NewInstanceDialog::setSuggestedPack(QString name, QString version, InstanceTask* task)
+{
+    creationTask.reset(task);
+
+    ui->instNameTextBox->setPlaceholderText(name);
+    importVersion = version;
+
+    if (!task) {
         ui->iconButton->setIcon(APPLICATION->icons()->getIcon("default"));
         importIcon = false;
     }
@@ -214,7 +231,11 @@ InstanceTask * NewInstanceDialog::extractTask()
 {
     InstanceTask * extracted = creationTask.get();
     creationTask.release();
-    extracted->setName(instName());
+
+    InstanceName inst_name(ui->instNameTextBox->placeholderText().trimmed(), importVersion);
+    inst_name.setName(ui->instNameTextBox->text().trimmed());
+    extracted->setName(inst_name);
+
     extracted->setGroup(instGroup());
     extracted->setIcon(iconKey());
     return extracted;
diff --git a/launcher/ui/dialogs/NewInstanceDialog.h b/launcher/ui/dialogs/NewInstanceDialog.h
index a3c8cd1c..185ab9e6 100644
--- a/launcher/ui/dialogs/NewInstanceDialog.h
+++ b/launcher/ui/dialogs/NewInstanceDialog.h
@@ -37,7 +37,6 @@
 
 #include <QDialog>
 
-#include "BaseVersion.h"
 #include "ui/pages/BasePageProvider.h"
 #include "InstanceTask.h"
 
@@ -61,7 +60,8 @@ public:
 
     void updateDialogState();
 
-    void setSuggestedPack(const QString & name = QString(), InstanceTask * task = nullptr);
+    void setSuggestedPack(QString name = QString(), InstanceTask * task = nullptr);
+    void setSuggestedPack(QString name, QString version, InstanceTask * task = nullptr);
     void setSuggestedIconFromFile(const QString &path, const QString &name);
     void setSuggestedIcon(const QString &key);
 
@@ -95,5 +95,7 @@ private:
     QString importIconPath;
     QString importIconName;
 
+    QString importVersion;
+
     void importIconNow();
 };
diff --git a/launcher/ui/pages/modplatform/atlauncher/AtlPage.cpp b/launcher/ui/pages/modplatform/atlauncher/AtlPage.cpp
index 7901b90b..87544445 100644
--- a/launcher/ui/pages/modplatform/atlauncher/AtlPage.cpp
+++ b/launcher/ui/pages/modplatform/atlauncher/AtlPage.cpp
@@ -117,7 +117,7 @@ void AtlPage::suggestCurrent()
     }
 
     auto uiSupport = new AtlUserInteractionSupportImpl(this);
-    dialog->setSuggestedPack(selected.name + " " + selectedVersion, new ATLauncher::PackInstallTask(uiSupport, selected.name, selectedVersion));
+    dialog->setSuggestedPack(selected.name, selectedVersion, new ATLauncher::PackInstallTask(uiSupport, selected.name, selectedVersion));
 
     auto editedLogoName = selected.safeName;
     auto url = QString(BuildConfig.ATL_DOWNLOAD_SERVER_URL + "launcher/images/%1.png").arg(selected.safeName.toLower());
diff --git a/launcher/ui/pages/modplatform/ftb/FtbPage.cpp b/launcher/ui/pages/modplatform/ftb/FtbPage.cpp
index 504d7f7b..8975d74e 100644
--- a/launcher/ui/pages/modplatform/ftb/FtbPage.cpp
+++ b/launcher/ui/pages/modplatform/ftb/FtbPage.cpp
@@ -127,7 +127,7 @@ void FtbPage::suggestCurrent()
         return;
     }
 
-    dialog->setSuggestedPack(selected.name + " " + selectedVersion, new ModpacksCH::PackInstallTask(selected, selectedVersion, this));
+    dialog->setSuggestedPack(selected.name, selectedVersion, new ModpacksCH::PackInstallTask(selected, selectedVersion, this));
     for(auto art : selected.art) {
         if(art.type == "square") {
             QString editedLogoName;
diff --git a/launcher/ui/pages/modplatform/legacy_ftb/Page.cpp b/launcher/ui/pages/modplatform/legacy_ftb/Page.cpp
index 6ffbd312..0208b36b 100644
--- a/launcher/ui/pages/modplatform/legacy_ftb/Page.cpp
+++ b/launcher/ui/pages/modplatform/legacy_ftb/Page.cpp
@@ -176,7 +176,7 @@ void Page::suggestCurrent()
         return;
     }
 
-    dialog->setSuggestedPack(selected.name + " " + selectedVersion, new PackInstallTask(APPLICATION->network(), selected, selectedVersion));
+    dialog->setSuggestedPack(selected.name, selectedVersion, new PackInstallTask(APPLICATION->network(), selected, selectedVersion));
     QString editedLogoName;
     if(selected.logo.toLower().startsWith("ftb"))
     {
diff --git a/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp b/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp
index df29c0c3..16fec82c 100644
--- a/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp
+++ b/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp
@@ -294,7 +294,7 @@ void ModrinthPage::suggestCurrent()
 
     for (auto& ver : current.versions) {
         if (ver.id == selectedVersion) {
-            dialog->setSuggestedPack(current.name + " " + ver.version, new InstanceImportTask(ver.download_url, this));
+            dialog->setSuggestedPack(current.name, ver.version, new InstanceImportTask(ver.download_url, this));
             auto iconName = current.iconName;
             m_model->getLogo(iconName, current.iconUrl.toString(),
                              [this, iconName](QString logo) { dialog->setSuggestedIconFromFile(logo, iconName); });
diff --git a/launcher/ui/pages/modplatform/technic/TechnicPage.cpp b/launcher/ui/pages/modplatform/technic/TechnicPage.cpp
index b8c1e00a..b15af244 100644
--- a/launcher/ui/pages/modplatform/technic/TechnicPage.cpp
+++ b/launcher/ui/pages/modplatform/technic/TechnicPage.cpp
@@ -271,11 +271,11 @@ void TechnicPage::selectVersion() {
 
     if (!current.isSolder)
     {
-        dialog->setSuggestedPack(current.name + " " + selectedVersion, new Technic::SingleZipPackInstallTask(current.url, current.minecraftVersion));
+        dialog->setSuggestedPack(current.name, selectedVersion, new Technic::SingleZipPackInstallTask(current.url, current.minecraftVersion));
     }
     else
     {
-        dialog->setSuggestedPack(current.name + " " + selectedVersion, new Technic::SolderPackInstallTask(APPLICATION->network(), current.url, current.slug, selectedVersion, current.minecraftVersion));
+        dialog->setSuggestedPack(current.name, selectedVersion, new Technic::SolderPackInstallTask(APPLICATION->network(), current.url, current.slug, selectedVersion, current.minecraftVersion));
     }
 }
 
-- 
cgit 


From 3a9d58e31c70159f574b5cc24a104f81c9d981ed Mon Sep 17 00:00:00 2001
From: flow <flowlnlnln@gmail.com>
Date: Thu, 28 Jul 2022 22:35:40 -0300
Subject: feat: add override handling in modrinth pack update

Signed-off-by: flow <flowlnlnln@gmail.com>
---
 .../modrinth/ModrinthInstanceCreationTask.cpp      | 42 ++++++++++++++++++----
 1 file changed, 35 insertions(+), 7 deletions(-)

(limited to 'launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp')

diff --git a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
index 06ac29c3..212b3447 100644
--- a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
+++ b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
@@ -8,6 +8,8 @@
 #include "minecraft/MinecraftInstance.h"
 #include "minecraft/PackProfile.h"
 
+#include "modplatform/helpers/OverrideUtils.h"
+
 #include "net/ChecksumValidator.h"
 
 #include "settings/INISettingsObject.h"
@@ -58,7 +60,9 @@ bool ModrinthCreationTask::updateInstance()
     // Remove repeated files, we don't need to download them!
     QDir old_inst_dir(inst->instanceRoot());
 
-    QString old_index_path(FS::PathCombine(old_inst_dir.absolutePath(), "mrpack", "modrinth.index.json"));
+    QString old_index_folder(FS::PathCombine(old_inst_dir.absolutePath(), "mrpack"));
+
+    QString old_index_path(FS::PathCombine(old_index_folder, "modrinth.index.json"));
     QFileInfo old_index_file(old_index_path);
     if (old_index_file.exists()) {
         std::vector<Modrinth::File> old_files;
@@ -66,7 +70,7 @@ bool ModrinthCreationTask::updateInstance()
 
         // Let's remove all duplicated, identical resources!
         auto files_iterator = m_files.begin();
-begin:
+    begin:
         while (files_iterator != m_files.end()) {
             auto const& file = *files_iterator;
 
@@ -78,7 +82,7 @@ begin:
                     qDebug() << "Removed file at" << file.path << "from list of downloads";
                     files_iterator = m_files.erase(files_iterator);
                     old_files_iterator = old_files.erase(old_files_iterator);
-                    goto begin; // Sorry :c
+                    goto begin;  // Sorry :c
                 }
 
                 old_files_iterator++;
@@ -87,18 +91,32 @@ begin:
             files_iterator++;
         }
 
+        QDir old_minecraft_dir(inst->gameRoot());
         // Some files were removed from the old version, and some will be downloaded in an updated version,
         // so we're fine removing them!
         if (!old_files.empty()) {
-            QDir old_minecraft_dir(inst->gameRoot());
             for (auto const& file : old_files) {
-                qWarning() << "Removing" << file.path;
+                qDebug() << "Removing" << file.path;
                 old_minecraft_dir.remove(file.path);
             }
         }
+
+        // We will remove all the previous overrides, to prevent duplicate files!
+        // TODO: Currently 'overrides' will always override the stuff on update. How do we preserve unchanged overrides?
+        // FIXME: We may want to do something about disabled mods.
+        auto old_overrides = Override::readOverrides("overrides", old_index_folder);
+        for (auto entry : old_overrides) {
+            qDebug() << "Removing" << entry;
+            old_minecraft_dir.remove(entry);
+        }
+
+        auto old_client_overrides = Override::readOverrides("client-overrides", old_index_folder);
+        for (auto entry : old_overrides) {
+            qDebug() << "Removing" << entry;
+            old_minecraft_dir.remove(entry);
+        }
     }
 
-    // TODO: Currently 'overrides' will always override the stuff on update. How do we preserve unchanged overrides?
 
     setOverride(true);
     qDebug() << "Will override instance!";
@@ -112,12 +130,14 @@ bool ModrinthCreationTask::createInstance()
 {
     QEventLoop loop;
 
+    QString parent_folder(FS::PathCombine(m_stagingPath, "mrpack"));
+
     QString index_path = FS::PathCombine(m_stagingPath, "modrinth.index.json");
     if (m_files.empty() && !parseManifest(index_path, m_files))
         return false;
 
     // Keep index file in case we need it some other time (like when changing versions)
-    QString new_index_place(FS::PathCombine(m_stagingPath, "mrpack", "modrinth.index.json"));
+    QString new_index_place(FS::PathCombine(parent_folder, "modrinth.index.json"));
     FS::ensureFilePathExists(new_index_place);
     QFile::rename(index_path, new_index_place);
 
@@ -125,6 +145,10 @@ bool ModrinthCreationTask::createInstance()
 
     auto override_path = FS::PathCombine(m_stagingPath, "overrides");
     if (QFile::exists(override_path)) {
+        // Create a list of overrides in "overrides.txt" inside mrpack/
+        Override::createOverrides("overrides", parent_folder, override_path);
+
+        // Apply the overrides
         if (!QFile::rename(override_path, mcPath)) {
             setError(tr("Could not rename the overrides folder:\n") + "overrides");
             return false;
@@ -134,6 +158,10 @@ bool ModrinthCreationTask::createInstance()
     // Do client overrides
     auto client_override_path = FS::PathCombine(m_stagingPath, "client-overrides");
     if (QFile::exists(client_override_path)) {
+        // Create a list of overrides in "client-overrides.txt" inside mrpack/
+        Override::createOverrides("client-overrides", parent_folder, client_override_path);
+
+        // Apply the overrides
         if (!FS::overrideFolder(mcPath, client_override_path)) {
             setError(tr("Could not rename the client overrides folder:\n") + "client overrides");
             return false;
-- 
cgit 


From be769d07f1498a212599234a3ad14c6b9431d206 Mon Sep 17 00:00:00 2001
From: flow <flowlnlnln@gmail.com>
Date: Sun, 31 Jul 2022 19:50:09 -0300
Subject: fix: correctly set all managed pack fields in Modrinth pack

Signed-off-by: flow <flowlnlnln@gmail.com>
---
 .../modrinth/ModrinthInstanceCreationTask.cpp      | 28 +++++++++++++++-------
 .../modrinth/ModrinthInstanceCreationTask.h        | 10 ++++++--
 2 files changed, 27 insertions(+), 11 deletions(-)

(limited to 'launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp')

diff --git a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
index 212b3447..b5140f34 100644
--- a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
+++ b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
@@ -5,7 +5,6 @@
 #include "InstanceList.h"
 #include "Json.h"
 
-#include "minecraft/MinecraftInstance.h"
 #include "minecraft/PackProfile.h"
 
 #include "modplatform/helpers/OverrideUtils.h"
@@ -43,8 +42,8 @@ bool ModrinthCreationTask::updateInstance()
     if (!parseManifest(index_path, m_files))
         return false;
 
-    auto version_id = inst->getManagedPackVersionID();
-    auto version_str = !version_id.isEmpty() ? tr(" (version %1)").arg(version_id) : "";
+    auto version_name = inst->getManagedPackVersionName();
+    auto version_str = !version_name.isEmpty() ? tr(" (version %1)").arg(version_name) : "";
 
     auto info = CustomMessageBox::selectable(m_parent, tr("Similar modpack was found!"),
                                              tr("One or more of your instances are from this same modpack%1. Do you want to create a "
@@ -66,7 +65,7 @@ bool ModrinthCreationTask::updateInstance()
     QFileInfo old_index_file(old_index_path);
     if (old_index_file.exists()) {
         std::vector<Modrinth::File> old_files;
-        parseManifest(old_index_path, old_files);
+        parseManifest(old_index_path, old_files, false);
 
         // Let's remove all duplicated, identical resources!
         auto files_iterator = m_files.begin();
@@ -121,6 +120,8 @@ bool ModrinthCreationTask::updateInstance()
     setOverride(true);
     qDebug() << "Will override instance!";
 
+    m_instance = inst;
+
     // We let it go through the createInstance() stage, just with a couple modifications for updating
     return false;
 }
@@ -189,7 +190,7 @@ bool ModrinthCreationTask::createInstance()
         instance.setIconKey("modrinth");
     }
 
-    instance.setManagedPack("modrinth", getManagedPackID(), m_managed_name, m_managed_id, {});
+    instance.setManagedPack("modrinth", getManagedPackID(), m_managed_name, m_managed_version_id, version());
     instance.setName(name());
     instance.saveNow();
 
@@ -229,10 +230,17 @@ bool ModrinthCreationTask::createInstance()
 
     loop.exec();
 
+    if (m_instance) {
+        auto inst = m_instance.value();
+
+        inst->copyManagedPack(instance);
+        inst->setName(instance.name());
+    }
+
     return ended_well;
 }
 
-bool ModrinthCreationTask::parseManifest(QString index_path, std::vector<Modrinth::File>& files)
+bool ModrinthCreationTask::parseManifest(QString index_path, std::vector<Modrinth::File>& files, bool set_managed_info)
 {
     try {
         auto doc = Json::requireDocument(index_path);
@@ -244,8 +252,10 @@ bool ModrinthCreationTask::parseManifest(QString index_path, std::vector<Modrint
                 throw JSONValidationError("Unknown game: " + game);
             }
 
-            m_managed_version_id = Json::ensureString(obj, "versionId", "Managed ID");
-            m_managed_name = Json::ensureString(obj, "name", "Managed Name");
+            if (set_managed_info) {
+                m_managed_version_id = Json::ensureString(obj, "versionId", {}, "Managed ID");
+                m_managed_name = Json::ensureString(obj, "name", {}, "Managed Name");
+            }
 
             auto jsonFiles = Json::requireIsArrayOf<QJsonObject>(obj, "files", "modrinth.index.json");
             bool had_optional = false;
@@ -348,7 +358,7 @@ QString ModrinthCreationTask::getManagedPackID() const
 {
     if (!m_source_url.isEmpty()) {
         QRegularExpression regex(R"(data\/(.*)\/versions)");
-        return regex.match(m_source_url).captured(0);
+        return regex.match(m_source_url).captured(1);
     }
 
     return {};
diff --git a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.h b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.h
index 4e804e58..bcf80682 100644
--- a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.h
+++ b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.h
@@ -2,6 +2,10 @@
 
 #include "InstanceCreationTask.h"
 
+#include <optional>
+
+#include "minecraft/MinecraftInstance.h"
+
 #include "modplatform/modrinth/ModrinthPackManifest.h"
 
 #include "net/NetJob.h"
@@ -11,7 +15,7 @@ class ModrinthCreationTask final : public InstanceCreationTask {
 
    public:
     ModrinthCreationTask(QString staging_path, SettingsObjectPtr global_settings, QWidget* parent, QString source_url = {})
-        : InstanceCreationTask(), m_parent(parent)
+        : InstanceCreationTask(), m_parent(parent), m_source_url(std::move(source_url))
     {
         setStagingPath(staging_path);
         setParentSettings(global_settings);
@@ -24,7 +28,7 @@ class ModrinthCreationTask final : public InstanceCreationTask {
     bool createInstance() override;
 
    private:
-    bool parseManifest(QString, std::vector<Modrinth::File>&);
+    bool parseManifest(QString, std::vector<Modrinth::File>&, bool set_managed_info = true);
     QString getManagedPackID() const;
 
    private:
@@ -36,4 +40,6 @@ class ModrinthCreationTask final : public InstanceCreationTask {
 
     std::vector<Modrinth::File> m_files;
     NetJob::Ptr m_files_job;
+
+    std::optional<InstancePtr> m_instance;
 };
-- 
cgit 


From 87002fb8f84b104a221de07ba99ba0eb5cc6bbb6 Mon Sep 17 00:00:00 2001
From: flow <flowlnlnln@gmail.com>
Date: Sun, 31 Jul 2022 18:21:59 -0300
Subject: fix: hook up setAbortStatus in instance import tasks

Signed-off-by: flow <flowlnlnln@gmail.com>
---
 launcher/InstanceCreationTask.cpp                         | 15 +++++++++++++--
 launcher/InstanceImportTask.cpp                           |  9 ++++++++-
 launcher/InstanceImportTask.h                             |  1 -
 launcher/InstanceList.cpp                                 |  1 +
 launcher/modplatform/flame/FlameInstanceCreationTask.cpp  |  5 ++++-
 .../modplatform/modrinth/ModrinthInstanceCreationTask.cpp |  5 ++++-
 .../modplatform/modrinth/ModrinthInstanceCreationTask.h   |  1 -
 7 files changed, 30 insertions(+), 7 deletions(-)

(limited to 'launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp')

diff --git a/launcher/InstanceCreationTask.cpp b/launcher/InstanceCreationTask.cpp
index c8c91997..d7663c2d 100644
--- a/launcher/InstanceCreationTask.cpp
+++ b/launcher/InstanceCreationTask.cpp
@@ -2,11 +2,22 @@
 
 #include <QDebug>
 
-InstanceCreationTask::InstanceCreationTask() {}
+InstanceCreationTask::InstanceCreationTask() = default;
 
 void InstanceCreationTask::executeTask()
 {
-    if (updateInstance() || createInstance()) {
+    if (updateInstance()) {
+        emitSucceeded();
+        return;
+    }
+
+    // If this is set, it means we're updating an instance. Since the previous step likely
+    // removed some old files, we'd better not let the user abort the next task, since it'd
+    // put the instance in an invalid state.
+    // TODO: Figure out an unexpensive way of making such file removal a recoverable transaction.
+    setAbortStatus(!shouldOverride());
+
+    if (createInstance()) {
         emitSucceeded();
         return;
     }
diff --git a/launcher/InstanceImportTask.cpp b/launcher/InstanceImportTask.cpp
index 56aabb2d..4819a6ff 100644
--- a/launcher/InstanceImportTask.cpp
+++ b/launcher/InstanceImportTask.cpp
@@ -63,15 +63,20 @@ InstanceImportTask::InstanceImportTask(const QUrl sourceUrl, QWidget* parent)
 
 bool InstanceImportTask::abort()
 {
+    if (!canAbort())
+        return false;
+
     if (m_filesNetJob)
         m_filesNetJob->abort();
     m_extractFuture.cancel();
 
-    return false;
+    return Task::abort();
 }
 
 void InstanceImportTask::executeTask()
 {
+    setAbortStatus(true);
+
     if (m_sourceUrl.isLocalFile()) {
         m_archivePath = m_sourceUrl.toLocalFile();
         processZipPack();
@@ -274,6 +279,7 @@ void InstanceImportTask::processFlame()
     connect(inst_creation_task, &Task::finished, inst_creation_task, &InstanceCreationTask::deleteLater);
 
     connect(this, &Task::aborted, inst_creation_task, &InstanceCreationTask::abort);
+    connect(inst_creation_task, &Task::abortStatusChanged, this, &Task::setAbortStatus);
 
     inst_creation_task->start();
 }
@@ -336,6 +342,7 @@ void InstanceImportTask::processModrinth()
     connect(inst_creation_task, &Task::finished, inst_creation_task, &InstanceCreationTask::deleteLater);
 
     connect(this, &Task::aborted, inst_creation_task, &InstanceCreationTask::abort);
+    connect(inst_creation_task, &Task::abortStatusChanged, this, &Task::setAbortStatus);
 
     inst_creation_task->start();
 }
diff --git a/launcher/InstanceImportTask.h b/launcher/InstanceImportTask.h
index acb1f9d6..ef70c819 100644
--- a/launcher/InstanceImportTask.h
+++ b/launcher/InstanceImportTask.h
@@ -58,7 +58,6 @@ class InstanceImportTask : public InstanceTask
 public:
     explicit InstanceImportTask(const QUrl sourceUrl, QWidget* parent = nullptr);
 
-    bool canAbort() const override { return true; }
     bool abort() override;
     const QVector<Flame::File> &getBlockedFiles() const
     {
diff --git a/launcher/InstanceList.cpp b/launcher/InstanceList.cpp
index 5f604f9a..bf25d2d0 100644
--- a/launcher/InstanceList.cpp
+++ b/launcher/InstanceList.cpp
@@ -785,6 +785,7 @@ class InstanceStaging : public Task {
         connect(child, &Task::succeeded, this, &InstanceStaging::childSucceded);
         connect(child, &Task::failed, this, &InstanceStaging::childFailed);
         connect(child, &Task::aborted, this, &InstanceStaging::childAborted);
+        connect(child, &Task::abortStatusChanged, this, &InstanceStaging::setAbortStatus);
         connect(child, &Task::status, this, &InstanceStaging::setStatus);
         connect(child, &Task::progress, this, &InstanceStaging::setProgress);
         connect(&m_backoffTimer, &QTimer::timeout, this, &InstanceStaging::childSucceded);
diff --git a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp
index 76ac11af..c8b2e297 100644
--- a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp
+++ b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp
@@ -27,6 +27,9 @@ static const FlameAPI api;
 
 bool FlameCreationTask::abort()
 {
+    if (!canAbort())
+        return false;
+
     if (m_process_update_file_info_job)
         m_process_update_file_info_job->abort();
     if (m_files_job)
@@ -34,7 +37,7 @@ bool FlameCreationTask::abort()
     if (m_mod_id_resolver)
         m_mod_id_resolver->abort();
 
-    return true;
+    return Task::abort();
 }
 
 bool FlameCreationTask::updateInstance()
diff --git a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
index b5140f34..3234d92b 100644
--- a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
+++ b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
@@ -19,9 +19,12 @@
 
 bool ModrinthCreationTask::abort()
 {
+    if (!canAbort())
+        return false;
+
     if (m_files_job)
         return m_files_job->abort();
-    return true;
+    return Task::abort();
 }
 
 bool ModrinthCreationTask::updateInstance()
diff --git a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.h b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.h
index bcf80682..e87e4fb9 100644
--- a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.h
+++ b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.h
@@ -22,7 +22,6 @@ class ModrinthCreationTask final : public InstanceCreationTask {
     }
 
     bool abort() override;
-    bool canAbort() const override { return true; }
 
     bool updateInstance() override;
     bool createInstance() override;
-- 
cgit 


From eda6cf11ef53c11ed2691399ec1adbc83ca0a4d6 Mon Sep 17 00:00:00 2001
From: flow <flowlnlnln@gmail.com>
Date: Sun, 31 Jul 2022 20:29:12 -0300
Subject: feat(ui): improve info dialog before updating an instance

Adds a 'Cancel' option, and add a note about doing a backup before
updating.

Signed-off-by: flow <flowlnlnln@gmail.com>
---
 launcher/InstanceCreationTask.cpp                  |  6 ++++
 launcher/InstanceCreationTask.h                    |  3 ++
 launcher/InstanceImportTask.cpp                    |  2 ++
 .../flame/FlameInstanceCreationTask.cpp            | 21 +++++++++----
 .../modrinth/ModrinthInstanceCreationTask.cpp      | 36 ++++++++++++++--------
 .../modrinth/ModrinthInstanceCreationTask.h        |  2 +-
 6 files changed, 50 insertions(+), 20 deletions(-)

(limited to 'launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp')

diff --git a/launcher/InstanceCreationTask.cpp b/launcher/InstanceCreationTask.cpp
index d7663c2d..3908bb9a 100644
--- a/launcher/InstanceCreationTask.cpp
+++ b/launcher/InstanceCreationTask.cpp
@@ -11,6 +11,12 @@ void InstanceCreationTask::executeTask()
         return;
     }
 
+    // When the user aborted in the update stage.
+    if (m_abort) {
+        emitAborted();
+        return;
+    }
+
     // If this is set, it means we're updating an instance. Since the previous step likely
     // removed some old files, we'd better not let the user abort the next task, since it'd
     // put the instance in an invalid state.
diff --git a/launcher/InstanceCreationTask.h b/launcher/InstanceCreationTask.h
index 68c5de59..9b51c448 100644
--- a/launcher/InstanceCreationTask.h
+++ b/launcher/InstanceCreationTask.h
@@ -36,6 +36,9 @@ class InstanceCreationTask : public InstanceTask {
    protected:
     void setError(QString message) { m_error_message = message; };
 
+   protected:
+    bool m_abort = false;
+
    private:
     QString m_error_message;
 };
diff --git a/launcher/InstanceImportTask.cpp b/launcher/InstanceImportTask.cpp
index 4819a6ff..e35913da 100644
--- a/launcher/InstanceImportTask.cpp
+++ b/launcher/InstanceImportTask.cpp
@@ -279,6 +279,7 @@ void InstanceImportTask::processFlame()
     connect(inst_creation_task, &Task::finished, inst_creation_task, &InstanceCreationTask::deleteLater);
 
     connect(this, &Task::aborted, inst_creation_task, &InstanceCreationTask::abort);
+    connect(inst_creation_task, &Task::aborted, this, &Task::abort);
     connect(inst_creation_task, &Task::abortStatusChanged, this, &Task::setAbortStatus);
 
     inst_creation_task->start();
@@ -342,6 +343,7 @@ void InstanceImportTask::processModrinth()
     connect(inst_creation_task, &Task::finished, inst_creation_task, &InstanceCreationTask::deleteLater);
 
     connect(this, &Task::aborted, inst_creation_task, &InstanceCreationTask::abort);
+    connect(inst_creation_task, &Task::aborted, this, &Task::abort);
     connect(inst_creation_task, &Task::abortStatusChanged, this, &Task::setAbortStatus);
 
     inst_creation_task->start();
diff --git a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp
index c8b2e297..22ee0998 100644
--- a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp
+++ b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp
@@ -66,16 +66,25 @@ bool FlameCreationTask::updateInstance()
     auto version_id = inst->getManagedPackVersionName();
     auto version_str = !version_id.isEmpty() ? tr(" (version %1)").arg(version_id) : "";
 
-    auto info = CustomMessageBox::selectable(m_parent, tr("Similar modpack was found!"),
-                                             tr("One or more of your instances are from this same modpack%1. Do you want to create a "
-                                                "separate instance, or update the existing one?")
-                                                 .arg(version_str),
-                                             QMessageBox::Information, QMessageBox::Ok | QMessageBox::Abort);
+    auto info = CustomMessageBox::selectable(
+        m_parent, tr("Similar modpack was found!"),
+        tr("One or more of your instances are from this same modpack%1. Do you want to create a "
+           "separate instance, or update the existing one?\n\nNOTE: Make sure you made a backup of your important instance data before "
+           "updating, as worlds can be corrupted and some configuration may be lost (due to pack overrides).")
+            .arg(version_str), QMessageBox::Information, QMessageBox::Ok | QMessageBox::Reset | QMessageBox::Abort);
     info->setButtonText(QMessageBox::Ok, tr("Update existing instance"));
     info->setButtonText(QMessageBox::Abort, tr("Create new instance"));
+    info->setButtonText(QMessageBox::Reset, tr("Cancel"));
 
-    if (info->exec() && info->clickedButton() == info->button(QMessageBox::Abort))
+    info->exec();
+
+    if (info->clickedButton() == info->button(QMessageBox::Abort))
+        return false;
+
+    if (info->clickedButton() == info->button(QMessageBox::Reset)) {
+        m_abort = true;
         return false;
+    }
 
     QDir old_inst_dir(inst->instanceRoot());
 
diff --git a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
index 3234d92b..449d7387 100644
--- a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
+++ b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
@@ -42,22 +42,32 @@ bool ModrinthCreationTask::updateInstance()
     }
 
     QString index_path = FS::PathCombine(m_stagingPath, "modrinth.index.json");
-    if (!parseManifest(index_path, m_files))
+    if (!parseManifest(index_path, m_files, true, false))
         return false;
 
     auto version_name = inst->getManagedPackVersionName();
     auto version_str = !version_name.isEmpty() ? tr(" (version %1)").arg(version_name) : "";
 
-    auto info = CustomMessageBox::selectable(m_parent, tr("Similar modpack was found!"),
-                                             tr("One or more of your instances are from this same modpack%1. Do you want to create a "
-                                                "separate instance, or update the existing one?")
-                                                 .arg(version_str),
-                                             QMessageBox::Information, QMessageBox::Ok | QMessageBox::Abort);
-    info->setButtonText(QMessageBox::Ok, tr("Update existing instance"));
-    info->setButtonText(QMessageBox::Abort, tr("Create new instance"));
+    auto info = CustomMessageBox::selectable(
+        m_parent, tr("Similar modpack was found!"),
+        tr("One or more of your instances are from this same modpack%1. Do you want to create a "
+           "separate instance, or update the existing one?\n\nNOTE: Make sure you made a backup of your important instance data before "
+           "updating, as worlds can be corrupted and some configuration may be lost (due to pack overrides).")
+            .arg(version_str),
+        QMessageBox::Information, QMessageBox::Ok | QMessageBox::Reset | QMessageBox::Abort);
+    info->setButtonText(QMessageBox::Ok, tr("Create new instance"));
+    info->setButtonText(QMessageBox::Abort, tr("Update existing instance"));
+    info->setButtonText(QMessageBox::Reset, tr("Cancel"));
 
-    if (info->exec() && info->clickedButton() == info->button(QMessageBox::Abort))
+    info->exec();
+
+    if (info->clickedButton() == info->button(QMessageBox::Ok))
+        return false;
+
+    if (info->clickedButton() == info->button(QMessageBox::Reset)) {
+        m_abort = true;
         return false;
+    }
 
     // Remove repeated files, we don't need to download them!
     QDir old_inst_dir(inst->instanceRoot());
@@ -68,7 +78,7 @@ bool ModrinthCreationTask::updateInstance()
     QFileInfo old_index_file(old_index_path);
     if (old_index_file.exists()) {
         std::vector<Modrinth::File> old_files;
-        parseManifest(old_index_path, old_files, false);
+        parseManifest(old_index_path, old_files, false, false);
 
         // Let's remove all duplicated, identical resources!
         auto files_iterator = m_files.begin();
@@ -137,7 +147,7 @@ bool ModrinthCreationTask::createInstance()
     QString parent_folder(FS::PathCombine(m_stagingPath, "mrpack"));
 
     QString index_path = FS::PathCombine(m_stagingPath, "modrinth.index.json");
-    if (m_files.empty() && !parseManifest(index_path, m_files))
+    if (m_files.empty() && !parseManifest(index_path, m_files, true, true))
         return false;
 
     // Keep index file in case we need it some other time (like when changing versions)
@@ -243,7 +253,7 @@ bool ModrinthCreationTask::createInstance()
     return ended_well;
 }
 
-bool ModrinthCreationTask::parseManifest(QString index_path, std::vector<Modrinth::File>& files, bool set_managed_info)
+bool ModrinthCreationTask::parseManifest(QString index_path, std::vector<Modrinth::File>& files, bool set_managed_info, bool show_optional_dialog)
 {
     try {
         auto doc = Json::requireDocument(index_path);
@@ -274,7 +284,7 @@ bool ModrinthCreationTask::parseManifest(QString index_path, std::vector<Modrint
                         continue;
                     } else if (support == "optional") {
                         // TODO: Make a review dialog for choosing which ones the user wants!
-                        if (!had_optional) {
+                        if (!had_optional && show_optional_dialog) {
                             had_optional = true;
                             auto info = CustomMessageBox::selectable(
                                 m_parent, tr("Optional mod detected!"),
diff --git a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.h b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.h
index e87e4fb9..01d0d1bf 100644
--- a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.h
+++ b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.h
@@ -27,7 +27,7 @@ class ModrinthCreationTask final : public InstanceCreationTask {
     bool createInstance() override;
 
    private:
-    bool parseManifest(QString, std::vector<Modrinth::File>&, bool set_managed_info = true);
+    bool parseManifest(QString, std::vector<Modrinth::File>&, bool set_managed_info = true, bool show_optional_dialog = true);
     QString getManagedPackID() const;
 
    private:
-- 
cgit 


From 2dd372600c5e744067fda51f8ffae16fb1caa16c Mon Sep 17 00:00:00 2001
From: flow <flowlnlnln@gmail.com>
Date: Fri, 5 Aug 2022 21:25:21 -0300
Subject: fix: some abort-related issues

Signed-off-by: flow <flowlnlnln@gmail.com>
---
 launcher/InstanceCreationTask.cpp                              | 2 ++
 launcher/InstanceList.cpp                                      | 8 +++++---
 launcher/modplatform/flame/FlameInstanceCreationTask.cpp       | 1 +
 launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp | 3 ++-
 4 files changed, 10 insertions(+), 4 deletions(-)

(limited to 'launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp')

diff --git a/launcher/InstanceCreationTask.cpp b/launcher/InstanceCreationTask.cpp
index 3908bb9a..a82bdfe8 100644
--- a/launcher/InstanceCreationTask.cpp
+++ b/launcher/InstanceCreationTask.cpp
@@ -6,6 +6,8 @@ InstanceCreationTask::InstanceCreationTask() = default;
 
 void InstanceCreationTask::executeTask()
 {
+    setAbortStatus(true);
+
     if (updateInstance()) {
         emitSucceeded();
         return;
diff --git a/launcher/InstanceList.cpp b/launcher/InstanceList.cpp
index bf25d2d0..a4b8d8aa 100644
--- a/launcher/InstanceList.cpp
+++ b/launcher/InstanceList.cpp
@@ -796,10 +796,12 @@ class InstanceStaging : public Task {
     // FIXME/TODO: add ability to abort during instance commit retries
     bool abort() override
     {
-        if (m_child && m_child->canAbort())
-            return m_child->abort();
+        if (!canAbort())
+            return false;
 
-        return false;
+        m_child->abort();
+
+        return Task::abort();
     }
     bool canAbort() const override
     {
diff --git a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp
index 22ee0998..5acb6300 100644
--- a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp
+++ b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp
@@ -30,6 +30,7 @@ bool FlameCreationTask::abort()
     if (!canAbort())
         return false;
 
+    m_abort = true;
     if (m_process_update_file_info_job)
         m_process_update_file_info_job->abort();
     if (m_files_job)
diff --git a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
index 449d7387..a0c67876 100644
--- a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
+++ b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
@@ -22,8 +22,9 @@ bool ModrinthCreationTask::abort()
     if (!canAbort())
         return false;
 
+    m_abort = true;
     if (m_files_job)
-        return m_files_job->abort();
+        m_files_job->abort();
     return Task::abort();
 }
 
-- 
cgit 


From d2fdbec41dd664fb6cfc66ec6c79ee9f78eb8c7b Mon Sep 17 00:00:00 2001
From: flow <flowlnlnln@gmail.com>
Date: Fri, 5 Aug 2022 21:26:02 -0300
Subject: fix: move file deletion to the end of the instance update

This makes it harder for problems in the updating process to affect the
current instance. Network issues, for instance, will no longer put the
instance in an invalid state.

Still, a possible improvement to this would be passing that logic to
InstanceStaging instead, to be handled with the instance commiting
directly. However, as it is now, the code would become very spaguetti-y,
and given that the override operation in the commiting could also put
the instance into an invalid state, it seems to me that, in order to
fully error-proof this, we would need to do a copy operation on the
whole instance, in order to modify the copy, and only in the end
override everything an once with a rename. That also has the possibility
of corrupting the instance if done without super care, however, so I
think we may need to instead create an automatic backup system, with an
undo command of sorts, or something like that. This doesn't seem very
trivial though, so it'll probably need to wait until another PR. In the
meantime, the user is advised to always backup their instances before
doing this kind of action, as always.

What a long commit message o.O

Signed-off-by: flow <flowlnlnln@gmail.com>
---
 launcher/InstanceCreationTask.cpp                  | 41 ++++++++++++++++------
 launcher/InstanceCreationTask.h                    |  2 ++
 .../flame/FlameInstanceCreationTask.cpp            | 25 +++++++------
 .../modrinth/ModrinthInstanceCreationTask.cpp      | 22 ++++++++----
 4 files changed, 61 insertions(+), 29 deletions(-)

(limited to 'launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp')

diff --git a/launcher/InstanceCreationTask.cpp b/launcher/InstanceCreationTask.cpp
index a82bdfe8..1b1a8c15 100644
--- a/launcher/InstanceCreationTask.cpp
+++ b/launcher/InstanceCreationTask.cpp
@@ -1,6 +1,7 @@
 #include "InstanceCreationTask.h"
 
 #include <QDebug>
+#include <QFile>
 
 InstanceCreationTask::InstanceCreationTask() = default;
 
@@ -19,19 +20,37 @@ void InstanceCreationTask::executeTask()
         return;
     }
 
-    // If this is set, it means we're updating an instance. Since the previous step likely
-    // removed some old files, we'd better not let the user abort the next task, since it'd
-    // put the instance in an invalid state.
-    // TODO: Figure out an unexpensive way of making such file removal a recoverable transaction.
-    setAbortStatus(!shouldOverride());
+    if (!createInstance()) {
+        if (m_abort)
+            return;
 
-    if (createInstance()) {
-        emitSucceeded();
+        qWarning() << "Instance creation failed!";
+        if (!m_error_message.isEmpty())
+            qWarning() << "Reason: " << m_error_message;
+        emitFailed(tr("Error while creating new instance."));
         return;
     }
 
-    qWarning() << "Instance creation failed!";
-    if (!m_error_message.isEmpty())
-        qWarning() << "Reason: " << m_error_message;
-    emitFailed(tr("Error while creating new instance."));
+    // If this is set, it means we're updating an instance. So, we now need to remove the
+    // files scheduled to, and we'd better not let the user abort in the middle of it, since it'd
+    // put the instance in an invalid state.
+    if (shouldOverride()) {
+        setAbortStatus(false);
+        setStatus(tr("Removing old conflicting files..."));
+        qDebug() << "Removing old files";
+
+        for (auto path : m_files_to_remove) {
+            if (!QFile::exists(path))
+                continue;
+            qDebug() << "Removing" << path;
+            if (!QFile::remove(path)) {
+                qCritical() << "Couldn't remove the old conflicting files.";
+                emitFailed(tr("Failed to remove old conflicting files."));
+                return;
+            }
+        }
+    }
+
+    emitSucceeded();
+    return;
 }
diff --git a/launcher/InstanceCreationTask.h b/launcher/InstanceCreationTask.h
index 9b51c448..03ee1a7a 100644
--- a/launcher/InstanceCreationTask.h
+++ b/launcher/InstanceCreationTask.h
@@ -39,6 +39,8 @@ class InstanceCreationTask : public InstanceTask {
    protected:
     bool m_abort = false;
 
+    QStringList m_files_to_remove;
+
    private:
     QString m_error_message;
 };
diff --git a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp
index 5acb6300..e3521a38 100644
--- a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp
+++ b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp
@@ -120,15 +120,17 @@ bool FlameCreationTask::updateInstance()
             files_iterator++;
         }
 
-        QString old_minecraft_dir(inst->gameRoot());
+        QDir old_minecraft_dir(inst->gameRoot());
 
         // We will remove all the previous overrides, to prevent duplicate files!
         // TODO: Currently 'overrides' will always override the stuff on update. How do we preserve unchanged overrides?
         // FIXME: We may want to do something about disabled mods.
         auto old_overrides = Override::readOverrides("overrides", old_index_folder);
         for (auto entry : old_overrides) {
-            qDebug() << "Removing" << entry;
-            old_minecraft_dir.remove(entry);
+            if (entry.isEmpty())
+                continue;
+            qDebug() << "Scheduling" << entry << "for removal";
+            m_files_to_remove.append(old_minecraft_dir.absoluteFilePath(entry));
         }
 
         // Remove remaining old files (we need to do an API request to know which ids are which files...)
@@ -143,7 +145,7 @@ bool FlameCreationTask::updateInstance()
 
         QEventLoop loop;
 
-        connect(job, &NetJob::succeeded, this, [raw_response, fileIds, old_inst_dir, &old_files, old_minecraft_dir] {
+        connect(job, &NetJob::succeeded, this, [this, raw_response, fileIds, old_inst_dir, &old_files, old_minecraft_dir] {
             // Parse the API response
             QJsonParseError parse_error{};
             auto doc = QJsonDocument::fromJson(*raw_response, &parse_error);
@@ -180,10 +182,9 @@ bool FlameCreationTask::updateInstance()
                 if (file.fileName.isEmpty() || file.targetFolder.isEmpty())
                     continue;
 
-                qDebug() << "Removing" << file.fileName << "at" << file.targetFolder;
-                QString path(FS::PathCombine(old_minecraft_dir, file.targetFolder, file.fileName));
-                if (!QFile::remove(path))
-                    qDebug() << "Failed to remove file at" << path;
+                QString relative_path(FS::PathCombine(file.targetFolder, file.fileName));
+                qDebug() << "Scheduling" << relative_path << "for removal";
+                m_files_to_remove.append(old_minecraft_dir.absoluteFilePath(relative_path));
             }
         });
         connect(job, &NetJob::finished, &loop, &QEventLoop::quit);
@@ -334,14 +335,17 @@ bool FlameCreationTask::createInstance()
 
     loop.exec();
 
-    if (m_instance) {
+    bool did_succeed = getError().isEmpty();
+
+    if (m_instance && did_succeed) {
+        setAbortStatus(false);
         auto inst = m_instance.value();
 
         inst->copyManagedPack(instance);
         inst->setName(instance.name());
     }
 
-    return getError().isEmpty();
+    return did_succeed;
 }
 
 void FlameCreationTask::idResolverSucceeded(QEventLoop& loop)
@@ -421,7 +425,6 @@ void FlameCreationTask::setupDownloadJob(QEventLoop& loop)
     m_mod_id_resolver.reset();
     connect(m_files_job.get(), &NetJob::succeeded, this, [&]() {
         m_files_job.reset();
-        emitSucceeded();
     });
     connect(m_files_job.get(), &NetJob::failed, [&](QString reason) {
         m_files_job.reset();
diff --git a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
index a0c67876..7d19639c 100644
--- a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
+++ b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
@@ -105,12 +105,15 @@ bool ModrinthCreationTask::updateInstance()
         }
 
         QDir old_minecraft_dir(inst->gameRoot());
+
         // Some files were removed from the old version, and some will be downloaded in an updated version,
         // so we're fine removing them!
         if (!old_files.empty()) {
             for (auto const& file : old_files) {
-                qDebug() << "Removing" << file.path;
-                old_minecraft_dir.remove(file.path);
+                if (file.path.isEmpty())
+                    continue;
+                qDebug() << "Scheduling" << file.path << "for removal";
+                m_files_to_remove.append(old_minecraft_dir.absoluteFilePath(file.path));
             }
         }
 
@@ -119,14 +122,18 @@ bool ModrinthCreationTask::updateInstance()
         // FIXME: We may want to do something about disabled mods.
         auto old_overrides = Override::readOverrides("overrides", old_index_folder);
         for (auto entry : old_overrides) {
-            qDebug() << "Removing" << entry;
-            old_minecraft_dir.remove(entry);
+            if (entry.isEmpty())
+                continue;
+            qDebug() << "Scheduling" << entry << "for removal";
+            m_files_to_remove.append(old_minecraft_dir.absoluteFilePath(entry));
         }
 
         auto old_client_overrides = Override::readOverrides("client-overrides", old_index_folder);
         for (auto entry : old_overrides) {
-            qDebug() << "Removing" << entry;
-            old_minecraft_dir.remove(entry);
+            if (entry.isEmpty())
+                continue;
+            qDebug() << "Scheduling" << entry << "for removal";
+            m_files_to_remove.append(old_minecraft_dir.absoluteFilePath(entry));
         }
     }
 
@@ -244,7 +251,8 @@ bool ModrinthCreationTask::createInstance()
 
     loop.exec();
 
-    if (m_instance) {
+    if (m_instance && ended_well) {
+        setAbortStatus(false);
         auto inst = m_instance.value();
 
         inst->copyManagedPack(instance);
-- 
cgit 


From be8c6f218cfe3acc31335305bb40284f8f6cb582 Mon Sep 17 00:00:00 2001
From: flow <flowlnlnln@gmail.com>
Date: Sun, 21 Aug 2022 09:26:27 -0300
Subject: refactor: setAbortStatus -> setAbortable

Signed-off-by: flow <flowlnlnln@gmail.com>
---
 launcher/InstanceCreationTask.cpp                              | 4 ++--
 launcher/InstanceImportTask.cpp                                | 6 +++---
 launcher/InstanceList.cpp                                      | 2 +-
 launcher/modplatform/flame/FlameInstanceCreationTask.cpp       | 2 +-
 launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp | 2 +-
 launcher/tasks/Task.h                                          | 2 +-
 6 files changed, 9 insertions(+), 9 deletions(-)

(limited to 'launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp')

diff --git a/launcher/InstanceCreationTask.cpp b/launcher/InstanceCreationTask.cpp
index 1b1a8c15..3971effa 100644
--- a/launcher/InstanceCreationTask.cpp
+++ b/launcher/InstanceCreationTask.cpp
@@ -7,7 +7,7 @@ InstanceCreationTask::InstanceCreationTask() = default;
 
 void InstanceCreationTask::executeTask()
 {
-    setAbortStatus(true);
+    setAbortable(true);
 
     if (updateInstance()) {
         emitSucceeded();
@@ -35,7 +35,7 @@ void InstanceCreationTask::executeTask()
     // files scheduled to, and we'd better not let the user abort in the middle of it, since it'd
     // put the instance in an invalid state.
     if (shouldOverride()) {
-        setAbortStatus(false);
+        setAbortable(false);
         setStatus(tr("Removing old conflicting files..."));
         qDebug() << "Removing old files";
 
diff --git a/launcher/InstanceImportTask.cpp b/launcher/InstanceImportTask.cpp
index e35913da..b490620d 100644
--- a/launcher/InstanceImportTask.cpp
+++ b/launcher/InstanceImportTask.cpp
@@ -75,7 +75,7 @@ bool InstanceImportTask::abort()
 
 void InstanceImportTask::executeTask()
 {
-    setAbortStatus(true);
+    setAbortable(true);
 
     if (m_sourceUrl.isLocalFile()) {
         m_archivePath = m_sourceUrl.toLocalFile();
@@ -280,7 +280,7 @@ void InstanceImportTask::processFlame()
 
     connect(this, &Task::aborted, inst_creation_task, &InstanceCreationTask::abort);
     connect(inst_creation_task, &Task::aborted, this, &Task::abort);
-    connect(inst_creation_task, &Task::abortStatusChanged, this, &Task::setAbortStatus);
+    connect(inst_creation_task, &Task::abortStatusChanged, this, &Task::setAbortable);
 
     inst_creation_task->start();
 }
@@ -344,7 +344,7 @@ void InstanceImportTask::processModrinth()
 
     connect(this, &Task::aborted, inst_creation_task, &InstanceCreationTask::abort);
     connect(inst_creation_task, &Task::aborted, this, &Task::abort);
-    connect(inst_creation_task, &Task::abortStatusChanged, this, &Task::setAbortStatus);
+    connect(inst_creation_task, &Task::abortStatusChanged, this, &Task::setAbortable);
 
     inst_creation_task->start();
 }
diff --git a/launcher/InstanceList.cpp b/launcher/InstanceList.cpp
index a4b8d8aa..a414e0d5 100644
--- a/launcher/InstanceList.cpp
+++ b/launcher/InstanceList.cpp
@@ -785,7 +785,7 @@ class InstanceStaging : public Task {
         connect(child, &Task::succeeded, this, &InstanceStaging::childSucceded);
         connect(child, &Task::failed, this, &InstanceStaging::childFailed);
         connect(child, &Task::aborted, this, &InstanceStaging::childAborted);
-        connect(child, &Task::abortStatusChanged, this, &InstanceStaging::setAbortStatus);
+        connect(child, &Task::abortStatusChanged, this, &InstanceStaging::setAbortable);
         connect(child, &Task::status, this, &InstanceStaging::setStatus);
         connect(child, &Task::progress, this, &InstanceStaging::setProgress);
         connect(&m_backoffTimer, &QTimer::timeout, this, &InstanceStaging::childSucceded);
diff --git a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp
index e3521a38..a0eda1b0 100644
--- a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp
+++ b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp
@@ -338,7 +338,7 @@ bool FlameCreationTask::createInstance()
     bool did_succeed = getError().isEmpty();
 
     if (m_instance && did_succeed) {
-        setAbortStatus(false);
+        setAbortable(false);
         auto inst = m_instance.value();
 
         inst->copyManagedPack(instance);
diff --git a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
index 7d19639c..d4c37cb9 100644
--- a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
+++ b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
@@ -252,7 +252,7 @@ bool ModrinthCreationTask::createInstance()
     loop.exec();
 
     if (m_instance && ended_well) {
-        setAbortStatus(false);
+        setAbortable(false);
         auto inst = m_instance.value();
 
         inst->copyManagedPack(instance);
diff --git a/launcher/tasks/Task.h b/launcher/tasks/Task.h
index f2872643..3d607dca 100644
--- a/launcher/tasks/Task.h
+++ b/launcher/tasks/Task.h
@@ -107,7 +107,7 @@ class Task : public QObject, public QRunnable {
     virtual void start();
     virtual bool abort() { if(canAbort()) emitAborted(); return canAbort(); };
 
-    void setAbortStatus(bool can_abort) { m_can_abort = can_abort; emit abortStatusChanged(can_abort); }
+    void setAbortable(bool can_abort) { m_can_abort = can_abort; emit abortStatusChanged(can_abort); }
 
    protected:
     virtual void executeTask() = 0;
-- 
cgit 


From ddde885084a8eba61e691974edbc75186438ed55 Mon Sep 17 00:00:00 2001
From: flow <flowlnlnln@gmail.com>
Date: Sun, 21 Aug 2022 10:00:23 -0300
Subject: fix: show warning in case there's no old index when updating

Signed-off-by: flow <flowlnlnln@gmail.com>
---
 launcher/modplatform/flame/FlameInstanceCreationTask.cpp      | 11 +++++++++++
 .../modplatform/modrinth/ModrinthInstanceCreationTask.cpp     | 11 +++++++++++
 2 files changed, 22 insertions(+)

(limited to 'launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp')

diff --git a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp
index a0eda1b0..1b282770 100644
--- a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp
+++ b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp
@@ -195,6 +195,17 @@ bool FlameCreationTask::updateInstance()
         loop.exec();
 
         m_process_update_file_info_job = nullptr;
+    } else {
+        // We don't have an old index file, so we may duplicate stuff!
+        auto dialog = CustomMessageBox::selectable(m_parent,
+                tr("No index file."),
+                tr("We couldn't find a suitable index file for the older version. This may cause some of the files to be duplicated. Do you want to continue?"),
+                QMessageBox::Warning, QMessageBox::Ok | QMessageBox::Cancel);
+
+        if (dialog->exec() == QDialog::DialogCode::Rejected) {
+            m_abort = true;
+            return false;
+        }
     }
 
     setOverride(true);
diff --git a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
index d4c37cb9..c1898dd9 100644
--- a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
+++ b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
@@ -135,6 +135,17 @@ bool ModrinthCreationTask::updateInstance()
             qDebug() << "Scheduling" << entry << "for removal";
             m_files_to_remove.append(old_minecraft_dir.absoluteFilePath(entry));
         }
+    } else {
+        // We don't have an old index file, so we may duplicate stuff!
+        auto dialog = CustomMessageBox::selectable(m_parent,
+                tr("No index file."),
+                tr("We couldn't find a suitable index file for the older version. This may cause some of the files to be duplicated. Do you want to continue?"),
+                QMessageBox::Warning, QMessageBox::Ok | QMessageBox::Cancel);
+
+        if (dialog->exec() == QDialog::DialogCode::Rejected) {
+            m_abort = true;
+            return false;
+        }
     }
 
 
-- 
cgit 


From 06019f01e3e7b87e752ddc15deb850e272a82d21 Mon Sep 17 00:00:00 2001
From: flow <flowlnlnln@gmail.com>
Date: Sun, 11 Sep 2022 13:16:25 -0300
Subject: feat: add dialog to ask whether to chaneg instance's name

This prevents custom names from being lost when updating, by only
changing the name if the old instance name constains the old version,
so that we can update it if the user whishes to.

Signed-off-by: flow <flowlnlnln@gmail.com>
---
 launcher/InstanceList.cpp                          |  5 -----
 launcher/InstanceTask.cpp                          | 24 +++++++++++++++++++---
 launcher/InstanceTask.h                            |  4 ++++
 .../flame/FlameInstanceCreationTask.cpp            | 12 +++++++++--
 .../modrinth/ModrinthInstanceCreationTask.cpp      | 10 ++++++++-
 5 files changed, 44 insertions(+), 11 deletions(-)

(limited to 'launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp')

diff --git a/launcher/InstanceList.cpp b/launcher/InstanceList.cpp
index a414e0d5..47b0e75a 100644
--- a/launcher/InstanceList.cpp
+++ b/launcher/InstanceList.cpp
@@ -908,11 +908,6 @@ bool InstanceList::commitStagedInstance(QString path, InstanceName const& instan
                 qWarning() << "Failed to override" << path << "to" << destination;
                 return false;
             }
-
-            if (!inst)
-                inst = getInstanceById(instID);
-            if (inst)
-                inst->setName(instanceName.name());
         } else {
             if (!dir.rename(path, destination)) {
                 qWarning() << "Failed to move" << path << "to" << destination;
diff --git a/launcher/InstanceTask.cpp b/launcher/InstanceTask.cpp
index 43a0b947..da280731 100644
--- a/launcher/InstanceTask.cpp
+++ b/launcher/InstanceTask.cpp
@@ -1,5 +1,23 @@
 #include "InstanceTask.h"
 
+#include "ui/dialogs/CustomMessageBox.h"
+
+InstanceNameChange askForChangingInstanceName(QWidget* parent, QString old_name, QString new_name)
+{
+    auto dialog =
+        CustomMessageBox::selectable(parent, QObject::tr("Change instance name"),
+                                     QObject::tr("The instance's name seems to include the old version. Would you like to update it?\n\n"
+                                                 "Old name: %1\n"
+                                                 "New name: %2")
+                                         .arg(old_name, new_name),
+                                     QMessageBox::Question, QMessageBox::No | QMessageBox::Yes);
+    auto result = dialog->exec();
+
+    if (result == QMessageBox::Yes)
+        return InstanceNameChange::ShouldChange;
+    return InstanceNameChange::ShouldKeep;
+}
+
 QString InstanceName::name() const
 {
     if (!m_modified_name.isEmpty())
@@ -26,9 +44,9 @@ QString InstanceName::version() const
 
 void InstanceName::setName(InstanceName& other)
 {
-   m_original_name = other.m_original_name; 
-   m_original_version = other.m_original_version; 
-   m_modified_name = other.m_modified_name;
+    m_original_name = other.m_original_name;
+    m_original_version = other.m_original_version;
+    m_modified_name = other.m_modified_name;
 }
 
 InstanceTask::InstanceTask() : Task(), InstanceName() {}
diff --git a/launcher/InstanceTask.h b/launcher/InstanceTask.h
index 5d67a2f0..0987b557 100644
--- a/launcher/InstanceTask.h
+++ b/launcher/InstanceTask.h
@@ -3,6 +3,10 @@
 #include "settings/SettingsObject.h"
 #include "tasks/Task.h"
 
+/* Helpers */
+enum class InstanceNameChange { ShouldChange, ShouldKeep };
+[[nodiscard]] InstanceNameChange askForChangingInstanceName(QWidget* parent, QString old_name, QString new_name);
+
 struct InstanceName {
    public:
     InstanceName() = default;
diff --git a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp
index 1b282770..69a41e55 100644
--- a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp
+++ b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp
@@ -15,8 +15,8 @@
 
 #include "settings/INISettingsObject.h"
 
-#include "ui/dialogs/CustomMessageBox.h"
 #include "ui/dialogs/BlockedModsDialog.h"
+#include "ui/dialogs/CustomMessageBox.h"
 
 const static QMap<QString, QString> forgemap = { { "1.2.5", "3.4.9.171" },
                                                  { "1.4.2", "6.0.1.355" },
@@ -348,12 +348,20 @@ bool FlameCreationTask::createInstance()
 
     bool did_succeed = getError().isEmpty();
 
+    // Update information of the already installed instance, if any.
     if (m_instance && did_succeed) {
         setAbortable(false);
         auto inst = m_instance.value();
 
+        // Only change the name if it didn't use a custom name, so that the previous custom name
+        // is preserved, but if we're using the original one, we update the version string.
+        // NOTE: This needs to come before the copyManagedPack call!
+        if (inst->name().contains(inst->getManagedPackVersionName())) {
+            if (askForChangingInstanceName(m_parent, inst->name(), instance.name()) == InstanceNameChange::ShouldChange)
+                inst->setName(instance.name());
+        }
+
         inst->copyManagedPack(instance);
-        inst->setName(instance.name());
     }
 
     return did_succeed;
diff --git a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
index c1898dd9..2cb6e786 100644
--- a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
+++ b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
@@ -262,12 +262,20 @@ bool ModrinthCreationTask::createInstance()
 
     loop.exec();
 
+    // Update information of the already installed instance, if any.
     if (m_instance && ended_well) {
         setAbortable(false);
         auto inst = m_instance.value();
 
+        // Only change the name if it didn't use a custom name, so that the previous custom name
+        // is preserved, but if we're using the original one, we update the version string.
+        // NOTE: This needs to come before the copyManagedPack call!
+        if (inst->name().contains(inst->getManagedPackVersionName())) {
+            if (askForChangingInstanceName(m_parent, inst->name(), instance.name()) == InstanceNameChange::ShouldChange)
+                inst->setName(instance.name());
+        }
+
         inst->copyManagedPack(instance);
-        inst->setName(instance.name());
     }
 
     return ended_well;
-- 
cgit 


From 9ff364b0d3c12f9138037cce585c03c850721b82 Mon Sep 17 00:00:00 2001
From: timoreo <contact@timoreo.fr>
Date: Mon, 26 Sep 2022 11:50:31 +0200
Subject: huge nit: added const refs, everywhere

Signed-off-by: timoreo <contact@timoreo.fr>
---
 launcher/InstanceList.cpp                                      |  4 ++--
 launcher/InstanceList.h                                        |  4 ++--
 launcher/InstanceTask.cpp                                      |  2 +-
 launcher/InstanceTask.h                                        |  2 +-
 launcher/minecraft/VanillaInstanceCreationTask.cpp             |  4 +++-
 launcher/minecraft/VanillaInstanceCreationTask.h               |  4 +++-
 launcher/modplatform/flame/FlameAPI.cpp                        |  2 +-
 launcher/modplatform/flame/FlameAPI.h                          |  2 +-
 launcher/modplatform/flame/FlameInstanceCreationTask.cpp       |  4 ++--
 launcher/modplatform/flame/FlameInstanceCreationTask.h         |  2 +-
 launcher/modplatform/helpers/OverrideUtils.cpp                 |  4 ++--
 launcher/modplatform/helpers/OverrideUtils.h                   |  4 ++--
 launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp | 10 +++++-----
 launcher/modplatform/modrinth/ModrinthInstanceCreationTask.h   |  2 +-
 launcher/ui/dialogs/NewInstanceDialog.cpp                      |  7 ++++---
 launcher/ui/dialogs/NewInstanceDialog.h                        |  4 ++--
 16 files changed, 33 insertions(+), 28 deletions(-)

(limited to 'launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp')

diff --git a/launcher/InstanceList.cpp b/launcher/InstanceList.cpp
index 6b0e3a4b..cebd70d7 100644
--- a/launcher/InstanceList.cpp
+++ b/launcher/InstanceList.cpp
@@ -535,7 +535,7 @@ InstancePtr InstanceList::getInstanceById(QString instId) const
     return InstancePtr();
 }
 
-InstancePtr InstanceList::getInstanceByManagedName(QString managed_name) const
+InstancePtr InstanceList::getInstanceByManagedName(const QString& managed_name) const
 {
     if (managed_name.isEmpty())
         return {};
@@ -880,7 +880,7 @@ QString InstanceList::getStagedInstancePath()
     return path;
 }
 
-bool InstanceList::commitStagedInstance(QString path, InstanceName const& instanceName, QString groupName, bool should_override)
+bool InstanceList::commitStagedInstance(const QString& path, InstanceName const& instanceName, const QString& groupName, bool should_override)
 {
     QDir dir;
     QString instID;
diff --git a/launcher/InstanceList.h b/launcher/InstanceList.h
index df11b55a..3673298f 100644
--- a/launcher/InstanceList.h
+++ b/launcher/InstanceList.h
@@ -104,7 +104,7 @@ public:
     /* O(n) */
     InstancePtr getInstanceById(QString id) const;
     /* O(n) */
-    InstancePtr getInstanceByManagedName(QString managed_name) const;
+    InstancePtr getInstanceByManagedName(const QString& managed_name) const;
     QModelIndex getInstanceIndexById(const QString &id) const;
     QStringList getGroups();
     bool isGroupCollapsed(const QString &groupName);
@@ -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(QString keyPath, const InstanceName& instanceName, QString groupName, bool should_override);
+    bool commitStagedInstance(const QString& keyPath, const InstanceName& instanceName, const QString& groupName, bool should_override);
 
     /**
      * Destroy a previously created staging area given by @keyPath - used when creation fails.
diff --git a/launcher/InstanceTask.cpp b/launcher/InstanceTask.cpp
index da280731..55a44fd3 100644
--- a/launcher/InstanceTask.cpp
+++ b/launcher/InstanceTask.cpp
@@ -2,7 +2,7 @@
 
 #include "ui/dialogs/CustomMessageBox.h"
 
-InstanceNameChange askForChangingInstanceName(QWidget* parent, QString old_name, QString new_name)
+InstanceNameChange askForChangingInstanceName(QWidget* parent, const QString& old_name, const QString& new_name)
 {
     auto dialog =
         CustomMessageBox::selectable(parent, QObject::tr("Change instance name"),
diff --git a/launcher/InstanceTask.h b/launcher/InstanceTask.h
index 0987b557..e35533fc 100644
--- a/launcher/InstanceTask.h
+++ b/launcher/InstanceTask.h
@@ -5,7 +5,7 @@
 
 /* Helpers */
 enum class InstanceNameChange { ShouldChange, ShouldKeep };
-[[nodiscard]] InstanceNameChange askForChangingInstanceName(QWidget* parent, QString old_name, QString new_name);
+[[nodiscard]] InstanceNameChange askForChangingInstanceName(QWidget* parent, const QString& old_name, const QString& new_name);
 
 struct InstanceName {
    public:
diff --git a/launcher/minecraft/VanillaInstanceCreationTask.cpp b/launcher/minecraft/VanillaInstanceCreationTask.cpp
index fb11379c..c45daa9a 100644
--- a/launcher/minecraft/VanillaInstanceCreationTask.cpp
+++ b/launcher/minecraft/VanillaInstanceCreationTask.cpp
@@ -1,12 +1,14 @@
 #include "VanillaInstanceCreationTask.h"
 
+#include <utility>
+
 #include "FileSystem.h"
 #include "minecraft/MinecraftInstance.h"
 #include "minecraft/PackProfile.h"
 #include "settings/INISettingsObject.h"
 
 VanillaCreationTask::VanillaCreationTask(BaseVersionPtr version, QString loader, BaseVersionPtr loader_version)
-    : InstanceCreationTask(), m_version(version), m_using_loader(true), m_loader(loader), m_loader_version(loader_version)
+    : InstanceCreationTask(), m_version(std::move(version)), m_using_loader(true), m_loader(std::move(loader)), m_loader_version(std::move(loader_version))
 {}
 
 bool VanillaCreationTask::createInstance()
diff --git a/launcher/minecraft/VanillaInstanceCreationTask.h b/launcher/minecraft/VanillaInstanceCreationTask.h
index 540ecb70..7a37bbd6 100644
--- a/launcher/minecraft/VanillaInstanceCreationTask.h
+++ b/launcher/minecraft/VanillaInstanceCreationTask.h
@@ -2,10 +2,12 @@
 
 #include "InstanceCreationTask.h"
 
+#include <utility>
+
 class VanillaCreationTask final : public InstanceCreationTask {
     Q_OBJECT
    public:
-    VanillaCreationTask(BaseVersionPtr version) : InstanceCreationTask(), m_version(version) {}
+    VanillaCreationTask(BaseVersionPtr version) : InstanceCreationTask(), m_version(std::move(version)) {}
     VanillaCreationTask(BaseVersionPtr version, QString loader, BaseVersionPtr loader_version);
 
     bool createInstance() override;
diff --git a/launcher/modplatform/flame/FlameAPI.cpp b/launcher/modplatform/flame/FlameAPI.cpp
index f8f50dc6..4d71da21 100644
--- a/launcher/modplatform/flame/FlameAPI.cpp
+++ b/launcher/modplatform/flame/FlameAPI.cpp
@@ -184,7 +184,7 @@ auto FlameAPI::getProjects(QStringList addonIds, QByteArray* response) const ->
     return netJob;
 }
 
-auto FlameAPI::getFiles(QStringList fileIds, QByteArray* response) const -> NetJob*
+auto FlameAPI::getFiles(const QStringList& fileIds, QByteArray* response) const -> NetJob*
 {
     auto* netJob = new NetJob(QString("Flame::GetFiles"), APPLICATION->network());
 
diff --git a/launcher/modplatform/flame/FlameAPI.h b/launcher/modplatform/flame/FlameAPI.h
index 5e6166b9..4c6ca64c 100644
--- a/launcher/modplatform/flame/FlameAPI.h
+++ b/launcher/modplatform/flame/FlameAPI.h
@@ -12,7 +12,7 @@ class FlameAPI : public NetworkModAPI {
     auto getLatestVersion(VersionSearchArgs&& args) -> ModPlatform::IndexedVersion;
 
     auto getProjects(QStringList addonIds, QByteArray* response) const -> NetJob* override;
-    auto getFiles(QStringList fileIds, QByteArray* response) const -> NetJob*;
+    auto getFiles(const QStringList& fileIds, QByteArray* response) const -> NetJob*;
 
    private:
     inline auto getSortFieldInt(QString sortString) const -> int
diff --git a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp
index 69a41e55..48ac02e0 100644
--- a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp
+++ b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp
@@ -126,7 +126,7 @@ bool FlameCreationTask::updateInstance()
         // TODO: Currently 'overrides' will always override the stuff on update. How do we preserve unchanged overrides?
         // FIXME: We may want to do something about disabled mods.
         auto old_overrides = Override::readOverrides("overrides", old_index_folder);
-        for (auto entry : old_overrides) {
+        for (const auto& entry : old_overrides) {
             if (entry.isEmpty())
                 continue;
             qDebug() << "Scheduling" << entry << "for removal";
@@ -320,7 +320,7 @@ bool FlameCreationTask::createInstance()
         qDebug() << "Found jarmods:";
         QDir jarmodsDir(jarmodsPath);
         QStringList jarMods;
-        for (auto info : jarmodsDir.entryInfoList(QDir::NoDotAndDotDot | QDir::Files)) {
+        for (const auto& info : jarmodsDir.entryInfoList(QDir::NoDotAndDotDot | QDir::Files)) {
             qDebug() << info.fileName();
             jarMods.push_back(info.absoluteFilePath());
         }
diff --git a/launcher/modplatform/flame/FlameInstanceCreationTask.h b/launcher/modplatform/flame/FlameInstanceCreationTask.h
index ccb5f827..ded0e2ce 100644
--- a/launcher/modplatform/flame/FlameInstanceCreationTask.h
+++ b/launcher/modplatform/flame/FlameInstanceCreationTask.h
@@ -14,7 +14,7 @@ class FlameCreationTask final : public InstanceCreationTask {
     Q_OBJECT
 
    public:
-    FlameCreationTask(QString staging_path, SettingsObjectPtr global_settings, QWidget* parent)
+    FlameCreationTask(const QString& staging_path, SettingsObjectPtr global_settings, QWidget* parent)
         : InstanceCreationTask(), m_parent(parent)
     {
         setStagingPath(staging_path);
diff --git a/launcher/modplatform/helpers/OverrideUtils.cpp b/launcher/modplatform/helpers/OverrideUtils.cpp
index 3496a286..65b5f760 100644
--- a/launcher/modplatform/helpers/OverrideUtils.cpp
+++ b/launcher/modplatform/helpers/OverrideUtils.cpp
@@ -6,7 +6,7 @@
 
 namespace Override {
 
-void createOverrides(QString name, QString parent_folder, QString override_path)
+void createOverrides(const QString& name, const QString& parent_folder, const QString& override_path)
 {
     QString file_path(FS::PathCombine(parent_folder, name + ".txt"));
     if (QFile::exists(file_path))
@@ -33,7 +33,7 @@ void createOverrides(QString name, QString parent_folder, QString override_path)
     file.close();
 }
 
-QStringList readOverrides(QString name, QString parent_folder)
+QStringList readOverrides(const QString& name, const QString& parent_folder)
 {
     QString file_path(FS::PathCombine(parent_folder, name + ".txt"));
 
diff --git a/launcher/modplatform/helpers/OverrideUtils.h b/launcher/modplatform/helpers/OverrideUtils.h
index 73f059d6..536261a2 100644
--- a/launcher/modplatform/helpers/OverrideUtils.h
+++ b/launcher/modplatform/helpers/OverrideUtils.h
@@ -9,12 +9,12 @@ namespace Override {
  *
  *  If there's already an existing such file, it will be ovewritten.
  */
-void createOverrides(QString name, QString parent_folder, QString override_path);
+void createOverrides(const QString& name, const QString& parent_folder, const QString& override_path);
 
 /** This reads an existing overrides archive, returning a list of overrides.
  *
  *  If there's no such file in `parent_folder`, it will return an empty list.
  */
-QStringList readOverrides(QString name, QString parent_folder);
+QStringList readOverrides(const QString& name, const QString& parent_folder);
 
 }  // namespace Override
diff --git a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
index 2cb6e786..f9709551 100644
--- a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
+++ b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
@@ -121,7 +121,7 @@ bool ModrinthCreationTask::updateInstance()
         // TODO: Currently 'overrides' will always override the stuff on update. How do we preserve unchanged overrides?
         // FIXME: We may want to do something about disabled mods.
         auto old_overrides = Override::readOverrides("overrides", old_index_folder);
-        for (auto entry : old_overrides) {
+        for (const auto& entry : old_overrides) {
             if (entry.isEmpty())
                 continue;
             qDebug() << "Scheduling" << entry << "for removal";
@@ -129,7 +129,7 @@ bool ModrinthCreationTask::updateInstance()
         }
 
         auto old_client_overrides = Override::readOverrides("client-overrides", old_index_folder);
-        for (auto entry : old_overrides) {
+        for (const auto& entry : old_overrides) {
             if (entry.isEmpty())
                 continue;
             qDebug() << "Scheduling" << entry << "for removal";
@@ -235,7 +235,7 @@ bool ModrinthCreationTask::createInstance()
         dl->addValidator(new Net::ChecksumValidator(file.hashAlgorithm, file.hash));
         m_files_job->addNetAction(dl);
 
-        if (file.downloads.size() > 0) {
+        if (!file.downloads.empty()) {
             // FIXME: This really needs to be put into a ConcurrentTask of
             // MultipleOptionsTask's , once those exist :)
             connect(dl.get(), &NetAction::failed, [this, &file, path, dl] {
@@ -281,7 +281,7 @@ bool ModrinthCreationTask::createInstance()
     return ended_well;
 }
 
-bool ModrinthCreationTask::parseManifest(QString index_path, std::vector<Modrinth::File>& files, bool set_managed_info, bool show_optional_dialog)
+bool ModrinthCreationTask::parseManifest(const QString& index_path, std::vector<Modrinth::File>& files, bool set_managed_info, bool show_optional_dialog)
 {
     try {
         auto doc = Json::requireDocument(index_path);
@@ -300,7 +300,7 @@ bool ModrinthCreationTask::parseManifest(QString index_path, std::vector<Modrint
 
             auto jsonFiles = Json::requireIsArrayOf<QJsonObject>(obj, "files", "modrinth.index.json");
             bool had_optional = false;
-            for (auto modInfo : jsonFiles) {
+            for (const auto& modInfo : jsonFiles) {
                 Modrinth::File file;
                 file.path = Json::requireString(modInfo, "path");
 
diff --git a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.h b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.h
index 01d0d1bf..e459aadf 100644
--- a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.h
+++ b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.h
@@ -27,7 +27,7 @@ class ModrinthCreationTask final : public InstanceCreationTask {
     bool createInstance() override;
 
    private:
-    bool parseManifest(QString, std::vector<Modrinth::File>&, bool set_managed_info = true, bool show_optional_dialog = true);
+    bool parseManifest(const QString&, std::vector<Modrinth::File>&, bool set_managed_info = true, bool show_optional_dialog = true);
     QString getManagedPackID() const;
 
    private:
diff --git a/launcher/ui/dialogs/NewInstanceDialog.cpp b/launcher/ui/dialogs/NewInstanceDialog.cpp
index 04fecbde..d203795a 100644
--- a/launcher/ui/dialogs/NewInstanceDialog.cpp
+++ b/launcher/ui/dialogs/NewInstanceDialog.cpp
@@ -51,6 +51,7 @@
 #include <QFileDialog>
 #include <QValidator>
 #include <QDialogButtonBox>
+#include <utility>
 
 #include "ui/widgets/PageContainer.h"
 #include "ui/pages/modplatform/VanillaPage.h"
@@ -177,7 +178,7 @@ NewInstanceDialog::~NewInstanceDialog()
     delete ui;
 }
 
-void NewInstanceDialog::setSuggestedPack(QString name, InstanceTask* task)
+void NewInstanceDialog::setSuggestedPack(const QString& name, InstanceTask* task)
 {
     creationTask.reset(task);
 
@@ -193,12 +194,12 @@ void NewInstanceDialog::setSuggestedPack(QString name, InstanceTask* task)
     m_buttons->button(QDialogButtonBox::Ok)->setEnabled(allowOK);
 }
 
-void NewInstanceDialog::setSuggestedPack(QString name, QString version, InstanceTask* task)
+void NewInstanceDialog::setSuggestedPack(const QString& name, QString version, InstanceTask* task)
 {
     creationTask.reset(task);
 
     ui->instNameTextBox->setPlaceholderText(name);
-    importVersion = version;
+    importVersion = std::move(version);
 
     if (!task) {
         ui->iconButton->setIcon(APPLICATION->icons()->getIcon("default"));
diff --git a/launcher/ui/dialogs/NewInstanceDialog.h b/launcher/ui/dialogs/NewInstanceDialog.h
index 185ab9e6..961f512e 100644
--- a/launcher/ui/dialogs/NewInstanceDialog.h
+++ b/launcher/ui/dialogs/NewInstanceDialog.h
@@ -60,8 +60,8 @@ public:
 
     void updateDialogState();
 
-    void setSuggestedPack(QString name = QString(), InstanceTask * task = nullptr);
-    void setSuggestedPack(QString name, QString version, InstanceTask * task = nullptr);
+    void setSuggestedPack(const QString& name = QString(), InstanceTask * task = nullptr);
+    void setSuggestedPack(const QString& name, QString version, InstanceTask * task = nullptr);
     void setSuggestedIconFromFile(const QString &path, const QString &name);
     void setSuggestedIcon(const QString &key);
 
-- 
cgit 


From dd6f670dec7dfd1a9ad6f4595ad5447ac735c737 Mon Sep 17 00:00:00 2001
From: timoreo <contact@timoreo.fr>
Date: Mon, 26 Sep 2022 11:50:55 +0200
Subject: fix: Fixed memory leak

Signed-off-by: timoreo <contact@timoreo.fr>
---
 .../modplatform/modrinth/ModrinthInstanceCreationTask.cpp     | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

(limited to 'launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp')

diff --git a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
index f9709551..ddeea224 100644
--- a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
+++ b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
@@ -238,11 +238,12 @@ bool ModrinthCreationTask::createInstance()
         if (!file.downloads.empty()) {
             // FIXME: This really needs to be put into a ConcurrentTask of
             // MultipleOptionsTask's , once those exist :)
-            connect(dl.get(), &NetAction::failed, [this, &file, path, dl] {
-                auto dl = Net::Download::makeFile(file.downloads.dequeue(), path);
-                dl->addValidator(new Net::ChecksumValidator(file.hashAlgorithm, file.hash));
-                m_files_job->addNetAction(dl);
-                dl->succeeded();
+            auto param = dl.toWeakRef();
+            connect(dl.get(), &NetAction::failed, [this, &file, path, param] {
+                auto ndl = Net::Download::makeFile(file.downloads.dequeue(), path);
+                ndl->addValidator(new Net::ChecksumValidator(file.hashAlgorithm, file.hash));
+                m_files_job->addNetAction(ndl);
+                if (auto shared = param.lock()) shared->succeeded();
             });
         }
     }
-- 
cgit