X2CommunityCore / X2WOTCCommunityHighlander

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

Make guaranteed crit abilities display as having 100% crit chance #1339

Closed Iridar closed 4 months ago

Iridar commented 4 months ago

Fixes #1298

The Problem

There are two main components to X2AbilityToHitCalc_StandardAim, the GetHitChance() which is used both for hit chance preview in UI and in the second component, InternalRollForAbilityHit(), which does the actual hit roll.

The bHitsAreCrits bool flag is not processed in GetHitChance() at all, only in the roll function, where it will replace eHit_Success hit result with eHit_Crit. So outside the ability description, the player has no way of knowing that the ability will crit.

Here's how it looks in-game:

20240503003422_1 20240503003420_1

Proposed Fix

Give the bHitsAreCrits the same treatment as bGuaranteedHit:

  1. At the start of GetHitChance(), super.AddModifier() is called to add 100 to crit chance using the super function from X2AbilityToHitCalc class.
  2. AddModifier() function itself in X2AbilityToHitCalc_StandardAim is adjusted to disallow adding crit modifiers if the bHitsAreCrits flag is set.

As the result, the ability will have one and only one crit chance modifier that equals to 100, and all other crit chance modifiers, such as from flanking, will be ignored. So in-game UI for bHitsAreCrits abilities will always display exactly 100%.

Here's how it looks:

20240503003043_1 20240503003040_1

Addressing Concerns

This change does not alter handling of bHitsAreCrits in InternalRollForAbilityHit(), so nothing changes functionality-wise. Custom ToHitCalcs that override GetHitChance() or AddModifier() , at worst, will display incorrect crit chance, which they would already be doing in regards to bHitsAreCrits abilities.

Extended Information

Extended Information mod seems to correctly process the changed logic:

20240503004238_1 20240503004235_1

Iridar commented 4 months ago

Note: bAllowCrit is not being checked because InternalRollForAbilityHit() does not check it for bHitsAreCrits functionality either, so we can just assume bHitsAreCrits overrides bAllowCrit.