Skip to content

Commit 18c3e03

Browse files
dsarnoclaude
andauthored
Fix UnityEvent wiring via manage_components set_property (#757)
* Fix UnityEvent wiring via manage_components set_property (#631) UnityEventBase-derived fields set via reflection create disconnected objects that Unity's serialization layer doesn't track, causing m_PersistentCalls to be empty on save. Route these types through SerializedObject/SerializedProperty instead. Adds recursive SerializedProperty setter supporting Generic structs, arrays, ObjectReference, and leaf types (int, bool, float, string, enum). Includes fuzzy child property matching to handle batch_execute stripping underscores from nested JSON keys (tracked in #756). Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Fix batch_execute mangling nested value keys (#756) batch_execute NormalizeParameterKeys recursively converted all JSON keys to camelCase, including nested value data like Unity serialized property paths (m_PersistentCalls to mPersistentCalls). This broke UnityEvent wiring and any tool receiving structured nested data through batch_execute. Stop recursing into values -- only normalize top-level parameter keys. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Guard against null value in SerializedProperty string setter Address Sourcery review: check value == null before accessing value.Type to prevent NullReferenceException in the string case of SetSerializedPropertyRecursive. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent e670b07 commit 18c3e03

8 files changed

Lines changed: 765 additions & 21 deletions

File tree

MCPForUnity/Editor/Helpers/ComponentOps.cs

Lines changed: 318 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using Newtonsoft.Json.Linq;
55
using UnityEditor;
66
using UnityEngine;
7+
using UnityEngine.Events;
78

89
namespace MCPForUnity.Editor.Helpers
910
{
@@ -164,6 +165,15 @@ public static bool SetProperty(Component component, string propertyName, JToken
164165
BindingFlags flags = BindingFlags.Public | BindingFlags.Instance | BindingFlags.IgnoreCase;
165166
string normalizedName = ParamCoercion.NormalizePropertyName(propertyName);
166167

168+
// UnityEventBase-derived types must be set via SerializedProperty, not reflection.
169+
// Reflection creates a disconnected object that Unity's serialization layer doesn't track,
170+
// causing m_PersistentCalls to be empty when the scene is saved.
171+
Type memberType = ResolveMemberType(type, propertyName, normalizedName);
172+
if (memberType != null && typeof(UnityEventBase).IsAssignableFrom(memberType))
173+
{
174+
return SetViaSerializedProperty(component, propertyName, normalizedName, value, out error);
175+
}
176+
167177
// Try property first - check both original and normalized names for backwards compatibility
168178
PropertyInfo propInfo = type.GetProperty(propertyName, flags)
169179
?? type.GetProperty(normalizedName, flags);
@@ -374,6 +384,314 @@ private static bool AllowsMultiple(GameObject target, Type componentType)
374384

375385
return true;
376386
}
387+
388+
// --- UnityEvent SerializedProperty support ---
389+
390+
private static Type ResolveMemberType(Type componentType, string propertyName, string normalizedName)
391+
{
392+
BindingFlags flags = BindingFlags.Public | BindingFlags.Instance | BindingFlags.IgnoreCase;
393+
394+
PropertyInfo propInfo = componentType.GetProperty(propertyName, flags)
395+
?? componentType.GetProperty(normalizedName, flags);
396+
if (propInfo != null)
397+
return propInfo.PropertyType;
398+
399+
FieldInfo fieldInfo = componentType.GetField(propertyName, flags)
400+
?? componentType.GetField(normalizedName, flags);
401+
if (fieldInfo != null)
402+
return fieldInfo.FieldType;
403+
404+
fieldInfo = FindSerializedFieldInHierarchy(componentType, propertyName)
405+
?? FindSerializedFieldInHierarchy(componentType, normalizedName);
406+
if (fieldInfo != null)
407+
return fieldInfo.FieldType;
408+
409+
return null;
410+
}
411+
412+
private static bool SetViaSerializedProperty(Component component, string propertyName, string normalizedName, JToken value, out string error)
413+
{
414+
error = null;
415+
var so = new SerializedObject(component);
416+
417+
SerializedProperty prop = so.FindProperty(propertyName)
418+
?? so.FindProperty(normalizedName);
419+
if (prop == null)
420+
{
421+
error = $"SerializedProperty '{propertyName}' not found on component '{component.GetType().Name}'.";
422+
return false;
423+
}
424+
425+
if (!SetSerializedPropertyRecursive(prop, value, out error, 0))
426+
return false;
427+
428+
so.ApplyModifiedProperties();
429+
return true;
430+
}
431+
432+
private static bool SetSerializedPropertyRecursive(SerializedProperty prop, JToken value, out string error, int depth)
433+
{
434+
error = null;
435+
const int MaxDepth = 20;
436+
if (depth > MaxDepth)
437+
{
438+
error = $"Maximum recursion depth ({MaxDepth}) exceeded.";
439+
return false;
440+
}
441+
442+
try
443+
{
444+
// Array + JArray
445+
if (prop.isArray && prop.propertyType != SerializedPropertyType.String && value is JArray jArray)
446+
{
447+
prop.arraySize = jArray.Count;
448+
prop.serializedObject.ApplyModifiedProperties();
449+
prop.serializedObject.Update();
450+
451+
for (int i = 0; i < jArray.Count; i++)
452+
{
453+
var element = prop.GetArrayElementAtIndex(i);
454+
if (!SetSerializedPropertyRecursive(element, jArray[i], out error, depth + 1))
455+
return false;
456+
}
457+
return true;
458+
}
459+
460+
// Generic (struct/class) + JObject
461+
if (prop.propertyType == SerializedPropertyType.Generic && !prop.isArray && value is JObject jObj)
462+
{
463+
foreach (var kvp in jObj)
464+
{
465+
var child = FindPropertyRelativeFuzzy(prop, kvp.Key);
466+
if (child == null)
467+
{
468+
error = $"Sub-property '{kvp.Key}' not found under '{prop.propertyPath}'.";
469+
return false;
470+
}
471+
if (!SetSerializedPropertyRecursive(child, kvp.Value, out error, depth + 1))
472+
return false;
473+
}
474+
return true;
475+
}
476+
477+
// ObjectReference
478+
if (prop.propertyType == SerializedPropertyType.ObjectReference)
479+
return SetObjectReference(prop, value, out error);
480+
481+
// Leaf types
482+
switch (prop.propertyType)
483+
{
484+
case SerializedPropertyType.Integer:
485+
int intVal = ParamCoercion.CoerceInt(value, int.MinValue);
486+
if (intVal == int.MinValue && value?.Type != JTokenType.Integer)
487+
{
488+
if (value == null || value.Type == JTokenType.Null ||
489+
(value.Type == JTokenType.String && !int.TryParse(value.ToString(), out _)))
490+
{
491+
error = "Expected integer value.";
492+
return false;
493+
}
494+
}
495+
prop.intValue = intVal;
496+
return true;
497+
498+
case SerializedPropertyType.Boolean:
499+
if (value == null || value.Type == JTokenType.Null)
500+
{
501+
error = "Expected boolean value.";
502+
return false;
503+
}
504+
prop.boolValue = ParamCoercion.CoerceBool(value, false);
505+
return true;
506+
507+
case SerializedPropertyType.Float:
508+
float floatVal = ParamCoercion.CoerceFloat(value, float.NaN);
509+
if (float.IsNaN(floatVal))
510+
{
511+
error = "Expected float value.";
512+
return false;
513+
}
514+
prop.floatValue = floatVal;
515+
return true;
516+
517+
case SerializedPropertyType.String:
518+
prop.stringValue = value == null || value.Type == JTokenType.Null ? null : value.ToString();
519+
return true;
520+
521+
case SerializedPropertyType.Enum:
522+
return SetEnum(prop, value, out error);
523+
524+
default:
525+
error = $"Unsupported SerializedPropertyType: {prop.propertyType} at '{prop.propertyPath}'.";
526+
return false;
527+
}
528+
}
529+
catch (Exception ex)
530+
{
531+
error = $"Error setting '{prop.propertyPath}': {ex.Message}";
532+
return false;
533+
}
534+
}
535+
536+
private static bool SetObjectReference(SerializedProperty prop, JToken value, out string error)
537+
{
538+
error = null;
539+
540+
if (value == null || value.Type == JTokenType.Null)
541+
{
542+
prop.objectReferenceValue = null;
543+
return true;
544+
}
545+
546+
if (value.Type == JTokenType.Integer)
547+
{
548+
int id = value.Value<int>();
549+
var resolved = EditorUtility.InstanceIDToObject(id);
550+
if (resolved == null)
551+
{
552+
error = $"No object found with instanceID {id}.";
553+
return false;
554+
}
555+
prop.objectReferenceValue = resolved;
556+
return true;
557+
}
558+
559+
if (value is JObject jObj)
560+
{
561+
var idToken = jObj["instanceID"];
562+
if (idToken != null)
563+
{
564+
int id = ParamCoercion.CoerceInt(idToken, 0);
565+
var resolved = EditorUtility.InstanceIDToObject(id);
566+
if (resolved == null)
567+
{
568+
error = $"No object found with instanceID {id}.";
569+
return false;
570+
}
571+
prop.objectReferenceValue = resolved;
572+
return true;
573+
}
574+
575+
var guidToken = jObj["guid"];
576+
if (guidToken != null)
577+
{
578+
string path = AssetDatabase.GUIDToAssetPath(guidToken.ToString());
579+
if (string.IsNullOrEmpty(path))
580+
{
581+
error = $"No asset found for GUID '{guidToken}'.";
582+
return false;
583+
}
584+
prop.objectReferenceValue = AssetDatabase.LoadAssetAtPath<UnityEngine.Object>(path);
585+
return true;
586+
}
587+
588+
var pathToken = jObj["path"];
589+
if (pathToken != null)
590+
{
591+
string sanitized = AssetPathUtility.SanitizeAssetPath(pathToken.ToString());
592+
var resolved = AssetDatabase.LoadAssetAtPath<UnityEngine.Object>(sanitized);
593+
if (resolved == null)
594+
{
595+
error = $"No asset found at path '{pathToken}'.";
596+
return false;
597+
}
598+
prop.objectReferenceValue = resolved;
599+
return true;
600+
}
601+
602+
error = "Object reference must contain 'instanceID', 'guid', or 'path'.";
603+
return false;
604+
}
605+
606+
if (value.Type == JTokenType.String)
607+
{
608+
string strVal = value.ToString();
609+
if (strVal.StartsWith("Assets/", StringComparison.OrdinalIgnoreCase) || strVal.Contains("/"))
610+
{
611+
string sanitized = AssetPathUtility.SanitizeAssetPath(strVal);
612+
var resolved = AssetDatabase.LoadAssetAtPath<UnityEngine.Object>(sanitized);
613+
if (resolved == null)
614+
{
615+
error = $"No asset found at path '{strVal}'.";
616+
return false;
617+
}
618+
prop.objectReferenceValue = resolved;
619+
return true;
620+
}
621+
error = $"Cannot resolve object reference from string '{strVal}'.";
622+
return false;
623+
}
624+
625+
error = $"Unsupported object reference format: {value.Type}.";
626+
return false;
627+
}
628+
629+
/// <summary>
630+
/// Finds a child SerializedProperty by name, falling back to underscore-insensitive matching.
631+
/// The batch_execute transport can strip underscores from JSON keys
632+
/// (e.g. m_PersistentCalls → mPersistentCalls), so we iterate immediate children
633+
/// and compare with underscores removed.
634+
/// </summary>
635+
private static SerializedProperty FindPropertyRelativeFuzzy(SerializedProperty parent, string key)
636+
{
637+
var child = parent.FindPropertyRelative(key);
638+
if (child != null) return child;
639+
640+
string normalizedKey = key.Replace("_", "").ToLowerInvariant();
641+
642+
var end = parent.GetEndProperty();
643+
var iter = parent.Copy();
644+
if (!iter.Next(true)) return null;
645+
646+
while (!SerializedProperty.EqualContents(iter, end))
647+
{
648+
if (iter.depth == parent.depth + 1)
649+
{
650+
string normalizedName = iter.name.Replace("_", "").ToLowerInvariant();
651+
if (normalizedName == normalizedKey)
652+
return parent.FindPropertyRelative(iter.name);
653+
}
654+
if (!iter.Next(false))
655+
break;
656+
}
657+
658+
return null;
659+
}
660+
661+
private static bool SetEnum(SerializedProperty prop, JToken value, out string error)
662+
{
663+
error = null;
664+
var names = prop.enumNames;
665+
if (names == null || names.Length == 0)
666+
{
667+
error = "Enum has no names.";
668+
return false;
669+
}
670+
671+
if (value.Type == JTokenType.Integer)
672+
{
673+
int idx = value.Value<int>();
674+
if (idx < 0 || idx >= names.Length)
675+
{
676+
error = $"Enum index out of range: {idx}.";
677+
return false;
678+
}
679+
prop.enumValueIndex = idx;
680+
return true;
681+
}
682+
683+
string s = value.ToString();
684+
for (int i = 0; i < names.Length; i++)
685+
{
686+
if (string.Equals(names[i], s, StringComparison.OrdinalIgnoreCase))
687+
{
688+
prop.enumValueIndex = i;
689+
return true;
690+
}
691+
}
692+
error = $"Unknown enum name '{s}'.";
693+
return false;
694+
}
377695
}
378696
}
379697

MCPForUnity/Editor/Tools/BatchExecute.cs

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -225,31 +225,11 @@ private static JObject NormalizeParameterKeys(JObject source)
225225
foreach (var property in source.Properties())
226226
{
227227
string normalizedName = ToCamelCase(property.Name);
228-
normalized[normalizedName] = NormalizeToken(property.Value);
228+
normalized[normalizedName] = property.Value;
229229
}
230230
return normalized;
231231
}
232232

233-
private static JArray NormalizeArray(JArray source)
234-
{
235-
var normalized = new JArray();
236-
foreach (var token in source)
237-
{
238-
normalized.Add(NormalizeToken(token));
239-
}
240-
return normalized;
241-
}
242-
243-
private static JToken NormalizeToken(JToken token)
244-
{
245-
return token switch
246-
{
247-
JObject obj => NormalizeParameterKeys(obj),
248-
JArray arr => NormalizeArray(arr),
249-
_ => token.DeepClone()
250-
};
251-
}
252-
253233
private static string ToCamelCase(string key) => StringCaseUtility.ToCamelCase(key);
254234
}
255235
}
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
using UnityEngine;
2+
using UnityEngine.Events;
3+
4+
namespace TestNamespace
5+
{
6+
public class UnityEventTestComponent : MonoBehaviour
7+
{
8+
public UnityEvent onSimpleEvent;
9+
public UnityEvent<float> onFloatEvent;
10+
11+
[SerializeField]
12+
private UnityEvent _onPrivateEvent;
13+
}
14+
}

TestProjects/UnityMCPTests/Assets/Scripts/TestAsmdef/UnityEventTestComponent.cs.meta

Lines changed: 11 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)