NEW: Add APIs to change or erase existing bindings on actions.

Packages/com.unity.inputsystem/CHANGELOG.md

@@ -11,6 +11,22 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Added a `clickCount` control to the `Mouse` class, which specifies the click count for the last mouse click (to allow distinguishing between single-, double- and multi-clicks).
 - Support for Bluetooth Xbox One controllers on macOS.
 
+#### Actions
+
+- New API for changing bindings on actions
+```
+    // Several variations exist that allow to look up bindings in various ways.
+    myAction.ChangeBindingWithPath("<Gamepad>/buttonSouth")
+        .WithPath("<Keyboard>/space");
+
+    // Can also replace the binding wholesale.
+    myAction.ChangeBindingWithPath("<Keyboard>/space")
+        .To(new InputBinding { ... });
+
+    // Can also remove bindings programmatically now.
+    myAction.ChangeBindingWithPath("<Keyboard>/space").Erase();
+```
+
 ### Changed
 
 - `Joystick.axes` and `Joystick.buttons` have been removed.

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

@@ -600,6 +600,40 @@ internal void ThrowIfModifyingBindingsIsNotAllowed()
                     $"Cannot modify bindings on action '{this}' while its action map is enabled");
         }
 
+        internal int BindingIndexOnActionToBindingIndexOnMap(int indexOfBindingOnAction)
+        {
+            // We don't want to hit InputAction.bindings here as this requires setting up per-action
+            // binding info which we then nuke as part of the override process. Calling ApplyBindingOverride
+            // repeatedly with an index would thus cause the same data to be computed and thrown away
+            // over and over.
+            // Instead we manually search through the map's bindings to find the right binding index
+            // in the map.
+
+            var actionMap = GetOrCreateActionMap();
+            var bindingsInMap = actionMap.m_Bindings;
+            var bindingCountInMap = bindingsInMap.LengthSafe();
+            var actionName = name;
+
+            var currentBindingIndexOnAction = -1;
+            for (var i = 0; i < bindingCountInMap; ++i)
+            {
+                ref var binding = ref bindingsInMap[i];
+
+                // Match both name and ID on binding.
+                if (string.Compare(binding.action, actionName, StringComparison.InvariantCultureIgnoreCase) != 0 &&
+                    binding.action != m_Id)
+                    continue;
+
+                ++currentBindingIndexOnAction;
+                if (currentBindingIndexOnAction == indexOfBindingOnAction)
+                    return i;
+            }
+
+            throw new ArgumentOutOfRangeException(
+                $"Binding index {indexOfBindingOnAction} is out of range for action '{this}' with {currentBindingIndexOnAction + 1} bindings",
+                nameof(indexOfBindingOnAction));
+        }
+
         /// <summary>
         /// Information provided to action callbacks about what triggered an action.
         /// </summary>

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

@@ -493,9 +493,9 @@ IEnumerator IEnumerable.GetEnumerator()
         /// </remarks>
         internal ReadOnlyArray<InputBinding> GetBindingsForSingleAction(InputAction action)
         {
-            Debug.Assert(action != null);
-            Debug.Assert(action.m_ActionMap == this);
-            Debug.Assert(!action.isSingletonAction || m_SingletonAction == action);
+            Debug.Assert(action != null, "Action cannot be null");
+            Debug.Assert(action.m_ActionMap == this, "Action must be in action map");
+            Debug.Assert(!action.isSingletonAction || m_SingletonAction == action, "Action is not a singleton action");
 
             // See if we need to refresh.
             if (m_BindingsForEachAction == null)
@@ -886,6 +886,18 @@ internal void ResolveBindings()
             }
         }
 
+        internal int FindBinding(InputBinding match)
+        {
+            var numBindings = m_Bindings.LengthSafe();
+            for (var i = 0; i < numBindings; ++i)
+            {
+                ref var binding = ref m_Bindings[i];
+                if (match.Matches(ref binding))
+                    return i;
+            }
+            return -1;
+        }
+
         #region Serialization
 
         // Action maps are serialized in two different ways. For storage as imported assets in Unity's Library/ folder

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

