feat: refactored Instance ImportTask

Signed-off-by: Trial97 <alexandru.tripon97@gmail.com>

launcher/InstanceImportTask.cpp

@@ -45,14 +45,15 @@
 #include "icons/IconList.h"
 #include "icons/IconUtils.h"
 
-#include "modplatform/technic/TechnicPackProcessor.h"
-#include "modplatform/modrinth/ModrinthInstanceCreationTask.h"
 #include "modplatform/flame/FlameInstanceCreationTask.h"
+#include "modplatform/modrinth/ModrinthInstanceCreationTask.h"
+#include "modplatform/technic/TechnicPackProcessor.h"
 
 #include "settings/INISettingsObject.h"
 
 #include <QtConcurrentRun>
 #include <algorithm>
+#include <memory>
 
 #include <quazip/quazipdir.h>
 
@@ -65,15 +66,8 @@ bool InstanceImportTask::abort()
     if (!canAbort())
         return false;
 
-    if (m_filesNetJob)
+    if (task)
-        m_filesNetJob->abort();
+        task->abort();
-    if (m_extractFuture.isRunning()) {
-        // NOTE: The tasks created by QtConcurrent::run() can't actually get cancelled,
-        // but we can use this call to check the state when the extraction finishes.
-        m_extractFuture.cancel();
-        m_extractFuture.waitForFinished();
-    }
-
     return Task::abort();
 }
 
@@ -86,7 +80,6 @@ void InstanceImportTask::executeTask()
         processZipPack();
     } else {
         setStatus(tr("Downloading modpack:\n%1").arg(m_sourceUrl.toString()));
-        m_downloadRequired = true;
 
         const QString path(m_sourceUrl.host() + '/' + m_sourceUrl.path());
 
@@ -94,153 +87,153 @@ void InstanceImportTask::executeTask()
         entry->setStale(true);
         m_archivePath = entry->getFullPath();
 
-        m_filesNetJob.reset(new NetJob(tr("Modpack download"), APPLICATION->network()));
+        auto filesNetJob = makeShared<NetJob>(tr("Modpack download"), APPLICATION->network());
-        m_filesNetJob->addNetAction(Net::Download::makeCached(m_sourceUrl, entry));
+        filesNetJob->addNetAction(Net::Download::makeCached(m_sourceUrl, entry));
 
-        connect(m_filesNetJob.get(), &NetJob::succeeded, this, &InstanceImportTask::downloadSucceeded);
+        connect(filesNetJob.get(), &NetJob::succeeded, this, &InstanceImportTask::processZipPack);
-        connect(m_filesNetJob.get(), &NetJob::progress, this, &InstanceImportTask::downloadProgressChanged);
+        connect(filesNetJob.get(), &NetJob::progress, this, &InstanceImportTask::setProgress);
-        connect(m_filesNetJob.get(), &NetJob::stepProgress, this, &InstanceImportTask::propogateStepProgress);
+        connect(filesNetJob.get(), &NetJob::stepProgress, this, &InstanceImportTask::propogateStepProgress);
-        connect(m_filesNetJob.get(), &NetJob::failed, this, &InstanceImportTask::downloadFailed);
+        connect(filesNetJob.get(), &NetJob::failed, this, &InstanceImportTask::emitFailed);
-        connect(m_filesNetJob.get(), &NetJob::aborted, this, &InstanceImportTask::downloadAborted);
+        connect(filesNetJob.get(), &NetJob::aborted, this, &InstanceImportTask::emitAborted);
-
+        task.reset(filesNetJob);
-        m_filesNetJob->start();
+        filesNetJob->start();
     }
 }
 
-void InstanceImportTask::downloadSucceeded()
+QString InstanceImportTask::getRootFromZip(QuaZip* zip, const QString& root)
 {
-    processZipPack();
+    if (!isRunning()) {
-    m_filesNetJob.reset();
+        return {};
-}
+    }
+    QuaZipDir rootDir(zip, root);
+    for (auto&& fileName : rootDir.entryList(QDir::Files)) {
+        setDetails(fileName);
+        if (fileName == "instance.cfg") {
+            qDebug() << "MultiMC:" << true;
+            m_modpackType = ModpackType::MultiMC;
+            return root;
+        }
+        if (fileName == "manifest.json") {
+            qDebug() << "Flame:" << true;
+            m_modpackType = ModpackType::Flame;
+            return root;
+        }
 
-void InstanceImportTask::downloadFailed(QString reason)
+        QCoreApplication::processEvents();
-{
+    }
-    emitFailed(reason);
-    m_filesNetJob.reset();
-}
 
