diff --git a/MCPForUnity/Editor/Helpers/GameObjectSerializer.cs b/MCPForUnity/Editor/Helpers/GameObjectSerializer.cs index 2e2c61d..538aa76 100644 --- a/MCPForUnity/Editor/Helpers/GameObjectSerializer.cs +++ b/MCPForUnity/Editor/Helpers/GameObjectSerializer.cs @@ -524,6 +524,7 @@ namespace MCPForUnity.Editor.Helpers new ColorConverter(), new RectConverter(), new BoundsConverter(), + new Matrix4x4Converter(), // Fix #478: Safe Matrix4x4 serialization for Cinemachine new UnityEngineObjectConverter() // Handles serialization of references }, ReferenceLoopHandling = ReferenceLoopHandling.Ignore, diff --git a/MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.uss b/MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.uss index 703ddc4..d909df7 100644 --- a/MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.uss +++ b/MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.uss @@ -4,7 +4,6 @@ flex-direction: column; flex-grow: 1; overflow: hidden; - box-sizing: border-box; } /* Title */ diff --git a/MCPForUnity/Runtime/Serialization/UnityTypeConverters.cs b/MCPForUnity/Runtime/Serialization/UnityTypeConverters.cs index c76b280..47c2652 100644 --- a/MCPForUnity/Runtime/Serialization/UnityTypeConverters.cs +++ b/MCPForUnity/Runtime/Serialization/UnityTypeConverters.cs @@ -160,6 +160,67 @@ namespace MCPForUnity.Runtime.Serialization } } + /// + /// Safe converter for Matrix4x4 that only accesses raw matrix elements (m00-m33). + /// Avoids computed properties (lossyScale, rotation, inverse) that call ValidTRS() + /// and can crash Unity on non-TRS matrices (common in Cinemachine components). + /// Fixes: https://github.com/CoplayDev/unity-mcp/issues/478 + /// + public class Matrix4x4Converter : JsonConverter + { + public override void WriteJson(JsonWriter writer, Matrix4x4 value, JsonSerializer serializer) + { + writer.WriteStartObject(); + // Only access raw matrix elements - NEVER computed properties like lossyScale/rotation + writer.WritePropertyName("m00"); writer.WriteValue(value.m00); + writer.WritePropertyName("m01"); writer.WriteValue(value.m01); + writer.WritePropertyName("m02"); writer.WriteValue(value.m02); + writer.WritePropertyName("m03"); writer.WriteValue(value.m03); + writer.WritePropertyName("m10"); writer.WriteValue(value.m10); + writer.WritePropertyName("m11"); writer.WriteValue(value.m11); + writer.WritePropertyName("m12"); writer.WriteValue(value.m12); + writer.WritePropertyName("m13"); writer.WriteValue(value.m13); + writer.WritePropertyName("m20"); writer.WriteValue(value.m20); + writer.WritePropertyName("m21"); writer.WriteValue(value.m21); + writer.WritePropertyName("m22"); writer.WriteValue(value.m22); + writer.WritePropertyName("m23"); writer.WriteValue(value.m23); + writer.WritePropertyName("m30"); writer.WriteValue(value.m30); + writer.WritePropertyName("m31"); writer.WriteValue(value.m31); + writer.WritePropertyName("m32"); writer.WriteValue(value.m32); + writer.WritePropertyName("m33"); writer.WriteValue(value.m33); + writer.WriteEndObject(); + } + + public override Matrix4x4 ReadJson(JsonReader reader, Type objectType, Matrix4x4 existingValue, bool hasExistingValue, JsonSerializer serializer) + { + if (reader.TokenType == JsonToken.Null) + return new Matrix4x4(); // Return zero matrix for null (consistent with missing field defaults) + + if (reader.TokenType != JsonToken.StartObject) + throw new JsonSerializationException($"Expected JSON object or null when deserializing Matrix4x4, got '{reader.TokenType}'."); + + JObject jo = JObject.Load(reader); + var matrix = new Matrix4x4(); + matrix.m00 = jo["m00"]?.Value() ?? 0f; + matrix.m01 = jo["m01"]?.Value() ?? 0f; + matrix.m02 = jo["m02"]?.Value() ?? 0f; + matrix.m03 = jo["m03"]?.Value() ?? 0f; + matrix.m10 = jo["m10"]?.Value() ?? 0f; + matrix.m11 = jo["m11"]?.Value() ?? 0f; + matrix.m12 = jo["m12"]?.Value() ?? 0f; + matrix.m13 = jo["m13"]?.Value() ?? 0f; + matrix.m20 = jo["m20"]?.Value() ?? 0f; + matrix.m21 = jo["m21"]?.Value() ?? 0f; + matrix.m22 = jo["m22"]?.Value() ?? 0f; + matrix.m23 = jo["m23"]?.Value() ?? 0f; + matrix.m30 = jo["m30"]?.Value() ?? 0f; + matrix.m31 = jo["m31"]?.Value() ?? 0f; + matrix.m32 = jo["m32"]?.Value() ?? 0f; + matrix.m33 = jo["m33"]?.Value() ?? 0f; + return matrix; + } + } + // Converter for UnityEngine.Object references (GameObjects, Components, Materials, Textures, etc.) public class UnityEngineObjectConverter : JsonConverter { diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/Matrix4x4ConverterTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/Matrix4x4ConverterTests.cs new file mode 100644 index 0000000..221996f --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/Matrix4x4ConverterTests.cs @@ -0,0 +1,134 @@ +using System.Linq; +using MCPForUnity.Runtime.Serialization; +using Newtonsoft.Json; +using Newtonsoft.Json.Linq; +using NUnit.Framework; +using UnityEngine; + +namespace MCPForUnityTests.EditMode.Helpers +{ + /// + /// Tests for Matrix4x4Converter to ensure it safely serializes matrices + /// without accessing dangerous computed properties (lossyScale, rotation). + /// Regression test for https://github.com/CoplayDev/unity-mcp/issues/478 + /// + public class Matrix4x4ConverterTests + { + private JsonSerializerSettings _settings; + + [SetUp] + public void SetUp() + { + _settings = new JsonSerializerSettings + { + Converters = { new Matrix4x4Converter() } + }; + } + + [Test] + public void Serialize_IdentityMatrix_ReturnsCorrectJson() + { + var matrix = Matrix4x4.identity; + var json = JsonConvert.SerializeObject(matrix, _settings); + + Assert.That(json, Does.Contain("\"m00\":1")); + Assert.That(json, Does.Contain("\"m11\":1")); + Assert.That(json, Does.Contain("\"m22\":1")); + Assert.That(json, Does.Contain("\"m33\":1")); + Assert.That(json, Does.Contain("\"m01\":0")); + } + + [Test] + public void Deserialize_IdentityMatrix_ReturnsIdentity() + { + var original = Matrix4x4.identity; + var json = JsonConvert.SerializeObject(original, _settings); + var result = JsonConvert.DeserializeObject(json, _settings); + + Assert.That(result, Is.EqualTo(original)); + } + + [Test] + public void Serialize_TranslationMatrix_PreservesValues() + { + var matrix = Matrix4x4.Translate(new Vector3(10, 20, 30)); + var json = JsonConvert.SerializeObject(matrix, _settings); + var result = JsonConvert.DeserializeObject(json, _settings); + + Assert.That(result.m03, Is.EqualTo(10f)); + Assert.That(result.m13, Is.EqualTo(20f)); + Assert.That(result.m23, Is.EqualTo(30f)); + } + + [Test] + public void Serialize_DegenerateMatrix_DoesNotCrashAndRoundtrips() + { + // This is the key test - a degenerate matrix that would crash + // if we accessed lossyScale or rotation properties + var matrix = new Matrix4x4(); + matrix.m00 = 0; matrix.m11 = 0; matrix.m22 = 0; // Degenerate - determinant = 0 + + // This should NOT throw or crash - the old code would fail here + var json = JsonConvert.SerializeObject(matrix, _settings); + var result = JsonConvert.DeserializeObject(json, _settings); + + // Verify JSON only contains raw mXY properties + var jo = JObject.Parse(json); + var expectedProps = new[] + { + "m00", "m01", "m02", "m03", + "m10", "m11", "m12", "m13", + "m20", "m21", "m22", "m23", + "m30", "m31", "m32", "m33" + }; + CollectionAssert.AreEquivalent(expectedProps, jo.Properties().Select(p => p.Name).ToArray()); + + // Verify values roundtrip correctly (all zeros for degenerate matrix) + Assert.That(result.m00, Is.EqualTo(0f)); + Assert.That(result.m11, Is.EqualTo(0f)); + Assert.That(result.m22, Is.EqualTo(0f)); + Assert.That(result, Is.EqualTo(matrix)); + } + + [Test] + public void Serialize_NonTRSMatrix_DoesNotCrash() + { + // Projection matrices are NOT valid TRS matrices + // Accessing lossyScale/rotation on them causes ValidTRS() assertion + var matrix = Matrix4x4.Perspective(60f, 1.77f, 0.1f, 1000f); + + // Verify it's not a valid TRS matrix + Assert.That(matrix.ValidTRS(), Is.False, "Test requires non-TRS matrix"); + + // This should NOT throw - the fix ensures we never access computed properties + Assert.DoesNotThrow(() => + { + var json = JsonConvert.SerializeObject(matrix, _settings); + var result = JsonConvert.DeserializeObject(json, _settings); + }); + } + + [Test] + public void Deserialize_NullToken_ReturnsZeroMatrix() + { + var json = "null"; + var result = JsonConvert.DeserializeObject(json, _settings); + + // Returns zero matrix (consistent with missing field defaults of 0f) + Assert.That(result, Is.EqualTo(new Matrix4x4())); + } + + [Test] + public void Serialize_DoesNotContainDangerousProperties() + { + var matrix = Matrix4x4.TRS(Vector3.one, Quaternion.identity, Vector3.one); + var json = JsonConvert.SerializeObject(matrix, _settings); + + // Ensure we're not serializing the dangerous computed properties + Assert.That(json, Does.Not.Contain("lossyScale")); + Assert.That(json, Does.Not.Contain("rotation")); + Assert.That(json, Does.Not.Contain("inverse")); + Assert.That(json, Does.Not.Contain("transpose")); + } + } +} diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/Matrix4x4ConverterTests.cs.meta b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/Matrix4x4ConverterTests.cs.meta new file mode 100644 index 0000000..3509337 --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/Matrix4x4ConverterTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: d2c5dd202b4944675ae606581676a24a +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/MCPForUnityTests.Editor.asmdef b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/MCPForUnityTests.Editor.asmdef index 1207b26..a98ec13 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/MCPForUnityTests.Editor.asmdef +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/MCPForUnityTests.Editor.asmdef @@ -3,6 +3,7 @@ "rootNamespace": "", "references": [ "MCPForUnity.Editor", + "MCPForUnity.Runtime", "TestAsmdef", "UnityEngine.TestRunner", "UnityEditor.TestRunner" @@ -23,4 +24,4 @@ ], "versionDefines": [], "noEngineReferences": false -} \ No newline at end of file +}