From 46e403b20b8f14269aebc163f2bc481d3dea43c5 Mon Sep 17 00:00:00 2001 From: flow Date: Thu, 9 Jun 2022 19:53:29 -0300 Subject: [PATCH 1/6] fix: properly parse mrpacks without the 'env' field It's optional, so some files may not have it (like most of FO). --- launcher/InstanceImportTask.cpp | 35 +++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/launcher/InstanceImportTask.cpp b/launcher/InstanceImportTask.cpp index 09c2a333..73f05d44 100644 --- a/launcher/InstanceImportTask.cpp +++ b/launcher/InstanceImportTask.cpp @@ -582,6 +582,7 @@ void InstanceImportTask::processMultiMC() emitSucceeded(); } +// https://docs.modrinth.com/docs/modpacks/format_definition/ void InstanceImportTask::processModrinth() { std::vector files; @@ -600,26 +601,30 @@ void InstanceImportTask::processModrinth() auto jsonFiles = Json::requireIsArrayOf(obj, "files", "modrinth.index.json"); bool had_optional = false; - for (auto& modInfo : jsonFiles) { + for (auto modInfo : jsonFiles) { Modrinth::File file; file.path = Json::requireString(modInfo, "path"); auto env = Json::ensureObject(modInfo, "env"); - 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(); - } + // '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"; + if (file.path.endsWith(".jar")) + file.path += ".disabled"; + } } QJsonObject hashes = Json::requireObject(modInfo, "hashes"); From 1b878030aaba832ab416786c5f0dbc69da0e2166 Mon Sep 17 00:00:00 2001 From: flow Date: Thu, 9 Jun 2022 19:54:50 -0300 Subject: [PATCH 2/6] fix: enable using more than one download url in mrpacks Kinda, it's ugly and hackish, since we don't have the facilities to do this properly (yet!) --- launcher/InstanceImportTask.cpp | 52 ++++++++++++++----- .../modrinth/ModrinthPackManifest.h | 7 ++- 2 files changed, 43 insertions(+), 16 deletions(-) diff --git a/launcher/InstanceImportTask.cpp b/launcher/InstanceImportTask.cpp index 73f05d44..74991e36 100644 --- a/launcher/InstanceImportTask.cpp +++ b/launcher/InstanceImportTask.cpp @@ -645,18 +645,32 @@ void InstanceImportTask::processModrinth() } 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(); - file.download = Json::requireString(Json::ensureArray(modInfo, "downloads").first(), "Download URL for " + file.path); + auto download_url = QUrl(download.toString()); - if (!file.download.isValid()) { - qDebug() << QString("Download URL (%1) for %2 is not a correctly formatted URL").arg(file.download.toString(), file.path); - throw JSONValidationError(tr("Download URL for %1 is not a correctly formatted URL").arg(file.path)); - } - else if (!Modrinth::validateDownloadUrl(file.download)) { - qDebug() << QString("Download URL (%1) for %2 is from a non-whitelisted by Modrinth domain").arg(file.download.toString(), file.path); - non_whitelisted_files.push_back(file); + 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 { + if (!Modrinth::validateDownloadUrl(download_url)) { + qDebug() << QString("Download URL (%1) for %2 is from a non-whitelisted by Modrinth domain").arg(download_url.toString(), file.path); + if(is_last && file.downloads.isEmpty()) + non_whitelisted_files.push_back(file); + } + + file.downloads.push_back(download_url); + } } files.push_back(file); @@ -665,7 +679,10 @@ void InstanceImportTask::processModrinth() if (!non_whitelisted_files.empty()) { QString text; for (const auto& file : non_whitelisted_files) { - text += tr("Filepath: %1
URL: %2
").arg(file.path, file.download.toString()); + text += tr("Filepath: %1
").arg(file.path); + for(auto d : file.downloads) + text += tr("URL:") + QString("%2").arg(d.toString()); + text += "
"; } auto message_dialog = new ScrollMessageBox(m_parent, tr("Non-whitelisted mods found"), @@ -740,13 +757,24 @@ void InstanceImportTask::processModrinth() instance.saveNow(); m_filesNetJob = new NetJob(tr("Mod download"), APPLICATION->network()); - for (auto &file : files) + for (auto file : files) { auto path = FS::PathCombine(m_stagingPath, ".minecraft", file.path); - qDebug() << "Will download" << file.download << "to" << path; - auto dl = Net::Download::makeFile(file.download, path); + qDebug() << "Will try to download" << file.downloads.front() << "to" << path; + auto dl = Net::Download::makeFile(file.downloads.front(), path); dl->addValidator(new Net::ChecksumValidator(file.hashAlgorithm, file.hash)); m_filesNetJob->addNetAction(dl); + + if (file.downloads.size() > 1) { + // 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, [&]() { diff --git a/launcher/modplatform/modrinth/ModrinthPackManifest.h b/launcher/modplatform/modrinth/ModrinthPackManifest.h index e5fc9a70..b2083f57 100644 --- a/launcher/modplatform/modrinth/ModrinthPackManifest.h +++ b/launcher/modplatform/modrinth/ModrinthPackManifest.h @@ -40,6 +40,7 @@ #include #include +#include #include #include #include @@ -48,14 +49,12 @@ class MinecraftInstance; namespace Modrinth { -struct File -{ +struct File { QString path; QCryptographicHash::Algorithm hashAlgorithm; QByteArray hash; - // TODO: should this support multiple download URLs, like the JSON does? - QUrl download; + QQueue downloads; }; struct ModpackExtra { From b3c8f9d508b9110c13b3abf0f54d3f4927292559 Mon Sep 17 00:00:00 2001 From: flow Date: Thu, 9 Jun 2022 19:57:51 -0300 Subject: [PATCH 3/6] revert: don't check modrinth whitelisted hosts people didn't seem to like it, and its not required --- launcher/InstanceImportTask.cpp | 27 ------------------- .../modrinth/ModrinthPackManifest.cpp | 16 ----------- 2 files changed, 43 deletions(-) diff --git a/launcher/InstanceImportTask.cpp b/launcher/InstanceImportTask.cpp index 74991e36..1ccf7ffc 100644 --- a/launcher/InstanceImportTask.cpp +++ b/launcher/InstanceImportTask.cpp @@ -586,7 +586,6 @@ void InstanceImportTask::processMultiMC() void InstanceImportTask::processModrinth() { std::vector files; - std::vector non_whitelisted_files; QString minecraftVersion, fabricVersion, quiltVersion, forgeVersion; try { QString indexPath = FS::PathCombine(m_stagingPath, "modrinth.index.json"); @@ -663,12 +662,6 @@ void InstanceImportTask::processModrinth() throw JSONValidationError(tr("Download URL for %1 is not a correctly formatted URL").arg(file.path)); } else { - if (!Modrinth::validateDownloadUrl(download_url)) { - qDebug() << QString("Download URL (%1) for %2 is from a non-whitelisted by Modrinth domain").arg(download_url.toString(), file.path); - if(is_last && file.downloads.isEmpty()) - non_whitelisted_files.push_back(file); - } - file.downloads.push_back(download_url); } } @@ -676,26 +669,6 @@ void InstanceImportTask::processModrinth() files.push_back(file); } - if (!non_whitelisted_files.empty()) { - QString text; - for (const auto& file : non_whitelisted_files) { - text += tr("Filepath: %1
").arg(file.path); - for(auto d : file.downloads) - text += tr("URL:") + QString("%2").arg(d.toString()); - text += "
"; - } - - auto message_dialog = new ScrollMessageBox(m_parent, tr("Non-whitelisted mods found"), - tr("The following mods have URLs that are not whitelisted by Modrinth.\n" - "Proceed with caution!"), - text); - message_dialog->setModal(true); - if (message_dialog->exec() == QDialog::Rejected) { - emitFailed("Aborted"); - return; - } - } - auto dependencies = Json::requireObject(obj, "dependencies", "modrinth.index.json"); for (auto it = dependencies.begin(), end = dependencies.end(); it != end; ++it) { QString name = it.key(); diff --git a/launcher/modplatform/modrinth/ModrinthPackManifest.cpp b/launcher/modplatform/modrinth/ModrinthPackManifest.cpp index 33116231..cc12f62f 100644 --- a/launcher/modplatform/modrinth/ModrinthPackManifest.cpp +++ b/launcher/modplatform/modrinth/ModrinthPackManifest.cpp @@ -95,19 +95,6 @@ void loadIndexedVersions(Modpack& pack, QJsonDocument& doc) pack.versionsLoaded = true; } -auto validateDownloadUrl(QUrl url) -> bool -{ - static QSet domainWhitelist{ - "cdn.modrinth.com", - "github.com", - "raw.githubusercontent.com", - "gitlab.com" - }; - - auto domain = url.host(); - return domainWhitelist.contains(domain); -} - auto loadIndexedVersion(QJsonObject &obj) -> ModpackVersion { ModpackVersion file; @@ -137,9 +124,6 @@ auto loadIndexedVersion(QJsonObject &obj) -> ModpackVersion auto url = Json::requireString(parent, "url"); - if(!validateDownloadUrl(url)) - continue; - file.download_url = url; if(is_primary) break; From 54144154f9761726edda4adf811e86b883f9603b Mon Sep 17 00:00:00 2001 From: flow Date: Sat, 11 Jun 2022 13:43:09 -0300 Subject: [PATCH 4/6] fix: apply client overrides in mrpacks another oopsie x.x --- launcher/FileSystem.cpp | 43 +++++++++++++++++++++++++++++++++ launcher/FileSystem.h | 4 +++ launcher/InstanceImportTask.cpp | 18 +++++++++++--- 3 files changed, 61 insertions(+), 4 deletions(-) diff --git a/launcher/FileSystem.cpp b/launcher/FileSystem.cpp index 6de20de6..3837d75f 100644 --- a/launcher/FileSystem.cpp +++ b/launcher/FileSystem.cpp @@ -454,4 +454,47 @@ bool createShortCut(QString location, QString dest, QStringList args, QString na return false; #endif } + +QStringList listFolderPaths(QDir root) +{ + auto createAbsPath = [](QFileInfo const& entry) { return FS::PathCombine(entry.path(), entry.fileName()); }; + + QStringList entries; + + root.refresh(); + for (auto entry : root.entryInfoList(QDir::Filter::Files)) { + entries.append(createAbsPath(entry)); + } + + for (auto entry : root.entryInfoList(QDir::Filter::AllDirs | QDir::Filter::NoDotAndDotDot)) { + entries.append(listFolderPaths(createAbsPath(entry))); + } + + return entries; +} + +bool overrideFolder(QString overwritten_path, QString override_path) +{ + if (!FS::ensureFolderPathExists(overwritten_path)) + return false; + + QStringList paths_to_override; + QDir root_override (override_path); + for (auto file : listFolderPaths(root_override)) { + QString destination = file; + destination.replace(override_path, overwritten_path); + + qDebug() << QString("Applying override %1 in %2").arg(file, destination); + + if (QFile::exists(destination)) + QFile::remove(destination); + if (!QFile::rename(file, destination)) { + qCritical() << QString("Failed to apply override from %1 to %2").arg(file, destination); + return false; + } + } + + return true; +} + } diff --git a/launcher/FileSystem.h b/launcher/FileSystem.h index 8f6e8b48..bc942ab3 100644 --- a/launcher/FileSystem.h +++ b/launcher/FileSystem.h @@ -124,4 +124,8 @@ QString getDesktopDir(); // call it *name* and assign it the icon *icon* // return true if operation succeeded bool createShortCut(QString location, QString dest, QStringList args, QString name, QString iconLocation); + +// Overrides one folder with the contents of another, preserving items exclusive to the first folder +// Equivalent to doing QDir::rename, but allowing for overrides +bool overrideFolder(QString overwritten_path, QString override_path); } diff --git a/launcher/InstanceImportTask.cpp b/launcher/InstanceImportTask.cpp index 1ccf7ffc..3dcd92c8 100644 --- a/launcher/InstanceImportTask.cpp +++ b/launcher/InstanceImportTask.cpp @@ -696,16 +696,26 @@ void InstanceImportTask::processModrinth() emitFailed(tr("Could not understand pack index:\n") + e.cause()); return; } + + auto mcPath = FS::PathCombine(m_stagingPath, ".minecraft"); - QString overridePath = FS::PathCombine(m_stagingPath, "overrides"); - if (QFile::exists(overridePath)) { - QString mcPath = FS::PathCombine(m_stagingPath, ".minecraft"); - if (!QFile::rename(overridePath, mcPath)) { + 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); From 29e5a213a5be2d3716018b64241ac030ca2b0af5 Mon Sep 17 00:00:00 2001 From: flow Date: Sat, 11 Jun 2022 14:19:51 -0300 Subject: [PATCH 5/6] fix: dequeue first added file in mrpack import Co-authored-by: Sefa Eyeoglu --- launcher/InstanceImportTask.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/launcher/InstanceImportTask.cpp b/launcher/InstanceImportTask.cpp index 3dcd92c8..1498db6f 100644 --- a/launcher/InstanceImportTask.cpp +++ b/launcher/InstanceImportTask.cpp @@ -744,7 +744,7 @@ void InstanceImportTask::processModrinth() { 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.front(), path); + auto dl = Net::Download::makeFile(file.downloads.dequeue(), path); dl->addValidator(new Net::ChecksumValidator(file.hashAlgorithm, file.hash)); m_filesNetJob->addNetAction(dl); From 37160f973f1d2fed450f40f38a55b8445787c4bd Mon Sep 17 00:00:00 2001 From: flow Date: Sat, 11 Jun 2022 14:31:50 -0300 Subject: [PATCH 6/6] fix: account for the dequeued url when checking the number of urls Co-authored-by: Sefa Eyeoglu --- launcher/InstanceImportTask.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/launcher/InstanceImportTask.cpp b/launcher/InstanceImportTask.cpp index 1498db6f..d5684805 100644 --- a/launcher/InstanceImportTask.cpp +++ b/launcher/InstanceImportTask.cpp @@ -748,7 +748,7 @@ void InstanceImportTask::processModrinth() dl->addValidator(new Net::ChecksumValidator(file.hashAlgorithm, file.hash)); m_filesNetJob->addNetAction(dl); - if (file.downloads.size() > 1) { + 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]{