-void InstanceImportTask::downloadProgressChanged(qint64 current, qint64 total)
+    // Recurse the search to non-ignored subfolders
-{
+    for (auto&& fileName : rootDir.entryList(QDir::Dirs)) {
-    setProgress(current, total);
+        if ("overrides/" == fileName)
-}
+            continue;
 
-void InstanceImportTask::downloadAborted()
+        QString result = getRootFromZip(zip, root + fileName);
-{
+        if (!result.isEmpty())
-    emitAborted();
+            return result;
-    m_filesNetJob.reset();
+    }
+
+    return {};
 }
 
 void InstanceImportTask::processZipPack()
 {
-    setStatus(tr("Extracting modpack"));
+    setStatus(tr("Attempting to determine instance type"));
     QDir extractDir(m_stagingPath);
     qDebug() << "Attempting to create instance from" << m_archivePath;
 
     // open the zip and find relevant files in it
-    m_packZip.reset(new QuaZip(m_archivePath));
+    auto packZip = std::make_shared<QuaZip>(m_archivePath);
-    if (!m_packZip->open(QuaZip::mdUnzip))
+    if (!packZip->open(QuaZip::mdUnzip)) {
-    {
         emitFailed(tr("Unable to open supplied modpack zip file."));
         return;
     }
 
-    QuaZipDir packZipDir(m_packZip.get());
+    QuaZipDir packZipDir(packZip.get());
+    qDebug() << "Attempting to determine instance type";
 
-    // https://docs.modrinth.com/docs/modpacks/format_definition/#storage
-    bool modrinthFound = packZipDir.exists("/modrinth.index.json");
-    bool technicFound = packZipDir.exists("/bin/modpack.jar") || packZipDir.exists("/bin/version.json");
     QString root;
 
     // NOTE: Prioritize modpack platforms that aren't searched for recursively.
     // Especially Flame has a very common filename for its manifest, which may appear inside overrides for example
-    if(modrinthFound)
+    // https://docs.modrinth.com/docs/modpacks/format_definition/#storage
-    {
+    if (packZipDir.exists("/modrinth.index.json")) {
         // process as Modrinth pack
-        qDebug() << "Modrinth:" << modrinthFound;
+        qDebug() << "Modrinth:" << true;
         m_modpackType = ModpackType::Modrinth;
-    }
+    } else if (packZipDir.exists("/bin/modpack.jar") || packZipDir.exists("/bin/version.json")) {
-    else if (technicFound)
-    {
         // process as Technic pack
-        qDebug() << "Technic:" << technicFound;
+        qDebug() << "Technic:" << true;
         extractDir.mkpath(".minecraft");
         extractDir.cd(".minecraft");
         m_modpackType = ModpackType::Technic;
+    } else {
+        root = getRootFromZip(packZip.get());
+        setDetails("");
     }
-    else
+    if (m_modpackType == ModpackType::Unknown) {
-    {
-        QStringList paths_to_ignore { "overrides/" };
-
-        if (QString mmcRoot = MMCZip::findFolderOfFileInZip(m_packZip.get(), "instance.cfg", paths_to_ignore); !mmcRoot.isNull()) {
-            // process as MultiMC instance/pack
-            qDebug() << "MultiMC:" << mmcRoot;
-            root = mmcRoot;
-            m_modpackType = ModpackType::MultiMC;
-        } else if (QString flameRoot = MMCZip::findFolderOfFileInZip(m_packZip.get(), "manifest.json", paths_to_ignore); !flameRoot.isNull()) {
-            // process as Flame pack
-            qDebug() << "Flame:" << flameRoot;
-            root = flameRoot;
-            m_modpackType = ModpackType::Flame;
-        }
-    }
-    if(m_modpackType == ModpackType::Unknown)
-    {
         emitFailed(tr("Archive does not contain a recognized modpack type."));
         return;
     }
+    setStatus(tr("Extracting modpack"));
 
     // make sure we extract just the pack
-    m_extractFuture = QtConcurrent::run(QThreadPool::globalInstance(), MMCZip::extractSubDir, m_packZip.get(), root, extractDir.absolutePath());
+    auto zipTask = makeShared<MMCZip::ExtractZipTask>(packZip, extractDir, root);
-    connect(&m_extractFutureWatcher, &QFutureWatcher<QStringList>::finished, this, &InstanceImportTask::extractFinished);
+
-    m_extractFutureWatcher.setFuture(m_extractFuture);
+    auto progressStep = std::make_shared<TaskStepProgress>();
+    connect(zipTask.get(), &Task::finished, this, [this, progressStep] {
+        progressStep->state = TaskStepState::Succeeded;
+        stepProgress(*progressStep);
+    });
+
+    connect(zipTask.get(), &Task::succeeded, this, &InstanceImportTask::extractFinished);
+    connect(zipTask.get(), &Task::aborted, this, &InstanceImportTask::emitAborted);
+    connect(zipTask.get(), &Task::failed, this, [this, progressStep](QString reason) {
+        progressStep->state = TaskStepState::Failed;
+        stepProgress(*progressStep);
+        emitFailed(reason);
+    });
+    connect(zipTask.get(), &Task::stepProgress, this, &InstanceImportTask::propogateStepProgress);
+
+    connect(zipTask.get(), &Task::progress, this, [this, progressStep](qint64 current, qint64 total) {
+        progressStep->update(current, total);
+        stepProgress(*progressStep);
+    });
+    connect(zipTask.get(), &Task::status, this, [this, progressStep](QString status) {
+        progressStep->status = status;
+        stepProgress(*progressStep);
+    });
+    task.reset(zipTask);
+    zipTask->start();
 }
 
 void InstanceImportTask::extractFinished()
 {
-    m_packZip.reset();
-
-    if (m_extractFuture.isCanceled())
-        return;
-    if (!m_extractFuture.result().has_value()) {
-        emitFailed(tr("Failed to extract modpack"));
-        return;
-    }
-
     QDir extractDir(m_stagingPath);
 
     qDebug() << "Fixing permissions for extracted pack files...";
     QDirIterator it(extractDir, QDirIterator::Subdirectories);
-    while (it.hasNext())
+    while (it.hasNext()) {
-    {
         auto filepath = it.next();
         QFileInfo file(filepath);
         auto permissions = QFile::permissions(filepath);
         auto origPermissions = permissions;
-        if(file.isDir())
+        if (file.isDir()) {
-        {
             // Folder +rwx for current user
             permissions |= QFileDevice::Permission::ReadUser | QFileDevice::Permission::WriteUser | QFileDevice::Permission::ExeUser;
-        }
+        } else {
-        else
-        {
             // File +rw for current user
             permissions |= QFileDevice::Permission::ReadUser | QFileDevice::Permission::WriteUser;
         }
-        if(origPermissions != permissions)
+        if (origPermissions != permissions) {
-        {
+            if (!QFile::setPermissions(filepath, permissions)) {
-            if(!QFile::setPermissions(filepath, permissions))
-            {
                 logWarning(tr("Could not fix permissions for %1").arg(filepath));
-            }
+            } else {
-            else
-            {
                 qDebug() << "Fixed" << filepath;
             }
         }
     }
 
-    switch(m_modpackType)
+    switch (m_modpackType) {
-    {
         case ModpackType::MultiMC:
             processMultiMC();
             return;
@@ -276,7 +269,8 @@ void InstanceImportTask::processFlame()
         if (original_instance_id_it != m_extra_info.constEnd())
             original_instance_id = original_instance_id_it.value();
 
-        inst_creation_task = makeShared<FlameCreationTask>(m_stagingPath, m_globalSettings, m_parent, pack_id, pack_version_id, original_instance_id);
+        inst_creation_task =
+            makeShared<FlameCreationTask>(m_stagingPath, m_globalSettings, m_parent, pack_id, pack_version_id, original_instance_id);
     } else {
         // FIXME: Find a way to get IDs in directly imported ZIPs
         inst_creation_task = makeShared<FlameCreationTask>(m_stagingPath, m_globalSettings, m_parent, QString(), QString());
@@ -286,7 +280,7 @@ void InstanceImportTask::processFlame()
     inst_creation_task->setIcon(m_instIcon);
     inst_creation_task->setGroup(m_instGroup);
     inst_creation_task->setConfirmUpdate(shouldConfirmUpdate());
-    
+
     connect(inst_creation_task.get(), &Task::succeeded, this, [this, inst_creation_task] {
         setOverride(inst_creation_task->shouldOverride(), inst_creation_task->originalInstanceID());
         emitSucceeded();
@@ -362,7 +356,8 @@ void InstanceImportTask::processModrinth()
         if (original_instance_id_it != m_extra_info.constEnd())
             original_instance_id = original_instance_id_it.value();
 
-        inst_creation_task = new ModrinthCreationTask(m_stagingPath, m_globalSettings, m_parent, pack_id, pack_version_id, original_instance_id);
+        inst_creation_task =
+            new ModrinthCreationTask(m_stagingPath, m_globalSettings, m_parent, pack_id, pack_version_id, original_instance_id);
     } else {
         QString pack_id;
         if (!m_sourceUrl.isEmpty()) {
@@ -378,7 +373,7 @@ void InstanceImportTask::processModrinth()
     inst_creation_task->setIcon(m_instIcon);
     inst_creation_task->setGroup(m_instGroup);
     inst_creation_task->setConfirmUpdate(shouldConfirmUpdate());
-    
+
     connect(inst_creation_task, &Task::succeeded, this, [this, inst_creation_task] {
         setOverride(inst_creation_task->shouldOverride(), inst_creation_task->originalInstanceID());
         emitSucceeded();

launcher/InstanceImportTask.h

@@ -35,64 +35,46 @@
 
 #pragma once
 
-#include "InstanceTask.h"
-#include "net/NetJob.h"
-#include <QUrl>
 #include <QFuture>
 #include <QFutureWatcher>
-#include "settings/SettingsObject.h"
+#include <QUrl>
-#include "QObjectPtr.h"
+#include "InstanceTask.h"
-#include "modplatform/flame/PackManifest.h"
 
+#include <memory>
 #include <optional>
 
 class QuaZip;
-namespace Flame
+namespace Flame {
-{
+class FileResolvingTask;
-    class FileResolvingTask;
 }
 
-class InstanceImportTask : public InstanceTask
+class InstanceImportTask : public InstanceTask {
-{
     Q_OBJECT
-public:
+   public:
     explicit InstanceImportTask(const QUrl sourceUrl, QWidget* parent = nullptr, QMap<QString, QString>&& extra_info = {});
 
     bool abort() override;
-    const QVector<Flame::File> &getBlockedFiles() const
-    {
-        return m_blockedMods;
-    }
 
-protected:
+   protected:
     //! Entry point for tasks.
     virtual void executeTask() override;
 
-private:
+   private:
-    void processZipPack();
     void processMultiMC();
     void processTechnic();
     void processFlame();
     void processModrinth();
+    QString getRootFromZip(QuaZip* zip, const QString& root = "");
 
-private slots:
+   private slots:
-    void downloadSucceeded();
+    void processZipPack();
-    void downloadFailed(QString reason);
-    void downloadProgressChanged(qint64 current, qint64 total);
-    void downloadAborted();
     void extractFinished();
 
-private: /* data */
+   private: /* data */
-    NetJob::Ptr m_filesNetJob;
-    shared_qobject_ptr<Flame::FileResolvingTask> m_modIdResolver;
     QUrl m_sourceUrl;
     QString m_archivePath;
-    bool m_downloadRequired = false;
+    Task::Ptr task;
-    std::unique_ptr<QuaZip> m_packZip;
+    enum class ModpackType {
-    QFuture<std::optional<QStringList>> m_extractFuture;
-    QFutureWatcher<std::optional<QStringList>> m_extractFutureWatcher;
-    QVector<Flame::File> m_blockedMods;
-    enum class ModpackType{
         Unknown,
         MultiMC,
         Technic,
@@ -104,6 +86,6 @@ private: /* data */
     // the source URL / the resource it points to alone.
     QMap<QString, QString> m_extra_info;
 
-    //FIXME: nuke
+    // FIXME: nuke
     QWidget* m_parent;
 };

launcher/MMCZip.cpp

@@ -501,4 +501,109 @@ bool ExportToZipTask::abort()
     return false;
 }
 
+void ExtractZipTask::executeTask()
+{
+    m_zip_future = QtConcurrent::run(QThreadPool::globalInstance(), [this]() { return extractZip(); });
+    connect(&m_zip_watcher, &QFutureWatcher<ZipResult>::finished, this, &ExtractZipTask::finish);
+    m_zip_watcher.setFuture(m_zip_future);
+}
+
+auto ExtractZipTask::extractZip() -> ZipResult
+{
+    auto target = m_output_dir.absolutePath();
+    auto target_top_dir = QUrl::fromLocalFile(target);
+
+    QStringList extracted;
+
+    qDebug() << "Extracting subdir" << m_subdirectory << "from" << m_input->getZipName() << "to" << target;
+    auto numEntries = m_input->getEntriesCount();
+    if (numEntries < 0) {
+        return ZipResult(tr("Failed to enumerate files in archive"));
+    }
+    if (numEntries == 0) {
+        logWarning(tr("Extracting empty archives seems odd..."));
+        return ZipResult();
+    }
+    if (!m_input->goToFirstFile()) {
+        return ZipResult(tr("Failed to seek to first file in zip"));
+    }
+
+    setStatus("Extracting files...");
+    setProgress(0, numEntries);
+    do {
+        if (m_zip_future.isCanceled())
+            return ZipResult();
+        setProgress(m_progress + 1, m_progressTotal);
+        QString file_name = m_input->getCurrentFileName();
+        if (!file_name.startsWith(m_subdirectory))
+            continue;
+
+        auto relative_file_name = QDir::fromNativeSeparators(file_name.remove(0, m_subdirectory.size()));
+        auto original_name = relative_file_name;
+        setStatus("Unziping: " + relative_file_name);
+
+        // Fix subdirs/files ending with a / getting transformed into absolute paths
+        if (relative_file_name.startsWith('/'))
+            relative_file_name = relative_file_name.mid(1);
+
+        // Fix weird "folders with a single file get squashed" thing
+        QString sub_path;
+        if (relative_file_name.contains('/') && !relative_file_name.endsWith('/')) {
+            sub_path = relative_file_name.section('/', 0, -2) + '/';
+            FS::ensureFolderPathExists(FS::PathCombine(target, sub_path));
+
+            relative_file_name = relative_file_name.split('/').last();
+        }
+
+        QString target_file_path;
+        if (relative_file_name.isEmpty()) {
+            target_file_path = target + '/';
+        } else {
+            target_file_path = FS::PathCombine(target_top_dir.toLocalFile(), sub_path, relative_file_name);
+            if (relative_file_name.endsWith('/') && !target_file_path.endsWith('/'))
+                target_file_path += '/';
+        }
+
+        if (!target_top_dir.isParentOf(QUrl::fromLocalFile(target_file_path))) {
+            return ZipResult(tr("Extracting %1 was cancelled, because it was effectively outside of the target path %2")
+                                 .arg(relative_file_name, target));
+        }
+
+        if (!JlCompress::extractFile(m_input.get(), "", target_file_path)) {
+            JlCompress::removeFile(extracted);
+            return ZipResult(tr("Failed to extract file %1 to %2").arg(original_name, target_file_path));
+        }
+
+        extracted.append(target_file_path);
+        QFile::setPermissions(target_file_path,
+                              QFileDevice::Permission::ReadUser | QFileDevice::Permission::WriteUser | QFileDevice::Permission::ExeUser);
+
+        qDebug() << "Extracted file" << relative_file_name << "to" << target_file_path;
+    } while (m_input->goToNextFile());
+
+    return ZipResult();
+}
+
+void ExtractZipTask::finish()
+{
+    if (m_zip_future.isCanceled()) {
+        emitAborted();
+    } else if (auto result = m_zip_future.result(); result.has_value()) {
+        emitFailed(result.value());
+    } else {
+        emitSucceeded();
+    }
+}
+
+bool ExtractZipTask::abort()
+{
+    if (m_zip_future.isRunning()) {
+        m_zip_future.cancel();
+        // NOTE: Here we don't do `emitAborted()` because it will be done when `m_build_zip_future` actually cancels, which may not occur
+        // immediately.
+        return true;
+    }
+    return false;
+}
+
 }  // namespace MMCZip

launcher/MMCZip.h

@@ -189,4 +189,29 @@ class ExportToZipTask : public Task {
     QFuture<ZipResult> m_build_zip_future;
     QFutureWatcher<ZipResult> m_build_zip_watcher;
 };
+
+class ExtractZipTask : public Task {
+   public:
+    ExtractZipTask(std::shared_ptr<QuaZip> input, QDir outputDir, QString subdirectory = "")
+        : m_input(input), m_output_dir(outputDir), m_subdirectory(subdirectory)
+    {}
+    virtual ~ExtractZipTask() = default;
+
+    typedef std::optional<QString> ZipResult;
+
+   protected:
+    virtual void executeTask() override;
+    bool abort() override;
+
+    ZipResult extractZip();
+    void finish();
+
+   private:
+    std::shared_ptr<QuaZip> m_input;
+    QDir m_output_dir;
+    QString m_subdirectory;
+
+    QFuture<ZipResult> m_zip_future;
+    QFutureWatcher<ZipResult> m_zip_watcher;
+};
 }  // namespace MMCZip

launcher/ui/pages/modplatform/ImportPage.cpp

@@ -38,6 +38,7 @@
 #include "ui_ImportPage.h"
 
 #include <QFileDialog>
+#include <QMimeDatabase>
 #include <QValidator>
 
 #include "ui/dialogs/NewInstanceDialog.h"