Skip to content

Conversation

@msanatan
Copy link
Member

@msanatan msanatan commented Jan 8, 2026

…d fail-fast support

  • Add DetermineCallSucceeded() to check command success via IMcpResponse interface or JObject/JToken 'success' field
  • Track invocationFailureCount and set anyCommandFailed flag when commands fail
  • Implement fail-fast behavior to stop batch execution on first failure when failFast=true
  • Update commandResults to use computed callSucceeded value instead of hardcoded true

feat(game_object_create): enhance asset

Summary by Sourcery

Improve batch command execution error handling and enhance game object creation prefab/asset resolution.

New Features:

  • Detect individual command success or failure in batch execution based on IMcpResponse or JSON 'success' field.
  • Support fail-fast behavior in batch execution to stop processing on the first failed command.
  • Allow GameObject creation to resolve non-prefab asset files by name and handle more flexible prefab path inputs.

Bug Fixes:

  • Prevent misreporting of batch command invocations as successful when underlying commands fail.
  • Avoid attempting to instantiate missing or non-GameObject assets by returning explicit error responses when assets cannot be loaded or are not GameObjects.

Enhancements:

  • Track separate success and failure counts and expose per-command success flags in batch execution results.
  • Refine prefab path handling to distinguish between paths, extensions, and file names for more robust asset lookup in GameObject creation.

Summary by CodeRabbit

  • Bug Fixes
    • Improved command success/failure detection in batch execution with accurate result tracking
    • Enhanced fail-fast behavior to properly exit batch operations on command failures
    • Expanded prefab path resolution to support flexible formats (names, file paths, extensions)
    • Strengthened error handling with clearer messaging when asset resolution fails

✏️ Tip: You can customize this high-level summary in your review settings.

…d fail-fast support

- Add DetermineCallSucceeded() to check command success via IMcpResponse interface or JObject/JToken 'success' field
- Track invocationFailureCount and set anyCommandFailed flag when commands fail
- Implement fail-fast behavior to stop batch execution on first failure when failFast=true
- Update commandResults to use computed callSucceeded value instead of hardcoded true

feat(game_object_create): enhance asset
@msanatan msanatan self-assigned this Jan 8, 2026
@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented Jan 8, 2026

Reviewer's Guide

Refines batch command execution success detection and fail-fast behavior while enhancing GameObject creation to better resolve prefab/asset paths and surface clearer error conditions.

Sequence diagram for batch command execution with improved success detection and fail-fast

sequenceDiagram
    actor Caller
    participant BatchExecute
    participant CommandRegistry
    participant ToolCommand

    Caller->>BatchExecute: HandleCommand(params)
    loop For each toolName in commands
        BatchExecute->>CommandRegistry: InvokeCommandAsync(toolName, commandParams)
        CommandRegistry->>ToolCommand: Execute(commandParams)
        ToolCommand-->>CommandRegistry: result
        CommandRegistry-->>BatchExecute: result

        BatchExecute->>BatchExecute: DetermineCallSucceeded(result)
        alt callSucceeded
            BatchExecute->>BatchExecute: increment invocationSuccessCount
        else callFailed
            BatchExecute->>BatchExecute: increment invocationFailureCount
            BatchExecute->>BatchExecute: set anyCommandFailed = true
            alt failFast is true
                BatchExecute->>BatchExecute: break command loop
            end
        end

        BatchExecute->>BatchExecute: commandResults.Add({ tool, callSucceeded, result })
    end

    alt anyCommandFailed
        BatchExecute-->>Caller: ErrorResponse("One or more commands failed", data)
    else allSucceeded
        BatchExecute-->>Caller: SuccessResponse(data)
    end
Loading

Class diagram for updated batch execution and GameObject creation types

