From fd6755c93f3f3f7551f9b7c11d1bbbb48c22e210 Mon Sep 17 00:00:00 2001 From: flow Date: Sun, 19 Jun 2022 14:26:15 -0300 Subject: [PATCH] change: mod metadata improvements - Use slug instead of name - Keep temporary status before having local details Signed-off-by: flow --- launcher/ModDownloadTask.cpp | 7 ++-- launcher/minecraft/mod/MetadataHandler.h | 12 +++--- launcher/minecraft/mod/Mod.cpp | 38 ++++++++++++------- launcher/minecraft/mod/Mod.h | 4 +- launcher/minecraft/mod/ModFolderModel.cpp | 30 ++++++++++----- .../mod/tasks/LocalModUpdateTask.cpp | 12 ++++-- .../minecraft/mod/tasks/ModFolderLoadTask.cpp | 2 +- launcher/modplatform/packwiz/Packwiz.cpp | 36 +++++++++--------- launcher/modplatform/packwiz/Packwiz.h | 15 ++++---- launcher/modplatform/packwiz/Packwiz_test.cpp | 7 ++-- 10 files changed, 96 insertions(+), 67 deletions(-) diff --git a/launcher/ModDownloadTask.cpp b/launcher/ModDownloadTask.cpp index 7d35ff69..2b0343f4 100644 --- a/launcher/ModDownloadTask.cpp +++ b/launcher/ModDownloadTask.cpp @@ -47,10 +47,9 @@ void ModDownloadTask::downloadSucceeded() { m_filesNetJob.reset(); auto name = std::get<0>(to_delete); - if (!name.isEmpty()) { - // If they have the same name, we keep the metadata. - // This is a workaround for mods that change names between versions ;c - mods->uninstallMod(std::get<1>(to_delete), name == m_mod.name); + auto filename = std::get<1>(to_delete); + if (!name.isEmpty() && filename != m_mod_version.fileName) { + mods->uninstallMod(filename, true); } } diff --git a/launcher/minecraft/mod/MetadataHandler.h b/launcher/minecraft/mod/MetadataHandler.h index d5f01c42..39723b49 100644 --- a/launcher/minecraft/mod/MetadataHandler.h +++ b/launcher/minecraft/mod/MetadataHandler.h @@ -37,9 +37,9 @@ class Metadata { return Packwiz::V1::createModFormat(index_dir, mod_pack, mod_version); } - static auto create(QDir& index_dir, Mod& internal_mod) -> ModStruct + static auto create(QDir& index_dir, Mod& internal_mod, QString mod_slug) -> ModStruct { - return Packwiz::V1::createModFormat(index_dir, internal_mod); + return Packwiz::V1::createModFormat(index_dir, internal_mod, mod_slug); } static void update(QDir& index_dir, ModStruct& mod) @@ -47,9 +47,9 @@ class Metadata { Packwiz::V1::updateModIndex(index_dir, mod); } - static void remove(QDir& index_dir, QString& mod_name) + static void remove(QDir& index_dir, QString mod_slug) { - Packwiz::V1::deleteModIndex(index_dir, mod_name); + Packwiz::V1::deleteModIndex(index_dir, mod_slug); } static void remove(QDir& index_dir, QVariant& mod_id) @@ -57,9 +57,9 @@ class Metadata { Packwiz::V1::deleteModIndex(index_dir, mod_id); } - static auto get(QDir& index_dir, QString& mod_name) -> ModStruct + static auto get(QDir& index_dir, QString mod_slug) -> ModStruct { - return Packwiz::V1::getIndexForMod(index_dir, mod_name); + return Packwiz::V1::getIndexForMod(index_dir, mod_slug); } static auto get(QDir& index_dir, QVariant& mod_id) -> ModStruct diff --git a/launcher/minecraft/mod/Mod.cpp b/launcher/minecraft/mod/Mod.cpp index bba7b342..7227fc94 100644 --- a/launcher/minecraft/mod/Mod.cpp +++ b/launcher/minecraft/mod/Mod.cpp @@ -147,25 +147,36 @@ void Mod::setStatus(ModStatus status) if (m_localDetails) { m_localDetails->status = status; } else { - m_temp_status = status; + if (!m_temp_status.get()) + m_temp_status.reset(new ModStatus()); + + *m_temp_status = status; } } -void Mod::setMetadata(Metadata::ModStruct* metadata) +void Mod::setMetadata(const Metadata::ModStruct& metadata) { if (status() == ModStatus::NoMetadata) setStatus(ModStatus::Installed); if (m_localDetails) { - m_localDetails->metadata.reset(metadata); + m_localDetails->metadata = std::make_shared(std::move(metadata)); } else { - m_temp_metadata.reset(metadata); + m_temp_metadata = std::make_shared(std::move(metadata)); } } auto Mod::destroy(QDir& index_dir, bool preserve_metadata) -> bool { - if (!preserve_metadata && status() != ModStatus::NoMetadata) - Metadata::remove(index_dir, metadata()->mod_id()); + if (!preserve_metadata) { + qDebug() << QString("Destroying metadata for '%1' on purpose").arg(name()); + + if (metadata()) { + Metadata::remove(index_dir, metadata()->slug); + } else { + auto n = name(); + Metadata::remove(index_dir, n); + } + } m_type = MOD_UNKNOWN; return FS::deletePath(m_file.filePath()); @@ -182,7 +193,7 @@ auto Mod::name() const -> QString if (!d_name.isEmpty()) return d_name; - if (status() != ModStatus::NoMetadata) + if (metadata()) return metadata()->name; return m_name; @@ -211,7 +222,7 @@ auto Mod::authors() const -> QStringList auto Mod::status() const -> ModStatus { if (!m_localDetails) - return m_temp_status; + return m_temp_status ? *m_temp_status : ModStatus::NoMetadata; return details().status; } @@ -235,11 +246,10 @@ void Mod::finishResolvingWithDetails(std::shared_ptr details) m_resolved = true; m_localDetails = details; - if (m_localDetails && m_temp_metadata && m_temp_metadata->isValid()) { - m_localDetails->metadata = m_temp_metadata; - if (status() == ModStatus::NoMetadata) - setStatus(ModStatus::Installed); - } + setStatus(m_temp_status ? *m_temp_status : ModStatus::NoMetadata); - setStatus(m_temp_status); + if (m_localDetails && m_temp_metadata && m_temp_metadata->isValid()) { + setMetadata(*m_temp_metadata); + m_temp_metadata.reset(); + } } diff --git a/launcher/minecraft/mod/Mod.h b/launcher/minecraft/mod/Mod.h index abb8a52d..cbbbd362 100644 --- a/launcher/minecraft/mod/Mod.h +++ b/launcher/minecraft/mod/Mod.h @@ -77,7 +77,7 @@ public: auto metadata() const -> const std::shared_ptr; void setStatus(ModStatus status); - void setMetadata(Metadata::ModStruct* metadata); + void setMetadata(const Metadata::ModStruct& metadata); auto enable(bool value) -> bool; @@ -111,7 +111,7 @@ protected: std::shared_ptr m_temp_metadata; /* Set the mod status while it doesn't have local details just yet */ - ModStatus m_temp_status = ModStatus::NotInstalled; + std::shared_ptr m_temp_status; std::shared_ptr m_localDetails; diff --git a/launcher/minecraft/mod/ModFolderModel.cpp b/launcher/minecraft/mod/ModFolderModel.cpp index adc828c2..d8170067 100644 --- a/launcher/minecraft/mod/ModFolderModel.cpp +++ b/launcher/minecraft/mod/ModFolderModel.cpp @@ -65,15 +65,21 @@ void ModFolderModel::startWatching() update(); + // Watch the mods folder is_watching = m_watcher->addPath(m_dir.absolutePath()); - if (is_watching) - { + if (is_watching) { qDebug() << "Started watching " << m_dir.absolutePath(); - } - else - { + } else { qDebug() << "Failed to start watching " << m_dir.absolutePath(); } + + // Watch the mods index folder + is_watching = m_watcher->addPath(indexDir().absolutePath()); + if (is_watching) { + qDebug() << "Started watching " << indexDir().absolutePath(); + } else { + qDebug() << "Failed to start watching " << indexDir().absolutePath(); + } } void ModFolderModel::stopWatching() @@ -82,14 +88,18 @@ void ModFolderModel::stopWatching() return; is_watching = !m_watcher->removePath(m_dir.absolutePath()); - if (!is_watching) - { + if (!is_watching) { qDebug() << "Stopped watching " << m_dir.absolutePath(); - } - else - { + } else { qDebug() << "Failed to stop watching " << m_dir.absolutePath(); } + + is_watching = !m_watcher->removePath(indexDir().absolutePath()); + if (!is_watching) { + qDebug() << "Stopped watching " << indexDir().absolutePath(); + } else { + qDebug() << "Failed to stop watching " << indexDir().absolutePath(); + } } bool ModFolderModel::update() diff --git a/launcher/minecraft/mod/tasks/LocalModUpdateTask.cpp b/launcher/minecraft/mod/tasks/LocalModUpdateTask.cpp index f0ef795d..4b878918 100644 --- a/launcher/minecraft/mod/tasks/LocalModUpdateTask.cpp +++ b/launcher/minecraft/mod/tasks/LocalModUpdateTask.cpp @@ -47,12 +47,18 @@ void LocalModUpdateTask::executeTask() auto old_metadata = Metadata::get(m_index_dir, m_mod.addonId); if (old_metadata.isValid()) { emit hasOldMod(old_metadata.name, old_metadata.filename); + if (m_mod.slug.isEmpty()) + m_mod.slug = old_metadata.slug; } auto pw_mod = Metadata::create(m_index_dir, m_mod, m_mod_version); - Metadata::update(m_index_dir, pw_mod); - - emitSucceeded(); + if (pw_mod.isValid()) { + Metadata::update(m_index_dir, pw_mod); + emitSucceeded(); + } else { + qCritical() << "Tried to update an invalid mod!"; + emitFailed(tr("Invalid metadata")); + } } auto LocalModUpdateTask::abort() -> bool diff --git a/launcher/minecraft/mod/tasks/ModFolderLoadTask.cpp b/launcher/minecraft/mod/tasks/ModFolderLoadTask.cpp index 276414e4..4ffb626a 100644 --- a/launcher/minecraft/mod/tasks/ModFolderLoadTask.cpp +++ b/launcher/minecraft/mod/tasks/ModFolderLoadTask.cpp @@ -71,7 +71,7 @@ void ModFolderLoadTask::run() auto metadata = m_result->mods[chopped_id].metadata(); if (metadata) { - mod.setMetadata(new Metadata::ModStruct(*metadata)); + mod.setMetadata(*metadata); m_result->mods[mod.internal_id()].setStatus(ModStatus::Installed); m_result->mods.remove(chopped_id); diff --git a/launcher/modplatform/packwiz/Packwiz.cpp b/launcher/modplatform/packwiz/Packwiz.cpp index 8bd66088..c3561093 100644 --- a/launcher/modplatform/packwiz/Packwiz.cpp +++ b/launcher/modplatform/packwiz/Packwiz.cpp @@ -55,11 +55,11 @@ auto getRealIndexName(QDir& index_dir, QString normalized_fname, bool should_fin } // Helpers -static inline auto indexFileName(QString const& mod_name) -> QString +static inline auto indexFileName(QString const& mod_slug) -> QString { - if(mod_name.endsWith(".pw.toml")) - return mod_name; - return QString("%1.pw.toml").arg(mod_name); + if(mod_slug.endsWith(".pw.toml")) + return mod_slug; + return QString("%1.pw.toml").arg(mod_slug); } static ModPlatform::ProviderCapabilities ProviderCaps; @@ -95,6 +95,7 @@ auto V1::createModFormat(QDir& index_dir, ModPlatform::IndexedPack& mod_pack, Mo { Mod mod; + mod.slug = mod_pack.slug; mod.name = mod_pack.name; mod.filename = mod_version.fileName; @@ -116,12 +117,10 @@ auto V1::createModFormat(QDir& index_dir, ModPlatform::IndexedPack& mod_pack, Mo return mod; } -auto V1::createModFormat(QDir& index_dir, ::Mod& internal_mod) -> Mod +auto V1::createModFormat(QDir& index_dir, ::Mod& internal_mod, QString slug) -> Mod { - auto mod_name = internal_mod.name(); - // Try getting metadata if it exists - Mod mod { getIndexForMod(index_dir, mod_name) }; + Mod mod { getIndexForMod(index_dir, slug) }; if(mod.isValid()) return mod; @@ -139,7 +138,7 @@ void V1::updateModIndex(QDir& index_dir, Mod& mod) // Ensure the corresponding mod's info exists, and create it if not - auto normalized_fname = indexFileName(mod.name); + auto normalized_fname = indexFileName(mod.slug); auto real_fname = getRealIndexName(index_dir, normalized_fname); QFile index_file(index_dir.absoluteFilePath(real_fname)); @@ -187,12 +186,13 @@ void V1::updateModIndex(QDir& index_dir, Mod& mod) } } + index_file.flush(); index_file.close(); } -void V1::deleteModIndex(QDir& index_dir, QString& mod_name) +void V1::deleteModIndex(QDir& index_dir, QString& mod_slug) { - auto normalized_fname = indexFileName(mod_name); + auto normalized_fname = indexFileName(mod_slug); auto real_fname = getRealIndexName(index_dir, normalized_fname); if (real_fname.isEmpty()) return; @@ -200,12 +200,12 @@ void V1::deleteModIndex(QDir& index_dir, QString& mod_name) QFile index_file(index_dir.absoluteFilePath(real_fname)); if (!index_file.exists()) { - qWarning() << QString("Tried to delete non-existent mod metadata for %1!").arg(mod_name); + qWarning() << QString("Tried to delete non-existent mod metadata for %1!").arg(mod_slug); return; } if (!index_file.remove()) { - qWarning() << QString("Failed to remove metadata for mod %1!").arg(mod_name); + qWarning() << QString("Failed to remove metadata for mod %1!").arg(mod_slug); } } @@ -221,11 +221,11 @@ void V1::deleteModIndex(QDir& index_dir, QVariant& mod_id) } } -auto V1::getIndexForMod(QDir& index_dir, QString& index_file_name) -> Mod +auto V1::getIndexForMod(QDir& index_dir, QString slug) -> Mod { Mod mod; - auto normalized_fname = indexFileName(index_file_name); + auto normalized_fname = indexFileName(slug); auto real_fname = getRealIndexName(index_dir, normalized_fname, true); if (real_fname.isEmpty()) return {}; @@ -233,7 +233,7 @@ auto V1::getIndexForMod(QDir& index_dir, QString& index_file_name) -> Mod QFile index_file(index_dir.absoluteFilePath(real_fname)); if (!index_file.open(QIODevice::ReadOnly)) { - qWarning() << QString("Failed to open mod metadata for %1").arg(index_file_name); + qWarning() << QString("Failed to open mod metadata for %1").arg(slug); return {}; } @@ -247,11 +247,13 @@ auto V1::getIndexForMod(QDir& index_dir, QString& index_file_name) -> Mod index_file.close(); if (!table) { - qWarning() << QString("Could not open file %1!").arg(indexFileName(index_file_name)); + qWarning() << QString("Could not open file %1!").arg(normalized_fname); qWarning() << "Reason: " << QString(errbuf); return {}; } + mod.slug = slug; + { // Basic info mod.name = stringEntry(table, "name"); mod.filename = stringEntry(table, "filename"); diff --git a/launcher/modplatform/packwiz/Packwiz.h b/launcher/modplatform/packwiz/Packwiz.h index 9d643703..3ec80377 100644 --- a/launcher/modplatform/packwiz/Packwiz.h +++ b/launcher/modplatform/packwiz/Packwiz.h @@ -40,6 +40,7 @@ auto intEntry(toml_table_t* parent, const char* entry_name) -> int; class V1 { public: struct Mod { + QString slug {}; QString name {}; QString filename {}; // FIXME: make side an enum @@ -58,7 +59,7 @@ class V1 { public: // This is a totally heuristic, but should work for now. - auto isValid() const -> bool { return !name.isEmpty() && !project_id.isNull(); } + auto isValid() const -> bool { return !slug.isEmpty() && !project_id.isNull(); } // Different providers can use different names for the same thing // Modrinth-specific @@ -71,9 +72,9 @@ class V1 { * */ static auto createModFormat(QDir& index_dir, ModPlatform::IndexedPack& mod_pack, ModPlatform::IndexedVersion& mod_version) -> Mod; /* Generates the object representing the information in a mod.pw.toml file via - * its common representation in the launcher. + * its common representation in the launcher, plus a necessary slug. * */ - static auto createModFormat(QDir& index_dir, ::Mod& internal_mod) -> Mod; + static auto createModFormat(QDir& index_dir, ::Mod& internal_mod, QString slug) -> Mod; /* Updates the mod index for the provided mod. * This creates a new index if one does not exist already @@ -81,16 +82,16 @@ class V1 { * */ static void updateModIndex(QDir& index_dir, Mod& mod); - /* Deletes the metadata for the mod with the given name. If the metadata doesn't exist, it does nothing. */ - static void deleteModIndex(QDir& index_dir, QString& mod_name); + /* Deletes the metadata for the mod with the given slug. If the metadata doesn't exist, it does nothing. */ + static void deleteModIndex(QDir& index_dir, QString& mod_slug); /* Deletes the metadata for the mod with the given id. If the metadata doesn't exist, it does nothing. */ static void deleteModIndex(QDir& index_dir, QVariant& mod_id); - /* Gets the metadata for a mod with a particular name. + /* Gets the metadata for a mod with a particular file name. * If the mod doesn't have a metadata, it simply returns an empty Mod object. * */ - static auto getIndexForMod(QDir& index_dir, QString& index_file_name) -> Mod; + static auto getIndexForMod(QDir& index_dir, QString slug) -> Mod; /* Gets the metadata for a mod with a particular id. * If the mod doesn't have a metadata, it simply returns an empty Mod object. diff --git a/launcher/modplatform/packwiz/Packwiz_test.cpp b/launcher/modplatform/packwiz/Packwiz_test.cpp index d6251148..aa0c35df 100644 --- a/launcher/modplatform/packwiz/Packwiz_test.cpp +++ b/launcher/modplatform/packwiz/Packwiz_test.cpp @@ -32,10 +32,11 @@ class PackwizTest : public QObject { QString source = QFINDTESTDATA("testdata"); QDir index_dir(source); - QString name_mod("borderless-mining.pw.toml"); - QVERIFY(index_dir.entryList().contains(name_mod)); + QString slug_mod("borderless-mining"); + QString file_name = slug_mod + ".pw.toml"; + QVERIFY(index_dir.entryList().contains(file_name)); - auto metadata = Packwiz::V1::getIndexForMod(index_dir, name_mod); + auto metadata = Packwiz::V1::getIndexForMod(index_dir, slug_mod); QVERIFY(metadata.isValid());