@@ -39,12 +39,6 @@ public static void ApplyBindingOverride(this InputAction action, string newPath,
             ApplyBindingOverride(action, new InputBinding {overridePath = newPath, groups = group, path = path});
         }
 
-        // Apply the given override to the action.
-        //
-        // NOTE: Ignores the action name in the override.
-        // NOTE: Action must be disabled while applying overrides.
-        // NOTE: If there's already an override on the respective binding, replaces the override.
-
         /// <summary>
         ///
         /// </summary>
@@ -71,35 +65,9 @@ public static void ApplyBindingOverride(this InputAction action, int bindingInde
             if (action == null)
                 throw new ArgumentNullException(nameof(action));
 
-            // We don't want to hit InputAction.bindings here as this requires setting up per-action
-            // binding info which we then nuke as part of the override process. Calling ApplyBindingOverride
-            // repeatedly with an index would thus cause the same data to be computed and thrown away
-            // over and over.
-            // Instead we manually search through the map's bindings to find the right binding index
-            // in the map.
-
-            var actionMap = action.GetOrCreateActionMap();
-            var bindingsInMap = actionMap.m_Bindings;
-            var bindingCountInMap = bindingsInMap?.Length ?? 0;
-            var actionName = action.name;
-
-            var currentBindingIndexOnAction = -1;
-            for (var i = 0; i < bindingCountInMap; ++i)
-            {
-                if (string.Compare(bindingsInMap[i].action, actionName, StringComparison.InvariantCultureIgnoreCase) != 0)
-                    continue;
-
-                ++currentBindingIndexOnAction;
-                if (currentBindingIndexOnAction == bindingIndex)
-                {
-                    bindingOverride.action = actionName;
-                    ApplyBindingOverride(actionMap, i, bindingOverride);
-                    return;
-                }
-            }
-
-            throw new ArgumentOutOfRangeException(
-                $"Binding index {bindingIndex} is out of range for action '{action}' with {currentBindingIndexOnAction} bindings", "bindingIndex");
+            var indexOnMap = action.BindingIndexOnActionToBindingIndexOnMap(bindingIndex);
+            bindingOverride.action = action.name;
+            ApplyBindingOverride(action.GetOrCreateActionMap(), indexOnMap, bindingOverride);
         }
 
         public static void ApplyBindingOverride(this InputAction action, int bindingIndex, string path)

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

@@ -2,10 +2,6 @@
 using UnityEngine.Experimental.Input.Layouts;
 using UnityEngine.Experimental.Input.Utilities;
 
