Skip to content

feat: add set_import_settings action to manage_texture tool#982

Merged
Scriptwonder merged 6 commits into
CoplayDev:betafrom
zaferdace:beta
Mar 25, 2026
Merged

feat: add set_import_settings action to manage_texture tool#982
Scriptwonder merged 6 commits into
CoplayDev:betafrom
zaferdace:beta

Conversation

@zaferdace

@zaferdace zaferdace commented Mar 24, 2026

Copy link
Copy Markdown
Contributor

Summary

The manage_texture tool currently supports creating textures with import settings but the modify action doesn't allow changing import settings on existing textures. This makes it impossible to change a texture's type (e.g. Default → Sprite) via MCP without falling back to editor scripts or YAML patching.

Changes

  • New action: set_import_settings — Directly change import settings (textureType, spriteImportMode, compression, etc.) on an existing texture without any pixel manipulation
  • modify action — Now accepts import_settings and as_sprite parameters alongside pixel operations

Usage

// Change texture type to Sprite
manage_texture(action="set_import_settings", path="Assets/UI/icon.png", import_settings={"textureType": "Sprite", "spriteImportMode": "Single"})

// Shorthand
manage_texture(action="set_import_settings", path="Assets/UI/icon.png", as_sprite=true)

// Also works with modify action
manage_texture(action="modify", path="Assets/UI/icon.png", import_settings={"textureType": "Sprite"})

Motivation

Working on a Figma-to-Unity UI pipeline where exported PNGs need to be converted to Sprite type before they can be assigned to Image components. Currently this requires either editor scripts or manual .meta file patching. This change enables the full workflow through MCP.

Summary by Sourcery

Add support for updating texture import settings via the manage_texture tool without requiring pixel modifications.

New Features:

  • Introduce a set_import_settings action to update import settings on existing textures without altering pixel data.
  • Allow the modify action to apply import settings and sprite shorthand options in addition to pixel edits.

Enhancements:

  • Avoid re-encoding and writing texture data to disk when modify is called without pixel changes, only updating import settings instead.

Summary by CodeRabbit

  • New Features

    • Added a dedicated "set import settings" command to configure texture import options per asset.
    • Quick shorthand to mark textures as sprites with sensible defaults.
  • Improvements

    • modify command accepts import-setting flags without pixel data; pixel write occurs only when pixels are provided.
    • Import-setting options validated more strictly (requires at least one import flag or pixels; disallows mixing sprite shorthand with other import flags) with clearer success/error responses.

…odify action

- New action: set_import_settings — change textureType, spriteImportMode,
  compression, etc. on existing textures without pixel manipulation
- modify action now supports import_settings and as_sprite parameters
- Enables changing texture type to Sprite via MCP without editor scripts
@sourcery-ai

sourcery-ai Bot commented Mar 24, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Adds a new set_import_settings action to the manage_texture tool and extends the modify action so that texture import settings (including sprite configuration) can be updated independently of pixel edits.

Sequence diagram for manage_texture set_import_settings action

sequenceDiagram
    actor ToolingUser
    participant MCPClient
    participant ManageTexture
    participant AssetPathUtility
    participant AssetDatabase
    participant TextureImporter

    ToolingUser->>MCPClient: manage_texture(action=set_import_settings, path, import_settings/as_sprite)
    MCPClient->>ManageTexture: HandleCommand(params)
    ManageTexture->>ManageTexture: Extract action
    ManageTexture->>ManageTexture: Route to SetImportSettings(params)

    ManageTexture->>AssetPathUtility: SanitizeAssetPath(path)
    AssetPathUtility-->>ManageTexture: fullPath
    ManageTexture->>ManageTexture: AssetExists(fullPath)?
    ManageTexture-->>MCPClient: ErrorResponse if not found

    ManageTexture->>ManageTexture: Read import_settings and as_sprite tokens
    alt import_settings provided
        ManageTexture->>ManageTexture: ConfigureTextureImporter(fullPath, importSettingsToken)
        ManageTexture->>TextureImporter: Apply textureType, spriteImportMode, compression
        TextureImporter-->>ManageTexture: Import settings applied
    else as_sprite provided
        ManageTexture->>ManageTexture: ConfigureAsSprite(fullPath, asSpriteToken)
        ManageTexture->>TextureImporter: Set textureType Sprite and sprite settings
        TextureImporter-->>ManageTexture: Sprite settings applied
    else no settings
        ManageTexture-->>MCPClient: ErrorResponse [Either import_settings or as_sprite required]
    end

    ManageTexture->>AssetDatabase: ImportAsset(fullPath, ForceUpdate) (inside configuration helpers)
    AssetDatabase-->>ManageTexture: Asset reimported

    ManageTexture-->>MCPClient: SuccessResponse [Import settings updated]
    MCPClient-->>ToolingUser: Report success and updated path
