Make it easier to add tools #301
Conversation
This will allow us to more easily collect tools
Updated the relative links in both README-DEV.md and README-DEV-zh.md to use direct filenames instead of paths relative to the docs directory, improving link correctness when files are accessed from the root directory.
Updated the link to TELEMETRY.md in README.md to point to the new docs/ directory location to ensure users can access the telemetry documentation correctly. Also moved the TELEMETRY.md file to the docs/ directory as part of the documentation restructuring.
Moved the CursorHelp.md file to the docs directory to better organize documentation files and improve project structure.
…and path corrections - Clarified that the `name` argument in `@mcp_for_unity_tool` decorator is optional and defaults to the function name - Added documentation about using all FastMCP `mcp.tool` function decorator options - Updated class naming documentation to mention snake_case conversion by default - Corrected Python file path from `tools/screenshot_tool.py` to `UnityMcpServer~/src/tools/screenshot_tool.py` - Enhanced documentation for tool discovery and usage examples
Rearranged the development section in README.md to better organize the documentation flow. Added a dedicated section for "Adding Custom Tools" with a link to the new CUSTOM_TOOLS.md file, and renamed the previous "For Developers" section to "Contributing to the Project" to better reflect its content. This improves discoverability and organization of the development setup documentation.
|
Warning Rate limit exceeded@msanatan has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 10 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughAdds Python and C# registration surfaces and auto-discovery for MCP tools: a Python decorator-based registry and package importer, a C# McpForUnityTool attribute and reflection-based CommandRegistry.Initialize(), migrates many server tools to module-level @mcp_for_unity_tool, updates bridge startup, tests, docs, and installer flows. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Bridge as MCPForUnityBridge
participant Registry as CommandRegistry
participant Tools as Editor Tool Classes
Bridge->>Bridge: Start()
rect rgba(200,230,255,0.28)
Bridge->>Registry: Initialize()
Registry->>Registry: if _initialized -> return
Registry->>Tools: reflect assemblies for [McpForUnityTool]
Tools-->>Registry: annotated classes (types)
Registry->>Registry: bind HandleCommand -> handlers (snake_case)
Registry-->>Bridge: registry ready
end
sequenceDiagram
autonumber
participant Loader as tools.__init__
participant FS as tools package
participant Importer as importlib
participant Reg as registry
participant MCP as MCP Server
Loader->>FS: scan modules (skip private)
FS->>Importer: import module
Importer-->>Reg: module-level @mcp_for_unity_tool decorators register handlers
Loader->>Reg: get_registered_tools()
alt tools found
loop per tool
Loader->>MCP: register tool (apply wrappers/telemetry as configured)
end
else none found
Loader-->>Loader: no tools to register
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
UnityMcpBridge/UnityMcpServer~/src/registry/__init__.py (1)
1-14: LGTM with optional nitpick.The registry package correctly exposes the public API for tool registration and discovery. The imports and
__all__definition are correct.Optional: Consider sorting
__all__alphabetically for consistency with isort conventions.The current order is logical (decorator, query, cleanup), but alphabetical sorting can improve consistency across the codebase.
__all__ = [ - 'mcp_for_unity_tool', - 'get_registered_tools', 'clear_registry' + 'get_registered_tools', + 'mcp_for_unity_tool', ]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (32)
README-zh.md(2 hunks)README.md(2 hunks)TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/CommandRegistryTests.cs(2 hunks)UnityMcpBridge/Editor/MCPForUnityBridge.cs(1 hunks)UnityMcpBridge/Editor/Tools/CommandRegistry.cs(1 hunks)UnityMcpBridge/Editor/Tools/ManageAsset.cs(1 hunks)UnityMcpBridge/Editor/Tools/ManageEditor.cs(1 hunks)UnityMcpBridge/Editor/Tools/ManageGameObject.cs(1 hunks)UnityMcpBridge/Editor/Tools/ManageScene.cs(1 hunks)UnityMcpBridge/Editor/Tools/ManageScript.cs(1 hunks)UnityMcpBridge/Editor/Tools/ManageShader.cs(1 hunks)UnityMcpBridge/Editor/Tools/McpForUnityToolAttribute.cs(1 hunks)UnityMcpBridge/Editor/Tools/McpForUnityToolAttribute.cs.meta(1 hunks)UnityMcpBridge/Editor/Tools/MenuItems/ManageMenuItem.cs(1 hunks)UnityMcpBridge/Editor/Tools/Prefabs/ManagePrefabs.cs(1 hunks)UnityMcpBridge/Editor/Tools/ReadConsole.cs(1 hunks)UnityMcpBridge/UnityMcpServer~/src/registry/__init__.py(1 hunks)UnityMcpBridge/UnityMcpServer~/src/registry/tool_registry.py(1 hunks)UnityMcpBridge/UnityMcpServer~/src/tools/__init__.py(1 hunks)UnityMcpBridge/UnityMcpServer~/src/tools/manage_asset.py(1 hunks)UnityMcpBridge/UnityMcpServer~/src/tools/manage_editor.py(1 hunks)UnityMcpBridge/UnityMcpServer~/src/tools/manage_gameobject.py(1 hunks)UnityMcpBridge/UnityMcpServer~/src/tools/manage_menu_item.py(1 hunks)UnityMcpBridge/UnityMcpServer~/src/tools/manage_prefabs.py(1 hunks)UnityMcpBridge/UnityMcpServer~/src/tools/manage_scene.py(1 hunks)UnityMcpBridge/UnityMcpServer~/src/tools/manage_script.py(1 hunks)UnityMcpBridge/UnityMcpServer~/src/tools/manage_script_edits.py(0 hunks)UnityMcpBridge/UnityMcpServer~/src/tools/manage_shader.py(1 hunks)UnityMcpBridge/UnityMcpServer~/src/tools/read_console.py(1 hunks)UnityMcpBridge/UnityMcpServer~/src/tools/resource_tools.py(2 hunks)UnityMcpBridge/UnityMcpServer~/src/tools/script_apply_edits.py(1 hunks)docs/CUSTOM_TOOLS.md(1 hunks)
💤 Files with no reviewable changes (1)
- UnityMcpBridge/UnityMcpServer~/src/tools/manage_script_edits.py
🧰 Additional context used
🧬 Code graph analysis (25)
UnityMcpBridge/Editor/Tools/ManageScene.cs (8)
UnityMcpBridge/Editor/Tools/ManageAsset.cs (1)
McpForUnityTool(25-1330)UnityMcpBridge/Editor/Tools/ManageEditor.cs (1)
McpForUnityTool(18-626)UnityMcpBridge/Editor/Tools/ManageGameObject.cs (1)
McpForUnityTool(22-2281)UnityMcpBridge/Editor/Tools/ManageScript.cs (1)
McpForUnityTool(52-2553)UnityMcpBridge/Editor/Tools/ManageShader.cs (1)
McpForUnityTool(15-342)UnityMcpBridge/Editor/Tools/MenuItems/ManageMenuItem.cs (1)
McpForUnityTool(7-41)UnityMcpBridge/Editor/Tools/Prefabs/ManagePrefabs.cs (1)
McpForUnityTool(12-274)UnityMcpBridge/Editor/Tools/ReadConsole.cs (1)
McpForUnityTool(17-570)
UnityMcpBridge/Editor/Tools/ManageEditor.cs (3)
UnityMcpBridge/Editor/Tools/ManageAsset.cs (1)
McpForUnityTool(25-1330)UnityMcpBridge/Editor/Tools/ManageGameObject.cs (1)
McpForUnityTool(22-2281)UnityMcpBridge/Editor/Tools/ManageScene.cs (1)
McpForUnityTool(17-474)
UnityMcpBridge/Editor/Tools/ManageAsset.cs (8)
UnityMcpBridge/Editor/Tools/ManageEditor.cs (1)
McpForUnityTool(18-626)UnityMcpBridge/Editor/Tools/ManageGameObject.cs (1)
McpForUnityTool(22-2281)UnityMcpBridge/Editor/Tools/ManageScene.cs (1)
McpForUnityTool(17-474)UnityMcpBridge/Editor/Tools/ManageScript.cs (1)
McpForUnityTool(52-2553)UnityMcpBridge/Editor/Tools/ManageShader.cs (1)
McpForUnityTool(15-342)UnityMcpBridge/Editor/Tools/MenuItems/ManageMenuItem.cs (1)
McpForUnityTool(7-41)UnityMcpBridge/Editor/Tools/Prefabs/ManagePrefabs.cs (1)
McpForUnityTool(12-274)UnityMcpBridge/Editor/Tools/ReadConsole.cs (1)
McpForUnityTool(17-570)
UnityMcpBridge/Editor/Tools/MenuItems/ManageMenuItem.cs (3)
UnityMcpBridge/Editor/Tools/ManageAsset.cs (1)
McpForUnityTool(25-1330)UnityMcpBridge/Editor/Tools/ManageEditor.cs (1)
McpForUnityTool(18-626)UnityMcpBridge/Editor/Tools/ManageGameObject.cs (1)
McpForUnityTool(22-2281)
UnityMcpBridge/UnityMcpServer~/src/registry/__init__.py (1)
UnityMcpBridge/UnityMcpServer~/src/registry/tool_registry.py (3)
mcp_for_unity_tool(10-41)get_registered_tools(44-46)clear_registry(49-51)
UnityMcpBridge/UnityMcpServer~/src/tools/read_console.py (3)
UnityMcpBridge/UnityMcpServer~/src/registry/tool_registry.py (1)
mcp_for_unity_tool(10-41)UnityMcpBridge/UnityMcpServer~/src/unity_connection.py (1)
send_command_with_retry(416-436)UnityMcpBridge/UnityMcpServer~/src/tools/resource_tools.py (1)
_coerce_int(20-42)
UnityMcpBridge/Editor/Tools/ManageScript.cs (5)
UnityMcpBridge/Editor/Tools/ManageAsset.cs (1)
McpForUnityTool(25-1330)UnityMcpBridge/Editor/Tools/ManageEditor.cs (1)
McpForUnityTool(18-626)UnityMcpBridge/Editor/Tools/ManageGameObject.cs (1)
McpForUnityTool(22-2281)UnityMcpBridge/Editor/Tools/ManageShader.cs (1)
McpForUnityTool(15-342)UnityMcpBridge/Editor/Tools/MenuItems/ManageMenuItem.cs (1)
McpForUnityTool(7-41)
UnityMcpBridge/Editor/Tools/ReadConsole.cs (4)
UnityMcpBridge/Editor/Tools/ManageAsset.cs (1)
McpForUnityTool(25-1330)UnityMcpBridge/Editor/Tools/ManageEditor.cs (1)
McpForUnityTool(18-626)UnityMcpBridge/Editor/Tools/ManageGameObject.cs (1)
McpForUnityTool(22-2281)UnityMcpBridge/Editor/Tools/MenuItems/ManageMenuItem.cs (1)
McpForUnityTool(7-41)
UnityMcpBridge/Editor/Tools/CommandRegistry.cs (1)
UnityMcpBridge/Editor/Helpers/McpLog.cs (4)
McpLog(6-30)Info(15-19)Error(26-29)Warn(21-24)
UnityMcpBridge/UnityMcpServer~/src/tools/manage_shader.py (2)
UnityMcpBridge/UnityMcpServer~/src/registry/tool_registry.py (1)
mcp_for_unity_tool(10-41)UnityMcpBridge/UnityMcpServer~/src/unity_connection.py (1)
send_command_with_retry(416-436)
UnityMcpBridge/UnityMcpServer~/src/tools/script_apply_edits.py (2)
UnityMcpBridge/UnityMcpServer~/src/registry/tool_registry.py (1)
mcp_for_unity_tool(10-41)UnityMcpBridge/UnityMcpServer~/src/unity_connection.py (1)
send_command_with_retry(416-436)
UnityMcpBridge/Editor/Tools/ManageShader.cs (2)
UnityMcpBridge/Editor/Tools/ManageAsset.cs (1)
McpForUnityTool(25-1330)UnityMcpBridge/Editor/Tools/ManageGameObject.cs (1)
McpForUnityTool(22-2281)
UnityMcpBridge/UnityMcpServer~/src/tools/manage_prefabs.py (2)
UnityMcpBridge/UnityMcpServer~/src/registry/tool_registry.py (1)
mcp_for_unity_tool(10-41)UnityMcpBridge/UnityMcpServer~/src/unity_connection.py (1)
send_command_with_retry(416-436)
UnityMcpBridge/UnityMcpServer~/src/tools/manage_scene.py (3)
UnityMcpBridge/UnityMcpServer~/src/registry/tool_registry.py (1)
mcp_for_unity_tool(10-41)UnityMcpBridge/UnityMcpServer~/src/unity_connection.py (1)
send_command_with_retry(416-436)UnityMcpBridge/UnityMcpServer~/src/tools/resource_tools.py (1)
_coerce_int(20-42)
UnityMcpBridge/Editor/MCPForUnityBridge.cs (1)
UnityMcpBridge/Editor/Tools/CommandRegistry.cs (2)
CommandRegistry(14-128)Initialize(22-28)
UnityMcpBridge/Editor/Tools/Prefabs/ManagePrefabs.cs (3)
UnityMcpBridge/Editor/Tools/ManageAsset.cs (1)
McpForUnityTool(25-1330)UnityMcpBridge/Editor/Tools/ManageEditor.cs (1)
McpForUnityTool(18-626)UnityMcpBridge/Editor/Tools/ManageGameObject.cs (1)
McpForUnityTool(22-2281)
UnityMcpBridge/UnityMcpServer~/src/tools/manage_menu_item.py (2)
UnityMcpBridge/UnityMcpServer~/src/registry/tool_registry.py (1)
mcp_for_unity_tool(10-41)UnityMcpBridge/UnityMcpServer~/src/unity_connection.py (1)
async_send_command_with_retry(439-452)
UnityMcpBridge/UnityMcpServer~/src/tools/manage_editor.py (3)
UnityMcpBridge/UnityMcpServer~/src/registry/tool_registry.py (1)
mcp_for_unity_tool(10-41)UnityMcpBridge/UnityMcpServer~/src/telemetry.py (2)
is_telemetry_enabled(458-460)record_tool_usage(403-429)UnityMcpBridge/UnityMcpServer~/src/unity_connection.py (1)
send_command_with_retry(416-436)
UnityMcpBridge/Editor/Tools/ManageGameObject.cs (2)
UnityMcpBridge/Editor/Tools/ManageAsset.cs (1)
McpForUnityTool(25-1330)UnityMcpBridge/Editor/Tools/ManageScene.cs (1)
McpForUnityTool(17-474)
UnityMcpBridge/UnityMcpServer~/src/tools/__init__.py (2)
UnityMcpBridge/UnityMcpServer~/src/telemetry_decorator.py (1)
telemetry_tool(17-107)UnityMcpBridge/UnityMcpServer~/src/registry/tool_registry.py (2)
get_registered_tools(44-46)mcp_for_unity_tool(10-41)
UnityMcpBridge/UnityMcpServer~/src/tools/manage_script.py (2)
UnityMcpBridge/UnityMcpServer~/src/registry/tool_registry.py (1)
mcp_for_unity_tool(10-41)UnityMcpBridge/UnityMcpServer~/src/unity_connection.py (1)
send_command_with_retry(416-436)
UnityMcpBridge/UnityMcpServer~/src/tools/manage_gameobject.py (2)
UnityMcpBridge/UnityMcpServer~/src/registry/tool_registry.py (1)
mcp_for_unity_tool(10-41)UnityMcpBridge/UnityMcpServer~/src/unity_connection.py (1)
send_command_with_retry(416-436)
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/CommandRegistryTests.cs (1)
UnityMcpBridge/Editor/Tools/CommandRegistry.cs (1)
CommandRegistry(14-128)
UnityMcpBridge/UnityMcpServer~/src/tools/manage_asset.py (3)
UnityMcpBridge/UnityMcpServer~/src/registry/tool_registry.py (1)
mcp_for_unity_tool(10-41)UnityMcpBridge/UnityMcpServer~/src/unity_connection.py (1)
async_send_command_with_retry(439-452)UnityMcpBridge/UnityMcpServer~/src/tools/resource_tools.py (1)
_coerce_int(20-42)
UnityMcpBridge/UnityMcpServer~/src/tools/resource_tools.py (2)
UnityMcpBridge/UnityMcpServer~/src/registry/tool_registry.py (1)
mcp_for_unity_tool(10-41)UnityMcpBridge/UnityMcpServer~/src/tools/manage_asset.py (1)
_coerce_int(41-54)
🪛 markdownlint-cli2 (0.18.1)
docs/CUSTOM_TOOLS.md
9-9: Images should have alternate text (alt text)
(MD045, no-alt-text)
11-11: Images should have alternate text (alt text)
(MD045, no-alt-text)
14-14: Images should have alternate text (alt text)
(MD045, no-alt-text)
15-15: Images should have alternate text (alt text)
(MD045, no-alt-text)
41-41: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
42-42: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
43-43: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
44-44: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
45-45: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
46-46: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
47-47: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
48-48: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
49-49: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
50-50: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
51-51: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
116-116: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
135-135: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
147-147: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
160-160: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
176-176: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🪛 Ruff (0.13.2)
UnityMcpBridge/UnityMcpServer~/src/registry/__init__.py
10-14: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
UnityMcpBridge/UnityMcpServer~/src/tools/read_console.py
52-52: Do not catch blind exception: Exception
(BLE001)
85-86: try-except-pass detected, consider logging the exception
(S110)
85-85: Do not catch blind exception: Exception
(BLE001)
UnityMcpBridge/UnityMcpServer~/src/tools/manage_shader.py
58-58: Do not catch blind exception: Exception
(BLE001)
60-60: Use explicit conversion flag
Replace with conversion flag
(RUF010)
UnityMcpBridge/UnityMcpServer~/src/tools/script_apply_edits.py
27-29: Avoid specifying long messages outside the exception class
(TRY003)
55-55: Avoid specifying long messages outside the exception class
(TRY003)
68-68: Avoid specifying long messages outside the exception class
(TRY003)
71-71: Function definition does not bind loop variable lines
(B023)
72-72: Ambiguous variable name: l
(E741)
72-72: Function definition does not bind loop variable lines
(B023)
73-73: Ambiguous variable name: l
(E741)
73-73: Function definition does not bind loop variable lines
(B023)
89-90: Avoid specifying long messages outside the exception class
(TRY003)
436-436: Comment contains ambiguous ‑ (NON-BREAKING HYPHEN). Did you mean - (HYPHEN-MINUS)?
(RUF003)
436-436: Comment contains ambiguous ‑ (NON-BREAKING HYPHEN). Did you mean - (HYPHEN-MINUS)?
(RUF003)
648-648: Do not catch blind exception: Exception
(BLE001)
679-679: Do not catch blind exception: Exception
(BLE001)
729-729: Do not catch blind exception: Exception
(BLE001)
790-790: Do not catch blind exception: Exception
(BLE001)
830-830: Do not catch blind exception: Exception
(BLE001)
883-883: Do not catch blind exception: Exception
(BLE001)
895-895: Consider [*diff[:800], "... (diff truncated) ..."] instead of concatenation
Replace with [*diff[:800], "... (diff truncated) ..."]
(RUF005)
899-899: Do not catch blind exception: Exception
(BLE001)
904-904: Do not catch blind exception: Exception
(BLE001)
924-924: Consider [*diff[:2000], "... (diff truncated) ..."] instead of concatenation
Replace with [*diff[:2000], "... (diff truncated) ..."]
(RUF005)
UnityMcpBridge/UnityMcpServer~/src/tools/manage_prefabs.py
57-57: Do not catch blind exception: Exception
(BLE001)
UnityMcpBridge/UnityMcpServer~/src/tools/manage_scene.py
34-34: Do not catch blind exception: Exception
(BLE001)
55-55: Do not catch blind exception: Exception
(BLE001)
56-56: Use explicit conversion flag
Replace with conversion flag
(RUF010)
UnityMcpBridge/UnityMcpServer~/src/tools/manage_editor.py
56-56: Do not catch blind exception: Exception
(BLE001)
57-57: Use explicit conversion flag
Replace with conversion flag
(RUF010)
UnityMcpBridge/UnityMcpServer~/src/tools/__init__.py
17-17: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
38-38: Do not catch blind exception: Exception
(BLE001)
UnityMcpBridge/UnityMcpServer~/src/tools/manage_script.py
119-119: Do not catch blind exception: Exception
(BLE001)
146-147: try-except-pass detected, consider logging the exception
(S110)
146-146: Do not catch blind exception: Exception
(BLE001)
156-157: try-except-pass detected, consider logging the exception
(S110)
156-156: Do not catch blind exception: Exception
(BLE001)
190-191: try-except-pass detected, consider logging the exception
(S110)
190-190: Do not catch blind exception: Exception
(BLE001)
212-213: try-except-pass detected, consider logging the exception
(S110)
212-212: Do not catch blind exception: Exception
(BLE001)
221-222: try-except-pass detected, consider logging the exception
(S110)
221-221: Do not catch blind exception: Exception
(BLE001)
248-250: try-except-pass detected, consider logging the exception
(S110)
248-248: Do not catch blind exception: Exception
(BLE001)
276-277: try-except-pass detected, consider logging the exception
(S110)
276-276: Do not catch blind exception: Exception
(BLE001)
283-283: Local variable lines is assigned to but never used
Remove assignment to unused variable lines
(F841)
285-285: Local variable prev is assigned to but never used
Remove assignment to unused variable prev
(F841)
287-294: Consider moving this statement to an else block
(TRY300)
295-295: Do not catch blind exception: Exception
(BLE001)
330-330: Do not catch blind exception: Exception
(BLE001)
345-346: try-except-pass detected, consider logging the exception
(S110)
345-345: Do not catch blind exception: Exception
(BLE001)
348-349: try-except-pass detected, consider logging the exception
(S110)
348-348: Do not catch blind exception: Exception
(BLE001)
496-496: Do not catch blind exception: Exception
(BLE001)
499-499: Use explicit conversion flag
Replace with conversion flag
(RUF010)
524-530: Consider moving this statement to an else block
(TRY300)
531-531: Do not catch blind exception: Exception
(BLE001)
551-551: Do not catch blind exception: Exception
(BLE001)
UnityMcpBridge/UnityMcpServer~/src/tools/manage_gameobject.py
144-144: Do not catch blind exception: Exception
(BLE001)
145-145: Use explicit conversion flag
Replace with conversion flag
(RUF010)
UnityMcpBridge/UnityMcpServer~/src/tools/manage_asset.py
53-53: Do not catch blind exception: Exception
(BLE001)
UnityMcpBridge/UnityMcpServer~/src/tools/resource_tools.py
168-169: try-except-continue detected, consider logging the exception
(S112)
168-168: Do not catch blind exception: Exception
(BLE001)
185-185: Do not catch blind exception: Exception
(BLE001)
345-345: Do not catch blind exception: Exception
(BLE001)
391-391: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (7)
UnityMcpBridge/Editor/Tools/ManageEditor.cs (1)
18-18: LGTM!The addition of the
[McpForUnityTool("manage_editor")]attribute aligns with the new auto-discovery pattern introduced in this PR. The attribute name matches the tool's purpose and is consistent with similar changes across other editor tools.UnityMcpBridge/Editor/Tools/MenuItems/ManageMenuItem.cs (1)
7-7: LGTM!The
[McpForUnityTool("manage_menu_item")]attribute is correctly applied to enable auto-discovery for this tool, consistent with the broader refactoring to decorator-based registration.UnityMcpBridge/Editor/Tools/ManageScript.cs (1)
52-52: LGTM!The
[McpForUnityTool("manage_script")]attribute is correctly placed above the class declaration, enabling the new auto-discovery mechanism for script management tools.UnityMcpBridge/Editor/Tools/Prefabs/ManagePrefabs.cs (1)
12-12: LGTM!The
[McpForUnityTool("manage_prefabs")]attribute is correctly applied, consistent with the pattern used across all tool classes in this PR.UnityMcpBridge/Editor/Tools/ManageAsset.cs (1)
25-25: LGTM!The
[McpForUnityTool("manage_asset")]attribute is correctly added to enable auto-discovery, following the same pattern as other editor tools in this PR.UnityMcpBridge/Editor/Tools/McpForUnityToolAttribute.cs.meta (1)
1-12: LGTM!Standard Unity meta file for the new
McpForUnityToolAttribute.csclass. The GUID and importer settings are correctly configured.UnityMcpBridge/Editor/Tools/McpForUnityToolAttribute.cs (1)
1-37: LGTM!The
McpForUnityToolAttributeclass is well-designed for the auto-discovery pattern:
- Correct attribute targets (Class only, single instance)
- Immutable
CommandNameproperty- Clear distinction between auto-generated and explicit command names
- Comprehensive XML documentation
The design aligns perfectly with the usage pattern seen across all tool classes in this PR.
- Added links to developer READMEs in CUSTOM_TOOLS.md to guide users to the appropriate documentation
- Fixed typo in README-DEV.md ("roote" → "root") for improved clarity
- These changes improve the developer experience by providing better documentation navigation and correcting technical inaccuracies
Updated the tool registration process to properly chain the mcp.tool decorator and telemetry wrapper, ensuring the wrapped function is correctly assigned to tool_info['func'] for proper tool execution and telemetry tracking. This change improves the reliability of tool registration and monitoring.
Refactored the server repair logic to implement a full rebuild of the MCP server installation using the embedded source. The new RebuildMcpServer method now: - Uses embedded server source instead of attempting repair of existing installation - Deletes the entire existing server directory before re-copying - Handles UV process cleanup for the target path - Simplifies the installation flow by removing the complex Python environment repair logic - Maintains the same installation behavior but with a cleaner, more reliable rebuild approach This change improves reliability of server installations by ensuring a clean slate rebuild rather than attempting to repair potentially corrupted environments.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
UnityMcpBridge/UnityMcpServer~/src/tools/__init__.py (1)
53-58: Telemetry still not applied correctly - decorator order is backwards.The past review comment about telemetry was not properly addressed. The current code registers the unwrapped function with MCP, then wraps it with telemetry afterwards. This means the MCP-registered tool won't emit telemetry.
The issue:
mcp.tool()decorator registers the function it receives with FastMCP. By the time telemetry wrapping happens on line 56, the unwrapped function is already registered.Apply this diff to fix the decorator order:
- # Apply the @mcp.tool decorator and telemetry - wrapped = mcp.tool( - name=tool_name, description=description, **kwargs)(func) - wrapped = telemetry_tool(tool_name)(wrapped) + # Apply telemetry first, then register the wrapped function with MCP + wrapped = telemetry_tool(tool_name)(func) + wrapped = mcp.tool( + name=tool_name, description=description, **kwargs)(wrapped) tool_info['func'] = wrapped
🧹 Nitpick comments (2)
UnityMcpBridge/UnityMcpServer~/src/tools/__init__.py (1)
17-17: Consider sorting__all__alphabetically.While not critical, sorting the export list improves consistency and maintainability.
Apply this diff:
-__all__ = ['register_all_tools', 'mcp_for_unity_tool'] +__all__ = ['mcp_for_unity_tool', 'register_all_tools']docs/CUSTOM_TOOLS.md (1)
244-270: LGTM! Comprehensive best practices and debugging guidance.The sections provide practical, actionable advice covering:
- Proper coding patterns for both languages
- Concrete log file locations for debugging
- Useful debugging techniques
Minor formatting fix: Add spaces in heading markers at lines 253 and 267 for consistency with Markdown standards:
-###Python +### Python -###C# +### C#
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
UnityMcpBridge/Editor/Dependencies/DependencyManager.cs(0 hunks)UnityMcpBridge/Editor/Helpers/ServerInstaller.cs(1 hunks)UnityMcpBridge/Editor/Windows/MCPForUnityEditorWindow.cs(1 hunks)UnityMcpBridge/README.md(1 hunks)UnityMcpBridge/UnityMcpServer~/src/tools/__init__.py(1 hunks)docs/CUSTOM_TOOLS.md(1 hunks)
💤 Files with no reviewable changes (1)
- UnityMcpBridge/Editor/Dependencies/DependencyManager.cs
🧰 Additional context used
🧬 Code graph analysis (2)
UnityMcpBridge/UnityMcpServer~/src/tools/__init__.py (2)
UnityMcpBridge/UnityMcpServer~/src/telemetry_decorator.py (1)
telemetry_tool(17-107)UnityMcpBridge/UnityMcpServer~/src/registry/tool_registry.py (2)
get_registered_tools(44-46)mcp_for_unity_tool(10-41)
UnityMcpBridge/Editor/Windows/MCPForUnityEditorWindow.cs (1)
UnityMcpBridge/Editor/Helpers/ServerInstaller.cs (2)
ServerInstaller(12-699)RebuildMcpServer(426-483)
🪛 markdownlint-cli2 (0.18.1)
docs/CUSTOM_TOOLS.md
253-253: No space inside hashes on closed atx style heading
(MD020, no-missing-space-closed-atx)
267-267: No space inside hashes on closed atx style heading
(MD020, no-missing-space-closed-atx)
🪛 Ruff (0.13.2)
UnityMcpBridge/UnityMcpServer~/src/tools/__init__.py
17-17: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
38-38: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (8)
UnityMcpBridge/README.md (1)
32-32: LGTM! Improved user-facing description.The updated wording is clearer and removes unnecessary implementation details, making it more user-friendly.
UnityMcpBridge/UnityMcpServer~/src/tools/__init__.py (2)
27-40: LGTM! Auto-discovery logic is well-structured.The module discovery correctly:
- Skips private modules
- Handles import failures gracefully with logging
- Uses appropriate standard library tools
The bare exception catch at line 38 is appropriate here as import errors can vary widely.
43-45: LGTM! Good defensive programming.The early exit when no tools are registered prevents unnecessary processing and provides clear feedback via logging.
docs/CUSTOM_TOOLS.md (5)
1-9: LGTM! Clear introduction.The opening effectively explains the auto-discovery feature and provides helpful cross-references to developer documentation.
18-45: LGTM! Python example follows best practices.The example demonstrates:
- Proper type hints with
Annotatedfor parameter documentation- Correct use of
send_command_with_retry()- Good parameter filtering pattern to remove
Nonevalues- Appropriate return type structure
75-135: LGTM! C# documentation is accurate and comprehensive.The C# examples correctly demonstrate:
- Proper attribute usage with explicit and auto-generated names
- Safe parameter extraction with null-conditional operators
- Appropriate type conversion patterns
- Use of Response helper methods
- Auto-discovery requirements
137-242: Excellent complete example with production-quality code!The screenshot tool example demonstrates:
- Python side: Clean decorator usage, proper parameter handling, and error response structure
- C# side: Comprehensive Unity API usage including camera setup, RenderTexture manipulation, proper resource cleanup, file I/O with directory creation, and robust error handling
This serves as an excellent reference implementation for custom tool creators.
Note: The C# example uses
Object.FindFirstObjectByType<Camera>()(line 199), which requires Unity 2023.1+. If broader Unity version support is needed, consider documenting this requirement or providing a fallback using the olderFindObjectOfType<Camera>()API.
272-285: LGTM! Practical troubleshooting section.Covers the most common issues developers will encounter:
- Tool registration failures
- Name conflicts
- Communication issues
The verification steps (e.g.,
CommandRegistry.GetAllCommandNames()) are especially helpful for debugging.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
UnityMcpBridge/Editor/Tools/CommandRegistry.cs (1)
22-28: The initialization race condition flagged previously remains unresolved.Although
_initializedis now set afterAutoDiscoverTools()completes, multiple threads can still concurrently enterInitialize(), see_initialized == false, and both executeAutoDiscoverTools(). This can lead to duplicate registrations or dictionary corruption.The fix suggested in the previous review (adding a lock with double-checked locking) has not been applied.
Apply the previously suggested fix:
private static readonly Dictionary<string, Func<JObject, object>> _handlers = new(); private static bool _initialized = false; +private static readonly object _initLock = new(); /// <summary> /// Initialize and auto-discover all tools marked with [McpForUnityTool] /// </summary> public static void Initialize() { if (_initialized) return; - - AutoDiscoverTools(); - _initialized = true; + lock (_initLock) + { + if (_initialized) return; + AutoDiscoverTools(); + _initialized = true; + } }
🧹 Nitpick comments (1)
UnityMcpBridge/Editor/Tools/CommandRegistry.cs (1)
33-41: Consider compiling the regex patterns for better performance.The regex patterns are recompiled on every call to
ToSnakeCase(). While this method is called infrequently (once per tool during initialization), using compiled regex or static instances would improve performance slightly.Apply this diff to use compiled regex:
+private static readonly Regex UpperFollowedByLower = new Regex("(.)([A-Z][a-z]+)", RegexOptions.Compiled); +private static readonly Regex LowerFollowedByUpper = new Regex("([a-z0-9])([A-Z])", RegexOptions.Compiled); + /// <summary> /// Convert PascalCase or camelCase to snake_case /// </summary> private static string ToSnakeCase(string name) { if (string.IsNullOrEmpty(name)) return name; // Insert underscore before uppercase letters (except first) - var s1 = Regex.Replace(name, "(.)([A-Z][a-z]+)", "$1_$2"); - var s2 = Regex.Replace(s1, "([a-z0-9])([A-Z])", "$1_$2"); + var s1 = UpperFollowedByLower.Replace(name, "$1_$2"); + var s2 = LowerFollowedByUpper.Replace(s1, "$1_$2"); return s2.ToLower(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
UnityMcpBridge/Editor/Dependencies/PlatformDetectors/LinuxPlatformDetector.cs(0 hunks)UnityMcpBridge/Editor/Dependencies/PlatformDetectors/MacOSPlatformDetector.cs(0 hunks)UnityMcpBridge/Editor/Dependencies/PlatformDetectors/WindowsPlatformDetector.cs(0 hunks)UnityMcpBridge/Editor/Tools/CommandRegistry.cs(1 hunks)
💤 Files with no reviewable changes (3)
- UnityMcpBridge/Editor/Dependencies/PlatformDetectors/MacOSPlatformDetector.cs
- UnityMcpBridge/Editor/Dependencies/PlatformDetectors/WindowsPlatformDetector.cs
- UnityMcpBridge/Editor/Dependencies/PlatformDetectors/LinuxPlatformDetector.cs
🧰 Additional context used
🧬 Code graph analysis (1)
UnityMcpBridge/Editor/Tools/CommandRegistry.cs (1)
UnityMcpBridge/Editor/Helpers/McpLog.cs (4)
McpLog(6-30)Info(15-19)Error(26-29)Warn(21-24)
🔇 Additional comments (2)
UnityMcpBridge/Editor/Tools/CommandRegistry.cs (2)
46-70: Solid exception handling for assembly scanning.The nested try-catch pattern effectively handles both assembly-level and overall failures during tool discovery. The defensive approach with
!a.IsDynamicfiltering and graceful fallback onGetTypes()failures prevents common reflection pitfalls.
118-127: Improved error handling with explicit exception.Throwing
InvalidOperationExceptioninstead of returning null is a better approach. It makes the error explicit and prevents null reference exceptions downstream, aligning with fail-fast error handling principles.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Closes #298. Essentially users who want to add tools can use a custom decorator in the
toolsfolder and a C# attribute anywhere in their code. It's not as easy as full C# code, but it's much better than what was there beforeSummary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Chores
Breaking Changes