Claude Skill Example Upload #380
Conversation
* Update the .Bat file to include runtime folder * Fix the inconsistent EditorPrefs variable so the GUI change on Script Validation could cause real change.
String to Int for consistency
Allows users to generate/compile codes during Playmode
Upload the unity_claude_skill that can be uploaded to Claude for a combo of unity-mcp-skill.
WalkthroughThis pull request updates the README.md documentation to add instructional guidance for Claude Desktop users. It introduces a new step directing users to manually download and upload Unity_Skills via a linked guide and compressed file package. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes This change is purely documentation content addition with no logic, behavior, or code modifications. Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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 |
|
@msanatan Do you think this is a good file to upload in case someone want to use Claude Skills with Claude MCP? .zip is probably the easiest way to install a skill in Claude. |
1. Fix Original Roslyn Compilation Custom Tool to fit the V8 standard 2. Add a new panel in the GUI to see and toggle/untoggle the tools. The toggle feature will be implemented in the future, right now its implemented here to discuss with the team if this is a good feature to add; 3. Add few missing summary in certain tools
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
CustomTools/RoslynRuntimeCompilation/ManageRuntimeCompilation.cs (1)
78-231: Sanitizeassembly_namebefore combining intodllPath
assembly_namecomes directly from@paramsand is interpolated into a filename:
- Line 89:
string assemblyName = @params["assembly_name"]?.ToString() ?? ...;- Line 107:
string dllPath = Path.Combine(DynamicAssembliesPath, $"{assemblyName}.dll");Without sanitization, a malicious or malformed
assembly_namecan:
- Introduce directory separators /
..segments → potential directory traversal relative toApplication.temporaryCachePath.- Contain characters invalid for filenames, which will cause runtime exceptions during
FileStreamcreation (caught, but avoidable).The AI summary also mentions a removed
SanitizeAssemblyFileNamehelper; this change effectively drops that protection. I’d strongly recommend reintroducing sanitization before buildingdllPath.One concrete approach:
- string assemblyName = @params["assembly_name"]?.ToString() ?? $"DynamicAssembly_{DateTime.Now.Ticks}"; + string assemblyName = SanitizeAssemblyName( + @params["assembly_name"]?.ToString() + ) ?? $"DynamicAssembly_{DateTime.Now.Ticks}"; @@ - string dllPath = Path.Combine(DynamicAssembliesPath, $"{assemblyName}.dll"); + string dllPath = Path.Combine(DynamicAssembliesPath, $"{assemblyName}.dll");And add a helper in this class (outside the method):
+ private static string SanitizeAssemblyName(string name) + { + if (string.IsNullOrWhiteSpace(name)) + return null; + + // Strip any directory components to prevent traversal + name = Path.GetFileName(name); + + var invalid = Path.GetInvalidFileNameChars(); + var safe = new string( + name.Select(ch => invalid.Contains(ch) ? '_' : ch).ToArray() + ); + + return string.IsNullOrWhiteSpace(safe) ? null : safe; + }The rest of the refactor in this method (ErrorResponse/SuccessResponse usage, diagnostics packaging, success payload) looks coherent.
🧹 Nitpick comments (11)
MCPForUnity/Editor/Tools/Prefabs/ManagePrefabs.cs (1)
134-134: Parameter name fallback provides flexibility but could be documented.Line 134 accepts both
targetandnameparameters, withtargettaking precedence. This backwards compatibility is reasonable, but consider documenting this behavior in API documentation or a comment if this is intentional support for legacy callers.MCPForUnity/Editor/MenuItems/MCPForUnityMenu.cs (1)
32-38: Toggle now closes only a single MCP window instance
HasOpenInstances+GetWindowis fine for the common single‑instance case, but if this window is ever allowed to be multi‑instance, this will only close one instance where previously you were closing all. If multi‑instance is not desired, this is fine; otherwise consider iterating all open instances or enforcing single‑instance explicitly on the window class.MCPForUnity/Editor/Services/IToolDiscoveryService.cs (1)
16-22: New tool metadata fields and enablement API look coherent, but interface change is breakingThe added
AssemblyName,AssetPath, andIsBuiltInfields line up with the extraction logic inToolDiscoveryService, and the newGetEnabledTools/IsToolEnabled/SetToolEnabledmethods match the per‑tool enablement behavior you’re implementing in both the discovery service and WebSocket client.One thing to be aware of is that extending
IToolDiscoveryServicelike this is a breaking change for any external implementers. If this interface is considered public extension surface, you may want to either:
- Introduce a new derived interface for the enablement API, or
- Provide guidance in release notes that custom implementations must be updated.
If it’s effectively internal to this package, the current change is fine.
Also applies to: 51-64
MCPForUnity/Editor/Windows/Components/Common.uss (1)
472-485: Consider adding light theme overrides for tool styles.Dark theme overrides are provided for
.tool-tag,.tool-item, and description classes. For consistency with other components in this file (which have both light and dark theme overrides), consider adding.unity-theme-lightoverrides for these tool classes if the default styles don't render well in light theme.MCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.cs (1)
46-72: Consider moving inline styles to USS for consistency.The inline margin styles (
style.marginRight = 4) applied here would be better defined in the USS file alongside the.tool-action-buttonclass for maintainability. This keeps styling centralized.private void RegisterCallbacks() { if (enableAllButton != null) { enableAllButton.AddToClassList("tool-action-button"); - enableAllButton.style.marginRight = 4; enableAllButton.clicked += () => SetAllToolsState(true); } if (disableAllButton != null) { disableAllButton.AddToClassList("tool-action-button"); - disableAllButton.style.marginRight = 4; disableAllButton.clicked += () => SetAllToolsState(false); }Then in
Common.uss:.tool-action-button { flex-grow: 1; min-width: 0; height: 26px; margin-bottom: 4px; margin-right: 4px; /* add this */ }MCPForUnity/Editor/Services/ToolDiscoveryService.cs (4)
16-17: Inconsistent indentation on line 17.The
_summaryCachefield declaration is missing proper indentation alignment with the surrounding code.private readonly Dictionary<Type, string> _scriptPathCache = new(); - private readonly Dictionary<string, string> _summaryCache = new(); + private readonly Dictionary<string, string> _summaryCache = new();
139-150: Minor: Remove unreachable blank line.Line 150 (blank line after
return metadata;) is unreachable and appears to be a formatting artifact.if (metadata.IsBuiltIn) { string summaryDescription = ExtractSummaryDescription(type, metadata); if (!string.IsNullOrWhiteSpace(summaryDescription)) { metadata.Description = summaryDescription; } } return metadata; - }
301-306: Minor indentation inconsistency.Line 304 has extra leading whitespace compared to surrounding code.
var scriptClass = script.GetClass(); if (scriptClass == type) { - resolvedPath = assetPath.Replace('\\', '/'); + resolvedPath = assetPath.Replace('\\', '/'); break; }
378-390: Fallback regex may capture wrong summary in multi-class files.The fallback regex at line 383 matches the first
<summary>block in the file. If a file contains multiple classes with XML doc comments, this could return the summary of a different class.Consider logging a warning when the specific class match fails but the generic fallback succeeds, to aid debugging.
CustomTools/RoslynRuntimeCompilation/ManageRuntimeCompilation.cs (2)
41-76: Consistent error handling for missing/unknownactionvaluesThe switch-based dispatch plus
ErrorResponsereturns for missing or unknownactionvalues is clear and consistent, and the messages listing valid actions are helpful. The only minor concern is that the list of valid actions is duplicated in both error messages and the switch; if you add new actions later, consider centralizing the list (e.g., a static string constant) to avoid drift.
222-229: Consider limiting stack traces and raw exception details in serialized responsesSeveral catch blocks now return
ErrorResponsepayloads that includeexceptionand fullstack_trace(Lines 222–229, 390–393, 418–421, 437–440). This is great for debugging but can leak local file paths and implementation details over the MCP boundary.If these responses are exposed to non-local or less-trusted clients, consider:
- Gating stack traces behind a debug/development flag, or
- Returning a short diagnostic ID/message in the serialized data and logging full details only to the Unity console.
This would tighten information exposure while still keeping diagnostics available during development.
Also applies to: 390-393, 418-421, 437-440
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
claude_skill_unity.zipis excluded by!**/*.zip
📒 Files selected for processing (19)
CustomTools/RoslynRuntimeCompilation/ManageRuntimeCompilation.cs(17 hunks)CustomTools/RoslynRuntimeCompilation/RoslynRuntime.md(0 hunks)CustomTools/RoslynRuntimeCompilation/runtime_compilation_tool.py(0 hunks)MCPForUnity/Editor/Constants/EditorPrefKeys.cs(1 hunks)MCPForUnity/Editor/MenuItems/MCPForUnityMenu.cs(1 hunks)MCPForUnity/Editor/Services/IToolDiscoveryService.cs(2 hunks)MCPForUnity/Editor/Services/ToolDiscoveryService.cs(6 hunks)MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs(1 hunks)MCPForUnity/Editor/Tools/ExecuteMenuItem.cs(1 hunks)MCPForUnity/Editor/Tools/Prefabs/ManagePrefabs.cs(1 hunks)MCPForUnity/Editor/Windows/Components/Common.uss(2 hunks)MCPForUnity/Editor/Windows/Components/Tools.meta(1 hunks)MCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.cs(1 hunks)MCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.cs.meta(1 hunks)MCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.uxml(1 hunks)MCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.uxml.meta(1 hunks)MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs(7 hunks)MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.uss(1 hunks)MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.uxml(1 hunks)
💤 Files with no reviewable changes (2)
- CustomTools/RoslynRuntimeCompilation/RoslynRuntime.md
- CustomTools/RoslynRuntimeCompilation/runtime_compilation_tool.py
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: JohanHoltby
Repo: CoplayDev/unity-mcp PR: 309
File: MCPForUnity/Editor/Helpers/ServerInstaller.cs:478-508
Timestamp: 2025-10-13T13:41:00.086Z
Learning: In the MCPForUnityTools feature (MCPForUnity/Editor/Helpers/ServerInstaller.cs), the design intentionally forces users to have only one .py file per MCPForUnityTools folder to keep file tracking simple. Package-style tools (subdirectories with __init__.py) are not supported.
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 316
File: TestProjects/UnityMCPTests/Assets/Tests/EditMode/Resources.meta:1-8
Timestamp: 2025-10-13T13:27:23.040Z
Learning: UnityMcpBridge is a legacy project kept for backwards compatibility; MCPForUnity is the only active Unity plugin project. GUID collisions between UnityMcpBridge and MCPForUnity are acceptable.
📚 Learning: 2025-10-03T22:11:46.002Z
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 301
File: docs/CUSTOM_TOOLS.md:54-62
Timestamp: 2025-10-03T22:11:46.002Z
Learning: In Unity MCP, the `description` parameter in the `mcp_for_unity_tool` decorator is technically optional but should always be included as a best practice. Without it, there's a higher chance that MCP clients will not parse the tool correctly. All Unity MCP tools should include the description in the decorator for compatibility.
Applied to files:
MCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.cs.metaMCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.csMCPForUnity/Editor/Windows/Components/Common.ussMCPForUnity/Editor/Tools/ExecuteMenuItem.csMCPForUnity/Editor/Constants/EditorPrefKeys.csCustomTools/RoslynRuntimeCompilation/ManageRuntimeCompilation.cs
📚 Learning: 2025-10-13T13:27:23.040Z
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 316
File: TestProjects/UnityMCPTests/Assets/Tests/EditMode/Resources.meta:1-8
Timestamp: 2025-10-13T13:27:23.040Z
Learning: UnityMcpBridge is a legacy project kept for backwards compatibility; MCPForUnity is the only active Unity plugin project. GUID collisions between UnityMcpBridge and MCPForUnity are acceptable.
Applied to files:
MCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.cs.metaMCPForUnity/Editor/Windows/Components/Tools.metaMCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.uxml.metaMCPForUnity/Editor/Windows/MCPForUnityEditorWindow.csMCPForUnity/Editor/Services/ToolDiscoveryService.csMCPForUnity/Editor/Constants/EditorPrefKeys.cs
📚 Learning: 2025-10-13T13:41:00.086Z
Learnt from: JohanHoltby
Repo: CoplayDev/unity-mcp PR: 309
File: MCPForUnity/Editor/Helpers/ServerInstaller.cs:478-508
Timestamp: 2025-10-13T13:41:00.086Z
Learning: In the MCPForUnityTools feature (MCPForUnity/Editor/Helpers/ServerInstaller.cs), the design intentionally forces users to have only one .py file per MCPForUnityTools folder to keep file tracking simple. Package-style tools (subdirectories with __init__.py) are not supported.
Applied to files:
MCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.cs.metaMCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.csMCPForUnity/Editor/Windows/MCPForUnityEditorWindow.csMCPForUnity/Editor/Constants/EditorPrefKeys.cs
📚 Learning: 2025-09-03T16:00:55.839Z
Learnt from: dsarno
Repo: CoplayDev/unity-mcp PR: 0
File: :0-0
Timestamp: 2025-09-03T16:00:55.839Z
Learning: ComponentResolver in UnityMcpBridge/Editor/Tools/ManageGameObject.cs is a nested static class within ManageGameObject, not a sibling type. The `using static MCPForUnity.Editor.Tools.ManageGameObject;` import is required to access ComponentResolver methods directly without the outer class qualifier.
Applied to files:
MCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.cs.metaMCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.csMCPForUnity/Editor/Windows/MCPForUnityEditorWindow.csMCPForUnity/Editor/Services/ToolDiscoveryService.csCustomTools/RoslynRuntimeCompilation/ManageRuntimeCompilation.cs
📚 Learning: 2025-11-05T18:23:12.349Z
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 368
File: MCPForUnity/UnityMcpServer~/src/resources/menu_items.py:15-15
Timestamp: 2025-11-05T18:23:12.349Z
Learning: In Unity MCP, the `name` parameter in the `mcp_for_unity_resource` decorator is the external API name exposed to MCP clients (LLMs, AI agents). The command string passed to `async_send_command_with_retry` or `async_send_with_unity_instance` (e.g., "get_menu_items") is the internal command identifier that must match the C# side. These are decoupled, allowing external API naming to evolve independently of internal command routing.
Applied to files:
MCPForUnity/Editor/Constants/EditorPrefKeys.cs
🧬 Code graph analysis (4)
MCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.cs (3)
MCPForUnity/Editor/Services/ToolDiscoveryService.cs (3)
ToolMetadata(62-70)ToolMetadata(107-157)InvalidateCache(233-236)MCPForUnity/Editor/Services/MCPServiceLocator.cs (1)
MCPServiceLocator(11-85)MCPForUnity/Editor/Constants/EditorPrefKeys.cs (1)
EditorPrefKeys(7-43)
MCPForUnity/Editor/Services/IToolDiscoveryService.cs (2)
MCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.cs (1)
IsBuiltIn(268-268)MCPForUnity/Editor/Services/ToolDiscoveryService.cs (4)
ToolMetadata(62-70)ToolMetadata(107-157)IsToolEnabled(79-94)SetToolEnabled(96-105)
MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs (1)
MCPForUnity/Editor/Helpers/McpLog.cs (2)
McpLog(7-52)Info(37-41)
MCPForUnity/Editor/Services/ToolDiscoveryService.cs (4)
MCPForUnity/Editor/Tools/ManageGameObject.cs (1)
Type(2281-2295)MCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.cs (1)
IsBuiltIn(268-268)MCPForUnity/Editor/Constants/EditorPrefKeys.cs (1)
EditorPrefKeys(7-43)MCPForUnity/Editor/Helpers/AssetPathUtility.cs (2)
AssetPathUtility(15-201)GetMcpPackageRootPath(41-80)
🔇 Additional comments (27)
MCPForUnity/Editor/Tools/Prefabs/ManagePrefabs.cs (2)
13-15: Well-documented class purpose.The XML documentation comment clearly describes the class responsibility and is appropriately placed. This improves discoverability for developers using IntelliSense.
20-54: Sound command routing and error handling.The HandleCommand method properly validates inputs, routes to appropriate action handlers, and wraps logic in comprehensive exception handling. Parameter validation is thorough and error messages are descriptive.
MCPForUnity/Editor/Tools/ExecuteMenuItem.cs (1)
10-12: Good addition of XML documentation.The summary clearly describes the tool's purpose and complements the attribute metadata. This enhances discoverability and maintainability.
MCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.uxml.meta (1)
2-2: McpToolsSection UXML meta looks consistentThe new GUID and ScriptedImporter reference look consistent with a standard UXML asset; no issues from this diff alone.
Also applies to: 10-10
MCPForUnity/Editor/Constants/EditorPrefKeys.cs (1)
28-30: New EditorPrefs keys are well‑namespaced and consistentThe per‑tool prefixes and active‑panel key are clearly named and scoped under the existing
MCPForUnity.*namespace, which should make future migrations and debugging easier. No issues here.MCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.cs.meta (1)
1-11: McpToolsSection script meta is standardMonoImporter settings (executionOrder 0, empty defaultReferences, etc.) look like a normal script meta file; no concerns from this diff.
MCPForUnity/Editor/Windows/Components/Tools.meta (1)
2-3: Tools folder meta change looks correctMarking this as
folderAsset: yeswith aDefaultImporteris appropriate for a folder; the GUID change is fine as long as asset references were updated by Unity (which it typically handles).MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.uxml (1)
2-14: Tabbed layout and panel structure look good—ensure name sync with code-behindThe new root
VisualElementplus tab toolbar and separatesettings-panel/tools-panelscroll views provide a clean structure for multi‑panel UI, and the containers (settings-container,tools-container) are well named.Just double‑check that
MCPForUnityEditorWindow.cs(and any tooling UI initializers) query these elements by the same names and expected types (ScrollViewvsVisualElement), since the root element type and hierarchy changed.MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs (1)
420-466: Remove this review comment—it references code that does not exist in the repository.The review discusses
_toolDiscoveryService.GetEnabledTools()and specific logging, but the actual code at lines 420–466 callsDiscoverAllTools()instead. TheGetEnabledTools()method does not exist inIToolDiscoveryService, and the log messages differ from what the review claims. Additionally,DiscoverAllTools()is guaranteed to return a non-nullList<ToolMetadata>, so the null-safety concern is invalid for the actual codebase.Likely an incorrect or invalid review comment.
MCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.uxml (1)
1-15: Well-structured UXML layout for the Tools section.The element naming conventions are consistent with the C# controller expectations in
McpToolsSection.cs, and the CSS class assignments align with the styles defined inCommon.uss. The hierarchical structure cleanly separates the header, actions, and dynamic content areas.MCPForUnity/Editor/Windows/Components/Common.uss (1)
296-364: LGTM - Clean and consistent tool styling.The new tool section styles follow the existing patterns in this file and provide good visual hierarchy for tool items, tags, and descriptions.
MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.uss (1)
1-41: LGTM - Proper layout updates for tabbed interface.The root container updates (
flex-direction: column,flex-grow: 1) and new tab/panel styles correctly support the multi-panel tabbed interface. The.hiddenutility class follows standard patterns.MCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.cs (3)
74-117: LGTM - Well-structured refresh logic.The refresh flow properly clears state, re-discovers tools, categorizes them (built-in vs custom), and updates the UI accordingly. The defensive checks for null containers and the early return for empty tool lists are good practices.
146-203: LGTM - Comprehensive tool row creation.The tool row UI correctly displays the toggle, tags (default state, output type, polling), description, and parameter summary. The tooltip on the toggle provides useful context.
215-235: LGTM - Efficient bulk state change implementation.The
SetAllToolsStatemethod correctly handles both cases: tools with UI toggles (usingSetValueWithoutNotifyto avoid recursive callbacks) and tools without toggles in the map. DeferringUpdateSummaryuntil after the loop is a good optimization.MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs (5)
34-47: LGTM - Clean enum definition and safe window iteration.The
ActivePanelenum provides type-safe tab state, andCloseAllWindowscorrectly creates a copy viaToArray()before iterating to avoid collection modification issues.
89-111: LGTM - Good validation with clear error messages.The null checks for panel and container elements with descriptive error logging will help diagnose UXML loading issues.
150-164: LGTM - Proper Tools section initialization with graceful fallback.The Tools section loads the UXML, instantiates the controller, and calls
Refresh(). The warning log on failure provides visibility without breaking the window.
212-261: LGTM - Robust tab switching with persistence.The tab toggle callbacks correctly enforce mutual exclusivity (at least one tab must be selected), and the panel state is persisted via
EditorPrefs. TheSetValueWithoutNotifycalls prevent recursive callback invocations.
176-180: LGTM - Defensive cleanup in OnDisable.Using
RemoveWherewith a predicate that checks for both null and the current instance is a good defensive pattern that cleans up any stale references.MCPForUnity/Editor/Services/ToolDiscoveryService.cs (5)
72-77: LGTM!Clean implementation that leverages the existing caching in
DiscoverAllTools().
79-94: LGTM!The fallback to
AutoRegisterwhen no preference exists is sensible default behavior.
96-105: LGTM!Simple and correct preference setter with proper null guard.
318-352: LGTM!Solid two-tier detection: path-based check first (more precise), with assembly name fallback. The path normalization handles cross-platform differences correctly.
253-260: The review references non-existent code. Lines 253-260 and the methodEnsurePreferenceInitialized()do not exist inToolDiscoveryService.cs(which contains only 184 lines). The code snippet aboutmetadata.IsBuiltIn && !metadata.AutoRegisterand EditorPrefs logic is not present in the current file. This review comment does not apply to the current state of the codebase.Likely an incorrect or invalid review comment.
CustomTools/RoslynRuntimeCompilation/ManageRuntimeCompilation.cs (2)
20-27: Tool attribute description addition looks correct and matches guidanceThe expanded XML summary and the
[McpForUnityTool(name:"runtime_compilation", Description = "...")]attribute give this tool a clear, discoverable description for MCP clients, which aligns with the earlier guidance to always include descriptions for tools. Based on learnings, this is a good change.
233-249: SuccessResponse/ErrorResponse refactor across query/history methods looks consistentThe move to
SuccessResponse/ErrorResponsein:
ListLoadedAssemblies(Lines 233–249),GetAssemblyTypes(Lines 251–282),ExecuteWithRoslyn(Lines 288–359),GetCompilationHistory(Lines 365–393),SaveCompilationHistory(Lines 399–421),ClearCompilationHistory(Lines 427–440),is applied consistently and yields a clear
{ message, data }-style pattern in each case. Payload shapes (counts, lists, diagnostics, etc.) look internally consistent and should be easy for clients to consume. I don’t see functional regressions here.Also applies to: 251-282, 288-359, 365-393, 399-421, 427-440
|
|
||
| namespace MCPForUnity.Editor.Tools | ||
| { | ||
| [McpForUnityTool("execute_menu_item", AutoRegister = false)] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n MCPForUnity/Editor/Tools/ExecuteMenuItem.cs | head -30Repository: CoplayDev/unity-mcp
Length of output: 1424
🏁 Script executed:
rg "\[McpForUnityTool\(" -A 2 -B 0 | head -50Repository: CoplayDev/unity-mcp
Length of output: 2840
🏁 Script executed:
fd -t f "ExecuteMenuItem.cs" --exec head -20 {}Repository: CoplayDev/unity-mcp
Length of output: 910
🏁 Script executed:
rg "McpForUnityTool" MCPForUnity/Editor/Tools/ -A 1 | grep -E "(Description|McpForUnityTool)" | head -20Repository: CoplayDev/unity-mcp
Length of output: 2114
🏁 Script executed:
cat -n docs/CUSTOM_TOOLS.md | sed -n '50,70p'Repository: CoplayDev/unity-mcp
Length of output: 846
Add description parameter to the [McpForUnityTool] decorator.
The decorator is missing the description parameter. Per best practices in Unity MCP, the description parameter should always be included to ensure MCP clients parse the tool correctly and maintain compatibility. Add a clear description of the tool's functionality:
- [McpForUnityTool("execute_menu_item", AutoRegister = false)]
+ [McpForUnityTool("execute_menu_item", Description = "Executes a Unity Editor menu item by its path.", AutoRegister = false)]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| [McpForUnityTool("execute_menu_item", AutoRegister = false)] | |
| [McpForUnityTool("execute_menu_item", Description = "Executes a Unity Editor menu item by its path.", AutoRegister = false)] |
🤖 Prompt for AI Agents
In MCPForUnity/Editor/Tools/ExecuteMenuItem.cs around line 9, the
McpForUnityTool attribute is missing the required description parameter; update
the attribute to include a descriptive description string (for example:
"Executes a Unity Editor menu item by name or path; used by MCP to trigger
editor commands") so MCP clients can parse the tool correctly and maintain
compatibility.
This reverts commit ae8cfe5.
|
Going to merge it right now for Claude users who are interested into trying this. Also update the Readme to direct claude desktop users to try this. |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-27T21:09:34.975Z
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 401
File: MCPForUnity/Editor/Clients/Configurators/VSCodeConfigurator.cs:10-18
Timestamp: 2025-11-27T21:09:34.975Z
Learning: VS Code GitHub Copilot MCP configuration supports mcp.json files placed directly in the Code/User directory: %APPDATA%\Code\User\mcp.json on Windows, ~/Library/Application Support/Code/User/mcp.json on macOS, and ~/.config/Code/User/mcp.json on Linux. This is in addition to workspace-scoped .vscode/mcp.json files.
Applied to files:
README.md
🪛 markdownlint-cli2 (0.18.1)
README.md
188-188: Link text should be descriptive
(MD059, descriptive-link-text)
| ### 🛠️ Step 3: Configure Your MCP Client | ||
| Connect your MCP Client (Claude, Cursor, etc.) to the HTTP server from Step 2 (auto) or via Manual Configuration (below). | ||
| For **Claude Desktop** Users, try using our manually scrapped Unity_Skills by downloading and uploading the claude_skill_unity.zip following this [link](https://www.claude.com/blog/skills). |
There was a problem hiding this comment.
Fix typo and use descriptive link text.
Three issues on this line:
- Typo: "scrapped" should be "scraped" (meaning extracted/gathered from documentation).
- Descriptive link text (MD059): The link text "[link]" is not descriptive. Consider something like "Claude Skills guide" or "upload instructions".
- Link destination: The current link points to the blog announcement, but users upload skills by clicking "Upload skill" in Settings > Capabilities. You may want to link to the official help article instead:
https://support.claude.com/en/articles/12512180-using-skills-in-claude.
Suggested fix:
-For **Claude Desktop** Users, try using our manually scrapped Unity_Skills by downloading and uploading the claude_skill_unity.zip following this [link](https://www.claude.com/blog/skills).
+For **Claude Desktop** Users, try using our manually scraped Unity_Skills by downloading and uploading the claude_skill_unity.zip following this [upload guide](https://support.claude.com/en/articles/12512180-using-skills-in-claude).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| For **Claude Desktop** Users, try using our manually scrapped Unity_Skills by downloading and uploading the claude_skill_unity.zip following this [link](https://www.claude.com/blog/skills). | |
| For **Claude Desktop** Users, try using our manually scraped Unity_Skills by downloading and uploading the claude_skill_unity.zip following this [upload guide](https://support.claude.com/en/articles/12512180-using-skills-in-claude). |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
188-188: Link text should be descriptive
(MD059, descriptive-link-text)
🤖 Prompt for AI Agents
In README.md around line 188, fix the typo "scrapped" -> "scraped", replace the
non-descriptive link text "[link]" with descriptive text like "[Claude Skills
guide]" or "[upload instructions]", and update the URL to the official help
article https://support.claude.com/en/articles/12512180-using-skills-in-claude;
also adjust the sentence to instruct users to upload via Settings > Capabilities
> Upload skill so the guidance and link are accurate.
1 participant
Use https://github.com/yusufkaraaslan/Skill_Seekers to create a detailed Claude Skill that can assist Unity-MCP with a comprehensive scraped doc. It consists of only .md files that are scraped from the documentation to help with generation/edit.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.