search

wakystuf / ESG-Mod

An Endless Space 2 balance and overhaul mod
31 stars 4 forks source link

Reawakening merge fixes #1475

Closed captaincobbs closed 2 months ago

captaincobbs commented 2 months ago

Fixes #1477, #1471, #1469, #1470, #1463, #1460, #1459

captaincobbs commented 2 months ago

This is ready for review, please actually read through the changes I changed the structure of a lot of stuff

wakystuf commented 2 months ago

Generally I think this looks good. A couple things: 1) How much has this code been tested? To me the most sensitive one is the change to the initial planet FIDSI, though I suspect that one will be fine since it's simply using the new variables 2) Are you sure that this line should not be targeting //ClassDiplomaticEmpire ? I don't believe that RelationPointsTrend is a property of ClassEmpire, but I'm going purely from memory. <BinaryModifier TargetProperty="RelationPointsTrend" Operation="Addition" Left="$(HasDiplomacyRelic)" BinaryOperation="Multiplication" Right="1" Path="ClassEmpire"/>

captaincobbs commented 2 months ago

  1. Yup, it works. The functionality is the same, the variable it was changed to (PlanetInitialFIDSIPerPopulation) directly adds to PlanetInitialFIDSI but its GUI element isn't bound to the same limitations, thus allowing me to fix the incorrect tooltips. There should be no functional change, I tracked the math on multiple anomalies and it all added up.

  2. RelationPointsTrend is a hard-coded variable that adds +1 Relation to all known Minor Factions, it's only in ClassEmpire because it's hard coded, it is indeed in ClassEmpire.

wakystuf commented 2 months ago

OK sounds good then; I went ahead and merged