Loading

Sequence diagram for manage_texture modify action with optional import settings

sequenceDiagram
    actor ToolingUser
    participant MCPClient
    participant ManageTexture
    participant TextureOps
    participant FileIO
    participant AssetDatabase
    participant TextureImporter

    ToolingUser->>MCPClient: manage_texture(action=modify, path, set_pixels, import_settings/as_sprite)
    MCPClient->>ManageTexture: HandleCommand(params)
    ManageTexture->>ManageTexture: Route to ModifyTexture(params)

    ManageTexture->>ManageTexture: Load texture and create editableTexture

    opt pixel modifications provided
        ManageTexture->>ManageTexture: Apply pixel changes
        ManageTexture->>TextureOps: EncodeTexture(editableTexture, fullPath)
        TextureOps-->>ManageTexture: imageData
        alt imageData invalid
            ManageTexture-->>MCPClient: ErrorResponse [Failed to encode texture]
        else imageData valid
            ManageTexture->>FileIO: WriteAllBytes(absolutePath, imageData)
            FileIO-->>ManageTexture: Data written
            ManageTexture->>AssetDatabase: ImportAsset(fullPath, ForceUpdate)
            AssetDatabase-->>ManageTexture: Texture reimported
        end
    end

    ManageTexture->>ManageTexture: DestroyImmediate(editableTexture)

    opt import_settings provided
        ManageTexture->>ManageTexture: ConfigureTextureImporter(fullPath, importSettingsToken)
        ManageTexture->>TextureImporter: Apply import settings
        TextureImporter-->>ManageTexture: Settings applied
    end

    opt as_sprite provided
        ManageTexture->>ManageTexture: ConfigureAsSprite(fullPath, asSpriteToken or null)
        ManageTexture->>TextureImporter: Set sprite configuration
        TextureImporter-->>ManageTexture: Sprite settings applied
    end

    ManageTexture-->>MCPClient: SuccessResponse [Texture modified]
    MCPClient-->>ToolingUser: Report success
Loading

Class diagram for ManageTexture import settings handling

classDiagram
    class ManageTexture {
        <<static>>
        +List~string~ SupportedActions
        +object HandleCommand(JObject params)
        -object ModifyTexture(JObject params)
        -object SetImportSettings(JObject params)
        -void ConfigureTextureImporter(string path, JToken importSettings)
        -void ConfigureAsSprite(string path, JToken spriteSettings)
        -bool AssetExists(string path)
    }

    class ErrorResponse {
        +string message
        +ErrorResponse(string message)
    }

    class SuccessResponse {
        +string message
        +object payload
        +SuccessResponse(string message)
        +SuccessResponse(string message, object payload)
    }

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

    class TextureOps {
        +static byte[] EncodeTexture(Texture2D texture, string path)
    }

    class AssetDatabase {
        +static void ImportAsset(string path, ImportAssetOptions options)
    }

    class TextureImporter {
        +TextureImporter GetAtPath(string path)
        +string textureType
        +string spriteImportMode
        +string compression
        +void SaveAndReimport()
    }

    ManageTexture --> ErrorResponse : returns
    ManageTexture --> SuccessResponse : returns
    ManageTexture ..> AssetPathUtility : uses
    ManageTexture ..> TextureOps : uses
    ManageTexture ..> AssetDatabase : reimports
    ManageTexture ..> TextureImporter : configures
    AssetDatabase ..> TextureImporter : triggers reimport
