From 10f19c9c8f83278dd9ac20a438f4d3f78672774d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Akan=20Sidenvall?= Date: Mon, 29 Sep 2025 21:54:13 +0200 Subject: [PATCH 01/27] FIX: InputActionReference returning incorrect action when action has been called and cached, then Set and then calling action again. --- Assets/Tests/InputSystem/CoreTests_Actions.cs | 18 ++++++++++++++++++ .../Actions/InputActionReference.cs | 1 + 2 files changed, 19 insertions(+) diff --git a/Assets/Tests/InputSystem/CoreTests_Actions.cs b/Assets/Tests/InputSystem/CoreTests_Actions.cs index 0a4d551fdd..f459b6c80a 100644 --- a/Assets/Tests/InputSystem/CoreTests_Actions.cs +++ b/Assets/Tests/InputSystem/CoreTests_Actions.cs @@ -11576,6 +11576,24 @@ public void Actions_CanResolveActionReference_EvenAfterActionHasBeenRenamed() Assert.That(referencedAction, Is.SameAs(action)); } + [Test(Description = "https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1584")] + [Category("Actions")] + public void Actions_CanResolveActionReferenceAndThenSetItToAnotherAction() + { + var map = new InputActionMap("map"); + var action1 = map.AddAction("action1"); + var action2 = map.AddAction("action2"); + var asset = ScriptableObject.CreateInstance(); + asset.AddActionMap(map); + + var reference = ScriptableObject.CreateInstance(); + reference.Set(asset, "map", "action2"); + Assert.That(reference.action, Is.SameAs(action2)); // Redundant, but important for test case + + reference.Set(asset, "map", "action1"); + Assert.That(reference.action, Is.SameAs(action1)); + } + [Test] [Category("Actions")] public void Actions_CanDisableAllEnabledActionsInOneGo() diff --git a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs index 5b56e431a9..12f0d25e74 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs @@ -136,6 +136,7 @@ private void SetInternal(InputActionAsset asset, InputAction action) m_Asset = asset; m_ActionId = action.id.ToString(); name = GetDisplayName(action); + m_Action = action; ////REVIEW: should this dirty the asset if IDs had not been generated yet? } From f38dd0475690e07dc0353f3c85ad3f0594f97842 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Akan=20Sidenvall?= Date: Mon, 29 Sep 2025 21:58:54 +0200 Subject: [PATCH 02/27] Updated change log. --- Packages/com.unity.inputsystem/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/Packages/com.unity.inputsystem/CHANGELOG.md b/Packages/com.unity.inputsystem/CHANGELOG.md index ce6fe06286..3a6ce07739 100644 --- a/Packages/com.unity.inputsystem/CHANGELOG.md +++ b/Packages/com.unity.inputsystem/CHANGELOG.md @@ -28,6 +28,7 @@ however, it has to be formatted properly to pass verification tests. - Fixed the compilation warnings when used with Unity 6.4 (ISX-2349). - Fixed an issue where `InputSystemUIInputModule.localMultiPlayerRoot` could not be set to `null` when using `MultiplayerEventSystem`. [ISXB-1610](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1610) - Fixed an issue in `Keyboard` where the sub-script operator would return a `null` key control for the deprecated key `Key.IMESelected`. Now, an aliased `KeyControl`mapping to the IMESelected bit is returned for compability reasons. It is still strongly advised to not rely on this key since `IMESelected` bit isn't strictly a key and will be removed from the `Key` enumeration type in a future major revision. [ISXB-1541](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1541). +- Fixed an issue in `InputActionReference` where `Set` was not updating the cached action leading to incorrect evaluation of `InputActionReference.action` after first being cached, then set, then read again. This could lead to reference not being reset if set programmatically during play-mode. [ISXB-1584](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1584). ## [1.14.2] - 2025-08-05 From 79c430d70b2c9be82e30bdc89ed00b97b51b29e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Akan=20Sidenvall?= Date: Tue, 30 Sep 2025 15:28:40 +0200 Subject: [PATCH 03/27] Improvements to testing, refactoring of tests, bug fixes to InputActionReference, simplification of InputActionImporter. --- Assets/Tests/InputSystem/CoreTests_Actions.cs | 55 -------- .../CoreTests_Actions_Reference.cs | 126 ++++++++++++++++++ .../CoreTests_Actions_Reference.cs.meta | 3 + .../Actions/InputActionReference.cs | 45 ++++--- .../Actions/InputActionSetupExtensions.cs | 2 +- .../AssetImporter/InputActionImporter.cs | 21 +-- .../InputActionReferencePropertyDrawer.cs | 46 ++++--- 7 files changed, 194 insertions(+), 104 deletions(-) create mode 100644 Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs create mode 100644 Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs.meta diff --git a/Assets/Tests/InputSystem/CoreTests_Actions.cs b/Assets/Tests/InputSystem/CoreTests_Actions.cs index f459b6c80a..85827972c8 100644 --- a/Assets/Tests/InputSystem/CoreTests_Actions.cs +++ b/Assets/Tests/InputSystem/CoreTests_Actions.cs @@ -11539,61 +11539,6 @@ public void Actions_CanCloneActionMaps() Assert.That(clone.actions[1].bindings[0].path, Is.EqualTo("/gamepad/rightStick")); } - [Test] - [Category("Actions")] - public void Actions_CanResolveActionReference() - { - var map = new InputActionMap("map"); - map.AddAction("action1"); - var action2 = map.AddAction("action2"); - var asset = ScriptableObject.CreateInstance(); - asset.AddActionMap(map); - - var reference = ScriptableObject.CreateInstance(); - reference.Set(asset, "map", "action2"); - - var referencedAction = reference.action; - - Assert.That(referencedAction, Is.SameAs(action2)); - } - - [Test] - [Category("Actions")] - public void Actions_CanResolveActionReference_EvenAfterActionHasBeenRenamed() - { - var map = new InputActionMap("map"); - var action = map.AddAction("oldName"); - var asset = ScriptableObject.CreateInstance(); - asset.AddActionMap(map); - - var reference = ScriptableObject.CreateInstance(); - reference.Set(asset, "map", "oldName"); - - action.Rename("newName"); - - var referencedAction = reference.action; - - Assert.That(referencedAction, Is.SameAs(action)); - } - - [Test(Description = "https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1584")] - [Category("Actions")] - public void Actions_CanResolveActionReferenceAndThenSetItToAnotherAction() - { - var map = new InputActionMap("map"); - var action1 = map.AddAction("action1"); - var action2 = map.AddAction("action2"); - var asset = ScriptableObject.CreateInstance(); - asset.AddActionMap(map); - - var reference = ScriptableObject.CreateInstance(); - reference.Set(asset, "map", "action2"); - Assert.That(reference.action, Is.SameAs(action2)); // Redundant, but important for test case - - reference.Set(asset, "map", "action1"); - Assert.That(reference.action, Is.SameAs(action1)); - } - [Test] [Category("Actions")] public void Actions_CanDisableAllEnabledActionsInOneGo() diff --git a/Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs b/Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs new file mode 100644 index 0000000000..55775945ef --- /dev/null +++ b/Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs @@ -0,0 +1,126 @@ +using NUnit.Framework; +using UnityEngine; +using UnityEngine.InputSystem; + +partial class CoreTests +{ + private static InputActionAsset CreateAssetWithTwoActions() + { + var map1 = new InputActionMap("map1"); + map1.AddAction("action1"); + map1.AddAction("action2"); + var asset = ScriptableObject.CreateInstance(); + asset.AddActionMap(map1); + return asset; + } + + private static InputActionAsset CreateAssetWithSingleAction() + { + var map2 = new InputActionMap("map2"); + map2.AddAction("action3"); + var asset = ScriptableObject.CreateInstance(); + asset.AddActionMap(map2); + return asset; + } + + [Test] + public void Actions_Reference_AssetAndActionsReturnsNull_IfNotSet() + { + var reference = ScriptableObject.CreateInstance(); + Assert.That(reference.asset, Is.Null); + Assert.That(reference.action, Is.Null); + Assert.That(reference.ToDisplayName(), Is.Null); + } + + [Test] + public void Actions_Reference_SetNull() + { + var reference = ScriptableObject.CreateInstance(); + reference.Set(null); + } + + [Test] + [Category("Actions")] + public void Actions_Reference_CanResolveAction() + { + var asset = CreateAssetWithTwoActions(); + var reference = ScriptableObject.CreateInstance(); + + reference.Set(asset, "map1", "action2"); + + Assert.That(reference.action, Is.SameAs(asset.FindAction("map1/action2"))); + } + + [Test] + [Category("Actions")] + public void Actions_Reference_CanResolveAction_EvenAfterActionHasBeenRenamed() + { + var map = new InputActionMap("map"); + var action = map.AddAction("oldName"); + var asset = ScriptableObject.CreateInstance(); + asset.AddActionMap(map); + + var reference = ScriptableObject.CreateInstance(); + reference.Set(asset, "map", "oldName"); + + action.Rename("newName"); + + var referencedAction = reference.action; + + Assert.That(referencedAction, Is.SameAs(action)); + } + + [Test(Description = "https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1584")] + [Category("Actions")] + public void Actions_Reference_CanResolveActionAfterReassignment() + { + var asset1 = CreateAssetWithTwoActions(); + var asset2 = CreateAssetWithSingleAction(); + + var reference = ScriptableObject.CreateInstance(); + reference.Set(asset1, "map1", "action1"); + Assert.That(reference.action, Is.Not.Null); + Assert.That(reference.action, Is.SameAs(asset1.FindAction("map1/action1"))); // Redundant, but important for test case + Assert.That(reference.asset, Is.SameAs(asset1)); + Assert.That(reference.name, Is.EqualTo("map1/action1")); + Assert.That(reference.ToDisplayName(), Is.EqualTo("map1/action1")); + Assert.That(reference.ToString(), Is.EqualTo(":map1/action1")); + + reference.Set(asset2, "map2", "action3"); + Assert.That(reference.action, Is.Not.Null); + Assert.That(reference.action, Is.SameAs(asset2.FindAction("map2/action3"))); + Assert.That(reference.asset, Is.SameAs(asset2)); + Assert.That(reference.name, Is.EqualTo("map2/action3")); + Assert.That(reference.ToDisplayName(), Is.EqualTo("map2/action3")); + Assert.That(reference.ToString(), Is.EqualTo(":map2/action3")); + } + + [Test] + public void Actions_Reference_NameShouldReflectReferencedAction() + { + var map = new InputActionMap("map"); + var action1 = map.AddAction("action1"); + var asset = ScriptableObject.CreateInstance(); + asset.AddActionMap(map); + + var reference = ScriptableObject.CreateInstance(); + reference.Set(asset, "map", "action1"); + + Assert.That(reference.action, Is.SameAs(action1)); + Assert.That(reference.asset, Is.SameAs(asset)); + Assert.That(reference.name, Is.EqualTo("map/action1")); + Assert.That(reference.ToDisplayName(), Is.EqualTo("map/action1")); + Assert.That(reference.ToString(), Is.EqualTo(":map/action1")); + + // Delete the referenced action + + asset.RemoveAction("map/action1"); + + // TODO reference need to react to this + Assert.That(reference.action, Is.SameAs(action1)); + Assert.That(reference.asset, Is.SameAs(asset)); + Assert.That(reference.name, Is.EqualTo("map/action1")); // Unexpected when no longer existing + Assert.That(reference.ToDisplayName(), Is.EqualTo("map/action1")); // Unexpected when no longer existing + Assert.That(reference.ToString(), Is.EqualTo(":map/action1")); // Unexpected when no longer existing + } +} diff --git a/Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs.meta b/Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs.meta new file mode 100644 index 0000000000..e5be0ba2f8 --- /dev/null +++ b/Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs.meta @@ -0,0 +1,3 @@ +fileFormatVersion: 2 +guid: af3ddc97be234713880f2a5a2b348259 +timeCreated: 1759228337 \ No newline at end of file diff --git a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs index 12f0d25e74..7ef611ca85 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs @@ -40,11 +40,10 @@ public class InputActionReference : ScriptableObject /// is not initialized or if the asset has been deleted. /// /// InputActionAsset of the referenced action. - public InputActionAsset asset => m_Asset; + public InputActionAsset asset => m_Action?.m_ActionMap != null ? m_Action.m_ActionMap.asset : m_Asset; /// - /// The action that the reference resolves to. Null if the action - /// cannot be found. + /// The action that the reference resolves to. Null if the action cannot be found. /// /// The action that reference points to. /// @@ -54,15 +53,11 @@ public InputAction action { get { - if (m_Action == null) - { - if (m_Asset == null) - return null; - - m_Action = m_Asset.FindAction(new Guid(m_ActionId)); - } - - return m_Action; + if (m_Action != null) + return m_Action; + if (!m_Asset) + return null; + return (m_Action = m_Asset.FindAction(new Guid(m_ActionId)));; } } @@ -76,10 +71,12 @@ public InputAction action /// that is itself contained in an . public void Set(InputAction action) { + // TODO Throw if attempting to set reference on an input action reference that resides in an asset. + if (action == null) { - m_Asset = default; - m_ActionId = default; + m_Asset = null; + m_ActionId = null; return; } @@ -133,6 +130,8 @@ private void SetInternal(InputActionAsset asset, InputAction action) throw new ArgumentException( $"Action '{action}' is not contained in asset '{asset}'", nameof(action)); + // If we are setting the reference in edit-mode, we want the state to be reflected in the serialized + // object and hence assign serialized fields. This is a destructive operation. m_Asset = asset; m_ActionId = action.id.ToString(); name = GetDisplayName(action); @@ -149,8 +148,8 @@ public override string ToString() { try { - var action = this.action; - return $"{m_Asset.name}:{action.actionMap.name}/{action.name}"; + var value = action; + return $"{m_Asset.name}:{value.actionMap.name}/{value.name}"; } catch { @@ -204,17 +203,21 @@ public static InputActionReference Create(InputAction action) /// Clears the cached field for all current objects. /// /// - /// After calling this, the next call to will retrieve a new reference from the existing just as if - /// using it for the first time. The serialized and fields are not touched and will continue to hold their current values. + /// After calling this, the next call to will retrieve a new + /// reference from the existing just as if using it for the first time. + /// The serialized and fields are not touched and will continue + /// to hold their current values. /// - /// This method is used to clear the Action references when exiting PlayMode since those objects are no longer valid. + /// This method is used to clear the Action references when exiting PlayMode since those objects are no + /// longer valid. /// internal static void ResetCachedAction() { var allActionRefs = Resources.FindObjectsOfTypeAll(typeof(InputActionReference)); - foreach (InputActionReference obj in allActionRefs) + foreach (var obj in allActionRefs) { - obj.m_Action = null; + var reference = (InputActionReference)obj; + reference.m_Action = null; } } diff --git a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionSetupExtensions.cs b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionSetupExtensions.cs index 6a1aa3e7a5..a1c9a9ae52 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionSetupExtensions.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionSetupExtensions.cs @@ -240,7 +240,7 @@ public static void RemoveAction(this InputAction action) action.m_SingletonActionBindings = bindingsForAction; // Remove bindings to action from map. - var newActionMapBindingCount = actionMap.m_Bindings.Length - bindingsForAction.Length; + var newActionMapBindingCount = actionMap.m_Bindings != null ? actionMap.m_Bindings.Length - bindingsForAction.Length : 0; if (newActionMapBindingCount == 0) { actionMap.m_Bindings = null; diff --git a/Packages/com.unity.inputsystem/InputSystem/Editor/AssetImporter/InputActionImporter.cs b/Packages/com.unity.inputsystem/InputSystem/Editor/AssetImporter/InputActionImporter.cs index ddc2ef6430..64ba9e08af 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Editor/AssetImporter/InputActionImporter.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Editor/AssetImporter/InputActionImporter.cs @@ -285,8 +285,15 @@ internal static IEnumerable LoadInputActionReferencesFromA .Cast(); } - // Get all InputActionReferences from assets in the project. By default it only gets the assets in the "Assets" folder. - internal static IEnumerable LoadInputActionReferencesFromAssetDatabase(string[] foldersPath = null, bool skipProjectWide = false) + /// + /// Gets all instances available in assets in the project. + /// By default, it only gets the assets located in the "Assets" folder. + /// + /// + /// + /// + internal static IEnumerable LoadInputActionReferencesFromAssetDatabase( + string[] foldersPath = null, bool skipProjectWide = false) { string[] searchFolders = null; // If folderPath is null, search in "Assets" folder. @@ -307,15 +314,13 @@ internal static IEnumerable LoadInputActionReferencesFromA foreach (var guid in inputActionReferenceGUIDs) { var assetPath = AssetDatabase.GUIDToAssetPath(guid); - var assetInputActionReferenceList = LoadInputActionReferencesFromAsset(assetPath).ToList(); - - if (skipProjectWide && assetInputActionReferenceList.Count() > 0) + foreach (var assetInputActionReference in LoadInputActionReferencesFromAsset(assetPath)) { - if (assetInputActionReferenceList[0].m_Asset == InputSystem.actions) + if (skipProjectWide && assetInputActionReference.m_Asset == InputSystem.actions) continue; - } - inputActionReferencesList.AddRange(assetInputActionReferenceList); + inputActionReferencesList.Add(assetInputActionReference); + } } return inputActionReferencesList; } diff --git a/Packages/com.unity.inputsystem/InputSystem/Editor/PropertyDrawers/InputActionReferencePropertyDrawer.cs b/Packages/com.unity.inputsystem/InputSystem/Editor/PropertyDrawers/InputActionReferencePropertyDrawer.cs index 398c99b14b..a6d00a3720 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Editor/PropertyDrawers/InputActionReferencePropertyDrawer.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Editor/PropertyDrawers/InputActionReferencePropertyDrawer.cs @@ -3,6 +3,7 @@ #if UNITY_EDITOR && UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS using UnityEditor; using UnityEditor.Search; +using ObjectField = UnityEditor.Search.ObjectField; namespace UnityEngine.InputSystem.Editor { @@ -18,31 +19,38 @@ internal sealed class InputActionReferencePropertyDrawer : PropertyDrawer InputActionReferenceSearchProviders.CreateInputActionReferenceSearchProviderForProjectWideActions(), }, string.Empty, SearchConstants.PickerSearchFlags); - public override void OnGUI(Rect position, SerializedProperty property, GUIContent label) { - // Sets the property to null if the action is not found in the asset. - ValidatePropertyWithDanglingInputActionReferences(property); + EditorGUI.BeginProperty(position, label, property); + EditorGUI.BeginChangeCheck(); - ObjectField.DoObjectField(position, property, typeof(InputActionReference), label, - m_Context, SearchConstants.PickerViewFlags); - } + // Reassign null if property is a dangling project-wide input action + // var explicitlyReassigned = false; + var current = property.objectReferenceValue; + // if (reference != null && reference.asset == InputSystem.actions) + // { + // var action = reference?.asset?.FindAction(reference.action.id); + // if (action is null) + // property.objectReferenceValue = null; // TODO This cannot be right?! + // } - static void ValidatePropertyWithDanglingInputActionReferences(SerializedProperty property) - { - if (property?.objectReferenceValue is InputActionReference reference) + // Pick an InputActionReference using custom picker. We need to use this overload taking an object + // in order to be in control of the actual assignment to the serialized property. + // This is important since we should NEVER assign a direct reference to a ScriptableObject residing + // in an asset. Instead, we should instantiate a new ScriptableObject instance to prevent destructive + // operations that would mutate the asset. + var candidate = ObjectField.DoObjectField(position, current, typeof(InputActionReference), + label, m_Context, SearchConstants.PickerViewFlags); + + // Only assign the value if it was actually changed by the user. + if (EditorGUI.EndChangeCheck() && !Equals(candidate, current)) { - // Check only if the reference is a project-wide action. - if (reference?.asset == InputSystem.actions) - { - var action = reference?.asset?.FindAction(reference.action.id); - if (action is null) - { - property.objectReferenceValue = null; - property.serializedObject.ApplyModifiedProperties(); - } - } + var reference = candidate as InputActionReference; + property.objectReferenceValue = reference ? + InputActionReference.Create(reference.action) : null; } + + EditorGUI.EndProperty(); } } } From 27b0311532a037bc46cc18af90c14c32d66b96fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Akan=20Sidenvall?= Date: Tue, 30 Sep 2025 15:29:42 +0200 Subject: [PATCH 04/27] Updated changelog --- Packages/com.unity.inputsystem/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/Packages/com.unity.inputsystem/CHANGELOG.md b/Packages/com.unity.inputsystem/CHANGELOG.md index 3a6ce07739..e26db7a9a8 100644 --- a/Packages/com.unity.inputsystem/CHANGELOG.md +++ b/Packages/com.unity.inputsystem/CHANGELOG.md @@ -29,6 +29,7 @@ however, it has to be formatted properly to pass verification tests. - Fixed an issue where `InputSystemUIInputModule.localMultiPlayerRoot` could not be set to `null` when using `MultiplayerEventSystem`. [ISXB-1610](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1610) - Fixed an issue in `Keyboard` where the sub-script operator would return a `null` key control for the deprecated key `Key.IMESelected`. Now, an aliased `KeyControl`mapping to the IMESelected bit is returned for compability reasons. It is still strongly advised to not rely on this key since `IMESelected` bit isn't strictly a key and will be removed from the `Key` enumeration type in a future major revision. [ISXB-1541](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1541). - Fixed an issue in `InputActionReference` where `Set` was not updating the cached action leading to incorrect evaluation of `InputActionReference.action` after first being cached, then set, then read again. This could lead to reference not being reset if set programmatically during play-mode. [ISXB-1584](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1584). +- Fixed an issue where `InputActionSetupExtensions.Remove` will result in `NullReferenceException` if the action has no bindings. (ISXB-1688). ## [1.14.2] - 2025-08-05 From 7c5305a79357ad4180d4e5bf4198901204776035 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Akan=20Sidenvall?= Date: Tue, 30 Sep 2025 22:06:15 +0200 Subject: [PATCH 05/27] FIX: RemoveActionMap does not remove contained actions. Additional bug fixing of InputActionReference, added more tests. --- .../CoreTests_Actions_Reference.cs | 37 +++++++++++++++---- .../Actions/InputActionReference.cs | 24 ++++++++++-- .../Actions/InputActionSetupExtensions.cs | 7 ++++ 3 files changed, 57 insertions(+), 11 deletions(-) diff --git a/Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs b/Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs index 55775945ef..348ad634d8 100644 --- a/Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs +++ b/Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs @@ -1,6 +1,8 @@ +using System; using NUnit.Framework; using UnityEngine; using UnityEngine.InputSystem; +using Object = System.Object; partial class CoreTests { @@ -95,8 +97,10 @@ public void Actions_Reference_CanResolveActionAfterReassignment() Assert.That(reference.ToString(), Is.EqualTo(":map2/action3")); } - [Test] - public void Actions_Reference_NameShouldReflectReferencedAction() + [TestCase(typeof(InputAction))] + [TestCase(typeof(InputActionMap))] + [TestCase(typeof(InputActionAsset))] + public void Actions_Reference_NameShouldReflectReferencedAction(Type typeToDelete) { var map = new InputActionMap("map"); var action1 = map.AddAction("action1"); @@ -112,15 +116,34 @@ public void Actions_Reference_NameShouldReflectReferencedAction() Assert.That(reference.ToDisplayName(), Is.EqualTo("map/action1")); Assert.That(reference.ToString(), Is.EqualTo(":map/action1")); - // Delete the referenced action - - asset.RemoveAction("map/action1"); + // Delete the referenced action directly or indirectly + if (typeToDelete == typeof(InputAction)) + asset.RemoveAction("map/action1"); + else if (typeToDelete == typeof(InputActionMap)) + asset.RemoveActionMap("map"); + else if (typeToDelete == typeof(InputActionAsset)) + UnityEngine.Object.DestroyImmediate(asset); // TODO reference need to react to this - Assert.That(reference.action, Is.SameAs(action1)); + Assert.That(reference.action, Is.Null); Assert.That(reference.asset, Is.SameAs(asset)); Assert.That(reference.name, Is.EqualTo("map/action1")); // Unexpected when no longer existing Assert.That(reference.ToDisplayName(), Is.EqualTo("map/action1")); // Unexpected when no longer existing - Assert.That(reference.ToString(), Is.EqualTo(":map/action1")); // Unexpected when no longer existing + //Assert.That(reference.ToString(), Is.EqualTo(":" + new Guid(action1.m_Id))); // Unexpected when no longer existing } + + // List of bugs addressed on this branch: + // (FIXED) InputActionReference - Do not set m_Action when calling Set. + // (FIXED) InputActionReference.asset - Returns incorrect asset after being .Set if .action is called before Set. + // (FIXED) InputActionReference - Assigned with Set during play-mode corrupts InputActionAsset by mutating asset reference into pointing into potentially another asset and overwrites existign asset. + // (FIXED) InputActionReferencePropertyDrawer - Assigns direct reference into InputActionAsset to InputActionReference fields leading to corruption. + // InputActionReference - Contains stale reference and looks like a valid reference if the action is deleted. .action returns removed action. + // InputActionReference - Do not invalidate if action, action map or asset is deleted/destroyed. .action still returns the action. + // (FIXED) InputActionAsset.RemoveActionMap - Do not remove actions within the map and they keep a stale reference to the removed map. + // (FIXED) InputActionAsset.RemoveAction - Throws exception if action do not have any bindings. + + + // TODO Make a test where action map is deleted + // TODO Make a test where asset is deleted + // TODO Make an undo resolve test } diff --git a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs index 7ef611ca85..f3e901f52f 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs @@ -53,11 +53,12 @@ public InputAction action { get { - if (m_Action != null) + // Note that we check m_Action since it would be null if we haven't yet resolved the action. + // We also need to check that m_Action.actionMap isn't null since it would be null if the + // action was deleted. + if (m_Action != null && m_Action.actionMap != null && m_Asset) return m_Action; - if (!m_Asset) - return null; - return (m_Action = m_Asset.FindAction(new Guid(m_ActionId)));; + return (m_Action = ResolveAction()); } } @@ -236,5 +237,20 @@ public InputAction ToInputAction() { return action; } + + private InputAction ResolveAction() + { + return m_Asset == null ? null : m_Asset.FindAction(new Guid(m_ActionId)); + } + + // OnValidate() is stripped out from player builds (editor only). + /*private void OnValidate() + { + var resolvedAction = ResolveAction(); + if (resolvedAction == null) + { + Debug.Log("RESOLVED TO NULL"); + } + }*/ } } diff --git a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionSetupExtensions.cs b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionSetupExtensions.cs index a1c9a9ae52..e6e1b3b1f8 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionSetupExtensions.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionSetupExtensions.cs @@ -104,6 +104,13 @@ public static void RemoveActionMap(this InputActionAsset asset, InputActionMap m if (map.m_Asset != asset) return; + // First remove all actions from map (They become "singleton actions" or may get collected if not referenced). + if (map.m_Actions != null) + { + for (var i = map.m_Actions.Length - 1; i >= 0; --i) + RemoveAction(map.m_Actions[i]); + } + ArrayHelpers.Erase(ref asset.m_ActionMaps, map); map.m_Asset = null; asset.OnSetupChanged(); From b02a52efffbf1dd0d8815bc5362ea7478375c10f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Akan=20Sidenvall?= Date: Wed, 1 Oct 2025 09:38:06 +0200 Subject: [PATCH 06/27] More bug fixes and test cases for input action reference --- .../CoreTests_Actions_Reference.cs | 148 +++++++++++++----- .../Actions/InputActionReference.cs | 56 +++---- 2 files changed, 134 insertions(+), 70 deletions(-) diff --git a/Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs b/Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs index 348ad634d8..d4621c7901 100644 --- a/Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs +++ b/Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs @@ -6,6 +6,8 @@ partial class CoreTests { + #region Helpers + private static InputActionAsset CreateAssetWithTwoActions() { var map1 = new InputActionMap("map1"); @@ -25,68 +27,130 @@ private static InputActionAsset CreateAssetWithSingleAction() return asset; } - [Test] - public void Actions_Reference_AssetAndActionsReturnsNull_IfNotSet() + private void AssertDefaults(InputActionReference reference) { - var reference = ScriptableObject.CreateInstance(); Assert.That(reference.asset, Is.Null); Assert.That(reference.action, Is.Null); + Assert.That(reference.name, Is.EqualTo(string.Empty)); Assert.That(reference.ToDisplayName(), Is.Null); + Assert.That(reference.ToString(), Is.EqualTo($" ({typeof(InputActionReference).FullName})")); } + #endregion + [Test] - public void Actions_Reference_SetNull() + [Category("Actions")] + public void Actions_Reference_Defaults() { + AssertDefaults(ScriptableObject.CreateInstance()); + } + + [TestCase(false, "map1", "action1")] + [TestCase(true, null, "action1")] + [TestCase(true, "map1", null)] + [Category("Actions")] + public void Actions_Reference_SetByNameThrows_IfAnyArgumentIsNull(bool validAsset, string mapName, string actionName) + { + var asset = CreateAssetWithTwoActions(); var reference = ScriptableObject.CreateInstance(); - reference.Set(null); + Assert.Throws(() => reference.Set(validAsset ? asset : null, mapName, actionName)); } - [Test] + [TestCase("doesNotExist", "action1")] + [TestCase("map1", "doesNotExist")] [Category("Actions")] - public void Actions_Reference_CanResolveAction() + public void Actions_Reference_SetByNameThrows_IfNoMatchingMapOrActionExists(string mapName, string actionName) { var asset = CreateAssetWithTwoActions(); var reference = ScriptableObject.CreateInstance(); + Assert.Throws(() => reference.Set(asset, mapName, actionName)); + } - reference.Set(asset, "map1", "action2"); + [Test] + [Category("Actions")] + public void Actions_Reference_SetByReferenceThrows_IfActionDoNotBelongToAnAsset() + { + // Case 1: Action was never part of any asset + var action = new InputAction("action"); + var reference = ScriptableObject.CreateInstance(); + Assert.Throws(() => reference.Set(action)); - Assert.That(reference.action, Is.SameAs(asset.FindAction("map1/action2"))); + // Case 2: Action was part of an asset but then removed + var asset = CreateAssetWithTwoActions(); + var action1 = asset.FindAction("map1/action1"); + asset.RemoveAction("map1/action1"); + Assert.Throws(() => reference.Set(action1)); } [Test] [Category("Actions")] - public void Actions_Reference_CanResolveAction_EvenAfterActionHasBeenRenamed() + public void Actions_Reference_CreateReturnsReference_WhenCreatedFromValidAction() { - var map = new InputActionMap("map"); - var action = map.AddAction("oldName"); - var asset = ScriptableObject.CreateInstance(); - asset.AddActionMap(map); + var asset = CreateAssetWithTwoActions(); + var action = asset.FindAction("map1/action2"); + + var reference = InputActionReference.Create(action); + Assert.That(reference.action, Is.SameAs(action)); + } + + [Test] + [Category("Actions")] + public void Actions_Reference_CreateReturnsNullReferenceObject_IfActionIsNull() + { + var reference = InputActionReference.Create(null); + Assert.That(reference.action, Is.Null); + } + + [TestCase(false)] + [TestCase(true)] + [Category("Actions")] + public void Actions_Reference_CanResolveAction_WhenSet(bool setByReference) + { + var asset = CreateAssetWithTwoActions(); + var action = asset.FindAction("map1/action2"); var reference = ScriptableObject.CreateInstance(); - reference.Set(asset, "map", "oldName"); + if (setByReference) + reference.Set(action); + else + reference.Set(asset, "map1", "action2"); - action.Rename("newName"); + Assert.That(reference.action, Is.SameAs(action)); + } + + [Test] + [Category("Actions")] + public void Actions_Reference_CanResolveAction_AfterActionHasBeenRenamed() + { + var asset = CreateAssetWithSingleAction(); + var reference = ScriptableObject.CreateInstance(); + reference.Set(asset, "map2", "action3"); + var action = asset.FindAction("map2/action3"); - var referencedAction = reference.action; + action.Rename("newName"); + Assert.That(reference.action, Is.SameAs(action)); + } - Assert.That(referencedAction, Is.SameAs(action)); + [Test] + [Category("Actions")] + public void Actions_Reference_CanResolveToDefaultState_AfterActionHasBeenSetToNull() + { + var asset = CreateAssetWithTwoActions(); + var reference = InputActionReference.Create(asset.FindAction("map1/action1")); + reference.Set(null); + AssertDefaults(reference); } [Test(Description = "https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1584")] [Category("Actions")] - public void Actions_Reference_CanResolveActionAfterReassignment() + public void Actions_Reference_CanResolveActionAfterReassignmentToActionFromAnotherAsset() { var asset1 = CreateAssetWithTwoActions(); var asset2 = CreateAssetWithSingleAction(); var reference = ScriptableObject.CreateInstance(); reference.Set(asset1, "map1", "action1"); - Assert.That(reference.action, Is.Not.Null); - Assert.That(reference.action, Is.SameAs(asset1.FindAction("map1/action1"))); // Redundant, but important for test case - Assert.That(reference.asset, Is.SameAs(asset1)); - Assert.That(reference.name, Is.EqualTo("map1/action1")); - Assert.That(reference.ToDisplayName(), Is.EqualTo("map1/action1")); - Assert.That(reference.ToString(), Is.EqualTo(":map1/action1")); + Assert.That(reference.action, Is.Not.Null); // Looks redundant, but important for test case to resolve reference.Set(asset2, "map2", "action3"); Assert.That(reference.action, Is.Not.Null); @@ -100,35 +164,38 @@ public void Actions_Reference_CanResolveActionAfterReassignment() [TestCase(typeof(InputAction))] [TestCase(typeof(InputActionMap))] [TestCase(typeof(InputActionAsset))] - public void Actions_Reference_NameShouldReflectReferencedAction(Type typeToDelete) + public void Actions_Reference_ShouldReevaluateIfAssociatedEntityIsDeleted(Type typeToDelete) { - var map = new InputActionMap("map"); - var action1 = map.AddAction("action1"); - var asset = ScriptableObject.CreateInstance(); - asset.AddActionMap(map); + var asset = CreateAssetWithTwoActions(); + var action = asset.FindAction("map1/action1"); + + // var map = new InputActionMap("map"); + // var action1 = map.AddAction("action1"); + // var asset = ScriptableObject.CreateInstance(); + // asset.AddActionMap(map); var reference = ScriptableObject.CreateInstance(); - reference.Set(asset, "map", "action1"); + reference.Set(asset, "map1", "action1"); - Assert.That(reference.action, Is.SameAs(action1)); + Assert.That(reference.action, Is.SameAs(action)); Assert.That(reference.asset, Is.SameAs(asset)); - Assert.That(reference.name, Is.EqualTo("map/action1")); - Assert.That(reference.ToDisplayName(), Is.EqualTo("map/action1")); - Assert.That(reference.ToString(), Is.EqualTo(":map/action1")); + Assert.That(reference.name, Is.EqualTo("map1/action1")); + Assert.That(reference.ToDisplayName(), Is.EqualTo("map1/action1")); + Assert.That(reference.ToString(), Is.EqualTo(":map1/action1")); // Delete the referenced action directly or indirectly if (typeToDelete == typeof(InputAction)) - asset.RemoveAction("map/action1"); + asset.RemoveAction("map1/action1"); else if (typeToDelete == typeof(InputActionMap)) - asset.RemoveActionMap("map"); + asset.RemoveActionMap("map1"); else if (typeToDelete == typeof(InputActionAsset)) UnityEngine.Object.DestroyImmediate(asset); // TODO reference need to react to this Assert.That(reference.action, Is.Null); Assert.That(reference.asset, Is.SameAs(asset)); - Assert.That(reference.name, Is.EqualTo("map/action1")); // Unexpected when no longer existing - Assert.That(reference.ToDisplayName(), Is.EqualTo("map/action1")); // Unexpected when no longer existing + Assert.That(reference.name, Is.EqualTo("map1/action1")); // Unexpected when no longer existing + Assert.That(reference.ToDisplayName(), Is.EqualTo("map1/action1")); // Unexpected when no longer existing //Assert.That(reference.ToString(), Is.EqualTo(":" + new Guid(action1.m_Id))); // Unexpected when no longer existing } @@ -141,7 +208,8 @@ public void Actions_Reference_NameShouldReflectReferencedAction(Type typeToDelet // InputActionReference - Do not invalidate if action, action map or asset is deleted/destroyed. .action still returns the action. // (FIXED) InputActionAsset.RemoveActionMap - Do not remove actions within the map and they keep a stale reference to the removed map. // (FIXED) InputActionAsset.RemoveAction - Throws exception if action do not have any bindings. - + // (FIXED) InputActionReference.Set(null) - Does not update m_Action. Hence .actions returns an incorrect reference. Also doesn't update ScriptableObject.name consistently with default ScriptableObject. + // (FIXED) InputActionReference.Create - Returns null if action is null which is not inline with xmldoc description and inconsistent since it is allowed to create a reference and set it to null. If you want a null reference you should not call Create in the first place. // TODO Make a test where action map is deleted // TODO Make a test where asset is deleted diff --git a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs index f3e901f52f..6ebfa90791 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs @@ -72,12 +72,12 @@ public InputAction action /// that is itself contained in an . public void Set(InputAction action) { - // TODO Throw if attempting to set reference on an input action reference that resides in an asset. - if (action == null) { m_Asset = null; m_ActionId = null; + m_Action = null; + name = string.Empty; // Scriptable object default name is empty string. return; } @@ -116,27 +116,27 @@ public void Set(InputActionAsset asset, string mapName, string actionName) if (actionMap == null) throw new ArgumentException($"No action map '{mapName}' in '{asset}'", nameof(mapName)); - var action = actionMap.FindAction(actionName); - if (action == null) + var foundAction = actionMap.FindAction(actionName); + if (foundAction == null) throw new ArgumentException($"No action '{actionName}' in map '{mapName}' of asset '{asset}'", nameof(actionName)); - SetInternal(asset, action); + SetInternal(asset, foundAction); } - private void SetInternal(InputActionAsset asset, InputAction action) + private void SetInternal(InputActionAsset assetArg, InputAction actionArg) { - var actionMap = action.actionMap; - if (!asset.actionMaps.Contains(actionMap)) + var actionMap = actionArg.actionMap; + if (!assetArg.actionMaps.Contains(actionMap)) throw new ArgumentException( - $"Action '{action}' is not contained in asset '{asset}'", nameof(action)); + $"Action '{actionArg}' is not contained in asset '{assetArg}'", nameof(actionArg)); // If we are setting the reference in edit-mode, we want the state to be reflected in the serialized // object and hence assign serialized fields. This is a destructive operation. - m_Asset = asset; - m_ActionId = action.id.ToString(); - name = GetDisplayName(action); - m_Action = action; + m_Asset = assetArg; + m_ActionId = actionArg.id.ToString(); + m_Action = actionArg; + name = GetDisplayName(actionArg); ////REVIEW: should this dirty the asset if IDs had not been generated yet? } @@ -147,23 +147,19 @@ private void SetInternal(InputActionAsset asset, InputAction action) /// A string representation of the reference. public override string ToString() { - try - { - var value = action; + var value = action; // Indirect resolve + if (value == null) + return base.ToString(); + if (value.actionMap != null) return $"{m_Asset.name}:{value.actionMap.name}/{value.name}"; - } - catch - { - if (m_Asset != null) - return $"{m_Asset.name}:{m_ActionId}"; - } - - return base.ToString(); + return $"{m_Asset.name}:{m_ActionId}"; } - internal static string GetDisplayName(InputAction action) + private static string GetDisplayName(InputAction action) { - return !string.IsNullOrEmpty(action?.actionMap?.name) ? $"{action.actionMap?.name}/{action.name}" : action?.name; + return !string.IsNullOrEmpty(action?.actionMap?.name) + ? $"{action.actionMap?.name}/{action.name}" + : action?.name; } /// @@ -193,8 +189,6 @@ public static implicit operator InputAction(InputActionReference reference) /// A new InputActionReference referencing . public static InputActionReference Create(InputAction action) { - if (action == null) - return null; var reference = CreateInstance(); reference.Set(action); return reference; @@ -232,7 +226,9 @@ internal static void ResetCachedAction() /// [NonSerialized] private InputAction m_Action; - // Make annoying Microsoft code analyzer happy. + /// + /// Equivalent to . + /// public InputAction ToInputAction() { return action; @@ -240,7 +236,7 @@ public InputAction ToInputAction() private InputAction ResolveAction() { - return m_Asset == null ? null : m_Asset.FindAction(new Guid(m_ActionId)); + return m_Asset ? m_Asset.FindAction(new Guid(m_ActionId)) : null; } // OnValidate() is stripped out from player builds (editor only). From 0dbc8f5b617ddfb2e01e35a222f09f40768aeaef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Akan=20Sidenvall?= Date: Thu, 2 Oct 2025 10:28:46 +0200 Subject: [PATCH 07/27] Additional simplification, fixes and test cases. Added test assembly to support serialization within editor test. Added test utilities. --- .../EditorPrefsTestUtils.cs | 52 ++++++ .../EditorPrefsTestUtils.cs.meta | 3 + .../InputActionReferenceEditorTests.cs | 176 ++++++++++++++++++ .../InputActionReferenceEditorTests.cs.meta | 3 + .../InputActionsEditorTests.cs.meta | 11 +- Assets/Tests/InputSystem.Editor/TestUtils.cs | 87 ++++++++- .../Unity.InputSystem.Tests.Editor.asmdef | 3 +- Assets/Tests/InputSystem.TestSupport.meta | 8 + .../InputActionBehaviour.cs | 18 ++ .../InputActionBehaviour.cs.meta | 3 + .../Unity.InputSystem.TestSupport.asmdef | 16 ++ .../Unity.InputSystem.TestSupport.asmdef.meta | 7 + .../CoreTests_Actions_Reference.cs | 2 +- .../Actions/InputActionReference.cs | 48 +++++ .../AssetImporter/InputActionImporter.cs | 25 ++- .../Tests/TestFixture/AssetDatabaseUtils.cs | 29 ++- 16 files changed, 467 insertions(+), 24 deletions(-) create mode 100644 Assets/Tests/InputSystem.Editor/EditorPrefsTestUtils.cs create mode 100644 Assets/Tests/InputSystem.Editor/EditorPrefsTestUtils.cs.meta create mode 100644 Assets/Tests/InputSystem.Editor/InputActionReferenceEditorTests.cs create mode 100644 Assets/Tests/InputSystem.Editor/InputActionReferenceEditorTests.cs.meta create mode 100644 Assets/Tests/InputSystem.TestSupport.meta create mode 100644 Assets/Tests/InputSystem.TestSupport/InputActionBehaviour.cs create mode 100644 Assets/Tests/InputSystem.TestSupport/InputActionBehaviour.cs.meta create mode 100644 Assets/Tests/InputSystem.TestSupport/Unity.InputSystem.TestSupport.asmdef create mode 100644 Assets/Tests/InputSystem.TestSupport/Unity.InputSystem.TestSupport.asmdef.meta diff --git a/Assets/Tests/InputSystem.Editor/EditorPrefsTestUtils.cs b/Assets/Tests/InputSystem.Editor/EditorPrefsTestUtils.cs new file mode 100644 index 0000000000..276140f6f7 --- /dev/null +++ b/Assets/Tests/InputSystem.Editor/EditorPrefsTestUtils.cs @@ -0,0 +1,52 @@ +using UnityEditor; + +namespace Tests.InputSystem.Editor +{ + /// + /// Utility to simplify editor tests with respect to editor preferences. + /// + internal static class EditorPrefsTestUtils + { + private const string EnterPlayModeOptionsEnabledKey = "EnterPlayModeOptionsEnabled"; + private const string EnterPlayModeOptionsKey = "EnterPlayModeOptions"; + + private static bool _savedEnterPlayModeOptionsEnabled; + private static int _savedEnterPlayModeOptions; + + /// + /// Call this from a tests SetUp routine to save editor preferences so they can be restored after the test. + /// + public static void SaveEditorPrefs() + { + _savedEnterPlayModeOptionsEnabled = EditorPrefs.GetBool(EnterPlayModeOptionsEnabledKey, false); + _savedEnterPlayModeOptions = EditorPrefs.GetInt(EnterPlayModeOptionsKey, (int)EnterPlayModeOptions.None); + } + + /// + /// Call this from a tests TearDown routine to restore editor preferences to the state it had before the test. + /// + public static void RestoreEditorPrefs() + { + EditorPrefs.SetBool(EnterPlayModeOptionsEnabledKey, _savedEnterPlayModeOptionsEnabled); + EditorPrefs.SetInt(EnterPlayModeOptionsKey, _savedEnterPlayModeOptions); + } + + /// + /// Call this from within a test to temporarily enable domain reload. + /// + public static void EnableDomainReload() + { + EditorPrefs.SetBool(EnterPlayModeOptionsEnabledKey, false); + } + + /// + /// Call this from within a test to temporarily disable domain reload (and scene reloads). + /// + public static void DisableDomainReload() + { + EditorPrefs.SetBool(EnterPlayModeOptionsEnabledKey, true); + EditorPrefs.SetInt(EnterPlayModeOptionsKey, (int)(EnterPlayModeOptions.DisableDomainReload | + EnterPlayModeOptions.DisableSceneReload)); + } + } +} diff --git a/Assets/Tests/InputSystem.Editor/EditorPrefsTestUtils.cs.meta b/Assets/Tests/InputSystem.Editor/EditorPrefsTestUtils.cs.meta new file mode 100644 index 0000000000..0a1780f95f --- /dev/null +++ b/Assets/Tests/InputSystem.Editor/EditorPrefsTestUtils.cs.meta @@ -0,0 +1,3 @@ +fileFormatVersion: 2 +guid: d50a191facb44cd39e68985750838db9 +timeCreated: 1759311333 \ No newline at end of file diff --git a/Assets/Tests/InputSystem.Editor/InputActionReferenceEditorTests.cs b/Assets/Tests/InputSystem.Editor/InputActionReferenceEditorTests.cs new file mode 100644 index 0000000000..edd7e4dda4 --- /dev/null +++ b/Assets/Tests/InputSystem.Editor/InputActionReferenceEditorTests.cs @@ -0,0 +1,176 @@ +using System; +using System.Collections; +using System.Linq; +using NUnit.Framework; +using UnityEditor; +using UnityEditor.SceneManagement; +using UnityEngine; +using UnityEngine.InputSystem; +using UnityEngine.InputSystem.Editor; +using UnityEngine.SceneManagement; +using UnityEngine.TestTools; +using Object = UnityEngine.Object; + +namespace Tests.InputSystem.Editor +{ + /// + /// Editor tests for . + /// + /// + /// This test need fixed asset paths since mid-test domain reloads would otherwise discard data. + /// + /// Be aware that if you get failures in editor tests that switch between play mode and edit mode via coroutines + /// you might get misleading stack traces that indicate errors in different places than they actually happen. + /// At least this have been observered for exception stack traces. + /// + internal class InputActionReferenceEditorTests + { + private Scene m_Scene; + + private const string assetPath = "Assets/__InputActionReferenceEditorTestsActions.inputactions"; + private const string dummyPath = "Assets/__InputActionReferenceEditorTestsDummy.asset"; + private const string scenePath = "Assets/__InputActionReferenceEditorTestsScene.unity"; + + private void CreateAsset() + { + var asset = ScriptableObject.CreateInstance(); + + var map1 = new InputActionMap("map1"); + map1.AddAction("action1"); + map1.AddAction("action2"); + asset.AddActionMap(map1); + + System.IO.File.WriteAllText(assetPath, asset.ToJson()); + Object.DestroyImmediate(asset); + AssetDatabase.ImportAsset(assetPath); + } + + [SetUp] + public void SetUp() + { + // This looks odd, but when we yield into play mode from our test coroutine we may get a domain reload + // (depending on editor preferences) which will trigger another SetUp() mid-test. + if (Application.isPlaying) + return; + + m_Scene = EditorSceneManager.NewScene(NewSceneSetup.EmptyScene); + CreateAsset(); + + var go = new GameObject("Root"); + var behaviour = go.AddComponent(); + var reference = InputActionImporter.LoadInputActionReferencesFromAsset(assetPath).First( + r => "action1".Equals(r.action.name)); + behaviour.referenceAsField = reference; + behaviour.referenceAsReference = reference; + + TestUtils.SaveScene(m_Scene, scenePath); + } + + [TearDown] + public void TearDown() + { + // This looks odd, but when we yield into play mode from our test coroutine we may get a domain reload + // (depending on editor preferences) which will trigger another TearDown() mid-test. + if (Application.isPlaying) + return; + + // Close scene + EditorSceneManager.CloseScene(m_Scene, true); + + // Clean-up + AssetDatabase.DeleteAsset(dummyPath); + AssetDatabase.DeleteAsset(assetPath); + AssetDatabase.DeleteAsset(scenePath); + } + + private static InputActionBehaviour GetBehaviour() => Object.FindObjectOfType(); + private static InputActionAsset GetAsset() => AssetDatabase.LoadAssetAtPath(assetPath); + + // For unclear reason, NUnit fails to assert throwing exceptions after transition into play-mode. + // So until that can be sorted out, we do it manually (in the same way) ourselves. + private static void AssertThrows(Action action) where T : Exception + { + var exceptionThrown = false; + try + { + action(); + } + catch (InvalidOperationException) + { + exceptionThrown = true; + } + Assert.IsTrue(exceptionThrown, $"Expected exception of type {typeof(T)} to be thrown but it was not."); + } + + [UnityTest] + [Description("https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1584")] + public IEnumerator ReferenceSetInPlaymodeShouldBeRestored_WhenExitingPlaymode() + { + // Edit-mode section + { + // Sanity check our initial edit-mode state + var obj = GetBehaviour(); + Assert.That(obj.referenceAsField.action, Is.SameAs(GetAsset().FindAction("map1/action1"))); + Assert.That(obj.referenceAsReference.action, Is.SameAs(GetAsset().FindAction("map1/action1"))); + + // Enter play-mode (This will lead to domain reload by default). + yield return new EnterPlayMode(); + } + + // Play-mode section + { + var obj = GetBehaviour(); + var editModeAction = GetAsset().FindAction("map1/action1"); + var playModeAction = GetAsset().FindAction("map1/action2"); + + // Make sure our action reference is consistent in play-mode + Assert.That(obj.referenceAsField.action, Is.SameAs(editModeAction)); + + // ISXB-1584: Attempting assignment of persisted input action reference in play-mode in editor. + // Rationale: We cannot allow this since it would corrupt the source asset since changes applied to SO + // mapped to an asset isn't reverted when exiting play-mode. + // + // Here we would like to do: + // Assert.Throws(() => obj.reference.Set(null)); + // + // But we can't since it would fail with NullReferenceException. + // It turns out that because of the domain reload / Unity’s internal serialization quirks, the obj is + // sometimes null inside the lambda when NUnit captures it for execution. So to work around this we + // instead do the same kind of check manually for now which doesn't seem to have this problem. + // + // It is odd since NUnit does basically does the same thing (apart from wrapping the lambda as a + // TestDelegate). So the WHY for this problem remains unclear for now. + AssertThrows(() => obj.referenceAsField.Set(playModeAction)); + AssertThrows(() => obj.referenceAsReference.Set(editModeAction)); + + // Make sure there were no side-effects. + Assert.That(obj.referenceAsField.action, Is.SameAs(editModeAction)); + Assert.That(obj.referenceAsReference.action, Is.SameAs(editModeAction)); + + // Correct usage is to use a run-time assigned input action reference instead. It is up to the user + // to decide whether this reference should additionally be persisted (which is possible by saving it to + // and asset, or by using SerializeReference). + obj.referenceAsField = InputActionReference.Create(playModeAction); + obj.referenceAsReference = InputActionReference.Create(playModeAction); + + // Makes sure we have the expected reference. + Assert.That(obj.referenceAsField.action, Is.SameAs(playModeAction)); + Assert.That(obj.referenceAsReference.action, Is.SameAs(playModeAction)); + + // Exit play-mode (This will lead to domain reload by default). + yield return new ExitPlayMode(); + } + + // Edit-mode section + { + // Make sure our reference is back to its initial edit mode state + var obj = GetBehaviour(); + var editModeAction = GetAsset().FindAction("map1/action1"); + Assert.That(obj.referenceAsField.action, Is.SameAs(editModeAction)); + Assert.That(obj.referenceAsReference.action, Is.SameAs(editModeAction)); + } + + yield return null; + } + } +} diff --git a/Assets/Tests/InputSystem.Editor/InputActionReferenceEditorTests.cs.meta b/Assets/Tests/InputSystem.Editor/InputActionReferenceEditorTests.cs.meta new file mode 100644 index 0000000000..2493eb6d72 --- /dev/null +++ b/Assets/Tests/InputSystem.Editor/InputActionReferenceEditorTests.cs.meta @@ -0,0 +1,3 @@ +fileFormatVersion: 2 +guid: a468197148d54148907ffb0e1c053c65 +timeCreated: 1759304679 \ No newline at end of file diff --git a/Assets/Tests/InputSystem.Editor/InputActionsEditorTests.cs.meta b/Assets/Tests/InputSystem.Editor/InputActionsEditorTests.cs.meta index abaccf56c6..6ce6403d42 100644 --- a/Assets/Tests/InputSystem.Editor/InputActionsEditorTests.cs.meta +++ b/Assets/Tests/InputSystem.Editor/InputActionsEditorTests.cs.meta @@ -1,2 +1,11 @@ fileFormatVersion: 2 -guid: 2a061f6298b7df240a1842d1e02a957d \ No newline at end of file +guid: 2a061f6298b7df240a1842d1e02a957d +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/InputSystem.Editor/TestUtils.cs b/Assets/Tests/InputSystem.Editor/TestUtils.cs index 5650acc2f8..bb7e884aec 100644 --- a/Assets/Tests/InputSystem.Editor/TestUtils.cs +++ b/Assets/Tests/InputSystem.Editor/TestUtils.cs @@ -1,7 +1,12 @@ +using System; +using System.Linq; +using UnityEditor; +using UnityEditor.SceneManagement; using UnityEngine.InputSystem; using UnityEngine.InputSystem.Editor; +using UnityEngine.SceneManagement; -// Replicated in this test assembly to avoid building public API picked up by PackageValidator +// Replicated in this test assembly to avoid building public API picked up by PackageValidator. internal class TestUtils { #if UNITY_EDITOR @@ -24,5 +29,85 @@ public static void RestoreDialogs() Dialog.ControlScheme.SetDeleteControlScheme(null); } + /// + /// Returns the (default) name of a scene asset created at the given path. + /// + /// The path to the asset. + /// Scene name. + public static string GetSceneName(string scenePath) + { + return System.IO.Path.GetFileNameWithoutExtension(scenePath); + } + + /// + /// Saves a scene to the given path. + /// + /// The scene to be saved. + /// The target scene path. + /// If true, force saves the scene by making it explicitly dirty. + /// Throws exception if failed to save scene + /// (if dirty or was true) + public static void SaveScene(Scene scene, string scenePath, bool makeDirty = true) + { + var wasDirty = scene.isDirty; + if (makeDirty) + EditorSceneManager.MarkSceneDirty(scene); + var wasSaved = EditorSceneManager.SaveScene(scene, scenePath); + if (!wasSaved && (wasDirty || makeDirty)) + throw new Exception($"Failed to save scene to {scenePath}"); + } + + /// + /// Adds a scene asset to build settings unless it is already present. + /// + /// The scene path to the scene asset. + /// Value tuple where first argument (int) specifies the build index of the scene and the second + /// argument (bool) specifies whether the scene was added (true) or already present (false). + public static ValueTuple AddSceneToEditorBuildSettings(string scenePath) + { + // Determine if scene is already present or not. + var existingScenes = EditorBuildSettings.scenes; + var index = FindSceneIndexByPath(existingScenes, scenePath); + if (index >= 0) + return new ValueTuple(index, false); + + // Add the new scene. + var newScenes = new EditorBuildSettingsScene[existingScenes.Length + 1]; + Array.Copy(existingScenes, newScenes, existingScenes.Length); + newScenes[existingScenes.Length] = new EditorBuildSettingsScene(scenePath, true); + EditorBuildSettings.scenes = newScenes; + + return new ValueTuple(existingScenes.Length, true); + } + + /// + /// Removes a scene asset from editor build settings given its path. + /// + /// The asset path of the scene to be removed. + /// true if the scene was removed, false if it do not exist within editor build settings scenes. + public static bool RemoveSceneFromEditorBuildSettings(string scenePath) + { + var existingScenes = EditorBuildSettings.scenes; + var index = FindSceneIndexByPath(existingScenes, scenePath); + if (index < 0) + return false; + + var newScenes = new EditorBuildSettingsScene[existingScenes.Length - 1]; + Array.Copy(existingScenes, newScenes, index); + Array.Copy(existingScenes, index + 1, newScenes, index, existingScenes.Length - index - 1); + EditorBuildSettings.scenes = newScenes; + return true; + } + + private static int FindSceneIndexByPath(EditorBuildSettingsScene[] scenes, string scenePath) + { + for (var i = 0; i < scenes.Length; ++i) + { + if (scenes[i].path == scenePath) + return i; + } + return -1; + } + #endif // UNITY_EDITOR } diff --git a/Assets/Tests/InputSystem.Editor/Unity.InputSystem.Tests.Editor.asmdef b/Assets/Tests/InputSystem.Editor/Unity.InputSystem.Tests.Editor.asmdef index c5ecbdbc5d..0ddc350ce0 100644 --- a/Assets/Tests/InputSystem.Editor/Unity.InputSystem.Tests.Editor.asmdef +++ b/Assets/Tests/InputSystem.Editor/Unity.InputSystem.Tests.Editor.asmdef @@ -5,7 +5,8 @@ "UnityEngine.TestRunner", "UnityEditor.TestRunner", "Unity.InputSystem", - "Unity.InputSystem.TestFramework" + "Unity.InputSystem.TestFramework", + "Unity.InputSystem.TestSupport" ], "includePlatforms": [ "Editor" diff --git a/Assets/Tests/InputSystem.TestSupport.meta b/Assets/Tests/InputSystem.TestSupport.meta new file mode 100644 index 0000000000..631710d522 --- /dev/null +++ b/Assets/Tests/InputSystem.TestSupport.meta @@ -0,0 +1,8 @@ +fileFormatVersion: 2 +guid: 4e07537d3c95f4d54842701a8eda9588 +folderAsset: yes +DefaultImporter: + externalObjects: {} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/InputSystem.TestSupport/InputActionBehaviour.cs b/Assets/Tests/InputSystem.TestSupport/InputActionBehaviour.cs new file mode 100644 index 0000000000..bc64dd0ff2 --- /dev/null +++ b/Assets/Tests/InputSystem.TestSupport/InputActionBehaviour.cs @@ -0,0 +1,18 @@ +using UnityEngine; +using UnityEngine.InputSystem; + +// This class and this assembly should NOT be considered API, it is only public and its own assembly for the following +// reasons: +// 1) Unity only supports serialization of public types stored in a file with the same name. +// If this wasn't the case, this type would be internal to the test assembly. +// 2) Editor test assemblies cannot contain MonoBehaviour that should be added to scene game objects. +// Hence, this assembly needs to be a regular assembly and referenced by editor test assembly to use this +// MonoBehaviour in a scene. +// +// Note that we serialize both as field and reference. Note that this should not make any difference for Unity.Object +// types, but is included for completeness. +public sealed class InputActionBehaviour : MonoBehaviour +{ + [SerializeField] public InputActionReference referenceAsField; + [SerializeReference] public InputActionReference referenceAsReference; +} diff --git a/Assets/Tests/InputSystem.TestSupport/InputActionBehaviour.cs.meta b/Assets/Tests/InputSystem.TestSupport/InputActionBehaviour.cs.meta new file mode 100644 index 0000000000..9827fbe6af --- /dev/null +++ b/Assets/Tests/InputSystem.TestSupport/InputActionBehaviour.cs.meta @@ -0,0 +1,3 @@ +fileFormatVersion: 2 +guid: ca0cb2a0912943afa6dd586c959f79e3 +timeCreated: 1759319676 \ No newline at end of file diff --git a/Assets/Tests/InputSystem.TestSupport/Unity.InputSystem.TestSupport.asmdef b/Assets/Tests/InputSystem.TestSupport/Unity.InputSystem.TestSupport.asmdef new file mode 100644 index 0000000000..25833308dd --- /dev/null +++ b/Assets/Tests/InputSystem.TestSupport/Unity.InputSystem.TestSupport.asmdef @@ -0,0 +1,16 @@ +{ + "name": "Unity.InputSystem.TestSupport", + "rootNamespace": "", + "references": [ + "GUID:75469ad4d38634e559750d17036d5f7c" + ], + "includePlatforms": [], + "excludePlatforms": [], + "allowUnsafeCode": true, + "overrideReferences": true, + "precompiledReferences": [], + "autoReferenced": false, + "defineConstraints": [], + "versionDefines": [], + "noEngineReferences": false +} \ No newline at end of file diff --git a/Assets/Tests/InputSystem.TestSupport/Unity.InputSystem.TestSupport.asmdef.meta b/Assets/Tests/InputSystem.TestSupport/Unity.InputSystem.TestSupport.asmdef.meta new file mode 100644 index 0000000000..484a730ede --- /dev/null +++ b/Assets/Tests/InputSystem.TestSupport/Unity.InputSystem.TestSupport.asmdef.meta @@ -0,0 +1,7 @@ +fileFormatVersion: 2 +guid: 94b8393c82ebb4677a21b5ab4ab8019b +AssemblyDefinitionImporter: + externalObjects: {} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs b/Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs index d4621c7901..ddd5cd1fca 100644 --- a/Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs +++ b/Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs @@ -204,7 +204,7 @@ public void Actions_Reference_ShouldReevaluateIfAssociatedEntityIsDeleted(Type t // (FIXED) InputActionReference.asset - Returns incorrect asset after being .Set if .action is called before Set. // (FIXED) InputActionReference - Assigned with Set during play-mode corrupts InputActionAsset by mutating asset reference into pointing into potentially another asset and overwrites existign asset. // (FIXED) InputActionReferencePropertyDrawer - Assigns direct reference into InputActionAsset to InputActionReference fields leading to corruption. - // InputActionReference - Contains stale reference and looks like a valid reference if the action is deleted. .action returns removed action. + // InputActionReference - Contains stale reference and looks like a valid reference if the action is deleted. .action returns removed action. This is a presentation (UI) issue, reference should be fixed when referenced entities are removed to allow resolving itself after e.g. Undo. // InputActionReference - Do not invalidate if action, action map or asset is deleted/destroyed. .action still returns the action. // (FIXED) InputActionAsset.RemoveActionMap - Do not remove actions within the map and they keep a stale reference to the removed map. // (FIXED) InputActionAsset.RemoveAction - Throws exception if action do not have any bindings. diff --git a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs index 6ebfa90791..9394e4bc1e 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs @@ -1,5 +1,6 @@ using System; using System.Linq; +using UnityEditor; ////REVIEW: Can we somehow make this a simple struct? The one problem we have is that we can't put struct instances as sub-assets into //// the import (i.e. InputActionImporter can't do AddObjectToAsset with them). However, maybe there's a way around that. The thing @@ -62,6 +63,41 @@ public InputAction action } } + private static bool CanSetReference(InputActionReference reference) + { + // TODO Consider an alternative approach where importer would build a registry of immutable references. + // That allows us to remove the #if UNITY_EDITOR and skip these checks. +#if UNITY_EDITOR + // If the asset isn't persisted it cannot be part of an InputActionAsset file. + // We use this check first since it can allow us to avoid GC pressure. + var instanceID = reference.GetInstanceID(); + var isPersistent = AssetDatabase.TryGetGUIDAndLocalFileIdentifier(instanceID, out _, out long _); + if (!isPersistent) + return true; + + // "Immutable" input action references are always sub-assets of InputActionAsset. + var isSubAsset = AssetDatabase.IsSubAsset(instanceID); + if (!isSubAsset) + return true; + + // If we cannot get the path of our reference, we cannot be a persisted asset within an InputActionAsset. + var path = AssetDatabase.GetAssetPath(reference); + if (path == null) + return true; + + // If we cannot get the main asset we cannot be a persisted asset within an InputActionAsset. + // Also we check that it is the expected type. + var mainAsset = AssetDatabase.LoadMainAssetAtPath(path); + if (mainAsset == null) + return true; + + // We can only allow setting the reference if it is not part of an persisted InputActionAsset. + return (mainAsset is not InputActionAsset); +#else + return true; +#endif + } + /// /// Initialize the reference to refer to the given action. /// @@ -72,6 +108,18 @@ public InputAction action /// that is itself contained in an . public void Set(InputAction action) { + // Prevent accidental mutation of the source asset if this InputActionReference is a persisted object + // residing as a sub-asset within a .inputactions asset. + // This is not needed for players since scriptable objects aren't serialized back from within a player. + if (!CanSetReference(this)) + { + throw new InvalidOperationException("Attempting to mutate an immutable InputActionReference instance. " + + "This is not allowed since it would modify the source asset." + + "Instead use InputActionReference.Create(action) to create a new " + + "in-memory instance or serialize it as a separate asset if it " + + "survive domain reloads."); + } + if (action == null) { m_Asset = null; diff --git a/Packages/com.unity.inputsystem/InputSystem/Editor/AssetImporter/InputActionImporter.cs b/Packages/com.unity.inputsystem/InputSystem/Editor/AssetImporter/InputActionImporter.cs index 64ba9e08af..f53ce2e857 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Editor/AssetImporter/InputActionImporter.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Editor/AssetImporter/InputActionImporter.cs @@ -275,35 +275,34 @@ private static void GenerateWrapperCode(AssetImportContext ctx, InputActionAsset } } -#if UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS internal static IEnumerable LoadInputActionReferencesFromAsset(string assetPath) { // Get all InputActionReferences are stored at the same asset path as InputActionAsset // Note we exclude 'hidden' action references (which are present to support one of the pre releases) return AssetDatabase.LoadAllAssetsAtPath(assetPath).Where( - o => o is InputActionReference && !((InputActionReference)o).hideFlags.HasFlag(HideFlags.HideInHierarchy)) + o => o is InputActionReference && + !((InputActionReference)o).hideFlags.HasFlag(HideFlags.HideInHierarchy)) .Cast(); } +#if UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS + private static readonly string[] s_DefaultAssetSearchFolders = new string[] { "Assets" }; + /// /// Gets all instances available in assets in the project. /// By default, it only gets the assets located in the "Assets" folder. /// - /// - /// + /// Optional array of directory paths to be searched. + /// If true, excludes project-wide input actions from the result. /// internal static IEnumerable LoadInputActionReferencesFromAssetDatabase( string[] foldersPath = null, bool skipProjectWide = false) { - string[] searchFolders = null; - // If folderPath is null, search in "Assets" folder. - if (foldersPath == null) - { - searchFolders = new string[] { "Assets" }; - } - - // Get all InputActionReference from assets in "Asset" folder. It does not search inside "Packages" folder. - var inputActionReferenceGUIDs = AssetDatabase.FindAssets($"t:{typeof(InputActionReference).Name}", searchFolders); + // Get all InputActionReference from assets in "Asset" folder by default. + // It does not search inside "Packages" folder. + const string inputActionReferenceFilter = "t:" + nameof(InputActionReference); + var inputActionReferenceGUIDs = AssetDatabase.FindAssets(inputActionReferenceFilter, + foldersPath ?? s_DefaultAssetSearchFolders); // To find all the InputActionReferences, the GUID of the asset containing at least one action reference is // used to find the asset path. This is because InputActionReferences are stored in the asset database as sub-assets of InputActionAsset. diff --git a/Packages/com.unity.inputsystem/Tests/TestFixture/AssetDatabaseUtils.cs b/Packages/com.unity.inputsystem/Tests/TestFixture/AssetDatabaseUtils.cs index 5b3fe5d73b..63228d2017 100644 --- a/Packages/com.unity.inputsystem/Tests/TestFixture/AssetDatabaseUtils.cs +++ b/Packages/com.unity.inputsystem/Tests/TestFixture/AssetDatabaseUtils.cs @@ -9,6 +9,8 @@ namespace UnityEngine.InputSystem /// Provides convenience functions for creating and managing assets for test purposes. /// Note that all returned paths are converted to Unix paths when running on Windows /// for consistency and to avoid mixed path names. + /// + /// This file should NOT be in this assembly, it should reside in the editor test assembly and be internal. public static class AssetDatabaseUtils { private const string kAssetPath = "Assets"; @@ -33,7 +35,8 @@ public static void ExternalMoveFileOrDirectory(string source, string dest) } // Create an asset at the given path containing the given text content. - private static T CreateAssetAtPath(string path, string content) where T : UnityEngine.Object + private static T CreateAssetAtPath(string path, string content) + where T : ScriptableObject { Debug.Assert(!File.Exists(path)); @@ -42,9 +45,20 @@ private static T CreateAssetAtPath(string path, string content) where T : Uni { CreateDirectories(Path.GetDirectoryName(path)); - File.WriteAllText(path, content); - AssetDatabase.ImportAsset(path); - obj = AssetDatabase.LoadAssetAtPath(path); + if (content != null) + { + // Create asset as a file with explicit content + File.WriteAllText(path, content); + AssetDatabase.ImportAsset(path); + obj = AssetDatabase.LoadAssetAtPath(path); + } + else + { + // Use default serialization (YAML) to create asset. + obj = ScriptableObject.CreateInstance(); + AssetDatabase.CreateAsset(obj, path); + } + if (obj == null) throw new Exception($"Failed to create asset at \"{path}\""); } @@ -99,7 +113,8 @@ public static string CreateDirectory() // Creates an asset in the given directory path with an explicit or random file name containing the // given content or the default content based on type. - public static T CreateAsset(string directoryPath, string filename = null, string content = null) where T : UnityEngine.Object + public static T CreateAsset(string directoryPath, string filename = null, string content = null) + where T : ScriptableObject { Debug.Assert(directoryPath == null || directoryPath.Contains(RootPath())); Debug.Assert(filename == null || !filename.Contains("/")); @@ -124,7 +139,7 @@ public static T CreateAsset(string directoryPath, string filename = null, str // Creates an asset at the given path containing the specified content. // If path is null, a unique random file name is assigned, if content is null the default content based // on type (extension) is used. - public static T CreateAsset(string path = null, string content = null) where T : UnityEngine.Object + public static T CreateAsset(string path = null, string content = null) where T : UnityEngine.ScriptableObject { if (path == null) path = RandomAssetFilePath(RootPath(), AssetFileExtensionFromType(typeof(T))); @@ -208,7 +223,7 @@ private static string DefaultContentFromType(Type type) { if (type == typeof(InputActionAsset)) return "{}"; - return string.Empty; + return null; } } } From 651526e568ca53799d1a38b8da904a8e4deeee41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Akan=20Sidenvall?= Date: Fri, 3 Oct 2025 13:26:14 +0200 Subject: [PATCH 08/27] Added more tests, code cleanup, refactored QA script to QA asset folder. --- Assets/Editor/DumpInputActionReferences.cs | 33 ++ .../Editor/DumpInputActionReferences.cs.meta | 11 + .../EditorPrefsTestUtils.cs | 26 +- .../InputActionReferenceEditorTests.cs | 307 ++++++++++-------- ...ctionReferencePropertyDrawerEditorTests.cs | 164 ++++++++++ ...ReferencePropertyDrawerEditorTests.cs.meta | 3 + .../CoreTests_Actions_Reference.cs | 13 +- .../Actions/InputActionReference.cs | 154 ++++----- .../AssetImporter/InputActionImporter.cs | 31 ++ .../InputActionReferencePropertyDrawer.cs | 41 ++- .../InputSystem/InputSystem.cs | 2 +- 11 files changed, 539 insertions(+), 246 deletions(-) create mode 100644 Assets/Editor/DumpInputActionReferences.cs create mode 100644 Assets/Editor/DumpInputActionReferences.cs.meta create mode 100644 Assets/Tests/InputSystem.Editor/InputActionReferencePropertyDrawerEditorTests.cs create mode 100644 Assets/Tests/InputSystem.Editor/InputActionReferencePropertyDrawerEditorTests.cs.meta diff --git a/Assets/Editor/DumpInputActionReferences.cs b/Assets/Editor/DumpInputActionReferences.cs new file mode 100644 index 0000000000..c3703b453b --- /dev/null +++ b/Assets/Editor/DumpInputActionReferences.cs @@ -0,0 +1,33 @@ +using System.Collections; +using System.Collections.Generic; +using System.Text; +using UnityEngine; +using UnityEngine.InputSystem; + +internal static class DumpInputActionReferences +{ + private static void DumpReferences(StringBuilder sb, string prefix, InputActionReference[] references) + { + sb.Append(prefix + ":\n"); + foreach (var reference in references) + { + var s = reference.action != null ? "Yes" : "No"; + sb.Append($"- {reference.name} (Resolved: {s}, Asset: {reference.asset})\n"); + } + } + + private static void DumpReferences() + { + var sb = new StringBuilder(); + DumpReferences(sb, "Loaded objects", Object.FindObjectsByType( + FindObjectsInactive.Include, FindObjectsSortMode.InstanceID)); + DumpReferences(sb, "All objects:", Resources.FindObjectsOfTypeAll()); + Debug.Log(sb.ToString()); + } + + [UnityEditor.MenuItem("QA Tools/Dump Input Action References to Console", false, 100)] + private static void Dump() + { + DumpReferences(); + } +} diff --git a/Assets/Editor/DumpInputActionReferences.cs.meta b/Assets/Editor/DumpInputActionReferences.cs.meta new file mode 100644 index 0000000000..7f1391d91d --- /dev/null +++ b/Assets/Editor/DumpInputActionReferences.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 68f73e449eb4b41ee946193cedeface2 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/InputSystem.Editor/EditorPrefsTestUtils.cs b/Assets/Tests/InputSystem.Editor/EditorPrefsTestUtils.cs index 276140f6f7..bedc2009e0 100644 --- a/Assets/Tests/InputSystem.Editor/EditorPrefsTestUtils.cs +++ b/Assets/Tests/InputSystem.Editor/EditorPrefsTestUtils.cs @@ -25,12 +25,36 @@ public static void SaveEditorPrefs() /// /// Call this from a tests TearDown routine to restore editor preferences to the state it had before the test. /// + /// Note that if domain reloads have not been disabled and you have a domain reload mid-test, + /// this utility will fail to restore editor preferences since the saved data will be lost. public static void RestoreEditorPrefs() { EditorPrefs.SetBool(EnterPlayModeOptionsEnabledKey, _savedEnterPlayModeOptionsEnabled); EditorPrefs.SetInt(EnterPlayModeOptionsKey, _savedEnterPlayModeOptions); } + /// + /// Returns whether domain reloads are disabled. + /// + /// true if domain reloads have been disabled, else false. + public static bool IsDomainReloadsDisabled() + { + return EditorPrefs.GetBool(EnterPlayModeOptionsEnabledKey, false) && + (EditorPrefs.GetInt(EnterPlayModeOptionsKey, (int)EnterPlayModeOptions.None) & + (int)EnterPlayModeOptions.DisableDomainReload) != 0; + } + + /// + /// Returns whether scene reloads are disabled. + /// + /// true if scene reloads have been disabled, else false. + public static bool IsSceneReloadsDisabled() + { + return EditorPrefs.GetBool(EnterPlayModeOptionsEnabledKey, false) && + (EditorPrefs.GetInt(EnterPlayModeOptionsKey, (int)EnterPlayModeOptions.None) & + (int)EnterPlayModeOptions.DisableSceneReload) != 0; + } + /// /// Call this from within a test to temporarily enable domain reload. /// @@ -44,9 +68,9 @@ public static void EnableDomainReload() /// public static void DisableDomainReload() { - EditorPrefs.SetBool(EnterPlayModeOptionsEnabledKey, true); EditorPrefs.SetInt(EnterPlayModeOptionsKey, (int)(EnterPlayModeOptions.DisableDomainReload | EnterPlayModeOptions.DisableSceneReload)); + EditorPrefs.SetBool(EnterPlayModeOptionsEnabledKey, true); } } } diff --git a/Assets/Tests/InputSystem.Editor/InputActionReferenceEditorTests.cs b/Assets/Tests/InputSystem.Editor/InputActionReferenceEditorTests.cs index edd7e4dda4..66c3463e0d 100644 --- a/Assets/Tests/InputSystem.Editor/InputActionReferenceEditorTests.cs +++ b/Assets/Tests/InputSystem.Editor/InputActionReferenceEditorTests.cs @@ -2,6 +2,7 @@ using System.Collections; using System.Linq; using NUnit.Framework; +using Tests.InputSystem.Editor; using UnityEditor; using UnityEditor.SceneManagement; using UnityEngine; @@ -11,166 +12,186 @@ using UnityEngine.TestTools; using Object = UnityEngine.Object; -namespace Tests.InputSystem.Editor +/// +/// Editor tests for . +/// +/// +/// This test need fixed asset paths since mid-test domain reloads would otherwise discard data. +/// +/// Be aware that if you get failures in editor tests that switch between play mode and edit mode via coroutines +/// you might get misleading stack traces that indicate errors in different places than they actually happen. +/// At least this have been observered for exception stack traces. +/// +internal class InputActionReferenceEditorTestsWithScene { - /// - /// Editor tests for . - /// - /// - /// This test need fixed asset paths since mid-test domain reloads would otherwise discard data. - /// - /// Be aware that if you get failures in editor tests that switch between play mode and edit mode via coroutines - /// you might get misleading stack traces that indicate errors in different places than they actually happen. - /// At least this have been observered for exception stack traces. - /// - internal class InputActionReferenceEditorTests + private const string TestCategory = "Editor"; + + private Scene m_Scene; + + private const string assetPath = "Assets/__InputActionReferenceEditorTests.inputactions"; + private const string dummyPath = "Assets/__InputActionReferenceEditorTestsDummy.asset"; + private const string scenePath = "Assets/__InputActionReferenceEditorTestsScene.unity"; + + private void CreateAsset() { - private Scene m_Scene; + var asset = ScriptableObject.CreateInstance(); - private const string assetPath = "Assets/__InputActionReferenceEditorTestsActions.inputactions"; - private const string dummyPath = "Assets/__InputActionReferenceEditorTestsDummy.asset"; - private const string scenePath = "Assets/__InputActionReferenceEditorTestsScene.unity"; + var map1 = new InputActionMap("map1"); + map1.AddAction("action1"); + map1.AddAction("action2"); + asset.AddActionMap(map1); - private void CreateAsset() - { - var asset = ScriptableObject.CreateInstance(); + System.IO.File.WriteAllText(assetPath, asset.ToJson()); + Object.DestroyImmediate(asset); + AssetDatabase.ImportAsset(assetPath); + } + + [SetUp] + public void SetUp() + { + // This looks odd, but when we yield into play mode from our test coroutine we may get a domain reload + // (depending on editor preferences) which will trigger another SetUp() mid-test. + if (Application.isPlaying) + return; - var map1 = new InputActionMap("map1"); - map1.AddAction("action1"); - map1.AddAction("action2"); - asset.AddActionMap(map1); + EditorPrefsTestUtils.SaveEditorPrefs(); - System.IO.File.WriteAllText(assetPath, asset.ToJson()); - Object.DestroyImmediate(asset); - AssetDatabase.ImportAsset(assetPath); - } + m_Scene = EditorSceneManager.NewScene(NewSceneSetup.EmptyScene); + CreateAsset(); + + var go = new GameObject("Root"); + var behaviour = go.AddComponent(); + var reference = InputActionImporter.LoadInputActionReferencesFromAsset(assetPath).First( + r => "action1".Equals(r.action.name)); + behaviour.referenceAsField = reference; + behaviour.referenceAsReference = reference; + + TestUtils.SaveScene(m_Scene, scenePath); + } + + [TearDown] + public void TearDown() + { + // This looks odd, but when we yield into play mode from our test coroutine we may get a domain reload + // (depending on editor preferences) which will trigger another TearDown() mid-test. + if (Application.isPlaying) + return; + + EditorPrefsTestUtils.RestoreEditorPrefs(); + + // Close scene + EditorSceneManager.CloseScene(m_Scene, true); + + // Clean-up + AssetDatabase.DeleteAsset(dummyPath); + AssetDatabase.DeleteAsset(assetPath); + AssetDatabase.DeleteAsset(scenePath); + } + + private void DisableDomainReloads() + { + // Assumes off before running tests. + Debug.Assert(!EditorPrefsTestUtils.IsDomainReloadsDisabled()); + Debug.Assert(!EditorPrefsTestUtils.IsSceneReloadsDisabled()); + + // Safe to store since state wouldn't be reset by domain reload. + EditorPrefsTestUtils.DisableDomainReload(); + } + + private static InputActionBehaviour GetBehaviour() => Object.FindObjectOfType(); + private static InputActionAsset GetAsset() => AssetDatabase.LoadAssetAtPath(assetPath); - [SetUp] - public void SetUp() + // For unclear reason, NUnit fails to assert throwing exceptions after transition into play-mode. + // So until that can be sorted out, we do it manually (in the same way) ourselves. + private static void AssertThrows(Action action) where T : Exception + { + var exceptionThrown = false; + try { - // This looks odd, but when we yield into play mode from our test coroutine we may get a domain reload - // (depending on editor preferences) which will trigger another SetUp() mid-test. - if (Application.isPlaying) - return; - - m_Scene = EditorSceneManager.NewScene(NewSceneSetup.EmptyScene); - CreateAsset(); - - var go = new GameObject("Root"); - var behaviour = go.AddComponent(); - var reference = InputActionImporter.LoadInputActionReferencesFromAsset(assetPath).First( - r => "action1".Equals(r.action.name)); - behaviour.referenceAsField = reference; - behaviour.referenceAsReference = reference; - - TestUtils.SaveScene(m_Scene, scenePath); + action(); } - - [TearDown] - public void TearDown() + catch (InvalidOperationException) { - // This looks odd, but when we yield into play mode from our test coroutine we may get a domain reload - // (depending on editor preferences) which will trigger another TearDown() mid-test. - if (Application.isPlaying) - return; - - // Close scene - EditorSceneManager.CloseScene(m_Scene, true); - - // Clean-up - AssetDatabase.DeleteAsset(dummyPath); - AssetDatabase.DeleteAsset(assetPath); - AssetDatabase.DeleteAsset(scenePath); + exceptionThrown = true; } + Assert.IsTrue(exceptionThrown, $"Expected exception of type {typeof(T)} to be thrown but it was not."); + } + + private static bool[] _disableDomainReloadsValues = new bool[] { false, true }; + + [UnityTest] + [Category(TestCategory)] + [Description("https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1584")] + public IEnumerator ReferenceSetInPlaymodeShouldBeRestored_WhenExitingPlaymode( + [ValueSource(nameof(_disableDomainReloadsValues))] bool disableDomainReloads) + { + if (disableDomainReloads) + DisableDomainReloads(); - private static InputActionBehaviour GetBehaviour() => Object.FindObjectOfType(); - private static InputActionAsset GetAsset() => AssetDatabase.LoadAssetAtPath(assetPath); + // Edit-mode section + { + // Sanity check our initial edit-mode state + var obj = GetBehaviour(); + Assert.That(obj.referenceAsField.action, Is.SameAs(GetAsset().FindAction("map1/action1"))); + Assert.That(obj.referenceAsReference.action, Is.SameAs(GetAsset().FindAction("map1/action1"))); + + // Enter play-mode (This will lead to domain reload by default). + yield return new EnterPlayMode(); + } - // For unclear reason, NUnit fails to assert throwing exceptions after transition into play-mode. - // So until that can be sorted out, we do it manually (in the same way) ourselves. - private static void AssertThrows(Action action) where T : Exception + // Play-mode section { - var exceptionThrown = false; - try - { - action(); - } - catch (InvalidOperationException) - { - exceptionThrown = true; - } - Assert.IsTrue(exceptionThrown, $"Expected exception of type {typeof(T)} to be thrown but it was not."); + var obj = GetBehaviour(); + var editModeAction = GetAsset().FindAction("map1/action1"); + var playModeAction = GetAsset().FindAction("map1/action2"); + + // Make sure our action reference is consistent in play-mode + Assert.That(obj.referenceAsField.action, Is.SameAs(editModeAction)); + + // ISXB-1584: Attempting assignment of persisted input action reference in play-mode in editor. + // Rationale: We cannot allow this since it would corrupt the source asset since changes applied to SO + // mapped to an asset isn't reverted when exiting play-mode. + // + // Here we would like to do: + // Assert.Throws(() => obj.reference.Set(null)); + // + // But we can't since it would fail with NullReferenceException. + // It turns out that because of the domain reload / Unity’s internal serialization quirks, the obj is + // sometimes null inside the lambda when NUnit captures it for execution. So to work around this we + // instead do the same kind of check manually for now which doesn't seem to have this problem. + // + // It is odd since NUnit does basically does the same thing (apart from wrapping the lambda as a + // TestDelegate). So the WHY for this problem remains unclear for now. + AssertThrows(() => obj.referenceAsField.Set(playModeAction)); + AssertThrows(() => obj.referenceAsReference.Set(editModeAction)); + + // Make sure there were no side-effects. + Assert.That(obj.referenceAsField.action, Is.SameAs(editModeAction)); + Assert.That(obj.referenceAsReference.action, Is.SameAs(editModeAction)); + + // Correct usage is to use a run-time assigned input action reference instead. It is up to the user + // to decide whether this reference should additionally be persisted (which is possible by saving it to + // and asset, or by using SerializeReference). + obj.referenceAsField = InputActionReference.Create(playModeAction); + obj.referenceAsReference = InputActionReference.Create(playModeAction); + + // Makes sure we have the expected reference. + Assert.That(obj.referenceAsField.action, Is.SameAs(playModeAction)); + Assert.That(obj.referenceAsReference.action, Is.SameAs(playModeAction)); + + // Exit play-mode (This will lead to domain reload by default). + yield return new ExitPlayMode(); } - [UnityTest] - [Description("https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1584")] - public IEnumerator ReferenceSetInPlaymodeShouldBeRestored_WhenExitingPlaymode() + // Edit-mode section { - // Edit-mode section - { - // Sanity check our initial edit-mode state - var obj = GetBehaviour(); - Assert.That(obj.referenceAsField.action, Is.SameAs(GetAsset().FindAction("map1/action1"))); - Assert.That(obj.referenceAsReference.action, Is.SameAs(GetAsset().FindAction("map1/action1"))); - - // Enter play-mode (This will lead to domain reload by default). - yield return new EnterPlayMode(); - } - - // Play-mode section - { - var obj = GetBehaviour(); - var editModeAction = GetAsset().FindAction("map1/action1"); - var playModeAction = GetAsset().FindAction("map1/action2"); - - // Make sure our action reference is consistent in play-mode - Assert.That(obj.referenceAsField.action, Is.SameAs(editModeAction)); - - // ISXB-1584: Attempting assignment of persisted input action reference in play-mode in editor. - // Rationale: We cannot allow this since it would corrupt the source asset since changes applied to SO - // mapped to an asset isn't reverted when exiting play-mode. - // - // Here we would like to do: - // Assert.Throws(() => obj.reference.Set(null)); - // - // But we can't since it would fail with NullReferenceException. - // It turns out that because of the domain reload / Unity’s internal serialization quirks, the obj is - // sometimes null inside the lambda when NUnit captures it for execution. So to work around this we - // instead do the same kind of check manually for now which doesn't seem to have this problem. - // - // It is odd since NUnit does basically does the same thing (apart from wrapping the lambda as a - // TestDelegate). So the WHY for this problem remains unclear for now. - AssertThrows(() => obj.referenceAsField.Set(playModeAction)); - AssertThrows(() => obj.referenceAsReference.Set(editModeAction)); - - // Make sure there were no side-effects. - Assert.That(obj.referenceAsField.action, Is.SameAs(editModeAction)); - Assert.That(obj.referenceAsReference.action, Is.SameAs(editModeAction)); - - // Correct usage is to use a run-time assigned input action reference instead. It is up to the user - // to decide whether this reference should additionally be persisted (which is possible by saving it to - // and asset, or by using SerializeReference). - obj.referenceAsField = InputActionReference.Create(playModeAction); - obj.referenceAsReference = InputActionReference.Create(playModeAction); - - // Makes sure we have the expected reference. - Assert.That(obj.referenceAsField.action, Is.SameAs(playModeAction)); - Assert.That(obj.referenceAsReference.action, Is.SameAs(playModeAction)); - - // Exit play-mode (This will lead to domain reload by default). - yield return new ExitPlayMode(); - } - - // Edit-mode section - { - // Make sure our reference is back to its initial edit mode state - var obj = GetBehaviour(); - var editModeAction = GetAsset().FindAction("map1/action1"); - Assert.That(obj.referenceAsField.action, Is.SameAs(editModeAction)); - Assert.That(obj.referenceAsReference.action, Is.SameAs(editModeAction)); - } - - yield return null; + // Make sure our reference is back to its initial edit mode state + var obj = GetBehaviour(); + var editModeAction = GetAsset().FindAction("map1/action1"); + Assert.That(obj.referenceAsField.action, Is.SameAs(editModeAction)); + Assert.That(obj.referenceAsReference.action, Is.SameAs(editModeAction)); } + + yield return null; } } diff --git a/Assets/Tests/InputSystem.Editor/InputActionReferencePropertyDrawerEditorTests.cs b/Assets/Tests/InputSystem.Editor/InputActionReferencePropertyDrawerEditorTests.cs new file mode 100644 index 0000000000..951372043c --- /dev/null +++ b/Assets/Tests/InputSystem.Editor/InputActionReferencePropertyDrawerEditorTests.cs @@ -0,0 +1,164 @@ +using System; +using System.Collections; +using NUnit.Framework; +using UnityEditor; +using UnityEngine; +using UnityEngine.InputSystem; +using UnityEngine.InputSystem.Editor; +using UnityEngine.TestTools; +using UnityEngine.UIElements; +using Object = UnityEngine.Object; + +/// +/// Tests for custom property drawer associated with InputActionReference. +/// +internal sealed class InputActionReferencePropertyDrawerEditorTests +{ + private const string assetPath = "Assets/__InputActionReferencePropertyDrawerEditorTests.inputactions"; + + private GameObject go; + private InputActionBehaviour behaviour; + private SerializedObject serializedObject; + private SerializedProperty serializedProperty; + private bool onGuiCalled; + private bool fieldCalled; + private EditorWindow window; + + private Object fieldObj; + private Object fieldResult; + + [SetUp] + public void SetUp() + { + go = new GameObject("Host"); + behaviour = go.AddComponent(); + serializedObject = new SerializedObject(behaviour); + serializedProperty = serializedObject.FindProperty(nameof(InputActionBehaviour.referenceAsField)); + window = EditorWindow.GetWindow(); + } + + [TearDown] + public void TearDown() + { + window.Close(); + serializedObject.Dispose(); + Object.DestroyImmediate(go); + + AssetDatabase.DeleteAsset(assetPath); + } + + private void CreateAsset() + { + var asset = ScriptableObject.CreateInstance(); + + var map = new InputActionMap("map"); + map.AddAction("action1"); + map.AddAction("action2"); + asset.AddActionMap(map); + + System.IO.File.WriteAllText(assetPath, asset.ToJson()); + Object.DestroyImmediate(asset); + AssetDatabase.ImportAsset(assetPath); + } + + private Object Field(Rect position, Object obj, Type type, GUIContent label) + { + fieldObj = obj; + fieldCalled = true; + return fieldResult; + } + + private IEnumerator RenderDrawer(InputActionReferencePropertyDrawer drawer, bool mockField = false) + { + onGuiCalled = false; + fieldCalled = false; + + var container = new IMGUIContainer(() => + { + var rect = new Rect(0, 0, 200, 20); + var label = new GUIContent("Reference"); + if (mockField) + drawer.OnGUI(rect, serializedProperty, label, Field); + else + drawer.OnGUI(rect, serializedProperty, label); + onGuiCalled = true; + }); + + window.rootVisualElement.Add(container); + window.Repaint(); + + var timeoutTime = Time.realtimeSinceStartupAsDouble + 3.0; + do + { + yield return null; // let Unity process a frame + } + while (!onGuiCalled && Time.realtimeSinceStartupAsDouble < timeoutTime); + } + + private class TestHostWindow : EditorWindow {} + + [UnityTest] + [Description("This is a weak test but is mainly for coverage of the regular OnGUI")] + public IEnumerator ExecutesWithoutExceptions() + { + var drawer = new InputActionReferencePropertyDrawer(); + yield return RenderDrawer(drawer); + Assert.That(onGuiCalled, Is.True); + } + + [UnityTest] + [Description("Verifies that when InputActionReference field is null, null is passed to Inspector field UI")] + public IEnumerator FieldObjectIsNullWhenReferenceIsNull() + { + var drawer = new InputActionReferencePropertyDrawer(); + yield return RenderDrawer(drawer, true); + Assert.That(onGuiCalled, Is.True); + Assert.That(fieldObj, Is.Null); + } + + [UnityTest] + [Description("Verifies that when InputActionReference field is not null and reference is valid, " + + "null is passed to Inspector field UI")] + public IEnumerator FieldObjectIsReferenceWhenReferenceIsValid() + { + CreateAsset(); + var asset = AssetDatabase.LoadAssetAtPath(assetPath); + var action = asset.FindAction("map/action1"); + Assert.That(action, Is.Not.Null); // Sanity check + + // Simulate reference being assigned + var reference = InputActionReference.Create(action); + serializedProperty.objectReferenceValue = reference; + serializedObject.ApplyModifiedPropertiesWithoutUndo(); + + var drawer = new InputActionReferencePropertyDrawer(); + yield return RenderDrawer(drawer, true); + Assert.That(onGuiCalled, Is.True); + Assert.That(fieldObj, Is.SameAs(reference)); + } + + [UnityTest] + [Description("Verifies that when InputActionReference field is not null and reference is broken, " + + "null is passed to Inspector field UI")] + public IEnumerator FieldObjectIsNullWhenReferenceIsInvalid() + { + CreateAsset(); + var asset = AssetDatabase.LoadAssetAtPath(assetPath); + var action = asset.FindAction("map/action1"); + Assert.That(action, Is.Not.Null); // Sanity check + + // Simulate reference being assigned + var reference = InputActionReference.Create(action); + serializedProperty.objectReferenceValue = reference; + serializedObject.ApplyModifiedPropertiesWithoutUndo(); + + // Delete the action to make the reference invalid + asset.RemoveAction("map/action1"); + InputActionAssetManager.SaveAsset(assetPath, asset.ToJson()); + + var drawer = new InputActionReferencePropertyDrawer(); + yield return RenderDrawer(drawer, true); + Assert.That(onGuiCalled, Is.True); + Assert.That(fieldObj, Is.Null); + } +} diff --git a/Assets/Tests/InputSystem.Editor/InputActionReferencePropertyDrawerEditorTests.cs.meta b/Assets/Tests/InputSystem.Editor/InputActionReferencePropertyDrawerEditorTests.cs.meta new file mode 100644 index 0000000000..8a84ff2313 --- /dev/null +++ b/Assets/Tests/InputSystem.Editor/InputActionReferencePropertyDrawerEditorTests.cs.meta @@ -0,0 +1,3 @@ +fileFormatVersion: 2 +guid: b84098284c2a46bb914384e5b682b950 +timeCreated: 1759473326 \ No newline at end of file diff --git a/Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs b/Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs index ddd5cd1fca..4be2d2f5ef 100644 --- a/Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs +++ b/Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs @@ -2,7 +2,6 @@ using NUnit.Framework; using UnityEngine; using UnityEngine.InputSystem; -using Object = System.Object; partial class CoreTests { @@ -109,6 +108,8 @@ public void Actions_Reference_CanResolveAction_WhenSet(bool setByReference) var asset = CreateAssetWithTwoActions(); var action = asset.FindAction("map1/action2"); + // Note that setting reference is allowed when its an in-memory object and not a reference object backed + // by an asset. var reference = ScriptableObject.CreateInstance(); if (setByReference) reference.Set(action); @@ -169,11 +170,6 @@ public void Actions_Reference_ShouldReevaluateIfAssociatedEntityIsDeleted(Type t var asset = CreateAssetWithTwoActions(); var action = asset.FindAction("map1/action1"); - // var map = new InputActionMap("map"); - // var action1 = map.AddAction("action1"); - // var asset = ScriptableObject.CreateInstance(); - // asset.AddActionMap(map); - var reference = ScriptableObject.CreateInstance(); reference.Set(asset, "map1", "action1"); @@ -204,12 +200,13 @@ public void Actions_Reference_ShouldReevaluateIfAssociatedEntityIsDeleted(Type t // (FIXED) InputActionReference.asset - Returns incorrect asset after being .Set if .action is called before Set. // (FIXED) InputActionReference - Assigned with Set during play-mode corrupts InputActionAsset by mutating asset reference into pointing into potentially another asset and overwrites existign asset. // (FIXED) InputActionReferencePropertyDrawer - Assigns direct reference into InputActionAsset to InputActionReference fields leading to corruption. - // InputActionReference - Contains stale reference and looks like a valid reference if the action is deleted. .action returns removed action. This is a presentation (UI) issue, reference should be fixed when referenced entities are removed to allow resolving itself after e.g. Undo. - // InputActionReference - Do not invalidate if action, action map or asset is deleted/destroyed. .action still returns the action. + // (SOMEWHAT FIXED) InputActionReference - Contains stale reference and looks like a valid reference if the action is deleted. .action returns removed action. This is a presentation (UI) issue, reference should be fixed when referenced entities are removed to allow resolving itself after e.g. Undo. + // (FIXED) InputActionReference - Do not invalidate if action, action map or asset is deleted/destroyed. .action still returns the action. // (FIXED) InputActionAsset.RemoveActionMap - Do not remove actions within the map and they keep a stale reference to the removed map. // (FIXED) InputActionAsset.RemoveAction - Throws exception if action do not have any bindings. // (FIXED) InputActionReference.Set(null) - Does not update m_Action. Hence .actions returns an incorrect reference. Also doesn't update ScriptableObject.name consistently with default ScriptableObject. // (FIXED) InputActionReference.Create - Returns null if action is null which is not inline with xmldoc description and inconsistent since it is allowed to create a reference and set it to null. If you want a null reference you should not call Create in the first place. + // InputActionSerializationHelpers.RemoveAction - Isn't removing InputActionReference objects. This might be a bug unless importer will make sure the object is deleted. // TODO Make a test where action map is deleted // TODO Make a test where asset is deleted diff --git a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs index 9394e4bc1e..a9cf7ad4b7 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs @@ -1,6 +1,4 @@ using System; -using System.Linq; -using UnityEditor; ////REVIEW: Can we somehow make this a simple struct? The one problem we have is that we can't put struct instances as sub-assets into //// the import (i.e. InputActionImporter can't do AddObjectToAsset with them). However, maybe there's a way around that. The thing @@ -56,46 +54,13 @@ public InputAction action { // Note that we check m_Action since it would be null if we haven't yet resolved the action. // We also need to check that m_Action.actionMap isn't null since it would be null if the - // action was deleted. + // action was deleted. Additionally we need to check m_Asset since it could also have been deleted. if (m_Action != null && m_Action.actionMap != null && m_Asset) return m_Action; - return (m_Action = ResolveAction()); - } - } - - private static bool CanSetReference(InputActionReference reference) - { - // TODO Consider an alternative approach where importer would build a registry of immutable references. - // That allows us to remove the #if UNITY_EDITOR and skip these checks. -#if UNITY_EDITOR - // If the asset isn't persisted it cannot be part of an InputActionAsset file. - // We use this check first since it can allow us to avoid GC pressure. - var instanceID = reference.GetInstanceID(); - var isPersistent = AssetDatabase.TryGetGUIDAndLocalFileIdentifier(instanceID, out _, out long _); - if (!isPersistent) - return true; - - // "Immutable" input action references are always sub-assets of InputActionAsset. - var isSubAsset = AssetDatabase.IsSubAsset(instanceID); - if (!isSubAsset) - return true; - - // If we cannot get the path of our reference, we cannot be a persisted asset within an InputActionAsset. - var path = AssetDatabase.GetAssetPath(reference); - if (path == null) - return true; - - // If we cannot get the main asset we cannot be a persisted asset within an InputActionAsset. - // Also we check that it is the expected type. - var mainAsset = AssetDatabase.LoadMainAssetAtPath(path); - if (mainAsset == null) - return true; - // We can only allow setting the reference if it is not part of an persisted InputActionAsset. - return (mainAsset is not InputActionAsset); -#else - return true; -#endif + // Attempt to resolve action based on asset and GUID. + return (m_Action = m_Asset ? m_Asset.FindAction(new Guid(m_ActionId)) : null); + } } /// @@ -108,18 +73,6 @@ private static bool CanSetReference(InputActionReference reference) /// that is itself contained in an . public void Set(InputAction action) { - // Prevent accidental mutation of the source asset if this InputActionReference is a persisted object - // residing as a sub-asset within a .inputactions asset. - // This is not needed for players since scriptable objects aren't serialized back from within a player. - if (!CanSetReference(this)) - { - throw new InvalidOperationException("Attempting to mutate an immutable InputActionReference instance. " + - "This is not allowed since it would modify the source asset." + - "Instead use InputActionReference.Create(action) to create a new " + - "in-memory instance or serialize it as a separate asset if it " + - "survive domain reloads."); - } - if (action == null) { m_Asset = null; @@ -174,10 +127,7 @@ public void Set(InputActionAsset asset, string mapName, string actionName) private void SetInternal(InputActionAsset assetArg, InputAction actionArg) { - var actionMap = actionArg.actionMap; - if (!assetArg.actionMaps.Contains(actionMap)) - throw new ArgumentException( - $"Action '{actionArg}' is not contained in asset '{assetArg}'", nameof(actionArg)); + CheckImmutableReference(); // If we are setting the reference in edit-mode, we want the state to be reflected in the serialized // object and hence assign serialized fields. This is a destructive operation. @@ -185,8 +135,6 @@ private void SetInternal(InputActionAsset assetArg, InputAction actionArg) m_ActionId = actionArg.id.ToString(); m_Action = actionArg; name = GetDisplayName(actionArg); - - ////REVIEW: should this dirty the asset if IDs had not been generated yet? } /// @@ -246,22 +194,31 @@ public static InputActionReference Create(InputAction action) /// Clears the cached field for all current objects. /// /// - /// After calling this, the next call to will retrieve a new - /// reference from the existing just as if using it for the first time. - /// The serialized and fields are not touched and will continue - /// to hold their current values. - /// /// This method is used to clear the Action references when exiting PlayMode since those objects are no /// longer valid. /// - internal static void ResetCachedAction() + internal static void InvalidateAll() { + // It might be possible that Object.FindObjectOfTypeAll(true) would be sufficient here since we only + // need to invalidate non-serialized data on active/loaded objects. This returns a lot more, but for + // now we keep it like this to not change more than we have to. Optimizing this can be done separately. var allActionRefs = Resources.FindObjectsOfTypeAll(typeof(InputActionReference)); foreach (var obj in allActionRefs) - { - var reference = (InputActionReference)obj; - reference.m_Action = null; - } + ((InputActionReference)obj).Invalidate(); + } + + /// + /// Clears the cached field for this instance. + /// + /// + /// After calling this, the next call to will resolve a new + /// reference from the existing just as if using it for the first time. + /// The serialized and fields are not touched and will continue + /// to hold their current values. Also the name remains valid since the underlying reference data is unmodified. + /// + internal void Invalidate() + { + m_Action = null; } [SerializeField] internal InputActionAsset m_Asset; @@ -282,19 +239,62 @@ public InputAction ToInputAction() return action; } - private InputAction ResolveAction() + /// + /// Checks if this input action reference instance can be safely mutated without side effects. + /// + /// + /// This check isn't needed in player builds since ScriptableObject would never be persisted if mutated + /// in a player. + /// + /// Thrown if this input action reference is part of an + /// input actions asset and mutating it would have side-effects on the projects assets. + [System.Diagnostics.Conditional("UNITY_EDITOR")] + private void CheckImmutableReference() { - return m_Asset ? m_Asset.FindAction(new Guid(m_ActionId)) : null; - } + // Note that we do a lot of checking here, but it is only for a rather slim (unintended) use case in + // editor and not in final builds. The alternative would be to set a non-serialized field on the reference + // when importing assets which would simplify this class, but it adds complexity to import stage and + // is more difficult to assess from a asset version portability perspective. + static bool CanSetReference(InputActionReference reference) + { + // If the asset isn't persisted it cannot be part of an InputActionAsset file. + // We use this check first since it can allow us to avoid GC pressure. + var instanceID = reference.GetInstanceID(); + var isPersistent = UnityEditor.AssetDatabase.TryGetGUIDAndLocalFileIdentifier(instanceID, out _, out long _); + if (!isPersistent) + return true; + + // "Immutable" input action references are always sub-assets of InputActionAsset. + var isSubAsset = UnityEditor.AssetDatabase.IsSubAsset(instanceID); + if (!isSubAsset) + return true; + + // If we cannot get the path of our reference, we cannot be a persisted asset within an InputActionAsset. + var path = UnityEditor.AssetDatabase.GetAssetPath(reference); + if (path == null) + return true; + + // If we cannot get the main asset we cannot be a persisted asset within an InputActionAsset. + // Also we check that it is the expected type. + var mainAsset = UnityEditor.AssetDatabase.LoadMainAssetAtPath(path); + if (mainAsset == null) + return true; + + // We can only allow setting the reference if it is not part of an persisted InputActionAsset. + return (mainAsset is not InputActionAsset); + } - // OnValidate() is stripped out from player builds (editor only). - /*private void OnValidate() - { - var resolvedAction = ResolveAction(); - if (resolvedAction == null) + // Prevent accidental mutation of the source asset if this InputActionReference is a persisted object + // residing as a sub-asset within a .inputactions asset. + // This is not needed for players since scriptable objects aren't serialized back from within a player. + if (!CanSetReference(this)) { - Debug.Log("RESOLVED TO NULL"); + throw new InvalidOperationException("Attempting to mutate an immutable InputActionReference instance. " + + "This is not allowed since it would modify the source asset." + + "Instead use InputActionReference.Create(action) to create a new " + + "in-memory instance or serialize it as a separate asset if it " + + "survive domain reloads."); } - }*/ + } } } diff --git a/Packages/com.unity.inputsystem/InputSystem/Editor/AssetImporter/InputActionImporter.cs b/Packages/com.unity.inputsystem/InputSystem/Editor/AssetImporter/InputActionImporter.cs index f53ce2e857..0fb8416436 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Editor/AssetImporter/InputActionImporter.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Editor/AssetImporter/InputActionImporter.cs @@ -180,6 +180,8 @@ private static void CreateInputActionReferences(InputActionAsset asset, AddObjec { foreach (var action in map.actions) { + // Note that it is important action is set before being added to asset, otherwise the immutability + // check within InputActionReference would throw. var actionReference = ScriptableObject.CreateInstance(); actionReference.Set(action); addObjectToAsset(action.m_Id, actionReference, actionIcon); @@ -350,6 +352,17 @@ public static string NameFromAssetPath(string assetPath) return Path.GetFileNameWithoutExtension(assetPath); } + private static bool ContainsInputActionAssetPath(string[] assetPaths) + { + foreach (var assetPath in assetPaths) + { + if (IsInputActionAssetPath(assetPath)) + return true; + } + + return false; + } + // This processor was added to address this issue: // https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-749 // @@ -377,11 +390,29 @@ private static void OnPostprocessAllAssets(string[] importedAssets, string[] del string[] movedAssets, string[] movedFromAssetPaths) #endif { + var needToInvalidate = false; foreach (var assetPath in importedAssets) { if (IsInputActionAssetPath(assetPath)) + { + needToInvalidate = true; CheckAndRenameJsonNameIfDifferent(assetPath); + } } + + if (!needToInvalidate) + needToInvalidate = ContainsInputActionAssetPath(deletedAssets); + if (!needToInvalidate) + needToInvalidate = ContainsInputActionAssetPath(movedAssets); + if (!needToInvalidate) + needToInvalidate = ContainsInputActionAssetPath(movedFromAssetPaths); + + // Invalidate all references to make sure there are no dangling references after reimport among + // our "live objects. We only need to invalidate loaded sub-assets and not assets/prefabs/SO etc + // since they may still contain effectively obsolete InputActionReferences but those references + // will resolve. + if (needToInvalidate) + InputActionReference.InvalidateAll(); } private static void CheckAndRenameJsonNameIfDifferent(string assetPath) diff --git a/Packages/com.unity.inputsystem/InputSystem/Editor/PropertyDrawers/InputActionReferencePropertyDrawer.cs b/Packages/com.unity.inputsystem/InputSystem/Editor/PropertyDrawers/InputActionReferencePropertyDrawer.cs index a6d00a3720..8f459e035d 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Editor/PropertyDrawers/InputActionReferencePropertyDrawer.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Editor/PropertyDrawers/InputActionReferencePropertyDrawer.cs @@ -13,37 +13,41 @@ namespace UnityEngine.InputSystem.Editor [CustomPropertyDrawer(typeof(InputActionReference))] internal sealed class InputActionReferencePropertyDrawer : PropertyDrawer { + // Custom search providers for asset-based, and project-wide actions input action reference sub-assets. private readonly SearchContext m_Context = UnityEditor.Search.SearchService.CreateContext(new[] { InputActionReferenceSearchProviders.CreateInputActionReferenceSearchProviderForAssets(), InputActionReferenceSearchProviders.CreateInputActionReferenceSearchProviderForProjectWideActions(), }, string.Empty, SearchConstants.PickerSearchFlags); - public override void OnGUI(Rect position, SerializedProperty property, GUIContent label) + public void OnGUI(Rect position, SerializedProperty property, GUIContent label, + System.Func doField = null) { EditorGUI.BeginProperty(position, label, property); EditorGUI.BeginChangeCheck(); - // Reassign null if property is a dangling project-wide input action - // var explicitlyReassigned = false; var current = property.objectReferenceValue; - // if (reference != null && reference.asset == InputSystem.actions) - // { - // var action = reference?.asset?.FindAction(reference.action.id); - // if (action is null) - // property.objectReferenceValue = null; // TODO This cannot be right?! - // } + + // If the reference is set but cannot be resolved, we want to be consistent with standard unity objects + // which resolve to display "Missing (Type)". This isn't really feasible since our SO isn't destroyed + // directly if referenced from other assets and we have two-layer indirection (InputAction isn't an asset). + // Hence, the next best thing we can do is pass null to object field to show "None (Type)" to indicate + // a broken reference that need to be replaced. If however, other code would manage to delete the SO, + // "Missing (Type)" should be the result. + var obj = current; + var currentReference = current as InputActionReference; + if (currentReference && currentReference.action == null) + obj = null; // none/missing // Pick an InputActionReference using custom picker. We need to use this overload taking an object - // in order to be in control of the actual assignment to the serialized property. - // This is important since we should NEVER assign a direct reference to a ScriptableObject residing - // in an asset. Instead, we should instantiate a new ScriptableObject instance to prevent destructive - // operations that would mutate the asset. - var candidate = ObjectField.DoObjectField(position, current, typeof(InputActionReference), + // in order to be in control of the actual assignment to the serialized property so we can substitute. + // We allow substituting the object field here to at least enable some test coverage. + var candidate = doField != null ? doField(position, obj, typeof(InputActionReference), label) + : ObjectField.DoObjectField(position, obj, typeof(InputActionReference), label, m_Context, SearchConstants.PickerViewFlags); - // Only assign the value if it was actually changed by the user. - if (EditorGUI.EndChangeCheck() && !Equals(candidate, current)) + // Only assign the value was actually changed by the user. + if (EditorGUI.EndChangeCheck() && !Equals(candidate, current)) { var reference = candidate as InputActionReference; property.objectReferenceValue = reference ? @@ -52,6 +56,11 @@ public override void OnGUI(Rect position, SerializedProperty property, GUIConten EditorGUI.EndProperty(); } + + public override void OnGUI(Rect position, SerializedProperty property, GUIContent label) + { + OnGUI(position, property, label); + } } } diff --git a/Packages/com.unity.inputsystem/InputSystem/InputSystem.cs b/Packages/com.unity.inputsystem/InputSystem/InputSystem.cs index 34d4462e22..67989918e9 100644 --- a/Packages/com.unity.inputsystem/InputSystem/InputSystem.cs +++ b/Packages/com.unity.inputsystem/InputSystem/InputSystem.cs @@ -3665,7 +3665,7 @@ internal static void OnPlayModeChange(PlayModeStateChange change) InputActionState.DestroyAllActionMapStates(); // Clear the Action reference from all InputActionReference objects - InputActionReference.ResetCachedAction(); + InputActionReference.InvalidateAll(); // Restore settings. if (!string.IsNullOrEmpty(s_SystemObject.settings)) From 769d73cedfc06fc5a868a3ecd75df5892c2d90b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Akan=20Sidenvall?= Date: Fri, 3 Oct 2025 14:50:42 +0200 Subject: [PATCH 09/27] Updated CHANGELOG --- Packages/com.unity.inputsystem/CHANGELOG.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Packages/com.unity.inputsystem/CHANGELOG.md b/Packages/com.unity.inputsystem/CHANGELOG.md index cfea02b73a..df60c26316 100644 --- a/Packages/com.unity.inputsystem/CHANGELOG.md +++ b/Packages/com.unity.inputsystem/CHANGELOG.md @@ -30,7 +30,11 @@ however, it has to be formatted properly to pass verification tests. - Fixed formatting issues on processor documentation page. - Fixed an issue in `InputActionReference` where `Set` was not updating the cached action leading to incorrect evaluation of `InputActionReference.action` after first being cached, then set, then read again. This could lead to reference not being reset if set programmatically during play-mode. [ISXB-1584](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1584). - Fixed an issue where `InputActionSetupExtensions.Remove` will result in `NullReferenceException` if the action being removed has no bindings. (ISXB-1688). - +- Fixed an issue where assigning a ``MonoBehaviour` field of type `InputActionReference` to an instance stored in `.inputactions` asset via `InputActionReference.Set` in playmode corrupts the originally referenced asset. (ISXB-1692). +- Fixed an issue where `InputActionReference.Create(null)` returns `null` instead "of a reference object to the given action" as documented behaviour for the API. (ISXB-1693) +- Fixed an issue where `InputActtionReference.Set(null)` does not update the cached input action reference nor the `ScriptableObject.name` leading to incorrect action being returned and reference showing up as the path to the previously assigned action in Inspector. (ISXB-1694) +- Fixed an issue where `InputActionSetupExtension.RemoveActionMap` doesn't remove actions within the map, leaving them with stale references back to the map if accessing map from action. (ISXB-1699) +- Fixed an issue where Inspector field of type `InputActionReference` isn't reflecting broken reference in case action is deleted from asset, action map is deleted from asset, or the asset itself is deleted. (ISXB-1701) ## [1.14.2] - 2025-08-05 From 58f9c281b846a03a312b8f5ae0f9bc9667c4f11d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Akan=20Sidenvall?= Date: Fri, 3 Oct 2025 14:53:08 +0200 Subject: [PATCH 10/27] Removed development comments from source file. --- .../InputSystem/CoreTests_Actions_Reference.cs | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs b/Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs index 4be2d2f5ef..a365acd95c 100644 --- a/Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs +++ b/Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs @@ -195,20 +195,5 @@ public void Actions_Reference_ShouldReevaluateIfAssociatedEntityIsDeleted(Type t //Assert.That(reference.ToString(), Is.EqualTo(":" + new Guid(action1.m_Id))); // Unexpected when no longer existing } - // List of bugs addressed on this branch: - // (FIXED) InputActionReference - Do not set m_Action when calling Set. - // (FIXED) InputActionReference.asset - Returns incorrect asset after being .Set if .action is called before Set. - // (FIXED) InputActionReference - Assigned with Set during play-mode corrupts InputActionAsset by mutating asset reference into pointing into potentially another asset and overwrites existign asset. - // (FIXED) InputActionReferencePropertyDrawer - Assigns direct reference into InputActionAsset to InputActionReference fields leading to corruption. - // (SOMEWHAT FIXED) InputActionReference - Contains stale reference and looks like a valid reference if the action is deleted. .action returns removed action. This is a presentation (UI) issue, reference should be fixed when referenced entities are removed to allow resolving itself after e.g. Undo. - // (FIXED) InputActionReference - Do not invalidate if action, action map or asset is deleted/destroyed. .action still returns the action. - // (FIXED) InputActionAsset.RemoveActionMap - Do not remove actions within the map and they keep a stale reference to the removed map. - // (FIXED) InputActionAsset.RemoveAction - Throws exception if action do not have any bindings. - // (FIXED) InputActionReference.Set(null) - Does not update m_Action. Hence .actions returns an incorrect reference. Also doesn't update ScriptableObject.name consistently with default ScriptableObject. - // (FIXED) InputActionReference.Create - Returns null if action is null which is not inline with xmldoc description and inconsistent since it is allowed to create a reference and set it to null. If you want a null reference you should not call Create in the first place. - // InputActionSerializationHelpers.RemoveAction - Isn't removing InputActionReference objects. This might be a bug unless importer will make sure the object is deleted. - - // TODO Make a test where action map is deleted - // TODO Make a test where asset is deleted - // TODO Make an undo resolve test + // TODO Make an undo resolve test that destroys reference sub-asset, breaks reference, then undo and reference is valid again. } From 5497481febb23a66415e24faa08db1425a23b207 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Akan=20Sidenvall?= Date: Fri, 3 Oct 2025 14:56:01 +0200 Subject: [PATCH 11/27] Undo changes to AssetDatabaseUtils that were not needed in the end. --- .../Tests/TestFixture/AssetDatabaseUtils.cs | 29 +++++-------------- 1 file changed, 7 insertions(+), 22 deletions(-) diff --git a/Packages/com.unity.inputsystem/Tests/TestFixture/AssetDatabaseUtils.cs b/Packages/com.unity.inputsystem/Tests/TestFixture/AssetDatabaseUtils.cs index 63228d2017..5b3fe5d73b 100644 --- a/Packages/com.unity.inputsystem/Tests/TestFixture/AssetDatabaseUtils.cs +++ b/Packages/com.unity.inputsystem/Tests/TestFixture/AssetDatabaseUtils.cs @@ -9,8 +9,6 @@ namespace UnityEngine.InputSystem /// Provides convenience functions for creating and managing assets for test purposes. /// Note that all returned paths are converted to Unix paths when running on Windows /// for consistency and to avoid mixed path names. - /// - /// This file should NOT be in this assembly, it should reside in the editor test assembly and be internal. public static class AssetDatabaseUtils { private const string kAssetPath = "Assets"; @@ -35,8 +33,7 @@ public static void ExternalMoveFileOrDirectory(string source, string dest) } // Create an asset at the given path containing the given text content. - private static T CreateAssetAtPath(string path, string content) - where T : ScriptableObject + private static T CreateAssetAtPath(string path, string content) where T : UnityEngine.Object { Debug.Assert(!File.Exists(path)); @@ -45,20 +42,9 @@ private static T CreateAssetAtPath(string path, string content) { CreateDirectories(Path.GetDirectoryName(path)); - if (content != null) - { - // Create asset as a file with explicit content - File.WriteAllText(path, content); - AssetDatabase.ImportAsset(path); - obj = AssetDatabase.LoadAssetAtPath(path); - } - else - { - // Use default serialization (YAML) to create asset. - obj = ScriptableObject.CreateInstance(); - AssetDatabase.CreateAsset(obj, path); - } - + File.WriteAllText(path, content); + AssetDatabase.ImportAsset(path); + obj = AssetDatabase.LoadAssetAtPath(path); if (obj == null) throw new Exception($"Failed to create asset at \"{path}\""); } @@ -113,8 +99,7 @@ public static string CreateDirectory() // Creates an asset in the given directory path with an explicit or random file name containing the // given content or the default content based on type. - public static T CreateAsset(string directoryPath, string filename = null, string content = null) - where T : ScriptableObject + public static T CreateAsset(string directoryPath, string filename = null, string content = null) where T : UnityEngine.Object { Debug.Assert(directoryPath == null || directoryPath.Contains(RootPath())); Debug.Assert(filename == null || !filename.Contains("/")); @@ -139,7 +124,7 @@ public static T CreateAsset(string directoryPath, string filename = null, str // Creates an asset at the given path containing the specified content. // If path is null, a unique random file name is assigned, if content is null the default content based // on type (extension) is used. - public static T CreateAsset(string path = null, string content = null) where T : UnityEngine.ScriptableObject + public static T CreateAsset(string path = null, string content = null) where T : UnityEngine.Object { if (path == null) path = RandomAssetFilePath(RootPath(), AssetFileExtensionFromType(typeof(T))); @@ -223,7 +208,7 @@ private static string DefaultContentFromType(Type type) { if (type == typeof(InputActionAsset)) return "{}"; - return null; + return string.Empty; } } } From 7611d033d34e6cbc5236a8e5fa5860accf21ef61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Akan=20Sidenvall?= Date: Fri, 3 Oct 2025 21:36:00 +0200 Subject: [PATCH 12/27] xmldoc fixes --- .../InputSystem/Actions/InputActionReference.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs index a9cf7ad4b7..56578f2706 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs @@ -71,6 +71,8 @@ public InputAction action /// case the reference is reset to its default state which does not reference an action. /// is not contained in an /// that is itself contained in an . + /// If attempting to mutate a reference object + /// that is backed by an .inputactions asset. This is not allowed to prevent side-effects. public void Set(InputAction action) { if (action == null) @@ -101,6 +103,8 @@ public void Set(InputAction action) /// is null -or- /// is null or empty -or- /// is null or empty. + /// If attempting to mutate a reference object + /// that is backed by by .inputactions asset. This is not allowed to prevent side-effects. /// No action map called could /// be found in -or- no action called /// could be found in the action map called in . @@ -234,6 +238,7 @@ internal void Invalidate() /// /// Equivalent to . /// + /// The associated action reference if its a valid reference, else null. public InputAction ToInputAction() { return action; From 6dd0f88402dab3e93b016b0183635e44dcb9bd3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Akan=20Sidenvall?= Date: Sun, 5 Oct 2025 11:40:18 +0200 Subject: [PATCH 13/27] Replaced Conditional by preprocessor check since Conditional cannot work since referencing editor types. --- .../InputSystem/Actions/InputActionReference.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs index 56578f2706..17dfdb5ee8 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs @@ -253,9 +253,9 @@ public InputAction ToInputAction() /// /// Thrown if this input action reference is part of an /// input actions asset and mutating it would have side-effects on the projects assets. - [System.Diagnostics.Conditional("UNITY_EDITOR")] private void CheckImmutableReference() { + #if UNITY_EDITOR // Note that we do a lot of checking here, but it is only for a rather slim (unintended) use case in // editor and not in final builds. The alternative would be to set a non-serialized field on the reference // when importing assets which would simplify this class, but it adds complexity to import stage and @@ -300,6 +300,7 @@ static bool CanSetReference(InputActionReference reference) "in-memory instance or serialize it as a separate asset if it " + "survive domain reloads."); } + #endif // UNITY_EDITOR } } } From 22be817ae05b38fca4d2e164105b5c7591b30178 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Akan=20Sidenvall?= Date: Mon, 6 Oct 2025 09:47:38 +0200 Subject: [PATCH 14/27] Strengthened existing test by adding verification that reference is removed. --- Assets/Tests/InputSystem/CoreTests_Actions.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Assets/Tests/InputSystem/CoreTests_Actions.cs b/Assets/Tests/InputSystem/CoreTests_Actions.cs index 85827972c8..2e28963bb0 100644 --- a/Assets/Tests/InputSystem/CoreTests_Actions.cs +++ b/Assets/Tests/InputSystem/CoreTests_Actions.cs @@ -8239,10 +8239,12 @@ public void Actions_CanRemoveActionMapFromAsset() { var asset = ScriptableObject.CreateInstance(); - asset.AddActionMap(new InputActionMap("test")); + var map = new InputActionMap("test"); + asset.AddActionMap(map); asset.RemoveActionMap("test"); Assert.That(asset.actionMaps, Is.Empty); + Assert.That(map.asset, Is.Null); } [Test] From 007412e6e11ffb8bfe38f3a981a6296b7ca013a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Akan=20Sidenvall?= Date: Mon, 6 Oct 2025 09:48:02 +0200 Subject: [PATCH 15/27] Corrected and updated inline doc for InputActionReference invalidation --- .../InputSystem/Actions/InputActionReference.cs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs index 17dfdb5ee8..8698b0efdb 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs @@ -52,10 +52,16 @@ public InputAction action { get { - // Note that we check m_Action since it would be null if we haven't yet resolved the action. - // We also need to check that m_Action.actionMap isn't null since it would be null if the - // action was deleted. Additionally we need to check m_Asset since it could also have been deleted. - if (m_Action != null && m_Action.actionMap != null && m_Asset) + // Note that we need to check multiple things here that could invalidate the validity of the reference: + // 1) m_Action != null, this indicates if we have a resolved cached reference. + // 2) m_Action.actionMap != null, this would fail if the action has been removed from an action map + // and converted into a "singleton action". This would render the reference invalid since the action + // is no longer indirectly bound to m_Asset. + // 3) m_Action.actionMap.asset == m_Asset, needs to be checked to make sure that its action map + // have not been moved to another asset which would invalidate the reference since reference is + // defined by action GUID and asset reference. + // 4) m_Asset, a Unity object life-time check that would fail if the asset has been deleted. + if (m_Action != null && m_Action.actionMap != null && m_Action.actionMap.asset == m_Asset && m_Asset) return m_Action; // Attempt to resolve action based on asset and GUID. From a146ecdf664746d08c579e4cb3d639dedc70b1fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Akan=20Sidenvall?= Date: Mon, 6 Oct 2025 09:48:41 +0200 Subject: [PATCH 16/27] Simplified InputSystemProvider since there is no reason at all to use InputActionReference unless there is a UI to configure it --- .../Plugins/InputForUI/InputSystemProvider.cs | 116 +++++++----------- 1 file changed, 46 insertions(+), 70 deletions(-) diff --git a/Packages/com.unity.inputsystem/InputSystem/Plugins/InputForUI/InputSystemProvider.cs b/Packages/com.unity.inputsystem/InputSystem/Plugins/InputForUI/InputSystemProvider.cs index 98dec05a6b..c92abe6643 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Plugins/InputForUI/InputSystemProvider.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Plugins/InputForUI/InputSystemProvider.cs @@ -18,14 +18,18 @@ internal class InputSystemProvider : IEventProviderImpl DefaultInputActions m_DefaultInputActions; InputActionAsset m_InputActionAsset; - InputActionReference m_PointAction; - InputActionReference m_MoveAction; - InputActionReference m_SubmitAction; - InputActionReference m_CancelAction; - InputActionReference m_LeftClickAction; - InputActionReference m_MiddleClickAction; - InputActionReference m_RightClickAction; - InputActionReference m_ScrollWheelAction; + // Note that these are plain action references instead since InputActionReference do + // not provide any value when this integration doesn't have any UI. If this integration + // later gets a UI replace these by InputActionReference and remember to check for e.g. + // m_ActionReference != null && m_ActionReference.action != null. + InputAction m_PointAction; + InputAction m_MoveAction; + InputAction m_SubmitAction; + InputAction m_CancelAction; + InputAction m_LeftClickAction; + InputAction m_MiddleClickAction; + InputAction m_RightClickAction; + InputAction m_ScrollWheelAction; InputAction m_NextPreviousAction; @@ -221,7 +225,7 @@ InputDevice GetActiveDeviceFromDirection(NavigationEvent.Direction direction) case NavigationEvent.Direction.Right: case NavigationEvent.Direction.Down: if (m_MoveAction != null) - return m_MoveAction.action.activeControl.device; + return m_MoveAction.activeControl.device; break; case NavigationEvent.Direction.Next: case NavigationEvent.Direction.Previous: @@ -242,9 +246,9 @@ InputDevice GetActiveDeviceFromDirection(NavigationEvent.Direction direction) if (m_MoveAction == null) return (default, default); - var move = m_MoveAction.action.ReadValue(); + var move = m_MoveAction.ReadValue(); // Check if the action was "pressed" this frame to deal with repeating events - var axisWasPressed = m_MoveAction.action.WasPressedThisFrame(); + var axisWasPressed = m_MoveAction.WasPressedThisFrame(); return (move, axisWasPressed); } @@ -610,43 +614,29 @@ void UnregisterFixedActions() } } + InputAction FindActionAndRegisterCallback(string actionNameOrId, Action callback = null) + { + var action = m_InputActionAsset.FindAction(actionNameOrId); + if (action != null && callback != null) + action.performed += callback; + return action; + } + void RegisterActions() { // Invoke potential lister observing registration s_OnRegisterActions?.Invoke(m_InputActionAsset); - m_PointAction = InputActionReference.Create(m_InputActionAsset.FindAction(Actions.PointAction)); - m_MoveAction = InputActionReference.Create(m_InputActionAsset.FindAction(Actions.MoveAction)); - m_SubmitAction = InputActionReference.Create(m_InputActionAsset.FindAction(Actions.SubmitAction)); - m_CancelAction = InputActionReference.Create(m_InputActionAsset.FindAction(Actions.CancelAction)); - m_LeftClickAction = InputActionReference.Create(m_InputActionAsset.FindAction(Actions.LeftClickAction)); - m_MiddleClickAction = InputActionReference.Create(m_InputActionAsset.FindAction(Actions.MiddleClickAction)); - m_RightClickAction = InputActionReference.Create(m_InputActionAsset.FindAction(Actions.RightClickAction)); - m_ScrollWheelAction = InputActionReference.Create(m_InputActionAsset.FindAction(Actions.ScrollWheelAction)); - - if (m_PointAction != null && m_PointAction.action != null) - m_PointAction.action.performed += OnPointerPerformed; - - if (m_SubmitAction != null && m_SubmitAction.action != null) - m_SubmitAction.action.performed += OnSubmitPerformed; - - if (m_CancelAction != null && m_CancelAction.action != null) - m_CancelAction.action.performed += OnCancelPerformed; - - if (m_LeftClickAction != null && m_LeftClickAction.action != null) - m_LeftClickAction.action.performed += OnLeftClickPerformed; - - if (m_MiddleClickAction != null && m_MiddleClickAction.action != null) - m_MiddleClickAction.action.performed += OnMiddleClickPerformed; - - if (m_RightClickAction != null && m_RightClickAction.action != null) - m_RightClickAction.action.performed += OnRightClickPerformed; - - if (m_ScrollWheelAction != null && m_ScrollWheelAction.action != null) - m_ScrollWheelAction.action.performed += OnScrollWheelPerformed; + m_PointAction = FindActionAndRegisterCallback(Actions.PointAction, OnPointerPerformed); + m_MoveAction = FindActionAndRegisterCallback(Actions.MoveAction); // No callback for this action + m_SubmitAction = FindActionAndRegisterCallback(Actions.SubmitAction, OnSubmitPerformed); + m_CancelAction = FindActionAndRegisterCallback(Actions.CancelAction, OnCancelPerformed); + m_LeftClickAction = FindActionAndRegisterCallback(Actions.LeftClickAction, OnLeftClickPerformed); + m_MiddleClickAction = FindActionAndRegisterCallback(Actions.MiddleClickAction, OnMiddleClickPerformed); + m_RightClickAction = FindActionAndRegisterCallback(Actions.RightClickAction, OnRightClickPerformed); + m_ScrollWheelAction = FindActionAndRegisterCallback(Actions.ScrollWheelAction, OnScrollWheelPerformed); // When adding new actions, don't forget to add them to UnregisterActions - if (InputSystem.actions == null) { // If we've not loaded a user-created set of actions, just enable the UI actions from our defaults. @@ -656,37 +646,23 @@ void RegisterActions() m_InputActionAsset.Enable(); } - void UnregisterActions() + void UnregisterAction(ref InputAction action, Action callback = null) { - if (m_PointAction != null && m_PointAction.action != null) - m_PointAction.action.performed -= OnPointerPerformed; - - if (m_SubmitAction != null && m_SubmitAction.action != null) - m_SubmitAction.action.performed -= OnSubmitPerformed; - - if (m_CancelAction != null && m_CancelAction.action != null) - m_CancelAction.action.performed -= OnCancelPerformed; - - if (m_LeftClickAction != null && m_LeftClickAction.action != null) - m_LeftClickAction.action.performed -= OnLeftClickPerformed; - - if (m_MiddleClickAction != null && m_MiddleClickAction.action != null) - m_MiddleClickAction.action.performed -= OnMiddleClickPerformed; - - if (m_RightClickAction != null && m_RightClickAction.action != null) - m_RightClickAction.action.performed -= OnRightClickPerformed; - - if (m_ScrollWheelAction != null && m_ScrollWheelAction.action != null) - m_ScrollWheelAction.action.performed -= OnScrollWheelPerformed; + if (action != null && callback != null) + action.performed -= callback; + action = null; + } - m_PointAction = null; - m_MoveAction = null; - m_SubmitAction = null; - m_CancelAction = null; - m_LeftClickAction = null; - m_MiddleClickAction = null; - m_RightClickAction = null; - m_ScrollWheelAction = null; + void UnregisterActions() + { + UnregisterAction(ref m_PointAction, OnPointerPerformed); + UnregisterAction(ref m_MoveAction); // No callback for this action + UnregisterAction(ref m_SubmitAction, OnSubmitPerformed); + UnregisterAction(ref m_CancelAction, OnCancelPerformed); + UnregisterAction(ref m_LeftClickAction, OnLeftClickPerformed); + UnregisterAction(ref m_MiddleClickAction, OnMiddleClickPerformed); + UnregisterAction(ref m_RightClickAction, OnRightClickPerformed); + UnregisterAction(ref m_ScrollWheelAction, OnScrollWheelPerformed); if (m_InputActionAsset != null) m_InputActionAsset.Disable(); From 3af6d81174aa55a42a3aeeca01a75113ea553240 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Akan=20Sidenvall?= Date: Mon, 6 Oct 2025 09:49:09 +0200 Subject: [PATCH 17/27] Undo bug fix of RemoveActionMap which was correct and filed bug was As Designed --- Packages/com.unity.inputsystem/CHANGELOG.md | 1 - .../InputSystem/Actions/InputActionSetupExtensions.cs | 7 ------- 2 files changed, 8 deletions(-) diff --git a/Packages/com.unity.inputsystem/CHANGELOG.md b/Packages/com.unity.inputsystem/CHANGELOG.md index 9926ef3d0d..a684729db3 100644 --- a/Packages/com.unity.inputsystem/CHANGELOG.md +++ b/Packages/com.unity.inputsystem/CHANGELOG.md @@ -37,7 +37,6 @@ however, it has to be formatted properly to pass verification tests. - Fixed an issue where assigning a ``MonoBehaviour` field of type `InputActionReference` to an instance stored in `.inputactions` asset via `InputActionReference.Set` in playmode corrupts the originally referenced asset. (ISXB-1692). - Fixed an issue where `InputActionReference.Create(null)` returns `null` instead "of a reference object to the given action" as documented behaviour for the API. (ISXB-1693) - Fixed an issue where `InputActtionReference.Set(null)` does not update the cached input action reference nor the `ScriptableObject.name` leading to incorrect action being returned and reference showing up as the path to the previously assigned action in Inspector. (ISXB-1694) -- Fixed an issue where `InputActionSetupExtension.RemoveActionMap` doesn't remove actions within the map, leaving them with stale references back to the map if accessing map from action. (ISXB-1699) - Fixed an issue where Inspector field of type `InputActionReference` isn't reflecting broken reference in case action is deleted from asset, action map is deleted from asset, or the asset itself is deleted. (ISXB-1701) ## [1.14.2] - 2025-08-05 diff --git a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionSetupExtensions.cs b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionSetupExtensions.cs index e6e1b3b1f8..a1c9a9ae52 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionSetupExtensions.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionSetupExtensions.cs @@ -104,13 +104,6 @@ public static void RemoveActionMap(this InputActionAsset asset, InputActionMap m if (map.m_Asset != asset) return; - // First remove all actions from map (They become "singleton actions" or may get collected if not referenced). - if (map.m_Actions != null) - { - for (var i = map.m_Actions.Length - 1; i >= 0; --i) - RemoveAction(map.m_Actions[i]); - } - ArrayHelpers.Erase(ref asset.m_ActionMaps, map); map.m_Asset = null; asset.OnSetupChanged(); From a499b4039db190365ad5302cc80a0c3c57ffb108 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Akan=20Sidenvall?= Date: Mon, 6 Oct 2025 09:49:49 +0200 Subject: [PATCH 18/27] Improved inline comment --- Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs b/Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs index a365acd95c..dfb3b6b2a7 100644 --- a/Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs +++ b/Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs @@ -179,7 +179,9 @@ public void Actions_Reference_ShouldReevaluateIfAssociatedEntityIsDeleted(Type t Assert.That(reference.ToDisplayName(), Is.EqualTo("map1/action1")); Assert.That(reference.ToString(), Is.EqualTo(":map1/action1")); - // Delete the referenced action directly or indirectly + // Remove the referenced action directly or indirectly. Note that this doesn't destroy an action or action map + // since they are regular reference types. However, an InputActionReference is based on asset an and action ID + // So if a map is removed from an asset but remains valid in memory it is no longer a valid reference. if (typeToDelete == typeof(InputAction)) asset.RemoveAction("map1/action1"); else if (typeToDelete == typeof(InputActionMap)) From 4dfc4f4ed96b177e8fd8eb6e15eaa22f57f70ac2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Akan=20Sidenvall?= Date: Mon, 6 Oct 2025 11:03:33 +0200 Subject: [PATCH 19/27] Added missing guard to mimic inclusion of test based on PWA. --- .../InputActionReferencePropertyDrawerEditorTests.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Assets/Tests/InputSystem.Editor/InputActionReferencePropertyDrawerEditorTests.cs b/Assets/Tests/InputSystem.Editor/InputActionReferencePropertyDrawerEditorTests.cs index 951372043c..8fffb97f8d 100644 --- a/Assets/Tests/InputSystem.Editor/InputActionReferencePropertyDrawerEditorTests.cs +++ b/Assets/Tests/InputSystem.Editor/InputActionReferencePropertyDrawerEditorTests.cs @@ -1,3 +1,5 @@ +#if UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS // Mimic implementation guard + using System; using System.Collections; using NUnit.Framework; @@ -162,3 +164,5 @@ public IEnumerator FieldObjectIsNullWhenReferenceIsInvalid() Assert.That(fieldObj, Is.Null); } } + +#endif // UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS From 8c008bc1f08f17714057390aab85dcbf128af667 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Akan=20Sidenvall?= Date: Mon, 6 Oct 2025 11:16:03 +0200 Subject: [PATCH 20/27] Clarified in xmldoc that removing an action map still leaves all contained actions intact. --- .../InputSystem/Actions/InputActionSetupExtensions.cs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionSetupExtensions.cs b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionSetupExtensions.cs index a1c9a9ae52..df13f2979e 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionSetupExtensions.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionSetupExtensions.cs @@ -81,9 +81,13 @@ public static void AddActionMap(this InputActionAsset asset, InputActionMap map) /// /// Remove the given action map from the asset. /// - /// Asset to add the action map to. + /// The asset to remove the action from. /// An action map. If the given map is not part of the asset, the method /// does nothing. + /// Any action contained in the removed map will remain unchanged and still be + /// associated with its parent input action map. If this operation is successful, map will no + /// longer be associated with and will + /// return null. /// or is null. /// is currently enabled (see ) or is part of an that has s @@ -116,6 +120,10 @@ public static void RemoveActionMap(this InputActionAsset asset, InputActionMap m /// The name or ID (see ) of a map in the /// asset. Note that lookup is case-insensitive. If no map with the given name or ID is found, /// the method does nothing. + /// Any action contained in the removed map will remain unchanged and still be + /// associated with its parent input action map. If this operation is successful, map will no + /// longer be associated with and will + /// return null. /// or is null. /// The map referenced by is currently enabled /// (see ). From cce5b54ce0b1a10317ea39b2cf70438ea0269be5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5kan=20Sidenvall?= Date: Thu, 9 Oct 2025 20:50:01 +0200 Subject: [PATCH 21/27] Update Packages/com.unity.inputsystem/CHANGELOG.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- Packages/com.unity.inputsystem/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Packages/com.unity.inputsystem/CHANGELOG.md b/Packages/com.unity.inputsystem/CHANGELOG.md index 85fe026b77..6896984a6e 100644 --- a/Packages/com.unity.inputsystem/CHANGELOG.md +++ b/Packages/com.unity.inputsystem/CHANGELOG.md @@ -37,7 +37,7 @@ however, it has to be formatted properly to pass verification tests. - Fixed an issue where `InputActionSetupExtensions.Remove` will result in `NullReferenceException` if the action being removed has no bindings. (ISXB-1688). - Fixed an issue where assigning a ``MonoBehaviour` field of type `InputActionReference` to an instance stored in `.inputactions` asset via `InputActionReference.Set` in playmode corrupts the originally referenced asset. (ISXB-1692). - Fixed an issue where `InputActionReference.Create(null)` returns `null` instead "of a reference object to the given action" as documented behaviour for the API. (ISXB-1693) -- Fixed an issue where `InputActtionReference.Set(null)` does not update the cached input action reference nor the `ScriptableObject.name` leading to incorrect action being returned and reference showing up as the path to the previously assigned action in Inspector. (ISXB-1694) +- Fixed an issue where `InputActionReference.Set(null)` does not update the cached input action reference nor the `ScriptableObject.name` leading to incorrect action being returned and reference showing up as the path to the previously assigned action in Inspector. (ISXB-1694) - Fixed an issue where Inspector field of type `InputActionReference` isn't reflecting broken reference in case action is deleted from asset, action map is deleted from asset, or the asset itself is deleted. (ISXB-1701) ## [1.14.2] - 2025-08-05 From eda88d1ead0f8a410599c3c1253106d623717d57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5kan=20Sidenvall?= Date: Thu, 9 Oct 2025 20:53:15 +0200 Subject: [PATCH 22/27] Update Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../InputSystem/Actions/InputActionReference.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs index 8698b0efdb..7ea9585ef7 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs @@ -157,8 +157,8 @@ public override string ToString() if (value == null) return base.ToString(); if (value.actionMap != null) - return $"{m_Asset.name}:{value.actionMap.name}/{value.name}"; - return $"{m_Asset.name}:{m_ActionId}"; + return m_Asset != null ? $"{m_Asset.name}:{value.actionMap.name}/{value.name}" : $"{value.actionMap.name}/{value.name}"; + return m_Asset != null ? $"{m_Asset.name}:{m_ActionId}" : m_ActionId; } private static string GetDisplayName(InputAction action) From 3e3731db8c9a83c9019c42058f1d9ac66010f301 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Akan=20Sidenvall?= Date: Thu, 9 Oct 2025 21:04:31 +0200 Subject: [PATCH 23/27] Fixed warnings generated by editor --- .../InputSystem.Editor/InputActionReferenceEditorTests.cs | 2 +- .../InputActionReferencePropertyDrawerEditorTests.cs | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/Assets/Tests/InputSystem.Editor/InputActionReferenceEditorTests.cs b/Assets/Tests/InputSystem.Editor/InputActionReferenceEditorTests.cs index 66c3463e0d..b8ae447c81 100644 --- a/Assets/Tests/InputSystem.Editor/InputActionReferenceEditorTests.cs +++ b/Assets/Tests/InputSystem.Editor/InputActionReferenceEditorTests.cs @@ -98,7 +98,7 @@ private void DisableDomainReloads() EditorPrefsTestUtils.DisableDomainReload(); } - private static InputActionBehaviour GetBehaviour() => Object.FindObjectOfType(); + private static InputActionBehaviour GetBehaviour() => Object.FindFirstObjectByType(); private static InputActionAsset GetAsset() => AssetDatabase.LoadAssetAtPath(assetPath); // For unclear reason, NUnit fails to assert throwing exceptions after transition into play-mode. diff --git a/Assets/Tests/InputSystem.Editor/InputActionReferencePropertyDrawerEditorTests.cs b/Assets/Tests/InputSystem.Editor/InputActionReferencePropertyDrawerEditorTests.cs index 8fffb97f8d..4a976a776f 100644 --- a/Assets/Tests/InputSystem.Editor/InputActionReferencePropertyDrawerEditorTests.cs +++ b/Assets/Tests/InputSystem.Editor/InputActionReferencePropertyDrawerEditorTests.cs @@ -106,6 +106,7 @@ public IEnumerator ExecutesWithoutExceptions() var drawer = new InputActionReferencePropertyDrawer(); yield return RenderDrawer(drawer); Assert.That(onGuiCalled, Is.True); + Assert.That(fieldCalled, Is.False); } [UnityTest] @@ -115,6 +116,7 @@ public IEnumerator FieldObjectIsNullWhenReferenceIsNull() var drawer = new InputActionReferencePropertyDrawer(); yield return RenderDrawer(drawer, true); Assert.That(onGuiCalled, Is.True); + Assert.That(fieldCalled, Is.True); Assert.That(fieldObj, Is.Null); } @@ -136,6 +138,7 @@ public IEnumerator FieldObjectIsReferenceWhenReferenceIsValid() var drawer = new InputActionReferencePropertyDrawer(); yield return RenderDrawer(drawer, true); Assert.That(onGuiCalled, Is.True); + Assert.That(fieldCalled, Is.True); Assert.That(fieldObj, Is.SameAs(reference)); } @@ -161,6 +164,7 @@ public IEnumerator FieldObjectIsNullWhenReferenceIsInvalid() var drawer = new InputActionReferencePropertyDrawer(); yield return RenderDrawer(drawer, true); Assert.That(onGuiCalled, Is.True); + Assert.That(fieldCalled, Is.True); Assert.That(fieldObj, Is.Null); } } From 0c0ddc9d4f58f2bf962db43e53995e67f08e8782 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Akan=20Sidenvall?= Date: Thu, 9 Oct 2025 21:19:08 +0200 Subject: [PATCH 24/27] Eliminated warning and simplified code --- .../InputSystem/Actions/InputActionReference.cs | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs index 7ea9585ef7..0a170a7501 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs @@ -268,15 +268,8 @@ private void CheckImmutableReference() // is more difficult to assess from a asset version portability perspective. static bool CanSetReference(InputActionReference reference) { - // If the asset isn't persisted it cannot be part of an InputActionAsset file. - // We use this check first since it can allow us to avoid GC pressure. - var instanceID = reference.GetInstanceID(); - var isPersistent = UnityEditor.AssetDatabase.TryGetGUIDAndLocalFileIdentifier(instanceID, out _, out long _); - if (!isPersistent) - return true; - // "Immutable" input action references are always sub-assets of InputActionAsset. - var isSubAsset = UnityEditor.AssetDatabase.IsSubAsset(instanceID); + var isSubAsset = UnityEditor.AssetDatabase.IsSubAsset(reference); if (!isSubAsset) return true; @@ -288,7 +281,7 @@ static bool CanSetReference(InputActionReference reference) // If we cannot get the main asset we cannot be a persisted asset within an InputActionAsset. // Also we check that it is the expected type. var mainAsset = UnityEditor.AssetDatabase.LoadMainAssetAtPath(path); - if (mainAsset == null) + if (!mainAsset) return true; // We can only allow setting the reference if it is not part of an persisted InputActionAsset. From 9c7cb4442a4ccc0307d999c373ce654c74e54943 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Akan=20Sidenvall?= Date: Mon, 13 Oct 2025 12:34:35 +0200 Subject: [PATCH 25/27] Additional fix to not corrupt resources on Unity versions before Search API and project-wide actions were introduced. Minor test cleanup. --- .../InputSystem/CoreTests_Actions_Reference.cs | 5 +---- .../InputActionReferencePropertyDrawer.cs | 15 ++++++++++++++- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs b/Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs index dfb3b6b2a7..cd287734bf 100644 --- a/Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs +++ b/Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs @@ -189,13 +189,10 @@ public void Actions_Reference_ShouldReevaluateIfAssociatedEntityIsDeleted(Type t else if (typeToDelete == typeof(InputActionAsset)) UnityEngine.Object.DestroyImmediate(asset); - // TODO reference need to react to this Assert.That(reference.action, Is.Null); Assert.That(reference.asset, Is.SameAs(asset)); Assert.That(reference.name, Is.EqualTo("map1/action1")); // Unexpected when no longer existing Assert.That(reference.ToDisplayName(), Is.EqualTo("map1/action1")); // Unexpected when no longer existing - //Assert.That(reference.ToString(), Is.EqualTo(":" + new Guid(action1.m_Id))); // Unexpected when no longer existing + Assert.That(reference.ToString(), Is.EqualTo($"map1/action1 ({typeof(InputActionReference).FullName})")); // Unexpected when no longer existing } - - // TODO Make an undo resolve test that destroys reference sub-asset, breaks reference, then undo and reference is valid again. } diff --git a/Packages/com.unity.inputsystem/InputSystem/Editor/PropertyDrawers/InputActionReferencePropertyDrawer.cs b/Packages/com.unity.inputsystem/InputSystem/Editor/PropertyDrawers/InputActionReferencePropertyDrawer.cs index 8f459e035d..c9c9df4d31 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Editor/PropertyDrawers/InputActionReferencePropertyDrawer.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Editor/PropertyDrawers/InputActionReferencePropertyDrawer.cs @@ -1,9 +1,12 @@ // Note: If not UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS we do not use a custom property drawer and // picker for InputActionReferences but rather rely on default (classic) object picker. -#if UNITY_EDITOR && UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS +#if UNITY_EDITOR using UnityEditor; +using UnityEditor.UIElements; +#if UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS using UnityEditor.Search; using ObjectField = UnityEditor.Search.ObjectField; +#endif // UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS namespace UnityEngine.InputSystem.Editor { @@ -13,12 +16,14 @@ namespace UnityEngine.InputSystem.Editor [CustomPropertyDrawer(typeof(InputActionReference))] internal sealed class InputActionReferencePropertyDrawer : PropertyDrawer { + #if UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS // Custom search providers for asset-based, and project-wide actions input action reference sub-assets. private readonly SearchContext m_Context = UnityEditor.Search.SearchService.CreateContext(new[] { InputActionReferenceSearchProviders.CreateInputActionReferenceSearchProviderForAssets(), InputActionReferenceSearchProviders.CreateInputActionReferenceSearchProviderForProjectWideActions(), }, string.Empty, SearchConstants.PickerSearchFlags); + #endif public void OnGUI(Rect position, SerializedProperty property, GUIContent label, System.Func doField = null) @@ -39,12 +44,20 @@ public void OnGUI(Rect position, SerializedProperty property, GUIContent label, if (currentReference && currentReference.action == null) obj = null; // none/missing + #if UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS // Pick an InputActionReference using custom picker. We need to use this overload taking an object // in order to be in control of the actual assignment to the serialized property so we can substitute. // We allow substituting the object field here to at least enable some test coverage. var candidate = doField != null ? doField(position, obj, typeof(InputActionReference), label) : ObjectField.DoObjectField(position, obj, typeof(InputActionReference), label, m_Context, SearchConstants.PickerViewFlags); + #else + // Before Search API was introduced we need to do this differently to be able to use different + // presentation and modification object as well as to use the same assignment based on a new reference + // object to guarantee that there are no side-effects. + var candidate = doField != null ? doField(position, obj, typeof(InputActionReference), label) + : EditorGUI.ObjectField(position, new GUIContent(label), obj, typeof(InputActionReference), true); + #endif // Only assign the value was actually changed by the user. if (EditorGUI.EndChangeCheck() && !Equals(candidate, current)) From d96ebd1ce279534c9b6f2f8d63066bf6ddfc799f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5kan=20Sidenvall?= Date: Tue, 14 Oct 2025 09:57:49 +0200 Subject: [PATCH 26/27] Update CHANGELOG.md Moved entries to the Unreleased version in CHANGELOG.md since merge had put them in the wrong place. --- Packages/com.unity.inputsystem/CHANGELOG.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Packages/com.unity.inputsystem/CHANGELOG.md b/Packages/com.unity.inputsystem/CHANGELOG.md index 39cc8874b9..ee8d367a31 100644 --- a/Packages/com.unity.inputsystem/CHANGELOG.md +++ b/Packages/com.unity.inputsystem/CHANGELOG.md @@ -14,6 +14,12 @@ however, it has to be formatted properly to pass verification tests. - Fixed warnings being generated on Unity 6.3 (beta). (ISXB-1718). - Fixed an issue in `DeltaStateEvent.From` where unsafe code would throw exception or crash if internal pointer `currentStatePtr` was `null`. [ISXB-1637](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1637). - Fixed an issue in `InputTestFixture.Set` where attempting to change state of a device not belonging to the test fixture context would result in null pointer exception or crash. [ISXB-1637](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1637). +- Fixed an issue in `InputActionReference` where `Set` was not updating the cached action leading to incorrect evaluation of `InputActionReference.action` after first being cached, then set, then read again. This could lead to reference not being reset if set programmatically during play-mode. [ISXB-1584](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1584). +- Fixed an issue where `InputActionSetupExtensions.Remove` will result in `NullReferenceException` if the action being removed has no bindings. (ISXB-1688). +- Fixed an issue where assigning a ``MonoBehaviour` field of type `InputActionReference` to an instance stored in `.inputactions` asset via `InputActionReference.Set` in playmode corrupts the originally referenced asset. (ISXB-1692). +- Fixed an issue where `InputActionReference.Create(null)` returns `null` instead "of a reference object to the given action" as documented behaviour for the API. (ISXB-1693) +- Fixed an issue where `InputActionReference.Set(null)` does not update the cached input action reference nor the `ScriptableObject.name` leading to incorrect action being returned and reference showing up as the path to the previously assigned action in Inspector. (ISXB-1694) +- Fixed an issue where Inspector field of type `InputActionReference` isn't reflecting broken reference in case action is deleted from asset, action map is deleted from asset, or the asset itself is deleted. (ISXB-1701) ## [1.15.0] - 2025-10-03 @@ -36,12 +42,6 @@ however, it has to be formatted properly to pass verification tests. - Fixed an issue where the onAnyButtonPress callback would be triggered multiple times during unrelated events when a button is held down. See [ISXB-1005](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1005). - Fixed InputControl picker not updating correctly when the Input Actions Window was dirty. [ISXB-1221](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1221) - Fixed formatting issues on processor documentation page. -- Fixed an issue in `InputActionReference` where `Set` was not updating the cached action leading to incorrect evaluation of `InputActionReference.action` after first being cached, then set, then read again. This could lead to reference not being reset if set programmatically during play-mode. [ISXB-1584](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1584). -- Fixed an issue where `InputActionSetupExtensions.Remove` will result in `NullReferenceException` if the action being removed has no bindings. (ISXB-1688). -- Fixed an issue where assigning a ``MonoBehaviour` field of type `InputActionReference` to an instance stored in `.inputactions` asset via `InputActionReference.Set` in playmode corrupts the originally referenced asset. (ISXB-1692). -- Fixed an issue where `InputActionReference.Create(null)` returns `null` instead "of a reference object to the given action" as documented behaviour for the API. (ISXB-1693) -- Fixed an issue where `InputActionReference.Set(null)` does not update the cached input action reference nor the `ScriptableObject.name` leading to incorrect action being returned and reference showing up as the path to the previously assigned action in Inspector. (ISXB-1694) -- Fixed an issue where Inspector field of type `InputActionReference` isn't reflecting broken reference in case action is deleted from asset, action map is deleted from asset, or the asset itself is deleted. (ISXB-1701) ## [1.14.2] - 2025-08-05 From 3b9cceb87e42944374f2353ee7aee773c6985111 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Akan=20Sidenvall?= Date: Mon, 20 Oct 2025 16:20:11 +0200 Subject: [PATCH 27/27] Fixed typo in property name in RebindActionParameterUI. Improved exception message when attempting to use Set on a reference backed by a .inputactions --- Assets/Samples/RebindingUI/RebindActionParameterUI.cs | 2 +- .../InputSystem/Actions/InputActionReference.cs | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/Assets/Samples/RebindingUI/RebindActionParameterUI.cs b/Assets/Samples/RebindingUI/RebindActionParameterUI.cs index 55b0fdb209..bf88286231 100644 --- a/Assets/Samples/RebindingUI/RebindActionParameterUI.cs +++ b/Assets/Samples/RebindingUI/RebindActionParameterUI.cs @@ -32,7 +32,7 @@ public string bindingId /// /// The preference key to be used for persistence. /// - public string mPreferenceKey + public string preferenceKey { get => m_PreferenceKey; set => m_PreferenceKey = value; diff --git a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs index 0a170a7501..f55c729f27 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionReference.cs @@ -293,10 +293,11 @@ static bool CanSetReference(InputActionReference reference) // This is not needed for players since scriptable objects aren't serialized back from within a player. if (!CanSetReference(this)) { - throw new InvalidOperationException("Attempting to mutate an immutable InputActionReference instance. " + - "This is not allowed since it would modify the source asset." + - "Instead use InputActionReference.Create(action) to create a new " + - "in-memory instance or serialize it as a separate asset if it " + + throw new InvalidOperationException("Attempting to modify an immutable InputActionReference instance " + + "that is part of an .inputactions asset. This is not allowed since it would modify the source " + + "asset in which the reference is serialized and potentially corrupt it. " + + "Instead use InputActionReference.Create(action) to create a new mutable " + + "in-memory instance or serialize it as a separate asset if the intent is for changes to " + "survive domain reloads."); } #endif // UNITY_EDITOR