Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
660 changes: 547 additions & 113 deletions Assets/Tests/InputSystem/CoreTests_Actions.cs

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion Assets/Tests/InputSystem/CoreTests_Actions_Interactions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ public void Actions_StartedAndCanceledAreEnforcedImplicitly()

[Test]
[Category("Actions")]
public void Action_WithMultipleInteractions_DoesNotThrowWhenUsingMultipleMaps()
public void Actions_WithMultipleInteractions_DoNotThrowWhenUsingMultipleMaps()
{
var gamepad = InputSystem.AddDevice<Gamepad>();

Expand Down
4 changes: 4 additions & 0 deletions Packages/com.unity.inputsystem/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ however, it has to be formatted properly to pass verification tests.
* `UnityEngine.InputSystem.Android.LowLevel.AndroidAxis`
* `UnityEngine.InputSystem.Android.LowLevel.AndroidGameControllerState`
* `UnityEngine.InputSystem.Android.LowLevel.AndroidKeyCode`
- Adding or removing a device no longer leads to affected actions being temporarily disabled ([case 1379932](https://issuetracker.unity3d.com/issues/inputactionreferences-reading-resets-when-inputactionmap-has-an-action-for-the-other-hand-and-that-hand-starts-slash-stops-tracking)).
* If, for example, an action was bound to `<Gamepad>/buttonSouth` and was enabled, adding a second `Gamepad` would lead to the action being temporarily disabled, then updated, and finally re-enabled.
* This was especially noticeable if the action was currently in progress as it would get cancelled and then subsequently resumed.
* Now, an in-progress action will get cancelled if the device of its active control is removed. If its active control is not affected, however, the action will keep going regardless of whether controls are added or removed from its `InputAction.controls` list.

### Fixed

Expand Down
13 changes: 13 additions & 0 deletions Packages/com.unity.inputsystem/Documentation~/ActionBindings.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
* [Control schemes](#control-schemes)
* [Details](#details)
* [Binding resolution](#binding-resolution)
* [Binding resolution while Actions are enabled](#binding-resolution-while-actions-are-enabled)
* [Choosing which Devices to use](#choosing-which-devices-to-use)
* [Conflict resolution](#conflict-resolution)
* [Initial state check](#initial-state-check)
Expand Down Expand Up @@ -665,6 +666,18 @@ If there are multiple Bindings on the same Action that all reference the same Co

To query the Controls that an Action resolves to, you can use [`InputAction.controls`](../api/UnityEngine.InputSystem.InputAction.html#UnityEngine_InputSystem_InputAction_controls). You can also run this query if the Action is disabled.

To be notified when binding resolution happens, you can listen to [`InputSystem.onActionChange`](../api/UnityEngine.InputSystem.InputSystem.html#UnityEngine_InputSystem_InputSystem_onActionChange) which triggers [`InputActionChange.BoundControlsAboutToChange`](../api/UnityEngine.InputSystem.InputActionChange.html#UnityEngine_InputSystem_InputActionChange_BoundControlsAboutToChange) before modifying Control lists and triggers [`InputActionChange.BoundControlsChanged`](../api/UnityEngine.InputSystem.InputActionChange.html#UnityEngine_InputSystem_InputActionChange_BoundControlsChanged) after having updated them.

#### Binding resolution while Actions are enabled

In certain situations, the [Controls](../api/UnityEngine.InputSystem.InputAction.html#UnityEngine_InputSystem_InputAction_controls) bound to an Action have to be updated more than once. For example, if a new [Device](Devices.md) becomes usable with an Action, the Action may now pick up input from additional controls. Also, if Bindings are added, removed, or modified, Control lists will need to be updated.

This updating of Controls usually happens transparently in the background. However, when an Action is [enabled](../api/UnityEngine.InputSystem.InputAction.html#UnityEngine_InputSystem_InputAction_enabled) and especially when it is [in progress](../api/UnityEngine.InputSystem.InputAction.html#UnityEngine_InputSystem_InputAction_IsInProgress_), there may be a noticeable effect on the Action.

Adding or removing a device &ndash; either [globally](../api/UnityEngine.InputSystem.InputSystem.html#UnityEngine_InputSystem_InputSystem_devices) or to/from the [device list](../api/UnityEngine.InputSystem.InputActionAsset.html#UnityEngine_InputSystem_InputActionAsset_devices) of an Action &ndash; will remain transparent __except__ if an Action is in progress and it is the device of its [active Control](../api/UnityEngine.InputSystem.InputAction.html#UnityEngine_InputSystem_InputAction_activeControl) that is being removed. In this case, the Action will automatically be [cancelled](../api/UnityEngine.InputSystem.InputAction.html#UnityEngine_InputSystem_InputAction_canceled).

Modifying the [binding mask](../api/UnityEngine.InputSystem.InputActionAsset.html#UnityEngine_InputSystem_InputActionAsset_bindingMask) or modifying any of the Bindings (such as through [rebinding](#interactive-rebinding) or by adding or removing bindings) will, however, lead to all enabled Actions being temporarily disabled and then re-enabled and resumed.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missed these changes in my initial commit.


#### Choosing which Devices to use

>__Note__: [`InputUser`](UserManagement.md) and [`PlayerInput`](Components.md) make use of this facility automatically. They set [`InputActionMap.devices`](../api/UnityEngine.InputSystem.InputActionMap.html#UnityEngine_InputSystem_InputActionMap_devices) automatically based on the Devices that are paired to the user.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ public interface IInputActionCollection : IEnumerable<InputAction>
/// </summary>
/// <remarks>
/// If this is not null, only bindings that match the mask will be used.
///
/// Modifying this property while any of the actions in the collection are enabled will
/// lead to the actions getting disabled temporarily and then re-enabled.
/// </remarks>
InputBinding? bindingMask { get; set; }

Expand All @@ -31,6 +34,13 @@ public interface IInputActionCollection : IEnumerable<InputAction>
/// only one gamepad is listed here, then a "&lt;Gamepad&gt;/leftStick" binding will
/// only bind to the gamepad in the list and not to the one that is only available
/// globally.
///
/// Modifying this property after bindings in the collection have already been resolved,
/// will lead to <see cref="InputAction.controls"/> getting refreshed. If any of the actions
/// in the collection are currently in progress (see <see cref="InputAction.phase"/>),
/// the actions will remain unaffected and in progress except if the controls currently
/// driving them (see <see cref="InputAction.activeControl"/>) are no longer part of any
/// of the selected devices. In that case, the action is <see cref="InputAction.canceled"/>.
/// </remarks>
ReadOnlyArray<InputDevice>? devices { get; set; }

Expand Down
56 changes: 53 additions & 3 deletions Packages/com.unity.inputsystem/InputSystem/Actions/InputAction.cs
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ public InputBinding? bindingMask

var map = GetOrCreateActionMap();
if (map.m_State != null)
map.LazyResolveBindings();
map.LazyResolveBindings(fullResolve: true);
}
}

Expand Down Expand Up @@ -1095,6 +1095,22 @@ public unsafe bool IsPressed()
return false;
}

/// <summary>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this method really necessary when the inProgress property exists?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely not. Me being confused. Will remove.

/// Whether the action has been <see cref="InputActionPhase.Started"/> or <see cref="InputActionPhase.Performed"/>.
/// </summary>
/// <returns>True if the action is currently triggering.</returns>
/// <seealso cref="phase"/>
public unsafe bool IsInProgress()
{
var state = GetOrCreateActionMap().m_State;
if (state != null)
{
var actionStatePtr = &state.actionStates[m_ActionIndexInState];
return actionStatePtr->phase.IsInProgress();
}
return false;
}

/// <summary>
/// Returns true if the action's value crossed the press threshold (see <see cref="InputSettings.defaultButtonPressPoint"/>)
/// at any point in the frame.
Expand Down Expand Up @@ -1479,8 +1495,8 @@ private InputActionState.TriggerState currentState
{
if (m_ActionIndexInState == InputActionState.kInvalidIndex)
return new InputActionState.TriggerState();
Debug.Assert(m_ActionMap != null);
Debug.Assert(m_ActionMap.m_State != null);
Debug.Assert(m_ActionMap != null, "Action must have associated action map");
Debug.Assert(m_ActionMap.m_State != null, "Action map must have state at this point");
return m_ActionMap.m_State.FetchActionState(this);
}
}
Expand Down Expand Up @@ -1514,6 +1530,40 @@ private void CreateInternalActionMapForSingletonAction()
};
}

internal void RequestInitialStateCheckOnEnabledAction()
{
Debug.Assert(enabled, "This should only be called on actions that are enabled");

var map = GetOrCreateActionMap();
var state = map.m_State;
state.SetInitialStateCheckPending(m_ActionIndexInState);
}

// NOTE: This does *NOT* check whether the control is valid according to the binding it
// resolved from and/or the current binding mask. If, for example, the binding is
// "<Keyboard>/#(ä)" and the keyboard switches from a DE layout to a US layout, the
// key would still be considered valid even if the path in the binding would actually
// no longer resolve to it.
internal bool ActiveControlIsValid(InputControl control)
{
if (control == null)
return false;

// Device must still be added.
var device = control.device;
if (!device.added)
return false;

// If we have a device list in the map or asset, device
// must be in list.
var map = GetOrCreateActionMap();
var deviceList = map.devices;
if (deviceList != null && !deviceList.Value.ContainsReference(device))
return false;

return true;
}

internal InputBinding? FindEffectiveBindingMask()
{
if (m_BindingMask.HasValue)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ public InputBinding? bindingMask

m_BindingMask = value;

ReResolveIfNecessary();
ReResolveIfNecessary(fullResolve: true);
}
}

Expand Down Expand Up @@ -240,7 +240,7 @@ public ReadOnlyArray<InputDevice>? devices
set
{
if (m_Devices.Set(value))
ReResolveIfNecessary();
ReResolveIfNecessary(fullResolve: false);
}
}

Expand Down Expand Up @@ -866,15 +866,31 @@ internal void MarkAsDirty()
#endif
}

private void ReResolveIfNecessary()
internal void OnWantToChangeSetup()
{
if (m_ActionMaps.LengthSafe() > 0)
m_ActionMaps[0].OnWantToChangeSetup();
}

internal void OnSetupChanged()
{
MarkAsDirty();

if (m_ActionMaps.LengthSafe() > 0)
m_ActionMaps[0].OnSetupChanged();
else
m_SharedStateForAllMaps = null;
}

private void ReResolveIfNecessary(bool fullResolve)
{
if (m_SharedStateForAllMaps == null)
return;

Debug.Assert(m_ActionMaps != null && m_ActionMaps.Length > 0);
// State is share between all action maps in the asset. Resolving bindings for the
// first map will resolve them for all maps.
m_ActionMaps[0].LazyResolveBindings();
m_ActionMaps[0].LazyResolveBindings(fullResolve);
}

private void OnDestroy()
Expand Down
Loading