-
Notifications
You must be signed in to change notification settings - Fork 699
Feature: 🔍 Enhanced Component Data Reading via Serialization #93
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.
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
Feature: 🔍 Enhanced Component Data Reading via Serialization #93
Conversation
…ializer helper Moved serialization logic (GetGameObjectData, GetComponentData, metadata caching, JSON conversion helpers) from ManageGameObject tool to a dedicated GameObjectSerializer class in the Helpers namespace. This improves separation of concerns and reduces the size/complexity of ManageGameObject.cs. Updated ManageGameObject to use the new helper class.
|
You can see the full CodeRabbit of this PR on my fork: dsarno#2 |
The Unity Editor was crashing with ValidTRS() assertions when attempting to get components from certain GameObjects like the Main Camera. Investigation revealed the crash occurred during JSON serialization when reflection code accessed specific matrix properties (e.g., Camera.cullingMatrix, Transform.rotation, Transform.lossyScale). Accessing these properties appears to trigger internal Transform state validation failures, potentially due to interactions with the JSON serializer's reflection mechanism. This fix addresses the issue by: - Replacing LINQ iteration in GetComponentsFromTarget with a standard loop over a copied list to prevent potential premature serialization interactions. - Explicitly skipping known problematic Camera matrix properties (cullingMatrix, pixelRect, rect) and generic matrix properties (worldToLocalMatrix, localToWorldMatrix) within GetComponentData's reflection logic. - Retaining manual serialization for Transform component properties to avoid related reflection issues.
|
One of my Cursor cleanups deleted a bunch of @justinpbarnett 's comments in ManageGameObject.cs. I tried to restore them from a diff but there were too many and it didn't really work. We could either ask for AI help to regenerate inline comments, or I can try to rebuild the whole file based on your commented version (actually already did try that, and it wasn't that easy). Let me know what you think and thanks! |
…sform and Camera components to prevent serialization crashes
- Add get_components to the list of available actions in the docstring - Document required and optional parameters for get_components action - Clarify the return value structure for get_components - Improve overall documentation clarity for component data retrieval
|
@justinpbarnett Think you'll have a few minutes to try it? Spent a solid couple days putting it together. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enriches the ManageGameObject workflow by introducing robust serialization of component data (including non-public [SerializeField] fields) and by extracting serialization logic into a dedicated helper.
- Added an
includeNonPublicSerializedparameter to the Python tool for controlling private field serialization. - Introduced new
JsonConverterimplementations for Unity types (Vector, Color, Bounds, etc.). - Refactored the C#
ManageGameObjecteditor script to delegate serialization to the newGameObjectSerializerhelper. - Created
GameObjectSerializerinUnityMcpBridge.Editor.Helpersto centralize and cache reflection-based serialization.
Reviewed Changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| UnityMcpServer/src/tools/manage_gameobject.py | Added includeNonPublicSerialized argument and updated docs. |
| UnityMcpBridge/Runtime/Serialization/UnityTypeConverters.cs | Added converters for Vector2/3/4, Color, Rect, Bounds, Object. |
| UnityMcpBridge/Editor/Tools/ManageGameObject.cs | Refactored to pass flag and use GameObjectSerializer. |
| UnityMcpBridge/Editor/Helpers/GameObjectSerializer.cs | New helper class encapsulating gameobject/component serialization. |
Comments suppressed due to low confidence (6)
UnityMcpServer/src/tools/manage_gameobject.py:38
- [nitpick] Parameter name
includeNonPublicSerializeduses CamelCase and does not follow PEP8 snake_case conventions; consider renaming toinclude_non_public_serializedfor consistency.
includeNonPublicSerialized: bool = None, # Controls serialization of private [SerializeField] fields
UnityMcpBridge/Editor/Helpers/GameObjectSerializer.cs:124
- [nitpick] Numerous commented-out debug logs remain in
GameObjectSerializer; consider removing or gating them behind a debug flag to improve readability.
// --- Add Early Logging ---
UnityMcpServer/src/tools/manage_gameobject.py:65
- [nitpick] Docstring formatting for the
'get_components'section has inconsistent indentation and list alignment; consider normalizing indentation for better readability.
Action-specific details:
UnityMcpBridge/Editor/Helpers/GameObjectSerializer.cs:1
- [nitpick]
GameObjectSerializeris very large and handles many responsibilities; consider splitting it into smaller focused classes or regions (e.g., converters, caching, reflection) to improve maintainability.
using System;
UnityMcpBridge/Editor/Tools/ManageGameObject.cs:5
- [nitpick] This
usingdirective may no longer be needed after refactoring; consider removing unused imports to reduce clutter.
using Newtonsoft.Json; // Added for JsonSerializationException
UnityMcpBridge/Editor/Tools/ManageGameObject.cs:13
- [nitpick] Import of
UnityMcpBridge.Runtime.Serializationmay be unused in this file; verify and remove if unnecessary.
using UnityMcpBridge.Runtime.Serialization; // <<< Keep for Converters access? Might not be needed here directly
|
Hi, looks good to me at this point. I will first merge it and run it myself (some deprecations at this point). Let me know what you are cooking next! |
This PR enhances the
ManageGameObjecttool to supercharge its ability to read component data accurately and comprehensively! 🚀 Basically, it can read serialized data from a bunch of different kinds of standard and custom components, and can be extended to work with even more components.Summary:
JsonConverterimplementations (Vector3Converter,ColorConverter,UnityEngineObjectConverter, etc.). This means theget_componentsaction can reliably pull detailed info for all sorts of component data types.ManageGameObject.csinto a new, dedicatedGameObjectSerializerhelper class (UnityMcpBridge.Editor.Helpers.GameObjectSerializer).Enhanced Capabilities (
get_componentsaction):The
get_componentsaction now reliably retrieves detailed information for components attached to GameObjects, including:[SerializeField](usingincludeNonPublicSerialized: true) 👀Examples:
Get all component data for 'MCP_TestData':
{ "tool_name": "mcp_UnityMCP_manage_gameobject", "arguments": { "action": "get_components", "target": "MCP_TestData" } }{ "typeName": "McpTestComponent", "instanceID": 124350, // Instance ID will vary "properties": { "publicInt": 10, "publicFloat": 3.14, "publicString": "Hello MCP", "publicVector3": {"x": 1.0, "y": 2.0, "z": 3.0}, "publicColor": {"r": 1.0, "g": 0.0, "b": 0.0, "a": 1.0}, "publicGameObjectRef": {"name": "DoorwayZone", "instanceID": 124510}, // Instance ID will vary "publicMaterialRef": "Assets/Art/Materials/Bun_Tan.mat", "publicVectorList": [{"x": 1.0, "y": 1.0, "z": 1.0}, {"x": 0.0, "y": 0.0, "z": 0.0}], "nestedData": {"IsEnabled": true, "MoreData": {"Name": "Nested", "Value": 456}, "IntList": [7, 8, 9]} // ... other properties ... } }Inspect a UI element ('InventoryPanel'):
{ "tool_name": "mcp_UnityMCP_manage_gameobject", "arguments": { "action": "get_components", "target": "InventoryPanel", "search_method": "by_name" } }{ "typeName": "UnityEngine.UI.Image", "instanceID": -137898, // Instance ID will vary "properties": { "sprite": {"name": "UI_Square_Frame01", "instanceID": 124818}, // Instance ID will vary "color": {"r": 0.19, "g": 0.19, "b": 0.19, "a": 1.0}, "material": "Assets/Settings/URP/Resources/Materials/UI_Default.mat", "raycastTarget": true, "type": 1, // Sliced "pixelsPerUnitMultiplier": 1.0 // ... other properties ... } }Future Considerations:
GameObjectSerializerand theUnityMcpBridge.Runtime.Serializationconverters provide a solid foundation. We can easily add support for more complex or custom data types in the future by creating newJsonConverterimplementations, making this tool even more powerful! 💪(Note: This PR also implicitly includes commits that added missing essential package files (
.asmdef,.meta, runtime scripts) to the repository, which were necessary to resolve compilation errors when loading the package remotely via Git.)Summary by CodeRabbit
New Features
Enhancements