X2CommunityCore / X2WOTCCommunityHighlander

https://steamcommunity.com/workshop/filedetails/?id=1134256495
MIT License
60 stars 68 forks source link

XCGS_Unit:GetStatModifiers reports bogus modifiers for multipliers #313

Closed robojumper closed 4 years ago

robojumper commented 5 years ago

https://github.com/X2CommunityCore/X2WOTCCommunityHighlander/blob/8954b83b97d6296e26003dea22ea32da2af76c4f/X2WOTCCommunityHighlander/Src/XComGame/Classes/XComGameState_Unit.uc#L6514

Instead of returning the correct contribution (mult - 1) * current_value, it seems to return the value after multiplication mult * current_value, at least for MODOP_Multiplication (and probably MODOP_PostMultiplication too). mult * base_stat for both MODOP_Multiplication and MODOP_PostMultiplication as @MrNiceUK has pointed out to me.

The only user of this is X2AbilityToHitCalc_StandardAim, which computes a bogus to-hit chance: https://github.com/X2CommunityCore/X2WOTCCommunityHighlander/blob/8954b83b97d6296e26003dea22ea32da2af76c4f/X2WOTCCommunityHighlander/Src/XComGame/Classes/X2AbilityToHitCalc_StandardAim.uc#L364-L369

and a bogus to-crit chance:

https://github.com/X2CommunityCore/X2WOTCCommunityHighlander/blob/8954b83b97d6296e26003dea22ea32da2af76c4f/X2WOTCCommunityHighlander/Src/XComGame/Classes/X2AbilityToHitCalc_StandardAim.uc#L523-L528

where it gives you a positive bonus for multipliers between 0 and 1.

We should fix (or work around) the problem.

RedDoby commented 5 years ago

I tested this with Simple Red Fog and it is still giving a positive bonus modifier.

RedDoby commented 4 years ago

Are you sure that this is working as intended? I tested it last month with the Simple Red Fog mod and the Beta Highlander, and it was still giving the positive offensive modifiers for values 0-1.

MrNiceUK commented 4 years ago

Try the v1.19 non-beta on the workshop, not sure if this fix was in the beta release on the workshop?

RedDoby commented 4 years ago

I double checked the classes XComGameState_unit and X2AbilityToHitCalc_StandardAim for the CHE that I was using at the time of testing. These changes are in the code. I will try it again with the debugger and see what is going on.

RedDoby commented 4 years ago

This is using multiplicate red fog set at .87 with the cannon. Note that when I did a melee attack preview it stepped into the function GetStatModifiersFixed with the debugger and everything calculated correctly. When I did the preview and shot with the cannon or assault rifle it did not step into that function and still displayed the positive multiplier of 48.

[1695.12] XCom_HitRolls: ===InternalRollForAbilityHit=== [1695.13] XCom_HitRolls: Attacker ID: 1148 [1695.13] XCom_HitRolls: Target ID: 1195 [1695.13] XCom_HitRolls: Ability: Fire Weapon (StandardShot) [1695.13] XCom_HitRolls: =GetHitChance= [1695.13] XCom_HitRolls: Modifying eHit_Success +0 (Fire Weapon), New hit chance: 0 [1695.13] XCom_HitRolls: Modifying eHit_Crit +0 (Fire Weapon), New hit chance: 0 [1695.13] XCom_HitRolls: Modifying eHit_Success +65 (Aim), New hit chance: 65 [1695.13] XCom_HitRolls: Modifying eHit_Success +48 (Red Fog), New hit chance: 113 [1695.13] XCom_HitRolls: Modifying eHit_Success -10 (Weapon Accuracy), New hit chance: 103 [1695.13] XCom_HitRolls: Modifying eHit_Success +0 (Defense), New hit chance: 103 [1695.13] XCom_HitRolls: Modifying eHit_Success +0 (Weapon Range), New hit chance: 103 [1695.13] XCom_HitRolls: Modifying eHit_Graze +0 (Dodge), New hit chance: 103 [1695.13] XCom_HitRolls: Modifying eHit_Crit +0 (Character Skill), New hit chance: 103 [1695.13] XCom_HitRolls: Modifying eHit_Crit +0 (Weapon Crit), New hit chance: 103 [1695.13] XCom_HitRolls: ==FinalizeHitChance==

[1695.13] XCom_HitRolls: Starting values... [1695.13] XCom_HitRolls: eHit_Success: 103 [1695.13] XCom_HitRolls: eHit_Crit: 0 [1695.13] XCom_HitRolls: eHit_Graze: 0 [1695.13] XCom_HitRolls: eHit_Miss: 0 [1695.13] XCom_HitRolls: eHit_LightningReflexes: 0 [1695.13] XCom_HitRolls: eHit_Untouchable: 0 [1695.13] XCom_HitRolls: eHit_CounterAttack: 0 [1695.13] XCom_HitRolls: eHit_Parry: 0 [1695.13] XCom_HitRolls: eHit_Deflect: 0 [1695.13] XCom_HitRolls: eHit_Reflect: 0 [1695.13] XCom_HitRolls: Calculated values... [1695.13] XCom_HitRolls: eHit_Success: 100 [1695.13] XCom_HitRolls: eHit_Crit: 0 [1695.13] XCom_HitRolls: eHit_Graze: 0 [1695.13] XCom_HitRolls: eHit_Miss: 0 [1695.13] XCom_HitRolls: eHit_LightningReflexes: 0 [1695.13] XCom_HitRolls: eHit_Untouchable: 0 [1695.13] XCom_HitRolls: eHit_CounterAttack: 0 [1695.13] XCom_HitRolls: eHit_Parry: 0 [1695.13] XCom_HitRolls: eHit_Deflect: 0 [1695.13] XCom_HitRolls: eHit_Reflect: 0 [1695.13] XCom_HitRolls: Final hit chance (success + crit + graze) = 103 [1695.13] XCom_HitRolls: =InternalRollForAbilityHit= [1695.13] XCom_HitRolls: Final hit chance: 103 [1695.13] XCom_HitRolls: Random roll: 99 [1695.13] XCom_HitRolls: Checking table eHit_Success (100)... [1695.13] XCom_HitRolls: MATCH! [1695.14] XCom_HitRolls: ***HIT eHit_Success [1695.15] XCom_HitRolls: ===CalculateDamageAmount=== [1695.15] XCom_HitRolls: Attacker ID: 1148 With Item ID: 1149 Target ID: 1195 [1695.15] XCom_HitRolls: bIgnoreBaseDamage:'False' DamageTag:'None' [1695.15] XCom_HitRolls: Weapon damage: 7 Potential spread: 1 [1695.15] XCom_HitRolls: Damage with spread: 7 [1695.15] XCom_HitRolls: RollArmorMitigation
[1695.15] XCom_HitRolls: Total Mitigation: 0 [1695.15] XCom_HitRolls: Total Damage: 7

