diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7b919f0e..4014a5ed 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -109,34 +109,45 @@ jobs: godot-tests-linux: name: Godot tests / Linux runs-on: ubuntu-latest - container: - image: barichello/godot-ci:4.6.2 steps: - uses: actions/checkout@v4 + - uses: chickensoft-games/setup-godot@v2 + with: + version: 4.6.2 + use-dotnet: false + - name: Set up Python - timeout-minutes: 5 - run: | - apt-get update -qq && apt-get install -y -qq python3 python3-pip python3-venv curl > /dev/null - python3 -m venv /tmp/venv - /tmp/venv/bin/pip install -e ".[dev]" + uses: actions/setup-python@v5 + with: + python-version: "3.13" + + - name: Install dependencies + run: pip install -e ".[dev]" - name: Start MCP server - run: bash script/ci-start-server /tmp/venv/bin/python + run: bash script/ci-start-server - - name: Start Godot editor (headless) + - name: Import and validate GDScript + timeout-minutes: 3 run: | - timeout 30 godot --headless --path test_project --import || true - timeout 120 godot --headless --path test_project --editor & + godot --headless --path test_project --import > /tmp/godot-import.log 2>&1 || true + bash script/ci-check-gdscript /tmp/godot-import.log + + - name: Start Godot editor (headless) + run: godot --headless --path test_project --editor & - name: Run handler tests + timeout-minutes: 3 run: bash script/ci-godot-tests - name: Reload smoke test + timeout-minutes: 2 run: bash script/ci-reload-test - name: Quit smoke test + timeout-minutes: 2 run: bash script/ci-quit-test godot-tests-macos: diff --git a/CLAUDE.md b/CLAUDE.md index e2e80142..c3064ba6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -76,7 +76,7 @@ The dock checks the GitHub releases API on startup. If a newer version exists, a ### Python tests ```bash -pytest -v # 375 unit + integration tests +pytest -v # 387 unit + integration tests ``` ### Godot-side tests @@ -198,6 +198,10 @@ New features don't ship without tests. Regressions are caught before they merge. - **Re-entrant `_process()` during save**: `EditorInterface.save_scene()` internally renders a preview thumbnail, which triggers frame processing. If `Connection._process()` runs during this, WebSocket polling and command dispatch re-enter, crashing Godot (`SIGABRT` in `_save_scene_with_preview`). Fixed by setting `Connection.pause_processing = true` around save calls in `SceneHandler`. Any new handler that calls `save_scene()`, `save_scene_as()`, or `save_all_scenes()` must do the same. - **GDScript tests must not call `EditorInterface.save_scene()` or `scene_create`/`scene_open`**: These trigger modal dialogs or scene switches that freeze or crash the test runner. Test only validation/error paths for these operations in GDScript; full behavior is covered by Python integration tests. - **GDScript tests must not call `quit_editor` or `reload_plugin`**: These terminate or restart the plugin, killing the test runner. Tested via Python integration tests and CI smoke scripts (`script/ci-quit-test`, `script/ci-reload-test`). (Note: plugin command names stay `quit_editor` / `reload_plugin`; the MCP tool names are `editor_quit` / `editor_reload_plugin`.) +- **Resilient test discovery**: `_discover_suites()` in `test_handler.gd` catches per-file load errors and returns `{suites, errors}`. Individual broken test scripts do not prevent the rest from running. The `errors` list reports which scripts failed to load. +- **CI GDScript validation**: `script/ci-check-gdscript` runs before Godot tests in CI. It scans the `--import` log for `SCRIPT ERROR` / `Parse Error` lines and fails the build early if any GDScript has syntax errors, before the test runner even starts. +- **CI Linux runner**: Linux Godot CI uses `chickensoft-games/setup-godot@v2` on `ubuntu-latest` (not a Docker image). All three OS jobs (Linux, macOS, Windows) use the same chickensoft action for consistent Godot setup. Step timeouts are set on test and smoke steps to prevent CI hangs. +- **Sleep before test_run in CI**: `script/ci-godot-tests` includes a short sleep (8s) after Godot startup to let the editor filesystem scan settle before running tests. Without this, test discovery can miss files. ## What NOT to do @@ -206,3 +210,4 @@ New features don't ship without tests. Regressions are caught before they merge. - Don't use `pop_front()` on arrays in hot paths — use index + slice - Don't add error handling in individual tools — `GodotClient.send()` raises on errors - Don't use Python-style `"""docstrings"""` in GDScript — use `##` comments +- Don't forget the `overwrite` parameter on `animation_create` / `animation_create_simple` — without it, creating an animation with the same name errors instead of replacing diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index fe61eea4..6429722d 100644 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -29,6 +29,20 @@ test_run suite=scene # run one suite test_results_get # review last results ``` +### CI regression range helper + +When CI starts failing, identify the regression window (last green → first red): + +```bash +script/ci-find-regression-range hi-godot/godot-ai ci.yml main +``` + +If your local clone has a valid `origin` GitHub remote, you can omit `owner/repo`: + +```bash +script/ci-find-regression-range +``` + ## Dev Server with Auto-Reload For Python-side changes without restarting Godot: diff --git a/docs/implementation-plan.md b/docs/implementation-plan.md index e9fc4edf..665bc339 100644 --- a/docs/implementation-plan.md +++ b/docs/implementation-plan.md @@ -28,7 +28,7 @@ Historical bootstrap material, architecture detail, packaging mechanics, go/no-g - [x] Runtime feedback loop: `project.run`/`project.stop`, `editor.screenshot`, `performance.get_monitors`, `logs.clear` - [ ] Runtime iteration loop is complete enough for AI-driven feel tuning - [ ] Release/install path is complete enough for new users -- [~] Polished game-production extensions have started — `ui_*` (anchor presets, `ui_build_layout` composer), `theme_*` (color/constant/font-size/stylebox_flat/apply), and `animation_*` (AnimationPlayer + `animation_create_simple` composer) shipped; `camera_*`, `audio_*`, and animation preset helpers still pending +- [~] Polished game-production extensions have started — `ui_*` (anchor presets, `ui_build_layout` composer, `theme_override_*` pseudo-properties), `theme_*` (color/constant/font-size/stylebox_flat with per-side overrides/apply), and `animation_*` (AnimationPlayer + `animation_create_simple` composer + delete/validate + overwrite support) shipped; `camera_*`, `audio_*`, and animation preset helpers still pending ## What This Plan Optimizes For @@ -57,7 +57,7 @@ Historical bootstrap material, architecture detail, packaging mechanics, go/no-g ### High-Leverage Authoring - [x] `batch.execute` with stop-on-first-error semantics and optional grouped undo -- [x] `node.rename` with sibling-collision validation and char-safety checks (NodePath/script references in OTHER nodes are not auto-updated — documented in the tool) +- [x] `node.rename` with sibling-collision validation and char-safety checks (NodePath/script references in OTHER nodes are not auto-updated — documented in the tool). Now also allows renaming the scene root node. - [x] complex `node.set_property` (`Resource` via res:// path, `NodePath`, `Array`, `Dictionary`, `StringName`) - [x] `script.patch` shipped — anchor-based `old_text` → `new_text` replace with ambiguity detection and optional `replace_all` @@ -80,7 +80,7 @@ Historical bootstrap material, architecture detail, packaging mechanics, go/no-g - [x] batch execution is shipped with a clear contract - [x] multi-instance routing works in practice - [x] `script.patch` decision is made (shipped: anchor-based replace) -- [x] test coverage and smoke coverage increase where the new runtime loop needs it (375 Python + 312 GDScript = 687 total) +- [x] test coverage and smoke coverage increase where the new runtime loop needs it (387 Python + 333 GDScript = 720 total) --- @@ -92,7 +92,7 @@ See [Packaging & Distribution](packaging-distribution.md) for full detail. The s - [ ] PyPI / `uvx` path works reliably - [ ] desktop binary path is real, not aspirational - [~] plugin is downloadable from the Godot AssetLib — release ZIP workflow ships `godot-ai-plugin.zip` via GitHub Releases; AssetLib submission in progress; dock has self-update check -- [x] CI covers Python tests, Godot-side tests, and release-smoke install paths (3 OS × 2 Python + 3 OS Godot + release-smoke) +- [x] CI covers Python tests, Godot-side tests, and release-smoke install paths (3 OS × 2 Python + 3 OS Godot + release-smoke). Linux CI uses `chickensoft-games/setup-godot` on `ubuntu-latest`. GDScript parse validation (`ci-check-gdscript`) runs before tests. Step timeouts prevent hangs. - [x] bump-and-release workflow — `gh workflow run bump-and-release.yml -f bump=patch/minor/major` bumps versions, commits, tags, and triggers release build - [ ] compatibility guidance is published and maintained - [ ] a new user can get from zero to working in under 10 minutes @@ -132,9 +132,9 @@ These are not the next things to do blindly. They are the extensions that matter - `camera.*` for follow, bounds, zoom, and screen shake - `resource.create` / `resource.save` / `resource.instantiate` - `scene.instantiate` and `scene.inherit` - - [ ] critical path for reusable `button.tscn` / `enemy.tscn` instanced into many parent scenes — the piece that turns the UI composer and node_create flows into "real Godot project structure" instead of one-shot scene builds + - [~] `node_create` now supports a `scene_path` parameter for instancing a `.tscn` as a child node. This covers the basic "instance a prefab" use case. Dedicated `scene.instantiate` (with transform overrides) and `scene.inherit` (inherited scenes) are still pending for full reusable-scene workflows. - `animation_player.*` / `animation_tree.*` - - [~] AnimationPlayer scaffolding shipped (`animation_player_create`, `animation_create`, `animation_add_property_track`, `animation_add_method_track`, `animation_set_autoplay`, `animation_play`, `animation_stop`, `animation_list`, `animation_get`, `animation_create_simple` composer) + - [~] AnimationPlayer scaffolding shipped (`animation_player_create`, `animation_create`, `animation_add_property_track`, `animation_add_method_track`, `animation_set_autoplay`, `animation_play`, `animation_stop`, `animation_list`, `animation_get`, `animation_create_simple` composer, `animation_delete`, `animation_validate`). `animation_create` and `animation_create_simple` support `overwrite` parameter for re-creating animations in place. - [ ] **Preset helpers** — `animation_preset_fade`, `animation_preset_slide_in`, `animation_preset_shake`, `animation_preset_pulse_loop`. Thin wrappers over `animation_create_simple` that bake in the right transition / loop_mode / two-keyframe shape for each effect. Cuts a "fade in this Panel" from a 6-line tween spec to one call. - [ ] **Bezier and audio tracks** — `animation_add_bezier_track` (for hand-tuned curves where keyframe interpolation isn't enough) and `animation_add_audio_track` (timed AudioStreamPlayer cues; needs the audio resource handler first). - [ ] **`animation_tree.*`** — state-machine and blend-tree authoring for character locomotion (idle ↔ walk ↔ run blends, attack one-shots). Larger surface; depends on the AnimationPlayer being solid first. diff --git a/docs/testing-strategy.md b/docs/testing-strategy.md index 886b5f16..fc99b555 100644 --- a/docs/testing-strategy.md +++ b/docs/testing-strategy.md @@ -1,6 +1,6 @@ # Godot AI — Testing Strategy -*Updated 2026-04-14* +*Updated 2026-04-16* This document defines how Godot AI should prove that new capability is real, stable, and safe to extend. @@ -99,9 +99,17 @@ If a tool has undo semantics, readiness constraints, or cross-session behavior, The CI stack should exercise at least three tiers: -- Python unit and integration tests -- Godot-side editor test suites -- release-surface smoke, especially install and packaging paths once distribution work is active +- Python unit and integration tests (3 OS x 2 Python versions) +- Godot-side editor test suites (3 OS via `chickensoft-games/setup-godot@v2` on GitHub Actions runners) +- release-surface smoke, especially install and packaging paths once distribution work is active (3 OS) + +### CI hardening measures + +- **GDScript validation**: `script/ci-check-gdscript` runs after `--import` and before the editor launches. It scans the import log for `SCRIPT ERROR` / `Parse Error` lines and fails the build immediately if any GDScript file has syntax errors. This catches broken scripts before the test runner starts. +- **Step timeouts**: test and smoke steps have `timeout-minutes` set to prevent CI hangs from frozen Godot processes. +- **Filesystem scan settling**: `script/ci-godot-tests` includes a short sleep after editor startup so the filesystem scan completes and test discovery finds all suites. +- **Resilient test discovery**: `test_handler.gd` catches per-file load errors during `_discover_suites()`. A broken test file does not prevent the rest of the suite from running; errors are reported in the response alongside successful results. +- **Regression diagnostics**: `script/ci-find-regression-range` helps identify which commits introduced a CI regression by binary-searching recent history. This should stay aligned with the release work in [packaging-distribution.md](packaging-distribution.md). diff --git a/docs/tool-taxonomy.md b/docs/tool-taxonomy.md index bbb945fb..755e26d0 100644 --- a/docs/tool-taxonomy.md +++ b/docs/tool-taxonomy.md @@ -1,6 +1,6 @@ # Godot AI — Tool Taxonomy -*Updated 2026-04-14* +*Updated 2026-04-16* This document describes the intended Godot-native tool surface. @@ -157,7 +157,7 @@ These are the next layers once the core runtime loop is dependable. - `ui.*` for HUDs, menus, upgrade screens, theme/layout work - `camera.*` for follow, bounds, zoom, shake, and capture helpers -- `animation_*` — AnimationPlayer authoring shipped (player + animation creation, property/method tracks, autoplay, dev-time play/stop, list/get, `animation_create_simple` composer). Auto-attaches a default `AnimationLibrary` on first write. Works for 2D, 3D, and UI; `animation_tree.*`, bezier/audio tracks, preset helpers, and 3D material-fade coercion are tracked as follow-ups in `implementation-plan.md`. +- `animation_*` — AnimationPlayer authoring shipped (player + animation creation, property/method tracks, autoplay, dev-time play/stop, list/get, `animation_create_simple` composer, `animation_delete`, `animation_validate`). `animation_create` and `animation_create_simple` support an `overwrite` parameter to replace existing animations in place. Auto-attaches a default `AnimationLibrary` on first write. Works for 2D, 3D, and UI; `animation_tree.*`, bezier/audio tracks, preset helpers, and 3D material-fade coercion are tracked as follow-ups in `implementation-plan.md`. - `audio.*` These are the tools that move the project from "functional prototype" toward "readable and polished prototype." diff --git a/plugin/addons/godot_ai/handlers/animation_handler.gd b/plugin/addons/godot_ai/handlers/animation_handler.gd index 71ea509f..a0e00724 100644 --- a/plugin/addons/godot_ai/handlers/animation_handler.gd +++ b/plugin/addons/godot_ai/handlers/animation_handler.gd @@ -110,23 +110,20 @@ func create_animation(params: Dictionary) -> Dictionary: library = AnimationLibrary.new() created_library = true + var overwrite: bool = params.get("overwrite", false) + var old_anim: Animation = null if library.has_animation(anim_name): - return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, - "Animation '%s' already exists. Delete it first or choose a different name." % anim_name) + if not overwrite: + return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, + "Animation '%s' already exists. Pass overwrite=true or delete it first." % anim_name) + old_anim = library.get_animation(anim_name) var anim := Animation.new() anim.length = length anim.loop_mode = _LOOP_MODES[loop_mode_str] - _undo_redo.create_action("MCP: Create animation %s" % anim_name) - if created_library: - _undo_redo.add_do_method(player, "add_animation_library", "", library) - _undo_redo.add_undo_method(player, "remove_animation_library", "") - _undo_redo.add_do_reference(library) - _undo_redo.add_do_method(library, "add_animation", anim_name, anim) - _undo_redo.add_undo_method(library, "remove_animation", anim_name) - _undo_redo.add_do_reference(anim) - _undo_redo.commit_action() + _commit_animation_add("MCP: Create animation %s" % anim_name, + player, library, created_library, anim_name, anim, old_anim) return { "data": { @@ -135,6 +132,50 @@ func create_animation(params: Dictionary) -> Dictionary: "length": length, "loop_mode": loop_mode_str, "library_created": created_library, + "overwritten": old_anim != null, + "undoable": true, + } + } + + +# ============================================================================ +# animation_delete +# ============================================================================ + +func delete_animation(params: Dictionary) -> Dictionary: + var player_path: String = params.get("player_path", "") + var anim_name: String = params.get("animation_name", "") + + if player_path.is_empty(): + return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "Missing required param: player_path") + if anim_name.is_empty(): + return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "Missing required param: animation_name") + + var resolved := _resolve_player(player_path) + if resolved.has("error"): + return resolved + var player: AnimationPlayer = resolved.player + + if not player.has_animation(anim_name): + return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, + "Animation '%s' not found on player at %s" % [anim_name, player_path]) + + var library: AnimationLibrary = resolved.library + if library == null: + return McpErrorCodes.make(McpErrorCodes.INTERNAL_ERROR, "No default library found") + + var old_anim: Animation = library.get_animation(anim_name) + + _undo_redo.create_action("MCP: Delete animation %s" % anim_name) + _undo_redo.add_do_method(library, "remove_animation", anim_name) + _undo_redo.add_undo_method(library, "add_animation", anim_name, old_anim) + _undo_redo.add_do_reference(old_anim) # prevent GC so undo→redo works + _undo_redo.commit_action() + + return { + "data": { + "player_path": player_path, + "animation_name": anim_name, "undoable": true, } } @@ -495,6 +536,78 @@ func get_animation(params: Dictionary) -> Dictionary: } +# ============================================================================ +# animation_validate (read-only) +# ============================================================================ + +func validate_animation(params: Dictionary) -> Dictionary: + var player_path: String = params.get("player_path", "") + var anim_name: String = params.get("animation_name", "") + + if player_path.is_empty(): + return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "Missing required param: player_path") + if anim_name.is_empty(): + return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "Missing required param: animation_name") + + var resolved := _resolve_player_read(player_path) + if resolved.has("error"): + return resolved + var player: AnimationPlayer = resolved.player + + if not player.has_animation(anim_name): + return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, + "Animation '%s' not found on player at %s" % [anim_name, player_path]) + + var anim: Animation = player.get_animation(anim_name) + + var root_node: Node = null + if player.is_inside_tree(): + var rn := player.root_node + if rn != NodePath(): + root_node = player.get_node_or_null(rn) + if root_node == null: + root_node = player.get_parent() + + var broken_tracks: Array[Dictionary] = [] + var valid_count := 0 + + for i in anim.get_track_count(): + var track_path_str := str(anim.track_get_path(i)) + var colon := track_path_str.rfind(":") + var node_part: String + if colon >= 0: + node_part = track_path_str.substr(0, colon) + else: + node_part = track_path_str + + var target_node: Node = null + if root_node != null: + target_node = root_node.get_node_or_null(node_part) + + if target_node == null: + broken_tracks.append({ + "index": i, + "path": track_path_str, + "type": _track_type_to_string(anim.track_get_type(i)), + "issue": "node_not_found", + "node_path": node_part, + }) + else: + valid_count += 1 + + return { + "data": { + "player_path": player_path, + "animation_name": anim_name, + "track_count": anim.get_track_count(), + "valid_count": valid_count, + "broken_count": broken_tracks.size(), + "broken_tracks": broken_tracks, + "valid": broken_tracks.is_empty(), + } + } + + # ============================================================================ # animation_create_simple (composer) # ============================================================================ @@ -544,9 +657,13 @@ func create_simple(params: Dictionary) -> Dictionary: library = AnimationLibrary.new() created_library = true + var overwrite: bool = params.get("overwrite", false) + var old_anim: Animation = null if library.has_animation(anim_name): - return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, - "Animation '%s' already exists. Delete it first or choose a different name." % anim_name) + if not overwrite: + return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, + "Animation '%s' already exists. Pass overwrite=true or delete it first." % anim_name) + old_anim = library.get_animation(anim_name) # Compute auto length only when length is absent or null; reject explicit # invalid values instead of silently falling through to auto-compute. @@ -598,15 +715,8 @@ func create_simple(params: Dictionary) -> Dictionary: _do_add_property_track(anim, entry.track_path, "linear", entry.keyframes) # One atomic undo action. - _undo_redo.create_action("MCP: Create animation %s (%d tracks)" % [anim_name, anim.get_track_count()]) - if created_library: - _undo_redo.add_do_method(player, "add_animation_library", "", library) - _undo_redo.add_undo_method(player, "remove_animation_library", "") - _undo_redo.add_do_reference(library) - _undo_redo.add_do_method(library, "add_animation", anim_name, anim) - _undo_redo.add_undo_method(library, "remove_animation", anim_name) - _undo_redo.add_do_reference(anim) - _undo_redo.commit_action() + _commit_animation_add("MCP: Create animation %s (%d tracks)" % [anim_name, anim.get_track_count()], + player, library, created_library, anim_name, anim, old_anim) return { "data": { @@ -616,11 +726,45 @@ func create_simple(params: Dictionary) -> Dictionary: "loop_mode": loop_mode_str, "track_count": anim.get_track_count(), "library_created": created_library, + "overwritten": old_anim != null, "undoable": true, } } +# ============================================================================ +# Helpers — undo +# ============================================================================ + +## Shared undo setup for create_animation and create_simple. Handles both +## fresh-create and overwrite cases in a single atomic action. +func _commit_animation_add( + action_label: String, + player: AnimationPlayer, + library: AnimationLibrary, + created_library: bool, + anim_name: String, + anim: Animation, + old_anim: Animation, ## null when not overwriting +) -> void: + _undo_redo.create_action(action_label) + if created_library: + _undo_redo.add_do_method(player, "add_animation_library", "", library) + _undo_redo.add_undo_method(player, "remove_animation_library", "") + _undo_redo.add_do_reference(library) + if old_anim != null: + _undo_redo.add_do_method(library, "remove_animation", anim_name) + _undo_redo.add_do_method(library, "add_animation", anim_name, anim) + if old_anim != null: + _undo_redo.add_undo_method(library, "remove_animation", anim_name) + _undo_redo.add_undo_method(library, "add_animation", anim_name, old_anim) + _undo_redo.add_do_reference(old_anim) + else: + _undo_redo.add_undo_method(library, "remove_animation", anim_name) + _undo_redo.add_do_reference(anim) + _undo_redo.commit_action() + + # ============================================================================ # Helpers — resolution # ============================================================================ diff --git a/plugin/addons/godot_ai/handlers/editor_handler.gd b/plugin/addons/godot_ai/handlers/editor_handler.gd index 9d1e8c97..a425c2a1 100644 --- a/plugin/addons/godot_ai/handlers/editor_handler.gd +++ b/plugin/addons/godot_ai/handlers/editor_handler.gd @@ -119,7 +119,7 @@ func take_screenshot(params: Dictionary) -> Dictionary: var custom_azimuth = params.get("azimuth", null) var custom_fov = params.get("fov", null) - var viewport: SubViewport + var viewport: Viewport match source: "viewport": viewport = EditorInterface.get_editor_viewport_3d() @@ -128,6 +128,8 @@ func take_screenshot(params: Dictionary) -> Dictionary: "game": if not EditorInterface.is_playing_scene(): return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "Game is not running — use source='viewport' or start the project first") + # The game viewport is the editor window's root viewport, not a SubViewport. + # Using the main screen's viewport captures the game view area. viewport = EditorInterface.get_editor_main_screen().get_viewport() if viewport == null: return McpErrorCodes.make(McpErrorCodes.EDITOR_NOT_READY, "Could not access game viewport") diff --git a/plugin/addons/godot_ai/handlers/node_handler.gd b/plugin/addons/godot_ai/handlers/node_handler.gd index 8ea51083..c4c7c743 100644 --- a/plugin/addons/godot_ai/handlers/node_handler.gd +++ b/plugin/addons/godot_ai/handlers/node_handler.gd @@ -15,14 +15,7 @@ func create_node(params: Dictionary) -> Dictionary: var node_type: String = params.get("type", "") var node_name: String = params.get("name", "") var parent_path: String = params.get("parent_path", "") - - if node_type.is_empty(): - return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "Missing required param: type") - - if not ClassDB.class_exists(node_type): - return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "Unknown node type: %s" % node_type) - if not ClassDB.is_parent_class(node_type, "Node"): - return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "%s is not a Node type" % node_type) + var scene_path: String = params.get("scene_path", "") var scene_root := EditorInterface.get_edited_scene_root() if scene_root == null: @@ -34,9 +27,31 @@ func create_node(params: Dictionary) -> Dictionary: if parent == null: return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "Parent not found: %s" % parent_path) - var new_node: Node = ClassDB.instantiate(node_type) - if new_node == null: - return McpErrorCodes.make(McpErrorCodes.INTERNAL_ERROR, "Failed to instantiate %s" % node_type) + var new_node: Node + + if not scene_path.is_empty(): + # Scene instancing path — load and instantiate a PackedScene. + if not scene_path.begins_with("res://"): + return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "scene_path must start with res://") + if not ResourceLoader.exists(scene_path): + return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "Scene not found: %s" % scene_path) + var packed_scene = ResourceLoader.load(scene_path) + if packed_scene == null or not packed_scene is PackedScene: + return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "Resource at %s is not a PackedScene" % scene_path) + new_node = packed_scene.instantiate() + if new_node == null: + return McpErrorCodes.make(McpErrorCodes.INTERNAL_ERROR, "Failed to instantiate scene: %s" % scene_path) + else: + # ClassDB path — create by type. + if node_type.is_empty(): + return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "Missing required param: type (or provide scene_path)") + if not ClassDB.class_exists(node_type): + return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "Unknown node type: %s" % node_type) + if not ClassDB.is_parent_class(node_type, "Node"): + return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "%s is not a Node type" % node_type) + new_node = ClassDB.instantiate(node_type) + if new_node == null: + return McpErrorCodes.make(McpErrorCodes.INTERNAL_ERROR, "Failed to instantiate %s" % node_type) if not node_name.is_empty(): new_node.name = node_name @@ -48,15 +63,20 @@ func create_node(params: Dictionary) -> Dictionary: _undo_redo.add_undo_method(parent, "remove_child", new_node) _undo_redo.commit_action() - return { - "data": { - "name": new_node.name, - "type": new_node.get_class(), - "path": ScenePath.from_node(new_node, scene_root), - "parent_path": ScenePath.from_node(parent, scene_root), - "undoable": true, - } + # For instanced scenes, set owner on descendants so they serialize properly. + if not scene_path.is_empty(): + _set_owner_recursive(new_node, scene_root) + + var response := { + "name": new_node.name, + "type": new_node.get_class(), + "path": ScenePath.from_node(new_node, scene_root), + "parent_path": ScenePath.from_node(parent, scene_root), + "undoable": true, } + if not scene_path.is_empty(): + response["scene_path"] = scene_path + return {"data": response} func delete_node(params: Dictionary) -> Dictionary: @@ -214,9 +234,6 @@ func rename_node(params: Dictionary) -> Dictionary: if new_name.validate_node_name() != new_name: return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "Invalid characters in name: %s" % new_name) - if node == scene_root: - return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "Cannot rename the scene root") - var old_name := String(node.name) if old_name == new_name: return { @@ -230,10 +247,12 @@ func rename_node(params: Dictionary) -> Dictionary: } } - var parent := node.get_parent() - for sibling in parent.get_children(): - if sibling != node and String(sibling.name) == new_name: - return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "A sibling already has the name '%s'" % new_name) + # Scene root has no siblings, so skip sibling collision check. + if node != scene_root: + var parent := node.get_parent() + for sibling in parent.get_children(): + if sibling != node and String(sibling.name) == new_name: + return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "A sibling already has the name '%s'" % new_name) _undo_redo.create_action("MCP: Rename %s to %s" % [old_name, new_name]) _undo_redo.add_do_property(node, "name", new_name) diff --git a/plugin/addons/godot_ai/handlers/signal_handler.gd b/plugin/addons/godot_ai/handlers/signal_handler.gd index 85530cb9..145b4189 100644 --- a/plugin/addons/godot_ai/handlers/signal_handler.gd +++ b/plugin/addons/godot_ai/handlers/signal_handler.gd @@ -119,11 +119,15 @@ func _resolve_signal_params(params: Dictionary) -> Dictionary: var source := ScenePath.resolve(params.path, scene_root) if source == null: - return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "Source node not found: %s" % params.path) + source = _resolve_autoload(params.path) + if source == null: + return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "Source node not found: %s (not in scene tree or autoloads)" % params.path) var target := ScenePath.resolve(params.target, scene_root) if target == null: - return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "Target node not found: %s" % params.target) + target = _resolve_autoload(params.target) + if target == null: + return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "Target node not found: %s (not in scene tree or autoloads)" % params.target) return { "source": source, @@ -134,6 +138,18 @@ func _resolve_signal_params(params: Dictionary) -> Dictionary: } +## Attempt to resolve a path as an autoload singleton. +## Uses direct ProjectSettings lookup (O(1)) instead of scanning all properties. +func _resolve_autoload(path: String) -> Node: + var name := path.trim_prefix("/") + if not ProjectSettings.has_setting("autoload/" + name): + return null + var tree := Engine.get_main_loop() + if tree is SceneTree: + return (tree as SceneTree).root.get_node_or_null(name) + return null + + func _signal_response(source: Node, signal_name: String, target: Node, method: String, scene_root: Node) -> Dictionary: return { "source": ScenePath.from_node(source, scene_root), diff --git a/plugin/addons/godot_ai/handlers/test_handler.gd b/plugin/addons/godot_ai/handlers/test_handler.gd index 323a9686..4fd7d412 100644 --- a/plugin/addons/godot_ai/handlers/test_handler.gd +++ b/plugin/addons/godot_ai/handlers/test_handler.gd @@ -21,9 +21,16 @@ func run_tests(params: Dictionary) -> Dictionary: var test_filter: String = params.get("test_name", "") var verbose: bool = params.get("verbose", false) - var suites := _discover_suites() + var discovery := _discover_suites() + var suites: Array = discovery.suites if suites.is_empty(): - return {"data": {"error": "No test suites found in res://tests/", "total": 0}} + var msg := "No test suites found in res://tests/" + if not discovery.errors.is_empty(): + msg += " (%d script(s) failed to load: %s)" % [ + discovery.errors.size(), + ", ".join(discovery.errors), + ] + return {"data": {"error": msg, "total": 0, "load_errors": discovery.errors}} var ctx := { "undo_redo": _undo_redo, @@ -31,6 +38,8 @@ func run_tests(params: Dictionary) -> Dictionary: } var results := _runner.run_suites(suites, suite_filter, test_filter, ctx, verbose) + if not discovery.errors.is_empty(): + results["load_errors"] = discovery.errors return {"data": results} @@ -39,25 +48,35 @@ func get_test_results(params: Dictionary) -> Dictionary: return {"data": _runner.get_results(verbose)} -func _discover_suites() -> Array[McpTestSuite]: - var suites: Array[McpTestSuite] = [] +func _discover_suites() -> Dictionary: + ## Returns {"suites": Array, "errors": Array[String]}. + ## Resilient: a broken script doesn't kill discovery of the rest. + var suites := [] + var errors: Array[String] = [] var dir := DirAccess.open("res://tests") if dir == null: - return suites + return {"suites": suites, "errors": ["DirAccess.open('res://tests') returned null"]} dir.list_dir_begin() var file_name := dir.get_next() while not file_name.is_empty(): if file_name.begins_with("test_") and file_name.ends_with(".gd"): - var script := ResourceLoader.load("res://tests/" + file_name, "", ResourceLoader.CACHE_MODE_IGNORE) - if script: + var path := "res://tests/" + file_name + var script = ResourceLoader.load(path, "", ResourceLoader.CACHE_MODE_IGNORE) + if script == null: + errors.append(file_name) + elif script.can_instantiate(): var instance = script.new() if instance is McpTestSuite: suites.append(instance) + else: + errors.append("%s (not McpTestSuite)" % file_name) + else: + errors.append("%s (cannot instantiate)" % file_name) file_name = dir.get_next() - ## Sort by suite name for deterministic order - suites.sort_custom(func(a: McpTestSuite, b: McpTestSuite) -> bool: + ## Sort by suite name for deterministic order. + suites.sort_custom(func(a, b) -> bool: return a.suite_name() < b.suite_name() ) - return suites + return {"suites": suites, "errors": errors} diff --git a/plugin/addons/godot_ai/handlers/theme_handler.gd b/plugin/addons/godot_ai/handlers/theme_handler.gd index 6b291c33..49e5fda1 100644 --- a/plugin/addons/godot_ai/handlers/theme_handler.gd +++ b/plugin/addons/godot_ai/handlers/theme_handler.gd @@ -38,6 +38,16 @@ func create_theme(params: Dictionary) -> Dictionary: "Theme already exists at %s (pass overwrite=true to replace)" % path ) + # Ensure parent directory exists. make_dir_recursive is idempotent — + # no need to check dir_exists first (avoids TOCTOU race). + var dir_path := path.get_base_dir() + var mkdir_err := DirAccess.make_dir_recursive_absolute(dir_path) + if mkdir_err != OK and mkdir_err != ERR_ALREADY_EXISTS: + return McpErrorCodes.make( + McpErrorCodes.INTERNAL_ERROR, + "Failed to create directory: %s (error %d)" % [dir_path, mkdir_err] + ) + var theme := Theme.new() var save_err := ResourceSaver.save(theme, path) if save_err != OK: @@ -213,10 +223,19 @@ func set_stylebox_flat(params: Dictionary) -> Dictionary: sb.border_color = bc if params.has("border_width"): sb.set_border_width_all(int(params.border_width)) + for side_key in ["border_width_top", "border_width_bottom", "border_width_left", "border_width_right"]: + if params.has(side_key): + sb.set(side_key, int(params[side_key])) if params.has("corner_radius"): sb.set_corner_radius_all(int(params.corner_radius)) + for corner_key in ["corner_radius_top_left", "corner_radius_top_right", "corner_radius_bottom_left", "corner_radius_bottom_right"]: + if params.has(corner_key): + sb.set(corner_key, int(params[corner_key])) if params.has("content_margin"): sb.set_content_margin_all(float(params.content_margin)) + for margin_key in ["content_margin_top", "content_margin_bottom", "content_margin_left", "content_margin_right"]: + if params.has(margin_key): + sb.set(margin_key, float(params[margin_key])) if params.has("shadow_color"): var sc := _parse_color(params.shadow_color) if sc == null: diff --git a/plugin/addons/godot_ai/handlers/ui_handler.gd b/plugin/addons/godot_ai/handlers/ui_handler.gd index 0be08c67..287f4114 100644 --- a/plugin/addons/godot_ai/handlers/ui_handler.gd +++ b/plugin/addons/godot_ai/handlers/ui_handler.gd @@ -279,9 +279,71 @@ func _build_subtree(spec: Dictionary) -> Dictionary: return {"node": node, "created": created} +## Mapping from theme_override_* property prefixes to their add/remove methods. +const _THEME_OVERRIDE_MAP := { + "theme_override_colors/": { + "add": "add_theme_color_override", + "remove": "remove_theme_color_override", + "coerce_type": TYPE_COLOR, + }, + "theme_override_constants/": { + "add": "add_theme_constant_override", + "remove": "remove_theme_constant_override", + "coerce_type": TYPE_INT, + }, + "theme_override_font_sizes/": { + "add": "add_theme_font_size_override", + "remove": "remove_theme_font_size_override", + "coerce_type": TYPE_INT, + }, + "theme_override_styles/": { + "add": "add_theme_stylebox_override", + "remove": "remove_theme_stylebox_override", + "coerce_type": TYPE_OBJECT, + }, +} + + ## Apply a property to a newly-instantiated node. Handles Color/Vector2/NodePath ## coercion from JSON-friendly forms. Returns null on success, error dict on failure. func _apply_property(node: Node, prop: String, value: Variant) -> Variant: + # Handle theme_override_* pseudo-properties before the regular property scan. + for prefix in _THEME_OVERRIDE_MAP: + if prop.begins_with(prefix): + if not node is Control: + return McpErrorCodes.make( + McpErrorCodes.INVALID_PARAMS, + "theme_override_* requires a Control node (got %s)" % node.get_class() + ) + var override_name := prop.substr(prefix.length()) + var info: Dictionary = _THEME_OVERRIDE_MAP[prefix] + var coerce_type: int = info.coerce_type + + # For stylebox overrides, load from a res:// path. + if coerce_type == TYPE_OBJECT: + if value is String and value.begins_with("res://"): + var res := ResourceLoader.load(value) + if res == null or not res is StyleBox: + return McpErrorCodes.make( + McpErrorCodes.INVALID_PARAMS, + "Style resource not found or not a StyleBox: %s" % value + ) + node.call(info.add, override_name, res) + else: + return McpErrorCodes.make( + McpErrorCodes.INVALID_PARAMS, + "theme_override_styles/ expects a res:// path to a StyleBox" + ) + else: + var coercion := _coerce_for_type(value, coerce_type) + if not coercion.ok: + return McpErrorCodes.make( + McpErrorCodes.INVALID_PARAMS, + "Cannot coerce '%s' for %s" % [value, prop] + ) + node.call(info.add, override_name, coercion.value) + return null + var found := false var prop_type := TYPE_NIL for p in node.get_property_list(): diff --git a/plugin/addons/godot_ai/plugin.gd b/plugin/addons/godot_ai/plugin.gd index efc2bc13..39f88874 100644 --- a/plugin/addons/godot_ai/plugin.gd +++ b/plugin/addons/godot_ai/plugin.gd @@ -115,6 +115,8 @@ func _enter_tree() -> void: _dispatcher.register("animation_list", animation_handler.list_animations) _dispatcher.register("animation_get", animation_handler.get_animation) _dispatcher.register("animation_create_simple", animation_handler.create_simple) + _dispatcher.register("animation_delete", animation_handler.delete_animation) + _dispatcher.register("animation_validate", animation_handler.validate_animation) _connection.dispatcher = _dispatcher add_child(_connection) diff --git a/script/ci-check-gdscript b/script/ci-check-gdscript new file mode 100755 index 00000000..9bdfba79 --- /dev/null +++ b/script/ci-check-gdscript @@ -0,0 +1,33 @@ +#!/usr/bin/env bash +# Validate all GDScript files for parse errors. +# Uses a single Godot --import invocation (already run by CI) rather than +# spawning one process per file. Falls back to per-file checks if the +# import log isn't provided. +# +# Usage: +# bash script/ci-check-gdscript [import_log] +# +# If import_log is provided, scans it for SCRIPT ERROR / Parse Error lines. +# If omitted, runs a single import pass and checks its output. +set -euo pipefail + +PROJECT_DIR="${PROJECT_DIR:-test_project}" + +if [ "${1:-}" != "" ] && [ -f "$1" ]; then + IMPORT_LOG="$1" +else + echo "Running GDScript validation via --import..." + IMPORT_LOG=$(mktemp) + godot --headless --path "$PROJECT_DIR" --import > "$IMPORT_LOG" 2>&1 || true +fi + +# Check for parse errors in the import output. +ERRORS=$(grep -ciE "SCRIPT ERROR|Parse Error|Failed to load script" "$IMPORT_LOG" 2>/dev/null || echo "0") + +if [ "$ERRORS" -gt 0 ]; then + echo "GDScript parse errors found:" + grep -iE "SCRIPT ERROR|Parse Error|Failed to load script" "$IMPORT_LOG" | sed 's/^/ /' + exit 1 +fi + +echo "All GDScript files OK" diff --git a/script/ci-find-regression-range b/script/ci-find-regression-range new file mode 100755 index 00000000..017fbdfb --- /dev/null +++ b/script/ci-find-regression-range @@ -0,0 +1,115 @@ +#!/usr/bin/env bash +# Identify the "last green" and "first red" commits for a workflow. +# +# Usage: +# script/ci-find-regression-range [owner/repo] [workflow_file] [branch] +# +# Examples: +# script/ci-find-regression-range hi-godot/godot-ai ci.yml main +# +# Requirements: +# - python3 +# - GitHub API access (optionally authenticated via GH_TOKEN) +set -euo pipefail + +REPO="${1:-}" +WORKFLOW="${2:-ci.yml}" +BRANCH="${3:-main}" + +if [[ -z "$REPO" ]]; then + ORIGIN_URL="$(git config --get remote.origin.url || true)" + if [[ "$ORIGIN_URL" =~ github.com[:/]([^/]+/[^/.]+)(\.git)?$ ]]; then + REPO="${BASH_REMATCH[1]}" + else + echo "ERROR: Could not infer owner/repo from git remote." + echo "Usage: $0 owner/repo [workflow_file] [branch]" + exit 1 + fi +fi + +python3 - "$REPO" "$WORKFLOW" "$BRANCH" <<'PY' +import json +import os +import sys +import urllib.error +import urllib.parse +import urllib.request + +repo, workflow, branch = sys.argv[1:4] +base = f"https://api.github.com/repos/{repo}/actions/workflows/{workflow}/runs" +query = urllib.parse.urlencode({ + "branch": branch, + "event": "push", + "status": "completed", + "per_page": 100, +}) +url = f"{base}?{query}" + +headers = { + "Accept": "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", +} +token = os.getenv("GH_TOKEN") +if token: + headers["Authorization"] = f"Bearer {token}" + +req = urllib.request.Request(url, headers=headers) +try: + with urllib.request.urlopen(req, timeout=30) as resp: + payload = json.load(resp) +except urllib.error.HTTPError as exc: + body = exc.read().decode("utf-8", errors="replace") + print(f"ERROR: GitHub API request failed ({exc.code}): {body[:300]}") + sys.exit(1) +except Exception as exc: + print(f"ERROR: GitHub API request failed: {exc}") + sys.exit(1) + +runs = payload.get("workflow_runs", []) +if not runs: + print("No completed push runs found for this workflow/branch.") + sys.exit(1) + +recent_failures = [] +last_green = None +for run in runs: + concl = run.get("conclusion") + if concl == "success": + last_green = run + break + if concl == "failure": + recent_failures.append(run) + +if last_green is None: + print("No successful run found in the fetched window (last 100 runs).") + sys.exit(1) + +if not recent_failures: + print("Latest run is green (no current failure streak found).") + print(f"Last green SHA: {last_green.get('head_sha')} ({last_green.get('html_url')})") + sys.exit(0) + +first_red = recent_failures[-1] + +base_sha = last_green.get("head_sha") +head_sha = first_red.get("head_sha") +compare_url = f"https://github.com/{repo}/compare/{base_sha}...{head_sha}" + +print(f"Repository: {repo}") +print(f"Workflow: {workflow}") +print(f"Branch: {branch}") +print() +print("Last green run:") +print(f" id: {last_green.get('id')}") +print(f" date: {last_green.get('created_at')}") +print(f" sha: {base_sha}") +print(f" url: {last_green.get('html_url')}") +print() +print("First red run after that green:") +print(f" id: {first_red.get('id')}") +print(f" date: {first_red.get('created_at')}") +print(f" sha: {head_sha}") +print(f" url: {first_red.get('html_url')}") +print() +print(f"Compare commits: {compare_url}") +PY diff --git a/script/ci-godot-tests b/script/ci-godot-tests index 78e50bdd..4861bf41 100755 --- a/script/ci-godot-tests +++ b/script/ci-godot-tests @@ -79,6 +79,12 @@ if [ "$FINAL_COUNT" -eq 0 ] 2>/dev/null; then exit 1 fi +# Give the editor time to finish its filesystem scan and script class +# registration. Without this, test_run fires before DirAccess sees the +# test scripts — the plugin connects before the scan completes. +echo "Waiting for editor filesystem scan to settle..." +sleep 8 + # Call test_run echo "Running handler tests..." RESULT=$(curl -s "$SERVER_URL" -X POST "${HEADERS[@]}" \ diff --git a/src/godot_ai/handlers/animation.py b/src/godot_ai/handlers/animation.py index 7be507c4..5de7a4a1 100644 --- a/src/godot_ai/handlers/animation.py +++ b/src/godot_ai/handlers/animation.py @@ -26,17 +26,18 @@ async def animation_create( name: str, length: float, loop_mode: str = "none", + overwrite: bool = False, ) -> dict: require_writable(runtime) - return await runtime.send_command( - "animation_create", - { - "player_path": player_path, - "name": name, - "length": length, - "loop_mode": loop_mode, - }, - ) + params: dict = { + "player_path": player_path, + "name": name, + "length": length, + "loop_mode": loop_mode, + } + if overwrite: + params["overwrite"] = True + return await runtime.send_command("animation_create", params) async def animation_add_property_track( @@ -133,6 +134,30 @@ async def animation_get( ) +async def animation_delete( + runtime: Runtime, + player_path: str, + animation_name: str, +) -> dict: + require_writable(runtime) + return await runtime.send_command( + "animation_delete", + {"player_path": player_path, "animation_name": animation_name}, + ) + + +async def animation_validate( + runtime: Runtime, + player_path: str, + animation_name: str, +) -> dict: + # Read-only — no require_writable. + return await runtime.send_command( + "animation_validate", + {"player_path": player_path, "animation_name": animation_name}, + ) + + async def animation_create_simple( runtime: Runtime, player_path: str, @@ -140,6 +165,7 @@ async def animation_create_simple( tweens: list[dict[str, Any]], length: float | None = None, loop_mode: str = "none", + overwrite: bool = False, ) -> dict: require_writable(runtime) params: dict[str, Any] = { @@ -150,4 +176,6 @@ async def animation_create_simple( } if length is not None: params["length"] = length + if overwrite: + params["overwrite"] = True return await runtime.send_command("animation_create_simple", params) diff --git a/src/godot_ai/handlers/node.py b/src/godot_ai/handlers/node.py index 8927767b..5ce397f6 100644 --- a/src/godot_ai/handlers/node.py +++ b/src/godot_ai/handlers/node.py @@ -7,12 +7,18 @@ from godot_ai.tools._pagination import paginate -async def node_create(runtime: Runtime, type: str, name: str = "", parent_path: str = "") -> dict: +async def node_create( + runtime: Runtime, + type: str = "", + name: str = "", + parent_path: str = "", + scene_path: str = "", +) -> dict: require_writable(runtime) - return await runtime.send_command( - "create_node", - {"type": type, "name": name, "parent_path": parent_path}, - ) + params: dict = {"type": type, "name": name, "parent_path": parent_path} + if scene_path: + params["scene_path"] = scene_path + return await runtime.send_command("create_node", params) async def node_find( @@ -101,4 +107,3 @@ async def node_remove_from_group(runtime: Runtime, path: str, group: str) -> dic "remove_from_group", {"path": path, "group": group}, ) - diff --git a/src/godot_ai/handlers/project.py b/src/godot_ai/handlers/project.py index e7071298..44612b94 100644 --- a/src/godot_ai/handlers/project.py +++ b/src/godot_ai/handlers/project.py @@ -32,7 +32,12 @@ async def project_run(runtime: Runtime, mode: str = "main", scene: str = "") -> async def project_stop(runtime: Runtime) -> dict: - return await runtime.send_command("stop_project") + result = await runtime.send_command("stop_project") + # Give the editor one frame to settle and emit `readiness_changed`. + # Without this, a tool call immediately after stop may see stale + # readiness="playing" and reject the command. + await asyncio.sleep(0.15) + return result async def project_settings_set(runtime: Runtime, key: str, value: Any) -> dict: @@ -67,4 +72,3 @@ async def _fetch(key: str) -> tuple[str, object | None, str | None]: else: settings[key] = value return {"settings": settings, "errors": errors if errors else None} - diff --git a/src/godot_ai/handlers/theme.py b/src/godot_ai/handlers/theme.py index 20de0a97..1b14708e 100644 --- a/src/godot_ai/handlers/theme.py +++ b/src/godot_ai/handlers/theme.py @@ -92,6 +92,21 @@ async def theme_set_stylebox_flat( shadow_offset_x: float | None = None, shadow_offset_y: float | None = None, anti_aliasing: bool | None = None, + # Per-side border width overrides + border_width_top: int | None = None, + border_width_bottom: int | None = None, + border_width_left: int | None = None, + border_width_right: int | None = None, + # Per-corner radius overrides + corner_radius_top_left: int | None = None, + corner_radius_top_right: int | None = None, + corner_radius_bottom_left: int | None = None, + corner_radius_bottom_right: int | None = None, + # Per-side content margin overrides + content_margin_top: float | None = None, + content_margin_bottom: float | None = None, + content_margin_left: float | None = None, + content_margin_right: float | None = None, ) -> dict: require_writable(runtime) params: dict[str, Any] = { @@ -119,6 +134,23 @@ async def theme_set_stylebox_flat( params["shadow_offset_y"] = shadow_offset_y if anti_aliasing is not None: params["anti_aliasing"] = anti_aliasing + # Per-side overrides + for key, val in [ + ("border_width_top", border_width_top), + ("border_width_bottom", border_width_bottom), + ("border_width_left", border_width_left), + ("border_width_right", border_width_right), + ("corner_radius_top_left", corner_radius_top_left), + ("corner_radius_top_right", corner_radius_top_right), + ("corner_radius_bottom_left", corner_radius_bottom_left), + ("corner_radius_bottom_right", corner_radius_bottom_right), + ("content_margin_top", content_margin_top), + ("content_margin_bottom", content_margin_bottom), + ("content_margin_left", content_margin_left), + ("content_margin_right", content_margin_right), + ]: + if val is not None: + params[key] = val return await runtime.send_command("theme_set_stylebox_flat", params) diff --git a/src/godot_ai/tools/animation.py b/src/godot_ai/tools/animation.py index 9d131fd8..067e2700 100644 --- a/src/godot_ai/tools/animation.py +++ b/src/godot_ai/tools/animation.py @@ -68,6 +68,7 @@ async def animation_create( name: str, length: float, loop_mode: str = "none", + overwrite: bool = False, session_id: str = "", ) -> dict: """Create a new Animation clip inside an AnimationPlayer's default library. @@ -85,6 +86,8 @@ async def animation_create( "none" — play once and stop (default), "linear" — loop from beginning, "pingpong" — reverse and repeat. + overwrite: If true, replace an existing animation with the same name + instead of erroring. The old animation is captured for undo. session_id: Optional Godot session to target. Empty = active session. """ runtime = DirectRuntime.from_context(ctx, session_id=session_id or None) @@ -94,6 +97,55 @@ async def animation_create( name=name, length=length, loop_mode=loop_mode, + overwrite=overwrite, + ) + + @mcp.tool(meta=DEFER_META) + async def animation_delete( + ctx: Context, + player_path: str, + animation_name: str, + session_id: str = "", + ) -> dict: + """Delete an Animation clip from an AnimationPlayer's library. + + Removes the named animation. Undoable — Ctrl+Z in Godot restores it. + + Args: + player_path: Scene path to the AnimationPlayer. + animation_name: Name of the animation to delete. + session_id: Optional Godot session to target. Empty = active session. + """ + runtime = DirectRuntime.from_context(ctx, session_id=session_id or None) + return await animation_handlers.animation_delete( + runtime, + player_path=player_path, + animation_name=animation_name, + ) + + @mcp.tool(meta=DEFER_META) + async def animation_validate( + ctx: Context, + player_path: str, + animation_name: str, + session_id: str = "", + ) -> dict: + """Check all track paths in an animation resolve to actual nodes. + + Returns a validation report with valid_count, broken_count, and a list + of broken tracks with their index, path, and issue description. Use this + after restructuring a node tree to find animations targeting stale paths. + + Args: + player_path: Scene path to the AnimationPlayer. + animation_name: Name of the animation to validate. + session_id: Optional Godot session to target. Empty = active session. + """ + runtime = DirectRuntime.from_context(ctx, session_id=session_id or None) + return await animation_handlers.animation_validate( + runtime, + player_path=player_path, + animation_name=animation_name, ) @mcp.tool(meta=DEFER_META) @@ -324,6 +376,7 @@ async def animation_create_simple( tweens: Annotated[list[dict[str, Any]], JsonCoerced], length: float | None = None, loop_mode: str = "none", + overwrite: bool = False, session_id: str = "", ) -> dict: """Create a complete animation from a list of tween specs in one call. @@ -372,6 +425,7 @@ async def animation_create_simple( tweens: List of tween spec dicts (see above). length: Total duration in seconds. Auto-computed if omitted. loop_mode: "none" (default), "linear", or "pingpong". + overwrite: If true, replace an existing animation with the same name. session_id: Optional Godot session to target. Empty = active session. """ runtime = DirectRuntime.from_context(ctx, session_id=session_id or None) @@ -382,4 +436,5 @@ async def animation_create_simple( tweens=tweens, length=length, loop_mode=loop_mode, + overwrite=overwrite, ) diff --git a/src/godot_ai/tools/node.py b/src/godot_ai/tools/node.py index f2f3db71..87d7ce6d 100644 --- a/src/godot_ai/tools/node.py +++ b/src/godot_ai/tools/node.py @@ -13,9 +13,10 @@ def register_node_tools(mcp: FastMCP) -> None: @mcp.tool(meta=DEFER_META) async def node_create( ctx: Context, - type: str, + type: str = "", name: str = "", parent_path: str = "", + scene_path: str = "", session_id: str = "", ) -> dict: """Create (spawn / add) a new node (game object / entity) in the scene tree. @@ -28,6 +29,10 @@ async def node_create( type: The Godot node class (e.g. "Node3D", "MeshInstance3D", "Camera3D"). name: Optional name for the node. parent_path: Node path of the parent (e.g. "/Main"). Empty = scene root. + scene_path: File path of a PackedScene to instantiate (e.g. + "res://prefabs/enemy.tscn"). When provided, the scene is loaded + and instantiated instead of creating by type. Mutually exclusive + with type — if scene_path is given, type is ignored. session_id: Optional Godot session to target. Empty = active session. """ runtime = DirectRuntime.from_context(ctx, session_id=session_id or None) @@ -36,6 +41,7 @@ async def node_create( type=type, name=name, parent_path=parent_path, + scene_path=scene_path, ) @mcp.tool(meta=DEFER_META) @@ -174,7 +180,10 @@ async def node_set_property( """ runtime = DirectRuntime.from_context(ctx, session_id=session_id or None) return await node_handlers.node_set_property( - runtime, path=path, property=property, value=value, + runtime, + path=path, + property=property, + value=value, ) @mcp.tool(meta=DEFER_META) diff --git a/src/godot_ai/tools/theme.py b/src/godot_ai/tools/theme.py index b7afddb2..ed66967a 100644 --- a/src/godot_ai/tools/theme.py +++ b/src/godot_ai/tools/theme.py @@ -136,6 +136,18 @@ async def theme_set_stylebox_flat( shadow_offset_x: float | None = None, shadow_offset_y: float | None = None, anti_aliasing: bool | None = None, + border_width_top: int | None = None, + border_width_bottom: int | None = None, + border_width_left: int | None = None, + border_width_right: int | None = None, + corner_radius_top_left: int | None = None, + corner_radius_top_right: int | None = None, + corner_radius_bottom_left: int | None = None, + corner_radius_bottom_right: int | None = None, + content_margin_top: float | None = None, + content_margin_bottom: float | None = None, + content_margin_left: float | None = None, + content_margin_right: float | None = None, session_id: str = "", ) -> dict: """Compose a StyleBoxFlat and assign it to a theme slot. @@ -165,6 +177,18 @@ async def theme_set_stylebox_flat( shadow_offset_x: Horizontal shadow offset. shadow_offset_y: Vertical shadow offset. anti_aliasing: Whether to anti-alias borders and corners. + border_width_top: Border thickness for top side only (overrides border_width). + border_width_bottom: Border thickness for bottom side only. + border_width_left: Border thickness for left side only. + border_width_right: Border thickness for right side only. + corner_radius_top_left: Corner radius for top-left (overrides corner_radius). + corner_radius_top_right: Corner radius for top-right. + corner_radius_bottom_left: Corner radius for bottom-left. + corner_radius_bottom_right: Corner radius for bottom-right. + content_margin_top: Content margin for top side (overrides content_margin). + content_margin_bottom: Content margin for bottom side. + content_margin_left: Content margin for left side. + content_margin_right: Content margin for right side. session_id: Optional Godot session to target. Empty = active session. """ runtime = DirectRuntime.from_context(ctx, session_id=session_id or None) @@ -183,6 +207,18 @@ async def theme_set_stylebox_flat( shadow_offset_x=shadow_offset_x, shadow_offset_y=shadow_offset_y, anti_aliasing=anti_aliasing, + border_width_top=border_width_top, + border_width_bottom=border_width_bottom, + border_width_left=border_width_left, + border_width_right=border_width_right, + corner_radius_top_left=corner_radius_top_left, + corner_radius_top_right=corner_radius_top_right, + corner_radius_bottom_left=corner_radius_bottom_left, + corner_radius_bottom_right=corner_radius_bottom_right, + content_margin_top=content_margin_top, + content_margin_bottom=content_margin_bottom, + content_margin_left=content_margin_left, + content_margin_right=content_margin_right, ) @mcp.tool(meta=DEFER_META) @@ -205,6 +241,4 @@ async def theme_apply( session_id: Optional Godot session to target. Empty = active session. """ runtime = DirectRuntime.from_context(ctx, session_id=session_id or None) - return await theme_handlers.theme_apply( - runtime, node_path=node_path, theme_path=theme_path - ) + return await theme_handlers.theme_apply(runtime, node_path=node_path, theme_path=theme_path) diff --git a/test_project/tests/test_animation.gd b/test_project/tests/test_animation.gd index 350438d9..b0f5c2c3 100644 --- a/test_project/tests/test_animation.gd +++ b/test_project/tests/test_animation.gd @@ -884,3 +884,132 @@ func test_get_labels_value_and_method_tracks_distinctly() -> void: assert_contains(types, "method") assert_contains(types, "bezier") _remove_node(player_path) + + +# ============================================================================ +# Friction fix: animation_delete +# ============================================================================ + +func test_delete_animation_basic() -> void: + var player_path := _add_player("TestDeleteAnim") + if player_path.is_empty(): + return + _handler.create_animation({"player_path": player_path, "name": "to_delete", "length": 1.0}) + + var result := _handler.delete_animation({ + "player_path": player_path, "animation_name": "to_delete", + }) + assert_has_key(result, "data") + assert_true(result.data.undoable) + + # Verify it's gone. + var list_result := _handler.list_animations({"player_path": player_path}) + for anim in list_result.data.animations: + assert_true(anim.name != "to_delete", "Deleted anim should not appear") + + # Undo should restore it. + _undo_redo.undo() + var list_after := _handler.list_animations({"player_path": player_path}) + var found := false + for anim in list_after.data.animations: + if anim.name == "to_delete": + found = true + assert_true(found, "Undo should restore deleted animation") + + _remove_node(player_path) + + +func test_delete_animation_not_found() -> void: + var player_path := _add_player("TestDeleteNotFound") + if player_path.is_empty(): + return + var result := _handler.delete_animation({ + "player_path": player_path, "animation_name": "nope", + }) + assert_is_error(result, McpErrorCodes.INVALID_PARAMS) + _remove_node(player_path) + + +# ============================================================================ +# Friction fix: animation overwrite +# ============================================================================ + +func test_create_animation_overwrite() -> void: + var player_path := _add_player("TestOverwrite") + if player_path.is_empty(): + return + _handler.create_animation({"player_path": player_path, "name": "overme", "length": 1.0}) + + # Without overwrite, duplicate name should fail. + var fail_result := _handler.create_animation({ + "player_path": player_path, "name": "overme", "length": 2.0, + }) + assert_is_error(fail_result, McpErrorCodes.INVALID_PARAMS) + + # With overwrite, it should succeed. + var result := _handler.create_animation({ + "player_path": player_path, "name": "overme", "length": 2.0, "overwrite": true, + }) + assert_has_key(result, "data") + assert_eq(result.data.overwritten, true) + assert_eq(result.data.length, 2.0) + + _remove_node(player_path) + + +# ============================================================================ +# Friction fix: animation_validate +# ============================================================================ + +func test_validate_animation_all_valid() -> void: + var player_path := _add_player("TestValidateOk") + if player_path.is_empty(): + return + _handler.create_animation({"player_path": player_path, "name": "valid_test", "length": 1.0}) + _handler.add_property_track({ + "player_path": player_path, + "animation_name": "valid_test", + "track_path": ".:visible", + "keyframes": [{"time": 0.0, "value": true}], + }) + var result := _handler.validate_animation({ + "player_path": player_path, "animation_name": "valid_test", + }) + assert_has_key(result, "data") + assert_eq(result.data.valid, true) + assert_eq(result.data.broken_count, 0) + assert_eq(result.data.valid_count, 1) + _remove_node(player_path) + + +func test_validate_animation_broken_track() -> void: + var player_path := _add_player("TestValidateBroken") + if player_path.is_empty(): + return + _handler.create_animation({"player_path": player_path, "name": "broken_test", "length": 1.0}) + _handler.add_property_track({ + "player_path": player_path, + "animation_name": "broken_test", + "track_path": "NonExistentNode:visible", + "keyframes": [{"time": 0.0, "value": true}], + }) + var result := _handler.validate_animation({ + "player_path": player_path, "animation_name": "broken_test", + }) + assert_has_key(result, "data") + assert_eq(result.data.valid, false) + assert_eq(result.data.broken_count, 1) + assert_eq(result.data.broken_tracks[0].issue, "node_not_found") + assert_eq(result.data.broken_tracks[0].node_path, "NonExistentNode") + _remove_node(player_path) + + +func test_validate_animation_not_found() -> void: + var player_path := _add_player("TestValidateNotFound") + if player_path.is_empty(): + return + var result := _handler.validate_animation({ + "player_path": player_path, "animation_name": "nope", + }) + assert_is_error(result, McpErrorCodes.INVALID_PARAMS) + _remove_node(player_path) diff --git a/test_project/tests/test_editor.gd b/test_project/tests/test_editor.gd index ad0a4fc7..d8634375 100644 --- a/test_project/tests/test_editor.gd +++ b/test_project/tests/test_editor.gd @@ -299,3 +299,20 @@ func test_performance_monitors_unknown_filtered_out() -> void: var result := _handler.get_performance_monitors({"monitors": ["time/fps", "fake/monitor"]}) assert_eq(result.data.monitor_count, 1) assert_has_key(result.data.monitors, "time/fps") + + +# ----- Friction fix: screenshot source="game" ----- + +func test_screenshot_game_not_running_returns_error() -> void: + # When the game is not running, source="game" should return an error. + if EditorInterface.is_playing_scene(): + return # Can't test this path while game is running. + var result := _handler.take_screenshot({"source": "game"}) + assert_is_error(result, McpErrorCodes.INVALID_PARAMS) + assert_contains(result.error.message, "not running") + + +func test_screenshot_bogus_source() -> void: + var result := _handler.take_screenshot({"source": "bogus"}) + assert_is_error(result, McpErrorCodes.INVALID_PARAMS) + assert_contains(result.error.message, "Invalid source") diff --git a/test_project/tests/test_node.gd b/test_project/tests/test_node.gd index 0134065a..055886ee 100644 --- a/test_project/tests/test_node.gd +++ b/test_project/tests/test_node.gd @@ -360,8 +360,19 @@ func test_rename_node_basic() -> void: func test_rename_node_scene_root() -> void: - var result := _handler.rename_node({"path": "/Main", "new_name": "NewMain"}) - assert_is_error(result, McpErrorCodes.INVALID_PARAMS) + # Renaming the scene root is allowed (not the .tscn file path, just the node name). + var scene_root := EditorInterface.get_edited_scene_root() + if scene_root == null: + return + var old_name := String(scene_root.name) + var result := _handler.rename_node({"path": "/" + old_name, "new_name": "RenamedTestRoot"}) + assert_has_key(result, "data") + assert_eq(result.data.name, "RenamedTestRoot") + assert_true(result.data.undoable) + # Restore the original name to avoid polluting other tests. + var restore := _handler.rename_node({"path": "/RenamedTestRoot", "new_name": old_name}) + assert_has_key(restore, "data") + assert_eq(String(scene_root.name), old_name) func test_rename_node_missing_name() -> void: @@ -509,3 +520,53 @@ func test_set_selection_empty_clears() -> void: var result := _handler.set_selection({"paths": []}) assert_has_key(result, "data") assert_eq(result.data.count, 0) + + +# ============================================================================ +# Friction fix: scene instancing via node_create +# ============================================================================ + +func test_create_node_from_scene_path() -> void: + # Use the test project's own main.tscn as the scene to instance. + var scene_root := EditorInterface.get_edited_scene_root() + if scene_root == null: + return + var before_count := scene_root.get_child_count() + var result := _handler.create_node({ + "scene_path": "res://main.tscn", + "name": "InstancedMain", + }) + assert_has_key(result, "data") + assert_has_key(result.data, "scene_path") + assert_eq(result.data.scene_path, "res://main.tscn") + assert_true(result.data.undoable) + # Clean up: remove the instanced node. + var instanced := scene_root.find_child("InstancedMain", false, false) + if instanced: + scene_root.remove_child(instanced) + instanced.queue_free() + assert_eq(scene_root.get_child_count(), before_count, "Cleanup should restore child count") + + +func test_create_node_scene_path_not_found() -> void: + var result := _handler.create_node({ + "scene_path": "res://nonexistent_scene.tscn", + }) + assert_is_error(result, McpErrorCodes.INVALID_PARAMS) + assert_contains(result.error.message, "not found") + + +func test_create_node_scene_path_not_res() -> void: + var result := _handler.create_node({ + "scene_path": "/tmp/scene.tscn", + }) + assert_is_error(result, McpErrorCodes.INVALID_PARAMS) + assert_contains(result.error.message, "res://") + + +func test_create_node_requires_type_or_scene_path() -> void: + var result := _handler.create_node({"parent_path": ""}) + assert_is_error(result, McpErrorCodes.INVALID_PARAMS) + assert_contains(result.error.message, "type") + + diff --git a/test_project/tests/test_signal.gd b/test_project/tests/test_signal.gd index 260d8095..c3f5d657 100644 --- a/test_project/tests/test_signal.gd +++ b/test_project/tests/test_signal.gd @@ -93,3 +93,20 @@ func test_disconnect_signal_not_connected() -> void: "method": "_nonexistent_method", }) assert_is_error(result, McpErrorCodes.INVALID_PARAMS) + + +# ----- Friction fix: autoload resolution ----- + +func test_connect_signal_autoload_not_found() -> void: + # An autoload name that doesn't exist should produce a clear error. + var scene_root := EditorInterface.get_edited_scene_root() + if scene_root == null: + return + var result := _handler.connect_signal({ + "path": "NonExistentAutoload", + "signal": "ready", + "target": "/" + scene_root.name, + "method": "queue_free", + }) + assert_is_error(result, McpErrorCodes.INVALID_PARAMS) + assert_contains(result.error.message, "not found") diff --git a/test_project/tests/test_theme.gd b/test_project/tests/test_theme.gd index 50938bb9..97c610c3 100644 --- a/test_project/tests/test_theme.gd +++ b/test_project/tests/test_theme.gd @@ -297,3 +297,83 @@ func test_create_theme_missing_path_names_param_correctly() -> void: assert_contains(result.error.message, "path") # Make sure it's NOT using the default "theme_path" label. assert_true(result.error.message.find("theme_path") == -1, "Error should say 'path', not 'theme_path'") + + +# ----- Friction fix: auto-create parent directories ----- + +func test_create_theme_creates_parent_directories() -> void: + var nested_path := "res://tests/_mcp_nested_dir/subdir/test_theme.tres" + # Ensure clean state. + if FileAccess.file_exists(nested_path): + DirAccess.remove_absolute(nested_path) + if DirAccess.dir_exists_absolute("res://tests/_mcp_nested_dir/subdir"): + DirAccess.remove_absolute("res://tests/_mcp_nested_dir/subdir") + if DirAccess.dir_exists_absolute("res://tests/_mcp_nested_dir"): + DirAccess.remove_absolute("res://tests/_mcp_nested_dir") + + var result := _handler.create_theme({"path": nested_path}) + assert_has_key(result, "data") + assert_true(FileAccess.file_exists(nested_path), "Theme file should exist in nested dir") + + # Cleanup. + DirAccess.remove_absolute(nested_path) + DirAccess.remove_absolute("res://tests/_mcp_nested_dir/subdir") + DirAccess.remove_absolute("res://tests/_mcp_nested_dir") + + +# ----- Friction fix: per-side stylebox parameters ----- + +func test_set_stylebox_flat_per_side_border_width() -> void: + _make_theme() + var result := _handler.set_stylebox_flat({ + "theme_path": TEST_THEME_PATH, + "class_name": "Button", + "name": "normal", + "border_width": 1, + "border_width_top": 4, + "border_width_bottom": 2, + }) + assert_has_key(result, "data") + var theme: Theme = ResourceLoader.load(TEST_THEME_PATH) + var sb: StyleBoxFlat = theme.get_stylebox("normal", "Button") + assert_eq(sb.border_width_top, 4) + assert_eq(sb.border_width_bottom, 2) + assert_eq(sb.border_width_left, 1) # uniform fallback + assert_eq(sb.border_width_right, 1) + + +func test_set_stylebox_flat_per_corner_radius() -> void: + _make_theme() + var result := _handler.set_stylebox_flat({ + "theme_path": TEST_THEME_PATH, + "class_name": "Panel", + "name": "panel", + "corner_radius": 4, + "corner_radius_top_left": 12, + "corner_radius_bottom_right": 0, + }) + assert_has_key(result, "data") + var theme: Theme = ResourceLoader.load(TEST_THEME_PATH) + var sb: StyleBoxFlat = theme.get_stylebox("panel", "Panel") + assert_eq(sb.corner_radius_top_left, 12) + assert_eq(sb.corner_radius_top_right, 4) # uniform fallback + assert_eq(sb.corner_radius_bottom_left, 4) + assert_eq(sb.corner_radius_bottom_right, 0) + + +func test_set_stylebox_flat_per_side_content_margin() -> void: + _make_theme() + var result := _handler.set_stylebox_flat({ + "theme_path": TEST_THEME_PATH, + "class_name": "PanelContainer", + "name": "panel", + "content_margin": 8.0, + "content_margin_top": 16.0, + }) + assert_has_key(result, "data") + var theme: Theme = ResourceLoader.load(TEST_THEME_PATH) + var sb: StyleBoxFlat = theme.get_stylebox("panel", "PanelContainer") + assert_eq(sb.content_margin_top, 16.0) + assert_eq(sb.content_margin_bottom, 8.0) + assert_eq(sb.content_margin_left, 8.0) + assert_eq(sb.content_margin_right, 8.0) diff --git a/test_project/tests/test_ui.gd b/test_project/tests/test_ui.gd index 94847c43..e0d686a9 100644 --- a/test_project/tests/test_ui.gd +++ b/test_project/tests/test_ui.gd @@ -319,3 +319,88 @@ func test_build_layout_rejects_uncoercible_property() -> void: }) assert_is_error(result, McpErrorCodes.INVALID_PARAMS) assert_contains(result.error.message, "modulate") + + +# ============================================================================ +# Friction fix: theme_override_* properties in build_layout +# ============================================================================ + +func test_build_layout_theme_override_color() -> void: + var scene_root := EditorInterface.get_edited_scene_root() + if scene_root == null: + return + var result := _handler.build_layout({ + "tree": { + "type": "Label", + "name": "TestOverrideColor", + "properties": { + "text": "Red text", + "theme_override_colors/font_color": "#ff0000", + }, + }, + }) + assert_has_key(result, "data") + var label: Label = scene_root.find_child("TestOverrideColor", true, false) + assert_true(label != null, "Label should exist") + assert_true(label.has_theme_color_override("font_color"), "Color override should be set") + var c := label.get_theme_color("font_color") + assert_true(c.r > 0.9, "Red channel should be ~1.0") + # Cleanup. + label.get_parent().remove_child(label) + label.queue_free() + + +func test_build_layout_theme_override_constant() -> void: + var scene_root := EditorInterface.get_edited_scene_root() + if scene_root == null: + return + var result := _handler.build_layout({ + "tree": { + "type": "VBoxContainer", + "name": "TestOverrideConst", + "properties": { + "theme_override_constants/separation": 20, + }, + }, + }) + assert_has_key(result, "data") + var vbox := scene_root.find_child("TestOverrideConst", true, false) as VBoxContainer + assert_true(vbox != null, "VBoxContainer should exist") + assert_true(vbox.has_theme_constant_override("separation"), "Constant override should be set") + assert_eq(vbox.get_theme_constant("separation"), 20) + vbox.get_parent().remove_child(vbox) + vbox.queue_free() + + +func test_build_layout_theme_override_font_size() -> void: + var scene_root := EditorInterface.get_edited_scene_root() + if scene_root == null: + return + var result := _handler.build_layout({ + "tree": { + "type": "Label", + "name": "TestOverrideFontSize", + "properties": { + "text": "Big", + "theme_override_font_sizes/font_size": 32, + }, + }, + }) + assert_has_key(result, "data") + var label := scene_root.find_child("TestOverrideFontSize", true, false) as Label + assert_true(label != null, "Label should exist") + assert_true(label.has_theme_font_size_override("font_size"), "Font size override should be set") + assert_eq(label.get_theme_font_size("font_size"), 32) + label.get_parent().remove_child(label) + label.queue_free() + + +func test_build_layout_theme_override_rejects_non_control() -> void: + var result := _handler.build_layout({ + "tree": { + "type": "Node3D", + "properties": {"theme_override_colors/font_color": "#ff0000"}, + }, + }) + assert_is_error(result, McpErrorCodes.INVALID_PARAMS) + assert_contains(result.error.message, "theme_override_") diff --git a/tests/integration/test_mcp_tools.py b/tests/integration/test_mcp_tools.py index d17d79e1..7d4af30e 100644 --- a/tests/integration/test_mcp_tools.py +++ b/tests/integration/test_mcp_tools.py @@ -526,9 +526,7 @@ async def respond(): ) task = asyncio.create_task(respond()) - result = await client.call_tool( - "node_rename", {"path": "/Main/Player", "new_name": "Hero"} - ) + result = await client.call_tool("node_rename", {"path": "/Main/Player", "new_name": "Hero"}) await task assert result.data["name"] == "Hero" @@ -2534,9 +2532,7 @@ async def respond_b(): ) task = asyncio.create_task(respond_b()) - result = await client.call_tool( - "scene_get_roots", {"session_id": "proj-b@0002"} - ) + result = await client.call_tool("scene_get_roots", {"session_id": "proj-b@0002"}) await task assert result.data["current"] == "res://from_b.tscn" finally: @@ -2866,9 +2862,7 @@ async def respond(): ) task = asyncio.create_task(respond()) - result = await client.call_tool( - "theme_create", {"path": "res://ui/themes/game.tres"} - ) + result = await client.call_tool("theme_create", {"path": "res://ui/themes/game.tres"}) await task assert result.data["path"] == "res://ui/themes/game.tres" @@ -3022,6 +3016,50 @@ async def respond(): await task assert result.data["corner_radius"] == 8 + async def test_per_side_params_forwarded(self, mcp_stack): + client, plugin = mcp_stack + + async def respond(): + cmd = await plugin.recv_command() + assert cmd["command"] == "theme_set_stylebox_flat" + params = cmd["params"] + assert params["border_width"] == 1 + assert params["border_width_top"] == 4 + assert params["border_width_bottom"] == 2 + assert "border_width_left" not in params + assert params["corner_radius_top_left"] == 12 + assert params["content_margin_top"] == 16.0 + await plugin.send_response( + cmd["request_id"], + { + "path": "res://themes/game.tres", + "class_name": "Button", + "name": "normal", + "stylebox_class": "StyleBoxFlat", + "bg_color": {"r": 0, "g": 0, "b": 0, "a": 1}, + "border_width": 1, + "corner_radius": 4, + "undoable": True, + }, + ) + + task = asyncio.create_task(respond()) + result = await client.call_tool( + "theme_set_stylebox_flat", + { + "theme_path": "res://themes/game.tres", + "class_name": "Button", + "name": "normal", + "border_width": 1, + "border_width_top": 4, + "border_width_bottom": 2, + "corner_radius_top_left": 12, + "content_margin_top": 16.0, + }, + ) + await task + assert result.data["border_width"] == 1 + class TestThemeApplyTool: async def test_apply(self, mcp_stack): @@ -3072,9 +3110,7 @@ async def respond(): ) task = asyncio.create_task(respond()) - result = await client.call_tool( - "theme_apply", {"node_path": "/Main/HUD"} - ) + result = await client.call_tool("theme_apply", {"node_path": "/Main/HUD"}) await task assert result.data["cleared"] is True @@ -3103,9 +3139,7 @@ async def respond(): ) task = asyncio.create_task(respond()) - result = await client.call_tool( - "animation_player_create", {"parent_path": "/Main"} - ) + result = await client.call_tool("animation_player_create", {"parent_path": "/Main"}) await task assert result.data["path"] == "/Main/AnimationPlayer" assert result.data["undoable"] is True @@ -3275,8 +3309,14 @@ async def respond(): assert kf["method"] == "queue_free" await plugin.send_response( cmd["request_id"], - {"player_path": "/Main/AP", "animation_name": "die", - "target_node_path": ".", "keyframe_count": 1, "track_index": 0, "undoable": True}, + { + "player_path": "/Main/AP", + "animation_name": "die", + "target_node_path": ".", + "keyframe_count": 1, + "track_index": 0, + "undoable": True, + }, ) task = asyncio.create_task(respond()) @@ -3303,8 +3343,13 @@ async def respond(): assert cmd["params"]["animation_name"] == "idle" await plugin.send_response( cmd["request_id"], - {"player_path": "/Main/AP", "animation_name": "idle", - "previous_autoplay": "", "cleared": False, "undoable": True}, + { + "player_path": "/Main/AP", + "animation_name": "idle", + "previous_autoplay": "", + "cleared": False, + "undoable": True, + }, ) task = asyncio.create_task(respond()) @@ -3323,14 +3368,17 @@ async def respond(): assert cmd["params"]["animation_name"] == "" await plugin.send_response( cmd["request_id"], - {"player_path": "/Main/AP", "animation_name": "", "previous_autoplay": "idle", - "cleared": True, "undoable": True}, + { + "player_path": "/Main/AP", + "animation_name": "", + "previous_autoplay": "idle", + "cleared": True, + "undoable": True, + }, ) task = asyncio.create_task(respond()) - result = await client.call_tool( - "animation_set_autoplay", {"player_path": "/Main/AP"} - ) + result = await client.call_tool("animation_set_autoplay", {"player_path": "/Main/AP"}) await task assert result.data["cleared"] is True @@ -3344,8 +3392,12 @@ async def respond(): assert cmd["command"] == "animation_play" await plugin.send_response( cmd["request_id"], - {"player_path": "/Main/AP", "animation_name": "idle", - "undoable": False, "reason": "Runtime playback state — not saved with scene"}, + { + "player_path": "/Main/AP", + "animation_name": "idle", + "undoable": False, + "reason": "Runtime playback state — not saved with scene", + }, ) task = asyncio.create_task(respond()) @@ -3363,8 +3415,11 @@ async def respond(): assert cmd["command"] == "animation_stop" await plugin.send_response( cmd["request_id"], - {"player_path": "/Main/AP", "undoable": False, - "reason": "Runtime playback state — not saved with scene"}, + { + "player_path": "/Main/AP", + "undoable": False, + "reason": "Runtime playback state — not saved with scene", + }, ) task = asyncio.create_task(respond()) @@ -3416,8 +3471,14 @@ async def respond(): "loop_mode": "none", "track_count": 1, "tracks": [ - {"index": 0, "type": "value", "path": "Panel:modulate", - "interpolation": "linear", "key_count": 2, "keys": []} + { + "index": 0, + "type": "value", + "path": "Panel:modulate", + "interpolation": "linear", + "key_count": 2, + "keys": [], + } ], }, ) @@ -3435,10 +3496,13 @@ class TestAnimationCreateSimpleTool: async def test_create_simple(self, mcp_stack): client, plugin = mcp_stack tweens = [ - {"target": "Panel", "property": "modulate", - "from": {"r": 1, "g": 1, "b": 1, "a": 0}, - "to": {"r": 1, "g": 1, "b": 1, "a": 1}, - "duration": 0.5} + { + "target": "Panel", + "property": "modulate", + "from": {"r": 1, "g": 1, "b": 1, "a": 0}, + "to": {"r": 1, "g": 1, "b": 1, "a": 1}, + "duration": 0.5, + } ] async def respond(): @@ -3450,8 +3514,14 @@ async def respond(): assert "length" not in cmd["params"] await plugin.send_response( cmd["request_id"], - {"player_path": "/Main/AP", "name": "fade_in", "length": 0.5, - "loop_mode": "none", "track_count": 1, "undoable": True}, + { + "player_path": "/Main/AP", + "name": "fade_in", + "length": 0.5, + "loop_mode": "none", + "track_count": 1, + "undoable": True, + }, ) task = asyncio.create_task(respond()) @@ -3472,8 +3542,14 @@ async def respond(): assert isinstance(cmd["params"]["tweens"], list) await plugin.send_response( cmd["request_id"], - {"player_path": "/Main/AP", "name": "pulse", "length": 0.5, - "loop_mode": "pingpong", "track_count": 1, "undoable": True}, + { + "player_path": "/Main/AP", + "name": "pulse", + "length": 0.5, + "loop_mode": "pingpong", + "track_count": 1, + "undoable": True, + }, ) task = asyncio.create_task(respond()) @@ -3491,3 +3567,64 @@ async def respond(): ) await task assert result.data["undoable"] is True + + +class TestAnimationDeleteTool: + async def test_delete(self, mcp_stack): + client, plugin = mcp_stack + + async def respond(): + cmd = await plugin.recv_command() + assert cmd["command"] == "animation_delete" + assert cmd["params"]["player_path"] == "/Main/AP" + assert cmd["params"]["animation_name"] == "idle" + await plugin.send_response( + cmd["request_id"], + { + "player_path": "/Main/AP", + "animation_name": "idle", + "undoable": True, + }, + ) + + task = asyncio.create_task(respond()) + result = await client.call_tool( + "animation_delete", + {"player_path": "/Main/AP", "animation_name": "idle"}, + ) + await task + assert result.data["undoable"] is True + + +class TestAnimationValidateTool: + async def test_validate(self, mcp_stack): + client, plugin = mcp_stack + + async def respond(): + cmd = await plugin.recv_command() + assert cmd["command"] == "animation_validate" + assert cmd["params"]["player_path"] == "/Main/AP" + assert cmd["params"]["animation_name"] == "walk" + await plugin.send_response( + cmd["request_id"], + { + "player_path": "/Main/AP", + "animation_name": "walk", + "track_count": 2, + "valid_count": 1, + "broken_count": 1, + "broken_tracks": [ + {"index": 1, "path": "Gone:visible", "issue": "node_not_found"}, + ], + "valid": False, + }, + ) + + task = asyncio.create_task(respond()) + result = await client.call_tool( + "animation_validate", + {"player_path": "/Main/AP", "animation_name": "walk"}, + ) + await task + assert result.data["valid"] is False + assert result.data["broken_count"] == 1 diff --git a/tests/unit/test_runtime_handlers.py b/tests/unit/test_runtime_handlers.py index 43b850de..7f05f092 100644 --- a/tests/unit/test_runtime_handlers.py +++ b/tests/unit/test_runtime_handlers.py @@ -1303,12 +1303,8 @@ def test_session_activate_handler_not_found(): def test_session_activate_by_project_name_hint(): registry = SessionRegistry() - registry.register( - _make_session("aaaa-uuid-1", project_path="/home/user/projects/my_game/") - ) - registry.register( - _make_session("bbbb-uuid-2", project_path="/home/user/projects/other_tool/") - ) + registry.register(_make_session("aaaa-uuid-1", project_path="/home/user/projects/my_game/")) + registry.register(_make_session("bbbb-uuid-2", project_path="/home/user/projects/other_tool/")) runtime = DirectRuntime(registry=registry, client=StubClient()) result = session_handlers.session_activate(runtime, "my_game") @@ -1321,12 +1317,8 @@ def test_session_activate_by_project_name_hint(): def test_session_activate_by_project_path_substring(): registry = SessionRegistry() - registry.register( - _make_session("aaaa-uuid-1", project_path="/home/user/projects/my_game/") - ) - registry.register( - _make_session("bbbb-uuid-2", project_path="/tmp/other/") - ) + registry.register(_make_session("aaaa-uuid-1", project_path="/home/user/projects/my_game/")) + registry.register(_make_session("bbbb-uuid-2", project_path="/tmp/other/")) runtime = DirectRuntime(registry=registry, client=StubClient()) result = session_handlers.session_activate(runtime, "projects") @@ -1337,12 +1329,8 @@ def test_session_activate_by_project_path_substring(): def test_session_activate_ambiguous_hint_errors_with_candidates(): registry = SessionRegistry() - registry.register( - _make_session("aaaa-uuid", project_path="/home/user/game_project_one/") - ) - registry.register( - _make_session("bbbb-uuid", project_path="/home/user/game_project_two/") - ) + registry.register(_make_session("aaaa-uuid", project_path="/home/user/game_project_one/")) + registry.register(_make_session("bbbb-uuid", project_path="/home/user/game_project_two/")) runtime = DirectRuntime(registry=registry, client=StubClient()) result = session_handlers.session_activate(runtime, "game_project") @@ -2147,9 +2135,7 @@ async def test_batch_execute_rejects_empty_list(): async def test_ui_set_anchor_preset_handler_defaults(): client = StubClient() runtime = DirectRuntime(registry=SessionRegistry(), client=client) - result = await ui_handlers.ui_set_anchor_preset( - runtime, path="/Main/HUD", preset="full_rect" - ) + result = await ui_handlers.ui_set_anchor_preset(runtime, path="/Main/HUD", preset="full_rect") assert client.calls[-1]["command"] == "set_anchor_preset" assert client.calls[-1]["params"] == { "path": "/Main/HUD", @@ -2193,9 +2179,7 @@ async def test_ui_build_layout_handler_forwards_tree_and_parent(): "properties": {"separation": 16}, "children": [{"type": "Label", "properties": {"text": "Paused"}}], } - result = await ui_handlers.ui_build_layout( - runtime, tree=tree, parent_path="/Main/HUD" - ) + result = await ui_handlers.ui_build_layout(runtime, tree=tree, parent_path="/Main/HUD") assert client.calls[-1]["command"] == "build_layout" assert client.calls[-1]["params"] == {"tree": tree, "parent_path": "/Main/HUD"} assert result["node_count"] == 5 @@ -2216,9 +2200,7 @@ async def test_ui_build_layout_handler_defaults_parent_to_empty(): async def test_theme_create_handler(): client = StubClient() runtime = DirectRuntime(registry=SessionRegistry(), client=client) - result = await theme_handlers.theme_create( - runtime, path="res://ui/themes/game.tres" - ) + result = await theme_handlers.theme_create(runtime, path="res://ui/themes/game.tres") assert client.calls[-1]["command"] == "create_theme" assert client.calls[-1]["params"] == { "path": "res://ui/themes/game.tres", @@ -2230,9 +2212,7 @@ async def test_theme_create_handler(): async def test_theme_create_handler_overwrite_passthrough(): client = StubClient() runtime = DirectRuntime(registry=SessionRegistry(), client=client) - await theme_handlers.theme_create( - runtime, path="res://ui/themes/game.tres", overwrite=True - ) + await theme_handlers.theme_create(runtime, path="res://ui/themes/game.tres", overwrite=True) assert client.calls[-1]["params"]["overwrite"] is True @@ -2331,6 +2311,34 @@ async def test_theme_set_stylebox_flat_handler_forwards_all_fields(): assert params["content_margin"] == 12.0 +async def test_theme_set_stylebox_flat_handler_per_side_params(): + """Per-side border/corner/margin params should be forwarded when provided.""" + client = StubClient() + runtime = DirectRuntime(registry=SessionRegistry(), client=client) + await theme_handlers.theme_set_stylebox_flat( + runtime, + theme_path="res://themes/game.tres", + class_name="Button", + name="normal", + border_width=1, + border_width_top=4, + border_width_bottom=2, + corner_radius_top_left=12, + content_margin_top=16.0, + ) + params = client.calls[-1]["params"] + assert params["border_width"] == 1 + assert params["border_width_top"] == 4 + assert params["border_width_bottom"] == 2 + assert params["corner_radius_top_left"] == 12 + assert params["content_margin_top"] == 16.0 + # Unset per-side params should be absent. + assert "border_width_left" not in params + assert "border_width_right" not in params + assert "corner_radius_top_right" not in params + assert "content_margin_bottom" not in params + + async def test_theme_apply_handler(): client = StubClient() runtime = DirectRuntime(registry=SessionRegistry(), client=client) @@ -2483,9 +2491,7 @@ async def test_animation_set_autoplay_handler(): async def test_animation_set_autoplay_clears_with_empty(): client = StubClient() runtime = DirectRuntime(registry=SessionRegistry(), client=client) - result = await animation_handlers.animation_set_autoplay( - runtime, player_path="/Main/AP" - ) + result = await animation_handlers.animation_set_autoplay(runtime, player_path="/Main/AP") assert client.calls[-1]["params"]["animation_name"] == "" assert result["cleared"] is True @@ -2511,9 +2517,7 @@ async def test_animation_stop_handler(): async def test_animation_list_handler(): client = StubClient() runtime = DirectRuntime(registry=SessionRegistry(), client=client) - result = await animation_handlers.animation_list( - runtime, player_path="/Main/AP" - ) + result = await animation_handlers.animation_list(runtime, player_path="/Main/AP") assert client.calls[-1]["command"] == "animation_list" assert result["count"] == 2 assert result["animations"][0]["name"] == "idle" @@ -2658,6 +2662,107 @@ async def test_animation_player_create_requires_writable(): runtime = DirectRuntime(registry=registry, client=client) with pytest.raises(GodotCommandError): - await animation_handlers.animation_player_create( - runtime, parent_path="/Main" - ) + await animation_handlers.animation_player_create(runtime, parent_path="/Main") + + +async def test_animation_delete_handler(): + client = StubClient() + runtime = DirectRuntime(registry=SessionRegistry(), client=client) + await animation_handlers.animation_delete( + runtime, player_path="/Main/AP", animation_name="idle" + ) + assert client.calls[-1]["command"] == "animation_delete" + assert client.calls[-1]["params"] == { + "player_path": "/Main/AP", + "animation_name": "idle", + } + + +async def test_animation_create_overwrite_param(): + client = StubClient() + runtime = DirectRuntime(registry=SessionRegistry(), client=client) + await animation_handlers.animation_create( + runtime, + player_path="/Main/AP", + name="test", + length=1.0, + overwrite=True, + ) + assert client.calls[-1]["params"]["overwrite"] is True + + +async def test_animation_create_no_overwrite_by_default(): + client = StubClient() + runtime = DirectRuntime(registry=SessionRegistry(), client=client) + await animation_handlers.animation_create( + runtime, player_path="/Main/AP", name="test", length=1.0 + ) + assert "overwrite" not in client.calls[-1]["params"] + + +async def test_animation_create_simple_overwrite_param(): + client = StubClient() + runtime = DirectRuntime(registry=SessionRegistry(), client=client) + await animation_handlers.animation_create_simple( + runtime, + player_path="/Main/AP", + name="test", + tweens=[{"target": ".", "property": "visible", "from": True, "to": False, "duration": 1.0}], + overwrite=True, + ) + assert client.calls[-1]["params"]["overwrite"] is True + + +async def test_node_create_scene_path_param(): + client = StubClient() + runtime = DirectRuntime(registry=SessionRegistry(), client=client) + await node_handlers.node_create(runtime, scene_path="res://main.tscn", name="Instanced") + assert client.calls[-1]["params"]["scene_path"] == "res://main.tscn" + assert client.calls[-1]["params"]["name"] == "Instanced" + + +async def test_animation_validate_handler(): + client = StubClient() + runtime = DirectRuntime(registry=SessionRegistry(), client=client) + await animation_handlers.animation_validate( + runtime, player_path="/Main/AP", animation_name="idle" + ) + assert client.calls[-1]["command"] == "animation_validate" + assert client.calls[-1]["params"] == { + "player_path": "/Main/AP", + "animation_name": "idle", + } + + +async def test_animation_validate_does_not_require_writable(): + """animation_validate is read-only — must not call require_writable.""" + from godot_ai.sessions.registry import Session + + client = StubClient() + session = Session( + session_id="s1", + godot_version="4.4", + project_path="/tmp/p", + plugin_version="0.1", + readiness="playing", + ) + registry = SessionRegistry() + registry.register(session) + runtime = DirectRuntime(registry=registry, client=client) + # Should NOT raise even when readiness is "playing". + await animation_handlers.animation_validate( + runtime, player_path="/Main/AP", animation_name="idle" + ) + + +async def test_project_stop_handler_has_settle_delay(): + """project_stop should include a brief settle delay.""" + import time + + client = StubClient() + runtime = DirectRuntime(registry=SessionRegistry(), client=client) + t0 = time.monotonic() + await project_handlers.project_stop(runtime) + elapsed = time.monotonic() - t0 + assert elapsed >= 0.1, f"Expected >= 0.1s settle delay, got {elapsed:.3f}s" + assert client.calls[-1]["command"] == "stop_project"