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/ui/pages/modplatform/VanillaPage.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'launcher/ui/pages') 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 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(-) (limited to 'launcher/ui/pages') 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 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(-) (limited to 'launcher/ui/pages') 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