Fix development of I:R provinces not being summed when determining pr…

…imary source for CK3 provinces (#1546) #patch

fixes #1544

ImperatorToCK3.UnitTests/CK3/Provinces/ProvincesTests.cs

@@ -1,5 +1,17 @@
 using commonItems;
+using commonItems.Colors;
 using commonItems.Mods;
+using ImperatorToCK3.CK3.Cultures;
+using ImperatorToCK3.CK3.Provinces;
+using ImperatorToCK3.CK3.Religions;
+using ImperatorToCK3.CK3.Titles;
+using ImperatorToCK3.Imperator.Countries;
+using ImperatorToCK3.Imperator.Geography;
+using ImperatorToCK3.Mappers.Culture;
+using ImperatorToCK3.Mappers.Province;
+using ImperatorToCK3.Mappers.Region;
+using ImperatorToCK3.Mappers.Religion;
+using System;
 using System.Collections.Generic;
 using System.Linq;
 using Xunit;
@@ -9,6 +21,9 @@ namespace ImperatorToCK3.UnitTests.CK3.Provinces;
 [Collection("Sequential")]
 [CollectionDefinition("Sequential", DisableParallelization = true)]
 public class ProvincesTests {
+	private const string ImperatorRoot = "TestFiles/Imperator/game";
+	private static readonly ModFilesystem irModFS = new(ImperatorRoot, Array.Empty<Mod>());
+
 	private const string CK3Root = "TestFiles/CK3ProvincesTests";
 	private readonly ModFilesystem ck3ModFs = new(CK3Root, new List<Mod>());
 	private readonly Date ck3BookmarkDate = new("867.1.1");
@@ -47,4 +62,70 @@ public void ProvincesAreProperlyLoadedFromFilesystem() {
 			}
 		);
 	}
+
+	[Fact]
+	public void PrimaryImperatorProvinceIsProperlyDeterminedForCK3Province() {
+		var conversionDate = new Date(476, 1, 1);
+		var config = new Configuration { CK3BookmarkDate = conversionDate };
+		var titles = new Title.LandedTitles();
+		var titlesReader = new BufferedReader(
+			"""
+			c_county1={
+				b_barony1={province=1}
+			}
+			""");
+		titles.LoadTitles(titlesReader);
+
+		// Scenario 1: Sum of civilisation in country 1 outweighs single more civilized province in country 2.
+		var irWorld = new ImperatorToCK3.Imperator.World(config);
+		// Country 1 (civilisation 9 in total)
+		var country1 = new Country(1);
+		var irProvince1 = new ImperatorToCK3.Imperator.Provinces.Province(1) { CivilizationValue = 1, OwnerCountry = country1};
+		irWorld.Provinces.Add(irProvince1);
+		var irProvince2 = new ImperatorToCK3.Imperator.Provinces.Province(2) { CivilizationValue = 2, OwnerCountry = country1 };
+		irWorld.Provinces.Add(irProvince2);
+		var irProvince3 = new ImperatorToCK3.Imperator.Provinces.Province(3) { CivilizationValue = 3, OwnerCountry = country1 };
+		irWorld.Provinces.Add(irProvince3);
+		var irProvince4 = new ImperatorToCK3.Imperator.Provinces.Province(4) { CivilizationValue = 2, OwnerCountry = country1 };
+		irWorld.Provinces.Add(irProvince4);
+		var irProvince5 = new ImperatorToCK3.Imperator.Provinces.Province(5) { CivilizationValue = 1, OwnerCountry = country1 };
+		irWorld.Provinces.Add(irProvince5);
+		// Country 2 (civilisation 5 in total)
+		var country2 = new Country(2);
+		var irProvince6 = new ImperatorToCK3.Imperator.Provinces.Province(6) { CivilizationValue = 5, OwnerCountry = country2 };
+		irWorld.Provinces.Add(irProvince6);
+
+		var provinceMapper = new ProvinceMapper();
+		const string provinceMappingsPath = "TestFiles/LandedTitlesTests/province_mappings.txt";
+		provinceMapper.LoadMappings(provinceMappingsPath, "6_to_1");
+
+		var ck3Provinces = new ProvinceCollection { new(1) };
+		var ck3RegionMapper = new CK3RegionMapper();
+		AreaCollection areas = new();
+		areas.LoadAreas(irModFS, irWorld.Provinces);
+		var irRegionMapper = new ImperatorRegionMapper(areas);
+		irRegionMapper.LoadRegions(irModFS, new ColorFactory());
+		var cultures = new CultureCollection(new ColorFactory(), new PillarCollection());
+		var cultureMapper = new CultureMapper(irRegionMapper, ck3RegionMapper, cultures);
+		var religions = new ReligionCollection(titles);
+		var religionMapper = new ReligionMapper(religions, irRegionMapper, ck3RegionMapper);
+		ck3Provinces.ImportImperatorProvinces(irWorld, titles, cultureMapper, religionMapper, provinceMapper, conversionDate, config);
+
+		var targetProvince = ck3Provinces[1];
+		Assert.Equal((ulong)1, targetProvince.Id);
+		var primarySourceProvince = targetProvince.PrimaryImperatorProvince;
+		Assert.NotNull(primarySourceProvince);
+		Assert.Equal((ulong)3, primarySourceProvince.Id); // most developed province in country 1
+
+		// Scenario 2: Single developed province in country 2 outweighs sum of civilisation in country 1.
+		irProvince6.CivilizationValue = 100;
+		ck3Provinces = new ProvinceCollection { new(1) };
+		ck3Provinces.ImportImperatorProvinces(irWorld, titles, cultureMapper, religionMapper, provinceMapper, conversionDate, config);
+
+		targetProvince = ck3Provinces[1];
+		Assert.Equal((ulong)1, targetProvince.Id);
+		primarySourceProvince = targetProvince.PrimaryImperatorProvince;
+		Assert.NotNull(primarySourceProvince);
+		Assert.Equal((ulong)6, primarySourceProvince.Id); // province of country 2
+	}
 }

