Fix color property conflict handling when creating materials

[dsarno]

Motivation

• Fix a logic bug where a specified colorProperty could be blocked from being applied if unrelated default color keys (_BaseColor/_Color) existed in properties.
• Ensure the color token application semantics align with expectations that an explicit colorProperty only defers to that specific property.
• Preserve existing behavior that properties should win when a matching color key is present while allowing fallbacks otherwise.

Description

• Replaced the single compound condition with a shouldApplyColor boolean and branch logic in MCPForUnity/Editor/Tools/ManageMaterial.cs.
• New logic: apply the colorToken if properties is null, or if an explicit colorProperty is provided and missing from properties, otherwise only apply when neither _BaseColor nor _Color exist in properties.
• Kept the existing color parsing via MaterialOps.ParseColor and the material SetColor sequence unchanged.

Testing

• No automated tests were run.

---

Codex Task

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[greptile-apps]

Greptile Summary

Fixed a logic bug in material color property application where an explicit colorProperty could be incorrectly blocked from being applied if unrelated default color keys (_BaseColor or _Color) existed in the properties dictionary.

• Replaced complex compound condition with clearer branching logic using shouldApplyColor boolean
• When colorProperty is specified, now only checks that specific property in the dictionary (not the default fallback keys)
• Preserves existing behavior where properties dictionary values take precedence over colorToken when keys match
• Maintains fallback behavior for default color properties when colorProperty is not specified

Confidence Score: 5/5

• This PR is safe to merge with minimal risk
• The change correctly fixes a genuine logic bug by refactoring a complex boolean condition into clearer branching logic. The new code preserves all existing correct behaviors while fixing the case where an explicit colorProperty was incorrectly blocked. The refactoring improves code readability without introducing new logic paths or edge cases.
• No files require special attention

Important Files Changed

Filename	Overview
MCPForUnity/Editor/Tools/ManageMaterial.cs	Fixed logic bug where explicit colorProperty could be blocked by unrelated default color keys in properties

Sequence Diagram

sequenceDiagram
    participant Caller
    participant CreateMaterial
    participant PropsDict
    participant Material
    
    Caller->>CreateMaterial: Call with colorToken and colorProperty and properties
    
    alt colorToken is null
        CreateMaterial-->>Caller: Skip color application
    else colorToken provided
        alt properties is null
            CreateMaterial->>Material: Apply colorToken to fallback property
        else colorProperty specified
            CreateMaterial->>PropsDict: Check colorProperty key
            alt colorProperty NOT in dict
                CreateMaterial->>Material: Apply colorToken to colorProperty
            else colorProperty in dict
                CreateMaterial-->>Material: Skip to let properties win
            end
        else colorProperty not specified
            CreateMaterial->>PropsDict: Check _BaseColor and _Color keys
            alt Neither key in dict
                CreateMaterial->>Material: Apply colorToken to fallback
            else At least one key in dict
                CreateMaterial-->>Material: Skip to let properties win
            end
        end
    end
    
    CreateMaterial->>Material: Apply remaining properties
    CreateMaterial-->>Caller: Return success

Loading