-////TODO: support for removing bindings
-
-////TODO: rename AddBinding to just AddBinding
-
 namespace UnityEngine.Experimental.Input
 {
     /// <summary>
@@ -130,10 +126,12 @@ public static BindingSyntax AddBinding(this InputAction action, InputBinding bin
                 throw new ArgumentException("Binding path cannot be null or empty", nameof(binding));
             action.ThrowIfModifyingBindingsIsNotAllowed();
 
+            ////REVIEW: should this reference actions by ID?
             Debug.Assert(action.m_Name != null || action.isSingletonAction);
-            binding.action = action.m_Name;
+            binding.action = action.name;
 
             var actionMap = action.GetOrCreateActionMap();
+            actionMap.ThrowIfModifyingBindingsIsNotAllowed();
             var bindingIndex = AddBindingInternal(actionMap, binding);
             return new BindingSyntax(actionMap, action, bindingIndex);
         }
@@ -142,7 +140,7 @@ public static BindingSyntax AddBinding(this InputAction action, InputBinding bin
             string interactions = null, string groups = null, string action = null)
         {
             if (string.IsNullOrEmpty(path))
-                throw new ArgumentException("Binding path cannot be null or empty", "path");
+                throw new ArgumentException("Binding path cannot be null or empty", nameof(path));
 
             return AddBinding(actionMap, new InputBinding
             {
@@ -196,6 +194,8 @@ public static CompositeSyntax AddCompositeBinding(this InputAction action, strin
                 throw new ArgumentException("Composite name cannot be null or empty", nameof(composite));
 
             var actionMap = action.GetOrCreateActionMap();
+            actionMap.ThrowIfModifyingBindingsIsNotAllowed();
+
             ////REVIEW: use 'name' instead of 'path' field here?
             var binding = new InputBinding {path = composite, interactions = interactions, isComposite = true, action = action.name};
             var bindingIndex = AddBindingInternal(actionMap, binding);
@@ -225,6 +225,49 @@ private static int AddBindingInternal(InputActionMap map, InputBinding binding)
             return bindingIndex;
         }
 
+        public static BindingSyntax ChangeBinding(this InputAction action, int index)
+        {
+            if (action == null)
+                throw new ArgumentNullException(nameof(action));
+
+            var indexOnMap = action.BindingIndexOnActionToBindingIndexOnMap(index);
+            return new BindingSyntax(action.GetOrCreateActionMap(), action, indexOnMap);
+        }
+
+        public static BindingSyntax ChangeBindingWithId(this InputAction action, string id)
+        {
+            return action.ChangeBinding(new InputBinding {m_Id = id});
+        }
+
+        public static BindingSyntax ChangeBindingWithId(this InputAction action, Guid id)
+        {
+            return action.ChangeBinding(new InputBinding {id = id});
+        }
+
+        public static BindingSyntax ChangeBindingWithGroup(this InputAction action, string group)
+        {
+            return action.ChangeBinding(new InputBinding {groups = group});
+        }
+
+        public static BindingSyntax ChangeBindingWithPath(this InputAction action, string path)
+        {
+            return action.ChangeBinding(new InputBinding {path = path});
+        }
+
+        public static BindingSyntax ChangeBinding(this InputAction action, InputBinding match)
+        {
+            if (action == null)
+                throw new ArgumentNullException(nameof(action));
+
+            var actionMap = action.GetOrCreateActionMap();
+            actionMap.ThrowIfModifyingBindingsIsNotAllowed();
+            var bindingIndex = actionMap.FindBinding(match);
+            if (bindingIndex == -1)
+                throw new ArgumentException($"Cannot find binding matching '{match}' in '{action}'", nameof(match));
+
+            return new BindingSyntax(actionMap, action, bindingIndex);
+        }
+
         ////TODO: update binding mask if necessary
         ////REVIEW: should we allow renaming singleton actions to empty/null names?
         /// <summary>
@@ -249,7 +292,7 @@ public static void Rename(this InputAction action, string newName)
 
             // Make sure name isn't already taken in map.
             var actionMap = action.actionMap;
-            if (actionMap != null && actionMap.TryGetAction(newName) != null)
+            if (actionMap?.TryGetAction(newName) != null)
                 throw new InvalidOperationException(
                     $"Cannot rename '{action}' to '{newName}' in map '{actionMap}' as the map already contains an action with that name");
 
@@ -326,7 +369,8 @@ internal BindingSyntax(InputActionMap map, InputAction action, int bindingIndex)
                 m_BindingIndex = bindingIndex;
             }
 
-            public BindingSyntax ChainedWith(string binding, string interactions = null, string group = null)
+            ////TODO: implement chained bindings and make public
+            internal BindingSyntax ChainedWith(string binding, string interactions = null, string group = null)
             {
                 throw new NotImplementedException();
                 /*
@@ -342,6 +386,20 @@ public BindingSyntax ChainedWith(string binding, string interactions = null, str
                 */
             }
 
+            public BindingSyntax WithName(string name)
+            {
+                m_ActionMap.m_Bindings[m_BindingIndex].name = name;
+                // No need to clear cached data.
+                return this;
+            }
+
+            public BindingSyntax WithPath(string path)
+            {
+                m_ActionMap.m_Bindings[m_BindingIndex].path = path;
+                // No need to clear cached data.
+                return this;
+            }
+
             public BindingSyntax WithGroup(string group)
             {
                 if (string.IsNullOrEmpty(group))
@@ -449,7 +507,43 @@ public BindingSyntax WithProcessor<TProcessor>()
                 return WithProcessor(processorName);
             }
 
-            public BindingSyntax WithChild(string binding, string interactions = null, string groups = null)
+            public BindingSyntax Triggering(InputAction action)
+            {
+                if (action == null)
+                    throw new ArgumentNullException(nameof(action));
+                if (action.isSingletonAction)
+                    throw new ArgumentException(
+                        $"Cannot change the action a binding triggers on singleton action '{action}'", nameof(action));
+                m_ActionMap.m_Bindings[m_BindingIndex].action = action.name;
+                m_ActionMap.ClearPerActionCachedBindingData();
+                return this;
+            }
+
+            public BindingSyntax To(InputBinding binding)
+            {
+                m_ActionMap.m_Bindings[m_BindingIndex] = binding;
+                m_ActionMap.ClearPerActionCachedBindingData();
+
+                // If it's a singleton action, we force the binding to stay with the action.
+                if (m_ActionMap.m_SingletonAction != null)
+                    m_ActionMap.m_Bindings[m_BindingIndex].action = m_Action.name;
+
+                return this;
+            }
+
+            public void Erase()
+            {
+                ArrayHelpers.EraseAt(ref m_ActionMap.m_Bindings, m_BindingIndex);
+                m_ActionMap.ClearPerActionCachedBindingData();
+
+                // We have switched to a different binding array. For singleton actions, we need to
+                // sync up the reference that the action itself has.
+                if (m_ActionMap.m_SingletonAction != null)
+                    m_ActionMap.m_SingletonAction.m_SingletonActionBindings = m_ActionMap.m_Bindings;
+            }
+
+            ////REVIEW: do we really want to go this direction?
+            internal BindingSyntax WithChild(string binding, string interactions = null, string groups = null)
             {
                 /*
                 var child = m_Action != null
@@ -462,15 +556,7 @@ public BindingSyntax WithChild(string binding, string interactions = null, strin
                 throw new NotImplementedException();
             }
 
-            public BindingSyntax Triggering(InputAction action)
-            {
-                if (action == null)
-                    throw new ArgumentNullException(nameof(action));
-                m_ActionMap.m_Bindings[m_BindingIndex].action = action.name;
-                return this;
-            }
-
-            public BindingSyntax And => throw new NotImplementedException();
+            internal BindingSyntax And => throw new NotImplementedException();
         }
 
         public struct CompositeSyntax

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

@@ -57,6 +57,11 @@ public Guid id
                     m_Guid = new Guid(m_Id);
                 return m_Guid;
             }
+            set
+            {
+                m_Guid = value;
+                m_Id = m_Guid.ToString();
+            }
         }
 
         /// <summary>
@@ -171,7 +176,8 @@ public string action
             set => m_Action = value;
         }
 
-        public bool chainWithPrevious
+        ////TODO: make public when chained bindings are implemented fully
+        internal bool chainWithPrevious
         {
             get => (m_Flags & Flags.ThisAndPreviousCombine) == Flags.ThisAndPreviousCombine;
             set
@@ -329,6 +335,7 @@ internal bool Matches(ref InputBinding other)
             {
                 ////TODO: handle "map/action" format
                 ////TODO: handle "map/*" format
+                ////REVIEW: this will not be able to handle cases where one binding references an action by ID and the other by name but both do mean the same action
                 if (other.action == null
                     || !StringHelpers.CharacterSeparatedListsHaveAtLeastOneCommonElement(action, other.action, kSeparator))
                     return false;
@@ -341,6 +348,12 @@ internal bool Matches(ref InputBinding other)
                     return false;
             }
 
+            if (!string.IsNullOrEmpty(m_Id))
+            {
+                if (other.id != id)
+                    return false;
+            }
+
             return true;
         }