Detect invalid province mappings (#2772) #minor

This detects land -> water or water -> land province mappings.
Also fixed the few Invictus province mappings that were detected as
invalid.

Author: IhateTrains

ImperatorToCK3.UnitTests/Mappers/Province/ProvinceMapperTests.cs

@@ -1,4 +1,7 @@
-using ImperatorToCK3.Mappers.Province;
+using commonItems.Mods;
+using ImperatorToCK3.CommonUtils.Map;
+using ImperatorToCK3.Mappers.Province;
+using System;
 using System.IO;
 using Xunit;
 
@@ -8,7 +11,11 @@ namespace ImperatorToCK3.UnitTests.Mappers.Province;
 [CollectionDefinition("Sequential", DisableParallelization = true)]
 public class ProvinceMapperTests {
 	private const string TestFilesPath = "TestFiles/MapperTests/ProvinceMapper";
-
+	private const string ImperatorRoot = "TestFiles/MapperTests/ProvinceMapper/Imperator/game";
+	private const string CK3Root = "TestFiles/MapperTests/ProvinceMapper/CK3/game";
+	private static readonly ModFilesystem IRModFS = new(ImperatorRoot, []);
+	private static readonly ModFilesystem CK3ModFS = new(CK3Root, []);
+	
 	[Fact]
 	public void EmptyMappingsDefaultToEmpty() {
 		var mapper = new ProvinceMapper();
@@ -45,4 +52,23 @@ public void CanLookupCK3Provinces() {
 		Assert.Equal((ulong)2, mapper.GetCK3ProvinceNumbers(2)[0]);
 		Assert.Equal((ulong)1, mapper.GetCK3ProvinceNumbers(2)[1]);
 	}
+
+	[Fact]
+	public void TypeMismatchesAreDetected() {
+		var mapper = new ProvinceMapper();
+		var mappingsPath = Path.Combine(TestFilesPath, "type_mismatches.txt");
+		mapper.LoadMappings(mappingsPath);
+
+		var irMapData = new MapData(IRModFS);
+		var ck3MapData = new MapData(CK3ModFS);
+
+		var output = new StringWriter();
+		Console.SetOut(output);
+		
+		mapper.DetectInvalidMappings(irMapData, ck3MapData);
+		string log = output.ToString();
+		Assert.Contains("I:R land province 1 is mapped to CK3 water province 2! Fix the province mappings!", log);
+		Assert.Contains("I:R water province 2 is mapped to CK3 land province 1! Fix the province mappings!", log);
+		Assert.DoesNotContain(" 3 is mapped", log); // Mapping 3 -> 3 has land on both ends, so should not be reported.
+	}
 }

ImperatorToCK3.UnitTests/TestFiles/MapperTests/ProvinceMapper/CK3/game/map_data/default.map

@@ -0,0 +1,3 @@
+definitions = definition.csv
+uninhabitable = LIST { 1}
+sea_zones = LIST { 2 }

ImperatorToCK3.UnitTests/TestFiles/MapperTests/ProvinceMapper/CK3/game/map_data/definition.csv

@@ -0,0 +1,4 @@
+0;0;0;0;x;x;
+1;42;3;128;VESTFIRDIR;x;
+2;84;6;1;REYKJAVIK;x;
+3;126;9;2;AKUREYRI;x;

ImperatorToCK3.UnitTests/TestFiles/MapperTests/ProvinceMapper/Imperator/game/map_data/default.map

@@ -0,0 +1,3 @@
+definitions = definition.csv
+uninhabitable = LIST { 1}
+sea_zones = LIST { 2 }

ImperatorToCK3.UnitTests/TestFiles/MapperTests/ProvinceMapper/Imperator/game/map_data/definition.csv

@@ -0,0 +1,4 @@
+0;0;0;0;x;x;
+1;42;3;128;VESTFIRDIR;x;
+2;84;6;1;REYKJAVIK;x;
+3;126;9;2;AKUREYRI;x;

ImperatorToCK3.UnitTests/TestFiles/MapperTests/ProvinceMapper/type_mismatches.txt

@@ -0,0 +1,5 @@
+test_version = {
+	link = { imp = 1 ck3 = 2 } # land to water
+	link = { imp = 2 ck3 = 1 } # water to land
+	link = { imp = 3 ck3 = 3 } # land to land, OK
+}

ImperatorToCK3/CK3/World.cs

@@ -75,7 +75,7 @@ public World(Imperator.World impWorld, Configuration config, Thread? irCoaExtrac
 		Logger.Info("*** Hello CK3, let's get painting. ***");
 
 		warMapper.DetectUnmappedWarGoals(impWorld.ModFS);
-		
+
 		DetermineCK3Dlcs(config);
 		LoadAndDetectCK3Mods(config);
 
@@ -90,14 +90,14 @@ public World(Imperator.World impWorld, Configuration config, Thread? irCoaExtrac
 		WorldOutputter.CreateModFolder(outputModPath);
 		// This will also convert all Liquid templates into simple text files.
 		WorldOutputter.CopyBlankModFilesToOutput(outputModPath, config.GetCK3ModFlags());
-		
+
 		// Include a fake mod pointing to blankMod in the output folder.
 		LoadedMods.Add(new Mod("blankMod", outputModPath));
 		ModFS = new ModFilesystem(Path.Combine(config.CK3Path, "game"), LoadedMods);
 
 		var ck3Defines = new Defines();
 		ck3Defines.LoadDefines(ModFS);
-		
+
 		ColorFactory ck3ColorFactory = new();
 		// Now that we have the mod filesystem, we can initialize the localization database.
 		Parallel.Invoke(
@@ -121,10 +121,11 @@ public World(Imperator.World impWorld, Configuration config, Thread? irCoaExtrac
 				FileTweaker.ModifyAndRemovePartsOfFiles(ModFS, outputModPath, config).Wait();
 			}
 		);
-		
+
 		System.Collections.Generic.OrderedDictionary<string, bool> ck3ModFlags = config.GetCK3ModFlags();
-		
+
 		Parallel.Invoke(
+			() => provinceMapper.DetectInvalidMappings(impWorld.MapData, MapData), // depends on ProvinceMapper and MapData
 			() => { // depends on ck3ColorFactory and CulturalPillars
 				// Load CK3 cultures from CK3 mod filesystem.
 				Logger.Info("Loading cultural pillars...");
@@ -142,9 +143,9 @@ public World(Imperator.World impWorld, Configuration config, Thread? irCoaExtrac
 			},
 			() => LoadMenAtArmsTypes(ModFS, ScriptValues), // depends on ScriptValues
 			() => { // depends on LocDB and CK3CoaMapper
-				// Load vanilla CK3 landed titles and their history
+					// Load vanilla CK3 landed titles and their history
 				LandedTitles.LoadTitles(ModFS, LocDB, ck3ColorFactory);
-				
+
 				if (config.StaticDeJure) {
 					Logger.Info("Setting static de jure kingdoms and empires...");
 
@@ -154,25 +155,25 @@ public World(Imperator.World impWorld, Configuration config, Thread? irCoaExtrac
 
 					Logger.IncrementProgress();
 				}
-		
+
 				LandedTitles.SetCoatsOfArms(CK3CoaMapper);
-		
+
 				LandedTitles.LoadHistory(config, ModFS);
 				LandedTitles.LoadCulturalNamesFromConfigurables();
 			}
 		);
-		
+
 		// Load regions.
 		ck3RegionMapper = new CK3RegionMapper(ModFS, LandedTitles);
 		imperatorRegionMapper = impWorld.ImperatorRegionMapper;
-		
+
 		CultureMapper cultureMapper = null!;
 		TraitMapper traitMapper = null!;
 		DNAFactory dnaFactory = null!;
 		Parallel.Invoke(
 			() => { // depends on ck3ColorFactory and landed titles being loaded 
-				// Load CK3 religions from game and blankMod.
-				// Holy sites need to be loaded after landed titles.
+					// Load CK3 religions from game and blankMod.
+					// Holy sites need to be loaded after landed titles.
 				Religions.LoadDoctrines(ModFS);
 				Logger.Info("Loaded CK3 doctrines.");
 				Religions.LoadConverterHolySites("configurables/converter_holy_sites.txt");
@@ -190,15 +191,15 @@ public World(Imperator.World impWorld, Configuration config, Thread? irCoaExtrac
 				Religions.RemoveChristianAndIslamicSyncretismFromAllFaiths();
 				// Now that all the faiths are loaded, remove liege entries from the history of religious head titles.
 				LandedTitles.RemoveLiegeEntriesFromReligiousHeadHistory(Religions);
-				
+
 				Religions.LoadReplaceableHolySites("configurables/replaceable_holy_sites.txt");
 				Logger.Info("Loaded replaceable holy sites.");
 			},
-			
+
 			() => cultureMapper = new CultureMapper(imperatorRegionMapper, ck3RegionMapper, Cultures),
-			
+
 			() => traitMapper = new("configurables/trait_map.txt", ModFS),
-			
+
 			() => {
 				Logger.Info("Initializing DNA factory...");
 				dnaFactory = new(impWorld.ModFS, ModFS);
@@ -210,14 +211,14 @@ public World(Imperator.World impWorld, Configuration config, Thread? irCoaExtrac
 				Logger.IncrementProgress();
 			}
 		);
-		
+
 		var religionMapper = new ReligionMapper(Religions, imperatorRegionMapper, ck3RegionMapper);
-		
+
 		Parallel.Invoke(
 			() => Cultures.ImportTechnology(impWorld.Countries, cultureMapper, provinceMapper, impWorld.InventionsDB, impWorld.LocDB, ck3ModFlags),
-			
+
 			() => { // depends on religionMapper
-				// Check if all I:R religions have a base mapping.
+					// Check if all I:R religions have a base mapping.
 				foreach (var irReligionId in impWorld.Religions.Select(r => r.Id)) {
 					var baseMapping = religionMapper.Match(irReligionId, null, null, null, null, config);
 					if (baseMapping is null) {
@@ -231,25 +232,25 @@ public World(Imperator.World impWorld, Configuration config, Thread? irCoaExtrac
 				}
 			},
 			() => { // depends on cultureMapper
-				// Check if all I:R cultures have a base mapping.
+					// Check if all I:R cultures have a base mapping.
 				var irCultureIds = impWorld.CulturesDB.SelectMany(g => g.Select(c => c.Id));
 				foreach (var irCultureId in irCultureIds) {
-                	var baseMapping = cultureMapper.Match(irCultureId, null, null, null);
-                	if (baseMapping is null) {
+					var baseMapping = cultureMapper.Match(irCultureId, null, null, null);
+					if (baseMapping is null) {
 						string cultureStr = "ID: " + irCultureId;
 						var localizedName = impWorld.LocDB.GetLocBlockForKey(irCultureId)?["english"];
 						if (localizedName is not null) {
 							cultureStr += $", name: {localizedName}";
 						}
-                		Logger.Warn($"No base mapping found for I:R culture {cultureStr}!");
-                	}
-                }
+						Logger.Warn($"No base mapping found for I:R culture {cultureStr}!");
+					}
+				}
 			},
 			() => { // depends on TraitMapper and CK3 characters being loaded
 				Characters.RemoveUndefinedTraits(traitMapper);
 			}
 		);
-		
+
 		Characters.ImportImperatorCharacters(
 			impWorld,
 			religionMapper,
@@ -324,14 +325,14 @@ public World(Imperator.World impWorld, Configuration config, Thread? irCoaExtrac
 			impWorld.CoaMapper,
 			countyLevelGovernorships
 		);
-		
+
 		// Give counties to rulers and governors.
 		OverwriteCountiesHistory(impWorld.Countries, impWorld.JobsDB.Governorships, countyLevelCountries, countyLevelGovernorships, impWorld.Characters, impWorld.Provinces, CorrectedDate);
 		ImportImperatorHoldingsIfNotDisabledByConfiguration(impWorld, config);
 
 		LandedTitles.ImportDevelopmentFromImperator(Provinces, CorrectedDate, config.ImperatorCivilizationWorth);
 		LandedTitles.RemoveInvalidLandlessTitles(config.CK3BookmarkDate);
-		
+
 		// Apply region-specific tweaks.
 		HandleIcelandAndFaroeIslands(impWorld, config);
 
@@ -343,15 +344,15 @@ public World(Imperator.World impWorld, Configuration config, Thread? irCoaExtrac
 		if (!config.StaticDeJure) {
 			LandedTitles.SetDeJureKingdomsAndEmpires(config.CK3BookmarkDate, Cultures, Characters, MapData, LocDB);
 		}
-		
+
 		Dynasties.SetCoasForRulingDynasties(LandedTitles, config.CK3BookmarkDate);
-		
+
 		Characters.RemoveEmployerIdFromLandedCharacters(LandedTitles, CorrectedDate);
 		Characters.PurgeUnneededCharacters(LandedTitles, Dynasties, DynastyHouses, config.CK3BookmarkDate);
 		// We could convert Imperator character DNA while importing the characters.
 		// But that'd be wasteful, because some of them are purged. So, we do it now.
 		Characters.ConvertImperatorCharacterDNA(dnaFactory);
-		
+
 		// If there's a gap between the I:R save date and the CK3 bookmark date,
 		// generate successors for old I:R characters instead of making them live for centuries.
 		if (config.CK3BookmarkDate.DiffInYears(impWorld.EndDate) > 1) {
@@ -361,20 +362,20 @@ public World(Imperator.World impWorld, Configuration config, Thread? irCoaExtrac
 		// Gold needs to be distributed after characters' successors are generated.
 		Characters.DistributeCountriesGold(LandedTitles, config);
 		Characters.ImportLegions(LandedTitles, impWorld.Units, impWorld.Characters, CorrectedDate, unitTypeMapper, MenAtArmsTypes, provinceMapper, LocDB, config);
-		
+
 		// After the purging of unneeded characters, we should clean up the title history.
 		LandedTitles.CleanUpHistory(Characters, config.CK3BookmarkDate);
-		
+
 		// Now that the title history is basically done, convert officials as council members and courtiers.
 		LandedTitles.ImportImperatorGovernmentOffices(impWorld.JobsDB.OfficeJobs, Religions, impWorld.EndDate);
 
 		Parallel.Invoke(
 			() => ImportImperatorWars(impWorld, config.CK3BookmarkDate),
-			
+
 			() => {
 				var holySiteEffectMapper = new HolySiteEffectMapper("configurables/holy_site_effect_mappings.txt");
 				Religions.DetermineHolySites(Provinces, impWorld.Religions, holySiteEffectMapper, config.CK3BookmarkDate);
-		
+
 				Religions.GenerateMissingReligiousHeads(LandedTitles, Characters, Provinces, Cultures, config.CK3BookmarkDate);
 				Logger.IncrementProgress();
 			},

ImperatorToCK3/CommonUtils/Map/MapData.cs

@@ -48,10 +48,33 @@ public override readonly int GetHashCode() {
 	private readonly HashSet<ulong> mapEdgeProvinces = [];
 
 	private string provincesMapFilename = "provinces.png";
+	private string adjacenciesFilename = "adjacencies.csv";
 
 	public MapData(ModFilesystem modFS) {
-		string adjacenciesFilename = "adjacencies.csv";
+		ParseDefaultMap(modFS);
 
+		Logger.Info("Loading province positions...");
+		DetermineProvincePositions(modFS);
+		Logger.IncrementProgress();
+
+		Logger.Info("Loading province adjacencies...");
+		LoadAdjacencies(adjacenciesFilename, modFS);
+
+		DetermineMapEdgeProvinces(modFS);
+
+		Logger.Info("Determining province neighbors...");
+		var provincesMapPath = GetProvincesMapPath(modFS);
+		if (provincesMapPath is not null) {
+			using Image<Rgb24> provincesMap = Image.Load<Rgb24>(provincesMapPath);
+			DetermineNeighbors(provincesMap, ProvinceDefinitions);
+		}
+
+		GroupStaticWaterProvinces();
+
+		Logger.IncrementProgress();
+	}
+
+	private void ParseDefaultMap(ModFilesystem modFS) {
 		Logger.Info("Loading default map data...");
 		const string defaultMapPath = "map_data/default.map";
 		var defaultMapParser = new Parser();
@@ -92,26 +115,6 @@ public MapData(ModFilesystem modFS) {
 		defaultMapParser.IgnoreAndLogUnregisteredItems();
 		defaultMapParser.ParseGameFile(defaultMapPath, modFS);
 		Logger.IncrementProgress();
-
-		Logger.Info("Loading province positions...");
-		DetermineProvincePositions(modFS);
-		Logger.IncrementProgress();
-
-		Logger.Info("Loading province adjacencies...");
-		LoadAdjacencies(adjacenciesFilename, modFS);
-
-		DetermineMapEdgeProvinces(modFS);
-
-		Logger.Info("Determining province neighbors...");
-		var provincesMapPath = GetProvincesMapPath(modFS);
-		if (provincesMapPath is not null) {
-			using Image<Rgb24> provincesMap = Image.Load<Rgb24>(provincesMapPath);
-			DetermineNeighbors(provincesMap, ProvinceDefinitions);
-		}
-
-		GroupStaticWaterProvinces();
-
-		Logger.IncrementProgress();
 	}
 
 	private void GroupStaticWaterProvinces() {
@@ -199,6 +202,7 @@ public IReadOnlySet<ulong> GetNeighborProvinceIds(ulong provinceId) {
 
 	private bool IsStaticWater(ulong provinceId) => ProvinceDefinitions[provinceId].IsStaticWater;
 	private bool IsRiver(ulong provinceId) => ProvinceDefinitions[provinceId].IsRiver;
+	internal bool IsLand(ulong provinceId) => ProvinceDefinitions[provinceId].IsLand;
 
 	public FrozenSet<ulong> ColorableImpassableProvinceIds => ProvinceDefinitions
 		.Where(p => p.IsColorableImpassable).Select(p => p.Id)

ImperatorToCK3/CommonUtils/Map/ProvinceDefinition.cs

@@ -16,5 +16,5 @@ public void AddSpecialCategory(SpecialProvinceCategory category) {
 	                            specialCategories.Contains(SpecialProvinceCategory.ColorableImpassable);
 	public bool IsStaticWater => specialCategories.Contains(SpecialProvinceCategory.StaticWater);
 	public bool IsRiver => specialCategories.Contains(SpecialProvinceCategory.River);
-	public bool IsLand => !IsStaticWater && !IsRiver;
+	public bool IsLand => (!IsStaticWater && !IsRiver) || IsColorableImpassable; // handles provinces 1107 and 1108 being both impassable_mountains and lakes as of CK3 1.17.1
 }