Skip to content

Don't try to set culture and religion for water provinces #2110

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 22, 2024
Merged

Don't try to set culture and religion for water provinces #2110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
85ecae1
Don't try to set culture and religion for water provinces
IhateTrains Aug 22, 2024
6a40c33
Remove province mapping from unknown province
IhateTrains Aug 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion ImperatorToCK3.UnitTests/CK3/Characters/CharacterCollectionTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,8 @@ public void ImperatorCountriesGoldCanBeDistributedAmongRulerAndVassals() {
new List<KeyValuePair<Country, Dependency?>>());

var provinces = new ProvinceCollection(ck3ModFS);
provinces.ImportImperatorProvinces(imperatorWorld, titles, cultureMapper, religionMapper, provinceMapper, conversionDate, config);
var ck3MapData = new MapData(ck3ModFS);
provinces.ImportImperatorProvinces(imperatorWorld, ck3MapData, titles, cultureMapper, religionMapper, provinceMapper, conversionDate, config);

titles.ImportImperatorGovernorships(
imperatorWorld,
Expand Down
6 changes: 4 additions & 2 deletions ImperatorToCK3.UnitTests/CK3/Provinces/ProvincesTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,9 @@
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 ck3MapData = new MapData(ck3ModFs);
ck3MapData.ProvinceDefinitions.Add(new ProvinceDefinition(1) {});

Check notice on line 116 in ImperatorToCK3.UnitTests/CK3/Provinces/ProvincesTests.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

ImperatorToCK3.UnitTests/CK3/Provinces/ProvincesTests.cs#L116 

Remove the initializer; it is redundant.
ck3Provinces.ImportImperatorProvinces(irWorld, ck3MapData, titles, cultureMapper, religionMapper, provinceMapper, conversionDate, config);

var targetProvince = ck3Provinces[1];
Assert.Equal((ulong)1, targetProvince.Id);
Expand All @@ -123,7 +125,7 @@
// 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);
ck3Provinces.ImportImperatorProvinces(irWorld, ck3MapData, titles, cultureMapper, religionMapper, provinceMapper, conversionDate, config);

targetProvince = ck3Provinces[1];
Assert.Equal((ulong)1, targetProvince.Id);
Expand Down
24 changes: 20 additions & 4 deletions ImperatorToCK3.UnitTests/CK3/Titles/LandedTitlesTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,11 @@ public void GovernorshipsCanBeRecognizedAsCountyLevel() {
);

var provinces = new ProvinceCollection(ck3ModFS);
provinces.ImportImperatorProvinces(imperatorWorld, titles, cultureMapper, religionMapper, provinceMapper, conversionDate, config);
var ck3MapData = new MapData(ck3ModFS);
ck3MapData.ProvinceDefinitions.Add(new(1));
ck3MapData.ProvinceDefinitions.Add(new(2));
ck3MapData.ProvinceDefinitions.Add(new(3));
provinces.ImportImperatorProvinces(imperatorWorld, ck3MapData, titles, cultureMapper, religionMapper, provinceMapper, conversionDate, config);
// Country 589 is imported as duchy-level title, so its governorship of galatia_region will be county level.
titles.ImportImperatorGovernorships(imperatorWorld, provinces, tagTitleMapper, irLocDB, ck3LocDB, config, provinceMapper, definiteFormMapper, irRegionMapper, coaMapper, countyLevelGovernorships);

Expand Down Expand Up @@ -372,7 +376,11 @@ public void DevelopmentIsCorrectlyCalculatedFor1ProvinceTo1BaronyCountyMapping()
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 ck3MapData = new MapData(ck3ModFS);
ck3MapData.ProvinceDefinitions.Add(new(1));
ck3MapData.ProvinceDefinitions.Add(new(2));
ck3MapData.ProvinceDefinitions.Add(new(3));
ck3Provinces.ImportImperatorProvinces(irWorld, ck3MapData, titles, cultureMapper, religionMapper, provinceMapper, conversionDate, config);

var date = config.CK3BookmarkDate;
titles.ImportDevelopmentFromImperator(ck3Provinces, date, defaultConfig.ImperatorCivilizationWorth);
Expand Down Expand Up @@ -404,7 +412,11 @@ public void DevelopmentFromImperatorProvinceCanBeUsedForMultipleCK3Provinces() {
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 ck3MapData = new MapData(ck3ModFS);
ck3MapData.ProvinceDefinitions.Add(new(1));
ck3MapData.ProvinceDefinitions.Add(new(2));
ck3MapData.ProvinceDefinitions.Add(new(3));
ck3Provinces.ImportImperatorProvinces(irWorld, ck3MapData, titles, cultureMapper, religionMapper, provinceMapper, conversionDate, config);

var date = config.CK3BookmarkDate;
titles.ImportDevelopmentFromImperator(ck3Provinces, date, defaultConfig.ImperatorCivilizationWorth);
Expand Down Expand Up @@ -438,7 +450,11 @@ public void DevelopmentOfCountyIsCalculatedFromAllCountyProvinces() {
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 ck3MapData = new MapData(ck3ModFS);
ck3MapData.ProvinceDefinitions.Add(new(1));
ck3MapData.ProvinceDefinitions.Add(new(2));
ck3MapData.ProvinceDefinitions.Add(new(3));
ck3Provinces.ImportImperatorProvinces(irWorld, ck3MapData, titles, cultureMapper, religionMapper, provinceMapper, conversionDate, config);

var date = config.CK3BookmarkDate;
titles.ImportDevelopmentFromImperator(ck3Provinces, date, defaultConfig.ImperatorCivilizationWorth);
Expand Down
9 changes: 8 additions & 1 deletion ImperatorToCK3/CK3/Provinces/ProvinceCollection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using commonItems.Collections;
using commonItems.Mods;
using ImperatorToCK3.CK3.Titles;
using ImperatorToCK3.CommonUtils.Map;
using ImperatorToCK3.Exceptions;
using ImperatorToCK3.Mappers.Culture;
using ImperatorToCK3.Mappers.Province;
Expand All @@ -22,7 +23,7 @@
LoadProvincesHistory(ck3ModFs);
}

private void LoadProvinceDefinitions(ModFilesystem ck3ModFs) { // TODO: get rid of this (duplicates functionality of ProvinceDefinitions class)

Check warning on line 26 in ImperatorToCK3/CK3/Provinces/ProvinceCollection.cs

View workflow job for this annotation

GitHub Actions / build (self-hosted, linux) 

TODO get rid of this (duplicates functionality of ProvinceDefinitions class) (https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0026.md)

Check warning on line 26 in ImperatorToCK3/CK3/Provinces/ProvinceCollection.cs

View workflow job for this annotation

GitHub Actions / build (macos-14) 

TODO get rid of this (duplicates functionality of ProvinceDefinitions class) (https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0026.md)

Check warning on line 26 in ImperatorToCK3/CK3/Provinces/ProvinceCollection.cs

View workflow job for this annotation

GitHub Actions / test (macos-14) 

TODO get rid of this (duplicates functionality of ProvinceDefinitions class) (https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0026.md)

Check warning on line 26 in ImperatorToCK3/CK3/Provinces/ProvinceCollection.cs

View workflow job for this annotation

GitHub Actions / build (self-hosted, windows) 

TODO get rid of this (duplicates functionality of ProvinceDefinitions class) (https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0026.md)

Check warning on line 26 in ImperatorToCK3/CK3/Provinces/ProvinceCollection.cs

View workflow job for this annotation

GitHub Actions / test (self-hosted, windows) 

TODO get rid of this (duplicates functionality of ProvinceDefinitions class) (https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0026.md)
Logger.Info("Loading CK3 province definitions...");

var filePath = ck3ModFs.GetActualFileLocation("map_data/definition.csv");
Expand Down Expand Up @@ -104,6 +105,7 @@

public void ImportImperatorProvinces(
Imperator.World irWorld,
MapData ck3MapData,
Title.LandedTitles titles,
CultureMapper cultureMapper,
ReligionMapper religionMapper,
Expand All @@ -115,8 +117,13 @@

int importedIRProvsCount = 0;
int modifiedCK3ProvsCount = 0;

var provinceDefs = ck3MapData.ProvinceDefinitions;
var landProvinces = this
.Where(p => provinceDefs.TryGetValue(p.Id, out var def) && def.IsLand);

// Imperator provinces map to a subset of CK3 provinces. We'll only rewrite those we are responsible for.
Parallel.ForEach(this, province => {
Parallel.ForEach(landProvinces, province => {
var sourceProvinceIds = provinceMapper.GetImperatorProvinceNumbers(province.Id);
// Provinces we're not affecting will not be in this list.
if (sourceProvinceIds.Count == 0) {
Expand Down
2 changes: 1 addition & 1 deletion ImperatorToCK3/CK3/World.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ public World(Imperator.World impWorld, Configuration config) {
Provinces.ImportVanillaProvinces(ModFS);

// Next we import Imperator provinces and translate them ontop a significant part of all imported provinces.
Provinces.ImportImperatorProvinces(impWorld, LandedTitles, cultureMapper, religionMapper, provinceMapper, CorrectedDate, config);
Provinces.ImportImperatorProvinces(impWorld, MapData, LandedTitles, cultureMapper, religionMapper, provinceMapper, CorrectedDate, config);
Provinces.LoadPrehistory();

var countyLevelGovernorships = new List<Governorship>();
Expand Down
Loading
Loading