From 9a45115ee405dcb1e734c7b5e4f06eccb36701f2 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Fri, 23 Aug 2024 19:19:55 +0100 Subject: [PATCH] Don't throw during sort if a non-blueprint-master has a blueprint master as a master The game will not load the blueprint master before the other plugin, so if the latter overrides any records from the former, those changes won't be seen in-game. There are probably many (pre-CK) plugins that have the official BlueprintShips-Starfield.esm blueprint master as a master but not actually override any of its records. Ideally such plugins would be updated to remove the unnecessary master or uninstalled, but as having it doesn't seem to do any harm, I don't think it's worth preventing LOOT from sorting in this situation. --- src/api/sorting/plugin_sort.cpp | 30 +++++++--- .../api/internals/sorting/plugin_sort_test.h | 55 ++++++++++--------- 2 files changed, 50 insertions(+), 35 deletions(-) diff --git a/src/api/sorting/plugin_sort.cpp b/src/api/sorting/plugin_sort.cpp index 1522e281..e300ee73 100644 --- a/src/api/sorting/plugin_sort.cpp +++ b/src/api/sorting/plugin_sort.cpp @@ -92,6 +92,8 @@ void ValidateSpecificAndHardcodedEdges( const std::vector::const_iterator& firstNonMaster, const std::vector::const_iterator& end, const std::vector& hardcodedPlugins) { + const auto logger = getLogger(); + const auto isNonMaster = [&](const std::string& name) { return IsInRange(firstNonMaster, end, name); }; @@ -107,10 +109,16 @@ void ValidateSpecificAndHardcodedEdges( Vertex(it->GetName(), EdgeType::masterFlag)}); } - if (isBlueprintMaster(master)) { - throw CyclicInteractionError(std::vector{ - Vertex(master, EdgeType::master), - Vertex(it->GetName(), EdgeType::blueprintMaster)}); + if (isBlueprintMaster(master) && logger) { + // Log a warning instead of throwing an exception because the game will + // just ignore this master, and the issue can't be fixed without + // editing the plugin and the blueprint master may not actually have + // any of its records overridden. + logger->warn( + "The master plugin \"{}\" has the blueprint master \"{}\" as one " + "of its masters", + it->GetName(), + master); } } @@ -177,10 +185,16 @@ void ValidateSpecificAndHardcodedEdges( for (auto it = firstNonMaster; it != end; ++it) { for (const auto& master : it->GetMasters()) { - if (isBlueprintMaster(master)) { - throw CyclicInteractionError(std::vector{ - Vertex(master, EdgeType::master), - Vertex(it->GetName(), EdgeType::blueprintMaster)}); + if (isBlueprintMaster(master) && logger) { + // Log a warning instead of throwing an exception because the game will + // just ignore this master, and the issue can't be fixed without + // editing the plugin and the blueprint master may not actually have + // any of its records overridden. + logger->warn( + "The non-master plugin \"{}\" has the blueprint master \"{}\" as " + "one of its masters", + it->GetName(), + master); } } diff --git a/src/tests/api/internals/sorting/plugin_sort_test.h b/src/tests/api/internals/sorting/plugin_sort_test.h index c307614c..aaadd432 100644 --- a/src/tests/api/internals/sorting/plugin_sort_test.h +++ b/src/tests/api/internals/sorting/plugin_sort_test.h @@ -645,8 +645,9 @@ TEST_P(PluginSortTest, } } -TEST_P(PluginSortTest, - sortingShouldThrowIfAMasterEdgeWouldPutABlueprintMasterBeforeAMaster) { +TEST_P( + PluginSortTest, + sortingShouldNotThrowIfAMasterEdgeWouldPutABlueprintMasterBeforeAMaster) { if (GetParam() != GameType::starfield) { return; } @@ -655,21 +656,26 @@ TEST_P(PluginSortTest, ASSERT_NO_THROW(loadInstalledPlugins(game_, false)); - try { - SortPlugins(game_, game_.GetLoadOrder()); - FAIL(); - } catch (const CyclicInteractionError& e) { - ASSERT_EQ(2, e.GetCycle().size()); - EXPECT_EQ(blankFullEsm, e.GetCycle()[0].GetName()); - EXPECT_EQ(EdgeType::master, e.GetCycle()[0].GetTypeOfEdgeToNextVertex()); - EXPECT_EQ(blankMasterDependentEsm, e.GetCycle()[1].GetName()); - EXPECT_EQ(EdgeType::blueprintMaster, - e.GetCycle()[1].GetTypeOfEdgeToNextVertex()); - } + const auto sorted = SortPlugins(game_, game_.GetLoadOrder()); + + EXPECT_EQ(std::vector({ + masterFile, + blankEsm, + blankDifferentEsm, + blankMasterDependentEsm, + blankMediumEsm, + blankEsl, + blankEsp, + blankDifferentEsp, + blankMasterDependentEsp, + blankFullEsm, + }), + sorted); } -TEST_P(PluginSortTest, - sortingShouldThrowIfAMasterEdgeWouldPutABlueprintMasterBeforeANonMaster) { +TEST_P( + PluginSortTest, + sortingShouldNotThrowIfAMasterEdgeWouldPutABlueprintMasterBeforeANonMaster) { if (GetParam() != GameType::starfield) { return; } @@ -687,17 +693,11 @@ TEST_P(PluginSortTest, CreatePluginSortingData(esp->GetName()), CreatePluginSortingData(blueprint->GetName())}; - try { - SortPlugins(std::move(pluginsSortingData), {Group()}, {}, {}); - FAIL(); - } catch (const CyclicInteractionError& e) { - ASSERT_EQ(2, e.GetCycle().size()); - EXPECT_EQ(blankFullEsm, e.GetCycle()[0].GetName()); - EXPECT_EQ(EdgeType::master, e.GetCycle()[0].GetTypeOfEdgeToNextVertex()); - EXPECT_EQ(blankMasterDependentEsp, e.GetCycle()[1].GetName()); - EXPECT_EQ(EdgeType::blueprintMaster, - e.GetCycle()[1].GetTypeOfEdgeToNextVertex()); - } + const auto sorted = + SortPlugins(std::move(pluginsSortingData), {Group()}, {}, {}); + + EXPECT_EQ(std::vector({blankMasterDependentEsp, blankFullEsm}), + sorted); } TEST_P( @@ -987,7 +987,8 @@ TEST_P( std::move(pluginsSortingData), {Group()}, {}, {blueprint->GetName()}); EXPECT_EQ(std::vector({ - blankEsm, blankDifferentEsm, + blankEsm, + blankDifferentEsm, }), sorted); }