From 7c8c3748b557f86b695453c38675e9e450b7ca54 Mon Sep 17 00:00:00 2001 From: Merlin Date: Wed, 16 Nov 2022 21:28:18 -0800 Subject: [PATCH 1/5] Basic DAG implementation Not actually being used yet since it needs a chunk of work to integrate cleanly with how I assume prefabs work. The algorithm is theoretically there though. --- .../Editor/UdonSharpPrefabDAG.cs | 157 ++++++++++++++++++ .../Editor/UdonSharpPrefabDAG.cs.meta | 3 + 2 files changed, 160 insertions(+) create mode 100644 Packages/com.vrchat.UdonSharp/Editor/UdonSharpPrefabDAG.cs create mode 100644 Packages/com.vrchat.UdonSharp/Editor/UdonSharpPrefabDAG.cs.meta diff --git a/Packages/com.vrchat.UdonSharp/Editor/UdonSharpPrefabDAG.cs b/Packages/com.vrchat.UdonSharp/Editor/UdonSharpPrefabDAG.cs new file mode 100644 index 00000000..3a282ccf --- /dev/null +++ b/Packages/com.vrchat.UdonSharp/Editor/UdonSharpPrefabDAG.cs @@ -0,0 +1,157 @@ +using System; +using System.Collections; +using System.Collections.Generic; +using System.Linq; +using UnityEditor; +using UnityEngine; + +namespace UdonSharpEditor +{ + /// + /// Contains a set of Directed Acyclic Graphs that may or may not be connected at any point. The set of DAGs is rooted on prefabs that contain no nested prefabs and are not variants. + /// If a prefab nests another prefab, the prefab is considered a 'child' of the prefab that it is nesting. Because the parents must be visited and resolved before the children. + /// + // ReSharper disable once InconsistentNaming + internal class UdonSharpPrefabDAG : IEnumerable + { + private class Vertex + { + public GameObject Prefab; + public List Children = new List(); + public List Parents = new List(); + } + + private List _vertices = new List(); + private Dictionary _vertexLookup = new Dictionary(); + private List _sortedVertices = new List(); + + public UdonSharpPrefabDAG(IEnumerable allPrefabRoots) + { + foreach (GameObject prefabRoot in allPrefabRoots) + { + Vertex vert = new Vertex() { Prefab = prefabRoot }; + _vertices.Add(vert); + _vertexLookup.Add(prefabRoot, vert); + } + + foreach (Vertex vertex in _vertices) + { + if (PrefabUtility.IsPartOfVariantPrefab(vertex.Prefab)) + { + Vertex parent = _vertexLookup[PrefabUtility.GetCorrespondingObjectFromSource(vertex.Prefab)]; + Debug.Assert(parent != vertex); + + vertex.Parents.Add(parent); + parent.Children.Add(vertex); + } + + foreach (GameObject child in vertex.Prefab.GetComponentsInChildren(true).Select(e => e.gameObject)) + { + if (child == vertex.Prefab) + { + continue; + } + + if (PrefabUtility.IsAnyPrefabInstanceRoot(child)) + { + GameObject parentPrefab = PrefabUtility.GetCorrespondingObjectFromSource(child); + + Debug.Assert(parentPrefab); + Debug.Assert(parentPrefab != child); + + Vertex parent = _vertexLookup[parentPrefab]; + + vertex.Parents.Add(parent); + parent.Children.Add(vertex); + } + } + } + + // Do sorting + HashSet visitedVertices = new HashSet(); + + // Orphaned nodes with no parents or children go first + foreach (Vertex vertex in _vertices) + { + if (vertex.Children.Count == 0 && vertex.Parents.Count == 0) + { + visitedVertices.Add(vertex); + _sortedVertices.Add(vertex.Prefab); + } + } + + Queue openSet = new Queue(); + + // Find root nodes with no parents + foreach (Vertex vertex in _vertices) + { + if (!visitedVertices.Contains(vertex) && vertex.Parents.Count == 0) + { + openSet.Enqueue(vertex); + } + } + + while (openSet.Count > 0) + { + Vertex vertex = openSet.Dequeue(); + + if (visitedVertices.Contains(vertex)) + { + continue; + } + + if (vertex.Parents.Count > 0) + { + bool neededParentVisit = false; + + foreach (Vertex vertexParent in vertex.Parents) + { + if (!visitedVertices.Contains(vertexParent)) + { + neededParentVisit = true; + openSet.Enqueue(vertexParent); + } + } + + if (neededParentVisit) + { + // Re-queue to visit after we have traversed the node's parents + openSet.Enqueue(vertex); + continue; + } + } + + visitedVertices.Add(vertex); + _sortedVertices.Add(vertex.Prefab); + + foreach (Vertex vertexChild in vertex.Children) + { + openSet.Enqueue(vertexChild); + } + } + + // Sanity check + foreach (Vertex vertex in _vertices) + { + if (!visitedVertices.Contains(vertex)) + { + throw new Exception($"Invalid DAG state: node '{vertex.Prefab}' was not visited."); + } + } + } + + /// + /// Iterates the DAG in topological order where all parents are visited before their children. + /// Will iterate orphan nodes that don't have any parents or children first. + /// + public IEnumerator GetEnumerator() + { + return _sortedVertices.GetEnumerator(); + } + + IEnumerator IEnumerable.GetEnumerator() + { + return GetEnumerator(); + } + } +} diff --git a/Packages/com.vrchat.UdonSharp/Editor/UdonSharpPrefabDAG.cs.meta b/Packages/com.vrchat.UdonSharp/Editor/UdonSharpPrefabDAG.cs.meta new file mode 100644 index 00000000..65c08889 --- /dev/null +++ b/Packages/com.vrchat.UdonSharp/Editor/UdonSharpPrefabDAG.cs.meta @@ -0,0 +1,3 @@ +fileFormatVersion: 2 +guid: 667b852e57f544c593e121b6400c7643 +timeCreated: 1668580475 \ No newline at end of file From a0ed2fabb93081bbff01dec22cbce40a02b0c6cc Mon Sep 17 00:00:00 2001 From: Merlin Date: Mon, 28 Nov 2022 01:02:52 -0800 Subject: [PATCH 2/5] Add behaviour dirtying for prefabs This was way more annoying to figure out because the API docs for PrefabUtility.GetPropertyModifications() tell you literally nothing of value --- .../Editor/UdonSharpEditorUtility.cs | 135 ++++++++++++++++-- .../Editor/UdonSharpPrefabDAG.cs | 13 +- 2 files changed, 135 insertions(+), 13 deletions(-) diff --git a/Packages/com.vrchat.UdonSharp/Editor/UdonSharpEditorUtility.cs b/Packages/com.vrchat.UdonSharp/Editor/UdonSharpEditorUtility.cs index b70e0a3f..504fca68 100644 --- a/Packages/com.vrchat.UdonSharp/Editor/UdonSharpEditorUtility.cs +++ b/Packages/com.vrchat.UdonSharp/Editor/UdonSharpEditorUtility.cs @@ -276,7 +276,15 @@ private static bool HasSceneBehaviourUpgradeFlag(UdonBehaviour behaviour) { return behaviour.publicVariables.TryGetVariableValue(UDONSHARP_SCENE_BEHAVIOUR_UPGRADE_MARKER, out bool sceneBehaviourUpgraded) && sceneBehaviourUpgraded; } - + + private static readonly FieldInfo _publicVariablesBytesStrField = typeof(UdonBehaviour) + .GetFields(BindingFlags.NonPublic | BindingFlags.Instance) + .First(fieldInfo => fieldInfo.Name == "serializedPublicVariablesBytesString"); + + private static readonly FieldInfo _publicVariablesObjectReferences = typeof(UdonBehaviour) + .GetFields(BindingFlags.NonPublic | BindingFlags.Instance) + .First(e => e.Name == "publicVariablesUnityEngineObjects"); + /// /// Runs a two pass upgrade of a set of prefabs, assumes all dependencies of the prefabs are included, otherwise the process could fail to maintain references. /// First creates a new UdonSharpBehaviour proxy script and hooks it to a given UdonBehaviour. Then in a second pass goes over all behaviours and serializes their data into the C# proxy and wipes their old data out. @@ -310,15 +318,6 @@ bool NeedsNewProxy(UdonBehaviour udonBehaviour) { if (!IsUdonSharpBehaviour(udonBehaviour)) return false; - - // The behaviour originates from a parent prefab so we don't want to modify this copy of the prefab - if (PrefabUtility.GetCorrespondingObjectFromOriginalSource(udonBehaviour) != udonBehaviour) - { - if (!PrefabUtility.IsPartOfPrefabInstance(udonBehaviour)) - UdonSharpUtils.LogWarning($"Nested prefab with UdonSharpBehaviours detected during prefab upgrade: {udonBehaviour.gameObject}, nested prefabs are not eligible for automatic upgrade."); - - return false; - } if (GetProxyBehaviour(udonBehaviour)) return false; @@ -340,6 +339,122 @@ bool NeedsSerializationUpgrade(UdonBehaviour udonBehaviour) return false; } + UdonSharpPrefabDAG prefabDag = new UdonSharpPrefabDAG(prefabRoots); + + // Walk up from children -> parents and mark prefab deltas on all U# behaviours to be upgraded + // Prevents versioning from being overwritten when a parent prefab is upgraded + foreach (string prefabPath in prefabDag.Reverse()) + { + GameObject prefabRoot = PrefabUtility.LoadPrefabContents(prefabPath); + + bool needsSave = false; + + try + { + HashSet behavioursToPrepare = new HashSet(); + + foreach (UdonBehaviour behaviour in prefabRoot.GetComponentsInChildren(true)) + { + if (PrefabUtility.GetCorrespondingObjectFromSource(behaviour) != behaviour && (NeedsNewProxy(behaviour) || NeedsSerializationUpgrade(behaviour))) + { + behavioursToPrepare.Add(behaviour); + } + } + + // Deltas are stored per-gameobject I guess??? It might be per-instance-root which would suck even more though... + // We take care to not accidentally hit any non-U#-behaviour deltas here + // These APIs are not documented properly at all and the only mentions of them on forum posts are how they don't work with no solutions posted :)))) + if (behavioursToPrepare.Count > 0) + { + HashSet rootGameObjects = new HashSet(behavioursToPrepare.Select(e => e.gameObject)); + HashSet originalObjects = new HashSet(behavioursToPrepare.Select(PrefabUtility.GetCorrespondingObjectFromOriginalSource)); + + foreach (GameObject rootGameObject in rootGameObjects) + { + List propertyModifications = PrefabUtility.GetPropertyModifications(rootGameObject)?.ToList(); + + if (propertyModifications != null) + { + propertyModifications = propertyModifications.Where( + modification => + { + if (!originalObjects.Contains(modification.target)) + { + return true; + } + + if (modification.propertyPath == "serializedPublicVariablesBytesString" || + modification.propertyPath.StartsWith("publicVariablesUnityEngineObjects", StringComparison.Ordinal)) + { + UdonSharpUtils.Log($"Removed property override for {modification.propertyPath} on {modification.target}"); + return false; + } + + return true; + }).ToList(); + } + else + { + propertyModifications = new List(); + } + + foreach (UdonBehaviour behaviour in behavioursToPrepare) + { + if (behaviour.gameObject != rootGameObject) + continue; + + UdonBehaviour originalBehaviour = PrefabUtility.GetCorrespondingObjectFromOriginalSource(behaviour); + + propertyModifications.Add(new PropertyModification() + { + target = originalBehaviour, + propertyPath = "serializedPublicVariablesBytesString", + value = (string)_publicVariablesBytesStrField.GetValue(behaviour) + }); + + List objectRefs = (List)_publicVariablesObjectReferences.GetValue(behaviour); + + propertyModifications.Add(new PropertyModification() + { + target = originalBehaviour, + propertyPath = "publicVariablesUnityEngineObjects.Array.size", + value = objectRefs.Count.ToString() + }); + + for (int i = 0; i < objectRefs.Count; ++i) + { + propertyModifications.Add(new PropertyModification() + { + target = originalBehaviour, + propertyPath = $"publicVariablesUnityEngineObjects.Array.data[{i}]", + objectReference = objectRefs[i], + value = "" + }); + } + } + + PrefabUtility.SetPropertyModifications(rootGameObject, propertyModifications.ToArray()); + EditorUtility.SetDirty(rootGameObject); + + needsSave = true; + } + + UdonSharpUtils.Log($"Marking delta on prefab {prefabRoot} because it is not the original definition."); + } + + if (needsSave) + { + PrefabUtility.SaveAsPrefabAsset(prefabRoot, prefabPath); + } + } + finally + { + PrefabUtility.UnloadPrefabContents(prefabRoot); + } + } + + return; + HashSet phase1FixupPrefabRoots = new HashSet(); // Phase 1 Pruning - Add missing proxy behaviours diff --git a/Packages/com.vrchat.UdonSharp/Editor/UdonSharpPrefabDAG.cs b/Packages/com.vrchat.UdonSharp/Editor/UdonSharpPrefabDAG.cs index 3a282ccf..1280ae66 100644 --- a/Packages/com.vrchat.UdonSharp/Editor/UdonSharpPrefabDAG.cs +++ b/Packages/com.vrchat.UdonSharp/Editor/UdonSharpPrefabDAG.cs @@ -12,7 +12,7 @@ namespace UdonSharpEditor /// If a prefab nests another prefab, the prefab is considered a 'child' of the prefab that it is nesting. Because the parents must be visited and resolved before the children. /// // ReSharper disable once InconsistentNaming - internal class UdonSharpPrefabDAG : IEnumerable + internal class UdonSharpPrefabDAG : IEnumerable { private class Vertex { @@ -24,6 +24,7 @@ private class Vertex private List _vertices = new List(); private Dictionary _vertexLookup = new Dictionary(); private List _sortedVertices = new List(); + private List _sortedPaths = new List(); public UdonSharpPrefabDAG(IEnumerable allPrefabRoots) { @@ -55,6 +56,8 @@ public UdonSharpPrefabDAG(IEnumerable allPrefabRoots) if (PrefabUtility.IsAnyPrefabInstanceRoot(child)) { GameObject parentPrefab = PrefabUtility.GetCorrespondingObjectFromSource(child); + + parentPrefab = parentPrefab.transform.root.gameObject; Debug.Assert(parentPrefab); Debug.Assert(parentPrefab != child); @@ -138,15 +141,19 @@ public UdonSharpPrefabDAG(IEnumerable allPrefabRoots) throw new Exception($"Invalid DAG state: node '{vertex.Prefab}' was not visited."); } } + + _sortedPaths = _sortedVertices.Select(AssetDatabase.GetAssetPath).ToList(); } /// /// Iterates the DAG in topological order where all parents are visited before their children. /// Will iterate orphan nodes that don't have any parents or children first. + /// We return paths here because of the assumption that Unity may create a new set of prefab objects when reimporting a prefab after saving it. + /// The upgrade process is expected to load the prefabs from their path in sequence of upgrade /// - public IEnumerator GetEnumerator() + public IEnumerator GetEnumerator() { - return _sortedVertices.GetEnumerator(); + return _sortedPaths.GetEnumerator(); } IEnumerator IEnumerable.GetEnumerator() From 43c8395a3441318b07243afa870da217fe4b66b2 Mon Sep 17 00:00:00 2001 From: Merlin Date: Tue, 29 Nov 2022 00:34:35 -0800 Subject: [PATCH 3/5] Improve DAG - Handles prefabs being linked that don't have U# behaviours now - Added exception handling that falls back to unsorted prefab set and logs an error in the case of a failure - Turned assertions into exceptions because assertions don't give enough info --- .../Editor/UdonSharpPrefabDAG.cs | 180 ++++++++++-------- 1 file changed, 101 insertions(+), 79 deletions(-) diff --git a/Packages/com.vrchat.UdonSharp/Editor/UdonSharpPrefabDAG.cs b/Packages/com.vrchat.UdonSharp/Editor/UdonSharpPrefabDAG.cs index 1280ae66..746d1229 100644 --- a/Packages/com.vrchat.UdonSharp/Editor/UdonSharpPrefabDAG.cs +++ b/Packages/com.vrchat.UdonSharp/Editor/UdonSharpPrefabDAG.cs @@ -2,6 +2,7 @@ using System.Collections; using System.Collections.Generic; using System.Linq; +using UdonSharp; using UnityEditor; using UnityEngine; @@ -28,121 +29,142 @@ private class Vertex public UdonSharpPrefabDAG(IEnumerable allPrefabRoots) { - foreach (GameObject prefabRoot in allPrefabRoots) + try { - Vertex vert = new Vertex() { Prefab = prefabRoot }; - _vertices.Add(vert); - _vertexLookup.Add(prefabRoot, vert); - } - - foreach (Vertex vertex in _vertices) - { - if (PrefabUtility.IsPartOfVariantPrefab(vertex.Prefab)) + foreach (GameObject prefabRoot in allPrefabRoots) { - Vertex parent = _vertexLookup[PrefabUtility.GetCorrespondingObjectFromSource(vertex.Prefab)]; - Debug.Assert(parent != vertex); - - vertex.Parents.Add(parent); - parent.Children.Add(vertex); + Vertex vert = new Vertex() { Prefab = prefabRoot }; + _vertices.Add(vert); + _vertexLookup.Add(prefabRoot, vert); } - - foreach (GameObject child in vertex.Prefab.GetComponentsInChildren(true).Select(e => e.gameObject)) + + foreach (Vertex vertex in _vertices) { - if (child == vertex.Prefab) - { - continue; - } - - if (PrefabUtility.IsAnyPrefabInstanceRoot(child)) + if (PrefabUtility.IsPartOfVariantPrefab(vertex.Prefab)) { - GameObject parentPrefab = PrefabUtility.GetCorrespondingObjectFromSource(child); - - parentPrefab = parentPrefab.transform.root.gameObject; - - Debug.Assert(parentPrefab); - Debug.Assert(parentPrefab != child); + Vertex parent = _vertexLookup[PrefabUtility.GetCorrespondingObjectFromSource(vertex.Prefab)]; - Vertex parent = _vertexLookup[parentPrefab]; + if (parent == vertex) + { + throw new Exception($"Parent of vertex cannot be the same as the vertex '{vertex.Prefab}'"); + } vertex.Parents.Add(parent); parent.Children.Add(vertex); } - } - } - - // Do sorting - HashSet visitedVertices = new HashSet(); - // Orphaned nodes with no parents or children go first - foreach (Vertex vertex in _vertices) - { - if (vertex.Children.Count == 0 && vertex.Parents.Count == 0) - { - visitedVertices.Add(vertex); - _sortedVertices.Add(vertex.Prefab); - } - } + foreach (GameObject child in vertex.Prefab.GetComponentsInChildren(true).Select(e => e.gameObject)) + { + if (child == vertex.Prefab) + { + continue; + } - Queue openSet = new Queue(); + if (PrefabUtility.IsAnyPrefabInstanceRoot(child)) + { + GameObject parentPrefab = PrefabUtility.GetCorrespondingObjectFromSource(child); + + if (parentPrefab == null) + { + throw new Exception($"ParentPrefab of '{child}' is null"); + } + + parentPrefab = parentPrefab.transform.root.gameObject; + + if (parentPrefab == child) + { + throw new Exception($"ParentPrefab cannot be the same as child '{child}'"); + } + + // If a nested prefab is referenced that does *not* have any UdonBehaviours on it, it will not be in the vertex list, and does not need to be linked. + if (_vertexLookup.TryGetValue(parentPrefab, out Vertex parent)) + { + vertex.Parents.Add(parent); + parent.Children.Add(vertex); + } + } + } + } + + // Do sorting + HashSet visitedVertices = new HashSet(); - // Find root nodes with no parents - foreach (Vertex vertex in _vertices) - { - if (!visitedVertices.Contains(vertex) && vertex.Parents.Count == 0) + // Orphaned nodes with no parents or children go first + foreach (Vertex vertex in _vertices) { - openSet.Enqueue(vertex); + if (vertex.Children.Count == 0 && vertex.Parents.Count == 0) + { + visitedVertices.Add(vertex); + _sortedVertices.Add(vertex.Prefab); + } } - } - while (openSet.Count > 0) - { - Vertex vertex = openSet.Dequeue(); + Queue openSet = new Queue(); - if (visitedVertices.Contains(vertex)) + // Find root nodes with no parents + foreach (Vertex vertex in _vertices) { - continue; + if (!visitedVertices.Contains(vertex) && vertex.Parents.Count == 0) + { + openSet.Enqueue(vertex); + } } - if (vertex.Parents.Count > 0) + while (openSet.Count > 0) { - bool neededParentVisit = false; + Vertex vertex = openSet.Dequeue(); - foreach (Vertex vertexParent in vertex.Parents) + if (visitedVertices.Contains(vertex)) { - if (!visitedVertices.Contains(vertexParent)) + continue; + } + + if (vertex.Parents.Count > 0) + { + bool neededParentVisit = false; + + foreach (Vertex vertexParent in vertex.Parents) + { + if (!visitedVertices.Contains(vertexParent)) + { + neededParentVisit = true; + openSet.Enqueue(vertexParent); + } + } + + if (neededParentVisit) { - neededParentVisit = true; - openSet.Enqueue(vertexParent); + // Re-queue to visit after we have traversed the node's parents + openSet.Enqueue(vertex); + continue; } } - if (neededParentVisit) + visitedVertices.Add(vertex); + _sortedVertices.Add(vertex.Prefab); + + foreach (Vertex vertexChild in vertex.Children) { - // Re-queue to visit after we have traversed the node's parents - openSet.Enqueue(vertex); - continue; + openSet.Enqueue(vertexChild); } } - visitedVertices.Add(vertex); - _sortedVertices.Add(vertex.Prefab); - - foreach (Vertex vertexChild in vertex.Children) + // Sanity check + foreach (Vertex vertex in _vertices) { - openSet.Enqueue(vertexChild); + if (!visitedVertices.Contains(vertex)) + { + throw new Exception($"Invalid DAG state: node '{vertex.Prefab}' was not visited."); + } } - } - // Sanity check - foreach (Vertex vertex in _vertices) + _sortedPaths = _sortedVertices.Select(AssetDatabase.GetAssetPath).ToList(); + } + catch (Exception e) { - if (!visitedVertices.Contains(vertex)) - { - throw new Exception($"Invalid DAG state: node '{vertex.Prefab}' was not visited."); - } + UdonSharpUtils.LogError($"Exception while sorting prefabs for upgrade. Falling back to non-sorted set, nested prefabs may not upgrade properly. Exception: {e}"); + _sortedPaths = allPrefabRoots.Select(AssetDatabase.GetAssetPath).ToList(); } - - _sortedPaths = _sortedVertices.Select(AssetDatabase.GetAssetPath).ToList(); } /// From 98c565708806d692f7efe66ce5072302913318f0 Mon Sep 17 00:00:00 2001 From: Merlin Date: Tue, 29 Nov 2022 00:36:42 -0800 Subject: [PATCH 4/5] Explicit names Random thing I saw and wanted to make more consistent --- .../Editor/UdonSharpEditorManager.cs | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/Packages/com.vrchat.UdonSharp/Editor/UdonSharpEditorManager.cs b/Packages/com.vrchat.UdonSharp/Editor/UdonSharpEditorManager.cs index 513d2809..8c89549e 100644 --- a/Packages/com.vrchat.UdonSharp/Editor/UdonSharpEditorManager.cs +++ b/Packages/com.vrchat.UdonSharp/Editor/UdonSharpEditorManager.cs @@ -907,7 +907,7 @@ private static IEnumerable GetAllPrefabsWithUdonSharpBehaviours() if (prefabRoot == null) continue; - var prefabAssetType = PrefabUtility.GetPrefabAssetType(prefabRoot); + PrefabAssetType prefabAssetType = PrefabUtility.GetPrefabAssetType(prefabRoot); if (prefabAssetType == PrefabAssetType.Model || prefabAssetType == PrefabAssetType.MissingAsset) continue; @@ -919,7 +919,7 @@ private static IEnumerable GetAllPrefabsWithUdonSharpBehaviours() bool hasUdonSharpBehaviour = false; - foreach (var behaviour in behaviourScratch) + foreach (UdonBehaviour behaviour in behaviourScratch) { if (UdonSharpEditorUtility.IsUdonSharpBehaviour(behaviour)) { @@ -942,7 +942,7 @@ private static IImmutableSet GetRootPrefabGameObjectRefs(IEnumerable HashSet newObjectSet = new HashSet(); - foreach (var unityObject in unityObjects) + foreach (Object unityObject in unityObjects) { if (unityObject == null) continue; @@ -953,7 +953,7 @@ private static IImmutableSet GetRootPrefabGameObjectRefs(IEnumerable if (!PrefabUtility.IsPartOfPrefabAsset(unityObject)) continue; - var prefabAssetType = PrefabUtility.GetPrefabAssetType(unityObject); + PrefabAssetType prefabAssetType = PrefabUtility.GetPrefabAssetType(unityObject); if (prefabAssetType == PrefabAssetType.Model || prefabAssetType == PrefabAssetType.MissingAsset) continue; @@ -995,12 +995,12 @@ private static HashSet GetBehaviourComponentOrGameObjectReferences(UdonB if (behaviour == null) return unityObjects; - var unityObjectList = (List)_serializedObjectReferencesField.GetValue(behaviour); + List unityObjectList = (List)_serializedObjectReferencesField.GetValue(behaviour); if (unityObjectList == null) return unityObjects; - foreach (var unityObject in unityObjectList) + foreach (Object unityObject in unityObjectList) { if (unityObject is Component || unityObject is GameObject) unityObjects.Add(unityObject); @@ -1013,7 +1013,7 @@ private static HashSet CollectScenePrefabRoots(List a { HashSet prefabRoots = new HashSet(); - foreach (var udonBehaviour in allBehaviours) + foreach (UdonBehaviour udonBehaviour in allBehaviours) { if (!PrefabUtility.IsPartOfPrefabInstance(udonBehaviour)) continue; @@ -1033,7 +1033,7 @@ private static IEnumerable CollectAllReferencedPrefabRoots(IEnumerab { HashSet gameObjectsToVisit = new HashSet(); - foreach (var rootBehaviour in rootSet) + foreach (UdonBehaviour rootBehaviour in rootSet) { HashSet objects = GetBehaviourComponentOrGameObjectReferences(rootBehaviour); gameObjectsToVisit.UnionWith(GetRootPrefabGameObjectRefs(objects.Count != 0 ? objects : null)); @@ -1052,14 +1052,14 @@ private static IEnumerable CollectAllReferencedPrefabRoots(IEnumerab { HashSet newVisitSet = new HashSet(); - foreach (var gameObject in gameObjectsToVisit) + foreach (GameObject gameObject in gameObjectsToVisit) { if (visitedSet.Contains(gameObject)) continue; visitedSet.Add(gameObject); - foreach (var udonBehaviour in gameObject.GetComponentsInChildren(true)) + foreach (UdonBehaviour udonBehaviour in gameObject.GetComponentsInChildren(true)) { HashSet objectRefs; @@ -1081,7 +1081,7 @@ private static IEnumerable CollectAllReferencedPrefabRoots(IEnumerab objectRefs = GetBehaviourComponentOrGameObjectReferences(udonBehaviour); } - foreach (var newObjRef in GetRootPrefabGameObjectRefs((objectRefs != null && objectRefs.Count != 0) ? objectRefs : null)) + foreach (GameObject newObjRef in GetRootPrefabGameObjectRefs((objectRefs != null && objectRefs.Count != 0) ? objectRefs : null)) { if (!visitedSet.Contains(newObjRef)) newVisitSet.Add(newObjRef); @@ -1584,9 +1584,9 @@ private static void SanitizeProxyBehaviours(GameObject[] allGameObjects, bool ap private static bool AreUdonSharpScriptsUpdated() { - var allPrograms = UdonSharpProgramAsset.GetAllUdonSharpPrograms(); + UdonSharpProgramAsset[] allPrograms = UdonSharpProgramAsset.GetAllUdonSharpPrograms(); - foreach (var programAsset in allPrograms) + foreach (UdonSharpProgramAsset programAsset in allPrograms) { if (programAsset.ScriptVersion != UdonSharpProgramVersion.CurrentVersion) return false; @@ -1960,7 +1960,7 @@ private static void OnPostprocessAllAssets(string[] importedAssets, string[] del bool needsUpdate = false; - foreach (var behaviour in behaviours) + foreach (UdonBehaviour behaviour in behaviours) { if (!UdonSharpEditorUtility.IsUdonSharpBehaviour(behaviour)) continue; From 0d0450bf4338ed3344c3e4776824d44b194ac8c0 Mon Sep 17 00:00:00 2001 From: Merlin Date: Tue, 29 Nov 2022 00:39:30 -0800 Subject: [PATCH 5/5] Now proper support for nested prefab upgrades - Fixed marking deltas on prefabs for pre-pass because Unity's API for this is not documented at all and the API itself is the most confusing to use API you could imagine. --- .../Editor/UdonSharpEditorUtility.cs | 264 +++++++++++------- 1 file changed, 166 insertions(+), 98 deletions(-) diff --git a/Packages/com.vrchat.UdonSharp/Editor/UdonSharpEditorUtility.cs b/Packages/com.vrchat.UdonSharp/Editor/UdonSharpEditorUtility.cs index 504fca68..46e4941a 100644 --- a/Packages/com.vrchat.UdonSharp/Editor/UdonSharpEditorUtility.cs +++ b/Packages/com.vrchat.UdonSharp/Editor/UdonSharpEditorUtility.cs @@ -339,6 +339,46 @@ bool NeedsSerializationUpgrade(UdonBehaviour udonBehaviour) return false; } + HashSet phase1FixupPrefabRoots = new HashSet(); + + // Phase 1 Pruning - Add missing proxy behaviours + foreach (GameObject prefabRoot in prefabRoots) + { + if (!prefabRoot.GetComponentsInChildren(true).Any(NeedsNewProxy)) + continue; + + string prefabPath = AssetDatabase.GetAssetPath(prefabRoot); + + if (!prefabPath.IsNullOrWhitespace()) + phase1FixupPrefabRoots.Add(prefabPath); + } + + HashSet phase2FixupPrefabRoots = new HashSet(phase1FixupPrefabRoots); + + // Phase 2 Pruning - Check for behaviours that require their data ownership to be transferred Udon -> C# + foreach (GameObject prefabRoot in prefabRoots) + { + foreach (UdonBehaviour udonBehaviour in prefabRoot.GetComponentsInChildren(true)) + { + if (NeedsSerializationUpgrade(udonBehaviour)) + { + string prefabPath = AssetDatabase.GetAssetPath(prefabRoot); + + if (!prefabPath.IsNullOrWhitespace()) + phase2FixupPrefabRoots.Add(prefabPath); + + break; + } + } + } + + // Now we have a set of prefabs that we can actually load and run the two upgrade phases on. + // Todo: look at merging the two passes since we don't actually need to load prefabs into scenes apparently + + // Early out and avoid the edit scope + if (phase1FixupPrefabRoots.Count == 0 && phase2FixupPrefabRoots.Count == 0) + return; + UdonSharpPrefabDAG prefabDag = new UdonSharpPrefabDAG(prefabRoots); // Walk up from children -> parents and mark prefab deltas on all U# behaviours to be upgraded @@ -361,14 +401,36 @@ bool NeedsSerializationUpgrade(UdonBehaviour udonBehaviour) } } - // Deltas are stored per-gameobject I guess??? It might be per-instance-root which would suck even more though... + // Deltas are stored per-prefab-instance-root in a given prefab, don't question it. Thanks. // We take care to not accidentally hit any non-U#-behaviour deltas here // These APIs are not documented properly at all and the only mentions of them on forum posts are how they don't work with no solutions posted :)))) if (behavioursToPrepare.Count > 0) { - HashSet rootGameObjects = new HashSet(behavioursToPrepare.Select(e => e.gameObject)); + HashSet rootGameObjects = new HashSet(); + + foreach (UdonBehaviour behaviourToPrepare in behavioursToPrepare) + { + GameObject rootPrefab = PrefabUtility.GetOutermostPrefabInstanceRoot(behaviourToPrepare); + + rootGameObjects.Add(rootPrefab ? rootPrefab : behaviourToPrepare.gameObject); + } + HashSet originalObjects = new HashSet(behavioursToPrepare.Select(PrefabUtility.GetCorrespondingObjectFromOriginalSource)); + foreach (UdonBehaviour behaviour in behavioursToPrepare) + { + UdonBehaviour currentBehaviour = behaviour; + + while (currentBehaviour) + { + originalObjects.Add(currentBehaviour); + + UdonBehaviour newBehaviour = PrefabUtility.GetCorrespondingObjectFromSource(currentBehaviour); + + currentBehaviour = newBehaviour != currentBehaviour ? newBehaviour : null; + } + } + foreach (GameObject rootGameObject in rootGameObjects) { List propertyModifications = PrefabUtility.GetPropertyModifications(rootGameObject)?.ToList(); @@ -378,6 +440,11 @@ bool NeedsSerializationUpgrade(UdonBehaviour udonBehaviour) propertyModifications = propertyModifications.Where( modification => { + if (modification.target == null) + { + return true; + } + if (!originalObjects.Contains(modification.target)) { return true; @@ -386,24 +453,28 @@ bool NeedsSerializationUpgrade(UdonBehaviour udonBehaviour) if (modification.propertyPath == "serializedPublicVariablesBytesString" || modification.propertyPath.StartsWith("publicVariablesUnityEngineObjects", StringComparison.Ordinal)) { - UdonSharpUtils.Log($"Removed property override for {modification.propertyPath} on {modification.target}"); + // UdonSharpUtils.Log($"Removed property override for {modification.propertyPath} on {modification.target}"); return false; } return true; }).ToList(); + + // UdonSharpUtils.Log($"Modifications found on {rootGameObject}"); } else { propertyModifications = new List(); } - foreach (UdonBehaviour behaviour in behavioursToPrepare) + foreach (UdonBehaviour behaviour in rootGameObject.GetComponentsInChildren(true)) { - if (behaviour.gameObject != rootGameObject) + if (!behavioursToPrepare.Contains(behaviour)) + { continue; - - UdonBehaviour originalBehaviour = PrefabUtility.GetCorrespondingObjectFromOriginalSource(behaviour); + } + + UdonBehaviour originalBehaviour = PrefabUtility.GetCorrespondingObjectFromSource(behaviour); propertyModifications.Add(new PropertyModification() { @@ -439,7 +510,7 @@ bool NeedsSerializationUpgrade(UdonBehaviour udonBehaviour) needsSave = true; } - UdonSharpUtils.Log($"Marking delta on prefab {prefabRoot} because it is not the original definition."); + // UdonSharpUtils.Log($"Marking delta on prefab {prefabRoot} because it is not the original definition."); } if (needsSave) @@ -453,119 +524,116 @@ bool NeedsSerializationUpgrade(UdonBehaviour udonBehaviour) } } - return; - - HashSet phase1FixupPrefabRoots = new HashSet(); - - // Phase 1 Pruning - Add missing proxy behaviours - foreach (GameObject prefabRoot in prefabRoots) - { - if (!prefabRoot.GetComponentsInChildren(true).Any(NeedsNewProxy)) - continue; - - string prefabPath = AssetDatabase.GetAssetPath(prefabRoot); - - if (!prefabPath.IsNullOrWhitespace()) - phase1FixupPrefabRoots.Add(prefabRoot); - } - - HashSet phase2FixupPrefabRoots = new HashSet(phase1FixupPrefabRoots); - - // Phase 2 Pruning - Check for behaviours that require their data ownership to be transferred Udon -> C# - foreach (GameObject prefabRoot in prefabRoots) + if (phase2FixupPrefabRoots.Count > 0) + UdonSharpUtils.Log($"Running upgrade process on {phase2FixupPrefabRoots.Count} prefabs: {string.Join(", ", phase2FixupPrefabRoots.Select(Path.GetFileName))}"); + + foreach (string prefabRootPath in prefabDag) { - foreach (UdonBehaviour udonBehaviour in prefabRoot.GetComponentsInChildren(true)) + if (!phase1FixupPrefabRoots.Contains(prefabRootPath)) { - if (NeedsSerializationUpgrade(udonBehaviour)) - { - string prefabPath = AssetDatabase.GetAssetPath(prefabRoot); - - if (!prefabPath.IsNullOrWhitespace()) - phase2FixupPrefabRoots.Add(prefabRoot); - - break; - } + continue; } - } - - // Now we have a set of prefabs that we can actually load and run the two upgrade phases on. - // Todo: look at merging the two passes since we don't actually need to load prefabs into scenes apparently - // Early out and avoid the edit scope - if (phase1FixupPrefabRoots.Count == 0 && phase2FixupPrefabRoots.Count == 0) - return; - - if (phase2FixupPrefabRoots.Count > 0) - UdonSharpUtils.Log($"Running upgrade process on {phase2FixupPrefabRoots.Count} prefabs: {string.Join(", ", phase2FixupPrefabRoots.Select(e => e.name))}"); - - using (new UdonSharpEditorManager.AssetEditScope()) - { - foreach (GameObject prefabRoot in phase1FixupPrefabRoots) + GameObject prefabRoot = PrefabUtility.LoadPrefabContents(prefabRootPath); + + try { - try + bool needsSave = false; + + foreach (UdonBehaviour udonBehaviour in prefabRoot.GetComponentsInChildren(true)) { - foreach (UdonBehaviour udonBehaviour in prefabRoot.GetComponentsInChildren(true)) + if (!NeedsNewProxy(udonBehaviour)) { - if (!NeedsNewProxy(udonBehaviour)) - continue; + if (GetBehaviourVersion(udonBehaviour) == UdonSharpBehaviourVersion.V0) + { + SetBehaviourVersion(udonBehaviour, UdonSharpBehaviourVersion.V0DataUpgradeNeeded); + needsSave = true; + } - UdonSharpBehaviour newProxy = (UdonSharpBehaviour)udonBehaviour.gameObject.AddComponent(GetUdonSharpBehaviourType(udonBehaviour)); - newProxy.enabled = udonBehaviour.enabled; + continue; + } - SetBackingUdonBehaviour(newProxy, udonBehaviour); + UdonSharpBehaviour newProxy = (UdonSharpBehaviour)udonBehaviour.gameObject.AddComponent(GetUdonSharpBehaviourType(udonBehaviour)); + newProxy.enabled = udonBehaviour.enabled; - MoveComponentRelativeToComponent(newProxy, udonBehaviour, true); + SetBackingUdonBehaviour(newProxy, udonBehaviour); - SetBehaviourVersion(udonBehaviour, UdonSharpBehaviourVersion.V0DataUpgradeNeeded); - } + MoveComponentRelativeToComponent(newProxy, udonBehaviour, true); - // Phase2 is a superset of phase 1 upgrades, and AssetEditScope prevents flushing to disk anyways so just don't save here. - // PrefabUtility.SavePrefabAsset(prefabRoot); - - // UdonSharpUtils.Log($"Ran prefab upgrade phase 1 on {prefabRoot}"); + SetBehaviourVersion(udonBehaviour, UdonSharpBehaviourVersion.V0DataUpgradeNeeded); + + UdonSharpUtils.SetDirty(udonBehaviour); + UdonSharpUtils.SetDirty(newProxy); + + needsSave = true; } - catch (Exception e) + + if (needsSave) { - UdonSharpUtils.LogError($"Encountered exception while upgrading prefab {prefabRoot}, report exception to Merlin: {e}"); + PrefabUtility.SaveAsPrefabAsset(prefabRoot, prefabRootPath); } + + // UdonSharpUtils.Log($"Ran prefab upgrade phase 1 on {prefabRoot}"); + } + catch (Exception e) + { + UdonSharpUtils.LogError($"Encountered exception while upgrading prefab {prefabRootPath}, report exception to Merlin: {e}"); } + finally + { + PrefabUtility.UnloadPrefabContents(prefabRoot); + } + } - foreach (GameObject prefabRoot in phase2FixupPrefabRoots) + foreach (string prefabRootPath in prefabDag) + { + if (!phase2FixupPrefabRoots.Contains(prefabRootPath)) { - try - { - foreach (UdonBehaviour udonBehaviour in prefabRoot.GetComponentsInChildren(true)) - { - if (!NeedsSerializationUpgrade(udonBehaviour)) - continue; - - CopyUdonToProxy(GetProxyBehaviour(udonBehaviour), ProxySerializationPolicy.RootOnly); + continue; + } - // We can't remove this data for backwards compatibility :'( - // If we nuke the data, the unity object array on the underlying storage may change. - // Which means that if people have copies of this prefab in the scene with no object reference changes, their data will also get nuked which we do not want. - // Public variable data on the prefabs will never be touched again by U# after upgrading - // We will probably provide an optional upgrade process that strips this extra data, and takes into account all scenes in the project - - // foreach (string publicVarSymbol in udonBehaviour.publicVariables.VariableSymbols.ToArray()) - // udonBehaviour.publicVariables.RemoveVariable(publicVarSymbol); - - SetBehaviourVersion(udonBehaviour, UdonSharpBehaviourVersion.V1); - SetBehaviourUpgraded(udonBehaviour); - } + GameObject prefabRoot = PrefabUtility.LoadPrefabContents(prefabRootPath); - PrefabUtility.SavePrefabAsset(prefabRoot); - - // UdonSharpUtils.Log($"Ran prefab upgrade phase 2 on {prefabRoot}"); - } - catch (Exception e) + try + { + foreach (UdonBehaviour udonBehaviour in prefabRoot.GetComponentsInChildren(true)) { - UdonSharpUtils.LogError($"Encountered exception while upgrading prefab {prefabRoot}, report exception to Merlin: {e}"); + if (!NeedsSerializationUpgrade(udonBehaviour)) + continue; + + CopyUdonToProxy(GetProxyBehaviour(udonBehaviour), ProxySerializationPolicy.RootOnly); + + // We can't remove this data for backwards compatibility :'( + // If we nuke the data, the unity object array on the underlying storage may change. + // Which means that if people have copies of this prefab in the scene with no object reference changes, their data will also get nuked which we do not want. + // Public variable data on the prefabs will never be touched again by U# after upgrading + // We will probably provide an optional upgrade process that strips this extra data, and takes into account all scenes in the project + + // foreach (string publicVarSymbol in udonBehaviour.publicVariables.VariableSymbols.ToArray()) + // udonBehaviour.publicVariables.RemoveVariable(publicVarSymbol); + + SetBehaviourVersion(udonBehaviour, UdonSharpBehaviourVersion.V1); + SetBehaviourUpgraded(udonBehaviour); + + UdonSharpUtils.SetDirty(udonBehaviour); + UdonSharpUtils.SetDirty(GetProxyBehaviour(udonBehaviour)); } + + PrefabUtility.SaveAsPrefabAsset(prefabRoot, prefabRootPath); + + // UdonSharpUtils.Log($"Ran prefab upgrade phase 2 on {prefabRoot}"); + } + catch (Exception e) + { + UdonSharpUtils.LogError($"Encountered exception while upgrading prefab {prefabRootPath}, report exception to Merlin: {e}"); + } + finally + { + PrefabUtility.UnloadPrefabContents(prefabRoot); } - - UdonSharpUtils.Log("Prefab upgrade pass finished"); } + + UdonSharpUtils.Log("Prefab upgrade pass finished"); } internal static void UpgradeSceneBehaviours(IEnumerable behaviours) @@ -906,7 +974,7 @@ private static UdonSharpBehaviour FindProxyBehaviour_Internal(UdonBehaviour udon foreach (UdonSharpBehaviour udonSharpBehaviour in behaviours) { - IUdonBehaviour backingBehaviour = GetBackingUdonBehaviour(udonSharpBehaviour); + UdonBehaviour backingBehaviour = GetBackingUdonBehaviour(udonSharpBehaviour); if (backingBehaviour != null && ReferenceEquals(backingBehaviour, udonBehaviour)) { _proxyBehaviourLookup.Add(udonBehaviour, udonSharpBehaviour);