FIX: Project wide actions enabling actions overrides PlayerInput (ISXB-920) (#2144)

Author: ritamerkl

Assets/Tests/InputSystem/Plugins/PlayerInputTests.cs

@@ -55,7 +55,7 @@ public void PlayerInput_CanInstantiatePlayer()
 
         Assert.That(player, Is.Not.Null);
         Assert.That(player.playerIndex, Is.EqualTo(0));
-        Assert.That(player.actions, Is.SameAs(prefabPlayerInput.actions));
+        Assert.That(player.actions.actionMaps.Count, Is.EqualTo(prefabPlayerInput.actions.actionMaps.Count));
         Assert.That(player.devices, Is.EquivalentTo(new[] { gamepad }));
         Assert.That(player.currentControlScheme, Is.EqualTo("Gamepad"));
     }
@@ -108,7 +108,6 @@ public void PlayerInput_CanLinkSpecificDeviceToUI()
         var ui = prefab.AddComponent<InputSystemUIInputModule>();
         player.uiInputModule = ui;
         player.actions = InputActionAsset.FromJson(kActions);
-        ui.actionsAsset = player.actions;
 
         InputSystem.AddDevice<Gamepad>();
         InputSystem.AddDevice<Keyboard>();
@@ -117,6 +116,7 @@ public void PlayerInput_CanLinkSpecificDeviceToUI()
         var gamepad = InputSystem.AddDevice<Gamepad>();
 
         var instance = PlayerInput.Instantiate(prefab, pairWithDevices: gamepad);
+        ui.actionsAsset = instance.actions;
 
         Assert.That(instance.devices, Is.EquivalentTo(new[] { gamepad }));
         Assert.That(ui.actionsAsset.devices, Is.EquivalentTo(new[] { gamepad }));
@@ -149,11 +149,11 @@ public void PlayerInput_CanUseSameActionsForUIInputModule()
         eventSystemGO.SetActive(true);
         playerGO.SetActive(true);
 
-        Assert.That(actions.FindActionMap("Gameplay").enabled, Is.True);
-        Assert.That(actions.FindActionMap("UI").enabled, Is.True);
-        Assert.That(actions["UI/Navigate"].controls, Is.Empty);
-        Assert.That(actions["UI/Point"].controls, Is.EquivalentTo(new[] { mouse.position }));
-        Assert.That(actions["UI/Click"].controls, Is.EquivalentTo(new[] { mouse.leftButton }));
+        Assert.That(player.actions.FindActionMap("Gameplay").enabled, Is.True);
+        Assert.That(uiModule.actionsAsset.FindActionMap("UI").enabled, Is.True);
+        Assert.That(uiModule.actionsAsset["UI/Navigate"].controls, Is.Empty);
+        Assert.That(uiModule.actionsAsset["UI/Point"].controls, Is.EquivalentTo(new[] { mouse.position }));
+        Assert.That(uiModule.actionsAsset["UI/Click"].controls, Is.EquivalentTo(new[] { mouse.leftButton }));
     }
 
     [Test]
@@ -391,7 +391,23 @@ public void PlayerInput_CanAssignActionsToPlayer()
         var actions = InputActionAsset.FromJson(kActions);
         playerInput.actions = actions;
 
-        Assert.That(playerInput.actions, Is.SameAs(actions));
+        Assert.That(playerInput.actions.actionMaps.Count, Is.EqualTo(actions.actionMaps.Count));
+        Assert.That(playerInput.actions.actionMaps[0].name, Is.EqualTo(actions.actionMaps[0].name));
+    }
+
+    [Test]
+    [Category("PlayerInput")]
+    public void PlayerInput_CopiesActionAssetForFirstPlayer()
+    {
+        var go = new GameObject();
+        var playerInput = go.AddComponent<PlayerInput>();
+
+        var actions = InputActionAsset.FromJson(kActions);
+        playerInput.actions = actions;
+
+        Assert.That(playerInput.actions.actionMaps.Count, Is.EqualTo(actions.actionMaps.Count));
+        Assert.That(playerInput.actions.actionMaps[0].name, Is.EqualTo(actions.actionMaps[0].name));
+        Assert.That(playerInput.actions.GetInstanceID(), !Is.EqualTo(actions.GetInstanceID()));
     }
 
     [Test]
@@ -407,13 +423,13 @@ public void PlayerInput_AssigningNewActionsToPlayer_DisablesExistingActions()
         playerInput.defaultActionMap = "gameplay";
         playerInput.actions = actions1;
 
-        Assert.That(actions1.actionMaps[0].enabled, Is.True);
-        Assert.That(actions2.actionMaps[0].enabled, Is.False);
+        Assert.That(playerInput.actions.actionMaps[0].enabled, Is.True);
+        Assert.That(actions1.actionMaps[0].enabled, Is.False);
 
         playerInput.actions = actions2;
 
-        Assert.That(actions1.actionMaps[0].enabled, Is.False);
-        Assert.That(actions2.actionMaps[0].enabled, Is.True);
+        Assert.That(actions2.actionMaps[0].enabled, Is.False);
+        Assert.That(playerInput.actions.actionMaps[0].enabled, Is.True);
     }
 
     [Test]
@@ -1714,7 +1730,8 @@ InputDevice[] AddDevices()
 
         // Make sure that no cloning of actions happened on the prefab.
         // https://fogbugz.unity3d.com/f/cases/1319756/
-        Assert.That(playerPrefab.GetComponent<PlayerInput>().actions, Is.SameAs(playerPrefabActions));
+
+        Assert.That(playerPrefab.GetComponent<PlayerInput>().actions.actionMaps.Count, Is.EqualTo(playerPrefabActions.actionMaps.Count));
         Assert.That(playerPrefab.GetComponent<PlayerInput>().m_ActionsInitialized, Is.False);
     }
 
@@ -2421,13 +2438,13 @@ public void PlayerInput_DelegatesAreUpdate_WhenActionMapAddedAfterAssignment()
         // Disable the asset while adding another action map to it as none
         // of the actions in the asset can be enabled during modification
         //
-        actionAsset.Disable();
+        playerInput.actions.Disable();
         var keyboard = InputSystem.AddDevice<Keyboard>();
-        var newActionMap = actionAsset.AddActionMap("NewMap");
+        var newActionMap = playerInput.actions.AddActionMap("NewMap");
         var newAction = newActionMap.AddAction("NewAction");
         newAction.AddBinding("<Keyboard>/k", groups: "Keyboard");
-        actionAsset.AddControlScheme("Keyboard").WithRequiredDevice<Keyboard>();
-        actionAsset.Enable();
+        playerInput.actions.AddControlScheme("Keyboard").WithRequiredDevice<Keyboard>();
+        playerInput.actions.Enable();
 
         playerInput.currentActionMap = newActionMap;
         playerInput.ActivateInput();

Packages/com.unity.inputsystem/CHANGELOG.md

@@ -10,6 +10,9 @@ however, it has to be formatted properly to pass verification tests.
 
 ## [Unreleased] - yyyy-mm-dd
 
+### Fixed
+- Fixed an issue where all action maps were enabled initially for project wide actions, which overrode the PlayerInput action map configuration. [ISXB-920](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-920)
+
 ### Changed
 - Changed enum value `Key.IMESelected` to obsolete which was not a real key. Please use the ButtonControl `imeSelected`.
 

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

@@ -3609,12 +3609,6 @@ internal static void InitializeInEditor(IInputRuntime runtime = null)
             // this would cancel the import of large assets that are dependent on the InputSystem package and import it as a dependency.
             EditorApplication.delayCall += ShowRestartWarning;
 
-#if UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS
-            // Make sure project wide input actions are enabled.
-            // Note that this will always fail if entering play-mode within editor since not yet in play-mode.
-            EnableActions();
-#endif
-
             RunInitialUpdate();
 
             k_InputInitializeInEditorMarker.End();
@@ -3657,9 +3651,9 @@ internal static void OnPlayModeChange(PlayModeStateChange change)
                 case PlayModeStateChange.EnteredPlayMode:
                     s_SystemObject.enterPlayModeTime = InputRuntime.s_Instance.currentTime;
                     s_Manager.SyncAllDevicesAfterEnteringPlayMode();
-                    #if UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS
+#if UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS
                     EnableActions();
-                    #endif // UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS
+#endif // UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS
                     break;
 
                 case PlayModeStateChange.ExitingPlayMode:

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

@@ -342,8 +342,14 @@ public InputActionAsset actions
                         UninitializeActions();
                 }
 
+                var didChange = m_Actions != null;
+
                 m_Actions = value;
 
+                if (didChange || m_Enabled)
+                    // copy action asset for the first player so that the original asset stays untouched
+                    CopyActionAssetAndApplyBindingOverrides();
+
                 if (m_Enabled)
                 {
                     ClearCaches();
@@ -1373,14 +1379,7 @@ private void InitializeActions()
             for (var i = 0; i < s_AllActivePlayersCount; ++i)
                 if (s_AllActivePlayers[i].m_Actions == m_Actions && s_AllActivePlayers[i] != this)
                 {
-                    var oldActions = m_Actions;
-                    m_Actions = Instantiate(m_Actions);
-                    for (var actionMap = 0; actionMap < oldActions.actionMaps.Count; actionMap++)
-                    {
-                        for (var binding = 0; binding < oldActions.actionMaps[actionMap].bindings.Count; binding++)
-                            m_Actions.actionMaps[actionMap].ApplyBindingOverride(binding, oldActions.actionMaps[actionMap].bindings[binding]);
-                    }
-
+                    CopyActionAssetAndApplyBindingOverrides();
                     break;
                 }
 
@@ -1430,6 +1429,18 @@ private void InitializeActions()
             m_ActionsInitialized = true;
         }
 
+        private void CopyActionAssetAndApplyBindingOverrides()
+        {
+            // duplicate action asset to not operate on the original (as it might be used outside - eg project wide action asset or UIInputModule)
+            var oldActions = m_Actions;
+            m_Actions = Instantiate(m_Actions);
+            for (var actionMap = 0; actionMap < oldActions.actionMaps.Count; actionMap++)
+            {
+                for (var binding = 0; binding < oldActions.actionMaps[actionMap].bindings.Count; binding++)
+                    m_Actions.actionMaps[actionMap].ApplyBindingOverride(binding, oldActions.actionMaps[actionMap].bindings[binding]);
+            }
+        }
+
         private void UninitializeActions()
         {
             if (!m_ActionsInitialized)
@@ -1799,6 +1810,13 @@ void Reset()
 
         #endif
 
+        private void Awake()
+        {
+            // If an action asset is assigned copy it to avoid modifying the original asset.
+            if (m_Actions != null)
+                CopyActionAssetAndApplyBindingOverrides();
+        }
+
         private void OnEnable()
         {
             m_Enabled = true;