FIX: Actions not triggered correctly with multiple bindings to same control (case 1293808, #1256).

Author: Rene Damm

Assets/Tests/InputSystem/CoreTests_Actions.cs

@@ -165,12 +165,86 @@ public void Actions_CanTargetSameControlWithMultipleActions()
         }
     }
 
+    // We may be looking at a situation of having more than one binding path on the same action
+    // matches the same control in a system.
+    // https://fogbugz.unity3d.com/f/cases/1293808/
     [Test]
     [Category("Actions")]
-    [Ignore("TODO")]
-    public void TODO_Actions_WhenSeveralBindingsResolveToSameControl_ThenWhatDoWeDoXXX()
+    public void Actions_WhenSeveralBindingsResolveToSameControl_ControlIsAssociatedWithFirstActiveBinding()
     {
-        Assert.Fail();
+        var gamepad = InputSystem.AddDevice<Gamepad>();
+
+        var action1 = new InputAction();
+        var action2 = new InputAction();
+
+        var actionMap = new InputActionMap();
+        var action3 = actionMap.AddAction("action3");
+        var action4 = actionMap.AddAction("action4");
+
+        action1.AddBinding("<Gamepad>/buttonSouth");
+        action1.AddBinding("<Gamepad>/buttonSouth");
+
+        action2.AddBinding("<Gamepad>/buttonSouth");
+        action2.AddBinding("<Gamepad>/button*");
+
+        action3.AddBinding("<Gamepad>/buttonNorth");
+        action3.AddBinding("<Gamepad>/buttonSouth");
+        action3.AddBinding("<Gamepad>/buttonSouth");
+        action4.AddBinding("<Gamepad>/buttonSouth"); // Should not be removed; different action.
+
+        Assert.That(action1.controls, Is.EquivalentTo(new[] { gamepad.buttonSouth }));
+        Assert.That(action2.controls, Has.Exactly(1).SameAs(gamepad.buttonSouth));
+        Assert.That(action2.controls, Has.Count.EqualTo(4)); // North, south, east, west
+        Assert.That(action3.controls, Is.EquivalentTo(new[] { gamepad.buttonNorth, gamepad.buttonSouth }));
+        Assert.That(action4.controls, Is.EquivalentTo(new[] { gamepad.buttonSouth }));
+
+        Assert.That(action1.GetBindingIndexForControl(gamepad.buttonSouth), Is.EqualTo(0));
+        Assert.That(action2.GetBindingIndexForControl(gamepad.buttonSouth), Is.EqualTo(0));
+        Assert.That(action3.GetBindingIndexForControl(gamepad.buttonSouth), Is.EqualTo(1));
+        Assert.That(action4.GetBindingIndexForControl(gamepad.buttonSouth), Is.EqualTo(0));
+
+        // Go through a bit of pressing and releasing to make sure that the action state
+        // processing wasn't thrown off its track.
+
+        action1.Enable();
+        action2.Enable();
+        action3.Enable();
+        action4.Enable();
+
+        Press(gamepad.buttonSouth);
+
+        Assert.That(action1.triggered, Is.True);
+        Assert.That(action2.triggered, Is.True);
+        Assert.That(action3.triggered, Is.True);
+        Assert.That(action4.triggered, Is.True);
+
+        InputSystem.Update();
+
+        Assert.That(action1.triggered, Is.False);
+        Assert.That(action2.triggered, Is.False);
+        Assert.That(action3.triggered, Is.False);
+        Assert.That(action4.triggered, Is.False);
+
+        Release(gamepad.buttonSouth);
+
+        Assert.That(action1.triggered, Is.False);
+        Assert.That(action2.triggered, Is.False);
+        Assert.That(action3.triggered, Is.False);
+        Assert.That(action4.triggered, Is.False);
+
+        Press(gamepad.buttonSouth);
+
+        Assert.That(action1.triggered, Is.True);
+        Assert.That(action2.triggered, Is.True);
+        Assert.That(action3.triggered, Is.True);
+        Assert.That(action4.triggered, Is.True);
+
+        InputSystem.Update();
+
+        Assert.That(action1.triggered, Is.False);
+        Assert.That(action2.triggered, Is.False);
+        Assert.That(action3.triggered, Is.False);
+        Assert.That(action4.triggered, Is.False);
     }
 
     [Test]

Packages/com.unity.inputsystem/CHANGELOG.md

