Skip to content

Fix read_console crash on Unity 6.5 and batch-mode test stability (#761) #762

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dsarno merged 3 commits into from
Feb 16, 2026
+44 −88
Merged

Fix read_console crash on Unity 6.5 and batch-mode test stability (#761) #762

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
04553da
Fix read_console reflection crash on Unity 6.5 and batch-mode test cr…
dsarno Feb 16, 2026
46dd236
Add com.unity.test-framework as package dependency
dsarno Feb 16, 2026
3f95d6c
Address review: dispose SerializedObject, fix null stringValue, remov…
dsarno Feb 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions MCPForUnity/Editor/Helpers/ComponentOps.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ private static Type ResolveMemberType(Type componentType, string propertyName, s
private static bool SetViaSerializedProperty(Component component, string propertyName, string normalizedName, JToken value, out string error)
{
error = null;
var so = new SerializedObject(component);
using var so = new SerializedObject(component);

SerializedProperty prop = so.FindProperty(propertyName)
?? so.FindProperty(normalizedName);
Expand Down Expand Up @@ -515,7 +515,7 @@ private static bool SetSerializedPropertyRecursive(SerializedProperty prop, JTok
return true;

case SerializedPropertyType.String:
prop.stringValue = value == null || value.Type == JTokenType.Null ? null : value.ToString();
prop.stringValue = value == null || value.Type == JTokenType.Null ? string.Empty : value.ToString();
return true;

case SerializedPropertyType.Enum:
Expand Down
9 changes: 8 additions & 1 deletion MCPForUnity/Editor/Helpers/GameObjectSerializer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,11 +450,18 @@ public static object GetComponentData(Component c, bool includeNonPublicSerializ
propName == "worldToLocalMatrix" ||
propName == "localToWorldMatrix"))
{
// McpLog.Info($"[GetComponentData] Explicitly skipping Transform property: {propName}");
skipProperty = true;
}
// --- End Skip Transform Properties ---

// --- Skip Collider properties that cause native crashes via PhysX ---
if (typeof(Collider).IsAssignableFrom(componentType) &&
propName == "GeometryHolder")
{
skipProperty = true;
}
// --- End Skip Collider Properties ---

// Skip if flagged
if (skipProperty)
{
Expand Down
33 changes: 1 addition & 32 deletions MCPForUnity/Editor/Tools/ReadConsole.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ public static class ReadConsole
private static FieldInfo _messageField;
private static FieldInfo _fileField;
private static FieldInfo _lineField;
private static FieldInfo _instanceIdField;

// Note: Timestamp is not directly available in LogEntry; need to parse message or find alternative?

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

_instanceIdField = logEntryType.GetField("instanceID", instanceFlags);
if (_instanceIdField == null)
throw new Exception("Failed to reflect LogEntry.instanceID");

// (Calibration removed)

}
Expand All @@ -121,7 +115,7 @@ static ReadConsole()
_getCountMethod =
_getEntryMethod =
null;
_modeField = _messageField = _fileField = _lineField = _instanceIdField = null;
_modeField = _messageField = _fileField = _lineField = null;
}
}

Expand All @@ -140,7 +134,6 @@ public static object HandleCommand(JObject @params)
|| _messageField == null
|| _fileField == null
|| _lineField == null
|| _instanceIdField == null
)
{
// Log the error here as well for easier debugging in Unity Console
Expand Down Expand Up @@ -290,7 +283,6 @@ bool includeStacktrace
string file = (string)_fileField.GetValue(logEntryInstance);

int line = (int)_lineField.GetValue(logEntryInstance);
// int instanceId = (int)_instanceIdField.GetValue(logEntryInstance);

if (string.IsNullOrEmpty(message))
{
Expand Down Expand Up @@ -515,29 +507,6 @@ private static bool IsExplicitDebugLog(string fullMessage)
return false;
}

/// <summary>
/// Applies the "one level lower" remapping for filtering, like the old version.
/// This ensures compatibility with the filtering logic that expects remapped types.
/// </summary>
private static LogType GetRemappedTypeForFiltering(LogType unityType)
{
switch (unityType)
{
case LogType.Error:
return LogType.Warning; // Error becomes Warning
case LogType.Warning:
return LogType.Log; // Warning becomes Log
case LogType.Assert:
return LogType.Assert; // Assert remains Assert
case LogType.Log:
return LogType.Log; // Log remains Log
case LogType.Exception:
return LogType.Warning; // Exception becomes Warning
default:
return LogType.Log; // Default fallback
}
}

/// <summary>
/// Attempts to extract the stack trace part from a log message.
/// Unity log messages often have the stack trace appended after the main message,
Expand Down
3 changes: 2 additions & 1 deletion MCPForUnity/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
"documentationUrl": "https://github.com/CoplayDev/unity-mcp",
"licensesUrl": "https://github.com/CoplayDev/unity-mcp/blob/main/LICENSE",
"dependencies": {
"com.unity.nuget.newtonsoft-json": "3.0.2"
"com.unity.nuget.newtonsoft-json": "3.0.2",
"com.unity.test-framework": "1.1.31"
},
"keywords": [
"unity",
Expand Down
83 changes: 31 additions & 52 deletions TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/CodexConfigHelperTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
using System.Collections.Generic;
using System.Linq;
using NUnit.Framework;
using MCPForUnity.Editor.Helpers;
using MCPForUnity.External.Tommy;
Expand All @@ -10,6 +12,31 @@ namespace MCPForUnityTests.Editor.Helpers
{
public class CodexConfigHelperTests
{
/// <summary>
/// Validates that a TOML args array contains the expected uvx structure:
/// --from, a mcpforunityserver reference, mcp-for-unity package name,
/// and optionally --prerelease/explicit (only for prerelease builds).
/// </summary>
private static void AssertValidUvxArgs(TomlArray args)
{
var argValues = new List<string>();
foreach (TomlNode child in args.Children)
argValues.Add((child as TomlString).Value);

Assert.IsTrue(argValues.Contains("--from"), "Args should contain --from");
Assert.IsTrue(argValues.Any(a => a.Contains("mcpforunityserver")), "Args should contain PyPI package reference");
Assert.IsTrue(argValues.Contains("mcp-for-unity"), "Args should contain package name");

// Prerelease builds include --prerelease explicit before --from
int fromIndex = argValues.IndexOf("--from");
int prereleaseIndex = argValues.IndexOf("--prerelease");
if (prereleaseIndex >= 0)
{
Assert.IsTrue(prereleaseIndex < fromIndex, "--prerelease should come before --from");
Assert.AreEqual("explicit", argValues[prereleaseIndex + 1], "--prerelease should be followed by explicit");
}
}

/// <summary>
/// Mock platform service for testing
/// </summary>
Expand Down Expand Up @@ -244,19 +271,7 @@ public void BuildCodexServerBlock_OnWindows_IncludesSystemRootEnv()

// Verify args contains the proper uvx command structure
var args = argsNode as TomlArray;
Assert.IsTrue(args.ChildrenCount >= 5, "Args should contain --prerelease, explicit, --from, PyPI package reference, and package name");

var firstArg = (args[0] as TomlString).Value;
var secondArg = (args[1] as TomlString).Value;
var thirdArg = (args[2] as TomlString).Value;
var fourthArg = (args[3] as TomlString).Value;
var fifthArg = (args[4] as TomlString).Value;

Assert.AreEqual("--prerelease", firstArg, "First arg should be --prerelease");
Assert.AreEqual("explicit", secondArg, "Second arg should be explicit");
Assert.AreEqual("--from", thirdArg, "Third arg should be --from");
Assert.IsTrue(fourthArg.Contains("mcpforunityserver"), "Fourth arg should be PyPI package reference");
Assert.AreEqual("mcp-for-unity", fifthArg, "Fifth arg should be mcp-for-unity");
AssertValidUvxArgs(args);

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

// Verify args contains the proper uvx command structure
var args = argsNode as TomlArray;
Assert.IsTrue(args.ChildrenCount >= 5, "Args should contain --prerelease, explicit, --from, PyPI package reference, and package name");

var firstArg = (args[0] as TomlString).Value;
var secondArg = (args[1] as TomlString).Value;
var thirdArg = (args[2] as TomlString).Value;
var fourthArg = (args[3] as TomlString).Value;
var fifthArg = (args[4] as TomlString).Value;

Assert.AreEqual("--prerelease", firstArg, "First arg should be --prerelease");
Assert.AreEqual("explicit", secondArg, "Second arg should be explicit");
Assert.AreEqual("--from", thirdArg, "Third arg should be --from");
Assert.IsTrue(fourthArg.Contains("mcpforunityserver"), "Fourth arg should be PyPI package reference");
Assert.AreEqual("mcp-for-unity", fifthArg, "Fifth arg should be mcp-for-unity");
AssertValidUvxArgs(args);

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

// Verify args contains the proper uvx command structure
var args = argsNode as TomlArray;
Assert.IsTrue(args.ChildrenCount >= 5, "Args should contain --prerelease, explicit, --from, PyPI package reference, and package name");

var firstArg = (args[0] as TomlString).Value;
var secondArg = (args[1] as TomlString).Value;
var thirdArg = (args[2] as TomlString).Value;
var fourthArg = (args[3] as TomlString).Value;
var fifthArg = (args[4] as TomlString).Value;

Assert.AreEqual("--prerelease", firstArg, "First arg should be --prerelease");
Assert.AreEqual("explicit", secondArg, "Second arg should be explicit");
Assert.AreEqual("--from", thirdArg, "Third arg should be --from");
Assert.IsTrue(fourthArg.Contains("mcpforunityserver"), "Fourth arg should be PyPI package reference");
Assert.AreEqual("mcp-for-unity", fifthArg, "Fifth arg should be mcp-for-unity");
AssertValidUvxArgs(args);

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

// Verify args contains the proper uvx command structure
var args = argsNode as TomlArray;
Assert.IsTrue(args.ChildrenCount >= 5, "Args should contain --prerelease, explicit, --from, PyPI package reference, and package name");

var firstArg = (args[0] as TomlString).Value;
var secondArg = (args[1] as TomlString).Value;
var thirdArg = (args[2] as TomlString).Value;
var fourthArg = (args[3] as TomlString).Value;
var fifthArg = (args[4] as TomlString).Value;

Assert.AreEqual("--prerelease", firstArg, "First arg should be --prerelease");
Assert.AreEqual("explicit", secondArg, "Second arg should be explicit");
Assert.AreEqual("--from", thirdArg, "Third arg should be --from");
Assert.IsTrue(fourthArg.Contains("mcpforunityserver"), "Fourth arg should be PyPI package reference");
Assert.AreEqual("mcp-for-unity", fifthArg, "Fifth arg should be mcp-for-unity");
AssertValidUvxArgs(args);

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