From a5fa1ca7c5a7472c2c073c559d96fa90a85b5222 Mon Sep 17 00:00:00 2001 From: Remi Slysz Date: Wed, 9 Feb 2022 17:21:37 +0100 Subject: [PATCH 1/5] better builtin support in resourcereloader --- .../Runtime/Utilities/ResourceReloader.cs | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/com.unity.render-pipelines.core/Runtime/Utilities/ResourceReloader.cs b/com.unity.render-pipelines.core/Runtime/Utilities/ResourceReloader.cs index fba907fb29c..d24367bbecb 100644 --- a/com.unity.render-pipelines.core/Runtime/Utilities/ResourceReloader.cs +++ b/com.unity.render-pipelines.core/Runtime/Utilities/ResourceReloader.cs @@ -190,19 +190,23 @@ static UnityEngine.Object Load(string path, Type type, bool builtin) // Else the path is good. Attempt loading resource if AssetDatabase available. UnityEngine.Object result; - if (builtin && type == typeof(Shader)) + if (builtin) { - result = Shader.Find(path); + if (type == typeof(Shader)) + { + result = Shader.Find(path); + } + else + { + result = Resources.GetBuiltinResource(type, path); + + if (IsNull(result)) + result = AssetDatabase.GetBuiltinExtraResource(type, path); + } } else { result = AssetDatabase.LoadAssetAtPath(path, type); - - if (IsNull(result)) - result = Resources.GetBuiltinResource(type, path); - - if (IsNull(result)) - result = AssetDatabase.GetBuiltinExtraResource(type, path); } if (IsNull(result)) From 45caccc69388ad1cdd53342f740b506a058ba5a4 Mon Sep 17 00:00:00 2001 From: Remi Slysz Date: Thu, 10 Feb 2022 12:10:24 +0100 Subject: [PATCH 2/5] proper fix with error when using [ReloadGroup] on ScriptableObject --- .../Runtime/Utilities/ResourceReloader.cs | 86 ++++++++++--------- 1 file changed, 46 insertions(+), 40 deletions(-) diff --git a/com.unity.render-pipelines.core/Runtime/Utilities/ResourceReloader.cs b/com.unity.render-pipelines.core/Runtime/Utilities/ResourceReloader.cs index d24367bbecb..bee4f2633b3 100644 --- a/com.unity.render-pipelines.core/Runtime/Utilities/ResourceReloader.cs +++ b/com.unity.render-pipelines.core/Runtime/Utilities/ResourceReloader.cs @@ -55,7 +55,8 @@ public static bool ReloadAllNullIn(System.Object container, string basePath) return false; var changed = false; - foreach (var fieldInfo in container.GetType().GetFields(BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Instance | BindingFlags.Static)) + foreach (var fieldInfo in container.GetType() + .GetFields(BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Instance | BindingFlags.Static)) { //Recurse on sub-containers if (IsReloadGroup(fieldInfo)) @@ -71,7 +72,7 @@ public static bool ReloadAllNullIn(System.Object container, string basePath) if (attribute.paths.Length == 1) { changed |= SetAndLoadIfNull(container, fieldInfo, GetFullPath(basePath, attribute), - attribute.package == ReloadAttribute.Package.Builtin); + attribute.package); } else if (attribute.paths.Length > 1) { @@ -89,10 +90,10 @@ public static bool ReloadAllNullIn(System.Object container, string basePath) } else { - bool builtin = attribute.package == ReloadAttribute.Package.Builtin; //Find each null element and reload them for (int index = 0; index < attribute.paths.Length; ++index) - changed |= SetAndLoadIfNull(array, index, GetFullPath(basePath, attribute, index), builtin); + changed |= SetAndLoadIfNull(array, index, GetFullPath(basePath, attribute, index), + attribute.package); } } } @@ -105,12 +106,14 @@ public static bool ReloadAllNullIn(System.Object container, string basePath) static bool FixGroupIfNeeded(System.Object container, FieldInfo info) { + var type = info.FieldType; + if (type.IsSubclassOf(typeof(ScriptableObject))) + throw new Exception(@$"ReloadGroup attribute must not be used on {nameof(ScriptableObject)}. +If {nameof(ResourceReloader)} create an instance of it, it will be not saved as a file, resulting in corrupted ID wnem building."); + if (IsNull(container, info)) { - var type = info.FieldType; - var value = type.IsSubclassOf(typeof(ScriptableObject)) - ? ScriptableObject.CreateInstance(type) - : Activator.CreateInstance(type); + var value = Activator.CreateInstance(type); info.SetValue( container, @@ -126,17 +129,18 @@ static bool FixGroupIfNeeded(Array array, int index) { Assert.IsNotNull(array); + var type = array.GetType().GetElementType(); + if (type.IsSubclassOf(typeof(ScriptableObject))) + throw new Exception(@$"ReloadGroup attribute must not be used on {nameof(ScriptableObject)}. +If {nameof(ResourceReloader)} create an instance of it, it will be not saved as a file, resulting in corrupted ID wnem building."); + if (IsNull(array.GetValue(index))) { - var type = array.GetType().GetElementType(); var value = type.IsSubclassOf(typeof(ScriptableObject)) ? ScriptableObject.CreateInstance(type) : Activator.CreateInstance(type); - array.SetValue( - value, - index - ); + array.SetValue(value, index); return true; } @@ -147,10 +151,7 @@ static bool FixArrayIfNeeded(System.Object container, FieldInfo info, int length { if (IsNull(container, info) || ((Array)info.GetValue(container)).Length < length) { - info.SetValue( - container, - Activator.CreateInstance(info.FieldType, length) - ); + info.SetValue(container, Activator.CreateInstance(info.FieldType, length)); return true; } @@ -180,33 +181,32 @@ static bool IsNull(System.Object container, FieldInfo info) static bool IsNull(System.Object field) => field == null || field.Equals(null); - static UnityEngine.Object Load(string path, Type type, bool builtin) + static UnityEngine.Object Load(string path, Type type, ReloadAttribute.Package location) { // Check if asset exist. // Direct loading can be prevented by AssetDatabase being reloading. var guid = AssetDatabase.AssetPathToGUID(path); - if (!builtin && String.IsNullOrEmpty(guid)) + if (location == ReloadAttribute.Package.Root && String.IsNullOrEmpty(guid)) throw new Exception($"Cannot load. Incorrect path: {path}"); // Else the path is good. Attempt loading resource if AssetDatabase available. UnityEngine.Object result; - if (builtin) + switch (location) { - if (type == typeof(Shader)) - { - result = Shader.Find(path); - } - else - { - result = Resources.GetBuiltinResource(type, path); - - if (IsNull(result)) - result = AssetDatabase.GetBuiltinExtraResource(type, path); - } - } - else - { - result = AssetDatabase.LoadAssetAtPath(path, type); + case ReloadAttribute.Package.Builtin: + if (type == typeof(Shader)) + result = Shader.Find(path); + else + result = Resources.GetBuiltinResource(type, path); //handle wrong path error + break; + case ReloadAttribute.Package.BuiltinExtra: + result = AssetDatabase.GetBuiltinExtraResource(type, path); //handle wrong path error + break; + case ReloadAttribute.Package.Root: + result = AssetDatabase.LoadAssetAtPath(path, type); + break; + default: + throw new NotImplementedException($"Unknown {location}"); } if (IsNull(result)) @@ -219,23 +219,23 @@ static UnityEngine.Object Load(string path, Type type, bool builtin) } static bool SetAndLoadIfNull(System.Object container, FieldInfo info, - string path, bool builtin) + string path, ReloadAttribute.Package location) { if (IsNull(container, info)) { - info.SetValue(container, Load(path, info.FieldType, builtin)); + info.SetValue(container, Load(path, info.FieldType, location)); return true; } return false; } - static bool SetAndLoadIfNull(Array array, int index, string path, bool builtin) + static bool SetAndLoadIfNull(Array array, int index, string path, ReloadAttribute.Package location) { var element = array.GetValue(index); if (IsNull(element)) { - array.SetValue(Load(path, array.GetType().GetElementType(), builtin), index); + array.SetValue(Load(path, array.GetType().GetElementType(), location), index); return true; } @@ -284,7 +284,13 @@ public enum Package /// /// Used for resources inside the package. /// - Root + Root, + + /// + /// Used for builtin extra resources when the resource isn't part of the package (i.e. builtin + /// extra Sprite). + /// + BuiltinExtra, }; #if UNITY_EDITOR From 4c1a68f13d393cd71d9e8b7d7d68115d3079b344 Mon Sep 17 00:00:00 2001 From: Remi Slysz Date: Thu, 10 Feb 2022 12:12:52 +0100 Subject: [PATCH 3/5] Update Changelog --- com.unity.render-pipelines.core/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/com.unity.render-pipelines.core/CHANGELOG.md b/com.unity.render-pipelines.core/CHANGELOG.md index 7a4f60d9722..bfa9414d343 100644 --- a/com.unity.render-pipelines.core/CHANGELOG.md +++ b/com.unity.render-pipelines.core/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Add probe volume influence weight parameter - Added support for multiple Baking States to Prove Volumes. - Hidding Volume Components not available for the current pipeline on the Volume Profile Inspector. +- Added error on ResourceReloader when attempting to use [ReloadGroup] on ScriptableObject. ### Changed - Volume Component editor are now specified by `CustomEditorAttribute` instead of `VolumeComponentEditorAttribute`. @@ -27,6 +28,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Fixed the issue with the special Turkish i, when looking for the m_IsGlobal property in VolumeEditor. (case 1276892) - Fixed texture gather macros for GLCore and moved them from target 4.6 to target 4.5. - Fixed cubemap array macros for GLCore. +- Fixed regression on ResourceReloader due to change for supporting built-in resources. ## [14.0.0] - 2021-11-17 From 51f169d2c3e5a4c0581a2690ac129347ea97813f Mon Sep 17 00:00:00 2001 From: Remi Slysz Date: Thu, 10 Feb 2022 12:24:23 +0100 Subject: [PATCH 4/5] Add Shader.Find also for extra builtin resource --- .../Runtime/Utilities/ResourceReloader.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/com.unity.render-pipelines.core/Runtime/Utilities/ResourceReloader.cs b/com.unity.render-pipelines.core/Runtime/Utilities/ResourceReloader.cs index bee4f2633b3..150c197227a 100644 --- a/com.unity.render-pipelines.core/Runtime/Utilities/ResourceReloader.cs +++ b/com.unity.render-pipelines.core/Runtime/Utilities/ResourceReloader.cs @@ -200,7 +200,10 @@ static UnityEngine.Object Load(string path, Type type, ReloadAttribute.Package l result = Resources.GetBuiltinResource(type, path); //handle wrong path error break; case ReloadAttribute.Package.BuiltinExtra: - result = AssetDatabase.GetBuiltinExtraResource(type, path); //handle wrong path error + if (type == typeof(Shader)) + result = Shader.Find(path); + else + result = AssetDatabase.GetBuiltinExtraResource(type, path); //handle wrong path error break; case ReloadAttribute.Package.Root: result = AssetDatabase.LoadAssetAtPath(path, type); From c51afd242d830c61b6ab013263c1b03c808cc7b9 Mon Sep 17 00:00:00 2001 From: Remi Slysz Date: Thu, 10 Feb 2022 12:35:08 +0100 Subject: [PATCH 5/5] Merge code throwing issue with [ReloadGroup] and fix typo on error --- .../Runtime/Utilities/ResourceReloader.cs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/com.unity.render-pipelines.core/Runtime/Utilities/ResourceReloader.cs b/com.unity.render-pipelines.core/Runtime/Utilities/ResourceReloader.cs index 150c197227a..22c84c85b36 100644 --- a/com.unity.render-pipelines.core/Runtime/Utilities/ResourceReloader.cs +++ b/com.unity.render-pipelines.core/Runtime/Utilities/ResourceReloader.cs @@ -104,12 +104,17 @@ public static bool ReloadAllNullIn(System.Object container, string basePath) return changed; } - static bool FixGroupIfNeeded(System.Object container, FieldInfo info) + static void CheckReloadGroupSupportedType(Type type) { - var type = info.FieldType; if (type.IsSubclassOf(typeof(ScriptableObject))) throw new Exception(@$"ReloadGroup attribute must not be used on {nameof(ScriptableObject)}. -If {nameof(ResourceReloader)} create an instance of it, it will be not saved as a file, resulting in corrupted ID wnem building."); +If {nameof(ResourceReloader)} create an instance of it, it will be not saved as a file, resulting in corrupted ID when building."); + } + + static bool FixGroupIfNeeded(System.Object container, FieldInfo info) + { + var type = info.FieldType; + CheckReloadGroupSupportedType(type); if (IsNull(container, info)) { @@ -130,9 +135,7 @@ static bool FixGroupIfNeeded(Array array, int index) Assert.IsNotNull(array); var type = array.GetType().GetElementType(); - if (type.IsSubclassOf(typeof(ScriptableObject))) - throw new Exception(@$"ReloadGroup attribute must not be used on {nameof(ScriptableObject)}. -If {nameof(ResourceReloader)} create an instance of it, it will be not saved as a file, resulting in corrupted ID wnem building."); + CheckReloadGroupSupportedType(type); if (IsNull(array.GetValue(index))) {