From 761cdf555b6ff73e9a0fd50989586a4c27adcec3 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Wed, 8 Feb 2017 20:09:15 +0000 Subject: [PATCH] Reintroduce on-demand condition evaluation Extend the API to optionally allow it when getting plugin metadata and general messages. --- include/loot/database_interface.h | 33 ++++++++---- resources/l10n/template.pot | 22 ++++---- src/api/api_database.cpp | 35 +++++++++++-- src/api/api_database.h | 8 +-- src/gui/query/get_game_data_query.h | 4 +- src/gui/query/metadata_query.h | 50 ++++++++++++------- src/gui/query/update_masterlist_query.h | 1 - src/gui/state/game.cpp | 23 +++------ src/gui/state/game.h | 7 +-- .../api/interface/api_game_operations_test.h | 2 + .../api/interface/database_interface_test.h | 34 ++++++++++++- 11 files changed, 151 insertions(+), 68 deletions(-) diff --git a/include/loot/database_interface.h b/include/loot/database_interface.h index 4274fdb5..5c971323 100644 --- a/include/loot/database_interface.h +++ b/include/loot/database_interface.h @@ -74,9 +74,9 @@ public: * written. Otherwise, data will be written. */ virtual void WriteUserMetadata(const std::string& outputFile, - const bool overwrite) const = 0; + const bool overwrite) const = 0; - /** + /** * @brief Writes a minimal metadata file that only contains plugins with * Bash Tag suggestions and/or dirty info, plus the suggestions and * info themselves. @@ -86,8 +86,8 @@ public: * If `false` and `outputFile` already exists, no data will be * written. Otherwise, data will be written. */ - virtual void WriteMinimalList(const std::string& outputFile, - const bool overwrite) const = 0; + virtual void WriteMinimalList(const std::string& outputFile, + const bool overwrite) const = 0; /** * @} @@ -152,21 +152,26 @@ public: * @} * @name Non-plugin Data Access * @{ - */ + */ /** * @brief Gets the Bash Tags that are listed in the loaded metadata lists. * @details Bash Tag suggestions can include plugins not in this list. * @returns A set of Bash Tag names. - */ + */ virtual std::set GetKnownBashTags() const = 0; /** * @brief Get all general messages listen in the loaded metadata lists. + * @param evaluateConditions + * If true, any metadata conditions are evaluated before the metadata + * is returned, otherwise unevaluated metadata is returned. Evaluating + * general message conditions also clears the condition cache before + * evaluating conditions. * @returns A vector of messages supplied in the metadata lists but not * attached to any particular plugin. */ - virtual std::vector GetGeneralMessages() const = 0; + virtual std::vector GetGeneralMessages(bool evaluateConditions = false) const = 0; /** * @} @@ -182,22 +187,32 @@ public: * If true, any user metadata the plugin has is included in the * returned metadata, otherwise the metadata returned only includes * metadata from the masterlist. + * @param evaluateConditions + * If true, any metadata conditions are evaluated before the metadata + * is returned, otherwise unevaluated metadata is returned. Evaluating + * plugin metadata conditions does not clear the condition cache. * @returns A PluginMetadata object containing all the plugin's metadata. * If the plugin has no metadata, PluginMetadata.IsNameOnly() * will return true. */ virtual PluginMetadata GetPluginMetadata(const std::string& plugin, - bool includeUserMetadata = true) const = 0; + bool includeUserMetadata = true, + bool evaluateConditions = false) const = 0; /** * @brief Get a plugin's metadata loaded from the given userlist. * @param plugin * The filename of the plugin to look up user-added metadata for. + * @param evaluateConditions + * If true, any metadata conditions are evaluated before the metadata + * is returned, otherwise unevaluated metadata is returned. Evaluating + * plugin metadata conditions does not clear the condition cache. * @returns A PluginMetadata object containing the plugin's user-added * metadata. If the plugin has no metadata, * PluginMetadata.IsNameOnly() will return true. */ - virtual PluginMetadata GetPluginUserMetadata(const std::string& plugin) const = 0; + virtual PluginMetadata GetPluginUserMetadata(const std::string& plugin, + bool evaluateConditions = false) const = 0; /** * @brief Sets a plugin's user metadata, overwriting any existing user diff --git a/resources/l10n/template.pot b/resources/l10n/template.pot index 3173c7ae..a18c6be0 100644 --- a/resources/l10n/template.pot +++ b/resources/l10n/template.pot @@ -8,7 +8,7 @@ msgid "" msgstr "" "Project-Id-Version: LOOT 0.10.3\n" "Report-Msgid-Bugs-To: https://github.com/loot/loot/issues\n" -"POT-Creation-Date: 2017-02-06 18:49+0000\n" +"POT-Creation-Date: 2017-02-08 20:47+0000\n" "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" "Last-Translator: FULL NAME \n" "Language-Team: LANGUAGE \n" @@ -678,15 +678,19 @@ msgstr "" msgid "Parsing, merging and evaluating metadata..." msgstr "" -#: src/gui/query/metadata_query.h:104 src/gui/query/metadata_query.h:105 +#: src/gui/query/metadata_query.h:82 src/gui/query/metadata_query.h:92 +msgid "\"%1%\" contains a condition that could not be evaluated. Details: %2%" +msgstr "" + +#: src/gui/query/metadata_query.h:111 src/gui/query/metadata_query.h:112 msgid "N/A: No masterlist present" msgstr "" -#: src/gui/query/metadata_query.h:108 src/gui/query/metadata_query.h:109 +#: src/gui/query/metadata_query.h:115 src/gui/query/metadata_query.h:116 msgid "Unknown: Git repository missing" msgstr "" -#: src/gui/query/metadata_query.h:153 src/gui/query/metadata_query.h:154 +#: src/gui/query/metadata_query.h:169 src/gui/query/metadata_query.h:170 msgid "(edited)" msgstr "" @@ -697,10 +701,6 @@ msgid "" msgstr "" #: src/gui/query/sort_plugins_query.h:48 -msgid "Loading plugin contents..." -msgstr "" - -#: src/gui/query/sort_plugins_query.h:52 msgid "Sorting load order..." msgstr "" @@ -716,17 +716,17 @@ msgstr "" msgid "This plugin is incompatible with \"%1%\", but both are present." msgstr "" -#: src/gui/state/game.cpp:296 +#: src/gui/state/game.cpp:280 msgid "" "Cyclic interaction detected between plugins \"%1%\" and \"%2%\". Back cycle: " "%3%" msgstr "" -#: src/gui/state/game.cpp:325 +#: src/gui/state/game.cpp:309 msgid "You have not sorted your load order this session." msgstr "" -#: src/gui/state/game.cpp:374 +#: src/gui/state/game.cpp:353 msgid "" "An error occurred while parsing the metadata list(s): %1%.\n" "\n" diff --git a/src/api/api_database.cpp b/src/api/api_database.cpp index d94bc022..75d57b80 100644 --- a/src/api/api_database.cpp +++ b/src/api/api_database.cpp @@ -32,6 +32,7 @@ #include "loot/exception/file_access_error.h" #include "loot/yaml/plugin_metadata.h" #include "api/game/game.h" +#include "api/metadata/condition_evaluator.h" #include "api/plugin/plugin_sorter.h" namespace loot { @@ -133,7 +134,7 @@ std::set ApiDatabase::GetKnownBashTags() const { return masterlistTags; } -std::vector ApiDatabase::GetGeneralMessages() const { +std::vector ApiDatabase::GetGeneralMessages(bool evaluateConditions) const { auto masterlistMessages = game_.GetMasterlist().Messages(); auto userlistMessages = game_.GetUserlist().Messages(); @@ -141,22 +142,48 @@ std::vector ApiDatabase::GetGeneralMessages() const { masterlistMessages.insert(std::end(masterlistMessages), std::begin(userlistMessages), std::end(userlistMessages)); } + if (evaluateConditions) { + // Evaluate conditions from scratch. + game_.ClearCachedConditions(); + ConditionEvaluator evaluator(&game_); + for (auto it = std::begin(masterlistMessages); it != std::end(masterlistMessages);) { + if (!evaluator.evaluate(it->GetCondition())) + it = masterlistMessages.erase(it); + else + ++it; + } + } + return masterlistMessages; } PluginMetadata ApiDatabase::GetPluginMetadata(const std::string& plugin, - bool includeUserMetadata) const { + bool includeUserMetadata, + bool evaluateConditions) const { PluginMetadata metadata = game_.GetMasterlist().FindPlugin(plugin); if (includeUserMetadata) { metadata.MergeMetadata(game_.GetUserlist().FindPlugin(plugin)); } + if (evaluateConditions) { + ConditionEvaluator evaluator(&game_); + return evaluator.evaluateAll(metadata); + } + return metadata; } -PluginMetadata ApiDatabase::GetPluginUserMetadata(const std::string& plugin) const { - return game_.GetUserlist().FindPlugin(plugin); +PluginMetadata ApiDatabase::GetPluginUserMetadata(const std::string& plugin, + bool evaluateConditions) const { + PluginMetadata metadata = game_.GetUserlist().FindPlugin(plugin); + + if (evaluateConditions) { + ConditionEvaluator evaluator(&game_); + return evaluator.evaluateAll(metadata); + } + + return metadata; } void ApiDatabase::SetPluginUserMetadata(const PluginMetadata& pluginMetadata) { diff --git a/src/api/api_database.h b/src/api/api_database.h index 6f71c1ae..bc9bc01e 100644 --- a/src/api/api_database.h +++ b/src/api/api_database.h @@ -56,12 +56,14 @@ struct ApiDatabase : public DatabaseInterface { std::set GetKnownBashTags() const; - std::vector GetGeneralMessages() const; + std::vector GetGeneralMessages(bool evaluateConditions = false) const; PluginMetadata GetPluginMetadata(const std::string& plugin, - bool includeUserMetadata = true) const; + bool includeUserMetadata = true, + bool evaluateConditions = false) const; - PluginMetadata GetPluginUserMetadata(const std::string& plugin) const; + PluginMetadata GetPluginUserMetadata(const std::string& plugin, + bool evaluateConditions = false) const; void SetPluginUserMetadata(const PluginMetadata& pluginMetadata); diff --git a/src/gui/query/get_game_data_query.h b/src/gui/query/get_game_data_query.h index 7eace035..605adc28 100644 --- a/src/gui/query/get_game_data_query.h +++ b/src/gui/query/get_game_data_query.h @@ -53,8 +53,6 @@ public: if (isFirstLoad) state_.getCurrentGame().LoadMetadata(); - state_.getCurrentGame().EvaluateLoadedMetadata(); - //Sort plugins into their load order. std::vector> installed; std::vector loadOrder = state_.getCurrentGame().GetLoadOrder(); @@ -121,7 +119,7 @@ private: // Now merge masterlist and userlist metadata and evaluate, // putting any resulting metadata into the base of the pluginNode. - YAML::Node derivedNode = MetadataQuery::generateDerivedMetadata(plugin, masterlistMetadata, userlistMetadata); + YAML::Node derivedNode = MetadataQuery::generateDerivedMetadata(plugin->GetName()); for (auto it = derivedNode.begin(); it != derivedNode.end(); ++it) { const std::string key = it->first.as(); diff --git a/src/gui/query/metadata_query.h b/src/gui/query/metadata_query.h index 92c7ea92..2cc5bac6 100644 --- a/src/gui/query/metadata_query.h +++ b/src/gui/query/metadata_query.h @@ -64,28 +64,36 @@ protected: return metadata; } - YAML::Node generateDerivedMetadata(std::shared_ptr file, - const PluginMetadata& masterlistEntry, - const PluginMetadata& userlistEntry) { - auto metadata = getNonUserMetadata(file, masterlistEntry); - metadata.MergeMetadata(userlistEntry); - - return toYaml(file, metadata); - } - YAML::Node generateDerivedMetadata(const std::string& pluginName) { // Now rederive the displayed metadata from the masterlist and userlist. + std::shared_ptr plugin = nullptr; try { - auto plugin = state_.getCurrentGame().GetPlugin(pluginName); - - - PluginMetadata master(state_.getCurrentGame().GetMasterlistMetadata(pluginName)); - PluginMetadata user(state_.getCurrentGame().GetUserMetadata(pluginName)); - - return generateDerivedMetadata(plugin, master, user); + plugin = state_.getCurrentGame().GetPlugin(pluginName); } catch (...) { return YAML::Node(); } + + PluginMetadata master(pluginName); + try { + master = state_.getCurrentGame().GetMasterlistMetadata(pluginName, true); + } catch (std::exception& e) { + BOOST_LOG_TRIVIAL(error) << "\"" << pluginName << "\"'s masterlist metadata contains a condition that could not be evaluated. Details: " << e.what(); + master.SetMessages({ + Message(MessageType::error, (boost::format(boost::locale::translate("\"%1%\" contains a condition that could not be evaluated. Details: %2%")) % pluginName % e.what()).str()), + }); + } + + PluginMetadata user(pluginName); + try { + user = state_.getCurrentGame().GetUserMetadata(pluginName, true); + } catch (std::exception& e) { + BOOST_LOG_TRIVIAL(error) << "\"" << pluginName << "\"'s user metadata contains a condition that could not be evaluated. Details: " << e.what(); + user.SetMessages({ + Message(MessageType::error, (boost::format(boost::locale::translate("\"%1%\" contains a condition that could not be evaluated. Details: %2%")) % pluginName % e.what()).str()), + }); + } + + return generateDerivedMetadata(plugin, master, user); } YAML::Node getMasterlistInfo() { @@ -93,7 +101,6 @@ protected: YAML::Node masterlistNode; try { - MasterlistInfo info = state_.getCurrentGame().GetMasterlistInfo(); addSuffixIfModified(info); @@ -127,6 +134,15 @@ private: return simpleMessages; } + YAML::Node generateDerivedMetadata(std::shared_ptr file, + const PluginMetadata& masterlistEntry, + const PluginMetadata& userlistEntry) { + auto metadata = getNonUserMetadata(file, masterlistEntry); + metadata.MergeMetadata(userlistEntry); + + return toYaml(file, metadata); + } + YAML::Node toYaml(std::shared_ptr plugin, const PluginMetadata& metadata) { BOOST_LOG_TRIVIAL(info) << "Using message language: " << state_.getLanguage().GetName(); diff --git a/src/gui/query/update_masterlist_query.h b/src/gui/query/update_masterlist_query.h index 5997e6e9..bb4fe53b 100644 --- a/src/gui/query/update_masterlist_query.h +++ b/src/gui/query/update_masterlist_query.h @@ -53,7 +53,6 @@ private: } catch (std::exception&) { try { game_.LoadMetadata(); - game_.EvaluateLoadedMetadata(); } catch (...) {} throw; } diff --git a/src/gui/state/game.cpp b/src/gui/state/game.cpp index d29bb560..b3a860ba 100644 --- a/src/gui/state/game.cpp +++ b/src/gui/state/game.cpp @@ -302,7 +302,7 @@ void Game::DecrementLoadOrderSortCount() { } std::vector Game::GetMessages() const { - std::vector output(gameHandle_->GetDatabase()->GetGeneralMessages()); + std::vector output(gameHandle_->GetDatabase()->GetGeneralMessages(true)); output.insert(end(output), begin(messages_), end(messages_)); if (loadOrderSortCount_ == 0) @@ -324,12 +324,7 @@ void Game::ClearMessages() { } bool Game::UpdateMasterlist() { - if (gameHandle_->GetDatabase()->UpdateMasterlist(MasterlistPath().string(), RepoURL(), RepoBranch())) { - EvaluateLoadedMetadata(); - return true; - } - - return false; + return gameHandle_->GetDatabase()->UpdateMasterlist(MasterlistPath().string(), RepoURL(), RepoBranch()); } MasterlistInfo Game::GetMasterlistInfo() const { @@ -371,20 +366,18 @@ void Game::LoadMetadata() { } } -void Game::EvaluateLoadedMetadata() { - return gameHandle_->GetDatabase()->EvalLists(); -} - std::set Game::GetKnownBashTags() const { return gameHandle_->GetDatabase()->GetKnownBashTags(); } -PluginMetadata Game::GetMasterlistMetadata(const std::string & pluginName) const { - return gameHandle_->GetDatabase()->GetPluginMetadata(pluginName, false); +PluginMetadata Game::GetMasterlistMetadata(const std::string & pluginName, + bool evaluateConditions) const { + return gameHandle_->GetDatabase()->GetPluginMetadata(pluginName, false, evaluateConditions); } -PluginMetadata Game::GetUserMetadata(const std::string & pluginName) const { - return gameHandle_->GetDatabase()->GetPluginUserMetadata(pluginName); +PluginMetadata Game::GetUserMetadata(const std::string & pluginName, + bool evaluateConditions) const { + return gameHandle_->GetDatabase()->GetPluginUserMetadata(pluginName, evaluateConditions); } void Game::AddUserMetadata(const PluginMetadata & metadata) { diff --git a/src/gui/state/game.h b/src/gui/state/game.h index c4d26ffe..fa4a4425 100644 --- a/src/gui/state/game.h +++ b/src/gui/state/game.h @@ -81,10 +81,11 @@ public: MasterlistInfo GetMasterlistInfo() const; void LoadMetadata(); - void EvaluateLoadedMetadata(); std::set GetKnownBashTags() const; - PluginMetadata GetMasterlistMetadata(const std::string& pluginName) const; - PluginMetadata GetUserMetadata(const std::string& pluginName) const; + PluginMetadata GetMasterlistMetadata(const std::string& pluginName, + bool evaluateConditions = false) const; + PluginMetadata GetUserMetadata(const std::string& pluginName, + bool evaluateConditions = false) const; void AddUserMetadata(const PluginMetadata& metadata); void ClearUserMetadata(const std::string& pluginName); diff --git a/src/tests/api/interface/api_game_operations_test.h b/src/tests/api/interface/api_game_operations_test.h index 051ffe16..b7aa00f2 100644 --- a/src/tests/api/interface/api_game_operations_test.h +++ b/src/tests/api/interface/api_game_operations_test.h @@ -67,6 +67,7 @@ protected: << "globals:" << endl << " - type: say" << endl << " content: '" << generalMasterlistMessage << "'" << endl + << " condition: 'file(\"" << missingEsp << "\")'" << endl << "plugins:" << endl << " - name: " << blankEsm << endl << " after:" << endl @@ -74,6 +75,7 @@ protected: << " msg:" << endl << " - type: say" << endl << " content: '" << noteMessage << "'" << endl + << " condition: 'file(\"" << missingEsp << "\")'" << endl << " tag:" << endl << " - Actors.ACBS" << endl << " - Actors.AIData" << endl diff --git a/src/tests/api/interface/database_interface_test.h b/src/tests/api/interface/database_interface_test.h index 773fa3e0..a06ca3e7 100644 --- a/src/tests/api/interface/database_interface_test.h +++ b/src/tests/api/interface/database_interface_test.h @@ -112,7 +112,10 @@ protected: << " - " << blankDifferentEsm << endl << " - name: " << blankDifferentEsp << endl << " inc:" << endl - << " - " << blankEsp << endl; + << " - " << blankEsp << endl + << " tag:" << endl + << " - name: C.Climate" << endl + << " condition: 'file(\"" << missingEsp << "\")'" << endl; userlist.close(); } @@ -349,6 +352,15 @@ TEST_P(DatabaseInterfaceTest, getGeneralMessagesShouldGetGeneralMessagesFromTheM EXPECT_EQ(expectedMessages, messages); } +TEST_P(DatabaseInterfaceTest, getGeneralMessagesShouldReturnOnlyValidMessagesIfConditionsAreEvaluated) { + ASSERT_NO_THROW(GenerateMasterlist()); + ASSERT_NO_THROW(db_->LoadLists(masterlistPath.string(), "")); + + auto messages = db_->GetGeneralMessages(true); + + EXPECT_TRUE(messages.empty()); +} + TEST_P(DatabaseInterfaceTest, getPluginMetadataShouldReturnAnEmptyPluginMetadataObjectIfThePluginHasNoMetadata) { auto metadata = db_->GetPluginMetadata(blankEsm); @@ -382,6 +394,15 @@ TEST_P(DatabaseInterfaceTest, getPluginMetadataShouldReturnOnlyMasterlistMetadat EXPECT_EQ(expectedLoadAfter, metadata.GetLoadAfterFiles()); } +TEST_P(DatabaseInterfaceTest, getPluginMetadataShouldReturnOnlyValidMetadataForTheGivenPluginIfConditionsAreEvaluated) { + ASSERT_NO_THROW(GenerateMasterlist()); + ASSERT_NO_THROW(db_->LoadLists(masterlistPath.string(), "")); + + auto metadata = db_->GetPluginMetadata(blankEsm, false, true); + + EXPECT_TRUE(metadata.GetMessages().empty()); +} + TEST_P(DatabaseInterfaceTest, getPluginUserMetadataShouldReturnAnEmptyPluginMetadataObjectIfThePluginHasNoUserMetadata) { ASSERT_NO_THROW(GenerateMasterlist()); ASSERT_NO_THROW(GenerateUserlist()); @@ -405,6 +426,16 @@ TEST_P(DatabaseInterfaceTest, getPluginUserMetadataShouldReturnOnlyUserMetadataF EXPECT_EQ(expectedLoadAfter, metadata.GetLoadAfterFiles()); } +TEST_P(DatabaseInterfaceTest, getPluginUserMetadataShouldReturnOnlyValidMetadataForTheGivenPluginIfConditionsAreEvaluated) { + ASSERT_NO_THROW(GenerateMasterlist()); + ASSERT_NO_THROW(GenerateUserlist()); + ASSERT_NO_THROW(db_->LoadLists(masterlistPath.string(), userlistPath_.string())); + + auto metadata = db_->GetPluginMetadata(blankEsm, false, true); + + EXPECT_TRUE(metadata.GetMessages().empty()); +} + TEST_P(DatabaseInterfaceTest, setPluginUserMetadataShouldReplaceExistingUserMetadataWithTheGivenMetadata) { ASSERT_NO_THROW(GenerateMasterlist()); ASSERT_NO_THROW(GenerateUserlist()); @@ -451,7 +482,6 @@ TEST_P(DatabaseInterfaceTest, discardPluginUserMetadataShouldDiscardAllUserMetad auto metadata = db_->GetPluginUserMetadata(blankEsm); EXPECT_TRUE(metadata.HasNameOnly()); - } TEST_P(DatabaseInterfaceTest, discardPluginUserMetadataShouldNotDiscardMasterlistMetadataForTheGivenPlugin) {