RedDoby commented 4 years ago

Here is the same settings for a melee attack.

[2061.73] XCom_HitRolls: ===InternalRollForAbilityHit=== [2061.73] XCom_HitRolls: Attacker ID: 1168 [2061.73] XCom_HitRolls: Target ID: 1189 [2061.73] XCom_HitRolls: Ability: Slash (SwordSlice) [2061.73] XCom_HitRolls: =GetHitChance= [2061.73] XCom_HitRolls: Modifying eHit_Success +0 (Slash), New hit chance: 0 [2061.73] XCom_HitRolls: Modifying eHit_Crit +0 (Slash), New hit chance: 0 [2061.73] XCom_HitRolls: Modifying eHit_Success +68 (Aim), New hit chance: 68 [2061.73] XCom_HitRolls: Modifying eHit_Success -9 (Red Fog), New hit chance: 59 [2061.73] XCom_HitRolls: Modifying eHit_Success +20 (Weapon Accuracy), New hit chance: 79 [2061.73] XCom_HitRolls: Modifying eHit_Success -10 (Defense), New hit chance: 69 [2061.73] XCom_HitRolls: Modifying eHit_Success +0 (Weapon Range), New hit chance: 69 [2061.73] XCom_HitRolls: Modifying eHit_Success +0 (Melee Bonus), New hit chance: 69 [2061.73] XCom_HitRolls: Shooter is concealed, target cannot dodge. [2061.74] XCom_HitRolls: Modifying eHit_Crit +0 (Character Skill), New hit chance: 69 [2061.74] XCom_HitRolls: Modifying eHit_Crit +10 (Weapon Crit), New hit chance: 69 [2061.74] XCom_HitRolls: ==FinalizeHitChance==

[2061.74] XCom_HitRolls: Starting values... [2061.74] XCom_HitRolls: eHit_Success: 69 [2061.74] XCom_HitRolls: eHit_Crit: 10 [2061.74] XCom_HitRolls: eHit_Graze: 0 [2061.74] XCom_HitRolls: eHit_Miss: 0 [2061.74] XCom_HitRolls: eHit_LightningReflexes: 0 [2061.74] XCom_HitRolls: eHit_Untouchable: 0 [2061.74] XCom_HitRolls: eHit_CounterAttack: 0 [2061.74] XCom_HitRolls: eHit_Parry: 0 [2061.74] XCom_HitRolls: eHit_Deflect: 0 [2061.74] XCom_HitRolls: eHit_Reflect: 0 [2061.74] XCom_HitRolls: Calculated values... [2061.74] XCom_HitRolls: eHit_Success: 59 [2061.74] XCom_HitRolls: eHit_Crit: 10 [2061.74] XCom_HitRolls: eHit_Graze: 0 [2061.74] XCom_HitRolls: eHit_Miss: 31 [2061.74] XCom_HitRolls: eHit_LightningReflexes: 0 [2061.74] XCom_HitRolls: eHit_Untouchable: 0 [2061.74] XCom_HitRolls: eHit_CounterAttack: 0 [2061.74] XCom_HitRolls: eHit_Parry: 0 [2061.74] XCom_HitRolls: eHit_Deflect: 0 [2061.74] XCom_HitRolls: eHit_Reflect: 0 [2061.74] XCom_HitRolls: Final hit chance (success + crit + graze) = 69 [2061.74] XCom_HitRolls: =InternalRollForAbilityHit= [2061.74] XCom_HitRolls: Final hit chance: 69 [2061.74] XCom_HitRolls: Random roll: 50 [2061.74] XCom_HitRolls: Checking table eHit_Success (59)... [2061.74] XCom_HitRolls: MATCH! [2061.74] XCom_HitRolls: ***HIT eHit_Success

RedDoby commented 4 years ago

So the problem seems to be that for standardshot it is using the protected function int GetHitChance in X2AbilityToHitCalc.uc

MrNiceUK commented 4 years ago

What other mods are you using? GetStatModifiers() is only called twice within the base WotC code, and both of them are switched out to GetStatModifiersFixed() in the CHL. In otherwords, with just the CHL, no other mods, GetStatModifiers() is never called except through the GetStatModifiersFixed() wrapper...

RedDoby commented 4 years ago

Ahh. I think I get it now. XModBase_core_2_0_1 is still using GetStatModifiers, and simple red fog uses that. It's also worth noting that RPG is also using XModBase_core for it's ability modifiers. Glad we worked this out.

MrNiceUK commented 4 years ago

Yeah, XModBase has several "hidden" overrides...