Apologies for the long post, but the issue is deep in Enhanced Input land.
TLDR: Chorded actions can fail to block input due to an issue where a blocked trigger is getting overridden by an “unevaluated” trigger. Also an issue where the last mapping for an Input Action in IMC will be used to determine if there are any triggers and thus if we should even check it for evaluationg triggers
Duplicating fixes here
In UEnhancedPlayerInput::ProcessActionMappingEvent
// Fix
if (ActionData.TriggerStateTracker < TriggerStateTracker)
{
TriggerStateTracker.SetMappingTriggerApplied(bMappingTriggersApplied);
ActionData.TriggerStateTracker = TriggerStateTracker;
}
// Bug. bMappingTriggersApplied should only be set if we take the `TriggerStateTracker`
//ActionData.TriggerStateTracker = FMath::Max(ActionData.TriggerStateTracker, TriggerStateTracker);
//ActionData.TriggerStateTracker.SetMappingTriggerApplied(bMappingTriggersApplied);
In InputAction.h
struct FTriggerStateTracker
//Fix
bool operator>=(const FTriggerStateTracker& Other) const
{
if (GetState() == Other.GetState())
{
return bBlocking >= Other.bBlocking;
}
return GetState() >= Other.GetState();
}
bool operator< (const FTriggerStateTracker& Other) const { return !((*this) >= Other); }
// Bug, An unevaluated TriggerStateTracker is equivalent to a blocking one
// when used to choose which mapping to take, but is NOT equivalent when
// determining to use the mappings triggers or the input action's
//bool operator>=(const FTriggerStateTracker& Other) const { return GetState() >= Other.GetState(); }
//bool operator< (const FTriggerStateTracker& Other) const { return GetState() < Other.GetState(); }
Using Enhanced Input we were trying to set up input actions with chorded triggers
We had an IMC for our chorded actions and a lower priority one for non-chorded actions to make certain the chorded ones would consume the input of the non-chorded actions
Our action here is IA_Hotkey1AppendSelect and IA_Hotkey1Select.
Desired behavior:
Keyboard
Ctrl+1:- Triggers IA_Hotkey1AppendSelect
- Consumes/blocks (1)
- Does not trigger IA_Hotkey1Select
1:
- Triggers IA_Hotkey1Select
- Does not trigger IA_Hotkey1AppendSelect as Ctrl is not pressed
Gamepad
LT+LB:- Triggers IA_Hotkey1AppendSelect
- Consumes/blocks (1)
- Does not trigger IA_Hotkey1Select
LB:
- Triggers IA_Hotkey1Select
- Does not trigger IA_Hotkey1AppendSelect as LT is not pressed
Actual behavior:
Keyboard
Ctrl+1:- Triggers IA_Hotkey1AppendSelect
- Does not consume/block (1)
- Does trigger IA_Hotkey1Select
1:
- Triggers IA_Hotkey1Select
- Trigger IA_Hotkey1AppendSelect as Ctrl is not pressed
Gamepad
LT+LB:- Triggers IA_Hotkey1AppendSelect
- Consumes/blocks (1)
- Does not trigger IA_Hotkey1Select
LB:
- Triggers IA_Hotkey1Select
- Does not trigger IA_Hotkey1AppendSelect as LT is not pressed
Bug
The bug **only** occurs if you have multiple mappings to the same input action. If you remove the gamepad or the keyboard mapping, the problem disappears. The problem also shows there is an order of operations bug as only one of the two will have the bugCause
Problem 1
In `UEnhancedPlayerInput::ProcessActionMappingEvent` each mapping combines their `TriggerStateTracker` data into their associated `FInputActionInstance` ActionData.It does it with these two lines
ActionData.TriggerStateTracker = FMath::Max(ActionData.TriggerStateTracker, TriggerStateTracker);
ActionData.TriggerStateTracker.SetMappingTriggerApplied(bMappingTriggersApplied);
bMappingTriggersApplied
is critical because this is how triggers on an IMC’s mapping get applied. If this is false, it will always use only the triggers from the UInputAction
itself.
The above though causes an issue where if you have two mappings, but the second has no triggers, the first one will have its bMappingTriggersApplied
overridden. It should instead only set bMappingTriggersApplied
if it overwrites ActionData.TriggerStateTracker
.
Fix 1
// This fixes an issue where only the last bMappingTriggersApplied would be used
if (ActionData.TriggerStateTracker < TriggerStateTracker)
{
TriggerStateTracker.SetMappingTriggerApplied(bMappingTriggersApplied);
ActionData.TriggerStateTracker = TriggerStateTracker;
}
Problem 2
The FMath::Max
also is problematic, but much sneakier
bool operator>=(const FTriggerStateTracker& Other) const { return GetState() >= Other.GetState(); }
bool operator< (const FTriggerStateTracker& Other) const { return GetState() < Other.GetState(); }
ETriggerState FTriggerStateTracker::GetState() const
{
if(!bEvaluatedTriggers)
{
return NoTriggerState;
}
if (bBlocking)
{
return ETriggerState::None;
}
bool bTriggered = ((!bFoundExplicit || bAnyExplictTriggered) && bAllImplicitsTriggered);
return bTriggered ? ETriggerState::Triggered : (bFoundActiveTrigger ? ETriggerState::Ongoing : ETriggerState::None);
}
Notice the TriggerStateTracker
uses only the ETriggerState
to determine if we should use the current mappings TriggerStateTracker
.
UENUM()
enum class ETriggerState : uint8
{
// No inputs
None,
// Triggering is being monitored, but not yet been confirmed (e.g. a time based trigger that requires the trigger state to be maintained over several frames)
Ongoing,
// The trigger state has been met
Triggered,
};
If bBlocking
is true, it returns ETriggerState::None
, but if no triggers were evaluated as in no key was pressed, it returns NoTriggerState
. At the time of ProcessActionMappingEvent
that is also ETriggerState::None
. That means a default constructed TriggerStateTracker
and a blocked one are equivalent and thus FMath::Max
can depending on the order use the unevaluated one.
Fix 2
bool operator>=(const FTriggerStateTracker& Other) const
{
if (GetState() == Other.GetState())
{
return bBlocking >= Other.bBlocking;
}
return GetState() >= Other.GetState();
}
bool operator< (const FTriggerStateTracker& Other) const { return !((*this) >= Other); }
//bool operator>=(const FTriggerStateTracker& Other) const { return GetState() >= Other.GetState(); }
//bool operator< (const FTriggerStateTracker& Other) const { return GetState() < Other.GetState(); }
My fix means all else being equal, we should return bBlocking
being true.
Cause continued
If `NoTriggerState` is equal to `ETriggerState::None` why does it matter?UEnhancedPlayerInput::EvaluateInputDelegates
is where we process the input and determine if input actions should be triggered
if (ActionsWithEventsThisTick.Contains(Action))
{
// Apply action modifiers
FInputActionValue RawValue = ActionData.Value;
ActionData.Value = ApplyModifiers(ActionData.Modifiers, ActionData.Value, NonDilatedDeltaTime);
// Update what state to use for this data in the case of there being no triggers, otherwise we can get incorrect triggered
// states even if the modified value is Zero
if(ActionData.Value.Get<FVector>() != RawValue.Get<FVector>())
{
ActionData.TriggerStateTracker.SetStateForNoTriggers(ActionData.Value.IsNonZero() ? ETriggerState::Triggered : ETriggerState::None);
}
ETriggerState PrevState = ActionData.TriggerStateTracker.GetState();
// Evaluate action triggers. We must always call EvaluateTriggers to update any internal state, even when paused.
TriggerState = ActionData.TriggerStateTracker.EvaluateTriggers(this, ActionData.Triggers, ActionData.Value, NonDilatedDeltaTime);
TriggerState = ActionData.TriggerStateTracker.GetMappingTriggerApplied() ? FMath::Min(TriggerState, PrevState) : TriggerState;
// However, if the game is paused invalidate trigger unless the action allows it.
// TODO: Potential issues with e.g. hold event that's canceled due to pausing, but jumps straight back to its "triggered" state on unpause if the user continues to hold the key.
if (bGamePaused && !Action->bTriggerWhenPaused)
{
TriggerState = ETriggerState::None;
}
}
Below you can see we do set NoTriggerState
to ETriggerStateTriggered
as long as the key is pressed. In our case of pressing Ctrl+1, we see 1 is pressed, so this means IA_Hotkey1Select
will be considered triggered even when it should be blocked.
if(ActionData.Value.Get<FVector>() != RawValue.Get<FVector>())
{
ActionData.TriggerStateTracker.SetStateForNoTriggers(ActionData.Value.IsNonZero() ? ETriggerState::Triggered : ETriggerState::None);
}
ETriggerState PrevState = ActionData.TriggerStateTracker.GetState();
// Evaluate action triggers. We must always call EvaluateTriggers to update any internal state, even when paused.
TriggerState = ActionData.TriggerStateTracker.EvaluateTriggers(this, ActionData.Triggers, ActionData.Value, NonDilatedDeltaTime);
TriggerState = ActionData.TriggerStateTracker.GetMappingTriggerApplied() ? FMath::Min(TriggerState, PrevState) : TriggerState;
Also in the above code you can see where GetMappingTriggerApplied
is used to determine if it should use the mapping triggers, or if it should just use the input action