Skip to content

Commit c866993

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 1f3a6c5 commit c866993

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
@@ -352,9 +352,7 @@ private static object SetReference(JObject @params, JToken targetToken, string s
352352

353353
var previousValue = DescribeObjectReference(property.objectReferenceValue);
354354

355-
Undo.RecordObject(component, $"Set reference {property.propertyPath}");
356-
property.objectReferenceValue = validation.ResolvedObject;
357-
serializedObject.ApplyModifiedProperties();
355+
ApplyReferenceToProperty(component, serializedObject, property, validation.ResolvedObject);
358356
EditorUtility.SetDirty(component);
359357
MarkOwningSceneDirty(targetGo);
360358

@@ -474,6 +472,7 @@ private static object BatchWire(JObject @params, JToken targetToken, string sear
474472
Undo.SetCurrentGroupName($"Batch wire references on {component.GetType().Name}");
475473
int succeeded = 0;
476474
int failed = 0;
475+
var serializedObject = new SerializedObject(component);
477476

478477
try
479478
{
@@ -485,7 +484,7 @@ private static object BatchWire(JObject @params, JToken targetToken, string sear
485484
continue;
486485
}
487486

488-
var serializedObject = new SerializedObject(component);
487+
serializedObject.Update();
489488
var property = serializedObject.FindProperty(validation.PropertyName);
490489
if (property == null || property.propertyType != SerializedPropertyType.ObjectReference)
491490
{
@@ -500,9 +499,7 @@ private static object BatchWire(JObject @params, JToken targetToken, string sear
500499
continue;
501500
}
502501

503-
Undo.RecordObject(component, $"Set reference {validation.PropertyName}");
504-
property.objectReferenceValue = validation.ResolvedObject;
505-
serializedObject.ApplyModifiedProperties();
502+
ApplyReferenceToProperty(component, serializedObject, property, validation.ResolvedObject);
506503
var successResult = results.FirstOrDefault(r => r.Property == validation.PropertyName);
507504
if (successResult != null)
508505
{
@@ -732,6 +729,17 @@ private static bool TryGetComponentAndObjectReferenceProperty(JObject @params, J
732729
return true;
733730
}
734731

732+
/// <summary>
733+
/// Applies a resolved object reference to a SerializedProperty with Undo support.
734+
/// Shared by SetReference and BatchWire to avoid duplication.
735+
/// </summary>
736+
private static void ApplyReferenceToProperty(Component component, SerializedObject serializedObject, SerializedProperty property, UnityEngine.Object resolvedObject)
737+
{
738+
Undo.RecordObject(component, $"Set reference {property.propertyPath}");
739+
property.objectReferenceValue = resolvedObject;
740+
serializedObject.ApplyModifiedProperties();
741+
}
742+
735743
private static Type GetFieldType(Component component, string propertyName)
736744
{
737745
var so = new SerializedObject(component);
@@ -740,23 +748,48 @@ private static Type GetFieldType(Component component, string propertyName)
740748
return null;
741749

742750
BindingFlags flags = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.IgnoreCase;
743-
string normalizedName = ParamCoercion.NormalizePropertyName(propertyName);
744-
var field = component.GetType().GetField(propertyName, flags)
751+
752+
// Handle array element paths like "targets.Array.data[0]"
753+
string fieldName = propertyName;
754+
bool isArrayElement = propertyName.Contains(".Array.data[");
755+
if (isArrayElement)
756+
{
757+
// Extract the root field name before .Array.data[
758+
fieldName = propertyName.Substring(0, propertyName.IndexOf(".Array.data["));
759+
}
760+
761+
string normalizedName = ParamCoercion.NormalizePropertyName(fieldName);
762+
var field = component.GetType().GetField(fieldName, flags)
745763
?? component.GetType().GetField(normalizedName, flags);
746-
return field?.FieldType;
764+
765+
if (field == null)
766+
return null;
767+
768+
Type fieldType = field.FieldType;
769+
770+
// For array/list elements, extract the element type
771+
if (isArrayElement)
772+
{
773+
if (fieldType.IsArray)
774+
return fieldType.GetElementType();
775+
if (fieldType.IsGenericType && fieldType.GetGenericTypeDefinition() == typeof(List<>))
776+
return fieldType.GetGenericArguments()[0];
777+
}
778+
779+
return fieldType;
747780
}
748781

749782
private static UnityEngine.Object ResolveReference(JObject refParams)
750783
{
751-
var instanceId = refParams["reference_instance_id"]?.Value<int>();
752-
if (instanceId.HasValue)
753-
return GameObjectLookup.ResolveInstanceID(instanceId.Value);
784+
int instanceId = ParamCoercion.CoerceInt(refParams["reference_instance_id"], 0);
785+
if (instanceId != 0)
786+
return GameObjectLookup.ResolveInstanceID(instanceId);
754787

755-
var assetPath = refParams["reference_asset_path"]?.Value<string>();
788+
string assetPath = ParamCoercion.CoerceString(refParams["reference_asset_path"], null);
756789
if (!string.IsNullOrEmpty(assetPath))
757790
return AssetDatabase.LoadAssetAtPath<UnityEngine.Object>(assetPath);
758791

759-
var refPath = refParams["reference_path"]?.Value<string>();
792+
string refPath = ParamCoercion.CoerceString(refParams["reference_path"], null);
760793
if (!string.IsNullOrEmpty(refPath))
761794
return GameObjectLookup.FindByTarget(new JValue(refPath), "by_path", true) ?? GameObject.Find(refPath);
762795

0 commit comments

Comments
 (0)