From a907aff1176edc4be0cd4934eb419f62fbcd85ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Freire?= Date: Thu, 14 Dec 2023 18:10:07 +0100 Subject: [PATCH 01/10] Use project-wide actions for UI action map --- .../Plugins/InputForUI/InputSystemProvider.cs | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/Packages/com.unity.inputsystem/InputSystem/Plugins/InputForUI/InputSystemProvider.cs b/Packages/com.unity.inputsystem/InputSystem/Plugins/InputForUI/InputSystemProvider.cs index ebc4cabdca..f957de89c5 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Plugins/InputForUI/InputSystemProvider.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Plugins/InputForUI/InputSystemProvider.cs @@ -78,7 +78,6 @@ public void Initialize() m_TouchState.Reset(); m_SeenTouchEvents = false; - // TODO should UITK somehow override this? m_Cfg = Configuration.GetDefaultConfiguration(); RegisterActions(m_Cfg); } @@ -564,7 +563,7 @@ void UnregisterNextPreviousAction() void RegisterActions(Configuration cfg) { - m_InputActionAsset = InputActionAsset.FromJson(cfg.InputActionAssetAsJson); + m_InputActionAsset = cfg.ActionAsset; m_PointAction = InputActionReference.Create(m_InputActionAsset.FindAction(m_Cfg.PointAction)); m_MoveAction = InputActionReference.Create(m_InputActionAsset.FindAction(m_Cfg.MoveAction)); @@ -642,16 +641,12 @@ void UnregisterActions(Configuration cfg) // The Next/Previous action is not part of the input actions asset UnregisterNextPreviousAction(); -#if UNITY_EDITOR - UnityEngine.Object.DestroyImmediate(m_InputActionAsset); -#else - UnityEngine.Object.Destroy(m_InputActionAsset); -#endif + UnityEngine.Object.Destroy(m_InputActionAsset); // TODO check if this is ok } public struct Configuration { - public string InputActionAssetAsJson; + public InputActionAsset ActionAsset; public string PointAction; public string MoveAction; public string SubmitAction; @@ -663,14 +658,10 @@ public struct Configuration public static Configuration GetDefaultConfiguration() { - // TODO this is a weird way of doing that, is there an easier way? - var asset = new DefaultInputActions(); - var json = asset.asset.ToJson(); - UnityEngine.Object.DestroyImmediate(asset.asset); // TODO just Dispose doesn't work in edit mode - + return new Configuration { - InputActionAssetAsJson = json, + ActionAsset = InputSystem.actions, PointAction = "UI/Point", MoveAction = "UI/Navigate", SubmitAction = "UI/Submit", From c6e10e4ab3cea48c7d134d3d562409ebebb00f81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Freire?= Date: Thu, 14 Dec 2023 18:24:44 +0100 Subject: [PATCH 02/10] Show warning log when changing default UI actions --- .../ProjectWideActionsAsset.cs | 40 +++++++++++++++++++ .../InputActionsEditorWindowUtils.cs | 1 + 2 files changed, 41 insertions(+) diff --git a/Packages/com.unity.inputsystem/InputSystem/Editor/ProjectWideActions/ProjectWideActionsAsset.cs b/Packages/com.unity.inputsystem/InputSystem/Editor/ProjectWideActions/ProjectWideActionsAsset.cs index dd3ba85952..7fda6bb223 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Editor/ProjectWideActions/ProjectWideActionsAsset.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Editor/ProjectWideActions/ProjectWideActionsAsset.cs @@ -16,6 +16,7 @@ internal static class ProjectWideActionsAsset static string s_DefaultAssetPath = kDefaultAssetPath; static string s_AssetPath = kAssetPath; + static InputActionMap s_DefaultUIMap; #if UNITY_INCLUDE_TESTS internal static void SetAssetPaths(string defaultAssetPath, string assetPath) @@ -59,6 +60,8 @@ internal static InputActionAsset CreateNewActionAsset() asset.LoadFromJson(json); asset.name = kAssetName; + GetDefaultUIActionMap(); + AssetDatabase.AddObjectToAsset(asset, s_AssetPath); // Make sure all the elements in the asset have GUIDs and that they are indeed unique. @@ -92,6 +95,13 @@ internal static InputActionAsset CreateNewActionAsset() return asset; } + + private static void GetDefaultUIActionMap() + { + var json = File.ReadAllText(Path.Combine(Environment.CurrentDirectory, s_DefaultAssetPath)); + var actionMaps = InputActionMap.FromJson(json); + s_DefaultUIMap = actionMaps.FirstOrDefault(m => m.name == "UI"); + } private static void CreateInputActionReferences(InputActionAsset asset) { @@ -107,6 +117,36 @@ private static void CreateInputActionReferences(InputActionAsset asset) } } + internal static void CheckForDefaultUIActionMapChanges() + { + var asset = GetOrCreate(); + if (asset != null) + { + if (s_DefaultUIMap == null) + GetDefaultUIActionMap(); + + var uiMap = asset.actionMaps.FirstOrDefault(m => m.name == "UI"); + // "UI" action map has been removed or renamed. + if (uiMap == null) + { + Debug.LogWarning("The default UI action map does not exist.\r\n " + + "This will break the UI input at runtime. Please revert the changes to restore the default UI action map."); + return; + } + + // "UI" action map has been modified. + foreach (var action in s_DefaultUIMap) + { + if(uiMap.FindAction(action.name) == null) + { + Debug.LogWarning($"The UI action \"{action.name}\" has been modified.\r\n " + + "This will break the UI input at runtime. Please revert the changes to restore the default UI action map."); + return; + } + } + } + } + /// /// Updates the input action references in the asset by updating names, removing dangling references /// and adding new ones. diff --git a/Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorWindowUtils.cs b/Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorWindowUtils.cs index 937e0f1d7e..8376d4f0e9 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorWindowUtils.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorWindowUtils.cs @@ -18,6 +18,7 @@ public static void SaveAsset(SerializedObject serializedAsset) // For project-wide actions asset save works differently. The asset is in YAML format, not JSON. if (asset.name == ProjectWideActionsAsset.kAssetName) { + ProjectWideActionsAsset.CheckForDefaultUIActionMapChanges(); ProjectWideActionsAsset.UpdateInputActionReferences(); AssetDatabase.SaveAssets(); return; From d934c65819c2114daabc366b96643b23446dc330 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Freire?= Date: Thu, 21 Dec 2023 14:56:58 +0100 Subject: [PATCH 03/10] Fix rebase issue --- .../InputSystem/Plugins/InputForUI/InputSystemProvider.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/Packages/com.unity.inputsystem/InputSystem/Plugins/InputForUI/InputSystemProvider.cs b/Packages/com.unity.inputsystem/InputSystem/Plugins/InputForUI/InputSystemProvider.cs index f957de89c5..e3e27c7ebf 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Plugins/InputForUI/InputSystemProvider.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Plugins/InputForUI/InputSystemProvider.cs @@ -640,8 +640,6 @@ void UnregisterActions(Configuration cfg) // The Next/Previous action is not part of the input actions asset UnregisterNextPreviousAction(); - - UnityEngine.Object.Destroy(m_InputActionAsset); // TODO check if this is ok } public struct Configuration @@ -658,7 +656,6 @@ public struct Configuration public static Configuration GetDefaultConfiguration() { - return new Configuration { ActionAsset = InputSystem.actions, From 6fd971204d0f4c784bdc1636c7799440f91ef7b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Freire?= Date: Thu, 21 Dec 2023 16:03:04 +0100 Subject: [PATCH 04/10] Fix based on review comments --- .../ProjectWideActionsAsset.cs | 28 ++++++++----------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/Packages/com.unity.inputsystem/InputSystem/Editor/ProjectWideActions/ProjectWideActionsAsset.cs b/Packages/com.unity.inputsystem/InputSystem/Editor/ProjectWideActions/ProjectWideActionsAsset.cs index 7fda6bb223..f76a17c905 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Editor/ProjectWideActions/ProjectWideActionsAsset.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Editor/ProjectWideActions/ProjectWideActionsAsset.cs @@ -16,7 +16,6 @@ internal static class ProjectWideActionsAsset static string s_DefaultAssetPath = kDefaultAssetPath; static string s_AssetPath = kAssetPath; - static InputActionMap s_DefaultUIMap; #if UNITY_INCLUDE_TESTS internal static void SetAssetPaths(string defaultAssetPath, string assetPath) @@ -60,8 +59,6 @@ internal static InputActionAsset CreateNewActionAsset() asset.LoadFromJson(json); asset.name = kAssetName; - GetDefaultUIActionMap(); - AssetDatabase.AddObjectToAsset(asset, s_AssetPath); // Make sure all the elements in the asset have GUIDs and that they are indeed unique. @@ -95,12 +92,12 @@ internal static InputActionAsset CreateNewActionAsset() return asset; } - - private static void GetDefaultUIActionMap() + + private static InputActionMap GetDefaultUIActionMap() { var json = File.ReadAllText(Path.Combine(Environment.CurrentDirectory, s_DefaultAssetPath)); var actionMaps = InputActionMap.FromJson(json); - s_DefaultUIMap = actionMaps.FirstOrDefault(m => m.name == "UI"); + return actionMaps[actionMaps.IndexOf(x => x.name == "UI")]; } private static void CreateInputActionReferences(InputActionAsset asset) @@ -122,25 +119,24 @@ internal static void CheckForDefaultUIActionMapChanges() var asset = GetOrCreate(); if (asset != null) { - if (s_DefaultUIMap == null) - GetDefaultUIActionMap(); - - var uiMap = asset.actionMaps.FirstOrDefault(m => m.name == "UI"); + var defaultUIActionMap = GetDefaultUIActionMap(); + + var uiMap = asset.actionMaps[asset.actionMaps.IndexOf(x => x.name == "UI")]; // "UI" action map has been removed or renamed. if (uiMap == null) { - Debug.LogWarning("The default UI action map does not exist.\r\n " + - "This will break the UI input at runtime. Please revert the changes to restore the default UI action map."); + Debug.LogWarning("The action map named 'UI' does not exist.\r\n " + + "This will break the UI input at runtime. Please revert the changes to have an action map named 'UI'."); return; } // "UI" action map has been modified. - foreach (var action in s_DefaultUIMap) + foreach (var action in defaultUIActionMap.actions) { - if(uiMap.FindAction(action.name) == null) + if (uiMap.FindAction(action.name) == null) { - Debug.LogWarning($"The UI action \"{action.name}\" has been modified.\r\n " + - "This will break the UI input at runtime. Please revert the changes to restore the default UI action map."); + Debug.LogWarning($"The UI action \"{action.name}\" name has been modified.\r\n " + + $"This will break the UI input at runtime. Please make sure the action name with {action.name} exists."); return; } } From a2414e964544a644e23acbda0e3cb9c2fa705059 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Freire?= Date: Thu, 21 Dec 2023 16:07:49 +0100 Subject: [PATCH 05/10] Update CHANGELOG --- Packages/com.unity.inputsystem/CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Packages/com.unity.inputsystem/CHANGELOG.md b/Packages/com.unity.inputsystem/CHANGELOG.md index 7022a95f7f..17562c0239 100644 --- a/Packages/com.unity.inputsystem/CHANGELOG.md +++ b/Packages/com.unity.inputsystem/CHANGELOG.md @@ -10,6 +10,9 @@ however, it has to be formatted properly to pass verification tests. ## [Unreleased] +### Changed +- UI toolkit now uses the "UI" action map of project-wide actions as their default input actions. Removing bindings or renaming actions will break UI input for UI toolkit. + ### Fixed - Fixed missing confirmation popup when deleting a control scheme. - Fixed support for menu bar/customisable keyboard shortcuts used when interacting with Actions and Action Maps. From ededabc480e2e9a669e0296df88a6a76ac94e18a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Freire?= Date: Thu, 21 Dec 2023 17:12:25 +0100 Subject: [PATCH 06/10] Change file read method --- .../Editor/ProjectWideActions/ProjectWideActionsAsset.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Packages/com.unity.inputsystem/InputSystem/Editor/ProjectWideActions/ProjectWideActionsAsset.cs b/Packages/com.unity.inputsystem/InputSystem/Editor/ProjectWideActions/ProjectWideActionsAsset.cs index f76a17c905..e8cd740093 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Editor/ProjectWideActions/ProjectWideActionsAsset.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Editor/ProjectWideActions/ProjectWideActionsAsset.cs @@ -95,7 +95,7 @@ internal static InputActionAsset CreateNewActionAsset() private static InputActionMap GetDefaultUIActionMap() { - var json = File.ReadAllText(Path.Combine(Environment.CurrentDirectory, s_DefaultAssetPath)); + var json = File.ReadAllText(FileUtil.GetPhysicalPath(s_DefaultAssetPath)); var actionMaps = InputActionMap.FromJson(json); return actionMaps[actionMaps.IndexOf(x => x.name == "UI")]; } @@ -136,7 +136,7 @@ internal static void CheckForDefaultUIActionMapChanges() if (uiMap.FindAction(action.name) == null) { Debug.LogWarning($"The UI action \"{action.name}\" name has been modified.\r\n " + - $"This will break the UI input at runtime. Please make sure the action name with {action.name} exists."); + $"This will break the UI input at runtime. Please make sure the action name with \"{action.name}\" exists."); return; } } From ab8fa27b490e676a50ef816b83520f7f6d13d5a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Freire?= Date: Mon, 8 Jan 2024 13:11:30 +0100 Subject: [PATCH 07/10] Clean up code and comments --- .../ProjectWideActions/ProjectWideActionsAsset.cs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/Packages/com.unity.inputsystem/InputSystem/Editor/ProjectWideActions/ProjectWideActionsAsset.cs b/Packages/com.unity.inputsystem/InputSystem/Editor/ProjectWideActions/ProjectWideActionsAsset.cs index e8cd740093..c70fd2b9b6 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Editor/ProjectWideActions/ProjectWideActionsAsset.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Editor/ProjectWideActions/ProjectWideActionsAsset.cs @@ -93,7 +93,7 @@ internal static InputActionAsset CreateNewActionAsset() return asset; } - private static InputActionMap GetDefaultUIActionMap() + internal static InputActionMap GetDefaultUIActionMap() { var json = File.ReadAllText(FileUtil.GetPhysicalPath(s_DefaultAssetPath)); var actionMaps = InputActionMap.FromJson(json); @@ -114,25 +114,29 @@ private static void CreateInputActionReferences(InputActionAsset asset) } } + /// + /// Checks if the default UI action map has been modified or removed, to let the user know if their changes will + /// break the UI input at runtime, when using the UI Toolkit. + /// internal static void CheckForDefaultUIActionMapChanges() { var asset = GetOrCreate(); if (asset != null) { var defaultUIActionMap = GetDefaultUIActionMap(); + var uiMapIndex = asset.actionMaps.IndexOf(x => x.name == "UI"); - var uiMap = asset.actionMaps[asset.actionMaps.IndexOf(x => x.name == "UI")]; // "UI" action map has been removed or renamed. - if (uiMap == null) + if (uiMapIndex == -1) { Debug.LogWarning("The action map named 'UI' does not exist.\r\n " + "This will break the UI input at runtime. Please revert the changes to have an action map named 'UI'."); return; } - - // "UI" action map has been modified. + var uiMap = asset.m_ActionMaps[uiMapIndex]; foreach (var action in defaultUIActionMap.actions) { + // "UI" actions have been modified. if (uiMap.FindAction(action.name) == null) { Debug.LogWarning($"The UI action \"{action.name}\" name has been modified.\r\n " + From ab7dfa0b126855bb437ba8590cb8648898cc7de1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Freire?= Date: Mon, 8 Jan 2024 14:12:04 +0100 Subject: [PATCH 08/10] Add test to validate warnings --- .../CoreTests_ProjectWideActions.cs | 67 +++++++++++++++---- 1 file changed, 54 insertions(+), 13 deletions(-) diff --git a/Assets/Tests/InputSystem/CoreTests_ProjectWideActions.cs b/Assets/Tests/InputSystem/CoreTests_ProjectWideActions.cs index 5ead638bbc..14f5051732 100644 --- a/Assets/Tests/InputSystem/CoreTests_ProjectWideActions.cs +++ b/Assets/Tests/InputSystem/CoreTests_ProjectWideActions.cs @@ -4,10 +4,13 @@ using System.Collections.Generic; using System.IO; using System.Linq; +using System.Text.RegularExpressions; using NUnit.Framework; using UnityEditor; using UnityEngine; using UnityEngine.InputSystem; +using UnityEngine.InputSystem.Utilities; +using UnityEngine.TestTools; #if UNITY_EDITOR using UnityEngine.InputSystem.Editor; @@ -20,10 +23,11 @@ internal partial class CoreTests string m_TemplateAssetPath; #if UNITY_EDITOR - const int initialActionCount = 2; - const int initialMapCount = 1; + const int initialTotalActionCount = 12; + const int initialMapCount = 2; + const int initialFirstActionMapCount = 2; #else - const int initialActionCount = 19; + const int initialTotalActionCount = 19; const int initialMapCount = 2; #endif @@ -41,6 +45,8 @@ public override void Setup() var testAsset = ScriptableObject.CreateInstance(); AssetDatabase.CreateAsset(testAsset, TestAssetPath); + var defaultUIMapTemplate = ProjectWideActionsAsset.GetDefaultUIActionMap(); + // Create a template `InputActionAsset` containing some test actions. // This will then be used to populate the initially empty `TestActionsAsset` when it is first acessed. var templateActions = ScriptableObject.CreateInstance(); @@ -49,6 +55,9 @@ public override void Setup() map.AddAction("InitialActionOne"); map.AddAction("InitialActionTwo"); + // Add the default UI map to the template + templateActions.AddActionMap(defaultUIMapTemplate); + m_TemplateAssetPath = Path.Combine(Environment.CurrentDirectory, "Assets/ProjectWideActionsTemplate.inputactions"); File.WriteAllText(m_TemplateAssetPath, templateActions.ToJson()); @@ -82,7 +91,7 @@ public void ProjectWideActionsAsset_TemplateAssetIsInstalledOnFirstUse() Assert.That(asset, Is.Not.Null); Assert.That(asset.actionMaps.Count, Is.EqualTo(initialMapCount)); - Assert.That(asset.actionMaps[0].actions.Count, Is.EqualTo(initialActionCount)); + Assert.That(asset.actionMaps[0].actions.Count, Is.EqualTo(initialFirstActionMapCount)); Assert.That(asset.actionMaps[0].actions[0].name, Is.EqualTo("InitialActionOne")); } @@ -94,7 +103,7 @@ public void ProjectWideActionsAsset_CanModifySaveAndLoadAsset() Assert.That(asset, Is.Not.Null); Assert.That(asset.actionMaps.Count, Is.EqualTo(initialMapCount)); - Assert.That(asset.actionMaps[0].actions.Count, Is.EqualTo(initialActionCount)); + Assert.That(asset.actionMaps[0].actions.Count, Is.EqualTo(initialFirstActionMapCount)); Assert.That(asset.actionMaps[0].actions[0].name, Is.EqualTo("InitialActionOne")); asset.Disable(); // Cannot modify active actions @@ -107,7 +116,7 @@ public void ProjectWideActionsAsset_CanModifySaveAndLoadAsset() asset.actionMaps[0].actions[0].Rename("FirstAction"); // Add another map - asset.AddActionMap("ActionMapTwo").AddAction("AnotherAction"); + asset.AddActionMap("ActionMapThree").AddAction("AnotherAction"); // Save AssetDatabase.SaveAssets(); @@ -117,12 +126,44 @@ public void ProjectWideActionsAsset_CanModifySaveAndLoadAsset() Assert.That(asset, Is.Not.Null); Assert.That(asset.actionMaps.Count, Is.EqualTo(initialMapCount + 1)); - Assert.That(asset.actionMaps[0].actions.Count, Is.EqualTo(initialActionCount + 2)); - Assert.That(asset.actionMaps[1].actions.Count, Is.EqualTo(1)); + Assert.That(asset.actionMaps[0].actions.Count, Is.EqualTo(initialFirstActionMapCount + 2)); + Assert.That(asset.actionMaps[1].actions.Count, Is.EqualTo(10)); Assert.That(asset.actionMaps[0].actions[0].name, Is.EqualTo("FirstAction")); - Assert.That(asset.actionMaps[1].actions[0].name, Is.EqualTo("AnotherAction")); + Assert.That(asset.actionMaps[2].actions[0].name, Is.EqualTo("AnotherAction")); } + #if UNITY_2023_2_OR_NEWER + [Test] + [Category(TestCategory)] + public void ProjectWideActions_ShowsErrorWhenUIActionMapHasNameChanges() // This test is only relevant for the InputForUI module + { + var asset = ProjectWideActionsAsset.GetOrCreate(); + var indexOf = asset.m_ActionMaps.IndexOf(x => x.name == "UI"); + var uiMap = asset.m_ActionMaps[indexOf]; + + // Change the name of the UI action map + uiMap.m_Name = "UI2"; + + ProjectWideActionsAsset.CheckForDefaultUIActionMapChanges(); + + LogAssert.Expect(LogType.Warning, new Regex("The action map named 'UI' does not exist")); + + // Change the name of some UI map back to default and change the name of the actions + uiMap.m_Name = "UI"; + var defaultActionName0 = uiMap.m_Actions[0].m_Name; + var defaultActionName1 = uiMap.m_Actions[1].m_Name; + + uiMap.m_Actions[0].Rename("Navigation"); + uiMap.m_Actions[1].Rename("Show"); + + ProjectWideActionsAsset.CheckForDefaultUIActionMapChanges(); + + LogAssert.Expect(LogType.Warning, new Regex($"The UI action \"{defaultActionName0}\" name has been modified")); + LogAssert.Expect(LogType.Warning, new Regex($"The UI action \"{defaultActionName1}\" name has been modified")); + } + + #endif + #endif [Test] @@ -141,7 +182,7 @@ public void ProjectWideActions_ContainsTemplateActions() Assert.That(InputSystem.actions.actionMaps.Count, Is.EqualTo(initialMapCount)); #if UNITY_EDITOR - Assert.That(InputSystem.actions.actionMaps[0].actions.Count, Is.EqualTo(initialActionCount)); + Assert.That(InputSystem.actions.actionMaps[0].actions.Count, Is.EqualTo(initialFirstActionMapCount)); Assert.That(InputSystem.actions.actionMaps[0].actions[0].name, Is.EqualTo("InitialActionOne")); #else Assert.That(InputSystem.actions.actionMaps[0].actions.Count, Is.EqualTo(9)); @@ -154,14 +195,14 @@ public void ProjectWideActions_ContainsTemplateActions() public void ProjectWideActions_AppearInEnabledActions() { var enabledActions = InputSystem.ListEnabledActions(); - Assert.That(enabledActions, Has.Count.EqualTo(initialActionCount)); + Assert.That(enabledActions, Has.Count.EqualTo(initialTotalActionCount)); // Add more actions also work var action = new InputAction(name: "standaloneAction"); action.Enable(); enabledActions = InputSystem.ListEnabledActions(); - Assert.That(enabledActions, Has.Count.EqualTo(initialActionCount + 1)); + Assert.That(enabledActions, Has.Count.EqualTo(initialTotalActionCount + 1)); Assert.That(enabledActions, Has.Exactly(1).SameAs(action)); // Disabling works @@ -179,7 +220,7 @@ public void ProjectWideActions_CanReplaceExistingActions() Assert.That(InputSystem.actions, Is.Not.Null); Assert.That(InputSystem.actions.enabled, Is.True); var enabledActions = InputSystem.ListEnabledActions(); - Assert.That(enabledActions, Has.Count.EqualTo(initialActionCount)); + Assert.That(enabledActions, Has.Count.EqualTo(initialTotalActionCount)); // Build new asset var asset = ScriptableObject.CreateInstance(); From d296ac2daf68d5fa0deeeeff9ef12fdcf49132a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Freire?= Date: Mon, 8 Jan 2024 14:42:24 +0100 Subject: [PATCH 09/10] Guard code available on 2023.2+ --- .../Editor/ProjectWideActions/ProjectWideActionsAsset.cs | 4 +++- .../Editor/UITKAssetEditor/InputActionsEditorWindowUtils.cs | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/Packages/com.unity.inputsystem/InputSystem/Editor/ProjectWideActions/ProjectWideActionsAsset.cs b/Packages/com.unity.inputsystem/InputSystem/Editor/ProjectWideActions/ProjectWideActionsAsset.cs index c70fd2b9b6..7d37c69333 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Editor/ProjectWideActions/ProjectWideActionsAsset.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Editor/ProjectWideActions/ProjectWideActionsAsset.cs @@ -114,6 +114,7 @@ private static void CreateInputActionReferences(InputActionAsset asset) } } + #if UNITY_2023_2_OR_NEWER /// /// Checks if the default UI action map has been modified or removed, to let the user know if their changes will /// break the UI input at runtime, when using the UI Toolkit. @@ -141,12 +142,13 @@ internal static void CheckForDefaultUIActionMapChanges() { Debug.LogWarning($"The UI action \"{action.name}\" name has been modified.\r\n " + $"This will break the UI input at runtime. Please make sure the action name with \"{action.name}\" exists."); - return; } } } } + #endif + /// /// Updates the input action references in the asset by updating names, removing dangling references /// and adding new ones. diff --git a/Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorWindowUtils.cs b/Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorWindowUtils.cs index 8376d4f0e9..c22c899afc 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorWindowUtils.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorWindowUtils.cs @@ -18,7 +18,9 @@ public static void SaveAsset(SerializedObject serializedAsset) // For project-wide actions asset save works differently. The asset is in YAML format, not JSON. if (asset.name == ProjectWideActionsAsset.kAssetName) { +#if UNITY_2023_2_OR_NEWER ProjectWideActionsAsset.CheckForDefaultUIActionMapChanges(); +#endif ProjectWideActionsAsset.UpdateInputActionReferences(); AssetDatabase.SaveAssets(); return; From a79a613c593362842d4bce686b4d7a3536a86c76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Freire?= Date: Wed, 10 Jan 2024 14:22:45 +0100 Subject: [PATCH 10/10] Update based on review --- Assets/Tests/InputSystem/CoreTests_ProjectWideActions.cs | 4 ++-- Packages/com.unity.inputsystem/CHANGELOG.md | 2 +- .../Editor/ProjectWideActions/ProjectWideActionsAsset.cs | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Assets/Tests/InputSystem/CoreTests_ProjectWideActions.cs b/Assets/Tests/InputSystem/CoreTests_ProjectWideActions.cs index 14f5051732..47a2795dc3 100644 --- a/Assets/Tests/InputSystem/CoreTests_ProjectWideActions.cs +++ b/Assets/Tests/InputSystem/CoreTests_ProjectWideActions.cs @@ -158,8 +158,8 @@ public void ProjectWideActionsAsset_CanModifySaveAndLoadAsset() ProjectWideActionsAsset.CheckForDefaultUIActionMapChanges(); - LogAssert.Expect(LogType.Warning, new Regex($"The UI action \"{defaultActionName0}\" name has been modified")); - LogAssert.Expect(LogType.Warning, new Regex($"The UI action \"{defaultActionName1}\" name has been modified")); + LogAssert.Expect(LogType.Warning, new Regex($"The UI action '{defaultActionName0}' name has been modified")); + LogAssert.Expect(LogType.Warning, new Regex($"The UI action '{defaultActionName1}' name has been modified")); } #endif diff --git a/Packages/com.unity.inputsystem/CHANGELOG.md b/Packages/com.unity.inputsystem/CHANGELOG.md index 17562c0239..3f9377e1f3 100644 --- a/Packages/com.unity.inputsystem/CHANGELOG.md +++ b/Packages/com.unity.inputsystem/CHANGELOG.md @@ -11,7 +11,7 @@ however, it has to be formatted properly to pass verification tests. ## [Unreleased] ### Changed -- UI toolkit now uses the "UI" action map of project-wide actions as their default input actions. Removing bindings or renaming actions will break UI input for UI toolkit. +- From 2023.2 forward: UI toolkit now uses the "UI" action map of project-wide actions as their default input actions. Previously, the actions were hardcoded and were based on `DefaultInputActions` asset which didn't allow user changes. Also, removing bindings or renaming the 'UI' action map of project wide actions will break UI input for UI toolkit. ### Fixed - Fixed missing confirmation popup when deleting a control scheme. diff --git a/Packages/com.unity.inputsystem/InputSystem/Editor/ProjectWideActions/ProjectWideActionsAsset.cs b/Packages/com.unity.inputsystem/InputSystem/Editor/ProjectWideActions/ProjectWideActionsAsset.cs index 7d37c69333..e20bedefd4 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Editor/ProjectWideActions/ProjectWideActionsAsset.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Editor/ProjectWideActions/ProjectWideActionsAsset.cs @@ -140,8 +140,8 @@ internal static void CheckForDefaultUIActionMapChanges() // "UI" actions have been modified. if (uiMap.FindAction(action.name) == null) { - Debug.LogWarning($"The UI action \"{action.name}\" name has been modified.\r\n " + - $"This will break the UI input at runtime. Please make sure the action name with \"{action.name}\" exists."); + Debug.LogWarning($"The UI action '{action.name}' name has been modified.\r\n" + + $"This will break the UI input at runtime. Please make sure the action name with '{action.name}' exists."); } } }