Return an optional<string> from PluginMetadata::GetGroup()

It provides a much clearer representation of the field's value.

include/loot/metadata/plugin_metadata.h

@@ -26,6 +26,7 @@
 
 #include <cstdint>
 #include <list>
+#include <optional>
 #include <regex>
 #include <set>
 #include <string>
@@ -102,16 +103,11 @@ public:
 
   /**
    * Get the plugin's group.
-   * @return The name of the group this plugin belongs to.
+   * @return An optional containing the name of the group this plugin belongs to
+   *         if it was explicitly set, otherwise an optional containing no
+   *         value.
    */
-  LOOT_API std::string GetGroup() const;
-
-  /**
-   * Check if the plugin's group was set explicitly or if the default value was
-   * implied.
-   * @return True if the plugin's group was set explicitly, false otherwise.
-   */
-  LOOT_API bool IsGroupExplicit() const;
+  LOOT_API std::optional<std::string> GetGroup() const;
 
   /**
    * Get the plugins that the plugin must load after.
@@ -287,8 +283,7 @@ public:
 private:
   std::string name_;
   bool enabled_;
-  std::string group_;
-  bool isGroupExplicit_;
+  std::optional<std::string> group_;
   std::set<File> loadAfter_;
   std::set<File> requirements_;
   std::set<File> incompatibilities_;

src/api/metadata/condition_evaluator.cpp

@@ -87,8 +87,8 @@ PluginMetadata ConditionEvaluator::evaluateAll(
   evaluatedMetadata.SetEnabled(pluginMetadata.IsEnabled());
   evaluatedMetadata.SetLocations(pluginMetadata.GetLocations());
 
-  if (pluginMetadata.IsGroupExplicit()) {
-    evaluatedMetadata.SetGroup(pluginMetadata.GetGroup());
+  if (pluginMetadata.GetGroup()) {
+    evaluatedMetadata.SetGroup(pluginMetadata.GetGroup().value());
   }
 
   std::set<File> fileSet;

src/api/metadata/plugin_metadata.cpp

@@ -41,15 +41,11 @@ using std::vector;
 
 namespace loot {
 PluginMetadata::PluginMetadata() :
-    enabled_(true),
-    group_("default"),
-    isGroupExplicit_(false) {}
+    enabled_(true) {}
 
 PluginMetadata::PluginMetadata(const std::string& n) :
     name_(n),
-    enabled_(true),
-    group_("default"),
-    isGroupExplicit_(false) {
+    enabled_(true) {
   // If the name passed ends in '.ghost', that should be trimmed.
   if (boost::iends_with(name_, ".ghost"))
     name_ = name_.substr(0, name_.length() - 6);
@@ -68,9 +64,8 @@ void PluginMetadata::MergeMetadata(const PluginMetadata& plugin) {
   // but if the 'group' value is not explicit, ignore it.
   enabled_ = plugin.IsEnabled();
 
-  if (plugin.IsGroupExplicit()) {
+  if (plugin.GetGroup()) {
     group_ = plugin.GetGroup();
-    isGroupExplicit_ = true;
   }
 
   // Merge the following. If any files in the source already exist in the
@@ -108,11 +103,8 @@ PluginMetadata PluginMetadata::NewMetadata(const PluginMetadata& plugin) const {
 
   PluginMetadata p(*this);
 
-  if (!p.IsGroupExplicit()) {
-    p.group_ = plugin.group_;
-  }
   if (p.group_ == plugin.group_) {
-    p.isGroupExplicit_ = false;
+    p.group_ = std::nullopt;
   }
 
   // Compare this plugin against the given plugin.
@@ -195,11 +187,7 @@ std::string PluginMetadata::GetLowercasedName() const {
 
 bool PluginMetadata::IsEnabled() const { return enabled_; }
 
-std::string PluginMetadata::GetGroup() const { return group_; }
-
-bool PluginMetadata::IsGroupExplicit() const {
-  return isGroupExplicit_;
-}
+std::optional<std::string> PluginMetadata::GetGroup() const { return group_; }
 
 std::set<File> PluginMetadata::GetLoadAfterFiles() const { return loadAfter_; }
 
@@ -240,7 +228,6 @@ void PluginMetadata::SetEnabled(const bool e) { enabled_ = e; }
 
 void PluginMetadata::SetGroup(const std::string& group) {
   group_ = group;
-  isGroupExplicit_ = true;
 }
 
 void PluginMetadata::SetLoadAfterFiles(const std::set<File>& l) {
@@ -275,7 +262,7 @@ void PluginMetadata::SetLocations(const std::set<Location>& locations) {
 }
 
 bool PluginMetadata::HasNameOnly() const {
-  return !IsGroupExplicit() &&
+  return !group_.has_value() &&
          loadAfter_.empty() && requirements_.empty() &&
          incompatibilities_.empty() && messages_.empty() && tags_.empty() &&
          dirtyInfo_.empty() && cleanInfo_.empty() && locations_.empty();

src/api/metadata/yaml/plugin_metadata.h

@@ -55,8 +55,8 @@ struct convert<loot::PluginMetadata> {
     if (!rhs.IsEnabled())
       node["enabled"] = rhs.IsEnabled();
 
-    if (rhs.IsGroupExplicit())
-      node["group"] = rhs.GetGroup();
+    if (rhs.GetGroup())
+      node["group"] = rhs.GetGroup().value();
 
     if (!rhs.GetLoadAfterFiles().empty())
       node["after"] = rhs.GetLoadAfterFiles();
@@ -141,8 +141,8 @@ inline Emitter& operator<<(Emitter& out, const loot::PluginMetadata& rhs) {
     if (!rhs.IsEnabled())
       out << Key << "enabled" << Value << rhs.IsEnabled();
 
-    if (rhs.IsGroupExplicit())
-      out << Key << "group" << Value << YAML::SingleQuoted << rhs.GetGroup();
+    if (rhs.GetGroup())
+      out << Key << "group" << Value << YAML::SingleQuoted << rhs.GetGroup().value();
 
     if (!rhs.GetLoadAfterFiles().empty())
       out << Key << "after" << Value << rhs.GetLoadAfterFiles();

src/api/sorting/plugin_sorter.cpp

@@ -212,9 +212,10 @@ void PluginSorter::AddPluginVertices(Game& game) {
     auto metadata =
         game.GetDatabase()->GetPluginMetadata(plugin->GetName(), true, true).value_or(PluginMetadata(plugin->GetName()));
 
-    auto groupIt = groupPlugins.find(metadata.GetGroup());
+    auto groupName = metadata.GetGroup().value_or(Group().GetName());
+    auto groupIt = groupPlugins.find(groupName);
     if (groupIt == groupPlugins.end()) {
-      groupPlugins.emplace(metadata.GetGroup(),
+      groupPlugins.emplace(groupName,
                            std::vector<std::string>({plugin->GetName()}));
     } else {
       groupIt->second.push_back(plugin->GetName());

src/api/sorting/plugin_sorting_data.cpp

@@ -26,11 +26,14 @@
 
 #include <boost/algorithm/string.hpp>
 
+#include <loot/metadata/group.h>
+
 namespace loot {
 PluginSortingData::PluginSortingData(const Plugin& plugin,
                                      const PluginMetadata&& metadata) :
     plugin_(plugin),
-    PluginMetadata(metadata) {}
+    PluginMetadata(metadata),
+    group_(metadata.GetGroup().value_or(Group().GetName())) {}
 
 std::string PluginSortingData::GetName() const { return plugin_.GetName(); }
 
@@ -54,6 +57,8 @@ bool PluginSortingData::DoFormIDsOverlap(
   return plugin_.DoFormIDsOverlap(plugin.plugin_);
 }
 
+std::string PluginSortingData::GetGroup() const { return group_; }
+
 std::unordered_set<std::string> PluginSortingData::GetAfterGroupPlugins() const {
   return afterGroupPlugins_;
 }

src/api/sorting/plugin_sorting_data.h

@@ -40,16 +40,18 @@ public:
   size_t NumOverrideFormIDs() const;
   bool DoFormIDsOverlap(const PluginSortingData& plugin) const;
 
+  std::string GetGroup() const;
+
   std::unordered_set<std::string> GetAfterGroupPlugins() const;
   void SetAfterGroupPlugins(std::unordered_set<std::string> plugins);
   
   using PluginMetadata::GetLowercasedName;
-  using PluginMetadata::GetGroup;
   using PluginMetadata::GetLoadAfterFiles;
   using PluginMetadata::GetRequirements;
 
 private:
   const Plugin& plugin_;
+  std::string group_;
   std::unordered_set<std::string> afterGroupPlugins_;
 };
 }

src/tests/api/internals/metadata/condition_evaluator_test.h

@@ -129,8 +129,7 @@ TEST_P(ConditionEvaluatorTest, evaluateAllShouldEvaluateAllMetadataConditions) {
   EXPECT_NO_THROW(plugin = evaluator_.evaluateAll(plugin));
 
   std::set<File> expectedFiles({file1});
-  EXPECT_EQ("group1", plugin.GetGroup());
-  EXPECT_TRUE(plugin.IsGroupExplicit());
+  EXPECT_EQ("group1", plugin.GetGroup().value());
   EXPECT_EQ(expectedFiles, plugin.GetLoadAfterFiles());
   EXPECT_EQ(expectedFiles, plugin.GetRequirements());
   EXPECT_EQ(expectedFiles, plugin.GetIncompatibilities());
@@ -144,8 +143,7 @@ TEST_P(ConditionEvaluatorTest, evaluateAllShouldPreserveGroupExplicitness) {
   PluginMetadata plugin(blankEsm);
 
   EXPECT_NO_THROW(plugin = evaluator_.evaluateAll(plugin));
-  EXPECT_EQ("default", plugin.GetGroup());
-  EXPECT_FALSE(plugin.IsGroupExplicit());
+  EXPECT_FALSE(plugin.GetGroup());
 }
 }
 }

src/tests/api/internals/metadata/plugin_metadata_test.h

@@ -51,22 +51,22 @@ INSTANTIATE_TEST_CASE_P(,
 
 TEST_P(
     PluginMetadataTest,
-    defaultConstructorShouldLeaveNameEmptyAndEnableMetadataAndSetGroupToDefault) {
+    defaultConstructorShouldLeaveNameEmptyAndEnableMetadataAndLeaveGroupUnset) {
   PluginMetadata plugin;
 
   EXPECT_TRUE(plugin.GetName().empty());
   EXPECT_TRUE(plugin.IsEnabled());
-  EXPECT_EQ("default", plugin.GetGroup());
+  EXPECT_FALSE(plugin.GetGroup());
 }
 
 TEST_P(
     PluginMetadataTest,
-    stringConstructorShouldSetNameToGivenStringAndEnableMetadataAndSetGroupToDefault) {
+    stringConstructorShouldSetNameToGivenStringAndEnableMetadataAndLeaveGroupUnset) {
   PluginMetadata plugin(blankEsm);
 
   EXPECT_EQ(blankEsm, plugin.GetName());
   EXPECT_TRUE(plugin.IsEnabled());
-  EXPECT_EQ("default", plugin.GetGroup());
+  EXPECT_FALSE(plugin.GetGroup());
 }
 
 TEST_P(PluginMetadataTest,
@@ -299,8 +299,7 @@ TEST_P(PluginMetadataTest, newMetadataShouldUseSourcePluginGroupExplicitlyIfItIs
 
   PluginMetadata newMetadata = plugin1.NewMetadata(plugin2);
 
-  EXPECT_EQ("group1", newMetadata.GetGroup());
-  EXPECT_TRUE(newMetadata.IsGroupExplicit());
+  EXPECT_EQ("group1", newMetadata.GetGroup().value());
 }
 
 TEST_P(PluginMetadataTest,
@@ -312,8 +311,7 @@ TEST_P(PluginMetadataTest,
 
   PluginMetadata newMetadata = plugin1.NewMetadata(plugin2);
 
-  EXPECT_EQ("group2", newMetadata.GetGroup());
-  EXPECT_FALSE(newMetadata.IsGroupExplicit());
+  EXPECT_FALSE(newMetadata.GetGroup());
 }
 
 TEST_P(
@@ -327,8 +325,7 @@ TEST_P(
 
   PluginMetadata newMetadata = plugin1.NewMetadata(plugin2);
 
-  EXPECT_EQ("group1", newMetadata.GetGroup());
-  EXPECT_FALSE(newMetadata.IsGroupExplicit());
+  EXPECT_FALSE(newMetadata.GetGroup());
 }
 
 TEST_P(PluginMetadataTest,