Overhaul plugin metadata types' comparison operators

Change their implementations to check full object equality, with case insensitivity for filename strings.

The filename case insensitivity doesn't extend to filenames in condition strings, as conditions are compared as strings, but the risk of duplicates due to that is much less than the risk of treating non-duplicate objects as duplicates.

docs/metadata/data_structures/cleaning.rst

@@ -42,7 +42,9 @@ This structure holds information on which versions of a plugin are dirty or clea
 Equality
 --------
 
-Two cleaning data structures are equal if the values of their ``crc`` keys are identical.
+Two plugin cleaning data structures are equal if all their fields are equal.
+`util` field equality is case-sensitive. If the `info` field is a string, it
+is treated as a localised content data structure.
 
 Examples
 --------

docs/metadata/data_structures/file.rst

@@ -26,7 +26,8 @@ The scalar form is simply the value of the map form's ``name`` key. Using the sc
 Equality
 --------
 
-Two file data structures are equal if the lowercased values of their ``name`` keys are identical.
+Two file data structures are equal if all their fields are equal. ``name`` field
+equality is case-insensitive, the other fields use case-sensitive equality.
 
 Examples
 --------

docs/metadata/data_structures/localised_content.rst

@@ -27,3 +27,9 @@ The localised content data structure is a key-value string map.
   Spanish               es
   Swedish               sv
   ====================  =====
+
+Equality
+--------
+
+Two localised content data structures are equal if all their fields are equal.
+Field equality is case-sensitive.

docs/metadata/data_structures/location.rst

@@ -22,7 +22,8 @@ The scalar form is simply the value of the map form's ``link`` key. Using the sc
 Equality
 --------
 
-Two location data structures are equal if the values of their ``link`` keys are identical.
+Two location data structures are equal if all their fields are equal. Field
+equality is case-sensitive.
 
 Examples
 --------

docs/metadata/data_structures/message.rst

@@ -57,8 +57,11 @@ If a message's ``content`` value is a string, the message will use the string as
 Equality
 --------
 
-The equality of two message data structures is determined by comparing the values of their ``content`` keys. If a content key is a string, it is treated as a localised content list
-containing a single English-language string. The two message data structures are then equal if their localised content lists are identical.
+Two message data structures are equal if their `type`, `content` and `condition`
+fields are equal, after any `subs` values have been substituted into `content`
+strings. If the `content` field is a string, it is treated as a localised
+content list containing a single English-language string. String equality is
+case sensitive.
 
 Examples
 --------

docs/metadata/data_structures/tag.rst

@@ -22,7 +22,8 @@ The scalar form is simply the value of the map form's ``name`` key. Using the sc
 Equality
 --------
 
-Two tag data structures are equal if the values of their ``name`` keys are identical.
+Two tag data structures are equal if all their fields are equal. String equality
+is case-sensitive.
 
 Examples
 --------

include/loot/metadata/file.h

@@ -58,15 +58,14 @@ public:
   /**
    * A less-than operator implemented with no semantics so that File objects can
    * be stored in sets.
-   * @returns True if this File's name is case-insensitively lexicographically
-   *          less than the given File's name, false otherwise.
+   * @returns True if this File is less than the given File, false otherwise.
    */
   LOOT_API bool operator<(const File& rhs) const;
 
   /**
-   * Check if two File objects are equal by comparing their filenames.
-   * @returns True if the filenames are case-insensitively equal, false
-   *          otherwise.
+   * Check if two File objects are equal by comparing their fields.
+   * @returns True if the filenames are case-insensitively equal and all other
+   *          fields are case-sensitively equal, false otherwise.
    */
   LOOT_API bool operator==(const File& rhs) const;
 

include/loot/metadata/location.h

@@ -54,14 +54,14 @@ public:
   /**
    * A less-than operator implemented with no semantics so that Location objects
    * can be stored in sets.
-   * @returns True if this Location's URL string is lexicographically less than
-   *          the given Location's URL string, false otherwise.
+   * @returns True if this Location is less than the given Location, false 
+   *          otherwise.
    */
   LOOT_API bool operator<(const Location& rhs) const;
 
   /**
-   * Check if two Location objects are equal by comparing their URLs.
-   * @returns True if the URL strings are equal, false otherwise.
+   * Check if two Location objects are equal by comparing their fields.
+   * @returns True if the objects' fields are equal, false otherwise.
    */
   LOOT_API bool operator==(const Location& rhs) const;
 