ImperatorToCK3.UnitTests/TestFiles/LandedTitlesTests/province_mappings.txt

@@ -5,4 +5,17 @@ test_version = {
 					link={imp=4 ck3=4}
 					link={imp=5 ck3=5}
 					link={imp=6 ck3=6}
+}
+
+6_to_1 = {
+    link = {
+        imp = 1
+        imp = 2
+        imp = 3
+        imp = 4
+        imp = 5
+        imp = 6
+
+        ck3 = 1
+    }
 }

ImperatorToCK3/CK3/Provinces/ProvinceCollection.cs

@@ -146,32 +146,39 @@ public void LoadPrehistory() {
 	}
 
 	private static Imperator.Provinces.Province? DeterminePrimarySourceProvince(
-		IEnumerable<ulong> impProvinceNumbers,
+		IEnumerable<ulong> irProvinceIds,
 		Imperator.World irWorld
 	) {
-		// determine ownership by province development.
-		var theClaims = new Dictionary<ulong, List<Imperator.Provinces.Province>>(); // owner, offered province sources
-		var theShares = new Dictionary<ulong, int>(); // owner, development
-		ulong? winner = null;
-		long maxDev = -1;
-
-		foreach (var imperatorProvinceId in impProvinceNumbers) {
-			if (!irWorld.Provinces.TryGetValue(imperatorProvinceId, out var impProvince)) {
-				Logger.Warn($"Source province {imperatorProvinceId} is not on the list of known provinces!");
-				continue; // Broken mapping, or loaded a mod changing provinces without using it.
+		var irProvinces = new OrderedSet<Imperator.Provinces.Province>();
+		foreach (var provId in irProvinceIds) {
+			if (!irWorld.Provinces.TryGetValue(provId, out var irProvince)) {
+				// Broken mapping, or loaded a mod changing provinces without using it.
+				Logger.Warn($"Source province {provId} is not on the list of known provinces!");
+				continue;
 			}
 
-			var ownerId = impProvince.OwnerCountry?.Id ?? 0;
+			irProvinces.Add(irProvince);
+		}
+
+		// Determine ownership by province development.
+		var theClaims = new Dictionary<ulong, OrderedSet<Imperator.Provinces.Province>>(); // owner, offered province sources
+		var theShares = new Dictionary<ulong, double>(); // owner, sum of development
+		foreach (var irProvince in irProvinces) {
+			var ownerId = irProvince.OwnerCountry?.Id ?? 0;
 			if (!theClaims.ContainsKey(ownerId)) {
-				theClaims[ownerId] = new List<Imperator.Provinces.Province>();
+				theClaims[ownerId] = new OrderedSet<Imperator.Provinces.Province>();
 			}
 
-			theClaims[ownerId].Add(impProvince);
-
-			var devValue = (int)impProvince.BuildingCount + impProvince.GetPopCount();
-			theShares[ownerId] = devValue;
+			theClaims[ownerId].Add(irProvince);
+			if (!theShares.ContainsKey(ownerId)) {
+				theShares[ownerId] = 0;
+			}
+			theShares[ownerId] += irProvince.CivilizationValue;
 		}
+
 		// Let's see who the lucky winner is.
+		ulong? winner = null;
+		double maxDev = -1;
 		foreach (var (owner, development) in theShares) {
 			if (development > maxDev) {
 				winner = owner;
@@ -182,19 +189,8 @@ Imperator.World irWorld
 			return null;
 		}
 
-		// Now that we have a winning owner, let's find its largest province to use as a source.
-		maxDev = -1; // We can have winning provinces with weight = 0.
-
-		Imperator.Provinces.Province? provinceToReturn = null;
-		foreach (var province in theClaims[winner.Value]) {
-			long provinceWeight = province.BuildingCount + province.GetPopCount();
-
-			if (provinceWeight > maxDev) {
-				provinceToReturn = province;
-				maxDev = provinceWeight;
-			}
-		}
-
-		return provinceToReturn;
+		// Now that we have a winning owner, let's find the most developed province to use as a source.
+		return theClaims[winner.Value]
+			.MaxBy(p => p.CivilizationValue);
 	}
 }