Skip to content

Commit

Permalink
Don't throw during sort if a non-blueprint-master has a blueprint mas…
Browse files Browse the repository at this point in the history
…ter 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.
  • Loading branch information
Ortham committed Aug 24, 2024
1 parent ee7b8d3 commit 9a45115
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 35 deletions.
30 changes: 22 additions & 8 deletions src/api/sorting/plugin_sort.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ void ValidateSpecificAndHardcodedEdges(
const std::vector<PluginSortingData>::const_iterator& firstNonMaster,
const std::vector<PluginSortingData>::const_iterator& end,
const std::vector<std::string>& hardcodedPlugins) {
const auto logger = getLogger();

const auto isNonMaster = [&](const std::string& name) {
return IsInRange(firstNonMaster, end, name);
};
Expand All @@ -107,10 +109,16 @@ void ValidateSpecificAndHardcodedEdges(
Vertex(it->GetName(), EdgeType::masterFlag)});
}

if (isBlueprintMaster(master)) {
throw CyclicInteractionError(std::vector<Vertex>{
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);
}
}

Expand Down Expand Up @@ -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>{
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);
}
}

Expand Down
55 changes: 28 additions & 27 deletions src/tests/api/internals/sorting/plugin_sort_test.h
Original file line number Diff line number Diff line change
Expand Up @@ -645,8 +645,9 @@ TEST_P(PluginSortTest,
}
}

TEST_P(PluginSortTest,
sortingShouldThrowIfAMasterEdgeWouldPutABlueprintMasterBeforeAMaster) {
TEST_P(
PluginSortTest,
sortingShouldNotThrowIfAMasterEdgeWouldPutABlueprintMasterBeforeAMaster) {
if (GetParam() != GameType::starfield) {
return;
}
Expand All @@ -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<std::string>({
masterFile,
blankEsm,
blankDifferentEsm,
blankMasterDependentEsm,
blankMediumEsm,
blankEsl,
blankEsp,
blankDifferentEsp,
blankMasterDependentEsp,
blankFullEsm,
}),
sorted);
}

TEST_P(PluginSortTest,
sortingShouldThrowIfAMasterEdgeWouldPutABlueprintMasterBeforeANonMaster) {
TEST_P(
PluginSortTest,
sortingShouldNotThrowIfAMasterEdgeWouldPutABlueprintMasterBeforeANonMaster) {
if (GetParam() != GameType::starfield) {
return;
}
Expand All @@ -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<std::string>({blankMasterDependentEsp, blankFullEsm}),
sorted);
}

TEST_P(
Expand Down Expand Up @@ -987,7 +987,8 @@ TEST_P(
std::move(pluginsSortingData), {Group()}, {}, {blueprint->GetName()});

EXPECT_EQ(std::vector<std::string>({
blankEsm, blankDifferentEsm,
blankEsm,
blankDifferentEsm,
}),
sorted);
}
Expand Down

0 comments on commit 9a45115

Please sign in to comment.