From ec62d8e97334d3b5a30cea00858e7035468f3609 Mon Sep 17 00:00:00 2001 From: flow Date: Tue, 9 Aug 2022 01:58:22 -0300 Subject: refactor: move general code from mod model to its own model This aims to continue decoupling other types of resources (e.g. resource packs, shader packs, etc) from mods, so that we don't have to continuously watch our backs for changes to one of them affecting the others. To do so, this creates a more general list model for resources, based on the mods one, that allows you to extend it with functionality for other resources. I had to do some template and preprocessor stuff to get around the QObject limitation of not allowing templated classes, so that's sadge :c On the other hand, I tried cleaning up most general-purpose code in the mod model, and added some documentation, because it looks nice :D Signed-off-by: flow --- launcher/minecraft/mod/tasks/ModFolderLoadTask.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'launcher/minecraft/mod/tasks/ModFolderLoadTask.h') diff --git a/launcher/minecraft/mod/tasks/ModFolderLoadTask.h b/launcher/minecraft/mod/tasks/ModFolderLoadTask.h index 1f2015d2..86f3f67f 100644 --- a/launcher/minecraft/mod/tasks/ModFolderLoadTask.h +++ b/launcher/minecraft/mod/tasks/ModFolderLoadTask.h @@ -42,8 +42,9 @@ #include #include #include "minecraft/mod/Mod.h" +#include "tasks/Task.h" -class ModFolderLoadTask : public QObject, public QRunnable +class ModFolderLoadTask : public Task { Q_OBJECT public: @@ -56,16 +57,15 @@ public: } public: - ModFolderLoadTask(QDir& mods_dir, QDir& index_dir, bool is_indexed, bool clean_orphan = false); - void run(); -signals: - void succeeded(); + ModFolderLoadTask(QDir mods_dir, QDir index_dir, bool is_indexed, bool clean_orphan = false); + + void executeTask() override; private: void getFromMetadata(); private: - QDir& m_mods_dir, m_index_dir; + QDir m_mods_dir, m_index_dir; bool m_is_indexed; bool m_clean_orphan; ResultPtr m_result; -- cgit From e7cf9932a9695417d40d895ac6174186f074f053 Mon Sep 17 00:00:00 2001 From: flow Date: Fri, 12 Aug 2022 17:06:20 -0300 Subject: refactor: simplify Mod structure No need to keep track of pointers left and right. A single one already gives enough headaches! Signed-off-by: flow --- launcher/CMakeLists.txt | 4 +- launcher/minecraft/mod/Mod.cpp | 50 +++------ launcher/minecraft/mod/Mod.h | 14 +-- launcher/minecraft/mod/ModDetails.h | 33 ++++-- launcher/minecraft/mod/ModFolderModel.cpp | 2 +- launcher/minecraft/mod/tasks/LocalModParseTask.cpp | 114 ++++++++++----------- launcher/minecraft/mod/tasks/LocalModParseTask.h | 2 +- launcher/minecraft/mod/tasks/ModFolderLoadTask.cpp | 4 +- launcher/minecraft/mod/tasks/ModFolderLoadTask.h | 2 +- 9 files changed, 107 insertions(+), 118 deletions(-) (limited to 'launcher/minecraft/mod/tasks/ModFolderLoadTask.h') diff --git a/launcher/CMakeLists.txt b/launcher/CMakeLists.txt index badd0eaa..4f5fa2fc 100644 --- a/launcher/CMakeLists.txt +++ b/launcher/CMakeLists.txt @@ -381,8 +381,8 @@ ecm_add_test(minecraft/Library_test.cpp LINK_LIBRARIES Launcher_logic Qt${QT_VER # FIXME: shares data with FileSystem test # TODO: needs testdata -ecm_add_test(minecraft/mod/ModFolderModel_test.cpp LINK_LIBRARIES Launcher_logic Qt${QT_VERSION_MAJOR}::Test - TEST_NAME ModFolderModel) +ecm_add_test(minecraft/mod/ResourceFolderModel_test.cpp LINK_LIBRARIES Launcher_logic Qt${QT_VERSION_MAJOR}::Test + TEST_NAME ResourceFolderModel) ecm_add_test(minecraft/ParseUtils_test.cpp LINK_LIBRARIES Launcher_logic Qt${QT_VERSION_MAJOR}::Test TEST_NAME ParseUtils) diff --git a/launcher/minecraft/mod/Mod.cpp b/launcher/minecraft/mod/Mod.cpp index ed91d999..5e186471 100644 --- a/launcher/minecraft/mod/Mod.cpp +++ b/launcher/minecraft/mod/Mod.cpp @@ -44,13 +44,7 @@ #include "MetadataHandler.h" #include "Version.h" -namespace { - -ModDetails invalidDetails; - -} - -Mod::Mod(const QFileInfo& file) : Resource(file) +Mod::Mod(const QFileInfo& file) : Resource(file), m_local_details() { m_enabled = (file.suffix() != "disabled"); } @@ -59,7 +53,7 @@ Mod::Mod(const QDir& mods_dir, const Metadata::ModStruct& metadata) : Mod(mods_dir.absoluteFilePath(metadata.filename)) { m_name = metadata.name; - m_temp_metadata = std::make_shared(std::move(metadata)); + m_local_details.metadata = std::make_shared(std::move(metadata)); } auto Mod::enable(bool value) -> bool @@ -95,22 +89,14 @@ auto Mod::enable(bool value) -> bool void Mod::setStatus(ModStatus status) { - if (m_localDetails) { - m_localDetails->status = status; - } else { - m_temp_status = status; - } + m_local_details.status = status; } -void Mod::setMetadata(const Metadata::ModStruct& metadata) +void Mod::setMetadata(std::shared_ptr&& metadata) { if (status() == ModStatus::NoMetadata) setStatus(ModStatus::Installed); - if (m_localDetails) { - m_localDetails->metadata = std::make_shared(std::move(metadata)); - } else { - m_temp_metadata = std::make_shared(std::move(metadata)); - } + m_local_details.metadata = metadata; } std::pair Mod::compare(const Resource& other, SortType type) const @@ -176,7 +162,7 @@ auto Mod::destroy(QDir& index_dir, bool preserve_metadata) -> bool auto Mod::details() const -> const ModDetails& { - return m_localDetails ? *m_localDetails : invalidDetails; + return m_local_details; } auto Mod::name() const -> QString @@ -213,35 +199,29 @@ auto Mod::authors() const -> QStringList auto Mod::status() const -> ModStatus { - if (!m_localDetails) - return m_temp_status; return details().status; } auto Mod::metadata() -> std::shared_ptr { - if (m_localDetails) - return m_localDetails->metadata; - return m_temp_metadata; + return m_local_details.metadata; } auto Mod::metadata() const -> const std::shared_ptr { - if (m_localDetails) - return m_localDetails->metadata; - return m_temp_metadata; + return m_local_details.metadata; } -void Mod::finishResolvingWithDetails(std::shared_ptr details) +void Mod::finishResolvingWithDetails(ModDetails&& details) { m_is_resolving = false; m_is_resolved = true; - m_localDetails = details; - setStatus(m_temp_status); + std::shared_ptr metadata = details.metadata; + if (details.status == ModStatus::Unknown) + details.status = m_local_details.status; - if (m_localDetails && m_temp_metadata && m_temp_metadata->isValid()) { - setMetadata(*m_temp_metadata); - m_temp_metadata.reset(); - } + m_local_details = std::move(details); + if (metadata) + setMetadata(std::move(metadata)); } diff --git a/launcher/minecraft/mod/Mod.h b/launcher/minecraft/mod/Mod.h index 49326c83..b9b57058 100644 --- a/launcher/minecraft/mod/Mod.h +++ b/launcher/minecraft/mod/Mod.h @@ -68,7 +68,8 @@ public: auto metadata() const -> const std::shared_ptr; void setStatus(ModStatus status); - void setMetadata(const Metadata::ModStruct& metadata); + void setMetadata(std::shared_ptr&& metadata); + void setMetadata(const Metadata::ModStruct& metadata) { setMetadata(std::make_shared(metadata)); } auto enable(bool value) -> bool; @@ -78,17 +79,10 @@ public: // Delete all the files of this mod auto destroy(QDir& index_dir, bool preserve_metadata = false) -> bool; - void finishResolvingWithDetails(std::shared_ptr details); + void finishResolvingWithDetails(ModDetails&& details); protected: - /* If the mod has metadata, this will be filled in the constructor, and passed to - * the ModDetails when calling finishResolvingWithDetails */ - std::shared_ptr m_temp_metadata; - - /* Set the mod status while it doesn't have local details just yet */ - ModStatus m_temp_status = ModStatus::NoMetadata; - - std::shared_ptr m_localDetails; + ModDetails m_local_details; bool m_enabled = true; }; diff --git a/launcher/minecraft/mod/ModDetails.h b/launcher/minecraft/mod/ModDetails.h index 3e0a7ab0..118b156f 100644 --- a/launcher/minecraft/mod/ModDetails.h +++ b/launcher/minecraft/mod/ModDetails.h @@ -46,34 +46,49 @@ enum class ModStatus { Installed, // Both JAR and Metadata are present NotInstalled, // Only the Metadata is present NoMetadata, // Only the JAR is present + Unknown, // Default status }; struct ModDetails { /* Mod ID as defined in the ModLoader-specific metadata */ - QString mod_id; + QString mod_id = {}; /* Human-readable name */ - QString name; + QString name = {}; /* Human-readable mod version */ - QString version; + QString version = {}; /* Human-readable minecraft version */ - QString mcversion; + QString mcversion = {}; /* URL for mod's home page */ - QString homeurl; + QString homeurl = {}; /* Human-readable description */ - QString description; + QString description = {}; /* List of the author's names */ - QStringList authors; + QStringList authors = {}; /* Installation status of the mod */ - ModStatus status; + ModStatus status = ModStatus::Unknown; /* Metadata information, if any */ - std::shared_ptr metadata; + std::shared_ptr metadata = nullptr; + + ModDetails() = default; + + /** Metadata should be handled manually to properly set the mod status. */ + ModDetails(ModDetails& other) + : mod_id(other.mod_id) + , name(other.name) + , version(other.version) + , mcversion(other.mcversion) + , homeurl(other.homeurl) + , description(other.description) + , authors(other.authors) + , status(other.status) + {} }; diff --git a/launcher/minecraft/mod/ModFolderModel.cpp b/launcher/minecraft/mod/ModFolderModel.cpp index 8bdab16e..9e07dc89 100644 --- a/launcher/minecraft/mod/ModFolderModel.cpp +++ b/launcher/minecraft/mod/ModFolderModel.cpp @@ -296,7 +296,7 @@ void ModFolderModel::onParseSucceeded(int ticket, QString mod_id) auto result = cast_task->result(); if (result && resource) - resource->finishResolvingWithDetails(result->details); + resource->finishResolvingWithDetails(std::move(result->details)); emit dataChanged(index(row), index(row, columnCount(QModelIndex()) - 1)); diff --git a/launcher/minecraft/mod/tasks/LocalModParseTask.cpp b/launcher/minecraft/mod/tasks/LocalModParseTask.cpp index fe3716ce..8a0273c9 100644 --- a/launcher/minecraft/mod/tasks/LocalModParseTask.cpp +++ b/launcher/minecraft/mod/tasks/LocalModParseTask.cpp @@ -20,22 +20,22 @@ namespace { // OLD format: // https://github.com/MinecraftForge/FML/wiki/FML-mod-information-file/5bf6a2d05145ec79387acc0d45c958642fb049fc -std::shared_ptr ReadMCModInfo(QByteArray contents) +ModDetails ReadMCModInfo(QByteArray contents) { - auto getInfoFromArray = [&](QJsonArray arr)->std::shared_ptr + auto getInfoFromArray = [&](QJsonArray arr) -> ModDetails { if (!arr.at(0).isObject()) { - return nullptr; + return {}; } - std::shared_ptr details = std::make_shared(); + ModDetails details; auto firstObj = arr.at(0).toObject(); - details->mod_id = firstObj.value("modid").toString(); + details.mod_id = firstObj.value("modid").toString(); auto name = firstObj.value("name").toString(); // NOTE: ignore stupid example mods copies where the author didn't even bother to change the name if(name != "Example Mod") { - details->name = name; + details.name = name; } - details->version = firstObj.value("version").toString(); + details.version = firstObj.value("version").toString(); auto homeurl = firstObj.value("url").toString().trimmed(); if(!homeurl.isEmpty()) { @@ -45,8 +45,8 @@ std::shared_ptr ReadMCModInfo(QByteArray contents) homeurl.prepend("http://"); } } - details->homeurl = homeurl; - details->description = firstObj.value("description").toString(); + details.homeurl = homeurl; + details.description = firstObj.value("description").toString(); QJsonArray authors = firstObj.value("authorList").toArray(); if (authors.size() == 0) { // FIXME: what is the format of this? is there any? @@ -55,7 +55,7 @@ std::shared_ptr ReadMCModInfo(QByteArray contents) for (auto author: authors) { - details->authors.append(author.toString()); + details.authors.append(author.toString()); } return details; }; @@ -83,7 +83,7 @@ std::shared_ptr ReadMCModInfo(QByteArray contents) { qCritical() << "BAD stuff happened to mod json:"; qCritical() << contents; - return nullptr; + return {}; } auto arrVal = jsonDoc.object().value("modlist"); if(arrVal.isUndefined()) { @@ -94,13 +94,13 @@ std::shared_ptr ReadMCModInfo(QByteArray contents) return getInfoFromArray(arrVal.toArray()); } } - return nullptr; + return {}; } // https://github.com/MinecraftForge/Documentation/blob/5ab4ba6cf9abc0ac4c0abd96ad187461aefd72af/docs/gettingstarted/structuring.md -std::shared_ptr ReadMCModTOML(QByteArray contents) +ModDetails ReadMCModTOML(QByteArray contents) { - std::shared_ptr details = std::make_shared(); + ModDetails details; char errbuf[200]; // top-level table @@ -108,7 +108,7 @@ std::shared_ptr ReadMCModTOML(QByteArray contents) if(!tomlData) { - return nullptr; + return {}; } // array defined by [[mods]] @@ -116,7 +116,7 @@ std::shared_ptr ReadMCModTOML(QByteArray contents) if(!tomlModsArr) { qWarning() << "Corrupted mods.toml? Couldn't find [[mods]] array!"; - return nullptr; + return {}; } // we only really care about the first element, since multiple mods in one file is not supported by us at the moment @@ -124,33 +124,33 @@ std::shared_ptr ReadMCModTOML(QByteArray contents) if(!tomlModsTable0) { qWarning() << "Corrupted mods.toml? [[mods]] didn't have an element at index 0!"; - return nullptr; + return {}; } // mandatory properties - always in [[mods]] toml_datum_t modIdDatum = toml_string_in(tomlModsTable0, "modId"); if(modIdDatum.ok) { - details->mod_id = modIdDatum.u.s; + details.mod_id = modIdDatum.u.s; // library says this is required for strings free(modIdDatum.u.s); } toml_datum_t versionDatum = toml_string_in(tomlModsTable0, "version"); if(versionDatum.ok) { - details->version = versionDatum.u.s; + details.version = versionDatum.u.s; free(versionDatum.u.s); } toml_datum_t displayNameDatum = toml_string_in(tomlModsTable0, "displayName"); if(displayNameDatum.ok) { - details->name = displayNameDatum.u.s; + details.name = displayNameDatum.u.s; free(displayNameDatum.u.s); } toml_datum_t descriptionDatum = toml_string_in(tomlModsTable0, "description"); if(descriptionDatum.ok) { - details->description = descriptionDatum.u.s; + details.description = descriptionDatum.u.s; free(descriptionDatum.u.s); } @@ -173,7 +173,7 @@ std::shared_ptr ReadMCModTOML(QByteArray contents) } if(!authors.isEmpty()) { - details->authors.append(authors); + details.authors.append(authors); } toml_datum_t homeurlDatum = toml_string_in(tomlData, "displayURL"); @@ -200,7 +200,7 @@ std::shared_ptr ReadMCModTOML(QByteArray contents) homeurl.prepend("http://"); } } - details->homeurl = homeurl; + details.homeurl = homeurl; // this seems to be recursive, so it should free everything toml_free(tomlData); @@ -209,20 +209,20 @@ std::shared_ptr ReadMCModTOML(QByteArray contents) } // https://fabricmc.net/wiki/documentation:fabric_mod_json -std::shared_ptr ReadFabricModInfo(QByteArray contents) +ModDetails ReadFabricModInfo(QByteArray contents) { QJsonParseError jsonError; QJsonDocument jsonDoc = QJsonDocument::fromJson(contents, &jsonError); auto object = jsonDoc.object(); auto schemaVersion = object.contains("schemaVersion") ? object.value("schemaVersion").toInt(0) : 0; - std::shared_ptr details = std::make_shared(); + ModDetails details; - details->mod_id = object.value("id").toString(); - details->version = object.value("version").toString(); + details.mod_id = object.value("id").toString(); + details.version = object.value("version").toString(); - details->name = object.contains("name") ? object.value("name").toString() : details->mod_id; - details->description = object.value("description").toString(); + details.name = object.contains("name") ? object.value("name").toString() : details.mod_id; + details.description = object.value("description").toString(); if (schemaVersion >= 1) { @@ -230,10 +230,10 @@ std::shared_ptr ReadFabricModInfo(QByteArray contents) for (auto author: authors) { if(author.isObject()) { - details->authors.append(author.toObject().value("name").toString()); + details.authors.append(author.toObject().value("name").toString()); } else { - details->authors.append(author.toString()); + details.authors.append(author.toString()); } } @@ -243,7 +243,7 @@ std::shared_ptr ReadFabricModInfo(QByteArray contents) if (contact.contains("homepage")) { - details->homeurl = contact.value("homepage").toString(); + details.homeurl = contact.value("homepage").toString(); } } } @@ -251,50 +251,50 @@ std::shared_ptr ReadFabricModInfo(QByteArray contents) } // https://github.com/QuiltMC/rfcs/blob/master/specification/0002-quilt.mod.json.md -std::shared_ptr ReadQuiltModInfo(QByteArray contents) +ModDetails ReadQuiltModInfo(QByteArray contents) { QJsonParseError jsonError; QJsonDocument jsonDoc = QJsonDocument::fromJson(contents, &jsonError); auto object = Json::requireObject(jsonDoc, "quilt.mod.json"); auto schemaVersion = Json::ensureInteger(object.value("schema_version"), 0, "Quilt schema_version"); - std::shared_ptr details = std::make_shared(); + ModDetails details; // https://github.com/QuiltMC/rfcs/blob/be6ba280d785395fefa90a43db48e5bfc1d15eb4/specification/0002-quilt.mod.json.md if (schemaVersion == 1) { auto modInfo = Json::requireObject(object.value("quilt_loader"), "Quilt mod info"); - details->mod_id = Json::requireString(modInfo.value("id"), "Mod ID"); - details->version = Json::requireString(modInfo.value("version"), "Mod version"); + details.mod_id = Json::requireString(modInfo.value("id"), "Mod ID"); + details.version = Json::requireString(modInfo.value("version"), "Mod version"); auto modMetadata = Json::ensureObject(modInfo.value("metadata")); - details->name = Json::ensureString(modMetadata.value("name"), details->mod_id); - details->description = Json::ensureString(modMetadata.value("description")); + details.name = Json::ensureString(modMetadata.value("name"), details.mod_id); + details.description = Json::ensureString(modMetadata.value("description")); auto modContributors = Json::ensureObject(modMetadata.value("contributors")); // We don't really care about the role of a contributor here - details->authors += modContributors.keys(); + details.authors += modContributors.keys(); auto modContact = Json::ensureObject(modMetadata.value("contact")); if (modContact.contains("homepage")) { - details->homeurl = Json::requireString(modContact.value("homepage")); + details.homeurl = Json::requireString(modContact.value("homepage")); } } return details; } -std::shared_ptr ReadForgeInfo(QByteArray contents) +ModDetails ReadForgeInfo(QByteArray contents) { - std::shared_ptr details = std::make_shared(); + ModDetails details; // Read the data - details->name = "Minecraft Forge"; - details->mod_id = "Forge"; - details->homeurl = "http://www.minecraftforge.net/forum/"; + details.name = "Minecraft Forge"; + details.mod_id = "Forge"; + details.homeurl = "http://www.minecraftforge.net/forum/"; INIFile ini; if (!ini.loadFile(contents)) return details; @@ -304,35 +304,35 @@ std::shared_ptr ReadForgeInfo(QByteArray contents) QString revision = ini.get("forge.revision.number", "0").toString(); QString build = ini.get("forge.build.number", "0").toString(); - details->version = major + "." + minor + "." + revision + "." + build; + details.version = major + "." + minor + "." + revision + "." + build; return details; } -std::shared_ptr ReadLiteModInfo(QByteArray contents) +ModDetails ReadLiteModInfo(QByteArray contents) { - std::shared_ptr details = std::make_shared(); + ModDetails details; QJsonParseError jsonError; QJsonDocument jsonDoc = QJsonDocument::fromJson(contents, &jsonError); auto object = jsonDoc.object(); if (object.contains("name")) { - details->mod_id = details->name = object.value("name").toString(); + details.mod_id = details.name = object.value("name").toString(); } if (object.contains("version")) { - details->version = object.value("version").toString(""); + details.version = object.value("version").toString(""); } else { - details->version = object.value("revision").toString(""); + details.version = object.value("revision").toString(""); } - details->mcversion = object.value("mcversion").toString(); + details.mcversion = object.value("mcversion").toString(); auto author = object.value("author").toString(); if(!author.isEmpty()) { - details->authors.append(author); + details.authors.append(author); } - details->description = object.value("description").toString(); - details->homeurl = object.value("url").toString(); + details.description = object.value("description").toString(); + details.homeurl = object.value("url").toString(); return details; } @@ -366,7 +366,7 @@ void LocalModParseTask::processAsZip() file.close(); // to replace ${file.jarVersion} with the actual version, as needed - if (m_result->details && m_result->details->version == "${file.jarVersion}") + if (m_result->details.version == "${file.jarVersion}") { if (zip.setCurrentFile("META-INF/MANIFEST.MF")) { @@ -395,7 +395,7 @@ void LocalModParseTask::processAsZip() manifestVersion = "NONE"; } - m_result->details->version = manifestVersion; + m_result->details.version = manifestVersion; file.close(); } diff --git a/launcher/minecraft/mod/tasks/LocalModParseTask.h b/launcher/minecraft/mod/tasks/LocalModParseTask.h index dbecb449..e0a10218 100644 --- a/launcher/minecraft/mod/tasks/LocalModParseTask.h +++ b/launcher/minecraft/mod/tasks/LocalModParseTask.h @@ -13,7 +13,7 @@ class LocalModParseTask : public Task Q_OBJECT public: struct Result { - std::shared_ptr details; + ModDetails details; }; using ResultPtr = std::shared_ptr; ResultPtr result() const { diff --git a/launcher/minecraft/mod/tasks/ModFolderLoadTask.cpp b/launcher/minecraft/mod/tasks/ModFolderLoadTask.cpp index 44cb1d5f..a56ba8ab 100644 --- a/launcher/minecraft/mod/tasks/ModFolderLoadTask.cpp +++ b/launcher/minecraft/mod/tasks/ModFolderLoadTask.cpp @@ -38,8 +38,8 @@ #include "minecraft/mod/MetadataHandler.h" -ModFolderLoadTask::ModFolderLoadTask(QDir mods_dir, QDir index_dir, bool is_indexed, bool clean_orphan) - : Task(nullptr, false), m_mods_dir(mods_dir), m_index_dir(index_dir), m_is_indexed(is_indexed), m_clean_orphan(clean_orphan), m_result(new Result()) +ModFolderLoadTask::ModFolderLoadTask(QDir mods_dir, QDir index_dir, bool is_indexed, bool clean_orphan, QObject* parent) + : Task(parent, false), m_mods_dir(mods_dir), m_index_dir(index_dir), m_is_indexed(is_indexed), m_clean_orphan(clean_orphan), m_result(new Result()) {} void ModFolderLoadTask::executeTask() diff --git a/launcher/minecraft/mod/tasks/ModFolderLoadTask.h b/launcher/minecraft/mod/tasks/ModFolderLoadTask.h index 86f3f67f..840e95e1 100644 --- a/launcher/minecraft/mod/tasks/ModFolderLoadTask.h +++ b/launcher/minecraft/mod/tasks/ModFolderLoadTask.h @@ -57,7 +57,7 @@ public: } public: - ModFolderLoadTask(QDir mods_dir, QDir index_dir, bool is_indexed, bool clean_orphan = false); + ModFolderLoadTask(QDir mods_dir, QDir index_dir, bool is_indexed, bool clean_orphan = false, QObject* parent = nullptr); void executeTask() override; -- cgit