diff --git a/docs/api/sorting.rst b/docs/api/sorting.rst index d278d74b..38939f6a 100644 --- a/docs/api/sorting.rst +++ b/docs/api/sorting.rst @@ -26,7 +26,7 @@ Create plugin graph vertices Once loaded, a directed graph is created and the plugins are added to it in lexicographical order as vertices. Any metadata a plugin has in the masterlist and userlist are then merged into its vertex's data store. Plugin group -dependencies are also resolved and added as additional load after plugins. +dependencies are also resolved and added as group-derived plugins. Create plugin graph edges ============================== @@ -46,6 +46,12 @@ For each plugin: 4. Add edges coming from all the plugin's load after files that are installed plugins. +Group-derived interdependencies are then evaluated. Each plugin's group-derived +plugins are iterated over and individually checked to see if adding an edge from +the group-derived plugin to the plugin would cause a cycle, and if not the edge +is recorded. Once all potential edges have been checked, the recorded edges are +added to the graph. + At this point, all explicit interdependencies have been graphed. Plugin priority metadata values must now be propagated down the dependency trees to ensure that priority edges are added correctly later in the process. To do this: diff --git a/docs/metadata/data_structures/group.rst b/docs/metadata/data_structures/group.rst index 5eaf49c4..01d4b4da 100644 --- a/docs/metadata/data_structures/group.rst +++ b/docs/metadata/data_structures/group.rst @@ -19,8 +19,12 @@ This structure can be used to hold group definitions. It is a key-value map. The names of groups that this group loads after. Group names are case-sensitive. If undefined, the set is empty. The named groups must be defined when LOOT sorts plugins, but they don't need to be defined in the same - metadata file. If at sort time a group is defined to load after a group that - does not exist, a sorting error will occur. + metadata file. + + Sorting errors will occur if: + + - A group loads after another group that does not exist. + - Group loading is cyclic (e.g. A loads after B and B loads after A). Merging Groups -------------- diff --git a/docs/metadata/data_structures/plugin.rst b/docs/metadata/data_structures/plugin.rst index c647ae23..4b420f80 100644 --- a/docs/metadata/data_structures/plugin.rst +++ b/docs/metadata/data_structures/plugin.rst @@ -25,10 +25,33 @@ This is the structure that brings all the others together, and forms the main co be defined in the same metadata file. If at sort time the group does not exist, a sorting error will occur. - A plugin must load after all the plugins in the groups its group is defined to - load after. Group loading is resolved recursively. For example, if group C - loads after group B, and group B loads after group A, a plugin in C must load - after all the plugins in A even if no plugins in B are installed. + The plugin must load after all the plugins in the groups its group is defined + to load after, resolving them recursively. An exception exists if doing so + would introduce a cyclic dependency between two plugins without any other + group loading rules applied. + + For example, if for plugins A.esp, B.esp, C.esp and D.esp: + + - B.esp has A.esp as a master + - A.esp is in group A + - B.esp and C.esp are in the default group + - D.esp is in group D + - group A loads after the default group + - the default group loads after group D + + Then the load order must be D.esp, C.esp, A.esp, B.esp. Although A.esp's group + must load after B.esp's group, this would cause a cycle between A.esp and + B.esp, so the requirement is ignored for that pair of plugins. + + However, if for plugins A.esp, B.esp and C.esp in groups of the same names: + + 1. group B loads after group A + 2. group C loads after group B + 3. A.esp has C.esp as a master + + This will cause a sorting error, as neither group rule introduces a cyclic + dependency when combined in isolation with the third rule, but having all + three rules applied causes a cycle. .. describe:: priority diff --git a/src/api/sorting/plugin_sorter.cpp b/src/api/sorting/plugin_sorter.cpp index 09066f3f..f76ef093 100644 --- a/src/api/sorting/plugin_sorter.cpp +++ b/src/api/sorting/plugin_sorter.cpp @@ -131,6 +131,8 @@ std::vector PluginSorter::Sort(Game& game) { } AddSpecificEdges(); + AddGroupEdges(); + PropagatePriorities(); if (logger_) { @@ -268,11 +270,7 @@ void PluginSorter::AddPluginVertices(Game& game) { "\" set for plugin \"" + plugin.GetName() + "\" does not exist."); } else { - auto loadAfter = plugin.GetLoadAfterFiles(); - for (const auto& afterPlugin : groupsIt->second) { - loadAfter.insert(File(afterPlugin)); - } - plugin.SetLoadAfterFiles(loadAfter); + plugin.SetAfterGroupPlugins(groupsIt->second); } } @@ -473,6 +471,41 @@ void PluginSorter::AddSpecificEdges() { } } +void PluginSorter::AddGroupEdges() { + std::vector> acyclicEdgePairs; + for (const vertex_t& vertex : + boost::make_iterator_range(boost::vertices(graph_))) { + if (logger_) { + logger_->trace("Checking group edges for \"{}\".", + graph_[vertex].GetName()); + } + for (const auto& pluginName : graph_[vertex].GetAfterGroupPlugins()) { + vertex_t parentVertex; + if (GetVertexByName(pluginName, parentVertex)) { + if (EdgeCreatesCycle(parentVertex, vertex)) { + if (logger_) { + logger_->trace("Skipping edge from \"{}\" to \"{}\" as it would " + "create a cycle and one or both plugins belongs to the default " + "group.", + graph_[parentVertex].GetName(), + graph_[vertex].GetName()); + } + continue; + } + + acyclicEdgePairs.push_back(std::make_pair(parentVertex, vertex)); + } + } + } + + if (logger_) { + logger_->trace("Adding group edges that don't individually introduce cycles."); + } + for (const auto& edgePair : acyclicEdgePairs) { + AddEdge(edgePair.first, edgePair.second); + } +} + void PluginSorter::AddPriorityEdges() { for (const auto& vertex : boost::make_iterator_range(boost::vertices(graph_))) { diff --git a/src/api/sorting/plugin_sorter.h b/src/api/sorting/plugin_sorter.h index fe284fe3..94f7e006 100644 --- a/src/api/sorting/plugin_sorter.h +++ b/src/api/sorting/plugin_sorter.h @@ -61,6 +61,7 @@ private: void AddPluginVertices(Game& game); void AddSpecificEdges(); + void AddGroupEdges(); void AddPriorityEdges(); void AddOverlapEdges(); void AddTieBreakEdges(); diff --git a/src/api/sorting/plugin_sorting_data.cpp b/src/api/sorting/plugin_sorting_data.cpp index 78a9bd81..1966aa24 100644 --- a/src/api/sorting/plugin_sorting_data.cpp +++ b/src/api/sorting/plugin_sorting_data.cpp @@ -51,4 +51,12 @@ bool PluginSortingData::DoFormIDsOverlap( const PluginSortingData& plugin) const { return plugin_.DoFormIDsOverlap(plugin.plugin_); } + +std::unordered_set PluginSortingData::GetAfterGroupPlugins() const { + return afterGroupPlugins_; +} + +void PluginSortingData::SetAfterGroupPlugins(std::unordered_set plugins) { + afterGroupPlugins_ = plugins; +} } diff --git a/src/api/sorting/plugin_sorting_data.h b/src/api/sorting/plugin_sorting_data.h index 81155877..f1fe4a1e 100644 --- a/src/api/sorting/plugin_sorting_data.h +++ b/src/api/sorting/plugin_sorting_data.h @@ -40,6 +40,9 @@ public: size_t NumOverrideFormIDs() const; bool DoFormIDsOverlap(const PluginSortingData& plugin) const; + std::unordered_set GetAfterGroupPlugins() const; + void SetAfterGroupPlugins(std::unordered_set plugins); + using PluginMetadata::GetGlobalPriority; using PluginMetadata::GetGroup; using PluginMetadata::GetLoadAfterFiles; @@ -47,10 +50,10 @@ public: using PluginMetadata::GetRequirements; using PluginMetadata::SetGlobalPriority; using PluginMetadata::SetLocalPriority; - using PluginMetadata::SetLoadAfterFiles; private: const Plugin& plugin_; + std::unordered_set afterGroupPlugins_; }; } diff --git a/src/tests/api/internals/sorting/plugin_sorter_test.h b/src/tests/api/internals/sorting/plugin_sorter_test.h index 3fe46906..31c88599 100644 --- a/src/tests/api/internals/sorting/plugin_sorter_test.h +++ b/src/tests/api/internals/sorting/plugin_sorter_test.h @@ -87,7 +87,10 @@ protected: << " - group1" << endl << " - name: group3" << endl << " after:" << endl - << " - group2" << endl; + << " - group2" << endl + << " - name: group4" << endl + << " after:" << endl + << " - default" << endl; masterlist.close(); } @@ -207,15 +210,18 @@ TEST_P(PluginSorterTest, sortingShouldThrowIfAPluginHasAGroupThatDoesNotExist) { EXPECT_THROW(ps.Sort(game_), std::invalid_argument); } -TEST_P(PluginSorterTest, sortingShouldThrowIfAGroupIntroducesACycle) { +TEST_P(PluginSorterTest, sortingShouldThrowIfAddingTwoGroupEdgesIntroducesACycle) { ASSERT_NO_THROW(loadInstalledPlugins(game_, false)); GenerateMasterlist(); game_.GetDatabase()->LoadLists(masterlistPath_.string()); - PluginMetadata plugin(blankDifferentEsm); + PluginMetadata plugin(blankMasterDependentEsm); plugin.SetGroup("group1"); - plugin.SetLoadAfterFiles({ File(blankEsm) }); + game_.GetDatabase()->SetPluginUserMetadata(plugin); + + plugin = PluginMetadata(blankDifferentEsm); + plugin.SetGroup("group2"); game_.GetDatabase()->SetPluginUserMetadata(plugin); plugin = PluginMetadata(blankEsm); @@ -226,6 +232,39 @@ TEST_P(PluginSorterTest, sortingShouldThrowIfAGroupIntroducesACycle) { EXPECT_THROW(ps.Sort(game_), CyclicInteractionError); } +TEST_P(PluginSorterTest, sortingShouldIgnoreAGroupEdgeIfItWouldCauseACycle) { + ASSERT_NO_THROW(loadInstalledPlugins(game_, false)); + + GenerateMasterlist(); + game_.GetDatabase()->LoadLists(masterlistPath_.string()); + + PluginMetadata plugin(blankEsm); + plugin.SetGroup("group4"); + game_.GetDatabase()->SetPluginUserMetadata(plugin); + + PluginSorter ps; + std::vector expectedSortedOrder({ + masterFile, + blankDifferentEsm, + blankDifferentMasterDependentEsm, + blankEsm, + blankMasterDependentEsm, + blankEsp, + blankDifferentEsp, + blankMasterDependentEsp, + blankDifferentMasterDependentEsp, + blankPluginDependentEsp, + blankDifferentPluginDependentEsp, + }); + + if (GetParam() == GameType::fo4 || GetParam() == GameType::tes5se) { + expectedSortedOrder.insert(expectedSortedOrder.begin() + 3, blankEsl); + } + + std::vector sorted = ps.Sort(game_); + EXPECT_EQ(expectedSortedOrder, sorted); +} + TEST_P(PluginSorterTest, sortingShouldEvaluateRelativeGlobalPriorities) { ASSERT_NO_THROW(loadInstalledPlugins(game_, false)); PluginMetadata plugin(blankDifferentMasterDependentEsp);