FIX: Messages from PlayerInput not received when actions contain spaces (case 1214519, #1101).

Author: Rene Damm

Assets/Tests/InputSystem/Plugins/PlayerInputTests.cs

@@ -12,8 +12,11 @@
 using UnityEngine.InputSystem.Users;
 using UnityEngine.InputSystem.Processors;
 using UnityEngine.InputSystem.XInput;
+using UnityEngine.Profiling;
+using UnityEngine.TestTools.Constraints;
 using Object = UnityEngine.Object;
 using Gyroscope = UnityEngine.InputSystem.Gyroscope;
+using Is = UnityEngine.TestTools.Constraints.Is;
 
 /// <summary>
 /// Tests for <see cref="PlayerInput"/> and <see cref="PlayerInputManager"/>.
@@ -826,6 +829,25 @@ public void PlayerInput_ByDefaultChoosesMostSpecificControlSchemeAvailable()
         Assert.That(playerInput.currentControlScheme, Is.EqualTo("PS4Gamepad"));
     }
 
+    // https://fogbugz.unity3d.com/f/cases/1214519/
+    [Test]
+    [Category("PlayerInput")]
+    public void PlayerInput_CanHaveSpacesAndSpecialCharactersInActionNames()
+    {
+        InputSystem.AddDevice<Gamepad>();
+
+        var go = new GameObject();
+        var playerInput = go.AddComponent<PlayerInput>();
+        playerInput.defaultActionMap = "gameplay";
+        playerInput.notificationBehavior = PlayerNotifications.SendMessages;
+        playerInput.actions = InputActionAsset.FromJson(kActions);
+        var listener = go.AddComponent<MessageListener>();
+
+        Press((ButtonControl)playerInput.actions["Action With Spaces!!"].controls[0]);
+
+        Assert.That(listener.messages, Has.Exactly(1).With.Property("name").EqualTo("OnActionWithSpaces"));
+    }
+
     // Test setup where two players both use the keyboard but with two different control
     // schemes.
     [Test]
@@ -1133,6 +1155,42 @@ public void PlayerInput_CanReceiveMessageWhenValueActionIsCanceled()
             }));
     }
 
+    [Test]
+    [Category("PlayerInput")]
+    [TestCase(PlayerNotifications.SendMessages, typeof(MessageListener))]
+    [TestCase(PlayerNotifications.BroadcastMessages, typeof(MessageListener))]
+    [TestCase(PlayerNotifications.InvokeUnityEvents, typeof(PlayerInputEventListener))]
+    [TestCase(PlayerNotifications.InvokeCSharpEvents, typeof(PlayerInputCSharpEventListener))]
+    [Retry(2)] // Warm up JIT.
+    public void PlayerInput_TriggeringAction_DoesNotAllocateGCMemory(PlayerNotifications notificationBehavior, Type listenerType)
+    {
+        var gamepad = InputSystem.AddDevice<Gamepad>();
+
+        var go = new GameObject();
+        var playerInput = go.AddComponent<PlayerInput>();
+        playerInput.notificationBehavior = notificationBehavior;
+        playerInput.defaultActionMap = "gameplay";
+        playerInput.actions = InputActionAsset.FromJson(kActions);
+
+        var listener = (IListener)go.AddComponent(listenerType);
+        // We don't want the listener to actually record messages. They have an object field which will
+        // box values and thus allocate GC garbage. We *do* want the listener to actually read values
+        // to make sure that doesn't allocate anything.
+        listener.messages = null;
+
+        // First message is allowed to perform initialization work and thus allocate.
+        PressAndRelease(gamepad.buttonSouth);
+
+        var kProfilerRegion = "PlayerInput_TriggeringAction_DoesNotAllocateGCMemory";
+
+        Assert.That(() =>
+        {
+            Profiler.BeginSample(kProfilerRegion);
+            PressAndRelease(gamepad.buttonSouth);
+            Profiler.EndSample();
+        }, Is.Not.AllocatingGCMemory());
+    }
+
     [Test]
     [Category("PlayerInput")]
     [TestCase(PlayerNotifications.SendMessages, typeof(MessageListener))]
