From 7f4665e3cca891bdd42d67a5a82a8c6561d1c86e Mon Sep 17 00:00:00 2001 From: Trial97 Date: Fri, 14 Jul 2023 21:07:34 +0300 Subject: [PATCH 01/15] format Signed-off-by: Trial97 --- .../modplatform/flame/FileResolvingTask.cpp | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/launcher/modplatform/flame/FileResolvingTask.cpp b/launcher/modplatform/flame/FileResolvingTask.cpp index ce7a60551..f84002385 100644 --- a/launcher/modplatform/flame/FileResolvingTask.cpp +++ b/launcher/modplatform/flame/FileResolvingTask.cpp @@ -128,12 +128,13 @@ void Flame::FileResolvingTask::netJobFinished() m_checkJob->start(); } -void Flame::FileResolvingTask::modrinthCheckFinished() { +void Flame::FileResolvingTask::modrinthCheckFinished() +{ setProgress(2, 3); qDebug() << "Finished with blocked mods : " << blockedProjects.size(); for (auto it = blockedProjects.keyBegin(); it != blockedProjects.keyEnd(); it++) { - auto &out = *it; + auto& out = *it; auto bytes = blockedProjects[out]; if (!out->resolved) { continue; @@ -153,15 +154,13 @@ void Flame::FileResolvingTask::modrinthCheckFinished() { out->resolved = false; } } - //copy to an output list and filter out projects found on modrinth + // copy to an output list and filter out projects found on modrinth auto block = std::make_shared>(); auto it = blockedProjects.keys(); - std::copy_if(it.begin(), it.end(), std::back_inserter(*block), [](File *f) { - return !f->resolved; - }); - //Display not found mods early + std::copy_if(it.begin(), it.end(), std::back_inserter(*block), [](File* f) { return !f->resolved; }); + // Display not found mods early if (!block->empty()) { - //blocked mods found, we need the slug for displaying.... we need another job :D ! + // blocked mods found, we need the slug for displaying.... we need another job :D ! m_slugJob.reset(new NetJob("Slug Job", m_network)); int index = 0; for (auto mod : *block) { @@ -173,8 +172,8 @@ void Flame::FileResolvingTask::modrinthCheckFinished() { QObject::connect(dl.get(), &Net::Download::succeeded, [block, index, output]() { auto mod = block->at(index); // use the shared_ptr so it is captured and only freed when we are done auto json = QJsonDocument::fromJson(*output); - auto base = Json::requireString(Json::requireObject(Json::requireObject(Json::requireObject(json),"data"),"links"), - "websiteUrl"); + auto base = + Json::requireString(Json::requireObject(Json::requireObject(Json::requireObject(json), "data"), "links"), "websiteUrl"); auto link = QString("%1/download/%2").arg(base, QString::number(mod->fileId)); mod->websiteUrl = link; }); From d3dc00dcab31ecf4a8ae5886db310ff0bedc9018 Mon Sep 17 00:00:00 2001 From: Trial97 Date: Fri, 14 Jul 2023 21:07:53 +0300 Subject: [PATCH 02/15] made concurent task fail Signed-off-by: Trial97 --- .../modplatform/flame/FileResolvingTask.cpp | 1 - launcher/tasks/ConcurrentTask.cpp | 20 +++++++++---------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/launcher/modplatform/flame/FileResolvingTask.cpp b/launcher/modplatform/flame/FileResolvingTask.cpp index f84002385..7695dfcdf 100644 --- a/launcher/modplatform/flame/FileResolvingTask.cpp +++ b/launcher/modplatform/flame/FileResolvingTask.cpp @@ -112,7 +112,6 @@ void Flame::FileResolvingTask::netJobFinished() connect(m_checkJob.get(), &NetJob::failed, this, [this, step_progress](QString reason) { step_progress->state = TaskStepState::Failed; stepProgress(*step_progress); - emitFailed(reason); }); connect(m_checkJob.get(), &NetJob::stepProgress, this, &FileResolvingTask::propogateStepProgress); connect(m_checkJob.get(), &NetJob::progress, this, [this, step_progress](qint64 current, qint64 total) { diff --git a/launcher/tasks/ConcurrentTask.cpp b/launcher/tasks/ConcurrentTask.cpp index 9aada5e69..7c7cfaf72 100644 --- a/launcher/tasks/ConcurrentTask.cpp +++ b/launcher/tasks/ConcurrentTask.cpp @@ -118,8 +118,11 @@ void ConcurrentTask::startNext() if (m_aborted || m_doing.count() > m_total_max_size) return; - if (m_queue.isEmpty() && m_doing.isEmpty() && !wasSuccessful()) { - emitSucceeded(); + if (m_queue.isEmpty() && m_doing.isEmpty() && isRunning()) { + if (m_failed.isEmpty()) + emitSucceeded(); + else + emitFailed(tr("One or more subtasks failed")); return; } @@ -194,7 +197,7 @@ void ConcurrentTask::subTaskStatus(Task::Ptr task, const QString& msg) auto task_progress = m_task_progress.value(task->getUid()); task_progress->status = msg; task_progress->state = TaskStepState::Running; - + emit stepProgress(*task_progress); if (totalSize() == 1) { @@ -207,7 +210,7 @@ void ConcurrentTask::subTaskDetails(Task::Ptr task, const QString& msg) auto task_progress = m_task_progress.value(task->getUid()); task_progress->details = msg; task_progress->state = TaskStepState::Running; - + emit stepProgress(*task_progress); if (totalSize() == 1) { @@ -220,7 +223,7 @@ void ConcurrentTask::subTaskProgress(Task::Ptr task, qint64 current, qint64 tota auto task_progress = m_task_progress.value(task->getUid()); task_progress->update(current, total); - + emit stepProgress(*task_progress); updateStepProgress(*task_progress, Operation::CHANGED); updateState(); @@ -233,7 +236,7 @@ void ConcurrentTask::subTaskProgress(Task::Ptr task, qint64 current, qint64 tota void ConcurrentTask::subTaskStepProgress(Task::Ptr task, TaskStepProgress const& task_progress) { Operation op = Operation::ADDED; - + if (!m_task_progress.contains(task_progress.uid)) { m_task_progress.insert(task_progress.uid, std::make_shared(task_progress)); op = Operation::ADDED; @@ -254,12 +257,10 @@ void ConcurrentTask::subTaskStepProgress(Task::Ptr task, TaskStepProgress const& emit stepProgress(*tp.get()); updateStepProgress(*tp.get(), op); } - } void ConcurrentTask::updateStepProgress(TaskStepProgress const& changed_progress, Operation op) { - switch (op) { case Operation::ADDED: m_stepProgress += changed_progress.current; @@ -274,9 +275,8 @@ void ConcurrentTask::updateStepProgress(TaskStepProgress const& changed_progress m_stepTotalProgress -= changed_progress.old_total; m_stepProgress += changed_progress.current; m_stepTotalProgress += changed_progress.total; - break; + break; } - } void ConcurrentTask::updateState() From 0880d4d37b654ffe68c4497a78883e73933dfe98 Mon Sep 17 00:00:00 2001 From: Trial97 Date: Sat, 15 Jul 2023 09:16:37 +0300 Subject: [PATCH 03/15] made sure abort signal is catched in cocurent task Signed-off-by: Trial97 --- launcher/tasks/ConcurrentTask.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/launcher/tasks/ConcurrentTask.cpp b/launcher/tasks/ConcurrentTask.cpp index 7c7cfaf72..7bb02e57d 100644 --- a/launcher/tasks/ConcurrentTask.cpp +++ b/launcher/tasks/ConcurrentTask.cpp @@ -88,6 +88,7 @@ bool ConcurrentTask::abort() QMutableHashIterator doing_iter(m_doing); while (doing_iter.hasNext()) { auto task = doing_iter.next(); + disconnect(task.value().get(), &Task::aborted, this, 0); suceedeed &= (task.value())->abort(); } @@ -133,6 +134,9 @@ void ConcurrentTask::startNext() connect(next.get(), &Task::succeeded, this, [this, next]() { subTaskSucceeded(next); }); connect(next.get(), &Task::failed, this, [this, next](QString msg) { subTaskFailed(next, msg); }); + // this should never happen but if it does better to fail the task that being stuck + // most + connect(next.get(), &Task::aborted, this, [this, next] { subTaskFailed(next, "Aborted"); }); connect(next.get(), &Task::status, this, [this, next](QString msg) { subTaskStatus(next, msg); }); connect(next.get(), &Task::details, this, [this, next](QString msg) { subTaskDetails(next, msg); }); From a7e0c9db9694a153a94cf0c924296576b0e9b767 Mon Sep 17 00:00:00 2001 From: Trial97 Date: Sat, 15 Jul 2023 12:01:07 +0300 Subject: [PATCH 04/15] connected failed event for some tasks Signed-off-by: Trial97 --- launcher/modplatform/EnsureMetadataTask.cpp | 2 ++ .../modplatform/flame/FlameCheckUpdate.cpp | 7 ++--- launcher/modplatform/flame/FlameCheckUpdate.h | 8 +++++- .../flame/FlameInstanceCreationTask.cpp | 1 + .../modplatform/flame/FlamePackExportTask.cpp | 1 + .../modrinth/ModrinthCheckUpdate.cpp | 14 ++++------ launcher/ui/dialogs/ModUpdateDialog.cpp | 8 +++++- .../ui/pages/modplatform/flame/FlamePage.cpp | 12 ++++----- .../modplatform/modrinth/ModrinthPage.cpp | 5 ++++ .../pages/modplatform/technic/TechnicPage.cpp | 10 ++++--- .../ui/widgets/VariableSizedImageObject.cpp | 26 +++++++++++++------ 11 files changed, 63 insertions(+), 31 deletions(-) diff --git a/launcher/modplatform/EnsureMetadataTask.cpp b/launcher/modplatform/EnsureMetadataTask.cpp index c3eadd06d..22581e43e 100644 --- a/launcher/modplatform/EnsureMetadataTask.cpp +++ b/launcher/modplatform/EnsureMetadataTask.cpp @@ -148,6 +148,7 @@ void EnsureMetadataTask::executeTask() if (m_current_task) m_current_task.reset(); }); + connect(project_task.get(), &Task::failed, this, &EnsureMetadataTask::emitFailed); m_current_task = project_task; project_task->start(); @@ -158,6 +159,7 @@ void EnsureMetadataTask::executeTask() if (m_current_task) m_current_task.reset(); }); + connect(version_task.get(), &Task::failed, this, &EnsureMetadataTask::emitFailed); if (m_mods.size() > 1) setStatus(tr("Requesting metadata information from %1...").arg(ProviderCaps.readableName(m_provider))); diff --git a/launcher/modplatform/flame/FlameCheckUpdate.cpp b/launcher/modplatform/flame/FlameCheckUpdate.cpp index a2628e34c..092133f1a 100644 --- a/launcher/modplatform/flame/FlameCheckUpdate.cpp +++ b/launcher/modplatform/flame/FlameCheckUpdate.cpp @@ -23,7 +23,7 @@ bool FlameCheckUpdate::abort() return true; } -ModPlatform::IndexedPack getProjectInfo(ModPlatform::IndexedVersion& ver_info) +ModPlatform::IndexedPack FlameCheckUpdate::getProjectInfo(ModPlatform::IndexedVersion& ver_info) { ModPlatform::IndexedPack pack; @@ -56,6 +56,7 @@ ModPlatform::IndexedPack getProjectInfo(ModPlatform::IndexedVersion& ver_info) } }); + connect(get_project_job, &NetJob::failed, this, &FlameCheckUpdate::emitFailed); QObject::connect(get_project_job, &NetJob::finished, [&loop, get_project_job] { get_project_job->deleteLater(); loop.quit(); @@ -67,7 +68,7 @@ ModPlatform::IndexedPack getProjectInfo(ModPlatform::IndexedVersion& ver_info) return pack; } -ModPlatform::IndexedVersion getFileInfo(int addonId, int fileId) +ModPlatform::IndexedVersion FlameCheckUpdate::getFileInfo(int addonId, int fileId) { ModPlatform::IndexedVersion ver; @@ -99,7 +100,7 @@ ModPlatform::IndexedVersion getFileInfo(int addonId, int fileId) qDebug() << doc; } }); - + connect(get_file_info_job, &NetJob::failed, this, &FlameCheckUpdate::emitFailed); QObject::connect(get_file_info_job, &NetJob::finished, [&loop, get_file_info_job] { get_file_info_job->deleteLater(); loop.quit(); diff --git a/launcher/modplatform/flame/FlameCheckUpdate.h b/launcher/modplatform/flame/FlameCheckUpdate.h index 4a98d684f..0dbd199e7 100644 --- a/launcher/modplatform/flame/FlameCheckUpdate.h +++ b/launcher/modplatform/flame/FlameCheckUpdate.h @@ -8,7 +8,10 @@ class FlameCheckUpdate : public CheckUpdateTask { Q_OBJECT public: - FlameCheckUpdate(QList& mods, std::list& mcVersions, std::optional loaders, std::shared_ptr mods_folder) + FlameCheckUpdate(QList& mods, + std::list& mcVersions, + std::optional loaders, + std::shared_ptr mods_folder) : CheckUpdateTask(mods, mcVersions, loaders, mods_folder) {} @@ -19,6 +22,9 @@ class FlameCheckUpdate : public CheckUpdateTask { void executeTask() override; private: + ModPlatform::IndexedPack getProjectInfo(ModPlatform::IndexedVersion& ver_info); + ModPlatform::IndexedVersion getFileInfo(int addonId, int fileId); + NetJob* m_net_job = nullptr; bool m_was_aborted = false; diff --git a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp index b57db288a..fa5057e5c 100644 --- a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp +++ b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp @@ -229,6 +229,7 @@ bool FlameCreationTask::updateInstance() m_files_to_remove.append(old_minecraft_dir.absoluteFilePath(relative_path)); } }); + connect(job.get(), &Task::failed, this, [](QString reason) { qCritical() << "Failed to get files: " << reason; }); connect(job.get(), &Task::finished, &loop, &QEventLoop::quit); m_process_update_file_info_job = job; diff --git a/launcher/modplatform/flame/FlamePackExportTask.cpp b/launcher/modplatform/flame/FlamePackExportTask.cpp index ac0da2142..69f88127e 100644 --- a/launcher/modplatform/flame/FlamePackExportTask.cpp +++ b/launcher/modplatform/flame/FlamePackExportTask.cpp @@ -327,6 +327,7 @@ void FlamePackExportTask::getProjectsInfo() } buildZip(); }); + connect(projTask.get(), &Task::failed, this, &FlamePackExportTask::emitFailed); task.reset(projTask); task->start(); } diff --git a/launcher/modplatform/modrinth/ModrinthCheckUpdate.cpp b/launcher/modplatform/modrinth/ModrinthCheckUpdate.cpp index a7c22832a..c83ef2fd4 100644 --- a/launcher/modplatform/modrinth/ModrinthCheckUpdate.cpp +++ b/launcher/modplatform/modrinth/ModrinthCheckUpdate.cpp @@ -73,8 +73,6 @@ void ModrinthCheckUpdate::executeTask() auto response = std::make_shared(); auto job = api.latestVersions(hashes, best_hash_type, m_game_versions, m_loaders, response); - QEventLoop lock; - connect(job.get(), &Task::succeeded, this, [this, response, &mappings, best_hash_type, job] { QJsonParseError parse_error{}; QJsonDocument doc = QJsonDocument::fromJson(*response, &parse_error); @@ -83,7 +81,7 @@ void ModrinthCheckUpdate::executeTask() << " reason: " << parse_error.errorString(); qWarning() << *response; - failed(parse_error.errorString()); + emitFailed(parse_error.errorString()); return; } @@ -167,19 +165,17 @@ void ModrinthCheckUpdate::executeTask() } } } catch (Json::JsonException& e) { - failed(e.cause() + " : " + e.what()); + emitFailed(e.cause() + " : " + e.what()); + return; } + emitSucceeded(); }); - connect(job.get(), &Task::finished, &lock, &QEventLoop::quit); + connect(job.get(), &Task::failed, this, &ModrinthCheckUpdate::emitFailed); setStatus(tr("Waiting for the API response from Modrinth...")); setProgress(1, 3); m_net_job = qSharedPointerObjectCast(job); job->start(); - - lock.exec(); - - emitSucceeded(); } diff --git a/launcher/ui/dialogs/ModUpdateDialog.cpp b/launcher/ui/dialogs/ModUpdateDialog.cpp index 8618b9240..df45cbbcf 100644 --- a/launcher/ui/dialogs/ModUpdateDialog.cpp +++ b/launcher/ui/dialogs/ModUpdateDialog.cpp @@ -271,6 +271,8 @@ auto ModUpdateDialog::ensureMetadata() -> bool connect(modrinth_task.get(), &EnsureMetadataTask::metadataFailed, [this, &should_try_others](Mod* candidate) { onMetadataFailed(candidate, should_try_others.find(candidate->internal_id()).value(), ModPlatform::ResourceProvider::MODRINTH); }); + connect(modrinth_task.get(), &EnsureMetadataTask::failed, + [this](QString reason) { CustomMessageBox::selectable(this, tr("Error"), reason, QMessageBox::Critical)->exec(); }); if (modrinth_task->getHashingTask()) seq.addTask(modrinth_task->getHashingTask()); @@ -284,6 +286,8 @@ auto ModUpdateDialog::ensureMetadata() -> bool connect(flame_task.get(), &EnsureMetadataTask::metadataFailed, [this, &should_try_others](Mod* candidate) { onMetadataFailed(candidate, should_try_others.find(candidate->internal_id()).value(), ModPlatform::ResourceProvider::FLAME); }); + connect(flame_task.get(), &EnsureMetadataTask::failed, + [this](QString reason) { CustomMessageBox::selectable(this, tr("Error"), reason, QMessageBox::Critical)->exec(); }); if (flame_task->getHashingTask()) seq.addTask(flame_task->getHashingTask()); @@ -337,12 +341,14 @@ void ModUpdateDialog::onMetadataFailed(Mod* mod, bool try_others, ModPlatform::R auto task = makeShared(mod, index_dir, next(first_choice)); connect(task.get(), &EnsureMetadataTask::metadataReady, [this](Mod* candidate) { onMetadataEnsured(candidate); }); connect(task.get(), &EnsureMetadataTask::metadataFailed, [this](Mod* candidate) { onMetadataFailed(candidate, false); }); + connect(task.get(), &EnsureMetadataTask::failed, + [this](QString reason) { CustomMessageBox::selectable(this, tr("Error"), reason, QMessageBox::Critical)->exec(); }); m_second_try_metadata->addTask(task); } else { QString reason{ tr("Couldn't find a valid version on the selected mod provider(s)") }; - m_failed_metadata.append({mod, reason}); + m_failed_metadata.append({ mod, reason }); } } diff --git a/launcher/ui/pages/modplatform/flame/FlamePage.cpp b/launcher/ui/pages/modplatform/flame/FlamePage.cpp index cef26bb6b..133a3981a 100644 --- a/launcher/ui/pages/modplatform/flame/FlamePage.cpp +++ b/launcher/ui/pages/modplatform/flame/FlamePage.cpp @@ -34,6 +34,7 @@ */ #include "FlamePage.h" +#include "ui/dialogs/CustomMessageBox.h" #include "ui_FlamePage.h" #include @@ -42,9 +43,9 @@ #include "FlameModel.h" #include "InstanceImportTask.h" #include "Json.h" +#include "modplatform/flame/FlameAPI.h" #include "ui/dialogs/NewInstanceDialog.h" #include "ui/widgets/ProjectItem.h" -#include "modplatform/flame/FlameAPI.h" static FlameAPI api; @@ -171,6 +172,8 @@ void FlamePage::onSelectionChanged(QModelIndex curr, QModelIndex prev) suggestCurrent(); }); QObject::connect(netJob, &NetJob::finished, this, [response, netJob] { netJob->deleteLater(); }); + connect(netJob, &NetJob::failed, + [this](QString reason) { CustomMessageBox::selectable(this, tr("Error"), reason, QMessageBox::Critical)->exec(); }); netJob->start(); } else { for (auto version : current.versions) { @@ -252,10 +255,8 @@ void FlamePage::updateUi() text += "
" + tr(" by ") + authorStrs.join(", "); } - if(current.extraInfoLoaded) { - if (!current.extra.issuesUrl.isEmpty() - || !current.extra.sourceUrl.isEmpty() - || !current.extra.wikiUrl.isEmpty()) { + if (current.extraInfoLoaded) { + if (!current.extra.issuesUrl.isEmpty() || !current.extra.sourceUrl.isEmpty() || !current.extra.wikiUrl.isEmpty()) { text += "

" + tr("External links:") + "
"; } @@ -267,7 +268,6 @@ void FlamePage::updateUi() text += "- " + tr("Source code: %1").arg(current.extra.sourceUrl) + "
"; } - text += "
"; text += api.getModDescription(current.addonId).toUtf8(); diff --git a/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp b/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp index c71dd9038..68ce8fd63 100644 --- a/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp +++ b/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp @@ -35,6 +35,7 @@ */ #include "ModrinthPage.h" +#include "ui/dialogs/CustomMessageBox.h" #include "ui_ModrinthPage.h" #include "ModrinthModel.h" @@ -163,6 +164,8 @@ void ModrinthPage::onSelectionChanged(QModelIndex curr, QModelIndex prev) suggestCurrent(); }); QObject::connect(netJob, &NetJob::finished, this, [response, netJob] { netJob->deleteLater(); }); + connect(netJob, &NetJob::failed, + [this](QString reason) { CustomMessageBox::selectable(this, tr("Error"), reason, QMessageBox::Critical)->exec(); }); netJob->start(); } else updateUI(); @@ -215,6 +218,8 @@ void ModrinthPage::onSelectionChanged(QModelIndex curr, QModelIndex prev) suggestCurrent(); }); QObject::connect(netJob, &NetJob::finished, this, [response, netJob] { netJob->deleteLater(); }); + connect(netJob, &NetJob::failed, + [this](QString reason) { CustomMessageBox::selectable(this, tr("Error"), reason, QMessageBox::Critical)->exec(); }); netJob->start(); } else { diff --git a/launcher/ui/pages/modplatform/technic/TechnicPage.cpp b/launcher/ui/pages/modplatform/technic/TechnicPage.cpp index fc678fa20..c7f74d5ac 100644 --- a/launcher/ui/pages/modplatform/technic/TechnicPage.cpp +++ b/launcher/ui/pages/modplatform/technic/TechnicPage.cpp @@ -34,6 +34,7 @@ */ #include "TechnicPage.h" +#include "ui/dialogs/CustomMessageBox.h" #include "ui_TechnicPage.h" #include @@ -41,16 +42,15 @@ #include "ui/dialogs/NewInstanceDialog.h" #include "BuildConfig.h" +#include "Json.h" #include "TechnicModel.h" #include "modplatform/technic/SingleZipPackInstallTask.h" #include "modplatform/technic/SolderPackInstallTask.h" -#include "Json.h" #include "Application.h" #include "modplatform/technic/SolderPackManifest.h" -TechnicPage::TechnicPage(NewInstanceDialog* dialog, QWidget *parent) - : QWidget(parent), ui(new Ui::TechnicPage), dialog(dialog) +TechnicPage::TechnicPage(NewInstanceDialog* dialog, QWidget* parent) : QWidget(parent), ui(new Ui::TechnicPage), dialog(dialog) { ui->setupUi(this); connect(ui->searchButton, &QPushButton::clicked, this, &TechnicPage::triggerSearch); @@ -201,6 +201,8 @@ void TechnicPage::suggestCurrent() metadataLoaded(); }); + connect(jobPtr.get(), &NetJob::failed, + [this](QString reason) { CustomMessageBox::selectable(this, tr("Error"), reason, QMessageBox::Critical)->exec(); }); jobPtr = netJob; jobPtr->start(); @@ -252,6 +254,8 @@ void TechnicPage::metadataLoaded() netJob->addNetAction(Net::Download::makeByteArray(QUrl(url), response)); QObject::connect(netJob.get(), &NetJob::succeeded, this, &TechnicPage::onSolderLoaded); + connect(jobPtr.get(), &NetJob::failed, + [this](QString reason) { CustomMessageBox::selectable(this, tr("Error"), reason, QMessageBox::Critical)->exec(); }); jobPtr = netJob; jobPtr->start(); diff --git a/launcher/ui/widgets/VariableSizedImageObject.cpp b/launcher/ui/widgets/VariableSizedImageObject.cpp index 991b4a047..a787f61e6 100644 --- a/launcher/ui/widgets/VariableSizedImageObject.cpp +++ b/launcher/ui/widgets/VariableSizedImageObject.cpp @@ -101,14 +101,7 @@ void VariableSizedImageObject::loadImage(QTextDocument* doc, const QUrl& source, auto full_entry_path = entry->getFullPath(); auto source_url = source; - connect(job, &NetJob::succeeded, this, [this, doc, full_entry_path, source_url, posInDocument] { - qDebug() << "Loaded resource at" << full_entry_path; - - // If we flushed, don't proceed. - if (!m_fetching_images.contains(source_url)) - return; - - QImage image(full_entry_path); + auto loadImage = [this, doc, full_entry_path, source_url, posInDocument](const QImage& image) { doc->addResource(QTextDocument::ImageResource, source_url, image); parseImage(doc, image, posInDocument); @@ -120,6 +113,23 @@ void VariableSizedImageObject::loadImage(QTextDocument* doc, const QUrl& source, doc->setPageSize(size); m_fetching_images.remove(source_url); + }; + connect(job, &NetJob::succeeded, this, [this, full_entry_path, source_url, loadImage] { + qDebug() << "Loaded resource at:" << full_entry_path; + // If we flushed, don't proceed. + if (!m_fetching_images.contains(source_url)) + return; + + QImage image(full_entry_path); + loadImage(image); + }); + connect(job, &NetJob::failed, this, [this, full_entry_path, source_url, loadImage](QString reason) { + qWarning() << "Failed resource at:" << full_entry_path << " because:" << reason; + // If we flushed, don't proceed. + if (!m_fetching_images.contains(source_url)) + return; + + loadImage(QImage()); }); connect(job, &NetJob::finished, job, &NetJob::deleteLater); From 6cf7c0a647e513978e692f02babd0140d909c3af Mon Sep 17 00:00:00 2001 From: Trial97 Date: Sat, 15 Jul 2023 20:17:16 +0300 Subject: [PATCH 05/15] Connected last fail signals Signed-off-by: Trial97 --- .../mod/tasks/GetModDependenciesTask.cpp | 4 +++- launcher/modplatform/ResourceAPI.h | 3 +++ .../modplatform/helpers/NetworkResourceAPI.cpp | 15 ++++++++++++++- launcher/ui/pages/modplatform/ResourceModel.cpp | 8 ++++++++ 4 files changed, 28 insertions(+), 2 deletions(-) diff --git a/launcher/minecraft/mod/tasks/GetModDependenciesTask.cpp b/launcher/minecraft/mod/tasks/GetModDependenciesTask.cpp index f8ecdb33e..09fc66f59 100644 --- a/launcher/minecraft/mod/tasks/GetModDependenciesTask.cpp +++ b/launcher/minecraft/mod/tasks/GetModDependenciesTask.cpp @@ -180,7 +180,9 @@ Task::Ptr GetModDependenciesTask::prepareDependencyTask(const ModPlatform::Depen ResourceAPI::DependencySearchArgs args = { dep, m_version, m_loaderType }; ResourceAPI::DependencySearchCallbacks callbacks; - + callbacks.on_fail = [](QString reason, int) { + qCritical() << tr("A network error occurred. Could not load project dependenies:%1").arg(reason); + }; callbacks.on_succeed = [dep, provider, pDep, level, this](auto& doc, auto& pack) { try { QJsonArray arr; diff --git a/launcher/modplatform/ResourceAPI.h b/launcher/modplatform/ResourceAPI.h index d3277761e..6f92fa108 100644 --- a/launcher/modplatform/ResourceAPI.h +++ b/launcher/modplatform/ResourceAPI.h @@ -99,6 +99,7 @@ class ResourceAPI { }; struct VersionSearchCallbacks { std::function on_succeed; + std::function on_fail; }; struct ProjectInfoArgs { @@ -109,6 +110,7 @@ class ResourceAPI { }; struct ProjectInfoCallbacks { std::function on_succeed; + std::function on_fail; }; struct DependencySearchArgs { @@ -119,6 +121,7 @@ class ResourceAPI { struct DependencySearchCallbacks { std::function on_succeed; + std::function on_fail; }; public: diff --git a/launcher/modplatform/helpers/NetworkResourceAPI.cpp b/launcher/modplatform/helpers/NetworkResourceAPI.cpp index c278f800d..771486005 100644 --- a/launcher/modplatform/helpers/NetworkResourceAPI.cpp +++ b/launcher/modplatform/helpers/NetworkResourceAPI.cpp @@ -70,7 +70,7 @@ Task::Ptr NetworkResourceAPI::getProjectInfo(ProjectInfoArgs&& args, ProjectInfo callbacks.on_succeed(doc, args.pack); }); - + QObject::connect(job.get(), &NetJob::failed, [callbacks](QString reason) { callbacks.on_fail(reason); }); return job; } @@ -99,6 +99,13 @@ Task::Ptr NetworkResourceAPI::getProjectVersions(VersionSearchArgs&& args, Versi callbacks.on_succeed(doc, args.pack); }); + QObject::connect(netJob.get(), &NetJob::failed, [&netJob, callbacks](QString reason) { + int network_error_code = -1; + if (auto* failed_action = netJob->getFailedActions().at(0); failed_action && failed_action->m_reply) + network_error_code = failed_action->m_reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(); + + callbacks.on_fail(reason, network_error_code); + }); return netJob; } @@ -143,6 +150,12 @@ Task::Ptr NetworkResourceAPI::getDependencyVersion(DependencySearchArgs&& args, callbacks.on_succeed(doc, args.dependency); }); + QObject::connect(netJob.get(), &NetJob::failed, [&netJob, callbacks](QString reason) { + int network_error_code = -1; + if (auto* failed_action = netJob->getFailedActions().at(0); failed_action && failed_action->m_reply) + network_error_code = failed_action->m_reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(); + callbacks.on_fail(reason, network_error_code); + }); return netJob; }; diff --git a/launcher/ui/pages/modplatform/ResourceModel.cpp b/launcher/ui/pages/modplatform/ResourceModel.cpp index 49405a02b..170b016ad 100644 --- a/launcher/ui/pages/modplatform/ResourceModel.cpp +++ b/launcher/ui/pages/modplatform/ResourceModel.cpp @@ -178,6 +178,10 @@ void ResourceModel::loadEntry(QModelIndex& entry) return; versionRequestSucceeded(doc, pack, entry); }; + if (!callbacks.on_fail) + callbacks.on_fail = [](QString reason, int) { + QMessageBox::critical(nullptr, tr("Error"), tr("A network error occurred. Could not load project versions:%1").arg(reason)); + }; if (auto job = m_api->getProjectVersions(std::move(args), std::move(callbacks)); job) runInfoJob(job); @@ -194,6 +198,10 @@ void ResourceModel::loadEntry(QModelIndex& entry) return; infoRequestSucceeded(doc, pack, entry); }; + if (!callbacks.on_fail) + callbacks.on_fail = [](QString reason) { + QMessageBox::critical(nullptr, tr("Error"), tr("A network error occurred. Could not load project info:%1").arg(reason)); + }; if (auto job = m_api->getProjectInfo(std::move(args), std::move(callbacks)); job) runInfoJob(job); From f42514a1a3848f750241661138b89fdead1a3370 Mon Sep 17 00:00:00 2001 From: Alexandru Ionut Tripon Date: Sun, 16 Jul 2023 00:38:39 +0300 Subject: [PATCH 06/15] Update launcher/tasks/ConcurrentTask.cpp Co-authored-by: seth Signed-off-by: Alexandru Ionut Tripon --- launcher/tasks/ConcurrentTask.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/launcher/tasks/ConcurrentTask.cpp b/launcher/tasks/ConcurrentTask.cpp index 7bb02e57d..b8d6ae598 100644 --- a/launcher/tasks/ConcurrentTask.cpp +++ b/launcher/tasks/ConcurrentTask.cpp @@ -134,8 +134,7 @@ void ConcurrentTask::startNext() connect(next.get(), &Task::succeeded, this, [this, next]() { subTaskSucceeded(next); }); connect(next.get(), &Task::failed, this, [this, next](QString msg) { subTaskFailed(next, msg); }); - // this should never happen but if it does better to fail the task that being stuck - // most + // this should never happen but if it does, it's better to fail the task than get stuck connect(next.get(), &Task::aborted, this, [this, next] { subTaskFailed(next, "Aborted"); }); connect(next.get(), &Task::status, this, [this, next](QString msg) { subTaskStatus(next, msg); }); From 47dbb0911509804d8df795d2db0581ab73955e87 Mon Sep 17 00:00:00 2001 From: Trial97 Date: Sat, 7 Oct 2023 00:21:40 +0300 Subject: [PATCH 07/15] Improvements to concurrent task Signed-off-by: Trial97 --- launcher/tasks/ConcurrentTask.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/launcher/tasks/ConcurrentTask.cpp b/launcher/tasks/ConcurrentTask.cpp index 6cfd864cc..2ab69d04f 100644 --- a/launcher/tasks/ConcurrentTask.cpp +++ b/launcher/tasks/ConcurrentTask.cpp @@ -152,8 +152,12 @@ void ConcurrentTask::startNext() QMetaObject::invokeMethod(next.get(), &Task::start, Qt::QueuedConnection); // Allow going up the number of concurrent tasks in case of tasks being added in the middle of a running task. - for (int i = 0; i < num_starts; i++) + for (int i = 0; i < num_starts; i++) { + QCoreApplication::processEvents(); + if (m_aborted || m_doing.count() > m_total_max_size) + return; QMetaObject::invokeMethod(this, &ConcurrentTask::startNext, Qt::QueuedConnection); + } } void ConcurrentTask::subTaskSucceeded(Task::Ptr task) From 8dd640819dd961d1195f515713e0d8938763da54 Mon Sep 17 00:00:00 2001 From: Trial97 Date: Mon, 9 Oct 2023 01:50:14 +0300 Subject: [PATCH 08/15] removed processEvents from ConcurrentTask Signed-off-by: Trial97 --- launcher/net/NetJob.cpp | 18 +++-- launcher/net/NetJob.h | 5 +- launcher/tasks/ConcurrentTask.cpp | 96 ++++++++++++-------------- launcher/tasks/ConcurrentTask.h | 9 +-- launcher/tasks/MultipleOptionsTask.cpp | 11 ++- launcher/tasks/MultipleOptionsTask.h | 6 +- launcher/tasks/SequentialTask.cpp | 13 ++-- launcher/tasks/SequentialTask.h | 4 +- tests/Task_test.cpp | 9 ++- 9 files changed, 84 insertions(+), 87 deletions(-) diff --git a/launcher/net/NetJob.cpp b/launcher/net/NetJob.cpp index b99c5acb0..52f6f1517 100644 --- a/launcher/net/NetJob.cpp +++ b/launcher/net/NetJob.cpp @@ -37,6 +37,7 @@ #include "NetJob.h" #include "Application.h" +#include "tasks/ConcurrentTask.h" NetJob::NetJob(QString job_name, shared_qobject_ptr network) : ConcurrentTask(nullptr, job_name, APPLICATION->settings()->get("NumberOfConcurrentDownloads").toInt()), m_network(network) @@ -51,18 +52,15 @@ auto NetJob::addNetAction(NetAction::Ptr action) -> bool return true; } -void NetJob::startNext() +void NetJob::executeNextSubTask() { - if (m_queue.isEmpty() && m_doing.isEmpty()) { - // We're finished, check for failures and retry if we can (up to 3 times) - if (!m_failed.isEmpty() && m_try < 3) { - m_try += 1; - while (!m_failed.isEmpty()) - m_queue.enqueue(m_failed.take(*m_failed.keyBegin())); - } + // We're finished, check for failures and retry if we can (up to 3 times) + if (isRunning() && m_queue.isEmpty() && m_doing.isEmpty() && !m_failed.isEmpty() && m_try < 3) { + m_try += 1; + while (!m_failed.isEmpty()) + m_queue.enqueue(m_failed.take(*m_failed.keyBegin())); } - - ConcurrentTask::startNext(); + ConcurrentTask::executeNextSubTask(); } auto NetJob::size() const -> int diff --git a/launcher/net/NetJob.h b/launcher/net/NetJob.h index 1c4337ec6..f6c005809 100644 --- a/launcher/net/NetJob.h +++ b/launcher/net/NetJob.h @@ -55,8 +55,6 @@ class NetJob : public ConcurrentTask { explicit NetJob(QString job_name, shared_qobject_ptr network); ~NetJob() override = default; - void startNext() override; - auto size() const -> int; auto canAbort() const -> bool override; @@ -69,6 +67,9 @@ class NetJob : public ConcurrentTask { // Qt can't handle auto at the start for some reason? bool abort() override; + protected slots: + void executeNextSubTask() override; + protected: void updateState() override; diff --git a/launcher/tasks/ConcurrentTask.cpp b/launcher/tasks/ConcurrentTask.cpp index 2ab69d04f..97406d58f 100644 --- a/launcher/tasks/ConcurrentTask.cpp +++ b/launcher/tasks/ConcurrentTask.cpp @@ -35,7 +35,6 @@ */ #include "ConcurrentTask.h" -#include #include #include "tasks/Task.h" @@ -51,6 +50,10 @@ ConcurrentTask::~ConcurrentTask() if (task) task->deleteLater(); } + for (auto task : m_done) { + if (task) + task->deleteLater(); + } } auto ConcurrentTask::getStepProgress() const -> TaskStepProgressList @@ -65,15 +68,13 @@ void ConcurrentTask::addTask(Task::Ptr task) void ConcurrentTask::executeTask() { - // Start one task, startNext handles starting the up to the m_total_max_size - // while tracking the number currently being done - QMetaObject::invokeMethod(this, &ConcurrentTask::startNext, Qt::QueuedConnection); + for (auto i = 0; i <= m_total_max_size; i++) + executeNextSubTask(); } bool ConcurrentTask::abort() { m_queue.clear(); - m_aborted = true; if (m_doing.isEmpty()) { // Don't call emitAborted() here, we want to bypass the 'is the task running' check @@ -108,27 +109,33 @@ void ConcurrentTask::clear() m_failed.clear(); m_queue.clear(); - m_aborted = false; - m_progress = 0; m_stepProgress = 0; } -void ConcurrentTask::startNext() +void ConcurrentTask::executeNextSubTask() { - if (m_aborted || m_doing.count() > m_total_max_size) + if (!isRunning()) { return; - - if (m_queue.isEmpty() && m_doing.isEmpty() && !wasSuccessful()) { - emitSucceeded(); + } + if (m_queue.isEmpty()) { + if (m_doing.isEmpty()) { + // if (m_failed.isEmpty()) + emitSucceeded(); + // else + // emitFailed(tr("One or more subtasks failed")); + } + return; + } + if (m_doing.count() > m_total_max_size) { return; } - if (m_queue.isEmpty()) - return; - - Task::Ptr next = m_queue.dequeue(); + startSubTask(m_queue.dequeue()); +} +void ConcurrentTask::startSubTask(Task::Ptr next) +{ connect(next.get(), &Task::succeeded, this, [this, next]() { subTaskSucceeded(next); }); connect(next.get(), &Task::failed, this, [this, next](QString msg) { subTaskFailed(next, msg); }); connect(next.get(), &Task::aborted, this, [this, next] { subTaskFailed(next, "Aborted"); }); @@ -140,59 +147,42 @@ void ConcurrentTask::startNext() connect(next.get(), &Task::progress, this, [this, next](qint64 current, qint64 total) { subTaskProgress(next, current, total); }); m_doing.insert(next.get(), next); - qsizetype num_starts = qMin(m_queue.size(), m_total_max_size - m_doing.size()); + auto task_progress = std::make_shared(next->getUid()); m_task_progress.insert(next->getUid(), task_progress); updateState(); updateStepProgress(*task_progress.get(), Operation::ADDED); - QCoreApplication::processEvents(); - QMetaObject::invokeMethod(next.get(), &Task::start, Qt::QueuedConnection); +} - // Allow going up the number of concurrent tasks in case of tasks being added in the middle of a running task. - for (int i = 0; i < num_starts; i++) { - QCoreApplication::processEvents(); - if (m_aborted || m_doing.count() > m_total_max_size) - return; - QMetaObject::invokeMethod(this, &ConcurrentTask::startNext, Qt::QueuedConnection); - } +void ConcurrentTask::subTaskFinished(Task::Ptr task, TaskStepState state) +{ + m_done.insert(task.get(), task); + (state == TaskStepState::Succeeded ? m_succeeded : m_failed).insert(task.get(), task); + + m_doing.remove(task.get()); + + auto task_progress = m_task_progress.value(task->getUid()); + task_progress->state = state; + + disconnect(task.get(), 0, this, 0); + + emit stepProgress(*task_progress); + updateState(); + updateStepProgress(*task_progress, Operation::REMOVED); + QMetaObject::invokeMethod(this, &ConcurrentTask::executeNextSubTask, Qt::QueuedConnection); } void ConcurrentTask::subTaskSucceeded(Task::Ptr task) { - m_done.insert(task.get(), task); - m_succeeded.insert(task.get(), task); - - m_doing.remove(task.get()); - auto task_progress = m_task_progress.value(task->getUid()); - task_progress->state = TaskStepState::Succeeded; - - disconnect(task.get(), 0, this, 0); - - emit stepProgress(*task_progress); - updateState(); - updateStepProgress(*task_progress, Operation::REMOVED); - startNext(); + subTaskFinished(task, TaskStepState::Succeeded); } void ConcurrentTask::subTaskFailed(Task::Ptr task, [[maybe_unused]] const QString& msg) { - m_done.insert(task.get(), task); - m_failed.insert(task.get(), task); - - m_doing.remove(task.get()); - - auto task_progress = m_task_progress.value(task->getUid()); - task_progress->state = TaskStepState::Failed; - - disconnect(task.get(), 0, this, 0); - - emit stepProgress(*task_progress); - updateState(); - updateStepProgress(*task_progress, Operation::REMOVED); - startNext(); + subTaskFinished(task, TaskStepState::Failed); } void ConcurrentTask::subTaskStatus(Task::Ptr task, const QString& msg) diff --git a/launcher/tasks/ConcurrentTask.h b/launcher/tasks/ConcurrentTask.h index 00b1d48d6..07ea58575 100644 --- a/launcher/tasks/ConcurrentTask.h +++ b/launcher/tasks/ConcurrentTask.h @@ -72,10 +72,11 @@ class ConcurrentTask : public Task { protected slots: void executeTask() override; - virtual void startNext(); + virtual void executeNextSubTask(); void subTaskSucceeded(Task::Ptr); - void subTaskFailed(Task::Ptr, const QString& msg); + virtual void subTaskFailed(Task::Ptr, const QString& msg); + void subTaskFinished(Task::Ptr, TaskStepState); void subTaskStatus(Task::Ptr task, const QString& msg); void subTaskDetails(Task::Ptr task, const QString& msg); void subTaskProgress(Task::Ptr task, qint64 current, qint64 total); @@ -90,6 +91,8 @@ class ConcurrentTask : public Task { virtual void updateState(); + void startSubTask(Task::Ptr task); + protected: QString m_name; QString m_step_status; @@ -107,6 +110,4 @@ class ConcurrentTask : public Task { qint64 m_stepProgress = 0; qint64 m_stepTotalProgress = 100; - - bool m_aborted = false; }; diff --git a/launcher/tasks/MultipleOptionsTask.cpp b/launcher/tasks/MultipleOptionsTask.cpp index 89187a26d..b95fc3d5e 100644 --- a/launcher/tasks/MultipleOptionsTask.cpp +++ b/launcher/tasks/MultipleOptionsTask.cpp @@ -34,11 +34,12 @@ */ #include "MultipleOptionsTask.h" +#include #include -MultipleOptionsTask::MultipleOptionsTask(QObject* parent, const QString& task_name) : SequentialTask(parent, task_name) {} +MultipleOptionsTask::MultipleOptionsTask(QObject* parent, const QString& task_name) : ConcurrentTask(parent, task_name, 1) {} -void MultipleOptionsTask::startNext() +void MultipleOptionsTask::executeNextSubTask() { if (m_done.size() != m_failed.size()) { emitSucceeded(); @@ -51,7 +52,11 @@ void MultipleOptionsTask::startNext() return; } - ConcurrentTask::startNext(); + ConcurrentTask::executeNextSubTask(); + // not sure why this is needed here but tests fail without it + // as the MultipleOptionsTask is yet to be used not sure if + // it works correcly + QCoreApplication::processEvents(); } void MultipleOptionsTask::updateState() diff --git a/launcher/tasks/MultipleOptionsTask.h b/launcher/tasks/MultipleOptionsTask.h index a344343ef..9a88a9999 100644 --- a/launcher/tasks/MultipleOptionsTask.h +++ b/launcher/tasks/MultipleOptionsTask.h @@ -34,18 +34,18 @@ */ #pragma once -#include "SequentialTask.h" +#include "ConcurrentTask.h" /* This task type will attempt to do run each of it's subtasks in sequence, * until one of them succeeds. When that happens, the remaining tasks will not run. * */ -class MultipleOptionsTask : public SequentialTask { +class MultipleOptionsTask : public ConcurrentTask { Q_OBJECT public: explicit MultipleOptionsTask(QObject* parent = nullptr, const QString& task_name = ""); ~MultipleOptionsTask() override = default; private slots: - void startNext() override; + void executeNextSubTask() override; void updateState() override; }; diff --git a/launcher/tasks/SequentialTask.cpp b/launcher/tasks/SequentialTask.cpp index abf7536b9..180430c8c 100644 --- a/launcher/tasks/SequentialTask.cpp +++ b/launcher/tasks/SequentialTask.cpp @@ -36,18 +36,15 @@ #include "SequentialTask.h" #include +#include "tasks/ConcurrentTask.h" SequentialTask::SequentialTask(QObject* parent, QString task_name) : ConcurrentTask(parent, task_name, 1) {} -void SequentialTask::startNext() +void SequentialTask::subTaskFailed(Task::Ptr task, const QString& msg) { - if (m_failed.size() > 0) { - emitFailed(tr("One of the tasks failed!")); - qWarning() << m_failed.constBegin()->get()->failReason(); - return; - } - - ConcurrentTask::startNext(); + emitFailed(msg); + qWarning() << m_failed.constBegin()->get()->failReason(); + ConcurrentTask::subTaskFailed(task, msg); } void SequentialTask::updateState() diff --git a/launcher/tasks/SequentialTask.h b/launcher/tasks/SequentialTask.h index cec3b2be8..a7c101ab4 100644 --- a/launcher/tasks/SequentialTask.h +++ b/launcher/tasks/SequentialTask.h @@ -50,7 +50,9 @@ class SequentialTask : public ConcurrentTask { explicit SequentialTask(QObject* parent = nullptr, QString task_name = ""); ~SequentialTask() override = default; + protected slots: + virtual void subTaskFailed(Task::Ptr, const QString& msg) override; + protected: - void startNext() override; void updateState() override; }; diff --git a/tests/Task_test.cpp b/tests/Task_test.cpp index abc9be905..0740ba0a3 100644 --- a/tests/Task_test.cpp +++ b/tests/Task_test.cpp @@ -37,13 +37,13 @@ class BasicTask_MultiStep : public Task { class BigConcurrentTask : public ConcurrentTask { Q_OBJECT - void startNext() override + void executeNextSubTask() override { // This is here only to help fill the stack a bit more quickly (if there's an issue, of course :^)) // Each tasks thus adds 1024 * 4 bytes to the stack, at the very least. [[maybe_unused]] volatile std::array some_data_on_the_stack{}; - ConcurrentTask::startNext(); + ConcurrentTask::executeNextSubTask(); } }; @@ -71,11 +71,14 @@ class BigConcurrentTaskThread : public QThread { quit(); }); - m_deadline.start(); + if (thread() != QThread::currentThread()) { + QMetaObject::invokeMethod(this, &BigConcurrentTaskThread::start_timer, Qt::QueuedConnection); + } big_task.run(); exec(); } + void start_timer() { m_deadline.start(); } public: bool passed_the_deadline = false; From 1ac9757a86884d3c8299b8bf3749a82c9e7339ff Mon Sep 17 00:00:00 2001 From: Trial97 Date: Thu, 19 Oct 2023 21:51:51 +0300 Subject: [PATCH 09/15] Added back the processEvents Signed-off-by: Trial97 --- launcher/tasks/ConcurrentTask.cpp | 3 +++ launcher/tasks/MultipleOptionsTask.cpp | 5 ----- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/launcher/tasks/ConcurrentTask.cpp b/launcher/tasks/ConcurrentTask.cpp index 77d9ab29d..0183b5124 100644 --- a/launcher/tasks/ConcurrentTask.cpp +++ b/launcher/tasks/ConcurrentTask.cpp @@ -35,6 +35,7 @@ */ #include "ConcurrentTask.h" +#include #include #include "tasks/Task.h" @@ -132,6 +133,8 @@ void ConcurrentTask::executeNextSubTask() } startSubTask(m_queue.dequeue()); + + QCoreApplication::processEvents(); } void ConcurrentTask::startSubTask(Task::Ptr next) diff --git a/launcher/tasks/MultipleOptionsTask.cpp b/launcher/tasks/MultipleOptionsTask.cpp index b95fc3d5e..5afe03964 100644 --- a/launcher/tasks/MultipleOptionsTask.cpp +++ b/launcher/tasks/MultipleOptionsTask.cpp @@ -34,7 +34,6 @@ */ #include "MultipleOptionsTask.h" -#include #include MultipleOptionsTask::MultipleOptionsTask(QObject* parent, const QString& task_name) : ConcurrentTask(parent, task_name, 1) {} @@ -53,10 +52,6 @@ void MultipleOptionsTask::executeNextSubTask() } ConcurrentTask::executeNextSubTask(); - // not sure why this is needed here but tests fail without it - // as the MultipleOptionsTask is yet to be used not sure if - // it works correcly - QCoreApplication::processEvents(); } void MultipleOptionsTask::updateState() From 35a8ab2393e132481e7e0ee2a2eca338918d17f6 Mon Sep 17 00:00:00 2001 From: Trial97 Date: Fri, 20 Oct 2023 10:32:45 +0300 Subject: [PATCH 10/15] Removed processEvents Signed-off-by: Trial97 --- launcher/tasks/ConcurrentTask.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/launcher/tasks/ConcurrentTask.cpp b/launcher/tasks/ConcurrentTask.cpp index 0183b5124..92b13cfa8 100644 --- a/launcher/tasks/ConcurrentTask.cpp +++ b/launcher/tasks/ConcurrentTask.cpp @@ -35,7 +35,6 @@ */ #include "ConcurrentTask.h" -#include #include #include "tasks/Task.h" @@ -69,7 +68,7 @@ void ConcurrentTask::addTask(Task::Ptr task) void ConcurrentTask::executeTask() { - for (auto i = 0; i <= m_total_max_size; i++) + for (auto i = 0; i < m_total_max_size; i++) executeNextSubTask(); } @@ -133,8 +132,6 @@ void ConcurrentTask::executeNextSubTask() } startSubTask(m_queue.dequeue()); - - QCoreApplication::processEvents(); } void ConcurrentTask::startSubTask(Task::Ptr next) From 0ca3873f1c492ad78257eb9ca2c1568a90c353c0 Mon Sep 17 00:00:00 2001 From: Trial97 Date: Sun, 22 Oct 2023 17:48:19 +0300 Subject: [PATCH 11/15] small fix Signed-off-by: Trial97 --- launcher/tasks/ConcurrentTask.cpp | 4 ++-- launcher/tasks/SequentialTask.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/launcher/tasks/ConcurrentTask.cpp b/launcher/tasks/ConcurrentTask.cpp index 92b13cfa8..c042894b4 100644 --- a/launcher/tasks/ConcurrentTask.cpp +++ b/launcher/tasks/ConcurrentTask.cpp @@ -69,7 +69,7 @@ void ConcurrentTask::addTask(Task::Ptr task) void ConcurrentTask::executeTask() { for (auto i = 0; i < m_total_max_size; i++) - executeNextSubTask(); + QMetaObject::invokeMethod(this, &ConcurrentTask::executeNextSubTask, Qt::QueuedConnection); } bool ConcurrentTask::abort() @@ -127,7 +127,7 @@ void ConcurrentTask::executeNextSubTask() } return; } - if (m_doing.count() > m_total_max_size) { + if (m_doing.count() >= m_total_max_size) { return; } diff --git a/launcher/tasks/SequentialTask.cpp b/launcher/tasks/SequentialTask.cpp index 180430c8c..509d91cf7 100644 --- a/launcher/tasks/SequentialTask.cpp +++ b/launcher/tasks/SequentialTask.cpp @@ -43,7 +43,7 @@ SequentialTask::SequentialTask(QObject* parent, QString task_name) : ConcurrentT void SequentialTask::subTaskFailed(Task::Ptr task, const QString& msg) { emitFailed(msg); - qWarning() << m_failed.constBegin()->get()->failReason(); + qWarning() << msg; ConcurrentTask::subTaskFailed(task, msg); } From 2aeb8291767e1832f81f3a4e3f8e3736613175ed Mon Sep 17 00:00:00 2001 From: Trial97 Date: Sun, 22 Oct 2023 19:58:10 +0300 Subject: [PATCH 12/15] removed the concurent task destructor behaivior Signed-off-by: Trial97 --- launcher/modplatform/EnsureMetadataTask.cpp | 1 - launcher/tasks/ConcurrentTask.cpp | 14 +++++--------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/launcher/modplatform/EnsureMetadataTask.cpp b/launcher/modplatform/EnsureMetadataTask.cpp index a9ed30119..ce53ee62d 100644 --- a/launcher/modplatform/EnsureMetadataTask.cpp +++ b/launcher/modplatform/EnsureMetadataTask.cpp @@ -160,7 +160,6 @@ void EnsureMetadataTask::executeTask() if (m_current_task) m_current_task.reset(); }); - connect(version_task.get(), &Task::failed, this, &EnsureMetadataTask::emitFailed); if (m_mods.size() > 1) setStatus(tr("Requesting metadata information from %1...").arg(ProviderCaps.readableName(m_provider))); diff --git a/launcher/tasks/ConcurrentTask.cpp b/launcher/tasks/ConcurrentTask.cpp index c042894b4..6f4a94e7f 100644 --- a/launcher/tasks/ConcurrentTask.cpp +++ b/launcher/tasks/ConcurrentTask.cpp @@ -46,13 +46,9 @@ ConcurrentTask::ConcurrentTask(QObject* parent, QString task_name, int max_concu ConcurrentTask::~ConcurrentTask() { - for (auto task : m_queue) { + for (auto task : m_doing) { if (task) - task->deleteLater(); - } - for (auto task : m_done) { - if (task) - task->deleteLater(); + task->disconnect(this); } } @@ -118,6 +114,9 @@ void ConcurrentTask::executeNextSubTask() if (!isRunning()) { return; } + if (m_doing.count() >= m_total_max_size) { + return; + } if (m_queue.isEmpty()) { if (m_doing.isEmpty()) { if (m_failed.isEmpty()) @@ -127,9 +126,6 @@ void ConcurrentTask::executeNextSubTask() } return; } - if (m_doing.count() >= m_total_max_size) { - return; - } startSubTask(m_queue.dequeue()); } From dba54332fb7785400ffc6d9eac1f3a0b9d3412cd Mon Sep 17 00:00:00 2001 From: Trial97 Date: Thu, 26 Oct 2023 18:31:52 +0300 Subject: [PATCH 13/15] fixed some asan stuf Signed-off-by: Trial97 --- launcher/modplatform/modrinth/ModrinthCheckUpdate.cpp | 2 +- launcher/screenshots/ImgurUpload.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/launcher/modplatform/modrinth/ModrinthCheckUpdate.cpp b/launcher/modplatform/modrinth/ModrinthCheckUpdate.cpp index e56fc3d1d..e78061f27 100644 --- a/launcher/modplatform/modrinth/ModrinthCheckUpdate.cpp +++ b/launcher/modplatform/modrinth/ModrinthCheckUpdate.cpp @@ -72,7 +72,7 @@ void ModrinthCheckUpdate::executeTask() auto response = std::make_shared(); auto job = api.latestVersions(hashes, best_hash_type, m_game_versions, m_loaders, response); - connect(job.get(), &Task::succeeded, this, [this, response, &mappings, best_hash_type, job] { + connect(job.get(), &Task::succeeded, this, [this, response, mappings, best_hash_type, job] { QJsonParseError parse_error{}; QJsonDocument doc = QJsonDocument::fromJson(*response, &parse_error); if (parse_error.error != QJsonParseError::NoError) { diff --git a/launcher/screenshots/ImgurUpload.cpp b/launcher/screenshots/ImgurUpload.cpp index 7ed672eb7..15fb043e4 100644 --- a/launcher/screenshots/ImgurUpload.cpp +++ b/launcher/screenshots/ImgurUpload.cpp @@ -58,14 +58,14 @@ void ImgurUpload::init() QNetworkReply* ImgurUpload::getReply(QNetworkRequest& request) { - auto file = new QFile(m_fileInfo.absoluteFilePath()); + auto file = new QFile(m_fileInfo.absoluteFilePath(), this); if (!file->open(QFile::ReadOnly)) { emitFailed(); return nullptr; } - QHttpMultiPart* multipart = new QHttpMultiPart(QHttpMultiPart::FormDataType); + QHttpMultiPart* multipart = new QHttpMultiPart(QHttpMultiPart::FormDataType, this); file->setParent(multipart); QHttpPart filePart; filePart.setBodyDevice(file); From 2d91bd09d413177c9382e7b101cc4cd8f7d78b45 Mon Sep 17 00:00:00 2001 From: Trial97 Date: Sun, 26 Nov 2023 23:49:56 +0200 Subject: [PATCH 14/15] Fixed crash on abort function not initialized Signed-off-by: Trial97 --- launcher/ui/pages/modplatform/ResourceModel.cpp | 6 ++++++ launcher/ui/pages/modplatform/flame/FlameModel.cpp | 4 ++++ launcher/ui/pages/modplatform/modrinth/ModrinthModel.cpp | 4 ++++ 3 files changed, 14 insertions(+) diff --git a/launcher/ui/pages/modplatform/ResourceModel.cpp b/launcher/ui/pages/modplatform/ResourceModel.cpp index c83e844c3..6c02150bd 100644 --- a/launcher/ui/pages/modplatform/ResourceModel.cpp +++ b/launcher/ui/pages/modplatform/ResourceModel.cpp @@ -234,6 +234,12 @@ void ResourceModel::loadEntry(QModelIndex& entry) return; QMessageBox::critical(nullptr, tr("Error"), tr("A network error occurred. Could not load project info:%1").arg(reason)); }; + if (!callbacks.on_abort) + callbacks.on_abort = [this] { + if (!s_running_models.constFind(this).value()) + return; + qCritical() << tr("The request was abborted for an unknown reason"); + }; if (auto job = m_api->getProjectInfo(std::move(args), std::move(callbacks)); job) runInfoJob(job); diff --git a/launcher/ui/pages/modplatform/flame/FlameModel.cpp b/launcher/ui/pages/modplatform/flame/FlameModel.cpp index 8875a9452..3b266bcef 100644 --- a/launcher/ui/pages/modplatform/flame/FlameModel.cpp +++ b/launcher/ui/pages/modplatform/flame/FlameModel.cpp @@ -170,6 +170,10 @@ void ListModel::performPaginatedSearch() callbacks.on_fail = [this](QString reason) { searchRequestFailed(reason); }; callbacks.on_succeed = [this](auto& doc, auto& pack) { searchRequestForOneSucceeded(doc); }; + callbacks.on_abort = [this] { + qCritical() << "Search task aborted by an unknown reason!"; + searchRequestFailed("Abborted"); + }; static const FlameAPI api; if (auto job = api.getProjectInfo({ projectId }, std::move(callbacks)); job) { jobPtr = job; diff --git a/launcher/ui/pages/modplatform/modrinth/ModrinthModel.cpp b/launcher/ui/pages/modplatform/modrinth/ModrinthModel.cpp index f691a185d..227ed646b 100644 --- a/launcher/ui/pages/modplatform/modrinth/ModrinthModel.cpp +++ b/launcher/ui/pages/modplatform/modrinth/ModrinthModel.cpp @@ -140,6 +140,10 @@ void ModpackListModel::performPaginatedSearch() callbacks.on_fail = [this](QString reason) { searchRequestFailed(reason); }; callbacks.on_succeed = [this](auto& doc, auto& pack) { searchRequestForOneSucceeded(doc); }; + callbacks.on_abort = [this] { + qCritical() << "Search task aborted by an unknown reason!"; + searchRequestFailed("Abborted"); + }; static const ModrinthAPI api; if (auto job = api.getProjectInfo({ projectId }, std::move(callbacks)); job) { jobPtr = job; From 463ff4c6ae9362c70e4f70eea0805ce7b6931145 Mon Sep 17 00:00:00 2001 From: Trial97 Date: Mon, 27 Nov 2023 11:30:07 +0200 Subject: [PATCH 15/15] Removed assert permanantly Signed-off-by: Trial97 --- launcher/minecraft/mod/ResourceFolderModel.h | 1 - 1 file changed, 1 deletion(-) diff --git a/launcher/minecraft/mod/ResourceFolderModel.h b/launcher/minecraft/mod/ResourceFolderModel.h index d764280b6..90e3aac2d 100644 --- a/launcher/minecraft/mod/ResourceFolderModel.h +++ b/launcher/minecraft/mod/ResourceFolderModel.h @@ -306,7 +306,6 @@ void ResourceFolderModel::applyUpdates(QSet& current_set, QSet auto removed_it = m_resources.begin() + removed_index; Q_ASSERT(removed_it != m_resources.end()); - Q_ASSERT(removed_set.contains(removed_it->get()->internal_id())); if ((*removed_it)->isResolving()) { auto ticket = (*removed_it)->resolutionTicket();