Feature: 🔍 Enhanced Component Data Reading via Serialization #2
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.
WalkthroughThe pull request introduces a new static serializer class for Unity GameObjects and Components, along with a set of JSON converters for Unity types. It refactors GameObject management to include a new parameter for handling non-public serialized fields, enhances error handling in command processing, and adds assembly definition and metadata files for both the Editor and Runtime modules. Additionally, the server-side management script is updated to pass the new parameter. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/Server
participant M as ManageGameObject
participant S as GameObjectSerializer
participant R as ResponseHandler
User->>M: Send command with parameters<br>(includeNonPublicSerialized flag)
M->>S: Call GetGameObjectData / GetComponentData
S-->>M: Return serialized GameObject/component data
M->>R: Process command and build response
R-->>User: Return JSON response
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (10)
UnityMcpBridge/Runtime/Serialization.meta (1)
1-9: Remove Unintended Trailing ContentThe metadata configuration is mostly correct with
fileFormatVersion: 2, a valid GUID, and proper folder asset settings. However, line 9 contains an extraneous entry that does not match any valid key/value format. This stray line could potentially cause issues when Unity processes the asset metadata. Please remove line 9 if it is not intentional.UnityMcpBridge/Editor/UnityMcpBridge.cs (1)
270-270: Unnecessary whitespace line added.There appears to be an extra blank line added here that doesn't serve any functional purpose. While this won't affect functionality, it's generally good practice to avoid unnecessary whitespace changes in PRs to keep the diff clean.
-UnityMcpServer/src/tools/manage_gameobject.py (2)
38-38: Use consistent Python naming conventions.While this matches the Unity-side parameter name, consider using snake_case (e.g.,
include_non_public_serialized) for local Python arguments to preserve internal consistency. Alternatively, keep it as-is if seamless alignment with the Unity parameter is desired.
63-63: Clarify behavior when set to None vs. False.The docstring indicates behavior when
True, but doesn't mention what happens if this parameter isNoneor explicitlyFalse. Consider clarifying thatNoneresults in omitting the parameter entirely, andFalseactively excludes non-public serialized fields.UnityMcpBridge/Editor/Helpers/GameObjectSerializer.cs (1)
24-98: Offer a way to optionally include full component data in GetGameObjectData.
GetGameObjectDatacurrently returns high-level details about theGameObject, with a comment suggesting expanded component serialization. Consider adding an optional parameter that toggles retrieval of component data or references toGetComponentDatafor a more comprehensive or minimal approach.UnityMcpBridge/Editor/Tools/ManageGameObject.cs (5)
12-13: Remove unused import if not needed
using UnityMcpBridge.Runtime.Serialization;might be unnecessary if no direct references exist in this file. Consider removing it to keep imports clean.
26-29: Consider removing commented-out debug code
These debug comments seem helpful for tracing, but if they're no longer in use, removing them can reduce clutter.
1423-1441: New input serializer setup
Centralizing converters inJsonSerializerSettingsis a strong design choice. However, consider creating and reusing a static instance to avoid instantiating new serializer objects for each property set—this can improve performance.
1456-1487: Improved reflection-based property setting
Using the input serializer to convert JSON tokens into strongly typed objects is robust. One potential enhancement: carefully handle partial successes or type mismatches. If many properties fail to set, consider returning partial results or an aggregated response instead of multiple logs.
1695-1771: Robust custom conversion
Catching exceptions withToObject(...)is good. For large or repeated conversions, consider caching or using a shared serializer to reduce overhead. Also, ensure that each re-thrown exception is handled upstream to avoid unexpected crashes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
UnityMcpServer/src/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
UnityMcpBridge/Editor/Helpers/GameObjectSerializer.cs(1 hunks)UnityMcpBridge/Editor/Helpers/GameObjectSerializer.cs.meta(1 hunks)UnityMcpBridge/Editor/Tools/ManageGameObject.cs(19 hunks)UnityMcpBridge/Editor/UnityMcpBridge.Editor.asmdef(1 hunks)UnityMcpBridge/Editor/UnityMcpBridge.Editor.asmdef.meta(1 hunks)UnityMcpBridge/Editor/UnityMcpBridge.cs(1 hunks)UnityMcpBridge/Runtime.meta(1 hunks)UnityMcpBridge/Runtime/Serialization.meta(1 hunks)UnityMcpBridge/Runtime/Serialization/UnityTypeConverters.cs(1 hunks)UnityMcpBridge/Runtime/Serialization/UnityTypeConverters.cs.meta(1 hunks)UnityMcpBridge/Runtime/UnityMcpBridge.Runtime.asmdef(1 hunks)UnityMcpBridge/Runtime/UnityMcpBridge.Runtime.asmdef.meta(1 hunks)UnityMcpServer/src/tools/manage_gameobject.py(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
UnityMcpBridge/Runtime/Serialization/UnityTypeConverters.cs (2)
UnityMcpBridge/Editor/UnityMcpBridge.cs (1)
UnityMcpBridge(51-55)UnityMcpBridge/Editor/Helpers/GameObjectSerializer.cs (1)
JToken(357-376)
UnityMcpBridge/Editor/Helpers/GameObjectSerializer.cs (1)
UnityMcpBridge/Runtime/Serialization/UnityTypeConverters.cs (7)
Vector3Converter(11-34)Vector2Converter(36-56)QuaternionConverter(58-84)ColorConverter(86-112)RectConverter(114-140)BoundsConverter(142-161)UnityEngineObjectConverter(164-265)
UnityMcpBridge/Editor/Tools/ManageGameObject.cs (2)
UnityMcpBridge/Editor/Helpers/GameObjectSerializer.cs (3)
GameObjectSerializer(17-377)GetGameObjectData(24-99)GetComponentData(122-250)UnityMcpBridge/Runtime/Serialization/UnityTypeConverters.cs (7)
Vector3Converter(11-34)Vector2Converter(36-56)QuaternionConverter(58-84)ColorConverter(86-112)RectConverter(114-140)BoundsConverter(142-161)UnityEngineObjectConverter(164-265)
🔇 Additional comments (35)
UnityMcpBridge/Editor/Helpers/GameObjectSerializer.cs.meta (1)
1-2: Metadata File for GameObjectSerializer.cs Looks GoodThe file correctly specifies
fileFormatVersion: 2and includes a unique GUID required by Unity’s asset management.UnityMcpBridge/Runtime/Serialization/UnityTypeConverters.cs.meta (1)
1-2: Metadata File for UnityTypeConverters.cs ApprovedThe metadata file correctly defines
fileFormatVersion: 2and a unique GUID, ensuring proper tracking of UnityTypeConverters.cs.UnityMcpBridge/Runtime/UnityMcpBridge.Runtime.asmdef.meta (1)
1-8: Runtime Assembly Definition Metadata Looks GoodThe file correctly defines the assembly metadata with
fileFormatVersion: 2, provides a unique GUID, and includes appropriateAssemblyDefinitionImportersettings.UnityMcpBridge/Editor/UnityMcpBridge.Editor.asmdef.meta (1)
1-8: Editor Assembly Definition Metadata Looks GoodThe metadata file for the Editor assembly is correctly configured with
fileFormatVersion: 2, a unique GUID, and properAssemblyDefinitionImportersettings to manage Unity’s assembly references.UnityMcpBridge/Runtime/UnityMcpBridge.Runtime.asmdef (1)
1-16: Assembly definition file structure looks correct.This new assembly definition file correctly defines the runtime component of the UnityMcpBridge package. The assembly is properly named with the corresponding namespace, and includes the necessary reference to the Unity engine assembly (via GUID). The configuration enables auto-referencing so that other assemblies in the project can automatically access this runtime assembly.
This structure supports the new serialization functionality as mentioned in the PR objectives, allowing for a clean separation between editor and runtime components.
UnityMcpBridge/Runtime.meta (1)
1-8: Meta file correctly configured.This meta file is correctly structured for the Runtime folder asset. The GUID has been properly generated and will ensure that Unity can track this folder asset across project changes and source control operations.
UnityMcpBridge/Editor/UnityMcpBridge.cs (1)
271-280: Enhanced error handling looks good.This addition to the error handling flow properly addresses the case where a command deserializes to null, providing a clear and specific error message to the client. This is in line with the PR's goal of improving the robustness of the serialization process.
UnityMcpBridge/Editor/UnityMcpBridge.Editor.asmdef (1)
1-19: Assembly definition correctly references Runtime assembly.The Editor assembly definition is properly configured to:
- Reference the newly created Runtime assembly
- Include the Unity engine reference (GUID)
- Target only the Editor platform
- Enable auto-referencing
This structure supports the PR objective of extracting GameObject and Component serialization logic into a new helper class, allowing for proper separation of concerns between editor and runtime functionality.
UnityMcpServer/src/tools/manage_gameobject.py (1)
96-97: Good integration of new parameter into the request dictionary.This ensures the
includeNonPublicSerializedparameter is only included if notNone, avoiding unnecessary traffic. Nicely done.UnityMcpBridge/Editor/Helpers/GameObjectSerializer.cs (5)
101-116: Caching approach is well-structured – no major concerns.The
CachedMetadataclass is straightforward and likely beneficial for reflection performance. No immediate issues identified.
252-278: Utility method is clear and maintains consistent logging.
AddSerializableValueis straightforward. The try-catch with warnings is helpful for diagnosing reflection issues without halting execution.
280-335: Robust JSON-to-plain-object conversion.
ConvertJTokenToPlainObjectnicely converts nested objects, arrays, and primitive types. Handling unknown token types by returningnullis a practical fallback. No issues noted.
337-355: Custom serializer settings are thoughtfully configured.Including known Unity converters and ignoring reference loops fosters reliability. This solution should handle typical Unity object graphs effectively.
356-376: Graceful handling of serialization failures.
CreateTokenFromValuecatches and logsJsonSerializationExceptionand other exceptions, preventing unexpected crashes. This is a solid approach.UnityMcpBridge/Runtime/Serialization/UnityTypeConverters.cs (7)
11-34: Vector3Converter looks good.The read/write implementations are straightforward and align with standard JSON structures. No concerns.
36-56: Vector2Converter is well-implemented.Consistently mirrors the Vector3 approach, providing a clean JSON representation.
58-84: QuaternionConverter is properly handled.Serializing each component (x, y, z, w) individually is a valid approach. No issues.
86-112: ColorConverter correctly handles RGBA values.Straightforward serialization of color channels is correct for typical Unity usage.
114-140: RectConverter is aligned with Unity's typical layout.Width/height-based rectangular data are captured accurately.
142-161: BoundsConverter usage of nested serialization is appropriate.Using
serializer.Serializeforcenterandsizeis a neat way to reuseVector3conversions.
163-265: UnityEngineObjectConverter is comprehensive but may pose potential asset path security concerns.Overall, it handles both Editor and runtime modes well, logging warnings in runtime. Consider verifying that fetched asset paths come from trusted sources to mitigate possible malicious path usage. Otherwise, this approach is effective for typical use cases.
UnityMcpBridge/Editor/Tools/ManageGameObject.cs (14)
5-5: No concerns with new import
ImportingNewtonsoft.Jsonfor theJsonSerializationExceptionis appropriate given the new serialization logic.
47-49: Confirm the default for non-public serialization
DefaultingincludeNonPublicSerializedtotruecan expose private data and potentially impact performance (depending on how many private fields exist). Ensure this default aligns with security and performance requirements.
139-140: Flag is properly passed to retrieval method
ForwardingincludeNonPublicSerializedtoGetComponentsFromTargetensures consistent component serialization behavior. Good job.
622-622: Marking as modified
Assigningmodified = true;upon successful tag creation is correct to reflect changes.
749-752: Consistent new serializer usage
Including the call toHelpers.GameObjectSerializer.GetGameObjectData(...)helps keep responses uniform.
757-757: Ensuring consistency in modify flows
Returning updated GameObject data after modifications improves clarity for clients.Also applies to: 760-760
531-532: Refactored final instance serialization
Using the serializer helper for newly created GameObjects centralizes logic. Looks good.
823-824: Serializing search results
Fetching serialized data via the helper provides users with a consistent data structure.
828-828: Expanded signature for component retrieval
AddingincludeNonPublicSerializedto the method signature matches the new requirement for non-public field inclusion.
841-842: Enabling deeper component serialization
Passing theincludeNonPublicSerializedflag toGetComponentDatagives users more flexibility in retrieving private fields.
907-907: Inline reminder for new serializer helper
The comment clarifies the approach. No issues here.
953-953: Consistent removal response
Returning serializer data after removing a component aligns with the rest of the tool’s output pattern.
1002-1002: Serialized data after property setting
Providing updated GameObject data once properties are set helps confirm changes took effect.Also applies to: 1005-1005
1496-1692: Comprehensive nested property handling
The approach to handle arrays, lists, and especially Material/Shader properties shows thoroughness. Keep an eye on performance if very deep nesting or large arrays are common. Otherwise, this looks well-structured.
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.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
UnityMcpBridge/Editor/Helpers/GameObjectSerializer.cs (1)
114-114: Ensure thread safety for the metadata cache.As noted in previous feedback, if
_metadataCacheis accessed from multiple threads, you risk concurrency issues. Consider adding locking or using a thread-safe collection.
🧹 Nitpick comments (9)
UnityMcpBridge/Editor/Helpers/GameObjectSerializer.cs (3)
16-16: Remove stray text.There's an unexpected "tew" in the XML comment at line 16. It might be a leftover placeholder.
- /// </summary> tew + /// </summary>
38-90: Consider storing rotations as quaternions for future-proofing.Currently, rotations are stored and retrieved purely in Euler angles. While simpler, this can lead to gimbal lock issues or precision loss. You could store quaternions directly (or in parallel) for more robust rotation data handling.
435-446: Log serialization failures for debugging.When
CreateTokenFromValuereturns null due to serialization failures, the code silently moves on. If diagnosing serialization issues is important, consider adding a structured log or error response to handle these cases more visibly in production.UnityMcpBridge/Editor/Tools/ManageGameObject.cs (6)
12-13: Remove unused import if no runtime converters are used.The
UnityMcpBridge.Runtime.Serializationimport may be unnecessary if you’re not referencing those runtime converters directly in this file.-using UnityMcpBridge.Runtime.Serialization; // <<< Keep for Converters access? Might not be needed here directly
26-29: Avoid committed debug logs in production.You have debug snippets commented out. Consider removing them or wrapping them in a proper debug flag if they are no longer needed.
122-124: Improve error handling with fallback.Wrapping the
switch (action)block in a try-catch is good; you might also add more granular catches if certain actions can fail with known exceptions (e.g., reflection or IO).
839-887: Validate backward iteration logic.Reverse iteration of the copied components list is interesting for insertion at index 0. While this preserves the original order, it’s slightly less intuitive. Adding a clarifying comment or using a forward iteration with
Addcould be simpler.Also applies to: 890-891
1465-1465: Handle potential version mismatches in custom converters.Creating a dedicated input serializer with custom converters helps. Ensure you handle updates or version mismatches in external libraries (e.g., if the
JsonConvertersignatures change). A robust approach might be to define explicit tests covering each converter’s read/write logic.Also applies to: 1468-1485, 1486-1486, 1490-1490, 1493-1493, 1501-1502, 1507-1509, 1514-1515, 1517-1518, 1523-1525, 1532-1532
1694-1738: Check fallback logic in nested property assignment.If the final property or field type conversion fails, you skip the assignment. It might help to log or consolidate partial successes/failures in the response, especially for large complex hierarchical structures.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
UnityMcpBridge/Editor/Helpers/GameObjectSerializer.cs(1 hunks)UnityMcpBridge/Editor/Tools/ManageGameObject.cs(19 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
UnityMcpBridge/Editor/Helpers/GameObjectSerializer.cs (1)
UnityMcpBridge/Runtime/Serialization/UnityTypeConverters.cs (7)
Vector3Converter(11-34)Vector2Converter(36-56)QuaternionConverter(58-84)ColorConverter(86-112)RectConverter(114-140)BoundsConverter(142-161)UnityEngineObjectConverter(164-265)
UnityMcpBridge/Editor/Tools/ManageGameObject.cs (2)
UnityMcpBridge/Editor/Helpers/GameObjectSerializer.cs (4)
GameObjectSerializer(17-453)GetGameObjectData(24-99)GetComponentData(122-326)JToken(433-452)UnityMcpBridge/Runtime/Serialization/UnityTypeConverters.cs (7)
Vector3Converter(11-34)Vector2Converter(36-56)QuaternionConverter(58-84)ColorConverter(86-112)RectConverter(114-140)BoundsConverter(142-161)UnityEngineObjectConverter(164-265)
🔇 Additional comments (10)
UnityMcpBridge/Editor/Helpers/GameObjectSerializer.cs (1)
413-425: Validate reference loop ignoring strategy.Enabling
ReferenceLoopHandling.Ignorecan prevent infinite recursion but may also silently skip deeply nested data. Confirm this setting aligns with your desired debugging or reporting needs, or consider a more explicit approach (e.g., manually skipping certain references).UnityMcpBridge/Editor/Tools/ManageGameObject.cs (9)
5-5: No issues with adding Newtonsoft.Json reference.Importing this namespace for
JsonSerializationExceptionhandling appears appropriate.
47-49: Good addition of the serialization flag.Including
includeNonPublicSerializedby default totrueprovides comprehensive component data. However, if there's a use-case for more restricted data, consider defaulting tofalseor clarifying the setting’s purpose to avoid unintentional exposure of private fields.
139-140: Seamlessly passing the new parameter.Forwarding
includeNonPublicSerializedtoGetComponentsFromTargetis cleanly done. Confirm all calls toHandleCommandsupply this parameter as expected.
531-532: Helpful use of the new serializer.Returning serialized GameObject data from
Response.Successmakes the result more descriptive. Good job keeping the code DRY by delegating toGameObjectSerializer.
823-824: Consistent usage of serializer for results.Similarly, returning fully serialized data in
FindGameObjectsfosters consistent output.
828-829: Parameterizing field inclusion is a beneficial design.
GetComponentsFromTargetnow offers a convenient way to toggle private field serialization. This is an excellent extension point for user privacy or partial data retrieval.
951-951: Consistent reporting after adding a single component.Providing the updated GameObject data is a nice user experience detail.
Also applies to: 954-955
1047-1047: Great continuity.Returning updated GameObject data after setting component properties maintains uniform results across actions.
Also applies to: 1050-1051
1652-1688: Ensure shader property consistency.When assigning values to material shader properties by token type, be mindful of multi-dimensional data. For example, a 3-float color (RGB) with no alpha might default to 1.0 for alpha, which can differ from user expectations.
| // Handle fully qualified names first | ||
| Type type = Type.GetType(typeName); | ||
| if (type != null) return type; | ||
|
|
||
| // Handle common namespaces implicitly (add more as needed) | ||
| string[] namespaces = { "UnityEngine", "UnityEngine.UI", "UnityEngine.AI", "UnityEngine.Animations", "UnityEngine.Audio", "UnityEngine.EventSystems", "UnityEngine.InputSystem", "UnityEngine.Networking", "UnityEngine.Rendering", "UnityEngine.SceneManagement", "UnityEngine.Tilemaps", "UnityEngine.U2D", "UnityEngine.Video", "UnityEditor", "UnityEditor.AI", "UnityEditor.Animations", "UnityEditor.Experimental.GraphView", "UnityEditor.IMGUI.Controls", "UnityEditor.PackageManager.UI", "UnityEditor.SceneManagement", "UnityEditor.UI", "UnityEditor.U2D", "UnityEditor.VersionControl" }; // Add more relevant namespaces | ||
|
|
||
| foreach (string ns in namespaces) { | ||
| type = Type.GetType($"{ns}.{typeName}, {ns.Split('.')[0]}.CoreModule") ?? // Heuristic: Check CoreModule first for UnityEngine/UnityEditor | ||
| Type.GetType($"{ns}.{typeName}, {ns.Split('.')[0]}"); // Try assembly matching namespace root | ||
| if (type != null) return type; | ||
| } | ||
|
|
||
|
|
||
| // If not found, search all loaded assemblies (slower, last resort) | ||
| // Prioritize assemblies likely to contain game/editor types | ||
| Assembly[] priorityAssemblies = { | ||
| Assembly.Load("Assembly-CSharp"), // Main game scripts | ||
| Assembly.Load("Assembly-CSharp-Editor"), // Main editor scripts | ||
| // Add other important project assemblies if known | ||
| }; | ||
| foreach (var assembly in priorityAssemblies.Where(a => a != null)) | ||
| { | ||
| type = assembly.GetType(typeName) ?? assembly.GetType("UnityEngine." + typeName) ?? assembly.GetType("UnityEditor." + typeName); | ||
| if (type != null) return type; | ||
| } | ||
|
|
||
| // Search remaining assemblies | ||
| foreach (var assembly in AppDomain.CurrentDomain.GetAssemblies().Except(priorityAssemblies)) | ||
| { | ||
| try { // Protect against assembly loading errors | ||
| type = assembly.GetType(typeName); | ||
| if (type != null) return type; | ||
| // Also check with common namespaces if simple name given | ||
| foreach (string ns in namespaces) { | ||
| type = assembly.GetType($"{ns}.{typeName}"); | ||
| if (type != null) return type; | ||
| } | ||
| } catch (Exception ex) { | ||
| Debug.LogWarning($"[FindType] Error searching assembly {assembly.FullName}: {ex.Message}"); | ||
| } | ||
| } | ||
|
|
||
| Debug.LogWarning($"[FindType] Type not found after extensive search: '{typeName}'"); | ||
| return null; // Not found | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reflective type searching can be expensive.
You’re iterating many assemblies. If performance becomes an issue, caching discovered types might help reduce overhead.
…sform and Camera components to prevent serialization crashes
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
UnityMcpBridge/Editor/Helpers/GameObjectSerializer.cs (1)
114-114: Revisit thread safety for_metadataCache.
If multiple threads may invoke these serialization methods, the cache dictionary should be guarded. This mirrors a past review comment, so marking as duplicate.
🧹 Nitpick comments (3)
UnityMcpBridge/Editor/Helpers/GameObjectSerializer.cs (3)
16-16: Remove unintended text from XML documentation summary.
The trailing "tew" within the summary appears to be a stray fragment. Consider removing or correcting it.- /// </summary> tew + /// </summary>
91-98: Consider offering an option to serialize the entire component data.
Right now, component data retrieval is commented out while only component names are included. If users need detailed component data, consider exposing or toggling that feature.- // components = go.GetComponents<Component>().Select(c => GetComponentData(c)).ToList() + components = go.GetComponents<Component>().Select(c => GetComponentData(c)).ToList()
402-427: Consider reintroducing warnings or logs for reflection failures.
Currently, exceptions during property or field serialization are swallowed. Logging or partially reporting errors may help diagnose issues without halting serialization.catch (Exception e) { - // Debug.LogWarning($"[AddSerializableValue] ... + Debug.LogWarning($"[AddSerializableValue] Error processing '{name}': {e.Message}"); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
UnityMcpBridge/Editor/Helpers/GameObjectSerializer.cs(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
UnityMcpBridge/Editor/Helpers/GameObjectSerializer.cs (1)
UnityMcpBridge/Runtime/Serialization/UnityTypeConverters.cs (7)
Vector3Converter(11-34)Vector2Converter(36-56)QuaternionConverter(58-84)ColorConverter(86-112)RectConverter(114-140)BoundsConverter(142-161)UnityEngineObjectConverter(164-265)
🔇 Additional comments (2)
UnityMcpBridge/Editor/Helpers/GameObjectSerializer.cs (2)
131-157: Well-structured manual serialization ofTransform.
Explicitly skipping problematic properties likelossyScaleandrotationhelps prevent reflection errors. This is a robust approach for transform components.
160-220: Practical handling of camera-specific properties.
Defining a safe subset of serializable properties circumvents exceptions from complex camera matrices. Great job isolating them in a dedicated dictionary before serialization.
1 participant
This PR significantly refactors the
ManageGameObjecttool and supercharges its ability to read component data accurately and comprehensively! 🚀Summary:
ManageGameObject.csinto a new, dedicatedGameObjectSerializerhelper class (UnityMcpBridge.Editor.Helpers.GameObjectSerializer).JsonConverterimplementations (Vector3Converter,ColorConverter,UnityEngineObjectConverter, etc.). This means theget_componentsaction can reliably pull detailed info for all sorts of component data types.ManageGameObject.csto delegate serialization tasks to the newGameObjectSerializer.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