diff --git a/launcher/minecraft/MinecraftLoadAndCheck.cpp b/launcher/minecraft/MinecraftLoadAndCheck.cpp index b9fb7eb0c..c0a82e61e 100644 --- a/launcher/minecraft/MinecraftLoadAndCheck.cpp +++ b/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) { diff --git a/launcher/minecraft/PackProfile.cpp b/launcher/minecraft/PackProfile.cpp index f1d2473c2..92242d452 100644 --- a/launcher/minecraft/PackProfile.cpp +++ b/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, - const QString& filename, - const QString& componentJsonPattern, - ComponentContainer& container) +static PackProfile::Result loadPackProfile(PackProfile* parent, + const QString& filename, + const QString& componentJsonPattern, + ComponentContainer& container) { QFile componentsFile(filename); if (!componentsFile.exists()) { - qCWarning(instanceProfileC) << "Components file" << filename << "doesn't exist. This should never happen."; - return false; + auto message = QObject::tr("Components file %1 doesn't exist. This should never happen.").arg(filename); + 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; - } 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; + return result; } + // 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()) { - resolve(netmode); + if (auto result = load(); !result) { + return result; } + resolve(netmode); + return Result::Success(); } Task::Ptr PackProfile::getCurrentTask() diff --git a/launcher/minecraft/PackProfile.h b/launcher/minecraft/PackProfile.h index b2de26ea0..d812dfa48 100644 --- a/launcher/minecraft/PackProfile.h +++ b/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); diff --git a/launcher/ui/pages/instance/VersionPage.cpp b/launcher/ui/pages/instance/VersionPage.cpp index ab1c48ed4..975c44de2 100644 --- a/launcher/ui/pages/instance/VersionPage.cpp +++ b/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); - return true; + auto result = m_profile->reload(Net::Mode::Online); + if (!result) { + QMessageBox::critical(this, tr("Error"), result.error); + } + return result; } catch (const Exception& e) { QMessageBox::critical(this, tr("Error"), e.cause()); return false;