feat: add manage_profiler tool with session control, counters, memory snapshots, and Frame Debugger#1000
feat: add manage_profiler tool with session control, counters, memory snapshots, and Frame Debugger#1000Scriptwonder wants to merge 18 commits intoCoplayDev:betafrom
Conversation
…n profiling Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replaces the old 5-action read-only profiler with a comprehensive tool covering session control, generic counter reads, memory snapshots, and Frame Debugger. Adds "profiling" as a new opt-in tool group.
Reviewer's GuideAdds a new Sequence diagram for manage_profiler get_counters flowsequenceDiagram
actor User
participant CLI_profiler_get_counters as CLI_profiler_get_counters
participant MCP_Server_manage_profiler as MCP_Server_manage_profiler
participant UnityTransport as UnityTransport
participant UnityEditor_ManageProfiler as UnityEditor_ManageProfiler
participant CounterOps as CounterOps
User->>CLI_profiler_get_counters: profiler get-counters --category Render
CLI_profiler_get_counters->>MCP_Server_manage_profiler: manage_profiler(action=get_counters, category="Render")
MCP_Server_manage_profiler->>MCP_Server_manage_profiler: validate action, build params_dict
MCP_Server_manage_profiler->>UnityTransport: send_with_unity_instance("manage_profiler", params_dict)
UnityTransport->>UnityEditor_ManageProfiler: HandleCommand(params)
UnityEditor_ManageProfiler->>UnityEditor_ManageProfiler: parse action "get_counters"
UnityEditor_ManageProfiler->>CounterOps: GetCountersAsync(params)
CounterOps->>CounterOps: ResolveCategory(category)
CounterOps->>CounterOps: Start ProfilerRecorder instances
CounterOps-->>CounterOps: WaitOneFrameAsync via EditorApplication.update
CounterOps->>CounterOps: Read recorder values, dispose recorders
CounterOps-->>UnityEditor_ManageProfiler: SuccessResponse(counters data)
UnityEditor_ManageProfiler-->>UnityTransport: serialized SuccessResponse
UnityTransport-->>MCP_Server_manage_profiler: result dict
MCP_Server_manage_profiler-->>CLI_profiler_get_counters: result
CLI_profiler_get_counters-->>User: formatted output (counter values)
Updated class diagram for manage_profiler C# operationsclassDiagram
class ManageProfiler {
<<static>>
+HandleCommand(params JObject) Task~object~
}
class SessionOps {
<<static>>
+Start(params JObject) object
+Stop(params JObject) object
+Status(params JObject) object
+SetAreas(params JObject) object
}
class CounterOps {
<<static>>
+GetCountersAsync(params JObject) Task~object~
-GetRequestedCounters(p ToolParams, category ProfilerCategory) List~string~
-WaitOneFrameAsync() Task
-ResolveCategory(name string, out error string) ProfilerCategory?
-ValidCategories string[]
}
class FrameTimingOps {
<<static>>
+GetFrameTiming(params JObject) object
}
class ObjectMemoryOps {
<<static>>
+GetObjectMemory(params JObject) object
}
class MemorySnapshotOps {
<<static>>
-MemoryProfilerType Type
-HasPackage bool
+TakeSnapshotAsync(params JObject) Task~object~
+ListSnapshots(params JObject) object
+CompareSnapshots(params JObject) object
-PackageMissingError() ErrorResponse
}
class FrameDebuggerOps {
<<static>>
-UtilType Type
-EventCountProp PropertyInfo
-EnableMethod MethodInfo
-GetEventDataMethod MethodInfo
-ReflectionAvailable bool
+Enable(params JObject) object
+Disable(params JObject) object
+GetEvents(params JObject) object
-TryAddField(type Type, obj object, fieldName string, dict Dictionary~string, object~) void
}
class ToolParams {
+ToolParams(params JObject)
+Get(name string) string
+GetBool(name string) bool
+GetInt(name string) int?
+GetStringArray(name string) string[]
+GetRequired(name string) ToolParamResult~string~
}
class SuccessResponse {
+SuccessResponse(message string, data object)
}
class ErrorResponse {
+ErrorResponse(message string)
}
ManageProfiler --> SessionOps : uses
ManageProfiler --> CounterOps : uses
ManageProfiler --> FrameTimingOps : uses
ManageProfiler --> ObjectMemoryOps : uses
ManageProfiler --> MemorySnapshotOps : uses
ManageProfiler --> FrameDebuggerOps : uses
SessionOps --> ToolParams : uses
SessionOps --> SuccessResponse : returns
SessionOps --> ErrorResponse : returns
CounterOps --> ToolParams : uses
CounterOps --> SuccessResponse : returns
CounterOps --> ErrorResponse : returns
FrameTimingOps --> SuccessResponse : returns
FrameTimingOps --> ErrorResponse : returns
ObjectMemoryOps --> ToolParams : uses
ObjectMemoryOps --> SuccessResponse : returns
ObjectMemoryOps --> ErrorResponse : returns
MemorySnapshotOps --> ToolParams : uses
MemorySnapshotOps --> SuccessResponse : returns
MemorySnapshotOps --> ErrorResponse : returns
FrameDebuggerOps --> ToolParams : uses
FrameDebuggerOps --> SuccessResponse : returns
FrameDebuggerOps --> ErrorResponse : returns
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
📝 WalkthroughWalkthroughAdds a new Changes
Sequence DiagramsequenceDiagram
participant CLI as Client (CLI)
participant Server as Python Server
participant Unity as Unity Editor
participant Profiler as Unity Profiler API
CLI->>Server: profiler start --log-file=log.raw
activate Server
Server->>Server: normalize action, build params
Server->>Unity: send_command("manage_profiler", {action:"profiler_start", log_file:"log.raw"})
deactivate Server
activate Unity
Unity->>Unity: ManageProfiler.HandleCommand -> SessionOps.Start
Unity->>Profiler: Enable profiler, set log file, enable callstacks
Profiler-->>Unity: Success
Unity-->>Server: SuccessResponse
deactivate Unity
Server-->>CLI: display response
CLI->>Server: profiler get-counters --category=Scripts
activate Server
Server->>Unity: send_command("manage_profiler", {action:"get_counters", category:"Scripts"})
deactivate Server
activate Unity
Unity->>Unity: ManageProfiler -> CounterOps.GetCountersAsync
Unity->>Profiler: Start ProfilerRecorders, sample next frame
Profiler-->>Unity: counter values
Unity-->>Server: SuccessResponse(counters)
deactivate Unity
Server-->>CLI: display counters
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In
CounterOps.GetCountersAsync, consider wrapping the recorder creation/usage in a try/finally (or using-pattern) so that allProfilerRecorderinstances are disposed even ifWaitOneFrameAsyncor value reading throws, to avoid leaking native resources. - In
FrameDebuggerOps,EnableMethodis always invoked with(true, 0)andDisablewith(false, 0)regardless of the overload that was resolved; it would be more robust to inspectEnableMethod.GetParameters().Lengthand call the zero-arg or two-arg overload appropriately to avoid relying on exceptions for overload resolution.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `CounterOps.GetCountersAsync`, consider wrapping the recorder creation/usage in a try/finally (or using-pattern) so that all `ProfilerRecorder` instances are disposed even if `WaitOneFrameAsync` or value reading throws, to avoid leaking native resources.
- In `FrameDebuggerOps`, `EnableMethod` is always invoked with `(true, 0)` and `Disable` with `(false, 0)` regardless of the overload that was resolved; it would be more robust to inspect `EnableMethod.GetParameters().Length` and call the zero-arg or two-arg overload appropriately to avoid relying on exceptions for overload resolution.
## Individual Comments
### Comment 1
<location path="MCPForUnity/Editor/Tools/Profiler/Operations/MemorySnapshotOps.cs" line_range="37-38" />
<code_context>
+
+ try
+ {
+ var takeMethod = MemoryProfilerType.GetMethod("TakeSnapshot",
+ new[] { typeof(string), typeof(Action<string, bool>), typeof(Action<string, bool, UnityEditor.DebugScreenCapture>), typeof(uint) });
+
+ if (takeMethod == null)
</code_context>
<issue_to_address>
**issue (bug_risk):** DebugScreenCapture type reference is incorrect and will not compile as written.
The third parameter type should be `UnityEngine.DebugScreenCapture` (or just `DebugScreenCapture` since `using UnityEngine;` is present), not `UnityEditor.DebugScreenCapture`. As written this won’t compile and the reflection signature won’t match the real `TakeSnapshot` method.
</issue_to_address>
### Comment 2
<location path="MCPForUnity/Editor/Tools/Profiler/Operations/CounterOps.cs" line_range="37-46" />
<code_context>
+ counters = new Dictionary<string, object>()
+ });
+
+ // Start recorders
+ var recorders = new List<ProfilerRecorder>();
+ foreach (string name in counterNames)
+ {
+ recorders.Add(ProfilerRecorder.StartNew(category, name));
+ }
+
+ // Wait 1 frame for recorders to accumulate data
+ await WaitOneFrameAsync();
+
+ // Read values and dispose
+ var data = new Dictionary<string, object>();
+ for (int i = 0; i < recorders.Count; i++)
+ {
+ var recorder = recorders[i];
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Recorder disposal relies on the happy path; consider guarding disposal with try/finally.
Currently, recorders are only disposed if `WaitOneFrameAsync()` and the subsequent logic complete without throwing. Any exception after `StartNew` means the recorders can leak until domain reload. Please wrap the read/convert section in a `try/finally` that always iterates `recorders` and calls `Dispose()` to ensure cleanup even on failure.
</issue_to_address>
### Comment 3
<location path="unity-mcp-skill/references/tools-reference.md" line_range="1440" />
<code_context>
+| `counters` | list[str] | No | Specific counter names for get_counters. Omit to read all in category |
+| `object_path` | string | For get_object_memory | Scene hierarchy or asset path |
+| `log_file` | string | No | Path to `.raw` file for profiler_start recording |
+| `enable_callstacks` | bool | No | Enable allocation callstacks for profiler_start |
+| `areas` | dict[str, bool] | For profiler_set_areas | Area name to enabled/disabled mapping |
+| `snapshot_path` | string | No | Output path for memory_take_snapshot |
</code_context>
<issue_to_address>
**nitpick (typo):** Use the more standard spelling "call stacks".
Consider updating the description to: `Enable allocation call stacks for profiler_start.`
```suggestion
| `enable_callstacks` | bool | No | Enable allocation call stacks for profiler_start. |
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
MCPForUnity/Editor/Tools/Profiler/Operations/MemorySnapshotOps.cs
Outdated
Show resolved
Hide resolved
| | `counters` | list[str] | No | Specific counter names for get_counters. Omit to read all in category | | ||
| | `object_path` | string | For get_object_memory | Scene hierarchy or asset path | | ||
| | `log_file` | string | No | Path to `.raw` file for profiler_start recording | | ||
| | `enable_callstacks` | bool | No | Enable allocation callstacks for profiler_start | |
There was a problem hiding this comment.
nitpick (typo): Use the more standard spelling "call stacks".
Consider updating the description to: Enable allocation call stacks for profiler_start.
| | `enable_callstacks` | bool | No | Enable allocation callstacks for profiler_start | | |
| | `enable_callstacks` | bool | No | Enable allocation call stacks for profiler_start. | |
There was a problem hiding this comment.
Pull request overview
Adds a new opt-in manage_profiler tool (group: profiling) that exposes Unity Profiler session control, generic counter reads, memory snapshots, and Frame Debugger operations across the Unity package, Server tool layer, CLI, and docs.
Changes:
- Introduces Unity-side
manage_profilerdispatcher plus operations for session, counters, object memory, memory snapshots, frame timing, and Frame Debugger. - Adds Server-side
manage_profilerMCP tool + CLIprofilercommand group, plus a dedicated Python test suite. - Updates tool references/docs and bumps versions to
9.6.3-beta.11.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
MCPForUnity/Editor/Tools/Profiler/ManageProfiler.cs |
Unity tool entrypoint dispatching 14 profiler-related actions |
MCPForUnity/Editor/Tools/Profiler/Operations/SessionOps.cs |
Start/stop/status/area toggles for Unity Profiler session |
MCPForUnity/Editor/Tools/Profiler/Operations/CounterOps.cs |
Generic counter reads via ProfilerRecorder with 1-frame wait |
MCPForUnity/Editor/Tools/Profiler/Operations/FrameTimingOps.cs |
Frame timing capture via FrameTimingManager |
MCPForUnity/Editor/Tools/Profiler/Operations/ObjectMemoryOps.cs |
Per-object runtime memory sizing for scene objects/assets |
MCPForUnity/Editor/Tools/Profiler/Operations/MemorySnapshotOps.cs |
Memory Profiler snapshot take/list/compare via reflection |
MCPForUnity/Editor/Tools/Profiler/Operations/FrameDebuggerOps.cs |
Frame Debugger enable/disable + best-effort event extraction via reflection |
Server/src/services/tools/manage_profiler.py |
Server MCP tool wrapper forwarding profiler actions to Unity |
Server/tests/test_manage_profiler.py |
Unit tests for action validation/forwarding/tool registration |
Server/src/services/registry/tool_registry.py |
Adds the new profiling tool group description |
Server/src/cli/commands/profiler.py |
Adds CLI commands for the profiler tool actions |
Server/src/cli/main.py |
Registers optional CLI profiler command module |
manifest.json |
Adds manage_profiler to tool manifest + bumps version |
MCPForUnity/package.json |
Bumps Unity package version to 9.6.3-beta.11 |
Server/pyproject.toml |
Bumps Server package version to 9.6.3-beta.11 |
Server/README.md |
Updates install reference to v9.6.3-beta.11 |
README.md |
Documents new tool and adds it to the tool list |
docs/i18n/README-zh.md |
Chinese README update + tool list includes manage_profiler |
unity-mcp-skill/references/tools-reference.md |
Tool reference documentation for manage_profiler |
.claude/skills/unity-mcp-skill/references/tools-reference.md |
Tool reference documentation for manage_profiler |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try | ||
| { | ||
| if (EnableMethod != null) | ||
| { | ||
| EnableMethod.Invoke(null, new object[] { false, 0 }); | ||
| } | ||
| } |
There was a problem hiding this comment.
FrameDebuggerOps.Disable() has the same issue as Enable(): it invokes SetEnabled with two arguments (false, 0) even when the reflected method might be the 1-parameter overload. This will throw at runtime on Unity versions where only SetEnabled(bool) exists. Handle overloads explicitly (e.g., by parameter count) and fail fast if no suitable overload is available.
| // Try scene hierarchy first | ||
| var go = GameObject.Find(objectPath); | ||
| if (go != null) |
There was a problem hiding this comment.
GameObject.Find(objectPath) will not resolve hierarchy paths that start with a leading '/'. This is likely to fail with the suggested example /Player/Mesh (and the Python test also uses a leading slash). Consider normalizing objectPath (e.g., trim a single leading '/') before calling GameObject.Find, and keep examples consistent with what the resolver actually accepts.
| // Start recorders | ||
| var recorders = new List<ProfilerRecorder>(); | ||
| foreach (string name in counterNames) | ||
| { | ||
| recorders.Add(ProfilerRecorder.StartNew(category, name)); | ||
| } | ||
|
|
||
| // Wait 1 frame for recorders to accumulate data | ||
| await WaitOneFrameAsync(); | ||
|
|
||
| // Read values and dispose | ||
| var data = new Dictionary<string, object>(); | ||
| for (int i = 0; i < recorders.Count; i++) | ||
| { | ||
| var recorder = recorders[i]; | ||
| string name = counterNames[i]; | ||
| data[name] = recorder.Valid ? recorder.CurrentValueAsDouble : 0.0; | ||
| data[name + "_valid"] = recorder.Valid; | ||
| data[name + "_unit"] = recorder.Valid ? recorder.UnitType.ToString() : "Unknown"; | ||
| recorder.Dispose(); |
There was a problem hiding this comment.
Counter recorder creation isn't exception-safe: ProfilerRecorder.StartNew(category, name) can throw for invalid/unavailable counter names, and any recorders already started will leak because disposal only happens in the later read loop. Wrap recorder start/read logic in a try/finally that disposes all successfully-created recorders, and consider returning a per-counter error/valid flag when StartNew fails instead of failing the whole request.
| // Start recorders | |
| var recorders = new List<ProfilerRecorder>(); | |
| foreach (string name in counterNames) | |
| { | |
| recorders.Add(ProfilerRecorder.StartNew(category, name)); | |
| } | |
| // Wait 1 frame for recorders to accumulate data | |
| await WaitOneFrameAsync(); | |
| // Read values and dispose | |
| var data = new Dictionary<string, object>(); | |
| for (int i = 0; i < recorders.Count; i++) | |
| { | |
| var recorder = recorders[i]; | |
| string name = counterNames[i]; | |
| data[name] = recorder.Valid ? recorder.CurrentValueAsDouble : 0.0; | |
| data[name + "_valid"] = recorder.Valid; | |
| data[name + "_unit"] = recorder.Valid ? recorder.UnitType.ToString() : "Unknown"; | |
| recorder.Dispose(); | |
| // Start recorders and collect data in an exception-safe way | |
| var recorders = new List<ProfilerRecorder>(); | |
| var recorderNames = new List<string>(); | |
| var data = new Dictionary<string, object>(); | |
| try | |
| { | |
| // Create recorders per counter, but don't fail the entire request | |
| // if a specific counter is invalid/unavailable. | |
| foreach (string name in counterNames) | |
| { | |
| try | |
| { | |
| var recorder = ProfilerRecorder.StartNew(category, name); | |
| recorders.Add(recorder); | |
| recorderNames.Add(name); | |
| } | |
| catch (Exception ex) | |
| { | |
| // Record per-counter error information while keeping the | |
| // overall request alive. | |
| data[name] = 0.0; | |
| data[name + "_valid"] = false; | |
| data[name + "_unit"] = "Unknown"; | |
| data[name + "_error"] = ex.Message; | |
| } | |
| } | |
| // Wait 1 frame for recorders to accumulate data (if any were created) | |
| if (recorders.Count > 0) | |
| { | |
| await WaitOneFrameAsync(); | |
| } | |
| // Read values from successfully created recorders | |
| for (int i = 0; i < recorders.Count; i++) | |
| { | |
| var recorder = recorders[i]; | |
| string name = recorderNames[i]; | |
| data[name] = recorder.Valid ? recorder.CurrentValueAsDouble : 0.0; | |
| data[name + "_valid"] = recorder.Valid; | |
| data[name + "_unit"] = recorder.Valid ? recorder.UnitType.ToString() : "Unknown"; | |
| } | |
| } | |
| finally | |
| { | |
| // Ensure all successfully created recorders are disposed even if | |
| // an exception occurs during creation, waiting, or reading. | |
| for (int i = 0; i < recorders.Count; i++) | |
| { | |
| recorders[i].Dispose(); | |
| } |
| catch (Exception ex) | ||
| { | ||
| McpLog.Error($"[ManageProfiler] Action '{action}' failed: {ex}"); | ||
| return new ErrorResponse($"Error in action '{action}': {ex.Message}"); |
There was a problem hiding this comment.
ManageProfiler.HandleCommand() exception handling returns ErrorResponse without a stack trace payload, which is inconsistent with the pattern used by other tools (e.g., ManageBuild/ManageMaterial return new ErrorResponse(ex.Message, new { stackTrace = ex.StackTrace })). Including stackTrace here would improve debuggability and keep error shapes consistent across tools.
| return new ErrorResponse($"Error in action '{action}': {ex.Message}"); | |
| return new ErrorResponse($"Error in action '{action}': {ex.Message}", new { stackTrace = ex.StackTrace }); |
| try | ||
| { | ||
| if (EnableMethod != null) | ||
| { | ||
| EnableMethod.Invoke(null, new object[] { true, 0 }); | ||
| } | ||
| } |
There was a problem hiding this comment.
FrameDebuggerOps.Enable() always invokes SetEnabled with two arguments (true, 0). If the reflected overload is SetEnabled(bool) (possible because the fallback GetMethod("SetEnabled") is used), this will throw TargetParameterCountException and prevent enabling on those Unity versions. Consider branching on EnableMethod.GetParameters().Length (or resolving exact overloads) and return an error when no compatible overload is found; also avoid reporting enabled=true if the call was not actually made.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 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/Profiler/Operations/CounterOps.cs`:
- Around line 29-42: Validate any explicit counter names against the resolved
category handles before starting recorders (ensure names provided by the caller
exist in the list returned by GetRequestedCounters for the given categoryName)
and then wrap recorder creation and usage in a try/finally so any
ProfilerRecorder instances started with ProfilerRecorder.StartNew are always
disposed; specifically, when iterating counterNames to call
ProfilerRecorder.StartNew, guard against StartNew throwing by disposing any
recorders created so far, perform the awaited work (e.g., WaitOneFrameAsync and
read loop) inside the try, and in the finally iterate the recorders list to
Stop/Dispose them (follow the same pattern used in RenderingStatsOps.cs / using
var).
In `@MCPForUnity/Editor/Tools/Profiler/Operations/FrameDebuggerOps.cs`:
- Line 33: ReflectionAvailable currently ignores whether EnableMethod was
resolved, so Enable() and Disable() can return success even when EnableMethod is
null; update the logic so that ReflectionAvailable includes EnableMethod (i.e.,
require UtilType != null && EventCountProp != null && EnableMethod != null) or,
alternatively, explicitly check EnableMethod inside Enable() and Disable() and
return an ErrorResponse when EnableMethod == null before attempting invocation;
reference the ReflectionAvailable property and the Enable(), Disable(), and
EnableMethod identifiers when making the change.
- Around line 119-121: Validate and clamp pagination inputs before using them:
in FrameDebuggerOps where you create ToolParams and read page_size and cursor,
ensure cursor is >= 0 (reject or set to 0) and page_size > 0, then cap page_size
to a reasonable maximum (e.g. MAX_PAGE_SIZE) before computing end and entering
the loop; also ensure end is bounded by the available frame count (e.g.,
Math.Min(cursor + page_size, totalFrames)) to avoid negative indices and
unbounded GetFrameEventData calls. Apply the same validation in the other
pagination block around the code referenced at lines 147–150.
In `@MCPForUnity/Editor/Tools/Profiler/Operations/ObjectMemoryOps.cs`:
- Around line 21-23: The current lookup uses GameObject.Find(objectPath) which
fails for multi-level paths and inactive objects; replace this with a
hierarchy-aware resolver: split objectPath on '/' and traverse child transforms
(using Transform.Find or manual traversal) starting from scene root GameObjects
(or use GetComponentsInChildren<Transform>(true) to include inactive) to find
nested parts; update the code around the GameObject.Find call in ObjectMemoryOps
(the variable objectPath and the go lookup) to use the same FindGameObjectByPath
pattern from ManageRuntimeCompilation.cs so multi-level paths and inactive
GameObjects are resolved correctly.
In `@MCPForUnity/Editor/Tools/Profiler/Operations/SessionOps.cs`:
- Line 87: The direct call to prop.Value.ToObject<bool>() can throw on
non-boolean payloads; replace it with a safe validation step: check
prop.Value.Type == JTokenType.Boolean or attempt
bool.TryParse(prop.Value.ToString(), out var enabled) and only set enabled when
parsing succeeds, otherwise return the explicit ErrorResponse for bad input.
Update the code around the ToObject<bool>() usage in SessionOps (the prop/
enabled logic) to perform this validation and produce the existing ErrorResponse
contract when parsing fails.
- Around line 21-37: Start should be idempotent and return the profiler's
effective runtime state rather than echoing the requested parameters: in the
Start method, only enable recording or allocation callstacks when the incoming
flags (logFile, enableCallstacks) are explicitly set to true and do not disable
previously-enabled settings when they are omitted; compute the returned fields
(enabled, recording, log_file, allocation_callstacks) from the actual Profiler
properties (Profiler.enabled, Profiler.enableBinaryLog, Profiler.logFile,
Profiler.enableAllocationCallstacks) instead of the local requested variables so
repeated Start calls reflect the real runtime state.
In `@Server/src/cli/commands/profiler.py`:
- Around line 6-13: The profiler CLI group is missing the ping subcommand; add a
new subcommand by defining a function decorated with `@profiler.command`("ping")
(e.g., def ping(): ...) that calls the existing ping utility on manage_profiler
(import manage_profiler or the specific ping function) and surfaces the
result/exit code to the CLI (print or click.echo and set appropriate exit
behavior). Ensure the new ping function is placed alongside the other profiler
subcommands and that required imports for manage_profiler and click are present
so the command is registered under the "profiler" click.group.
- Around line 55-58: The current area-parsing loop silently treats malformed
values as False; update the parsing in the area handling block so each item in
area must contain an "=" and the RHS must be one of the allowed truthy or falsy
tokens ("true","1","yes","false","0","no") (case-insensitive); on malformed
input (missing "=" or unrecognized RHS) raise a ValueError or print a clear
error and exit instead of defaulting to False; keep the resulting dict named
areas and then call run_command("manage_profiler",
{"action":"profiler_set_areas","areas":areas}, config) as before so only
validated booleans are passed to manage_profiler.
In `@Server/tests/test_manage_profiler.py`:
- Around line 167-217: Tests use hardcoded "/tmp/..." paths which triggers Ruff
S108; update the three tests (test_profiler_start_forwards_log_file,
test_memory_take_snapshot_forwards_path,
test_memory_compare_forwards_both_paths) to accept the pytest tmp_path fixture
and pass string paths created from tmp_path (e.g., str(tmp_path /
"profile.raw"), str(tmp_path / "snap.snap"), str(tmp_path / "a.snap") /
"b.snap") into manage_profiler so manage_profiler calls remain unchanged; no
other logic needs changing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6f29ed2c-0642-43ff-b827-8ad94fa4d051
📒 Files selected for processing (20)
.claude/skills/unity-mcp-skill/references/tools-reference.mdMCPForUnity/Editor/Tools/Profiler/ManageProfiler.csMCPForUnity/Editor/Tools/Profiler/Operations/CounterOps.csMCPForUnity/Editor/Tools/Profiler/Operations/FrameDebuggerOps.csMCPForUnity/Editor/Tools/Profiler/Operations/FrameTimingOps.csMCPForUnity/Editor/Tools/Profiler/Operations/MemorySnapshotOps.csMCPForUnity/Editor/Tools/Profiler/Operations/ObjectMemoryOps.csMCPForUnity/Editor/Tools/Profiler/Operations/SessionOps.csMCPForUnity/package.jsonREADME.mdServer/README.mdServer/pyproject.tomlServer/src/cli/commands/profiler.pyServer/src/cli/main.pyServer/src/services/registry/tool_registry.pyServer/src/services/tools/manage_profiler.pyServer/tests/test_manage_profiler.pydocs/i18n/README-zh.mdmanifest.jsonunity-mcp-skill/references/tools-reference.md
| var counterNames = GetRequestedCounters(p, category); | ||
| if (counterNames.Count == 0) | ||
| return new SuccessResponse($"No counters found in category '{categoryName}'.", new | ||
| { | ||
| category = categoryName, | ||
| counters = new Dictionary<string, object>() | ||
| }); | ||
|
|
||
| // Start recorders | ||
| var recorders = new List<ProfilerRecorder>(); | ||
| foreach (string name in counterNames) | ||
| { | ||
| recorders.Add(ProfilerRecorder.StartNew(category, name)); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n MCPForUnity/Editor/Tools/Profiler/Operations/CounterOps.csRepository: CoplayDev/unity-mcp
Length of output: 6643
🌐 Web query:
ProfilerRecorder.StartNew exception throw behavior Unity documentation
💡 Result:
According to the official Unity documentation for ProfilerRecorder.StartNew (Unity 6.3 LTS and earlier versions), the method throws an exception if the capacity parameter is not greater than 0. The note states: "capacity parameter must be greater than 0, otherwise StartNew throws an exception." This is the primary documented exception-throwing behavior. No other specific exception conditions (e.g., invalid category, statName, or options) are explicitly mentioned in the API docs. Unity forum discussions suggest potential issues like passing negative capacity (e.g., -1 from targetFrameRate) can lead to memory allocator overflows or crashes, but these are not formally documented as exceptions from StartNew itself. Always ensure capacity > 0 when calling ProfilerRecorder.StartNew(ProfilerCategory category, string statName, int capacity, ProfilerRecorderOptions options) or similar overloads. Dispose recorders properly when done.
Citations:
- 1: https://docs.unity3d.com/6000.3/Documentation/ScriptReference/Unity.Profiling.ProfilerRecorder.StartNew.html
- 2: https://docs.unity3d.com/6000.2/Documentation/ScriptReference/Unity.Profiling.ProfilerRecorder.StartNew.html
- 3: https://docs.unity3d.com/2020.3/Documentation/ScriptReference/Unity.Profiling.ProfilerRecorder.StartNew.html
- 4: https://docs.unity3d.com/ScriptReference/Unity.Profiling.ProfilerRecorder.StartNew.html
- 5: https://discussions.unity.com/t/profilerrecorder-causing-overflow-in-memory-allocator/914860
🌐 Web query:
Unity ProfilerRecorder.StartNew(ProfilerCategory, string) exceptions invalid counter name
💡 Result:
There is no overload of ProfilerRecorder.StartNew(ProfilerCategory, string). The documented overloads require additional parameters: StartNew(ProfilerCategory category, string statName, int capacity, ProfilerRecorderOptions options) or StartNew(ProfilerMarker marker, int capacity, ProfilerRecorderOptions options). Calling a non-existent overload will result in a compile-time error, not a runtime exception for "invalid counter name". The official documentation explicitly states that StartNew throws an exception if capacity <= 0. Examples in the docs use the full 4-parameter overload, such as ProfilerRecorder.StartNew(ProfilerCategory.Memory, "System Used Memory") which implies default values for capacity and options in some contexts or C# overload resolution. If using the correct overload with an invalid statName (non-existent marker/counter in the category), the ProfilerRecorder.Valid property will be false, but no exception is documented for invalid names. Forum discussions indicate ProfilerRecorder may not work or return invalid for some statNames that don't exist. To check available stats, use ProfilerRecorderHandle.GetAvailable and ProfilerRecorderDescription.
Citations:
- 1: https://docs.unity3d.com/6000.3/Documentation/ScriptReference/Unity.Profiling.ProfilerRecorder.StartNew.html
- 2: https://docs.unity3d.com/6000.2/Documentation/ScriptReference/Unity.Profiling.ProfilerRecorder.StartNew.html
- 3: https://docs.unity3d.com/2022.3/Documentation/ScriptReference/Unity.Profiling.ProfilerRecorder-ctor.html
- 4: https://docs.unity3d.com/6000.3/Documentation/ScriptReference/Unity.Profiling.ProfilerRecorder.html
- 5: https://docs.unity3d.com/ScriptReference/Unity.Profiling.ProfilerRecorder.StartNew.html
- 6: https://docs.unity3d.com/6000.0/Documentation/ScriptReference/Unity.Profiling.ProfilerRecorder.html
- 7: https://docs.unity3d.com/2022.3/Documentation/ScriptReference/Unity.Profiling.LowLevel.Unsafe.ProfilerRecorderDescription.html
- 8: https://docs.unity3d.com/ScriptReference/Unity.Profiling.LowLevel.Unsafe.ProfilerRecorderDescription.html
🏁 Script executed:
grep -n "GetCountersAsync" MCPForUnity/Editor/Tools/Profiler/Operations/CounterOps.csRepository: CoplayDev/unity-mcp
Length of output: 142
🏁 Script executed:
rg "ProfilerRecorder.StartNew" --type cs -B 2 -A 2Repository: CoplayDev/unity-mcp
Length of output: 1059
🏁 Script executed:
cat -n MCPForUnity/Editor/Tools/Graphics/RenderingStatsOps.cs | head -50Repository: CoplayDev/unity-mcp
Length of output: 2433
Wrap recorder lifetime in try/finally and validate explicit counter names.
The explicit counters path bypasses validation against the resolved category. Additionally, recorder disposal only happens in the happy path. If ProfilerRecorder.StartNew() throws or if WaitOneFrameAsync() or the read loop fails, created recorders are leaked. Use try/finally to guarantee disposal (as shown in RenderingStatsOps.cs with using var), and validate explicit counter names against available handles before starting recorders.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@MCPForUnity/Editor/Tools/Profiler/Operations/CounterOps.cs` around lines 29 -
42, Validate any explicit counter names against the resolved category handles
before starting recorders (ensure names provided by the caller exist in the list
returned by GetRequestedCounters for the given categoryName) and then wrap
recorder creation and usage in a try/finally so any ProfilerRecorder instances
started with ProfilerRecorder.StartNew are always disposed; specifically, when
iterating counterNames to call ProfilerRecorder.StartNew, guard against StartNew
throwing by disposing any recorders created so far, perform the awaited work
(e.g., WaitOneFrameAsync and read loop) inside the try, and in the finally
iterate the recorders list to Stop/Dispose them (follow the same pattern used in
RenderingStatsOps.cs / using var).
| null, new[] { typeof(int) }, null) | ||
| ?? UtilType.GetMethod("GetFrameEventData", BindingFlags.Public | BindingFlags.Static); | ||
| } | ||
| ReflectionAvailable = UtilType != null && EventCountProp != null; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n MCPForUnity/Editor/Tools/Profiler/Operations/FrameDebuggerOps.csRepository: CoplayDev/unity-mcp
Length of output: 9708
Don't return a successful toggle when SetEnabled was never resolved.
ReflectionAvailable only checks UtilType and EventCountProp but not EnableMethod. When EnableMethod fails to resolve via reflection, Enable() and Disable() skip the invocation and still return success, falsely indicating the operation succeeded. Both methods should return an ErrorResponse when EnableMethod == null instead of silently continuing.
Suggested fix
- if (!ReflectionAvailable)
+ if (!ReflectionAvailable || EnableMethod == null)
{
return new ErrorResponse(
- "FrameDebuggerUtility not available via reflection in this Unity version.");
+ "FrameDebuggerUtility.SetEnabled not available via reflection in this Unity version.");
}
try
{
- if (EnableMethod != null)
- {
- EnableMethod.Invoke(null, new object[] { true, 0 });
- }
+ EnableMethod.Invoke(null, new object[] { true, 0 });Also applies to: Disable method (lines 85–89, 93–96)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@MCPForUnity/Editor/Tools/Profiler/Operations/FrameDebuggerOps.cs` at line 33,
ReflectionAvailable currently ignores whether EnableMethod was resolved, so
Enable() and Disable() can return success even when EnableMethod is null; update
the logic so that ReflectionAvailable includes EnableMethod (i.e., require
UtilType != null && EventCountProp != null && EnableMethod != null) or,
alternatively, explicitly check EnableMethod inside Enable() and Disable() and
return an ErrorResponse when EnableMethod == null before attempting invocation;
reference the ReflectionAvailable property and the Enable(), Disable(), and
EnableMethod identifiers when making the change.
| var p = new ToolParams(@params); | ||
| int pageSize = p.GetInt("page_size") ?? 50; | ||
| int cursor = p.GetInt("cursor") ?? 0; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "FrameDebuggerOps.cs" -type fRepository: CoplayDev/unity-mcp
Length of output: 129
🏁 Script executed:
cat -n ./MCPForUnity/Editor/Tools/Profiler/Operations/FrameDebuggerOps.cs | head -200Repository: CoplayDev/unity-mcp
Length of output: 9043
🏁 Script executed:
grep -r "class ToolParams" . --include="*.cs"Repository: CoplayDev/unity-mcp
Length of output: 244
🏁 Script executed:
cat -n ./MCPForUnity/Editor/Helpers/ToolParams.csRepository: CoplayDev/unity-mcp
Length of output: 10721
🏁 Script executed:
wc -l ./MCPForUnity/Editor/Tools/Profiler/Operations/FrameDebuggerOps.csRepository: CoplayDev/unity-mcp
Length of output: 133
Validate pagination parameters before using them in loops.
page_size and cursor lack input validation. Negative cursor values cause the loop to start at negative indices, making reflective calls with invalid frame event indices. Unbounded page_size allows a single request to trigger thousands of GetFrameEventData calls, creating a denial-of-service vector. Reject negative cursors, require page_size > 0, and cap the maximum page size before computing end.
Suggested fix
var p = new ToolParams(`@params`);
int pageSize = p.GetInt("page_size") ?? 50;
int cursor = p.GetInt("cursor") ?? 0;
+
+ if (cursor < 0)
+ return new ErrorResponse("cursor must be >= 0.");
+ if (pageSize <= 0)
+ return new ErrorResponse("page_size must be > 0.");
+ pageSize = Math.Min(pageSize, 200);Also applies to: 147-150
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@MCPForUnity/Editor/Tools/Profiler/Operations/FrameDebuggerOps.cs` around
lines 119 - 121, Validate and clamp pagination inputs before using them: in
FrameDebuggerOps where you create ToolParams and read page_size and cursor,
ensure cursor is >= 0 (reject or set to 0) and page_size > 0, then cap page_size
to a reasonable maximum (e.g. MAX_PAGE_SIZE) before computing end and entering
the loop; also ensure end is bounded by the available frame count (e.g.,
Math.Min(cursor + page_size, totalFrames)) to avoid negative indices and
unbounded GetFrameEventData calls. Apply the same validation in the other
pagination block around the code referenced at lines 147–150.
| var takeMethod = MemoryProfilerType.GetMethod("TakeSnapshot", | ||
| new[] { typeof(string), typeof(Action<string, bool>), typeof(Action<string, bool, UnityEditor.DebugScreenCapture>), typeof(uint) }); | ||
|
|
||
| if (takeMethod == null) | ||
| { | ||
| // Try 2-param overload: TakeSnapshot(string, Action<string, bool>) | ||
| takeMethod = MemoryProfilerType.GetMethod("TakeSnapshot", | ||
| new[] { typeof(string), typeof(Action<string, bool>) }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "MemorySnapshotOps.cs" -type fRepository: CoplayDev/unity-mcp
Length of output: 130
🏁 Script executed:
cat -n MCPForUnity/Editor/Tools/Profiler/Operations/MemorySnapshotOps.cs | sed -n '25,85p'Repository: CoplayDev/unity-mcp
Length of output: 3318
🏁 Script executed:
rg -i "captureflag|takesnapshot" --type cs -B 2 -A 2Repository: CoplayDev/unity-mcp
Length of output: 3626
🌐 Web query:
Unity MemoryProfiler.TakeSnapshot official API documentation CaptureFlags
💡 Result:
The official API documentation for Unity's MemoryProfiler.TakeSnapshot describes CaptureFlags as a flag mask (Unity.Profiling.Memory.CaptureFlags enum) that defines the content of the memory snapshot. TakeSnapshot method signatures: - public static void TakeSnapshot(string path, Action finishCallback, Unity.Profiling.Memory.CaptureFlags captureFlags); - public static void TakeSnapshot(string path, Action finishCallback, Action screenshotCallback, Unity.Profiling.Memory.CaptureFlags captureFlags); CaptureFlags enum values: - ManagedObjects: Captures the entire managed heap, types, field info, and connections from native to managed objects. - NativeObjects: Collects information about Native Objects. - NativeAllocations: Captures native memory used by Unity allocations, including subsystems, UnsafeUtility.Malloc, NativeArray, native collections, allocators, and reserved memory. Enables NativeAllocationSites and NativeStackTraces. - NativeAllocationSites: Specifies the allocation location for each native allocation (requires NativeAllocations and native callstack support). - NativeStackTraces: Collects call-stack symbols for native allocations (requires NativeAllocations, NativeAllocationSites, and source code access for builds). Recommended for general use: CaptureFlags.ManagedObjects | CaptureFlags.NativeObjects | CaptureFlags.NativeAllocations. Not all flags are collected depending on the build target; e.g., native stack data requires specific build support. Example usage: MemoryProfiler.TakeSnapshot("snapshot.tmpsnap", (path, success) => { /* handle */ }, CaptureFlags.ManagedObjects | CaptureFlags.NativeObjects | CaptureFlags.NativeAllocations); Official docs: Unity 6.0+ (current as of 2026).
Citations:
- 1: https://docs.unity3d.com/6000.3/Documentation/ScriptReference/Unity.Profiling.Memory.MemoryProfiler.TakeSnapshot.html
- 2: https://docs.unity3d.com/ScriptReference/Unity.Profiling.Memory.CaptureFlags.html
- 3: https://docs.unity3d.com/ScriptReference/Unity.Profiling.Memory.MemoryProfiler.TakeSnapshot.html
- 4: https://docs.unity3d.com/ScriptReference/Unity.Profiling.Memory.CaptureFlags.NativeStackTraces.html
- 5: https://docs.unity3d.com/ScriptReference/Unity.Profiling.Memory.MemoryProfiler.html
- 6: https://docs.unity3d.com/Packages/com.unity.memoryprofiler@0.2/api/Unity.MemoryProfiler.Editor.Format.CaptureFlags.html
- 7: https://docs.unity3d.com/ScriptReference/Unity.Profiling.Memory.CaptureFlags.ManagedObjects.html
- 8: https://docs.unity3d.com/2022.3/Documentation/ScriptReference/Unity.Profiling.Memory.CaptureFlags.html
- 9: https://docs.unity3d.com/ScriptReference/Unity.Profiling.Memory.CaptureFlags.NativeAllocations.html
🏁 Script executed:
find . -name "package.json" -o -name "package-lock.json" -o -name "manifest.json" | head -20Repository: CoplayDev/unity-mcp
Length of output: 296
🏁 Script executed:
rg -i "unity.profiling.memory|memoryprofiler" --type json -B 2 -A 2Repository: CoplayDev/unity-mcp
Length of output: 45
🏁 Script executed:
cat -n manifest.json | head -50Repository: CoplayDev/unity-mcp
Length of output: 1768
🏁 Script executed:
rg "com.unity.memoryprofiler" -B 1 -A 1Repository: CoplayDev/unity-mcp
Length of output: 3000
🏁 Script executed:
rg "CaptureFlags|targetFrameworkVersion" --type cs -A 1 | head -40Repository: CoplayDev/unity-mcp
Length of output: 45
🏁 Script executed:
rg "MemoryProfilerType\s*=" -B 2 -A 2Repository: CoplayDev/unity-mcp
Length of output: 595
🏁 Script executed:
# Verify if there's any local documentation or comments about CaptureFlags in the codebase
rg "using.*Profiling|using.*MemoryProfiler" --type csRepository: CoplayDev/unity-mcp
Length of output: 679
🏁 Script executed:
# Check what the actual parameter count logic expects
cat -n MCPForUnity/Editor/Tools/Profiler/Operations/MemorySnapshotOps.cs | sed -n '66,75p'Repository: CoplayDev/unity-mcp
Length of output: 641
Fix TakeSnapshot reflection signatures and parameter types to match official Unity API.
The code attempts to match (string, Action<string,bool>, Action<string,bool,DebugScreenCapture>, uint) and falls back to (string, Action<string,bool>), but the official Unity API uses CaptureFlags enum for the 4-parameter overload and adds a documented 3-parameter variant: (string, Action, CaptureFlags). Replace typeof(uint) with the correct CaptureFlags type lookup and add the missing 3-parameter overload to the fallback chain. The invocation at lines 68–74 also needs to pass a valid CaptureFlags value instead of 0u for the 4-parameter case.
| // Try scene hierarchy first | ||
| var go = GameObject.Find(objectPath); | ||
| if (go != null) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find and read the ObjectMemoryOps.cs file
find . -name "ObjectMemoryOps.cs" -type fRepository: CoplayDev/unity-mcp
Length of output: 128
🏁 Script executed:
cat -n "./MCPForUnity/Editor/Tools/Profiler/Operations/ObjectMemoryOps.cs"Repository: CoplayDev/unity-mcp
Length of output: 2449
🏁 Script executed:
# Check if there are any tests for this class
find . -name "*ObjectMemoryOps*" -o -name "*Profiler*Test*" | grep -i test
# Also check the ToolParams class to understand parameter handling
find . -name "ToolParams.cs" -type fRepository: CoplayDev/unity-mcp
Length of output: 105
🏁 Script executed:
cat -n "./MCPForUnity/Editor/Helpers/ToolParams.cs"Repository: CoplayDev/unity-mcp
Length of output: 10721
🏁 Script executed:
# Search for any documentation or comments about inactive objects
rg -i "inactive|active|gameobject.find" --type cs -A 3 -B 3
# Check if there are any related tests
find . -path "*/Test*" -name "*.cs" | xargs grep -l "ObjectMemoryOps\|GetObjectMemory" 2>/dev/null
# Check the Unity API behavior of GameObject.Find
# Also look for any hierarchy traversal methods being used elsewhere
rg "Find\|GetChildren\|GetComponent" --type cs -l | head -20Repository: CoplayDev/unity-mcp
Length of output: 50376
🏁 Script executed:
# Look for more context about the GetObjectMemory method and its intended usage
rg "GetObjectMemory|object_path" --type cs -B 3 -A 3
# Check if there are any documentation files or README that explains tool usage
find . -name "README*" -o -name "*.md" | head -10Repository: CoplayDev/unity-mcp
Length of output: 2134
🏁 Script executed:
# Check the tools reference for get_object_memory documentation
rg "get_object_memory" -A 10 -B 5 ./.claude/skills/unity-mcp-skill/references/tools-reference.mdRepository: CoplayDev/unity-mcp
Length of output: 2680
🏁 Script executed:
# Check if there are any tests for ObjectMemoryOps or GetObjectMemory
find . -path "*/Test*" -name "*.cs" -o -path "*/test*" -name "*.cs" | xargs grep -l "ObjectMemoryOps\|get_object_memory" 2>/dev/null | head -5
# Also check how FindGameObjectByPath is implemented (from ManageRuntimeCompilation.cs)
cat -n CustomTools/RoslynRuntimeCompilation/ManageRuntimeCompilation.cs | sed -n '1,300p' | grep -A 30 "FindGameObjectByPath"Repository: CoplayDev/unity-mcp
Length of output: 1664
🏁 Script executed:
# Find the FindGameObjectByPath method definition more clearly
rg "private static GameObject FindGameObjectByPath" -A 30 CustomTools/RoslynRuntimeCompilation/ManageRuntimeCompilation.csRepository: CoplayDev/unity-mcp
Length of output: 1144
Hierarchy lookup is incomplete: doesn't support multi-level paths or inactive objects.
object_path is documented as supporting hierarchy paths (e.g. "Player/Mesh"), but GameObject.Find is used only for active GameObjects by name, not path traversal. This means:
- Multi-level paths like "Player/Mesh" won't resolve (finds object named "Player/Mesh", not the nested hierarchy)
- Inactive GameObjects in the hierarchy are skipped entirely
See FindGameObjectByPath in ManageRuntimeCompilation.cs for the established pattern: split the path, traverse with Transform.Find for proper hierarchy resolution, and support inactive objects with GetComponentsInChildren<Transform>(true) where needed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@MCPForUnity/Editor/Tools/Profiler/Operations/ObjectMemoryOps.cs` around lines
21 - 23, The current lookup uses GameObject.Find(objectPath) which fails for
multi-level paths and inactive objects; replace this with a hierarchy-aware
resolver: split objectPath on '/' and traverse child transforms (using
Transform.Find or manual traversal) starting from scene root GameObjects (or use
GetComponentsInChildren<Transform>(true) to include inactive) to find nested
parts; update the code around the GameObject.Find call in ObjectMemoryOps (the
variable objectPath and the go lookup) to use the same FindGameObjectByPath
pattern from ManageRuntimeCompilation.cs so multi-level paths and inactive
GameObjects are resolved correctly.
| @click.group("profiler") | ||
| def profiler(): | ||
| """Unity Profiler session control, counter reads, memory snapshots, and Frame Debugger.""" | ||
| pass | ||
|
|
||
|
|
||
| # --- Session --- | ||
|
|
There was a problem hiding this comment.
Add missing ping subcommand for full profiler CLI surface.
manage_profiler includes a utility ping action, but this CLI group does not expose it, so users cannot quickly verify profiler availability from CLI.
Proposed fix
`@click.group`("profiler")
def profiler():
"""Unity Profiler session control, counter reads, memory snapshots, and Frame Debugger."""
pass
+@profiler.command("ping")
+@handle_unity_errors
+def ping():
+ """Verify profiler tool availability."""
+ config = get_config()
+ result = run_command("manage_profiler", {"action": "ping"}, config)
+ click.echo(format_output(result, config.format))
+
# --- Session ---🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Server/src/cli/commands/profiler.py` around lines 6 - 13, The profiler CLI
group is missing the ping subcommand; add a new subcommand by defining a
function decorated with `@profiler.command`("ping") (e.g., def ping(): ...) that
calls the existing ping utility on manage_profiler (import manage_profiler or
the specific ping function) and surfaces the result/exit code to the CLI (print
or click.echo and set appropriate exit behavior). Ensure the new ping function
is placed alongside the other profiler subcommands and that required imports for
manage_profiler and click are present so the command is registered under the
"profiler" click.group.
| for a in area: | ||
| name, _, val = a.partition("=") | ||
| areas[name.strip()] = val.strip().lower() in ("true", "1", "yes") | ||
| result = run_command("manage_profiler", {"action": "profiler_set_areas", "areas": areas}, config) |
There was a problem hiding this comment.
Reject malformed --area values instead of silently treating them as False.
Line 56-57 currently accepts invalid inputs like CPU or CPU=maybe and can unintentionally disable areas.
Proposed fix
def set_areas(area):
"""Toggle specific ProfilerAreas on or off."""
config = get_config()
areas = {}
+ truthy = {"true", "1", "yes"}
+ falsy = {"false", "0", "no"}
for a in area:
name, _, val = a.partition("=")
- areas[name.strip()] = val.strip().lower() in ("true", "1", "yes")
+ key = name.strip()
+ raw = val.strip().lower()
+ if not key or not _:
+ raise click.BadParameter(f"Invalid --area '{a}'. Expected Area=true|false.")
+ if raw not in truthy | falsy:
+ raise click.BadParameter(
+ f"Invalid boolean for --area '{a}'. Use true/false, 1/0, yes/no."
+ )
+ areas[key] = raw in truthy
result = run_command("manage_profiler", {"action": "profiler_set_areas", "areas": areas}, config)
click.echo(format_output(result, config.format))📝 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.
| for a in area: | |
| name, _, val = a.partition("=") | |
| areas[name.strip()] = val.strip().lower() in ("true", "1", "yes") | |
| result = run_command("manage_profiler", {"action": "profiler_set_areas", "areas": areas}, config) | |
| areas = {} | |
| truthy = {"true", "1", "yes"} | |
| falsy = {"false", "0", "no"} | |
| for a in area: | |
| name, _, val = a.partition("=") | |
| key = name.strip() | |
| raw = val.strip().lower() | |
| if not key or not _: | |
| raise click.BadParameter(f"Invalid --area '{a}'. Expected Area=true|false.") | |
| if raw not in truthy | falsy: | |
| raise click.BadParameter( | |
| f"Invalid boolean for --area '{a}'. Use true/false, 1/0, yes/no." | |
| ) | |
| areas[key] = raw in truthy | |
| result = run_command("manage_profiler", {"action": "profiler_set_areas", "areas": areas}, config) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Server/src/cli/commands/profiler.py` around lines 55 - 58, The current
area-parsing loop silently treats malformed values as False; update the parsing
in the area handling block so each item in area must contain an "=" and the RHS
must be one of the allowed truthy or falsy tokens
("true","1","yes","false","0","no") (case-insensitive); on malformed input
(missing "=" or unrecognized RHS) raise a ValueError or print a clear error and
exit instead of defaulting to False; keep the resulting dict named areas and
then call run_command("manage_profiler",
{"action":"profiler_set_areas","areas":areas}, config) as before so only
validated booleans are passed to manage_profiler.
| def test_profiler_start_forwards_log_file(mock_unity): | ||
| result = asyncio.run( | ||
| manage_profiler(SimpleNamespace(), action="profiler_start", log_file="/tmp/profile.raw") | ||
| ) | ||
| assert result["success"] is True | ||
| assert mock_unity["params"]["log_file"] == "/tmp/profile.raw" | ||
|
|
||
|
|
||
| def test_profiler_start_forwards_callstacks(mock_unity): | ||
| result = asyncio.run( | ||
| manage_profiler(SimpleNamespace(), action="profiler_start", enable_callstacks=True) | ||
| ) | ||
| assert result["success"] is True | ||
| assert mock_unity["params"]["enable_callstacks"] is True | ||
|
|
||
|
|
||
| def test_profiler_set_areas_forwards_areas(mock_unity): | ||
| areas = {"CPU": True, "Audio": False} | ||
| result = asyncio.run( | ||
| manage_profiler(SimpleNamespace(), action="profiler_set_areas", areas=areas) | ||
| ) | ||
| assert result["success"] is True | ||
| assert mock_unity["params"]["areas"] == areas | ||
|
|
||
|
|
||
| def test_get_object_memory_forwards_path(mock_unity): | ||
| result = asyncio.run( | ||
| manage_profiler(SimpleNamespace(), action="get_object_memory", object_path="/Player/Mesh") | ||
| ) | ||
| assert result["success"] is True | ||
| assert mock_unity["params"]["object_path"] == "/Player/Mesh" | ||
|
|
||
|
|
||
| def test_memory_take_snapshot_forwards_path(mock_unity): | ||
| result = asyncio.run( | ||
| manage_profiler(SimpleNamespace(), action="memory_take_snapshot", snapshot_path="/tmp/snap.snap") | ||
| ) | ||
| assert result["success"] is True | ||
| assert mock_unity["params"]["snapshot_path"] == "/tmp/snap.snap" | ||
|
|
||
|
|
||
| def test_memory_compare_forwards_both_paths(mock_unity): | ||
| result = asyncio.run( | ||
| manage_profiler( | ||
| SimpleNamespace(), action="memory_compare_snapshots", | ||
| snapshot_a="/tmp/a.snap", snapshot_b="/tmp/b.snap", | ||
| ) | ||
| ) | ||
| assert result["success"] is True | ||
| assert mock_unity["params"]["snapshot_a"] == "/tmp/a.snap" | ||
| assert mock_unity["params"]["snapshot_b"] == "/tmp/b.snap" |
There was a problem hiding this comment.
Replace hardcoded /tmp/... paths with pytest temp fixtures.
Lines 169/202/212 use fixed temp paths, which trips Ruff S108 and can break lint CI.
Proposed fix pattern
-def test_profiler_start_forwards_log_file(mock_unity):
+def test_profiler_start_forwards_log_file(mock_unity, tmp_path):
+ profile_path = tmp_path / "profile.raw"
result = asyncio.run(
- manage_profiler(SimpleNamespace(), action="profiler_start", log_file="/tmp/profile.raw")
+ manage_profiler(SimpleNamespace(), action="profiler_start", log_file=str(profile_path))
)
assert result["success"] is True
- assert mock_unity["params"]["log_file"] == "/tmp/profile.raw"
+ assert mock_unity["params"]["log_file"] == str(profile_path)Apply the same tmp_path pattern to snapshot path tests.
🧰 Tools
🪛 Ruff (0.15.7)
[error] 169-169: Probable insecure usage of temporary file or directory: "/tmp/profile.raw"
(S108)
[error] 172-172: Probable insecure usage of temporary file or directory: "/tmp/profile.raw"
(S108)
[error] 202-202: Probable insecure usage of temporary file or directory: "/tmp/snap.snap"
(S108)
[error] 205-205: Probable insecure usage of temporary file or directory: "/tmp/snap.snap"
(S108)
[error] 212-212: Probable insecure usage of temporary file or directory: "/tmp/a.snap"
(S108)
[error] 212-212: Probable insecure usage of temporary file or directory: "/tmp/b.snap"
(S108)
[error] 216-216: Probable insecure usage of temporary file or directory: "/tmp/a.snap"
(S108)
[error] 217-217: Probable insecure usage of temporary file or directory: "/tmp/b.snap"
(S108)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Server/tests/test_manage_profiler.py` around lines 167 - 217, Tests use
hardcoded "/tmp/..." paths which triggers Ruff S108; update the three tests
(test_profiler_start_forwards_log_file, test_memory_take_snapshot_forwards_path,
test_memory_compare_forwards_both_paths) to accept the pytest tmp_path fixture
and pass string paths created from tmp_path (e.g., str(tmp_path /
"profile.raw"), str(tmp_path / "snap.snap"), str(tmp_path / "a.snap") /
"b.snap") into manage_profiler so manage_profiler calls remain unchanged; no
other logic needs changing.
This reverts commit fbaa7ce.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
MCPForUnity/Editor/Tools/Profiler/Operations/CounterOps.cs (2)
37-64: Move recorder creation inside the try block for complete disposal safety.The try/finally now exists (addressing part of the prior feedback), but recorder creation at lines 39-42 occurs before the try block. If
StartNewthrows mid-loop (e.g., on the 3rd counter while 2 recorders already exist), those recorders leak. Moving the creation inside try ensures the finally block disposes whatever was successfully created.♻️ Proposed fix to fully encompass recorder lifecycle
- // Start recorders - var recorders = new List<ProfilerRecorder>(); - foreach (string name in counterNames) - { - recorders.Add(ProfilerRecorder.StartNew(category, name)); - } - var data = new Dictionary<string, object>(); + var recorders = new List<ProfilerRecorder>(); try { + // Start recorders + foreach (string name in counterNames) + { + recorders.Add(ProfilerRecorder.StartNew(category, name)); + } + // Wait 1 frame for recorders to accumulate data await WaitOneFrameAsync();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MCPForUnity/Editor/Tools/Profiler/Operations/CounterOps.cs` around lines 37 - 64, Move the ProfilerRecorder creation inside the try so partially-created recorders are always disposed; specifically, create the List<ProfilerRecorder> recorders and the loop that calls ProfilerRecorder.StartNew(category, name) within the try block (before awaiting WaitOneFrameAsync), leaving the finally block's disposal loop intact to call recorder.Dispose() for any recorders that were successfully added; reference the ProfilerRecorder type, StartNew method, the recorders list, and WaitOneFrameAsync to locate the code to change.
73-77: Consider validating explicit counter names upfront for clearer feedback.When the caller provides explicit counter names, they're used directly without checking if they exist in the category. While the code gracefully handles invalid counters (returning
0.0with_valid=false), validating upfront would provide clearer error messages. This is optional since the current behavior is safe.💡 Optional: validate explicit counters against available handles
private static List<string> GetRequestedCounters(ToolParams p, ProfilerCategory category) { var explicitCounters = p.GetStringArray("counters"); - if (explicitCounters != null && explicitCounters.Length > 0) - return explicitCounters.ToList(); - var allHandles = new List<ProfilerRecorderHandle>(); ProfilerRecorderHandle.GetAvailable(allHandles); - return allHandles + var availableNames = allHandles .Select(h => ProfilerRecorderHandle.GetDescription(h)) .Where(d => string.Equals(d.Category.Name, category.Name, StringComparison.OrdinalIgnoreCase)) .Select(d => d.Name) - .OrderBy(n => n) - .ToList(); + .ToHashSet(StringComparer.OrdinalIgnoreCase); + + if (explicitCounters != null && explicitCounters.Length > 0) + { + // Return only counters that actually exist; caller can check _valid metadata for others + return explicitCounters.Where(c => availableNames.Contains(c)).ToList(); + } + + return availableNames.OrderBy(n => n).ToList(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MCPForUnity/Editor/Tools/Profiler/Operations/CounterOps.cs` around lines 73 - 77, GetRequestedCounters currently returns explicitCounters directly without checking they exist on the provided ProfilerCategory; update GetRequestedCounters to validate each name in explicitCounters against the category's available counters (e.g., by calling a lookup method such as category.TryGetCounterHandle, category.GetAvailableCounterNames, category.GetCounterHandles or similar) and only return valid names (and report or log any invalid names via ToolParams or a diagnostic); use the function name GetRequestedCounters and variables explicitCounters and category to locate the code and implement the validation/filtering and optional logging for clearer feedback.MCPForUnity/Editor/Tools/Profiler/Operations/MemorySnapshotOps.cs (1)
109-123: Consider adding resilience for directory enumeration errors.
Directory.GetFilescan throwIOExceptionorUnauthorizedAccessException. If one directory fails, the entire method fails. Consider catching and logging errors per-directory to continue scanning remaining paths.♻️ Proposed fix
foreach (string dir in dirs) { if (!Directory.Exists(dir)) continue; - foreach (string file in Directory.GetFiles(dir, "*.snap")) + try { - var fi = new FileInfo(file); - snapshots.Add(new + foreach (string file in Directory.GetFiles(dir, "*.snap")) { - path = fi.FullName, - size_bytes = fi.Length, - size_mb = Math.Round(fi.Length / (1024.0 * 1024.0), 2), - created = fi.CreationTimeUtc.ToString("o"), - }); + var fi = new FileInfo(file); + snapshots.Add(new + { + path = fi.FullName, + size_bytes = fi.Length, + size_mb = Math.Round(fi.Length / (1024.0 * 1024.0), 2), + created = fi.CreationTimeUtc.ToString("o"), + }); + } + } + catch (Exception ex) + { + Debug.LogWarning($"[MemorySnapshotOps] Failed to enumerate {dir}: {ex.Message}"); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MCPForUnity/Editor/Tools/Profiler/Operations/MemorySnapshotOps.cs` around lines 109 - 123, The directory enumeration can throw and stop the whole scan; modify the foreach (string dir in dirs) loop in MemorySnapshotOps to wrap the Directory.GetFiles/Directory.Exists block in a try-catch that catches IOException and UnauthorizedAccessException (optionally Exception for safety), logs the error along with the failing dir and exception message, and continues to the next directory so snapshots.Add calls still run for other dirs; keep the existing anonymous object creation (path/size_bytes/size_mb/created) unchanged but ensure any exception does not abort the outer loop.
🤖 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/Profiler/Operations/MemorySnapshotOps.cs`:
- Around line 50-66: The callback Action<string,bool> (named callback) currently
constructs a FileInfo and sets tcs based on result without catching exceptions;
if FileInfo throws the exception is swallowed by Unity and tcs is never
completed, causing a hang. Wrap the callback body in try/catch, ensure any
exception sets tcs.TrySetResult with an ErrorResponse containing the exception
message (and include the path/context), and retain the existing success/error
branches to set SuccessResponse or ErrorResponse (use the same SuccessResponse
and ErrorResponse types) so the TaskCompletionSource (tcs) is always completed
even on FileInfo or IO errors.
In `@MCPForUnity/Editor/Tools/Profiler/Operations/SessionOps.cs`:
- Around line 77-79: The code indexes `@params` before checking it for null which
can throw NullReferenceException; in the method containing the lines that create
areasToken and return ErrorResponse (look for the variables `@params`, areasToken
and the ErrorResponse return referencing AreaNames in SessionOps.cs), first
guard that `@params` is not null and return the same ErrorResponse (or an
equivalent parameter-required response) if it is null, then proceed to cast
`@params`["areas"] to JObject; ensure the null-check order is changed so `@params`
is validated before any indexing into it.
---
Nitpick comments:
In `@MCPForUnity/Editor/Tools/Profiler/Operations/CounterOps.cs`:
- Around line 37-64: Move the ProfilerRecorder creation inside the try so
partially-created recorders are always disposed; specifically, create the
List<ProfilerRecorder> recorders and the loop that calls
ProfilerRecorder.StartNew(category, name) within the try block (before awaiting
WaitOneFrameAsync), leaving the finally block's disposal loop intact to call
recorder.Dispose() for any recorders that were successfully added; reference the
ProfilerRecorder type, StartNew method, the recorders list, and
WaitOneFrameAsync to locate the code to change.
- Around line 73-77: GetRequestedCounters currently returns explicitCounters
directly without checking they exist on the provided ProfilerCategory; update
GetRequestedCounters to validate each name in explicitCounters against the
category's available counters (e.g., by calling a lookup method such as
category.TryGetCounterHandle, category.GetAvailableCounterNames,
category.GetCounterHandles or similar) and only return valid names (and report
or log any invalid names via ToolParams or a diagnostic); use the function name
GetRequestedCounters and variables explicitCounters and category to locate the
code and implement the validation/filtering and optional logging for clearer
feedback.
In `@MCPForUnity/Editor/Tools/Profiler/Operations/MemorySnapshotOps.cs`:
- Around line 109-123: The directory enumeration can throw and stop the whole
scan; modify the foreach (string dir in dirs) loop in MemorySnapshotOps to wrap
the Directory.GetFiles/Directory.Exists block in a try-catch that catches
IOException and UnauthorizedAccessException (optionally Exception for safety),
logs the error along with the failing dir and exception message, and continues
to the next directory so snapshots.Add calls still run for other dirs; keep the
existing anonymous object creation (path/size_bytes/size_mb/created) unchanged
but ensure any exception does not abort the outer loop.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 09c4f69e-9d78-476f-83b5-b1999cc854c2
📒 Files selected for processing (4)
MCPForUnity/Editor/Tools/Profiler/Operations/CounterOps.csMCPForUnity/Editor/Tools/Profiler/Operations/FrameDebuggerOps.csMCPForUnity/Editor/Tools/Profiler/Operations/MemorySnapshotOps.csMCPForUnity/Editor/Tools/Profiler/Operations/SessionOps.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- MCPForUnity/Editor/Tools/Profiler/Operations/FrameDebuggerOps.cs
| Action<string, bool> callback = (path, result) => | ||
| { | ||
| if (result) | ||
| { | ||
| var fi = new FileInfo(path); | ||
| tcs.TrySetResult(new SuccessResponse("Memory snapshot captured.", new | ||
| { | ||
| path, | ||
| size_bytes = fi.Exists ? fi.Length : 0, | ||
| size_mb = fi.Exists ? Math.Round(fi.Length / (1024.0 * 1024.0), 2) : 0, | ||
| })); | ||
| } | ||
| else | ||
| { | ||
| tcs.TrySetResult(new ErrorResponse($"Snapshot capture failed for path: {path}")); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Add exception handling inside the callback to prevent hangs.
If FileInfo operations throw (e.g., invalid path characters, access denied), the exception is swallowed by Unity's invoke mechanism, leaving tcs unset. This causes the method to hang until the 30-second timeout.
🛡️ Proposed fix
Action<string, bool> callback = (path, result) =>
{
- if (result)
+ try
{
- var fi = new FileInfo(path);
- tcs.TrySetResult(new SuccessResponse("Memory snapshot captured.", new
+ if (result)
{
- path,
- size_bytes = fi.Exists ? fi.Length : 0,
- size_mb = fi.Exists ? Math.Round(fi.Length / (1024.0 * 1024.0), 2) : 0,
- }));
+ var fi = new FileInfo(path);
+ tcs.TrySetResult(new SuccessResponse("Memory snapshot captured.", new
+ {
+ path,
+ size_bytes = fi.Exists ? fi.Length : 0,
+ size_mb = fi.Exists ? Math.Round(fi.Length / (1024.0 * 1024.0), 2) : 0,
+ }));
+ }
+ else
+ {
+ tcs.TrySetResult(new ErrorResponse($"Snapshot capture failed for path: {path}"));
+ }
}
- else
+ catch (Exception ex)
{
- tcs.TrySetResult(new ErrorResponse($"Snapshot capture failed for path: {path}"));
+ tcs.TrySetResult(new ErrorResponse($"Snapshot callback error: {ex.Message}"));
}
};📝 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.
| Action<string, bool> callback = (path, result) => | |
| { | |
| if (result) | |
| { | |
| var fi = new FileInfo(path); | |
| tcs.TrySetResult(new SuccessResponse("Memory snapshot captured.", new | |
| { | |
| path, | |
| size_bytes = fi.Exists ? fi.Length : 0, | |
| size_mb = fi.Exists ? Math.Round(fi.Length / (1024.0 * 1024.0), 2) : 0, | |
| })); | |
| } | |
| else | |
| { | |
| tcs.TrySetResult(new ErrorResponse($"Snapshot capture failed for path: {path}")); | |
| } | |
| }; | |
| Action<string, bool> callback = (path, result) => | |
| { | |
| try | |
| { | |
| if (result) | |
| { | |
| var fi = new FileInfo(path); | |
| tcs.TrySetResult(new SuccessResponse("Memory snapshot captured.", new | |
| { | |
| path, | |
| size_bytes = fi.Exists ? fi.Length : 0, | |
| size_mb = fi.Exists ? Math.Round(fi.Length / (1024.0 * 1024.0), 2) : 0, | |
| })); | |
| } | |
| else | |
| { | |
| tcs.TrySetResult(new ErrorResponse($"Snapshot capture failed for path: {path}")); | |
| } | |
| } | |
| catch (Exception ex) | |
| { | |
| tcs.TrySetResult(new ErrorResponse($"Snapshot callback error: {ex.Message}")); | |
| } | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@MCPForUnity/Editor/Tools/Profiler/Operations/MemorySnapshotOps.cs` around
lines 50 - 66, The callback Action<string,bool> (named callback) currently
constructs a FileInfo and sets tcs based on result without catching exceptions;
if FileInfo throws the exception is swallowed by Unity and tcs is never
completed, causing a hang. Wrap the callback body in try/catch, ensure any
exception sets tcs.TrySetResult with an ErrorResponse containing the exception
message (and include the path/context), and retain the existing success/error
branches to set SuccessResponse or ErrorResponse (use the same SuccessResponse
and ErrorResponse types) so the TaskCompletionSource (tcs) is always completed
even on FileInfo or IO errors.
| var areasToken = @params["areas"] as JObject; | ||
| if (areasToken == null) | ||
| return new ErrorResponse($"'areas' parameter required. Valid areas: {string.Join(", ", AreaNames)}"); |
There was a problem hiding this comment.
Guard against null @params before indexing areas.
Line 77 can throw NullReferenceException when @params is null, bypassing your explicit ErrorResponse flow for bad input.
Proposed fix
- var areasToken = `@params`["areas"] as JObject;
- if (areasToken == null)
+ if (`@params` == null || `@params`["areas"] is not JObject areasToken)
return new ErrorResponse($"'areas' parameter required. Valid areas: {string.Join(", ", AreaNames)}");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@MCPForUnity/Editor/Tools/Profiler/Operations/SessionOps.cs` around lines 77 -
79, The code indexes `@params` before checking it for null which can throw
NullReferenceException; in the method containing the lines that create
areasToken and return ErrorResponse (look for the variables `@params`, areasToken
and the ErrorResponse return referencing AreaNames in SessionOps.cs), first
guard that `@params` is not null and return the same ErrorResponse (or an
equivalent parameter-required response) if it is null, then proceed to cast
`@params`["areas"] to JObject; ensure the null-check order is changed so `@params`
is validated before any indexing into it.
Summary
Replaces the hardcoded counter-read approach from #991 with a comprehensive profiling tool:
FrameTimingManager(sync) for frame timing and async 1-frame-waitProfilerRecorderpattern for generic countersprofiling(opt-in viamanage_tools) instead ofcore.rawfileget_counters— read any counter from any ProfilerCategory with proper error on unknown categoriesProfiler.GetRuntimeMemorySizeLong()on scene objects or assetscom.unity.memoryprofiler)FrameDebuggerUtilityreflectionBuilt on top of #991 (cherry-picked, attributed). Uses
SuccessResponse/ErrorResponsethroughout.Test plan
cd Server && uv run pytest tests/test_manage_profiler.py -v) — 40/40get_countersreturns non-zero values after 1-frame waitprofiler_start/profiler_stoptoggleProfiler.enabledmemory_take_snapshotwithcom.unity.memoryprofilerinstalledSummary by Sourcery
Add a new manage_profiler tool and CLI for Unity profiling with session control, counters, memory snapshots, and Frame Debugger support, and document and expose it across the package.
New Features:
Enhancements:
Build:
Documentation:
Tests:
Summary by CodeRabbit
New Features
Documentation
Tests