From 01b90809e8be2bef02644ba91feceb21b6f7cb8f Mon Sep 17 00:00:00 2001 From: flow Date: Thu, 20 Oct 2022 16:55:22 -0300 Subject: fix: memory leak when finishing blocked mods job Signed-off-by: flow --- launcher/modplatform/flame/FileResolvingTask.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'launcher/modplatform') diff --git a/launcher/modplatform/flame/FileResolvingTask.cpp b/launcher/modplatform/flame/FileResolvingTask.cpp index 1e7f5559..0b2431f7 100644 --- a/launcher/modplatform/flame/FileResolvingTask.cpp +++ b/launcher/modplatform/flame/FileResolvingTask.cpp @@ -66,7 +66,11 @@ void Flame::FileResolvingTask::netJobFinished() } index++; } - connect(job, &NetJob::finished, this, &Flame::FileResolvingTask::modrinthCheckFinished); + connect(job, &NetJob::finished, this, + [this, &job] { + modrinthCheckFinished(); + job->deleteLater(); + }); job->start(); } -- cgit From a6b13487f0e27c8bea1917720a0c5c1eb1eb206c Mon Sep 17 00:00:00 2001 From: Jamie Mansfield Date: Thu, 20 Oct 2022 18:30:32 +0100 Subject: ATLauncher: Abort install if optional mods dialog is closed This matches the behaviour of ATLauncher. --- launcher/modplatform/atlauncher/ATLPackInstallTask.cpp | 7 ++++++- launcher/modplatform/atlauncher/ATLPackInstallTask.h | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) (limited to 'launcher/modplatform') diff --git a/launcher/modplatform/atlauncher/ATLPackInstallTask.cpp b/launcher/modplatform/atlauncher/ATLPackInstallTask.cpp index a553eafd..68d75943 100644 --- a/launcher/modplatform/atlauncher/ATLPackInstallTask.cpp +++ b/launcher/modplatform/atlauncher/ATLPackInstallTask.cpp @@ -736,7 +736,12 @@ void PackInstallTask::downloadMods() QVector selectedMods; if (!optionalMods.isEmpty()) { setStatus(tr("Selecting optional mods...")); - selectedMods = m_support->chooseOptionalMods(m_version, optionalMods); + auto mods = m_support->chooseOptionalMods(m_version, optionalMods); + if (!mods.has_value()) { + emitAborted(); + return; + } + selectedMods = mods.value(); } setStatus(tr("Downloading mods...")); diff --git a/launcher/modplatform/atlauncher/ATLPackInstallTask.h b/launcher/modplatform/atlauncher/ATLPackInstallTask.h index ed4436f0..78cd87fb 100644 --- a/launcher/modplatform/atlauncher/ATLPackInstallTask.h +++ b/launcher/modplatform/atlauncher/ATLPackInstallTask.h @@ -62,7 +62,7 @@ public: /** * Requests a user interaction to select which optional mods should be installed. */ - virtual QVector chooseOptionalMods(PackVersion version, QVector mods) = 0; + virtual std::optional> chooseOptionalMods(PackVersion version, QVector mods) = 0; /** * Requests a user interaction to select a component version from a given version list -- cgit From 970e4b020ea14a1fbd5abe65517a3e26399280cb Mon Sep 17 00:00:00 2001 From: Sefa Eyeoglu Date: Sat, 22 Oct 2022 14:11:51 +0200 Subject: fix: fix segfault when resolving Flame resources Signed-off-by: Sefa Eyeoglu --- launcher/modplatform/flame/FileResolvingTask.cpp | 14 ++++++-------- launcher/modplatform/flame/FileResolvingTask.h | 3 ++- 2 files changed, 8 insertions(+), 9 deletions(-) (limited to 'launcher/modplatform') diff --git a/launcher/modplatform/flame/FileResolvingTask.cpp b/launcher/modplatform/flame/FileResolvingTask.cpp index 0b2431f7..c50abb8f 100644 --- a/launcher/modplatform/flame/FileResolvingTask.cpp +++ b/launcher/modplatform/flame/FileResolvingTask.cpp @@ -12,6 +12,8 @@ bool Flame::FileResolvingTask::abort() bool aborted = true; if (m_dljob) aborted &= m_dljob->abort(); + if (m_checkJob) + aborted &= m_checkJob->abort(); return aborted ? Task::abort() : false; } @@ -40,7 +42,7 @@ void Flame::FileResolvingTask::netJobFinished() setProgress(1, 3); int index = 0; // job to check modrinth for blocked projects - auto job = new NetJob("Modrinth check", m_network); + m_checkJob = new NetJob("Modrinth check", m_network); blockedProjects = QMap(); auto doc = Json::requireDocument(*result); auto array = Json::requireArray(doc.object()["data"]); @@ -60,19 +62,15 @@ void Flame::FileResolvingTask::netJobFinished() out.resolved = true; }); - job->addNetAction(dl); + m_checkJob->addNetAction(dl); blockedProjects.insert(&out, output); } } index++; } - connect(job, &NetJob::finished, this, - [this, &job] { - modrinthCheckFinished(); - job->deleteLater(); - }); + connect(m_checkJob.get(), &NetJob::finished, this, &Flame::FileResolvingTask::modrinthCheckFinished); - job->start(); + m_checkJob->start(); } void Flame::FileResolvingTask::modrinthCheckFinished() { diff --git a/launcher/modplatform/flame/FileResolvingTask.h b/launcher/modplatform/flame/FileResolvingTask.h index f71b87ce..8fc17ea9 100644 --- a/launcher/modplatform/flame/FileResolvingTask.h +++ b/launcher/modplatform/flame/FileResolvingTask.h @@ -30,8 +30,9 @@ protected slots: private: /* data */ shared_qobject_ptr m_network; Flame::Manifest m_toProcess; - std::shared_ptr result; + std::shared_ptr result; NetJob::Ptr m_dljob; + NetJob::Ptr m_checkJob; void modrinthCheckFinished(); -- cgit From b638a6ae950e472c6e6139cfdbaa2950e848efb0 Mon Sep 17 00:00:00 2001 From: flow Date: Mon, 24 Oct 2022 09:07:41 -0300 Subject: fix: retry mod search job after aborting it This way, we don't get stuck with an aborted job in our way! :o Signed-off-by: flow --- launcher/modplatform/helpers/NetworkModAPI.cpp | 1 + 1 file changed, 1 insertion(+) (limited to 'launcher/modplatform') diff --git a/launcher/modplatform/helpers/NetworkModAPI.cpp b/launcher/modplatform/helpers/NetworkModAPI.cpp index 866e7540..7633030e 100644 --- a/launcher/modplatform/helpers/NetworkModAPI.cpp +++ b/launcher/modplatform/helpers/NetworkModAPI.cpp @@ -15,6 +15,7 @@ void NetworkModAPI::searchMods(CallerType* caller, SearchArgs&& args) const QObject::connect(netJob, &NetJob::started, caller, [caller, netJob] { caller->setActiveJob(netJob); }); QObject::connect(netJob, &NetJob::failed, caller, &CallerType::searchRequestFailed); + QObject::connect(netJob, &NetJob::aborted, caller, &CallerType::searchRequestAborted); QObject::connect(netJob, &NetJob::succeeded, caller, [caller, response] { QJsonParseError parse_error{}; QJsonDocument doc = QJsonDocument::fromJson(*response, &parse_error); -- cgit From 93894f62ffcb8c33caf36e8801e163738cc2873d Mon Sep 17 00:00:00 2001 From: Sefa Eyeoglu Date: Fri, 28 Oct 2022 00:05:11 +0200 Subject: fix: avoid segfault for unexpected API reponse Signed-off-by: Sefa Eyeoglu --- .../modplatform/modrinth/ModrinthPackIndex.cpp | 55 ++++++++++++---------- 1 file changed, 31 insertions(+), 24 deletions(-) (limited to 'launcher/modplatform') diff --git a/launcher/modplatform/modrinth/ModrinthPackIndex.cpp b/launcher/modplatform/modrinth/ModrinthPackIndex.cpp index 3e53becb..ae45e096 100644 --- a/launcher/modplatform/modrinth/ModrinthPackIndex.cpp +++ b/launcher/modplatform/modrinth/ModrinthPackIndex.cpp @@ -1,20 +1,20 @@ // SPDX-License-Identifier: GPL-3.0-only /* -* PolyMC - Minecraft Launcher -* Copyright (c) 2022 flowln -* -* This program is free software: you can redistribute it and/or modify -* it under the terms of the GNU General Public License as published by -* the Free Software Foundation, version 3. -* -* This program is distributed in the hope that it will be useful, -* but WITHOUT ANY WARRANTY; without even the implied warranty of -* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -* GNU General Public License for more details. -* -* You should have received a copy of the GNU General Public License -* along with this program. If not, see . -*/ + * PolyMC - Minecraft Launcher + * Copyright (c) 2022 flowln + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, version 3. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ #include "ModrinthPackIndex.h" #include "ModrinthAPI.h" @@ -35,7 +35,7 @@ void Modrinth::loadIndexedPack(ModPlatform::IndexedPack& pack, QJsonObject& obj) pack.provider = ModPlatform::Provider::MODRINTH; pack.name = Json::requireString(obj, "title"); - + pack.slug = Json::ensureString(obj, "slug", ""); if (!pack.slug.isEmpty()) pack.websiteUrl = "https://modrinth.com/mod/" + pack.slug; @@ -59,23 +59,23 @@ void Modrinth::loadIndexedPack(ModPlatform::IndexedPack& pack, QJsonObject& obj) void Modrinth::loadExtraPackData(ModPlatform::IndexedPack& pack, QJsonObject& obj) { pack.extraData.issuesUrl = Json::ensureString(obj, "issues_url"); - if(pack.extraData.issuesUrl.endsWith('/')) + if (pack.extraData.issuesUrl.endsWith('/')) pack.extraData.issuesUrl.chop(1); pack.extraData.sourceUrl = Json::ensureString(obj, "source_url"); - if(pack.extraData.sourceUrl.endsWith('/')) + if (pack.extraData.sourceUrl.endsWith('/')) pack.extraData.sourceUrl.chop(1); pack.extraData.wikiUrl = Json::ensureString(obj, "wiki_url"); - if(pack.extraData.wikiUrl.endsWith('/')) + if (pack.extraData.wikiUrl.endsWith('/')) pack.extraData.wikiUrl.chop(1); pack.extraData.discordUrl = Json::ensureString(obj, "discord_url"); - if(pack.extraData.discordUrl.endsWith('/')) + if (pack.extraData.discordUrl.endsWith('/')) pack.extraData.discordUrl.chop(1); auto donate_arr = Json::ensureArray(obj, "donation_urls"); - for(auto d : donate_arr){ + for (auto d : donate_arr) { auto d_obj = Json::requireObject(d); ModPlatform::DonationData donate; @@ -104,7 +104,7 @@ void Modrinth::loadIndexedPackVersions(ModPlatform::IndexedPack& pack, auto obj = versionIter.toObject(); auto file = loadIndexedPackVersion(obj); - if(file.fileId.isValid()) // Heuristic to check if the returned value is valid + if (file.fileId.isValid()) // Heuristic to check if the returned value is valid unsortedVersions.append(file); } auto orderSortPredicate = [](const ModPlatform::IndexedVersion& a, const ModPlatform::IndexedVersion& b) -> bool { @@ -116,7 +116,8 @@ void Modrinth::loadIndexedPackVersions(ModPlatform::IndexedPack& pack, pack.versionsLoaded = true; } -auto Modrinth::loadIndexedPackVersion(QJsonObject &obj, QString preferred_hash_type, QString preferred_file_name) -> ModPlatform::IndexedVersion +auto Modrinth::loadIndexedPackVersion(QJsonObject& obj, QString preferred_hash_type, QString preferred_file_name) + -> ModPlatform::IndexedVersion { ModPlatform::IndexedVersion file; @@ -141,6 +142,12 @@ auto Modrinth::loadIndexedPackVersion(QJsonObject &obj, QString preferred_hash_t auto files = Json::requireArray(obj, "files"); int i = 0; + if (files.empty()) { + // This should not happen normally, but check just in case + qWarning() << "Modrinth returned an unexpected empty list of files:" << obj; + return {}; + } + // Find correct file (needed in cases where one version may have multiple files) // Will default to the last one if there's no primary (though I think Modrinth requires that // at least one file is primary, idk) @@ -167,7 +174,7 @@ auto Modrinth::loadIndexedPackVersion(QJsonObject &obj, QString preferred_hash_t file.fileName = Json::requireString(parent, "filename"); file.is_preferred = Json::requireBoolean(parent, "primary") || (files.count() == 1); auto hash_list = Json::requireObject(parent, "hashes"); - + if (hash_list.contains(preferred_hash_type)) { file.hash = Json::requireString(hash_list, preferred_hash_type); file.hash_type = preferred_hash_type; -- cgit From e6e92f2b0e9ef16bdbae371ccb284339a70cff02 Mon Sep 17 00:00:00 2001 From: flow Date: Fri, 28 Oct 2022 13:31:21 -0300 Subject: fix: only allow workarounds for blocked mods from MR when 100% safe If a version on Modrinth has more than a single mod loader associated, it means that it's possible we might get the wrong file for download, since individual files don't really have this kind of metadata in the API response. So, in such cases, it's best to let the user take care of it instead. Signed-off-by: flow --- launcher/modplatform/flame/FileResolvingTask.cpp | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) (limited to 'launcher/modplatform') diff --git a/launcher/modplatform/flame/FileResolvingTask.cpp b/launcher/modplatform/flame/FileResolvingTask.cpp index c50abb8f..25b56fbd 100644 --- a/launcher/modplatform/flame/FileResolvingTask.cpp +++ b/launcher/modplatform/flame/FileResolvingTask.cpp @@ -3,6 +3,8 @@ #include "Json.h" #include "net/Upload.h" +#include "modplatform/modrinth/ModrinthPackIndex.h" + Flame::FileResolvingTask::FileResolvingTask(const shared_qobject_ptr& network, Flame::Manifest& toProcess) : m_network(network), m_toProcess(toProcess) {} @@ -84,18 +86,21 @@ void Flame::FileResolvingTask::modrinthCheckFinished() { delete bytes; continue; } + QJsonDocument doc = QJsonDocument::fromJson(*bytes); auto obj = doc.object(); - auto array = Json::requireArray(obj,"files"); - for (auto file: array) { - auto fileObj = Json::requireObject(file); - auto primary = Json::requireBoolean(fileObj,"primary"); - if (primary) { - out->url = Json::requireUrl(fileObj,"url"); - qDebug() << "Found alternative on modrinth " << out->fileName; - break; - } + auto file = Modrinth::loadIndexedPackVersion(obj); + + // If there's more than one mod loader for this version, we can't know for sure + // which file is relative to each loader, so it's best to not use any one and + // let the user download it manually. + if (file.loaders.size() <= 1) { + out->url = file.downloadUrl; + qDebug() << "Found alternative on modrinth " << out->fileName; + } else { + out->resolved = false; } + delete bytes; } //copy to an output list and filter out projects found on modrinth -- cgit From 7956e6f04e32448b0043a49ff08873e117ba0a0b Mon Sep 17 00:00:00 2001 From: flow Date: Tue, 1 Nov 2022 19:48:26 -0300 Subject: fix: don't use forward-declared Ptr types in meta/ This would cause ODR violations when those headers were included in other places that also included stuff like "Version.h" (note the "meta/Version.h"), which can cause problems, especially in LTO. Signed-off-by: flow --- launcher/modplatform/atlauncher/ATLPackInstallTask.cpp | 4 ++-- launcher/modplatform/atlauncher/ATLPackInstallTask.h | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) (limited to 'launcher/modplatform') diff --git a/launcher/modplatform/atlauncher/ATLPackInstallTask.cpp b/launcher/modplatform/atlauncher/ATLPackInstallTask.cpp index 68d75943..291ad916 100644 --- a/launcher/modplatform/atlauncher/ATLPackInstallTask.cpp +++ b/launcher/modplatform/atlauncher/ATLPackInstallTask.cpp @@ -58,7 +58,7 @@ namespace ATLauncher { -static Meta::VersionPtr getComponentVersion(const QString& uid, const QString& version); +static Meta::Version::Ptr getComponentVersion(const QString& uid, const QString& version); PackInstallTask::PackInstallTask(UserInteractionSupport *support, QString packName, QString version, InstallMode installMode) { @@ -1037,7 +1037,7 @@ void PackInstallTask::install() emitSucceeded(); } -static Meta::VersionPtr getComponentVersion(const QString& uid, const QString& version) +static Meta::Version::Ptr getComponentVersion(const QString& uid, const QString& version) { auto vlist = APPLICATION->metadataIndex()->get(uid); if (!vlist) diff --git a/launcher/modplatform/atlauncher/ATLPackInstallTask.h b/launcher/modplatform/atlauncher/ATLPackInstallTask.h index 78cd87fb..90e25ae2 100644 --- a/launcher/modplatform/atlauncher/ATLPackInstallTask.h +++ b/launcher/modplatform/atlauncher/ATLPackInstallTask.h @@ -68,7 +68,7 @@ public: * Requests a user interaction to select a component version from a given version list * and constrained to a given Minecraft version. */ - virtual QString chooseVersion(Meta::VersionListPtr vlist, QString minecraftVersion) = 0; + virtual QString chooseVersion(Meta::VersionList::Ptr vlist, QString minecraftVersion) = 0; /** * Requests a user interaction to display a message. @@ -137,8 +137,8 @@ private: QString archivePath; QStringList jarmods; - Meta::VersionPtr minecraftVersion; - QMap componentsToInstall; + Meta::Version::Ptr minecraftVersion; + QMap componentsToInstall; QFuture> m_extractFuture; QFutureWatcher> m_extractFutureWatcher; -- cgit