bIndirectFire hits aren't guaranteed

[Iridar]

Related code

When an ability's hit is supposed to be guaranteed, current code just adds 100 to hit chance, and then chugs along merrily, potentially adding negative hit modifiers from other sources, such as Defense from a Smoke Grenade making grenades no longer guaranteed:

[0448.59] XCom_HitRolls: ===InternalRollForAbilityHit===
[0448.59] XCom_HitRolls: Attacker ID: 3115
[0448.59] XCom_HitRolls: Target ID: 2376
[0448.59] XCom_HitRolls: Ability: Throw Grenade (ThrowGrenade)
[0448.59] XCom_HitRolls: =GetHitChance=
[0448.59] XCom_HitRolls: Modifying eHit_Success +100 (Throw Grenade), New hit chance: 100
[0448.59] XCom_HitRolls: Modifying eHit_Success +0 (Throw Grenade), New hit chance: 100
[0448.59] XCom_HitRolls: Modifying eHit_Crit +0 (Throw Grenade), New hit chance: 100
[0448.59] XCom_HitRolls: Modifying eHit_Success -20 (Defensive Smoke), New hit chance: 80
[0448.59] XCom_HitRolls: ==FinalizeHitChance==

[0448.59] XCom_HitRolls: Starting values...
[0448.59] XCom_HitRolls: eHit_Success: 80
[0448.59] XCom_HitRolls: eHit_Crit: 0
[0448.59] XCom_HitRolls: eHit_Graze: 0
[0448.59] XCom_HitRolls: eHit_Miss: 0
[0448.59] XCom_HitRolls: eHit_LightningReflexes: 0
[0448.59] XCom_HitRolls: eHit_Untouchable: 0
[0448.59] XCom_HitRolls: eHit_CounterAttack: 0
[0448.59] XCom_HitRolls: eHit_Parry: 0
[0448.59] XCom_HitRolls: eHit_Deflect: 0
[0448.59] XCom_HitRolls: eHit_Reflect: 0
[0448.59] XCom_HitRolls: Calculated values...
[0448.59] XCom_HitRolls: eHit_Success: 80
[0448.59] XCom_HitRolls: eHit_Crit: 0
[0448.59] XCom_HitRolls: eHit_Graze: 0
[0448.59] XCom_HitRolls: eHit_Miss: 20
[0448.59] XCom_HitRolls: eHit_LightningReflexes: 0
[0448.59] XCom_HitRolls: eHit_Untouchable: 0
[0448.59] XCom_HitRolls: eHit_CounterAttack: 0
[0448.59] XCom_HitRolls: eHit_Parry: 0
[0448.59] XCom_HitRolls: eHit_Deflect: 0
[0448.59] XCom_HitRolls: eHit_Reflect: 0
[0448.59] XCom_HitRolls: Final hit chance (success + crit + graze) = 80
[0448.59] XCom_HitRolls: =InternalRollForAbilityHit=
[0448.59] XCom_HitRolls: Final hit chance: 80
[0448.59] XCom_HitRolls: Random roll: 95
[0448.59] XCom_HitRolls: Checking table eHit_Success (80)...
[0448.59] XCom_HitRolls: Checking table eHit_Crit (80)...
[0448.59] XCom_HitRolls: Checking table eHit_Graze (80)...
[0448.59] XCom_HitRolls: ***MISS eHit_Miss

[Tedster59]

bIndirectFire does the same thing, thus the Kiruka fix in some Mod Jam stuff to set bGuaranteedHit for these abilities too.

The modifier for shooting non-units such as objectives is also just a flat 100 and handled in a separate if statement further down:

if (UnitState != none && TargetState == none)
	{
		// when targeting non-units, we have a 100% chance to hit. They can't dodge or otherwise
		// mess up our shots
		m_ShotBreakdown.HideShotBreakdown = true;
		AddModifier(100, class'XLocalizedData'.default.OffenseStat, m_ShotBreakdown, eHit_Success, bDebugLog);
	}

Only the section for bGuaranteedHit calls super.AddModifier directly, the others call . I wonder if the short circuit condition in the version that's part of X2AbilityToHitCalc_StandardAim is triggering when it shouldn't.

Another possible explanation, the code that adds the units base aim to the modifiers is located inside the if (!bIndirectFire) block, so indirect attacks such as grenades are not getting the soldier's base aim stat added to start with, leaving the aim at the flat 100 before defense modifiers from effects come into play.

A band-aid fix would be to simply increase the values bGuaranteedHit and bIndirectFire add to aim. A proper fix would probably add some additional validation at the end to modify m_shotbreakdown if either of these variables are true.

[Iridar]

@Tedster59 sorry, I think I confused you with the issue name, this issue happens specifically with bIndirectFire abilities.

With both bIndirectFire and bGuaranteedHit the code adds 100 to hit chance, but the following distinct difference is that with bGuaranteedHit all other hit modifiers besides crit are ignored.

So bGuaranteedHit hits are in fact guaranteed, as far as I can tell.

Hits being "guaranteed" against interactive objects is a separate issue and was already handled in 1.25.0.

#1213 changes the code to treat bIndirectFire the same way as bGuaranteedHit for the purposes of hit modifiers.

The dev comment suggests that with bGuaranteedHit only Hit and Crit are possible outcomes, while with bIndirectFire Graze is possible too, but that comment seems to be out of touch with their own implementation that removes Graze as a possible outcome if the hit chance is 100.

I'm getting the impression that Indirect Fire as a concept is somewhat raw and unfinished. X2Effect_Persistent::GetToHitModifiers() receives bIndirectFire from the Hit Calc, suggesting that different effects may add different hit modifiers depending on if the ability is indirect fire or not.

For example, Suppression explicitly avoids adding modifiers to indirect fire abilities, as if to make it possible to implement some sort of "blind fire from behind cover" ability or to prevent Suppression from altering hit chance or scatter of grenade throws, which is not a mechanic that was ever implement in XCOM 2, but maybe it was planned at some point?

However, indirect fire ever ends up being used only for grenades and similar explosive weapons, which are never supposed to miss anyway.

With all that in mind, I feel relatively confident in making the code treat bIndirectFire and bGuaranteedHit the same way as far as ignoring hit modifiers goes, because it's the only way to reliably fix all instances of using bIndirectFire that assumed it's never supposed to miss, even if it's somewhat BC breaking and almost elevates bIndirectFire argument in GetToHitModifiers() to the deprecated status.

[Iridar]

#1216 that fixed this issue in 1.26 was reverted in 1.26.1 due to breaking certain abilities when used alongside XMB. Alternative approach to be figured out in 1.27.