From 575e6d5c93b8f31a07c5e6f1625788e8d1d5e2f0 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Fri, 4 Apr 2025 19:50:02 +0100 Subject: [PATCH] Add support for "constraint" in File metadata --- docs/metadata/data_structures/file.rst | 8 +- include/loot/database_interface.h | 6 ++ include/loot/metadata/file.h | 9 ++- src/api/api_database.cpp | 4 + src/api/api_database.h | 2 + src/api/metadata/file.cpp | 17 ++++- src/api/metadata/yaml/file.h | 75 ++++++++++++------- src/api/sorting/plugin_sort.cpp | 31 +++++++- .../api/interface/database_interface_test.h | 8 ++ src/tests/api/internals/metadata/file_test.h | 60 ++++++++++++--- .../api/internals/sorting/plugin_sort_test.h | 49 ++++++++++++ 11 files changed, 229 insertions(+), 40 deletions(-) diff --git a/docs/metadata/data_structures/file.rst b/docs/metadata/data_structures/file.rst index 118aea1b..7967cb70 100644 --- a/docs/metadata/data_structures/file.rst +++ b/docs/metadata/data_structures/file.rst @@ -28,10 +28,16 @@ Map Form A condition string that is evaluated to determine whether this file data should be used: if it evaluates to true, the data is used, otherwise it is ignored. See :doc:`../conditions` for details. +.. describe:: constraint + + A condition string that must also evaluate to true for the file's existence to be recognised. See :doc:`../conditions` for details. + + When a constraint is set, it's best to also set a ``display`` value that describes the constraint so that it is visible to users if a message is displayed for the file. + Scalar Form ----------- -The scalar form is simply the value of the map form's ``name`` key. Using the scalar form is equivalent to using the map form with undefined ``display`` and ``condition`` keys. +The scalar form is simply the value of the map form's ``name`` key. Using the scalar form is equivalent to using the map form with undefined ``display``, ``detail``, ``condition`` and ``constraint`` keys. Equality -------- diff --git a/include/loot/database_interface.h b/include/loot/database_interface.h index ee30515e..136b8627 100644 --- a/include/loot/database_interface.h +++ b/include/loot/database_interface.h @@ -107,6 +107,12 @@ public: virtual void WriteMinimalList(const std::filesystem::path& outputFile, const bool overwrite) const = 0; + /** + * @brief Evaluate the given condition string. + * @param condition A condition string. + */ + virtual bool Evaluate(const std::string& condition) const = 0; + /** * @} * @name Non-plugin Data Access diff --git a/include/loot/metadata/file.h b/include/loot/metadata/file.h index 526e082d..c9ea1335 100644 --- a/include/loot/metadata/file.h +++ b/include/loot/metadata/file.h @@ -57,12 +57,16 @@ public: * The detail message content, which may be appended to any messages * generated for this file. If multilingual, one language must be * English. + * @param constraint + * A condition string that must evaluate to true for the file's existence + * to be recognised. * @return A File object. */ LOOT_API explicit File(std::string_view name, std::string_view display = "", std::string_view condition = "", - const std::vector& detail = {}); + const std::vector& detail = {}, + std::string_view constraint = ""); /** * Get the filename of the file. @@ -85,10 +89,13 @@ public: */ LOOT_API std::vector GetDetail() const; + LOOT_API std::string GetConstraint() const; + private: Filename name_; std::string display_; std::vector detail_; + std::string constraint_; }; /** diff --git a/src/api/api_database.cpp b/src/api/api_database.cpp index e1c1794c..c19d2f5f 100644 --- a/src/api/api_database.cpp +++ b/src/api/api_database.cpp @@ -143,6 +143,10 @@ void ApiDatabase::WriteUserMetadata(const std::filesystem::path& outputFile, userlist_.Save(outputFile); } +bool ApiDatabase::Evaluate(const std::string& condition) const { + return conditionEvaluator_->Evaluate(condition); +} + ////////////////////////// // DB Access Functions ////////////////////////// diff --git a/src/api/api_database.h b/src/api/api_database.h index 88931d38..6e34392d 100644 --- a/src/api/api_database.h +++ b/src/api/api_database.h @@ -55,6 +55,8 @@ struct ApiDatabase final : public DatabaseInterface { void WriteMinimalList(const std::filesystem::path& outputFile, const bool overwrite) const override; + bool Evaluate(const std::string& condition) const override; + std::vector GetKnownBashTags() const override; std::vector GetGeneralMessages( diff --git a/src/api/metadata/file.cpp b/src/api/metadata/file.cpp index d8fd09b8..c199ac3c 100644 --- a/src/api/metadata/file.cpp +++ b/src/api/metadata/file.cpp @@ -28,11 +28,13 @@ namespace loot { File::File(std::string_view name, std::string_view display, std::string_view condition, - const std::vector& detail) : + const std::vector& detail, + std::string_view constraint) : ConditionalMetadata(condition), name_(Filename(name)), display_(display), - detail_(detail) {} + detail_(detail), + constraint_(constraint) {} Filename File::GetName() const { return name_; } @@ -40,9 +42,12 @@ std::string File::GetDisplayName() const { return display_; } std::vector File::GetDetail() const { return detail_; } +std::string File::GetConstraint() const { return constraint_; } + bool operator==(const File& lhs, const File& rhs) { return lhs.GetDisplayName() == rhs.GetDisplayName() && lhs.GetCondition() == rhs.GetCondition() && + lhs.GetConstraint() == rhs.GetConstraint() && lhs.GetName() == rhs.GetName() && lhs.GetDetail() == rhs.GetDetail(); } @@ -65,6 +70,14 @@ bool operator<(const File& lhs, const File& rhs) { return false; } + if (lhs.GetConstraint() < rhs.GetConstraint()) { + return true; + } + + if (rhs.GetConstraint() < lhs.GetConstraint()) { + return false; + } + if (lhs.GetName() < rhs.GetName()) { return true; } diff --git a/src/api/metadata/yaml/file.h b/src/api/metadata/yaml/file.h index 85673e5b..50bcb014 100644 --- a/src/api/metadata/yaml/file.h +++ b/src/api/metadata/yaml/file.h @@ -38,7 +38,7 @@ namespace loot { inline bool emitAsScalar(const File& file) { return !file.IsConditional() && file.GetDetail().empty() && - file.GetDisplayName().empty(); + file.GetDisplayName().empty() && file.GetConstraint().empty(); } } @@ -49,8 +49,13 @@ struct convert { Node node; node["name"] = std::string(rhs.GetName()); - if (rhs.IsConditional()) + if (rhs.IsConditional()) { node["condition"] = rhs.GetCondition(); + } + + if (!rhs.GetConstraint().empty()) { + node["constraint"] = rhs.GetConstraint(); + } if (!rhs.GetDisplayName().empty()) { node["display"] = rhs.GetDisplayName(); @@ -64,23 +69,32 @@ struct convert { } static bool decode(const Node& node, loot::File& rhs) { - if (!node.IsMap() && !node.IsScalar()) + if (!node.IsMap() && !node.IsScalar()) { throw RepresentationException( node.Mark(), "bad conversion: 'file' object must be a map or scalar"); + } if (node.IsMap()) { - if (!node["name"]) + if (!node["name"]) { throw RepresentationException( node.Mark(), "bad conversion: 'name' key missing from 'file' map object"); + } std::string name = node["name"].as(); - std::string condition, display; + std::string condition, constraint, display; std::vector detail; - if (node["condition"]) + if (node["condition"]) { condition = node["condition"].as(); - if (node["display"]) + } + + if (node["constraint"]) { + constraint = node["constraint"].as(); + } + + if (node["display"]) { display = node["display"].as(); + } if (node["detail"]) { if (node["detail"].IsSequence()) { @@ -94,28 +108,32 @@ struct convert { // Check now that at least one item in info is English if there are // multiple items. if (detail.size() > 1) { - bool found = false; - for (const auto& mc : detail) { - if (mc.GetLanguage() == loot::MessageContent::DEFAULT_LANGUAGE) - found = true; - } - if (!found) + const auto found = std::any_of( + detail.begin(), detail.end(), [](const loot::MessageContent& mc) { + return mc.GetLanguage() == loot::MessageContent::DEFAULT_LANGUAGE; + }); + + if (!found) { throw RepresentationException(node.Mark(), "bad conversion: multilingual messages " "must contain an English info string"); + } } - rhs = loot::File(name, display, condition, detail); - } else - rhs = loot::File(node.as()); + // Test condition syntax. + try { + loot::ParseCondition(condition); + loot::ParseCondition(constraint); + } catch (const std::exception& e) { + throw RepresentationException( + node.Mark(), + std::string("bad conversion: invalid condition syntax: ") + + e.what()); + } - // Test condition syntax. - try { - loot::ParseCondition(rhs.GetCondition()); - } catch (const std::exception& e) { - throw RepresentationException( - node.Mark(), - std::string("bad conversion: invalid condition syntax: ") + e.what()); + rhs = loot::File(name, display, condition, detail, constraint); + } else { + rhs = loot::File(node.as()); } return true; @@ -129,13 +147,20 @@ inline Emitter& operator<<(Emitter& out, const loot::File& rhs) { out << BeginMap << Key << "name" << Value << YAML::SingleQuoted << std::string(rhs.GetName()); - if (rhs.IsConditional()) + if (rhs.IsConditional()) { out << Key << "condition" << Value << YAML::SingleQuoted << rhs.GetCondition(); + } - if (!rhs.GetDisplayName().empty()) + if (!rhs.GetDisplayName().empty()) { out << Key << "display" << Value << YAML::SingleQuoted << rhs.GetDisplayName(); + } + + if (!rhs.GetConstraint().empty()) { + out << Key << "constraint" << Value << YAML::SingleQuoted + << rhs.GetConstraint(); + } if (rhs.GetDetail().size() == 1) { out << Key << "detail" << Value << YAML::SingleQuoted diff --git a/src/api/sorting/plugin_sort.cpp b/src/api/sorting/plugin_sort.cpp index 35095f5b..4e1b84ea 100644 --- a/src/api/sorting/plugin_sort.cpp +++ b/src/api/sorting/plugin_sort.cpp @@ -31,6 +31,29 @@ #include "api/sorting/plugin_graph.h" #include "loot/exception/undefined_group_error.h" +namespace { +std::vector FilterFilesByConstraint( + const loot::DatabaseInterface& db, + std::vector&& files) { + std::vector filtered; + for (auto&& file : files) { + if (db.Evaluate(file.GetConstraint())) { + filtered.push_back(std::move(file)); + } + } + + return filtered; +} + +void FilterByConstraint(const loot::DatabaseInterface& db, + loot::PluginMetadata& metadata) { + metadata.SetLoadAfterFiles( + FilterFilesByConstraint(db, metadata.GetLoadAfterFiles())); + metadata.SetRequirements( + FilterFilesByConstraint(db, metadata.GetRequirements())); +} +} + namespace loot { std::vector GetPluginsSortingData( const DatabaseInterface& db, @@ -42,12 +65,16 @@ std::vector GetPluginsSortingData( for (const auto& plugin : loadOrder) { const auto pluginFilename = plugin->GetName(); - const auto masterlistMetadata = + auto masterlistMetadata = db.GetPluginMetadata(pluginFilename, false, true) .value_or(PluginMetadata(pluginFilename)); - const auto userMetadata = db.GetPluginUserMetadata(pluginFilename, true) + auto userMetadata = db.GetPluginUserMetadata(pluginFilename, true) .value_or(PluginMetadata(pluginFilename)); + // Only use constained metadata when the constraints are true. + FilterByConstraint(db, masterlistMetadata); + FilterByConstraint(db, userMetadata); + const auto pluginSortingData = PluginSortingData(plugin, masterlistMetadata, userMetadata, i); diff --git a/src/tests/api/interface/database_interface_test.h b/src/tests/api/interface/database_interface_test.h index 208099f3..f6457b27 100644 --- a/src/tests/api/interface/database_interface_test.h +++ b/src/tests/api/interface/database_interface_test.h @@ -274,6 +274,14 @@ TEST_P(DatabaseInterfaceTest, writeUserMetadataShouldShouldWriteUserMetadata) { EXPECT_FALSE(GetFileContent(minimalOutputPath_).empty()); } +TEST_P(DatabaseInterfaceTest, evaluateShouldReturnTrueIfTheConditionIsTrue) { + EXPECT_TRUE(handle_->GetDatabase().Evaluate("file(\"Blank.esp\")")); +} + +TEST_P(DatabaseInterfaceTest, evaluateShouldReturnFalseIfTheConditionIsFalse) { + EXPECT_FALSE(handle_->GetDatabase().Evaluate("file(\"missing.esp\")")); +} + TEST_P(DatabaseInterfaceTest, getGroupsShouldReturnAllGroupsListedInTheLoadedMetadata) { ASSERT_NO_THROW(GenerateMasterlist()); diff --git a/src/tests/api/internals/metadata/file_test.h b/src/tests/api/internals/metadata/file_test.h index 914cad30..b630ccdc 100644 --- a/src/tests/api/internals/metadata/file_test.h +++ b/src/tests/api/internals/metadata/file_test.h @@ -38,19 +38,21 @@ TEST(File, defaultConstructorShouldInitialiseEmptyStrings) { EXPECT_EQ("", std::string(file.GetName())); EXPECT_EQ("", file.GetDisplayName()); EXPECT_EQ("", file.GetCondition()); + EXPECT_EQ("", file.GetConstraint()); } TEST(File, stringsConstructorShouldStoreGivenStrings) { std::vector detail = {MessageContent("text", "en")}; - File file("name", "display", "condition", detail); + File file("name", "display", "condition", detail, "constraint"); EXPECT_EQ("name", std::string(file.GetName())); EXPECT_EQ("display", file.GetDisplayName()); EXPECT_EQ("condition", file.GetCondition()); EXPECT_EQ(detail, file.GetDetail()); + EXPECT_EQ("constraint", file.GetConstraint()); } -TEST(File, equalityShouldBeCaseInsensitiveOnNameAndDisplay) { +TEST(File, equalityShouldBeCaseInsensitiveOnName) { File file1("name", "display", "condition"); File file2("name", "display", "condition"); @@ -67,7 +69,7 @@ TEST(File, equalityShouldBeCaseInsensitiveOnNameAndDisplay) { EXPECT_FALSE(file1 == file2); } -TEST(File, equalityShouldBeCaseSensitiveOnDisplayAndCondition) { +TEST(File, equalityShouldBeCaseSensitiveOnDisplayAndConditionAndConstraint) { File file1("name", "display", "condition"); File file2("name", "display", "condition"); @@ -83,6 +85,11 @@ TEST(File, equalityShouldBeCaseSensitiveOnDisplayAndCondition) { EXPECT_FALSE(file1 == file2); + file1 = File("name", "display", "condition", {}, "constraint"); + file2 = File("name", "display", "condition", {}, "Constraint"); + + EXPECT_FALSE(file1 == file2); + file1 = File("name", "display1", "condition"); file2 = File("name", "display2", "condition"); @@ -92,6 +99,11 @@ TEST(File, equalityShouldBeCaseSensitiveOnDisplayAndCondition) { file2 = File("name", "display", "condition2"); EXPECT_FALSE(file1 == file2); + + file1 = File("name", "display", "condition", {}, "constraint1"); + file2 = File("name", "display", "condition", {}, "constraint2"); + + EXPECT_FALSE(file1 == file2); } TEST(File, equalityShouldCompareTheDetailVectors) { @@ -133,6 +145,11 @@ TEST(File, inequalityShouldBeTheInverseOfEquality) { EXPECT_TRUE(file1 != file2); + file1 = File("name", "display", "condition", {}, "constraint"); + file2 = File("name", "display", "condition", {}, "Constraint"); + + EXPECT_TRUE(file1 != file2); + file1 = File("name", "display1", "condition"); file2 = File("name", "display2", "condition"); @@ -147,6 +164,11 @@ TEST(File, inequalityShouldBeTheInverseOfEquality) { file2 = File("", "", "", {MessageContent("Text", "en")}); EXPECT_TRUE(file1 != file2); + + file1 = File("name", "display", "condition", {}, "constraint1"); + file2 = File("name", "display", "condition", {}, "constraint2"); + + EXPECT_TRUE(file1 != file2); } TEST(File, @@ -172,7 +194,7 @@ TEST(File, TEST( File, - lessThanOperatorShouldUseCaseSensitiveLexicographicalComparisonForDisplayAndCondition) { + lessThanOperatorShouldUseCaseSensitiveLexicographicalComparisonForDisplayAndConditionAndConstraint) { File file1("name", "display", "condition"); File file2("name", "display", "condition"); @@ -191,6 +213,12 @@ TEST( EXPECT_TRUE(file2 < file1); EXPECT_FALSE(file1 < file2); + file1 = File("name", "display", "condition", {}, "constraint"); + file2 = File("name", "display", "condition", {}, "Constraint"); + + EXPECT_TRUE(file2 < file1); + EXPECT_FALSE(file1 < file2); + file1 = File("name", "display1"); file2 = File("name", "display2"); @@ -202,6 +230,12 @@ TEST( EXPECT_TRUE(file1 < file2); EXPECT_FALSE(file2 < file1); + + file1 = File("name", "display", "condition", {}, "constraint1"); + file2 = File("name", "display", "condition", {}, "constraint2"); + + EXPECT_TRUE(file1 < file2); + EXPECT_FALSE(file2 < file1); } TEST(File, lessThanOperatorShouldCompareTheDetailVectors) { @@ -349,13 +383,17 @@ TEST(File, getDisplayNameShouldReturnDisplayString) { } TEST(File, emittingAsYamlShouldSingleQuoteValues) { - File file( - "name1", "display1", "condition1", {MessageContent("english", "en")}); + File file("name1", + "display1", + "condition1", + {MessageContent("english", "en")}, + "constraint1"); YAML::Emitter emitter; emitter << file; std::string expected = "name: '" + std::string(file.GetName()) + "'\ncondition: '" + file.GetCondition() + "'\ndisplay: '" + file.GetDisplayName() + + "'\nconstraint: '" + file.GetConstraint() + "'\ndetail: '" + file.GetDetail()[0].GetText() + "'"; EXPECT_EQ(expected, emitter.c_str()); @@ -369,7 +407,7 @@ TEST(File, emittingAsYamlShouldOutputAsAScalarIfOnlyTheNameStringIsNotEmpty) { EXPECT_EQ("'" + std::string(file.GetName()) + "'", emitter.c_str()); } -TEST(File, emittingAsYamlShouldOmitAnEmptyConditionString) { +TEST(File, emittingAsYamlShouldOmitEmptyConditionAndConstraintStrings) { File file("name1", "display1"); YAML::Emitter emitter; emitter << file; @@ -402,7 +440,7 @@ TEST( TEST(File, encodingAsYamlShouldStoreDataCorrectly) { auto detail = {MessageContent("english", "en"), MessageContent("french", "fr")}; - File file("name1", "display1", "condition1", detail); + File file("name1", "display1", "condition1", detail, "constraint1"); YAML::Node node; node = file; @@ -410,6 +448,7 @@ TEST(File, encodingAsYamlShouldStoreDataCorrectly) { EXPECT_EQ(file.GetDisplayName(), node["display"].as()); EXPECT_EQ(file.GetCondition(), node["condition"].as()); EXPECT_EQ(file.GetDetail(), node["detail"].as>()); + EXPECT_EQ(file.GetConstraint(), node["constraint"].as()); } TEST(File, encodingAsYamlShouldOmitEmptyFields) { @@ -426,7 +465,7 @@ TEST(File, encodingAsYamlShouldOmitEmptyFields) { TEST(File, decodingFromYamlShouldSetDataCorrectly) { YAML::Node node = YAML::Load( "{name: name1, display: display1, condition: 'file(\"Foo.esp\")', " - "detail: 'details'}"); + "detail: 'details', constraint: 'file(\"Bar.esp\")'}"); File file = node.as(); std::vector expectedDetail = { @@ -436,6 +475,7 @@ TEST(File, decodingFromYamlShouldSetDataCorrectly) { EXPECT_EQ(node["display"].as(), file.GetDisplayName()); EXPECT_EQ(node["condition"].as(), file.GetCondition()); EXPECT_EQ(expectedDetail, file.GetDetail()); + EXPECT_EQ(node["constraint"].as(), file.GetConstraint()); } TEST(File, @@ -447,6 +487,7 @@ TEST(File, EXPECT_EQ(node["display"].as(), file.GetDisplayName()); EXPECT_TRUE(file.GetCondition().empty()); EXPECT_TRUE(file.GetDetail().empty()); + EXPECT_TRUE(file.GetConstraint().empty()); } TEST(File, decodingFromYamlWithAListOfMessageContentDetailsShouldReadThemAll) { @@ -493,6 +534,7 @@ TEST(File, decodingFromYamlScalarShouldLeaveDisplayNameAndConditionEmpty) { EXPECT_TRUE(file.GetDisplayName().empty()); EXPECT_TRUE(file.GetCondition().empty()); EXPECT_TRUE(file.GetDetail().empty()); + EXPECT_TRUE(file.GetConstraint().empty()); } TEST(File, decodingFromYamlShouldThrowIfAnInvalidMapIsGiven) { diff --git a/src/tests/api/internals/sorting/plugin_sort_test.h b/src/tests/api/internals/sorting/plugin_sort_test.h index 485ea94b..f9b13bc7 100644 --- a/src/tests/api/internals/sorting/plugin_sort_test.h +++ b/src/tests/api/internals/sorting/plugin_sort_test.h @@ -33,6 +33,55 @@ along with LOOT. If not, see namespace loot { namespace test { +class GetPluginsSortingDataTest : public CommonGameTestFixture { +protected: + GetPluginsSortingDataTest() : CommonGameTestFixture(GameType::tes4) {} +}; + +TEST_F(GetPluginsSortingDataTest, shouldFilterOutFilesWithFalseConstraints) { + Game game(GameType::tes4, gamePath, localPath); + + game.LoadPlugins({blankEsp}, true); + + const auto plugin = game.GetPlugin(blankEsp); + + const auto trueConstraint = "file(\"Blank.esm\")"; + const auto falseConstraint = "file(\"missing.esp\")"; + + std::filesystem::path masterlistPath = localPath / "masterlist.yaml"; + std::ofstream out(masterlistPath); + out << "{plugins: [{name: Blank.esp, after: [{name: A.esp, constraint: '" + << trueConstraint << "'}, {name: B.esp, constraint: '" << falseConstraint + << "'}], req: [{name: C.esp, constraint: '" << trueConstraint + << "'}, {name: D.esp, constraint: '" << falseConstraint << "'}]}]}"; + out.close(); + + game.GetDatabase().LoadMasterlist(masterlistPath); + + PluginMetadata userMetadata(blankEsp); + userMetadata.SetLoadAfterFiles( + {File(blankEsm, "", "", {}, trueConstraint), + File(blankDifferentEsm, "", "", {}, falseConstraint)}); + userMetadata.SetRequirements( + {File(blankDifferentEsp, "", "", {}, trueConstraint), + File(blankMasterDependentEsm, "", "", {}, falseConstraint)}); + + game.GetDatabase().SetPluginUserMetadata(userMetadata); + + const auto pluginsSortingData = GetPluginsSortingData( + game.GetDatabase(), {reinterpret_cast(plugin.get())}); + + ASSERT_EQ(1, pluginsSortingData.size()); + EXPECT_EQ(std::vector{File("A.esp", "", "", {}, trueConstraint)}, + pluginsSortingData[0].GetMasterlistLoadAfterFiles()); + EXPECT_EQ(std::vector{File("C.esp", "", "", {}, trueConstraint)}, + pluginsSortingData[0].GetMasterlistRequirements()); + EXPECT_EQ(std::vector{userMetadata.GetLoadAfterFiles()[0]}, + pluginsSortingData[0].GetUserLoadAfterFiles()); + EXPECT_EQ(std::vector{userMetadata.GetRequirements()[0]}, + pluginsSortingData[0].GetUserRequirements()); +} + class SortPluginsTest : public ::testing::Test { protected: PluginSortingData CreatePluginSortingData(const std::string& name,