diff --git a/ImperatorToCK3.UnitTests/CK3/Cultures/CultureCollectionTests.cs b/ImperatorToCK3.UnitTests/CK3/Cultures/CultureCollectionTests.cs index 55945349f..381da1054 100644 --- a/ImperatorToCK3.UnitTests/CK3/Cultures/CultureCollectionTests.cs +++ b/ImperatorToCK3.UnitTests/CK3/Cultures/CultureCollectionTests.cs @@ -6,10 +6,12 @@ using ImperatorToCK3.UnitTests.TestHelpers; using System; using System.Collections.Generic; +using System.IO; using Xunit; namespace ImperatorToCK3.UnitTests.CK3.Cultures; +[Collection("Sequential")] public class CultureCollectionTests { private static readonly ModFilesystem ck3ModFS = new("TestFiles/CK3/game", Array.Empty()); private static readonly PillarCollection pillars; @@ -78,4 +80,55 @@ public void ConverterLanguageCanBeMergedIntoExistingLanguage() { Assert.Equal("language_illyrian", cultures["albanian"].Language.Id); Assert.Equal("language_illyrian", cultures["dalmatian"].Language.Id); } + + [Fact] + public void WarnAboutCircularParentsLogsCorrectWarningsForAPairOfCultures() { + var cultures = new TestCK3CultureCollection(); + + // Create a circular dependency by making "french" a child of "roman" + // and "roman" a child of "french". + cultures.GenerateTestCulture("roman", "heritage_latin"); + cultures.GenerateTestCulture("french", "heritage_latin"); + cultures["french"].ParentCultureIds.Add("roman"); + cultures["roman"].ParentCultureIds.Add("french"); + + var output = new StringWriter(); + Console.SetOut(output); + cultures.WarnAboutCircularParents(); + var outputString = output.ToString(); + + Assert.Contains("[ERROR] Culture french is set as its own direct or indirect parent!", outputString); + Assert.Contains("[ERROR] Culture roman is set as its own direct or indirect parent!", outputString); + } + + [Fact] + public void WarnAboutCircularParentsLogsCorrectWarningForCultureBeingItsOwnParent() { + var cultures = new TestCK3CultureCollection(); + // Create a culture that is its own parent. + cultures.GenerateTestCulture("roman", "heritage_latin"); + cultures["roman"].ParentCultureIds.Add("roman"); + + var output = new StringWriter(); + Console.SetOut(output); + cultures.WarnAboutCircularParents(); + var outputString = output.ToString(); + + Assert.Contains("[ERROR] Culture roman is set as its own direct or indirect parent!", outputString); + } + + [Fact] + public void WarnAboutCircularParentsDoesNotLogAnythingForValidCultures() { + var cultures = new TestCK3CultureCollection(); + // Just French being a child of Roman, no circular dependency. + cultures.GenerateTestCulture("roman", "heritage_latin"); + cultures.GenerateTestCulture("french", "heritage_latin"); + cultures["french"].ParentCultureIds.Add("roman"); + + var output = new StringWriter(); + Console.SetOut(output); + cultures.WarnAboutCircularParents(); + var outputString = output.ToString(); + + Assert.DoesNotContain("[ERROR]", outputString); + } } \ No newline at end of file diff --git a/ImperatorToCK3/CK3/Cultures/CultureCollection.cs b/ImperatorToCK3/CK3/Cultures/CultureCollection.cs index e0413c6b8..a868d55ed 100644 --- a/ImperatorToCK3/CK3/Cultures/CultureCollection.cs +++ b/ImperatorToCK3/CK3/Cultures/CultureCollection.cs @@ -258,33 +258,35 @@ internal void WarnAboutCircularParents() { // For every culture, check if it isn't set as its own immediate or distant parent. Logger.Debug("Checking for circular culture parents..."); foreach (var culture in this) { - var allParents = GetAncestorsOfCulture(culture); + var allParents = GetAncestorsOfCulture(culture, alreadyChecked: []); if (allParents.Contains(culture.Id)) { - Logger.Error($"Culture {culture.Id} is set as its own parent!"); + Logger.Error($"Culture {culture.Id} is set as its own direct or indirect parent!"); } } } - private HashSet GetAncestorsOfCulture(Culture cultureToCheck, HashSet? alreadyChecked = null) { + private HashSet GetAncestorsOfCulture(Culture cultureToCheck, HashSet alreadyChecked) { HashSet allParents = []; // Get immediate parents. foreach (var parentCultureId in cultureToCheck.ParentCultureIds) { - // Avoid infinite recursion. - if (alreadyChecked?.Contains(parentCultureId) == true) { + allParents.Add(parentCultureId); + } + + // Prevent infinite recursion. + alreadyChecked.Add(cultureToCheck.Id); + + // For parents that haven't been checked yet, get their parents. + foreach (var parentCultureId in cultureToCheck.ParentCultureIds) { + if (alreadyChecked.Contains(parentCultureId)) { continue; } - - allParents.Add(parentCultureId); - if (!TryGetValue(parentCultureId, out var parentCulture)) { - Logger.Warn($"Parent culture {parentCultureId} not found for culture {cultureToCheck.Id}."); + Logger.Warn($"Parent culture {parentCultureId} not found for culture {cultureToCheck.Id}!"); continue; } - - // Add the parent's parents. - var parentParents = GetAncestorsOfCulture(parentCulture, allParents); - allParents.UnionWith(parentParents); + var parentAncestors = GetAncestorsOfCulture(parentCulture, alreadyChecked); + allParents.UnionWith(parentAncestors); } return allParents;