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

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

Merged
merged 1 commit into from
May 2, 2024

Conversation

Iridar
Copy link
Contributor

@Iridar Iridar commented May 2, 2024

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 Iridar added this to the 1.28.0 milestone May 2, 2024
@Iridar Iridar self-assigned this May 2, 2024
@Iridar
Copy link
Contributor Author

Iridar commented May 2, 2024

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

@Iridar Iridar merged commit 1402036 into X2CommunityCore:master May 2, 2024
4 checks passed
@Iridar Iridar deleted the 1298-guaranteed-crit-ui-fix branch May 2, 2024 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Guaranteed crit abilities do not display as guaranteed crits in the UI
2 participants