include/loot/metadata/message.h

@@ -79,14 +79,14 @@ public:
   /**
    * A less-than operator implemented with no semantics so that Message objects
    * can be stored in sets.
-   * @returns Returns true if this Message's content is lexicographically less
-   *          than the given Message's content, and false otherwise.
+   * @returns Returns true if this Message is less than the given Message, and
+   *          false otherwise.
    */
   LOOT_API bool operator<(const Message& rhs) const;
 
   /**
-   * Check if two Message objects are equal by comparing their content.
-   * @returns True if the contents are equal, false otherwise.
+   * Check if two Message objects are equal by comparing their fields.
+   * @returns True if the objects' fields are equal, false otherwise.
    */
   LOOT_API bool operator==(const Message& rhs) const;
 

include/loot/metadata/message_content.h

@@ -73,14 +73,14 @@ public:
   /**
    * A less-than operator implemented with no semantics so that MessageContent
    * objects can be stored in sets.
-   * @returns True if this MessageContent's text is lexicographically less than
-   *          the given MessageContent's text, false otherwise.
+   * @returns True if this MessageContent is less than the given 
+   *          MessageContent, false otherwise.
    */
   LOOT_API bool operator<(const MessageContent& rhs) const;
 
   /**
-   * Check if two MessageContent objects are equal by comparing their texts.
-   * @returns True if the texts are equal, false otherwise.
+   * Check if two MessageContent objects are equal by comparing their fields.
+   * @returns True if the objects' fields are equal, false otherwise.
    */
   LOOT_API bool operator==(const MessageContent& rhs) const;
 

include/loot/metadata/plugin_cleaning_data.h

@@ -84,14 +84,15 @@ public:
   /**
    * A less-than operator implemented with no semantics so that
    * PluginCleaningData objects can be stored in sets.
-   * @returns True if this PluginCleaningData's CRC is less than the given
-   *          PluginCleaningData's CRC, false otherwise.
+   * @returns True if this PluginCleaningData is less than the given
+   *          PluginCleaningData, false otherwise.
    */
   LOOT_API bool operator<(const PluginCleaningData& rhs) const;
 
   /**
-   * Check if two PluginCleaningData objects are equal by comparing their CRCs.
-   * @returns True if the CRCs are equal, false otherwise.
+   * Check if two PluginCleaningData objects are equal by comparing their 
+   * fields.
+   * @returns True if the objects' fields are equal, false otherwise.
    */
   LOOT_API bool operator==(const PluginCleaningData& rhs) const;
 

include/loot/metadata/tag.h

@@ -60,17 +60,13 @@ public:
   /**
    * A less-than operator implemented with no semantics so that Tag objects
    * can be stored in sets.
-   * @returns True if this Tag is suggested for addition and the other is not.
-   *          If both Tags are suggested for addition or both are suggested for
-   *          removal, returns true if this Tag's name is lexicographically less
-   *          than the given Tag's name, false otherwise.
+   * @returns True if this Tag is less than the given Tag, false otherwise.
    */
   LOOT_API bool operator<(const Tag& rhs) const;
 
   /**
    * Check if two Tag objects are equal.
-   * @returns True if both Tags are suggested for addition or both are suggested
-   *          for removal, and the Tag names are equal, false otherwise.
+   * @returns True if the objects' fields are equal, false otherwise.
    */
   LOOT_API bool operator==(const Tag& rhs) const;
 

src/api/metadata/file.cpp

@@ -24,8 +24,8 @@
 
 #include "loot/metadata/file.h"
 
-#include "api/metadata/yaml/file.h"
 #include "api/helpers/text.h"
