Handle parent directory renaming whilst asset is being edited. (#1049)

* Added test to ensure that an exception is not thrown when Saving input asset and the parent directory was renamed. (case 1207527)

* Removed cached asset path. We now use the asset guid to get the path when requested. (case 1207527)

When the parent directory is renamed the path becomes invalid. Its much safer to use the asset guid and resolve the path when its requested. There is no continuous use of the path in code so it should not have any performance impact.

* Code Formatting

* Discard the unused argument.

Fixes a warning.

* Ignore warning CA1801

* Test the file has been modified instead of an exception not being thrown.

We should test for the behaviour we actually want.

* Added CHANGELOG entry

* Use the [Unreleased] header instead of a unreleased version and date

Author: karljj1

Assets/Tests/InputSystem/CoreTests_Editor.cs

@@ -545,6 +545,36 @@ public void Editor_InputAsset_CanReplaceBindingGroupThroughSerialization()
         Assert.That(map.bindings[0].groups, Is.EqualTo(""));
     }
 
+    [Test]
+    [Category("Editor")]
+    public void Editor_InputActionAssetManager_CanMoveAssetOnDisk()
+    {
+        const string kAssetPath = "Assets/DirectoryBeforeRename/InputAsset." + InputActionAsset.Extension;
+        const string kAssetPathAfterMove = "Assets/DirectoryAfterRename/InputAsset." + InputActionAsset.Extension;
+        const string kDefaultContents = "{}";
+
+        AssetDatabase.CreateFolder("Assets", "DirectoryBeforeRename");
+        File.WriteAllText(kAssetPath, kDefaultContents);
+        AssetDatabase.ImportAsset(kAssetPath);
+
+        var asset = AssetDatabase.LoadAssetAtPath<InputActionAsset>(kAssetPath);
+        Assert.NotNull(asset, "Could not load asset: " + kAssetPath);
+
+        var inputActionAssetManager = new InputActionAssetManager(asset);
+        inputActionAssetManager.Initialize();
+        inputActionAssetManager.onDirtyChanged = (bool dirty) => {};
+
+        FileUtil.MoveFileOrDirectory("Assets/DirectoryBeforeRename", "Assets/DirectoryAfterRename");
+        AssetDatabase.Refresh();
+
+        Assert.DoesNotThrow(() => inputActionAssetManager.SaveChangesToAsset());
+
+        var fileContents = File.ReadAllText(kAssetPathAfterMove);
+        Assert.AreNotEqual(kDefaultContents, fileContents, "Expected file contents to have been modified after SaveChangesToAsset was called.");
+
+        AssetDatabase.DeleteAsset("Assets/DirectoryAfterRename");
+    }
+
     private class MonoBehaviourWithEmbeddedAction : MonoBehaviour
     {
         public InputAction action;

Packages/com.unity.inputsystem/CHANGELOG.md

@@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 Due to package verification, the latest version below is the unpublished version and the date is meaningless.
 however, it has to be formatted properly to pass verification tests.
 
-## [1.0.0-preview.5] - 2020-12-12
+## [Unreleased]
 
 ### Changed
 
@@ -19,6 +19,7 @@ however, it has to be formatted properly to pass verification tests.
 - XR controllers and HMDs have proper display names in the UI again. This regressed in preview.4 such that all XR controllers were displayed as just "XR Controller" in the UI and all HMDs were displayed as "XR HMD".
 - `InputSystemUIInputModule` no longer generates GC heap garbage every time mouse events are processed.
 - Fixed a bug where an internal array helper method was corrupting array contents leading to bugs in both `InputUser` and `Touch`.
+- Fixed exception when saving changes to an Input Action asset and the parent directory has been renamed. ([case 1207527](https://issuetracker.unity3d.com/issues/input-system-console-errors-appear-when-you-save-input-action-asset-after-changing-the-name-of-the-folder-containing-it))
 
 #### Actions
 

Packages/com.unity.inputsystem/InputSystem/Editor/AssetEditor/InputActionAssetManager.cs

@@ -15,15 +15,24 @@ internal class InputActionAssetManager : IDisposable
         [SerializeField] internal InputActionAsset m_AssetObjectForEditing;
         [SerializeField] private InputActionAsset m_ImportedAssetObject;
         [SerializeField] private string m_AssetGUID;
-        [SerializeField] private string m_AssetPath;
         [SerializeField] private string m_ImportedAssetJson;
         [SerializeField] private bool m_IsDirty;
 
         private SerializedObject m_SerializedObject;
 
         public string guid => m_AssetGUID;
 
-        public string path { get => m_AssetPath; set => m_AssetPath = value; }
+        public string path
+        {
+            get
+            {
+                Debug.Assert(!string.IsNullOrEmpty(m_AssetGUID), "Asset GUID is empty");
+                var assetPath = AssetDatabase.GUIDToAssetPath(m_AssetGUID);
+                if (string.IsNullOrEmpty(assetPath))
+                    throw new InvalidOperationException("Could not determine asset path for " + m_AssetGUID);
+                return assetPath;
+            }
+        }
 
         public string name
         {
@@ -32,8 +41,8 @@ public string name
                 if (m_ImportedAssetObject != null)
                     return m_ImportedAssetObject.name;
 
-                if (!string.IsNullOrEmpty(m_AssetPath))
-                    return Path.GetFileNameWithoutExtension(m_AssetPath);
+                if (!string.IsNullOrEmpty(path))
+                    return Path.GetFileNameWithoutExtension(path);
 
                 return string.Empty;
             }
@@ -55,8 +64,7 @@ private InputActionAsset importedAsset
         public InputActionAssetManager(InputActionAsset inputActionAsset)
         {
             m_ImportedAssetObject = inputActionAsset;
-            m_AssetPath = AssetDatabase.GetAssetPath(importedAsset);
-            m_AssetGUID = AssetDatabase.AssetPathToGUID(m_AssetPath);
+            Debug.Assert(AssetDatabase.TryGetGUIDAndLocalFileIdentifier(importedAsset, out m_AssetGUID, out long _), $"Failed to get asset {inputActionAsset.name} GUID");
         }
 
         public SerializedObject serializedObject => m_SerializedObject;
@@ -123,13 +131,7 @@ public void Cleanup()
 
         public void LoadImportedObjectFromGuid()
         {
-            Debug.Assert(!string.IsNullOrEmpty(m_AssetGUID));
-
-            m_AssetPath = AssetDatabase.GUIDToAssetPath(m_AssetGUID);
-            if (string.IsNullOrEmpty(m_AssetPath))
-                throw new InvalidOperationException("Could not determine asset path for " + m_AssetGUID);
-
-            m_ImportedAssetObject = AssetDatabase.LoadAssetAtPath<InputActionAsset>(m_AssetPath);
+            m_ImportedAssetObject = AssetDatabase.LoadAssetAtPath<InputActionAsset>(path);
         }
 
         public void ApplyChanges()
@@ -140,19 +142,20 @@ public void ApplyChanges()
 
         internal void SaveChangesToAsset()
         {
-            Debug.Assert(!string.IsNullOrEmpty(m_AssetPath));
+            Debug.Assert(importedAsset != null);
 
             // Update JSON.
             var asset = m_AssetObjectForEditing;
             m_ImportedAssetJson = asset.ToJson();
 
             // Write out, if changed.
-            var existingJson = File.ReadAllText(m_AssetPath);
+            var assetPath = path;
+            var existingJson = File.ReadAllText(assetPath);
             if (m_ImportedAssetJson != existingJson)
             {
                 ////TODO: has to be made to work with version control
-                File.WriteAllText(m_AssetPath, m_ImportedAssetJson);
-                AssetDatabase.ImportAsset(m_AssetPath);
+                File.WriteAllText(assetPath, m_ImportedAssetJson);
+                AssetDatabase.ImportAsset(assetPath);
             }
 
             m_IsDirty = false;

Packages/com.unity.inputsystem/InputSystem/Editor/AssetEditor/InputActionEditorWindow.cs

@@ -825,6 +825,8 @@ public static AssetDeleteResult OnWillDeleteAsset(string path, RemoveAssetOption
                 return default;
             }
 
+            #pragma warning disable CA1801 // unused parameters
+
             // Handle .inputactions asset being moved.
             // ReSharper disable once UnusedMember.Local
             public static AssetMoveResult OnWillMoveAsset(string sourcePath, string destinationPath)
@@ -836,12 +838,13 @@ public static AssetMoveResult OnWillMoveAsset(string sourcePath, string destinat
                 var window = FindEditorForAssetWithGUID(guid);
                 if (window != null)
                 {
-                    window.m_ActionAssetManager.path = destinationPath;
                     window.UpdateWindowTitle();
                 }
 
                 return default;
             }
+
+            #pragma warning restore CA1801
         }
     }
 }