FinalizeHitChance and OverrideFinalHitChanceFns don't include enough information for FinalizeHitChance mods

[Tedster59]

Issue #555 added these hooks, but they did not add information about the ability and target if available, such as in X2AbilityToHitCalc_StandardAim

We should add new versions that pass through the ability and target information if available.

GetAdditionalHitModifiers_CH receives this information, but FinalizeHitChance does not.

Proposed Solution: Update X2AbilityToHitCalc.FinalizeHitChance to take in XComGameState_Ability kAbility and AvailableTarget kTarget as optional parameters, update X2AbilityToHitCalc_StandardAim to pass them, and then add a new delegate that will pass these through as well.

Example use case: LW's aim rolls calculation does not respect checking for effects that implement the ShotsCannotGraze function because it does not have that information, this new version would allow LW to check for those effects and respect them.

[Iridar]

That's gonna be problematic. FinalizeHitChance lives in X2AbilityToHitCalc.uc.

The best we could do is create a FinalizeHitChance_CH that will be get called by GetHitChance(), and then wrap the original FinalizeHitChance in it, but that sounds like something that can very easily break if not handled properly by subclasses, especially mod-added ones.

It will also start creeping towards a backwards compatibility hell, because original OverrideHitChanceFns will still need to be supported (which were a terrible implementation, in my opinion).

[Tedster59]

Yeah, that sounds like a compatibility nightmare. I wonder what other mods are using this beyond LWOTC (and my standalone implementation), which is why I'd want to make sure the new stuff is optional and mods using the new hook can't expect the new additions to be passed in 100% of the time. XModBase and its MCO will probably step on its toes anyways.

[Tedster59]

As an alternative implementation, would it be possible to trigger a new event here that passes a tuple with the shotbreakdown, ability, target (or just pass them in as event parameters) along with a bool for CHL to know whether a mod used it or not? This seems less robust than the delegate implementation, but would likely be better for backwards compatibility as none of the previous functionality is touched.

e.g from this:

to something like this.

Any mod using this would have to deal with XModBase jank, but they can get around it by simply requiring my XMB Redux as a dependency or shipping it themselves in usual XMB style.

[Iridar]

No, GetHitChance is called by UI code, we can't trigger events here.

[Tedster59]

welp, so much for that plan then.