FIX: InvokeCSharpEvents on PlayerInput not triggering onActionTriggered (#904, #854).

Author: Rene Damm

Assets/Tests/InputSystem/Plugins/PlayerInputTests.cs

@@ -807,6 +807,64 @@ public void PlayerInput_PlayerIndex_DoesNotChange()
         Assert.That(playerInput2.playerIndex, Is.EqualTo(1));
     }
 
+    [Test]
+    [Category("PlayerInput")]
+    [TestCase(PlayerNotifications.SendMessages, typeof(MessageListener))]
+    [TestCase(PlayerNotifications.BroadcastMessages, typeof(MessageListener))]
+    [TestCase(PlayerNotifications.InvokeUnityEvents, typeof(PlayerInputEventListener), true)]
+    [TestCase(PlayerNotifications.InvokeCSharpEvents, typeof(PlayerInputCSharpEventListener), true)]
+    public void PlayerInput_CanReceiveNotificationWhenActionIsTriggered(PlayerNotifications notificationBehavior, Type listenerType, bool receivesAllPhases = false)
+    {
+        var gamepad = InputSystem.AddDevice<Gamepad>();
+
+        var go = new GameObject();
+        go.SetActive(false);
+        IListener listener;
+        if (notificationBehavior == PlayerNotifications.BroadcastMessages)
+        {
+            var child = new GameObject();
+            child.transform.parent = go.transform;
+            listener = (IListener)child.AddComponent(listenerType);
+        }
+        else
+        {
+            listener = (IListener)go.AddComponent(listenerType);
+        }
+        var playerInput = go.AddComponent<PlayerInput>();
+
+        playerInput.notificationBehavior = notificationBehavior;
+        playerInput.defaultActionMap = "gameplay";
+        playerInput.actions = InputActionAsset.FromJson(kActions);
+
+        go.SetActive(true);
+
+        Press(gamepad.buttonSouth);
+
+        if (receivesAllPhases)
+        {
+            Assert.That(listener.messages, Is.EquivalentTo(new[] { new Message("Fire Started", 1f), new Message("Fire Performed", 1f) }));
+        }
+        else
+        {
+            Assert.That(listener.messages, Is.EquivalentTo(new[] {new Message("OnFire", 1f)}));
+        }
+
+        listener.messages.Clear();
+
+        Release(gamepad.buttonSouth);
+
+        if (receivesAllPhases)
+        {
+            Assert.That(listener.messages, Is.EquivalentTo(new[] {new Message("Fire Canceled", 0f)}));
+        }
+        else
+        {
+            // 'Fire' is a button action. Unlike with value actions, PlayerInput should not
+            // send a message on button release (i.e. when the action cancels).
+            Assert.That(listener.messages, Is.Empty);
+        }
+    }
+
     [Test]
     [Category("PlayerInput")]
     public void PlayerInput_CanReceiveMessageWhenActionIsTriggered()
@@ -836,7 +894,7 @@ public void PlayerInput_CanReceiveMessageWhenActionIsTriggered()
 
     [Test]
     [Category("PlayerInput")]
-    public void PlayerInput_CanReceiveMessageWhenContinuousActionIsCanceled()
+    public void PlayerInput_CanReceiveMessageWhenValueActionIsCanceled()
     {
         var gamepad = InputSystem.AddDevice<Gamepad>();
 
@@ -867,26 +925,6 @@ public void PlayerInput_CanReceiveMessageWhenContinuousActionIsCanceled()
             }));
     }
 
-    [Test]
-    [Category("PlayerInput")]
-    public void PlayerInput_CanReceiveEventWhenActionIsTriggered()
-    {
-        var gamepad = InputSystem.AddDevice<Gamepad>();
-
-        var go = new GameObject();
-        var listener = go.AddComponent<MessageListener>();
-        var playerInput = go.AddComponent<PlayerInput>();
-        playerInput.notificationBehavior = PlayerNotifications.InvokeUnityEvents;
-        playerInput.defaultActionMap = "gameplay";
-        playerInput.actions = InputActionAsset.FromJson(kActions);
-        listener.SetUpEvents(playerInput);
-
-        Press(gamepad.buttonSouth);
-
-        Assert.That(listener.messages,
-            Is.EquivalentTo(new[] {new Message("gameplay/fire Started", 1f), new Message("gameplay/fire Performed", 1f)}));
-    }
-
     [Test]
     [Category("PlayerInput")]
     [TestCase(PlayerNotifications.SendMessages, typeof(MessageListener))]
