From 3f24c4cfe5afe47eb8ce6f0201b9099e4e50e8ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Mr=C3=A1zek?= Date: Sun, 23 Apr 2017 02:31:13 +0200 Subject: [PATCH] GH-1856 Make MultiMC fail hard when things are missing Things like: * jar mods * valid version files --- api/logic/CMakeLists.txt | 1 - api/logic/MMCZip.cpp | 8 +++ api/logic/minecraft/MinecraftProfile.cpp | 6 +- api/logic/minecraft/MojangVersionFormat.cpp | 1 - api/logic/minecraft/ProfilePatch.cpp | 10 ++- api/logic/minecraft/ProfilePatch.h | 1 - api/logic/minecraft/VersionBuildError.h | 41 ----------- api/logic/minecraft/VersionFile.cpp | 1 - .../minecraft/ftb/FTBProfileStrategy.cpp | 69 ++++++++++--------- api/logic/minecraft/onesix/OneSixInstance.cpp | 1 - .../onesix/OneSixProfileStrategy.cpp | 15 +--- .../minecraft/onesix/OneSixVersionFormat.cpp | 1 - 12 files changed, 57 insertions(+), 98 deletions(-) delete mode 100644 api/logic/minecraft/VersionBuildError.h diff --git a/api/logic/CMakeLists.txt b/api/logic/CMakeLists.txt index 096af6b10..d85d94037 100644 --- a/api/logic/CMakeLists.txt +++ b/api/logic/CMakeLists.txt @@ -263,7 +263,6 @@ set(MINECRAFT_SOURCES minecraft/Library.cpp minecraft/Library.h minecraft/MojangDownloadInfo.h - minecraft/VersionBuildError.h minecraft/VersionFile.cpp minecraft/VersionFile.h minecraft/ProfilePatch.cpp diff --git a/api/logic/MMCZip.cpp b/api/logic/MMCZip.cpp index 0f35bc707..50e352b4f 100644 --- a/api/logic/MMCZip.cpp +++ b/api/logic/MMCZip.cpp @@ -279,6 +279,14 @@ bool MMCZip::createModdedJar(QString sourceJarPath, QString targetJarPath, const qDebug() << "Adding folder " << filename.fileName() << " from " << filename.absoluteFilePath(); } + else + { + // Make sure we do not continue launching when something is missing or undefined... + zipOut.close(); + QFile::remove(targetJarPath); + qCritical() << "Failed to add unknown mod type" << mod.filename().fileName() << "to the jar."; + return false; + } } if (!mergeZipFiles(&zipOut, QFileInfo(sourceJarPath), addedFiles, metaInfFilter)) diff --git a/api/logic/minecraft/MinecraftProfile.cpp b/api/logic/minecraft/MinecraftProfile.cpp index 5f1a9f266..e7e88efdb 100644 --- a/api/logic/minecraft/MinecraftProfile.cpp +++ b/api/logic/minecraft/MinecraftProfile.cpp @@ -177,11 +177,11 @@ bool MinecraftProfile::revertToBase(int index) ProfilePatchPtr MinecraftProfile::versionPatch(const QString &id) { - for (auto file : m_patches) + for (auto patch : m_patches) { - if (file->getID() == id) + if (patch->getID() == id) { - return file; + return patch; } } return nullptr; diff --git a/api/logic/minecraft/MojangVersionFormat.cpp b/api/logic/minecraft/MojangVersionFormat.cpp index ea8dcd4d9..f73c8e49f 100644 --- a/api/logic/minecraft/MojangVersionFormat.cpp +++ b/api/logic/minecraft/MojangVersionFormat.cpp @@ -1,6 +1,5 @@ #include "MojangVersionFormat.h" #include "onesix/OneSixVersionFormat.h" -#include "VersionBuildError.h" #include "MojangDownloadInfo.h" #include "Json.h" diff --git a/api/logic/minecraft/ProfilePatch.cpp b/api/logic/minecraft/ProfilePatch.cpp index 6a90f64b5..3ca2ac68e 100644 --- a/api/logic/minecraft/ProfilePatch.cpp +++ b/api/logic/minecraft/ProfilePatch.cpp @@ -5,6 +5,7 @@ #include "meta/Version.h" #include "VersionFile.h" +#include "minecraft/MinecraftProfile.h" ProfilePatch::ProfilePatch(std::shared_ptr version) :m_metaVersion(version) @@ -23,6 +24,10 @@ void ProfilePatch::applyTo(MinecraftProfile* profile) { vfile->applyTo(profile); } + else + { + profile->applyProblemSeverity(getProblemSeverity()); + } } std::shared_ptr ProfilePatch::getVersionFile() @@ -35,7 +40,10 @@ std::shared_ptr ProfilePatch::getVersionFile() } return m_metaVersion->data(); } - return m_file; + else + { + return m_file; + } } std::shared_ptr ProfilePatch::getVersionList() diff --git a/api/logic/minecraft/ProfilePatch.h b/api/logic/minecraft/ProfilePatch.h index 4e72f9cfa..53f4c5e4d 100644 --- a/api/logic/minecraft/ProfilePatch.h +++ b/api/logic/minecraft/ProfilePatch.h @@ -48,7 +48,6 @@ public: void setRevertible (bool state); void setMovable (bool state); - const QList getProblems() override; ProblemSeverity getProblemSeverity() override; diff --git a/api/logic/minecraft/VersionBuildError.h b/api/logic/minecraft/VersionBuildError.h deleted file mode 100644 index f362c405b..000000000 --- a/api/logic/minecraft/VersionBuildError.h +++ /dev/null @@ -1,41 +0,0 @@ -#include "Exception.h" - -class VersionBuildError : public Exception -{ -public: - explicit VersionBuildError(QString cause) : Exception(cause) {} - virtual ~VersionBuildError() noexcept - { - } -}; - -/** - * the base version file was meant for a newer version of the vanilla launcher than we support - */ -class LauncherVersionError : public VersionBuildError -{ -public: - LauncherVersionError(int actual, int supported) - : VersionBuildError(QObject::tr( - "The base version file of this instance was meant for a newer (%1) " - "version of the vanilla launcher than this version of MultiMC supports (%2).") - .arg(actual) - .arg(supported)) {}; - virtual ~LauncherVersionError() noexcept - { - } -}; - -/** - * files required for the version are not (yet?) present - */ -class VersionIncomplete : public VersionBuildError -{ -public: - VersionIncomplete(QString missingPatch) - : VersionBuildError(QObject::tr("Version is incomplete: missing %1.") - .arg(missingPatch)) {}; - virtual ~VersionIncomplete() noexcept - { - } -}; diff --git a/api/logic/minecraft/VersionFile.cpp b/api/logic/minecraft/VersionFile.cpp index 37df06a14..859895497 100644 --- a/api/logic/minecraft/VersionFile.cpp +++ b/api/logic/minecraft/VersionFile.cpp @@ -8,7 +8,6 @@ #include "minecraft/MinecraftProfile.h" #include "ParseUtils.h" -#include "VersionBuildError.h" #include static bool isMinecraftVersion(const QString &uid) diff --git a/api/logic/minecraft/ftb/FTBProfileStrategy.cpp b/api/logic/minecraft/ftb/FTBProfileStrategy.cpp index b017277de..9fa4e6a16 100644 --- a/api/logic/minecraft/ftb/FTBProfileStrategy.cpp +++ b/api/logic/minecraft/ftb/FTBProfileStrategy.cpp @@ -1,7 +1,6 @@ #include "FTBProfileStrategy.h" #include "OneSixFTBInstance.h" -#include "minecraft/VersionBuildError.h" #include #include @@ -22,41 +21,43 @@ void FTBProfileStrategy::loadDefaultBuiltinPatches() ProfilePatchPtr minecraftPatch; { + std::shared_ptr< VersionFile > file; auto mcJson = m_instance->versionsPath().absoluteFilePath(mcVersion + "/" + mcVersion + ".json"); // load up the base minecraft patch if(QFile::exists(mcJson)) { - auto file = ProfileUtils::parseJsonFile(QFileInfo(mcJson), false); - file->uid = "net.minecraft"; - file->name = QObject::tr("Minecraft (tracked)"); - if(file->version.isEmpty()) - { - file->version = mcVersion; - } + file = ProfileUtils::parseJsonFile(QFileInfo(mcJson), false); for(auto addLib: file->libraries) { addLib->setHint("local"); addLib->setStoragePrefix(nativeInstance->librariesPath().absolutePath()); } - minecraftPatch = std::make_shared(file); - minecraftPatch->setVanilla(true); } else { - throw VersionIncomplete("net.minecraft : " + mcJson); + file = std::make_shared(); + file->addProblem(ProblemSeverity::Error, QObject::tr("Minecraft version is missing in the FTB data.")); } + file->uid = "net.minecraft"; + file->name = QObject::tr("Minecraft (tracked)"); + if(file->version.isEmpty()) + { + file->version = mcVersion; + } + minecraftPatch = std::make_shared(file); + minecraftPatch->setVanilla(true); minecraftPatch->setOrder(-2); } profile->appendPatch(minecraftPatch); ProfilePatchPtr packPatch; { + std::shared_ptr< VersionFile > file; auto mcJson = m_instance->minecraftRoot() + "/pack.json"; - // load up the base minecraft patch + // load up the base minecraft patch, if it's there... if(QFile::exists(mcJson)) { - auto file = ProfileUtils::parseJsonFile(QFileInfo(mcJson), false); - + file = ProfileUtils::parseJsonFile(QFileInfo(mcJson), false); // adapt the loaded file - the FTB patch file format is different than ours. file->minecraftVersion.clear(); file->mainJar = nullptr; @@ -65,33 +66,33 @@ void FTBProfileStrategy::loadDefaultBuiltinPatches() addLib->setHint("local"); addLib->setStoragePrefix(nativeInstance->librariesPath().absolutePath()); } - file->uid = "org.multimc.ftb.pack"; - file->name = QObject::tr("%1 (FTB pack)").arg(m_instance->name()); - if(file->version.isEmpty()) - { - file->version = QObject::tr("Unknown"); - QFile versionFile (FS::PathCombine(m_instance->instanceRoot(), "version")); - if(versionFile.exists()) - { - if(versionFile.open(QIODevice::ReadOnly)) - { - // FIXME: just guessing the encoding/charset here. - auto version = QString::fromUtf8(versionFile.readAll()); - file->version = version; - } - } - } - packPatch = std::make_shared(file); - packPatch->setVanilla(true); } else { - throw VersionIncomplete("org.multimc.ftb.pack : " + mcJson); + file = std::make_shared(); + file->addProblem(ProblemSeverity::Error, QObject::tr("Modpack version file is missing.")); } + file->uid = "org.multimc.ftb.pack"; + file->name = QObject::tr("%1 (FTB pack)").arg(m_instance->name()); + if(file->version.isEmpty()) + { + file->version = QObject::tr("Unknown"); + QFile versionFile (FS::PathCombine(m_instance->instanceRoot(), "version")); + if(versionFile.exists()) + { + if(versionFile.open(QIODevice::ReadOnly)) + { + // FIXME: just guessing the encoding/charset here. + auto version = QString::fromUtf8(versionFile.readAll()); + file->version = version; + } + } + } + packPatch = std::make_shared(file); + packPatch->setVanilla(true); packPatch->setOrder(1); } profile->appendPatch(packPatch); - } void FTBProfileStrategy::load() diff --git a/api/logic/minecraft/onesix/OneSixInstance.cpp b/api/logic/minecraft/onesix/OneSixInstance.cpp index 7e4e97b74..ecfd06474 100644 --- a/api/logic/minecraft/onesix/OneSixInstance.cpp +++ b/api/logic/minecraft/onesix/OneSixInstance.cpp @@ -23,7 +23,6 @@ #include "OneSixProfileStrategy.h" #include "minecraft/MinecraftProfile.h" -#include "minecraft/VersionBuildError.h" #include "minecraft/launch/ModMinecraftJar.h" #include "MMCZip.h" diff --git a/api/logic/minecraft/onesix/OneSixProfileStrategy.cpp b/api/logic/minecraft/onesix/OneSixProfileStrategy.cpp index b4be3356c..0b83c2e10 100644 --- a/api/logic/minecraft/onesix/OneSixProfileStrategy.cpp +++ b/api/logic/minecraft/onesix/OneSixProfileStrategy.cpp @@ -2,7 +2,6 @@ #include "OneSixInstance.h" #include "OneSixVersionFormat.h" -#include "minecraft/VersionBuildError.h" #include "Env.h" #include @@ -107,10 +106,6 @@ void OneSixProfileStrategy::loadDefaultBuiltinPatches() profilePatch = std::make_shared(metaVersion); profilePatch->setVanilla(true); } - if (!profilePatch) - { - throw VersionIncomplete(uid); - } profilePatch->setOrder(order); profile->appendPatch(profilePatch); }; @@ -291,6 +286,7 @@ bool OneSixProfileStrategy::customizePatch(ProfilePatchPtr patch) { return false; } + // FIXME: get rid of this try-catch. try { QSaveFile jsonFile(filename); @@ -311,10 +307,6 @@ bool OneSixProfileStrategy::customizePatch(ProfilePatchPtr patch) } load(); } - catch (VersionIncomplete &error) - { - qDebug() << "Version was incomplete:" << error.cause(); - } catch (Exception &error) { qWarning() << "Version could not be loaded:" << error.cause(); @@ -337,14 +329,11 @@ bool OneSixProfileStrategy::revertPatch(ProfilePatchPtr patch) } // just kill the file and reload bool result = QFile::remove(filename); + // FIXME: get rid of this try-catch. try { load(); } - catch (VersionIncomplete &error) - { - qDebug() << "Version was incomplete:" << error.cause(); - } catch (Exception &error) { qWarning() << "Version could not be loaded:" << error.cause(); diff --git a/api/logic/minecraft/onesix/OneSixVersionFormat.cpp b/api/logic/minecraft/onesix/OneSixVersionFormat.cpp index 632bf2d99..7ebf514f1 100644 --- a/api/logic/minecraft/onesix/OneSixVersionFormat.cpp +++ b/api/logic/minecraft/onesix/OneSixVersionFormat.cpp @@ -1,7 +1,6 @@ #include "OneSixVersionFormat.h" #include #include "minecraft/ParseUtils.h" -#include #include using namespace Json;