classDiagram
    class BatchExecute {
        +static Task<object> HandleCommand(JObject params)
        -static bool DetermineCallSucceeded(object result)
        -int invocationSuccessCount
        -int invocationFailureCount
        -bool anyCommandFailed
        -bool failFast
    }

    class IMcpResponse {
        <<interface>>
        +bool Success
    }

    class ErrorResponse {
        +ErrorResponse(string message)
        +ErrorResponse(string message, object data)
        +bool Success
        +string Message
        +object Data
    }

    class GameObjectCreate {
        +static object Handle(JObject params)
        -string prefabPath
        -bool saveAsPrefab
        -GameObject prefabAsset
    }

    class AssetDatabase {
        <<static>>
        +static string[] FindAssets(string filter)
        +static string GUIDToAssetPath(string guid)
        +static T LoadAssetAtPath~T~(string path)
    }

    class McpLog {
        <<static>>
        +static void Info(string message)
        +static void Warn(string message)
    }

    class JObject
    class JToken
    class GameObject

    BatchExecute ..> IMcpResponse : uses via DetermineCallSucceeded
    BatchExecute ..> ErrorResponse : returns on failure
    ErrorResponse ..|> IMcpResponse

    GameObjectCreate ..> AssetDatabase : resolves prefabPath
    GameObjectCreate ..> McpLog : logs resolution steps
    GameObjectCreate ..> ErrorResponse : returns on invalid asset
    GameObjectCreate ..> GameObject : instantiates prefab

    BatchExecute ..> JObject : HandleCommand params
    BatchExecute ..> JToken : DetermineCallSucceeded(result)
    GameObjectCreate ..> JObject : Handle params
Loading

Flow diagram for enhanced prefab and asset path resolution in GameObjectCreate

flowchart TD
    A["Start Handle(params)"] --> B{"saveAsPrefab is false?"}
    B -- no --> Z["Skip prefab/asset lookup"]
    B -- yes --> C{"prefabPath is null or empty?"}
    C -- yes --> Z
    C -- no --> D["Get extension = Path.GetExtension(prefabPath)"]

    D --> E{"prefabPath contains '/'?"}

    E -- no --> F{"extension is null or empty OR extension == .prefab?"}
    F -- yes --> G["Search for prefab by name (AssetDatabase.FindAssets)"]
    G --> H{"zero matches?"}
    H -- yes --> I["Warn and keep prefabPath as original"]
    H -- no --> J{"multiple matches?"}
    J -- yes --> K["Warn and keep prefabPath as original"]
    J -- no --> L["Set prefabPath to unique prefab path"]
    F -- no --> M{"extension != .prefab"}
    M -- yes --> N["Search for asset file by file name"]
    N --> O{"zero matches?"}
    O -- yes --> P["Return ErrorResponse(&quot;Asset file not found&quot;)"]
    O -- no --> Q{"multiple matches?"}
    Q -- yes --> R["Return ErrorResponse(&quot;Multiple assets found&quot;)"]
    Q -- no --> S["Set prefabPath to unique asset path"]

    E -- yes --> T{"extension is null or empty?"}
    T -- yes --> U["Warn and append .prefab to prefabPath"]
    T -- no --> V["Leave prefabPath unchanged"]

    I --> W
    K --> W
    L --> W
    S --> W
    U --> W
    V --> W

    W["LoadAssetAtPath<GameObject>(prefabPath)"] --> X{"prefabAsset is not null?"}
    X -- yes --> Y["Instantiate prefab and continue"]
    X -- no --> AA["Return ErrorResponse(&quot;Asset not found or not a GameObject at path&quot;)"]

    Z --> End["Continue with non-prefab creation path"]
    Y --> End
    P --> End
    R --> End
    AA --> End
Loading

File-Level Changes

Change Details Files
Track per-command success/failure and support fail-fast in batch execution.
  • Introduce DetermineCallSucceeded helper to infer success from IMcpResponse.Success or a JObject/JToken 'success' boolean field, defaulting to true when inconclusive.
  • Replace unconditional invocationSuccessCount increment with conditional success/failure counters and an anyCommandFailed flag.
  • Populate commandResults.callSucceeded with the computed success value instead of a hardcoded true.
  • Add fail-fast behavior to break out of the command loop when a command fails and failFast is true.
