Skip to content

Commit 8879ecb

Browse files
PereViaderPereViader
committed
Fix endless loop when the root prefab does not have the component and a variant has it
1 parent 44445a4 commit 8879ecb

10 files changed

Lines changed: 245 additions & 108 deletions

‎Assets/ManualReserialization/Scripts/AssetDatabaseUtils.cs‎

Lines changed: 89 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#if UNITY_EDITOR
22
using System;
33
using System.Collections.Generic;
4+
using System.Linq;
45
using UnityEditor;
56
using UnityEngine;
67

@@ -9,58 +10,112 @@ namespace PereViader.ManualReserialization
910
public static class AssetDatabaseUtils
1011
{
1112
/// <summary>
12-
/// Gets all the prefabs that have some component and sorts them first so the root prefabs come before the variants
13+
/// Lazily gets all prefab asset paths for prefabs that have a specific component type
14+
/// anywhere in their hierarchy, sorted so that base prefab paths come before their variants.
15+
/// GameObjects are loaded only transiently during checking/sorting or finally by the caller.
1316
/// </summary>
17+
/// <param name="type">The Component type to search for.</param>
18+
/// <returns>An IEnumerable of asset paths (strings) for qualifying prefabs, sorted by dependency.</returns>
1419
public static IEnumerable<GameObject> GetAllPrefabsWithComponentSortedByVariant(Type type)
1520
{
16-
var prefabs = GetAllPrefabsWithComponent(type);
17-
return PerformBreadthFirstSearch(prefabs, PrefabUtility.GetCorrespondingObjectFromSource);
21+
var pathsWithComponent = GetAllPrefabPathsWithComponent(type);
22+
23+
var dependencyCache = new Dictionary<string, string>();
24+
25+
Func<string, string> getDependencyPath = (currentPath) =>
26+
{
27+
if (dependencyCache.TryGetValue(currentPath, out var cachedDependencyPath))
28+
{
29+
return cachedDependencyPath;
30+
}
31+
32+
string resultDependencyPath = null;
33+
GameObject currentGO = AssetDatabase.LoadAssetAtPath<GameObject>(currentPath);
34+
if (currentGO is not null)
35+
{
36+
GameObject dependencyGO = PrefabUtility.GetCorrespondingObjectFromSource(currentGO);
37+
if (dependencyGO is not null)
38+
{
39+
resultDependencyPath = AssetDatabase.GetAssetPath(dependencyGO);
40+
if (string.IsNullOrEmpty(resultDependencyPath)) {
41+
// Should not happen for valid source objects, but handle defensively
42+
Debug.LogWarning($"Could not get asset path for dependency of {currentPath}");
43+
resultDependencyPath = null; // Treat as root if path is invalid
44+
}
45+
}
46+
}
47+
else {
48+
Debug.LogWarning($"Could not load GameObject at path '{currentPath}' during dependency check.");
49+
}
50+
51+
// Store in cache (even if null)
52+
dependencyCache[currentPath] = resultDependencyPath;
53+
return resultDependencyPath;
54+
};
55+
56+
foreach (var path in PerformBreadthFirstSort(pathsWithComponent, getDependencyPath))
57+
{
58+
yield return AssetDatabase.LoadAssetAtPath<GameObject>(path);
59+
}
1860
}
19-
61+
2062
/// <summary>
21-
/// Takes some Enumerable and sorts it so using a breath first search approach so that dependencies of each element must always come before dependants
63+
/// Lazily finds the asset paths of all prefabs that contain a given component type.
64+
/// Loads GameObjects only temporarily to check for the component.
2265
/// </summary>
23-
private static IEnumerable<T> PerformBreadthFirstSearch<T>(IEnumerable<T> elements, Func<T, T> getDependency) where T : class
66+
/// <param name="type">The Component type to search for.</param>
67+
/// <returns>An IEnumerable of asset paths (strings).</returns>
68+
private static IEnumerable<string> GetAllPrefabPathsWithComponent(Type type)
2469
{
25-
var queue = new Queue<T>(elements);
26-
var visited = new HashSet<T>(queue.Count);
27-
28-
while (queue.Count > 0)
70+
var guids = AssetDatabase.FindAssets("t:Prefab");
71+
foreach (var guid in guids)
2972
{
30-
var currentElement = queue.Dequeue();
31-
var dependency = getDependency(currentElement);
32-
if (dependency != null && !visited.Contains(dependency))
73+
var path = AssetDatabase.GUIDToAssetPath(guid);
74+
if (string.IsNullOrEmpty(path))
3375
{
34-
queue.Enqueue(currentElement);
76+
// Skip if path is invalid
3577
continue;
3678
}
37-
38-
visited.Add(currentElement);
39-
yield return currentElement;
79+
80+
var rootGameObject = AssetDatabase.LoadAssetAtPath<GameObject>(path);
81+
if (rootGameObject is null)
82+
{
83+
// Silently skip if prefab asset is broken/unloadable
84+
continue;
85+
}
86+
87+
if (rootGameObject.GetComponentInChildren(type, true) is not null)
88+
{
89+
yield return path;
90+
}
4091
}
4192
}
4293

43-
public static IEnumerable<GameObject> GetAllPrefabsWithComponent(Type type)
94+
private static IEnumerable<T> PerformBreadthFirstSort<T>(IEnumerable<T> items, Func<T, T> getDependency) where T : class // Changed name slightly for clarity
4495
{
45-
var guids = AssetDatabase.FindAssets("t:Prefab");
46-
foreach (var guid in guids)
96+
var uniqueItems = new HashSet<T>(items);
97+
if (uniqueItems.Count == 0)
4798
{
48-
var path = AssetDatabase.GUIDToAssetPath(guid);
49-
var assetsAtPath = AssetDatabase.LoadAllAssetsAtPath(path);
99+
yield break;
100+
}
50101

51-
foreach (var asset in assetsAtPath)
52-
{
53-
if (!(asset is GameObject gameObject))
54-
{
55-
continue;
56-
}
102+
var queue = new Queue<T>(uniqueItems);
103+
var visited = new HashSet<T>(uniqueItems.Count);
57104

58-
var component = gameObject.GetComponentInChildren(type);
59-
if (component != null)
60-
{
61-
yield return gameObject;
62-
continue;
63-
}
105+
while (queue.Count > 0)
106+
{
107+
var currentItem = queue.Dequeue();
108+
var dependency = getDependency(currentItem);
109+
110+
if (dependency is not null && uniqueItems.Contains(dependency) && !visited.Contains(dependency))
111+
{
112+
queue.Enqueue(currentItem);
113+
continue;
114+
}
115+
116+
if (visited.Add(currentItem))
117+
{
118+
yield return currentItem;
64119
}
65120
}
66121
}

‎Assets/ManualReserialization/Tests/DeleteAssetsTearDown.cs‎

Lines changed: 100 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
namespace PereViader.ManualReserialization.Tests
99
{
10-
public class DeleteAssetsTearDown
10+
public class DeleteAssetsTearDown : IDisposable
1111
{
1212
private readonly List<string> assetsToDestroy = new List<string>();
1313

@@ -59,7 +59,8 @@ public Action<Action<T>> CreatePrefabWithComponent<T>(string name) where T : Mon
5959
/// <param name="name">Name to identify the objects</param>
6060
/// <param name="alterVariant">Action where to change the prefab variant</param>
6161
/// <returns>Action where the first element is the prefab and the second is the variant</returns>
62-
public Action<Action<T, T>> CreatePrefabAndVariantWithComponent<T>(string name, Action<T> alterVariant) where T : MonoBehaviour
62+
public Action<Action<T, T>> CreatePrefabAndVariantWithComponent<T>(string name, Action<T> alterVariant)
63+
where T : MonoBehaviour
6364
{
6465
var pathPrefab = AssetDatabase.GenerateUniqueAssetPath("Assets/TestPrefabAsset.prefab");
6566
var pathVariant = AssetDatabase.GenerateUniqueAssetPath("Assets/TestPrefabVariantAsset.prefab");
@@ -128,7 +129,102 @@ public Action<Action<T>> CreateSceneWithGameObjectComponent<T>(string name) wher
128129
};
129130
}
130131

131-
public void TearDown()
132+
public (string rootPath, string variantPath) CreateVariantWhereRootLacksComponent<T>(string baseName)
133+
where T : MonoBehaviour
134+
{
135+
var rootPath = AssetDatabase.GenerateUniqueAssetPath($"Assets/{baseName}_Root.prefab");
136+
var variantPath = AssetDatabase.GenerateUniqueAssetPath($"Assets/{baseName}_Variant.prefab");
137+
// Ensure these paths are registered for deletion in TearDown
138+
assetsToDestroy.Add(variantPath);
139+
assetsToDestroy.Add(rootPath);
140+
141+
GameObject rootGo = null;
142+
GameObject variantGo = null; // Instance used to create the variant asset
143+
GameObject rootPrefabAsset = null;
144+
GameObject variantPrefabAsset = null;
145+
146+
try
147+
{
148+
// 1. Create the root GameObject (without the component)
149+
rootGo = new GameObject($"{baseName}_Root");
150+
151+
// --- Use SaveAsPrefabAssetAndConnect for the root ---
152+
// This creates the asset and connects rootGo to it.
153+
// It also handles destroying rootGo automatically on success.
154+
rootPrefabAsset = PrefabUtility.SaveAsPrefabAssetAndConnect(
155+
rootGo,
156+
rootPath,
157+
InteractionMode.AutomatedAction);
158+
159+
if (rootPrefabAsset == null)
160+
{
161+
// If rootGo wasn't destroyed automatically due to failure, clean it up.
162+
if (rootGo != null) GameObject.DestroyImmediate(rootGo);
163+
throw new Exception($"Failed to save root prefab at {rootPath}");
164+
}
165+
// Note: rootGo is likely destroyed or invalid after SaveAsPrefabAssetAndConnect succeeds.
166+
// We work with rootPrefabAsset asset from now on.
167+
168+
// 2. Create the variant instance based on the saved root prefab *asset*
169+
variantGo = (GameObject)PrefabUtility.InstantiatePrefab(rootPrefabAsset);
170+
if (variantGo == null) throw new Exception("Failed to instantiate prefab asset for variant creation.");
171+
variantGo.name = $"{baseName}_Variant";
172+
173+
// 3. Add the component ONLY to the variant instance
174+
var component = variantGo.AddComponent<T>();
175+
// --- Mark the instance dirty after adding component ---
176+
EditorUtility.SetDirty(component);
177+
EditorUtility.SetDirty(variantGo);
178+
179+
// 4. Save the modified variant instance as a new prefab asset (the variant)
180+
// Use SaveAsPrefabAsset, we don't want to connect variantGo back.
181+
variantPrefabAsset = PrefabUtility.SaveAsPrefabAsset(variantGo, variantPath);
182+
if (variantPrefabAsset == null) throw new Exception($"Failed to save variant prefab at {variantPath}");
183+
// --- Mark the new variant asset dirty ---
184+
EditorUtility.SetDirty(variantPrefabAsset);
185+
186+
// 5. Save assets *after* both prefabs are created and potentially dirtied
187+
AssetDatabase.SaveAssets();
188+
AssetDatabase.Refresh();
189+
190+
// --- Verification (Optional but recommended) ---
191+
// Reload assets fresh from disk to ensure links are correct after save/refresh
192+
var loadedRoot = AssetDatabase.LoadAssetAtPath<GameObject>(rootPath);
193+
var loadedVariant = AssetDatabase.LoadAssetAtPath<GameObject>(variantPath);
194+
if (loadedRoot == null || loadedVariant == null)
195+
throw new Exception("Failed to load created prefabs after save.");
196+
if (loadedRoot.GetComponent<T>() != null)
197+
throw new Exception("Test Setup Error: Root prefab incorrectly has the component.");
198+
if (loadedVariant.GetComponent<T>() == null)
199+
throw new Exception("Test Setup Error: Variant prefab lacks the component.");
200+
201+
var sourceOfVariant = PrefabUtility.GetCorrespondingObjectFromSource(loadedVariant);
202+
if (sourceOfVariant == null)
203+
throw new Exception("Test Setup Error: Variant link to source is null after save.");
204+
if (sourceOfVariant.GetInstanceID() != loadedRoot.GetInstanceID())
205+
throw new Exception(
206+
$"Test Setup Error: Variant points to the wrong source. Expected {loadedRoot.name}, got {sourceOfVariant.name}");
207+
208+
return (rootPath, variantPath);
209+
}
210+
finally
211+
{
212+
// Clean up the temporary scene instance used for the variant
213+
// rootGo should have been handled by SaveAsPrefabAssetAndConnect
214+
if (variantGo != null)
215+
{
216+
GameObject.DestroyImmediate(variantGo);
217+
}
218+
219+
// Belt-and-suspenders: Ensure rootGo is gone if SaveAsPrefabAssetAndConnect failed early
220+
if (rootGo != null && rootPrefabAsset == null)
221+
{
222+
GameObject.DestroyImmediate(rootGo);
223+
}
224+
}
225+
}
226+
227+
public void Dispose()
132228
{
133229
AssetDatabase.Refresh();
134230
foreach (var path in assetsToDestroy)
@@ -143,6 +239,7 @@ public void TearDown()
143239
Debug.LogError($"Could not delete asset with path {path}");
144240
}
145241
}
242+
146243
assetsToDestroy.Clear();
147244
}
148245
}

