feat: Add CLI for Unity MCP server #544
Conversation
- Add click-based CLI with 15+ command groups - Commands: gameobject, component, scene, asset, script, editor, prefab, material, lighting, ui, audio, animation, code - HTTP transport to communicate with Unity via MCP server - Output formats: text, json, table - Configuration via environment variables or CLI options - Comprehensive usage guide and unit tests
Reviewer's GuideIntroduce a new Sequence diagram for unity-mcp gameobject create commandsequenceDiagram
actor User
participant CLI as unity_mcp_CLI
participant GameobjectCmd as gameobject_create_command
participant Config as CLIConfig
participant Conn as run_command
participant HTTP as /api/command
participant Hub as PluginHub
participant Unity as Unity_Editor_instance
User->>CLI: unity-mcp gameobject create "MyCube" --primitive Cube
CLI->>Config: set_config from CLI options
CLI->>GameobjectCmd: dispatch click subcommand
GameobjectCmd->>Config: get_config
GameobjectCmd->>Conn: run_command("manage_gameobject", params, config)
Conn->>HTTP: POST /api/command {type: manage_gameobject, params, unity_instance?}
HTTP->>Hub: PluginHub.get_sessions()
Hub-->>HTTP: sessions list
HTTP->>Hub: send_command(session_id, command_type, params)
Hub-->>HTTP: result JSON
HTTP-->>Conn: JSONResponse(result)
Conn-->>GameobjectCmd: result dict
GameobjectCmd->>Conn: run_command("manage_components", add Rigidbody, config) (optional)
GameobjectCmd->>Conn: run_command("manage_components", add BoxCollider, config) (optional)
GameobjectCmd->>CLI: formatted output via format_output
CLI-->>User: text/json/table result
Class diagram for CLI configuration and utility modulesclassDiagram
class CLIConfig {
+str host
+int port
+int timeout
+str format
+str unity_instance
+from_env() CLIConfig
}
class Context {
+CLIConfig config
+bool verbose
+__init__()
}
class ConfigModule {
+CLIConfig _config
+get_config() CLIConfig
+set_config(config CLIConfig) void
}
class ConnectionModule {
+send_command(command_type str, params dict, config CLIConfig, timeout int) dict
+run_command(command_type str, params dict, config CLIConfig, timeout int) dict
+check_connection(config CLIConfig) bool
+run_check_connection(config CLIConfig) bool
+list_unity_instances(config CLIConfig) dict
+run_list_instances(config CLIConfig) dict
+warn_if_remote_host(config CLIConfig) void
}
class UnityConnectionError {
+UnityConnectionError(message str)
}
class OutputModule {
+format_output(data any, format_type str) str
+format_as_json(data any) str
+format_as_text(data any, indent int) str
+format_as_table(data any) str
+print_success(message str) void
+print_error(message str) void
+print_warning(message str) void
+print_info(message str) void
}
class MainCLI {
+cli(host str, port int, timeout int, format str, instance str, verbose bool) void
+status() void
+instances() void
+raw_command(command_type str, params str) void
+register_commands() void
+main() void
}
class GameobjectCommands {
+gameobject()
+find(search_term str, method str, include_inactive bool, limit int, cursor int)
+create(name str, primitive str, position tuple, rotation tuple, scale tuple, parent str, tag str, layer str, components str, save_prefab bool, prefab_path str)
+modify(target str, name str, position tuple, rotation tuple, scale tuple, parent str, tag str, layer str, active bool, add_components str, remove_components str, search_method str)
+delete(target str, search_method str, force bool)
+duplicate(target str, name str, offset tuple, search_method str)
+move(target str, reference str, direction str, distance float, local bool, search_method str)
}
MainCLI --> Context : uses
Context --> CLIConfig : holds
MainCLI --> ConfigModule : uses
MainCLI --> ConnectionModule : uses
MainCLI --> OutputModule : uses
ConfigModule --> CLIConfig : manages
ConnectionModule --> CLIConfig : reads
ConnectionModule --> UnityConnectionError : raises
OutputModule ..> CLIConfig : uses format field
GameobjectCommands --> ConfigModule : get_config
GameobjectCommands --> ConnectionModule : run_command
GameobjectCommands --> OutputModule : format_output
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
📝 WalkthroughWalkthroughIntroduces a comprehensive CLI for Unity MCP using Click framework. Adds 13 command groups (gameobject, component, scene, asset, script, code, editor, prefab, material, lighting, animation, audio, ui) with corresponding subcommands, HTTP server endpoints for CLI communication, configuration management, output formatting utilities, and extensive test coverage. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/CLI
participant Click as Click CLI
participant Config as Config
participant Connection as Connection Handler
participant Server as MCP Server
participant Unity as Unity Instances
User->>Click: Execute command (e.g., gameobject find)
Click->>Config: get_config()
Config-->>Click: CLIConfig
Click->>Connection: run_command(type, params, config)
Connection->>Connection: asyncio.run(send_command(...))
Connection->>Server: POST /api/command
Server->>Server: Select Unity instance
Server->>Unity: Forward command (manage_gameobject)
Unity-->>Server: Command result
Server-->>Connection: JSON response
Connection-->>Click: Dict result
Click->>Click: format_output(result, format_type)
Click->>User: Formatted output (text/JSON/table)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 9 issues, and left some high level feedback:
- The
/api/commandroute silently falls back to the first available session whenunity_instanceis missing or doesn’t match, which can be surprising in multi-instance setups—consider returning a 400/409 with a clear error when there’s an explicit but unresolvedunity_instance, and/or requiring disambiguation when multiple candidates exist. - Several commands inspect responses with ad‑hoc checks like
result.get("success") or result.get("data") or result.get("result"); it would be more robust to centralize response normalization/validation (e.g., a helper that extractssuccess,dataand standardizes error reporting) so all commands interpret server responses consistently.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `/api/command` route silently falls back to the first available session when `unity_instance` is missing or doesn’t match, which can be surprising in multi-instance setups—consider returning a 400/409 with a clear error when there’s an explicit but unresolved `unity_instance`, and/or requiring disambiguation when multiple candidates exist.
- Several commands inspect responses with ad‑hoc checks like `result.get("success") or result.get("data") or result.get("result")`; it would be more robust to centralize response normalization/validation (e.g., a helper that extracts `success`, `data` and standardizes error reporting) so all commands interpret server responses consistently.
## Individual Comments
### Comment 1
<location> `Server/src/main.py:328-330` </location>
<code_context>
+ session_id = sid
+ break
+
+ if not session_id:
+ # Use first available session
+ session_id = next(iter(sessions.sessions.keys()))
+
+ # Send command to Unity
</code_context>
<issue_to_address>
**issue (bug_risk):** Using the first available session when a specific `unity_instance` is provided but not found may be surprising behavior.
If `unity_instance` is provided but not found, silently falling back to the first session can cause commands to be sent to the wrong Unity instance, and the CLI has no easy way to detect this. It would be safer to return an explicit error in that case (e.g., `"Unity instance '<value>' not found"`), or otherwise handle "not provided" vs. "provided but missing" differently rather than auto-selecting.
</issue_to_address>
### Comment 2
<location> `Server/src/cli/commands/material.py:92-98` </location>
<code_context>
[email protected]("r", type=float)
[email protected]("g", type=float)
[email protected]("b", type=float)
[email protected]("a", type=float, default=1.0)
[email protected](
+ "--property", "-p",
+ default="_Color",
+ help="Color property name (default: _Color)."
+)
+def set_color(path: str, r: float, g: float, b: float, a: float, property: str):
+ """Set a material's color.
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Using a default on a required Click argument can lead to confusing or unsupported behavior.
Here `a` is a positional argument but also has `default=1.0`. In Click, defaults are usually used with `required=False` and options rather than required positional arguments, and behavior can vary between Click versions. Consider either making `a` an option (e.g., `--alpha`) with a default, or removing the default and requiring all four components explicitly.
</issue_to_address>
### Comment 3
<location> `Server/src/cli/commands/ui.py:91` </location>
<code_context>
+ default=(0, 0),
+ help="Anchored position X Y."
+)
+def create_text(name: str, parent: str, text: str, position: tuple):
+ """Create a UI Text element (TextMeshPro).
+
</code_context>
<issue_to_address>
**issue (bug_risk):** The `position` parameter for `create-text` is currently unused.
The CLI accepts `--position` and passes it to `create_text`, but the function never uses it (e.g., on a `RectTransform` anchored position). This makes the option misleading. Either apply the position (possibly via a `manage_components`/`manage_gameobject` call) or remove the option until it’s supported.
</issue_to_address>
### Comment 4
<location> `Server/src/cli/commands/audio.py:20-23` </location>
<code_context>
[email protected]("play")
[email protected]("target")
[email protected]("state_name")
[email protected](
+ "--layer", "-l",
+ default=0,
</code_context>
<issue_to_address>
**issue (bug_risk):** The `--clip` option in `audio play` is accepted but never used.
The `play` command takes a `clip` parameter but only toggles an `AudioSource` property and never sends the clip path to Unity. If clip selection isn’t supported yet, either remove this option or pass it through to `manage_components` (or equivalent) so the backend can actually use it.
</issue_to_address>
### Comment 5
<location> `Server/tests/test_cli.py:199-208` </location>
<code_context>
+ result = await check_connection()
+ assert result is False
+
+ @pytest.mark.asyncio
+ async def test_send_command_success(self, mock_unity_response):
+ """Test successful command sending."""
+ mock_response = MagicMock()
+ mock_response.status_code = 200
+ mock_response.json.return_value = mock_unity_response
+
+ with patch("httpx.AsyncClient") as mock_client:
+ mock_client.return_value.__aenter__.return_value.post = AsyncMock(
+ return_value=mock_response
+ )
+ mock_response.raise_for_status = MagicMock()
+
+ result = await send_command("test_command", {"param": "value"})
+ assert result == mock_unity_response
+
+ @pytest.mark.asyncio
+ async def test_send_command_connection_error(self):
+ """Test command sending with connection error."""
+ with patch("httpx.AsyncClient") as mock_client:
</code_context>
<issue_to_address>
**suggestion (testing):** Add tests for the specific httpx error branches in send_command
Current tests only exercise the happy path and a generic `Exception` from `send_command`, but the code has specific handling for `httpx.ConnectError`, `httpx.TimeoutException`, and `httpx.HTTPStatusError` with distinct messages. Please add tests that mock `httpx.AsyncClient.post` to raise each of these exceptions and assert that `UnityConnectionError` is raised with the correct message fragment, so the detailed error handling is verified and doesn't regress.
Suggested implementation:
```python
# Connection Tests
# =============================================================================
class TestConnection:
=======
assert "key" in table_result.lower() or "Key" in table_result
@pytest.mark.asyncio
async def test_send_command_httpx_connect_error():
"""send_command should wrap httpx.ConnectError in UnityConnectionError with a connection-related message."""
# Simulate a low-level connection error from httpx
with patch("httpx.AsyncClient") as mock_client:
mock_client.return_value.__aenter__.return_value.post = AsyncMock(
side_effect=httpx.ConnectError("Unable to connect")
)
with pytest.raises(UnityConnectionError) as exc_info:
await send_command("test_command", {"param": "value"})
message = str(exc_info.value).lower()
assert "connect" in message or "connection" in message
@pytest.mark.asyncio
async def test_send_command_httpx_timeout_error():
"""send_command should wrap httpx.TimeoutException in UnityConnectionError with a timeout-related message."""
with patch("httpx.AsyncClient") as mock_client:
mock_client.return_value.__aenter__.return_value.post = AsyncMock(
side_effect=httpx.TimeoutException("Request timed out")
)
with pytest.raises(UnityConnectionError) as exc_info:
await send_command("test_command", {"param": "value"})
message = str(exc_info.value).lower()
assert "timeout" in message or "timed out" in message
@pytest.mark.asyncio
async def test_send_command_httpx_http_status_error():
"""send_command should wrap httpx.HTTPStatusError in UnityConnectionError with an HTTP-status-related message."""
# Create a Response and Request to satisfy HTTPStatusError requirements
request = httpx.Request("POST", "http://unity-endpoint")
response = httpx.Response(status_code=500, request=request)
http_status_error = httpx.HTTPStatusError(
"Unity returned an error status",
request=request,
response=response,
)
with patch("httpx.AsyncClient") as mock_client:
# send_command is expected to call response.raise_for_status(), which will raise HTTPStatusError
mock_response = MagicMock()
mock_response.raise_for_status.side_effect = http_status_error
mock_client.return_value.__aenter__.return_value.post = AsyncMock(
return_value=mock_response
)
with pytest.raises(UnityConnectionError) as exc_info:
await send_command("test_command", {"param": "value"})
message = str(exc_info.value).lower()
assert "http" in message or "status" in message or "error response" in message
# =============================================================================
# Connection Tests
# =============================================================================
class TestConnection:
```
These tests assume:
1. `pytest`, `httpx`, `MagicMock`, `AsyncMock`, `patch`, `UnityConnectionError`, and `send_command` are already imported in `Server/tests/test_cli.py`. If any are missing, add appropriate imports at the top of the file.
2. The error messages raised by `send_command` for `httpx.ConnectError`, `httpx.TimeoutException`, and `httpx.HTTPStatusError` contain connection-, timeout-, and HTTP/status-related words respectively. If your actual messages differ, adjust the `assert` conditions in the tests to match the real message fragments (for example, assert for `"failed to connect to unity"` instead of the generic `"connect"` / `"connection"`).
</issue_to_address>
### Comment 6
<location> `Server/tests/test_cli.py:735-744` </location>
<code_context>
+class TestGlobalOptions:
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding a test that asserts warn_if_remote_host is triggered for non-local hosts
Current global option tests cover `--host`, `--port`, `--timeout`, and `--format`, but don’t check that `warn_if_remote_host` is invoked for non-local hosts. Please add a CLI test (e.g., `unity-mcp --host 10.0.0.5 status`) that captures stderr/stdout to assert the warning text appears and that the command still returns a valid status. This will help prevent regressions in the remote-host safety warning wiring.
Suggested implementation:
```python
class TestGlobalOptions:
"""Tests for global CLI options."""
def test_custom_host(self, runner, mock_unity_response):
"""Test custom host option."""
with patch("cli.main.run_check_connection", return_value=True):
with patch("cli.main.run_list_instances", return_value={"instances": []}):
result = runner.invoke(cli, ["--host", "192.168.1.100", "status"])
assert result.exit_code == 0
def test_remote_host_warns_and_succeeds(self, runner, mock_unity_response):
"""Test that a non-local host triggers a warning but the command still succeeds."""
with patch("cli.main.run_check_connection", return_value=True):
with patch("cli.main.run_list_instances", return_value={"instances": []}):
with patch("cli.main.warn_if_remote_host") as mock_warn_if_remote_host:
# Emit a known warning string to stderr when the remote host check runs
mock_warn_if_remote_host.side_effect = lambda host: click.echo(
"REMOTE HOST SAFETY WARNING", err=True
)
result = runner.invoke(cli, ["--host", "10.0.0.5", "status"])
# Command should still complete successfully
assert result.exit_code == 0
# Ensure the remote host warning hook was invoked with the non-local host
mock_warn_if_remote_host.assert_called_once_with("10.0.0.5")
# Ensure the warning text is visible in the CLI output (stderr captured by Click test runner)
assert "REMOTE HOST SAFETY WARNING" in result.output
def test_custom_port(self, runner, mock_unity_response):
```
1. Ensure `click` is imported at the top of `Server/tests/test_cli.py`, for example:
- Add `import click` alongside the other imports if it is not already present.
2. If the real `warn_if_remote_host` signature differs (e.g., additional parameters or different module path), adjust the patch target (`"cli.main.warn_if_remote_host"`) and `assert_called_once_with(...)` arguments accordingly.
3. If your test suite differentiates between `stdout` and `stderr` using a different mechanism than Click's `CliRunner`, adapt the `result.output` assertion to match your existing pattern (e.g., `result.stderr` if available).
</issue_to_address>
### Comment 7
<location> `Server/tests/test_cli.py:285-291` </location>
<code_context>
+ result = runner.invoke(cli, ["--version"])
+ assert result.exit_code == 0
+
+ def test_status_connected(self, runner, mock_instances_response):
+ """Test status command when connected."""
+ with patch("cli.main.run_check_connection", return_value=True):
+ with patch("cli.main.run_list_instances", return_value=mock_instances_response):
+ result = runner.invoke(cli, ["status"])
+ assert result.exit_code == 0
+ assert "Connected" in result.output
+
+ def test_status_disconnected(self, runner):
</code_context>
<issue_to_address>
**suggestion (testing):** Add a status test for when listing instances fails with UnityConnectionError
There’s an untested path where `run_check_connection` returns `True` but `run_list_instances` raises `UnityConnectionError`; in that case the CLI should print an informational message and exit cleanly. Please add a test that patches `run_check_connection` to return `True` and `run_list_instances` to raise `UnityConnectionError`, then assert exit code 0 and that the expected "Could not retrieve Unity instances" (or equivalent) message is printed.
Suggested implementation:
```python
def test_status_connected(self, runner, mock_instances_response):
"""Test status command when connected."""
with patch("cli.main.run_check_connection", return_value=True):
with patch("cli.main.run_list_instances", return_value=mock_instances_response):
result = runner.invoke(cli, ["status"])
assert result.exit_code == 0
assert "Connected" in result.output
def test_status_instances_error(self, runner):
"""Test status command when listing instances fails."""
with patch("cli.main.run_check_connection", return_value=True):
with patch("cli.main.run_list_instances", side_effect=UnityConnectionError("test error")):
result = runner.invoke(cli, ["status"])
assert result.exit_code == 0
assert "Could not retrieve Unity instances" in result.output
def test_status_disconnected(self, runner):
"""Test status command when disconnected."""
with patch("cli.main.run_check_connection", return_value=False):
result = runner.invoke(cli, ["status"])
assert result.exit_code == 1
assert "Cannot connect" in result.output
```
1. Ensure `UnityConnectionError` is imported at the top of `Server/tests/test_cli.py`, for example:
`from cli.main import UnityConnectionError` or from the module where it is defined (e.g. `from cli.exceptions import UnityConnectionError`), matching the existing codebase.
2. Confirm the exact message printed by the CLI when instance listing fails; if it differs from `"Could not retrieve Unity instances"`, update the assertion string in `test_status_instances_error` accordingly.
</issue_to_address>
### Comment 8
<location> `Server/tests/test_cli.py:306-277` </location>
<code_context>
+ result = runner.invoke(cli, ["instances"])
+ assert result.exit_code == 0
+
+ def test_raw_command(self, runner, mock_unity_response):
+ """Test raw command."""
+ with patch("cli.main.run_command", return_value=mock_unity_response):
+ result = runner.invoke(cli, ["raw", "test_command", '{"param": "value"}'])
+ assert result.exit_code == 0
+
+ def test_raw_command_invalid_json(self, runner):
</code_context>
<issue_to_address>
**suggestion (testing):** Add a raw command test that covers UnityConnectionError handling
Currently we only test the success and invalid-JSON paths. Since `cli.main.raw_command` should treat a `UnityConnectionError` from `run_command` as an error (message + exit code 1), please add a test that patches `cli.main.run_command` to raise `UnityConnectionError("some error")`, then asserts `runner.invoke` returns `exit_code == 1` and that the error message appears in the output/stderr.
Suggested implementation:
```python
# GameObject Command Tests
# =============================================================================
=======
def test_raw_command_invalid_json(self, runner):
"""Test raw command with invalid JSON."""
result = runner.invoke(cli, ["raw", "test_command", "invalid json"])
assert result.exit_code == 1
assert "Invalid JSON" in result.output
def test_raw_command_unity_connection_error(self, runner):
"""Test raw command handling UnityConnectionError from run_command."""
with patch("cli.main.run_command", side_effect=UnityConnectionError("some error")):
result = runner.invoke(cli, ["raw", "test_command", '{"param": "value"}'])
assert result.exit_code == 1
assert "some error" in result.output
# =============================================================================
# GameObject Command Tests
# =============================================================================
```
If `UnityConnectionError` is not already imported in `Server/tests/test_cli.py`, add an import at the top of the file, for example:
- If it lives in `cli.exceptions`:
```python
from cli.exceptions import UnityConnectionError
```
or match whatever module is used elsewhere in this file for other `UnityConnectionError`-related tests.
</issue_to_address>
### Comment 9
<location> `Server/tests/test_cli.py:126-135` </location>
<code_context>
+class TestOutputFormatting:
</code_context>
<issue_to_address>
**suggestion (testing):** Add a test for format_as_text on successful responses that still include success/message fields
`format_as_text` has special handling for dicts with `success` and `data`/`result` fields where it recurses into the inner payload. Please add a test for an input like `{"success": True, "message": "OK", "data": {"foo": "bar"}}` that asserts only the inner `data` is rendered and `success`/`message` are omitted, to lock in this meta-field stripping behavior.
Suggested implementation:
```python
class TestOutputFormatting:
"""Tests for output formatting utilities."""
def test_format_as_json(self):
"""Test JSON formatting."""
data = {"key": "value", "number": 42}
result = format_as_json(data)
parsed = json.loads(result)
assert parsed == data
def test_format_as_text_strips_meta_fields_on_success(self):
"""format_as_text should render only inner data for success-wrapped payloads."""
payload = {"success": True, "message": "OK", "data": {"foo": "bar"}}
rendered = format_as_text(payload)
# Only the inner data payload should be rendered; meta fields are stripped
assert "foo" in rendered
assert "bar" in rendered
assert "success" not in rendered
assert "message" not in rendered
def test_format_as_json_with_complex_types(self):
```
If `format_as_text` is not yet imported in this test module, you will also need to add an import at the top of `Server/tests/test_cli.py`, matching how `format_as_json` is imported (for example, `from Server.cli import format_as_json, format_as_text` or similar, depending on the existing import style).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| if not session_id: | ||
| # Use first available session | ||
| session_id = next(iter(sessions.sessions.keys())) |
There was a problem hiding this comment.
issue (bug_risk): Using the first available session when a specific unity_instance is provided but not found may be surprising behavior.
If unity_instance is provided but not found, silently falling back to the first session can cause commands to be sent to the wrong Unity instance, and the CLI has no easy way to detect this. It would be safer to return an explicit error in that case (e.g., "Unity instance '<value>' not found"), or otherwise handle "not provided" vs. "provided but missing" differently rather than auto-selecting.
| @click.argument("a", type=float, default=1.0) | ||
| @click.option( | ||
| "--property", "-p", | ||
| default="_Color", | ||
| help="Color property name (default: _Color)." | ||
| ) | ||
| def set_color(path: str, r: float, g: float, b: float, a: float, property: str): |
There was a problem hiding this comment.
issue (bug_risk): Using a default on a required Click argument can lead to confusing or unsupported behavior.
Here a is a positional argument but also has default=1.0. In Click, defaults are usually used with required=False and options rather than required positional arguments, and behavior can vary between Click versions. Consider either making a an option (e.g., --alpha) with a default, or removing the default and requiring all four components explicitly.
| default=(0, 0), | ||
| help="Anchored position X Y." | ||
| ) | ||
| def create_text(name: str, parent: str, text: str, position: tuple): |
There was a problem hiding this comment.
issue (bug_risk): The position parameter for create-text is currently unused.
The CLI accepts --position and passes it to create_text, but the function never uses it (e.g., on a RectTransform anchored position). This makes the option misleading. Either apply the position (possibly via a manage_components/manage_gameobject call) or remove the option until it’s supported.
| @click.option( | ||
| "--clip", "-c", | ||
| default=None, | ||
| help="Audio clip path to play." |
There was a problem hiding this comment.
issue (bug_risk): The --clip option in audio play is accepted but never used.
The play command takes a clip parameter but only toggles an AudioSource property and never sends the clip path to Unity. If clip selection isn’t supported yet, either remove this option or pass it through to manage_components (or equivalent) so the backend can actually use it.
| class TestOutputFormatting: | ||
| """Tests for output formatting utilities.""" | ||
|
|
||
| def test_format_as_json(self): | ||
| """Test JSON formatting.""" | ||
| data = {"key": "value", "number": 42} | ||
| result = format_as_json(data) | ||
| parsed = json.loads(result) | ||
| assert parsed == data | ||
|
|
There was a problem hiding this comment.
suggestion (testing): Add a test for format_as_text on successful responses that still include success/message fields
format_as_text has special handling for dicts with success and data/result fields where it recurses into the inner payload. Please add a test for an input like {"success": True, "message": "OK", "data": {"foo": "bar"}} that asserts only the inner data is rendered and success/message are omitted, to lock in this meta-field stripping behavior.
Suggested implementation:
class TestOutputFormatting:
"""Tests for output formatting utilities."""
def test_format_as_json(self):
"""Test JSON formatting."""
data = {"key": "value", "number": 42}
result = format_as_json(data)
parsed = json.loads(result)
assert parsed == data
def test_format_as_text_strips_meta_fields_on_success(self):
"""format_as_text should render only inner data for success-wrapped payloads."""
payload = {"success": True, "message": "OK", "data": {"foo": "bar"}}
rendered = format_as_text(payload)
# Only the inner data payload should be rendered; meta fields are stripped
assert "foo" in rendered
assert "bar" in rendered
assert "success" not in rendered
assert "message" not in rendered
def test_format_as_json_with_complex_types(self):If format_as_text is not yet imported in this test module, you will also need to add an import at the top of Server/tests/test_cli.py, matching how format_as_json is imported (for example, from Server.cli import format_as_json, format_as_text or similar, depending on the existing import style).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🤖 Fix all issues with AI agents
In @Server/src/cli/commands/animation.py:
- Around line 33-60: The play function accepts a layer parameter but never sends
it to Unity; update the params dict in play to include the layer (e.g.,
params["layer"] = layer) so the "manage_components" call receives the layer
value, or if layer support is not yet intended remove the layer parameter from
play's signature or add a clear TODO comment and validation; locate the play
function and modify the params construction (or signature) accordingly to ensure
the parameter is used or intentionally omitted.
- Line 48: The PR is trying to set "Play" as a property but Animator.Play is a
method; update the code that emits the component change (the entry with
"property": "Play") to either (A) call a method-invocation path in the
manage_components backend (implement method invocation support there and handle
calling Animator.Play(...)) or (B) replace this manage_components call with an
animation-specific tool (similar to ManageVFX.cs) that invokes Animator.Play on
the target component; locate the emitter that writes the "property": "Play"
entry and change it to emit a method-invoke request (or swap to the custom
animation tool) and wire up any required parameters for Animator.Play
accordingly.
In @Server/src/cli/commands/audio.py:
- Around line 18-57: The play command accepts a --clip option but never uses it;
update the play function so that when clip is not None you add it to the params
payload (e.g., params["clip"] = clip) before calling
run_command("manage_components", params, config); if specifying a clip is not a
supported use case, remove the --clip Click option and the clip parameter from
play instead.
In @Server/src/cli/commands/code.py:
- Line 1: The module docstring claims "search and read source code" but only the
read command is implemented; update the top-level docstring in
Server/src/cli/commands/code.py to accurately reflect implemented functionality
(e.g., "Read source code" or "Read source code (search not yet implemented)") or
add a clear TODO note about implementing the search command, and mention the
implemented symbol 'read' so reviewers can verify consistency.
In @Server/src/cli/commands/gameobject.py:
- Around line 181-189: The component addition loop currently calls
run_command("manage_components", ...) for each entry in component_list without
per-item error handling; wrap each run_command call in a try/except (or check
its return value) inside the for-loop that iterates component_list, log or
collect failures (including component name and error/response) and continue to
the next component, and after the loop report any collected failures (or
raise/return a consolidated error) so callers can see which components failed;
references: components, result, component_list, run_command,
"manage_components", name, config.
In @Server/src/cli/commands/lighting.py:
- Around line 66-68: The current guard incorrectly uses "or" across fields so a
falsy success with truthy data/result lets execution continue; update the check
on create_result to only verify success (or success and data) — e.g., replace
the condition with if not create_result.get("success") (or if not
(create_result.get("success") and create_result.get("data"))) before calling
click.echo(format_output(create_result, config.format)) and returning so the
command aborts when creation failed.
In @Server/src/cli/commands/material.py:
- Around line 171-176: The docstring/example uses an invalid value "by_id" for
the --search-method Click option; update the example(s) to use one of the
allowed choices ("by_name", "by_path", "by_tag", "by_layer", "by_component") or
alternatively add "by_id" to the Click option choices array on the
--search-method declaration so they match; ensure you change all occurrences
(including the example at the other noted location) so the example and the
click.option choices remain consistent.
In @Server/src/cli/commands/script.py:
- Around line 147-150: The params construction is using fragile string-splitting
on path; update the logic that builds params (the "params" dict in the delete
command) to use robust path utilities instead of path.split: derive the script
name with pathlib.Path(path).stem (or
os.path.splitext(os.path.basename(path))[0]) and derive the parent with
pathlib.Path(path).parent.as_posix() (or os.path.dirname(path)), falling back to
"Assets" when parent is empty or "."; then assign those values into the existing
params dict ("action": "delete", "name", "path") so behavior matches the safer
parsing used in the read command.
In @Server/src/cli/commands/ui.py:
- Around line 190-228: The --sprite option is accepted by create_image but never
applied; after the existing run_command call that adds the Image component in
create_image, if sprite is not None call run_command again to set the Image
component's sprite property on the created GameObject (use the same config and
target name), e.g. invoke the manage_components command to set the Image
component's "sprite" (or include it in the add call as a properties/payload
field) so the provided sprite path is applied to the component.
- Around line 84-132: create_text accepts a position tuple but never applies it
to the RectTransform; after adding the TextMeshProUGUI component (in
create_text) call run_command("manage_components", { "action": "set_property",
"target": name, "componentType": "RectTransform", "property":
"anchoredPosition", "value": list(position) }, config) (or use
"anchoredPosition3D"/"localPosition" if your backend expects 3D) and ensure you
pass the position variable (converted to the expected list/format) so the
RectTransform receives the anchored position.
In @Server/src/cli/utils/config.py:
- Around line 18-27: The from_env classmethod on CLIConfig currently casts
UNITY_MCP_HTTP_PORT and UNITY_MCP_TIMEOUT to int without validation, which can
raise ValueError; wrap the int conversions in validation/error handling inside
CLIConfig.from_env (catch ValueError/TypeError), provide a clear error message
including the env var name and invalid value (and optionally fallback to
defaults), and re-raise or exit gracefully so the app doesn’t crash with an
opaque traceback; reference the symbols CLIConfig.from_env, UNITY_MCP_HTTP_PORT,
and UNITY_MCP_TIMEOUT when locating and updating the code.
🧹 Nitpick comments (19)
Server/src/cli/utils/connection.py (2)
80-96: Consider adding exception chaining for better debugging.The exception handlers re-raise errors without preserving the exception chain. Adding
from e(orfrom Nonewhere deliberate) improves debugging by preserving the original traceback.♻️ Proposed fix to add exception chaining
except httpx.ConnectError as e: raise UnityConnectionError( f"Cannot connect to Unity MCP server at {cfg.host}:{cfg.port}. " f"Make sure the server is running and Unity is connected.\n" f"Error: {e}" - ) + ) from e except httpx.TimeoutException: raise UnityConnectionError( f"Connection to Unity timed out after {timeout or cfg.timeout}s. " f"Unity may be busy or unresponsive." - ) + ) from None except httpx.HTTPStatusError as e: raise UnityConnectionError( f"HTTP error from server: {e.response.status_code} - {e.response.text}" - ) + ) from e except Exception as e: - raise UnityConnectionError(f"Unexpected error: {e}") + raise UnityConnectionError(f"Unexpected error: {e}") from e
161-183: Consider logging endpoint failures for easier debugging.The try-except-continue pattern silently swallows errors when attempting each endpoint. If both endpoints fail, users only see the generic "No working endpoint found" message without knowing why each endpoint failed. Adding debug logging would aid troubleshooting.
💡 Suggested improvement with logging
+ import logging + logger = logging.getLogger(__name__) + async with httpx.AsyncClient() as client: for url in urls_to_try: try: response = await client.get(url, timeout=10) if response.status_code == 200: data = response.json() # Normalize response format if "instances" in data: return data elif "sessions" in data: # Convert sessions format to instances format instances = [] for session_id, details in data["sessions"].items(): instances.append({ "session_id": session_id, "project": details.get("project", "Unknown"), "hash": details.get("hash", ""), "unity_version": details.get("unity_version", "Unknown"), "connected_at": details.get("connected_at", ""), }) return {"success": True, "instances": instances} + else: + logger.debug(f"Endpoint {url} returned status {response.status_code}") - except Exception: + except Exception as e: + logger.debug(f"Failed to connect to {url}: {e}") continueServer/src/cli/commands/code.py (1)
44-44: Consider broader file type support.The command strips
.csextensions, assuming C# files. However, the command group is namedcode(notscript), which suggests it might be intended for broader source file reading. Consider whether this should support other file types or if the naming should be clarified.Server/src/main.py (2)
336-338: Uselogging.exceptionto preserve traceback.Line 337 uses
logger.error, which discards the exception traceback. For debugging production issues, uselogger.exceptioninstead to preserve the full stack trace.🔍 Suggested fix
except Exception as e: - logger.error(f"CLI command error: {e}") + logger.exception(f"CLI command error: {e}") return JSONResponse({"success": False, "error": str(e)}, status_code=500)Based on static analysis hints.
356-357: Add error logging for consistency.The
cli_instances_routeexception handler doesn't log errors, unlikecli_command_routeat line 337. For operational visibility and debugging, consider logging the exception here as well.📊 Suggested addition
except Exception as e: + logger.exception("CLI instances query error") return JSONResponse({"success": False, "error": str(e)}, status_code=500)Server/src/cli/utils/config.py (1)
25-25: Validate format value.Line 25 accepts any string for the
formatfield, but based on the codebase context (output.py uses "text", "json", "table"), only specific values are valid. Consider validating the format value and falling back to "text" or raising an error for invalid values.✨ Suggested validation
+ format_value = os.environ.get("UNITY_MCP_FORMAT", "text") + if format_value not in ("text", "json", "table"): + raise ValueError(f"Invalid UNITY_MCP_FORMAT: {format_value}. Must be one of: text, json, table") + return cls( host=os.environ.get("UNITY_MCP_HOST", "127.0.0.1"), port=int(os.environ.get("UNITY_MCP_HTTP_PORT", "8080")), timeout=int(os.environ.get("UNITY_MCP_TIMEOUT", "30")), - format=os.environ.get("UNITY_MCP_FORMAT", "text"), + format=format_value, unity_instance=os.environ.get("UNITY_MCP_INSTANCE"), )Server/src/cli/commands/animation.py (1)
63-85: Placeholder command doesn't execute.The
set-parametercommand is a non-functional placeholder that only prints informational messages (lines 84-85) without executing any Unity operations. While this may be intentional per the PR description, consider either:
- Implementing the functionality
- Adding a clear warning in the help text that this is not yet implemented
- Hiding the command until implementation is complete
📋 Suggested help text clarification
def set_parameter(target: str, param_name: str, value: str, param_type: str): - """Set an Animator parameter. + """Set an Animator parameter (placeholder - not yet implemented). \b Examples:Server/tests/test_cli.py (1)
735-770: Remove unusedmock_unity_responsefixture arguments.The
mock_unity_responsefixture is declared but never used intest_custom_host,test_custom_port, andtest_timeout_option. These tests mockrun_check_connectionandrun_list_instancesdirectly, so the fixture is unnecessary.♻️ Proposed fix
- def test_custom_host(self, runner, mock_unity_response): + def test_custom_host(self, runner): """Test custom host option.""" with patch("cli.main.run_check_connection", return_value=True): with patch("cli.main.run_list_instances", return_value={"instances": []}): result = runner.invoke(cli, ["--host", "192.168.1.100", "status"]) assert result.exit_code == 0 - def test_custom_port(self, runner, mock_unity_response): + def test_custom_port(self, runner): """Test custom port option.""" with patch("cli.main.run_check_connection", return_value=True): with patch("cli.main.run_list_instances", return_value={"instances": []}): result = runner.invoke(cli, ["--port", "9090", "status"]) assert result.exit_code == 0 ... - def test_timeout_option(self, runner, mock_unity_response): + def test_timeout_option(self, runner): """Test timeout option.""" with patch("cli.main.run_check_connection", return_value=True): with patch("cli.main.run_list_instances", return_value={"instances": []}): result = runner.invoke(cli, ["--timeout", "60", "status"]) assert result.exit_code == 0Server/src/cli/commands/audio.py (1)
96-130: Consider adding volume range validation.Unity's
AudioSource.volumeis clamped between 0.0 and 1.0. Adding client-side validation would provide clearer feedback to users.♻️ Suggested validation
def volume(target: str, level: float, search_method: Optional[str]): """Set audio volume on a target's AudioSource. ... """ config = get_config() + + if not 0.0 <= level <= 1.0: + print_error("Volume level must be between 0.0 and 1.0") + sys.exit(1) params: dict[str, Any] = {Server/src/cli/commands/asset.py (1)
246-249: Moveosimport to module level.The
osimport is inside the function, which is inconsistent with the module's import style. Move it to the top with other imports.♻️ Proposed fix
"""Asset CLI commands.""" import sys +import os import json import click from typing import Optional, AnyThen remove line 247:
- import os dir_path = os.path.dirname(path)Server/src/cli/commands/prefab.py (1)
20-24: Consider constraining mode values withclick.Choice.The mode option accepts any string, but Unity's PrefabStage only supports specific modes. Using
click.Choicewould provide better validation and help text.♻️ Suggested improvement
@click.option( "--mode", "-m", - default="InIsolation", - help="Prefab stage mode (InIsolation)." + type=click.Choice(["Normal", "InIsolation", "InContext"]), + default="InIsolation", + help="Prefab stage mode." )Server/src/cli/commands/ui.py (1)
43-45: Overly permissive success check may mask failures.The condition
result.get("success") or result.get("data") or result.get("result")could treat an error response as successful if it containsdataorresultkeys. Consider using onlyresult.get("success")for consistency with other command modules.♻️ Proposed fix
- if not (result.get("success") or result.get("data") or result.get("result")): + if not result.get("success"): click.echo(format_output(result, config.format)) returnApply this pattern to all occurrences in this file (lines 43, 108, 164, 213).
Server/src/cli/commands/scene.py (1)
224-245: Consider validating supersize range.The help text indicates supersize should be 1-4, but no validation enforces this. Adding client-side validation would provide clearer feedback.
♻️ Suggested validation
def screenshot(filename: Optional[str], supersize: int): """Capture a screenshot of the scene. ... """ config = get_config() + + if not 1 <= supersize <= 4: + print_error("Supersize must be between 1 and 4") + sys.exit(1) params: dict[str, Any] = {"action": "screenshot"}Server/src/cli/main.py (2)
31-36: Short option-hconflicts with Click's default--help.Using
-hfor--hostconflicts with the conventional-h/--helpshortcut that many CLI users expect. Click doesn't add-hby default, but this could cause confusion.Consider using a different short option
@click.option( - "--host", "-h", + "--host", "-H", default="127.0.0.1", envvar="UNITY_MCP_HOST", help="MCP server host address." )
180-260: Repetitive command registration can be simplified with a loop.The
register_commandsfunction repeats the same try/except pattern 13 times. This could be simplified using a loop, reducing maintenance burden and improving readability.♻️ Suggested refactor using a loop
def register_commands(): """Register all command groups.""" - try: - from cli.commands.gameobject import gameobject - cli.add_command(gameobject) - except ImportError: - pass - - try: - from cli.commands.component import component - cli.add_command(component) - except ImportError: - pass - - # ... (remaining repetitive blocks) + command_modules = [ + ("gameobject", "gameobject"), + ("component", "component"), + ("scene", "scene"), + ("asset", "asset"), + ("script", "script"), + ("code", "code"), + ("editor", "editor"), + ("prefab", "prefab"), + ("material", "material"), + ("lighting", "lighting"), + ("animation", "animation"), + ("audio", "audio"), + ("ui", "ui"), + ] + + for module_name, command_name in command_modules: + try: + module = __import__(f"cli.commands.{module_name}", fromlist=[command_name]) + cli.add_command(getattr(module, command_name)) + except ImportError: + passServer/src/cli/commands/material.py (1)
6-6: Unused importTuple.
Tupleis imported but not used in this module.Remove unused import
-from typing import Optional, Any, Tuple +from typing import Optional, AnyServer/src/cli/commands/gameobject.py (1)
291-294: Inconsistent type annotations onparamsdictionaries.The
paramsdictionary inmodify,delete,duplicate, andmovecommands lacks type annotations, unlikefindandcreatecommands which usedict[str, Any].Add consistent type annotations
- params = { + params: dict[str, Any] = { "action": "modify", "target": target, }Apply similar change to
delete,duplicate, andmovecommands.Also applies to: 354-357, 408-411, 478-485
Server/src/cli/utils/output.py (1)
5-5: Unused importUnion.
Unionis imported but not used in the module.Remove unused import
-from typing import Any, Dict, List, Optional, Union +from typing import Any, Dict, List, OptionalServer/src/cli/commands/editor.py (1)
8-8: Unused importprint_info.
print_infois imported but not used in this module.Remove unused import
-from cli.utils.output import format_output, print_error, print_success, print_info +from cli.utils.output import format_output, print_error, print_success
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Server/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (24)
Server/pyproject.tomlServer/src/cli/CLI_USAGE_GUIDE.mdServer/src/cli/__init__.pyServer/src/cli/commands/__init__.pyServer/src/cli/commands/animation.pyServer/src/cli/commands/asset.pyServer/src/cli/commands/audio.pyServer/src/cli/commands/code.pyServer/src/cli/commands/component.pyServer/src/cli/commands/editor.pyServer/src/cli/commands/gameobject.pyServer/src/cli/commands/lighting.pyServer/src/cli/commands/material.pyServer/src/cli/commands/prefab.pyServer/src/cli/commands/scene.pyServer/src/cli/commands/script.pyServer/src/cli/commands/ui.pyServer/src/cli/main.pyServer/src/cli/utils/__init__.pyServer/src/cli/utils/config.pyServer/src/cli/utils/connection.pyServer/src/cli/utils/output.pyServer/src/main.pyServer/tests/test_cli.py
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 368
File: MCPForUnity/UnityMcpServer~/src/resources/menu_items.py:15-15
Timestamp: 2025-11-05T18:23:12.349Z
Learning: In Unity MCP, the `name` parameter in the `mcp_for_unity_resource` decorator is the external API name exposed to MCP clients (LLMs, AI agents). The command string passed to `async_send_command_with_retry` or `async_send_with_unity_instance` (e.g., "get_menu_items") is the internal command identifier that must match the C# side. These are decoupled, allowing external API naming to evolve independently of internal command routing.
Learnt from: JohanHoltby
Repo: CoplayDev/unity-mcp PR: 309
File: MCPForUnity/Editor/Helpers/ServerInstaller.cs:478-508
Timestamp: 2025-10-13T13:41:00.086Z
Learning: In the MCPForUnityTools feature (MCPForUnity/Editor/Helpers/ServerInstaller.cs), the design intentionally forces users to have only one .py file per MCPForUnityTools folder to keep file tracking simple. Package-style tools (subdirectories with __init__.py) are not supported.
📚 Learning: 2025-12-29T04:54:17.743Z
Learnt from: dsarno
Repo: CoplayDev/unity-mcp PR: 490
File: Server/pyproject.toml:33-33
Timestamp: 2025-12-29T04:54:17.743Z
Learning: Pin the fastmcp dependency to an exact version in Server/pyproject.toml (e.g., exact string 2.14.1). Avoid range pins like >=2.13.0 to prevent breaking changes affecting MCP tools. Apply the same exact-version pinning approach according to the syntax of the package tool in use (e.g., Poetry: fastmcp = '2.14.1' or equivalent exact-specifier).
Applied to files:
Server/pyproject.toml
📚 Learning: 2025-10-13T13:41:00.086Z
Learnt from: JohanHoltby
Repo: CoplayDev/unity-mcp PR: 309
File: MCPForUnity/Editor/Helpers/ServerInstaller.cs:478-508
Timestamp: 2025-10-13T13:41:00.086Z
Learning: In the MCPForUnityTools feature (MCPForUnity/Editor/Helpers/ServerInstaller.cs), the design intentionally forces users to have only one .py file per MCPForUnityTools folder to keep file tracking simple. Package-style tools (subdirectories with __init__.py) are not supported.
Applied to files:
Server/pyproject.tomlServer/src/cli/CLI_USAGE_GUIDE.mdServer/src/cli/__init__.py
📚 Learning: 2025-09-04T01:01:11.927Z
Learnt from: dsarno
Repo: CoplayDev/unity-mcp PR: 260
File: UnityMcpBridge/UnityMcpServer~/src/server_version.txt:1-1
Timestamp: 2025-09-04T01:01:11.927Z
Learning: The UnityMcpBridge project is not maintaining changelogs yet, so don't suggest adding changelog entries for version bumps.
Applied to files:
Server/pyproject.toml
📚 Learning: 2025-10-13T13:27:23.040Z
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 316
File: TestProjects/UnityMCPTests/Assets/Tests/EditMode/Resources.meta:1-8
Timestamp: 2025-10-13T13:27:23.040Z
Learning: UnityMcpBridge is a legacy project kept for backwards compatibility; MCPForUnity is the only active Unity plugin project. GUID collisions between UnityMcpBridge and MCPForUnity are acceptable.
Applied to files:
Server/pyproject.toml
📚 Learning: 2025-11-05T18:23:12.349Z
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 368
File: MCPForUnity/UnityMcpServer~/src/resources/menu_items.py:15-15
Timestamp: 2025-11-05T18:23:12.349Z
Learning: In Unity MCP, the `name` parameter in the `mcp_for_unity_resource` decorator is the external API name exposed to MCP clients (LLMs, AI agents). The command string passed to `async_send_command_with_retry` or `async_send_with_unity_instance` (e.g., "get_menu_items") is the internal command identifier that must match the C# side. These are decoupled, allowing external API naming to evolve independently of internal command routing.
Applied to files:
Server/pyproject.tomlServer/src/main.pyServer/src/cli/CLI_USAGE_GUIDE.mdServer/src/cli/__init__.py
📚 Learning: 2025-10-03T22:11:46.002Z
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 301
File: docs/CUSTOM_TOOLS.md:54-62
Timestamp: 2025-10-03T22:11:46.002Z
Learning: In Unity MCP, the `description` parameter in the `mcp_for_unity_tool` decorator is technically optional but should always be included as a best practice. Without it, there's a higher chance that MCP clients will not parse the tool correctly. All Unity MCP tools should include the description in the decorator for compatibility.
Applied to files:
Server/pyproject.tomlServer/src/cli/CLI_USAGE_GUIDE.md
📚 Learning: 2025-09-05T16:22:04.960Z
Learnt from: dsarno
Repo: CoplayDev/unity-mcp PR: 265
File: README.md:204-204
Timestamp: 2025-09-05T16:22:04.960Z
Learning: In the Unity MCP project, the ServerInstaller.cs creates a symlink from ~/Library/AppSupport to ~/Library/Application Support on macOS to mitigate argument parsing and quoting issues in some MCP clients. The README documentation should use the shortened AppSupport path, not the full "Application Support" path with spaces.
Applied to files:
Server/src/cli/CLI_USAGE_GUIDE.md
📚 Learning: 2025-09-05T16:22:04.960Z
Learnt from: dsarno
Repo: CoplayDev/unity-mcp PR: 265
File: README.md:204-204
Timestamp: 2025-09-05T16:22:04.960Z
Learning: In the Unity MCP project, the ServerInstaller.cs creates a symlink from ~/Library/AppSupport to ~/Library/Application Support on macOS to avoid spaces in paths that can cause arg parsing and quoting issues in some MCP clients. The README documentation should use the shortened AppSupport path, not the full "Application Support" path.
Applied to files:
Server/src/cli/CLI_USAGE_GUIDE.md
🧬 Code graph analysis (10)
Server/src/cli/utils/connection.py (2)
Server/src/cli/main.py (1)
cli(70-103)Server/src/cli/utils/config.py (2)
get_config(34-39)CLIConfig(9-27)
Server/src/cli/main.py (16)
Server/src/cli/utils/config.py (3)
CLIConfig(9-27)set_config(42-45)get_config(34-39)Server/src/cli/utils/output.py (4)
format_output(8-23)print_error(176-178)print_success(171-173)print_info(186-188)Server/src/cli/utils/connection.py (5)
run_command(99-116)run_check_connection(139-141)run_list_instances(188-190)UnityConnectionError(13-15)warn_if_remote_host(18-37)Server/src/cli/commands/gameobject.py (1)
gameobject(14-16)Server/src/cli/commands/component.py (1)
component(14-16)Server/src/cli/commands/scene.py (1)
scene(13-15)Server/src/cli/commands/asset.py (1)
asset(14-16)Server/src/cli/commands/script.py (1)
script(14-16)Server/src/cli/commands/code.py (1)
code(13-15)Server/src/cli/commands/editor.py (1)
editor(13-15)Server/src/cli/commands/prefab.py (1)
prefab(13-15)Server/src/cli/commands/material.py (1)
material(14-16)Server/src/cli/commands/lighting.py (1)
lighting(13-15)Server/src/cli/commands/animation.py (1)
animation(13-15)Server/src/cli/commands/audio.py (1)
audio(13-15)Server/src/cli/commands/ui.py (1)
ui(13-15)
Server/src/cli/commands/code.py (4)
Server/src/cli/utils/config.py (1)
get_config(34-39)Server/src/cli/utils/output.py (3)
format_output(8-23)print_error(176-178)print_info(186-188)Server/src/cli/utils/connection.py (2)
run_command(99-116)UnityConnectionError(13-15)Server/src/cli/commands/script.py (1)
read(91-125)
Server/src/cli/commands/ui.py (5)
Server/src/cli/main.py (1)
cli(70-103)Server/src/cli/utils/config.py (1)
get_config(34-39)Server/src/cli/utils/output.py (3)
format_output(8-23)print_error(176-178)print_success(171-173)Server/src/cli/utils/connection.py (2)
run_command(99-116)UnityConnectionError(13-15)Server/src/cli/commands/component.py (1)
component(14-16)
Server/src/cli/commands/gameobject.py (3)
Server/src/cli/utils/config.py (1)
get_config(34-39)Server/src/cli/utils/output.py (3)
format_output(8-23)print_error(176-178)print_success(171-173)Server/src/cli/utils/connection.py (2)
run_command(99-116)UnityConnectionError(13-15)
Server/src/cli/commands/material.py (3)
Server/src/cli/utils/config.py (1)
get_config(34-39)Server/src/cli/utils/output.py (3)
format_output(8-23)print_error(176-178)print_success(171-173)Server/src/cli/utils/connection.py (2)
run_command(99-116)UnityConnectionError(13-15)
Server/src/cli/commands/script.py (3)
Server/src/cli/utils/config.py (1)
get_config(34-39)Server/src/cli/utils/output.py (2)
format_output(8-23)print_success(171-173)Server/src/cli/utils/connection.py (2)
run_command(99-116)UnityConnectionError(13-15)
Server/src/cli/commands/asset.py (3)
Server/src/cli/utils/config.py (1)
get_config(34-39)Server/src/cli/utils/output.py (3)
format_output(8-23)print_error(176-178)print_success(171-173)Server/src/cli/utils/connection.py (2)
run_command(99-116)UnityConnectionError(13-15)
Server/src/cli/utils/__init__.py (3)
Server/src/cli/utils/config.py (3)
CLIConfig(9-27)get_config(34-39)set_config(42-45)Server/src/cli/utils/connection.py (4)
run_command(99-116)run_check_connection(139-141)run_list_instances(188-190)UnityConnectionError(13-15)Server/src/cli/utils/output.py (5)
format_output(8-23)print_success(171-173)print_error(176-178)print_warning(181-183)print_info(186-188)
Server/src/cli/commands/prefab.py (3)
Server/src/cli/utils/config.py (1)
get_config(34-39)Server/src/cli/utils/output.py (3)
format_output(8-23)print_error(176-178)print_success(171-173)Server/src/cli/utils/connection.py (2)
run_command(99-116)UnityConnectionError(13-15)
🪛 markdownlint-cli2 (0.18.1)
Server/src/cli/CLI_USAGE_GUIDE.md
83-83: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
129-129: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Ruff (0.14.10)
Server/src/cli/commands/animation.py
33-33: Unused function argument: layer
(ARG001)
83-83: Local variable config is assigned to but never used
Remove assignment to unused variable config
(F841)
Server/src/main.py
336-336: Do not catch blind exception: Exception
(BLE001)
337-337: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
356-356: Do not catch blind exception: Exception
(BLE001)
Server/src/cli/utils/connection.py
29-29: Possible binding to all interfaces
(S104)
81-85: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
81-85: Avoid specifying long messages outside the exception class
(TRY003)
87-90: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
87-90: Avoid specifying long messages outside the exception class
(TRY003)
92-94: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
92-94: Avoid specifying long messages outside the exception class
(TRY003)
95-95: Do not catch blind exception: Exception
(BLE001)
96-96: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
96-96: Avoid specifying long messages outside the exception class
(TRY003)
135-135: Do not catch blind exception: Exception
(BLE001)
182-183: try-except-continue detected, consider logging the exception
(S112)
182-182: Do not catch blind exception: Exception
(BLE001)
185-185: Avoid specifying long messages outside the exception class
(TRY003)
Server/src/cli/commands/ui.py
91-91: Unused function argument: position
(ARG001)
147-147: Unused function argument: text
(ARG001)
195-195: Unused function argument: sprite
(ARG001)
Server/src/cli/commands/audio.py
31-31: Unused function argument: clip
(ARG001)
Server/tests/test_cli.py
738-738: Unused method argument: mock_unity_response
(ARG002)
745-745: Unused method argument: mock_unity_response
(ARG002)
764-764: Unused method argument: mock_unity_response
(ARG002)
Server/src/cli/utils/__init__.py
18-31: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
Server/src/cli/utils/output.py
188-188: String contains ambiguous ℹ (INFORMATION SOURCE). Did you mean i (LATIN SMALL LETTER I)?
(RUF001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Sourcery review
- GitHub Check: Sourcery review
🔇 Additional comments (34)
Server/pyproject.toml (2)
39-39: LGTM! Click dependency added appropriately.The Click dependency is added with a reasonable version constraint. Unlike
fastmcp(which requires exact pinning per project learnings), Click has strong semantic versioning guarantees and backwards compatibility, making the>=8.1.0constraint appropriate for a CLI framework.Based on learnings, only fastmcp requires exact version pinning to prevent breaking changes affecting MCP tools.
55-55: LGTM! Console script entry point configured correctly.The new
unity-mcpentry point is properly configured to invoke the CLI main function.Server/src/cli/CLI_USAGE_GUIDE.md (1)
1-734: LGTM! Comprehensive and well-structured CLI documentation.The usage guide is thorough, covering installation, syntax, common pitfalls, and extensive command examples. The documentation clearly explains argument vs. option syntax and multi-value parameter usage, which helps prevent common CLI mistakes.
Note: Static analysis warnings about missing language specifiers on lines 83 and 129 are false positives—those blocks show expected output and abstract patterns, not executable code.
Server/src/cli/utils/connection.py (6)
13-15: LGTM! Custom exception defined appropriately.
18-37: LGTM! Security warning appropriately implemented.The function correctly warns users when connecting to non-localhost servers. The static analysis warning about "0.0.0.0" (S104) is a false positive—the code checks whether the configured host equals "0.0.0.0", it doesn't bind to that address.
99-116: LGTM! Synchronous wrapper implemented correctly.
119-136: LGTM! Connection check implemented appropriately.The broad exception catch on line 135 is acceptable for a health check function—returning
Falseon any error is the correct behavior for a simple connectivity test.
139-141: LGTM! Synchronous wrapper implemented correctly.
188-190: LGTM! Synchronous wrapper implemented correctly.Server/src/cli/__init__.py (1)
1-3: LGTM! Package initializer properly configured.The CLI package is initialized with appropriate docstring and version metadata.
Server/src/cli/commands/__init__.py (1)
1-3: LGTM! Commands package initializer properly configured.The commands package is correctly initialized with a clear note that command registration happens in main.py, consistent with the pluggable architecture described in the PR.
Server/src/cli/commands/code.py (1)
63-64: code.py is correct; the bug is in script.py instead.Line 63 correctly checks for
"contents"(plural), which matches the actual response structure from themanage_scriptbackend (seeServer/src/services/tools/script_apply_edits.py:630). However, the relatedreadcommand inServer/src/cli/commands/script.py:117incorrectly checks for"content"(singular), which will cause it to fail silently and fall back to formatted output instead of displaying the script content directly. The script.py command should use"contents"to match the backend response and the code.py implementation.Likely an incorrect or invalid review comment.
Server/tests/test_cli.py (2)
1-27: Well-structured test fixtures and imports.The test suite is well-organized with clear section markers, appropriate fixtures for mocking CLI behavior, and comprehensive coverage of the CLI surface area.
806-874: Good integration-style test coverage with realistic workflows.The integration tests properly simulate multi-step workflows (create → modify → delete) with realistic response payloads, verifying both successful exit codes and expected output content.
Server/src/cli/commands/asset.py (1)
1-16: Clean module structure with consistent patterns.The asset command group follows a consistent pattern with proper error handling, JSON validation for properties, and appropriate confirmation prompts for destructive operations.
Server/src/cli/commands/prefab.py (1)
1-16: Well-implemented prefab command group.The prefab commands follow the established CLI patterns with consistent error handling and appropriate options for each operation.
Server/src/cli/utils/__init__.py (1)
1-31: Clean re-export module consolidating CLI utilities.The
__init__.pyproperly centralizes imports for easier consumption by command modules. The__all__list comprehensively covers the public API.Server/src/cli/commands/scene.py (2)
1-16: Comprehensive scene command group with good option coverage.The scene commands provide useful functionality with sensible defaults and consistent error handling patterns.
99-138: Smart scene loading with appropriate path/name detection.The load command intelligently differentiates between paths (ending in
.unity) and scene names, with proper validation for build index parsing.Server/src/cli/commands/script.py (3)
1-16: LGTM - Clean module structure and imports.The module setup follows the established patterns from other CLI command modules with appropriate imports and a well-defined Click group.
179-183: Good JSON validation with clear error messaging.The edit command properly validates JSON input before attempting the command, providing a clear error message on parse failure.
218-222: Unusedlevelparameter in validate command.The
leveloption is defined and parsed but not included in theparamsdictionary sent to the backend.🐛 Proposed fix
params: dict[str, Any] = { "uri": path, + "level": level, "include_diagnostics": True, }Likely an incorrect or invalid review comment.
Server/src/cli/main.py (2)
106-134: Status command implementation is well-structured.The status command provides useful feedback about server connectivity and displays connected Unity instances with relevant details (project, version, truncated hash). Good use of
print_infofor non-connected state.
163-170: Import inside function - acceptable for rarely-used command.The
jsonimport insideraw_commandis unconventional but acceptable since this is likely a power-user command that won't be called frequently.Server/src/cli/commands/component.py (3)
1-16: LGTM - Standard module structure.Clean imports and group definition following established patterns.
137-142: Good fallback strategy for value parsing.The silent fallback to string when JSON parsing fails is appropriate here, as users may pass simple string values that don't need JSON encoding.
194-200: No action needed—backend handler supports both property signatures.The
manage_componentsbackend handler already supports both the single property format (propertyandvaluefields) and batch properties format (propertiesdict). Integration tests confirm both signatures work correctly:
test_manage_components_set_property_singlevalidates the single property pathtest_manage_components_set_property_multiplevalidates the batch properties pathThe implementation explicitly handles both cases in sequence, so there is no API incompatibility.
Server/src/cli/commands/material.py (1)
141-149: Robust 3-tier value parsing.Good approach: try JSON first for complex types, then float for numeric values, then fall back to string. This provides flexibility for different property types.
Server/src/cli/commands/gameobject.py (1)
430-497: Well-designed relative move command.The move command has good directional options and clear separation of world/local space. The required
--referenceand--directionoptions ensure the command has the necessary context.Server/src/cli/utils/output.py (3)
26-31: Good defensive JSON serialization.The fallback to error object on serialization failure is a good defensive pattern that prevents crashes while providing debugging information.
60-70: Large list truncation is well-implemented.Shows first 5 and last 5 items with a clear indication of how many items are omitted. This balances information density with readability.
186-188: Intentional use of Unicode information symbol.The static analysis warning about the ambiguous
ℹ(INFORMATION SOURCE) character is a false positive - this is the intended emoji for the info message indicator. The character renders correctly in modern terminals.Server/src/cli/commands/editor.py (2)
63-132: Well-designed console command with dual functionality.The console command cleanly handles both reading and clearing the console using the
--clearflag, with early return after clear operation. The multiple filtering options (type, count, filter text, stacktrace) provide good flexibility.
266-299: Tests command implementation looks good.The test runner command appropriately handles optional timeout and mode selection. The results are formatted according to the global format setting.
| def play(target: str, state_name: str, layer: int, search_method: Optional[str]): | ||
| """Play an animation state on a target's Animator. | ||
| \b | ||
| Examples: | ||
| unity-mcp animation play "Player" "Walk" | ||
| unity-mcp animation play "Enemy" "Attack" --layer 1 | ||
| """ | ||
| config = get_config() | ||
|
|
||
| # Set Animator parameter to trigger state | ||
| params: dict[str, Any] = { | ||
| "action": "set_property", | ||
| "target": target, | ||
| "componentType": "Animator", | ||
| "property": "Play", | ||
| "value": state_name, | ||
| } | ||
|
|
||
| if search_method: | ||
| params["searchMethod"] = search_method | ||
|
|
||
| try: | ||
| result = run_command("manage_components", params, config) | ||
| click.echo(format_output(result, config.format)) | ||
| except UnityConnectionError as e: | ||
| print_error(str(e)) | ||
| sys.exit(1) |
There was a problem hiding this comment.
Unused layer parameter - likely missing from params.
The layer parameter is accepted (line 33) but never used in the command execution. It's not included in the params dict sent to Unity. This is likely a bug - if layer support is intended, add it to params. If not yet implemented, consider removing the option or adding a TODO comment.
🎯 Proposed fix if layer should be used
params: dict[str, Any] = {
"action": "set_property",
"target": target,
"componentType": "Animator",
"property": "Play",
"value": state_name,
+ "layer": layer,
}Or if not yet implemented:
@click.option(
"--layer", "-l",
default=0,
type=int,
- help="Animator layer."
+ help="Animator layer (not yet implemented)."
)Based on static analysis hints.
📝 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.
| def play(target: str, state_name: str, layer: int, search_method: Optional[str]): | |
| """Play an animation state on a target's Animator. | |
| \b | |
| Examples: | |
| unity-mcp animation play "Player" "Walk" | |
| unity-mcp animation play "Enemy" "Attack" --layer 1 | |
| """ | |
| config = get_config() | |
| # Set Animator parameter to trigger state | |
| params: dict[str, Any] = { | |
| "action": "set_property", | |
| "target": target, | |
| "componentType": "Animator", | |
| "property": "Play", | |
| "value": state_name, | |
| } | |
| if search_method: | |
| params["searchMethod"] = search_method | |
| try: | |
| result = run_command("manage_components", params, config) | |
| click.echo(format_output(result, config.format)) | |
| except UnityConnectionError as e: | |
| print_error(str(e)) | |
| sys.exit(1) | |
| def play(target: str, state_name: str, layer: int, search_method: Optional[str]): | |
| """Play an animation state on a target's Animator. | |
| \b | |
| Examples: | |
| unity-mcp animation play "Player" "Walk" | |
| unity-mcp animation play "Enemy" "Attack" --layer 1 | |
| """ | |
| config = get_config() | |
| # Set Animator parameter to trigger state | |
| params: dict[str, Any] = { | |
| "action": "set_property", | |
| "target": target, | |
| "componentType": "Animator", | |
| "property": "Play", | |
| "value": state_name, | |
| "layer": layer, | |
| } | |
| if search_method: | |
| params["searchMethod"] = search_method | |
| try: | |
| result = run_command("manage_components", params, config) | |
| click.echo(format_output(result, config.format)) | |
| except UnityConnectionError as e: | |
| print_error(str(e)) | |
| sys.exit(1) |
🧰 Tools
🪛 Ruff (0.14.10)
33-33: Unused function argument: layer
(ARG001)
🤖 Prompt for AI Agents
In @Server/src/cli/commands/animation.py around lines 33 - 60, The play function
accepts a layer parameter but never sends it to Unity; update the params dict in
play to include the layer (e.g., params["layer"] = layer) so the
"manage_components" call receives the layer value, or if layer support is not
yet intended remove the layer parameter from play's signature or add a clear
TODO comment and validation; locate the play function and modify the params
construction (or signature) accordingly to ensure the parameter is used or
intentionally omitted.
| "action": "set_property", | ||
| "target": target, | ||
| "componentType": "Animator", | ||
| "property": "Play", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's look at the animation.py file around line 48 with context
cat -n Server/src/cli/commands/animation.py | sed -n '40,60p'Repository: CoplayDev/unity-mcp
Length of output: 782
🏁 Script executed:
#!/bin/bash
# Search for Animator usage and Play method/property in the codebase
rg -nP -C3 'Animator.*Play|Play.*property|property.*Play' --type=pyRepository: CoplayDev/unity-mcp
Length of output: 924
🏁 Script executed:
#!/bin/bash
# Look for Unity/MCP API documentation or definitions
find . -type f \( -name "*.py" -o -name "*.md" -o -name "*.json" \) | xargs rg -l "Animator|manage_components" | head -20Repository: CoplayDev/unity-mcp
Length of output: 734
🏁 Script executed:
#!/bin/bash
# Check the manage_components implementation to understand the API
rg -nP -C5 'def manage_components|class.*Animator|Play' --type=py | head -100Repository: CoplayDev/unity-mcp
Length of output: 6923
🏁 Script executed:
#!/bin/bash
# Check the manage_components.py implementation
cat -n Server/src/services/tools/manage_components.py | head -150Repository: CoplayDev/unity-mcp
Length of output: 6004
🏁 Script executed:
#!/bin/bash
# Search for Play handling in C# or backend code
find . -type f -name "*.cs" | xargs rg -l "Play|Animator" | head -10Repository: CoplayDev/unity-mcp
Length of output: 10282
🏁 Script executed:
#!/bin/bash
# Look for any API documentation or schema that defines valid properties
rg -nP "Play|Animator.*property|property.*Animator" --type=py | grep -E "(define|schema|valid|property)" | head -20Repository: CoplayDev/unity-mcp
Length of output: 409
🏁 Script executed:
#!/bin/bash
# Search for manage_components in C# code
find . -type f -name "*.cs" | xargs rg -l "manage_components" | head -10Repository: CoplayDev/unity-mcp
Length of output: 9512
🏁 Script executed:
#!/bin/bash
# Look for Animator handling in C# code
find MCPForUnity -type f -name "*.cs" | xargs rg -A10 -B2 "Animator|Play" | head -150Repository: CoplayDev/unity-mcp
Length of output: 12347
🏁 Script executed:
#!/bin/bash
# Search for property handling/reflection in C# code
find MCPForUnity -type f -name "*.cs" | xargs rg -l "SetProperty|setProperty|property.*reflection" | head -10Repository: CoplayDev/unity-mcp
Length of output: 300
🏁 Script executed:
#!/bin/bash
# Read the ManageComponents.cs file
wc -l MCPForUnity/Editor/Tools/ManageComponents.csRepository: CoplayDev/unity-mcp
Length of output: 111
🏁 Script executed:
#!/bin/bash
# Get the key parts of ManageComponents.cs
cat -n MCPForUnity/Editor/Tools/ManageComponents.cs | head -200Repository: CoplayDev/unity-mcp
Length of output: 9723
🏁 Script executed:
#!/bin/bash
# Read ComponentOps.cs to understand property handling
cat -n MCPForUnity/Editor/Helpers/ComponentOps.cs | head -250Repository: CoplayDev/unity-mcp
Length of output: 11688
🏁 Script executed:
#!/bin/bash
# Continue reading ManageComponents.cs to see full SetProperty implementation and any special handling
cat -n MCPForUnity/Editor/Tools/ManageComponents.cs | sed -n '200,330p'Repository: CoplayDev/unity-mcp
Length of output: 5399
🏁 Script executed:
#!/bin/bash
# Search for any special Animator handling or custom properties
rg -n "Animator|Play" MCPForUnity/Editor/Tools/ManageComponents.cs MCPForUnity/Editor/Helpers/ComponentOps.csRepository: CoplayDev/unity-mcp
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Check if there are any integration tests that show expected behavior for Animator.Play
find . -type f -name "*test*.cs" -o -name "*Test*.cs" | xargs rg -l "Animator.*Play|animation.*play" -i 2>/dev/null | head -5Repository: CoplayDev/unity-mcp
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Check if there's any custom Animator implementation or extension in MCPForUnity
find MCPForUnity -type f -name "*.cs" | xargs rg -l "Animator" | head -10Repository: CoplayDev/unity-mcp
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Look for any tests or documentation about animation commands
find . -type f \( -name "*animation*" -o -name "*animator*" \) -type f | grep -E "\.(cs|py|md)$"Repository: CoplayDev/unity-mcp
Length of output: 101
🏁 Script executed:
#!/bin/bash
# Check CLI_USAGE_GUIDE.md for animation command documentation
grep -A 20 -B 5 "animation\|animator" Server/src/cli/CLI_USAGE_GUIDE.md -iRepository: CoplayDev/unity-mcp
Length of output: 1084
🏁 Script executed:
#!/bin/bash
# Let's examine the full animation.py file to understand the context
cat -n Server/src/cli/commands/animation.pyRepository: CoplayDev/unity-mcp
Length of output: 3080
🏁 Script executed:
#!/bin/bash
# Check if Animator is extended or wrapped anywhere in MCPForUnity
find MCPForUnity -type f -name "*.cs" | xargs rg -B5 -A10 "class.*Animator|Animator.*Play" 2>/dev/null | head -100Repository: CoplayDev/unity-mcp
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Check the test_manage_components.py to see what the expected behavior is
cat -n Server/tests/integration/test_manage_components.py | head -200Repository: CoplayDev/unity-mcp
Length of output: 7161
🏁 Script executed:
#!/bin/bash
# Search for any method invocation or custom extension for Animator.Play in C#
find MCPForUnity -type f -name "*.cs" | xargs rg -B3 -A3 "\.Play\(" | grep -A3 -B3 -i animatorRepository: CoplayDev/unity-mcp
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Check if there's a way to invoke methods (not just properties) on components
rg -n "MethodInfo|InvokeMethod|Invoke" MCPForUnity/Editor/Helpers/ComponentOps.cs MCPForUnity/Editor/Tools/ManageComponents.csRepository: CoplayDev/unity-mcp
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Let's verify: does Animator actually have a public Play property?
web_search "Unity Animator Play property vs method C# API"Repository: CoplayDev/unity-mcp
Length of output: 113
🌐 Web query:
Unity Animator Play property C# API documentation
💡 Result:
Animator.Play (C#) — signatures and description
-
Signatures:
- void Play(string stateName, int layer = -1, float normalizedTime = float.NegativeInfinity);
- void Play(int stateNameHash, int layer = -1, float normalizedTime = float.NegativeInfinity). [1]
-
Parameters:
- stateName: state full name (include layer name, e.g. "Base Layer.Bounce") — string version calls Animator.StringToHash internally.
- stateNameHash: precomputed hash (use 0 to change current state time).
- layer: layer index (-1 to search first match).
- normalizedTime: start time as 0..1 (NegativeInfinity means use default behavior). [1][2]
-
Behavior:
- Plays the specified Animator state immediately; normalizedTime controls where in the state to start. Using the string form will convert to a hash internally; for frequent calls use a precomputed hash for better performance. If transitions exist, blending behavior depends on transition settings. [1][2]
References:
[1] Unity Scripting API — Animator.Play (Unity Docs).
[2] Unity Scripting API — Animator.Play (Unity Docs, alternate version / examples).
🏁 Script executed:
#!/bin/bash
# Final check: search for any MethodInfo or method invocation support in the codebase
find MCPForUnity Server -type f -name "*.cs" -o -name "*.py" | xargs rg -l "MethodInfo|InvokeMethod|\.Invoke\(" | head -10Repository: CoplayDev/unity-mcp
Length of output: 382
🏁 Script executed:
#!/bin/bash
# Check if there's an alternative approach or fallback for animation in the codebase
rg -n "animation|animator" Server/src/cli/commands/ -i --type=py | grep -i "play\|state"Repository: CoplayDev/unity-mcp
Length of output: 946
"Play" is a method, not a property—the animation play command will fail.
Animator.Play() is a void method, not a settable property. The current implementation attempts to set a property named "Play" via reflection, which will fail at runtime with "Property or field 'Play' not found on component 'Animator'."
The manage_components backend only supports setting properties and fields, not invoking methods. Either implement method invocation support in the manage_components tool or use an alternative approach (e.g., a custom animation-specific tool similar to ManageVFX.cs).
🤖 Prompt for AI Agents
In @Server/src/cli/commands/animation.py at line 48, The PR is trying to set
"Play" as a property but Animator.Play is a method; update the code that emits
the component change (the entry with "property": "Play") to either (A) call a
method-invocation path in the manage_components backend (implement method
invocation support there and handle calling Animator.Play(...)) or (B) replace
this manage_components call with an animation-specific tool (similar to
ManageVFX.cs) that invokes Animator.Play on the target component; locate the
emitter that writes the "property": "Play" entry and change it to emit a
method-invoke request (or swap to the custom animation tool) and wire up any
required parameters for Animator.Play accordingly.
| @audio.command("play") | ||
| @click.argument("target") | ||
| @click.option( | ||
| "--clip", "-c", | ||
| default=None, | ||
| help="Audio clip path to play." | ||
| ) | ||
| @click.option( | ||
| "--search-method", | ||
| type=click.Choice(["by_name", "by_path", "by_id"]), | ||
| default=None, | ||
| help="How to find the target." | ||
| ) | ||
| def play(target: str, clip: Optional[str], search_method: Optional[str]): | ||
| """Play audio on a target's AudioSource. | ||
| \b | ||
| Examples: | ||
| unity-mcp audio play "MusicPlayer" | ||
| unity-mcp audio play "SFXSource" --clip "Assets/Audio/explosion.wav" | ||
| """ | ||
| config = get_config() | ||
|
|
||
| params: dict[str, Any] = { | ||
| "action": "set_property", | ||
| "target": target, | ||
| "componentType": "AudioSource", | ||
| "property": "Play", | ||
| "value": True, | ||
| } | ||
|
|
||
| if search_method: | ||
| params["searchMethod"] = search_method | ||
|
|
||
| try: | ||
| result = run_command("manage_components", params, config) | ||
| click.echo(format_output(result, config.format)) | ||
| except UnityConnectionError as e: | ||
| print_error(str(e)) | ||
| sys.exit(1) |
There was a problem hiding this comment.
The --clip option is declared but never used.
The clip parameter is accepted but never included in the params dictionary sent to Unity. If specifying an audio clip is a valid use case, it should be added to the payload; otherwise, remove the option.
🔧 Proposed fix to use the clip option
params: dict[str, Any] = {
"action": "set_property",
"target": target,
"componentType": "AudioSource",
"property": "Play",
"value": True,
}
+ if clip:
+ params["clip"] = clip
if search_method:
params["searchMethod"] = search_method🧰 Tools
🪛 Ruff (0.14.10)
31-31: Unused function argument: clip
(ARG001)
🤖 Prompt for AI Agents
In @Server/src/cli/commands/audio.py around lines 18 - 57, The play command
accepts a --clip option but never uses it; update the play function so that when
clip is not None you add it to the params payload (e.g., params["clip"] = clip)
before calling run_command("manage_components", params, config); if specifying a
clip is not a supported use case, remove the --clip Click option and the clip
parameter from play instead.
| @@ -0,0 +1,71 @@ | |||
| """Code CLI commands - search and read source code.""" | |||
There was a problem hiding this comment.
Docstring overpromises functionality.
The module docstring mentions "search and read source code" but only the read command is implemented. Consider updating the docstring to reflect the actual functionality or add a placeholder/TODO for the search command.
📝 Suggested docstring fix
-"""Code CLI commands - search and read source code."""
+"""Code CLI commands - read source code."""📝 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.
| """Code CLI commands - search and read source code.""" | |
| """Code CLI commands - read source code.""" |
🤖 Prompt for AI Agents
In @Server/src/cli/commands/code.py at line 1, The module docstring claims
"search and read source code" but only the read command is implemented; update
the top-level docstring in Server/src/cli/commands/code.py to accurately reflect
implemented functionality (e.g., "Read source code" or "Read source code (search
not yet implemented)") or add a clear TODO note about implementing the search
command, and mention the implemented symbol 'read' so reviewers can verify
consistency.
| # Add components separately since componentsToAdd doesn't work | ||
| if components and (result.get("success") or result.get("data") or result.get("result")): | ||
| component_list = [c.strip() for c in components.split(",")] | ||
| for component in component_list: | ||
| run_command("manage_components", { | ||
| "action": "add", | ||
| "target": name, | ||
| "componentType": component, | ||
| }, config) |
There was a problem hiding this comment.
Component addition loop lacks individual error handling.
If any component fails to add, the error isn't reported and subsequent components aren't attempted. Consider wrapping each run_command in try/except or collecting failures.
🔧 Suggested improvement with error collection
# Add components separately since componentsToAdd doesn't work
if components and (result.get("success") or result.get("data") or result.get("result")):
component_list = [c.strip() for c in components.split(",")]
+ failed_components = []
for component in component_list:
- run_command("manage_components", {
- "action": "add",
- "target": name,
- "componentType": component,
- }, config)
+ try:
+ run_command("manage_components", {
+ "action": "add",
+ "target": name,
+ "componentType": component,
+ }, config)
+ except UnityConnectionError:
+ failed_components.append(component)
+ if failed_components:
+ print_warning(f"Failed to add components: {', '.join(failed_components)}")📝 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.
| # Add components separately since componentsToAdd doesn't work | |
| if components and (result.get("success") or result.get("data") or result.get("result")): | |
| component_list = [c.strip() for c in components.split(",")] | |
| for component in component_list: | |
| run_command("manage_components", { | |
| "action": "add", | |
| "target": name, | |
| "componentType": component, | |
| }, config) | |
| # Add components separately since componentsToAdd doesn't work | |
| if components and (result.get("success") or result.get("data") or result.get("result")): | |
| component_list = [c.strip() for c in components.split(",")] | |
| failed_components = [] | |
| for component in component_list: | |
| try: | |
| run_command("manage_components", { | |
| "action": "add", | |
| "target": name, | |
| "componentType": component, | |
| }, config) | |
| except UnityConnectionError: | |
| failed_components.append(component) | |
| if failed_components: | |
| print_warning(f"Failed to add components: {', '.join(failed_components)}") |
🤖 Prompt for AI Agents
In @Server/src/cli/commands/gameobject.py around lines 181 - 189, The component
addition loop currently calls run_command("manage_components", ...) for each
entry in component_list without per-item error handling; wrap each run_command
call in a try/except (or check its return value) inside the for-loop that
iterates component_list, log or collect failures (including component name and
error/response) and continue to the next component, and after the loop report
any collected failures (or raise/return a consolidated error) so callers can see
which components failed; references: components, result, component_list,
run_command, "manage_components", name, config.
| params: dict[str, Any] = { | ||
| "action": "delete", | ||
| "name": path.split("/")[-1].replace(".cs", ""), | ||
| "path": "/".join(path.split("/")[:-1]) or "Assets", |
There was a problem hiding this comment.
Same path parsing fragility as in read command.
This uses the same path-splitting logic with the same edge case concerns.
🤖 Prompt for AI Agents
In @Server/src/cli/commands/script.py around lines 147 - 150, The params
construction is using fragile string-splitting on path; update the logic that
builds params (the "params" dict in the delete command) to use robust path
utilities instead of path.split: derive the script name with
pathlib.Path(path).stem (or os.path.splitext(os.path.basename(path))[0]) and
derive the parent with pathlib.Path(path).parent.as_posix() (or
os.path.dirname(path)), falling back to "Assets" when parent is empty or ".";
then assign those values into the existing params dict ("action": "delete",
"name", "path") so behavior matches the safer parsing used in the read command.
| @click.option( | ||
| "--position", | ||
| nargs=2, | ||
| type=float, | ||
| default=(0, 0), | ||
| help="Anchored position X Y." | ||
| ) | ||
| def create_text(name: str, parent: str, text: str, position: tuple): | ||
| """Create a UI Text element (TextMeshPro). | ||
| \b | ||
| Examples: | ||
| unity-mcp ui create-text "TitleText" --parent "MainUI" --text "Hello World" | ||
| """ | ||
| config = get_config() | ||
|
|
||
| try: | ||
| # Step 1: Create empty GameObject with parent | ||
| result = run_command("manage_gameobject", { | ||
| "action": "create", | ||
| "name": name, | ||
| "parent": parent, | ||
| }, config) | ||
|
|
||
| if not (result.get("success") or result.get("data") or result.get("result")): | ||
| click.echo(format_output(result, config.format)) | ||
| return | ||
|
|
||
| # Step 2: Add RectTransform and TextMeshProUGUI | ||
| run_command("manage_components", { | ||
| "action": "add", | ||
| "target": name, | ||
| "componentType": "TextMeshProUGUI", | ||
| }, config) | ||
|
|
||
| # Step 3: Set text content | ||
| run_command("manage_components", { | ||
| "action": "set_property", | ||
| "target": name, | ||
| "componentType": "TextMeshProUGUI", | ||
| "property": "text", | ||
| "value": text, | ||
| }, config) | ||
|
|
||
| click.echo(format_output(result, config.format)) | ||
| print_success(f"Created Text: {name}") | ||
| except UnityConnectionError as e: | ||
| print_error(str(e)) | ||
| sys.exit(1) |
There was a problem hiding this comment.
The --position and --text options are declared but never used in create_text.
The position tuple is accepted but not applied to the RectTransform, and while text is correctly used in line 125, the position functionality is missing. The position should be applied after component creation.
🔧 Proposed fix to apply position
# Step 3: Set text content
run_command("manage_components", {
"action": "set_property",
"target": name,
"componentType": "TextMeshProUGUI",
"property": "text",
"value": text,
}, config)
+
+ # Step 4: Set position if specified
+ if position != (0, 0):
+ run_command("manage_components", {
+ "action": "set_property",
+ "target": name,
+ "componentType": "RectTransform",
+ "property": "anchoredPosition",
+ "value": {"x": position[0], "y": position[1]},
+ }, config)🧰 Tools
🪛 Ruff (0.14.10)
91-91: Unused function argument: position
(ARG001)
🤖 Prompt for AI Agents
In @Server/src/cli/commands/ui.py around lines 84 - 132, create_text accepts a
position tuple but never applies it to the RectTransform; after adding the
TextMeshProUGUI component (in create_text) call run_command("manage_components",
{ "action": "set_property", "target": name, "componentType": "RectTransform",
"property": "anchoredPosition", "value": list(position) }, config) (or use
"anchoredPosition3D"/"localPosition" if your backend expects 3D) and ensure you
pass the position variable (converted to the expected list/format) so the
RectTransform receives the anchored position.
| @click.option( | ||
| "--text", "-t", | ||
| default="Button", | ||
| help="Button label text." | ||
| ) | ||
| def create_button(name: str, parent: str, text: str): | ||
| """Create a UI Button. | ||
| \b | ||
| Examples: | ||
| unity-mcp ui create-button "StartButton" --parent "MainUI" --text "Start Game" | ||
| """ | ||
| config = get_config() | ||
|
|
||
| try: | ||
| # Step 1: Create empty GameObject with parent | ||
| result = run_command("manage_gameobject", { | ||
| "action": "create", | ||
| "name": name, | ||
| "parent": parent, | ||
| }, config) | ||
|
|
||
| if not (result.get("success") or result.get("data") or result.get("result")): | ||
| click.echo(format_output(result, config.format)) | ||
| return | ||
|
|
||
| # Step 2: Add Button and Image components | ||
| for component in ["Image", "Button"]: | ||
| run_command("manage_components", { | ||
| "action": "add", | ||
| "target": name, | ||
| "componentType": component, | ||
| }, config) | ||
|
|
||
| click.echo(format_output(result, config.format)) | ||
| print_success(f"Created Button: {name}") | ||
| except UnityConnectionError as e: | ||
| print_error(str(e)) | ||
| sys.exit(1) |
There was a problem hiding this comment.
The --text option for create-button is declared but never used.
The text parameter is accepted but not applied to the button. Unity UI Buttons typically have a child Text component for the label. Either implement the text functionality or remove the option to avoid confusion.
🔧 Proposed fix to implement button text
# Step 2: Add Button and Image components
for component in ["Image", "Button"]:
run_command("manage_components", {
"action": "add",
"target": name,
"componentType": component,
}, config)
+
+ # Step 3: Create child text element if text specified
+ if text and text != "Button":
+ text_name = f"{name}_Text"
+ run_command("manage_gameobject", {
+ "action": "create",
+ "name": text_name,
+ "parent": name,
+ }, config)
+ run_command("manage_components", {
+ "action": "add",
+ "target": text_name,
+ "componentType": "TextMeshProUGUI",
+ }, config)
+ run_command("manage_components", {
+ "action": "set_property",
+ "target": text_name,
+ "componentType": "TextMeshProUGUI",
+ "property": "text",
+ "value": text,
+ }, config)🧰 Tools
🪛 Ruff (0.14.10)
147-147: Unused function argument: text
(ARG001)
| @click.option( | ||
| "--sprite", "-s", | ||
| default=None, | ||
| help="Sprite asset path." | ||
| ) | ||
| def create_image(name: str, parent: str, sprite: Optional[str]): | ||
| """Create a UI Image. | ||
| \b | ||
| Examples: | ||
| unity-mcp ui create-image "Background" --parent "MainUI" | ||
| unity-mcp ui create-image "Icon" --parent "MainUI" --sprite "Assets/Sprites/icon.png" | ||
| """ | ||
| config = get_config() | ||
|
|
||
| try: | ||
| # Step 1: Create empty GameObject with parent | ||
| result = run_command("manage_gameobject", { | ||
| "action": "create", | ||
| "name": name, | ||
| "parent": parent, | ||
| }, config) | ||
|
|
||
| if not (result.get("success") or result.get("data") or result.get("result")): | ||
| click.echo(format_output(result, config.format)) | ||
| return | ||
|
|
||
| # Step 2: Add Image component | ||
| run_command("manage_components", { | ||
| "action": "add", | ||
| "target": name, | ||
| "componentType": "Image", | ||
| }, config) | ||
|
|
||
| click.echo(format_output(result, config.format)) | ||
| print_success(f"Created Image: {name}") | ||
| except UnityConnectionError as e: | ||
| print_error(str(e)) | ||
| sys.exit(1) |
There was a problem hiding this comment.
The --sprite option for create-image is declared but never used.
The sprite parameter is accepted but not applied to the Image component. If a sprite path is provided, it should be set on the Image component.
🔧 Proposed fix to apply sprite
# Step 2: Add Image component
run_command("manage_components", {
"action": "add",
"target": name,
"componentType": "Image",
}, config)
+
+ # Step 3: Set sprite if specified
+ if sprite:
+ run_command("manage_components", {
+ "action": "set_property",
+ "target": name,
+ "componentType": "Image",
+ "property": "sprite",
+ "value": sprite,
+ }, config)🧰 Tools
🪛 Ruff (0.14.10)
195-195: Unused function argument: sprite
(ARG001)
🤖 Prompt for AI Agents
In @Server/src/cli/commands/ui.py around lines 190 - 228, The --sprite option is
accepted by create_image but never applied; after the existing run_command call
that adds the Image component in create_image, if sprite is not None call
run_command again to set the Image component's sprite property on the created
GameObject (use the same config and target name), e.g. invoke the
manage_components command to set the Image component's "sprite" (or include it
in the add call as a properties/payload field) so the provided sprite path is
applied to the component.
| @classmethod | ||
| def from_env(cls) -> "CLIConfig": | ||
| """Create config from environment variables.""" | ||
| return cls( | ||
| host=os.environ.get("UNITY_MCP_HOST", "127.0.0.1"), | ||
| port=int(os.environ.get("UNITY_MCP_HTTP_PORT", "8080")), | ||
| timeout=int(os.environ.get("UNITY_MCP_TIMEOUT", "30")), | ||
| format=os.environ.get("UNITY_MCP_FORMAT", "text"), | ||
| unity_instance=os.environ.get("UNITY_MCP_INSTANCE"), | ||
| ) |
There was a problem hiding this comment.
Add error handling for environment variable parsing.
Lines 23-24 convert environment variables to integers without error handling. If UNITY_MCP_HTTP_PORT or UNITY_MCP_TIMEOUT contain invalid values, the conversion will raise ValueError and crash the application.
Consider adding try-except blocks or validation to provide clearer error messages.
🛡️ Suggested improvement
@classmethod
def from_env(cls) -> "CLIConfig":
"""Create config from environment variables."""
+ try:
+ port = int(os.environ.get("UNITY_MCP_HTTP_PORT", "8080"))
+ except ValueError:
+ raise ValueError(f"Invalid UNITY_MCP_HTTP_PORT value: {os.environ.get('UNITY_MCP_HTTP_PORT')}")
+
+ try:
+ timeout = int(os.environ.get("UNITY_MCP_TIMEOUT", "30"))
+ except ValueError:
+ raise ValueError(f"Invalid UNITY_MCP_TIMEOUT value: {os.environ.get('UNITY_MCP_TIMEOUT')}")
+
return cls(
host=os.environ.get("UNITY_MCP_HOST", "127.0.0.1"),
- port=int(os.environ.get("UNITY_MCP_HTTP_PORT", "8080")),
- timeout=int(os.environ.get("UNITY_MCP_TIMEOUT", "30")),
+ port=port,
+ timeout=timeout,
format=os.environ.get("UNITY_MCP_FORMAT", "text"),
unity_instance=os.environ.get("UNITY_MCP_INSTANCE"),
)🤖 Prompt for AI Agents
In @Server/src/cli/utils/config.py around lines 18 - 27, The from_env
classmethod on CLIConfig currently casts UNITY_MCP_HTTP_PORT and
UNITY_MCP_TIMEOUT to int without validation, which can raise ValueError; wrap
the int conversions in validation/error handling inside CLIConfig.from_env
(catch ValueError/TypeError), provide a clear error message including the env
var name and invalid value (and optionally fallback to defaults), and re-raise
or exit gracefully so the app doesn’t crash with an opaque traceback; reference
the symbols CLIConfig.from_env, UNITY_MCP_HTTP_PORT, and UNITY_MCP_TIMEOUT when
locating and updating the code.
1 participant
Hi team! The reason for this CLI is that the current MCP repo - even accelerated by some optimization - still needs sometime waiting and sometime can be undeterministic, with a lot of details of the calls and parameters hidden from the user's sight to save space. I am recently working on a game-based skill benchmark, and this skill caught my attention. To make the generation more automated, CLI seems to be an intuitive way and a new, faster approach to executing user commands.
Some advantages I can think of:
And some cons I can think of:
That would be all, looking forward to the team's feedback. Happy coding! @msanatan @dsarno
PS: Did a solid amount of AI coding with this one, then did throughful test myself (CLI tools are easy to test!)
Summary by Sourcery
Introduce a Click-based
unity-mcpCLI for interacting with the Unity MCP server over HTTP, including routes on the server to accept CLI commands and list Unity instances, along with configuration, output formatting, and tests.New Features:
unity-mcpcommand-line entry point with global options and multiple command groups for game objects, components, scenes, assets, scripts, editor control, prefabs, materials, lighting, audio, UI, animation, code, and raw commands.Enhancements:
Build:
unity-mcpconsole script inpyproject.tomland add Click as a runtime dependency.Documentation:
CLI_USAGE_GUIDE.md) describing installation, command structure, common pitfalls, and detailed command reference for AI assistants and developers.Tests:
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.