@@ -1411,9 +1449,9 @@ public void TODO_PlayerInput_TriggeringAction_DoesNotAllocate()
                 {
                     ""name"" : ""gameplay"",
                     ""actions"" : [
-                        { ""name"" : ""fire"", ""type"" : ""button"" },
-                        { ""name"" : ""look"", ""type"" : ""value"" },
-                        { ""name"" : ""move"", ""type"" : ""value"" }
+                        { ""name"" : ""Fire"", ""type"" : ""button"" },
+                        { ""name"" : ""Look"", ""type"" : ""value"" },
+                        { ""name"" : ""Move"", ""type"" : ""value"" }
                     ],
                     ""bindings"" : [
                         { ""path"" : ""<Gamepad>/buttonSouth"", ""action"" : ""fire"", ""groups"" : ""Gamepad"" },
@@ -1516,43 +1554,6 @@ private class MessageListener : MonoBehaviour, IListener
     {
         public List<Message> messages { get; } = new List<Message>();
 
-        public void SetUpEvents(PlayerInput player)
-        {
-            var fireAction = player.actions.FindAction("gameplay/fire");
-            var lookAction = player.actions.FindAction("gameplay/look");
-            var moveAction = player.actions.FindAction("gameplay/move");
-
-            var fireEvent = new PlayerInput.ActionEvent(fireAction);
-            var lookEvent = new PlayerInput.ActionEvent(lookAction);
-            var moveEvent = new PlayerInput.ActionEvent(moveAction);
-
-            fireEvent.AddListener(OnFireEvent);
-            lookEvent.AddListener(OnLookEvent);
-            moveEvent.AddListener(OnMoveEvent);
-
-            player.actionEvents = new[]
-            {
-                fireEvent,
-                lookEvent,
-                moveEvent,
-            };
-        }
-
-        private void OnFireEvent(InputAction.CallbackContext context)
-        {
-            messages.Add(new Message { name = "gameplay/fire " + context.phase, value = context.ReadValue<float>() });
-        }
-
-        private void OnLookEvent(InputAction.CallbackContext context)
-        {
-            messages.Add(new Message { name = "gameplay/look " + context.phase, value = context.ReadValue<Vector2>() });
-        }
-
-        private void OnMoveEvent(InputAction.CallbackContext context)
-        {
-            messages.Add(new Message { name = "gameplay/move" + context.phase, value = context.ReadValue<Vector2>() });
-        }
-
         // ReSharper disable once UnusedMember.Local
         public void OnFire(InputValue value)
         {
@@ -1614,16 +1615,50 @@ public void OnEnable()
             var playerInput = GetComponent<PlayerInput>();
             Debug.Assert(playerInput != null, "Must have PlayerInput component");
 
-            foreach (var item in playerInput.actionEvents)
-                item.AddListener(OnAction);
+            SetUpActionEvents(playerInput);
 
             playerInput.deviceLostEvent.AddListener(OnDeviceLost);
             playerInput.deviceRegainedEvent.AddListener(OnDeviceRegained);
         }
 
-        private void OnAction(InputAction.CallbackContext context)
+        private void SetUpActionEvents(PlayerInput player)
         {
-            messages.Add(new Message(context.action.ToString()));
+            var fireAction = player.actions.FindAction("gameplay/fire");
+            var lookAction = player.actions.FindAction("gameplay/look");
+            var moveAction = player.actions.FindAction("gameplay/move");
+
+            var fireEvent = new PlayerInput.ActionEvent(fireAction);
+            var lookEvent = new PlayerInput.ActionEvent(lookAction);
+            var moveEvent = new PlayerInput.ActionEvent(moveAction);
+
+            fireEvent.AddListener(OnFireEvent);
+            lookEvent.AddListener(OnLookEvent);
+            moveEvent.AddListener(OnMoveEvent);
+
+            player.actionEvents = new[]
+            {
+                fireEvent,
+                lookEvent,
+                moveEvent,
+            };
+        }
+
+        // We have separate methods for these rather than one that we reuse for each listener in order to
+        // guarantee that PlayerInput is indeed calling the right method.
+
+        private void OnFireEvent(InputAction.CallbackContext context)
+        {
+            messages.Add(new Message($"Fire {context.phase}", context.ReadValueAsObject()));
+        }
+
+        private void OnLookEvent(InputAction.CallbackContext context)
+        {
+            messages.Add(new Message($"Look {context.phase}", context.ReadValueAsObject()));
+        }
+
+        private void OnMoveEvent(InputAction.CallbackContext context)
+        {
+            messages.Add(new Message($"Move {context.phase}", context.ReadValueAsObject()));
         }
 
         private void OnDeviceLost(PlayerInput player)
@@ -1648,15 +1683,14 @@ public void OnEnable()
             var playerInput = GetComponent<PlayerInput>();
             Debug.Assert(playerInput != null, "Must have PlayerInput component");
 
-
             playerInput.onActionTriggered += OnAction;
             playerInput.onDeviceLost += OnDeviceLost;
             playerInput.onDeviceRegained += OnDeviceRegained;
         }
 
         private void OnAction(InputAction.CallbackContext context)
         {
-            messages.Add(new Message(context.action.ToString()));
+            messages.Add(new Message($"{context.action.name} {context.phase}", context.ReadValueAsObject()));
         }
 
         private void OnDeviceLost(PlayerInput player)

Packages/com.unity.inputsystem/CHANGELOG.md

@@ -23,6 +23,7 @@ however, it has to be formatted properly to pass verification tests.
 - Fixed `anyKey` not appearing in control picker for `Keyboard`.
 - The text on the "Listen" button is no longer clipped off on 2019.3.
 - Controls bound to actions through composites no longer show up as duplicates in the input debugger.
+- Fixed `Invoke CSharp Events` when selected in `PlayerInput` not triggering `PlayerInput.onActionTriggered`.
 
 ### Changed
 

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

@@ -325,6 +325,7 @@ public InputAction this[string actionNameOrId]
             }
         }
 
+        ////REVIEW: inconsistent naming; elsewhere we use "onActionTriggered" (which in turn is inconsistent with InputAction.started etc)
         /// <summary>
         /// Add or remove a callback that is triggered when an action in the map changes its <see cref="InputActionPhase">
         /// phase</see>.

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

@@ -1,6 +1,8 @@
 using UnityEngine.InputSystem.Layouts;
 using UnityEngine.InputSystem.LowLevel;
 
+////REVIEW: generalize this to AnyButton and add to more devices?
+
 namespace UnityEngine.InputSystem.Controls
 {
     /// <summary>