FIX: JSON Binding overrides are set to empty string when null (#1644)

* (De)serialize null strings with string "null"

* Call BindingOverrideJson.ToBinding() and FromBinding()

* Add tests

* Update changelog

Author: jfreire-unity

Assets/Tests/InputSystem/CoreTests_Actions_Rebinding.cs

@@ -1346,6 +1346,63 @@ public void Actions_CanSaveAndLoadRebinds()
         Assert.That(secondActionInAsset.bindings[0].overrideProcessors, Is.EqualTo("invert"));
     }
 
+    [Test]
+    [Category("Actions")]
+    public void Actions_CanSaveAndLoadRebindOnlyForOverridesSet()
+    {
+        var action = new InputAction();
+        action.AddBinding("<Gamepad>/buttonSouth", processors: "invert");
+        action.AddBinding("<Keyboard>/space", interactions: "tap");
+
+        // Check that the effective paths are correct
+        Assert.That(action.bindings[0].effectivePath, Is.EqualTo("<Gamepad>/buttonSouth"));
+        Assert.That(action.bindings[0].effectiveInteractions, Is.EqualTo(null));
+        Assert.That(action.bindings[0].effectiveProcessors, Is.EqualTo("invert"));
+
+        Assert.That(action.bindings[1].effectivePath, Is.EqualTo("<Keyboard>/space"));
+        Assert.That(action.bindings[1].effectiveInteractions, Is.EqualTo("tap"));
+        Assert.That(action.bindings[1].effectiveProcessors, Is.EqualTo(null));
+
+        action.ApplyBindingOverride(0, "<Gamepad>/buttonWest");
+        action.ApplyBindingOverride(1, new InputBinding
+        {
+            overridePath =  "<Keyboard>/a",
+            overrideInteractions = "",
+            overrideProcessors = "multiply",
+        });
+
+        var actionRebindsJson = action.SaveBindingOverridesAsJson();
+        Debug.Log(actionRebindsJson.ToString());
+
+        action.LoadBindingOverridesFromJson(actionRebindsJson);
+
+        // Check that effective binding path changed
+        Assert.That(action.bindings[0].effectivePath, Is.EqualTo("<Gamepad>/buttonWest"));
+        Assert.That(action.bindings[0].overridePath, Is.EqualTo("<Gamepad>/buttonWest"));
+        Assert.That(action.bindings[0].path, Is.EqualTo("<Gamepad>/buttonSouth"));
+        // Check that effective interaction maintained it's null value
+        Assert.That(action.bindings[0].effectiveInteractions, Is.EqualTo(null));
+        Assert.That(action.bindings[0].overrideInteractions, Is.EqualTo(null));
+        Assert.That(action.bindings[0].interactions, Is.EqualTo(null));
+        // Check that effective interaction maintained it's previously set value
+        Assert.That(action.bindings[0].effectiveProcessors, Is.EqualTo("invert"));
+        Assert.That(action.bindings[0].overrideProcessors, Is.EqualTo(null));
+        Assert.That(action.bindings[0].processors, Is.EqualTo("invert"));
+
+        // Check that effective binding path changed
+        Assert.That(action.bindings[1].effectivePath, Is.EqualTo("<Keyboard>/a"));
+        Assert.That(action.bindings[1].overridePath, Is.EqualTo("<Keyboard>/a"));
+        Assert.That(action.bindings[1].path, Is.EqualTo("<Keyboard>/space"));
+        // Check that effective binding was disabled
+        Assert.That(action.bindings[1].effectiveInteractions, Is.EqualTo(""));
+        Assert.That(action.bindings[1].overrideInteractions, Is.EqualTo(""));
+        Assert.That(action.bindings[1].interactions, Is.EqualTo("tap"));
+        // Check that effective binding which was null before, now has changed
+        Assert.That(action.bindings[1].effectiveProcessors, Is.EqualTo("multiply"));
+        Assert.That(action.bindings[1].overrideProcessors, Is.EqualTo("multiply"));
+        Assert.That(action.bindings[1].processors, Is.EqualTo(null));
+    }
+
     [Test]
     [Category("Actions")]
     public void Actions_ActionMapFindBinding_ShouldReturnNegativeOne_IfEmpty()

