Fix for Foulborn Choir of the Storm calculation

[EminGul]

Fixes #9410 .

Description of the problem being solved:

Foulborn Choir of the Storm along with Radiant Faith was producing incorrect values for both the % increased mana gained (in-game values round down) and for the base ES granted by Radiant Faith when the amulet is equipped.

Steps taken to verify a working solution:

• Import given PoB (https://pobb.in/JX6dPx2-Tt5H)
• Remove the custom modifier that gives 478% increased maximum mana and edit/uncomment the equipped Foulborn Choir of the Storm to have its "Mana is increased by 50% of Overcapped Lightning Resistance" active.
• Verify corrected values (with 957% overcapped lightning resistance, Foulborn Choir of the Storm should give 478% increased maximum mana and with 16,777 mana reserved Radiant Faith should give 1,677 base energy shield).
• Run docker tests

Link to a build that showcases this PR:

Supplied PoB: https://pobb.in/JX6dPx2-Tt5H
With Step 2 above applied: https://pobb.in/iT5syI5O5TeM

Before screenshot:

After screenshot:

Note: I did not want to change the stat calculations and steps in CalcPerform.lua to avoid causing more bugs, but the issue is caused by doActorLifeManaReservation being called more than once (once in CalcPerform.lua and afterwards in CalcDefence.lua). Only the latter call has the correct value of maximum mana and hence reserved mana/base ES granted by Radiant Faith because it accounts the increases by Foulborn Choir of the Storm by that point in the calculations.

Edit: Seems to be similar to #9399

[OriginalThing]

Just want to point out this kills the aura part of Radiant Faith, so it doesn't apply to minions or get exported as a party buff (Mana Guardian).

Using ReplaceMod/ReplaceModInternal doesn't seem to work for the ExtraAura as it gets confused and the Energy Shield overwrites the Armour aura (somehow i end up with 2 Energy Shield extra auras?!). Is making a special case LIST type to look at the nested mod type in ReplaceModInternal a good idea? Or are there some attributes we can set so they don't look the same to the current matching algo? Or is there a way to combine the armour and energy shield as a single Extra Aura mod ? However I think you'll encounter the same issue where you have to calculate auras (Purity of Elements/Lightning), then resistances/defence, then mana reservation, then extra auras which is quite a bit different to the current calculation order. So I think you have to either calculate resistances earlier, or calculate extra auras later.

I do find my PR (#9399) a little ugly, but I was trying not to alter the current code flow too much for anything other than the Foulborn Choir to reduce the chance of introducing bugs.

[EminGul]

Just want to point out this kills the aura part of Radiant Faith, so it doesn't apply to minions or get exported as a party buff (Mana Guardian).

Using ReplaceMod/ReplaceModInternal doesn't seem to work for the ExtraAura as it gets confused and the Energy Shield overwrites the Armour aura (somehow i end up with 2 Energy Shield extra auras?!). Is making a special case LIST type to look at the nested mod type in ReplaceModInternal a good idea? Or are there some attributes we can set so they don't look the same to the current matching algo? Or is there a way to combine the armour and energy shield as a single Extra Aura mod ? However I think you'll encounter the same issue where you have to calculate auras (Purity of Elements/Lightning), then resistances/defence, then mana reservation, then extra auras which is quite a bit different to the current calculation order. So I think you have to either calculate resistances earlier, or calculate extra auras later.

I do find my PR (#9399) a little ugly, but I was trying not to alter the current code flow too much for anything other than the Foulborn Choir to reduce the chance of introducing bugs.

Fair enough, I'll close this PR as I was trying to do minimal changes/not change code flow but didn't realize it killed the aura portion. Where are you viewing the 2 ES auras? On my end its overwritten in the output.

Calculating extra auras later makes the most sense to me but is obviously prone to more issues, so it's probably something the core maintainers will have to reevaluate.