@@ -27,6 +27,18 @@ however, it has to be formatted properly to pass verification tests.
 - Fixed action with multiple bindings getting stuck in `Performed` state when two or more controls are pressed at the same time ([case 1295535](https://issuetracker.unity3d.com/issues/input-system-not-registering-multiple-inputs)).
   * Regression introduced in 1.1-preview.2.
 
+#### Actions
+
+- Fixed actions not triggering correctly when multiple bindings on the same action were referencing the same control ([case 1293808](https://issuetracker.unity3d.com/product/unity/issues/guid/1293808/)).
+  * Bindings will now "claim" controls during resolution. If several bindings __on the same action__ resolve to the same control, only the first such binding will successfully resolve to the control. Subsequent bindings will only resolve to controls not already referenced by other bindings on the action.
+  ```CSharp
+  var action = new InputAction();
+  action.AddBinding("<Gamepad>/buttonSouth");
+  action.AddBinding("<Gamepad>/buttonSouth"); // Will be ignored.
+  action.AddBinding("<Gamepad>/button*"); // Will only receive buttonWest, buttonEast, and buttonNorth.
+  ```
+  * This also means that `InputAction.controls` will now only contain any control at most once.
+
 ### Added
 
 - Added a new high-performance way to iterate over changed controls in an event.

Packages/com.unity.inputsystem/Documentation~/ActionBindings.md

@@ -660,6 +660,23 @@ Note that a single [Binding path](Controls.md#control-paths) can match multiple
 
 * A Binding path can also contain wildcards, such as `<Gamepad>/button*`. This matches any Control on any gamepad with a name starting with "button", which matches all the four action buttons on any connected gamepad. A different example: `*/{Submit}` matches any Control tagged with the "Submit" [usage](Controls.md#control-usages) on any Device.
 
+If there are multiple Bindings on the same Action that all reference the same Control(s), only the first such Binding will successfully bind to the control.
+
+```CSharp
+var action1 = new InputAction();
+
+action1.AddBinding("<Gamepad>/buttonSouth");
+action1.AddBinding("<Gamepad>/buttonSouth"); // This binding will be ignored.
+
+var action2 = new InputAction();
+
+action2.AddBinding("<Gamepad>/buttonSouth");
+// Add a binding that implicitly matches the first binding, too. When binding resolution
+// happens, this binding will only receive buttonNorth, buttonWest, and buttonEast, but not
+// buttonSouth as the first binding already received that control.
+action2.AddBinding("<Gamepad>/button*");
+```
+
 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.
 
 #### Choosing which Devices to use

Packages/com.unity.inputsystem/Documentation~/Actions.md

@@ -6,6 +6,7 @@
     * [Embedding Actions in MonoBehaviours](#embedding-actions-in-monobehaviours)
     * [Loading Actions from JSON](#loading-actions-from-json)
     * [Creating Actions in code](#creating-actions-in-code)
+      * [Default Actions](#default-actions)
 * [Using Actions](#using-actions)
     * [Responding to Actions](#responding-to-actions)
       * [Action callbacks](#action-callbacks)
@@ -183,6 +184,21 @@ asset.AddActionMap(gameplayMap);
 var lookAction = gameplayMap.AddAction("look", "<Gamepad>/leftStick");
 ```
 
+#### Default Actions
+
+An [asset](./ActionAssets.md) called `DefaultInputActions.inputactions` containing a default setup of Actions comes with the Input System Package. You can reference this asset directly in your projects like any other Unity asset. However, the asset is also available in code form through the [`DefaultInputActions`](../api/UnityEngine.InputSystem.DefaultInputActions.html) class.
+
+```CSharp
+void Start()
+{
+    // Create an instance of the default actions.
+    var actions = new DefaultInputActions();
+    actions.Player.Look.performed += OnLook;
+    actions.Player.Move.performed += OnMove;
+    actions.Enable();
+}
+```
+
 ## Using Actions
 
 For an Action to do something, you must first enable it. You can do this either by individually enabling Actions, or by enabling them in bulk through Action Maps. The second method is more efficient in all scenarios.

Packages/com.unity.inputsystem/InputSystem/Actions/InputAction.cs

@@ -427,7 +427,48 @@ public InputBinding? bindingMask
         /// will first resolve controls on the action (and for all actions in the map and/or
         /// the asset). See <a href="../manual/ActionBindings.html#binding-resolution">Binding Resolution</a>
         /// in the manual for details.
+        ///
+        /// To map a control in this array to an index into <see cref="bindings"/>, use
+        /// <see cref="InputActionRebindingExtensions.GetBindingIndexForControl"/>.
+        ///
+        /// <example>
+        /// <code>
+        /// // Map control list to binding indices.
+        /// var bindingIndices = myAction.controls.Select(c => myAction.GetBindingIndexForControl(c));
+        /// </code>
+        /// </example>
+        ///
+        /// Note that this array will not contain the same control multiple times even if more than
+        /// one binding on an action references the same control. Instead, the first binding on
+        /// an action that resolves to a particular control will essentially "own" the control
+        /// and subsequent bindings for the action will be blocked from resolving to the same control.
+        ///
+        /// <example>
+        /// <code>
+        /// var action1 = new InputAction();
+        /// action1.AddBinding("&lt;Gamepad&gt;/buttonSouth");
+        /// action1.AddBinding("&lt;Gamepad&gt;/buttonSouth"); // This binding will be ignored.
+        ///
+        /// // Contains only one instance of buttonSouth which is associated
+        /// // with the first binding (at index #0).
+        /// var action1Controls = action1.controls;
+        ///
+        /// var action2 = new InputAction();
+        /// action2.AddBinding("&lt;Gamepad&gt;/buttonSouth");
+        /// // Add a binding that implicitly matches the first binding, too. When binding resolution
+        /// // happens, this binding will only receive buttonNorth, buttonWest, and buttonEast, but not
+        /// // buttonSouth as the first binding already received that control.
+        /// action2.AddBinding("&lt;Gamepad&gt;/button*");
+        ///
+        /// // Contains only all four face buttons (buttonSouth, buttonNorth, buttonEast, buttonWest)
+        /// // but buttonSouth is associated with the first button and only buttonNorth, buttonEast,
+        /// // and buttonWest are associated with the second binding.
+        /// var action2Controls = action2.controls;
+        /// </code>
+        /// </example>
         /// </remarks>
+        /// <seealso cref="InputActionRebindingExtensions.GetBindingIndexForControl"/>
+        /// <seealso cref="bindings"/>
         public ReadOnlyArray<InputControl> controls
         {
             get

Packages/com.unity.inputsystem/InputSystem/Actions/InputActionRebindingExtensions.cs

@@ -1672,6 +1672,7 @@ public RebindingOperation WithControlsHavingToMatchPath(string path)
                 return this;
             }
 
+            ////REVIEW: This API has been confusing for users who usually will do something like WithControlsExcluding("Mouse"); find a more intuitive way to do this
             /// <summary>
             /// Prevent specific controls from being considered as candidate controls.
             /// </summary>

Packages/com.unity.inputsystem/InputSystem/Actions/InputBindingResolver.cs

@@ -252,6 +252,74 @@ public unsafe void AddActionMap(InputActionMap map)
                                 !action.m_BindingMask.Value.Matches(ref unresolvedBinding,
                                     InputBinding.MatchOptions.EmptyGroupMatchesAny));
 
+                        // If the binding isn't disabled, look up controls now. We do this first as we may still disable the
+                        // binding if it doesn't resolve to any controls or resolves only to controls already bound to by
+                        // other bindings.
+                        //
+                        // NOTE: We continuously add controls here to `resolvedControls`. Once we've completed our
+                        //       pass over the bindings in the map, `resolvedControls` will have all the controls for
+                        //       the current map.
+                        if (!bindingIsDisabled && !isComposite)
+                        {
+                            firstControlIndex = memory.controlCount + resolvedControls.Count;
+                            if (devicesForThisMap != null)
+                            {
+                                // Search in devices for only this map.
+                                var list = devicesForThisMap.Value;
+                                for (var i = 0; i < list.Count; ++i)
+                                {
+                                    var device = list[i];
+                                    if (!device.added)
+                                        continue; // Skip devices that have been removed.
+                                    numControls += InputControlPath.TryFindControls(device, path, 0, ref resolvedControls);
+                                }
+                            }
+                            else
+                            {
+                                // Search globally.
+                                numControls = InputSystem.FindControls(path, ref resolvedControls);
+                            }
+
+                            // Check for controls that are already bound to the action through other
+                            // bindings. The first binding that grabs a specific control gets to "own" it.
+                            if (numControls > 0)
+                            {
+                                for (var i = 0; i < n; ++i)
+                                {
+                                    ref var otherBindingState = ref bindingStatesPtr[bindingStartIndex + i];
+
+                                    // Skip if binding has no controls.
+                                    if (otherBindingState.controlCount == 0)
+                                        continue;
+
+                                    // Skip if binding isn't from same action.
+                                    if (otherBindingState.actionIndex != actionStartIndex + actionIndexInMap)
+                                        continue;
+
+                                    // Check for controls in the set that we just resolved that are also on the other
+                                    // binding. Each such control we find, we kick out of the list.
+                                    for (var k = 0; k < numControls; ++k)
+                                    {
+                                        var controlOnCurrentBinding = resolvedControls[firstControlIndex + k - memory.controlCount];
+                                        var controlIndexOnOtherBinding = resolvedControls.IndexOf(controlOnCurrentBinding,
+                                            otherBindingState.controlStartIndex - memory.controlCount, otherBindingState.controlCount);
+                                        if (controlIndexOnOtherBinding != -1)
+                                        {
+                                            // Control is bound to a previous binding. Remove it from the current binding.
+                                            resolvedControls.RemoveAt(firstControlIndex + k - memory.controlCount);
+                                            --numControls;
+                                            --k;
+                                        }
+                                    }
+                                }
+                            }
+
+                            // Disable binding if it doesn't resolve to any controls.
+                            // NOTE: This also happens to bindings that got all their resolved controls removed because other bindings from the same
+                            //       action already grabbed them.
+                            if (numControls == 0)
+                                bindingIsDisabled = true;
+                        }
 
                         // If the binding isn't disabled, resolve its controls, processors, and interactions.
                         if (!bindingIsDisabled)
@@ -326,30 +394,6 @@ public unsafe void AddActionMap(InputActionMap map)
                                     currentCompositeAction = null;
                                     currentCompositeActionIndexInMap = InputActionState.kInvalidIndex;
                                 }
-
-                                // Look up controls.
-                                //
-                                // NOTE: We continuously add controls here to `resolvedControls`. Once we've completed our
-                                //       pass over the bindings in the map, `resolvedControls` will have all the controls for
-                                //       the current map.
-                                firstControlIndex = memory.controlCount + resolvedControls.Count;
-                                if (devicesForThisMap != null)
-                                {
-                                    // Search in devices for only this map.
-                                    var list = devicesForThisMap.Value;
-                                    for (var i = 0; i < list.Count; ++i)
-                                    {
-                                        var device = list[i];
-                                        if (!device.added)
-                                            continue; // Skip devices that have been removed.
-                                        numControls += InputControlPath.TryFindControls(device, path, 0, ref resolvedControls);
-                                    }
-                                }
-                                else
-                                {
-                                    // Search globally.
-                                    numControls = InputSystem.FindControls(path, ref resolvedControls);
-                                }
                             }
                         }
 

Packages/com.unity.inputsystem/InputSystem/Controls/InputControlList.cs

@@ -348,15 +348,29 @@ public void CopyTo(TControl[] array, int arrayIndex)
 
         public int IndexOf(TControl item)
         {
+            return IndexOf(item, 0);
+        }
+
+        public int IndexOf(TControl item, int startIndex, int count = -1)
+        {
+            if (startIndex < 0)
+                throw new ArgumentOutOfRangeException(nameof(startIndex), "startIndex cannot be negative");
+
             if (m_Count == 0)
                 return -1;
 
+            if (count < 0)
+                count = Mathf.Max(m_Count - startIndex, 0);
+
+            if (startIndex + count > m_Count)
+                throw new ArgumentOutOfRangeException(nameof(count));
+
             var index = ToIndex(item);
             var indices = (ulong*)m_Indices.GetUnsafeReadOnlyPtr();
 
-            for (var i = 0; i < m_Count; ++i)
-                if (indices[i] == index)
-                    return i;
+            for (var i = 0; i < count; ++i)
+                if (indices[startIndex + i] == index)
+                    return startIndex + i;
 
             return -1;
         }
@@ -376,6 +390,11 @@ public bool Contains(TControl item)
             return IndexOf(item) != -1;
         }
 
+        public bool Contains(TControl item, int startIndex, int count = -1)
+        {
+            return IndexOf(item, startIndex, count) != -1;
+        }
+
         public void SwapElements(int index1, int index2)
         {
             if (index1 < 0 || index1 >= m_Count)

Packages/com.unity.inputsystem/InputSystem/Plugins/Users/InputUser.cs

@@ -5,6 +5,8 @@
 using UnityEngine.InputSystem.Utilities;
 using UnityEngine.Profiling;
 
+////REVIEW: remove users automatically when exiting play mode?
+
 ////REVIEW: do we need to handle the case where devices are added to a user that are each associated with a different user account
 
 ////REVIEW: how should we handle pairings of devices *not* called for by a control scheme? should that result in a failed match?

Packages/com.unity.inputsystem/InputSystem/Utilities/PrimitiveValue.cs

@@ -218,7 +218,7 @@ public PrimitiveValue ConvertTo(TypeCode type)
                 case TypeCode.Int64: return ToInt64();
                 case TypeCode.UInt16: return ToInt16();
                 case TypeCode.UInt32: return ToInt32();
-                case TypeCode.UInt64: return ToInt64();
+                case TypeCode.UInt64: return ToUInt64();
                 case TypeCode.Single: return ToSingle();
                 case TypeCode.Double: return ToDouble();
                 case TypeCode.Empty: return new PrimitiveValue();