From ec9ddc4f225c89ea3ccbf24c9a3be36848aa3172 Mon Sep 17 00:00:00 2001 From: flow Date: Sun, 31 Jul 2022 19:48:38 -0300 Subject: chore: add helper function for copying managed pack data between insts. Signed-off-by: flow --- launcher/BaseInstance.cpp | 10 ++++++++++ launcher/BaseInstance.h | 1 + 2 files changed, 11 insertions(+) diff --git a/launcher/BaseInstance.cpp b/launcher/BaseInstance.cpp index e6d4d8e3..3b261678 100644 --- a/launcher/BaseInstance.cpp +++ b/launcher/BaseInstance.cpp @@ -154,6 +154,16 @@ void BaseInstance::setManagedPack(const QString& type, const QString& id, const settings()->set("ManagedPackVersionName", version); } +void BaseInstance::copyManagedPack(BaseInstance& other) +{ + settings()->set("ManagedPack", other.isManagedPack()); + settings()->set("ManagedPackType", other.getManagedPackType()); + settings()->set("ManagedPackID", other.getManagedPackID()); + settings()->set("ManagedPackName", other.getManagedPackName()); + settings()->set("ManagedPackVersionID", other.getManagedPackVersionID()); + settings()->set("ManagedPackVersionName", other.getManagedPackVersionName()); +} + int BaseInstance::getConsoleMaxLines() const { auto lineSetting = m_settings->getSetting("ConsoleMaxLines"); diff --git a/launcher/BaseInstance.h b/launcher/BaseInstance.h index 3af104e9..653d1378 100644 --- a/launcher/BaseInstance.h +++ b/launcher/BaseInstance.h @@ -147,6 +147,7 @@ public: QString getManagedPackVersionID(); QString getManagedPackVersionName(); void setManagedPack(const QString& type, const QString& id, const QString& name, const QString& versionId, const QString& version); + void copyManagedPack(BaseInstance& other); /// guess log level from a line of game log virtual MessageLevel::Enum guessLevel(const QString &line, MessageLevel::Enum level) -- cgit From 941d75824af8d8e3deb1dfc0597c9493911f0cf0 Mon Sep 17 00:00:00 2001 From: flow Date: Thu, 7 Jul 2022 20:31:24 -0300 Subject: refactor: add instance creation abstraction and move vanilla This is so that 1. Code is more cleanly separated, and 2. Allows to more easily add instance updating :) Signed-off-by: flow --- launcher/CMakeLists.txt | 2 + launcher/InstanceCreationTask.cpp | 42 +++++-------------- launcher/InstanceCreationTask.h | 47 ++++++++++++++-------- launcher/minecraft/VanillaInstanceCreationTask.cpp | 32 +++++++++++++++ launcher/minecraft/VanillaInstanceCreationTask.h | 20 +++++++++ launcher/ui/pages/modplatform/VanillaPage.cpp | 10 ++--- 6 files changed, 99 insertions(+), 54 deletions(-) create mode 100644 launcher/minecraft/VanillaInstanceCreationTask.cpp create mode 100644 launcher/minecraft/VanillaInstanceCreationTask.h diff --git a/launcher/CMakeLists.txt b/launcher/CMakeLists.txt index 848d2e51..c51cd6bd 100644 --- a/launcher/CMakeLists.txt +++ b/launcher/CMakeLists.txt @@ -297,6 +297,8 @@ set(MINECRAFT_SOURCES minecraft/Library.cpp minecraft/Library.h minecraft/MojangDownloadInfo.h + minecraft/VanillaInstanceCreationTask.cpp + minecraft/VanillaInstanceCreationTask.h minecraft/VersionFile.cpp minecraft/VersionFile.h minecraft/VersionFilterData.h diff --git a/launcher/InstanceCreationTask.cpp b/launcher/InstanceCreationTask.cpp index e01bf306..c8c91997 100644 --- a/launcher/InstanceCreationTask.cpp +++ b/launcher/InstanceCreationTask.cpp @@ -1,40 +1,18 @@ #include "InstanceCreationTask.h" -#include "settings/INISettingsObject.h" -#include "FileSystem.h" -//FIXME: remove this -#include "minecraft/MinecraftInstance.h" -#include "minecraft/PackProfile.h" +#include -InstanceCreationTask::InstanceCreationTask(BaseVersionPtr version) -{ - m_version = version; - m_usingLoader = false; -} - -InstanceCreationTask::InstanceCreationTask(BaseVersionPtr version, QString loader, BaseVersionPtr loaderVersion) -{ - m_version = version; - m_usingLoader = true; - m_loader = loader; - m_loaderVersion = loaderVersion; -} +InstanceCreationTask::InstanceCreationTask() {} void InstanceCreationTask::executeTask() { - setStatus(tr("Creating instance from version %1").arg(m_version->name())); - { - auto instanceSettings = std::make_shared(FS::PathCombine(m_stagingPath, "instance.cfg")); - instanceSettings->suspendSave(); - MinecraftInstance inst(m_globalSettings, instanceSettings, m_stagingPath); - auto components = inst.getPackProfile(); - components->buildingFromScratch(); - components->setComponentVersion("net.minecraft", m_version->descriptor(), true); - if(m_usingLoader) - components->setComponentVersion(m_loader, m_loaderVersion->descriptor()); - inst.setName(m_instName); - inst.setIconKey(m_instIcon); - instanceSettings->resumeSave(); + if (updateInstance() || createInstance()) { + emitSucceeded(); + return; } - emitSucceeded(); + + qWarning() << "Instance creation failed!"; + if (!m_error_message.isEmpty()) + qWarning() << "Reason: " << m_error_message; + emitFailed(tr("Error while creating new instance.")); } diff --git a/launcher/InstanceCreationTask.h b/launcher/InstanceCreationTask.h index 23367c3f..af854713 100644 --- a/launcher/InstanceCreationTask.h +++ b/launcher/InstanceCreationTask.h @@ -1,26 +1,39 @@ #pragma once -#include "tasks/Task.h" -#include "net/NetJob.h" -#include -#include "settings/SettingsObject.h" #include "BaseVersion.h" #include "InstanceTask.h" -class InstanceCreationTask : public InstanceTask -{ +class InstanceCreationTask : public InstanceTask { Q_OBJECT -public: - explicit InstanceCreationTask(BaseVersionPtr version); - explicit InstanceCreationTask(BaseVersionPtr version, QString loader, BaseVersionPtr loaderVersion); + public: + InstanceCreationTask(); + virtual ~InstanceCreationTask() = default; -protected: - //! Entry point for tasks. - virtual void executeTask() override; + protected: + void executeTask() final override; -private: /* data */ - BaseVersionPtr m_version; - bool m_usingLoader; - QString m_loader; - BaseVersionPtr m_loaderVersion; + /** + * Tries to update an already existing instance. + * + * This can be implemented by subclasses to provide a way of updating an already existing + * instance, according to that implementation's concept of 'identity' (i.e. instances that + * are updates / downgrades of one another). + * + * If this returns true, createInstance() will not run, so you should do all update steps in here. + * Otherwise, createInstance() is run as normal. + */ + virtual bool updateInstance() { return false; }; + + /** + * Creates a new instance. + * + * Returns whether the instance creation was successful (true) or not (false). + */ + virtual bool createInstance() { return false; }; + + protected: + void setError(QString message) { m_error_message = message; }; + + private: + QString m_error_message; }; diff --git a/launcher/minecraft/VanillaInstanceCreationTask.cpp b/launcher/minecraft/VanillaInstanceCreationTask.cpp new file mode 100644 index 00000000..2d1593b6 --- /dev/null +++ b/launcher/minecraft/VanillaInstanceCreationTask.cpp @@ -0,0 +1,32 @@ +#include "VanillaInstanceCreationTask.h" + +#include "FileSystem.h" +#include "settings/INISettingsObject.h" +#include "minecraft/MinecraftInstance.h" +#include "minecraft/PackProfile.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) +{} + +bool VanillaCreationTask::createInstance() +{ + setStatus(tr("Creating instance from version %1").arg(m_version->name())); + + auto instance_settings = std::make_shared(FS::PathCombine(m_stagingPath, "instance.cfg")); + instance_settings->suspendSave(); + { + MinecraftInstance inst(m_globalSettings, instance_settings, m_stagingPath); + auto components = inst.getPackProfile(); + components->buildingFromScratch(); + components->setComponentVersion("net.minecraft", m_version->descriptor(), true); + if(m_using_loader) + components->setComponentVersion(m_loader, m_loader_version->descriptor()); + + inst.setName(m_instName); + inst.setIconKey(m_instIcon); + } + instance_settings->resumeSave(); + + return true; +} diff --git a/launcher/minecraft/VanillaInstanceCreationTask.h b/launcher/minecraft/VanillaInstanceCreationTask.h new file mode 100644 index 00000000..540ecb70 --- /dev/null +++ b/launcher/minecraft/VanillaInstanceCreationTask.h @@ -0,0 +1,20 @@ +#pragma once + +#include "InstanceCreationTask.h" + +class VanillaCreationTask final : public InstanceCreationTask { + Q_OBJECT + public: + VanillaCreationTask(BaseVersionPtr version) : InstanceCreationTask(), m_version(version) {} + VanillaCreationTask(BaseVersionPtr version, QString loader, BaseVersionPtr loader_version); + + bool createInstance() override; + + private: + // Version to update to / create of the instance. + BaseVersionPtr m_version; + + bool m_using_loader = false; + QString m_loader; + BaseVersionPtr m_loader_version; +}; diff --git a/launcher/ui/pages/modplatform/VanillaPage.cpp b/launcher/ui/pages/modplatform/VanillaPage.cpp index a026947f..99190f31 100644 --- a/launcher/ui/pages/modplatform/VanillaPage.cpp +++ b/launcher/ui/pages/modplatform/VanillaPage.cpp @@ -39,12 +39,12 @@ #include #include "Application.h" +#include "Filter.h" +#include "Version.h" #include "meta/Index.h" #include "meta/VersionList.h" +#include "minecraft/VanillaInstanceCreationTask.h" #include "ui/dialogs/NewInstanceDialog.h" -#include "Filter.h" -#include "InstanceCreationTask.h" -#include "Version.h" VanillaPage::VanillaPage(NewInstanceDialog *dialog, QWidget *parent) : QWidget(parent), dialog(dialog), ui(new Ui::VanillaPage) @@ -217,11 +217,11 @@ void VanillaPage::suggestCurrent() // There isn't a selected version if the version list is empty if(ui->loaderVersionList->selectedVersion() == nullptr) - dialog->setSuggestedPack(m_selectedVersion->descriptor(), new InstanceCreationTask(m_selectedVersion)); + dialog->setSuggestedPack(m_selectedVersion->descriptor(), new VanillaCreationTask(m_selectedVersion)); else { dialog->setSuggestedPack(m_selectedVersion->descriptor(), - new InstanceCreationTask(m_selectedVersion, m_selectedLoader, + new VanillaCreationTask(m_selectedVersion, m_selectedLoader, m_selectedLoaderVersion)); } dialog->setSuggestedIcon("default"); -- cgit From 4441b373385f9b7f77deed2a27751337951f38f6 Mon Sep 17 00:00:00 2001 From: flow Date: Thu, 7 Jul 2022 21:10:41 -0300 Subject: refactor: move modrinth modpack import to separate file Signed-off-by: flow --- launcher/CMakeLists.txt | 2 + launcher/InstanceImportTask.cpp | 206 ++----------------- .../modrinth/ModrinthInstanceCreationTask.cpp | 225 +++++++++++++++++++++ .../modrinth/ModrinthInstanceCreationTask.h | 39 ++++ 4 files changed, 282 insertions(+), 190 deletions(-) create mode 100644 launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp create mode 100644 launcher/modplatform/modrinth/ModrinthInstanceCreationTask.h diff --git a/launcher/CMakeLists.txt b/launcher/CMakeLists.txt index c51cd6bd..68439ee8 100644 --- a/launcher/CMakeLists.txt +++ b/launcher/CMakeLists.txt @@ -495,6 +495,8 @@ set(MODRINTH_SOURCES modplatform/modrinth/ModrinthPackManifest.h modplatform/modrinth/ModrinthCheckUpdate.cpp modplatform/modrinth/ModrinthCheckUpdate.h + modplatform/modrinth/ModrinthInstanceCreationTask.cpp + modplatform/modrinth/ModrinthInstanceCreationTask.h ) set(MODPACKSCH_SOURCES diff --git a/launcher/InstanceImportTask.cpp b/launcher/InstanceImportTask.cpp index de0afc96..b19b5fa6 100644 --- a/launcher/InstanceImportTask.cpp +++ b/launcher/InstanceImportTask.cpp @@ -52,8 +52,8 @@ #include "minecraft/PackProfile.h" #include "modplatform/flame/FileResolvingTask.h" #include "modplatform/flame/PackManifest.h" -#include "modplatform/modrinth/ModrinthPackManifest.h" #include "modplatform/technic/TechnicPackProcessor.h" +#include "modplatform/modrinth/ModrinthInstanceCreationTask.h" #include "Application.h" #include "icons/IconList.h" @@ -256,6 +256,7 @@ void InstanceImportTask::extractFinished() void InstanceImportTask::extractAborted() { emitFailed(tr("Instance import has been aborted.")); + emit aborted(); return; } @@ -584,198 +585,23 @@ void InstanceImportTask::processMultiMC() emitSucceeded(); } -// https://docs.modrinth.com/docs/modpacks/format_definition/ void InstanceImportTask::processModrinth() { - std::vector files; - QString minecraftVersion, fabricVersion, quiltVersion, forgeVersion; - 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); - } - - auto jsonFiles = Json::requireIsArrayOf(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); - } - } - - files.push_back(file); - } + auto* inst_creation_task = new ModrinthCreationTask(m_stagingPath, m_globalSettings, m_parent, m_sourceUrl.toString()); - 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) { - emitFailed(tr("Could not understand pack index:\n") + e.cause()); - return; - } + inst_creation_task->setName(m_instName); + inst_creation_task->setIcon(m_instIcon); + inst_creation_task->setGroup(m_instGroup); - 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)) { - emitFailed(tr("Could not rename the overrides folder:\n") + "overrides"); - return; - } - } - - // 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)) { - emitFailed(tr("Could not rename the client overrides folder:\n") + "client overrides"); - return; - } - } - - QString configPath = FS::PathCombine(m_stagingPath, "instance.cfg"); - auto instanceSettings = std::make_shared(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.saveNow(); - - m_filesNetJob = new NetJob(tr("Mod download"), APPLICATION->network()); - for (auto file : 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_filesNetJob->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_filesNetJob->addNetAction(dl); - dl->succeeded(); - }); - } - } - connect(m_filesNetJob.get(), &NetJob::succeeded, this, [&]() - { - m_filesNetJob.reset(); - emitSucceeded(); - } - ); - connect(m_filesNetJob.get(), &NetJob::failed, [&](const QString &reason) - { - m_filesNetJob.reset(); - emitFailed(reason); + connect(inst_creation_task, &Task::succeeded, this, &InstanceImportTask::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(this, &Task::aborted, inst_creation_task, [inst_creation_task] { + inst_creation_task->abort(); }); - connect(m_filesNetJob.get(), &NetJob::progress, [&](qint64 current, qint64 total) - { - setProgress(current, total); - }); - setStatus(tr("Downloading mods...")); - m_filesNetJob->start(); + + inst_creation_task->start(); } 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(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(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 {}; +} diff --git a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.h b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.h new file mode 100644 index 00000000..61f7dd5c --- /dev/null +++ b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.h @@ -0,0 +1,39 @@ +#pragma once + +#include "InstanceCreationTask.h" + +#include "modplatform/modrinth/ModrinthPackManifest.h" + +#include "net/NetJob.h" + +class ModrinthCreationTask final : public InstanceCreationTask { + Q_OBJECT + + public: + ModrinthCreationTask(QString staging_path, SettingsObjectPtr global_settings, QWidget* parent, QString source_url = {}) + : InstanceCreationTask(), m_parent(parent) + { + setStagingPath(staging_path); + setParentSettings(global_settings); + } + + bool abort() override; + bool canAbort() const override { return true; } + + bool updateInstance() override; + bool createInstance() override; + + private: + bool parseManifest(); + QString getManagedPackID() const; + + private: + QWidget* m_parent = nullptr; + + QString minecraftVersion, fabricVersion, quiltVersion, forgeVersion; + QString m_managed_id, m_managed_version_id, m_managed_name; + QString m_source_url; + + std::vector m_files; + NetJob::Ptr m_files_job; +}; -- cgit From 208ed73e59c46ee7966f463558c07805a9b541e6 Mon Sep 17 00:00:00 2001 From: flow 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 --- 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(-) 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 m_child; + InstanceList * m_parent; + unique_qobject_ptr 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 + +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 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(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& 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&); QString getManagedPackID() const; private: -- cgit From 242fb156a281ef188c9fd75969c5d70ba6f8c140 Mon Sep 17 00:00:00 2001 From: flow Date: Thu, 21 Jul 2022 16:40:28 -0300 Subject: feat: add 'getFiles' by fileIds route in Flame API Signed-off-by: flow --- launcher/modplatform/flame/FlameAPI.cpp | 23 +++++++++++++++++++++++ launcher/modplatform/flame/FlameAPI.h | 1 + 2 files changed, 24 insertions(+) diff --git a/launcher/modplatform/flame/FlameAPI.cpp b/launcher/modplatform/flame/FlameAPI.cpp index 9c74918b..f8f50dc6 100644 --- a/launcher/modplatform/flame/FlameAPI.cpp +++ b/launcher/modplatform/flame/FlameAPI.cpp @@ -183,3 +183,26 @@ auto FlameAPI::getProjects(QStringList addonIds, QByteArray* response) const -> return netJob; } + +auto FlameAPI::getFiles(QStringList fileIds, QByteArray* response) const -> NetJob* +{ + auto* netJob = new NetJob(QString("Flame::GetFiles"), APPLICATION->network()); + + QJsonObject body_obj; + QJsonArray files_arr; + for (auto& fileId : fileIds) { + files_arr.append(fileId); + } + + body_obj["fileIds"] = files_arr; + + QJsonDocument body(body_obj); + auto body_raw = body.toJson(); + + netJob->addNetAction(Net::Upload::makeByteArray(QString("https://api.curseforge.com/v1/mods/files"), response, body_raw)); + + QObject::connect(netJob, &NetJob::finished, [response, netJob] { delete response; netJob->deleteLater(); }); + QObject::connect(netJob, &NetJob::failed, [body_raw] { qDebug() << body_raw; }); + + return netJob; +} diff --git a/launcher/modplatform/flame/FlameAPI.h b/launcher/modplatform/flame/FlameAPI.h index 4eac0664..5e6166b9 100644 --- a/launcher/modplatform/flame/FlameAPI.h +++ b/launcher/modplatform/flame/FlameAPI.h @@ -12,6 +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*; private: inline auto getSortFieldInt(QString sortString) const -> int -- cgit From 2246c3359bcd07d8bfab949d79a1e428a948f1b7 Mon Sep 17 00:00:00 2001 From: flow Date: Thu, 21 Jul 2022 16:41:44 -0300 Subject: refactor: add `throw_on_blocked` arg to Flame file parse Signed-off-by: flow --- launcher/modplatform/flame/PackManifest.cpp | 4 ++-- launcher/modplatform/flame/PackManifest.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/launcher/modplatform/flame/PackManifest.cpp b/launcher/modplatform/flame/PackManifest.cpp index 12a4b990..81395fcd 100644 --- a/launcher/modplatform/flame/PackManifest.cpp +++ b/launcher/modplatform/flame/PackManifest.cpp @@ -61,7 +61,7 @@ void Flame::loadManifest(Flame::Manifest& m, const QString& filepath) loadManifestV1(m, obj); } -bool Flame::File::parseFromObject(const QJsonObject& obj) +bool Flame::File::parseFromObject(const QJsonObject& obj, bool throw_on_blocked) { fileName = Json::requireString(obj, "fileName"); // This is a piece of a Flame project JSON pulled out into the file metadata (here) for convenience @@ -91,7 +91,7 @@ bool Flame::File::parseFromObject(const QJsonObject& obj) // may throw, if the project is blocked QString rawUrl = Json::ensureString(obj, "downloadUrl"); url = QUrl(rawUrl, QUrl::TolerantMode); - if (!url.isValid()) { + if (!url.isValid() && throw_on_blocked) { throw JSONValidationError(QString("Invalid URL: %1").arg(rawUrl)); } diff --git a/launcher/modplatform/flame/PackManifest.h b/launcher/modplatform/flame/PackManifest.h index 677db1c3..a69e1321 100644 --- a/launcher/modplatform/flame/PackManifest.h +++ b/launcher/modplatform/flame/PackManifest.h @@ -46,7 +46,7 @@ namespace Flame struct File { // NOTE: throws JSONValidationError - bool parseFromObject(const QJsonObject& object); + bool parseFromObject(const QJsonObject& object, bool throw_on_blocked = true); int projectId = 0; int fileId = 0; -- cgit From 72d2ca234e80fe65bb6a7d5fe106b01d9dc6f096 Mon Sep 17 00:00:00 2001 From: flow Date: Fri, 8 Jul 2022 18:44:43 -0300 Subject: refactor: move flame modpack import to separate file Signed-off-by: flow --- launcher/CMakeLists.txt | 2 + launcher/InstanceCreationTask.h | 2 + launcher/InstanceImportTask.cpp | 298 ++------------------- .../flame/FlameInstanceCreationTask.cpp | 284 ++++++++++++++++++++ .../modplatform/flame/FlameInstanceCreationTask.h | 32 +++ 5 files changed, 337 insertions(+), 281 deletions(-) create mode 100644 launcher/modplatform/flame/FlameInstanceCreationTask.cpp create mode 100644 launcher/modplatform/flame/FlameInstanceCreationTask.h diff --git a/launcher/CMakeLists.txt b/launcher/CMakeLists.txt index 68439ee8..7bd92fac 100644 --- a/launcher/CMakeLists.txt +++ b/launcher/CMakeLists.txt @@ -486,6 +486,8 @@ set(FLAME_SOURCES modplatform/flame/FileResolvingTask.cpp modplatform/flame/FlameCheckUpdate.cpp modplatform/flame/FlameCheckUpdate.h + modplatform/flame/FlameInstanceCreationTask.h + modplatform/flame/FlameInstanceCreationTask.cpp ) set(MODRINTH_SOURCES diff --git a/launcher/InstanceCreationTask.h b/launcher/InstanceCreationTask.h index af854713..68c5de59 100644 --- a/launcher/InstanceCreationTask.h +++ b/launcher/InstanceCreationTask.h @@ -31,6 +31,8 @@ class InstanceCreationTask : public InstanceTask { */ virtual bool createInstance() { return false; }; + QString getError() const { return m_error_message; } + protected: void setError(QString message) { m_error_message = message; }; diff --git a/launcher/InstanceImportTask.cpp b/launcher/InstanceImportTask.cpp index 4bdf9cd2..72c2496f 100644 --- a/launcher/InstanceImportTask.cpp +++ b/launcher/InstanceImportTask.cpp @@ -50,10 +50,9 @@ #include "Json.h" #include "minecraft/MinecraftInstance.h" #include "minecraft/PackProfile.h" -#include "modplatform/flame/FileResolvingTask.h" -#include "modplatform/flame/PackManifest.h" #include "modplatform/technic/TechnicPackProcessor.h" #include "modplatform/modrinth/ModrinthInstanceCreationTask.h" +#include "modplatform/flame/FlameInstanceCreationTask.h" #include "Application.h" #include "icons/IconList.h" @@ -262,287 +261,24 @@ void InstanceImportTask::extractAborted() void InstanceImportTask::processFlame() { - const static QMap forgemap = { - {"1.2.5", "3.4.9.171"}, - {"1.4.2", "6.0.1.355"}, - {"1.4.7", "6.6.2.534"}, - {"1.5.2", "7.8.1.737"} - }; - Flame::Manifest pack; - try - { - QString configPath = FS::PathCombine(m_stagingPath, "manifest.json"); - Flame::loadManifest(pack, configPath); - QFile::remove(configPath); - } - catch (const JSONValidationError &e) - { - emitFailed(tr("Could not understand pack manifest:\n") + e.cause()); - return; - } - if(!pack.overrides.isEmpty()) - { - QString overridePath = FS::PathCombine(m_stagingPath, pack.overrides); - if (QFile::exists(overridePath)) - { - QString mcPath = FS::PathCombine(m_stagingPath, "minecraft"); - if (!QFile::rename(overridePath, mcPath)) - { - emitFailed(tr("Could not rename the overrides folder:\n") + pack.overrides); - return; - } - } - else - { - logWarning(tr("The specified overrides folder (%1) is missing. Maybe the modpack was already used before?").arg(pack.overrides)); - } - } - - QString forgeVersion; - QString fabricVersion; - // TODO: is Quilt relevant here? - for(auto &loader: pack.minecraft.modLoaders) - { - auto id = loader.id; - if(id.startsWith("forge-")) - { - id.remove("forge-"); - forgeVersion = id; - continue; - } - if(id.startsWith("fabric-")) - { - id.remove("fabric-"); - fabricVersion = id; - continue; - } - logWarning(tr("Unknown mod loader in manifest: %1").arg(id)); - } + auto* inst_creation_task = new FlameCreationTask(m_stagingPath, m_globalSettings, m_parent); - QString configPath = FS::PathCombine(m_stagingPath, "instance.cfg"); - auto instanceSettings = std::make_shared(configPath); - MinecraftInstance instance(m_globalSettings, instanceSettings, m_stagingPath); - auto mcVersion = pack.minecraft.version; - // Hack to correct some 'special sauce'... - if(mcVersion.endsWith('.')) - { - mcVersion.remove(QRegularExpression("[.]+$")); - logWarning(tr("Mysterious trailing dots removed from Minecraft version while importing pack.")); - } - auto components = instance.getPackProfile(); - components->buildingFromScratch(); - components->setComponentVersion("net.minecraft", mcVersion, true); - if(!forgeVersion.isEmpty()) - { - // FIXME: dirty, nasty, hack. Proper solution requires dependency resolution and knowledge of the metadata. - if(forgeVersion == "recommended") - { - if(forgemap.contains(mcVersion)) - { - forgeVersion = forgemap[mcVersion]; - } - else - { - logWarning(tr("Could not map recommended Forge version for Minecraft %1").arg(mcVersion)); - } - } - components->setComponentVersion("net.minecraftforge", forgeVersion); - } - if(!fabricVersion.isEmpty()) - { - components->setComponentVersion("net.fabricmc.fabric-loader", fabricVersion); - } - if (m_instIcon != "default") - { - instance.setIconKey(m_instIcon); - } - else - { - if(pack.name.contains("Direwolf20")) - { - instance.setIconKey("steve"); - } - else if(pack.name.contains("FTB") || pack.name.contains("Feed The Beast")) - { - instance.setIconKey("ftb_logo"); - } - else - { - // default to something other than the MultiMC default to distinguish these - instance.setIconKey("flame"); - } - } - QString jarmodsPath = FS::PathCombine(m_stagingPath, "minecraft", "jarmods"); - QFileInfo jarmodsInfo(jarmodsPath); - if(jarmodsInfo.isDir()) - { - // install all the jar mods - qDebug() << "Found jarmods:"; - QDir jarmodsDir(jarmodsPath); - QStringList jarMods; - for (auto info: jarmodsDir.entryInfoList(QDir::NoDotAndDotDot | QDir::Files)) - { - qDebug() << info.fileName(); - jarMods.push_back(info.absoluteFilePath()); - } - auto profile = instance.getPackProfile(); - profile->installJarMods(jarMods); - // nuke the original files - FS::deletePath(jarmodsPath); - } - instance.setName(m_instName); - m_modIdResolver = new Flame::FileResolvingTask(APPLICATION->network(), pack); - connect(m_modIdResolver.get(), &Flame::FileResolvingTask::succeeded, [&]() - { - auto results = m_modIdResolver->getResults(); - //first check for blocked mods - QString text; - QList urls; - auto anyBlocked = false; - for(const auto& result: results.files.values()) { - if (!result.resolved || result.url.isEmpty()) { - text += QString("%1: %2
").arg(result.fileName, result.websiteUrl); - urls.append(QUrl(result.websiteUrl)); - anyBlocked = true; - } - } - if(anyBlocked) { - qWarning() << "Blocked mods found, displaying mod list"; - - auto message_dialog = new BlockedModsDialog(m_parent, - tr("Blocked mods found"), - tr("The following mods were blocked on third party launchers.
" - "You will need to manually download them and add them to the modpack"), - text, - urls); - message_dialog->setModal(true); - - if (message_dialog->exec()) { - m_filesNetJob = new NetJob(tr("Mod download"), APPLICATION->network()); - for (const auto &result: m_modIdResolver->getResults().files) { - QString filename = result.fileName; - if (!result.required) { - filename += ".disabled"; - } - - auto relpath = FS::PathCombine("minecraft", result.targetFolder, filename); - auto path = FS::PathCombine(m_stagingPath, relpath); - - switch (result.type) { - case Flame::File::Type::Folder: { - logWarning(tr("This 'Folder' may need extracting: %1").arg(relpath)); - // fall-through intentional, we treat these as plain old mods and dump them wherever. - } - case Flame::File::Type::SingleFile: - case Flame::File::Type::Mod: { - if (!result.url.isEmpty()) { - qDebug() << "Will download" << result.url << "to" << path; - auto dl = Net::Download::makeFile(result.url, path); - m_filesNetJob->addNetAction(dl); - } - break; - } - case Flame::File::Type::Modpack: - logWarning( - tr("Nesting modpacks in modpacks is not implemented, nothing was downloaded: %1").arg( - relpath)); - break; - case Flame::File::Type::Cmod2: - case Flame::File::Type::Ctoc: - case Flame::File::Type::Unknown: - logWarning(tr("Unrecognized/unhandled PackageType for: %1").arg(relpath)); - break; - } - } - m_modIdResolver.reset(); - connect(m_filesNetJob.get(), &NetJob::succeeded, this, [&]() { - m_filesNetJob.reset(); - emitSucceeded(); - } - ); - connect(m_filesNetJob.get(), &NetJob::failed, [&](QString reason) { - m_filesNetJob.reset(); - emitFailed(reason); - }); - connect(m_filesNetJob.get(), &NetJob::progress, [&](qint64 current, qint64 total) { - setProgress(current, total); - }); - setStatus(tr("Downloading mods...")); - m_filesNetJob->start(); - } else { - m_modIdResolver.reset(); - emitFailed("Canceled"); - } - } else { - //TODO extract to function ? - m_filesNetJob = new NetJob(tr("Mod download"), APPLICATION->network()); - for (const auto &result: m_modIdResolver->getResults().files) { - QString filename = result.fileName; - if (!result.required) { - filename += ".disabled"; - } - - auto relpath = FS::PathCombine("minecraft", result.targetFolder, filename); - auto path = FS::PathCombine(m_stagingPath, relpath); - - switch (result.type) { - case Flame::File::Type::Folder: { - logWarning(tr("This 'Folder' may need extracting: %1").arg(relpath)); - // fall-through intentional, we treat these as plain old mods and dump them wherever. - } - case Flame::File::Type::SingleFile: - case Flame::File::Type::Mod: { - if (!result.url.isEmpty()) { - qDebug() << "Will download" << result.url << "to" << path; - auto dl = Net::Download::makeFile(result.url, path); - m_filesNetJob->addNetAction(dl); - } - break; - } - case Flame::File::Type::Modpack: - logWarning( - tr("Nesting modpacks in modpacks is not implemented, nothing was downloaded: %1").arg( - relpath)); - break; - case Flame::File::Type::Cmod2: - case Flame::File::Type::Ctoc: - case Flame::File::Type::Unknown: - logWarning(tr("Unrecognized/unhandled PackageType for: %1").arg(relpath)); - break; - } - } - m_modIdResolver.reset(); - connect(m_filesNetJob.get(), &NetJob::succeeded, this, [&]() { - m_filesNetJob.reset(); - emitSucceeded(); - } - ); - connect(m_filesNetJob.get(), &NetJob::failed, [&](QString reason) { - m_filesNetJob.reset(); - emitFailed(reason); - }); - connect(m_filesNetJob.get(), &NetJob::progress, [&](qint64 current, qint64 total) { - setProgress(current, total); - }); - setStatus(tr("Downloading mods...")); - m_filesNetJob->start(); - } - } - ); - connect(m_modIdResolver.get(), &Flame::FileResolvingTask::failed, [&](QString reason) - { - m_modIdResolver.reset(); - emitFailed(tr("Unable to resolve mod IDs:\n") + reason); - }); - connect(m_modIdResolver.get(), &Flame::FileResolvingTask::progress, [&](qint64 current, qint64 total) - { - setProgress(current, total); - }); - connect(m_modIdResolver.get(), &Flame::FileResolvingTask::status, [&](QString status) - { - setStatus(status); + inst_creation_task->setName(m_instName); + inst_creation_task->setIcon(m_instIcon); + inst_creation_task->setGroup(m_instGroup); + + connect(inst_creation_task, &Task::succeeded, this, [this, inst_creation_task] { + setOverride(inst_creation_task->shouldOverride()); + emitSucceeded(); }); - m_modIdResolver->start(); + 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, inst_creation_task, &InstanceCreationTask::deleteLater); + + connect(this, &Task::aborted, inst_creation_task, &InstanceCreationTask::abort); + + inst_creation_task->start(); } void InstanceImportTask::processTechnic() diff --git a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp new file mode 100644 index 00000000..431c8b3d --- /dev/null +++ b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp @@ -0,0 +1,284 @@ +#include "FlameInstanceCreationTask.h" + +#include "modplatform/flame/PackManifest.h" + +#include "Application.h" +#include "FileSystem.h" +#include "Json.h" + +#include "minecraft/MinecraftInstance.h" +#include "minecraft/PackProfile.h" + +#include "settings/INISettingsObject.h" + +#include "ui/dialogs/BlockedModsDialog.h" + +bool FlameCreationTask::abort() +{ + if (m_files_job) + m_files_job->abort(); + if (m_mod_id_resolver) + m_mod_id_resolver->abort(); + + return true; +} + +const static QMap forgemap = { { "1.2.5", "3.4.9.171" }, + { "1.4.2", "6.0.1.355" }, + { "1.4.7", "6.6.2.534" }, + { "1.5.2", "7.8.1.737" } }; + +bool FlameCreationTask::createInstance() +{ + QEventLoop loop; + + Flame::Manifest pack; + try { + QString configPath = FS::PathCombine(m_stagingPath, "manifest.json"); + Flame::loadManifest(pack, configPath); + QFile::remove(configPath); + } catch (const JSONValidationError& e) { + setError(tr("Could not understand pack manifest:\n") + e.cause()); + return false; + } + + if (!pack.overrides.isEmpty()) { + QString overridePath = FS::PathCombine(m_stagingPath, pack.overrides); + if (QFile::exists(overridePath)) { + QString mcPath = FS::PathCombine(m_stagingPath, "minecraft"); + if (!QFile::rename(overridePath, mcPath)) { + setError(tr("Could not rename the overrides folder:\n") + pack.overrides); + return false; + } + } else { + logWarning( + tr("The specified overrides folder (%1) is missing. Maybe the modpack was already used before?").arg(pack.overrides)); + } + } + + QString forgeVersion; + QString fabricVersion; + // TODO: is Quilt relevant here? + for (auto& loader : pack.minecraft.modLoaders) { + auto id = loader.id; + if (id.startsWith("forge-")) { + id.remove("forge-"); + forgeVersion = id; + continue; + } + if (id.startsWith("fabric-")) { + id.remove("fabric-"); + fabricVersion = id; + continue; + } + logWarning(tr("Unknown mod loader in manifest: %1").arg(id)); + } + + QString configPath = FS::PathCombine(m_stagingPath, "instance.cfg"); + auto instanceSettings = std::make_shared(configPath); + MinecraftInstance instance(m_globalSettings, instanceSettings, m_stagingPath); + auto mcVersion = pack.minecraft.version; + + // Hack to correct some 'special sauce'... + if (mcVersion.endsWith('.')) { + mcVersion.remove(QRegularExpression("[.]+$")); + logWarning(tr("Mysterious trailing dots removed from Minecraft version while importing pack.")); + } + + auto components = instance.getPackProfile(); + components->buildingFromScratch(); + components->setComponentVersion("net.minecraft", mcVersion, true); + if (!forgeVersion.isEmpty()) { + // FIXME: dirty, nasty, hack. Proper solution requires dependency resolution and knowledge of the metadata. + if (forgeVersion == "recommended") { + if (forgemap.contains(mcVersion)) { + forgeVersion = forgemap[mcVersion]; + } else { + logWarning(tr("Could not map recommended Forge version for Minecraft %1").arg(mcVersion)); + } + } + components->setComponentVersion("net.minecraftforge", forgeVersion); + } + if (!fabricVersion.isEmpty()) + components->setComponentVersion("net.fabricmc.fabric-loader", fabricVersion); + + if (m_instIcon != "default") { + instance.setIconKey(m_instIcon); + } else { + if (pack.name.contains("Direwolf20")) { + instance.setIconKey("steve"); + } else if (pack.name.contains("FTB") || pack.name.contains("Feed The Beast")) { + instance.setIconKey("ftb_logo"); + } else { + instance.setIconKey("flame"); + } + } + + QString jarmodsPath = FS::PathCombine(m_stagingPath, "minecraft", "jarmods"); + QFileInfo jarmodsInfo(jarmodsPath); + if (jarmodsInfo.isDir()) { + // install all the jar mods + qDebug() << "Found jarmods:"; + QDir jarmodsDir(jarmodsPath); + QStringList jarMods; + for (auto info : jarmodsDir.entryInfoList(QDir::NoDotAndDotDot | QDir::Files)) { + qDebug() << info.fileName(); + jarMods.push_back(info.absoluteFilePath()); + } + auto profile = instance.getPackProfile(); + profile->installJarMods(jarMods); + // nuke the original files + FS::deletePath(jarmodsPath); + } + + instance.setName(m_instName); + + m_mod_id_resolver = new Flame::FileResolvingTask(APPLICATION->network(), pack); + connect(m_mod_id_resolver.get(), &Flame::FileResolvingTask::succeeded, this, [this, &loop]{ + idResolverSucceeded(loop); + }); + connect(m_mod_id_resolver.get(), &Flame::FileResolvingTask::failed, [&](QString reason) { + m_mod_id_resolver.reset(); + setError(tr("Unable to resolve mod IDs:\n") + reason); + }); + connect(m_mod_id_resolver.get(), &Flame::FileResolvingTask::progress, this, &FlameCreationTask::setProgress); + connect(m_mod_id_resolver.get(), &Flame::FileResolvingTask::status, this, &FlameCreationTask::setStatus); + + m_mod_id_resolver->start(); + + loop.exec(); + + return getError().isEmpty(); +} + +void FlameCreationTask::idResolverSucceeded(QEventLoop& loop) +{ + auto results = m_mod_id_resolver->getResults(); + // first check for blocked mods + QString text; + QList urls; + auto anyBlocked = false; + for (const auto& result : results.files.values()) { + if (!result.resolved || result.url.isEmpty()) { + text += QString("%1: %2
").arg(result.fileName, result.websiteUrl); + urls.append(QUrl(result.websiteUrl)); + anyBlocked = true; + } + } + if (anyBlocked) { + qWarning() << "Blocked mods found, displaying mod list"; + + auto message_dialog = new BlockedModsDialog(m_parent, tr("Blocked mods found"), + tr("The following mods were blocked on third party launchers.
" + "You will need to manually download them and add them to the modpack"), + text, + urls); + message_dialog->setModal(true); + + if (message_dialog->exec()) { + m_files_job = new NetJob(tr("Mod download"), APPLICATION->network()); + for (const auto& result : m_mod_id_resolver->getResults().files) { + QString filename = result.fileName; + if (!result.required) { + filename += ".disabled"; + } + + auto relpath = FS::PathCombine("minecraft", result.targetFolder, filename); + auto path = FS::PathCombine(m_stagingPath, relpath); + + switch (result.type) { + case Flame::File::Type::Folder: { + logWarning(tr("This 'Folder' may need extracting: %1").arg(relpath)); + // fall-through intentional, we treat these as plain old mods and dump them wherever. + } + case Flame::File::Type::SingleFile: + case Flame::File::Type::Mod: { + if (!result.url.isEmpty()) { + qDebug() << "Will download" << result.url << "to" << path; + auto dl = Net::Download::makeFile(result.url, path); + m_files_job->addNetAction(dl); + } + break; + } + case Flame::File::Type::Modpack: + logWarning(tr("Nesting modpacks in modpacks is not implemented, nothing was downloaded: %1").arg(relpath)); + break; + case Flame::File::Type::Cmod2: + case Flame::File::Type::Ctoc: + case Flame::File::Type::Unknown: + logWarning(tr("Unrecognized/unhandled PackageType for: %1").arg(relpath)); + break; + } + } + + 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(); + setError(reason); + }); + connect(m_files_job.get(), &NetJob::progress, [&](qint64 current, qint64 total) { setProgress(current, total); }); + connect(m_files_job.get(), &NetJob::finished, &loop, &QEventLoop::quit); + + setStatus(tr("Downloading mods...")); + m_files_job->start(); + } else { + m_mod_id_resolver.reset(); + setError("Canceled"); + } + } else { + // TODO extract to function ? + m_files_job = new NetJob(tr("Mod download"), APPLICATION->network()); + for (const auto& result : m_mod_id_resolver->getResults().files) { + QString filename = result.fileName; + if (!result.required) { + filename += ".disabled"; + } + + auto relpath = FS::PathCombine("minecraft", result.targetFolder, filename); + auto path = FS::PathCombine(m_stagingPath, relpath); + + switch (result.type) { + case Flame::File::Type::Folder: { + logWarning(tr("This 'Folder' may need extracting: %1").arg(relpath)); + // fall-through intentional, we treat these as plain old mods and dump them wherever. + } + case Flame::File::Type::SingleFile: + case Flame::File::Type::Mod: { + if (!result.url.isEmpty()) { + qDebug() << "Will download" << result.url << "to" << path; + auto dl = Net::Download::makeFile(result.url, path); + m_files_job->addNetAction(dl); + } + break; + } + case Flame::File::Type::Modpack: + logWarning(tr("Nesting modpacks in modpacks is not implemented, nothing was downloaded: %1").arg(relpath)); + break; + case Flame::File::Type::Cmod2: + case Flame::File::Type::Ctoc: + case Flame::File::Type::Unknown: + logWarning(tr("Unrecognized/unhandled PackageType for: %1").arg(relpath)); + break; + } + } + + 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(); + setError(reason); + }); + connect(m_files_job.get(), &NetJob::progress, [&](qint64 current, qint64 total) { setProgress(current, total); }); + connect(m_files_job.get(), &NetJob::finished, &loop, &QEventLoop::quit); + + setStatus(tr("Downloading mods...")); + m_files_job->start(); + } +} diff --git a/launcher/modplatform/flame/FlameInstanceCreationTask.h b/launcher/modplatform/flame/FlameInstanceCreationTask.h new file mode 100644 index 00000000..efb099d9 --- /dev/null +++ b/launcher/modplatform/flame/FlameInstanceCreationTask.h @@ -0,0 +1,32 @@ +#pragma once + +#include "InstanceCreationTask.h" + +#include "modplatform/flame/FileResolvingTask.h" + +#include "net/NetJob.h" + +class FlameCreationTask final : public InstanceCreationTask { + Q_OBJECT + + public: + FlameCreationTask(QString staging_path, SettingsObjectPtr global_settings, QWidget* parent) + : InstanceCreationTask(), m_parent(parent) + { + setStagingPath(staging_path); + setParentSettings(global_settings); + } + + bool abort() override; + + bool createInstance() override; + + private slots: + void idResolverSucceeded(QEventLoop&); + + private: + QWidget* m_parent = nullptr; + + shared_qobject_ptr m_mod_id_resolver; + NetJob::Ptr m_files_job; +}; -- cgit From ebd46705d503a8627e759327e93c5a8432d4e47f Mon Sep 17 00:00:00 2001 From: flow Date: Thu, 21 Jul 2022 16:43:08 -0300 Subject: refactor: move creation of CF file download task to a separate function Signed-off-by: flow --- .../flame/FlameInstanceCreationTask.cpp | 143 +++++++-------------- .../modplatform/flame/FlameInstanceCreationTask.h | 1 + 2 files changed, 51 insertions(+), 93 deletions(-) diff --git a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp index 431c8b3d..6ec1060c 100644 --- a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp +++ b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp @@ -154,6 +154,7 @@ bool FlameCreationTask::createInstance() void FlameCreationTask::idResolverSucceeded(QEventLoop& loop) { auto results = m_mod_id_resolver->getResults(); + // first check for blocked mods QString text; QList urls; @@ -176,109 +177,65 @@ void FlameCreationTask::idResolverSucceeded(QEventLoop& loop) message_dialog->setModal(true); if (message_dialog->exec()) { - m_files_job = new NetJob(tr("Mod download"), APPLICATION->network()); - for (const auto& result : m_mod_id_resolver->getResults().files) { - QString filename = result.fileName; - if (!result.required) { - filename += ".disabled"; - } - - auto relpath = FS::PathCombine("minecraft", result.targetFolder, filename); - auto path = FS::PathCombine(m_stagingPath, relpath); - - switch (result.type) { - case Flame::File::Type::Folder: { - logWarning(tr("This 'Folder' may need extracting: %1").arg(relpath)); - // fall-through intentional, we treat these as plain old mods and dump them wherever. - } - case Flame::File::Type::SingleFile: - case Flame::File::Type::Mod: { - if (!result.url.isEmpty()) { - qDebug() << "Will download" << result.url << "to" << path; - auto dl = Net::Download::makeFile(result.url, path); - m_files_job->addNetAction(dl); - } - break; - } - case Flame::File::Type::Modpack: - logWarning(tr("Nesting modpacks in modpacks is not implemented, nothing was downloaded: %1").arg(relpath)); - break; - case Flame::File::Type::Cmod2: - case Flame::File::Type::Ctoc: - case Flame::File::Type::Unknown: - logWarning(tr("Unrecognized/unhandled PackageType for: %1").arg(relpath)); - break; - } - } - - 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(); - setError(reason); - }); - connect(m_files_job.get(), &NetJob::progress, [&](qint64 current, qint64 total) { setProgress(current, total); }); - connect(m_files_job.get(), &NetJob::finished, &loop, &QEventLoop::quit); - - setStatus(tr("Downloading mods...")); - m_files_job->start(); + setupDownloadJob(loop); } else { m_mod_id_resolver.reset(); setError("Canceled"); } } else { - // TODO extract to function ? - m_files_job = new NetJob(tr("Mod download"), APPLICATION->network()); - for (const auto& result : m_mod_id_resolver->getResults().files) { - QString filename = result.fileName; - if (!result.required) { - filename += ".disabled"; - } + setupDownloadJob(loop); + } +} + +void FlameCreationTask::setupDownloadJob(QEventLoop& loop) +{ + m_files_job = new NetJob(tr("Mod download"), APPLICATION->network()); + for (const auto& result : m_mod_id_resolver->getResults().files) { + QString filename = result.fileName; + if (!result.required) { + filename += ".disabled"; + } - auto relpath = FS::PathCombine("minecraft", result.targetFolder, filename); - auto path = FS::PathCombine(m_stagingPath, relpath); + auto relpath = FS::PathCombine("minecraft", result.targetFolder, filename); + auto path = FS::PathCombine(m_stagingPath, relpath); - switch (result.type) { - case Flame::File::Type::Folder: { - logWarning(tr("This 'Folder' may need extracting: %1").arg(relpath)); - // fall-through intentional, we treat these as plain old mods and dump them wherever. - } - case Flame::File::Type::SingleFile: - case Flame::File::Type::Mod: { - if (!result.url.isEmpty()) { - qDebug() << "Will download" << result.url << "to" << path; - auto dl = Net::Download::makeFile(result.url, path); - m_files_job->addNetAction(dl); - } - break; + switch (result.type) { + case Flame::File::Type::Folder: { + logWarning(tr("This 'Folder' may need extracting: %1").arg(relpath)); + // fall-through intentional, we treat these as plain old mods and dump them wherever. + } + case Flame::File::Type::SingleFile: + case Flame::File::Type::Mod: { + if (!result.url.isEmpty()) { + qDebug() << "Will download" << result.url << "to" << path; + auto dl = Net::Download::makeFile(result.url, path); + m_files_job->addNetAction(dl); } - case Flame::File::Type::Modpack: - logWarning(tr("Nesting modpacks in modpacks is not implemented, nothing was downloaded: %1").arg(relpath)); - break; - case Flame::File::Type::Cmod2: - case Flame::File::Type::Ctoc: - case Flame::File::Type::Unknown: - logWarning(tr("Unrecognized/unhandled PackageType for: %1").arg(relpath)); - break; + break; } + case Flame::File::Type::Modpack: + logWarning(tr("Nesting modpacks in modpacks is not implemented, nothing was downloaded: %1").arg(relpath)); + break; + case Flame::File::Type::Cmod2: + case Flame::File::Type::Ctoc: + case Flame::File::Type::Unknown: + logWarning(tr("Unrecognized/unhandled PackageType for: %1").arg(relpath)); + break; } + } - 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(); - setError(reason); - }); - connect(m_files_job.get(), &NetJob::progress, [&](qint64 current, qint64 total) { setProgress(current, total); }); - connect(m_files_job.get(), &NetJob::finished, &loop, &QEventLoop::quit); + 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(); + setError(reason); + }); + connect(m_files_job.get(), &NetJob::progress, [&](qint64 current, qint64 total) { setProgress(current, total); }); + connect(m_files_job.get(), &NetJob::finished, &loop, &QEventLoop::quit); - setStatus(tr("Downloading mods...")); - m_files_job->start(); - } + setStatus(tr("Downloading mods...")); + m_files_job->start(); } diff --git a/launcher/modplatform/flame/FlameInstanceCreationTask.h b/launcher/modplatform/flame/FlameInstanceCreationTask.h index efb099d9..be11f805 100644 --- a/launcher/modplatform/flame/FlameInstanceCreationTask.h +++ b/launcher/modplatform/flame/FlameInstanceCreationTask.h @@ -23,6 +23,7 @@ class FlameCreationTask final : public InstanceCreationTask { private slots: void idResolverSucceeded(QEventLoop&); + void setupDownloadJob(QEventLoop&); private: QWidget* m_parent = nullptr; -- cgit From 795d6f35eed689677186a26ba54ce9e77613859d Mon Sep 17 00:00:00 2001 From: flow Date: Thu, 14 Jul 2022 16:41:49 -0300 Subject: feat: add curseforge modpack updating Signed-off-by: flow --- .../flame/FlameInstanceCreationTask.cpp | 195 ++++++++++++++++++--- .../modplatform/flame/FlameInstanceCreationTask.h | 7 +- launcher/modplatform/flame/PackManifest.cpp | 24 ++- launcher/modplatform/flame/PackManifest.h | 8 +- 4 files changed, 202 insertions(+), 32 deletions(-) diff --git a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp index 6ec1060c..f0f02bc8 100644 --- a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp +++ b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp @@ -1,5 +1,6 @@ #include "FlameInstanceCreationTask.h" +#include "modplatform/flame/FlameAPI.h" #include "modplatform/flame/PackManifest.h" #include "Application.h" @@ -11,10 +12,23 @@ #include "settings/INISettingsObject.h" +#include "ui/dialogs/CustomMessageBox.h" #include "ui/dialogs/BlockedModsDialog.h" +// NOTE: Because of CF's ToS, I don't know if it counts as caching data, so it'll be disabled for now +#define DO_DIFF_UPDATE 0 + +const static QMap forgemap = { { "1.2.5", "3.4.9.171" }, + { "1.4.2", "6.0.1.355" }, + { "1.4.7", "6.6.2.534" }, + { "1.5.2", "7.8.1.737" } }; + +static const FlameAPI api; + bool FlameCreationTask::abort() { + if (m_process_update_file_info_job) + m_process_update_file_info_job->abort(); if (m_files_job) m_files_job->abort(); if (m_mod_id_resolver) @@ -23,43 +37,186 @@ bool FlameCreationTask::abort() return true; } -const static QMap forgemap = { { "1.2.5", "3.4.9.171" }, - { "1.4.2", "6.0.1.355" }, - { "1.4.7", "6.6.2.534" }, - { "1.5.2", "7.8.1.737" } }; +bool FlameCreationTask::updateInstance() +{ + auto instance_list = APPLICATION->instances(); + + // FIXME: How to handle situations when there's more than one install already for a given modpack? + auto inst = instance_list->getInstanceByManagedName(originalName()); + + if (!inst) { + inst = instance_list->getInstanceById(originalName()); + + if (!inst) + return false; + } + + QString index_path(FS::PathCombine(m_stagingPath, "manifest.json")); + + try { + Flame::loadManifest(m_pack, index_path); + } catch (const JSONValidationError& e) { + setError(tr("Could not understand pack manifest:\n") + e.cause()); + return false; + } + + 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); + 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; + + QDir old_inst_dir(inst->instanceRoot()); + + QString old_index_path(FS::PathCombine(old_inst_dir.absolutePath(), "flame", "manifest.json")); + QFileInfo old_index_file(old_index_path); + if (old_index_file.exists()) { + Flame::Manifest old_pack; + Flame::loadManifest(old_pack, old_index_path); + + auto& old_files = old_pack.files; + +#if DO_DIFF_UPDATE + // Remove repeated files, we don't need to download them! + auto& files = m_pack.files; + + // Let's remove all duplicated, identical resources! + auto files_iterator = files.begin(); + while (files_iterator != files.end()) { + auto const& file = files_iterator; + + auto old_file = old_files.find(file.key()); + if (old_file != old_files.end()) { + // We found a match, but is it a different version? + if (old_file->fileId == file->fileId) { + qDebug() << "Removed file at" << file->targetFolder << "with id" << file->fileId << "from list of downloads"; + + old_files.remove(file.key()); + files_iterator = files.erase(files_iterator); + } + } + + files_iterator++; + } +#endif + // Remove remaining old files (we need to do an API request to know which ids are which files...) + QStringList fileIds; + + for (auto& file : old_files) { + fileIds.append(QString::number(file.fileId)); + } + + auto* raw_response = new QByteArray; + auto job = api.getFiles(fileIds, raw_response); + + QEventLoop loop; + + connect(job, &NetJob::succeeded, this, [raw_response, fileIds, old_inst_dir, &old_files] { + // Parse the API response + QJsonParseError parse_error{}; + auto doc = QJsonDocument::fromJson(*raw_response, &parse_error); + if (parse_error.error != QJsonParseError::NoError) { + qWarning() << "Error while parsing JSON response from Flame files task at " << parse_error.offset + << " reason: " << parse_error.errorString(); + qWarning() << *raw_response; + return; + } + + try { + QJsonArray entries; + if (fileIds.size() == 1) + entries = { Json::requireObject(Json::requireObject(doc), "data") }; + else + entries = Json::requireArray(Json::requireObject(doc), "data"); + + for (auto entry : entries) { + auto entry_obj = Json::requireObject(entry); + + Flame::File file; + // We don't care about blocked mods, we just need local data to delete the file + file.parseFromObject(entry_obj, false); + + auto id = Json::requireInteger(entry_obj, "id"); + old_files.insert(id, file); + } + } catch (Json::JsonException& e) { + qCritical() << e.cause() << e.what(); + } + + // Delete the files + for (auto& file : old_files) { + if (file.fileName.isEmpty() || file.targetFolder.isEmpty()) + continue; + + qDebug() << "Removing" << file.fileName << "at" << file.targetFolder; + QString path(FS::PathCombine(old_inst_dir.absolutePath(), "minecraft", file.targetFolder, file.fileName)); + if (!QFile::remove(path)) + qDebug() << "Failed to remove file at" << path; + } + }); + connect(job, &NetJob::finished, &loop, &QEventLoop::quit); + + m_process_update_file_info_job = job; + job->start(); + + loop.exec(); + + m_process_update_file_info_job = nullptr; + } + // 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; +} bool FlameCreationTask::createInstance() { QEventLoop loop; - Flame::Manifest pack; try { - QString configPath = FS::PathCombine(m_stagingPath, "manifest.json"); - Flame::loadManifest(pack, configPath); - QFile::remove(configPath); + QString index_path(FS::PathCombine(m_stagingPath, "manifest.json")); + if (!m_pack.is_loaded) + Flame::loadManifest(m_pack, index_path); + + // Keep index file in case we need it some other time (like when changing versions) + QString new_index_place(FS::PathCombine(m_stagingPath, "flame", "manifest.json")); + FS::ensureFilePathExists(new_index_place); + QFile::rename(index_path, new_index_place); + } catch (const JSONValidationError& e) { setError(tr("Could not understand pack manifest:\n") + e.cause()); return false; } - if (!pack.overrides.isEmpty()) { - QString overridePath = FS::PathCombine(m_stagingPath, pack.overrides); + if (!m_pack.overrides.isEmpty()) { + QString overridePath = FS::PathCombine(m_stagingPath, m_pack.overrides); if (QFile::exists(overridePath)) { QString mcPath = FS::PathCombine(m_stagingPath, "minecraft"); if (!QFile::rename(overridePath, mcPath)) { - setError(tr("Could not rename the overrides folder:\n") + pack.overrides); + setError(tr("Could not rename the overrides folder:\n") + m_pack.overrides); return false; } } else { logWarning( - tr("The specified overrides folder (%1) is missing. Maybe the modpack was already used before?").arg(pack.overrides)); + tr("The specified overrides folder (%1) is missing. Maybe the modpack was already used before?").arg(m_pack.overrides)); } } QString forgeVersion; QString fabricVersion; // TODO: is Quilt relevant here? - for (auto& loader : pack.minecraft.modLoaders) { + for (auto& loader : m_pack.minecraft.modLoaders) { auto id = loader.id; if (id.startsWith("forge-")) { id.remove("forge-"); @@ -77,7 +234,7 @@ bool FlameCreationTask::createInstance() QString configPath = FS::PathCombine(m_stagingPath, "instance.cfg"); auto instanceSettings = std::make_shared(configPath); MinecraftInstance instance(m_globalSettings, instanceSettings, m_stagingPath); - auto mcVersion = pack.minecraft.version; + auto mcVersion = m_pack.minecraft.version; // Hack to correct some 'special sauce'... if (mcVersion.endsWith('.')) { @@ -105,9 +262,9 @@ bool FlameCreationTask::createInstance() if (m_instIcon != "default") { instance.setIconKey(m_instIcon); } else { - if (pack.name.contains("Direwolf20")) { + if (m_pack.name.contains("Direwolf20")) { instance.setIconKey("steve"); - } else if (pack.name.contains("FTB") || pack.name.contains("Feed The Beast")) { + } else if (m_pack.name.contains("FTB") || m_pack.name.contains("Feed The Beast")) { instance.setIconKey("ftb_logo"); } else { instance.setIconKey("flame"); @@ -133,10 +290,8 @@ bool FlameCreationTask::createInstance() instance.setName(m_instName); - m_mod_id_resolver = new Flame::FileResolvingTask(APPLICATION->network(), pack); - connect(m_mod_id_resolver.get(), &Flame::FileResolvingTask::succeeded, this, [this, &loop]{ - idResolverSucceeded(loop); - }); + 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); }); connect(m_mod_id_resolver.get(), &Flame::FileResolvingTask::failed, [&](QString reason) { m_mod_id_resolver.reset(); setError(tr("Unable to resolve mod IDs:\n") + reason); diff --git a/launcher/modplatform/flame/FlameInstanceCreationTask.h b/launcher/modplatform/flame/FlameInstanceCreationTask.h index be11f805..99822d93 100644 --- a/launcher/modplatform/flame/FlameInstanceCreationTask.h +++ b/launcher/modplatform/flame/FlameInstanceCreationTask.h @@ -19,6 +19,7 @@ class FlameCreationTask final : public InstanceCreationTask { bool abort() override; + bool updateInstance() override; bool createInstance() override; private slots: @@ -29,5 +30,9 @@ class FlameCreationTask final : public InstanceCreationTask { QWidget* m_parent = nullptr; shared_qobject_ptr m_mod_id_resolver; - NetJob::Ptr m_files_job; + Flame::Manifest m_pack; + + // Handle to allow aborting + NetJob* m_process_update_file_info_job = nullptr; + NetJob::Ptr m_files_job = nullptr; }; diff --git a/launcher/modplatform/flame/PackManifest.cpp b/launcher/modplatform/flame/PackManifest.cpp index 81395fcd..22008297 100644 --- a/launcher/modplatform/flame/PackManifest.cpp +++ b/launcher/modplatform/flame/PackManifest.cpp @@ -29,21 +29,29 @@ static void loadMinecraftV1(Flame::Minecraft& m, QJsonObject& minecraft) } } -static void loadManifestV1(Flame::Manifest& m, QJsonObject& manifest) +static void loadManifestV1(Flame::Manifest& pack, QJsonObject& manifest) { auto mc = Json::requireObject(manifest, "minecraft"); - loadMinecraftV1(m.minecraft, mc); - m.name = Json::ensureString(manifest, QString("name"), "Unnamed"); - m.version = Json::ensureString(manifest, QString("version"), QString()); - m.author = Json::ensureString(manifest, QString("author"), "Anonymous"); + + loadMinecraftV1(pack.minecraft, mc); + + pack.name = Json::ensureString(manifest, QString("name"), "Unnamed"); + pack.version = Json::ensureString(manifest, QString("version"), QString()); + pack.author = Json::ensureString(manifest, QString("author"), "Anonymous"); + auto arr = Json::ensureArray(manifest, "files", QJsonArray()); - for (QJsonValueRef item : arr) { + for (auto item : arr) { auto obj = Json::requireObject(item); + Flame::File file; loadFileV1(file, obj); - m.files.insert(file.fileId,file); + + pack.files.insert(file.fileId,file); } - m.overrides = Json::ensureString(manifest, "overrides", "overrides"); + + pack.overrides = Json::ensureString(manifest, "overrides", "overrides"); + + pack.is_loaded = true; } void Flame::loadManifest(Flame::Manifest& m, const QString& filepath) diff --git a/launcher/modplatform/flame/PackManifest.h b/launcher/modplatform/flame/PackManifest.h index a69e1321..0b7461d8 100644 --- a/launcher/modplatform/flame/PackManifest.h +++ b/launcher/modplatform/flame/PackManifest.h @@ -35,11 +35,11 @@ #pragma once -#include -#include +#include #include +#include #include -#include +#include namespace Flame { @@ -97,6 +97,8 @@ struct Manifest //File id -> File QMap files; QString overrides; + + bool is_loaded = false; }; void loadManifest(Flame::Manifest & m, const QString &filepath); -- cgit From eed73c90785ec977ee975d403270f9138aa6960c Mon Sep 17 00:00:00 2001 From: flow Date: Fri, 8 Jul 2022 19:44:44 -0300 Subject: refactor: clean up InstanceImportTask a bit Also removes a divide by two in the download progress (why was it there???) Signed-off-by: flow --- launcher/InstanceImportTask.cpp | 44 +++++++++++++++++------------------------ 1 file changed, 18 insertions(+), 26 deletions(-) diff --git a/launcher/InstanceImportTask.cpp b/launcher/InstanceImportTask.cpp index 72c2496f..da57ddeb 100644 --- a/launcher/InstanceImportTask.cpp +++ b/launcher/InstanceImportTask.cpp @@ -35,34 +35,26 @@ */ #include "InstanceImportTask.h" -#include + #include "Application.h" -#include "BaseInstance.h" #include "FileSystem.h" #include "MMCZip.h" #include "NullInstance.h" + #include "icons/IconList.h" #include "icons/IconUtils.h" -#include "settings/INISettingsObject.h" -// FIXME: this does not belong here, it's Minecraft/Flame specific -#include -#include "Json.h" -#include "minecraft/MinecraftInstance.h" -#include "minecraft/PackProfile.h" #include "modplatform/technic/TechnicPackProcessor.h" #include "modplatform/modrinth/ModrinthInstanceCreationTask.h" #include "modplatform/flame/FlameInstanceCreationTask.h" -#include "Application.h" -#include "icons/IconList.h" -#include "net/ChecksumValidator.h" - -#include "ui/dialogs/CustomMessageBox.h" -#include "ui/dialogs/BlockedModsDialog.h" +#include "settings/INISettingsObject.h" +#include #include +#include + InstanceImportTask::InstanceImportTask(const QUrl sourceUrl, QWidget* parent) { m_sourceUrl = sourceUrl; @@ -80,26 +72,26 @@ bool InstanceImportTask::abort() void InstanceImportTask::executeTask() { - if (m_sourceUrl.isLocalFile()) - { + if (m_sourceUrl.isLocalFile()) { m_archivePath = m_sourceUrl.toLocalFile(); processZipPack(); - } - else - { + } else { setStatus(tr("Downloading modpack:\n%1").arg(m_sourceUrl.toString())); m_downloadRequired = true; - const QString path = m_sourceUrl.host() + '/' + m_sourceUrl.path(); + const QString path(m_sourceUrl.host() + '/' + m_sourceUrl.path()); + auto entry = APPLICATION->metacache()->resolveEntry("general", path); entry->setStale(true); + m_archivePath = entry->getFullPath(); + m_filesNetJob = new NetJob(tr("Modpack download"), APPLICATION->network()); m_filesNetJob->addNetAction(Net::Download::makeCached(m_sourceUrl, entry)); - m_archivePath = entry->getFullPath(); - auto job = m_filesNetJob.get(); - connect(job, &NetJob::succeeded, this, &InstanceImportTask::downloadSucceeded); - connect(job, &NetJob::progress, this, &InstanceImportTask::downloadProgressChanged); - connect(job, &NetJob::failed, this, &InstanceImportTask::downloadFailed); + + connect(m_filesNetJob.get(), &NetJob::succeeded, this, &InstanceImportTask::downloadSucceeded); + connect(m_filesNetJob.get(), &NetJob::progress, this, &InstanceImportTask::downloadProgressChanged); + connect(m_filesNetJob.get(), &NetJob::failed, this, &InstanceImportTask::downloadFailed); + m_filesNetJob->start(); } } @@ -118,7 +110,7 @@ void InstanceImportTask::downloadFailed(QString reason) void InstanceImportTask::downloadProgressChanged(qint64 current, qint64 total) { - setProgress(current / 2, total); + setProgress(current, total); } void InstanceImportTask::processZipPack() -- cgit From 6131346e2f80c5ce4377fcc608be4f3929f43f91 Mon Sep 17 00:00:00 2001 From: flow 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 --- 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(-) 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(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 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 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; @@ -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 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 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 -#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 7024acac06885f9d9e24125afef04231aebdd113 Mon Sep 17 00:00:00 2001 From: flow Date: Thu, 28 Jul 2022 22:34:24 -0300 Subject: feat: add override helper functions These help us keep track of relevant metadata information about overrides, so that we know what they are when we update a pack. Signed-off-by: flow --- launcher/CMakeLists.txt | 2 + launcher/modplatform/helpers/OverrideUtils.cpp | 59 ++++++++++++++++++++++++++ launcher/modplatform/helpers/OverrideUtils.h | 20 +++++++++ 3 files changed, 81 insertions(+) create mode 100644 launcher/modplatform/helpers/OverrideUtils.cpp create mode 100644 launcher/modplatform/helpers/OverrideUtils.h diff --git a/launcher/CMakeLists.txt b/launcher/CMakeLists.txt index 7bd92fac..2ff700ad 100644 --- a/launcher/CMakeLists.txt +++ b/launcher/CMakeLists.txt @@ -461,6 +461,8 @@ set(API_SOURCES modplatform/helpers/NetworkModAPI.cpp modplatform/helpers/HashUtils.h modplatform/helpers/HashUtils.cpp + modplatform/helpers/OverrideUtils.h + modplatform/helpers/OverrideUtils.cpp ) set(FTB_SOURCES diff --git a/launcher/modplatform/helpers/OverrideUtils.cpp b/launcher/modplatform/helpers/OverrideUtils.cpp new file mode 100644 index 00000000..3496a286 --- /dev/null +++ b/launcher/modplatform/helpers/OverrideUtils.cpp @@ -0,0 +1,59 @@ +#include "OverrideUtils.h" + +#include + +#include "FileSystem.h" + +namespace Override { + +void createOverrides(QString name, QString parent_folder, QString override_path) +{ + QString file_path(FS::PathCombine(parent_folder, name + ".txt")); + if (QFile::exists(file_path)) + QFile::remove(file_path); + + FS::ensureFilePathExists(file_path); + + QFile file(file_path); + file.open(QFile::WriteOnly); + + QDirIterator override_iterator(override_path, QDirIterator::Subdirectories); + while (override_iterator.hasNext()) { + auto override_file_path = override_iterator.next(); + QFileInfo info(override_file_path); + if (info.isFile()) { + // Absolute path with temp directory -> relative path + override_file_path = override_file_path.split(name).last().remove(0, 1); + + file.write(override_file_path.toUtf8()); + file.write("\n"); + } + } + + file.close(); +} + +QStringList readOverrides(QString name, QString parent_folder) +{ + QString file_path(FS::PathCombine(parent_folder, name + ".txt")); + + QFile file(file_path); + if (!file.exists()) + return {}; + + QStringList previous_overrides; + + file.open(QFile::ReadOnly); + + QString entry; + do { + entry = file.readLine(); + previous_overrides.append(entry.trimmed()); + } while (!entry.isEmpty()); + + file.close(); + + return previous_overrides; +} + +} // namespace Override diff --git a/launcher/modplatform/helpers/OverrideUtils.h b/launcher/modplatform/helpers/OverrideUtils.h new file mode 100644 index 00000000..73f059d6 --- /dev/null +++ b/launcher/modplatform/helpers/OverrideUtils.h @@ -0,0 +1,20 @@ +#pragma once + +#include + +namespace Override { + +/** This creates a file in `parent_folder` that holds information about which + * overrides are in `override_path`. + * + * If there's already an existing such file, it will be ovewritten. + */ +void createOverrides(QString name, QString parent_folder, 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); + +} // namespace Override -- cgit From 3a9d58e31c70159f574b5cc24a104f81c9d981ed Mon Sep 17 00:00:00 2001 From: flow Date: Thu, 28 Jul 2022 22:35:40 -0300 Subject: feat: add override handling in modrinth pack update Signed-off-by: flow --- .../modrinth/ModrinthInstanceCreationTask.cpp | 42 ++++++++++++++++++---- 1 file changed, 35 insertions(+), 7 deletions(-) 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 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 Date: Sun, 31 Jul 2022 19:50:09 -0300 Subject: fix: correctly set all managed pack fields in Modrinth pack Signed-off-by: flow --- .../modrinth/ModrinthInstanceCreationTask.cpp | 28 +++++++++++++++------- .../modrinth/ModrinthInstanceCreationTask.h | 10 ++++++-- 2 files changed, 27 insertions(+), 11 deletions(-) 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 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& files) +bool ModrinthCreationTask::parseManifest(QString index_path, std::vector& files, bool set_managed_info) { try { auto doc = Json::requireDocument(index_path); @@ -244,8 +252,10 @@ bool ModrinthCreationTask::parseManifest(QString index_path, std::vector(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 + +#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&); + bool parseManifest(QString, std::vector&, bool set_managed_info = true); QString getManagedPackID() const; private: @@ -36,4 +40,6 @@ class ModrinthCreationTask final : public InstanceCreationTask { std::vector m_files; NetJob::Ptr m_files_job; + + std::optional m_instance; }; -- cgit From 8c0816c1669c6acedd798b4d0b49c7a567cdcf1a Mon Sep 17 00:00:00 2001 From: flow Date: Sun, 31 Jul 2022 16:45:01 -0300 Subject: feat: add override awareness to CF modpack updating Signed-off-by: flow --- .../flame/FlameInstanceCreationTask.cpp | 39 +++++++++++++++------- 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp index 202caa28..4d70e223 100644 --- a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp +++ b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp @@ -11,14 +11,13 @@ #include "minecraft/MinecraftInstance.h" #include "minecraft/PackProfile.h" +#include "modplatform/helpers/OverrideUtils.h" + #include "settings/INISettingsObject.h" #include "ui/dialogs/CustomMessageBox.h" #include "ui/dialogs/BlockedModsDialog.h" -// NOTE: Because of CF's ToS, I don't know if it counts as caching data, so it'll be disabled for now -#define DO_DIFF_UPDATE 0 - const static QMap forgemap = { { "1.2.5", "3.4.9.171" }, { "1.4.2", "6.0.1.355" }, { "1.4.7", "6.6.2.534" }, @@ -77,7 +76,9 @@ bool FlameCreationTask::updateInstance() QDir old_inst_dir(inst->instanceRoot()); - QString old_index_path(FS::PathCombine(old_inst_dir.absolutePath(), "flame", "manifest.json")); + QString old_index_folder(FS::PathCombine(old_inst_dir.absolutePath(), "flame")); + QString old_index_path(FS::PathCombine(old_index_folder, "manifest.json")); + QFileInfo old_index_file(old_index_path); if (old_index_file.exists()) { Flame::Manifest old_pack; @@ -85,11 +86,9 @@ bool FlameCreationTask::updateInstance() auto& old_files = old_pack.files; -#if DO_DIFF_UPDATE - // Remove repeated files, we don't need to download them! auto& files = m_pack.files; - // Let's remove all duplicated, identical resources! + // Remove repeated files, we don't need to download them! auto files_iterator = files.begin(); while (files_iterator != files.end()) { auto const& file = files_iterator; @@ -107,7 +106,18 @@ bool FlameCreationTask::updateInstance() files_iterator++; } -#endif + + QString 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); + } + // Remove remaining old files (we need to do an API request to know which ids are which files...) QStringList fileIds; @@ -120,7 +130,7 @@ bool FlameCreationTask::updateInstance() QEventLoop loop; - connect(job, &NetJob::succeeded, this, [raw_response, fileIds, old_inst_dir, &old_files] { + connect(job, &NetJob::succeeded, 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); @@ -158,7 +168,7 @@ bool FlameCreationTask::updateInstance() continue; qDebug() << "Removing" << file.fileName << "at" << file.targetFolder; - QString path(FS::PathCombine(old_inst_dir.absolutePath(), "minecraft", file.targetFolder, file.fileName)); + QString path(FS::PathCombine(old_minecraft_dir, file.targetFolder, file.fileName)); if (!QFile::remove(path)) qDebug() << "Failed to remove file at" << path; } @@ -172,7 +182,6 @@ bool FlameCreationTask::updateInstance() m_process_update_file_info_job = nullptr; } - // TODO: Currently 'overrides' will always override the stuff on update. How do we preserve unchanged overrides? setOverride(true); qDebug() << "Will override instance!"; @@ -185,13 +194,15 @@ bool FlameCreationTask::createInstance() { QEventLoop loop; + QString parent_folder(FS::PathCombine(m_stagingPath, "flame")); + try { QString index_path(FS::PathCombine(m_stagingPath, "manifest.json")); if (!m_pack.is_loaded) Flame::loadManifest(m_pack, index_path); // Keep index file in case we need it some other time (like when changing versions) - QString new_index_place(FS::PathCombine(m_stagingPath, "flame", "manifest.json")); + QString new_index_place(FS::PathCombine(parent_folder, "manifest.json")); FS::ensureFilePathExists(new_index_place); QFile::rename(index_path, new_index_place); @@ -203,6 +214,9 @@ bool FlameCreationTask::createInstance() if (!m_pack.overrides.isEmpty()) { QString overridePath = FS::PathCombine(m_stagingPath, m_pack.overrides); if (QFile::exists(overridePath)) { + // Create a list of overrides in "overrides.txt" inside flame/ + Override::createOverrides("overrides", parent_folder, overridePath); + QString mcPath = FS::PathCombine(m_stagingPath, "minecraft"); if (!QFile::rename(overridePath, mcPath)) { setError(tr("Could not rename the overrides folder:\n") + m_pack.overrides); @@ -338,6 +352,7 @@ void FlameCreationTask::idResolverSucceeded(QEventLoop& loop) } else { m_mod_id_resolver.reset(); setError("Canceled"); + loop.quit(); } } else { setupDownloadJob(loop); -- cgit From 4b0ceea8941826134c949b1c2fb80e05c174e5ec Mon Sep 17 00:00:00 2001 From: flow Date: Sun, 31 Jul 2022 19:56:14 -0300 Subject: fix: correctly set managed pack fields in CF pack Signed-off-by: flow --- launcher/modplatform/flame/FlameInstanceCreationTask.cpp | 9 +++++++++ launcher/modplatform/flame/FlameInstanceCreationTask.h | 6 ++++++ 2 files changed, 15 insertions(+) diff --git a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp index 4d70e223..76ac11af 100644 --- a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp +++ b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp @@ -186,6 +186,8 @@ bool FlameCreationTask::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; } @@ -319,6 +321,13 @@ bool FlameCreationTask::createInstance() loop.exec(); + if (m_instance) { + auto inst = m_instance.value(); + + inst->copyManagedPack(instance); + inst->setName(instance.name()); + } + return getError().isEmpty(); } diff --git a/launcher/modplatform/flame/FlameInstanceCreationTask.h b/launcher/modplatform/flame/FlameInstanceCreationTask.h index 99822d93..ccb5f827 100644 --- a/launcher/modplatform/flame/FlameInstanceCreationTask.h +++ b/launcher/modplatform/flame/FlameInstanceCreationTask.h @@ -2,6 +2,10 @@ #include "InstanceCreationTask.h" +#include + +#include "minecraft/MinecraftInstance.h" + #include "modplatform/flame/FileResolvingTask.h" #include "net/NetJob.h" @@ -35,4 +39,6 @@ class FlameCreationTask final : public InstanceCreationTask { // Handle to allow aborting NetJob* m_process_update_file_info_job = nullptr; NetJob::Ptr m_files_job = nullptr; + + std::optional m_instance; }; -- cgit From 654157096985d0cec0d3b7bb53f39a5c0157206e Mon Sep 17 00:00:00 2001 From: flow Date: Thu, 28 Jul 2022 15:58:04 -0300 Subject: fix: simplify abort handling and add missing emits Signed-off-by: flow --- launcher/InstanceImportTask.cpp | 11 ++++++++--- launcher/InstanceImportTask.h | 1 + launcher/InstanceList.cpp | 17 ++++++++++------- launcher/modplatform/atlauncher/ATLPackInstallTask.cpp | 18 ++++++++++++++++++ launcher/modplatform/atlauncher/ATLPackInstallTask.h | 1 + launcher/modplatform/legacy_ftb/PackFetchTask.cpp | 14 ++++++++++++++ launcher/modplatform/legacy_ftb/PackFetchTask.h | 2 ++ launcher/modplatform/legacy_ftb/PackInstallTask.cpp | 6 ++++++ launcher/modplatform/legacy_ftb/PackInstallTask.h | 1 + launcher/modplatform/modpacksch/FTBPackInstallTask.cpp | 3 +-- launcher/modplatform/technic/SolderPackInstallTask.cpp | 8 ++++++++ launcher/modplatform/technic/SolderPackInstallTask.h | 1 + launcher/ui/MainWindow.cpp | 4 ++++ launcher/ui/dialogs/ProgressDialog.cpp | 5 ++--- launcher/ui/pages/modplatform/legacy_ftb/Page.cpp | 8 +++++++- launcher/ui/pages/modplatform/legacy_ftb/Page.h | 1 + 16 files changed, 85 insertions(+), 16 deletions(-) diff --git a/launcher/InstanceImportTask.cpp b/launcher/InstanceImportTask.cpp index 09e65064..56aabb2d 100644 --- a/launcher/InstanceImportTask.cpp +++ b/launcher/InstanceImportTask.cpp @@ -91,6 +91,7 @@ void InstanceImportTask::executeTask() connect(m_filesNetJob.get(), &NetJob::succeeded, this, &InstanceImportTask::downloadSucceeded); connect(m_filesNetJob.get(), &NetJob::progress, this, &InstanceImportTask::downloadProgressChanged); connect(m_filesNetJob.get(), &NetJob::failed, this, &InstanceImportTask::downloadFailed); + connect(m_filesNetJob.get(), &NetJob::aborted, this, &InstanceImportTask::downloadAborted); m_filesNetJob->start(); } @@ -113,6 +114,12 @@ void InstanceImportTask::downloadProgressChanged(qint64 current, qint64 total) setProgress(current, total); } +void InstanceImportTask::downloadAborted() +{ + emitAborted(); + m_filesNetJob.reset(); +} + void InstanceImportTask::processZipPack() { setStatus(tr("Extracting modpack")); @@ -246,9 +253,7 @@ void InstanceImportTask::extractFinished() void InstanceImportTask::extractAborted() { - emitFailed(tr("Instance import has been aborted.")); - emit aborted(); - return; + emitAborted(); } void InstanceImportTask::processFlame() diff --git a/launcher/InstanceImportTask.h b/launcher/InstanceImportTask.h index 48ba2161..acb1f9d6 100644 --- a/launcher/InstanceImportTask.h +++ b/launcher/InstanceImportTask.h @@ -80,6 +80,7 @@ private slots: void downloadSucceeded(); void downloadFailed(QString reason); void downloadProgressChanged(qint64 current, qint64 total); + void downloadAborted(); void extractFinished(); void extractAborted(); diff --git a/launcher/InstanceList.cpp b/launcher/InstanceList.cpp index 0e59150e..5f604f9a 100644 --- a/launcher/InstanceList.cpp +++ b/launcher/InstanceList.cpp @@ -784,6 +784,7 @@ class InstanceStaging : public Task { m_child.reset(child); connect(child, &Task::succeeded, this, &InstanceStaging::childSucceded); connect(child, &Task::failed, this, &InstanceStaging::childFailed); + connect(child, &Task::aborted, this, &InstanceStaging::childAborted); connect(child, &Task::status, this, &InstanceStaging::setStatus); connect(child, &Task::progress, this, &InstanceStaging::setProgress); connect(&m_backoffTimer, &QTimer::timeout, this, &InstanceStaging::childSucceded); @@ -794,17 +795,14 @@ class InstanceStaging : public Task { // FIXME/TODO: add ability to abort during instance commit retries bool abort() override { - if (m_child && m_child->canAbort()) { + if (m_child && m_child->canAbort()) return m_child->abort(); - } + return false; } bool canAbort() const override { - if (m_child && m_child->canAbort()) { - return true; - } - return false; + return (m_child && m_child->canAbort()); } protected: @@ -834,7 +832,12 @@ class InstanceStaging : public Task { emitFailed(reason); } - private: + void childAborted() + { + emitAborted(); + } + +private: InstanceList * m_parent; /* * WHY: the whole reason why this uses an exponential backoff retry scheme is antivirus on Windows. diff --git a/launcher/modplatform/atlauncher/ATLPackInstallTask.cpp b/launcher/modplatform/atlauncher/ATLPackInstallTask.cpp index 13cab256..a553eafd 100644 --- a/launcher/modplatform/atlauncher/ATLPackInstallTask.cpp +++ b/launcher/modplatform/atlauncher/ATLPackInstallTask.cpp @@ -90,6 +90,7 @@ void PackInstallTask::executeTask() QObject::connect(netJob, &NetJob::succeeded, this, &PackInstallTask::onDownloadSucceeded); QObject::connect(netJob, &NetJob::failed, this, &PackInstallTask::onDownloadFailed); + QObject::connect(netJob, &NetJob::aborted, this, &PackInstallTask::onDownloadAborted); } void PackInstallTask::onDownloadSucceeded() @@ -169,6 +170,12 @@ void PackInstallTask::onDownloadFailed(QString reason) emitFailed(reason); } +void PackInstallTask::onDownloadAborted() +{ + jobPtr.reset(); + emitAborted(); +} + void PackInstallTask::deleteExistingFiles() { setStatus(tr("Deleting existing files...")); @@ -675,6 +682,11 @@ void PackInstallTask::installConfigs() abortable = true; setProgress(current, total); }); + connect(jobPtr.get(), &NetJob::aborted, [&]{ + abortable = false; + jobPtr.reset(); + emitAborted(); + }); jobPtr->start(); } @@ -831,6 +843,12 @@ void PackInstallTask::downloadMods() abortable = true; setProgress(current, total); }); + connect(jobPtr.get(), &NetJob::aborted, [&] + { + abortable = false; + jobPtr.reset(); + emitAborted(); + }); jobPtr->start(); } diff --git a/launcher/modplatform/atlauncher/ATLPackInstallTask.h b/launcher/modplatform/atlauncher/ATLPackInstallTask.h index a7124d59..ed4436f0 100644 --- a/launcher/modplatform/atlauncher/ATLPackInstallTask.h +++ b/launcher/modplatform/atlauncher/ATLPackInstallTask.h @@ -93,6 +93,7 @@ protected: private slots: void onDownloadSucceeded(); void onDownloadFailed(QString reason); + void onDownloadAborted(); void onModsDownloaded(); void onModsExtracted(); diff --git a/launcher/modplatform/legacy_ftb/PackFetchTask.cpp b/launcher/modplatform/legacy_ftb/PackFetchTask.cpp index 4da6a866..36aa60c7 100644 --- a/launcher/modplatform/legacy_ftb/PackFetchTask.cpp +++ b/launcher/modplatform/legacy_ftb/PackFetchTask.cpp @@ -59,6 +59,7 @@ void PackFetchTask::fetch() QObject::connect(jobPtr.get(), &NetJob::succeeded, this, &PackFetchTask::fileDownloadFinished); QObject::connect(jobPtr.get(), &NetJob::failed, this, &PackFetchTask::fileDownloadFailed); + QObject::connect(jobPtr.get(), &NetJob::aborted, this, &PackFetchTask::fileDownloadAborted); jobPtr->start(); } @@ -98,6 +99,14 @@ void PackFetchTask::fetchPrivate(const QStringList & toFetch) delete data; }); + QObject::connect(job, &NetJob::aborted, this, [this, job, data]{ + emit aborted(); + job->deleteLater(); + + data->clear(); + delete data; + }); + job->start(); } } @@ -204,4 +213,9 @@ void PackFetchTask::fileDownloadFailed(QString reason) emit failed(reason); } +void PackFetchTask::fileDownloadAborted() +{ + emit aborted(); +} + } diff --git a/launcher/modplatform/legacy_ftb/PackFetchTask.h b/launcher/modplatform/legacy_ftb/PackFetchTask.h index f1667e90..8f3c4f3b 100644 --- a/launcher/modplatform/legacy_ftb/PackFetchTask.h +++ b/launcher/modplatform/legacy_ftb/PackFetchTask.h @@ -33,10 +33,12 @@ private: protected slots: void fileDownloadFinished(); void fileDownloadFailed(QString reason); + void fileDownloadAborted(); signals: void finished(ModpackList publicPacks, ModpackList thirdPartyPacks); void failed(QString reason); + void aborted(); void privateFileDownloadFinished(Modpack modpack); void privateFileDownloadFailed(QString reason, QString packCode); diff --git a/launcher/modplatform/legacy_ftb/PackInstallTask.cpp b/launcher/modplatform/legacy_ftb/PackInstallTask.cpp index e2e61a7c..209ad884 100644 --- a/launcher/modplatform/legacy_ftb/PackInstallTask.cpp +++ b/launcher/modplatform/legacy_ftb/PackInstallTask.cpp @@ -86,6 +86,7 @@ void PackInstallTask::downloadPack() connect(netJobContainer.get(), &NetJob::succeeded, this, &PackInstallTask::onDownloadSucceeded); connect(netJobContainer.get(), &NetJob::failed, this, &PackInstallTask::onDownloadFailed); connect(netJobContainer.get(), &NetJob::progress, this, &PackInstallTask::onDownloadProgress); + connect(netJobContainer.get(), &NetJob::aborted, this, &PackInstallTask::onDownloadAborted); netJobContainer->start(); progress(1, 4); @@ -110,6 +111,11 @@ void PackInstallTask::onDownloadProgress(qint64 current, qint64 total) setStatus(tr("Downloading zip for %1 (%2%)").arg(m_pack.name).arg(current / 10)); } +void PackInstallTask::onDownloadAborted() +{ + emitAborted(); +} + void PackInstallTask::unzip() { progress(2, 4); diff --git a/launcher/modplatform/legacy_ftb/PackInstallTask.h b/launcher/modplatform/legacy_ftb/PackInstallTask.h index da4c0da5..da791e06 100644 --- a/launcher/modplatform/legacy_ftb/PackInstallTask.h +++ b/launcher/modplatform/legacy_ftb/PackInstallTask.h @@ -38,6 +38,7 @@ private slots: void onDownloadSucceeded(); void onDownloadFailed(QString reason); void onDownloadProgress(qint64 current, qint64 total); + void onDownloadAborted(); void onUnzipFinished(); void onUnzipCanceled(); diff --git a/launcher/modplatform/modpacksch/FTBPackInstallTask.cpp b/launcher/modplatform/modpacksch/FTBPackInstallTask.cpp index f17a311a..97ce1dc6 100644 --- a/launcher/modplatform/modpacksch/FTBPackInstallTask.cpp +++ b/launcher/modplatform/modpacksch/FTBPackInstallTask.cpp @@ -65,9 +65,8 @@ bool PackInstallTask::abort() if (m_mod_id_resolver_task) aborted &= m_mod_id_resolver_task->abort(); - // FIXME: This should be 'emitAborted()', but InstanceStaging doesn't connect to the abort signal yet... if (aborted) - emitFailed(tr("Aborted")); + emitAborted(); return aborted; } diff --git a/launcher/modplatform/technic/SolderPackInstallTask.cpp b/launcher/modplatform/technic/SolderPackInstallTask.cpp index 65b6ca51..19731b38 100644 --- a/launcher/modplatform/technic/SolderPackInstallTask.cpp +++ b/launcher/modplatform/technic/SolderPackInstallTask.cpp @@ -77,6 +77,7 @@ void Technic::SolderPackInstallTask::executeTask() auto job = m_filesNetJob.get(); connect(job, &NetJob::succeeded, this, &Technic::SolderPackInstallTask::fileListSucceeded); connect(job, &NetJob::failed, this, &Technic::SolderPackInstallTask::downloadFailed); + connect(job, &NetJob::aborted, this, &Technic::SolderPackInstallTask::downloadAborted); m_filesNetJob->start(); } @@ -127,6 +128,7 @@ void Technic::SolderPackInstallTask::fileListSucceeded() connect(m_filesNetJob.get(), &NetJob::succeeded, this, &Technic::SolderPackInstallTask::downloadSucceeded); connect(m_filesNetJob.get(), &NetJob::progress, this, &Technic::SolderPackInstallTask::downloadProgressChanged); connect(m_filesNetJob.get(), &NetJob::failed, this, &Technic::SolderPackInstallTask::downloadFailed); + connect(m_filesNetJob.get(), &NetJob::aborted, this, &Technic::SolderPackInstallTask::downloadAborted); m_filesNetJob->start(); } @@ -171,6 +173,12 @@ void Technic::SolderPackInstallTask::downloadProgressChanged(qint64 current, qin setProgress(current / 2, total); } +void Technic::SolderPackInstallTask::downloadAborted() +{ + emitAborted(); + m_filesNetJob.reset(); +} + void Technic::SolderPackInstallTask::extractFinished() { if (!m_extractFuture.result()) diff --git a/launcher/modplatform/technic/SolderPackInstallTask.h b/launcher/modplatform/technic/SolderPackInstallTask.h index 117a7bd6..aa14ce88 100644 --- a/launcher/modplatform/technic/SolderPackInstallTask.h +++ b/launcher/modplatform/technic/SolderPackInstallTask.h @@ -61,6 +61,7 @@ namespace Technic void downloadSucceeded(); void downloadFailed(QString reason); void downloadProgressChanged(qint64 current, qint64 total); + void downloadAborted(); void extractFinished(); void extractAborted(); diff --git a/launcher/ui/MainWindow.cpp b/launcher/ui/MainWindow.cpp index 58b1ae80..5729b44d 100644 --- a/launcher/ui/MainWindow.cpp +++ b/launcher/ui/MainWindow.cpp @@ -1656,6 +1656,10 @@ void MainWindow::runModalTask(Task *task) CustomMessageBox::selectable(this, tr("Warnings"), warnings.join('\n'), QMessageBox::Warning)->show(); } }); + connect(task, &Task::aborted, [this] + { + CustomMessageBox::selectable(this, tr("Task aborted"), tr("The task has been aborted by the user."), QMessageBox::Information)->show(); + }); ProgressDialog loadDialog(this); loadDialog.setSkipButton(true, tr("Abort")); loadDialog.execWithTask(task); diff --git a/launcher/ui/dialogs/ProgressDialog.cpp b/launcher/ui/dialogs/ProgressDialog.cpp index 3c7f53d3..6f9cc8e0 100644 --- a/launcher/ui/dialogs/ProgressDialog.cpp +++ b/launcher/ui/dialogs/ProgressDialog.cpp @@ -43,8 +43,7 @@ void ProgressDialog::setSkipButton(bool present, QString label) void ProgressDialog::on_skipButton_clicked(bool checked) { Q_UNUSED(checked); - if (task->abort()) - QDialog::reject(); + task->abort(); } ProgressDialog::~ProgressDialog() @@ -81,7 +80,7 @@ int ProgressDialog::execWithTask(Task* task) connect(task, &Task::stepStatus, this, &ProgressDialog::changeStatus); connect(task, &Task::progress, this, &ProgressDialog::changeProgress); - connect(task, &Task::aborted, [this] { onTaskFailed(tr("Aborted by user")); }); + connect(task, &Task::aborted, [this] { QDialog::reject(); }); m_is_multi_step = task->isMultiStep(); if (!m_is_multi_step) { diff --git a/launcher/ui/pages/modplatform/legacy_ftb/Page.cpp b/launcher/ui/pages/modplatform/legacy_ftb/Page.cpp index 0208b36b..98ab8799 100644 --- a/launcher/ui/pages/modplatform/legacy_ftb/Page.cpp +++ b/launcher/ui/pages/modplatform/legacy_ftb/Page.cpp @@ -146,6 +146,7 @@ void Page::openedImpl() { connect(ftbFetchTask.get(), &PackFetchTask::finished, this, &Page::ftbPackDataDownloadSuccessfully); connect(ftbFetchTask.get(), &PackFetchTask::failed, this, &Page::ftbPackDataDownloadFailed); + connect(ftbFetchTask.get(), &PackFetchTask::aborted, this, &Page::ftbPackDataDownloadAborted); connect(ftbFetchTask.get(), &PackFetchTask::privateFileDownloadFinished, this, &Page::ftbPrivatePackDataDownloadSuccessfully); connect(ftbFetchTask.get(), &PackFetchTask::privateFileDownloadFailed, this, &Page::ftbPrivatePackDataDownloadFailed); @@ -220,7 +221,12 @@ void Page::ftbPackDataDownloadSuccessfully(ModpackList publicPacks, ModpackList void Page::ftbPackDataDownloadFailed(QString reason) { - //TODO: Display the error + CustomMessageBox::selectable(this, tr("Error"), reason, QMessageBox::Critical)->show(); +} + +void Page::ftbPackDataDownloadAborted() +{ + CustomMessageBox::selectable(this, tr("Task aborted"), tr("The task has been aborted by the user."), QMessageBox::Information)->show(); } void Page::ftbPrivatePackDataDownloadSuccessfully(Modpack pack) diff --git a/launcher/ui/pages/modplatform/legacy_ftb/Page.h b/launcher/ui/pages/modplatform/legacy_ftb/Page.h index 52db7d91..1de8b40a 100644 --- a/launcher/ui/pages/modplatform/legacy_ftb/Page.h +++ b/launcher/ui/pages/modplatform/legacy_ftb/Page.h @@ -95,6 +95,7 @@ private: private slots: void ftbPackDataDownloadSuccessfully(ModpackList publicPacks, ModpackList thirdPartyPacks); void ftbPackDataDownloadFailed(QString reason); + void ftbPackDataDownloadAborted(); void ftbPrivatePackDataDownloadSuccessfully(Modpack pack); void ftbPrivatePackDataDownloadFailed(QString reason, QString packCode); -- cgit From 6a50fa35ec0dcb8b91ffc0ce70d4d8b9ee5d9fb8 Mon Sep 17 00:00:00 2001 From: flow Date: Sun, 31 Jul 2022 17:56:51 -0300 Subject: feat: add canAbort() status change in Task By now, it's a recurring pattern of wanting to restrict aborting in certain situations. This avoids further code duplication, and adds a signal that external users can hook up to to respond to such change. Signed-off-by: flow --- launcher/tasks/Task.h | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/launcher/tasks/Task.h b/launcher/tasks/Task.h index 2baf0188..f2872643 100644 --- a/launcher/tasks/Task.h +++ b/launcher/tasks/Task.h @@ -68,7 +68,7 @@ class Task : public QObject, public QRunnable { virtual QStringList warnings() const; - virtual bool canAbort() const { return false; } + virtual bool canAbort() const { return m_can_abort; } auto getState() const -> State { return m_state; } @@ -96,6 +96,10 @@ class Task : public QObject, public QRunnable { void status(QString status); void stepStatus(QString status); + /** Emitted when the canAbort() status has changed. + */ + void abortStatusChanged(bool can_abort); + public slots: // QRunnable's interface void run() override { start(); } @@ -103,6 +107,8 @@ 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); } + protected: virtual void executeTask() = 0; @@ -125,4 +131,8 @@ class Task : public QObject, public QRunnable { // TODO: Nuke in favor of QLoggingCategory bool m_show_debug = true; + + private: + // Change using setAbortStatus + bool m_can_abort = false; }; -- cgit From 87002fb8f84b104a221de07ba99ba0eb5cc6bbb6 Mon Sep 17 00:00:00 2001 From: flow Date: Sun, 31 Jul 2022 18:21:59 -0300 Subject: fix: hook up setAbortStatus in instance import tasks Signed-off-by: flow --- 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(-) 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 -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 &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 68facd6b93d1a08a7c2417aa0d0ec614b0feecbe Mon Sep 17 00:00:00 2001 From: flow Date: Sun, 31 Jul 2022 18:28:56 -0300 Subject: fix(ui): hook up abort status signal in ProgressDialog Now we have a visual indication on when tasks are abortable! Signed-off-by: flow --- launcher/ui/dialogs/ProgressDialog.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/launcher/ui/dialogs/ProgressDialog.cpp b/launcher/ui/dialogs/ProgressDialog.cpp index 6f9cc8e0..258a32e4 100644 --- a/launcher/ui/dialogs/ProgressDialog.cpp +++ b/launcher/ui/dialogs/ProgressDialog.cpp @@ -81,6 +81,7 @@ int ProgressDialog::execWithTask(Task* task) connect(task, &Task::progress, this, &ProgressDialog::changeProgress); connect(task, &Task::aborted, [this] { QDialog::reject(); }); + connect(task, &Task::abortStatusChanged, ui->skipButton, &QPushButton::setEnabled); m_is_multi_step = task->isMultiStep(); if (!m_is_multi_step) { -- cgit From eda6cf11ef53c11ed2691399ec1adbc83ca0a4d6 Mon Sep 17 00:00:00 2001 From: flow 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 --- 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(-) 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 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& files, bool set_managed_info) +bool ModrinthCreationTask::parseManifest(QString index_path, std::vector& 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&, bool set_managed_info = true); + bool parseManifest(QString, std::vector&, 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 Date: Fri, 5 Aug 2022 21:25:21 -0300 Subject: fix: some abort-related issues Signed-off-by: flow --- 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(-) 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 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 --- launcher/InstanceCreationTask.cpp | 41 ++++++++++++++++------ launcher/InstanceCreationTask.h | 2 ++ .../flame/FlameInstanceCreationTask.cpp | 25 +++++++------ .../modrinth/ModrinthInstanceCreationTask.cpp | 22 ++++++++---- 4 files changed, 61 insertions(+), 29 deletions(-) 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 +#include 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 9eb35ea7c8b30c4bfa02e2ba218671902d92ff21 Mon Sep 17 00:00:00 2001 From: flow Date: Sat, 20 Aug 2022 11:33:34 -0300 Subject: fix: don't load specific settings for managed pack info This avoids loading all settings for all instances when searching for one with a specific managed pack name. Signed-off-by: flow --- launcher/BaseInstance.cpp | 48 +++++++++++++++++++++++------------------------ launcher/BaseInstance.h | 12 ++++++------ 2 files changed, 30 insertions(+), 30 deletions(-) diff --git a/launcher/BaseInstance.cpp b/launcher/BaseInstance.cpp index 3b261678..2995df6f 100644 --- a/launcher/BaseInstance.cpp +++ b/launcher/BaseInstance.cpp @@ -114,54 +114,54 @@ QString BaseInstance::getPostExitCommand() return settings()->get("PostExitCommand").toString(); } -bool BaseInstance::isManagedPack() +bool BaseInstance::isManagedPack() const { - return settings()->get("ManagedPack").toBool(); + return m_settings->get("ManagedPack").toBool(); } -QString BaseInstance::getManagedPackType() +QString BaseInstance::getManagedPackType() const { - return settings()->get("ManagedPackType").toString(); + return m_settings->get("ManagedPackType").toString(); } -QString BaseInstance::getManagedPackID() +QString BaseInstance::getManagedPackID() const { - return settings()->get("ManagedPackID").toString(); + return m_settings->get("ManagedPackID").toString(); } -QString BaseInstance::getManagedPackName() +QString BaseInstance::getManagedPackName() const { - return settings()->get("ManagedPackName").toString(); + return m_settings->get("ManagedPackName").toString(); } -QString BaseInstance::getManagedPackVersionID() +QString BaseInstance::getManagedPackVersionID() const { - return settings()->get("ManagedPackVersionID").toString(); + return m_settings->get("ManagedPackVersionID").toString(); } -QString BaseInstance::getManagedPackVersionName() +QString BaseInstance::getManagedPackVersionName() const { - return settings()->get("ManagedPackVersionName").toString(); + return m_settings->get("ManagedPackVersionName").toString(); } void BaseInstance::setManagedPack(const QString& type, const QString& id, const QString& name, const QString& versionId, const QString& version) { - settings()->set("ManagedPack", true); - settings()->set("ManagedPackType", type); - settings()->set("ManagedPackID", id); - settings()->set("ManagedPackName", name); - settings()->set("ManagedPackVersionID", versionId); - settings()->set("ManagedPackVersionName", version); + m_settings->set("ManagedPack", true); + m_settings->set("ManagedPackType", type); + m_settings->set("ManagedPackID", id); + m_settings->set("ManagedPackName", name); + m_settings->set("ManagedPackVersionID", versionId); + m_settings->set("ManagedPackVersionName", version); } void BaseInstance::copyManagedPack(BaseInstance& other) { - settings()->set("ManagedPack", other.isManagedPack()); - settings()->set("ManagedPackType", other.getManagedPackType()); - settings()->set("ManagedPackID", other.getManagedPackID()); - settings()->set("ManagedPackName", other.getManagedPackName()); - settings()->set("ManagedPackVersionID", other.getManagedPackVersionID()); - settings()->set("ManagedPackVersionName", other.getManagedPackVersionName()); + m_settings->set("ManagedPack", other.isManagedPack()); + m_settings->set("ManagedPackType", other.getManagedPackType()); + m_settings->set("ManagedPackID", other.getManagedPackID()); + m_settings->set("ManagedPackName", other.getManagedPackName()); + m_settings->set("ManagedPackVersionID", other.getManagedPackVersionID()); + m_settings->set("ManagedPackVersionName", other.getManagedPackVersionName()); } int BaseInstance::getConsoleMaxLines() const diff --git a/launcher/BaseInstance.h b/launcher/BaseInstance.h index 653d1378..21cc3413 100644 --- a/launcher/BaseInstance.h +++ b/launcher/BaseInstance.h @@ -140,12 +140,12 @@ public: QString getPostExitCommand(); QString getWrapperCommand(); - bool isManagedPack(); - QString getManagedPackType(); - QString getManagedPackID(); - QString getManagedPackName(); - QString getManagedPackVersionID(); - QString getManagedPackVersionName(); + bool isManagedPack() const; + QString getManagedPackType() const; + QString getManagedPackID() const; + QString getManagedPackName() const; + QString getManagedPackVersionID() const; + QString getManagedPackVersionName() const; void setManagedPack(const QString& type, const QString& id, const QString& name, const QString& versionId, const QString& version); void copyManagedPack(BaseInstance& other); -- cgit From be8c6f218cfe3acc31335305bb40284f8f6cb582 Mon Sep 17 00:00:00 2001 From: flow Date: Sun, 21 Aug 2022 09:26:27 -0300 Subject: refactor: setAbortStatus -> setAbortable Signed-off-by: flow --- 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(-) 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 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 --- launcher/modplatform/flame/FlameInstanceCreationTask.cpp | 11 +++++++++++ .../modplatform/modrinth/ModrinthInstanceCreationTask.cpp | 11 +++++++++++ 2 files changed, 22 insertions(+) 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 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 --- 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(-) 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 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 4f6d964217f6addd4f29969168a7d40ed4729dde Mon Sep 17 00:00:00 2001 From: flow Date: Sun, 11 Sep 2022 13:24:58 -0300 Subject: fix: don't change groups when updating an instance Signed-off-by: flow --- launcher/InstanceList.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/launcher/InstanceList.cpp b/launcher/InstanceList.cpp index 47b0e75a..6b0e3a4b 100644 --- a/launcher/InstanceList.cpp +++ b/launcher/InstanceList.cpp @@ -913,9 +913,11 @@ bool InstanceList::commitStagedInstance(QString path, InstanceName const& instan qWarning() << "Failed to move" << path << "to" << destination; return false; } + + m_instanceGroupIndex[instID] = groupName; + m_groupNameCache.insert(groupName); } - m_instanceGroupIndex[instID] = groupName; - m_groupNameCache.insert(groupName); + instanceSet.insert(instID); emit instancesChanged(); -- cgit From 9ff364b0d3c12f9138037cce585c03c850721b82 Mon Sep 17 00:00:00 2001 From: timoreo Date: Mon, 26 Sep 2022 11:50:31 +0200 Subject: huge nit: added const refs, everywhere Signed-off-by: timoreo --- 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(-) 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 + #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 + 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& files, bool set_managed_info, bool show_optional_dialog) +bool ModrinthCreationTask::parseManifest(const QString& index_path, std::vector& 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(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&, bool set_managed_info = true, bool show_optional_dialog = true); + bool parseManifest(const QString&, std::vector&, 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 #include #include +#include #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 Date: Mon, 26 Sep 2022 11:50:55 +0200 Subject: fix: Fixed memory leak Signed-off-by: timoreo --- .../modplatform/modrinth/ModrinthInstanceCreationTask.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) 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