Skip to content

fix: handle bare Assets/ path in manage_shader#743

Merged
dsarno merged 1 commit into
CoplayDev:betafrom
dsarno:fix/shader-double-assets-path
Feb 13, 2026
Merged

fix: handle bare Assets/ path in manage_shader#743
dsarno merged 1 commit into
CoplayDev:betafrom
dsarno:fix/shader-double-assets-path

Conversation

@dsarno

@dsarno dsarno commented Feb 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • manage_shader was creating files at Assets/Assets/ when path: Assets/ was passed
  • Root cause: Trim(/) stripped the trailing slash before StartsWith(Assets/) could match
  • Fix: also check for bare Assets (case-insensitive) after trimming

Test plan

  • 621 EditMode tests pass
  • path: Assets/ now correctly defaults to Assets/Shaders/
  • path: Assets/Shaders still works as before

Summary by Sourcery

Bug Fixes:

  • Fix handling of bare Assets or Assets/ paths so shaders are no longer created under an unintended Assets/Assets/ subdirectory.

Summary by CodeRabbit

  • Bug Fixes
    • Improved shader path handling to correctly normalize root directory references and ensure proper fallback to default directories for edge cases.

Trim(/) was stripping the trailing slash before the StartsWith check,
so Assets/ became Assets which did not match Assets/. Now handles
the bare Assets case explicitly.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@sourcery-ai

sourcery-ai Bot commented Feb 13, 2026

Copy link
Copy Markdown
Contributor
Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adjusts ManageShader path handling so a bare Assets/ path is treated as the project root for shaders instead of producing Assets/Assets/... paths.

Class diagram for updated ManageShader path handling

classDiagram
    class ManageShader {
        +static object HandleCommand(JObject params)
    }

    class AssetPathUtility {
        +static string NormalizeSeparators(string path)
    }

    ManageShader ..> AssetPathUtility : uses
Loading

File-Level Changes

Change Details Files
Fix handling of a bare Assets/ path passed to manage_shader so it no longer creates files under Assets/Assets/.
  • Normalize and trim the relative directory path, as before
  • Add a special case for relativeDir equal to Assets (case-insensitive) to map it to an empty relative path
  • Fall back to the existing StartsWith("Assets/", ...) logic only when the path is not the bare Assets directory
MCPForUnity/Editor/Tools/ManageShader.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

@coderabbitai

coderabbitai Bot commented Feb 13, 2026

Copy link
Copy Markdown
Contributor

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

The change modifies shader path normalization in ManageShader.cs to explicitly handle the exact "Assets" path (case-insensitive) by treating it as the root directory, while preserving existing behavior for paths starting with "Assets/". Added explicit braces for code clarity.

Changes

Cohort / File(s) Summary
Path Normalization
MCPForUnity/Editor/Tools/ManageShader.cs
Added explicit handling for exact "Assets" path to normalize it to root directory; existing "Assets/" prefix handling preserved; includes structural braces improvement.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

  • PR #453: Applies the same "Assets" path normalization fix to ManageScript.cs for consistent path handling across the codebase.

Poem

🐰 A path once cryptic, now made clear,
"Assets" itself finds home right here,
Root normalized, shader paths shine bright,
With braces placed and logic right,
The rabbit hops through cleaner code! ✨

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

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:

  • Consider centralizing the "Assets" and "Assets/" literals into a shared constant or helper to avoid duplication and make future path handling changes less error-prone.
  • You might simplify the path logic by handling the "Assets" prefix in one place (e.g., checking StartsWith("Assets", ...) and branching based on length and following separator) instead of having separate equality and StartsWith checks.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider centralizing the "Assets" and "Assets/" literals into a shared constant or helper to avoid duplication and make future path handling changes less error-prone.
- You might simplify the path logic by handling the "Assets" prefix in one place (e.g., checking StartsWith("Assets", ...) and branching based on length and following separator) instead of having separate equality and StartsWith checks.

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.

@dsarno dsarno merged commit 8d62d97 into CoplayDev:beta Feb 13, 2026
1 of 2 checks passed
@dsarno dsarno deleted the fix/shader-double-assets-path branch February 13, 2026 04:28
msanatan pushed a commit to msanatan/unity-mcp that referenced this pull request Feb 25, 2026
Trim(/) was stripping the trailing slash before the StartsWith check,
so Assets/ became Assets which did not match Assets/. Now handles
the bare Assets case explicitly.

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
msanatan pushed a commit to msanatan/unity-mcp that referenced this pull request Feb 25, 2026
Trim(/) was stripping the trailing slash before the StartsWith check,
so Assets/ became Assets which did not match Assets/. Now handles
the bare Assets case explicitly.

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
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