From 6659fb43b81d243de8b0bc78a2f4e8654a575d13 Mon Sep 17 00:00:00 2001 From: HowXu Date: Sat, 24 Oct 2026 11:41:35 +0800 Subject: [PATCH 01/13] feat(modpacks): Persist pending native manifests Add native files-only filtering and atomic persistence, with one lightweight Qt Test target. Assisted-by: OpenCode:gpt-6.1-sol-fast Signed-off-by: HowXu --- launcher/CMakeLists.txt | 2 + .../helpers/PendingModpackManifest.cpp | 112 +++++++++++ .../helpers/PendingModpackManifest.h | 33 ++++ tests/CMakeLists.txt | 5 + tests/PendingModpackManifest_test.cpp | 177 ++++++++++++++++++ 5 files changed, 329 insertions(+) create mode 100644 launcher/modplatform/helpers/PendingModpackManifest.cpp create mode 100644 launcher/modplatform/helpers/PendingModpackManifest.h create mode 100644 tests/PendingModpackManifest_test.cpp diff --git a/launcher/CMakeLists.txt b/launcher/CMakeLists.txt index 8f0be97de..a14f6f278 100644 --- a/launcher/CMakeLists.txt +++ b/launcher/CMakeLists.txt @@ -542,6 +542,8 @@ set(API_SOURCES modplatform/helpers/HashUtils.cpp modplatform/helpers/OverrideUtils.h modplatform/helpers/OverrideUtils.cpp + modplatform/helpers/PendingModpackManifest.h + modplatform/helpers/PendingModpackManifest.cpp modplatform/helpers/ExportToModList.h modplatform/helpers/ExportToModList.cpp diff --git a/launcher/modplatform/helpers/PendingModpackManifest.cpp b/launcher/modplatform/helpers/PendingModpackManifest.cpp new file mode 100644 index 000000000..d1d350361 --- /dev/null +++ b/launcher/modplatform/helpers/PendingModpackManifest.cpp @@ -0,0 +1,112 @@ +// SPDX-License-Identifier: GPL-3.0-only + +#include "PendingModpackManifest.h" + +#include +#include +#include +#include +#include +#include + +#include + +namespace Modpack { + +QString pendingManifestPath(const QString& instanceRoot, Format format) +{ + return QDir(instanceRoot).filePath(format == Format::Modrinth ? "mrpack/remaining.index.json" : "flame/remaining.manifest.json"); +} + +PendingManifest::PendingManifest(Format format, QJsonObject original) : m_format(format), m_original(std::move(original)) {} + +QJsonObject PendingManifest::remaining() const +{ + auto result = m_original; + if (!m_original.value("files").isArray()) + return result; + + QJsonArray files; + for (const auto& value : m_original.value("files").toArray()) { + const auto file = value.toObject(); + QString key; + if (m_format == Format::Modrinth) { + key = file.value("path").toString(); + } else if (file.value("projectID").isDouble() && file.value("fileID").isDouble()) { + key = QString::number(file.value("projectID").toInteger()) + ':' + QString::number(file.value("fileID").toInteger()); + } + if (key.isEmpty() || !m_completed.contains(key)) + files.append(value); + } + result.insert("files", files); + return result; +} + +void PendingManifest::markCompleted(const QString& key) +{ + m_completed.insert(key); +} + +bool PendingManifest::save(const QString& path, QString* error) const +{ + if (error) + error->clear(); + if (!m_original.value("files").isArray()) { + if (error) + *error = QObject::tr("The modpack manifest must contain a files array."); + return false; + } + + QSaveFile file(path); + if (!file.open(QIODevice::WriteOnly)) { + if (error) + *error = file.errorString(); + return false; + } + const auto data = QJsonDocument(remaining()).toJson(); + if (file.write(data) != data.size()) { + if (error) + *error = file.errorString(); + file.cancelWriting(); + return false; + } + if (!file.commit()) { + if (error) + *error = file.errorString(); + return false; + } + return true; +} + +std::optional PendingManifest::load(Format format, const QString& path, QString* error) +{ + if (error) + error->clear(); + QFile file(path); + if (!file.open(QIODevice::ReadOnly)) { + if (error) + *error = file.errorString(); + return std::nullopt; + } + const auto data = file.readAll(); + if (file.error() != QFileDevice::NoError) { + if (error) + *error = file.errorString(); + return std::nullopt; + } + QJsonParseError parseError; + const auto document = QJsonDocument::fromJson(data, &parseError); + if (parseError.error != QJsonParseError::NoError) { + if (error) + *error = parseError.errorString(); + return std::nullopt; + } + if (!document.isObject() || !document.object().value("files").isArray()) { + if (error) + *error = QObject::tr("The modpack manifest must be an object containing a files array."); + return std::nullopt; + } + return PendingManifest(format, document.object()); +} + +} // namespace Modpack diff --git a/launcher/modplatform/helpers/PendingModpackManifest.h b/launcher/modplatform/helpers/PendingModpackManifest.h new file mode 100644 index 000000000..07fa0e647 --- /dev/null +++ b/launcher/modplatform/helpers/PendingModpackManifest.h @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: GPL-3.0-only + +#pragma once + +#include +#include +#include + +#include + +namespace Modpack { + +enum class Format { Modrinth, CurseForge }; +enum class InstallMode { Create, Resume }; + +QString pendingManifestPath(const QString& instanceRoot, Format format); + +class PendingManifest { + public: + PendingManifest(Format format, QJsonObject original); + + QJsonObject remaining() const; + void markCompleted(const QString& key); + bool save(const QString& path, QString* error) const; + static std::optional load(Format format, const QString& path, QString* error); + + private: + Format m_format; + QJsonObject m_original; + QSet m_completed; +}; + +} // namespace Modpack diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 300fea2aa..80d27e7d7 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -48,6 +48,11 @@ ecm_add_test(JavaVersion_test.cpp LINK_LIBRARIES Launcher_logic Qt${QT_VERSION_M ecm_add_test(Packwiz_test.cpp LINK_LIBRARIES Launcher_logic Qt${QT_VERSION_MAJOR}::Test TEST_NAME Packwiz) +ecm_add_test(PendingModpackManifest_test.cpp ../launcher/modplatform/helpers/PendingModpackManifest.cpp + LINK_LIBRARIES Qt${QT_VERSION_MAJOR}::Core Qt${QT_VERSION_MAJOR}::Test + TEST_NAME PendingModpackManifest) +target_include_directories(PendingModpackManifest PRIVATE "${CMAKE_SOURCE_DIR}/launcher") + ecm_add_test(Index_test.cpp LINK_LIBRARIES Launcher_logic Qt${QT_VERSION_MAJOR}::Test TEST_NAME Index) diff --git a/tests/PendingModpackManifest_test.cpp b/tests/PendingModpackManifest_test.cpp new file mode 100644 index 000000000..68d355340 --- /dev/null +++ b/tests/PendingModpackManifest_test.cpp @@ -0,0 +1,177 @@ +// SPDX-License-Identifier: GPL-3.0-only + +#include +#include +#include +#include +#include +#include +#include + +#include "modplatform/helpers/PendingModpackManifest.h" + +class PendingModpackManifestTest : public QObject { + Q_OBJECT + + private slots: + void test_FilterModrinth() + { + const auto original = QJsonDocument::fromJson(R"({"files":[ + {"path":"mods/done.jar"}, {"path":"mods/failed.jar"}, {"path":"mods/not-started.jar"} + ]})") + .object(); + Modpack::PendingManifest manifest(Modpack::Format::Modrinth, original); + manifest.markCompleted("mods/done.jar"); + manifest.markCompleted("mods/done.jar"); + manifest.markCompleted("unknown.jar"); + + const auto expected = QJsonDocument::fromJson(R"({"files":[ + {"path":"mods/failed.jar"}, {"path":"mods/not-started.jar"} + ]})") + .object(); + QCOMPARE(manifest.remaining(), expected); + QCOMPARE(original.value("files").toArray().size(), 3); + } + + void test_FilterCurseForge() + { + const auto original = QJsonDocument::fromJson(R"({"files":[ + {"projectID":12,"fileID":34,"required":true}, + {"projectID":12,"fileID":35,"required":false}, + {"projectID":56,"fileID":34,"required":true} + ]})") + .object(); + Modpack::PendingManifest manifest(Modpack::Format::CurseForge, original); + manifest.markCompleted("12:34"); + const auto expected = QJsonDocument::fromJson(R"({"files":[ + {"projectID":12,"fileID":35,"required":false}, + {"projectID":56,"fileID":34,"required":true} + ]})") + .object(); + QCOMPARE(manifest.remaining(), expected); + } + + void test_PreserveOriginalFields() + { + const auto original = QJsonDocument::fromJson(R"({ + "formatVersion":1,"name":"Pack","dependencies":{"minecraft":"1.21.1","fabric-loader":"0.16.0"}, + "unknown":{"nested":[1,true,null]},"files":[ + {"path":"mods/done.jar"}, + {"path":"mods/optional.jar","env":{"client":"optional","server":"unsupported"}, + "downloads":["https:\/\/first.example/mod.jar","https:\/\/backup.example/mod.jar"], + "hashes":{"sha512":"original-hash"},"fileSize":42,"unknown":[false,{"keep":"me"}]} + ] + })") + .object(); + Modpack::PendingManifest manifest(Modpack::Format::Modrinth, original); + manifest.markCompleted("mods/done.jar"); + auto expected = original; + expected.insert("files", QJsonArray{ original.value("files").toArray().at(1) }); + QCOMPARE(manifest.remaining(), expected); + + const auto curseForge = QJsonDocument::fromJson(R"({ + "manifestType":"minecraftModpack","minecraft":{"version":"1.21.1","modLoaders":[{"id":"neoforge-21.1"}]}, + "overrides":"overrides","unknown":true, + "files":[{"projectID":1,"fileID":2,"required":false,"unknown":{"keep":true}}] + })") + .object(); + Modpack::PendingManifest cfManifest(Modpack::Format::CurseForge, curseForge); + QCOMPARE(cfManifest.remaining(), curseForge); + } + + void test_InvalidAndEmptyManifest() + { + QTemporaryDir temporary; + QVERIFY(temporary.isValid()); + const auto path = temporary.filePath("manifest.json"); + QString error; + for (const auto format : { Modpack::Format::Modrinth, Modpack::Format::CurseForge }) { + Modpack::PendingManifest empty(format, QJsonObject{ { "files", QJsonArray{} } }); + QVERIFY2(empty.save(path, &error), qPrintable(error)); + const auto loaded = Modpack::PendingManifest::load(format, path, &error); + QVERIFY2(loaded.has_value(), qPrintable(error)); + QCOMPARE(loaded->remaining(), QJsonObject({ { "files", QJsonArray{} } })); + + const QList invalid{ "{broken", "[]", "{}", R"({"files":null})", R"({"files":{}})", R"({"files":"not an array"})" }; + for (const auto& data : invalid) { + QFile file(path); + QVERIFY(file.open(QIODevice::WriteOnly)); + QCOMPARE(file.write(data), data.size()); + file.close(); + error.clear(); + QVERIFY(!Modpack::PendingManifest::load(format, path, &error).has_value()); + QVERIFY(!error.isEmpty()); + } + + error.clear(); + QVERIFY(!Modpack::PendingManifest::load(format, temporary.filePath("missing.json"), &error).has_value()); + QVERIFY(!error.isEmpty()); + + Modpack::PendingManifest invalidManifest(format, QJsonObject{ { "files", "invalid" } }); + QCOMPARE(invalidManifest.remaining().value("files").toString(), QString("invalid")); + QFile oldFile(path); + QVERIFY(oldFile.open(QIODevice::ReadOnly)); + const auto oldData = oldFile.readAll(); + oldFile.close(); + error.clear(); + QVERIFY(!invalidManifest.save(path, &error)); + QVERIFY(!error.isEmpty()); + QVERIFY(oldFile.open(QIODevice::ReadOnly)); + QCOMPARE(oldFile.readAll(), oldData); + } + } + + void test_SaveAndReload() + { + QTemporaryDir temporary; + QVERIFY(temporary.isValid()); + QCOMPARE(Modpack::pendingManifestPath(temporary.path(), Modpack::Format::Modrinth), + temporary.filePath("mrpack/remaining.index.json")); + QCOMPARE(Modpack::pendingManifestPath(temporary.path(), Modpack::Format::CurseForge), + temporary.filePath("flame/remaining.manifest.json")); + + const auto original = QJsonDocument::fromJson(R"({"name":"Pack","files":[ + {"path":"mods/a.jar","projectID":1,"fileID":2}, + {"path":"mods/b.jar","projectID":3,"fileID":4} + ]})") + .object(); + for (const auto format : { Modpack::Format::Modrinth, Modpack::Format::CurseForge }) { + const auto path = Modpack::pendingManifestPath(temporary.path(), format); + QVERIFY(QDir().mkpath(QFileInfo(path).absolutePath())); + Modpack::PendingManifest manifest(format, original); + QString error; + QVERIFY2(manifest.save(path, &error), qPrintable(error)); + QFile oldFile(path); + QVERIFY(oldFile.open(QIODevice::ReadOnly)); + const auto oldData = oldFile.readAll(); + oldFile.close(); + + manifest.markCompleted(format == Modpack::Format::Modrinth ? "mods/a.jar" : "1:2"); + // A regular file cannot serve as a parent directory, regardless of write permissions. + QVERIFY(!manifest.save(QDir(path).filePath("remaining.json"), &error)); + QVERIFY(!error.isEmpty()); + QVERIFY(oldFile.open(QIODevice::ReadOnly)); + QCOMPARE(oldFile.readAll(), oldData); + oldFile.close(); + + QVERIFY2(manifest.save(path, &error), qPrintable(error)); + auto loaded = Modpack::PendingManifest::load(format, path, &error); + QVERIFY2(loaded.has_value(), qPrintable(error)); + const auto expected = QJsonDocument::fromJson(R"({"name":"Pack","files":[ + {"path":"mods/b.jar","projectID":3,"fileID":4} + ]})") + .object(); + QCOMPARE(loaded->remaining(), expected); + loaded->markCompleted(format == Modpack::Format::Modrinth ? "mods/b.jar" : "3:4"); + QVERIFY2(loaded->save(path, &error), qPrintable(error)); + const auto completed = Modpack::PendingManifest::load(format, path, &error); + QVERIFY2(completed.has_value(), qPrintable(error)); + QCOMPARE(completed->remaining(), QJsonObject({ { "name", "Pack" }, { "files", QJsonArray{} } })); + QVERIFY(!manifest.save(temporary.filePath("missing/manifest.json"), nullptr)); + } + } +}; + +QTEST_GUILESS_MAIN(PendingModpackManifestTest) + +#include "PendingModpackManifest_test.moc" From 6a4ebe9b8edddab8cddccfc1a9fd5bd516ef0b2e Mon Sep 17 00:00:00 2001 From: HowXu Date: Sat, 24 Oct 2026 11:49:25 +0800 Subject: [PATCH 02/13] fix(net): Track download failures and close sinks Distinguish network failures from local and parsing errors, clear stale replies on retries, and close canceled sinks before completion. Cover retry followed by local failure using real loopback networking. Assisted-by: OpenCode:gpt-6.1-sol-fast Signed-off-by: HowXu --- launcher/meta/BaseEntity.cpp | 6 ++- launcher/meta/BaseEntity.h | 3 ++ launcher/minecraft/ComponentUpdateTask.cpp | 42 ++++++++++++++-- launcher/minecraft/ComponentUpdateTask.h | 4 +- launcher/minecraft/MinecraftLoadAndCheck.cpp | 11 +++- launcher/minecraft/MinecraftLoadAndCheck.h | 2 + launcher/minecraft/update/AssetUpdateTask.cpp | 3 ++ launcher/minecraft/update/AssetUpdateTask.h | 2 + .../update/LegacyFMLLibrariesTask.cpp | 3 ++ .../minecraft/update/LegacyFMLLibrariesTask.h | 2 + launcher/minecraft/update/LibrariesTask.cpp | 2 + launcher/minecraft/update/LibrariesTask.h | 2 + launcher/net/FileSink.cpp | 4 ++ launcher/net/NetJob.cpp | 18 ++++++- launcher/net/NetJob.h | 3 +- launcher/net/NetRequest.cpp | 13 ++--- tests/Task_test.cpp | 50 +++++++++++++++++++ 17 files changed, 154 insertions(+), 16 deletions(-) diff --git a/launcher/meta/BaseEntity.cpp b/launcher/meta/BaseEntity.cpp index f2f3e13fb..a6987adfe 100644 --- a/launcher/meta/BaseEntity.cpp +++ b/launcher/meta/BaseEntity.cpp @@ -113,6 +113,7 @@ BaseEntityLoadTask::BaseEntityLoadTask(BaseEntity* parent, Net::Mode mode, bool void BaseEntityLoadTask::executeTask() { + m_downloadFailed = false; const QString fname = QDir("meta").absoluteFilePath(m_entity->localFilename()); auto hashMatches = false; // the file exists on disk try to load it @@ -176,7 +177,10 @@ void BaseEntityLoadTask::executeTask() dl->addValidator(new ParsingValidator(m_entity)); m_task->addNetAction(dl); m_task->setAskRetry(false); - connect(m_task.get(), &Task::failed, this, &BaseEntityLoadTask::emitFailed); + connect(m_task.get(), &Task::failed, this, [this](const QString& reason) { + m_downloadFailed = m_task->hasNetworkFailure(); + emitFailed(reason); + }); connect(m_task.get(), &Task::succeeded, this, [this]() { m_entity->m_load_status = BaseEntity::LoadStatus::Remote; m_entity->m_file_sha256 = m_entity->m_sha256; diff --git a/launcher/meta/BaseEntity.h b/launcher/meta/BaseEntity.h index 32d8bdbb8..fa394ff84 100644 --- a/launcher/meta/BaseEntity.h +++ b/launcher/meta/BaseEntity.h @@ -44,6 +44,7 @@ class BaseEntity { virtual void parse(const QJsonObject& obj) = 0; [[nodiscard]] Task::Ptr loadTask(Net::Mode loadType = Net::Mode::Online, bool forceReload = false); + Task::Ptr currentLoadTask() const { return m_task; } protected: QString m_sha256; // the expected sha256 @@ -64,11 +65,13 @@ class BaseEntityLoadTask : public Task { virtual void executeTask() override; virtual bool canAbort() const override; virtual bool abort() override; + bool downloadFailed() const { return m_downloadFailed; } private: BaseEntity* m_entity; Net::Mode m_mode; bool m_force_reload = false; + bool m_downloadFailed = false; NetJob::Ptr m_task; }; } // namespace Meta diff --git a/launcher/minecraft/ComponentUpdateTask.cpp b/launcher/minecraft/ComponentUpdateTask.cpp index 73203f74b..bc3390c59 100644 --- a/launcher/minecraft/ComponentUpdateTask.cpp +++ b/launcher/minecraft/ComponentUpdateTask.cpp @@ -8,6 +8,7 @@ #include "ProblemProvider.h" #include "Version.h" #include "cassert" +#include "meta/BaseEntity.h" #include "meta/Index.h" #include "meta/Version.h" #include "minecraft/MinecraftInstance.h" @@ -81,6 +82,13 @@ Net::Mode ComponentUpdateTask::netMode() void ComponentUpdateTask::executeTask() { + m_downloadFailed = false; + for (const auto& status : d->remoteLoadStatusList) { + if (status.task) + disconnect(status.task.get(), nullptr, this, nullptr); + } + d->remoteLoadStatusList.clear(); + d->remoteTasksInProgress = 0; qCDebug(instanceProfileResolveC) << "Loading components"; setStatus(tr("Loading components")); loadComponents(); @@ -97,7 +105,7 @@ LoadResult composeLoadResult(LoadResult a, LoadResult b) return a; } -static LoadResult loadComponent(ComponentPtr component, Task::Ptr& loadTask, Net::Mode netmode) +static LoadResult loadComponent(ComponentPtr component, Task::Ptr& loadTask, QList& metadataTasks, Net::Mode netmode) { if (component->m_loaded) { qCDebug(instanceProfileResolveC) << component->getName() << "is already loaded"; @@ -131,7 +139,15 @@ static LoadResult loadComponent(ComponentPtr component, Task::Ptr& loadTask, Net component->m_loaded = true; result = LoadResult::LoadedLocal; } else { - loadTask = APPLICATION->metadataIndex()->loadVersion(component->m_uid, component->m_version, netmode); + auto index = APPLICATION->metadataIndex(); + loadTask = index->loadVersion(component->m_uid, component->m_version, netmode); + // Keep these exact tasks: another component can replace an entity's current load task. + if (netmode == Net::Mode::Online) { + if (index->status() != Meta::BaseEntity::LoadStatus::Remote) + metadataTasks.append(index->currentLoadTask()); + metadataTasks.append(index->get(component->m_uid)->currentLoadTask()); + } + metadataTasks.append(metaVersion->currentLoadTask()); loadTask->start(); if (netmode == Net::Mode::Online) result = LoadResult::RequiresRemote; @@ -183,6 +199,7 @@ void ComponentUpdateTask::loadComponents() // load all the components OR their lists... for (auto component : d->m_profile->d->components) { Task::Ptr loadTask; + QList metadataTasks; LoadResult singleResult; RemoteLoadStatus::Type loadType; component->resetComponentProblems(); @@ -205,7 +222,7 @@ void ComponentUpdateTask::loadComponents() } } #else - singleResult = loadComponent(component, loadTask, d->netmode); + singleResult = loadComponent(component, loadTask, metadataTasks, d->netmode); loadType = RemoteLoadStatus::Type::Version; #endif if (singleResult == LoadResult::LoadedLocal) { @@ -216,7 +233,20 @@ void ComponentUpdateTask::loadComponents() qCDebug(instanceProfileResolveC) << d->m_profile->d->m_instance->name() << "|" << "Remote loading is being run for" << component->getName(); connect(loadTask.get(), &Task::succeeded, this, [this, taskIndex]() { remoteLoadSucceeded(taskIndex); }); - connect(loadTask.get(), &Task::failed, this, [this, taskIndex](const QString& error) { remoteLoadFailed(taskIndex, error); }); + connect(loadTask.get(), &Task::failed, this, [this, taskIndex, metadataTasks](const QString& error) { + bool downloadFailed = false; + for (const auto& task : metadataTasks) { + if (!task || task->getState() != Task::State::Failed) + continue; + auto metadataTask = qobject_cast(task.get()); + if (!metadataTask || !metadataTask->downloadFailed()) { + downloadFailed = false; + break; + } + downloadFailed = true; + } + remoteLoadFailed(taskIndex, error, downloadFailed); + }); connect(loadTask.get(), &Task::aborted, this, [this, taskIndex]() { remoteLoadFailed(taskIndex, tr("Aborted")); }); RemoteLoadStatus status; status.type = loadType; @@ -765,7 +795,7 @@ void ComponentUpdateTask::remoteLoadSucceeded(size_t taskIndex) checkIfAllFinished(); } -void ComponentUpdateTask::remoteLoadFailed(size_t taskIndex, const QString& msg) +void ComponentUpdateTask::remoteLoadFailed(size_t taskIndex, const QString& msg, bool downloadFailed) { if (static_cast(d->remoteLoadStatusList.size()) < taskIndex) { qCWarning(instanceProfileResolveC) << "Got task index outside of results" << taskIndex; @@ -780,6 +810,8 @@ void ComponentUpdateTask::remoteLoadFailed(size_t taskIndex, const QString& msg) return; } qCDebug(instanceProfileResolveC) << "Remote task" << taskIndex << "failed:" << msg; + // Only classify the combined failure as a download failure if every failed load agrees. + m_downloadFailed = (d->remoteLoadSuccessful || m_downloadFailed) && downloadFailed; d->remoteLoadSuccessful = false; taskSlot.succeeded = false; taskSlot.finished = true; diff --git a/launcher/minecraft/ComponentUpdateTask.h b/launcher/minecraft/ComponentUpdateTask.h index 2ef9737ba..075054e96 100644 --- a/launcher/minecraft/ComponentUpdateTask.h +++ b/launcher/minecraft/ComponentUpdateTask.h @@ -20,6 +20,7 @@ class ComponentUpdateTask : public Task { bool canAbort() const override; bool abort() override; Net::Mode netMode(); + bool downloadFailed() const { return m_downloadFailed; } protected: void executeTask() override; @@ -33,9 +34,10 @@ class ComponentUpdateTask : public Task { void finalizeComponents(); void remoteLoadSucceeded(size_t index); - void remoteLoadFailed(size_t index, const QString& msg); + void remoteLoadFailed(size_t index, const QString& msg, bool downloadFailed = false); void checkIfAllFinished(); private: std::unique_ptr d; + bool m_downloadFailed = false; }; diff --git a/launcher/minecraft/MinecraftLoadAndCheck.cpp b/launcher/minecraft/MinecraftLoadAndCheck.cpp index c26fb8b60..39155887c 100644 --- a/launcher/minecraft/MinecraftLoadAndCheck.cpp +++ b/launcher/minecraft/MinecraftLoadAndCheck.cpp @@ -1,4 +1,5 @@ #include "MinecraftLoadAndCheck.h" +#include "ComponentUpdateTask.h" #include "MinecraftInstance.h" #include "PackProfile.h" @@ -6,6 +7,10 @@ MinecraftLoadAndCheck::MinecraftLoadAndCheck(MinecraftInstance* inst, Net::Mode void MinecraftLoadAndCheck::executeTask() { + m_downloadFailed = false; + if (m_task) + disconnect(m_task.get(), nullptr, this, nullptr); + m_task.reset(); // add offline metadata load task auto components = m_inst->getPackProfile(); if (auto result = components->reload(m_netmode); !result) { @@ -19,7 +24,11 @@ void MinecraftLoadAndCheck::executeTask() return; } connect(m_task.get(), &Task::succeeded, this, &MinecraftLoadAndCheck::emitSucceeded); - connect(m_task.get(), &Task::failed, this, &MinecraftLoadAndCheck::emitFailed); + connect(m_task.get(), &Task::failed, this, [this](const QString& reason) { + auto task = qobject_cast(m_task.get()); + m_downloadFailed = task && task->downloadFailed(); + emitFailed(reason); + }); connect(m_task.get(), &Task::aborted, this, &MinecraftLoadAndCheck::emitAborted); propagateFromOther(m_task.get()); } diff --git a/launcher/minecraft/MinecraftLoadAndCheck.h b/launcher/minecraft/MinecraftLoadAndCheck.h index c05698bca..ea0897af5 100644 --- a/launcher/minecraft/MinecraftLoadAndCheck.h +++ b/launcher/minecraft/MinecraftLoadAndCheck.h @@ -28,6 +28,7 @@ class MinecraftLoadAndCheck : public Task { void executeTask() override; bool canAbort() const override; + bool downloadFailed() const { return m_downloadFailed; } public slots: bool abort() override; @@ -35,4 +36,5 @@ class MinecraftLoadAndCheck : public Task { MinecraftInstance* m_inst = nullptr; Task::Ptr m_task; Net::Mode m_netmode; + bool m_downloadFailed = false; }; diff --git a/launcher/minecraft/update/AssetUpdateTask.cpp b/launcher/minecraft/update/AssetUpdateTask.cpp index 22eea47b2..fee7a1626 100644 --- a/launcher/minecraft/update/AssetUpdateTask.cpp +++ b/launcher/minecraft/update/AssetUpdateTask.cpp @@ -18,6 +18,7 @@ AssetUpdateTask::AssetUpdateTask(MinecraftInstance* inst) void AssetUpdateTask::executeTask() { + m_downloadFailed = false; setStatus(tr("Updating assets index...")); auto components = m_inst->getPackProfile(); auto profile = components->getProfile(); @@ -93,12 +94,14 @@ void AssetUpdateTask::assetIndexFinished() void AssetUpdateTask::assetIndexFailed(QString reason) { + m_downloadFailed = downloadJob->hasNetworkFailure(); qDebug() << m_inst->name() << ": Failed asset index download"; emitFailed(tr("Failed to download the assets index:\n%1").arg(reason)); } void AssetUpdateTask::assetsFailed(QString reason) { + m_downloadFailed = downloadJob->hasNetworkFailure(); emitFailed(tr("Failed to download assets:\n%1").arg(reason)); } diff --git a/launcher/minecraft/update/AssetUpdateTask.h b/launcher/minecraft/update/AssetUpdateTask.h index 56aecc293..79356d7fd 100644 --- a/launcher/minecraft/update/AssetUpdateTask.h +++ b/launcher/minecraft/update/AssetUpdateTask.h @@ -12,6 +12,7 @@ class AssetUpdateTask : public Task { void executeTask() override; bool canAbort() const override; + bool downloadFailed() const { return m_downloadFailed; } public: static QString resourceUrl(); @@ -27,4 +28,5 @@ class AssetUpdateTask : public Task { private: MinecraftInstance* m_inst; NetJob::Ptr downloadJob; + bool m_downloadFailed = false; }; diff --git a/launcher/minecraft/update/LegacyFMLLibrariesTask.cpp b/launcher/minecraft/update/LegacyFMLLibrariesTask.cpp index a3bcc145f..9332a442f 100644 --- a/launcher/minecraft/update/LegacyFMLLibrariesTask.cpp +++ b/launcher/minecraft/update/LegacyFMLLibrariesTask.cpp @@ -16,6 +16,8 @@ LegacyFMLLibrariesTask::LegacyFMLLibrariesTask(MinecraftInstance* inst) } void LegacyFMLLibrariesTask::executeTask() { + m_downloadFailed = false; + fmlLibsToProcess.clear(); // Get the mod list MinecraftInstance* inst = (MinecraftInstance*)m_inst; auto components = inst->getPackProfile(); @@ -110,6 +112,7 @@ void LegacyFMLLibrariesTask::fmllibsFinished() } void LegacyFMLLibrariesTask::fmllibsFailed(QString reason) { + m_downloadFailed = downloadJob->hasNetworkFailure(); QStringList failed = downloadJob->getFailedFiles(); QString failed_all = failed.join("\n"); emitFailed(tr("Failed to download the following files:\n%1\n\nReason:%2\nPlease try again.").arg(failed_all, reason)); diff --git a/launcher/minecraft/update/LegacyFMLLibrariesTask.h b/launcher/minecraft/update/LegacyFMLLibrariesTask.h index 2591f7c9f..5700dca3a 100644 --- a/launcher/minecraft/update/LegacyFMLLibrariesTask.h +++ b/launcher/minecraft/update/LegacyFMLLibrariesTask.h @@ -14,6 +14,7 @@ class LegacyFMLLibrariesTask : public Task { void executeTask() override; bool canAbort() const override; + bool downloadFailed() const { return m_downloadFailed; } private slots: void fmllibsFinished(); @@ -29,4 +30,5 @@ class LegacyFMLLibrariesTask : public Task { MinecraftInstance* m_inst; NetJob::Ptr downloadJob; QList fmlLibsToProcess; + bool m_downloadFailed = false; }; diff --git a/launcher/minecraft/update/LibrariesTask.cpp b/launcher/minecraft/update/LibrariesTask.cpp index f725af18d..1a66ad453 100644 --- a/launcher/minecraft/update/LibrariesTask.cpp +++ b/launcher/minecraft/update/LibrariesTask.cpp @@ -12,6 +12,7 @@ LibrariesTask::LibrariesTask(MinecraftInstance* inst) void LibrariesTask::executeTask() { + m_downloadFailed = false; setStatus(tr("Downloading required library files...")); qDebug() << m_inst->name() << ": downloading libraries"; MinecraftInstance* inst = (MinecraftInstance*)m_inst; @@ -78,6 +79,7 @@ bool LibrariesTask::canAbort() const void LibrariesTask::jarlibFailed(QString reason) { + m_downloadFailed = downloadJob->hasNetworkFailure(); emitFailed(tr("Game update failed: it was impossible to fetch the required libraries.\nReason:\n%1").arg(reason)); } diff --git a/launcher/minecraft/update/LibrariesTask.h b/launcher/minecraft/update/LibrariesTask.h index 838f9d9b4..69d1b18ea 100644 --- a/launcher/minecraft/update/LibrariesTask.h +++ b/launcher/minecraft/update/LibrariesTask.h @@ -12,6 +12,7 @@ class LibrariesTask : public Task { void executeTask() override; bool canAbort() const override; + bool downloadFailed() const { return m_downloadFailed; } private slots: void jarlibFailed(QString reason); @@ -22,4 +23,5 @@ class LibrariesTask : public Task { private: MinecraftInstance* m_inst; NetJob::Ptr downloadJob; + bool m_downloadFailed = false; }; diff --git a/launcher/net/FileSink.cpp b/launcher/net/FileSink.cpp index 47838f62c..656857d34 100644 --- a/launcher/net/FileSink.cpp +++ b/launcher/net/FileSink.cpp @@ -95,7 +95,11 @@ Task::State FileSink::abort() { if (m_output_file) { m_output_file->cancelWriting(); + // cancelWriting only prevents commit; destruction closes the handle and + // removes the temporary file before the request emits its terminal signal. + m_output_file.reset(); } + m_wroteAnyData = false; failAllValidators(); return Task::State::Failed; } diff --git a/launcher/net/NetJob.cpp b/launcher/net/NetJob.cpp index 3dd1c09cf..1a24377fe 100644 --- a/launcher/net/NetJob.cpp +++ b/launcher/net/NetJob.cpp @@ -130,7 +130,7 @@ auto NetJob::abort() -> bool return fullyAborted; } -auto NetJob::getFailedActions() -> QList +auto NetJob::getFailedActions() const -> QList { QList failed; for (auto index : m_failed) { @@ -139,6 +139,22 @@ auto NetJob::getFailedActions() -> QList return failed; } +bool NetJob::hasNetworkFailure() const +{ + const auto failed = getFailedActions(); + if (failed.isEmpty()) + return false; + + for (const auto* request : failed) { + if (!request) + return false; + const auto error = request->error(); + if (error == QNetworkReply::NoError || error == QNetworkReply::OperationCanceledError) + return false; + } + return true; +} + auto NetJob::getFailedFiles() -> QList { QList failed; diff --git a/launcher/net/NetJob.h b/launcher/net/NetJob.h index e8351f686..9c1281ef3 100644 --- a/launcher/net/NetJob.h +++ b/launcher/net/NetJob.h @@ -61,7 +61,8 @@ class NetJob : public ConcurrentTask { auto canAbort() const -> bool override; auto addNetAction(Net::NetRequest::Ptr action) -> bool; - auto getFailedActions() -> QList; + auto getFailedActions() const -> QList; + bool hasNetworkFailure() const; auto getFailedFiles() -> QList; void setAskRetry(bool askRetry); diff --git a/launcher/net/NetRequest.cpp b/launcher/net/NetRequest.cpp index db8ae45db..63105e3d9 100644 --- a/launcher/net/NetRequest.cpp +++ b/launcher/net/NetRequest.cpp @@ -79,6 +79,7 @@ void NetRequest::executeTask() return; } + m_reply.reset(); QNetworkRequest request(m_url); m_state = m_sink->init(request); switch (m_state) { @@ -102,10 +103,10 @@ void NetRequest::executeTask() return; } -#if defined(LAUNCHER_APPLICATION) - auto user_agent = APPLICATION->getUserAgent(); -#else auto user_agent = BuildConfig.USER_AGENT; +#if defined(LAUNCHER_APPLICATION) + if (auto app = APPLICATION_DYN) + user_agent = app->getUserAgent(); #endif request.setHeader(QNetworkRequest::UserAgentHeader, user_agent.toUtf8()); @@ -113,10 +114,10 @@ void NetRequest::executeTask() header_proxy->writeHeaders(request); } -#if defined(LAUNCHER_APPLICATION) - request.setTransferTimeout(APPLICATION->settings()->get("RequestTimeout").toInt() * 1000); -#else request.setTransferTimeout(); +#if defined(LAUNCHER_APPLICATION) + if (auto app = APPLICATION_DYN) + request.setTransferTimeout(app->settings()->get("RequestTimeout").toInt() * 1000); #endif m_last_progress_time = m_clock.now(); diff --git a/tests/Task_test.cpp b/tests/Task_test.cpp index 1c95f702d..265730003 100644 --- a/tests/Task_test.cpp +++ b/tests/Task_test.cpp @@ -1,7 +1,15 @@ +#include +#include +#include +#include +#include +#include #include #include #include +#include +#include #include #include #include @@ -245,6 +253,48 @@ class TaskTest : public QObject { QVERIFY2(QTest::qWaitFor([&t]() { return t.isFinished(); }, 1000), "Task didn't finish as it should."); } + void test_networkRetryLocalFailureClearsNetworkError() + { + QTemporaryDir temp; + QVERIFY(temp.isValid()); + QVERIFY(QDir(temp.path()).mkdir("downloads")); + const auto parentPath = temp.filePath("downloads"); + + QTcpServer portReservation; + QVERIFY(portReservation.listen(QHostAddress::LocalHost, 0)); + const auto port = portReservation.serverPort(); + + QNetworkAccessManager network; + network.setProxy(QNetworkProxy::NoProxy); + NetJob job("Network retry followed by local failure", &network); + job.setAskRetry(false); + auto download = Net::Download::makeFile(QUrl(QString("http://127.0.0.1:%1/file").arg(port)), parentPath + "/file"); + job.addNetAction(download); + + int failureCount = 0; + QObject observer; + connect(download.get(), &Task::failed, &observer, [&] { + if (++failureCount != 1) + return; + + QVERIFY(download->error() != QNetworkReply::NoError); + QVERIFY(download->error() != QNetworkReply::OperationCanceledError); + // Move the directory aside, including any still-open temporary save file. + QVERIFY(QDir(temp.path()).rename("downloads", "failed-downloads")); + QFile blocker(parentPath); + QVERIFY(blocker.open(QIODevice::WriteOnly)); + }); + + portReservation.close(); + job.start(); + + QVERIFY2(QTest::qWaitFor([&job] { return job.isFinished(); }, 10000), "NetJob did not finish its retries."); + QVERIFY(!job.wasSuccessful()); + QVERIFY(failureCount > 1); + QCOMPARE(download->error(), QNetworkReply::NoError); + QVERIFY(!job.hasNetworkFailure()); + } + void test_stackOverflowInConcurrentTask() { QEventLoop loop; From b7a8fddfbd890ffca59b027da6e9d807e6d3920e Mon Sep 17 00:00:00 2001 From: HowXu Date: Sat, 24 Oct 2026 11:49:31 +0800 Subject: [PATCH 03/13] feat(modpacks): Add incomplete instance keep policy Add the default-off automatic keep setting and shared creation handling. Preserve supported new instances after content interruption or game dependency network failure without changing Skip. Assisted-by: OpenCode:gpt-6.1-sol-fast Signed-off-by: HowXu --- launcher/Application.cpp | 1 + launcher/InstanceCreationTask.cpp | 117 ++++++++++++++++++++-- launcher/InstanceCreationTask.h | 7 ++ launcher/InstanceTask.cpp | 13 +++ launcher/InstanceTask.h | 1 + launcher/ui/pages/global/LauncherPage.cpp | 2 + launcher/ui/pages/global/LauncherPage.ui | 11 ++ 7 files changed, 146 insertions(+), 6 deletions(-) diff --git a/launcher/Application.cpp b/launcher/Application.cpp index fda21d2bf..411b02e77 100644 --- a/launcher/Application.cpp +++ b/launcher/Application.cpp @@ -838,6 +838,7 @@ Application::Application(int& argc, char** argv) : QApplication(argc, argv) m_settings->registerSetting("ModMetadataDisabled", false); m_settings->registerSetting("ModDependenciesDisabled", false); m_settings->registerSetting("SkipModpackUpdatePrompt", false); + m_settings->registerSetting("KeepIncompleteModpackInstances", false); m_settings->registerSetting("ShowModIncompat", false); // Minecraft offline player name diff --git a/launcher/InstanceCreationTask.cpp b/launcher/InstanceCreationTask.cpp index 7e74a9336..58c6f711f 100644 --- a/launcher/InstanceCreationTask.cpp +++ b/launcher/InstanceCreationTask.cpp @@ -2,14 +2,24 @@ #include #include +#include +#include +#include +#include +#include "FileSystem.h" #include "InstanceTask.h" #include "minecraft/MinecraftLoadAndCheck.h" +#include "minecraft/PackProfile.h" +#include "minecraft/update/AssetUpdateTask.h" +#include "minecraft/update/LegacyFMLLibrariesTask.h" +#include "minecraft/update/LibrariesTask.h" +#include "settings/Setting.h" #include "tasks/SequentialTask.h" bool InstanceCreationTask::abort() { - if (!canAbort()) { + if (!canAbort() || !isRunning()) { return false; } @@ -18,15 +28,29 @@ bool InstanceCreationTask::abort() return m_gameFilesTask->abort(); } - return InstanceTask::abort(); + // createInstance() may be running a nested event loop. Let it stop its content + // task and decide whether to keep the instance before staging can be removed. + return true; } void InstanceCreationTask::executeTask() { + m_abort = false; + m_contentIncomplete = false; + m_gameDownloadFailed = false; + m_error_message.clear(); + m_gameFilesTask.reset(); + m_instance.reset(); setAbortable(true); - if (updateInstance()) { - emitSucceeded(); + auto updated = updateInstance(); + if (!isRunning()) + return; + if (updated) { + if (m_abort) + emitAborted(); + else + emitSucceeded(); return; } @@ -37,9 +61,13 @@ void InstanceCreationTask::executeTask() } m_instance = createInstance(); + if (!isRunning()) + return; if (!m_instance) { - if (m_abort) + if (m_abort && m_error_message.isEmpty()) { + emitAborted(); return; + } qWarning() << "Instance creation failed!"; if (!m_error_message.isEmpty()) { @@ -52,6 +80,19 @@ void InstanceCreationTask::executeTask() return; } + if (m_contentIncomplete && !shouldOverride()) { + finishIncompleteInstance( + tr("Modpack installation is incomplete. Remaining files must be downloaded before playing; game files " + "have not been downloaded yet.")); + return; + } + if (m_abort) { + // Destroy the profile before InstanceStaging removes its directory. + m_instance.reset(); + emitAborted(); + return; + } + // If this is set, it means we're updating an instance. So, we now need to remove the // files scheduled to, and we'd better not let the user abort in the middle of it, since it'd // put the instance in an invalid state. @@ -91,13 +132,29 @@ void InstanceCreationTask::executeTask() return; } auto task = makeShared(); - task->addTask(makeShared(m_instance.get(), Net::Mode::Online)); + auto loadTask = makeShared(m_instance.get(), Net::Mode::Online); + connect(loadTask.get(), &Task::failed, this, [this, source = loadTask.get()] { m_gameDownloadFailed = source->downloadFailed(); }); + task->addTask(loadTask); for (const auto& t : updateTasks) { + // These connections must run before SequentialTask forwards the failure. + if (auto source = qobject_cast(t.get())) { + connect(source, &Task::failed, this, [this, source] { m_gameDownloadFailed = source->downloadFailed(); }); + } else if (auto source = qobject_cast(t.get())) { + connect(source, &Task::failed, this, [this, source] { m_gameDownloadFailed = source->downloadFailed(); }); + } else if (auto source = qobject_cast(t.get())) { + connect(source, &Task::failed, this, [this, source] { m_gameDownloadFailed = source->downloadFailed(); }); + } task->addTask(t); } connect(task.get(), &Task::finished, this, [this, task] { + if (!isRunning()) + return; if (task->wasSuccessful() || m_abort) { emitSucceeded(); + } else if (!shouldOverride() && keepOnGameDownloadFailure() && m_gameDownloadFailed) { + finishIncompleteInstance(tr("Game file downloads did not finish. The modpack content is complete, but game files must be " + "downloaded before playing.\n%1") + .arg(task->failReason())); } else { emitFailed(tr("Could not download game files: %1").arg(task->failReason())); } @@ -110,6 +167,54 @@ void InstanceCreationTask::executeTask() } } +void InstanceCreationTask::finishIncompleteInstance(const QString& warning) +{ + setAbortable(false); + auto settings = m_instance->settings(); + m_instance->saveNow(); + + // Instance settings are saved synchronously by INISettingsObject. Check the + // persisted identity as well: its setters do not return disk write errors. + QSettings config(FS::PathCombine(m_instance->instanceRoot(), "instance.cfg"), QSettings::IniFormat); + config.setFallbacksEnabled(false); + config.sync(); + bool configSaved = QFile::exists(config.fileName()) && config.status() == QSettings::NoError; + for (const auto& key : { "name", "InstanceType", "iconKey", "ManagedPack", "ManagedPackType", "ManagedPackID", "ManagedPackName", + "ManagedPackVersionID", "ManagedPackVersionName" }) { + auto setting = settings->getSetting(key); + if (config.value(key, setting->defValue()).toString() != setting->get().toString()) + configSaved = false; + } + // saveNow() has no result API. Check the saved component configuration using + // public profile accessors, without reloading or starting metadata downloads. + auto profile = m_instance->getPackProfile(); + QFile profileFile(FS::PathCombine(m_instance->instanceRoot(), "mmc-pack.json")); + QJsonObject savedProfile; + if (profileFile.open(QIODevice::ReadOnly)) { + savedProfile = QJsonDocument::fromJson(profileFile.readAll()).object(); + profileFile.close(); + } + auto savedComponents = savedProfile.value("components").toArray(); + bool profileSaved = savedProfile.value("formatVersion").toInt() == 1 && savedProfile.value("components").isArray() && + savedComponents.size() == profile->rowCount(); + for (int i = 0; profileSaved && i < profile->rowCount(); ++i) { + auto component = profile->getComponent(static_cast(i)); + auto saved = savedComponents[i].toObject(); + profileSaved = saved.value("uid").toString() == component->m_uid && saved.value("version").toString() == component->m_version && + saved.value("dependencyOnly").toBool() == component->m_dependencyOnly && + saved.value("important").toBool() == component->m_important && + saved.value("disabled").toBool() == component->m_disabled; + } + if (!configSaved || !profileSaved) { + m_instance.reset(); + emitFailed(tr("Could not save the incomplete instance configuration.")); + return; + } + + logWarning(warning); + emitSucceeded(); +} + void InstanceCreationTask::scheduleToDelete(QWidget* parent, QDir dir, QString path, bool checkDisabled) { if (path.isEmpty()) { diff --git a/launcher/InstanceCreationTask.h b/launcher/InstanceCreationTask.h index 416cf81db..a0e64df18 100644 --- a/launcher/InstanceCreationTask.h +++ b/launcher/InstanceCreationTask.h @@ -38,6 +38,9 @@ class InstanceCreationTask : public InstanceTask { protected: void setError(const QString& message) { m_error_message = message; }; + // Set only after the content task has stopped and the pending manifest has been saved. + void setContentIncomplete(bool incomplete) { m_contentIncomplete = incomplete; } + virtual bool keepOnGameDownloadFailure() const { return false; } void scheduleToDelete(QWidget* parent, QDir dir, QString path, bool checkDisabled = false); protected: @@ -47,6 +50,10 @@ class InstanceCreationTask : public InstanceTask { ShouldDeleteSaves m_shouldDeleteSaves; private: + void finishIncompleteInstance(const QString& warning); + + bool m_contentIncomplete = false; + bool m_gameDownloadFailed = false; QString m_error_message; std::unique_ptr m_instance; Task::Ptr m_gameFilesTask; diff --git a/launcher/InstanceTask.cpp b/launcher/InstanceTask.cpp index 01998a7aa..5576aaddb 100644 --- a/launcher/InstanceTask.cpp +++ b/launcher/InstanceTask.cpp @@ -93,3 +93,16 @@ ShouldDeleteSaves askIfShouldDeleteSaves(QWidget* parent) auto result = dialog->exec(); return result == QMessageBox::Yes ? ShouldDeleteSaves::Yes : ShouldDeleteSaves::No; } + +bool askToKeepIncompleteModpack(QWidget* parent, SettingsObject* settings) +{ + if (settings->get("KeepIncompleteModpackInstances").toBool()) + return true; + + auto dialog = CustomMessageBox::selectable( + parent, QObject::tr("Keep incomplete modpack?"), + QObject::tr("The modpack download did not finish. Keep the incomplete instance and download the remaining files before launching?"), + QMessageBox::Question, QMessageBox::Yes | QMessageBox::No, QMessageBox::No); + dialog->setEscapeButton(QMessageBox::No); + return dialog->exec() == QMessageBox::Yes; +} diff --git a/launcher/InstanceTask.h b/launcher/InstanceTask.h index 125930a27..24c339c91 100644 --- a/launcher/InstanceTask.h +++ b/launcher/InstanceTask.h @@ -10,6 +10,7 @@ enum class ShouldUpdate { Update, SkipUpdating, Cancel }; [[nodiscard]] ShouldUpdate askIfShouldUpdate(QWidget* parent, QString original_version_name); enum class ShouldDeleteSaves { NotAsked, Yes, No }; [[nodiscard]] ShouldDeleteSaves askIfShouldDeleteSaves(QWidget* parent); +bool askToKeepIncompleteModpack(QWidget* parent, SettingsObject* settings); struct InstanceName { public: diff --git a/launcher/ui/pages/global/LauncherPage.cpp b/launcher/ui/pages/global/LauncherPage.cpp index c78d99f62..fbb19d910 100644 --- a/launcher/ui/pages/global/LauncherPage.cpp +++ b/launcher/ui/pages/global/LauncherPage.cpp @@ -247,6 +247,7 @@ void LauncherPage::applySettings() s->set("ModDependenciesDisabled", !ui->dependenciesEnableBtn->isChecked()); s->set("ShowModIncompat", ui->showModIncompatCheckBox->isChecked()); s->set("SkipModpackUpdatePrompt", !ui->modpackUpdatePromptBtn->isChecked()); + s->set("KeepIncompleteModpackInstances", ui->keepIncompleteModpackInstancesCheckBox->isChecked()); const auto showInDiscord = ui->showInDiscordBtn->isChecked(); s->set("AlwaysShowInDiscord", showInDiscord); APPLICATION->discord()->showAlways(showInDiscord); @@ -301,6 +302,7 @@ void LauncherPage::loadSettings() ui->dependenciesEnableBtn->setChecked(!s->get("ModDependenciesDisabled").toBool()); ui->showModIncompatCheckBox->setChecked(s->get("ShowModIncompat").toBool()); ui->modpackUpdatePromptBtn->setChecked(!s->get("SkipModpackUpdatePrompt").toBool()); + ui->keepIncompleteModpackInstancesCheckBox->setChecked(s->get("KeepIncompleteModpackInstances").toBool()); ui->showInDiscordBtn->setChecked(s->get("AlwaysShowInDiscord").toBool()); } diff --git a/launcher/ui/pages/global/LauncherPage.ui b/launcher/ui/pages/global/LauncherPage.ui index 50e069a74..d4db71ec9 100644 --- a/launcher/ui/pages/global/LauncherPage.ui +++ b/launcher/ui/pages/global/LauncherPage.ui @@ -467,6 +467,16 @@ + + + + Keep new Modrinth and CurseForge instances when modpack downloads fail or are canceled, without asking. Remaining files will be downloaded before launch. + + + Always keep incomplete modpack instances + + + @@ -706,6 +716,7 @@ dependenciesEnableBtn showModIncompatCheckBox modpackUpdatePromptBtn + keepIncompleteModpackInstancesCheckBox downloadGameFilesBtn lineLimitSpinBox checkStopLogging From 6529e1fe1a6fbc862706bb4b0f09a57b8f86ca04 Mon Sep 17 00:00:00 2001 From: HowXu Date: Sat, 24 Oct 2026 11:49:35 +0800 Subject: [PATCH 04/13] feat(modrinth): Preserve resumable pack contents Share content installation between creation and resume, retain native pending entries, reuse validated files, and drain writes before a keep decision. Assisted-by: OpenCode:gpt-6.1-sol-fast Signed-off-by: HowXu --- launcher/CMakeLists.txt | 2 + .../modrinth/ModrinthInstanceCreationTask.cpp | 227 ++++------ .../modrinth/ModrinthInstanceCreationTask.h | 10 +- .../modrinth/ModrinthPackInstallTask.cpp | 428 ++++++++++++++++++ .../modrinth/ModrinthPackInstallTask.h | 74 +++ 5 files changed, 586 insertions(+), 155 deletions(-) create mode 100644 launcher/modplatform/modrinth/ModrinthPackInstallTask.cpp create mode 100644 launcher/modplatform/modrinth/ModrinthPackInstallTask.h diff --git a/launcher/CMakeLists.txt b/launcher/CMakeLists.txt index a14f6f278..3eded8171 100644 --- a/launcher/CMakeLists.txt +++ b/launcher/CMakeLists.txt @@ -593,6 +593,8 @@ set(MODRINTH_SOURCES modplatform/modrinth/ModrinthCheckUpdate.h modplatform/modrinth/ModrinthInstanceCreationTask.cpp modplatform/modrinth/ModrinthInstanceCreationTask.h + modplatform/modrinth/ModrinthPackInstallTask.h + modplatform/modrinth/ModrinthPackInstallTask.cpp modplatform/modrinth/ModrinthPackExportTask.cpp modplatform/modrinth/ModrinthPackExportTask.h ) diff --git a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp index 2ca6b082a..4b0b39f4e 100644 --- a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp +++ b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp @@ -1,4 +1,5 @@ #include "ModrinthInstanceCreationTask.h" +#include "ModrinthPackInstallTask.h" #include "Application.h" #include "FileSystem.h" @@ -9,26 +10,16 @@ #include "minecraft/MinecraftInstance.h" #include "minecraft/PackProfile.h" -#include "minecraft/mod/Mod.h" -#include "modplatform/EnsureMetadataTask.h" #include "modplatform/helpers/OverrideUtils.h" -#include "net/ChecksumValidator.h" - -#include "net/ApiDownload.h" -#include "net/ApiHeaderProxy.h" -#include "net/NetJob.h" - -#include "modplatform/ModIndex.h" #include "settings/INISettingsObject.h" #include "ui/dialogs/CustomMessageBox.h" -#include "ui/dialogs/UntrustedModsDialog.h" #include "ui/pages/modplatform/OptionalModDialog.h" #include #include -#include +#include #include bool ModrinthCreationTask::abort() @@ -37,6 +28,12 @@ bool ModrinthCreationTask::abort() return false; } + if (m_installingContent) { + // The child owns the stop barrier. Do not signal InstanceStaging until + // all sinks have closed and the keep/discard decision has been made. + m_abort = true; + return m_task->abort(); + } if (m_task) { m_task->abort(); } @@ -170,14 +167,16 @@ std::unique_ptr ModrinthCreationTask::createInstance() QString parentFolder(FS::PathCombine(m_stagingPath, "mrpack")); QString indexPath = FS::PathCombine(m_stagingPath, "modrinth.index.json"); - if (m_files.empty() && !parseManifest(indexPath, m_files, true, true)) { + if (m_files.empty() && !parseManifest(indexPath, m_files, true, shouldOverride())) { return nullptr; } // Keep index file in case we need it some other time (like when changing versions) QString newIndexPlace(FS::PathCombine(parentFolder, "modrinth.index.json")); - FS::ensureFilePathExists(newIndexPlace); - FS::move(indexPath, newIndexPlace); + if (!FS::ensureFilePathExists(newIndexPlace) || !FS::move(indexPath, newIndexPlace)) { + setError(tr("Could not save the original modpack index.")); + return nullptr; + } auto mcPath = FS::PathCombine(m_stagingPath, m_root_path); @@ -206,9 +205,10 @@ std::unique_ptr ModrinthCreationTask::createInstance() } } - if (!promptForUntrustedMods()) { - m_abort = true; - emitAborted(); + // Keep the existing update root (including .minecraft) visible to gameRoot() + // even when this archive has no overrides to create that directory first. + if (!FS::ensureFolderPathExists(mcPath)) { + setError(tr("Could not create the Minecraft directory.")); return nullptr; } @@ -220,22 +220,17 @@ std::unique_ptr ModrinthCreationTask::createInstance() components->buildingFromScratch(); components->setComponentVersion("net.minecraft", m_minecraft_version, true); - QString loader; if (!m_fabric_version.isEmpty()) { components->setComponentVersion("net.fabricmc.fabric-loader", m_fabric_version); - loader = ModPlatform::getModLoaderAsString(ModPlatform::ModLoaderType::Fabric); } if (!m_quilt_version.isEmpty()) { components->setComponentVersion("org.quiltmc.quilt-loader", m_quilt_version); - loader = ModPlatform::getModLoaderAsString(ModPlatform::ModLoaderType::Quilt); } if (!m_forge_version.isEmpty()) { components->setComponentVersion("net.minecraftforge", m_forge_version); - loader = ModPlatform::getModLoaderAsString(ModPlatform::ModLoaderType::Forge); } if (!m_neoForge_version.isEmpty()) { components->setComponentVersion("net.neoforged", m_neoForge_version); - loader = ModPlatform::getModLoaderAsString(ModPlatform::ModLoaderType::NeoForge); } if (m_instIcon != "default") { @@ -254,108 +249,72 @@ std::unique_ptr ModrinthCreationTask::createInstance() instance->setName(name()); instance->saveNow(); - auto downloadMods = makeShared(tr("Mod Download Modrinth"), APPLICATION->network()); - - auto rootModpackPath = FS::PathCombine(m_stagingPath, m_root_path); - auto rootModpackUrl = QUrl::fromLocalFile(rootModpackPath); - // TODO make this work with other sorts of resource - QHash resources; - for (auto& file : m_files) { - auto fileName = file.path; - fileName = FS::RemoveInvalidPathChars(fileName); - auto filePath = FS::PathCombine(rootModpackPath, fileName); - if (!rootModpackUrl.isParentOf(QUrl::fromLocalFile(filePath))) { - // This means we somehow got out of the root folder, so abort here to prevent exploits - setError(tr("One of the files has a path that leads to an arbitrary location (%1). This is a security risk and isn't allowed.") - .arg(fileName)); - return nullptr; - } - if (fileName.startsWith("mods/")) { - auto* mod = new Mod(filePath); - ModDetails d; - d.mod_id = filePath; - mod->setDetails(d); - resources[file.hash.toHex()] = mod; - } - if (file.downloads.empty()) { - setError(tr("The file '%1' is missing a download link. This is invalid in the pack format.").arg(fileName)); - return nullptr; - } - qDebug() << "Will try to download" << file.downloads.front() << "to" << filePath; - - Net::ModrinthDownloadMeta meta{ - .reason = m_instance.has_value() ? "update" : "modpack", - .gameVersion = m_minecraft_version, - .loader = loader, - }; - - QUrl downloadUrl = file.downloads.dequeue(); - auto dl = Net::ApiDownload::makeFile(downloadUrl, filePath, Net::Download::Option::NoOptions, meta); - dl->addValidator(new Net::ChecksumValidator(file.hashAlgorithm, file.hash)); - downloadMods->addNetAction(dl); - if (!file.downloads.empty()) { - // FIXME: This really needs to be put into a ConcurrentTask of - // MultipleOptionsTask's , once those exist :) - auto param = dl.toWeakRef(); - connect(dl.get(), &Task::failed, [&file, filePath, param, downloadMods, meta] { - QUrl fallbackUrl = file.downloads.dequeue(); - auto ndl = Net::ApiDownload::makeFile(fallbackUrl, filePath, Net::Download::Option::NoOptions, meta); - ndl->addValidator(new Net::ChecksumValidator(file.hashAlgorithm, file.hash)); - downloadMods->addNetAction(ndl); - if (auto shared = param.lock()) { - shared->succeeded(); + auto contentManifest = m_manifest; + if (shouldOverride()) { + // Keep the existing update difference/optional-selection algorithm. Its + // already-selected runtime paths are not a persisted remaining manifest. + QJsonArray selectedFiles; + for (const auto& file : m_files) { + for (const auto& value : m_manifest["files"].toArray()) { + auto entry = value.toObject(); + auto path = entry["path"].toString().replace('\\', '/'); + if (path == file.path || path + ".disabled" == file.path) { + entry["path"] = file.path; + entry["env"] = QJsonObject{ { "client", "required" } }; + selectedFiles.append(entry); + break; } - }); + } } + contentManifest["files"] = selectedFiles; + } + auto content = + makeShared(instance.get(), contentManifest, Modpack::InstallMode::Create, m_trustedSource, m_parent); + content->m_isUpdate = shouldOverride(); + connect(content.get(), &Task::finished, &loop, &QEventLoop::quit); + propagateFromOther(content.get()); + m_task = content; + m_installingContent = true; + content->start(); + if (!content->isFinished()) { + loop.exec(); + } + m_installingContent = false; + m_task.reset(); + const bool endedWell = content->wasSuccessful(); + for (const auto& warning : content->warnings()) { + logWarning(warning); } - - bool endedWell = false; - - connect(downloadMods.get(), &NetJob::succeeded, this, [&endedWell]() { endedWell = true; }); - connect(downloadMods.get(), &NetJob::failed, [this, &endedWell](const QString& reason) { - endedWell = false; - setError(reason); - }); - connect(downloadMods.get(), &NetJob::finished, &loop, &QEventLoop::quit); - connect(downloadMods.get(), &NetJob::progress, [this](qint64 current, qint64 total) { - setDetails(tr("%1 out of %2 complete").arg(current).arg(total)); - setProgress(current, total); - }); - connect(downloadMods.get(), &NetJob::stepProgress, this, &ModrinthCreationTask::propagateStepProgress); - - setStatus(tr("Downloading mods...")); - downloadMods->start(); - m_task = downloadMods; - - loop.exec(); - if (!endedWell) { - for (auto* resource : resources) { - delete resource; + if (!shouldOverride()) { + setAbortable(false); + } + setError(content->failReason()); + if (!shouldOverride() && content->canPreserve() && askToKeepIncompleteModpack(m_parent, m_globalSettings)) { + Modpack::PendingManifest pending(Modpack::Format::Modrinth, content->remainingManifest()); + QString error; + if (!pending.save(Modpack::pendingManifestPath(instance->instanceRoot(), Modpack::Format::Modrinth), &error)) { + // A failed save is an error, even when the initial interruption was cancellation. + m_abort = false; + setError(tr("Could not save remaining modpack files: %1").arg(error)); + return nullptr; + } + components->saveNow(); + instance->saveNow(); + // The common creation task validates the saved profile and config + // through public APIs before committing the incomplete instance. + setContentIncomplete(true); + return instance; + } + if (content->getState() == Task::State::AbortedByUser || m_abort) { + m_abort = true; + // Let the common task emit the terminal signal only after local + // instance/profile destructors have finished saving the staging tree. + setError({}); } return nullptr; } - QEventLoop ensureMetaLoop; - QDir folder = FS::PathCombine(instance->modsRoot(), ".index"); - auto ensureMetadataTask = makeShared(resources, folder, ModPlatform::ResourceProvider::MODRINTH); - connect(ensureMetadataTask.get(), &Task::succeeded, this, [&endedWell]() { endedWell = true; }); - connect(ensureMetadataTask.get(), &Task::finished, &ensureMetaLoop, &QEventLoop::quit); - connect(ensureMetadataTask.get(), &Task::progress, [this](qint64 current, qint64 total) { - setDetails(tr("%1 out of %2 complete").arg(current).arg(total)); - setProgress(current, total); - }); - connect(ensureMetadataTask.get(), &Task::stepProgress, this, &ModrinthCreationTask::propagateStepProgress); - - ensureMetadataTask->start(); - m_task = ensureMetadataTask; - - ensureMetaLoop.exec(); - for (auto* resource : resources) { - delete resource; - } - resources.clear(); - // Update information of the already installed instance, if any. if (m_instance && endedWell) { setAbortable(false); @@ -392,6 +351,7 @@ bool ModrinthCreationTask::parseManifest(const QString& indexPath, std::vector #include -#include +#include #include #include #include @@ -17,7 +17,6 @@ class ModrinthCreationTask final : public InstanceCreationTask { struct File { QString path; - QCryptographicHash::Algorithm hashAlgorithm; QByteArray hash; QQueue downloads; bool required = true; @@ -44,11 +43,12 @@ class ModrinthCreationTask final : public InstanceCreationTask { bool updateInstance() override; std::unique_ptr createInstance() override; + protected: + bool keepOnGameDownloadFailure() const override { return !shouldOverride(); } + private: bool parseManifest(const QString&, std::vector&, bool setInternalData = true, bool showOptionalDialog = true); - [[nodiscard]] bool promptForUntrustedMods(); - private: QWidget* m_parent = nullptr; bool m_trustedSource; @@ -58,6 +58,8 @@ class ModrinthCreationTask final : public InstanceCreationTask { std::vector m_files; Task::Ptr m_task; + QJsonObject m_manifest; + bool m_installingContent = false; std::optional m_instance; diff --git a/launcher/modplatform/modrinth/ModrinthPackInstallTask.cpp b/launcher/modplatform/modrinth/ModrinthPackInstallTask.cpp new file mode 100644 index 000000000..703a734a6 --- /dev/null +++ b/launcher/modplatform/modrinth/ModrinthPackInstallTask.cpp @@ -0,0 +1,428 @@ +// SPDX-License-Identifier: GPL-3.0-only +#include "ModrinthPackInstallTask.h" + +#include +#include +#include +#include +#include +#include + +#include "Application.h" +#include "BuildConfig.h" +#include "FileSystem.h" +#include "Json.h" +#include "minecraft/MinecraftInstance.h" +#include "minecraft/mod/Mod.h" +#include "modplatform/EnsureMetadataTask.h" +#include "net/ApiDownload.h" +#include "net/ChecksumValidator.h" +#include "ui/dialogs/UntrustedModsDialog.h" +#include "ui/pages/modplatform/OptionalModDialog.h" + +namespace { +// NetJob::abort() emits its terminal signal before its requests have necessarily +// closed their sinks. Drain them instead, including starts already queued by ConcurrentTask. +class DrainingNetJob final : public NetJob { + public: + using NetJob::NetJob; + + bool abort() override + { + m_stopping = true; + m_queue.clear(); + setAskRetry(false); + QTimer::singleShot(0, this, [this] { + const auto active = m_doing.values(); + for (const auto& task : active) { + task->abort(); + } + executeNextSubTask(); + }); + return true; + } + + protected: + void executeNextSubTask() override + { + if (!isRunning()) { + return; + } + if (m_stopping) { + if (m_doing.isEmpty()) { + emitAborted(); + } + return; + } + NetJob::executeNextSubTask(); + } + + private: + bool m_stopping = false; +}; + +bool matchesHash(const QString& path, const QByteArray& hash) +{ + QFile file(path); + QCryptographicHash hasher(QCryptographicHash::Sha512); + return file.open(QIODevice::ReadOnly) && hasher.addData(&file) && hasher.result() == hash; +} +} // namespace + +ModrinthPackInstallTask::ModrinthPackInstallTask(MinecraftInstance* instance, + QJsonObject manifest, + Modpack::InstallMode mode, + bool trustedSource, + QWidget* parent) + : m_instance(instance) + , m_pending(Modpack::Format::Modrinth, std::move(manifest)) + , m_mode(mode) + , m_trustedSource(trustedSource) + , m_parent(parent) +{ + setAbortable(true); +} + +ModrinthPackInstallTask::~ModrinthPackInstallTask() = default; + +QJsonObject ModrinthPackInstallTask::remainingManifest() const +{ + return m_pending.remaining(); +} + +bool ModrinthPackInstallTask::canPreserve() const +{ + return !m_isUpdate && m_validated && isFinished(); +} + +bool ModrinthPackInstallTask::abort() +{ + if (!isRunning()) { + return false; + } + m_aborted = true; + if (m_downloadJob && m_downloadJob->isRunning()) { + m_downloadJob->abort(); + } + // EnsureMetadataTask::abort disconnects its completion signals. Let this + // display-only work settle instead of committing a directory it still uses. + return true; +} + +bool ModrinthPackInstallTask::validatePath(const QString& path) const +{ + const auto root = QDir::cleanPath(QFileInfo(m_instance->gameRoot()).absoluteFilePath()); + const auto target = QDir::cleanPath(FS::PathCombine(root, path)); + if (path.isEmpty() || QDir::isAbsolutePath(path) || path.contains(':') || path.contains(QChar::Null) || + path.split('/').contains("..") || !target.startsWith(root + '/')) { + return false; + } + // Check every existing component, not just lexical containment: overrides or + // a user-edited resume directory may contain a symlink to outside the instance. + auto current = target; + while (current.startsWith(root + '/') || current == root) { + if (QFileInfo(current).isSymLink()) { + return false; + } + if (current == root) { + break; + } + current = QFileInfo(current).absolutePath(); + } + return true; +} + +bool ModrinthPackInstallTask::prepare() +{ + const auto manifest = m_pending.remaining(); + QStringList optionalPaths; + QStringList untrusted; + QSet targets; + try { + const auto pendingPath = Modpack::pendingManifestPath(m_instance->instanceRoot(), Modpack::Format::Modrinth); + if (!validatePath("mods/.index") || QFileInfo(pendingPath).isSymLink() || + QFileInfo(QFileInfo(pendingPath).absolutePath()).isSymLink()) { + throw JSONValidationError(tr("The modpack directory contains an unsafe symbolic link.")); + } + if (Json::requireInteger(manifest, "formatVersion") != 1 || Json::requireString(manifest, "game") != "minecraft") { + throw JSONValidationError(tr("Unsupported Modrinth pack format.")); + } + const auto dependencies = Json::requireObject(manifest, "dependencies"); + Json::requireString(dependencies, "minecraft"); + for (auto it = dependencies.begin(); it != dependencies.end(); ++it) { + if (it.key() != "minecraft" && it.key() != "fabric-loader" && it.key() != "quilt-loader" && it.key() != "forge" && + it.key() != "neoforge") { + throw JSONValidationError(tr("Unknown dependency type: %1").arg(it.key())); + } + Json::requireString(it.value(), it.key()); + } + const auto entries = Json::requireIsArrayOf(manifest, "files"); + for (const auto& entry : entries) { + File file; + file.key = Json::requireString(entry, "path"); + const auto path = FS::RemoveInvalidPathChars(QString(file.key).replace('\\', '/')); + auto env = entry["env"].toObject(); + const auto support = env.isEmpty() ? QStringLiteral("required") : env["client"].toString("unsupported"); + file.optional = support == "optional"; + file.unsupported = support == "unsupported"; + if (!validatePath(path)) { + throw JSONValidationError(tr("Unsafe file path: %1").arg(file.key)); + } + file.enabledPath = QDir::cleanPath(path); + file.disabledPath = file.enabledPath + ".disabled"; + if (file.optional && !validatePath(file.disabledPath)) { + throw JSONValidationError(tr("Unsafe file path: %1").arg(file.key)); + } + if (targets.contains(file.enabledPath)) { + throw JSONValidationError(tr("Unsafe or duplicate file path: %1").arg(file.key)); + } + targets.insert(file.enabledPath); + if (file.optional) { + if (targets.contains(file.disabledPath)) { + throw JSONValidationError(tr("Duplicate file path: %1").arg(file.key)); + } + targets.insert(file.disabledPath); + } + // Unsupported client entries never become impossible remaining downloads. + if (!file.unsupported) { + auto hashes = Json::requireObject(entry, "hashes"); + const auto sha512 = Json::requireString(hashes, "sha512").toLatin1(); + file.hash = QByteArray::fromHex(sha512); + if (sha512.size() != 128 || file.hash.size() != 64 || file.hash.toHex() != sha512.toLower()) { + throw JSONValidationError(tr("Invalid SHA-512 hash for %1").arg(file.key)); + } + for (const auto& value : Json::requireArray(entry, "downloads")) { + const QUrl url(Json::requireString(value, "download URL")); + if (url.isValid()) { + file.downloads.enqueue(url); + if (url.scheme() != "https" || url.host() != BuildConfig.MODRINTH_DOWNLOAD_HOST) { + untrusted.append(file.enabledPath); + } + } + } + if (file.downloads.isEmpty()) { + throw JSONValidationError(tr("The file '%1' is missing a download link.").arg(file.key)); + } + if (file.optional) { + optionalPaths.append(file.enabledPath); + } + } + m_files.push_back(std::move(file)); + } + } catch (const JSONValidationError& e) { + m_error = tr("Could not understand pack index:\n%1").arg(e.cause()); + return false; + } + + if (!m_trustedSource) { + const QDir root(m_instance->gameRoot()); + QDirIterator iter(m_instance->modsRoot(), QDir::Files, QDirIterator::Subdirectories); + while (iter.hasNext()) { + untrusted.append(root.relativeFilePath(iter.next())); + } + untrusted.removeDuplicates(); + if (!untrusted.isEmpty()) { + UntrustedModsDialog dialog(untrusted, m_parent); + if (dialog.exec() != QDialog::Accepted) { + m_aborted = true; + return false; + } + } + } + if (m_aborted) { + return false; + } + if (!optionalPaths.isEmpty()) { + OptionalModDialog dialog(m_parent, optionalPaths); + if (dialog.exec() != QDialog::Accepted) { + m_aborted = true; + return false; + } + const auto selected = dialog.getResult(); + for (auto& file : m_files) { + if (file.optional) { + file.enabled = selected.contains(file.enabledPath); + } + } + } + if (m_aborted) { + return false; + } + m_validated = true; + return true; +} + +bool ModrinthPackInstallTask::saveProgress() +{ + if (m_mode != Modpack::InstallMode::Resume) { + return true; + } + QString error; + if (!m_pending.save(Modpack::pendingManifestPath(m_instance->instanceRoot(), Modpack::Format::Modrinth), &error)) { + m_error = tr("Could not save remaining modpack files: %1").arg(error); + return false; + } + return true; +} + +bool ModrinthPackInstallTask::completeFile(File& file) +{ + // Optional selection changes must not leave enabled and disabled duplicates. + if (file.optional) { + const auto other = FS::PathCombine(m_instance->gameRoot(), file.alternatePath()); + if (QFile::exists(other) && !QFile::remove(other)) { + m_error = tr("Could not remove duplicate optional file: %1").arg(other); + return false; + } + } + file.completed = true; + m_pending.markCompleted(file.key); + return saveProgress(); +} + +void ModrinthPackInstallTask::executeTask() +{ + if (!prepare()) { + finish(); + return; + } + for (auto& file : m_files) { + if (file.unsupported) { + file.completed = true; + m_pending.markCompleted(file.key); + continue; + } + const auto path = FS::PathCombine(m_instance->gameRoot(), file.path()); + // Updates retain their old difference-based download behavior. + if (!m_isUpdate) { + bool valid = matchesHash(path, file.hash); + if (file.optional && !valid) { + const auto otherPath = FS::PathCombine(m_instance->gameRoot(), file.alternatePath()); + if (matchesHash(otherPath, file.hash)) { + if ((QFile::exists(path) && !QFile::remove(path)) || !QFile::rename(otherPath, path)) { + m_error = tr("Could not move optional file from %1 to %2.").arg(otherPath, path); + finish(); + return; + } + valid = true; + } + } + if (valid && !completeFile(file)) { + finish(); + return; + } + } + } + if (!saveProgress()) { + finish(); + return; + } + downloadNextRound(); +} + +void ModrinthPackInstallTask::downloadNextRound() +{ + if (m_aborted || !m_error.isEmpty()) { + finish(); + return; + } + auto job = makeShared(tr("Mod Download Modrinth"), APPLICATION->network()); + const auto dependencies = m_pending.remaining()["dependencies"].toObject(); + QString loader; + if (dependencies.contains("fabric-loader")) + loader = ModPlatform::getModLoaderAsString(ModPlatform::ModLoaderType::Fabric); + if (dependencies.contains("quilt-loader")) + loader = ModPlatform::getModLoaderAsString(ModPlatform::ModLoaderType::Quilt); + if (dependencies.contains("forge")) + loader = ModPlatform::getModLoaderAsString(ModPlatform::ModLoaderType::Forge); + if (dependencies.contains("neoforge")) + loader = ModPlatform::getModLoaderAsString(ModPlatform::ModLoaderType::NeoForge); + Net::ModrinthDownloadMeta meta{ .reason = m_isUpdate ? "update" : "modpack", + .gameVersion = dependencies["minecraft"].toString(), + .loader = loader }; + for (auto& file : m_files) { + if (file.completed || file.downloads.isEmpty()) { + continue; + } + const auto path = FS::PathCombine(m_instance->gameRoot(), file.path()); + auto request = Net::ApiDownload::makeFile(file.downloads.dequeue(), path, Net::Download::Option::NoOptions, meta); + request->addValidator(new Net::ChecksumValidator(QCryptographicHash::Sha512, file.hash)); + connect(request.get(), &Task::succeeded, this, [this, &file] { + if (!file.completed && m_error.isEmpty() && !completeFile(file)) { + m_downloadJob->abort(); + } + }); + job->addNetAction(request); + } + if (job->size() == 0) { + finishDownloads(); + return; + } + // A queued continuation also keeps request signal stacks out of staging commit. + connect(job.get(), &Task::finished, this, [this] { + QTimer::singleShot(0, this, [this] { + if (m_aborted || !m_error.isEmpty()) { + finish(); + return; + } + bool hasFallback = false; + for (const auto& file : m_files) { + hasFallback |= !file.completed && !file.downloads.isEmpty(); + } + if (hasFallback) { + downloadNextRound(); + } else { + finishDownloads(); + } + }); + }); + propagateFromOther(job.get()); + m_downloadJob = job; + setStatus(tr("Downloading mods...")); + job->start(); +} + +void ModrinthPackInstallTask::finishDownloads() +{ + QHash resources; + for (const auto& file : m_files) { + if (!file.completed) { + m_error = tr("Could not download all modpack files."); + } else if (!file.unsupported && file.path().startsWith("mods/")) { + auto mod = std::make_unique(FS::PathCombine(m_instance->gameRoot(), file.path())); + ModDetails details; + details.mod_id = mod->fileinfo().absoluteFilePath(); + mod->setDetails(details); + resources[file.hash.toHex()] = mod.get(); + m_resources.push_back(std::move(mod)); + } + } + if (!m_error.isEmpty() || resources.isEmpty()) { + finish(); + return; + } + auto metadata = makeShared(resources, QDir(FS::PathCombine(m_instance->modsRoot(), ".index")), + ModPlatform::ResourceProvider::MODRINTH); + connect(metadata.get(), &Task::failed, this, + [this](const QString& reason) { logWarning(tr("Could not load mod display metadata: %1").arg(reason)); }); + connect(metadata.get(), &Task::finished, this, [this] { QTimer::singleShot(0, this, &ModrinthPackInstallTask::finish); }); + propagateFromOther(metadata.get()); + m_metadataTask = metadata; + metadata->start(); +} + +void ModrinthPackInstallTask::finish() +{ + if (!isRunning()) { + return; + } + if (!m_error.isEmpty()) { + emitFailed(m_error); + } else if (m_aborted) { + emitAborted(); + } else { + // ModpackResumeTask owns marker deletion after successful content work. + // Resume progress, including an empty files array, stays persisted until then. + emitSucceeded(); + } +} diff --git a/launcher/modplatform/modrinth/ModrinthPackInstallTask.h b/launcher/modplatform/modrinth/ModrinthPackInstallTask.h new file mode 100644 index 000000000..f55dc9b30 --- /dev/null +++ b/launcher/modplatform/modrinth/ModrinthPackInstallTask.h @@ -0,0 +1,74 @@ +// SPDX-License-Identifier: GPL-3.0-only +#pragma once + +#include +#include +#include +#include +#include + +#include "modplatform/helpers/PendingModpackManifest.h" +#include "net/NetJob.h" +#include "tasks/Task.h" + +class MinecraftInstance; +class Resource; +class QWidget; + +class ModrinthPackInstallTask final : public Task { + Q_OBJECT + + public: + ModrinthPackInstallTask(MinecraftInstance* instance, + QJsonObject manifest, + Modpack::InstallMode mode, + bool trustedSource, + QWidget* parent = nullptr); + ~ModrinthPackInstallTask() override; + + bool abort() override; + QJsonObject remainingManifest() const; + bool canPreserve() const; + + protected: + void executeTask() override; + + private: + friend class ModrinthCreationTask; + struct File { + QString key; + QString enabledPath; + QString disabledPath; + QByteArray hash; + QQueue downloads; + bool optional = false; + bool enabled = true; + bool unsupported = false; + bool completed = false; + + QString path() const { return enabled ? enabledPath : disabledPath; } + QString alternatePath() const { return enabled ? disabledPath : enabledPath; } + }; + + bool prepare(); + bool validatePath(const QString& path) const; + bool completeFile(File& file); + bool saveProgress(); + void downloadNextRound(); + void finishDownloads(); + void finish(); + + MinecraftInstance* m_instance; + Modpack::PendingManifest m_pending; + Modpack::InstallMode m_mode; + bool m_trustedSource; + QWidget* m_parent; + bool m_isUpdate = false; + bool m_validated = false; + bool m_aborted = false; + QString m_error; + std::vector m_files; + NetJob::Ptr m_downloadJob; + Task::Ptr m_metadataTask; + std::vector> m_resources; +}; From 24c0010f445c67c07055a3d8b737fdf9554f509d Mon Sep 17 00:00:00 2001 From: HowXu Date: Sat, 24 Oct 2026 11:52:56 +0800 Subject: [PATCH 05/13] feat(curseforge): Resume pending pack content Resolve remaining native IDs, retain interrupted downloads, validate manual files and resource processing, and rollback uncommitted worlds safely. Assisted-by: OpenCode:gpt-6.1-sol-fast Signed-off-by: HowXu --- launcher/CMakeLists.txt | 2 + .../modplatform/flame/FileResolvingTask.cpp | 45 +- .../modplatform/flame/FileResolvingTask.h | 4 + .../flame/FlameInstanceCreationTask.cpp | 349 +++-------- .../flame/FlameInstanceCreationTask.h | 20 +- .../flame/FlamePackInstallTask.cpp | 591 ++++++++++++++++++ .../modplatform/flame/FlamePackInstallTask.h | 57 ++ 7 files changed, 777 insertions(+), 291 deletions(-) create mode 100644 launcher/modplatform/flame/FlamePackInstallTask.cpp create mode 100644 launcher/modplatform/flame/FlamePackInstallTask.h diff --git a/launcher/CMakeLists.txt b/launcher/CMakeLists.txt index 3eded8171..bc2527abf 100644 --- a/launcher/CMakeLists.txt +++ b/launcher/CMakeLists.txt @@ -582,6 +582,8 @@ set(FLAME_SOURCES modplatform/flame/FlameCheckUpdate.h modplatform/flame/FlameInstanceCreationTask.h modplatform/flame/FlameInstanceCreationTask.cpp + modplatform/flame/FlamePackInstallTask.h + modplatform/flame/FlamePackInstallTask.cpp modplatform/flame/FlamePackExportTask.h modplatform/flame/FlamePackExportTask.cpp ) diff --git a/launcher/modplatform/flame/FileResolvingTask.cpp b/launcher/modplatform/flame/FileResolvingTask.cpp index 27cd18836..1a56e64c3 100644 --- a/launcher/modplatform/flame/FileResolvingTask.cpp +++ b/launcher/modplatform/flame/FileResolvingTask.cpp @@ -19,6 +19,7 @@ #include "FileResolvingTask.h" #include +#include "FileSystem.h" #include "Json.h" #include "modplatform/ModIndex.h" #include "modplatform/flame/FlameAPI.h" @@ -47,7 +48,10 @@ bool Flame::FileResolvingTask::abort() void Flame::FileResolvingTask::executeTask() { + m_filesResolved = false; + m_downloadFailed = false; if (m_manifest.files.isEmpty()) { // no file to resolve so leave it empty and emit success immediately + m_filesResolved = true; emitSucceeded(); return; } @@ -68,6 +72,7 @@ void Flame::FileResolvingTask::executeTask() netJobFinished(response); }); connect(m_task.get(), &Task::failed, this, [this, step_progress](QString reason) { + m_downloadFailed = true; step_progress->state = TaskStepState::Failed; stepProgress(*step_progress); emitFailed(reason); @@ -124,20 +129,30 @@ void Flame::FileResolvingTask::netJobFinished(QByteArray* response) qCritical() << "Non-JSON data returned from the CF API"; qCritical() << e.cause(); - emitFailed(tr("Invalid data returned from the API.")); + emitFailed(tr("Invalid file data returned by the CurseForge API: %1").arg(e.cause())); return; } QStringList hashes; + QMap versions; for (QJsonValueRef file : array) { try { auto obj = Json::requireObject(file); + const auto fileid = Json::requireInteger(obj, "id"); + const auto projectid = Json::requireInteger(obj, "modId"); + if (!m_manifest.files.contains(fileid) || versions.contains(fileid) || m_manifest.files[fileid].projectId != projectid) + throw Json::JsonException(tr("Unexpected or duplicate file ID %1 returned by the API.").arg(fileid)); + const auto name = Json::requireString(obj, "fileName"); + if (name.isEmpty() || name == "." || name == ".." || name.contains('/') || name.contains('\\') || + name != FS::RemoveInvalidPathChars(name)) + throw Json::JsonException(tr("Invalid filename returned by the API: %1").arg(name)); auto version = FlameMod::loadIndexedPackVersion(obj); - auto fileid = version.fileId.toInt(); - Q_ASSERT(fileid != 0); - Q_ASSERT(m_manifest.files.contains(fileid)); - m_manifest.files[fileid].version = version; + const auto hash = version.hash.toLatin1(); + if (!((version.hash_type == "sha1" && hash.size() == 40) || (version.hash_type == "md5" && hash.size() == 32)) || + QByteArray::fromHex(hash).toHex() != hash.toLower()) + throw Json::JsonException(tr("Invalid file hash returned by the API for file %1.").arg(fileid)); + versions.insert(fileid, version); auto url = QUrl(version.downloadUrl, QUrl::TolerantMode); if (!url.isValid() && "sha1" == version.hash_type && !version.hash.isEmpty()) { hashes.push_back(version.hash); @@ -146,11 +161,20 @@ void Flame::FileResolvingTask::netJobFinished(QByteArray* response) qCritical() << "Non-JSON data returned from the CF API"; qCritical() << e.cause(); - emitFailed(tr("Invalid data returned from the API.")); + emitFailed(tr("Invalid file data returned by the CurseForge API: %1").arg(e.cause())); return; } } + if (versions.size() != m_manifest.files.size()) { + emitFailed(tr("The API did not return information for all requested files.")); + return; + } + for (auto file = versions.constBegin(); file != versions.constEnd(); ++file) + m_manifest.files[file.key()].version = file.value(); + // Only a completely validated file response may become a metadata-only + // failure later. Never infer this from partially populated file fields. + m_filesResolved = true; if (hashes.isEmpty()) { getFlameProjects(); return; @@ -235,6 +259,11 @@ void Flame::FileResolvingTask::getFlameProjects() qWarning() << "Error while parsing JSON response from Modrinth projects task at" << parse_error.offset << "reason:" << parse_error.errorString(); qWarning() << *response; + step_progress->state = TaskStepState::Failed; + stepProgress(*step_progress); + emitFailed(tr("Could not parse the CurseForge project response at offset %1: %2") + .arg(parse_error.offset) + .arg(parse_error.errorString())); return; } @@ -261,6 +290,10 @@ void Flame::FileResolvingTask::getFlameProjects() } catch (Json::JsonException& e) { qDebug() << e.cause(); qDebug() << doc; + step_progress->state = TaskStepState::Failed; + stepProgress(*step_progress); + emitFailed(tr("Invalid project data returned by the CurseForge API: %1").arg(e.cause())); + return; } step_progress->state = TaskStepState::Succeeded; stepProgress(*step_progress); diff --git a/launcher/modplatform/flame/FileResolvingTask.h b/launcher/modplatform/flame/FileResolvingTask.h index 21fa53d2d..60ae586b2 100644 --- a/launcher/modplatform/flame/FileResolvingTask.h +++ b/launcher/modplatform/flame/FileResolvingTask.h @@ -31,6 +31,8 @@ class FileResolvingTask : public Task { bool abort() override; const Flame::Manifest& getResults() const { return m_manifest; } + bool filesResolved() const { return m_filesResolved; } + bool downloadFailed() const { return m_downloadFailed; } protected: virtual void executeTask() override; @@ -44,5 +46,7 @@ class FileResolvingTask : public Task { private: /* data */ Flame::Manifest m_manifest; Task::Ptr m_task; + bool m_filesResolved = false; + bool m_downloadFailed = false; }; } // namespace Flame diff --git a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp index 3775f3391..249e07af3 100644 --- a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp +++ b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp @@ -37,8 +37,6 @@ #include "InstanceTask.h" #include "QObjectPtr.h" -#include "minecraft/mod/tasks/LocalResourceUpdateTask.h" -#include "modplatform/flame/FileResolvingTask.h" #include "modplatform/flame/FlameAPI.h" #include "modplatform/flame/FlameModIndex.h" #include "modplatform/flame/PackManifest.h" @@ -56,20 +54,15 @@ #include "settings/INISettingsObject.h" #include "SysInfo.h" -#include "tasks/ConcurrentTask.h" -#include "ui/dialogs/BlockedModsDialog.h" #include "ui/dialogs/CustomMessageBox.h" #include +#include #include #include "HardwareInfo.h" #include "meta/Index.h" -#include "minecraft/World.h" -#include "minecraft/mod/tasks/LocalResourceParse.h" -#include "net/ApiDownload.h" #include "ui/dialogs/UntrustedModsDialog.h" -#include "ui/pages/modplatform/OptionalModDialog.h" static const FlameAPI api; @@ -78,12 +71,15 @@ bool FlameCreationTask::abort() if (!canAbort()) return false; + if (m_contentTask) { + m_abort = true; + m_contentTask->abort(); + // createInstance() waits for the content task before offering to keep + // staging. Never let an early abort signal delete that directory. + return true; + } if (m_processUpdateFileInfoJob) m_processUpdateFileInfoJob->abort(); - if (m_filesJob) - m_filesJob->abort(); - if (m_modIdResolver) - m_modIdResolver->abort(); return InstanceCreationTask::abort(); } @@ -338,16 +334,40 @@ std::unique_ptr FlameCreationTask::createInstance() QEventLoop loop; QString parent_folder(FS::PathCombine(m_stagingPath, "flame")); + QJsonObject original_manifest; + + // Imported overrides and all parents of files written below must remain + // inside staging, including symlinks that a lexical path check cannot see. + QDirIterator paths(m_stagingPath, QDir::AllEntries | QDir::Hidden | QDir::System | QDir::NoDotAndDotDot, QDirIterator::Subdirectories); + while (paths.hasNext()) { + if (QFileInfo(paths.next()).isSymLink()) { + setError(tr("The modpack contains a symbolic link. Installing it is not allowed.")); + return nullptr; + } + } try { QString index_path(FS::PathCombine(m_stagingPath, "manifest.json")); + original_manifest = Json::requireObject(Json::requireDocument(index_path)); + if (!original_manifest.value("files").isArray()) { + setError(tr("The pack manifest does not contain a valid file list.")); + return nullptr; + } + for (const auto& entry : original_manifest.value("files").toArray()) { + if (entry.toObject().value("projectID").toInt() <= 0 || entry.toObject().value("fileID").toInt() <= 0) { + setError(tr("The pack manifest contains an invalid file ID.")); + return nullptr; + } + } if (!m_pack.is_loaded) Flame::loadManifest(m_pack, index_path); // Keep index file in case we need it some other time (like when changing versions) QString new_index_place(FS::PathCombine(parent_folder, "manifest.json")); - FS::ensureFilePathExists(new_index_place); - FS::move(index_path, new_index_place); + if (!FS::ensureFilePathExists(new_index_place) || !FS::move(index_path, new_index_place)) { + setError(tr("Could not save the original pack manifest.")); + return nullptr; + } } catch (const JSONValidationError& e) { setError(tr("Could not understand pack manifest:\n") + e.cause()); @@ -491,23 +511,52 @@ std::unique_ptr FlameCreationTask::createInstance() instance->setName(name()); - m_modIdResolver.reset(new Flame::FileResolvingTask(m_pack)); - connect(m_modIdResolver.get(), &Flame::FileResolvingTask::succeeded, this, [this, &loop] { idResolverSucceeded(loop); }); - connect(m_modIdResolver.get(), &Flame::FileResolvingTask::failed, [this, &loop](QString reason) { - m_modIdResolver.reset(); - setError(tr("Unable to resolve mod IDs:\n") + reason); - loop.quit(); - }); - connect(m_modIdResolver.get(), &Flame::FileResolvingTask::aborted, &loop, &QEventLoop::quit); - connect(m_modIdResolver.get(), &Flame::FileResolvingTask::progress, this, &FlameCreationTask::setProgress); - connect(m_modIdResolver.get(), &Flame::FileResolvingTask::status, this, &FlameCreationTask::setStatus); - connect(m_modIdResolver.get(), &Flame::FileResolvingTask::stepProgress, this, &FlameCreationTask::propagateStepProgress); - connect(m_modIdResolver.get(), &Flame::FileResolvingTask::details, this, &FlameCreationTask::setDetails); - m_modIdResolver->start(); - - loop.exec(); + auto content_manifest = original_manifest; + if (shouldOverride()) { + // updateInstance() already excluded unchanged IDs. This transient copy + // must not replace the complete manifest saved above. + QJsonArray files; + for (const auto& entry : original_manifest.value("files").toArray()) { + if (m_pack.files.contains(entry.toObject().value("fileID").toInt())) + files.append(entry); + } + content_manifest.insert("files", files); + } + if (m_abort) + return nullptr; + m_contentTask = + makeShared(instance.get(), content_manifest, Modpack::InstallMode::Create, m_trustedSource, m_parent); + auto content = m_contentTask; + connect(content.get(), &Task::finished, &loop, &QEventLoop::quit, Qt::QueuedConnection); + connect(content.get(), &Task::warningLogged, this, &FlameCreationTask::logWarning); + propagateFromOther(content.get()); + content->start(); + if (!content->isFinished()) + loop.exec(); + m_contentTask.reset(); - bool did_succeed = getError().isEmpty(); + bool did_succeed = content->wasSuccessful() && !m_abort; + if (!did_succeed && !shouldOverride() && content->canPreserve()) { + setAbortable(false); + if (askToKeepIncompleteModpack(m_parent, m_globalSettings)) { + if (!content->canPreserve()) { + setError(tr("The incomplete instance contains an unsafe path and cannot be saved.")); + return nullptr; + } + Modpack::PendingManifest pending(Modpack::Format::CurseForge, content->remainingManifest()); + QString error; + if (!pending.save(Modpack::pendingManifestPath(instance->instanceRoot(), Modpack::Format::CurseForge), &error)) { + setError(tr("Could not save remaining modpack files:\n%1").arg(error)); + return nullptr; + } + instance->saveNow(); + setContentIncomplete(true); + m_abort = false; + return instance; + } + } + if (!did_succeed && !m_abort) + setError(content->failReason()); // Update information of the already installed instance, if any. if (m_instance && did_succeed) { @@ -522,243 +571,3 @@ std::unique_ptr FlameCreationTask::createInstance() } return nullptr; } - -void FlameCreationTask::idResolverSucceeded(QEventLoop& loop) -{ - auto results = m_modIdResolver->getResults().files; - - QStringList optionalFiles; - for (auto& result : results) { - if (!result.required) { - optionalFiles << FS::PathCombine(result.targetFolder, result.version.fileName); - } - } - - if (!optionalFiles.empty()) { - OptionalModDialog optionalModDialog(m_parent, optionalFiles); - if (optionalModDialog.exec() == QDialog::Rejected) { - emitAborted(); - loop.quit(); - return; - } - - m_selectedOptionalMods = optionalModDialog.getResult(); - } - - // first check for blocked mods - QList blocked_mods; - auto anyBlocked = false; - for (const auto& result : results.values()) { - if (result.resourceType != ModPlatform::ResourceType::Mod) { - m_otherResources.append(std::make_pair(result.version.fileName, result.targetFolder)); - } - - // skip optional mods that were not selected - if (result.version.downloadUrl.isEmpty()) { - BlockedMod blocked_mod; - blocked_mod.name = result.version.fileName; - blocked_mod.websiteUrl = QString("%1/download/%2").arg(result.pack.websiteUrl, QString::number(result.fileId)); - blocked_mod.hash = result.version.hash; - blocked_mod.matched = false; - blocked_mod.localPath = ""; - blocked_mod.targetFolder = result.targetFolder; - auto fileName = result.version.fileName; - fileName = FS::RemoveInvalidPathChars(fileName); - auto relpath = FS::PathCombine(result.targetFolder, fileName); - blocked_mod.disabled = !result.required && !m_selectedOptionalMods.contains(relpath); - - blocked_mods.append(blocked_mod); - - anyBlocked = true; - } - } - if (anyBlocked) { - qWarning() << "Blocked mods found, displaying mod list"; - - BlockedModsDialog message_dialog(m_parent, tr("Blocked mods found"), - tr("The following files are not available for download in third party launchers.
" - "You will need to manually download them and add them to the instance."), - blocked_mods); - - message_dialog.setModal(true); - - if (message_dialog.exec()) { - qDebug() << "Post dialog blocked mods list:" << blocked_mods; - copyBlockedMods(blocked_mods); - setupDownloadJob(loop); - } else { - m_modIdResolver.reset(); - setError("Canceled"); - loop.quit(); - } - } else { - setupDownloadJob(loop); - } -} - -void FlameCreationTask::setupDownloadJob(QEventLoop& loop) -{ - m_filesJob.reset(new NetJob(tr("Mod Download Flame"), APPLICATION->network())); - auto results = m_modIdResolver->getResults().files; - - for (const auto& result : results) { - auto fileName = result.version.fileName; - fileName = FS::RemoveInvalidPathChars(fileName); - auto relpath = FS::PathCombine(result.targetFolder, fileName); - - if (!result.required && !m_selectedOptionalMods.contains(relpath)) { - relpath += ".disabled"; - } - - relpath = FS::PathCombine("minecraft", relpath); - auto path = FS::PathCombine(m_stagingPath, relpath); - - if (!result.version.downloadUrl.isEmpty()) { - qDebug() << "Will download" << result.version.downloadUrl << "to" << path; - auto dl = Net::ApiDownload::makeFile(result.version.downloadUrl, path); - m_filesJob->addNetAction(dl); - } - } - - connect(m_filesJob.get(), &NetJob::finished, this, [this, &loop]() { - m_filesJob.reset(); - validateOtherResources(loop); - }); - connect(m_filesJob.get(), &NetJob::failed, [this](QString reason) { - m_filesJob.reset(); - setError(reason); - }); - connect(m_filesJob.get(), &NetJob::progress, this, [this](qint64 current, qint64 total) { - setDetails(tr("%1 out of %2 complete").arg(current).arg(total)); - setProgress(current, total); - }); - connect(m_filesJob.get(), &NetJob::stepProgress, this, &FlameCreationTask::propagateStepProgress); - - setStatus(tr("Downloading mods...")); - m_filesJob->start(); -} - -/// @brief copy the matched blocked mods to the instance staging area -/// @param blocked_mods list of the blocked mods and their matched paths -void FlameCreationTask::copyBlockedMods(QList const& blocked_mods) -{ - setStatus(tr("Copying Blocked Mods...")); - setAbortable(false); - int i = 0; - int total = blocked_mods.length(); - setProgress(i, total); - for (auto const& mod : blocked_mods) { - if (!mod.matched) { - qDebug() << mod.name << "was not matched to a local file, skipping copy"; - continue; - } - - auto destPath = FS::PathCombine(m_stagingPath, "minecraft", mod.targetFolder, mod.name); - if (mod.disabled) - destPath += ".disabled"; - - setStatus(tr("Copying Blocked Mods (%1 out of %2 are done)").arg(QString::number(i), QString::number(total))); - - qDebug() << "Will try to copy" << mod.localPath << "to" << destPath; - - if (mod.move) { - if (!FS::move(mod.localPath, destPath)) { - qDebug() << "Move of" << mod.localPath << "to" << destPath << "Failed"; - } - } else { - if (!FS::copy(mod.localPath, destPath)()) { - qDebug() << "Copy of" << mod.localPath << "to" << destPath << "Failed"; - } - } - - i++; - setProgress(i, total); - } - - setAbortable(true); -} - -void FlameCreationTask::validateOtherResources(QEventLoop& loop) -{ - qDebug() << "Validating whether other resources are in the right place"; - QStringList zipMods; - for (auto [fileName, targetFolder] : m_otherResources) { - qDebug() << "Checking" << fileName << "..."; - auto localPath = FS::PathCombine(m_stagingPath, "minecraft", targetFolder, fileName); - - /// @brief check the target and move the the file - /// @return path where file can now be found - auto validatePath = [&localPath, this](QString fileName, QString targetFolder, QString realTarget) { - if (targetFolder != realTarget) { - qDebug() << "Target folder of" << fileName << "is incorrect, it belongs in" << realTarget; - auto destPath = FS::PathCombine(m_stagingPath, "minecraft", realTarget, fileName); - qDebug() << "Moving" << localPath << "to" << destPath; - if (FS::move(localPath, destPath)) { - return destPath; - } - } else { - qDebug() << "Target folder of" << fileName << "is correct at" << targetFolder; - } - return localPath; - }; - - auto installWorld = [this](QString worldPath) { - qDebug() << "Installing World from" << worldPath; - QFileInfo worldFileInfo(worldPath); - World w(worldFileInfo); - if (!w.isValid()) { - qDebug() << "World at" << worldPath << "is not valid, skipping install."; - } else { - w.install(FS::PathCombine(m_stagingPath, "minecraft", "saves")); - } - }; - - QFileInfo localFileInfo(localPath); - auto type = ResourceUtils::identify(localFileInfo); - - QString worldPath; - - switch (type) { - case ModPlatform::ResourceType::Mod: - validatePath(fileName, targetFolder, "mods"); - zipMods.push_back(fileName); - break; - case ModPlatform::ResourceType::ResourcePack: - validatePath(fileName, targetFolder, "resourcepacks"); - break; - case ModPlatform::ResourceType::TexturePack: - validatePath(fileName, targetFolder, "texturepacks"); - break; - case ModPlatform::ResourceType::DataPack: - validatePath(fileName, targetFolder, "datapacks"); - break; - case ModPlatform::ResourceType::ShaderPack: - // in theory flame API can't do this but who knows, that *may* change ? - // better to handle it if it *does* occur in the future - validatePath(fileName, targetFolder, "shaderpacks"); - break; - case ModPlatform::ResourceType::World: - worldPath = validatePath(fileName, targetFolder, "saves"); - installWorld(worldPath); - break; - case ModPlatform::ResourceType::Unknown: - /* fallthrough */ - default: - qDebug() << "Can't Identify" << fileName << "at" << localPath << ", leaving it where it is."; - break; - } - } - // TODO make this work with other sorts of resource - auto task = makeShared("CreateModMetadata", APPLICATION->settings()->get("NumberOfConcurrentTasks").toInt()); - auto results = m_modIdResolver->getResults().files; - auto folder = FS::PathCombine(m_stagingPath, "minecraft", "mods", ".index"); - for (auto file : results) { - if (file.targetFolder != "mods" || (file.version.fileName.endsWith(".zip") && !zipMods.contains(file.version.fileName))) { - continue; - } - task->addTask(makeShared(folder, file.pack, file.version)); - } - connect(task.get(), &Task::finished, &loop, &QEventLoop::quit); - m_processUpdateFileInfoJob = task; - task->start(); -} diff --git a/launcher/modplatform/flame/FlameInstanceCreationTask.h b/launcher/modplatform/flame/FlameInstanceCreationTask.h index 8710bd367..082b93217 100644 --- a/launcher/modplatform/flame/FlameInstanceCreationTask.h +++ b/launcher/modplatform/flame/FlameInstanceCreationTask.h @@ -41,11 +41,7 @@ #include "minecraft/MinecraftInstance.h" -#include "modplatform/flame/FileResolvingTask.h" - -#include "net/NetJob.h" - -#include "ui/dialogs/BlockedModsDialog.h" +#include "modplatform/flame/FlamePackInstallTask.h" class FlameCreationTask final : public InstanceCreationTask { Q_OBJECT @@ -72,30 +68,24 @@ class FlameCreationTask final : public InstanceCreationTask { std::unique_ptr createInstance() override; private slots: - void idResolverSucceeded(QEventLoop&); - void setupDownloadJob(QEventLoop&); - void copyBlockedMods(QList const& blocked_mods); - void validateOtherResources(QEventLoop& loop); QString getVersionForLoader(QString uid, QString loaderType, QString version, QString mcVersion); [[nodiscard]] bool promptForUntrustedMods(); + protected: + bool keepOnGameDownloadFailure() const override { return !shouldOverride(); } + private: QWidget* m_parent = nullptr; bool m_trustedSource; - shared_qobject_ptr m_modIdResolver; Flame::Manifest m_pack; // Handle to allow aborting Task::Ptr m_processUpdateFileInfoJob = nullptr; - NetJob::Ptr m_filesJob = nullptr; + shared_qobject_ptr m_contentTask; QString m_managedId, m_managedVersionId; - QList> m_otherResources; - std::optional m_instance; - - QStringList m_selectedOptionalMods; }; diff --git a/launcher/modplatform/flame/FlamePackInstallTask.cpp b/launcher/modplatform/flame/FlamePackInstallTask.cpp new file mode 100644 index 000000000..19f97e711 --- /dev/null +++ b/launcher/modplatform/flame/FlamePackInstallTask.cpp @@ -0,0 +1,591 @@ +// SPDX-License-Identifier: GPL-3.0-only +/* + * Content installation extracted from FlameInstanceCreationTask.cpp. + * Copyright (C) 2022 Sefa Eyeoglu + * Copyright 2013-2021 MultiMC Contributors + * Portions licensed under Apache-2.0; see FlameInstanceCreationTask.cpp. + */ + +#include "FlamePackInstallTask.h" + +#include +#include +#include +#include +#include +#include +#include + +#include "Application.h" +#include "FileSystem.h" +#include "MMCZip.h" +#include "archive/ArchiveReader.h" +#include "minecraft/MinecraftInstance.h" +#include "minecraft/World.h" +#include "minecraft/mod/tasks/LocalResourceParse.h" +#include "minecraft/mod/tasks/LocalResourceUpdateTask.h" +#include "modplatform/flame/FileResolvingTask.h" +#include "net/ApiDownload.h" +#include "net/ChecksumValidator.h" +#include "tasks/ConcurrentTask.h" +#include "ui/dialogs/BlockedModsDialog.h" +#include "ui/dialogs/UntrustedModsDialog.h" +#include "ui/pages/modplatform/OptionalModDialog.h" + +namespace { +QString fileKey(const Flame::File& file) +{ + return QString::number(file.projectId) + ':' + QString::number(file.fileId); +} + +bool safePath(const QString& root, const QString& path) +{ + const auto relative = QDir(root).relativeFilePath(path); + if (QDir::isAbsolutePath(relative) || relative == ".." || relative.startsWith("../")) + return false; + auto current = QDir(root).absolutePath(); + if (QFileInfo(current).isSymLink()) + return false; + for (const auto& part : relative.split('/')) { + current = QDir(current).filePath(part); + if (QFileInfo(current).isSymLink()) + return false; + } + return true; +} + +bool safeTree(const QString& root) +{ + if (!safePath(root, root)) + return false; + QDirIterator it(root, QDir::AllEntries | QDir::Hidden | QDir::System | QDir::NoDotAndDotDot, QDirIterator::Subdirectories); + while (it.hasNext()) { + if (QFileInfo(it.next()).isSymLink()) + return false; + } + return true; +} + +std::optional hashAlgorithm(const Flame::File& file) +{ + const auto hex = file.version.hash.toLatin1(); + if (QByteArray::fromHex(hex).toHex() != hex.toLower()) + return {}; + if (file.version.hash_type == "sha1" && file.version.hash.size() == 40) + return QCryptographicHash::Sha1; + if (file.version.hash_type == "md5" && file.version.hash.size() == 32) + return QCryptographicHash::Md5; + return {}; +} + +// NetJob::abort() can finish before all requests have unwound. Cancel queued +// work, but drain active requests normally before reporting a terminal state. +class ContentDownloadJob final : public NetJob { + public: + using NetJob::NetJob; + bool abort() override + { + m_stopping = true; + m_queue.clear(); + setAskRetry(false); + QMetaObject::invokeMethod(this, &ContentDownloadJob::executeNextSubTask, Qt::QueuedConnection); + return true; + } + + protected: + void executeNextSubTask() override + { + if (!m_stopping) { + NetJob::executeNextSubTask(); + } else if (isRunning() && m_doing.isEmpty()) { + emitAborted(); + } + } + + private: + bool m_stopping = false; +}; + +bool containsWorld(const QString& path) +{ + // Inspect the archive directly: World only recognizes an on-disk .zip + // suffix, whereas optional downloads can be named .zip.disabled. + MMCZip::ArchiveReader archive(path); + bool found = false; + return archive.parse([&](MMCZip::ArchiveReader::File* entry, bool& stop) { + found = entry->isFile() && QFileInfo(entry->filename()).fileName().compare("level.dat", Qt::CaseInsensitive) == 0; + stop = found; + return true; + }) && found; +} +} // namespace + +FlamePackInstallTask::FlamePackInstallTask(MinecraftInstance* instance, + QJsonObject manifest, + Modpack::InstallMode mode, + bool trustedSource, + QWidget* parent) + : m_instance(instance) + , m_pending(Modpack::Format::CurseForge, std::move(manifest)) + , m_mode(mode) + , m_trustedSource(trustedSource) + , m_parent(parent) +{} + +bool FlamePackInstallTask::abort() +{ + m_abort = true; + setStatus(tr("Stopping installation; waiting for active requests to finish...")); + if (m_dialog) + m_dialog->reject(); + if (m_filesJob) + m_filesJob->abort(); + // FileResolvingTask has the same early-abort issue. Let its API requests + // settle; executeTask() observes the cancellation before writing any files. + return true; +} + +bool FlamePackInstallTask::canPreserve() const +{ + return m_canPreserve && safeTree(m_instance->instanceRoot()); +} + +void FlamePackInstallTask::fail(const QString& reason, bool unsafe) +{ + if (m_error.isEmpty()) + m_error = reason; + if (unsafe) + m_canPreserve = false; +} + +bool FlamePackInstallTask::checkPath(const QString& path) +{ + if (safePath(m_instance->instanceRoot(), path)) + return true; + fail(tr("Unsafe modpack path: %1").arg(path), true); + return false; +} + +bool FlamePackInstallTask::runTask(const Task::Ptr& task) +{ + QEventLoop loop; + connect(task.get(), &Task::finished, &loop, &QEventLoop::quit, Qt::QueuedConnection); + propagateFromOther(task.get()); + task->start(); + if (!task->isFinished()) + loop.exec(); + return task->wasSuccessful(); +} + +bool FlamePackInstallTask::validFile(const Flame::File& file, const QString& path) const +{ + auto algorithm = hashAlgorithm(file); + if (!algorithm || !safePath(m_instance->instanceRoot(), path) || !QFileInfo(path).isFile()) + return false; + QFile input(path); + QCryptographicHash hash(*algorithm); + return input.open(QIODevice::ReadOnly) && hash.addData(&input) && + hash.result().toHex().compare(file.version.hash.toLatin1(), Qt::CaseInsensitive) == 0; +} + +QString FlamePackInstallTask::reusableFile(const Flame::File& file, bool disabled) +{ + QStringList folders{ file.targetFolder }; + if (file.resourceType != ModPlatform::ResourceType::Mod) + folders << "mods" << "resourcepacks" << "texturepacks" << "datapacks" << "shaderpacks" << "saves"; + folders.removeDuplicates(); + for (const auto& folder : folders) { + const auto enabledPath = FS::PathCombine(m_instance->gameRoot(), folder, file.version.fileName); + const auto path = enabledPath + (disabled ? ".disabled" : ""); + const auto alternate = enabledPath + (disabled ? "" : ".disabled"); + if (!checkPath(path) || !checkPath(alternate)) + return {}; + if (!validFile(file, path) && validFile(file, alternate)) { + if ((QFile::exists(path) && !QFile::remove(path)) || !FS::move(alternate, path)) { + fail(tr("Could not restore the selected state of %1.").arg(path)); + return {}; + } + } + if (validFile(file, path)) + return path; + } + return {}; +} + +bool FlamePackInstallTask::saveProgress() +{ + if (m_mode != Modpack::InstallMode::Resume) + return true; + // Keep even an empty marker here; ModpackResumeTask owns its removal after + // the complete content task has succeeded. + const auto path = Modpack::pendingManifestPath(m_instance->instanceRoot(), Modpack::Format::CurseForge); + QString error; + if (!checkPath(path) || !m_pending.save(path, &error)) { + fail(tr("Could not save remaining modpack files: %1").arg(error), true); + if (m_filesJob) + m_filesJob->abort(); + return false; + } + return true; +} + +bool FlamePackInstallTask::installWorld(const QString& path, bool disabled, QString* installedPath) +{ + const auto saves = QDir(m_instance->gameRoot()).filePath("saves"); + if (!checkPath(saves) || !FS::ensureFolderPathExists(saves)) { + fail(tr("Could not create the world saves directory: %1").arg(saves)); + return false; + } + QTemporaryDir temporary(QDir(saves).filePath(".flame-world-XXXXXX")); + const auto extracted = QDir(temporary.path()).filePath("world"); + if (!temporary.isValid() || !checkPath(extracted)) { + fail(tr("Could not create a temporary directory for world installation.")); + return false; + } + + // World::install() does not reliably propagate extraction failure. Use the + // same extractor, checking its result and every archive path before writing. + MMCZip::ArchiveReader archive(path); + QString prefix; + bool foundLevel = false; + if (!archive.parse([&](MMCZip::ArchiveReader::File* entry) { + const auto name = entry->filename(); + if (QDir::isAbsolutePath(name) || name.contains('\\') || name.split('/').contains("..") || + !checkPath(QDir(extracted).filePath(name))) { + fail(tr("Unsafe world archive path: %1").arg(name), true); + return false; + } + if (!foundLevel && entry->isFile() && QFileInfo(name).fileName().compare("level.dat", Qt::CaseInsensitive) == 0) { + prefix = name.chopped(QString("level.dat").size()); + foundLevel = true; + } + return true; + }) || + !foundLevel || !FS::ensureFolderPathExists(extracted) || !MMCZip::extractSubDir(&archive, prefix, extracted).has_value()) { + fail(tr("Could not install world: %1").arg(path)); + return false; + } + if (!safeTree(extracted)) { + fail(tr("The installed resource contains an unsafe symbolic link."), true); + return false; + } + World world{ QFileInfo(extracted) }; + if (!world.isValid()) { + fail(tr("The extracted world is not valid: %1").arg(path)); + return false; + } + // An unselected optional world remains a verified disabled archive. The + // temporary extracted copy is only for validation and is removed on return. + if (disabled) + return true; + const auto folder = FS::DirNameFromString(world.name(), saves); + const auto target = QDir(saves).filePath(folder); + if (folder.isEmpty() || !checkPath(target) || !QDir(saves).mkdir(folder)) { + fail(tr("Could not install world: %1").arg(path)); + return false; + } + // Successful mkdir reserves a fresh directory. Only this call's directory + // may be rolled back if the move or the following marker save fails. + *installedPath = target; + if (!FS::move(extracted, target)) { + fail(tr("Could not install world: %1").arg(path)); + rollbackWorld(target); + installedPath->clear(); + return false; + } + return true; +} + +void FlamePackInstallTask::rollbackWorld(const QString& path) +{ + if (path.isEmpty()) + return; + if (!checkPath(path) || !FS::deletePath(path)) { + const auto reason = tr("Could not remove the uncommitted world directory: %1").arg(path); + logWarning(reason); + fail(reason, true); + } +} + +bool FlamePackInstallTask::completeFile(const Flame::File& file, QString path, bool disabled) +{ + const auto key = fileKey(file); + if (m_completed.contains(key)) + return true; + if (!validFile(file, path)) { + fail(tr("File is missing or failed hash validation: %1").arg(path)); + return false; + } + const auto alternate = disabled ? path.chopped(9) : path + ".disabled"; + if (!checkPath(alternate)) + return false; + if (QFile::exists(alternate) && !QFile::remove(alternate)) { + fail(tr("Could not remove duplicate resource: %1").arg(alternate)); + return false; + } + + auto type = file.resourceType; + if (type != ModPlatform::ResourceType::Mod) + type = ResourceUtils::identify(QFileInfo(path)); + // A project explicitly classified as a world must never be marked complete + // merely because resource identification failed. + if (file.resourceType == ModPlatform::ResourceType::World) + type = ModPlatform::ResourceType::World; + if (type == ModPlatform::ResourceType::Unknown && file.version.fileName.endsWith(".zip") && containsWorld(path)) + type = ModPlatform::ResourceType::World; + QString folder; + switch (type) { + case ModPlatform::ResourceType::Mod: + folder = "mods"; + break; + case ModPlatform::ResourceType::ResourcePack: + folder = "resourcepacks"; + break; + case ModPlatform::ResourceType::TexturePack: + folder = "texturepacks"; + break; + case ModPlatform::ResourceType::DataPack: + folder = "datapacks"; + break; + case ModPlatform::ResourceType::ShaderPack: + folder = "shaderpacks"; + break; + case ModPlatform::ResourceType::World: + folder = "saves"; + break; + default: + break; + } + if (!folder.isEmpty()) { + const auto destination = FS::PathCombine(m_instance->gameRoot(), folder, QFileInfo(path).fileName()); + if (!checkPath(destination)) + return false; + if (path != destination) { + if (!FS::ensureFilePathExists(destination) || !FS::move(path, destination)) { + fail(tr("Could not move resource to %1").arg(destination)); + return false; + } + path = destination; + } + const auto otherState = disabled ? path.chopped(9) : path + ".disabled"; + if (!checkPath(otherState)) + return false; + if (QFile::exists(otherState) && !QFile::remove(otherState)) { + fail(tr("Could not remove duplicate resource: %1").arg(otherState)); + return false; + } + } + QString installedWorld; + if (type == ModPlatform::ResourceType::World && !installWorld(path, disabled, &installedWorld)) + return false; + if (type == ModPlatform::ResourceType::World && !safeTree(m_instance->instanceRoot())) { + fail(tr("The installed resource contains an unsafe symbolic link."), true); + rollbackWorld(installedWorld); + return false; + } + + const auto previous = m_pending; + m_pending.markCompleted(key); + if (!saveProgress()) { + m_pending = previous; + rollbackWorld(installedWorld); + return false; + } + m_completed.insert(key); + if (type == ModPlatform::ResourceType::Mod) + m_metadataFiles.append(file); + return true; +} + +void FlamePackInstallTask::executeTask() +{ + const auto original = m_pending.remaining(); + Flame::Manifest manifest; + if (original.value("manifestType").toString() != "minecraftModpack" || original.value("manifestVersion").toInt() != 1 || + !original.value("files").isArray() || !safeTree(m_instance->instanceRoot()) || !checkPath(m_instance->gameRoot()) || + !checkPath(Modpack::pendingManifestPath(m_instance->instanceRoot(), Modpack::Format::CurseForge))) { + emitFailed(tr("Invalid or unsafe CurseForge manifest.")); + return; + } + for (const auto& value : original.value("files").toArray()) { + const auto entry = value.toObject(); + Flame::File file; + file.projectId = entry.value("projectID").toInt(); + file.fileId = entry.value("fileID").toInt(); + file.required = entry.value("required").toBool(true); + file.resourceType = ModPlatform::ResourceType::Unknown; + if (file.projectId <= 0 || file.fileId <= 0 || manifest.files.contains(file.fileId)) { + emitFailed(tr("Invalid or duplicate CurseForge file ID.")); + return; + } + manifest.files.insert(file.fileId, file); + } + if (m_mode == Modpack::InstallMode::Resume && !m_trustedSource) { + QStringList mods; + QDirIterator it(QDir(m_instance->gameRoot()).filePath("mods"), QDir::Files, QDirIterator::Subdirectories); + while (it.hasNext()) + mods.append(QDir(m_instance->gameRoot()).relativeFilePath(it.next())); + if (!mods.isEmpty()) { + UntrustedModsDialog dialog(mods, m_parent); + m_dialog = &dialog; + const auto accepted = dialog.exec() == QDialog::Accepted; + m_dialog = nullptr; + if (!accepted) { + emitAborted(); + return; + } + } + } + m_canPreserve = true; + auto resolver = makeShared(manifest); + const auto resolved = runTask(resolver); + const auto results = resolver->getResults().files; + if (!resolved) { + if (resolver->filesResolved()) + logWarning(tr("Could not load project metadata: %1").arg(resolver->failReason())); + else + fail(tr("Unable to resolve mod IDs:\n%1").arg(resolver->failReason()), !resolver->downloadFailed()); + } + if (m_error.isEmpty() && resolver->filesResolved()) { + QStringList optionalFiles; + QSet destinations; + for (const auto& file : results) { + const auto name = file.version.fileName; + if (name.isEmpty() || name == "." || name == ".." || name.contains('/') || name.contains('\\') || + name != FS::RemoveInvalidPathChars(name) || !hashAlgorithm(file)) { + fail(tr("Invalid file information returned for CurseForge file %1.").arg(file.fileId), true); + break; + } + const auto destination = FS::PathCombine(file.targetFolder, name); + if (destinations.contains(destination) || destinations.contains(destination + ".disabled")) { + fail(tr("Multiple CurseForge files have the same destination: %1").arg(destination), true); + break; + } + destinations.insert(destination); + destinations.insert(destination + ".disabled"); + if (!checkPath(FS::PathCombine(m_instance->gameRoot(), file.targetFolder, name)) || + !checkPath(FS::PathCombine(m_instance->gameRoot(), file.targetFolder, name + ".disabled"))) + break; + if (!file.required) + optionalFiles.append(FS::PathCombine(file.targetFolder, name)); + } + QStringList selected; + if (m_error.isEmpty() && !m_abort && !optionalFiles.isEmpty()) { + OptionalModDialog dialog(m_parent, optionalFiles); + m_dialog = &dialog; + if (dialog.exec() == QDialog::Rejected) + m_abort = true; + selected = dialog.getResult(); + m_dialog = nullptr; + } + + QMap> blocked; + QMap blockedFiles; + m_filesJob = makeShared(tr("Mod Download Flame"), APPLICATION->network()); + for (const auto& file : results) { + if (m_abort || !m_error.isEmpty()) + break; + auto relative = FS::PathCombine(file.targetFolder, file.version.fileName); + const auto disabled = !file.required && !selected.contains(relative); + const auto path = QDir(m_instance->gameRoot()).filePath(relative) + (disabled ? ".disabled" : ""); + // Reuse hash-verified downloads in either state, including resources + // already moved before a previous progress save failed. + const auto existing = reusableFile(file, disabled); + if (!m_error.isEmpty()) + break; + if (!existing.isEmpty()) { + if (!completeFile(file, existing, disabled)) + break; + continue; + } + if (file.version.downloadUrl.isEmpty()) { + BlockedMod mod; + mod.name = file.version.fileName; + mod.websiteUrl = QString("%1/download/%2").arg(file.pack.websiteUrl, QString::number(file.fileId)); + mod.hash = file.version.hash; + mod.matched = false; + mod.targetFolder = file.targetFolder; + mod.disabled = disabled; + blocked[file.version.hash_type].append(mod); + blockedFiles.insert(FS::PathCombine(mod.targetFolder, mod.name), file); + continue; + } + if (!FS::ensureFilePathExists(path)) { + fail(tr("Could not create resource directory: %1").arg(path), true); + break; + } + auto download = Net::ApiDownload::makeFile(file.version.downloadUrl, path); + download->addValidator(new Net::ChecksumValidator(*hashAlgorithm(file), file.version.hash)); + connect( + download.get(), &Task::succeeded, this, + [this, file, path, disabled] { + if (m_canPreserve && !completeFile(file, path, disabled) && m_filesJob) + m_filesJob->abort(); + }, + Qt::QueuedConnection); + m_filesJob->addNetAction(download); + } + for (auto group = blocked.begin(); group != blocked.end() && !m_abort && m_error.isEmpty(); ++group) { + BlockedModsDialog dialog(m_parent, tr("Blocked mods found"), + tr("The following files must be downloaded manually before installation can finish."), group.value(), + group.key()); + m_dialog = &dialog; + if (dialog.exec() != QDialog::Accepted) + m_abort = true; + m_dialog = nullptr; + for (const auto& mod : group.value()) { + if (m_abort || !m_error.isEmpty()) + break; + const auto file = blockedFiles.value(FS::PathCombine(mod.targetFolder, mod.name)); + const auto path = FS::PathCombine(m_instance->gameRoot(), mod.targetFolder, mod.name) + (mod.disabled ? ".disabled" : ""); + // Matching in the dialog is not sufficient: verify the source + // again and check the copy/move and installed file as well. + QFile source(mod.localPath); + QCryptographicHash hash(*hashAlgorithm(file)); + if (!mod.matched || !source.open(QIODevice::ReadOnly) || !hash.addData(&source) || + hash.result().toHex().compare(file.version.hash.toLatin1(), Qt::CaseInsensitive) != 0) { + fail(tr("A required manually downloaded file is missing: %1").arg(mod.name)); + break; + } + source.close(); + if (!checkPath(path) || !FS::ensureFilePathExists(path) || + !(mod.move ? FS::move(mod.localPath, path) : FS::copy(mod.localPath, path)()) || + !completeFile(file, path, mod.disabled)) { + fail(tr("Could not install manually downloaded file: %1").arg(mod.name)); + break; + } + } + } + if (!m_abort && m_error.isEmpty()) { + const auto succeeded = runTask(m_filesJob); + if (!succeeded && !m_abort) + fail(m_filesJob->failReason()); + } + m_filesJob.reset(); + } + + // Metadata is presentation, not installation state. Its failure must not + // put already validated downloads back into the remaining manifest. + if (!m_abort && m_canPreserve && !m_metadataFiles.isEmpty()) { + auto metadata = makeShared("CreateModMetadata", APPLICATION->settings()->get("NumberOfConcurrentTasks").toInt()); + const auto folder = FS::PathCombine(m_instance->gameRoot(), "mods", ".index"); + if (checkPath(folder) && FS::ensureFolderPathExists(folder)) { + for (auto file : m_metadataFiles) { + if (file.pack.addonId == QString::number(file.projectId)) + metadata->addTask(makeShared(folder, file.pack, file.version)); + } + if (!runTask(metadata)) + logWarning(tr("Could not save mod metadata: %1").arg(metadata->failReason())); + } else if (m_canPreserve) { + logWarning(tr("Could not create the mod metadata directory.")); + } + } + if (m_error.isEmpty() && !m_abort && !m_pending.remaining().value("files").toArray().isEmpty()) + fail(tr("Some modpack files have not been installed.")); + if (!m_error.isEmpty()) + emitFailed(m_error); + else if (m_abort) + emitAborted(); + else + emitSucceeded(); +} diff --git a/launcher/modplatform/flame/FlamePackInstallTask.h b/launcher/modplatform/flame/FlamePackInstallTask.h new file mode 100644 index 000000000..9737a22d5 --- /dev/null +++ b/launcher/modplatform/flame/FlamePackInstallTask.h @@ -0,0 +1,57 @@ +// SPDX-License-Identifier: GPL-3.0-only + +#pragma once + +#include + +#include "modplatform/flame/PackManifest.h" +#include "modplatform/helpers/PendingModpackManifest.h" +#include "net/NetJob.h" +#include "tasks/Task.h" + +class MinecraftInstance; +class QDialog; +class QWidget; + +class FlamePackInstallTask final : public Task { + Q_OBJECT + + public: + FlamePackInstallTask(MinecraftInstance* instance, + QJsonObject manifest, + Modpack::InstallMode mode, + bool trustedSource, + QWidget* parent = nullptr); + + bool canAbort() const override { return true; } + bool abort() override; + QJsonObject remainingManifest() const { return m_pending.remaining(); } + bool canPreserve() const; + + protected: + void executeTask() override; + + private: + bool runTask(const Task::Ptr& task); + bool checkPath(const QString& path); + bool validFile(const Flame::File& file, const QString& path) const; + QString reusableFile(const Flame::File& file, bool disabled); + bool completeFile(const Flame::File& file, QString path, bool disabled); + bool saveProgress(); + bool installWorld(const QString& path, bool disabled, QString* installedPath); + void rollbackWorld(const QString& path); + void fail(const QString& reason, bool unsafe = false); + + MinecraftInstance* m_instance; + Modpack::PendingManifest m_pending; + Modpack::InstallMode m_mode; + bool m_trustedSource; + QWidget* m_parent; + bool m_abort = false; + bool m_canPreserve = false; + QString m_error; + QPointer m_dialog; + NetJob::Ptr m_filesJob; + QSet m_completed; + QList m_metadataFiles; +}; From 37c21c19ac9733d40b9afb14a31609779a4ad2dc Mon Sep 17 00:00:00 2001 From: HowXu Date: Sat, 24 Oct 2026 11:52:59 +0800 Subject: [PATCH 06/13] feat(launch): Complete pending packs before launch Resume content before normal launch steps, reject explicit offline launches with pending content, and remove markers only after successful completion. Assisted-by: OpenCode:gpt-6.1-sol-fast Signed-off-by: HowXu --- launcher/CMakeLists.txt | 3 + launcher/LaunchController.cpp | 9 ++ launcher/minecraft/MinecraftInstance.cpp | 5 + launcher/minecraft/auth/AuthSession.h | 2 + launcher/modplatform/ModpackResumeTask.cpp | 123 +++++++++++++++++++++ launcher/modplatform/ModpackResumeTask.h | 34 ++++++ 6 files changed, 176 insertions(+) create mode 100644 launcher/modplatform/ModpackResumeTask.cpp create mode 100644 launcher/modplatform/ModpackResumeTask.h diff --git a/launcher/CMakeLists.txt b/launcher/CMakeLists.txt index bc2527abf..68bb88abb 100644 --- a/launcher/CMakeLists.txt +++ b/launcher/CMakeLists.txt @@ -532,6 +532,9 @@ set(API_SOURCES modplatform/EnsureMetadataTask.h modplatform/EnsureMetadataTask.cpp + modplatform/ModpackResumeTask.h + modplatform/ModpackResumeTask.cpp + modplatform/CheckUpdateTask.h modplatform/flame/FlameAPI.h diff --git a/launcher/LaunchController.cpp b/launcher/LaunchController.cpp index 1bda4a111..f6c2f3cf8 100644 --- a/launcher/LaunchController.cpp +++ b/launcher/LaunchController.cpp @@ -39,6 +39,7 @@ #include "launch/steps/PrintServers.h" #include "minecraft/auth/AccountData.h" #include "minecraft/auth/AccountList.h" +#include "modplatform/ModpackResumeTask.h" #include "net/NetUtils.h" #include "ui/InstanceWindow.h" @@ -70,6 +71,13 @@ void LaunchController::executeTask() return; } + if (m_wantedLaunchMode == LaunchMode::Offline && ModpackResumeTask::hasPendingManifest(m_instance->instanceRoot())) { + const auto reason = tr("This modpack installation is incomplete. Launch online to finish installing it before playing."); + QMessageBox::warning(m_parentWidget, tr("Incomplete modpack installation"), reason); + emitFailed(reason); + return; + } + if (!JavaCommon::checkJVMArgs(m_instance->settings()->get("JvmArgs").toString(), m_parentWidget)) { emitFailed(tr("Invalid Java arguments specified. Please fix this first.")); return; @@ -370,6 +378,7 @@ void LaunchController::launchInstance() return; } + m_session->explicitlyOffline = m_wantedLaunchMode == LaunchMode::Offline; m_launcher = m_instance->createLaunchTask(m_session, m_targetToJoin); if (!m_launcher) { emitFailed(tr("Couldn't instantiate a launcher.")); diff --git a/launcher/minecraft/MinecraftInstance.cpp b/launcher/minecraft/MinecraftInstance.cpp index adb94ac47..57a277a1c 100644 --- a/launcher/minecraft/MinecraftInstance.cpp +++ b/launcher/minecraft/MinecraftInstance.cpp @@ -76,6 +76,7 @@ #include "minecraft/update/FoldersTask.h" #include "minecraft/update/LegacyFMLLibrariesTask.h" #include "minecraft/update/LibrariesTask.h" +#include "modplatform/ModpackResumeTask.h" #include "java/JavaUtils.h" @@ -1159,6 +1160,10 @@ LaunchTask* MinecraftInstance::createLaunchTask(AuthSessionPtr session, Minecraf auto process = LaunchTask::create(this); auto pptr = process.get(); + if (ModpackResumeTask::hasPendingManifest(instanceRoot())) { + process->appendStep(makeShared(pptr, makeShared(this, session->explicitlyOffline))); + } + APPLICATION->icons()->saveIcon(iconKey(), FS::PathCombine(gameRoot(), "icon.png"), "PNG"); // print a header diff --git a/launcher/minecraft/auth/AuthSession.h b/launcher/minecraft/auth/AuthSession.h index ab29798dc..60798c6b4 100644 --- a/launcher/minecraft/auth/AuthSession.h +++ b/launcher/minecraft/auth/AuthSession.h @@ -33,6 +33,8 @@ struct AuthSession { QString authlib_injector_auth_url; // the actual launch mode for this session LaunchMode launchMode; + // whether the user explicitly requested an offline launch + bool explicitlyOffline = false; }; using AuthSessionPtr = std::shared_ptr; diff --git a/launcher/modplatform/ModpackResumeTask.cpp b/launcher/modplatform/ModpackResumeTask.cpp new file mode 100644 index 000000000..60202af0f --- /dev/null +++ b/launcher/modplatform/ModpackResumeTask.cpp @@ -0,0 +1,123 @@ +// SPDX-License-Identifier: GPL-3.0-only + +#include "ModpackResumeTask.h" + +#include +#include + +#include "minecraft/MinecraftInstance.h" +#include "modplatform/flame/FlamePackInstallTask.h" +#include "modplatform/helpers/PendingModpackManifest.h" +#include "modplatform/modrinth/ModrinthPackInstallTask.h" + +namespace { +bool markerExists(const QString& path) +{ + const QFileInfo info(path); + return info.exists() || info.isSymLink(); +} +} // namespace + +ModpackResumeTask::ModpackResumeTask(MinecraftInstance* instance, bool explicitlyOffline, QWidget* parent) + : m_instance(instance), m_explicitlyOffline(explicitlyOffline), m_parent(parent) +{ + setAbortable(true); +} + +bool ModpackResumeTask::hasPendingManifest(const QString& instanceRoot) +{ + return markerExists(Modpack::pendingManifestPath(instanceRoot, Modpack::Format::Modrinth)) || + markerExists(Modpack::pendingManifestPath(instanceRoot, Modpack::Format::CurseForge)); +} + +void ModpackResumeTask::executeTask() +{ + m_aborted = false; + m_task.reset(); + const auto modrinthPath = Modpack::pendingManifestPath(m_instance->instanceRoot(), Modpack::Format::Modrinth); + const auto curseForgePath = Modpack::pendingManifestPath(m_instance->instanceRoot(), Modpack::Format::CurseForge); + const bool hasModrinth = markerExists(modrinthPath); + const bool hasCurseForge = markerExists(curseForgePath); + if (!hasModrinth && !hasCurseForge) { + emitSucceeded(); + return; + } + if (m_explicitlyOffline) { + emitFailed(tr("This modpack installation is incomplete. Launch online to finish installing it before playing.")); + return; + } + if (hasModrinth && hasCurseForge) { + emitFailed( + tr("Cannot resume this modpack because both Modrinth and CurseForge remaining manifests exist. " + "Resolve the conflicting manifests before launching.")); + return; + } + + const auto format = hasModrinth ? Modpack::Format::Modrinth : Modpack::Format::CurseForge; + m_pendingPath = hasModrinth ? modrinthPath : curseForgePath; + QString error; + const auto manifest = Modpack::PendingManifest::load(format, m_pendingPath, &error); + if (!manifest) { + emitFailed(tr("Could not read the remaining modpack manifest at %1:\n%2").arg(m_pendingPath, error)); + return; + } + + if (hasModrinth) { + m_task = + makeShared(m_instance, manifest->remaining(), Modpack::InstallMode::Resume, false, m_parent.data()); + } else { + m_task = makeShared(m_instance, manifest->remaining(), Modpack::InstallMode::Resume, false, m_parent.data()); + } + propagateFromOther(m_task.get()); + connect(m_task.get(), &Task::abortStatusChanged, this, &Task::setAbortable); + connect(m_task.get(), &Task::abortButtonTextChanged, this, &Task::setAbortButtonText); + connect(m_task.get(), &Task::warningLogged, this, &ModpackResumeTask::logWarning); + connect(m_task.get(), &Task::finished, this, &ModpackResumeTask::installFinished); + setAbortable(m_task->canAbort()); + setStatus(tr("Resuming modpack installation")); + m_task->start(); +} + +void ModpackResumeTask::installFinished() +{ + if (!isRunning()) + return; + + if (m_aborted || m_task->getState() == Task::State::AbortedByUser) { + emitAborted(); + return; + } + if (!m_task->wasSuccessful()) { + emitFailed(tr("Could not finish installing the modpack:\n%1").arg(m_task->failReason())); + return; + } + + QFile marker(m_pendingPath); + if (!marker.remove()) { + emitFailed(tr("The modpack files were installed, but the remaining manifest at %1 could not be removed:\n%2") + .arg(m_pendingPath, marker.errorString())); + return; + } + emitSucceeded(); +} + +bool ModpackResumeTask::canAbort() const +{ + return m_task ? m_task->canAbort() : Task::canAbort(); +} + +bool ModpackResumeTask::abort() +{ + if (!isRunning() || !canAbort()) + return false; + + if (!m_task) + return Task::abort(); + + m_aborted = true; + if (!m_task->abort()) { + m_aborted = false; + return false; + } + return true; +} diff --git a/launcher/modplatform/ModpackResumeTask.h b/launcher/modplatform/ModpackResumeTask.h new file mode 100644 index 000000000..555d25fae --- /dev/null +++ b/launcher/modplatform/ModpackResumeTask.h @@ -0,0 +1,34 @@ +// SPDX-License-Identifier: GPL-3.0-only + +#pragma once + +#include +#include + +#include "tasks/Task.h" + +class MinecraftInstance; + +class ModpackResumeTask : public Task { + Q_OBJECT + + public: + ModpackResumeTask(MinecraftInstance* instance, bool explicitlyOffline, QWidget* parent = nullptr); + + static bool hasPendingManifest(const QString& instanceRoot); + bool canAbort() const override; + bool abort() override; + + protected: + void executeTask() override; + + private: + void installFinished(); + + MinecraftInstance* m_instance; + bool m_explicitlyOffline; + QPointer m_parent; + Task::Ptr m_task; + QString m_pendingPath; + bool m_aborted = false; +}; From e124a0a8c757912eebdd9d6b29a76fdc69e32791 Mon Sep 17 00:00:00 2001 From: HowXu Date: Sat, 24 Oct 2026 12:02:15 +0800 Subject: [PATCH 07/13] fix(modrinth): Verify files before completing entries Keep successful requests without a valid landed file pending for fallback URLs, and verify SHA-512 before optional cleanup or manifest filtering. Assisted-by: OpenCode:gpt-6.1-sol-fast Signed-off-by: HowXu --- .../modrinth/ModrinthPackInstallTask.cpp | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/launcher/modplatform/modrinth/ModrinthPackInstallTask.cpp b/launcher/modplatform/modrinth/ModrinthPackInstallTask.cpp index 703a734a6..8132d2d5b 100644 --- a/launcher/modplatform/modrinth/ModrinthPackInstallTask.cpp +++ b/launcher/modplatform/modrinth/ModrinthPackInstallTask.cpp @@ -267,6 +267,12 @@ bool ModrinthPackInstallTask::saveProgress() bool ModrinthPackInstallTask::completeFile(File& file) { + // A successful request may not have committed a file (for example, HTTP 204). + // Leave it pending so another download URL can be tried. + if (!matchesHash(FS::PathCombine(m_instance->gameRoot(), file.path()), file.hash)) { + return false; + } + // Optional selection changes must not leave enabled and disabled duplicates. if (file.optional) { const auto other = FS::PathCombine(m_instance->gameRoot(), file.alternatePath()); @@ -308,6 +314,9 @@ void ModrinthPackInstallTask::executeTask() } } if (valid && !completeFile(file)) { + if (m_error.isEmpty()) { + m_error = tr("The file changed while installing: %1").arg(path); + } finish(); return; } @@ -348,7 +357,10 @@ void ModrinthPackInstallTask::downloadNextRound() auto request = Net::ApiDownload::makeFile(file.downloads.dequeue(), path, Net::Download::Option::NoOptions, meta); request->addValidator(new Net::ChecksumValidator(QCryptographicHash::Sha512, file.hash)); connect(request.get(), &Task::succeeded, this, [this, &file] { - if (!file.completed && m_error.isEmpty() && !completeFile(file)) { + if (file.completed || !m_error.isEmpty()) { + return; + } + if (!completeFile(file) && !m_error.isEmpty()) { m_downloadJob->abort(); } }); From 09795e3a15cf53a54a3e08f08b043156f3f00805 Mon Sep 17 00:00:00 2001 From: HowXu Date: Sat, 24 Oct 2026 12:55:42 +0800 Subject: [PATCH 08/13] fix: update the text display Signed-off-by: HowXu --- launcher/InstanceCreationTask.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/launcher/InstanceCreationTask.cpp b/launcher/InstanceCreationTask.cpp index 58c6f711f..07c9efb30 100644 --- a/launcher/InstanceCreationTask.cpp +++ b/launcher/InstanceCreationTask.cpp @@ -82,8 +82,8 @@ void InstanceCreationTask::executeTask() if (m_contentIncomplete && !shouldOverride()) { finishIncompleteInstance( - tr("Modpack installation is incomplete. Remaining files must be downloaded before playing; game files " - "have not been downloaded yet.")); + tr("Modpack installation is incomplete. Remaining files must be downloaded before playing.") + ); return; } if (m_abort) { From 69913dc7c7add1e5c82d8c017f0a920cabd47b9e Mon Sep 17 00:00:00 2001 From: HowXu Date: Sat, 24 Oct 2026 13:02:48 +0800 Subject: [PATCH 09/13] fix(launch): Show progress before resuming modpack downloads Signed-off-by: HowXu --- launcher/launch/TaskStepWrapper.cpp | 3 +- tests/CMakeLists.txt | 3 + tests/TaskStepWrapper_test.cpp | 89 +++++++++++++++++++++++++++++ 3 files changed, 94 insertions(+), 1 deletion(-) create mode 100644 tests/TaskStepWrapper_test.cpp diff --git a/launcher/launch/TaskStepWrapper.cpp b/launcher/launch/TaskStepWrapper.cpp index acf790a8f..e05654da9 100644 --- a/launcher/launch/TaskStepWrapper.cpp +++ b/launcher/launch/TaskStepWrapper.cpp @@ -29,7 +29,8 @@ void TaskStepWrapper::executeTask() void TaskStepWrapper::proceed() { - m_task->start(); + // Let the progress dialog connect and become visible before tasks enter nested event loops. + QMetaObject::invokeMethod(m_task.get(), &Task::start, Qt::QueuedConnection); } void TaskStepWrapper::updateFinished() diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 80d27e7d7..32439569f 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -39,6 +39,9 @@ ecm_add_test(ParseUtils_test.cpp LINK_LIBRARIES Launcher_logic Qt${QT_VERSION_MA ecm_add_test(Task_test.cpp LINK_LIBRARIES Launcher_logic Qt${QT_VERSION_MAJOR}::Test TEST_NAME Task) +ecm_add_test(TaskStepWrapper_test.cpp LINK_LIBRARIES Launcher_logic Qt${QT_VERSION_MAJOR}::Test + TEST_NAME TaskStepWrapper) + ecm_add_test(INIFile_test.cpp LINK_LIBRARIES Launcher_logic Qt${QT_VERSION_MAJOR}::Test TEST_NAME INIFile) diff --git a/tests/TaskStepWrapper_test.cpp b/tests/TaskStepWrapper_test.cpp new file mode 100644 index 000000000..373ceeb7f --- /dev/null +++ b/tests/TaskStepWrapper_test.cpp @@ -0,0 +1,89 @@ +// SPDX-License-Identifier: GPL-3.0-only + +#include +#include +#include +#include +#include + +#include "launch/LaunchTask.h" +#include "launch/TaskStepWrapper.h" +#include "ui/dialogs/ProgressDialog.h" + +// Exercise launch steps without an instance, authentication or a game process. +class TestLaunchTask : public LaunchTask { + public: + TestLaunchTask() : LaunchTask(nullptr) {} + + void executeTask() override + { + state = Running; + onStepFinished(); + } + + protected: + void emitSucceeded() override { Task::emitSucceeded(); } + void emitFailed(QString reason) override { Task::emitFailed(reason); } +}; + +// CurseForge installation waits for network tasks in a nested event loop. +class BlockingInstallTask : public Task { + Q_OBJECT + + signals: + void downloading(); + + private: + void executeTask() override + { + QEventLoop loop; + QTimer::singleShot(0, &loop, [&] { + setStatus("Downloading mods..."); + setProgress(25, 100); + emit downloading(); + loop.quit(); + }); + loop.exec(); + emitSucceeded(); + } +}; + +class TaskStepWrapperTest : public QObject { + Q_OBJECT + + private slots: + void progressDialogIsVisibleDuringBlockingInstall() + { + TestLaunchTask launch; + auto install = makeShared(); + auto step = makeShared(&launch, install); + launch.appendStep(step); + + ProgressDialog dialog; + bool visibleDuringDownload = false; + QString displayedStatus; + int displayedProgress = -1; + connect(install.get(), &BlockingInstallTask::downloading, this, [&] { + visibleDuringDownload = dialog.isVisible(); + displayedStatus = dialog.findChild("globalStatusLabel")->text(); + displayedProgress = dialog.findChild("globalProgressBar")->value(); + }); + connect(&launch, &LaunchTask::requestProgress, this, [&](Task* task) { + // Match LaunchController's progress-dialog handoff. + launch.proceed(); + QCOMPARE(dialog.execWithTask(task), QDialog::Accepted); + }); + + launch.start(); + + QVERIFY(launch.wasSuccessful()); + QVERIFY(visibleDuringDownload); + QCOMPARE(displayedStatus, QString("Downloading mods...")); + QCOMPARE(displayedProgress, 25); + QVERIFY(!dialog.isVisible()); + } +}; + +QTEST_MAIN(TaskStepWrapperTest) + +#include "TaskStepWrapper_test.moc" From 82eb68ca19dbbf2c3a611d81c23084cb374902f2 Mon Sep 17 00:00:00 2001 From: HowXu Date: Sat, 24 Oct 2026 13:26:45 +0800 Subject: [PATCH 10/13] fix(launch): Confirm modpack trust before showing resume progress Signed-off-by: HowXu --- launcher/minecraft/MinecraftInstance.cpp | 2 +- launcher/modplatform/ModpackResumeTask.cpp | 23 +++- launcher/modplatform/ModpackResumeTask.h | 9 +- .../flame/FlamePackInstallTask.cpp | 16 +++ .../modplatform/flame/FlamePackInstallTask.h | 4 + .../modrinth/ModrinthPackInstallTask.cpp | 15 +++ .../modrinth/ModrinthPackInstallTask.h | 4 + tests/TaskStepWrapper_test.cpp | 100 +++++++++++++++++- 8 files changed, 162 insertions(+), 11 deletions(-) diff --git a/launcher/minecraft/MinecraftInstance.cpp b/launcher/minecraft/MinecraftInstance.cpp index 57a277a1c..e32a52a1d 100644 --- a/launcher/minecraft/MinecraftInstance.cpp +++ b/launcher/minecraft/MinecraftInstance.cpp @@ -1161,7 +1161,7 @@ LaunchTask* MinecraftInstance::createLaunchTask(AuthSessionPtr session, Minecraf auto pptr = process.get(); if (ModpackResumeTask::hasPendingManifest(instanceRoot())) { - process->appendStep(makeShared(pptr, makeShared(this, session->explicitlyOffline))); + process->appendStep(makeShared(pptr, this, session->explicitlyOffline)); } APPLICATION->icons()->saveIcon(iconKey(), FS::PathCombine(gameRoot(), "icon.png"), "PNG"); diff --git a/launcher/modplatform/ModpackResumeTask.cpp b/launcher/modplatform/ModpackResumeTask.cpp index 60202af0f..09f640357 100644 --- a/launcher/modplatform/ModpackResumeTask.cpp +++ b/launcher/modplatform/ModpackResumeTask.cpp @@ -18,8 +18,8 @@ bool markerExists(const QString& path) } } // namespace -ModpackResumeTask::ModpackResumeTask(MinecraftInstance* instance, bool explicitlyOffline, QWidget* parent) - : m_instance(instance), m_explicitlyOffline(explicitlyOffline), m_parent(parent) +ModpackResumeTask::ModpackResumeTask(LaunchTask* parent, MinecraftInstance* instance, bool explicitlyOffline, QWidget* parentWidget) + : LaunchStep(parent), m_instance(instance), m_explicitlyOffline(explicitlyOffline), m_parentWidget(parentWidget) { setAbortable(true); } @@ -63,10 +63,15 @@ void ModpackResumeTask::executeTask() } if (hasModrinth) { - m_task = - makeShared(m_instance, manifest->remaining(), Modpack::InstallMode::Resume, false, m_parent.data()); + auto task = makeShared(m_instance, manifest->remaining(), Modpack::InstallMode::Resume, false, + m_parentWidget.data()); + connect(task.get(), &ModrinthPackInstallTask::readyForDownload, this, &ModpackResumeTask::reportProgress, Qt::QueuedConnection); + m_task = task; } else { - m_task = makeShared(m_instance, manifest->remaining(), Modpack::InstallMode::Resume, false, m_parent.data()); + auto task = + makeShared(m_instance, manifest->remaining(), Modpack::InstallMode::Resume, false, m_parentWidget.data()); + connect(task.get(), &FlamePackInstallTask::readyForDownload, this, &ModpackResumeTask::reportProgress, Qt::QueuedConnection); + m_task = task; } propagateFromOther(m_task.get()); connect(m_task.get(), &Task::abortStatusChanged, this, &Task::setAbortable); @@ -78,6 +83,14 @@ void ModpackResumeTask::executeTask() m_task->start(); } +void ModpackResumeTask::reportProgress() +{ + // Installers queue their content work after this request, so blocking + // download loops run inside the progress dialog rather than enclosing it. + if (isRunning() && !m_aborted) + emit progressReportingRequest(); +} + void ModpackResumeTask::installFinished() { if (!isRunning()) diff --git a/launcher/modplatform/ModpackResumeTask.h b/launcher/modplatform/ModpackResumeTask.h index 555d25fae..e4ad68dac 100644 --- a/launcher/modplatform/ModpackResumeTask.h +++ b/launcher/modplatform/ModpackResumeTask.h @@ -5,15 +5,15 @@ #include #include -#include "tasks/Task.h" +#include "launch/LaunchStep.h" class MinecraftInstance; -class ModpackResumeTask : public Task { +class ModpackResumeTask : public LaunchStep { Q_OBJECT public: - ModpackResumeTask(MinecraftInstance* instance, bool explicitlyOffline, QWidget* parent = nullptr); + ModpackResumeTask(LaunchTask* parent, MinecraftInstance* instance, bool explicitlyOffline, QWidget* parentWidget = nullptr); static bool hasPendingManifest(const QString& instanceRoot); bool canAbort() const override; @@ -23,11 +23,12 @@ class ModpackResumeTask : public Task { void executeTask() override; private: + void reportProgress(); void installFinished(); MinecraftInstance* m_instance; bool m_explicitlyOffline; - QPointer m_parent; + QPointer m_parentWidget; Task::Ptr m_task; QString m_pendingPath; bool m_aborted = false; diff --git a/launcher/modplatform/flame/FlamePackInstallTask.cpp b/launcher/modplatform/flame/FlamePackInstallTask.cpp index 19f97e711..fcdf72014 100644 --- a/launcher/modplatform/flame/FlamePackInstallTask.cpp +++ b/launcher/modplatform/flame/FlamePackInstallTask.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include "Application.h" #include "FileSystem.h" @@ -437,6 +438,21 @@ void FlamePackInstallTask::executeTask() } } m_canPreserve = true; + emit readyForDownload(); + if (m_mode == Modpack::InstallMode::Resume) { + // Start inside the progress dialog's event loop, not the other way around. + QTimer::singleShot(0, this, [this, manifest] { installFiles(manifest); }); + } else { + installFiles(manifest); + } +} + +void FlamePackInstallTask::installFiles(const Flame::Manifest& manifest) +{ + if (m_abort) { + emitAborted(); + return; + } auto resolver = makeShared(manifest); const auto resolved = runTask(resolver); const auto results = resolver->getResults().files; diff --git a/launcher/modplatform/flame/FlamePackInstallTask.h b/launcher/modplatform/flame/FlamePackInstallTask.h index 9737a22d5..e16382bb7 100644 --- a/launcher/modplatform/flame/FlamePackInstallTask.h +++ b/launcher/modplatform/flame/FlamePackInstallTask.h @@ -28,10 +28,14 @@ class FlamePackInstallTask final : public Task { QJsonObject remainingManifest() const { return m_pending.remaining(); } bool canPreserve() const; + signals: + void readyForDownload(); + protected: void executeTask() override; private: + void installFiles(const Flame::Manifest& manifest); bool runTask(const Task::Ptr& task); bool checkPath(const QString& path); bool validFile(const Flame::File& file, const QString& path) const; diff --git a/launcher/modplatform/modrinth/ModrinthPackInstallTask.cpp b/launcher/modplatform/modrinth/ModrinthPackInstallTask.cpp index 8132d2d5b..42feb88a4 100644 --- a/launcher/modplatform/modrinth/ModrinthPackInstallTask.cpp +++ b/launcher/modplatform/modrinth/ModrinthPackInstallTask.cpp @@ -292,6 +292,21 @@ void ModrinthPackInstallTask::executeTask() finish(); return; } + emit readyForDownload(); + if (m_mode == Modpack::InstallMode::Resume) { + // The queued progress request must run before the blocking content work. + QTimer::singleShot(0, this, &ModrinthPackInstallTask::installFiles); + } else { + installFiles(); + } +} + +void ModrinthPackInstallTask::installFiles() +{ + if (m_aborted) { + finish(); + return; + } for (auto& file : m_files) { if (file.unsupported) { file.completed = true; diff --git a/launcher/modplatform/modrinth/ModrinthPackInstallTask.h b/launcher/modplatform/modrinth/ModrinthPackInstallTask.h index f55dc9b30..04408ca83 100644 --- a/launcher/modplatform/modrinth/ModrinthPackInstallTask.h +++ b/launcher/modplatform/modrinth/ModrinthPackInstallTask.h @@ -30,6 +30,9 @@ class ModrinthPackInstallTask final : public Task { QJsonObject remainingManifest() const; bool canPreserve() const; + signals: + void readyForDownload(); + protected: void executeTask() override; @@ -51,6 +54,7 @@ class ModrinthPackInstallTask final : public Task { }; bool prepare(); + void installFiles(); bool validatePath(const QString& path) const; bool completeFile(File& file); bool saveProgress(); diff --git a/tests/TaskStepWrapper_test.cpp b/tests/TaskStepWrapper_test.cpp index 373ceeb7f..9b91746e4 100644 --- a/tests/TaskStepWrapper_test.cpp +++ b/tests/TaskStepWrapper_test.cpp @@ -1,19 +1,28 @@ // SPDX-License-Identifier: GPL-3.0-only +#include #include +#include +#include +#include #include #include +#include #include #include #include "launch/LaunchTask.h" #include "launch/TaskStepWrapper.h" +#include "modplatform/ModpackResumeTask.h" +#include "modplatform/helpers/PendingModpackManifest.h" +#include "settings/INISettingsObject.h" #include "ui/dialogs/ProgressDialog.h" +#include "ui/dialogs/UntrustedModsDialog.h" // Exercise launch steps without an instance, authentication or a game process. class TestLaunchTask : public LaunchTask { public: - TestLaunchTask() : LaunchTask(nullptr) {} + TestLaunchTask(MinecraftInstance* instance = nullptr) : LaunchTask(instance) {} void executeTask() override { @@ -52,6 +61,95 @@ class TaskStepWrapperTest : public QObject { Q_OBJECT private slots: + void resumeProgressFollowsTrustConfirmation_data() + { + QTest::addColumn("modrinth"); + QTest::addColumn("acceptTrust"); + QTest::newRow("Modrinth-reject") << true << false; + QTest::newRow("CurseForge-reject") << false << false; + QTest::newRow("Modrinth-accept") << true << true; + QTest::newRow("CurseForge-accept") << false << true; + } + + void resumeProgressFollowsTrustConfirmation() + { + QFETCH(bool, modrinth); + QFETCH(bool, acceptTrust); + QTemporaryDir temp; + QVERIFY(temp.isValid()); + QVERIFY(QDir(temp.path()).mkpath("minecraft/mods")); + QFile mod(temp.filePath("minecraft/mods/untrusted.jar")); + QVERIFY(mod.open(QIODevice::WriteOnly)); + mod.close(); + + INISettingsObject globalSettings(temp.filePath("global.cfg")); + for (const auto& key : + { "ShowGameTime", "RecordGameTime", "PreLaunchCommand", "WrapperCommand", "PostExitCommand", "ShowConsole", "AutoCloseConsole", + "ShowConsoleOnError", "LogPrePostOutput", "ConsoleOverflowStop", "EnableDiscordRichPresence" }) { + globalSettings.registerSetting(key, false); + } + globalSettings.registerSetting("ConsoleMaxLines", 1000); + MinecraftInstance instance(&globalSettings, std::make_unique(temp.filePath("instance.cfg")), temp.path()); + + const auto format = modrinth ? Modpack::Format::Modrinth : Modpack::Format::CurseForge; + const QJsonObject manifest = + modrinth ? QJsonObject{ { "formatVersion", 1 }, + { "game", "minecraft" }, + { "dependencies", QJsonObject{ { "minecraft", "1.20.1" } } }, + { "files", QJsonArray{} } } + : QJsonObject{ { "manifestType", "minecraftModpack" }, { "manifestVersion", 1 }, { "files", QJsonArray{} } }; + const auto pendingPath = Modpack::pendingManifestPath(temp.path(), format); + QVERIFY(QDir().mkpath(QFileInfo(pendingPath).absolutePath())); + Modpack::PendingManifest pending(format, manifest); + QString error; + QVERIFY2(pending.save(pendingPath, &error), qPrintable(error)); + + TestLaunchTask launch(&instance); + auto resume = makeShared(&launch, &instance, false); + launch.appendStep(resume); + int progressRequests = 0; + connect(&launch, &LaunchTask::requestProgress, this, [&](Task* task) { + ++progressRequests; + ProgressDialog dialog; + launch.proceed(); + // Cancel at the handoff: queued content work must settle without + // starting real network requests or trapping the modal dialog. + QVERIFY(task->abort()); + dialog.execWithTask(task); + }); + + bool confirmedDialogShown = false; + bool progressVisibleDuringConfirmation = false; + QTimer observeDialog; + connect(&observeDialog, &QTimer::timeout, this, [&] { + for (auto* widget : QApplication::topLevelWidgets()) { + auto* confirmation = qobject_cast(widget); + if (!confirmation || !confirmation->isVisible()) + continue; + confirmedDialogShown = true; + for (auto* other : QApplication::topLevelWidgets()) { + if (qobject_cast(other) && other->isVisible()) + progressVisibleDuringConfirmation = true; + } + if (acceptTrust) + confirmation->accept(); + else + confirmation->reject(); + } + }); + observeDialog.start(); + launch.start(); + QTRY_VERIFY_WITH_TIMEOUT(launch.isFinished(), 5000); + + QVERIFY(confirmedDialogShown); + QVERIFY(!progressVisibleDuringConfirmation); + QCOMPARE(progressRequests, acceptTrust ? 1 : 0); + QCOMPARE(resume->getState(), Task::State::AbortedByUser); + const auto remaining = Modpack::PendingManifest::load(format, pendingPath, &error); + QVERIFY2(remaining.has_value(), qPrintable(error)); + QCOMPARE(remaining->remaining(), manifest); + } + void progressDialogIsVisibleDuringBlockingInstall() { TestLaunchTask launch; From 14144cc2456577dc3f6b80de0426ae4a2b25b031 Mon Sep 17 00:00:00 2001 From: HowXu Date: Sat, 24 Oct 2026 13:34:35 +0800 Subject: [PATCH 11/13] fix(curseforge): now directly cancel download when abort Signed-off-by: HowXu --- launcher/CMakeLists.txt | 1 + .../modplatform/flame/ContentDownloadJob.h | 46 ++++++++++++ .../flame/FlamePackInstallTask.cpp | 31 +------- tests/Task_test.cpp | 70 +++++++++++++++++++ 4 files changed, 119 insertions(+), 29 deletions(-) create mode 100644 launcher/modplatform/flame/ContentDownloadJob.h diff --git a/launcher/CMakeLists.txt b/launcher/CMakeLists.txt index 68bb88abb..a21184cf2 100644 --- a/launcher/CMakeLists.txt +++ b/launcher/CMakeLists.txt @@ -587,6 +587,7 @@ set(FLAME_SOURCES modplatform/flame/FlameInstanceCreationTask.cpp modplatform/flame/FlamePackInstallTask.h modplatform/flame/FlamePackInstallTask.cpp + modplatform/flame/ContentDownloadJob.h modplatform/flame/FlamePackExportTask.h modplatform/flame/FlamePackExportTask.cpp ) diff --git a/launcher/modplatform/flame/ContentDownloadJob.h b/launcher/modplatform/flame/ContentDownloadJob.h new file mode 100644 index 000000000..2e152d4c7 --- /dev/null +++ b/launcher/modplatform/flame/ContentDownloadJob.h @@ -0,0 +1,46 @@ +// SPDX-License-Identifier: GPL-3.0-only + +#pragma once + +#include "net/NetJob.h" + +namespace Flame { + +// Interrupt downloads, but wait for their sinks to close before reporting a +// terminal state, unlike NetJob::abort() which can finish before requests unwind. +class ContentDownloadJob final : public NetJob { + public: + using NetJob::NetJob; + + bool abort() override + { + m_stopping = true; + m_queue.clear(); + setAskRetry(false); + QMetaObject::invokeMethod( + this, + [this] { + const auto active = m_doing.values(); + for (const auto& task : active) + task->abort(); + executeNextSubTask(); + }, + Qt::QueuedConnection); + return true; + } + + protected: + void executeNextSubTask() override + { + if (!m_stopping) { + NetJob::executeNextSubTask(); + } else if (isRunning() && m_doing.isEmpty()) { + emitAborted(); + } + } + + private: + bool m_stopping = false; +}; + +} // namespace Flame diff --git a/launcher/modplatform/flame/FlamePackInstallTask.cpp b/launcher/modplatform/flame/FlamePackInstallTask.cpp index fcdf72014..f50609a03 100644 --- a/launcher/modplatform/flame/FlamePackInstallTask.cpp +++ b/launcher/modplatform/flame/FlamePackInstallTask.cpp @@ -25,6 +25,7 @@ #include "minecraft/World.h" #include "minecraft/mod/tasks/LocalResourceParse.h" #include "minecraft/mod/tasks/LocalResourceUpdateTask.h" +#include "modplatform/flame/ContentDownloadJob.h" #include "modplatform/flame/FileResolvingTask.h" #include "net/ApiDownload.h" #include "net/ChecksumValidator.h" @@ -79,34 +80,6 @@ std::optional hashAlgorithm(const Flame::File& fi return {}; } -// NetJob::abort() can finish before all requests have unwound. Cancel queued -// work, but drain active requests normally before reporting a terminal state. -class ContentDownloadJob final : public NetJob { - public: - using NetJob::NetJob; - bool abort() override - { - m_stopping = true; - m_queue.clear(); - setAskRetry(false); - QMetaObject::invokeMethod(this, &ContentDownloadJob::executeNextSubTask, Qt::QueuedConnection); - return true; - } - - protected: - void executeNextSubTask() override - { - if (!m_stopping) { - NetJob::executeNextSubTask(); - } else if (isRunning() && m_doing.isEmpty()) { - emitAborted(); - } - } - - private: - bool m_stopping = false; -}; - bool containsWorld(const QString& path) { // Inspect the archive directly: World only recognizes an on-disk .zip @@ -497,7 +470,7 @@ void FlamePackInstallTask::installFiles(const Flame::Manifest& manifest) QMap> blocked; QMap blockedFiles; - m_filesJob = makeShared(tr("Mod Download Flame"), APPLICATION->network()); + m_filesJob = makeShared(tr("Mod Download Flame"), APPLICATION->network()); for (const auto& file : results) { if (m_abort || !m_error.isEmpty()) break; diff --git a/tests/Task_test.cpp b/tests/Task_test.cpp index 265730003..207b53aea 100644 --- a/tests/Task_test.cpp +++ b/tests/Task_test.cpp @@ -2,12 +2,15 @@ #include #include #include +#include #include +#include #include #include #include #include +#include #include #include #include @@ -295,6 +298,73 @@ class TaskTest : public QObject { QVERIFY(!job.hasNetworkFailure()); } + void test_curseForgeAbortInterruptsActiveDownload() + { + QTemporaryDir temp; + QVERIFY(temp.isValid()); + QTcpServer server; + QVERIFY(server.listen(QHostAddress::LocalHost, 0)); + int requests = 0; + connect(&server, &QTcpServer::newConnection, this, [&] { + auto* socket = server.nextPendingConnection(); + connect(socket, &QTcpSocket::readyRead, socket, [&, socket] { + socket->readAll(); + if (socket->property("responded").toBool()) + return; + socket->setProperty("responded", true); + ++requests; + // Send only a small part of a large response, then keep it open. + socket->write("HTTP/1.1 200 OK\r\nContent-Length: 67108864\r\nConnection: close\r\n\r\n"); + socket->write(QByteArray(65536, 'x')); + auto* stream = new QTimer(socket); + connect(stream, &QTimer::timeout, socket, [socket] { socket->write(QByteArray(4096, 'x')); }); + connect(socket, &QTcpSocket::disconnected, stream, &QTimer::stop); + stream->start(20); + }); + }); + + QNetworkAccessManager network; + network.setProxy(QNetworkProxy::NoProxy); + Flame::ContentDownloadJob job("Cancel large CurseForge download", &network, 1); + const auto url = QString("http://127.0.0.1:%1/").arg(server.serverPort()); + auto active = Net::Download::makeFile(QUrl(url + "active"), temp.filePath("active.jar")); + auto queued = Net::Download::makeFile(QUrl(url + "queued"), temp.filePath("queued.jar")); + job.addNetAction(active); + job.addNetAction(queued); + QSignalSpy activeFinished(active.get(), &Task::finished); + bool settledBeforeJobFinished = false; + connect(&job, &Task::finished, this, [&] { + settledBeforeJobFinished = + activeFinished.count() == 1 && QDir(temp.path()).entryList(QDir::Files | QDir::Hidden | QDir::NoDotAndDotDot).isEmpty(); + }); + + job.start(); + const bool receivedPartialFile = QTest::qWaitFor([&] { return active->getProgress() > 0; }, 3000); + const bool abortAccepted = job.abort(); + const bool stoppedPromptly = QTest::qWaitFor([&] { return job.isFinished(); }, 1000); + // Always release the deliberately stalled response, including on the old code. + if (!stoppedPromptly) { + active->abort(); + QVERIFY(QTest::qWaitFor([&] { return job.isFinished(); }, 1000)); + } + const auto activeState = active->getState(); + const auto queuedState = queued->getState(); + // Requests own their replies: delete them while the network manager is alive. + job.clear(); + active.reset(); + queued.reset(); + QCoreApplication::sendPostedEvents(nullptr, QEvent::DeferredDelete); + + QVERIFY(receivedPartialFile); + QVERIFY(abortAccepted); + QVERIFY2(stoppedPromptly, "Cancellation waited for the server to finish sending the file."); + QCOMPARE(job.getState(), Task::State::AbortedByUser); + QCOMPARE(activeState, Task::State::AbortedByUser); + QCOMPARE(queuedState, Task::State::Inactive); + QCOMPARE(requests, 1); + QVERIFY(settledBeforeJobFinished); + } + void test_stackOverflowInConcurrentTask() { QEventLoop loop; From 6f38b055d13d9619ea3a991cf864824333908629 Mon Sep 17 00:00:00 2001 From: HowXu Date: Sat, 24 Oct 2026 19:11:07 +0800 Subject: [PATCH 12/13] fix(ci): variable shadowing and gui tests headless Signed-off-by: HowXu --- launcher/InstanceCreationTask.cpp | 14 ++++++++------ tests/CMakeLists.txt | 1 + 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/launcher/InstanceCreationTask.cpp b/launcher/InstanceCreationTask.cpp index 07c9efb30..d51dd3cf9 100644 --- a/launcher/InstanceCreationTask.cpp +++ b/launcher/InstanceCreationTask.cpp @@ -137,12 +137,14 @@ void InstanceCreationTask::executeTask() task->addTask(loadTask); for (const auto& t : updateTasks) { // These connections must run before SequentialTask forwards the failure. - if (auto source = qobject_cast(t.get())) { - connect(source, &Task::failed, this, [this, source] { m_gameDownloadFailed = source->downloadFailed(); }); - } else if (auto source = qobject_cast(t.get())) { - connect(source, &Task::failed, this, [this, source] { m_gameDownloadFailed = source->downloadFailed(); }); - } else if (auto source = qobject_cast(t.get())) { - connect(source, &Task::failed, this, [this, source] { m_gameDownloadFailed = source->downloadFailed(); }); + if (auto librariesTask = qobject_cast(t.get())) { + connect(librariesTask, &Task::failed, this, + [this, librariesTask] { m_gameDownloadFailed = librariesTask->downloadFailed(); }); + } else if (auto legacyLibrariesTask = qobject_cast(t.get())) { + connect(legacyLibrariesTask, &Task::failed, this, + [this, legacyLibrariesTask] { m_gameDownloadFailed = legacyLibrariesTask->downloadFailed(); }); + } else if (auto assetTask = qobject_cast(t.get())) { + connect(assetTask, &Task::failed, this, [this, assetTask] { m_gameDownloadFailed = assetTask->downloadFailed(); }); } task->addTask(t); } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 32439569f..f4ac39e40 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -41,6 +41,7 @@ ecm_add_test(Task_test.cpp LINK_LIBRARIES Launcher_logic Qt${QT_VERSION_MAJOR}:: ecm_add_test(TaskStepWrapper_test.cpp LINK_LIBRARIES Launcher_logic Qt${QT_VERSION_MAJOR}::Test TEST_NAME TaskStepWrapper) +set_property(TEST TaskStepWrapper APPEND PROPERTY ENVIRONMENT "QT_QPA_PLATFORM=offscreen") ecm_add_test(INIFile_test.cpp LINK_LIBRARIES Launcher_logic Qt${QT_VERSION_MAJOR}::Test TEST_NAME INIFile) From 74906d1dfd17b7c83046f241a0c07fee4fd60468 Mon Sep 17 00:00:00 2001 From: HowXu Date: Sun, 25 Oct 2026 10:35:36 +0800 Subject: [PATCH 13/13] fix: remove user dialog / remove tests / set download cache as one default action Signed-off-by: HowXu --- launcher/Application.cpp | 1 - launcher/InstanceTask.cpp | 13 -- launcher/InstanceTask.h | 1 - .../flame/FlameInstanceCreationTask.cpp | 24 +-- .../modrinth/ModrinthInstanceCreationTask.cpp | 2 +- launcher/ui/pages/global/LauncherPage.cpp | 2 - launcher/ui/pages/global/LauncherPage.ui | 11 -- tests/CMakeLists.txt | 9 - tests/PendingModpackManifest_test.cpp | 177 ----------------- tests/TaskStepWrapper_test.cpp | 187 ------------------ tests/Task_test.cpp | 120 ----------- 11 files changed, 10 insertions(+), 537 deletions(-) delete mode 100644 tests/PendingModpackManifest_test.cpp delete mode 100644 tests/TaskStepWrapper_test.cpp diff --git a/launcher/Application.cpp b/launcher/Application.cpp index 411b02e77..fda21d2bf 100644 --- a/launcher/Application.cpp +++ b/launcher/Application.cpp @@ -838,7 +838,6 @@ Application::Application(int& argc, char** argv) : QApplication(argc, argv) m_settings->registerSetting("ModMetadataDisabled", false); m_settings->registerSetting("ModDependenciesDisabled", false); m_settings->registerSetting("SkipModpackUpdatePrompt", false); - m_settings->registerSetting("KeepIncompleteModpackInstances", false); m_settings->registerSetting("ShowModIncompat", false); // Minecraft offline player name diff --git a/launcher/InstanceTask.cpp b/launcher/InstanceTask.cpp index 5576aaddb..01998a7aa 100644 --- a/launcher/InstanceTask.cpp +++ b/launcher/InstanceTask.cpp @@ -93,16 +93,3 @@ ShouldDeleteSaves askIfShouldDeleteSaves(QWidget* parent) auto result = dialog->exec(); return result == QMessageBox::Yes ? ShouldDeleteSaves::Yes : ShouldDeleteSaves::No; } - -bool askToKeepIncompleteModpack(QWidget* parent, SettingsObject* settings) -{ - if (settings->get("KeepIncompleteModpackInstances").toBool()) - return true; - - auto dialog = CustomMessageBox::selectable( - parent, QObject::tr("Keep incomplete modpack?"), - QObject::tr("The modpack download did not finish. Keep the incomplete instance and download the remaining files before launching?"), - QMessageBox::Question, QMessageBox::Yes | QMessageBox::No, QMessageBox::No); - dialog->setEscapeButton(QMessageBox::No); - return dialog->exec() == QMessageBox::Yes; -} diff --git a/launcher/InstanceTask.h b/launcher/InstanceTask.h index 24c339c91..125930a27 100644 --- a/launcher/InstanceTask.h +++ b/launcher/InstanceTask.h @@ -10,7 +10,6 @@ enum class ShouldUpdate { Update, SkipUpdating, Cancel }; [[nodiscard]] ShouldUpdate askIfShouldUpdate(QWidget* parent, QString original_version_name); enum class ShouldDeleteSaves { NotAsked, Yes, No }; [[nodiscard]] ShouldDeleteSaves askIfShouldDeleteSaves(QWidget* parent); -bool askToKeepIncompleteModpack(QWidget* parent, SettingsObject* settings); struct InstanceName { public: diff --git a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp index 249e07af3..c921c7d7d 100644 --- a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp +++ b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp @@ -538,22 +538,16 @@ std::unique_ptr FlameCreationTask::createInstance() bool did_succeed = content->wasSuccessful() && !m_abort; if (!did_succeed && !shouldOverride() && content->canPreserve()) { setAbortable(false); - if (askToKeepIncompleteModpack(m_parent, m_globalSettings)) { - if (!content->canPreserve()) { - setError(tr("The incomplete instance contains an unsafe path and cannot be saved.")); - return nullptr; - } - Modpack::PendingManifest pending(Modpack::Format::CurseForge, content->remainingManifest()); - QString error; - if (!pending.save(Modpack::pendingManifestPath(instance->instanceRoot(), Modpack::Format::CurseForge), &error)) { - setError(tr("Could not save remaining modpack files:\n%1").arg(error)); - return nullptr; - } - instance->saveNow(); - setContentIncomplete(true); - m_abort = false; - return instance; + Modpack::PendingManifest pending(Modpack::Format::CurseForge, content->remainingManifest()); + QString error; + if (!pending.save(Modpack::pendingManifestPath(instance->instanceRoot(), Modpack::Format::CurseForge), &error)) { + setError(tr("Could not save remaining modpack files:\n%1").arg(error)); + return nullptr; } + instance->saveNow(); + setContentIncomplete(true); + m_abort = false; + return instance; } if (!did_succeed && !m_abort) setError(content->failReason()); diff --git a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp index 4b0b39f4e..a5b2446f5 100644 --- a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp +++ b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp @@ -290,7 +290,7 @@ std::unique_ptr ModrinthCreationTask::createInstance() setAbortable(false); } setError(content->failReason()); - if (!shouldOverride() && content->canPreserve() && askToKeepIncompleteModpack(m_parent, m_globalSettings)) { + if (!shouldOverride() && content->canPreserve()) { Modpack::PendingManifest pending(Modpack::Format::Modrinth, content->remainingManifest()); QString error; if (!pending.save(Modpack::pendingManifestPath(instance->instanceRoot(), Modpack::Format::Modrinth), &error)) { diff --git a/launcher/ui/pages/global/LauncherPage.cpp b/launcher/ui/pages/global/LauncherPage.cpp index fbb19d910..c78d99f62 100644 --- a/launcher/ui/pages/global/LauncherPage.cpp +++ b/launcher/ui/pages/global/LauncherPage.cpp @@ -247,7 +247,6 @@ void LauncherPage::applySettings() s->set("ModDependenciesDisabled", !ui->dependenciesEnableBtn->isChecked()); s->set("ShowModIncompat", ui->showModIncompatCheckBox->isChecked()); s->set("SkipModpackUpdatePrompt", !ui->modpackUpdatePromptBtn->isChecked()); - s->set("KeepIncompleteModpackInstances", ui->keepIncompleteModpackInstancesCheckBox->isChecked()); const auto showInDiscord = ui->showInDiscordBtn->isChecked(); s->set("AlwaysShowInDiscord", showInDiscord); APPLICATION->discord()->showAlways(showInDiscord); @@ -302,7 +301,6 @@ void LauncherPage::loadSettings() ui->dependenciesEnableBtn->setChecked(!s->get("ModDependenciesDisabled").toBool()); ui->showModIncompatCheckBox->setChecked(s->get("ShowModIncompat").toBool()); ui->modpackUpdatePromptBtn->setChecked(!s->get("SkipModpackUpdatePrompt").toBool()); - ui->keepIncompleteModpackInstancesCheckBox->setChecked(s->get("KeepIncompleteModpackInstances").toBool()); ui->showInDiscordBtn->setChecked(s->get("AlwaysShowInDiscord").toBool()); } diff --git a/launcher/ui/pages/global/LauncherPage.ui b/launcher/ui/pages/global/LauncherPage.ui index d4db71ec9..50e069a74 100644 --- a/launcher/ui/pages/global/LauncherPage.ui +++ b/launcher/ui/pages/global/LauncherPage.ui @@ -467,16 +467,6 @@ - - - - Keep new Modrinth and CurseForge instances when modpack downloads fail or are canceled, without asking. Remaining files will be downloaded before launch. - - - Always keep incomplete modpack instances - - - @@ -716,7 +706,6 @@ dependenciesEnableBtn showModIncompatCheckBox modpackUpdatePromptBtn - keepIncompleteModpackInstancesCheckBox downloadGameFilesBtn lineLimitSpinBox checkStopLogging diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index f4ac39e40..300fea2aa 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -39,10 +39,6 @@ ecm_add_test(ParseUtils_test.cpp LINK_LIBRARIES Launcher_logic Qt${QT_VERSION_MA ecm_add_test(Task_test.cpp LINK_LIBRARIES Launcher_logic Qt${QT_VERSION_MAJOR}::Test TEST_NAME Task) -ecm_add_test(TaskStepWrapper_test.cpp LINK_LIBRARIES Launcher_logic Qt${QT_VERSION_MAJOR}::Test - TEST_NAME TaskStepWrapper) -set_property(TEST TaskStepWrapper APPEND PROPERTY ENVIRONMENT "QT_QPA_PLATFORM=offscreen") - ecm_add_test(INIFile_test.cpp LINK_LIBRARIES Launcher_logic Qt${QT_VERSION_MAJOR}::Test TEST_NAME INIFile) @@ -52,11 +48,6 @@ ecm_add_test(JavaVersion_test.cpp LINK_LIBRARIES Launcher_logic Qt${QT_VERSION_M ecm_add_test(Packwiz_test.cpp LINK_LIBRARIES Launcher_logic Qt${QT_VERSION_MAJOR}::Test TEST_NAME Packwiz) -ecm_add_test(PendingModpackManifest_test.cpp ../launcher/modplatform/helpers/PendingModpackManifest.cpp - LINK_LIBRARIES Qt${QT_VERSION_MAJOR}::Core Qt${QT_VERSION_MAJOR}::Test - TEST_NAME PendingModpackManifest) -target_include_directories(PendingModpackManifest PRIVATE "${CMAKE_SOURCE_DIR}/launcher") - ecm_add_test(Index_test.cpp LINK_LIBRARIES Launcher_logic Qt${QT_VERSION_MAJOR}::Test TEST_NAME Index) diff --git a/tests/PendingModpackManifest_test.cpp b/tests/PendingModpackManifest_test.cpp deleted file mode 100644 index 68d355340..000000000 --- a/tests/PendingModpackManifest_test.cpp +++ /dev/null @@ -1,177 +0,0 @@ -// SPDX-License-Identifier: GPL-3.0-only - -#include -#include -#include -#include -#include -#include -#include - -#include "modplatform/helpers/PendingModpackManifest.h" - -class PendingModpackManifestTest : public QObject { - Q_OBJECT - - private slots: - void test_FilterModrinth() - { - const auto original = QJsonDocument::fromJson(R"({"files":[ - {"path":"mods/done.jar"}, {"path":"mods/failed.jar"}, {"path":"mods/not-started.jar"} - ]})") - .object(); - Modpack::PendingManifest manifest(Modpack::Format::Modrinth, original); - manifest.markCompleted("mods/done.jar"); - manifest.markCompleted("mods/done.jar"); - manifest.markCompleted("unknown.jar"); - - const auto expected = QJsonDocument::fromJson(R"({"files":[ - {"path":"mods/failed.jar"}, {"path":"mods/not-started.jar"} - ]})") - .object(); - QCOMPARE(manifest.remaining(), expected); - QCOMPARE(original.value("files").toArray().size(), 3); - } - - void test_FilterCurseForge() - { - const auto original = QJsonDocument::fromJson(R"({"files":[ - {"projectID":12,"fileID":34,"required":true}, - {"projectID":12,"fileID":35,"required":false}, - {"projectID":56,"fileID":34,"required":true} - ]})") - .object(); - Modpack::PendingManifest manifest(Modpack::Format::CurseForge, original); - manifest.markCompleted("12:34"); - const auto expected = QJsonDocument::fromJson(R"({"files":[ - {"projectID":12,"fileID":35,"required":false}, - {"projectID":56,"fileID":34,"required":true} - ]})") - .object(); - QCOMPARE(manifest.remaining(), expected); - } - - void test_PreserveOriginalFields() - { - const auto original = QJsonDocument::fromJson(R"({ - "formatVersion":1,"name":"Pack","dependencies":{"minecraft":"1.21.1","fabric-loader":"0.16.0"}, - "unknown":{"nested":[1,true,null]},"files":[ - {"path":"mods/done.jar"}, - {"path":"mods/optional.jar","env":{"client":"optional","server":"unsupported"}, - "downloads":["https:\/\/first.example/mod.jar","https:\/\/backup.example/mod.jar"], - "hashes":{"sha512":"original-hash"},"fileSize":42,"unknown":[false,{"keep":"me"}]} - ] - })") - .object(); - Modpack::PendingManifest manifest(Modpack::Format::Modrinth, original); - manifest.markCompleted("mods/done.jar"); - auto expected = original; - expected.insert("files", QJsonArray{ original.value("files").toArray().at(1) }); - QCOMPARE(manifest.remaining(), expected); - - const auto curseForge = QJsonDocument::fromJson(R"({ - "manifestType":"minecraftModpack","minecraft":{"version":"1.21.1","modLoaders":[{"id":"neoforge-21.1"}]}, - "overrides":"overrides","unknown":true, - "files":[{"projectID":1,"fileID":2,"required":false,"unknown":{"keep":true}}] - })") - .object(); - Modpack::PendingManifest cfManifest(Modpack::Format::CurseForge, curseForge); - QCOMPARE(cfManifest.remaining(), curseForge); - } - - void test_InvalidAndEmptyManifest() - { - QTemporaryDir temporary; - QVERIFY(temporary.isValid()); - const auto path = temporary.filePath("manifest.json"); - QString error; - for (const auto format : { Modpack::Format::Modrinth, Modpack::Format::CurseForge }) { - Modpack::PendingManifest empty(format, QJsonObject{ { "files", QJsonArray{} } }); - QVERIFY2(empty.save(path, &error), qPrintable(error)); - const auto loaded = Modpack::PendingManifest::load(format, path, &error); - QVERIFY2(loaded.has_value(), qPrintable(error)); - QCOMPARE(loaded->remaining(), QJsonObject({ { "files", QJsonArray{} } })); - - const QList invalid{ "{broken", "[]", "{}", R"({"files":null})", R"({"files":{}})", R"({"files":"not an array"})" }; - for (const auto& data : invalid) { - QFile file(path); - QVERIFY(file.open(QIODevice::WriteOnly)); - QCOMPARE(file.write(data), data.size()); - file.close(); - error.clear(); - QVERIFY(!Modpack::PendingManifest::load(format, path, &error).has_value()); - QVERIFY(!error.isEmpty()); - } - - error.clear(); - QVERIFY(!Modpack::PendingManifest::load(format, temporary.filePath("missing.json"), &error).has_value()); - QVERIFY(!error.isEmpty()); - - Modpack::PendingManifest invalidManifest(format, QJsonObject{ { "files", "invalid" } }); - QCOMPARE(invalidManifest.remaining().value("files").toString(), QString("invalid")); - QFile oldFile(path); - QVERIFY(oldFile.open(QIODevice::ReadOnly)); - const auto oldData = oldFile.readAll(); - oldFile.close(); - error.clear(); - QVERIFY(!invalidManifest.save(path, &error)); - QVERIFY(!error.isEmpty()); - QVERIFY(oldFile.open(QIODevice::ReadOnly)); - QCOMPARE(oldFile.readAll(), oldData); - } - } - - void test_SaveAndReload() - { - QTemporaryDir temporary; - QVERIFY(temporary.isValid()); - QCOMPARE(Modpack::pendingManifestPath(temporary.path(), Modpack::Format::Modrinth), - temporary.filePath("mrpack/remaining.index.json")); - QCOMPARE(Modpack::pendingManifestPath(temporary.path(), Modpack::Format::CurseForge), - temporary.filePath("flame/remaining.manifest.json")); - - const auto original = QJsonDocument::fromJson(R"({"name":"Pack","files":[ - {"path":"mods/a.jar","projectID":1,"fileID":2}, - {"path":"mods/b.jar","projectID":3,"fileID":4} - ]})") - .object(); - for (const auto format : { Modpack::Format::Modrinth, Modpack::Format::CurseForge }) { - const auto path = Modpack::pendingManifestPath(temporary.path(), format); - QVERIFY(QDir().mkpath(QFileInfo(path).absolutePath())); - Modpack::PendingManifest manifest(format, original); - QString error; - QVERIFY2(manifest.save(path, &error), qPrintable(error)); - QFile oldFile(path); - QVERIFY(oldFile.open(QIODevice::ReadOnly)); - const auto oldData = oldFile.readAll(); - oldFile.close(); - - manifest.markCompleted(format == Modpack::Format::Modrinth ? "mods/a.jar" : "1:2"); - // A regular file cannot serve as a parent directory, regardless of write permissions. - QVERIFY(!manifest.save(QDir(path).filePath("remaining.json"), &error)); - QVERIFY(!error.isEmpty()); - QVERIFY(oldFile.open(QIODevice::ReadOnly)); - QCOMPARE(oldFile.readAll(), oldData); - oldFile.close(); - - QVERIFY2(manifest.save(path, &error), qPrintable(error)); - auto loaded = Modpack::PendingManifest::load(format, path, &error); - QVERIFY2(loaded.has_value(), qPrintable(error)); - const auto expected = QJsonDocument::fromJson(R"({"name":"Pack","files":[ - {"path":"mods/b.jar","projectID":3,"fileID":4} - ]})") - .object(); - QCOMPARE(loaded->remaining(), expected); - loaded->markCompleted(format == Modpack::Format::Modrinth ? "mods/b.jar" : "3:4"); - QVERIFY2(loaded->save(path, &error), qPrintable(error)); - const auto completed = Modpack::PendingManifest::load(format, path, &error); - QVERIFY2(completed.has_value(), qPrintable(error)); - QCOMPARE(completed->remaining(), QJsonObject({ { "name", "Pack" }, { "files", QJsonArray{} } })); - QVERIFY(!manifest.save(temporary.filePath("missing/manifest.json"), nullptr)); - } - } -}; - -QTEST_GUILESS_MAIN(PendingModpackManifestTest) - -#include "PendingModpackManifest_test.moc" diff --git a/tests/TaskStepWrapper_test.cpp b/tests/TaskStepWrapper_test.cpp deleted file mode 100644 index 9b91746e4..000000000 --- a/tests/TaskStepWrapper_test.cpp +++ /dev/null @@ -1,187 +0,0 @@ -// SPDX-License-Identifier: GPL-3.0-only - -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -#include "launch/LaunchTask.h" -#include "launch/TaskStepWrapper.h" -#include "modplatform/ModpackResumeTask.h" -#include "modplatform/helpers/PendingModpackManifest.h" -#include "settings/INISettingsObject.h" -#include "ui/dialogs/ProgressDialog.h" -#include "ui/dialogs/UntrustedModsDialog.h" - -// Exercise launch steps without an instance, authentication or a game process. -class TestLaunchTask : public LaunchTask { - public: - TestLaunchTask(MinecraftInstance* instance = nullptr) : LaunchTask(instance) {} - - void executeTask() override - { - state = Running; - onStepFinished(); - } - - protected: - void emitSucceeded() override { Task::emitSucceeded(); } - void emitFailed(QString reason) override { Task::emitFailed(reason); } -}; - -// CurseForge installation waits for network tasks in a nested event loop. -class BlockingInstallTask : public Task { - Q_OBJECT - - signals: - void downloading(); - - private: - void executeTask() override - { - QEventLoop loop; - QTimer::singleShot(0, &loop, [&] { - setStatus("Downloading mods..."); - setProgress(25, 100); - emit downloading(); - loop.quit(); - }); - loop.exec(); - emitSucceeded(); - } -}; - -class TaskStepWrapperTest : public QObject { - Q_OBJECT - - private slots: - void resumeProgressFollowsTrustConfirmation_data() - { - QTest::addColumn("modrinth"); - QTest::addColumn("acceptTrust"); - QTest::newRow("Modrinth-reject") << true << false; - QTest::newRow("CurseForge-reject") << false << false; - QTest::newRow("Modrinth-accept") << true << true; - QTest::newRow("CurseForge-accept") << false << true; - } - - void resumeProgressFollowsTrustConfirmation() - { - QFETCH(bool, modrinth); - QFETCH(bool, acceptTrust); - QTemporaryDir temp; - QVERIFY(temp.isValid()); - QVERIFY(QDir(temp.path()).mkpath("minecraft/mods")); - QFile mod(temp.filePath("minecraft/mods/untrusted.jar")); - QVERIFY(mod.open(QIODevice::WriteOnly)); - mod.close(); - - INISettingsObject globalSettings(temp.filePath("global.cfg")); - for (const auto& key : - { "ShowGameTime", "RecordGameTime", "PreLaunchCommand", "WrapperCommand", "PostExitCommand", "ShowConsole", "AutoCloseConsole", - "ShowConsoleOnError", "LogPrePostOutput", "ConsoleOverflowStop", "EnableDiscordRichPresence" }) { - globalSettings.registerSetting(key, false); - } - globalSettings.registerSetting("ConsoleMaxLines", 1000); - MinecraftInstance instance(&globalSettings, std::make_unique(temp.filePath("instance.cfg")), temp.path()); - - const auto format = modrinth ? Modpack::Format::Modrinth : Modpack::Format::CurseForge; - const QJsonObject manifest = - modrinth ? QJsonObject{ { "formatVersion", 1 }, - { "game", "minecraft" }, - { "dependencies", QJsonObject{ { "minecraft", "1.20.1" } } }, - { "files", QJsonArray{} } } - : QJsonObject{ { "manifestType", "minecraftModpack" }, { "manifestVersion", 1 }, { "files", QJsonArray{} } }; - const auto pendingPath = Modpack::pendingManifestPath(temp.path(), format); - QVERIFY(QDir().mkpath(QFileInfo(pendingPath).absolutePath())); - Modpack::PendingManifest pending(format, manifest); - QString error; - QVERIFY2(pending.save(pendingPath, &error), qPrintable(error)); - - TestLaunchTask launch(&instance); - auto resume = makeShared(&launch, &instance, false); - launch.appendStep(resume); - int progressRequests = 0; - connect(&launch, &LaunchTask::requestProgress, this, [&](Task* task) { - ++progressRequests; - ProgressDialog dialog; - launch.proceed(); - // Cancel at the handoff: queued content work must settle without - // starting real network requests or trapping the modal dialog. - QVERIFY(task->abort()); - dialog.execWithTask(task); - }); - - bool confirmedDialogShown = false; - bool progressVisibleDuringConfirmation = false; - QTimer observeDialog; - connect(&observeDialog, &QTimer::timeout, this, [&] { - for (auto* widget : QApplication::topLevelWidgets()) { - auto* confirmation = qobject_cast(widget); - if (!confirmation || !confirmation->isVisible()) - continue; - confirmedDialogShown = true; - for (auto* other : QApplication::topLevelWidgets()) { - if (qobject_cast(other) && other->isVisible()) - progressVisibleDuringConfirmation = true; - } - if (acceptTrust) - confirmation->accept(); - else - confirmation->reject(); - } - }); - observeDialog.start(); - launch.start(); - QTRY_VERIFY_WITH_TIMEOUT(launch.isFinished(), 5000); - - QVERIFY(confirmedDialogShown); - QVERIFY(!progressVisibleDuringConfirmation); - QCOMPARE(progressRequests, acceptTrust ? 1 : 0); - QCOMPARE(resume->getState(), Task::State::AbortedByUser); - const auto remaining = Modpack::PendingManifest::load(format, pendingPath, &error); - QVERIFY2(remaining.has_value(), qPrintable(error)); - QCOMPARE(remaining->remaining(), manifest); - } - - void progressDialogIsVisibleDuringBlockingInstall() - { - TestLaunchTask launch; - auto install = makeShared(); - auto step = makeShared(&launch, install); - launch.appendStep(step); - - ProgressDialog dialog; - bool visibleDuringDownload = false; - QString displayedStatus; - int displayedProgress = -1; - connect(install.get(), &BlockingInstallTask::downloading, this, [&] { - visibleDuringDownload = dialog.isVisible(); - displayedStatus = dialog.findChild("globalStatusLabel")->text(); - displayedProgress = dialog.findChild("globalProgressBar")->value(); - }); - connect(&launch, &LaunchTask::requestProgress, this, [&](Task* task) { - // Match LaunchController's progress-dialog handoff. - launch.proceed(); - QCOMPARE(dialog.execWithTask(task), QDialog::Accepted); - }); - - launch.start(); - - QVERIFY(launch.wasSuccessful()); - QVERIFY(visibleDuringDownload); - QCOMPARE(displayedStatus, QString("Downloading mods...")); - QCOMPARE(displayedProgress, 25); - QVERIFY(!dialog.isVisible()); - } -}; - -QTEST_MAIN(TaskStepWrapperTest) - -#include "TaskStepWrapper_test.moc" diff --git a/tests/Task_test.cpp b/tests/Task_test.cpp index 207b53aea..1c95f702d 100644 --- a/tests/Task_test.cpp +++ b/tests/Task_test.cpp @@ -1,18 +1,7 @@ -#include -#include -#include -#include -#include -#include -#include -#include #include #include #include -#include -#include -#include #include #include #include @@ -256,115 +245,6 @@ class TaskTest : public QObject { QVERIFY2(QTest::qWaitFor([&t]() { return t.isFinished(); }, 1000), "Task didn't finish as it should."); } - void test_networkRetryLocalFailureClearsNetworkError() - { - QTemporaryDir temp; - QVERIFY(temp.isValid()); - QVERIFY(QDir(temp.path()).mkdir("downloads")); - const auto parentPath = temp.filePath("downloads"); - - QTcpServer portReservation; - QVERIFY(portReservation.listen(QHostAddress::LocalHost, 0)); - const auto port = portReservation.serverPort(); - - QNetworkAccessManager network; - network.setProxy(QNetworkProxy::NoProxy); - NetJob job("Network retry followed by local failure", &network); - job.setAskRetry(false); - auto download = Net::Download::makeFile(QUrl(QString("http://127.0.0.1:%1/file").arg(port)), parentPath + "/file"); - job.addNetAction(download); - - int failureCount = 0; - QObject observer; - connect(download.get(), &Task::failed, &observer, [&] { - if (++failureCount != 1) - return; - - QVERIFY(download->error() != QNetworkReply::NoError); - QVERIFY(download->error() != QNetworkReply::OperationCanceledError); - // Move the directory aside, including any still-open temporary save file. - QVERIFY(QDir(temp.path()).rename("downloads", "failed-downloads")); - QFile blocker(parentPath); - QVERIFY(blocker.open(QIODevice::WriteOnly)); - }); - - portReservation.close(); - job.start(); - - QVERIFY2(QTest::qWaitFor([&job] { return job.isFinished(); }, 10000), "NetJob did not finish its retries."); - QVERIFY(!job.wasSuccessful()); - QVERIFY(failureCount > 1); - QCOMPARE(download->error(), QNetworkReply::NoError); - QVERIFY(!job.hasNetworkFailure()); - } - - void test_curseForgeAbortInterruptsActiveDownload() - { - QTemporaryDir temp; - QVERIFY(temp.isValid()); - QTcpServer server; - QVERIFY(server.listen(QHostAddress::LocalHost, 0)); - int requests = 0; - connect(&server, &QTcpServer::newConnection, this, [&] { - auto* socket = server.nextPendingConnection(); - connect(socket, &QTcpSocket::readyRead, socket, [&, socket] { - socket->readAll(); - if (socket->property("responded").toBool()) - return; - socket->setProperty("responded", true); - ++requests; - // Send only a small part of a large response, then keep it open. - socket->write("HTTP/1.1 200 OK\r\nContent-Length: 67108864\r\nConnection: close\r\n\r\n"); - socket->write(QByteArray(65536, 'x')); - auto* stream = new QTimer(socket); - connect(stream, &QTimer::timeout, socket, [socket] { socket->write(QByteArray(4096, 'x')); }); - connect(socket, &QTcpSocket::disconnected, stream, &QTimer::stop); - stream->start(20); - }); - }); - - QNetworkAccessManager network; - network.setProxy(QNetworkProxy::NoProxy); - Flame::ContentDownloadJob job("Cancel large CurseForge download", &network, 1); - const auto url = QString("http://127.0.0.1:%1/").arg(server.serverPort()); - auto active = Net::Download::makeFile(QUrl(url + "active"), temp.filePath("active.jar")); - auto queued = Net::Download::makeFile(QUrl(url + "queued"), temp.filePath("queued.jar")); - job.addNetAction(active); - job.addNetAction(queued); - QSignalSpy activeFinished(active.get(), &Task::finished); - bool settledBeforeJobFinished = false; - connect(&job, &Task::finished, this, [&] { - settledBeforeJobFinished = - activeFinished.count() == 1 && QDir(temp.path()).entryList(QDir::Files | QDir::Hidden | QDir::NoDotAndDotDot).isEmpty(); - }); - - job.start(); - const bool receivedPartialFile = QTest::qWaitFor([&] { return active->getProgress() > 0; }, 3000); - const bool abortAccepted = job.abort(); - const bool stoppedPromptly = QTest::qWaitFor([&] { return job.isFinished(); }, 1000); - // Always release the deliberately stalled response, including on the old code. - if (!stoppedPromptly) { - active->abort(); - QVERIFY(QTest::qWaitFor([&] { return job.isFinished(); }, 1000)); - } - const auto activeState = active->getState(); - const auto queuedState = queued->getState(); - // Requests own their replies: delete them while the network manager is alive. - job.clear(); - active.reset(); - queued.reset(); - QCoreApplication::sendPostedEvents(nullptr, QEvent::DeferredDelete); - - QVERIFY(receivedPartialFile); - QVERIFY(abortAccepted); - QVERIFY2(stoppedPromptly, "Cancellation waited for the server to finish sending the file."); - QCOMPARE(job.getState(), Task::State::AbortedByUser); - QCOMPARE(activeState, Task::State::AbortedByUser); - QCOMPARE(queuedState, Task::State::Inactive); - QCOMPARE(requests, 1); - QVERIFY(settledBeforeJobFinished); - } - void test_stackOverflowInConcurrentTask() { QEventLoop loop;