From 086bb2f273fadc14b12877bd96a94e54efa5058a Mon Sep 17 00:00:00 2001 From: Alex Tyrer Date: Thu, 5 Dec 2024 17:38:05 +0000 Subject: [PATCH 1/3] [Input System] PlayerInput component - ensure any added action map delegates are updated on ActivateInput() (case ISXB-711) If action maps are added to an asset *after* the asset has been assigned to the PlayerInput component then the delegates need to be updated. Before this change no On callbacks were created for the added maps leading to input loss. Adding this sync in the ActivateInput() seems like the best compromise. --- .../Plugins/PlayerInput/PlayerInput.cs | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInput.cs b/Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInput.cs index 66929b7c2b..a6dca7ecba 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInput.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInput.cs @@ -950,6 +950,8 @@ public TDevice GetDevice() /// public void ActivateInput() { + UpdateDelegates(); + m_InputActive = true; // If we have no current action map but there's a default @@ -960,6 +962,22 @@ public void ActivateInput() m_CurrentActionMap?.Enable(); } + // Users can add and remove actions maps *after* assigning an InputActionAsset to the PlayerInput component. + // This ensures "actionTriggered" delegates are assigned for new maps (case isxb-711) + // + private int m_MapCount = 0; + private void UpdateDelegates() + { + if (m_Actions == null) + return; + if (m_MapCount != m_Actions.actionMaps.Count) + { + InstallOnActionTriggeredHook(); + CacheMessageNames(); + m_MapCount = m_Actions.actionMaps.Count; + } + } + /// /// Disable input on the player, by disabling the current action map /// @@ -1515,6 +1533,8 @@ private void CacheMessageNames() private void ClearCaches() { + if (m_ActionMessageNames != null) + m_ActionMessageNames.Clear(); } /// From be3e972318fae690856a2bb70267212aded4410e Mon Sep 17 00:00:00 2001 From: Alex Tyrer Date: Mon, 9 Dec 2024 09:51:54 +0000 Subject: [PATCH 2/3] [Input System] Use hashCode to check for changes to the InputActionAsset assigned to the PlayerInput component. o This ensures the delegates for all the action maps in the asset are kept updated when changes are made after initial assignment to the PlayerInputComponent. --- Packages/com.unity.inputsystem/CHANGELOG.md | 1 + .../Plugins/PlayerInput/PlayerInput.cs | 15 ++++++++++++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/Packages/com.unity.inputsystem/CHANGELOG.md b/Packages/com.unity.inputsystem/CHANGELOG.md index 865e7ba999..98d51886ea 100644 --- a/Packages/com.unity.inputsystem/CHANGELOG.md +++ b/Packages/com.unity.inputsystem/CHANGELOG.md @@ -30,6 +30,7 @@ however, it has to be formatted properly to pass verification tests. - Fixed tooltip support in the UI Toolkit version of the Input Actions Asset editor. - Fixed documentation to clarify bindings with modifiers `overrideModifiersNeedToBePressedFirst` configuration [ISXB-806](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-806). - Fixed an issue in `Samples/Visualizers/GamepadVisualizer.unity` sample where the visualization wouldn't handle device disconnects or current device changes properly (ISXB-1243). +- Fixed an issue where action map delegates were not updated when the asset already assigned to the PlayerInput component were changed [ISXB-711](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-711). ### Changed - Added back the InputManager to InputSystem project-wide asset migration code with performance improvement (ISX-2086). diff --git a/Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInput.cs b/Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInput.cs index a6dca7ecba..1375503db1 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInput.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInput.cs @@ -965,16 +965,25 @@ public void ActivateInput() // Users can add and remove actions maps *after* assigning an InputActionAsset to the PlayerInput component. // This ensures "actionTriggered" delegates are assigned for new maps (case isxb-711) // - private int m_MapCount = 0; + private int m_AllMapsHashCode = 0; private void UpdateDelegates() { if (m_Actions == null) + { + m_AllMapsHashCode = 0; return; - if (m_MapCount != m_Actions.actionMaps.Count) + } + + int allMapsHashCode = 0; + foreach (var actionMap in m_Actions.actionMaps) + { + allMapsHashCode ^= actionMap.GetHashCode(); + } + if (m_AllMapsHashCode != allMapsHashCode) { InstallOnActionTriggeredHook(); CacheMessageNames(); - m_MapCount = m_Actions.actionMaps.Count; + m_AllMapsHashCode = allMapsHashCode; } } From 96d72847c5b651fb29fa4a30738f1519e2ab7104 Mon Sep 17 00:00:00 2001 From: Alex Tyrer Date: Tue, 10 Dec 2024 15:57:36 +0000 Subject: [PATCH 3/3] [Input System] Added a test for action bindings delegates being created after assigning an asset to the PlayerInput component (case ISXB-711) --- .../InputSystem/Plugins/PlayerInputTests.cs | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/Assets/Tests/InputSystem/Plugins/PlayerInputTests.cs b/Assets/Tests/InputSystem/Plugins/PlayerInputTests.cs index ece203817b..7bf1d5ee81 100644 --- a/Assets/Tests/InputSystem/Plugins/PlayerInputTests.cs +++ b/Assets/Tests/InputSystem/Plugins/PlayerInputTests.cs @@ -2405,6 +2405,39 @@ public void PlayerInput_CanDisableAfterAssigningAction_WithControlSchemesAndInte player.SetActive(false); // Should cause full rebinding and not assert } + [Test] + [Category("PlayerInput")] + public void PlayerInput_DelegatesAreUpdate_WhenActionMapAddedAfterAssignment() + { + var gamepad = InputSystem.AddDevice(); + + var go = new GameObject(); + var listener = go.AddComponent(); + var playerInput = go.AddComponent(); + playerInput.defaultActionMap = "Other"; + var actionAsset = InputActionAsset.FromJson(kActions); + playerInput.actions = actionAsset; + + // 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(); + var keyboard = InputSystem.AddDevice(); + var newActionMap = actionAsset.AddActionMap("NewMap"); + var newAction = newActionMap.AddAction("NewAction"); + newAction.AddBinding("/k", groups: "Keyboard"); + actionAsset.AddControlScheme("Keyboard").WithRequiredDevice(); + actionAsset.Enable(); + + playerInput.currentActionMap = newActionMap; + playerInput.ActivateInput(); + listener.messages.Clear(); + + Press(keyboard.kKey); + + Assert.That(listener.messages, Has.Exactly(1).With.Property("name").EqualTo("OnNewAction")); + } + private struct Message : IEquatable { public string name { get; set; } @@ -2477,6 +2510,11 @@ public void OnOtherAction(InputValue value) messages?.Add(new Message { name = "OnOtherAction", value = value.Get() }); } + public void OnNewAction(InputValue value) + { + messages?.Add(new Message { name = "OnNewAction", value = value.Get() }); + } + // ReSharper disable once UnusedMember.Local public void OnActionWithSpaces(InputValue value) {