Loading

File-Level Changes

Change Details Files
Wire new set_import_settings action into the manage_texture command dispatcher.
  • Register the set_import_settings action in the list of supported actions.
  • Route action="set_import_settings" requests in HandleCommand to a new SetImportSettings handler method.
MCPForUnity/Editor/Tools/ManageTexture.cs
Allow ModifyTexture to update import settings and avoid unnecessary re-encoding when no pixel data is changed.
  • Change ModifyTexture so that encoding and writing the texture back to disk only happens when setPixelsToken is present, avoiding unused pixel work for import-only updates.
  • After pixel modifications, apply import settings via ConfigureTextureImporter when import_settings/importSettings is provided.
  • Support an as_sprite shorthand parameter in ModifyTexture that calls ConfigureAsSprite, preserving legacy sprite configuration behavior.
MCPForUnity/Editor/Tools/ManageTexture.cs
Implement SetImportSettings helper for import-only texture updates without modifying pixels.
  • Add SetImportSettings method that validates the path, sanitizes it, checks asset existence, and applies import settings and/or sprite shorthand without loading or modifying texture pixels.
  • Use ConfigureTextureImporter when an import_settings/importSettings token is provided, or ConfigureAsSprite when as_sprite is provided, returning an error if neither is supplied.
  • Wrap SetImportSettings logic in try/catch and return structured SuccessResponse/ErrorResponse messages including the resolved path.
MCPForUnity/Editor/Tools/ManageTexture.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 Mar 24, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Added a new set_import_settings action and handler to ManageTexture, refactored ModifyTexture to apply import settings independently of pixel edits, and added CLI support (texture set-import-settings) plus updated tool metadata to permit the new action.

Changes

Cohort / File(s) Summary
ManageTexture editor tool
MCPForUnity/Editor/Tools/ManageTexture.cs
Added "set_import_settings" action and SetImportSettings(JObject) handler. Modified ModifyTexture to detect/validate import-setting parameters (import_settings / as_sprite), allow applying importer configuration without pixel operations, and added helpers to call ConfigureTextureImporter or ConfigureAsSprite.
Server CLI command
Server/src/cli/commands/texture.py
Made texture modify accept optional --set-pixels plus import-setting flags (--texture-type, --sprite-mode, --sprite-ppu, --max-size, --compression, --generate-mipmaps/--no-mipmaps, --srgb/--linear, --readable/--no-readable, --as-sprite). Added validation for flag combinations and conditional request building. Added new texture set-import-settings command that sends action: "set_import_settings".
Tool metadata / service signature
Server/src/services/tools/manage_texture.py
Extended MCP metadata and function signature to include "set_import_settings" in the allowed action Literal and updated the tool description to list the new action.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant ManageTexture as "ManageTexture (Editor)"
    participant TextureImporter as "TextureImporter"
    participant AssetDB as "AssetDatabase"

    rect rgba(135,206,250,0.5)
    Client->>ManageTexture: send action + params ("set_import_settings" or "modify")
    end

    alt set_import_settings
        ManageTexture->>AssetDB: Validate asset path
        ManageTexture->>TextureImporter: ConfigureTextureImporter(import_settings) / ConfigureAsSprite()
        TextureImporter-->>AssetDB: Apply settings & Reimport
        AssetDB-->>ManageTexture: success/error
        ManageTexture-->>Client: response
    else modify with pixels
        ManageTexture->>ManageTexture: Load/modify pixels, encode bytes
        ManageTexture->>AssetDB: Write texture bytes
        ManageTexture->>TextureImporter: ConfigureImportIfProvided(import_settings)
        TextureImporter-->>AssetDB: Reimport
        AssetDB-->>ManageTexture: success
        ManageTexture-->>Client: response
    else modify without pixels
        ManageTexture->>TextureImporter: ConfigureImportIfProvided(import_settings)
        TextureImporter-->>AssetDB: Apply settings & Reimport (if needed)
        AssetDB-->>ManageTexture: success
        ManageTexture-->>Client: response
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐇 I hop through assets, nudging flags with care,
No pixels painted — just settings in the air.
Sprite or texture, I tidy each bit,
Reimport, then nap in a sunlit git.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 30.77% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding a new set_import_settings action to the manage_texture tool.
Description check ✅ Passed The PR description is comprehensive, covering the motivation, changes, usage examples, and new functionality. However, it does not explicitly follow the template structure with all required sections.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


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:

  • The logic that interprets import_settings and as_sprite is now duplicated between ModifyTexture and SetImportSettings; consider extracting a shared helper that applies these settings so both paths stay in sync and are easier to maintain.
  • In ModifyTexture, AssetDatabase.ImportAsset is only called when pixel data changes, while SetImportSettings relies on ConfigureTextureImporter to trigger any reimport; double-check that ConfigureTextureImporter always forces an import or consider explicitly reimporting after changing importer settings for consistency between actions.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The logic that interprets `import_settings` and `as_sprite` is now duplicated between `ModifyTexture` and `SetImportSettings`; consider extracting a shared helper that applies these settings so both paths stay in sync and are easier to maintain.