MCPForUnity/Editor/Tools/BatchExecute.cs
Improve GameObjectCreate prefab/asset path handling and error reporting.
  • Skip prefab instantiation path resolution when saveAsPrefab is true.
  • Refine extension handling so bare names or '.prefab' extensions are treated as prefabs, and paths with no extension but containing '/' get '.prefab' appended with a more accurate log message.
  • Add support for non-prefab asset filenames with extensions by searching the AssetDatabase for matching files and resolving to a unique asset path or returning clear error responses on zero/multiple matches.
  • Change behavior when the resolved prefabPath does not load a GameObject to return an ErrorResponse instead of logging a warning and proceeding.
MCPForUnity/Editor/Tools/GameObjects/GameObjectCreate.cs

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@msanatan msanatan merged commit a17df1e into CoplayDev:main Jan 8, 2026
0 of 2 checks passed
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 8, 2026

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Two changes improve command execution reliability and prefab resolution: BatchExecute adds a helper to dynamically detect command success from varied result shapes and conditionally exit early when failFast is enabled, while GameObjectCreate adds extension handling and AssetDatabase name-based prefab resolution with immediate error responses for missing assets.

Changes

Cohort / File(s) Summary
Command Execution Success Detection
MCPForUnity/Editor/Tools/BatchExecute.cs
Adds DetermineCallSucceeded() helper that introspects result objects (IMcpResponse, JObject, JToken) to infer command success; replaces unconditional success assumption; integrates success detection into loop control (failFast exit), invocation counters, and result payload.
Prefab Path Resolution & Error Handling
MCPForUnity/Editor/Tools/GameObjects/GameObjectCreate.cs
Introduces extension and path detection logic to resolve prefab references by name via AssetDatabase when paths lack directory separators; handles various extension cases (.prefab, none, non-prefab); changes missing asset handling from warning to immediate ErrorResponse, causing early termination on resolution failure.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 Command success now takes many shapes,
No blind assumptions help us escape!
Prefabs find their names with cheer,
Fail fast when paths aren't clear,
Batch logic grows ever wise! ✨

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ea55c44 and c2649fb.

📒 Files selected for processing (2)
  • MCPForUnity/Editor/Tools/BatchExecute.cs
  • MCPForUnity/Editor/Tools/GameObjects/GameObjectCreate.cs

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@msanatan msanatan deleted the fix-adding-3d-models-to-scene branch January 8, 2026 05:44
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • In DetermineCallSucceeded, the result is JToken token branch can throw for non-container tokens (e.g., JValue) when indexing with token["success"]; consider restricting this path to JObject/JContainer or checking token.Type == JTokenType.Object before accessing by key.
  • DetermineCallSucceeded defaults to true for null or unrecognized result types, which may mask failures; consider either defaulting to false or at least logging when the method falls back to this default so unexpected result shapes are visible.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `DetermineCallSucceeded`, the `result is JToken token` branch can throw for non-container tokens (e.g., `JValue`) when indexing with `token["success"]`; consider restricting this path to `JObject`/`JContainer` or checking `token.Type == JTokenType.Object` before accessing by key.
- `DetermineCallSucceeded` defaults to `true` for `null` or unrecognized result types, which may mask failures; consider either defaulting to `false` or at least logging when the method falls back to this default so unexpected result shapes are visible.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

ZhangJiaLong90524 pushed a commit to ZhangJiaLong90524/unity-mcp-2020-compatible that referenced this pull request Jan 14, 2026
上游變更 (v7.0.0 → v9.0.3):
- GameObject 工具集重構 (CoplayDev#518)
- VFX 管理功能 (CoplayDev#520)
- 批次執行錯誤處理改進 (CoplayDev#531)
- Windows 長路徑問題修復 (CoplayDev#534)
- HTTP/Stdio 傳輸 UX 改進 (CoplayDev#530)
- LLM 工具註解功能 (CoplayDev#480)

衝突解決:
- modify/delete:接受刪除(架構已重構)
- content:接受 v9.0.3 版本(2020 相容性修正將另行處理)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant