From ad73e5d54f42c69aec952cfe30ec1fa3153d5cdf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Malrat?= Date: Wed, 18 Sep 2024 15:28:37 -0400 Subject: [PATCH 1/2] Fixed Action Maps contextual menu in Action Editor UI that could display unrelated items. --- Packages/com.unity.inputsystem/CHANGELOG.md | 1 + .../Editor/UITKAssetEditor/Views/ContextMenu.cs | 4 +++- .../UITKAssetEditor/Views/InputActionMapsTreeViewItem.cs | 7 +++++++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/Packages/com.unity.inputsystem/CHANGELOG.md b/Packages/com.unity.inputsystem/CHANGELOG.md index 3e896e29f5..84a82b8395 100644 --- a/Packages/com.unity.inputsystem/CHANGELOG.md +++ b/Packages/com.unity.inputsystem/CHANGELOG.md @@ -13,6 +13,7 @@ however, it has to be formatted properly to pass verification tests. ### Fixed - Fixed Multiple interactions could breaks on Composite Binding. [ISXB-619](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-619) - Fixed memory leak when the OnScreenStick component was destroyed [ISXB-1070](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1070). Contribution by [LukeUnityDev](https://github.com/LukeUnityDev). +- Fixed Action Maps contextual menu in Action Editor UI that could display unrelated items. ### Changed - Renamed editor Resources directories to PackageResources to fix package validation warnings. diff --git a/Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/ContextMenu.cs b/Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/ContextMenu.cs index 1ec973f564..a19e4d31de 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/ContextMenu.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/ContextMenu.cs @@ -27,8 +27,9 @@ internal static class ContextMenu #region ActionMaps public static void GetContextMenuForActionMapItem(ActionMapsView mapView, InputActionMapsTreeViewItem treeViewItem, int index) { - _ = new ContextualMenuManipulator(menuEvent => + var manipulator = new ContextualMenuManipulator(menuEvent => { + //menuEvent.menu.ClearItems(); // TODO: AddAction should enable m_RenameOnActionAdded menuEvent.menu.AppendAction(add_Action_String, _ => mapView.Dispatch(Commands.AddAction())); menuEvent.menu.AppendSeparator(); @@ -43,6 +44,7 @@ public static void GetContextMenuForActionMapItem(ActionMapsView mapView, InputA if (CopyPasteHelper.HasPastableClipboardData(typeof(InputActionMap))) menuEvent.menu.AppendAction(paste_String, _ => mapView.PasteItems(copiedAction)); }) { target = treeViewItem }; + treeViewItem.contextualMenuManipulator = manipulator; } // Add "Add Action Map" option to empty space under the ListView. Matches with old IMGUI style (ISX-1519). diff --git a/Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/InputActionMapsTreeViewItem.cs b/Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/InputActionMapsTreeViewItem.cs index 126e78feab..c66921f24a 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/InputActionMapsTreeViewItem.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/InputActionMapsTreeViewItem.cs @@ -18,6 +18,8 @@ internal class InputActionMapsTreeViewItem : VisualElement private const string kRenameTextField = "rename-text-field"; public event EventCallback EditTextFinished; + internal ContextualMenuManipulator contextualMenuManipulator; + // for testing purposes to know if the item is focused to accept input internal bool IsFocused { get; private set; } = false; @@ -87,6 +89,11 @@ public void Reset() m_IsEditing = false; } EditTextFinished = null; + if (contextualMenuManipulator != null) + { + contextualMenuManipulator.target = null; + contextualMenuManipulator = null; + } } public void FocusOnRenameTextField() From eb3996f9f37731b61537b06356f1715ace52c2f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Malrat?= Date: Thu, 19 Sep 2024 10:22:24 -0400 Subject: [PATCH 2/2] reworked to reduce allocation --- Packages/com.unity.inputsystem/CHANGELOG.md | 2 +- .../Editor/UITKAssetEditor/Views/ContextMenu.cs | 6 ++---- .../Views/InputActionMapsTreeViewItem.cs | 14 +++++++------- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/Packages/com.unity.inputsystem/CHANGELOG.md b/Packages/com.unity.inputsystem/CHANGELOG.md index 84a82b8395..0c82a0b60d 100644 --- a/Packages/com.unity.inputsystem/CHANGELOG.md +++ b/Packages/com.unity.inputsystem/CHANGELOG.md @@ -13,7 +13,7 @@ however, it has to be formatted properly to pass verification tests. ### Fixed - Fixed Multiple interactions could breaks on Composite Binding. [ISXB-619](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-619) - Fixed memory leak when the OnScreenStick component was destroyed [ISXB-1070](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1070). Contribution by [LukeUnityDev](https://github.com/LukeUnityDev). -- Fixed Action Maps contextual menu in Action Editor UI that could display unrelated items. +- Fixed Action Maps contextual menu in Action Editor UI that occasionally displays unrelated items. ### Changed - Renamed editor Resources directories to PackageResources to fix package validation warnings. diff --git a/Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/ContextMenu.cs b/Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/ContextMenu.cs index a19e4d31de..a8163243e3 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/ContextMenu.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/ContextMenu.cs @@ -27,9 +27,8 @@ internal static class ContextMenu #region ActionMaps public static void GetContextMenuForActionMapItem(ActionMapsView mapView, InputActionMapsTreeViewItem treeViewItem, int index) { - var manipulator = new ContextualMenuManipulator(menuEvent => + treeViewItem.OnContextualMenuPopulateEvent = (menuEvent => { - //menuEvent.menu.ClearItems(); // TODO: AddAction should enable m_RenameOnActionAdded menuEvent.menu.AppendAction(add_Action_String, _ => mapView.Dispatch(Commands.AddAction())); menuEvent.menu.AppendSeparator(); @@ -43,8 +42,7 @@ public static void GetContextMenuForActionMapItem(ActionMapsView mapView, InputA var copiedAction = CopyPasteHelper.GetCopiedClipboardType() == typeof(InputAction); if (CopyPasteHelper.HasPastableClipboardData(typeof(InputActionMap))) menuEvent.menu.AppendAction(paste_String, _ => mapView.PasteItems(copiedAction)); - }) { target = treeViewItem }; - treeViewItem.contextualMenuManipulator = manipulator; + }); } // Add "Add Action Map" option to empty space under the ListView. Matches with old IMGUI style (ISX-1519). diff --git a/Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/InputActionMapsTreeViewItem.cs b/Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/InputActionMapsTreeViewItem.cs index c66921f24a..8c99f99ae4 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/InputActionMapsTreeViewItem.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/InputActionMapsTreeViewItem.cs @@ -1,6 +1,7 @@ // UITK TreeView is not supported in earlier versions // Therefore the UITK version of the InputActionAsset Editor is not available on earlier Editor versions either. #if UNITY_EDITOR && UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS +using System; using System.Threading.Tasks; using UnityEditor; using UnityEngine.InputSystem.Editor; @@ -17,8 +18,7 @@ internal class InputActionMapsTreeViewItem : VisualElement private const string kRenameTextField = "rename-text-field"; public event EventCallback EditTextFinished; - - internal ContextualMenuManipulator contextualMenuManipulator; + public Action OnContextualMenuPopulateEvent; // for testing purposes to know if the item is focused to accept input internal bool IsFocused { get; private set; } = false; @@ -43,6 +43,11 @@ public InputActionMapsTreeViewItem() RegisterCallback(OnMouseDownEventForRename); renameTextfield.RegisterCallback(e => IsFocused = true); renameTextfield.RegisterCallback(e => { OnEditTextFinished(); IsFocused = false; }); + _ = new ContextualMenuManipulator(menuBuilder => + { + OnContextualMenuPopulateEvent?.Invoke(menuBuilder); + }) + { target = this }; } public Label label => this.Q