@@ -1791,14 +1849,6 @@ public void TODO_PlayerInput_CanSetUpSplitScreen_AndManuallyAllocatePlayersToScr
         Assert.Fail();
     }
 
-    [Test]
-    [Category("PlayerInput")]
-    [Ignore("TODO")]
-    public void TODO_PlayerInput_TriggeringAction_DoesNotAllocate()
-    {
-        Assert.Fail();
-    }
-
     // An action is either
     //   (a) button-like, or
     //   (b) axis-like, or
@@ -1823,7 +1873,8 @@ public void TODO_PlayerInput_TriggeringAction_DoesNotAllocate()
                     ""actions"" : [
                         { ""name"" : ""Fire"", ""type"" : ""button"" },
                         { ""name"" : ""Look"", ""type"" : ""value"" },
-                        { ""name"" : ""Move"", ""type"" : ""value"" }
+                        { ""name"" : ""Move"", ""type"" : ""value"" },
+                        { ""name"" : ""Action With Spaces!!"", ""type"" : ""value"" }
                     ],
                     ""bindings"" : [
                         { ""path"" : ""<Gamepad>/buttonSouth"", ""action"" : ""fire"", ""groups"" : ""Gamepad"" },
@@ -1836,7 +1887,8 @@ public void TODO_PlayerInput_TriggeringAction_DoesNotAllocate()
                         { ""path"" : ""<Keyboard>/w"", ""name"" : ""up"", ""action"" : ""move"", ""groups"" : ""Keyboard&Mouse"", ""isPartOfComposite"" : true },
                         { ""path"" : ""<Keyboard>/s"", ""name"" : ""down"", ""action"" : ""move"", ""groups"" : ""Keyboard&Mouse"", ""isPartOfComposite"" : true },
                         { ""path"" : ""<Mouse>/delta"", ""action"" : ""look"", ""groups"" : ""Keyboard&Mouse"" },
-                        { ""path"" : ""<Mouse>/leftButton"", ""action"" : ""fire"", ""groups"" : ""Keyboard&Mouse"" }
+                        { ""path"" : ""<Mouse>/leftButton"", ""action"" : ""fire"", ""groups"" : ""Keyboard&Mouse"" },
+                        { ""path"" : ""<Gamepad>/buttonNorth"", ""action"" : ""Action With Spaces!!"", ""groups"" : ""Gamepad"" }
                     ]
                 },
                 {
@@ -1884,8 +1936,8 @@ public void TODO_PlayerInput_TriggeringAction_DoesNotAllocate()
 
     private struct Message : IEquatable<Message>
     {
-        public string name;
-        public object value;
+        public string name { get; set; }
+        public object value { get; set; }
 
         public Message(string name, object value = null)
         {
@@ -1923,65 +1975,71 @@ public override int GetHashCode()
 
     private interface IListener
     {
-        List<Message> messages { get; }
+        List<Message> messages { get; set; }
     }
 
     private class MessageListener : MonoBehaviour, IListener
     {
-        public List<Message> messages { get; } = new List<Message>();
+        public List<Message> messages { get; set; } = new List<Message>();
 
         // ReSharper disable once UnusedMember.Local
         public void OnFire(InputValue value)
         {
-            messages.Add(new Message { name = "OnFire", value = value.Get<float>() });
+            messages?.Add(new Message { name = "OnFire", value = value.Get<float>() });
         }
 
         // ReSharper disable once UnusedMember.Local
         public void OnLook(InputValue value)
         {
-            messages.Add(new Message { name = "OnLook", value = value.Get<Vector2>() });
+            messages?.Add(new Message { name = "OnLook", value = value.Get<Vector2>() });
         }
 
         // ReSharper disable once UnusedMember.Local
         public void OnMove(InputValue value)
         {
-            messages.Add(new Message { name = "OnMove", value = value.Get<Vector2>() });
+            messages?.Add(new Message { name = "OnMove", value = value.Get<Vector2>() });
         }
 
         // ReSharper disable once UnusedMember.Local
         public void OnOtherAction(InputValue value)
         {
-            messages.Add(new Message { name = "OnOtherAction", value = value.Get<float>() });
+            messages?.Add(new Message { name = "OnOtherAction", value = value.Get<float>() });
+        }
+
+        // ReSharper disable once UnusedMember.Local
+        public void OnActionWithSpaces(InputValue value)
+        {
+            messages?.Add(new Message { name = "OnActionWithSpaces", value = value.Get<float>() });
         }
 
         // ReSharper disable once UnusedMember.Local
         public void OnDeviceLost(PlayerInput player)
         {
-            messages.Add(new Message { name = "OnDeviceLost", value = player});
+            messages?.Add(new Message { name = "OnDeviceLost", value = player});
         }
 
         // ReSharper disable once UnusedMember.Local
         public void OnDeviceRegained(PlayerInput player)
         {
-            messages.Add(new Message { name = "OnDeviceRegained", value = player});
+            messages?.Add(new Message { name = "OnDeviceRegained", value = player});
         }
 
         // ReSharper disable once UnusedMember.Local
         public void OnControlsChanged(PlayerInput player)
         {
-            messages.Add(new Message { name = "OnControlsChanged", value = player});
+            messages?.Add(new Message { name = "OnControlsChanged", value = player});
         }
 
         // ReSharper disable once UnusedMember.Local
         public void OnPlayerJoined(PlayerInput player)
         {
-            messages.Add(new Message { name = "OnPlayerJoined", value = player});
+            messages?.Add(new Message { name = "OnPlayerJoined", value = player});
         }
 
         // ReSharper disable once UnusedMember.Local
         public void OnPlayerLeft(PlayerInput player)
         {
-            messages.Add(new Message { name = "OnPlayerLeft", value = player});
+            messages?.Add(new Message { name = "OnPlayerLeft", value = player});
         }
     }
 
@@ -1990,7 +2048,7 @@ public void OnPlayerLeft(PlayerInput player)
     [Preserve]
     private class PlayerInputEventListener : MonoBehaviour, IListener
     {
-        public List<Message> messages { get; } = new List<Message>();
+        public List<Message> messages { get; set; } = new List<Message>();
 
         public void OnEnable()
         {
@@ -2031,40 +2089,40 @@ private void SetUpActionEvents(PlayerInput player)
 
         private void OnFireEvent(InputAction.CallbackContext context)
         {
-            messages.Add(new Message($"Fire {context.phase}", context.ReadValueAsObject()));
+            messages?.Add(new Message($"Fire {context.phase}", context.ReadValueAsObject()));
         }
 
         private void OnLookEvent(InputAction.CallbackContext context)
         {
-            messages.Add(new Message($"Look {context.phase}", context.ReadValueAsObject()));
+            messages?.Add(new Message($"Look {context.phase}", context.ReadValueAsObject()));
         }
 
         private void OnMoveEvent(InputAction.CallbackContext context)
         {
-            messages.Add(new Message($"Move {context.phase}", context.ReadValueAsObject()));
+            messages?.Add(new Message($"Move {context.phase}", context.ReadValueAsObject()));
         }
 
         private void OnDeviceLost(PlayerInput player)
         {
-            messages.Add(new Message("OnDeviceLost", player));
+            messages?.Add(new Message("OnDeviceLost", player));
         }
 
         private void OnDeviceRegained(PlayerInput player)
         {
-            messages.Add(new Message("OnDeviceRegained", player));
+            messages?.Add(new Message("OnDeviceRegained", player));
         }
 
         private void OnControlsChanged(PlayerInput player)
         {
-            messages.Add(new Message("OnControlsChanged", player));
+            messages?.Add(new Message("OnControlsChanged", player));
         }
     }
 
     // il2cpp internally crashes if this gets stripped. Need to investigate, but for now we preserve it.
     [Preserve]
     private class PlayerInputCSharpEventListener : MonoBehaviour, IListener
     {
-        public List<Message> messages { get; } = new List<Message>();
+        public List<Message> messages { get; set; } = new List<Message>();
 
         public void OnEnable()
         {
@@ -2079,30 +2137,30 @@ public void OnEnable()
 
         private void OnAction(InputAction.CallbackContext context)
         {
-            messages.Add(new Message($"{context.action.name} {context.phase}", context.ReadValueAsObject()));
+            messages?.Add(new Message($"{context.action.name} {context.phase}", context.ReadValueAsObject()));
         }
 
         private void OnDeviceLost(PlayerInput player)
         {
-            messages.Add(new Message("OnDeviceLost", player));
+            messages?.Add(new Message("OnDeviceLost", player));
         }
 
         private void OnDeviceRegained(PlayerInput player)
         {
-            messages.Add(new Message("OnDeviceRegained", player));
+            messages?.Add(new Message("OnDeviceRegained", player));
         }
 
         private void OnControlsChanged(PlayerInput player)
         {
-            messages.Add(new Message("OnControlsChanged", player));
+            messages?.Add(new Message("OnControlsChanged", player));
         }
     }
 
     // il2cpp internally crashes if this gets stripped. Need to investigate, but for now we preserve it.
     [Preserve]
     private class PlayerManagerEventListener : MonoBehaviour, IListener
     {
-        public List<Message> messages { get; } = new List<Message>();
+        public List<Message> messages { get; set; } = new List<Message>();
 
         public void OnEnable()
         {
@@ -2128,7 +2186,7 @@ private void OnPlayerLeft(PlayerInput player)
     [Preserve]
     private class PlayerManagerCSharpEventListener : MonoBehaviour, IListener
     {
-        public List<Message> messages { get; } = new List<Message>();
+        public List<Message> messages { get; set; } = new List<Message>();
 
         public void OnEnable()
         {
@@ -2141,12 +2199,12 @@ public void OnEnable()
 
         private void OnPlayerJoined(PlayerInput player)
         {
-            messages.Add(new Message("OnPlayerJoined", player));
+            messages?.Add(new Message("OnPlayerJoined", player));
         }
 
         private void OnPlayerLeft(PlayerInput player)
         {
-            messages.Add(new Message("OnPlayerLeft", player));
+            messages?.Add(new Message("OnPlayerLeft", player));
         }
     }
 }

Packages/com.unity.inputsystem/CHANGELOG.md

@@ -21,6 +21,7 @@ however, it has to be formatted properly to pass verification tests.
 #### Actions
 
 - Controls are now re-resolved after adding or removing bindings from actions ([case 1218544](https://issuetracker.unity3d.com/issues/input-system-package-does-not-re-resolve-bindings-when-adding-a-new-binding-to-a-map-that-has-already-generated-its-state)).
+- Can now have spaces and special characters in action names when using `PlayerInput` with the `SendMessages` or `BroadcastMessages` behavior. Previously, an incorrect method name was generated (fix contributed by [BHSPitMonkey](https://github.com/BHSPitMonkey) in [#1022](https://github.com/Unity-Technologies/InputSystem/pull/1022); [case 1214519](https://issuetracker.unity3d.com/issues/player-input-send-messages-wont-trigger-when-input-action-name-contains-spaces)).
 - Adding a new action now sets `expectedControlType` to `Button` as expected ([case 1221015](https://issuetracker.unity3d.com/issues/input-system-default-value-of-expectedcontroltype-is-not-being-set-when-creating-a-new-action)).
 - Player joins with `PlayerInputManager` from button presses no longer fail if there are multiple devices of the same type present and the join was not on the first gamepad ([case 226920](https://fogbugz.unity3d.com/f/cases/1226920/)).
 - `PlayerInputEditor` no longer leads to the player's `InputActionAsset` mistakenly getting replaced with a clone when the inspector is open on a `PlayerInput` component ([case 1228636](https://issuetracker.unity3d.com/issues/action-map-gets-lost-on-play-when-prefab-is-highlighted-in-inspector)).

Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInput.cs

@@ -1298,9 +1298,7 @@ private void CacheMessageNames()
             {
                 action.MakeSureIdIsInPlace();
 
-                var name = action.name;
-                if (char.IsLower(name[0]))
-                    name = char.ToUpper(name[0]) + name.Substring(1);
+                var name = CSharpCodeHelpers.MakeTypeName(action.name);
                 m_ActionMessageNames[action.m_Id] = "On" + name;
             }
         }

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

@@ -2,6 +2,8 @@
 using System.Linq;
 using System.Text;
 
+////REVIEW: this seems like it should be #if UNITY_EDITOR
+
 namespace UnityEngine.InputSystem.Utilities
 {
     internal static class CSharpCodeHelpers