Fix for stack overflow when checking for circular culture parents

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<Mod>());
 	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);
+	}
 }

ImperatorToCK3/CK3/Cultures/CultureCollection.cs

@@ -130,51 +130,51 @@
 		return cultureData;
 	}
 
 	private void ValidateAndLoadCultures(OrderedDictionary<string, CultureData> culturesData, Configuration config) {
 		foreach (var (cultureId, data) in culturesData) {
 			if (data.InvalidatingCultureIds.Count != 0) {
 				bool isInvalidated = false;
 				foreach (var existingCulture in this) {
 					if (!data.InvalidatingCultureIds.Contains(existingCulture.Id)) {
 						continue;
 					}
 					Logger.Debug($"Culture {cultureId} is invalidated by existing {existingCulture.Id}.");
 					cultureReplacements[cultureId] = existingCulture.Id;
 					isInvalidated = true;
 				}
 				if (isInvalidated) {
 					continue;
 				}
 				Logger.Debug($"Loading optional culture {cultureId}...");
 			}
 			if (data.Heritage is null) {
 				// Special handling for TFE hunnic culture. #TODO: remove this when it's fixed on TFE side.
 				if (config.FallenEagleEnabled && cultureId == "hunnic" && PillarCollection.GetHeritageForId("heritage_turkic") is Pillar turkicHeritage) {
 					Logger.Debug("Applying turkic heritage to TFE hunnic culture.");
 					data.Heritage = turkicHeritage;
 				} else {
 					Logger.Warn($"Culture {cultureId} has no valid heritage defined! Skipping.");
 					continue;
 				}
 			}
 			if (data.Language is null) {
 				Logger.Warn($"Culture {cultureId} has no valid language defined! Skipping.");
 				continue;
 			}
 			if (data.NameLists.Count == 0) {
 				Logger.Warn($"Culture {cultureId} has no name list defined! Skipping.");
 				continue;
 			}
 			if (data.Color is null) {
 				Logger.Warn($"Culture {cultureId} has no color defined! Will use generated color.");
 				var color = new ColorHash().Rgb(cultureId);
 				data.Color = new Color(color.R, color.G, color.B);
 			}
 
 			AddOrReplace(new Culture(cultureId, data));
 		}
 	}
 
 	private void ReplaceInvalidatedParents() {
 		// Replace invalidated cultures in parent culture lists.
 		foreach (var culture in this) {
@@ -226,7 +226,7 @@
 		return cultureMapper.Match(irCulture, ck3ProvinceId, irProvinceId, country.HistoricalTag);
 	}
 
 	public void ImportTechnology(CountryCollection countries, CultureMapper cultureMapper, ProvinceMapper provinceMapper, InventionsDB inventionsDB, LocDB irLocDB, OrderedDictionary<string, bool> ck3ModFlags) { // TODO: add tests for this
 		Logger.Info("Converting Imperator inventions to CK3 innovations...");
 
 		var innovationMapper = new InnovationMapper();
@@ -258,33 +258,35 @@
 		// 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<string> GetAncestorsOfCulture(Culture cultureToCheck, HashSet<string>? alreadyChecked = null) {
+	private HashSet<string> GetAncestorsOfCulture(Culture cultureToCheck, HashSet<string> alreadyChecked) {
 		HashSet<string> 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;