Fix for stack overflow when checking for circular culture parents (#2612) #patch

Sentry event ID: 0ebe38c3b4b044eba5ab8406481e650f

Author: IhateTrains

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

@@ -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<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;