8 files changed, 105 insertions, 116 deletions

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<Metadata::ModStruct>(std::move(metadata));
+    m_local_details.metadata = std::make_shared<Metadata::ModStruct>(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::ModStruct>&& metadata)
 {
     if (status() == ModStatus::NoMetadata)
         setStatus(ModStatus::Installed);
 
-    if (m_localDetails) {
-        m_localDetails->metadata = std::make_shared<Metadata::ModStruct>(std::move(metadata));
-    } else {
-        m_temp_metadata = std::make_shared<Metadata::ModStruct>(std::move(metadata));
-    }
+    m_local_details.metadata = metadata;
 }
 
 std::pair<int, bool> 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<Metadata::ModStruct>
 {
-    if (m_localDetails)
-        return m_localDetails->metadata;
-    return m_temp_metadata;
+    return m_local_details.metadata;
 }
 
 auto Mod::metadata() const -> const std::shared_ptr<Metadata::ModStruct>
 {
-    if (m_localDetails)
-        return m_localDetails->metadata;
-    return m_temp_metadata;
+    return m_local_details.metadata;
 }
 
-void Mod::finishResolvingWithDetails(std::shared_ptr<ModDetails> 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::ModStruct> 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<Metadata::ModStruct>;
 
     void setStatus(ModStatus status);
-    void setMetadata(const Metadata::ModStruct& metadata);
+    void setMetadata(std::shared_ptr<Metadata::ModStruct>&& metadata);
+    void setMetadata(const Metadata::ModStruct& metadata) { setMetadata(std::make_shared<Metadata::ModStruct>(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<ModDetails> 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<Metadata::ModStruct> 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<ModDetails> 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::ModStruct> metadata;
+    std::shared_ptr<Metadata::ModStruct> 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<ModDetails> ReadMCModInfo(QByteArray contents)
+ModDetails ReadMCModInfo(QByteArray contents)
 {
-    auto getInfoFromArray = [&](QJsonArray arr)->std::shared_ptr<ModDetails>
+    auto getInfoFromArray = [&](QJsonArray arr) -> ModDetails
     {
         if (!arr.at(0).isObject()) {
-            return nullptr;
+            return {};
         }
-        std::shared_ptr<ModDetails> details = std::make_shared<ModDetails>();
+        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<ModDetails> 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<ModDetails> 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<ModDetails> 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<ModDetails> ReadMCModInfo(QByteArray contents)
             return getInfoFromArray(arrVal.toArray());
         }
     }
-    return nullptr;
+    return {};
 }
 
 // https://github.com/MinecraftForge/Documentation/blob/5ab4ba6cf9abc0ac4c0abd96ad187461aefd72af/docs/gettingstarted/structuring.md
-std::shared_ptr<ModDetails> ReadMCModTOML(QByteArray contents)
+ModDetails ReadMCModTOML(QByteArray contents)
 {
-    std::shared_ptr<ModDetails> details = std::make_shared<ModDetails>();
+    ModDetails details;
 
     char errbuf[200];
     // top-level table
@@ -108,7 +108,7 @@ std::shared_ptr<ModDetails> ReadMCModTOML(QByteArray contents)
 
     if(!tomlData)
     {
-        return nullptr;
+        return {};
     }
 
     // array defined by [[mods]]
@@ -116,7 +116,7 @@ std::shared_ptr<ModDetails> 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<ModDetails> 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<ModDetails> 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<ModDetails> 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<ModDetails> ReadMCModTOML(QByteArray contents)
 }
 
 // https://fabricmc.net/wiki/documentation:fabric_mod_json
-std::shared_ptr<ModDetails> 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<ModDetails> details = std::make_shared<ModDetails>();
+    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<ModDetails> 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<ModDetails> 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<ModDetails> ReadFabricModInfo(QByteArray contents)
 }
 
 // https://github.com/QuiltMC/rfcs/blob/master/specification/0002-quilt.mod.json.md
-std::shared_ptr<ModDetails> 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<ModDetails> details = std::make_shared<ModDetails>();
+    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<ModDetails> ReadForgeInfo(QByteArray contents)
+ModDetails ReadForgeInfo(QByteArray contents)
 {
-    std::shared_ptr<ModDetails> details = std::make_shared<ModDetails>();
+    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<ModDetails> 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<ModDetails> ReadLiteModInfo(QByteArray contents)
+ModDetails ReadLiteModInfo(QByteArray contents)
 {
-    std::shared_ptr<ModDetails> details = std::make_shared<ModDetails>();
+    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<ModDetails> details;
+        ModDetails details;
     };
     using ResultPtr = std::shared_ptr<Result>;
     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;