Merge pull request #3281 from PrismLauncher/backport-3255-to-release-9.x

[Backport release-9.x] propagate load component error

launcher/minecraft/MinecraftLoadAndCheck.cpp

@@ -8,7 +8,10 @@ void MinecraftLoadAndCheck::executeTask()
 {
     // add offline metadata load task
     auto components = m_inst->getPackProfile();
-    components->reload(m_netmode);
+    if (auto result = components->reload(m_netmode); !result) {
+        emitFailed(result.error);
+        return;
+    }
     m_task = components->getCurrentTask();
 
     if (!m_task) {

launcher/minecraft/PackProfile.cpp

@@ -173,29 +173,32 @@ static bool savePackProfile(const QString& filename, const ComponentContainer& c
 }
 
 // Read the given file into component containers
-static bool loadPackProfile(PackProfile* parent,
+static PackProfile::Result loadPackProfile(PackProfile* parent,
-                            const QString& filename,
+                                           const QString& filename,
-                            const QString& componentJsonPattern,
+                                           const QString& componentJsonPattern,
-                            ComponentContainer& container)
+                                           ComponentContainer& container)
 {
     QFile componentsFile(filename);
     if (!componentsFile.exists()) {
-        qCWarning(instanceProfileC) << "Components file" << filename << "doesn't exist. This should never happen.";
+        auto message = QObject::tr("Components file %1 doesn't exist. This should never happen.").arg(filename);
-        return false;
+        qCWarning(instanceProfileC) << message;
+        return PackProfile::Result::Error(message);
     }
     if (!componentsFile.open(QFile::ReadOnly)) {
-        qCCritical(instanceProfileC) << "Couldn't open" << componentsFile.fileName() << " for reading:" << componentsFile.errorString();
+        auto message = QObject::tr("Couldn't open %1 for reading: %2").arg(componentsFile.fileName(), componentsFile.errorString());
+        qCCritical(instanceProfileC) << message;
         qCWarning(instanceProfileC) << "Ignoring overridden order";
-        return false;
+        return PackProfile::Result::Error(message);
     }
 
     // and it's valid JSON
     QJsonParseError error;
     QJsonDocument doc = QJsonDocument::fromJson(componentsFile.readAll(), &error);
     if (error.error != QJsonParseError::NoError) {
-        qCCritical(instanceProfileC) << "Couldn't parse" << componentsFile.fileName() << ":" << error.errorString();
+        auto message = QObject::tr("Couldn't parse %1 as json: %2").arg(componentsFile.fileName(), error.errorString());
+        qCCritical(instanceProfileC) << message;
         qCWarning(instanceProfileC) << "Ignoring overridden order";
-        return false;
+        return PackProfile::Result::Error(message);
     }
 
     // and then read it and process it if all above is true.
@@ -212,11 +215,13 @@ static bool loadPackProfile(PackProfile* parent,
             container.append(componentFromJsonV1(parent, componentJsonPattern, comp_obj));
         }
     } catch ([[maybe_unused]] const JSONValidationError& err) {
-        qCCritical(instanceProfileC) << "Couldn't parse" << componentsFile.fileName() << ": bad file format";
+        auto message = QObject::tr("Couldn't parse %1 : bad file format").arg(componentsFile.fileName());
+        qCCritical(instanceProfileC) << message;
+        qCWarning(instanceProfileC) << "error:" << err.what();
         container.clear();
-        return false;
+        return PackProfile::Result::Error(message);
     }
-    return true;
+    return PackProfile::Result::Success();
 }
 
 // END: component file format
@@ -283,44 +288,43 @@ void PackProfile::save_internal()
     d->dirty = false;
 }
 
