Add manage_editor action for script compilation #104
Conversation
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on January 7. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
WalkthroughA new "request_script_compilation" action is added to both the C# editor tool and Python service layer. The C# implementation captures compilation state, triggers script compilation via Unity's API, and returns the compilation status. The Python service updates its action parameter type to recognize this new action. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
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 |
Greptile SummaryAdded
The implementation follows existing patterns in the codebase for adding new actions and properly integrates with the transport layer. Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant Python as manage_editor.py
participant Unity as ManageEditor.cs
participant CP as CompilationPipeline
Client->>Python: manage_editor(action="request_script_compilation")
Python->>Python: validate parameters & get Unity instance
Python->>Unity: send_with_unity_instance("manage_editor", params)
Unity->>Unity: HandleCommand(params)
Unity->>CP: check isCompiling (wasCompiling)
CP-->>Unity: current state
Unity->>CP: RequestScriptCompilation()
CP-->>Unity: request sent
Unity->>CP: check isCompiling (nowCompiling)
CP-->>Unity: updated state
Unity->>Unity: build SuccessResponse with wasCompiling, nowCompiling, note
Unity-->>Python: response {success, message, data}
Python->>Python: unwrap response structure
Python-->>Client: {success: true, message, data: {wasCompiling, nowCompiling, note}}
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| bool wasCompiling = CompilationPipeline.isCompiling; | ||
| CompilationPipeline.RequestScriptCompilation(); | ||
| bool nowCompiling = CompilationPipeline.isCompiling; |
There was a problem hiding this comment.
style: Checking CompilationPipeline.isCompiling immediately after RequestScriptCompilation() may not accurately reflect the compilation state due to async nature. Consider delaying the check or using EditorApplication.update callback.
Prompt To Fix With AI
This is a comment left during a code review.
Path: MCPForUnity/Editor/Tools/ManageEditor.cs
Line: 90:92
Comment:
**style:** Checking `CompilationPipeline.isCompiling` immediately after `RequestScriptCompilation()` may not accurately reflect the compilation state due to async nature. Consider delaying the check or using EditorApplication.update callback.
How can I resolve this? If you propose a fix, please make it concise.![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
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
MCPForUnity/Editor/Tools/ManageEditor.csServer/src/services/tools/manage_editor.py
🧰 Additional context used
🧬 Code graph analysis (1)
MCPForUnity/Editor/Tools/ManageEditor.cs (1)
MCPForUnity/Editor/Helpers/Response.cs (4)
SuccessResponse(11-33)SuccessResponse(28-32)ErrorResponse(35-67)ErrorResponse(61-66)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Greptile Review
🔇 Additional comments (3)
Server/src/services/tools/manage_editor.py (1)
17-17: LGTM!The addition of "request_script_compilation" to the action Literal type is straightforward and correctly exposes the new Unity backend capability to the Python service layer.
MCPForUnity/Editor/Tools/ManageEditor.cs (2)
5-5: LGTM!The
UnityEditor.Compilationnamespace import is necessary for accessing theCompilationPipelineAPI used in the new action handler.
148-148: LGTM!The error message correctly includes the new "request_script_compilation" action in the list of supported actions, maintaining consistency with the codebase.
| case "request_script_compilation": | ||
| try | ||
| { | ||
| bool wasCompiling = CompilationPipeline.isCompiling; | ||
| CompilationPipeline.RequestScriptCompilation(); | ||
| bool nowCompiling = CompilationPipeline.isCompiling; | ||
| string detail = nowCompiling | ||
| ? "Unity reports compilation in progress." | ||
| : "Unity is not currently compiling; if no pending script changes exist, the request may be a no-op."; | ||
| return new SuccessResponse( | ||
| "Script compilation request sent.", | ||
| new | ||
| { | ||
| wasCompiling, | ||
| nowCompiling, | ||
| note = detail | ||
| } | ||
| ); | ||
| } | ||
| catch (Exception e) | ||
| { | ||
| return new ErrorResponse($"Error requesting script compilation: {e.Message}"); | ||
| } |
There was a problem hiding this comment.
RequestScriptCompilation timing may not reflect actual compilation state.
The nowCompiling check on line 92 occurs immediately after calling RequestScriptCompilation(). Since this operation runs asynchronously, the state may not immediately reflect whether compilation will begin. The current note acknowledges the no-op case but doesn't document this async behavior.
For more reliable state tracking, consider using the CompilationPipeline.compilationStarted event instead of polling isCompiling immediately after the request.
🤖 Prompt for AI Agents
In MCPForUnity/Editor/Tools/ManageEditor.cs around lines 87 to 109, the code
reads CompilationPipeline.isCompiling immediately after
RequestScriptCompilation(), which can be misleading because compilation starts
asynchronously; replace the immediate check with a reliable approach by
subscribing to CompilationPipeline.compilationStarted (and optionally
compilationFinished) to detect when compilation actually begins, update the
success response to either return immediately with a clear note that compilation
is asynchronous or defer returning until compilationStarted fires (or provide a
follow-up callback/notification), and remove the immediate nowCompiling polling
logic so the response accurately reflects true compilation state.
Summary
Testing
Codex Task
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.