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/ui/dialogs/NewInstanceDialog.cpp | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) (limited to 'launcher/ui/dialogs/NewInstanceDialog.cpp') 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; -- 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(-) (limited to 'launcher/ui/dialogs/NewInstanceDialog.cpp') diff --git a/launcher/InstanceList.cpp b/launcher/InstanceList.cpp index 6b0e3a4b..cebd70d7 100644 --- a/launcher/InstanceList.cpp +++ b/launcher/InstanceList.cpp @@ -535,7 +535,7 @@ InstancePtr InstanceList::getInstanceById(QString instId) const return InstancePtr(); } -InstancePtr InstanceList::getInstanceByManagedName(QString managed_name) const +InstancePtr InstanceList::getInstanceByManagedName(const QString& managed_name) const { if (managed_name.isEmpty()) return {}; @@ -880,7 +880,7 @@ QString InstanceList::getStagedInstancePath() return path; } -bool InstanceList::commitStagedInstance(QString path, InstanceName const& instanceName, QString groupName, bool should_override) +bool InstanceList::commitStagedInstance(const QString& path, InstanceName const& instanceName, const QString& groupName, bool should_override) { QDir dir; QString instID; diff --git a/launcher/InstanceList.h b/launcher/InstanceList.h index df11b55a..3673298f 100644 --- a/launcher/InstanceList.h +++ b/launcher/InstanceList.h @@ -104,7 +104,7 @@ public: /* O(n) */ InstancePtr getInstanceById(QString id) const; /* O(n) */ - InstancePtr getInstanceByManagedName(QString managed_name) const; + InstancePtr getInstanceByManagedName(const QString& managed_name) const; QModelIndex getInstanceIndexById(const QString &id) const; QStringList getGroups(); bool isGroupCollapsed(const QString &groupName); @@ -133,7 +133,7 @@ public: * should_override is used when another similar instance already exists, and we want to override it * - for instance, when updating it. */ - bool commitStagedInstance(QString keyPath, const InstanceName& instanceName, QString groupName, bool should_override); + bool commitStagedInstance(const QString& keyPath, const InstanceName& instanceName, const QString& groupName, bool should_override); /** * Destroy a previously created staging area given by @keyPath - used when creation fails. diff --git a/launcher/InstanceTask.cpp b/launcher/InstanceTask.cpp index da280731..55a44fd3 100644 --- a/launcher/InstanceTask.cpp +++ b/launcher/InstanceTask.cpp @@ -2,7 +2,7 @@ #include "ui/dialogs/CustomMessageBox.h" -InstanceNameChange askForChangingInstanceName(QWidget* parent, QString old_name, QString new_name) +InstanceNameChange askForChangingInstanceName(QWidget* parent, const QString& old_name, const QString& new_name) { auto dialog = CustomMessageBox::selectable(parent, QObject::tr("Change instance name"), diff --git a/launcher/InstanceTask.h b/launcher/InstanceTask.h index 0987b557..e35533fc 100644 --- a/launcher/InstanceTask.h +++ b/launcher/InstanceTask.h @@ -5,7 +5,7 @@ /* Helpers */ enum class InstanceNameChange { ShouldChange, ShouldKeep }; -[[nodiscard]] InstanceNameChange askForChangingInstanceName(QWidget* parent, QString old_name, QString new_name); +[[nodiscard]] InstanceNameChange askForChangingInstanceName(QWidget* parent, const QString& old_name, const QString& new_name); struct InstanceName { public: diff --git a/launcher/minecraft/VanillaInstanceCreationTask.cpp b/launcher/minecraft/VanillaInstanceCreationTask.cpp index fb11379c..c45daa9a 100644 --- a/launcher/minecraft/VanillaInstanceCreationTask.cpp +++ b/launcher/minecraft/VanillaInstanceCreationTask.cpp @@ -1,12 +1,14 @@ #include "VanillaInstanceCreationTask.h" +#include + #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