Fix some warnings about possible null (#344)

* Update Fronter * Fix some warnings * Add tests * Formatting
ParadoxGameConverters · Oct 9, 2021 · 2c6460b · 2c6460b
1 parent 6351670
commit 2c6460b
Show file tree

Hide file tree

Showing 4 changed files with 57 additions and 16 deletions.
diff --git a/Fronter b/Fronter
diff --git a/ImperatorToCK3.UnitTests/CK3/Characters/CK3CharacterTests.cs b/ImperatorToCK3.UnitTests/CK3/Characters/CK3CharacterTests.cs
@@ -1,15 +1,15 @@
-using System;
-using commonItems;
+using commonItems;
 using ImperatorToCK3.CK3.Characters;
-using ImperatorToCK3.Mappers.Religion;
-using Xunit;
 using ImperatorToCK3.Mappers.Culture;
-using ImperatorToCK3.Mappers.Trait;
-using ImperatorToCK3.Mappers.Nickname;
+using ImperatorToCK3.Mappers.DeathReason;
 using ImperatorToCK3.Mappers.Localization;
+using ImperatorToCK3.Mappers.Nickname;
 using ImperatorToCK3.Mappers.Province;
-using ImperatorToCK3.Mappers.DeathReason;
+using ImperatorToCK3.Mappers.Religion;
+using ImperatorToCK3.Mappers.Trait;
+using System;
 using System.IO;
+using Xunit;
 
 namespace ImperatorToCK3.UnitTests.CK3.Characters {
 	[Collection("Sequential")]
@@ -76,6 +76,7 @@ public CK3CharacterBuilder WithDeathReasonMapper(DeathReasonMapper deathReasonMa
 		}
 
 		private readonly CK3CharacterBuilder builder = new();
+
 		[Fact]
 		public void AllLinksCanBeRemoved() {
 			var imperatorCharacter = new ImperatorToCK3.Imperator.Characters.Character(1);
@@ -122,6 +123,34 @@ public void AllLinksCanBeRemoved() {
 			Assert.Empty(character.Children);
 			Assert.Empty(character.Spouses);
 		}
+		[Fact]
+		public void BreakAllLinksWarnsWhenSpouseIsNull() {
+			var output = new StringWriter();
+			Console.SetOut(output);
+
+			var character = builder.Build();
+			character.Spouses.Add("spouseId", null);
+			character.BreakAllLinks();
+			Assert.Contains("[WARN] Spouse spouseId of imperator0 is null!", output.ToString());
+		}
+		[Fact]
+		public void BreakAllLinksWarnsWhenChildIsNull() {
+			var output = new StringWriter();
+			Console.SetOut(output);
+
+			var male = builder.Build();
+			male.Children.Add("childId", null);
+			male.BreakAllLinks();
+			Assert.Contains("[WARN] Child childId of imperator0 is null!", output.ToString());
+			output.Flush();
+
+			var impFemaleReader = new BufferedReader("female = yes");
+			var impFemaleCharacter = ImperatorToCK3.Imperator.Characters.Character.Parse(impFemaleReader, "1", null);
+			var female = builder.WithImperatorCharacter(impFemaleCharacter).Build();
+			female.Children.Add("child2Id", null);
+			female.BreakAllLinks();
+			Assert.Contains("[WARN] Child child2Id of imperator1 is null!", output.ToString());
+		}
 
 		[Fact]
 		public void TraitsCanBeInitializedFromImperator() {

diff --git a/ImperatorToCK3/CK3/Characters/Character.cs b/ImperatorToCK3/CK3/Characters/Character.cs
@@ -239,16 +239,28 @@ public void BreakAllLinks() {
 			RemoveMother();
 			Father?.RemoveChild(ID);
 			RemoveFather();
-			foreach (var spouse in Spouses.Values) {
+			foreach (var (spouseId, spouse) in Spouses) {
+				if (spouse is null) {
+					Logger.Warn($"Spouse {spouseId} of {ID} is null!");
+					continue;
+				}
 				spouse.RemoveSpouse(ID);
 			}
 			Spouses.Clear();
 			if (Female) {
-				foreach (var child in Children.Values) {
+				foreach (var (childId, child) in Children) {
+					if (child is null) {
+						Logger.Warn($"Child {childId} of {ID} is null!");
+						continue;
+					}
 					child.RemoveMother();
 				}
 			} else {
-				foreach (var child in Children.Values) {
+				foreach (var (childId, child) in Children) {
+					if (child is null) {
+						Logger.Warn($"Child {childId} of {ID} is null!");
+						continue;
+					}
 					child.RemoveFather();
 				}
 			}

diff --git a/ImperatorToCK3/CK3/Dynasties/Dynasty.cs b/ImperatorToCK3/CK3/Dynasties/Dynasty.cs
@@ -1,7 +1,7 @@
-using System.Collections.Generic;
-using ImperatorToCK3.Mappers.Localization;
+using commonItems;
 using ImperatorToCK3.Imperator.Families;
-using commonItems;
+using ImperatorToCK3.Mappers.Localization;
+using System.Collections.Generic;
 
 namespace ImperatorToCK3.CK3.Dynasties {
 	public class Dynasty {
@@ -20,7 +20,7 @@ public Dynasty(Family imperatorFamily, LocalizationMapper localizationMapper) {
 			}
 
 			foreach (var member in imperatorMembers.Values) {
-				var ck3Member = (member as Imperator.Characters.Character).CK3Character;
+				var ck3Member = (member as Imperator.Characters.Character)?.CK3Character;
 				if (ck3Member is not null) {
 					ck3Member.DynastyID = ID;
 				}
@@ -42,7 +42,7 @@ public Dynasty(Family imperatorFamily, LocalizationMapper localizationMapper) {
 			}
 		}
 		public string ID { get; } = string.Empty;
-		public string Name { get; private set; } = string.Empty;
+		public string Name { get; } = string.Empty;
 		public string Culture { get; private set; } = string.Empty;
 
 		public KeyValuePair<string, LocBlock> Localization { get; } = new();