Skip to content

Commit 51eb59f

Browse files
committed
Fix(MCP): Resolve ValidTRS crash during component serialization
The Unity Editor was crashing with ValidTRS() assertions when attempting to get components from certain GameObjects like the Main Camera. Investigation revealed the crash occurred during JSON serialization when reflection code accessed specific matrix properties (e.g., Camera.cullingMatrix, Transform.rotation, Transform.lossyScale). Accessing these properties appears to trigger internal Transform state validation failures, potentially due to interactions with the JSON serializer's reflection mechanism. This fix addresses the issue by: - Replacing LINQ iteration in GetComponentsFromTarget with a standard loop over a copied list to prevent potential premature serialization interactions. - Explicitly skipping known problematic Camera matrix properties (cullingMatrix, pixelRect, rect) and generic matrix properties (worldToLocalMatrix, localToWorldMatrix) within GetComponentData's reflection logic. - Retaining manual serialization for Transform component properties to avoid related reflection issues.
1 parent bd85072 commit 51eb59f

File tree

2 files changed

+135
-14
lines changed

2 files changed

+135
-14
lines changed

UnityMcpBridge/Editor/Helpers/GameObjectSerializer.cs

Lines changed: 86 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ namespace UnityMcpBridge.Editor.Helpers
1313
/// <summary>
1414
/// Handles serialization of GameObjects and Components for MCP responses.
1515
/// Includes reflection helpers and caching for performance.
16-
/// </summary>
16+
/// </summary> tew
1717
public static class GameObjectSerializer
1818
{
1919
// --- Data Serialization ---
@@ -121,9 +121,42 @@ public CachedMetadata(List<PropertyInfo> properties, List<FieldInfo> fields)
121121
// Add the flag parameter here
122122
public static object GetComponentData(Component c, bool includeNonPublicSerializedFields = true)
123123
{
124+
// --- Add Early Logging ---
125+
// Debug.Log($"[GetComponentData] Starting for component: {c?.GetType()?.FullName ?? "null"} (ID: {c?.GetInstanceID() ?? 0})");
126+
// --- End Early Logging ---
127+
124128
if (c == null) return null;
125129
Type componentType = c.GetType();
126130

131+
// --- Special handling for Transform to avoid reflection crashes and problematic properties ---
132+
if (componentType == typeof(Transform))
133+
{
134+
Transform tr = c as Transform;
135+
// Debug.Log($"[GetComponentData] Manually serializing Transform (ID: {tr.GetInstanceID()})");
136+
return new Dictionary<string, object>
137+
{
138+
{ "typeName", componentType.FullName },
139+
{ "instanceID", tr.GetInstanceID() },
140+
// Manually extract known-safe properties. Avoid Quaternion 'rotation' and 'lossyScale'.
141+
{ "position", CreateTokenFromValue(tr.position, typeof(Vector3))?.ToObject<object>() ?? new JObject() },
142+
{ "localPosition", CreateTokenFromValue(tr.localPosition, typeof(Vector3))?.ToObject<object>() ?? new JObject() },
143+
{ "eulerAngles", CreateTokenFromValue(tr.eulerAngles, typeof(Vector3))?.ToObject<object>() ?? new JObject() }, // Use Euler angles
144+
{ "localEulerAngles", CreateTokenFromValue(tr.localEulerAngles, typeof(Vector3))?.ToObject<object>() ?? new JObject() },
145+
{ "localScale", CreateTokenFromValue(tr.localScale, typeof(Vector3))?.ToObject<object>() ?? new JObject() },
146+
{ "right", CreateTokenFromValue(tr.right, typeof(Vector3))?.ToObject<object>() ?? new JObject() },
147+
{ "up", CreateTokenFromValue(tr.up, typeof(Vector3))?.ToObject<object>() ?? new JObject() },
148+
{ "forward", CreateTokenFromValue(tr.forward, typeof(Vector3))?.ToObject<object>() ?? new JObject() },
149+
{ "parentInstanceID", tr.parent?.gameObject.GetInstanceID() ?? 0 },
150+
{ "rootInstanceID", tr.root?.gameObject.GetInstanceID() ?? 0 },
151+
{ "childCount", tr.childCount },
152+
// Include standard Object/Component properties
153+
{ "name", tr.name },
154+
{ "tag", tr.tag },
155+
{ "gameObjectInstanceID", tr.gameObject?.GetInstanceID() ?? 0 }
156+
};
157+
}
158+
// --- End Special handling for Transform ---
159+
127160
var data = new Dictionary<string, object>
128161
{
129162
{ "typeName", componentType.FullName },
@@ -195,11 +228,18 @@ public static object GetComponentData(Component c, bool includeNonPublicSerializ
195228

196229
// --- Use cached metadata ---
197230
var serializablePropertiesOutput = new Dictionary<string, object>();
231+
232+
// --- Add Logging Before Property Loop ---
233+
// Debug.Log($"[GetComponentData] Starting property loop for {componentType.Name}...");
234+
// --- End Logging Before Property Loop ---
235+
198236
// Use cached properties
199237
foreach (var propInfo in cachedData.SerializableProperties)
200238
{
201-
// --- Skip known obsolete/problematic Component shortcut properties ---
202239
string propName = propInfo.Name;
240+
241+
// --- Skip known obsolete/problematic Component shortcut properties ---
242+
bool skipProperty = false;
203243
if (propName == "rigidbody" || propName == "rigidbody2D" || propName == "camera" ||
204244
propName == "light" || propName == "animation" || propName == "constantForce" ||
205245
propName == "renderer" || propName == "audio" || propName == "networkView" ||
@@ -208,35 +248,71 @@ public static object GetComponentData(Component c, bool includeNonPublicSerializ
208248
// Also skip potentially problematic Matrix properties prone to cycles/errors
209249
propName == "worldToLocalMatrix" || propName == "localToWorldMatrix")
210250
{
211-
continue; // Skip these properties
251+
// Debug.Log($"[GetComponentData] Explicitly skipping generic property: {propName}"); // Optional log
252+
skipProperty = true;
253+
}
254+
// --- End Skip Generic Properties ---
255+
256+
// --- Skip specific potentially problematic Camera properties ---
257+
if (componentType == typeof(Camera) &&
258+
(propName == "pixelRect" ||
259+
propName == "rect" ||
260+
propName == "cullingMatrix"))
261+
{
262+
// Debug.Log($"[GetComponentData] Explicitly skipping Camera property: {propName}");
263+
skipProperty = true;
264+
}
265+
// --- End Skip Camera Properties ---
266+
267+
// --- Skip specific potentially problematic Transform properties ---
268+
if (componentType == typeof(Transform) && (propName == "lossyScale" || propName == "rotation"))
269+
{
270+
// Debug.Log($"[GetComponentData] Explicitly skipping Transform property: {propName}");
271+
skipProperty = true;
272+
}
273+
// --- End Skip Transform Properties ---
274+
275+
// Skip if flagged
276+
if (skipProperty)
277+
{
278+
continue;
212279
}
213-
// --- End Skip ---
214280

215281
try
216282
{
283+
// --- Add detailed logging ---
284+
// Debug.Log($"[GetComponentData] Accessing: {componentType.Name}.{propName}");
285+
// --- End detailed logging ---
217286
object value = propInfo.GetValue(c);
218287
Type propType = propInfo.PropertyType;
219288
AddSerializableValue(serializablePropertiesOutput, propName, propType, value);
220289
}
221290
catch (Exception ex)
222291
{
223-
Debug.LogWarning($"Could not read property {propName} on {componentType.Name}: {ex.Message}");
292+
// Debug.LogWarning($"Could not read property {propName} on {componentType.Name}: {ex.Message}");
224293
}
225294
}
226295

296+
// --- Add Logging Before Field Loop ---
297+
// Debug.Log($"[GetComponentData] Starting field loop for {componentType.Name}...");
298+
// --- End Logging Before Field Loop ---
299+
227300
// Use cached fields
228301
foreach (var fieldInfo in cachedData.SerializableFields)
229302
{
230303
try
231304
{
305+
// --- Add detailed logging for fields ---
306+
// Debug.Log($"[GetComponentData] Accessing Field: {componentType.Name}.{fieldInfo.Name}");
307+
// --- End detailed logging for fields ---
232308
object value = fieldInfo.GetValue(c);
233309
string fieldName = fieldInfo.Name;
234310
Type fieldType = fieldInfo.FieldType;
235311
AddSerializableValue(serializablePropertiesOutput, fieldName, fieldType, value);
236312
}
237313
catch (Exception ex)
238314
{
239-
Debug.LogWarning($"Could not read field {fieldInfo.Name} on {componentType.Name}: {ex.Message}");
315+
// Debug.LogWarning($"Could not read field {fieldInfo.Name} on {componentType.Name}: {ex.Message}");
240316
}
241317
}
242318
// --- End Use cached metadata ---
@@ -273,7 +349,7 @@ private static void AddSerializableValue(Dictionary<string, object> dict, string
273349
catch (Exception e)
274350
{
275351
// Catch potential errors during JToken conversion or addition to dictionary
276-
Debug.LogWarning($"[AddSerializableValue] Error processing value for '{name}' (Type: {type.FullName}): {e.Message}. Skipping.");
352+
// Debug.LogWarning($"[AddSerializableValue] Error processing value for '{name}' (Type: {type.FullName}): {e.Message}. Skipping.");
277353
}
278354
}
279355

@@ -329,7 +405,7 @@ private static object ConvertJTokenToPlainObject(JToken token)
329405
{
330406
return jValue.Value;
331407
}
332-
Debug.LogWarning($"Unsupported JTokenType encountered: {token.Type}. Returning null.");
408+
// Debug.LogWarning($"Unsupported JTokenType encountered: {token.Type}. Returning null.");
333409
return null;
334410
}
335411
}
@@ -365,12 +441,12 @@ private static JToken CreateTokenFromValue(object value, Type type)
365441
}
366442
catch (JsonSerializationException e)
367443
{
368-
Debug.LogWarning($"[GameObjectSerializer] Newtonsoft.Json Error serializing value of type {type.FullName}: {e.Message}. Skipping property/field.");
444+
// Debug.LogWarning($"[GameObjectSerializer] Newtonsoft.Json Error serializing value of type {type.FullName}: {e.Message}. Skipping property/field.");
369445
return null; // Indicate serialization failure
370446
}
371447
catch (Exception e) // Catch other unexpected errors
372448
{
373-
Debug.LogWarning($"[GameObjectSerializer] Unexpected error serializing value of type {type.FullName}: {e}. Skipping property/field.");
449+
// Debug.LogWarning($"[GameObjectSerializer] Unexpected error serializing value of type {type.FullName}: {e}. Skipping property/field.");
374450
return null; // Indicate serialization failure
375451
}
376452
}

UnityMcpBridge/Editor/Tools/ManageGameObject.cs

Lines changed: 49 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -837,12 +837,57 @@ private static object GetComponentsFromTarget(string target, string searchMethod
837837

838838
try
839839
{
840-
Component[] components = targetGo.GetComponents<Component>();
841-
// Use the new serializer helper and pass the flag
842-
var componentData = components.Select(c => Helpers.GameObjectSerializer.GetComponentData(c, includeNonPublicSerialized)).ToList();
840+
// --- Get components, immediately copy to list, and null original array ---
841+
Component[] originalComponents = targetGo.GetComponents<Component>();
842+
List<Component> componentsToIterate = new List<Component>(originalComponents ?? Array.Empty<Component>()); // Copy immediately, handle null case
843+
int componentCount = componentsToIterate.Count;
844+
originalComponents = null; // Null the original reference
845+
// Debug.Log($"[GetComponentsFromTarget] Found {componentCount} components on {targetGo.name}. Copied to list, nulled original. Starting REVERSE for loop...");
846+
// --- End Copy and Null ---
847+
848+
var componentData = new List<object>();
849+
850+
for (int i = componentCount - 1; i >= 0; i--) // Iterate backwards over the COPY
851+
{
852+
Component c = componentsToIterate[i]; // Use the copy
853+
if (c == null)
854+
{
855+
// Debug.LogWarning($"[GetComponentsFromTarget REVERSE for] Encountered a null component at index {i} on {targetGo.name}. Skipping.");
856+
continue; // Safety check
857+
}
858+
// Debug.Log($"[GetComponentsFromTarget REVERSE for] Processing component: {c.GetType()?.FullName ?? "null"} (ID: {c.GetInstanceID()}) at index {i} on {targetGo.name}");
859+
try
860+
{
861+
var data = Helpers.GameObjectSerializer.GetComponentData(c, includeNonPublicSerialized);
862+
if (data != null) // Ensure GetComponentData didn't return null
863+
{
864+
componentData.Insert(0, data); // Insert at beginning to maintain original order in final list
865+
}
866+
// else
867+
// {
868+
// Debug.LogWarning($"[GetComponentsFromTarget REVERSE for] GetComponentData returned null for component {c.GetType().FullName} (ID: {c.GetInstanceID()}) on {targetGo.name}. Skipping addition.");
869+
// }
870+
}
871+
catch (Exception ex)
872+
{
873+
Debug.LogError($"[GetComponentsFromTarget REVERSE for] Error processing component {c.GetType().FullName} (ID: {c.GetInstanceID()}) on {targetGo.name}: {ex.Message}\n{ex.StackTrace}");
874+
// Optionally add placeholder data or just skip
875+
componentData.Insert(0, new JObject( // Insert error marker at beginning
876+
new JProperty("typeName", c.GetType().FullName + " (Serialization Error)"),
877+
new JProperty("instanceID", c.GetInstanceID()),
878+
new JProperty("error", ex.Message)
879+
));
880+
}
881+
}
882+
// Debug.Log($"[GetComponentsFromTarget] Finished REVERSE for loop.");
883+
884+
// Cleanup the list we created
885+
componentsToIterate.Clear();
886+
componentsToIterate = null;
887+
843888
return Response.Success(
844889
$"Retrieved {componentData.Count} components from '{targetGo.name}'.",
845-
componentData
890+
componentData // List was built in original order
846891
);
847892
}
848893
catch (Exception e)

0 commit comments

Comments
 (0)