Packages/com.unity.inputsystem/CHANGELOG.md

@@ -15,6 +15,7 @@ however, it has to be formatted properly to pass verification tests.
 - Fixed InputAction.bindings.count not getting correctly updated after removing bindings with Erase().
 - Fixed an issue where connecting a gamepad in the editor with certain settings will cause memory and performance to degrade ([case UUM-19480](https://issuetracker.unity3d.com/product/unity/issues/guid/UUM-19480)).
 - Fixed issue leading to a stack overflow crash during device initialization in `InsertControlBitRangeNode` (case ISXB-405).
+- Fixed the issue where saving and loading override bindings to JSON would set unassigned overrides (that were `null`) to assigned overrides (as an empty string `""`).
 
 ## [1.5.0] - 2023-01-24
 

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

@@ -1463,15 +1463,25 @@ internal struct BindingOverrideJson
             public string interactions;
             public string processors;
 
-            public static BindingOverrideJson FromBinding(InputBinding binding)
+            public static BindingOverrideJson FromBinding(InputBinding binding, InputAction bindingAction = null)
             {
                 return new BindingOverrideJson
                 {
-                    action = binding.action,
-                    id = binding.m_Id,
-                    path = binding.overridePath,
-                    interactions = binding.overrideInteractions,
-                    processors = binding.overrideProcessors,
+                    action = bindingAction != null && !bindingAction.isSingletonAction ? $"{bindingAction.actionMap.name}/{bindingAction.name}" : "",
+                    id = binding.id.ToString() ,
+                    path = binding.overridePath ?? "null",
+                    interactions = binding.overrideInteractions ?? "null",
+                    processors = binding.overrideProcessors ?? "null"
+                };
+            }
+
+            public static InputBinding ToBinding(BindingOverrideJson bindingOverride)
+            {
+                return new InputBinding
+                {
+                    overridePath = bindingOverride.path != "null" ? bindingOverride.path : null,
+                    overrideInteractions = bindingOverride.interactions != "null" ? bindingOverride.interactions : null,
+                    overrideProcessors = bindingOverride.processors != "null" ? bindingOverride.processors : null,
                 };
             }
         }

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

@@ -891,7 +891,6 @@ public static void ApplyBindingOverrides(this InputActionMap actionMap, IEnumera
             if (overrides == null)
                 throw new ArgumentNullException(nameof(overrides));
 
-
             foreach (var binding in overrides)
                 ApplyBindingOverride(actionMap, binding);
         }
@@ -903,7 +902,6 @@ public static void RemoveBindingOverrides(this InputActionMap actionMap, IEnumer
             if (overrides == null)
                 throw new ArgumentNullException(nameof(overrides));
 
-
             foreach (var binding in overrides)
                 RemoveBindingOverride(actionMap, binding);
         }
@@ -1145,14 +1143,7 @@ private static void AddBindingOverrideJsonTo(this IInputActionCollection2 action
             if (action == null)
                 action = actions.FindAction(binding.action);
 
-            var @override = new InputActionMap.BindingOverrideJson
-            {
-                action = action != null && !action.isSingletonAction ? $"{action.actionMap.name}/{action.name}" : null,
-                id = binding.id.ToString(),
-                path = binding.overridePath,
-                interactions = binding.overrideInteractions,
-                processors = binding.overrideProcessors
-            };
+            var @override = InputActionMap.BindingOverrideJson.FromBinding(binding, action);
 
             list.Add(@override);
         }
@@ -1262,16 +1253,10 @@ private static void LoadBindingOverridesFromJsonInternal(this IInputActionCollec
                     var bindingIndex = actions.FindBinding(new InputBinding { m_Id = entry.id }, out var action);
                     if (bindingIndex != -1)
                     {
-                        action.ApplyBindingOverride(bindingIndex, new InputBinding
-                        {
-                            overridePath = entry.path,
-                            overrideInteractions = entry.interactions,
-                            overrideProcessors = entry.processors,
-                        });
+                        action.ApplyBindingOverride(bindingIndex, InputActionMap.BindingOverrideJson.ToBinding(entry));
                         continue;
                     }
                 }
-
                 Debug.LogWarning("Could not override binding as no existing binding was found with the id: " + entry.id);
             }
         }