‎Assets/ManualReserialization/Tests/TestMonoBehaviourReserializer.cs‎

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,21 +8,14 @@ namespace PereViader.ManualReserialization.Tests
88
[TestFixture]
99
public class TestMonoBehaviourReserializer
1010
{
11-
private readonly DeleteAssetsTearDown deleteAssetsTearDown = new DeleteAssetsTearDown();
12-
13-
[TearDown]
14-
public void TearDown()
15-
{
16-
deleteAssetsTearDown.TearDown();
17-
}
18-
1911
[Test]
2012
public void TestReserializeMonoBehaviourWithPublicToFindPrefab()
2113
{
14+
using DeleteAssetsTearDown deleteAssetsTearDown = new();
2215
var asset1 = deleteAssetsTearDown.CreatePrefabWithComponent<MonoBehaviourWithPublicToFind>("asset1");
2316
var asset2 = deleteAssetsTearDown.CreatePrefabWithComponent<MonoBehaviourWithPublicToFind>("asset2");
2417

25-
MonoBehaviourReserializer.Reserialize<MonoBehaviourWithPublicToFind>(x =>
18+
MonoBehaviourReserializer.Reserialize<MonoBehaviourWithPublicToFind>((x, m) =>
2619
{
2720
x.toFind.newValue = x.toFind.previousValue + 1;
2821
});
@@ -34,10 +27,11 @@ public void TestReserializeMonoBehaviourWithPublicToFindPrefab()
3427
[Test]
3528
public void TestReserializeMonoBehaviourWithDoubleNestedToFindPrefab()
3629
{
30+
using DeleteAssetsTearDown deleteAssetsTearDown = new();
3731
var asset1 = deleteAssetsTearDown.CreatePrefabWithComponent<MonoBehaviourWithDoubleNestedToFind>("asset1");
3832
var asset2 = deleteAssetsTearDown.CreatePrefabWithComponent<MonoBehaviourWithDoubleNestedToFind>("asset2");
3933

40-
MonoBehaviourReserializer.Reserialize<MonoBehaviourWithDoubleNestedToFind>(x =>
34+
MonoBehaviourReserializer.Reserialize<MonoBehaviourWithDoubleNestedToFind>((x, m) =>
4135
{
4236
x.doubleNestedToFind.toFind.toFind.newValue = x.doubleNestedToFind.toFind.toFind.previousValue + 1;
4337
});
@@ -49,10 +43,11 @@ public void TestReserializeMonoBehaviourWithDoubleNestedToFindPrefab()
4943
[Test]
5044
public void TestReserializeMonoBehaviourWithPublicToFindScene()
5145
{
46+
using DeleteAssetsTearDown deleteAssetsTearDown = new();
5247
var asset1 = deleteAssetsTearDown.CreateSceneWithGameObjectComponent<MonoBehaviourWithPublicToFind>("asset1");
5348
var asset2 = deleteAssetsTearDown.CreateSceneWithGameObjectComponent<MonoBehaviourWithPublicToFind>("asset2");
5449

55-
MonoBehaviourReserializer.Reserialize<MonoBehaviourWithPublicToFind>(x =>
50+
MonoBehaviourReserializer.Reserialize<MonoBehaviourWithPublicToFind>((x, m) =>
5651
{
5752
x.toFind.newValue = x.toFind.previousValue + 1;
5853
});
@@ -64,10 +59,11 @@ public void TestReserializeMonoBehaviourWithPublicToFindScene()
6459
[Test]
6560
public void TestReserializeMonoBehaviourWithDoubleNestedToFindScene()
6661
{
62+
using DeleteAssetsTearDown deleteAssetsTearDown = new();
6763
var asset1 = deleteAssetsTearDown.CreateSceneWithGameObjectComponent<MonoBehaviourWithDoubleNestedToFind>("asset1");
6864
var asset2 = deleteAssetsTearDown.CreateSceneWithGameObjectComponent<MonoBehaviourWithDoubleNestedToFind>("asset2");
6965

70-
MonoBehaviourReserializer.Reserialize<MonoBehaviourWithDoubleNestedToFind>(x =>
66+
MonoBehaviourReserializer.Reserialize<MonoBehaviourWithDoubleNestedToFind>((x, m) =>
7167
{
7268
x.doubleNestedToFind.toFind.toFind.newValue = x.doubleNestedToFind.toFind.toFind.previousValue + 1;
7369
});
@@ -79,9 +75,10 @@ public void TestReserializeMonoBehaviourWithDoubleNestedToFindScene()
7975
[Test]
8076
public void TestReserializeMonoBehaviourWithPublicToFindPrefabVariantUnchanged()
8177
{
78+
using DeleteAssetsTearDown deleteAssetsTearDown = new();
8279
var asset = deleteAssetsTearDown.CreatePrefabAndVariantWithComponent<MonoBehaviourWithPublicToFind>("asset1", x => { });
8380

84-
MonoBehaviourReserializer.Reserialize<MonoBehaviourWithPublicToFind>(x =>
81+
MonoBehaviourReserializer.Reserialize<MonoBehaviourWithPublicToFind>((x, m) =>
8582
{
8683
x.toFind.newValue = x.toFind.previousValue + 1;
8784
});
@@ -97,11 +94,12 @@ public void TestReserializeMonoBehaviourWithPublicToFindPrefabVariantUnchanged()
9794
[Test]
9895
public void TestReserializeMonoBehaviourWithPublicToFindPrefabVariantChanged()
9996
{
97+
using DeleteAssetsTearDown deleteAssetsTearDown = new();
10098
var asset = deleteAssetsTearDown.CreatePrefabAndVariantWithComponent<MonoBehaviourWithPublicToFind>("asset",
10199
x => x.toFind.previousValue = 5
102100
);
103101

104-
MonoBehaviourReserializer.Reserialize<MonoBehaviourWithPublicToFind>(x =>
102+
MonoBehaviourReserializer.Reserialize<MonoBehaviourWithPublicToFind>((x, m) =>
105103
{
106104
x.toFind.newValue = x.toFind.previousValue + 1;
107105
});

0 commit comments

Comments
 (0)