[fixes] Fixes and Skill upload#707
Merged
Scriptwonder merged 1 commit intoCoplayDev:betafrom Feb 10, 2026
Merged
Conversation
1. Some fixes I made during a very comprehensive scene generation, including VFX generating magenta due to lack of renderer material 2. I put the skills right in the .claude now, so us (developers who actually pull this repo) can directly use it. I still keep the skills under the parent folder in case users wants to download it.
Contributor
Reviewer's GuideAdds a new particle_create action and Unity-side implementation with safer renderer material handling, improves component-add error messages for disallowed multiples, wires the new VFX action through the MCP server with a regression test, and introduces in-repo Unity MCP skill/reference docs under .claude/ for developers. Sequence diagram for new particle_create VFX workflowsequenceDiagram
actor DevClient
participant MCPServer as MCPServer_manage_vfx_py
participant UnityBridge as Unity_MCP_Server
participant ManageVFX as ManageVFX
participant ParticleControl as ParticleControl
participant RendererHelpers as RendererHelpers
DevClient->>MCPServer: call manage_vfx(tool="manage_vfx", action="particle_create", params)
MCPServer->>UnityBridge: send tool request manage_vfx action="particle_create"
UnityBridge->>ManageVFX: HandleParticleSystemAction(@params, action="create")
ManageVFX->>ParticleControl: Create(@params)
ParticleControl->>ParticleControl: FindTargetGameObject(@params)
alt GameObject not found
ParticleControl->>ParticleControl: new GameObject(objectName)
ParticleControl->>ParticleControl: Undo.RegisterCreatedObjectUndo(go, "Create objectName")
end
ParticleControl->>ParticleControl: Apply position/rotation/scale via ManageVfxCommon
ParticleControl->>ParticleControl: GetComponent(ParticleSystem)
alt ParticleSystem missing
ParticleControl->>ParticleControl: AddComponent(ParticleSystem)
end
ParticleControl->>ParticleControl: GetComponent(ParticleSystemRenderer)
alt Renderer exists
ParticleControl->>RendererHelpers: EnsureMaterial(renderer)
RendererHelpers->>RendererHelpers: IsUsableMaterial(existingMaterial)
alt Material invalid or missing
RendererHelpers->>RendererHelpers: Determine VFXComponentType
RendererHelpers->>RendererHelpers: GetDefaultVFXMaterial(componentType)
RendererHelpers->>RendererHelpers: Undo.RecordObject(renderer)
RendererHelpers->>RendererHelpers: Assign defaultMat to renderer.sharedMaterial
RendererHelpers-->>ParticleControl: return
else Material valid
RendererHelpers-->>ParticleControl: return
end
end
ParticleControl->>ParticleControl: Configure main module (playOnAwake, loop)
ParticleControl->>ParticleControl: EditorUtility.SetDirty(go)
ParticleControl->>ParticleControl: MarkSceneDirty(activeScene)
ParticleControl-->>ManageVFX: { success, message, target, targetId, createdGameObject, addedParticleSystem, assignedMaterial }
ManageVFX-->>UnityBridge: wrap as particle_create result
UnityBridge-->>MCPServer: return tool result
MCPServer-->>DevClient: respond with particle_create outcome
Class diagram for updated VFX and component helper utilitiesclassDiagram
class ManageVFX {
<<static>>
-HandleParticleSystemAction(@params, action)
}
class ParticleControl {
<<static>>
+Create(@params) object
+EnableModule(@params) object
%% other particle control methods unchanged
}
class RendererHelpers {
<<static>>
+EnsureMaterial(renderer Renderer) void
-IsUsableMaterial(material Material) bool
+ApplyCommonRendererProperties(renderer Renderer) void
}
class ComponentOps {
<<static>>
+AddComponent(target GameObject, componentType Type, error out string) Component
-ApplyDefaultValues(component Component) void
-AllowsMultiple(target GameObject, componentType Type) bool
}
class GameObjectComponentHelpers {
<<static>>
+AddComponentInternal(targetGo GameObject, typeName string, searchMethod string) object
+RemoveComponentInternal(targetGo GameObject, typeName string) object
-AllowsMultiple(componentType Type) bool
}
ManageVFX ..> ParticleControl : uses
ParticleControl ..> RendererHelpers : uses EnsureMaterial
ComponentOps ..> GameObject : operates_on
GameObjectComponentHelpers ..> GameObject : operates_on
ComponentOps ..> Component : creates
GameObjectComponentHelpers ..> Component : creates
RendererHelpers ..> Renderer : configures
Renderer <|-- ParticleSystemRenderer
note for ComponentOps "Improved error messages when component already exists and DisallowMultipleComponent prevents duplicates"
note for GameObjectComponentHelpers "Uses AllowsMultiple to prevent adding disallowed duplicate components and returns clearer ErrorResponse messages"
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
📝 WalkthroughWalkthroughAdds comprehensive skill documentation for unity-mcp-orchestrator, implements duplicate-component prevention across helper classes, enhances material validation in renderers, and introduces a new particle creation action to the VFX toolchain with corresponding server-side support and tests. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Server as Server<br/>(manage_vfx)
participant Unity as Unity Editor<br/>(ManageVFX)
participant ParticleControl
participant GameObjectOps
participant RendererHelpers
Client->>Server: particle_create action<br/>(target, position, rotation, etc.)
Server->>Unity: send_with_unity_instance<br/>(particle_create params)
Unity->>ManageVFX: HandleParticleSystemAction<br/>("create")
ManageVFX->>ParticleControl: ParticleControl.Create(params)
ParticleControl->>GameObjectOps: Locate or create<br/>target GameObject
GameObjectOps-->>ParticleControl: GameObject reference
ParticleControl->>ParticleControl: Apply position,<br/>rotation, scale
ParticleControl->>ParticleControl: Ensure ParticleSystem<br/>component exists
ParticleControl->>RendererHelpers: EnsureMaterial<br/>(renderer)
RendererHelpers->>RendererHelpers: Validate existing material<br/>via IsUsableMaterial
RendererHelpers-->>ParticleControl: Material assigned/<br/>validated
ParticleControl->>ParticleControl: Configure playOnAwake,<br/>loop settings
ParticleControl->>ParticleControl: Mark GameObject &<br/>scene dirty
ParticleControl-->>Unity: Result object<br/>(success, details)
Unity-->>Server: Response payload
Server-->>Client: Success response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new
AllowsMultiplelogic is now duplicated betweenComponentOpsandGameObjectComponentHelpers; consider centralizing this into a shared helper to keep the rules for multi-instance components consistent over time. - In
ComponentOps.AddComponenttheAllowsMultiple(GameObject target, Type componentType)signature takes aGameObjectthat isn’t used beyond a null check; you could simplify this API (or actually use the target) to make the intent clearer.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `AllowsMultiple` logic is now duplicated between `ComponentOps` and `GameObjectComponentHelpers`; consider centralizing this into a shared helper to keep the rules for multi-instance components consistent over time.
- In `ComponentOps.AddComponent` the `AllowsMultiple(GameObject target, Type componentType)` signature takes a `GameObject` that isn’t used beyond a null check; you could simplify this API (or actually use the target) to make the intent clearer.
## Individual Comments
### Comment 1
<location> `MCPForUnity/Editor/Helpers/ComponentOps.cs:363-372` </location>
<code_context>
}
}
+
+ private static bool AllowsMultiple(GameObject target, Type componentType)
+ {
+ if (target == null || componentType == null)
+ {
+ return false;
+ }
+
+ if (Attribute.IsDefined(componentType, typeof(DisallowMultipleComponent), inherit: true))
+ {
+ return false;
+ }
+
+ return true;
+ }
}
</code_context>
<issue_to_address>
**nitpick:** The `target` parameter in `AllowsMultiple` is unused and could be removed or leveraged.
This helper ignores the `GameObject target` and only checks `DisallowMultipleComponent`. Consider either removing `target` to simplify the API, or using it to handle Unity’s built-in single-instance components (e.g., Transform-like types) so the pre-check and error messages are more accurate.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+363
to
+372
| private static bool AllowsMultiple(GameObject target, Type componentType) | ||
| { | ||
| if (target == null || componentType == null) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| if (Attribute.IsDefined(componentType, typeof(DisallowMultipleComponent), inherit: true)) | ||
| { | ||
| return false; |
Contributor
There was a problem hiding this comment.
nitpick: The target parameter in AllowsMultiple is unused and could be removed or leveraged.
This helper ignores the GameObject target and only checks DisallowMultipleComponent. Consider either removing target to simplify the API, or using it to handle Unity’s built-in single-instance components (e.g., Transform-like types) so the pre-check and error messages are more accurate.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @.claude/skills/unity-mcp-skill/references/tools-reference.md:
- Around line 5-15: Add a new "VFX Tools" section to the tools-reference that
documents the manage_vfx tool and its actions; include the action prefixes
particle_ (e.g., particle_create), vfx_, line_, and trail_, list key parameters
for each common action (spawn settings, lifetime, emissionRate, prefab/template
id, position/rotation/scale, parent, attachBone, color/gradient,
velocity/spread, duration, loop, and any resource keys), and show which actions
are handled by the manage_vfx tool (e.g., particle_create, particle_update,
vfx_play, vfx_stop, line_draw, trail_emit). Reference the manage_vfx tool name
and the particle_create action so reviewers can locate and verify the new doc
entry.
In @.claude/skills/unity-mcp-skill/references/workflows.md:
- Around line 581-589: The retry loop using read_resource (variables
max_retries, attempt, editor_state) currently uses a bare except: which also
catches SystemExit/KeyboardInterrupt; change it to catch only Exception (e.g.,
except Exception:) and preserve the exponential backoff sleep behavior so only
runtime errors are retried while allowing interrupts/termination to propagate.
In `@MCPForUnity/Editor/Helpers/ComponentOps.cs`:
- Around line 363-376: Remove the unused GameObject target parameter from the
private static AllowsMultiple method in ComponentOps.cs and simplify its body to
only check the DisallowMultipleComponent attribute on the passed-in Type; update
every call site that currently passes a GameObject + Type to invoke the new
signature AllowsMultiple(Type componentType) instead, and remove the obsolete
null-check for target; note there is a duplicate method AllowsMultiple(Type) in
GameObjectComponentHelpers.cs but only change ComponentOps.cs as requested.
In `@MCPForUnity/Editor/Tools/Vfx/ParticleControl.cs`:
- Around line 54-59: When adding a ParticleSystem to an existing GameObject, use
the Unity Undo API so the operation is undoable: replace the direct call to
go.AddComponent<ParticleSystem>() with Undo.AddComponent<ParticleSystem>(go)
when createdGameObject is false (or generally when not creating the GameObject).
Update the block that sets ps and addedParticleSystem (the variables ps,
addedParticleSystem and the logic that checks createdGameObject) so the
ParticleSystem is created via Undo.AddComponent when appropriate, preserving
addedParticleSystem semantics.
🧹 Nitpick comments (3)
Server/tests/test_manage_vfx_actions.py (1)
12-16: Prefix unusedsend_fnwith_to silence lint warning.The Ruff ARG001 warning flags the unused
send_fnparameter. Since the mock must match the real signature, prefixing it is the idiomatic fix.🔧 Proposed fix
- async def fake_send_with_unity_instance(send_fn, unity_instance, tool_name, params): + async def fake_send_with_unity_instance(_send_fn, unity_instance, tool_name, params):MCPForUnity/Editor/Helpers/RendererHelpers.cs (1)
53-62: Consider logging when no default material could be found.When
componentTypeis null (unsupported renderer type) orGetOrCreateDefaultVFXMaterialreturns null, the method silently leaves the broken material in place. A warning here would help diagnose cases where the fallback also fails.Proposed improvement
if (componentType.HasValue) { Material defaultMat = RenderPipelineUtility.GetOrCreateDefaultVFXMaterial(componentType.Value); if (defaultMat != null) { Undo.RecordObject(renderer, "Assign default VFX material"); renderer.sharedMaterial = defaultMat; EditorUtility.SetDirty(renderer); } + else + { + McpLog.Warn($"[RendererHelpers] Could not find or create a default VFX material for {componentType.Value}."); + } + } + else + { + McpLog.Warn($"[RendererHelpers] Unsupported renderer type '{renderer.GetType().Name}', cannot assign default material."); }MCPForUnity/Editor/Tools/GameObjects/GameObjectComponentHelpers.cs (1)
101-109: Consider consolidatingAllowsMultiplewithComponentOps.AllowsMultipleto avoid drift.Both
GameObjectComponentHelpers.AllowsMultiple(Type componentType)(lines 101-109) andComponentOps.AllowsMultiple(GameObject target, Type componentType)contain equivalent logic—they both check for theDisallowMultipleComponentattribute. TheGameObjectparameter inComponentOpsis only used for a null guard and doesn't affect the core attribute check. A single shared implementation would eliminate duplicate logic and reduce maintenance risk.
Comment on lines
+5
to
+15
| ## Table of Contents | ||
|
|
||
| - [Infrastructure Tools](#infrastructure-tools) | ||
| - [Scene Tools](#scene-tools) | ||
| - [GameObject Tools](#gameobject-tools) | ||
| - [Script Tools](#script-tools) | ||
| - [Asset Tools](#asset-tools) | ||
| - [Material & Shader Tools](#material--shader-tools) | ||
| - [Editor Control Tools](#editor-control-tools) | ||
| - [Testing Tools](#testing-tools) | ||
|
|
Contributor
There was a problem hiding this comment.
manage_vfx tool is missing from this reference.
This PR adds the particle_create action and the VFX tool is a significant part of the toolchain, yet there's no VFX Tools section in this reference document. Consider adding a section covering manage_vfx with its action prefixes (particle_*, vfx_*, line_*, trail_*) and key parameters.
🤖 Prompt for AI Agents
In @.claude/skills/unity-mcp-skill/references/tools-reference.md around lines 5
- 15, Add a new "VFX Tools" section to the tools-reference that documents the
manage_vfx tool and its actions; include the action prefixes particle_ (e.g.,
particle_create), vfx_, line_, and trail_, list key parameters for each common
action (spawn settings, lifetime, emissionRate, prefab/template id,
position/rotation/scale, parent, attachBone, color/gradient, velocity/spread,
duration, loop, and any resource keys), and show which actions are handled by
the manage_vfx tool (e.g., particle_create, particle_update, vfx_play, vfx_stop,
line_draw, trail_emit). Reference the manage_vfx tool name and the
particle_create action so reviewers can locate and verify the new doc entry.
Comment on lines
+581
to
+589
| max_retries = 5 | ||
| for attempt in range(max_retries): | ||
| try: | ||
| editor_state = read_resource("mcpforunity://editor/state") | ||
| if editor_state["ready_for_tools"]: | ||
| break | ||
| except: | ||
| time.sleep(2 ** attempt) # Exponential backoff | ||
| ``` |
Contributor
There was a problem hiding this comment.
Bare except: in example code — prefer except Exception:.
Users may copy-paste this pattern. A bare except: catches KeyboardInterrupt and SystemExit, which is rarely intended.
📝 Suggested fix
- except:
+ except Exception:
time.sleep(2 ** attempt) # Exponential backoff📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| max_retries = 5 | |
| for attempt in range(max_retries): | |
| try: | |
| editor_state = read_resource("mcpforunity://editor/state") | |
| if editor_state["ready_for_tools"]: | |
| break | |
| except: | |
| time.sleep(2 ** attempt) # Exponential backoff | |
| ``` | |
| max_retries = 5 | |
| for attempt in range(max_retries): | |
| try: | |
| editor_state = read_resource("mcpforunity://editor/state") | |
| if editor_state["ready_for_tools"]: | |
| break | |
| except Exception: | |
| time.sleep(2 ** attempt) # Exponential backoff |
🤖 Prompt for AI Agents
In @.claude/skills/unity-mcp-skill/references/workflows.md around lines 581 -
589, The retry loop using read_resource (variables max_retries, attempt,
editor_state) currently uses a bare except: which also catches
SystemExit/KeyboardInterrupt; change it to catch only Exception (e.g., except
Exception:) and preserve the exponential backoff sleep behavior so only runtime
errors are retried while allowing interrupts/termination to propagate.
Comment on lines
+363
to
+376
| private static bool AllowsMultiple(GameObject target, Type componentType) | ||
| { | ||
| if (target == null || componentType == null) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| if (Attribute.IsDefined(componentType, typeof(DisallowMultipleComponent), inherit: true)) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| return true; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all AllowsMultiple method definitions
rg -n "AllowsMultiple" --type=cs -B2 -A10Repository: CoplayDev/unity-mcp
Length of output: 7603
Remove unused target parameter.
The target parameter is never used in the method logic—it's only null-checked. All call sites (lines 56, 67) already access target immediately before calling AllowsMultiple, making the null guard redundant. Remove the parameter and simplify the method.
Note: GameObjectComponentHelpers.cs contains an identical AllowsMultiple(Type componentType) method (line 101), indicating duplicated logic. However, both implementations are private static, so consolidation would require refactoring the access levels. At minimum, remove the unused parameter here.
🤖 Prompt for AI Agents
In `@MCPForUnity/Editor/Helpers/ComponentOps.cs` around lines 363 - 376, Remove
the unused GameObject target parameter from the private static AllowsMultiple
method in ComponentOps.cs and simplify its body to only check the
DisallowMultipleComponent attribute on the passed-in Type; update every call
site that currently passes a GameObject + Type to invoke the new signature
AllowsMultiple(Type componentType) instead, and remove the obsolete null-check
for target; note there is a duplicate method AllowsMultiple(Type) in
GameObjectComponentHelpers.cs but only change ComponentOps.cs as requested.
Comment on lines
+54
to
+59
| var ps = go.GetComponent<ParticleSystem>(); | ||
| if (ps == null) | ||
| { | ||
| ps = go.AddComponent<ParticleSystem>(); | ||
| addedParticleSystem = true; | ||
| } |
Contributor
There was a problem hiding this comment.
Use Undo.AddComponent for undo-safe ParticleSystem addition.
When the target GameObject already exists (createdGameObject == false), calling go.AddComponent<ParticleSystem>() bypasses the Undo system. The user won't be able to Ctrl+Z the component addition on a pre-existing object.
Proposed fix
var ps = go.GetComponent<ParticleSystem>();
if (ps == null)
{
- ps = go.AddComponent<ParticleSystem>();
+ if (!EditorApplication.isPlaying)
+ {
+ ps = Undo.AddComponent<ParticleSystem>(go);
+ }
+ else
+ {
+ ps = go.AddComponent<ParticleSystem>();
+ }
addedParticleSystem = true;
}🤖 Prompt for AI Agents
In `@MCPForUnity/Editor/Tools/Vfx/ParticleControl.cs` around lines 54 - 59, When
adding a ParticleSystem to an existing GameObject, use the Unity Undo API so the
operation is undoable: replace the direct call to
go.AddComponent<ParticleSystem>() with Undo.AddComponent<ParticleSystem>(go)
when createdGameObject is false (or generally when not creating the GameObject).
Update the block that sets ps and addedParticleSystem (the variables ps,
addedParticleSystem and the logic that checks createdGameObject) so the
ParticleSystem is created via Undo.AddComponent when appropriate, preserving
addedParticleSystem semantics.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new particle_create action to the cross-language manage_vfx toolchain (Python server → Unity Editor tool) to create/prepare ParticleSystems and avoid magenta VFX by ensuring a valid renderer material, plus improves component-add error messages and vendors Unity-MCP “skill” documentation into .claude/.
Changes:
- Add
particle_createto the server-sidemanage_vfxaction allowlist and a Python unit test verifying request forwarding. - Implement Unity-side
particle_createhandling (create/prepare ParticleSystem, set transform, ensure renderer material) and update action docs/routing. - Improve Unity component-add error messaging when adding disallowed duplicate components; add
.claudeskill reference docs.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| Server/src/services/tools/manage_vfx.py | Adds particle_create to valid action list so server accepts/forwards it. |
| Server/tests/test_manage_vfx_actions.py | New unit test confirming particle_create params are forwarded to Unity transport. |
| MCPForUnity/Editor/Tools/Vfx/ParticleControl.cs | Implements ParticleControl.Create() to prepare/create ParticleSystem and ensure renderer material. |
| MCPForUnity/Editor/Tools/Vfx/ManageVFX.cs | Documents and routes particle_create → create handler; includes it in “valid actions” error text. |
| MCPForUnity/Editor/Helpers/RendererHelpers.cs | Enhances material validation and replaces invalid/unsupported materials to avoid magenta rendering. |
| MCPForUnity/Editor/Tools/GameObjects/GameObjectComponentHelpers.cs | Adds clearer error when adding non-multi components that already exist. |
| MCPForUnity/Editor/Helpers/ComponentOps.cs | Mirrors improved duplicate-component error messaging for low-level helper. |
| .claude/skills/unity-mcp-skill/SKILL.md | Adds Unity-MCP operator skill documentation into .claude. |
| .claude/skills/unity-mcp-skill/references/tools-reference.md | Adds tool reference documentation for Unity-MCP usage. |
| .claude/skills/unity-mcp-skill/references/workflows.md | Adds workflow examples/patterns for Unity-MCP usage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var ps = go.GetComponent<ParticleSystem>(); | ||
| if (ps == null) | ||
| { | ||
| ps = go.AddComponent<ParticleSystem>(); |
There was a problem hiding this comment.
particle_create uses go.AddComponent<ParticleSystem>(), which bypasses Unity's Undo system in edit mode (unlike other tools here that use Undo.AddComponent). Consider using Undo.AddComponent when !EditorApplication.isPlaying (and falling back to AddComponent in play mode) so the operation can be undone/redone and matches existing component-add patterns.
Suggested change
| ps = go.AddComponent<ParticleSystem>(); | |
| if (!EditorApplication.isPlaying) | |
| { | |
| ps = Undo.AddComponent<ParticleSystem>(go); | |
| } | |
| else | |
| { | |
| ps = go.AddComponent<ParticleSystem>(); | |
| } |
Comment on lines
+23
to
+33
| if (go == null) | ||
| { | ||
| string objectName = target; | ||
| int slashIndex = target.LastIndexOf('/'); | ||
| if (slashIndex >= 0 && slashIndex < target.Length - 1) | ||
| { | ||
| objectName = target.Substring(slashIndex + 1); | ||
| } | ||
|
|
||
| go = new GameObject(objectName); | ||
| createdGameObject = true; |
There was a problem hiding this comment.
When the target GameObject cannot be resolved, particle_create always creates a new GameObject named from target. If callers use searchMethod=by_id and the id is missing/invalid, this will create a GameObject named like an instance id, which is likely unintended. Consider only auto-creating when the lookup mode is name/path (or when searchMethod is unset), and returning an explicit error for by_id (and potentially by_tag/by_layer) when no match is found.
Comment on lines
+41
to
+52
| if (@params["position"] != null) | ||
| { | ||
| go.transform.position = ManageVfxCommon.ParseVector3(@params["position"]); | ||
| } | ||
| if (@params["rotation"] != null) | ||
| { | ||
| go.transform.eulerAngles = ManageVfxCommon.ParseVector3(@params["rotation"]); | ||
| } | ||
| if (@params["scale"] != null) | ||
| { | ||
| go.transform.localScale = ManageVfxCommon.ParseVector3(@params["scale"]); | ||
| } |
There was a problem hiding this comment.
particle_create mutates the target Transform (position/rotation/scale) without recording Undo. In edit mode this makes the operation non-undoable even though other VFX write operations call Undo.RecordObject. Consider calling Undo.RecordObject(go.transform, ...) (and/or Undo.RecordObject(go, ...)) before applying transform changes when !EditorApplication.isPlaying.
| return false; | ||
| } | ||
|
|
||
| return !Attribute.IsDefined(componentType, typeof(DisallowMultipleComponent), inherit: true); |
There was a problem hiding this comment.
There are now two separate AllowsMultiple implementations (here and in ComponentOps). This duplicates the logic for interpreting [DisallowMultipleComponent] and can drift over time. Consider extracting a shared helper (e.g., in ComponentOps or a common utility) and reusing it in both places.
Suggested change
| return !Attribute.IsDefined(componentType, typeof(DisallowMultipleComponent), inherit: true); | |
| return ComponentOps.AllowsMultiple(componentType); |
Comment on lines
+67
to
+74
| if (target.GetComponent(componentType) != null && !AllowsMultiple(target, componentType)) | ||
| { | ||
| error = $"Component '{componentType.Name}' already exists on '{target.name}' and this type does not allow multiple instances."; | ||
| } | ||
| else | ||
| { | ||
| error = $"Failed to add component '{componentType.Name}' to '{target.name}'. Unity may restrict this component on the current target."; | ||
| } |
There was a problem hiding this comment.
Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
Suggested change
| if (target.GetComponent(componentType) != null && !AllowsMultiple(target, componentType)) | |
| { | |
| error = $"Component '{componentType.Name}' already exists on '{target.name}' and this type does not allow multiple instances."; | |
| } | |
| else | |
| { | |
| error = $"Failed to add component '{componentType.Name}' to '{target.name}'. Unity may restrict this component on the current target."; | |
| } | |
| error = target.GetComponent(componentType) != null && !AllowsMultiple(target, componentType) | |
| ? $"Component '{componentType.Name}' already exists on '{target.name}' and this type does not allow multiple instances." | |
| : $"Failed to add component '{componentType.Name}' to '{target.name}'. Unity may restrict this component on the current target."; |
Comment on lines
+68
to
+75
| if (@params["playOnAwake"] != null) | ||
| { | ||
| main.playOnAwake = @params["playOnAwake"].ToObject<bool>(); | ||
| } | ||
| else | ||
| { | ||
| main.playOnAwake = false; | ||
| } |
There was a problem hiding this comment.
Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
msanatan
pushed a commit
to msanatan/unity-mcp
that referenced
this pull request
Feb 10, 2026
1. Some fixes I made during a very comprehensive scene generation, including VFX generating magenta due to lack of renderer material 2. I put the skills right in the .claude now, so us (developers who actually pull this repo) can directly use it. I still keep the skills under the parent folder in case users wants to download it.
4 tasks
msanatan
pushed a commit
to msanatan/unity-mcp
that referenced
this pull request
Feb 25, 2026
1. Some fixes I made during a very comprehensive scene generation, including VFX generating magenta due to lack of renderer material 2. I put the skills right in the .claude now, so us (developers who actually pull this repo) can directly use it. I still keep the skills under the parent folder in case users wants to download it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Type of Change
Save your change type
Changes Made
Testing/Screenshots/Recordings
Documentation Updates
tools/UPDATE_DOCS_PROMPT.md(recommended)tools/UPDATE_DOCS.mdRelated Issues
Additional Notes
Summary by Sourcery
Improve Unity VFX particle creation robustness, enhance component and renderer error handling, and embed Unity MCP skill documentation for local development use.
New Features:
Bug Fixes:
Enhancements:
Tests:
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation