NEW: Extended action code generator to provide support for adding and removing multiple callback instances. (#1593)

* NEW: Extended action code generator to provide support for adding and removing multiple callback instances via generated code.

* Aligned code generator with auto-formatter

* Adoptions to action code generator to align with formatter

* Updated generated files not previously covered.

* Temporarily ignored new tests to bisect

* temp modification

* Reenabled functional test

* Reenabled ignored test and modified path

* Updated changelog to match version

* remove generate c# flag on test asset

Co-authored-by: James McGill <[email protected]>

Author: ekcoh

Assets/Samples/InGameHints/InGameHintsActions.cs

@@ -330,7 +330,7 @@ public int FindBinding(InputBinding bindingMask, out InputAction action)
 
         // Gameplay
         private readonly InputActionMap m_Gameplay;
-        private IGameplayActions m_GameplayActionsCallbackInterface;
+        private List<IGameplayActions> m_GameplayActionsCallbackInterfaces = new List<IGameplayActions>();
         private readonly InputAction m_Gameplay_Move;
         private readonly InputAction m_Gameplay_Look;
         private readonly InputAction m_Gameplay_PickUp;
@@ -350,45 +350,58 @@ public struct GameplayActions
             public void Disable() { Get().Disable(); }
             public bool enabled => Get().enabled;
             public static implicit operator InputActionMap(GameplayActions set) { return set.Get(); }
+            public void AddCallbacks(IGameplayActions instance)
+            {
+                if (instance == null || m_Wrapper.m_GameplayActionsCallbackInterfaces.Contains(instance)) return;
+                m_Wrapper.m_GameplayActionsCallbackInterfaces.Add(instance);
+                @Move.started += instance.OnMove;
+                @Move.performed += instance.OnMove;
+                @Move.canceled += instance.OnMove;
+                @Look.started += instance.OnLook;
+                @Look.performed += instance.OnLook;
+                @Look.canceled += instance.OnLook;
+                @PickUp.started += instance.OnPickUp;
+                @PickUp.performed += instance.OnPickUp;
+                @PickUp.canceled += instance.OnPickUp;
+                @Drop.started += instance.OnDrop;
+                @Drop.performed += instance.OnDrop;
+                @Drop.canceled += instance.OnDrop;
+                @Throw.started += instance.OnThrow;
+                @Throw.performed += instance.OnThrow;
+                @Throw.canceled += instance.OnThrow;
+            }
+
+            private void UnregisterCallbacks(IGameplayActions instance)
+            {
+                @Move.started -= instance.OnMove;
+                @Move.performed -= instance.OnMove;
+                @Move.canceled -= instance.OnMove;
+                @Look.started -= instance.OnLook;
+                @Look.performed -= instance.OnLook;
+                @Look.canceled -= instance.OnLook;
+                @PickUp.started -= instance.OnPickUp;
+                @PickUp.performed -= instance.OnPickUp;
+                @PickUp.canceled -= instance.OnPickUp;
+                @Drop.started -= instance.OnDrop;
+                @Drop.performed -= instance.OnDrop;
+                @Drop.canceled -= instance.OnDrop;
+                @Throw.started -= instance.OnThrow;
+                @Throw.performed -= instance.OnThrow;
+                @Throw.canceled -= instance.OnThrow;
+            }
+
+            public void RemoveCallbacks(IGameplayActions instance)
+            {
+                if (m_Wrapper.m_GameplayActionsCallbackInterfaces.Remove(instance))
+                    UnregisterCallbacks(instance);
+            }
+
             public void SetCallbacks(IGameplayActions instance)
             {
-                if (m_Wrapper.m_GameplayActionsCallbackInterface != null)
-                {
-                    @Move.started -= m_Wrapper.m_GameplayActionsCallbackInterface.OnMove;
-                    @Move.performed -= m_Wrapper.m_GameplayActionsCallbackInterface.OnMove;
-                    @Move.canceled -= m_Wrapper.m_GameplayActionsCallbackInterface.OnMove;
-                    @Look.started -= m_Wrapper.m_GameplayActionsCallbackInterface.OnLook;
-                    @Look.performed -= m_Wrapper.m_GameplayActionsCallbackInterface.OnLook;
-                    @Look.canceled -= m_Wrapper.m_GameplayActionsCallbackInterface.OnLook;
-                    @PickUp.started -= m_Wrapper.m_GameplayActionsCallbackInterface.OnPickUp;
-                    @PickUp.performed -= m_Wrapper.m_GameplayActionsCallbackInterface.OnPickUp;
-                    @PickUp.canceled -= m_Wrapper.m_GameplayActionsCallbackInterface.OnPickUp;
-                    @Drop.started -= m_Wrapper.m_GameplayActionsCallbackInterface.OnDrop;
-                    @Drop.performed -= m_Wrapper.m_GameplayActionsCallbackInterface.OnDrop;
-                    @Drop.canceled -= m_Wrapper.m_GameplayActionsCallbackInterface.OnDrop;
-                    @Throw.started -= m_Wrapper.m_GameplayActionsCallbackInterface.OnThrow;
-                    @Throw.performed -= m_Wrapper.m_GameplayActionsCallbackInterface.OnThrow;
-                    @Throw.canceled -= m_Wrapper.m_GameplayActionsCallbackInterface.OnThrow;
-                }
-                m_Wrapper.m_GameplayActionsCallbackInterface = instance;
-                if (instance != null)
-                {
-                    @Move.started += instance.OnMove;
-                    @Move.performed += instance.OnMove;
-                    @Move.canceled += instance.OnMove;
-                    @Look.started += instance.OnLook;
-                    @Look.performed += instance.OnLook;
-                    @Look.canceled += instance.OnLook;
-                    @PickUp.started += instance.OnPickUp;
-                    @PickUp.performed += instance.OnPickUp;
-                    @PickUp.canceled += instance.OnPickUp;
-                    @Drop.started += instance.OnDrop;
-                    @Drop.performed += instance.OnDrop;
-                    @Drop.canceled += instance.OnDrop;
-                    @Throw.started += instance.OnThrow;
-                    @Throw.performed += instance.OnThrow;
-                    @Throw.canceled += instance.OnThrow;
-                }
+                foreach (var item in m_Wrapper.m_GameplayActionsCallbackInterfaces)
+                    UnregisterCallbacks(item);
+                m_Wrapper.m_GameplayActionsCallbackInterfaces.Clear();
+                AddCallbacks(instance);
             }
         }
         public GameplayActions @Gameplay => new GameplayActions(this);

Assets/Samples/SimpleDemo/SimpleControls.cs

@@ -225,7 +225,7 @@ public int FindBinding(InputBinding bindingMask, out InputAction action)
 
     // gameplay
     private readonly InputActionMap m_gameplay;
-    private IGameplayActions m_GameplayActionsCallbackInterface;
+    private List<IGameplayActions> m_GameplayActionsCallbackInterfaces = new List<IGameplayActions>();
     private readonly InputAction m_gameplay_fire;
     private readonly InputAction m_gameplay_move;
     private readonly InputAction m_gameplay_look;
@@ -241,33 +241,46 @@ public struct GameplayActions
         public void Disable() { Get().Disable(); }
         public bool enabled => Get().enabled;
         public static implicit operator InputActionMap(GameplayActions set) { return set.Get(); }
+        public void AddCallbacks(IGameplayActions instance)
+        {
+            if (instance == null || m_Wrapper.m_GameplayActionsCallbackInterfaces.Contains(instance)) return;
+            m_Wrapper.m_GameplayActionsCallbackInterfaces.Add(instance);
+            @fire.started += instance.OnFire;
+            @fire.performed += instance.OnFire;
+            @fire.canceled += instance.OnFire;
+            @move.started += instance.OnMove;
+            @move.performed += instance.OnMove;
+            @move.canceled += instance.OnMove;
+            @look.started += instance.OnLook;
+            @look.performed += instance.OnLook;
+            @look.canceled += instance.OnLook;
+        }
+
+        private void UnregisterCallbacks(IGameplayActions instance)
+        {
+            @fire.started -= instance.OnFire;
+            @fire.performed -= instance.OnFire;
+            @fire.canceled -= instance.OnFire;
+            @move.started -= instance.OnMove;
+            @move.performed -= instance.OnMove;
+            @move.canceled -= instance.OnMove;
+            @look.started -= instance.OnLook;
+            @look.performed -= instance.OnLook;
+            @look.canceled -= instance.OnLook;
+        }
+
+        public void RemoveCallbacks(IGameplayActions instance)
+        {
+            if (m_Wrapper.m_GameplayActionsCallbackInterfaces.Remove(instance))
+                UnregisterCallbacks(instance);
+        }
+
         public void SetCallbacks(IGameplayActions instance)
         {
-            if (m_Wrapper.m_GameplayActionsCallbackInterface != null)
-            {
-                @fire.started -= m_Wrapper.m_GameplayActionsCallbackInterface.OnFire;
-                @fire.performed -= m_Wrapper.m_GameplayActionsCallbackInterface.OnFire;
-                @fire.canceled -= m_Wrapper.m_GameplayActionsCallbackInterface.OnFire;
-                @move.started -= m_Wrapper.m_GameplayActionsCallbackInterface.OnMove;
-                @move.performed -= m_Wrapper.m_GameplayActionsCallbackInterface.OnMove;
-                @move.canceled -= m_Wrapper.m_GameplayActionsCallbackInterface.OnMove;
-                @look.started -= m_Wrapper.m_GameplayActionsCallbackInterface.OnLook;
-                @look.performed -= m_Wrapper.m_GameplayActionsCallbackInterface.OnLook;
-                @look.canceled -= m_Wrapper.m_GameplayActionsCallbackInterface.OnLook;
-            }
-            m_Wrapper.m_GameplayActionsCallbackInterface = instance;
-            if (instance != null)
-            {
-                @fire.started += instance.OnFire;
-                @fire.performed += instance.OnFire;
-                @fire.canceled += instance.OnFire;
-                @move.started += instance.OnMove;
-                @move.performed += instance.OnMove;
-                @move.canceled += instance.OnMove;
-                @look.started += instance.OnLook;
-                @look.performed += instance.OnLook;
-                @look.canceled += instance.OnLook;
-            }
+            foreach (var item in m_Wrapper.m_GameplayActionsCallbackInterfaces)
+                UnregisterCallbacks(item);
+            m_Wrapper.m_GameplayActionsCallbackInterfaces.Clear();
+            AddCallbacks(instance);
         }
     }
     public GameplayActions @gameplay => new GameplayActions(this);

