From 9ce773891229babff6008d19e2e1a5ee0c08f6ea Mon Sep 17 00:00:00 2001 From: Pagwin Date: Wed, 19 Nov 2025 10:43:52 -0500 Subject: [PATCH 1/6] Implemented #4369 Signed-off-by: Pagwin --- launcher/ui/pages/instance/ModFolderPage.cpp | 39 ++++++++++++++++++-- launcher/ui/pages/instance/ModFolderPage.h | 2 + 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/launcher/ui/pages/instance/ModFolderPage.cpp b/launcher/ui/pages/instance/ModFolderPage.cpp index 198f336f9..54932c2ee 100644 --- a/launcher/ui/pages/instance/ModFolderPage.cpp +++ b/launcher/ui/pages/instance/ModFolderPage.cpp @@ -38,6 +38,7 @@ #include "ModFolderPage.h" #include "ui/dialogs/ExportToModListDialog.h" +#include "ui/dialogs/InstallLoaderDialog.h" #include "ui_ExternalResourcesPage.h" #include @@ -65,6 +66,7 @@ #include "tasks/Task.h" #include "ui/dialogs/ProgressDialog.h" +inline void HandleNoModLoader(ModFolderPage* self); ModFolderPage::ModFolderPage(BaseInstance* inst, std::shared_ptr model, QWidget* parent) : ExternalResourcesPage(inst, model, parent), m_model(model) { @@ -145,7 +147,7 @@ void ModFolderPage::downloadMods() auto profile = static_cast(m_instance)->getPackProfile(); if (!profile->getModLoaders().has_value()) { - QMessageBox::critical(this, tr("Error"), tr("Please install a mod loader first!")); + HandleNoModLoader(this); return; } @@ -201,7 +203,7 @@ void ModFolderPage::updateMods(bool includeDeps) auto profile = static_cast(m_instance)->getPackProfile(); if (!profile->getModLoaders().has_value()) { - QMessageBox::critical(this, tr("Error"), tr("Please install a mod loader first!")); + HandleNoModLoader(this); return; } if (APPLICATION->settings()->get("ModMetadataDisabled").toBool()) { @@ -305,7 +307,7 @@ void ModFolderPage::changeModVersion() auto profile = static_cast(m_instance)->getPackProfile(); if (!profile->getModLoaders().has_value()) { - QMessageBox::critical(this, tr("Error"), tr("Please install a mod loader first!")); + HandleNoModLoader(this); return; } if (APPLICATION->settings()->get("ModMetadataDisabled").toBool()) { @@ -385,3 +387,34 @@ bool NilModFolderPage::shouldDisplay() const { return m_model->dir().exists(); } + +// Helper function so this doesn't need to be duplicated 3 times +inline void HandleNoModLoader(ModFolderPage* self) +{ + // QMessageBox::critical(self, tr("Error"), tr("Please install a mod loader first!")); + int resp = QMessageBox::question(self, self->tr("Missing ModLoader"), + self->tr("You need to install a mod loader before installing mods, would you like to do so?"), + QMessageBox::Yes | QMessageBox::No, QMessageBox::Yes); + switch (resp) { + case QMessageBox::Yes: { + // now how do I get the values this all needs + if (self->m_instance->typeName() != "Minecraft") { + // not what we need + return; + } + auto profile = static_cast(self->m_instance)->getPackProfile(); + InstallLoaderDialog dialog(profile, QString(), self); + dialog.exec(); + self->m_container->refreshContainer(); + break; + } + case QMessageBox::No: { + // Nothing happens the dialog is already closing + break; + } + default: { + // Unreachable + break; + } + } +} diff --git a/launcher/ui/pages/instance/ModFolderPage.h b/launcher/ui/pages/instance/ModFolderPage.h index b33992470..6360c9739 100644 --- a/launcher/ui/pages/instance/ModFolderPage.h +++ b/launcher/ui/pages/instance/ModFolderPage.h @@ -45,6 +45,8 @@ class ModFolderPage : public ExternalResourcesPage { Q_OBJECT + friend void HandleNoModLoader(ModFolderPage* self); + public: explicit ModFolderPage(BaseInstance* inst, std::shared_ptr model, QWidget* parent = nullptr); virtual ~ModFolderPage() = default; From 1dae1a210b6db38921d6205a7ee5d1fc7697d21a Mon Sep 17 00:00:00 2001 From: Pagwin Date: Thu, 20 Nov 2025 20:44:41 -0500 Subject: [PATCH 2/6] Made changes to address feedback from code review (#4374) Signed-off-by: Pagwin --- launcher/ui/pages/instance/ModFolderPage.cpp | 41 ++++++++++---------- launcher/ui/pages/instance/ModFolderPage.h | 2 +- 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/launcher/ui/pages/instance/ModFolderPage.cpp b/launcher/ui/pages/instance/ModFolderPage.cpp index 54932c2ee..a619743b3 100644 --- a/launcher/ui/pages/instance/ModFolderPage.cpp +++ b/launcher/ui/pages/instance/ModFolderPage.cpp @@ -66,7 +66,6 @@ #include "tasks/Task.h" #include "ui/dialogs/ProgressDialog.h" -inline void HandleNoModLoader(ModFolderPage* self); ModFolderPage::ModFolderPage(BaseInstance* inst, std::shared_ptr model, QWidget* parent) : ExternalResourcesPage(inst, model, parent), m_model(model) { @@ -147,8 +146,9 @@ void ModFolderPage::downloadMods() auto profile = static_cast(m_instance)->getPackProfile(); if (!profile->getModLoaders().has_value()) { - HandleNoModLoader(this); - return; + if (HandleNoModLoader()) { + return; + } } m_downloadDialog = new ResourceDownload::ModDownloadDialog(this, m_model, m_instance); @@ -203,8 +203,9 @@ void ModFolderPage::updateMods(bool includeDeps) auto profile = static_cast(m_instance)->getPackProfile(); if (!profile->getModLoaders().has_value()) { - HandleNoModLoader(this); - return; + if (HandleNoModLoader()) { + return; + } } if (APPLICATION->settings()->get("ModMetadataDisabled").toBool()) { QMessageBox::critical(this, tr("Error"), tr("Mod updates are unavailable when metadata is disabled!")); @@ -307,8 +308,9 @@ void ModFolderPage::changeModVersion() auto profile = static_cast(m_instance)->getPackProfile(); if (!profile->getModLoaders().has_value()) { - HandleNoModLoader(this); - return; + if (HandleNoModLoader()) { + return; + } } if (APPLICATION->settings()->get("ModMetadataDisabled").toBool()) { QMessageBox::critical(this, tr("Error"), tr("Mod updates are unavailable when metadata is disabled!")); @@ -389,28 +391,25 @@ bool NilModFolderPage::shouldDisplay() const } // Helper function so this doesn't need to be duplicated 3 times -inline void HandleNoModLoader(ModFolderPage* self) +inline bool ModFolderPage::HandleNoModLoader() { - // QMessageBox::critical(self, tr("Error"), tr("Please install a mod loader first!")); - int resp = QMessageBox::question(self, self->tr("Missing ModLoader"), - self->tr("You need to install a mod loader before installing mods, would you like to do so?"), + int resp = QMessageBox::question(this, this->tr("Missing ModLoader"), + this->tr("You need to install a mod loader before installing mods, would you like to do so?"), QMessageBox::Yes | QMessageBox::No, QMessageBox::Yes); switch (resp) { case QMessageBox::Yes: { - // now how do I get the values this all needs - if (self->m_instance->typeName() != "Minecraft") { - // not what we need - return; - } - auto profile = static_cast(self->m_instance)->getPackProfile(); - InstallLoaderDialog dialog(profile, QString(), self); + // Should be safe + auto profile = static_cast(this->m_instance)->getPackProfile(); + InstallLoaderDialog dialog(profile, QString(), this); dialog.exec(); - self->m_container->refreshContainer(); - break; + this->m_container->refreshContainer(); + // returning false so the caller can go and open up the dialog it was originally going to + return false; } case QMessageBox::No: { // Nothing happens the dialog is already closing - break; + // returning true so the caller doesn't go and continue with opening it's dialog without a mod loader + return true; } default: { // Unreachable diff --git a/launcher/ui/pages/instance/ModFolderPage.h b/launcher/ui/pages/instance/ModFolderPage.h index 6360c9739..296f1dd4e 100644 --- a/launcher/ui/pages/instance/ModFolderPage.h +++ b/launcher/ui/pages/instance/ModFolderPage.h @@ -45,7 +45,7 @@ class ModFolderPage : public ExternalResourcesPage { Q_OBJECT - friend void HandleNoModLoader(ModFolderPage* self); + bool HandleNoModLoader(); public: explicit ModFolderPage(BaseInstance* inst, std::shared_ptr model, QWidget* parent = nullptr); From fca8ac40fffac3ef31132f8c1c65474c638af8cf Mon Sep 17 00:00:00 2001 From: Pagwin Date: Fri, 21 Nov 2025 16:19:41 -0500 Subject: [PATCH 3/6] Minor remaining fixes for PR (#4374) - changed case of `handleNoModLoader` to match project style - made default case in switch statement for `handleNoModLoader` return true for safety Signed-off-by: Pagwin --- launcher/ui/pages/instance/ModFolderPage.cpp | 11 ++++++----- launcher/ui/pages/instance/ModFolderPage.h | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/launcher/ui/pages/instance/ModFolderPage.cpp b/launcher/ui/pages/instance/ModFolderPage.cpp index a619743b3..0e56aeff8 100644 --- a/launcher/ui/pages/instance/ModFolderPage.cpp +++ b/launcher/ui/pages/instance/ModFolderPage.cpp @@ -146,7 +146,7 @@ void ModFolderPage::downloadMods() auto profile = static_cast(m_instance)->getPackProfile(); if (!profile->getModLoaders().has_value()) { - if (HandleNoModLoader()) { + if (handleNoModLoader()) { return; } } @@ -203,7 +203,7 @@ void ModFolderPage::updateMods(bool includeDeps) auto profile = static_cast(m_instance)->getPackProfile(); if (!profile->getModLoaders().has_value()) { - if (HandleNoModLoader()) { + if (handleNoModLoader()) { return; } } @@ -308,7 +308,7 @@ void ModFolderPage::changeModVersion() auto profile = static_cast(m_instance)->getPackProfile(); if (!profile->getModLoaders().has_value()) { - if (HandleNoModLoader()) { + if (handleNoModLoader()) { return; } } @@ -391,7 +391,7 @@ bool NilModFolderPage::shouldDisplay() const } // Helper function so this doesn't need to be duplicated 3 times -inline bool ModFolderPage::HandleNoModLoader() +inline bool ModFolderPage::handleNoModLoader() { int resp = QMessageBox::question(this, this->tr("Missing ModLoader"), this->tr("You need to install a mod loader before installing mods, would you like to do so?"), @@ -413,7 +413,8 @@ inline bool ModFolderPage::HandleNoModLoader() } default: { // Unreachable - break; + // returning true as a safety measure + return true; } } } diff --git a/launcher/ui/pages/instance/ModFolderPage.h b/launcher/ui/pages/instance/ModFolderPage.h index 296f1dd4e..aadeecb20 100644 --- a/launcher/ui/pages/instance/ModFolderPage.h +++ b/launcher/ui/pages/instance/ModFolderPage.h @@ -45,7 +45,7 @@ class ModFolderPage : public ExternalResourcesPage { Q_OBJECT - bool HandleNoModLoader(); + inline bool handleNoModLoader(); public: explicit ModFolderPage(BaseInstance* inst, std::shared_ptr model, QWidget* parent = nullptr); From 5e2c8bdcf7d356e829640f7da2bc36c3e40b5823 Mon Sep 17 00:00:00 2001 From: Pagwin Date: Sat, 22 Nov 2025 17:10:22 -0500 Subject: [PATCH 4/6] Fixed Loader Install Cancel crash Fixed a crash which occurred when the mod loader dialog was cancelled after being reached from the no loader dialog Signed-off-by: Pagwin --- launcher/ui/pages/instance/ModFolderPage.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/launcher/ui/pages/instance/ModFolderPage.cpp b/launcher/ui/pages/instance/ModFolderPage.cpp index 0e56aeff8..5e06085a5 100644 --- a/launcher/ui/pages/instance/ModFolderPage.cpp +++ b/launcher/ui/pages/instance/ModFolderPage.cpp @@ -401,10 +401,10 @@ inline bool ModFolderPage::handleNoModLoader() // Should be safe auto profile = static_cast(this->m_instance)->getPackProfile(); InstallLoaderDialog dialog(profile, QString(), this); - dialog.exec(); + bool ret = dialog.exec(); this->m_container->refreshContainer(); // returning false so the caller can go and open up the dialog it was originally going to - return false; + return !ret; } case QMessageBox::No: { // Nothing happens the dialog is already closing From 55a212f3c4456fda43b0f76f9a354043d9531421 Mon Sep 17 00:00:00 2001 From: Pagwin Date: Sun, 23 Nov 2025 14:06:06 -0500 Subject: [PATCH 5/6] Grammar/Spelling nit fix for Missing Mod loader dialog Co-authored-by: Seth Flynn Signed-off-by: Pagwin --- launcher/ui/pages/instance/ModFolderPage.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/launcher/ui/pages/instance/ModFolderPage.cpp b/launcher/ui/pages/instance/ModFolderPage.cpp index 5e06085a5..0ba2bba93 100644 --- a/launcher/ui/pages/instance/ModFolderPage.cpp +++ b/launcher/ui/pages/instance/ModFolderPage.cpp @@ -393,8 +393,8 @@ bool NilModFolderPage::shouldDisplay() const // Helper function so this doesn't need to be duplicated 3 times inline bool ModFolderPage::handleNoModLoader() { - int resp = QMessageBox::question(this, this->tr("Missing ModLoader"), - this->tr("You need to install a mod loader before installing mods, would you like to do so?"), + int resp = QMessageBox::question(this, this->tr("Missing Mod Loader"), + this->tr("You need to install a compatible mod loader before installing mods. Would you like to do so?"), QMessageBox::Yes | QMessageBox::No, QMessageBox::Yes); switch (resp) { case QMessageBox::Yes: { From 331c1de9cd8cc536b24f51cac612fee14f9c047a Mon Sep 17 00:00:00 2001 From: Pagwin Date: Tue, 25 Nov 2025 13:34:31 -0500 Subject: [PATCH 6/6] Edited a comment for clarity Signed-off-by: Pagwin --- launcher/ui/pages/instance/ModFolderPage.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/launcher/ui/pages/instance/ModFolderPage.cpp b/launcher/ui/pages/instance/ModFolderPage.cpp index 0ba2bba93..7aee9e105 100644 --- a/launcher/ui/pages/instance/ModFolderPage.cpp +++ b/launcher/ui/pages/instance/ModFolderPage.cpp @@ -403,7 +403,9 @@ inline bool ModFolderPage::handleNoModLoader() InstallLoaderDialog dialog(profile, QString(), this); bool ret = dialog.exec(); this->m_container->refreshContainer(); - // returning false so the caller can go and open up the dialog it was originally going to + + // returning negation of dialog.exec which'll be true if the install loader dialog got canceled/closed + // and false if the user went through and installed a loader return !ret; } case QMessageBox::No: {