Skip to content

Commit 783d63c

Browse files
dsarnoclaude
dsarno
and
claude
authored
Fix read_console crash on Unity 6.5 and batch-mode test stability (#761) (#762)
* Fix read_console reflection crash on Unity 6.5 and batch-mode test crash (#761) - Remove unused LogEntry.instanceID reflection that fatally blocked read_console initialization on Unity 6000.5.0a6 - Skip Collider.GeometryHolder in GameObjectSerializer to prevent native PhysX SEGV in batch-mode test runs - Fix CodexConfigHelperTests to work with both release and prerelease package versions Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Add com.unity.test-framework as package dependency The package has a hard compile dependency on Test Framework (RunTests, TestJobManager, TestRunnerService, etc.) but did not declare it in package.json. Projects without Test Framework installed would get CS0234/CS0246 errors on import. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Address review: dispose SerializedObject, fix null stringValue, remove dead method - Wrap SerializedObject in using block in SetViaSerializedProperty - Use string.Empty instead of null for SerializedProperty.stringValue - Remove unused GetRemappedTypeForFiltering from ReadConsole Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent a0c119a commit 783d63c

5 files changed

Lines changed: 44 additions & 88 deletions

File tree

‎MCPForUnity/Editor/Helpers/ComponentOps.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -412,7 +412,7 @@ private static Type ResolveMemberType(Type componentType, string propertyName, s
412412
private static bool SetViaSerializedProperty(Component component, string propertyName, string normalizedName, JToken value, out string error)
413413
{
414414
error = null;
415-
var so = new SerializedObject(component);
415+
using var so = new SerializedObject(component);
416416

417417
SerializedProperty prop = so.FindProperty(propertyName)
418418
?? so.FindProperty(normalizedName);
@@ -515,7 +515,7 @@ private static bool SetSerializedPropertyRecursive(SerializedProperty prop, JTok
515515
return true;
516516

517517
case SerializedPropertyType.String:
518-
prop.stringValue = value == null || value.Type == JTokenType.Null ? null : value.ToString();
518+
prop.stringValue = value == null || value.Type == JTokenType.Null ? string.Empty : value.ToString();
519519
return true;
520520

521521
case SerializedPropertyType.Enum:

‎MCPForUnity/Editor/Helpers/GameObjectSerializer.cs‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -450,11 +450,18 @@ public static object GetComponentData(Component c, bool includeNonPublicSerializ
450450
propName == "worldToLocalMatrix" ||
451451
propName == "localToWorldMatrix"))
452452
{
453-
// McpLog.Info($"[GetComponentData] Explicitly skipping Transform property: {propName}");
454453
skipProperty = true;
455454
}
456455
// --- End Skip Transform Properties ---
457456

457+
// --- Skip Collider properties that cause native crashes via PhysX ---
458+
if (typeof(Collider).IsAssignableFrom(componentType) &&
459+
propName == "GeometryHolder")
460+
{
461+
skipProperty = true;
462+
}
463+
// --- End Skip Collider Properties ---
464+
458465
// Skip if flagged
459466
if (skipProperty)
460467
{

‎MCPForUnity/Editor/Tools/ReadConsole.cs‎

Lines changed: 1 addition & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,6 @@ public static class ReadConsole
3030
private static FieldInfo _messageField;
3131
private static FieldInfo _fileField;
3232
private static FieldInfo _lineField;
33-
private static FieldInfo _instanceIdField;
34-
3533
// Note: Timestamp is not directly available in LogEntry; need to parse message or find alternative?
3634

3735
// Static constructor for reflection setup
@@ -102,10 +100,6 @@ static ReadConsole()
102100
if (_lineField == null)
103101
throw new Exception("Failed to reflect LogEntry.line");
104102

105-
_instanceIdField = logEntryType.GetField("instanceID", instanceFlags);
106-
if (_instanceIdField == null)
107-
throw new Exception("Failed to reflect LogEntry.instanceID");
108-
109103
// (Calibration removed)
110104

111105
}
@@ -121,7 +115,7 @@ static ReadConsole()
121115
_getCountMethod =
122116
_getEntryMethod =
123117
null;
124-
_modeField = _messageField = _fileField = _lineField = _instanceIdField = null;
118+
_modeField = _messageField = _fileField = _lineField = null;
125119
}
126120
}
127121

@@ -140,7 +134,6 @@ public static object HandleCommand(JObject @params)
140134
|| _messageField == null
141135
|| _fileField == null
142136
|| _lineField == null
143-
|| _instanceIdField == null
144137
)
145138
{
146139
// Log the error here as well for easier debugging in Unity Console
@@ -290,7 +283,6 @@ bool includeStacktrace
290283
string file = (string)_fileField.GetValue(logEntryInstance);
291284

292285
int line = (int)_lineField.GetValue(logEntryInstance);
293-
// int instanceId = (int)_instanceIdField.GetValue(logEntryInstance);
294286

295287
if (string.IsNullOrEmpty(message))
296288
{
@@ -515,29 +507,6 @@ private static bool IsExplicitDebugLog(string fullMessage)
515507
return false;
516508
}
517509

518-
/// <summary>
519-
/// Applies the "one level lower" remapping for filtering, like the old version.
520-
/// This ensures compatibility with the filtering logic that expects remapped types.
521-
/// </summary>
522-
private static LogType GetRemappedTypeForFiltering(LogType unityType)
523-
{
524-
switch (unityType)
525-
{
526-
case LogType.Error:
527-
return LogType.Warning; // Error becomes Warning
528-
case LogType.Warning:
529-
return LogType.Log; // Warning becomes Log
530-
case LogType.Assert:
531-
return LogType.Assert; // Assert remains Assert
532-
case LogType.Log:
533-
return LogType.Log; // Log remains Log
534-
case LogType.Exception:
535-
return LogType.Warning; // Exception becomes Warning
536-
default:
537-
return LogType.Log; // Default fallback
538-
}
539-
}
540-
541510
/// <summary>
542511
/// Attempts to extract the stack trace part from a log message.
543512
/// Unity log messages often have the stack trace appended after the main message,

‎MCPForUnity/package.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@
77
"documentationUrl": "https://github.com/CoplayDev/unity-mcp",
88
"licensesUrl": "https://github.com/CoplayDev/unity-mcp/blob/main/LICENSE",
99
"dependencies": {
10-
"com.unity.nuget.newtonsoft-json": "3.0.2"
10+
"com.unity.nuget.newtonsoft-json": "3.0.2",
11+
"com.unity.test-framework": "1.1.31"
1112
},
1213
"keywords": [
1314
"unity",

‎TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/CodexConfigHelperTests.cs‎

Lines changed: 31 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
using System.Collections.Generic;
2+
using System.Linq;
13
using NUnit.Framework;
24
using MCPForUnity.Editor.Helpers;
35
using MCPForUnity.External.Tommy;
@@ -10,6 +12,31 @@ namespace MCPForUnityTests.Editor.Helpers
1012
{
1113
public class CodexConfigHelperTests
1214
{
15+
/// <summary>
16+
/// Validates that a TOML args array contains the expected uvx structure:
17+
/// --from, a mcpforunityserver reference, mcp-for-unity package name,
18+
/// and optionally --prerelease/explicit (only for prerelease builds).
19+
/// </summary>
20+
private static void AssertValidUvxArgs(TomlArray args)
21+
{
22+
var argValues = new List<string>();
23+
foreach (TomlNode child in args.Children)
24+
argValues.Add((child as TomlString).Value);
25+
26+
Assert.IsTrue(argValues.Contains("--from"), "Args should contain --from");
27+
Assert.IsTrue(argValues.Any(a => a.Contains("mcpforunityserver")), "Args should contain PyPI package reference");
28+
Assert.IsTrue(argValues.Contains("mcp-for-unity"), "Args should contain package name");
29+
30+
// Prerelease builds include --prerelease explicit before --from
31+
int fromIndex = argValues.IndexOf("--from");
32+
int prereleaseIndex = argValues.IndexOf("--prerelease");
33+
if (prereleaseIndex >= 0)
34+
{
35+
Assert.IsTrue(prereleaseIndex < fromIndex, "--prerelease should come before --from");
36+
Assert.AreEqual("explicit", argValues[prereleaseIndex + 1], "--prerelease should be followed by explicit");
37+
}
38+
}
39+
1340
/// <summary>
1441
/// Mock platform service for testing
1542
/// </summary>
@@ -244,19 +271,7 @@ public void BuildCodexServerBlock_OnWindows_IncludesSystemRootEnv()
244271

245272
// Verify args contains the proper uvx command structure
246273
var args = argsNode as TomlArray;
247-
Assert.IsTrue(args.ChildrenCount >= 5, "Args should contain --prerelease, explicit, --from, PyPI package reference, and package name");
248-
249-
var firstArg = (args[0] as TomlString).Value;
250-
var secondArg = (args[1] as TomlString).Value;
251-
var thirdArg = (args[2] as TomlString).Value;
252-
var fourthArg = (args[3] as TomlString).Value;
253-
var fifthArg = (args[4] as TomlString).Value;
254-
255-
Assert.AreEqual("--prerelease", firstArg, "First arg should be --prerelease");
256-
Assert.AreEqual("explicit", secondArg, "Second arg should be explicit");
257-
Assert.AreEqual("--from", thirdArg, "Third arg should be --from");
258-
Assert.IsTrue(fourthArg.Contains("mcpforunityserver"), "Fourth arg should be PyPI package reference");
259-
Assert.AreEqual("mcp-for-unity", fifthArg, "Fifth arg should be mcp-for-unity");
274+
AssertValidUvxArgs(args);
260275

261276
// Verify env.SystemRoot is present on Windows
262277
bool hasEnv = unityMcp.TryGetNode("env", out var envNode);
@@ -313,19 +328,7 @@ public void BuildCodexServerBlock_OnNonWindows_ExcludesEnv()
313328

314329
// Verify args contains the proper uvx command structure
315330
var args = argsNode as TomlArray;
316-
Assert.IsTrue(args.ChildrenCount >= 5, "Args should contain --prerelease, explicit, --from, PyPI package reference, and package name");
317-
318-
var firstArg = (args[0] as TomlString).Value;
319-
var secondArg = (args[1] as TomlString).Value;
320-
var thirdArg = (args[2] as TomlString).Value;
321-
var fourthArg = (args[3] as TomlString).Value;
322-
var fifthArg = (args[4] as TomlString).Value;
323-
324-
Assert.AreEqual("--prerelease", firstArg, "First arg should be --prerelease");
325-
Assert.AreEqual("explicit", secondArg, "Second arg should be explicit");
326-
Assert.AreEqual("--from", thirdArg, "Third arg should be --from");
327-
Assert.IsTrue(fourthArg.Contains("mcpforunityserver"), "Fourth arg should be PyPI package reference");
328-
Assert.AreEqual("mcp-for-unity", fifthArg, "Fifth arg should be mcp-for-unity");
331+
AssertValidUvxArgs(args);
329332

330333
// Verify env is NOT present on non-Windows platforms
331334
bool hasEnv = unityMcp.TryGetNode("env", out _);
@@ -384,19 +387,7 @@ public void UpsertCodexServerBlock_OnWindows_IncludesSystemRootEnv()
384387

385388
// Verify args contains the proper uvx command structure
386389
var args = argsNode as TomlArray;
387-
Assert.IsTrue(args.ChildrenCount >= 5, "Args should contain --prerelease, explicit, --from, PyPI package reference, and package name");
388-
389-
var firstArg = (args[0] as TomlString).Value;
390-
var secondArg = (args[1] as TomlString).Value;
391-
var thirdArg = (args[2] as TomlString).Value;
392-
var fourthArg = (args[3] as TomlString).Value;
393-
var fifthArg = (args[4] as TomlString).Value;
394-
395-
Assert.AreEqual("--prerelease", firstArg, "First arg should be --prerelease");
396-
Assert.AreEqual("explicit", secondArg, "Second arg should be explicit");
397-
Assert.AreEqual("--from", thirdArg, "Third arg should be --from");
398-
Assert.IsTrue(fourthArg.Contains("mcpforunityserver"), "Fourth arg should be PyPI package reference");
399-
Assert.AreEqual("mcp-for-unity", fifthArg, "Fifth arg should be mcp-for-unity");
390+
AssertValidUvxArgs(args);
400391

401392
// Verify env.SystemRoot is present on Windows
402393
bool hasEnv = unityMcp.TryGetNode("env", out var envNode);
@@ -462,19 +453,7 @@ public void UpsertCodexServerBlock_OnNonWindows_ExcludesEnv()
462453

463454
// Verify args contains the proper uvx command structure
464455
var args = argsNode as TomlArray;
465-
Assert.IsTrue(args.ChildrenCount >= 5, "Args should contain --prerelease, explicit, --from, PyPI package reference, and package name");
466-
467-
var firstArg = (args[0] as TomlString).Value;
468-
var secondArg = (args[1] as TomlString).Value;
469-
var thirdArg = (args[2] as TomlString).Value;
470-
var fourthArg = (args[3] as TomlString).Value;
471-
var fifthArg = (args[4] as TomlString).Value;
472-
473-
Assert.AreEqual("--prerelease", firstArg, "First arg should be --prerelease");
474-
Assert.AreEqual("explicit", secondArg, "Second arg should be explicit");
475-
Assert.AreEqual("--from", thirdArg, "Third arg should be --from");
476-
Assert.IsTrue(fourthArg.Contains("mcpforunityserver"), "Fourth arg should be PyPI package reference");
477-
Assert.AreEqual("mcp-for-unity", fifthArg, "Fifth arg should be mcp-for-unity");
456+
AssertValidUvxArgs(args);
478457

479458
// Verify env is NOT present on non-Windows platforms
480459
bool hasEnv = unityMcp.TryGetNode("env", out _);

0 commit comments

Comments
 (0)