Assets/Tests/InputSystem/CoreTests_Editor.cs

@@ -2943,6 +2943,117 @@ private void AssertAssetIsUnmodifiedAfterExitingPlayMode(Action<InputActionAsset
         Assert.That(actualAsset.ToJson(), Is.EqualTo(originalJson), message);
     }
 
+    [Test]
+    public void InputActionCodeGenerator_ShouldGenerateValidCSharpCode()
+    {
+        // Note that this only tests pre-generated code contents with respect to the code generator.
+        // The intent of this test is to capture changes to the generated source that would not be detected since code currently isn't automatically regenerated.
+        // Hence, one need to regenerate the source file below if code generator is updated to produce different output. This is not ideal and could be improved if dynamic compilation is used.
+
+        var directory = "Assets/Tests/InputSystem";
+        var csFilePath = $"{directory}/InputActionCodeGeneratorActions.cs";
+        var assetPath = $"{directory}/InputActionCodeGeneratorActions.inputactions";
+        var csFileContents = File.ReadAllText(csFilePath);
+        var asset = AssetDatabase.LoadAssetAtPath<InputActionAsset>(assetPath);
+
+        var generatedCode = InputActionCodeGenerator.GenerateWrapperCode(asset);
+
+        Assert.That(generatedCode, Is.EqualTo(csFileContents), $"Unexpected content, likely code generator changed. Regenerate source from {assetPath}.");
+    }
+
+    private sealed class InputActionCodeGeneratorActionsStub : InputActionCodeGeneratorActions.IGameplayActions
+    {
+        public int m_Action1Count = 0;
+        public int m_Action2Count = 0;
+
+        public void OnAction1(InputAction.CallbackContext context)
+        {
+            if (context.performed)
+                ++m_Action1Count;
+        }
+
+        public void OnAction2(InputAction.CallbackContext context)
+        {
+            if (context.performed)
+                ++m_Action2Count;
+        }
+    }
+
+    [Test]
+    public void InputActionCodeGenerator_ShouldGenerateClassWithSupportForRegisteringAndUnregisteringActions()
+    {
+        // Note that this is only testing pre-generated code. See test above for consistency check on file contents of generated source code.
+
+        var instance1 = new InputActionCodeGeneratorActionsStub();
+        var instance2 = new InputActionCodeGeneratorActionsStub();
+        var actions = new InputActionCodeGeneratorActions();
+
+        // Register using SetCallbacks
+        actions.gameplay.SetCallbacks(instance1);
+        actions.Enable();
+
+        var keyboard = InputSystem.AddDevice<Keyboard>();
+        PressAndRelease(keyboard.spaceKey);
+
+        Assert.That(instance1.m_Action1Count, Is.EqualTo(1));
+        Assert.That(instance1.m_Action2Count, Is.EqualTo(0));
+
+        // Unregister using SetCallbacks(null)
+
+        actions.gameplay.SetCallbacks(null);
+        PressAndRelease(keyboard.enterKey);
+
+        Assert.That(instance1.m_Action1Count, Is.EqualTo(1));
+        Assert.That(instance1.m_Action2Count, Is.EqualTo(0));
+
+        // Add using AddCallbacks
+        actions.gameplay.AddCallbacks(instance1);
+        PressAndRelease(keyboard.enterKey);
+
+        Assert.That(instance1.m_Action1Count, Is.EqualTo(1));
+        Assert.That(instance1.m_Action2Count, Is.EqualTo(1));
+
+        // Add duplicate using AddCallbacks (Expecting duplicate to be ignored)
+        actions.gameplay.AddCallbacks(instance1);
+        PressAndRelease(keyboard.enterKey);
+
+        Assert.That(instance1.m_Action1Count, Is.EqualTo(1));
+        Assert.That(instance1.m_Action2Count, Is.EqualTo(2));
+
+        // Remove previously registered instance
+        actions.gameplay.RemoveCallbacks(instance1);
+        PressAndRelease(keyboard.spaceKey);
+
+        Assert.That(instance1.m_Action1Count, Is.EqualTo(1));
+        Assert.That(instance1.m_Action2Count, Is.EqualTo(2));
+
+        // Attempt to remove non-existent instance
+        actions.gameplay.RemoveCallbacks(null);
+        actions.gameplay.RemoveCallbacks(instance2);
+
+        // Add multiple instances and remove single
+        actions.gameplay.AddCallbacks(instance1);
+        actions.gameplay.AddCallbacks(instance2);
+
+        actions.gameplay.RemoveCallbacks(instance1);
+        PressAndRelease(keyboard.spaceKey);
+
+        Assert.That(instance1.m_Action1Count, Is.EqualTo(1));
+        Assert.That(instance1.m_Action2Count, Is.EqualTo(2));
+        Assert.That(instance2.m_Action1Count, Is.EqualTo(1));
+        Assert.That(instance2.m_Action2Count, Is.EqualTo(0));
+
+        // Multiple callbacks
+        actions.gameplay.AddCallbacks(instance1);
+
+        PressAndRelease(keyboard.spaceKey);
+
+        Assert.That(instance1.m_Action1Count, Is.EqualTo(2));
+        Assert.That(instance1.m_Action2Count, Is.EqualTo(2));
+        Assert.That(instance2.m_Action1Count, Is.EqualTo(2));
+        Assert.That(instance2.m_Action2Count, Is.EqualTo(0));
+    }
+
 #if UNITY_STANDALONE // CodeDom API not available in most players.
 #if !NET_STANDARD_2_0 // Not possible to run when using .NET standard at the moment.
     [Test]