FIX: ISXB-1584, ISXB-1688, ISXB-1692, ISXB-1693, ISXB-1694, ISXB-1701 InputActionReference bug fixes and improved test coverage. (#2248)

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Author: ekcoh

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<InputActionReference>(
+            FindObjectsInactive.Include, FindObjectsSortMode.InstanceID));
+        DumpReferences(sb, "All objects:", Resources.FindObjectsOfTypeAll<InputActionReference>());
+        Debug.Log(sb.ToString());
+    }
+
+    [UnityEditor.MenuItem("QA Tools/Dump Input Action References to Console", false, 100)]
+    private static void Dump()
+    {
+        DumpReferences();
+    }
+}

Assets/Editor/DumpInputActionReferences.cs.meta

@@ -32,7 +32,7 @@ public string bindingId
         /// <summary>
         /// The preference key to be used for persistence.
         /// </summary>
-        public string mPreferenceKey
+        public string preferenceKey
         {
             get => m_PreferenceKey;
             set => m_PreferenceKey = value;

Assets/Samples/RebindingUI/RebindActionParameterUI.cs

@@ -0,0 +1,76 @@
+using UnityEditor;
+
+namespace Tests.InputSystem.Editor
+{
+    /// <summary>
+    /// Utility to simplify editor tests with respect to editor preferences.
+    /// </summary>
+    internal static class EditorPrefsTestUtils
+    {
+        private const string EnterPlayModeOptionsEnabledKey = "EnterPlayModeOptionsEnabled";
+        private const string EnterPlayModeOptionsKey = "EnterPlayModeOptions";
+
+        private static bool _savedEnterPlayModeOptionsEnabled;
+        private static int _savedEnterPlayModeOptions;
+
+        /// <summary>
+        /// Call this from a tests SetUp routine to save editor preferences so they can be restored after the test.
+        /// </summary>
+        public static void SaveEditorPrefs()
+        {
+            _savedEnterPlayModeOptionsEnabled = EditorPrefs.GetBool(EnterPlayModeOptionsEnabledKey, false);
+            _savedEnterPlayModeOptions = EditorPrefs.GetInt(EnterPlayModeOptionsKey, (int)EnterPlayModeOptions.None);
+        }
+
+        /// <summary>
+        /// Call this from a tests TearDown routine to restore editor preferences to the state it had before the test.
+        /// </summary>
+        /// <remarks>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.</remarks>
+        public static void RestoreEditorPrefs()
+        {
+            EditorPrefs.SetBool(EnterPlayModeOptionsEnabledKey, _savedEnterPlayModeOptionsEnabled);
+            EditorPrefs.SetInt(EnterPlayModeOptionsKey, _savedEnterPlayModeOptions);
+        }
+
+        /// <summary>
+        /// Returns whether domain reloads are disabled.
+        /// </summary>
+        /// <returns>true if domain reloads have been disabled, else false.</returns>
+        public static bool IsDomainReloadsDisabled()
+        {
+            return EditorPrefs.GetBool(EnterPlayModeOptionsEnabledKey, false) &&
+                (EditorPrefs.GetInt(EnterPlayModeOptionsKey, (int)EnterPlayModeOptions.None) &
+                    (int)EnterPlayModeOptions.DisableDomainReload) != 0;
+        }
+
+        /// <summary>
+        /// Returns whether scene reloads are disabled.
+        /// </summary>
+        /// <returns>true if scene reloads have been disabled, else false.</returns>
+        public static bool IsSceneReloadsDisabled()
+        {
+            return EditorPrefs.GetBool(EnterPlayModeOptionsEnabledKey, false) &&
+                (EditorPrefs.GetInt(EnterPlayModeOptionsKey, (int)EnterPlayModeOptions.None) &
+                    (int)EnterPlayModeOptions.DisableSceneReload) != 0;
+        }
+
+        /// <summary>
+        /// Call this from within a test to temporarily enable domain reload.
+        /// </summary>
+        public static void EnableDomainReload()
+        {
+            EditorPrefs.SetBool(EnterPlayModeOptionsEnabledKey, false);
+        }
+
+        /// <summary>
+        /// Call this from within a test to temporarily disable domain reload (and scene reloads).
+        /// </summary>
+        public static void DisableDomainReload()
+        {
+            EditorPrefs.SetInt(EnterPlayModeOptionsKey, (int)(EnterPlayModeOptions.DisableDomainReload |
+                EnterPlayModeOptions.DisableSceneReload));
+            EditorPrefs.SetBool(EnterPlayModeOptionsEnabledKey, true);
+        }
+    }
+}

Assets/Tests/InputSystem.Editor/EditorPrefsTestUtils.cs

@@ -0,0 +1,197 @@
+using System;
+using System.Collections;
+using System.Linq;
+using NUnit.Framework;
+using Tests.InputSystem.Editor;
+using UnityEditor;
+using UnityEditor.SceneManagement;
+using UnityEngine;
+using UnityEngine.InputSystem;
+using UnityEngine.InputSystem.Editor;
+using UnityEngine.SceneManagement;
+using UnityEngine.TestTools;
+using Object = UnityEngine.Object;
+
+/// <summary>
+/// Editor tests for <see cref="UnityEngine.InputSystem.InputActionReference"/>.
+/// </summary>
+/// <remarks>
+/// 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.
+/// </remarks>
+internal class InputActionReferenceEditorTestsWithScene
+{
+    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()
+    {
+        var asset = ScriptableObject.CreateInstance<InputActionAsset>();
+
+        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;
+
+        EditorPrefsTestUtils.SaveEditorPrefs();
+
+        m_Scene = EditorSceneManager.NewScene(NewSceneSetup.EmptyScene);
+        CreateAsset();
+
+        var go = new GameObject("Root");
+        var behaviour = go.AddComponent<InputActionBehaviour>();
+        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.FindFirstObjectByType<InputActionBehaviour>();
+    private static InputActionAsset GetAsset() => AssetDatabase.LoadAssetAtPath<InputActionAsset>(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<T>(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.");
+    }
+
+    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();
+
+        // 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<InvalidOperationException>(() => 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<InvalidOperationException>(() => obj.referenceAsField.Set(playModeAction));
+            AssertThrows<InvalidOperationException>(() => 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;
+    }
+}