+#include "api/metadata/yaml/file.h"
 
 namespace loot {
 File::File() {}
@@ -38,11 +38,28 @@ File::File(const std::string& name,
     ConditionalMetadata(condition) {}
 
 bool File::operator<(const File& rhs) const {
+  if (display_ < rhs.display_) {
+    return true;
+  }
+
+  if (rhs.display_ < display_) {
+    return false;
+  }
+
+  if (this->GetCondition() < rhs.GetCondition()) {
+    return true;
+  }
+
+  if (rhs.GetCondition() < this->GetCondition()) {
+    return false;
+  }
+
   return CompareFilenames(name_, rhs.name_) < 0;
 }
 
 bool File::operator==(const File& rhs) const {
-  return CompareFilenames(name_, rhs.name_) == 0;
+  return display_ == rhs.display_ && GetCondition() == rhs.GetCondition() &&
+         CompareFilenames(name_, rhs.name_) == 0;
 }
 
 std::string File::GetName() const { return name_; }

src/api/metadata/location.cpp

@@ -32,11 +32,19 @@ Location::Location(const std::string& url, const std::string& name) :
     name_(name) {}
 
 bool Location::operator<(const Location& rhs) const {
-  return url_ < rhs.url_;
+  if (url_ < rhs.url_) {
+    return true;
+  }
+
+  if (rhs.url_ < url_) {
+    return false;
+  }
+
+  return name_ < rhs.name_;
 }
 
 bool Location::operator==(const Location& rhs) const {
-  return url_ == rhs.url_;
+  return url_ == rhs.url_ && name_ == rhs.name_;
 }
 
 std::string Location::GetURL() const { return url_; }

src/api/metadata/message.cpp

@@ -59,11 +59,28 @@ Message::Message(const MessageType type,
 }
 
 bool Message::operator<(const Message& rhs) const {
+  if (type_ < rhs.type_) {
+    return true;
+  }
+
+  if (rhs.type_ < type_) {
+    return false;
+  }
+
+  if (GetCondition() < rhs.GetCondition()) {
+    return true;
+  }
+
+  if (rhs.GetCondition() < GetCondition()) {
+    return false;
+  }
+
   return content_ < rhs.GetContent();
 }
 
 bool Message::operator==(const Message& rhs) const {
-  return content_ == rhs.GetContent();
+  return type_ == rhs.type_ && GetCondition() == rhs.GetCondition() &&
+         content_ == rhs.GetContent();
 }
 
 MessageType Message::GetType() const { return type_; }

src/api/metadata/message_content.cpp

@@ -41,11 +41,19 @@ std::string MessageContent::GetText() const { return text_; }
 std::string MessageContent::GetLanguage() const { return language_; }
 
 bool MessageContent::operator<(const MessageContent& rhs) const {
-  return text_ < rhs.text_;
+  if (text_ < rhs.text_) {
+    return true;
+  }
+
+  if (rhs.text_ < text_) {
+    return false;
+  }
+
+  return language_ < rhs.language_;
 }
 
 bool MessageContent::operator==(const MessageContent& rhs) const {
-  return text_ == rhs.text_;
+  return text_ == rhs.text_ && language_ == rhs.language_;
 }
 MessageContent MessageContent::Choose(const std::vector<MessageContent> content,
                                       const std::string& language) {

src/api/metadata/plugin_cleaning_data.cpp

@@ -53,11 +53,52 @@ PluginCleaningData::PluginCleaningData(uint32_t crc,
     info_(info) {}
 
 bool PluginCleaningData::operator<(const PluginCleaningData& rhs) const {
-  return crc_ < rhs.GetCRC();
+  if (crc_ < rhs.crc_) {
+    return true;
+  }
+
+  if (rhs.crc_ < crc_) {
+    return false;
+  }
+
+  if (utility_ < rhs.utility_) {
+    return true;
+  }
+
+  if (rhs.utility_ < utility_) {
+    return false;
+  }
+
+  if (itm_ < rhs.itm_) {
+    return true;
+  }
+
+  if (rhs.itm_ < itm_) {
+    return false;
+  }
+
+  if (ref_ < rhs.ref_) {
+    return true;
+  }
+
+  if (rhs.ref_ < ref_) {
+    return false;
+  }
+
+  if (nav_ < rhs.nav_) {
+    return true;
+  }
+
+  if (rhs.nav_ < nav_) {
+    return false;
+  }
+
+  return info_ < rhs.info_;
 }
 
 bool PluginCleaningData::operator==(const PluginCleaningData& rhs) const {
-  return crc_ == rhs.GetCRC();
+  return crc_ == rhs.crc_ && utility_ == rhs.utility_ && info_ == rhs.info_ &&
+         itm_ == rhs.itm_ && ref_ == rhs.ref_ && nav_ == rhs.nav_;
 }
 
 uint32_t PluginCleaningData::GetCRC() const { return crc_; }

src/api/metadata/tag.cpp

@@ -37,15 +37,24 @@ Tag::Tag(const std::string& tag,
     ConditionalMetadata(condition) {}
 
 bool Tag::operator<(const Tag& rhs) const {
-  if (addTag_ != rhs.IsAddition())
-    return addTag_ && !rhs.IsAddition();
-  else
-    return GetName() < rhs.GetName();
+  if (addTag_ != rhs.addTag_) {
+    return addTag_ && !rhs.addTag_;
+  }
+  
+  if (name_ < rhs.name_) {
+    return true;
+  }
+
+  if (rhs.name_ < name_) {
+    return false;
+  }
+  
+  return GetCondition() < rhs.GetCondition();
 }
 
 bool Tag::operator==(const Tag& rhs) const {
-  return addTag_ == rhs.IsAddition() &&
-          GetName() == rhs.GetName();
+  return addTag_ == rhs.addTag_ && name_ == rhs.name_ &&
+         GetCondition() == rhs.GetCondition();
 }
 
 bool Tag::IsAddition() const { return addTag_; }

src/tests/api/interface/database_interface_test.h

@@ -529,7 +529,9 @@ TEST_P(DatabaseInterfaceTest,
   auto messages = db_->GetGeneralMessages();
 
   std::vector<Message> expectedMessages({
-      Message(MessageType::say, generalMasterlistMessage),
+      Message(MessageType::say,
+              generalMasterlistMessage,
+              "file(\"" + missingEsp + "\")"),
       Message(MessageType::say, generalUserlistMessage),
   });
   EXPECT_EQ(expectedMessages, messages);
@@ -577,12 +579,15 @@ TEST_P(DatabaseInterfaceTest,
 
   std::set<Tag> expectedTags({
       Tag("Actors.ACBS"),
+      Tag("Actors.ACBS", true, "file(\"" + missingEsp + "\")"),
       Tag("Actors.AIData"),
       Tag("C.Water", false),
   });
   EXPECT_EQ(expectedTags, metadata.GetTags());
-  EXPECT_EQ("file(\"" + missingEsp +
-            "\")" , metadata.GetTags().find(Tag("Actors.ACBS"))->GetCondition());
+  EXPECT_EQ("file(\"" + missingEsp + "\")",
+            metadata.GetTags()
+                .find(Tag("Actors.ACBS", true, "file(\"" + missingEsp + "\")"))
+                ->GetCondition());
 }
 
 TEST_P(
@@ -739,7 +744,8 @@ TEST_P(DatabaseInterfaceTest,
   auto messages = db_->GetGeneralMessages();
 
   std::vector<Message> expectedMessages({
-      Message(MessageType::say, generalMasterlistMessage),
+      Message(MessageType::say,
+              generalMasterlistMessage, "file(\"" + missingEsp + "\")"),
       Message(MessageType::say, generalUserlistMessage),
   });
   EXPECT_EQ(expectedMessages, messages);
@@ -786,7 +792,9 @@ TEST_P(
   auto messages = db_->GetGeneralMessages();
 
   std::vector<Message> expectedMessages({
-      Message(MessageType::say, generalMasterlistMessage),
+      Message(MessageType::say,
+              generalMasterlistMessage,
+              "file(\"" + missingEsp + "\")"),
   });
   EXPECT_EQ(expectedMessages, messages);
 

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

@@ -49,24 +49,63 @@ TEST(File, stringsConstructorShouldStoreGivenStrings) {
   EXPECT_EQ("condition", file.GetCondition());
 }
 
-TEST(File, filesWithCaseInsensitiveEqualNameStringsShouldBeEqual) {
-  File file1("name", "display1", "condition1");
-  File file2("Name", "display2", "condition2");
+TEST(
+    File,
+    equalityShouldBeCaseInsensitiveOnNameAndDisplay) {
+  File file1("name", "display", "condition");
+  File file2("name", "display", "condition");
 
   EXPECT_TRUE(file1 == file2);
-}
 
-TEST(File, filesWithDifferentNamesShouldBeUnequal) {
-  File file1("name1");
-  File file2("name2");
+  file1 = File("name", "display", "condition");
+  file2 = File("Name", "display", "condition");
+
+  EXPECT_TRUE(file1 == file2);
+
+  file1 = File("name1", "display", "condition");
+  file2 = File("name2", "display", "condition");
 
   EXPECT_FALSE(file1 == file2);
 }
 
-TEST(File,
-     lessThanOperatorShouldUseCaseInsensitiveLexicographicalNameComparison) {
-  File file1("name", "display1", "condition1");
-  File file2("Name", "display2", "condition2");
+TEST(File, equalityShouldBeCaseSensitiveOnDisplayAndCondition) {
+  File file1("name", "display", "condition");
+  File file2("name", "display", "condition");
+
+  EXPECT_TRUE(file1 == file2);
+
+  file1 = File("name", "display", "condition");
+  file2 = File("name", "Display", "condition");
+
+  EXPECT_FALSE(file1 == file2);
+
+  file1 = File("name", "display", "condition");
+  file2 = File("name", "display", "Condition");
+
+  EXPECT_FALSE(file1 == file2);
+
+  file1 = File("name", "display1", "condition");
+  file2 = File("name", "display2", "condition");
+
+  EXPECT_FALSE(file1 == file2);
+
+  file1 = File("name", "display", "condition1");
+  file2 = File("name", "display", "condition2");
+
+  EXPECT_FALSE(file1 == file2);
+}
+
+TEST(
+    File,
+    lessThanOperatorShouldUseCaseInsensitiveLexicographicalComparisonForName) {
+  File file1("name", "display", "condition");
+  File file2("name", "display", "condition");
+
+  EXPECT_FALSE(file1 < file2);
+  EXPECT_FALSE(file2 < file1);
+
+  file1 = File("name", "display", "condition");
+  file2 = File("Name", "display", "condition");
 
   EXPECT_FALSE(file1 < file2);
   EXPECT_FALSE(file2 < file1);
@@ -78,6 +117,40 @@ TEST(File,
   EXPECT_FALSE(file2 < file1);
 }
 
+TEST(
+    File,
+    lessThanOperatorShouldUseCaseSensitiveLexicographicalComparisonForDisplayAndCondition) {
+  File file1("name", "display", "condition");
+  File file2("name", "display", "condition");
+
+  EXPECT_FALSE(file1 < file2);
+  EXPECT_FALSE(file2 < file1);
+
+  file1 = File("name", "display", "condition");
+  file2 = File("name", "Display", "condition");
+
+  EXPECT_TRUE(file2 < file1);
+  EXPECT_FALSE(file1 < file2);
+
+  file1 = File("name", "display", "condition");
+  file2 = File("name", "display", "Condition");
+
+  EXPECT_TRUE(file2 < file1);
+  EXPECT_FALSE(file1 < file2);
+
+  file1 = File("name", "display1");
+  file2 = File("name", "display2");
+
+  EXPECT_TRUE(file1 < file2);
+  EXPECT_FALSE(file2 < file1);
+
+  file1 = File("name", "display", "condition1");
+  file2 = File("name", "display", "condition2");
+
+  EXPECT_TRUE(file1 < file2);
+  EXPECT_FALSE(file2 < file1);
+}
+
 TEST(File, emittingAsYamlShouldSingleQuoteValues) {
   File file("name1", "display1", "condition1");
   YAML::Emitter emitter;