- In `ModifyTexture`, `AssetDatabase.ImportAsset` is only called when pixel data changes, while `SetImportSettings` relies on `ConfigureTextureImporter` to trigger any reimport; double-check that `ConfigureTextureImporter` always forces an import or consider explicitly reimporting after changing importer settings for consistency between actions.

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.

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🧹 Nitpick comments (1)
MCPForUnity/Editor/Tools/ManageTexture.cs (1)

317-328: Good optimization to skip encoding when no pixel changes.

The conditional write logic is correct. However, when modify is called with only import_settings (no setPixels), the texture is still loaded into memory (lines 260-268) and Apply() is called (line 315), even though no pixel data is used.

Consider short-circuiting earlier when only import settings are being changed:

♻️ Suggested optimization
         try
         {
+            var setPixelsToken = `@params`["setPixels"] as JObject;
+            JToken importSettingsToken = `@params`["import_settings"] ?? `@params`["importSettings"];
+            JToken asSpriteToken = `@params`["as_sprite"];
+            
+            // Fast path: only import settings, no pixel changes
+            if (setPixelsToken == null && (importSettingsToken != null || asSpriteToken != null))
+            {
+                if (importSettingsToken != null)
+                    ConfigureTextureImporter(fullPath, importSettingsToken);
+                if (asSpriteToken != null && (asSpriteToken.Type == JTokenType.Boolean ? asSpriteToken.ToObject<bool>() : true))
+                    ConfigureAsSprite(fullPath, asSpriteToken.Type == JTokenType.Object ? asSpriteToken : null);
+                return new SuccessResponse($"Texture modified: {fullPath}");
+            }
+
             Texture2D texture = AssetDatabase.LoadAssetAtPath<Texture2D>(fullPath);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@MCPForUnity/Editor/Tools/ManageTexture.cs` around lines 317 - 328, In the
modify method the code loads the texture into editableTexture and calls Apply()
even when no pixel changes are requested; detect whether setPixelsToken (or any
setPixels input) is null/empty before creating editableTexture or calling
Apply(), and if only import_settings are being changed, skip texture
loading/Apply(), only update the TextureImporter settings and call
AssetDatabase.ImportAsset(fullPath, ImportAssetOptions.ForceUpdate); adjust
logic around setPixelsToken, editableTexture, TextureOps.EncodeTexture and the
Apply() call so encoding/writing only occurs when pixels were actually modified.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@MCPForUnity/Editor/Tools/ManageTexture.cs`:
- Around line 332-344: The code currently applies
ConfigureTextureImporter(fullPath, importSettingsToken) and then may call
ConfigureAsSprite(fullPath, ...) which can override textureType and other
settings; update ManageTexture.cs to detect when both import_settings
(importSettingsToken) and as_sprite (asSpriteToken) are provided and handle the
conflict: either reject the request and surface an error (e.g., throw/return
with a clear message indicating ConfigureTextureImporter and ConfigureAsSprite
are mutually exclusive) or enforce a documented precedence by skipping
ConfigureTextureImporter when as_sprite is present (or vice versa) and log which
path was chosen; implement the chosen behavior around the existing
ConfigureTextureImporter and ConfigureAsSprite calls and ensure the error/log
message references the parameter names import_settings/as_sprite and the
functions ConfigureTextureImporter/ConfigureAsSprite.

---

Nitpick comments:
In `@MCPForUnity/Editor/Tools/ManageTexture.cs`:
- Around line 317-328: In the modify method the code loads the texture into
editableTexture and calls Apply() even when no pixel changes are requested;
detect whether setPixelsToken (or any setPixels input) is null/empty before
creating editableTexture or calling Apply(), and if only import_settings are
being changed, skip texture loading/Apply(), only update the TextureImporter
settings and call AssetDatabase.ImportAsset(fullPath,
ImportAssetOptions.ForceUpdate); adjust logic around setPixelsToken,
editableTexture, TextureOps.EncodeTexture and the Apply() call so
encoding/writing only occurs when pixels were actually modified.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 40bcc60b-0048-4e44-bd79-096253d37859

📥 Commits

Reviewing files that changed from the base of the PR and between 49b749a and 0460426.

📒 Files selected for processing (1)
  • MCPForUnity/Editor/Tools/ManageTexture.cs

Comment thread MCPForUnity/Editor/Tools/ManageTexture.cs Outdated
- Extract shared ApplyImportSettingsParams helper (removes duplication)
- Fast path in modify: skip texture loading when only import settings change
- Make import_settings and as_sprite mutually exclusive with clear error
- Add error propagation from shared helper to both callers
@Scriptwonder

Copy link
Copy Markdown
Collaborator

Would you be also able to update the CLI command on this as well? Thanks!

- Add 'set-import-settings' CLI command with --texture-type, --sprite-mode,
  --as-sprite, --max-size, --compression, --generate-mipmaps flags
- Add 'set_import_settings' to Python MCP server action enum
- Fix C# key mapping: accept both 'spriteSettings' (from Python server)
  and 'as_sprite' (direct param) for sprite configuration
- Same fix for 'importSettings' / 'import_settings' keys

@coderabbitai coderabbitai 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.

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@MCPForUnity/Editor/Tools/ManageTexture.cs`:
- Around line 260-269: The modify path must validate import-setting changes
before mutating and rewriting the texture file: use
HasImportSettingsParams(`@params`) and call ApplyImportSettingsParams(fullPath,
`@params`) (or a validation-only variant) before performing the pixel-write branch
that uses setPixelsToken, so any import-setting error is returned without
persisting pixel changes; apply the same change to the other combined modify
branch referenced around the ApplyImportSettingsParams call at the later block
(lines ~336-339) so import-setting errors are checked and returned prior to any
file rewrite.
- Around line 721-749: HasImportSettingsParams and ApplyImportSettingsParams
currently treat any presence of the tokens (including {} or false) as a request,
allowing no-ops to report success; update HasImportSettingsParams to only return
true when import_settings is either a boolean true or a non-empty object (and
likewise for as_sprite: boolean true or non-empty object), then modify
ApplyImportSettingsParams to treat absent/empty/false tokens as "no request" and
to detect whether ConfigureTextureImporter / ConfigureAsSprite actually applied
changes (either by making those functions return a bool/flag or by checking
importer existence beforehand) and return an ErrorResponse or propagate failure
when nothing was applied so the set_import_settings and modify fast path do not
report success for no-ops.
- Around line 752-772: SetImportSettings currently reads raw JObject fields;
change it to accept/construct a ToolParams and use its validation/coercion
methods (e.g., RequireString/ GetString) instead of direct `@params`["path"]
lookups. Specifically, in SetImportSettings replace direct path retrieval with
ToolParams.RequireString("path") for the required path and use
ToolParams.Get/Require helpers when checking import settings
(HasImportSettingsParams) and passing parameters into ApplyImportSettingsParams
so the handler relies on ToolParams validation semantics.

In `@Server/src/cli/commands/texture.py`:
- Around line 543-604: The texture modify CLI still requires --set-pixels and
doesn't accept import settings or --as-sprite; update the texture modify command
handler (the function that implements the "texture modify" subcommand) to accept
the same import-related options as set_import_settings (texture_type,
sprite_mode, sprite_ppu, max_size, compression, generate_mipmaps/--no-mipmaps,
srgb/--linear, readable/--no-readable, and --as-sprite), make the --set-pixels
option optional, and when present build the params with keys "importSettings" or
"spriteSettings" (matching how set_import_settings constructs them) so the
handler can send both pixel edits and importSettings in a single
run_command("manage_texture", params, ...); also add validation to require at
least one of set_pixels or any import setting to be provided before calling
run_command.
- Around line 574-599: The current branch that handles as_sprite drops all other
flags; change it so that when as_sprite is True you still build import_settings
from the other flags and set import_settings["textureType"] = "Sprite" (instead
of short-circuiting), and only add params["spriteSettings"] for sprite-specific
defaults like pivot/pixelsPerUnit if needed; alternatively, if you prefer
rejecting mixed usage, detect when as_sprite and any of the other flags
(texture_type, sprite_mode, sprite_ppu, max_size, compression, generate_mipmaps,
srgb, readable) are provided and print_error + sys.exit(1). Update the logic
around the as_sprite branch and the params["importSettings"] assignment
(variables: as_sprite, import_settings, params, _TEXTURE_TYPES) to implement one
of these behaviors so flags are not silently ignored.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 54980455-1424-4ad6-bb78-221b699b71d8

📥 Commits

Reviewing files that changed from the base of the PR and between 89ff2cf and dc871b9.

📒 Files selected for processing (3)
  • MCPForUnity/Editor/Tools/ManageTexture.cs
  • Server/src/cli/commands/texture.py
  • Server/src/services/tools/manage_texture.py

Comment thread MCPForUnity/Editor/Tools/ManageTexture.cs
Comment thread MCPForUnity/Editor/Tools/ManageTexture.cs
Comment thread MCPForUnity/Editor/Tools/ManageTexture.cs
Comment thread Server/src/cli/commands/texture.py
Comment thread Server/src/cli/commands/texture.py Outdated
- Validate import settings before pixel write to prevent partial mutations
- Tighten HasImportSettingsParams: reject empty objects and false values
- Use ToolParams in SetImportSettings for consistent validation
- Add import-setting flags to modify CLI command (--set-pixels now optional)
- Guard --as-sprite against combining with other import-setting flags

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Server/src/cli/commands/texture.py`:
- Around line 604-606: The function set_import_settings currently types
parameters as non-optional but several are defined with default=None; update its
signature to use Optional[...] for all parameters that can be None (e.g.,
texture_type: Optional[str], sprite_mode: Optional[str], sprite_ppu:
Optional[float], max_size: Optional[str], compression: Optional[str],
generate_mipmaps: Optional[bool], srgb: Optional[bool], readable:
Optional[bool], as_sprite: Optional[bool]), import Optional from typing, and
mirror the approach used in the modify command to ensure annotations match the
default=None options.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9320b330-0083-4abb-ad66-ddc6abf465c4

📥 Commits

Reviewing files that changed from the base of the PR and between dc871b9 and 7306e91.

📒 Files selected for processing (2)
  • MCPForUnity/Editor/Tools/ManageTexture.cs
  • Server/src/cli/commands/texture.py

Comment thread Server/src/cli/commands/texture.py Outdated

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🧹 Nitpick comments (1)
Server/src/cli/commands/texture.py (1)

512-557: Extract shared import-settings assembly to avoid drift.

The import-flag validation and importSettings mapping are duplicated in both handlers. This is maintainable now, but likely to diverge as flags evolve.

♻️ Refactor sketch
+def _build_import_settings_from_flags(
+    texture_type: Optional[str],
+    sprite_mode: Optional[str],
+    sprite_ppu: Optional[float],
+    max_size: Optional[str],
+    compression: Optional[str],
+    generate_mipmaps: Optional[bool],
+    srgb: Optional[bool],
+    readable: Optional[bool],
+) -> dict[str, Any]:
+    import_settings: dict[str, Any] = {}
+    if texture_type:
+        import_settings["textureType"] = _TEXTURE_TYPES[texture_type]
+    if sprite_mode:
+        import_settings["spriteImportMode"] = _SPRITE_MODES[sprite_mode]
+    if sprite_ppu is not None:
+        import_settings["spritePixelsPerUnit"] = sprite_ppu
+    if max_size:
+        import_settings["maxTextureSize"] = int(max_size)
+    if compression:
+        import_settings["textureCompression"] = _COMPRESSIONS[compression]
+    if generate_mipmaps is not None:
+        import_settings["mipmapEnabled"] = generate_mipmaps
+    if srgb is not None:
+        import_settings["sRGBTexture"] = srgb
+    if readable is not None:
+        import_settings["isReadable"] = readable
+    return import_settings

Then both modify and set_import_settings can call this helper instead of duplicating field-by-field construction.

Also applies to: 624-652

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Server/src/cli/commands/texture.py` around lines 512 - 557, The
import-setting validation and construction logic is duplicated; extract it into
a single helper (e.g., _assemble_import_settings or
build_import_settings_from_flags) that takes the flag values (texture_type,
sprite_mode, sprite_ppu, max_size, compression, generate_mipmaps, srgb,
readable, as_sprite, set_pixels) and returns a tuple of (has_import_setting:
bool, import_settings: dict | None, sprite_settings: dict | None) or raises
ValueError for invalid combinations; replace the inlined blocks in the modify
handler and the set_import_settings handler with calls to this helper, preserve
current behavior (mutual-exclusion of --as-sprite with import flags, error on no
flags when set_pixels is None, mapping using
_TEXTURE_TYPES/_SPRITE_MODES/_COMPRESSIONS, and usage of _normalize_set_pixels),
and ensure the caller sets params["importSettings"] or params["spriteSettings"]
and params["setPixels"] from the helper's outputs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Server/src/cli/commands/texture.py`:
- Line 490: The CLI --max-size click.option currently restricts choices to 8192;
update the click.option declaration(s) for "--max-size" in
Server/src/cli/commands/texture.py (the click.option lines around the existing
max-size choices at the top-level command and the other occurrence) to include
"16384" in the Choice list so the CLI exposes the backend-supported
max_size=16384; ensure both places (the click.option entries at the original
lines ~490 and ~597) get the new "16384" choice string added.

---

Nitpick comments:
In `@Server/src/cli/commands/texture.py`:
- Around line 512-557: The import-setting validation and construction logic is
duplicated; extract it into a single helper (e.g., _assemble_import_settings or
build_import_settings_from_flags) that takes the flag values (texture_type,
sprite_mode, sprite_ppu, max_size, compression, generate_mipmaps, srgb,
readable, as_sprite, set_pixels) and returns a tuple of (has_import_setting:
bool, import_settings: dict | None, sprite_settings: dict | None) or raises
ValueError for invalid combinations; replace the inlined blocks in the modify
handler and the set_import_settings handler with calls to this helper, preserve
current behavior (mutual-exclusion of --as-sprite with import flags, error on no
flags when set_pixels is None, mapping using
_TEXTURE_TYPES/_SPRITE_MODES/_COMPRESSIONS, and usage of _normalize_set_pixels),
and ensure the caller sets params["importSettings"] or params["spriteSettings"]
and params["setPixels"] from the helper's outputs.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 062b1844-8e99-4144-9e23-9dabbfb64698

📥 Commits

Reviewing files that changed from the base of the PR and between 7306e91 and 34c30c8.

📒 Files selected for processing (1)
  • Server/src/cli/commands/texture.py

Comment thread Server/src/cli/commands/texture.py Outdated
…384 to max-size

- Extract _build_import_settings_from_flags and _apply_import_flags_to_params
  shared helpers to eliminate duplication between modify and set-import-settings
- Add 16384 to --max-size CLI choices (both commands) to match backend support
@Scriptwonder Scriptwonder merged commit 11dd3e2 into CoplayDev:beta Mar 25, 2026
2 checks passed
@Scriptwonder

Copy link
Copy Markdown
Collaborator

Thanks, will close this.

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.

2 participants