-bool PackProfile::load()
+PackProfile::Result PackProfile::load()
 {
     auto filename = componentsFilePath();
 
     // load the new component list and swap it with the current one...
     ComponentContainer newComponents;
-    if (!loadPackProfile(this, filename, patchesPattern(), newComponents)) {
+    if (auto result = loadPackProfile(this, filename, patchesPattern(), newComponents); !result) {
         qCritical() << d->m_instance->name() << "|" << "Failed to load the component config";
-        return false;
+        return result;
-    } else {
-        // FIXME: actually use fine-grained updates, not this...
-        beginResetModel();
-        // disconnect all the old components
-        for (auto component : d->components) {
-            disconnect(component.get(), &Component::dataChanged, this, &PackProfile::componentDataChanged);
-        }
-        d->components.clear();
-        d->componentIndex.clear();
-        for (auto component : newComponents) {
-            if (d->componentIndex.contains(component->m_uid)) {
-                qWarning() << d->m_instance->name() << "|" << "Ignoring duplicate component entry" << component->m_uid;
-                continue;
-            }
-            connect(component.get(), &Component::dataChanged, this, &PackProfile::componentDataChanged);
-            d->components.append(component);
-            d->componentIndex[component->m_uid] = component;
-        }
-        endResetModel();
-        d->loaded = true;
-        return true;
     }
+    // FIXME: actually use fine-grained updates, not this...
+    beginResetModel();
+    // disconnect all the old components
+    for (auto component : d->components) {
+        disconnect(component.get(), &Component::dataChanged, this, &PackProfile::componentDataChanged);
+    }
+    d->components.clear();
+    d->componentIndex.clear();
+    for (auto component : newComponents) {
+        if (d->componentIndex.contains(component->m_uid)) {
+            qWarning() << d->m_instance->name() << "|" << "Ignoring duplicate component entry" << component->m_uid;
+            continue;
+        }
+        connect(component.get(), &Component::dataChanged, this, &PackProfile::componentDataChanged);
+        d->components.append(component);
+        d->componentIndex[component->m_uid] = component;
+    }
+    endResetModel();
+    d->loaded = true;
+    return Result::Success();
 }
 
-void PackProfile::reload(Net::Mode netmode)
+PackProfile::Result PackProfile::reload(Net::Mode netmode)
 {
     // Do not reload when the update/resolve task is running. It is in control.
     if (d->m_updateTask) {
-        return;
+        return Result::Success();
     }
 
     // flush any scheduled saves to not lose state
@@ -329,9 +333,11 @@ void PackProfile::reload(Net::Mode netmode)
     // FIXME: differentiate when a reapply is required by propagating state from components
     invalidateLaunchProfile();
 
-    if (load()) {
+    if (auto result = load(); !result) {
-        resolve(netmode);
+        return result;
     }
+    resolve(netmode);
+    return Result::Success();
 }
 
 Task::Ptr PackProfile::getCurrentTask()

launcher/minecraft/PackProfile.h

@@ -62,6 +62,19 @@ class PackProfile : public QAbstractListModel {
    public:
     enum Columns { NameColumn = 0, VersionColumn, NUM_COLUMNS };
 
+    struct Result {
+        bool success;
+        QString error;
+
+        // Implicit conversion to bool
+        operator bool() const { return success; }
+
+        // Factory methods for convenience
+        static Result Success() { return { true, "" }; }
+
+        static Result Error(const QString& errorMessage) { return { false, errorMessage }; }
+    };
+
     explicit PackProfile(MinecraftInstance* instance);
     virtual ~PackProfile();
 
@@ -102,7 +115,7 @@ class PackProfile : public QAbstractListModel {
     bool revertToBase(int index);
 
     /// reload the list, reload all components, resolve dependencies
-    void reload(Net::Mode netmode);
+    Result reload(Net::Mode netmode);
 
     // reload all components, resolve dependencies
     void resolve(Net::Mode netmode);
@@ -169,7 +182,7 @@ class PackProfile : public QAbstractListModel {
     void disableInteraction(bool disable);
 
    private:
-    bool load();
+    Result load();
     bool installJarMods_internal(QStringList filepaths);
     bool installCustomJar_internal(QString filepath);
     bool installAgents_internal(QStringList filepaths);

launcher/ui/pages/instance/VersionPage.cpp

@@ -252,8 +252,11 @@ void VersionPage::updateButtons(int row)
 bool VersionPage::reloadPackProfile()
 {
     try {
-        m_profile->reload(Net::Mode::Online);
+        auto result = m_profile->reload(Net::Mode::Online);
-        return true;
+        if (!result) {
+            QMessageBox::critical(this, tr("Error"), result.error);
+        }
+        return result;
     } catch (const Exception& e) {
         QMessageBox::critical(this, tr("Error"), e.cause());
         return false;