[FEATURE] Batch Commands #418
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.
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
This reverts commit ae8cfe5.
This reverts commit f423c2f.
Merge and use the previous PR's solution to solve the windows issue.
Fix the layout problem of manage_script in the panel
Tested object generation/modification with batch and it works perfectly! We should push and let users test for a while and see PS: I tried both VS Copilot and Claude Desktop. Claude Desktop works but VS Copilot does not due to the nested structure of batch. Will look into it more.
WalkthroughAdds a batch command tool and UI/tooling to discover and enable/disable tools. Introduces BatchExecute (editor and server), per-tool enablement preferences and UI, CommandRegistry.InvokeCommandAsync, and supporting editor window, styles, and transport changes. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant BatchExecuteEditor as BatchExecute (Editor)
participant CommandRegistry
participant ToolHandler as Command
Client->>Server: batch_execute RPC (commands, flags)
Server->>BatchExecuteEditor: send_with_unity_instance("batch_execute", payload)
BatchExecuteEditor->>BatchExecuteEditor: Validate payload (<=25), normalize params
loop per command
BatchExecuteEditor->>CommandRegistry: InvokeCommandAsync(toolName, params)
CommandRegistry->>ToolHandler: Execute (sync/async)
ToolHandler-->>CommandRegistry: result / error
CommandRegistry-->>BatchExecuteEditor: Task result
BatchExecuteEditor->>BatchExecuteEditor: collect per-command result
end
BatchExecuteEditor->>Server: aggregate results (success/failure counts, per-command data)
Server-->>Client: return batch response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
MCPForUnity/Editor/Tools/BatchExecute.cs (1)
173-203: Consider handling edge cases inToCamelCase.The current implementation doesn't explicitly handle edge cases like:
- Consecutive underscores:
"asset__type"→"assetType"(works due toRemoveEmptyEntries)- Leading underscores:
"_asset_type"→"AssetType"(first part is empty, might behave unexpectedly)- Trailing underscores:
"asset_type_"→"assetType"(works due toRemoveEmptyEntries)While most edge cases are handled by
StringSplitOptions.RemoveEmptyEntries, leading underscores could produce unexpected capitalization. Consider adding a comment or explicit handling if this is intentional.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Server/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
MCPForUnity/Editor/Tools/BatchExecute.cs(1 hunks)MCPForUnity/Editor/Tools/BatchExecute.cs.meta(1 hunks)MCPForUnity/Editor/Tools/ManageAsset.cs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
MCPForUnity/Editor/Tools/BatchExecute.cs (3)
MCPForUnity/Editor/Tools/ManageAsset.cs (2)
McpForUnityTool(25-1511)HandleCommand(46-127)MCPForUnity/Editor/Helpers/McpLog.cs (2)
McpLog(7-52)Warn(43-46)MCPForUnity/Editor/Tools/CommandRegistry.cs (1)
CommandRegistry(37-407)
🔇 Additional comments (8)
MCPForUnity/Editor/Tools/BatchExecute.cs (7)
21-24: LGTM: Proper null validation.The null check for
@paramsis appropriate and provides a clear error message.
26-35: LGTM: Batch size validation is properly enforced.The code correctly validates that:
- The
commandsarray exists and is not empty- The batch size does not exceed the maximum of 25 commands
This aligns with the PR objectives and prevents potential resource exhaustion.
121-135: LGTM: Result aggregation and response handling are well-structured.The final response correctly:
- Determines overall success based on whether any command failed
- Includes comprehensive metadata (results, counts, parallel flags)
- Returns appropriate response type (SuccessResponse or ErrorResponse with data)
This provides excellent observability for debugging batch operations.
137-161: LGTM: Parameter normalization logic is well-structured.The recursive normalization correctly handles:
- JObject → normalizes all property keys to camelCase
- JArray → normalizes all elements
- Other tokens → deep clones to preserve immutability
This ensures consistent parameter format across all batched commands.
14-14: VerifyAutoRegister = falseis intentional and manual registration is configured.The
batch_executetool is marked withAutoRegister = false, which means it will not be automatically discovered and registered by theCommandRegistry. Confirm that:
- This is intentional design rather than an oversight
- Manual registration of
batch_executeis properly implemented elsewhere in the codebase
93-93: Verify thatCommandRegistry.InvokeCommandAsyncis defined.Line 93 calls
CommandRegistry.InvokeCommandAsync(toolName, commandParams), but this method is not present in the providedCommandRegistry.cscode snippet. OnlyGetHandler(string commandName)andExecuteCommand(string commandName, JObject @params, TaskCompletionSource<string> tcs)are shown. Confirm the method exists and returns aTask<string>or similar awaitable type, otherwise this will fail to compile.
173-203: Potential compatibility issue withAsSpanon .NET Standard 2.0.Line 198 uses
part.AsSpan(1), which is part of theSystem.MemoryAPI available natively in .NET Standard 2.1+ and .NET Core 2.1+. Unity versions prior to 2021.2 target .NET Standard 2.0, where this API is unavailable in the BCL. This will cause compilation errors unless the project includes theSystem.MemoryNuGet package as a dependency.If the project targets .NET Standard 2.0 without
System.Memory, replace with:- builder.Append(char.ToUpperInvariant(part[0])); - if (part.Length > 1) - { - builder.Append(part.AsSpan(1)); - } + builder.Append(char.ToUpperInvariant(part[0])); + if (part.Length > 1) + { + builder.Append(part.Substring(1)); + }MCPForUnity/Editor/Tools/ManageAsset.cs (1)
166-168: Clarify the necessity of this snake_case fallback.The inline comment indicates this fallback is for "snake_case payloads from batched commands," but
BatchExecute.cs(line 72) already normalizes parameter keys to camelCase viaNormalizeParameterKeysbefore invoking tools. This suggests the fallback might be redundant for batched commands.Is there another code path where
asset_type(snake_case) reachesManageAssetdirectly without normalization? If not, consider removing this fallback to avoid maintaining unnecessary compatibility code.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (14)
MCPForUnity/Editor/Tools/Prefabs/ManagePrefabs.cs (1)
13-15: Doc summary is accurate and helpfulThe XML summary accurately reflects the class responsibilities (managing prefab stages and creating prefabs from GameObjects) and improves discoverability in tooling. Optionally, you could mention the supported actions (
open_stage,close_stage,save_open_stage,create_from_gameobject) for even clearer API docs, but it’s not required.MCPForUnity/Editor/Tools/ManageScript.cs (1)
54-55: Consider adding adescriptiontoMcpForUnityToolFor better MCP client compatibility and tool discovery, consider updating the attribute to include a
descriptionargument (e.g.,description = "Manage C# scripts (create/read/update/delete/apply edits/validate).").
Based on learnings, all Unity MCP tools should includedescriptionin the decorator.CustomTools/RoslynRuntimeCompilation/ManageRuntimeCompilation.cs (1)
126-126: Consider explicit FileAccess for clarity.While
FileMode.Createdefaults to appropriate access flags, explicitly specifyingFileAccess.WriteandFileShare.Nonewould make the intent clearer and more defensive.- using (var stream = new FileStream(dllPath, FileMode.Create)) + using (var stream = new FileStream(dllPath, FileMode.Create, FileAccess.Write, FileShare.None))MCPForUnity/Editor/Tools/ExecuteMenuItem.cs (1)
9-13: Move XML summary above the attribute so it reliably binds to the classTo ensure the XML doc comment is emitted for
ExecuteMenuItem(and picked up by your built‑in tool metadata extractor), it’s safer to place the///summary above the[McpForUnityTool]attribute:/// <summary> /// Tool to execute a Unity Editor menu item by its path. /// </summary> [McpForUnityTool("execute_menu_item", AutoRegister = false)] public static class ExecuteMenuItem { ... }This avoids any ambiguity in how the compiler associates the doc comment.
Server/src/services/tools/batch_execute.py (1)
13-71: Solid validation and payload shaping; consider documenting the cross‑language batch size constantThe
batch_executetool’s validation and normalization look good: you enforce a non‑empty list, cap at 25 commands, ensure each item has a stringtooland dictparams, and shape the payload (commands,parallel,failFast,maxParallelism) to match the Unity BatchExecute expectations. The proxy call via"batch_execute"is consistent with the Unity command registry.Two small, optional nits:
- You now have
MAX_COMMANDS_PER_BATCH = 25here and a matching limit on the Unity side. A short comment referencing the Unity counterpart (or a shared config constant if you ever introduce one) would help avoid drift if the limit changes later.- Ruff’s TRY003/TRY004 suggestions (ValueError vs TypeError, long messages) are stylistic; given this is user input validation, your current choice of
ValueErrorwith descriptive messages is reasonable. Adjust only if you want to strictly satisfy those lints.MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs (1)
424-425: Registering only enabled tools aligns with the new discovery modelSwitching to
_toolDiscoveryService.GetEnabledTools()ensures the bridge only registers tools that are currently enabled, which fits the per‑tool enablement preferences added inToolDiscoveryService. The additional log withtools.Countis also helpful for diagnostics.If there’s any chance
GetEnabledTools()could returnnullin custom implementations, a defensive fallback to an empty list would avoid a possibleNullReferenceException, but with the current in‑repo implementation this looks fine.MCPForUnity/Editor/Windows/Components/Common.uss (1)
296-363: Tools section USS styles are consistent; one minor style-property nitThe new
.tool-*styles (items, tags, descriptions, parameters) and their dark‑theme overrides fit well with the existing layout and color system, and should render cleanly in both themes.One optional consistency tweak: for
.tool-parametersyou currently usefont-style: italic;while the rest of the file uses-unity-font-stylefor font styling. Swapping to-unity-font-style: italic;would align with the rest of the USS and Unity’s recommended property naming, but the current rule is likely to work as-is.Also applies to: 472-485
MCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.cs (1)
46-72: Consider moving inline styles to USS.The
marginRightinline styles (lines 51, 58) could be defined in the USS stylesheet for consistency with the rest of the styling approach. Thetool-action-buttonclass is already added via UXML, so theAddToClassListcalls are redundant but harmless.MCPForUnity/Editor/Services/ToolDiscoveryService.cs (3)
5-7: Script path & summary caches look correct, but consider broader invalidation.The added
_scriptPathCacheand_summaryCacheare reasonable optimizations for editor-only reflection and asset lookups. However,InvalidateCache()currently only nulls_cachedToolsand leaves these dictionaries intact; if you ever callInvalidateCache()without a domain reload (e.g., future “rescan tools” UX), script moves/renames could leave staleAssetPathor summaries. Consider clearing both caches inInvalidateCache()as well.Also applies to: 16-17
318-352: Built‑in detection heuristic is reasonable but couples to assembly name and folder layout.Using either the package root
Editor/Tools/prefix orAssemblyName == "MCPForUnity.Editor"is a pragmatic way to classify built‑ins, but it bakes in a hard‑coded assembly name and directory convention. If you expect future packages or renamed assemblies, consider centralizing these constants (e.g., in a config/constant) to avoid scattering magic strings.
233-236: Consider clearing per‑type/per‑asset caches in InvalidateCache.
InvalidateCache()only nulls_cachedTools. If you ever rely on it to reflect moves/renames or summary changes within the same editor domain,_scriptPathCacheand_summaryCachewill still serve stale entries. Clearing those dictionaries here would make cache invalidation more complete without affecting current behavior.MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs (3)
3-3: Duplicate using directives can be trimmed.You now import several namespaces twice (
Helpers,Services,Settings,Connection,ClientConfig). This is harmless but noisy; consider removing the duplicates to keep the header clean.Also applies to: 11-11, 14-20
30-37: New tools section fields and CloseAllWindows utility integrate cleanly.Adding
McpToolsSectionand tab/panel fields is straightforward, andCloseAllWindows()’s use ofOpenWindows.Where(...).ToArray()avoids collection‑modified‑during‑iteration issues. Note thatCloseAllOpenWindows()already performs a similar job (with extra logging), so there’s some overlap—if both are kept, it might be worth documenting their distinct intended callers.Also applies to: 42-56
195-209: Tools section initial Refresh is appropriate; consider future refresh triggers.Calling
toolsSection.Refresh()right after construction ensures the Tools tab is accurate on first open. If you later add mechanisms to change tools (e.g., dynamic discovery without domain reload), you may want a hook fromRefreshAllData()or a dedicated “Rescan” UI (which McpToolsSection already supports) to re‑sync this list.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
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/Services/IToolDiscoveryService.cs(2 hunks)MCPForUnity/Editor/Services/ToolDiscoveryService.cs(6 hunks)MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs(1 hunks)MCPForUnity/Editor/Tools/CommandRegistry.cs(1 hunks)MCPForUnity/Editor/Tools/ExecuteMenuItem.cs(1 hunks)MCPForUnity/Editor/Tools/ManageScript.cs(2 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(6 hunks)MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.uss(1 hunks)MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.uxml(1 hunks)Server/src/services/tools/batch_execute.py(1 hunks)
💤 Files with no reviewable changes (2)
- CustomTools/RoslynRuntimeCompilation/RoslynRuntime.md
- CustomTools/RoslynRuntimeCompilation/runtime_compilation_tool.py
✅ Files skipped from review due to trivial changes (1)
- MCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.cs.meta
🧰 Additional context used
🧠 Learnings (6)
📚 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.uxmlMCPForUnity/Editor/Constants/EditorPrefKeys.csMCPForUnity/Editor/Tools/ExecuteMenuItem.csMCPForUnity/Editor/Tools/ManageScript.csMCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.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.uxmlMCPForUnity/Editor/Constants/EditorPrefKeys.csMCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.csMCPForUnity/Editor/Windows/MCPForUnityEditorWindow.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/Tools/CommandRegistry.csMCPForUnity/Editor/Constants/EditorPrefKeys.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/Constants/EditorPrefKeys.csMCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.uxml.metaMCPForUnity/Editor/Windows/Components/Tools.metaMCPForUnity/Editor/Services/ToolDiscoveryService.csMCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs
📚 Learning: 2025-09-04T01:01:11.927Z
Learnt from: dsarno
Repo: CoplayDev/unity-mcp PR: 260
File: UnityMcpBridge/UnityMcpServer~/src/server_version.txt:1-1
Timestamp: 2025-09-04T01:01:11.927Z
Learning: The UnityMcpBridge project is not maintaining changelogs yet, so don't suggest adding changelog entries for version bumps.
Applied to files:
MCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.uxml.metaMCPForUnity/Editor/Windows/MCPForUnityEditorWindow.ussMCPForUnity/Editor/Tools/ManageScript.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/Tools/ManageScript.csCustomTools/RoslynRuntimeCompilation/ManageRuntimeCompilation.csMCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.csMCPForUnity/Editor/Services/ToolDiscoveryService.csMCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs
🧬 Code graph analysis (5)
MCPForUnity/Editor/Tools/CommandRegistry.cs (1)
MCPForUnity/Editor/Tools/BatchExecute.cs (2)
Task(19-135)JObject(137-151)
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)
CustomTools/RoslynRuntimeCompilation/ManageRuntimeCompilation.cs (2)
MCPForUnity/Editor/Tools/ExecuteMenuItem.cs (1)
McpForUnityTool(9-52)MCPForUnity/Editor/Tools/Prefabs/ManagePrefabs.cs (1)
McpForUnityTool(12-277)
MCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.cs (5)
MCPForUnity/Editor/Services/IToolDiscoveryService.cs (7)
List(44-44)List(54-54)ToolMetadata(8-22)ToolMetadata(49-49)InvalidateCache(69-69)IsToolEnabled(59-59)SetToolEnabled(64-64)MCPForUnity/Editor/Services/ToolDiscoveryService.cs (8)
List(19-60)List(72-77)List(159-193)ToolMetadata(62-70)ToolMetadata(107-157)InvalidateCache(233-236)IsToolEnabled(79-94)SetToolEnabled(96-105)MCPForUnity/Editor/Services/MCPServiceLocator.cs (1)
MCPServiceLocator(11-85)MCPForUnity/Editor/Constants/EditorPrefKeys.cs (1)
EditorPrefKeys(7-43)MCPForUnity/Editor/Tools/ManageGameObject.cs (1)
Type(2281-2295)
MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs (2)
MCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.cs (4)
McpToolsSection(16-269)McpToolsSection(29-34)VisualElement(146-203)Refresh(77-117)MCPForUnity/Editor/Constants/EditorPrefKeys.cs (1)
EditorPrefKeys(7-43)
🪛 Ruff (0.14.7)
Server/src/services/tools/batch_execute.py
34-34: Avoid specifying long messages outside the exception class
(TRY003)
37-39: Avoid specifying long messages outside the exception class
(TRY003)
44-44: Prefer TypeError exception for invalid type
(TRY004)
44-44: Avoid specifying long messages outside the exception class
(TRY003)
50-50: Avoid specifying long messages outside the exception class
(TRY003)
55-55: Prefer TypeError exception for invalid type
(TRY004)
55-55: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (24)
MCPForUnity/Editor/Tools/ManageScript.cs (1)
27-53: XML summary/remarks split looks goodSeparating the Roslyn installation guide into a
<remarks>block keeps the<summary>focused and improves IntelliSense/readability without affecting behavior.CustomTools/RoslynRuntimeCompilation/ManageRuntimeCompilation.cs (3)
22-26: LGTM: Tool registration improved.The explicit tool name and description enhance discoverability and clarify the tool's purpose.
89-89: Assembly name handling looks correct.The simplified null-coalescing approach is clean and provides a sensible default.
81-81: LGTM: Consistent response type migration.The migration to explicit
ErrorResponse/SuccessResponsetypes improves type safety and aligns with the pattern used in other tools likeExecuteMenuItemandManagePrefabs.Also applies to: 95-95, 145-145, 211-211, 224-224
MCPForUnity/Editor/Services/IToolDiscoveryService.cs (1)
16-22: New metadata fields and enablement API look consistent with ToolDiscoveryService
AssemblyName,AssetPath, andIsBuiltInonToolMetadatamatch howExtractToolMetadatapopulates and uses them, and the newGetEnabledTools/IsToolEnabled/SetToolEnabledmembers line up with the provided implementation snippets and WebSocket registration flow. No functional issues spotted; the additions should support the tools UI and per‑tool enablement cleanly.Be aware this is a breaking change for any external implementers of
IToolDiscoveryService, but within this package the surface looks coherent.Also applies to: 51-64
MCPForUnity/Editor/Tools/CommandRegistry.cs (1)
251-279: Unified async invocation helper is correctly wired for both sync and async handlers
InvokeCommandAsynccleanly abstracts over sync vs async handlers: it reusesGetHandlerInfo, validates the presence of the appropriate delegate, and returns either the async handler task directly orTask.FromResultfor sync results. This matches the existingExecuteCommandbranching and provides a suitable foundation for batch composition (as used byBatchExecute).No changes needed here.
MCPForUnity/Editor/Windows/Components/Tools.meta (1)
2-4: Folder meta update looks fineThe updated GUID and
folderAsset: yeswith a standardDefaultImporterblock are consistent with a normal folder meta. No issues anticipated from this change.MCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.uxml.meta (1)
2-2: UXML meta now targets the built‑in importer as expectedThe GUID update and
ScriptedImporter.scriptpointing to the built‑in UXML importer look correct for a*.uxmlasset. This should integrate cleanly with the new Tools UI component without functional side effects.Also applies to: 10-10
MCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.uxml (1)
1-15: LGTM!The UXML structure is well-organized with clear element naming that aligns with the corresponding C# controller (
McpToolsSection.cs). The layout follows Unity UIElements conventions and provides appropriate semantic structure for the Tools section.MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.uxml (1)
2-14: LGTM!The tabbed layout implementation is clean. The Settings tab defaults to active (
value="true") while the Tools panel is correctly hidden by default (class="hidden"). The structure properly supports panel switching via the C# controller.MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.uss (1)
20-37: LGTM!The new style rules are well-structured. The
.hiddenclass usingdisplay: noneis the correct approach for panel visibility toggling, and the tab toolbar styling withflex-grow: 1on buttons ensures even distribution.MCPForUnity/Editor/Constants/EditorPrefKeys.cs (1)
28-30: LGTM!The new EditorPrefs key constants follow the established naming conventions. The prefix keys correctly end with
.to support dynamic key construction for per-tool preferences.MCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.cs (5)
77-117: LGTM!The
Refreshmethod correctly handles tool discovery, sorting, and UI state management. The sorting logic (built-in first, then alphabetical) provides a consistent user experience, and edge cases (no tools) are properly handled.
119-144: LGTM!The
BuildCategorymethod properly persists foldout state usingEditorPrefs. The early return for empty tool lists is a good optimization.
146-203: LGTM!The
CreateToolRowmethod creates a well-structured UI for each tool with appropriate metadata display. The toggle correctly syncs with the discovery service, and the tags provide useful visual indicators for tool behavior.
205-235: LGTM!The state management is well-implemented. Using
SetValueWithoutNotifyprevents callback loops during bulk operations, and deferring the summary update until after all toggles are processed is efficient.
237-269: LGTM!The helper methods are concise and well-implemented. The null-safe
IsBuiltIncheck and the clear summary text formatting provide good UX.MCPForUnity/Editor/Services/ToolDiscoveryService.cs (4)
72-77: GetEnabledTools behavior is consistent with new IsToolEnabled API.
GetEnabledTools()correctly delegates toDiscoverAllTools()and filters usingIsToolEnabled, so all consumers (e.g., transport registration) will respect the per‑tool EditorPrefs state without duplicating preference logic.
125-150: Extended ToolMetadata population (assembly, asset path, built‑in flag, summary) is coherent.Adding
AssemblyName,AssetPath,IsBuiltIn, and optional summary override gives the UI richer, source‑aware metadata. The flow—populate metadata, computeIsBuiltIn, then optionally overrideDescriptionfrom XML summary—reads clearly and should be easy to extend.
268-316: Asset path resolution is robust for typical editor usage.
ResolveScriptAssetPathhandles null, caches perType, and safely logs & returns null on reflection/asset database issues. TheFindAssetsfilter usingtype.Name t:MonoScriptplus class comparison is a good compromise between correctness and performance for editor code.
354-404: Summary extraction is carefully scoped; minor robustness thoughts.The XML
<summary>regex with a class‑specific pattern plus a fallback generic summary match is a nice touch, and caching byAssetPathprevents repeated parsing. One small robustness improvement would be to short‑circuit ifRegex.Matchthrows for malformed patterns (e.g., future class names or attributes), but given the try/catch and warning log, current behavior is acceptable.MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs (3)
131-155: Panel/container wiring and tools section initialization are robust and fail fast.Querying
settings-panel,tools-panel, and the corresponding*-containerelements and exiting early with clear error logs if they’re missing makes UXML issues easy to diagnose. The tools section is only instantiated when its UXML loads, and you log a warning otherwise, which degrades gracefully. Adding the settings/connection/clientConfig roots intosettings-containermatches the new panel separation.Also applies to: 163-165, 178-180, 191-193, 195-209
267-316: Tab setup correctly guards against both tabs being off and restores saved state.
SetupTabs()un‑hides both panels, wires value‑changed callbacks that prevent both toggles being false, and restores the last active panel fromEditorPrefs.EditorWindowActivePanel. The use ofSetValueWithoutNotifyavoids infinite callback loops when you coerce a toggle back on.
318-336: SwitchPanel cleanly syncs visibility, toggles, and persisted state.The
SwitchPanelmethod centralizes panel visibility (DisplayStyle.Flex/None), uses null‑safe access for panels/toggles, and immediately updatesEditorPrefswith the newActivePanel. This should make tab behavior predictable across sessions.
CustomTools/RoslynRuntimeCompilation/ManageRuntimeCompilation.cs
Outdated
Show resolved
Hide resolved
1 participant
Tested object generation/modification with batch, and it works perfectly! A feature we discussed a long time ago, but finally had an initial tryout at this.
Description: This is a new tool called batch_execute that can aggregate multiple commands together (to a max_number of 25 defined by both the Python server and C# side) and send them at once to achieve a more transparent and efficient (user-side) execution. In the tool description, LLM is explicitly told to work in a batch and sequence way to divide tasks and then process them sequentially.
Here is an example using Claude Desktop on Windows:
Some notes:
1: The current batching is a custom tool that tries to aggregate all the commands together and send them at once. It does not save tokens, and I doubt it will save many tokens even if we divide the batch into each tool (For example, have the manage_gameobject tool accept multiple parameters about multiple objects to do batch). It would definitely be useful when trying to perform redundant tasks, such as generating hundreds of similar pieces, but I also struggle to think of a way to save or optimize the token beyond the current scope.
2: I tried both VS Copilot and Claude Desktop. Claude Desktop works, but VS Copilot does not due to the nested structure of the batch. Will look into it more, but it is more likely to be a VS Copilot limitation based on previous integration issues back at July.
3: About Multi-thread: The original plan is to implement a multi-threaded option, but this seems to be limited to not only read-only tools, but also only a small portion of them (Multi-thread in Unity in my journey has always been a pain:( ). So I discard the changes of multi-thread and let it run on the main thread as of current moment. Happy to discuss about this.
CC'ing @msanatan and @dsarno here, let me know if you have any feedback!
Summary by CodeRabbit
New Features
Improvements
Removals
✏️ Tip: You can customize this high-level summary in your review settings.