Skip to content
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.

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

XCGS_Unit:GetStatModifiers reports bogus modifiers for multipliers #313

Closed
robojumper opened this issue Oct 21, 2018 · 10 comments · Fixed by #796
Closed

XCGS_Unit:GetStatModifiers reports bogus modifiers for multipliers #313

robojumper opened this issue Oct 21, 2018 · 10 comments · Fixed by #796

Comments

@robojumper
Copy link
Member

robojumper commented Oct 21, 2018

native function GetStatModifiers(ECharStatType Stat, out array<XComGameState_Effect> Mods, out array<float> ModValues, optional XComGameStateHistory GameStateHistoryObject);

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:

AddModifier(UnitState.GetBaseStat(eStat_Offense), class'XLocalizedData'.default.OffenseStat, m_ShotBreakdown, eHit_Success, bDebugLog);
UnitState.GetStatModifiers(eStat_Offense, StatMods, StatModValues);
for (i = 0; i < StatMods.Length; ++i)
{
AddModifier(int(StatModValues[i]), StatMods[i].GetX2Effect().FriendlyName, m_ShotBreakdown, eHit_Success, bDebugLog);
}

and a bogus to-crit chance:

AddModifier(UnitState.GetBaseStat(eStat_CritChance), class'XLocalizedData'.default.CharCritChance, m_ShotBreakdown, eHit_Crit, bDebugLog);
UnitState.GetStatModifiers(eStat_CritChance, StatMods, StatModValues);
for (i = 0; i < StatMods.Length; ++i)
{
AddModifier(int(StatModValues[i]), StatMods[i].GetX2Effect().FriendlyName, m_ShotBreakdown, eHit_Crit, bDebugLog);
}

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

We should fix (or work around) the problem.

MrNiceUK added a commit to MrNiceUK/X2WOTCCommunityHighlander that referenced this issue Oct 22, 2018
MrNiceUK added a commit to MrNiceUK/X2WOTCCommunityHighlander that referenced this issue Oct 22, 2018
MrNiceUK added a commit to MrNiceUK/X2WOTCCommunityHighlander that referenced this issue May 12, 2019
@RedDoby
Copy link
Contributor

RedDoby commented Sep 17, 2019

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

@RedDoby
Copy link
Contributor

RedDoby commented Oct 21, 2019

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
Copy link
Contributor

MrNiceUK commented Oct 21, 2019

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

@RedDoby
Copy link
Contributor

RedDoby commented Oct 22, 2019

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
Copy link
Contributor

RedDoby commented Oct 22, 2019

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
Copy link
Contributor

RedDoby commented Oct 22, 2019

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
Copy link
Contributor

RedDoby commented Oct 22, 2019

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

@MrNiceUK
Copy link
Contributor

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
Copy link
Contributor

RedDoby commented Oct 22, 2019

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
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants