Skip to content

Commit b110b72

Browse files
committed
fix: address code review — shared helper, safe coercion, array support
- Extract ApplyReferenceToProperty shared helper to deduplicate SetReference and BatchWire write logic - BatchWire now reuses single SerializedObject instead of recreating per validation entry - ResolveReference uses ParamCoercion.CoerceInt instead of unsafe JToken.Value<int>() to prevent exceptions on malformed input - GetFieldType now handles array/list element paths (e.g., "targets.Array.data[0]") by extracting element type
1 parent 49f51cf commit b110b72

1 file changed

Lines changed: 48 additions & 15 deletions

File tree

MCPForUnity/Editor/Tools/ManageComponents.cs

Lines changed: 48 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -320,9 +320,7 @@ private static object SetReference(JObject @params, JToken targetToken, string s
320320

321321
var previousValue = DescribeObjectReference(property.objectReferenceValue);
322322

323-
Undo.RecordObject(component, $"Set reference {property.propertyPath}");
324-
property.objectReferenceValue = validation.ResolvedObject;
325-
serializedObject.ApplyModifiedProperties();
323+
ApplyReferenceToProperty(component, serializedObject, property, validation.ResolvedObject);
326324
EditorUtility.SetDirty(component);
327325
MarkOwningSceneDirty(targetGo);
328326

@@ -442,6 +440,7 @@ private static object BatchWire(JObject @params, JToken targetToken, string sear
442440
Undo.SetCurrentGroupName($"Batch wire references on {component.GetType().Name}");
443441
int succeeded = 0;
444442
int failed = 0;
443+
var serializedObject = new SerializedObject(component);
445444

446445
try
447446
{
@@ -453,7 +452,7 @@ private static object BatchWire(JObject @params, JToken targetToken, string sear
453452
continue;
454453
}
455454

456-
var serializedObject = new SerializedObject(component);
455+
serializedObject.Update();
457456
var property = serializedObject.FindProperty(validation.PropertyName);
458457
if (property == null || property.propertyType != SerializedPropertyType.ObjectReference)
459458
{
@@ -468,9 +467,7 @@ private static object BatchWire(JObject @params, JToken targetToken, string sear
468467
continue;
469468
}
470469

471-
Undo.RecordObject(component, $"Set reference {validation.PropertyName}");
472-
property.objectReferenceValue = validation.ResolvedObject;
473-
serializedObject.ApplyModifiedProperties();
470+
ApplyReferenceToProperty(component, serializedObject, property, validation.ResolvedObject);
474471
var successResult = results.FirstOrDefault(r => r.Property == validation.PropertyName);
475472
if (successResult != null)
476473
{
@@ -700,6 +697,17 @@ private static bool TryGetComponentAndObjectReferenceProperty(JObject @params, J
700697
return true;
701698
}
702699

700+
/// <summary>
701+
/// Applies a resolved object reference to a SerializedProperty with Undo support.
702+
/// Shared by SetReference and BatchWire to avoid duplication.
703+
/// </summary>
704+
private static void ApplyReferenceToProperty(Component component, SerializedObject serializedObject, SerializedProperty property, UnityEngine.Object resolvedObject)
705+
{
706+
Undo.RecordObject(component, $"Set reference {property.propertyPath}");
707+
property.objectReferenceValue = resolvedObject;
708+
serializedObject.ApplyModifiedProperties();
709+
}
710+
703711
private static Type GetFieldType(Component component, string propertyName)
704712
{
705713
var so = new SerializedObject(component);
@@ -708,23 +716,48 @@ private static Type GetFieldType(Component component, string propertyName)
708716
return null;
709717

710718
BindingFlags flags = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.IgnoreCase;
711-
string normalizedName = ParamCoercion.NormalizePropertyName(propertyName);
712-
var field = component.GetType().GetField(propertyName, flags)
719+
720+
// Handle array element paths like "targets.Array.data[0]"
721+
string fieldName = propertyName;
722+
bool isArrayElement = propertyName.Contains(".Array.data[");
723+
if (isArrayElement)
724+
{
725+
// Extract the root field name before .Array.data[
726+
fieldName = propertyName.Substring(0, propertyName.IndexOf(".Array.data["));
727+
}
728+
729+
string normalizedName = ParamCoercion.NormalizePropertyName(fieldName);
730+
var field = component.GetType().GetField(fieldName, flags)
713731
?? component.GetType().GetField(normalizedName, flags);
714-
return field?.FieldType;
732+
733+
if (field == null)
734+
return null;
735+
736+
Type fieldType = field.FieldType;
737+
738+
// For array/list elements, extract the element type
739+
if (isArrayElement)
740+
{
741+
if (fieldType.IsArray)
742+
return fieldType.GetElementType();
743+
if (fieldType.IsGenericType && fieldType.GetGenericTypeDefinition() == typeof(List<>))
744+
return fieldType.GetGenericArguments()[0];
745+
}
746+
747+
return fieldType;
715748
}
716749

717750
private static UnityEngine.Object ResolveReference(JObject refParams)
718751
{
719-
var instanceId = refParams["reference_instance_id"]?.Value<int>();
720-
if (instanceId.HasValue)
721-
return GameObjectLookup.ResolveInstanceID(instanceId.Value);
752+
int instanceId = ParamCoercion.CoerceInt(refParams["reference_instance_id"], 0);
753+
if (instanceId != 0)
754+
return GameObjectLookup.ResolveInstanceID(instanceId);
722755

723-
var assetPath = refParams["reference_asset_path"]?.Value<string>();
756+
string assetPath = ParamCoercion.CoerceString(refParams["reference_asset_path"], null);
724757
if (!string.IsNullOrEmpty(assetPath))
725758
return AssetDatabase.LoadAssetAtPath<UnityEngine.Object>(assetPath);
726759

727-
var refPath = refParams["reference_path"]?.Value<string>();
760+
string refPath = ParamCoercion.CoerceString(refParams["reference_path"], null);
728761
if (!string.IsNullOrEmpty(refPath))
729762
return GameObjectLookup.FindByTarget(new JValue(refPath), "by_path", true) ?? GameObject.Find(refPath);
730763

0 commit comments

Comments
 (0)