feat: animation_* MCP tool family (AnimationPlayer primitives + composer)#17
Merged
Conversation
…ser) Ships 10 MCP tools for AnimationPlayer authoring: animation_player_create, animation_create, animation_add_property_track, animation_add_method_track, animation_set_autoplay, animation_play, animation_stop, animation_list, animation_get, and animation_create_simple (high-level one-call composer). Enables hover pulses, slide-in menus, fade transitions, and damage shake for the roguelite benchmark. Animations live in-scene (save with .tscn); all mutating operations are fully undoable. AnimationTree, bezier/audio tracks, and preset helpers (fade/slide_in/shake/pulse_loop) are deferred. - plugin/addons/godot_ai/handlers/animation_handler.gd: new GDScript handler with full EditorUndoRedoManager integration, value coercion (Color/Vector2 from dict/string), and baseline-track-count undo pattern for track inserts - plugin/addons/godot_ai/plugin.gd: register AnimationHandler + 10 dispatch entries - src/godot_ai/handlers/animation.py: 10 thin async Python wrappers - src/godot_ai/tools/animation.py: FastMCP tool registration with DEFER_META - src/godot_ai/server.py: wire animation tools, update instructions blurb - test_project/tests/test_animation.gd: 30+ GDScript in-editor tests - tests/unit/test_runtime_handlers.py: unit tests for all 10 animation handlers - tests/integration/test_mcp_tools.py: integration tests for all 10 MCP tools - docs/implementation-plan.md: flip animation_player.* checkboxes, add juice scenarios All 373 Python tests pass; ruff clean. https://claude.ai/code/session_01EZM3nFsQL4eV9atno59x9e
Removes stale dev-dependency entry left over from reverted pyproject.toml change during test setup. https://claude.ai/code/session_01EZM3nFsQL4eV9atno59x9e
…ack path - Remove unused _parse_loop_mode/_parse_interpolation static helpers — the handler only uses the _LOOP_MODES/_INTERP_MODES dict lookups - Reject target_node_path containing ':' in add_method_track — method tracks take a bare NodePath, and a colon almost certainly means the caller confused the format with property-track 'NodePath:property'. Clear error message tells them the method name goes in the keyframe - Add GDScript test for the new validation https://claude.ai/code/session_01EZM3nFsQL4eV9atno59x9e
…ercion - _resolve_player no longer rejects vanilla AnimationPlayers that lack a default AnimationLibrary. Instead, create_animation and create_simple bundle an add_animation_library call into the same undo action when the library is missing, and surface library_created=true in the response. Ctrl-Z rolls back both the library and the animation atomically. Lets agents animate AnimationPlayers that came in via node_create or were dragged into the scene manually. - create_simple now rejects duplicate target:property tweens with a clear error (prevents silent track layering on the same property). - Close a test blind spot: existing add_property_track / create_simple tests only asserted track counts, so the value-coercion path was never actually exercised (main.tscn's root is Node3D, which has no modulate property — dicts were being stored raw and tests passed anyway). New tests target `.position` on the Node3D root and assert track_get_key_value returns a real Vector3, catching regressions in _coerce_for_type. - Method-track docstring now documents that transition fields are silently ignored and that target_node_path must be a bare NodePath (matching the new colon-rejection validator). - Module docstring now spells out 2D / 3D / UI applicability so tool-search clients understand animation_* isn't UI-specific. - animation_play docstring clarifies the empty-name delegation. - Adds GDScript tests for auto-library creation (plus undo rollback), duplicate-tween rejection, and animation_play(name=""). https://claude.ai/code/session_01EZM3nFsQL4eV9atno59x9e
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
implementation-plan.md - Break down the deferred animation work into concrete, scope-bounded bullets: preset helpers, bezier/audio tracks, animation_tree.*, 3D material-fade sub-resource coercion, missing TYPE_* coverage (Transform3D / Quaternion / NodePath / Vector3i / Rect2 / AABB), and library-keyed access for multi-library imports. tool-taxonomy.md - Replace the stub animation_player.* / animation_tree.* line with an accurate description of what shipped vs. what's deferred, and call out 2D / 3D / UI applicability so the taxonomy reflects reality. CLAUDE.md - Add "Auto-create missing dependencies in the same undo action" — the pattern animation_handler uses for default AnimationLibrary, written up so future handlers (material, theme follow-ups, particles, etc.) follow the same shape instead of erroring or doing two writes. - Add "Value coercion: assert on the stored Variant, not on counts" — documents the test blind spot found while reviewing this PR (Node3D scene root has no .modulate, so coerce silently fell through and count-only tests passed regardless). Future handlers taking JSON values must read back via track_get_key_value / equivalent and assert the Variant type. https://claude.ai/code/session_01EZM3nFsQL4eV9atno59x9e
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new animation_* MCP tool family to author AnimationPlayer animations (create player/clip, add property/method tracks, autoplay, preview play/stop, list/get, plus a one-call “composer”), with Godot-side undo/redo integration and accompanying unit/integration/editor tests.
Changes:
- Introduces
AnimationHandler(GDScript) and wires 10 new dispatcher commands in the Godot plugin. - Adds Python handler wrappers plus FastMCP tool registration and server wiring.
- Expands test coverage across unit, integration, and in-editor GDScript suites; updates implementation plan docs.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
plugin/addons/godot_ai/handlers/animation_handler.gd |
New Godot-side handler implementing AnimationPlayer authoring operations with UndoRedo and value coercion. |
plugin/addons/godot_ai/plugin.gd |
Registers AnimationHandler and dispatch entries for the new animation commands. |
src/godot_ai/handlers/animation.py |
Adds thin async Python wrappers for the 10 animation commands, including readiness gating for write ops. |
src/godot_ai/tools/animation.py |
Registers animation_* tools in FastMCP (including JsonCoerced support for keyframes/tweens). |
src/godot_ai/server.py |
Wires tool registration and updates the server “instructions blurb” to include animation_*. |
tests/unit/test_runtime_handlers.py |
Adds unit tests validating wrapper params and readiness behavior for animation handlers. |
tests/integration/test_mcp_tools.py |
Adds end-to-end MCP tool integration tests for all animation tools, including JSON-coercion cases. |
test_project/tests/test_animation.gd |
Adds in-editor GDScript suite to validate real scene mutations, undo/redo, and coercion against Godot types. |
docs/implementation-plan.md |
Updates plan/status to reflect shipped AnimationPlayer tool surface and adds benchmark “juice” scenarios. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+26
to
+33
| const _NAMED_TRANSITIONS := { | ||
| "linear": 1.0, | ||
| "ease_in": 2.0, | ||
| "ease_out": 0.5, | ||
| "ease_in_out": -2.0, | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
_NAMED_TRANSITIONS is defined but never used; transition parsing is hard-coded in _parse_transition. Remove the unused constant or refactor _parse_transition to use it to avoid duplicated/forgotten mappings.
Suggested change
| const _NAMED_TRANSITIONS := { | |
| "linear": 1.0, | |
| "ease_in": 2.0, | |
| "ease_out": 0.5, | |
| "ease_in_out": -2.0, | |
| } |
Comment on lines
+638
to
+652
| ## Resolve an animation by name, searching all libraries. | ||
| func _resolve_animation(player: AnimationPlayer, anim_name: String) -> Dictionary: | ||
| if not player.has_animation(anim_name): | ||
| return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, | ||
| "Animation '%s' not found on player. Available: %s" % [ | ||
| anim_name, | ||
| ", ".join(Array(player.get_animation_list())) | ||
| ]) | ||
| # Find which library owns it. | ||
| for lib_name in player.get_animation_library_list(): | ||
| var lib: AnimationLibrary = player.get_animation_library(lib_name) | ||
| if lib.has_animation(anim_name): | ||
| return {"animation": lib.get_animation(anim_name), "library": lib, "library_key": lib_name} | ||
| # Fallback — shouldn't happen if has_animation returned true. | ||
| return McpErrorCodes.make(McpErrorCodes.INTERNAL_ERROR, "Animation found by player but not in any library") |
There was a problem hiding this comment.
_resolve_animation() assumes anim_name is a bare clip name, but list_animations() returns "<library>/<clip>" for non-default libraries. If callers pass that back into animation_get/track tools, player.has_animation(anim_name) may succeed while the per-library lookup fails, leading to an INTERNAL_ERROR. Support library-qualified names (split on the first "/" and look up that library) or return separate library_key + name fields instead of overloading name.
Comment on lines
+455
to
+458
| for i in anim.get_track_count(): | ||
| var track_type := anim.track_get_type(i) | ||
| var type_name := "value" if track_type == Animation.TYPE_VALUE else "method" | ||
| var keys: Array[Dictionary] = [] |
There was a problem hiding this comment.
get_animation() maps any non-VALUE track type to "method". Godot animations can contain other track types (e.g. bezier/audio), so this will misreport existing animations and confuse clients. Map known track types explicitly (value/method/bezier/audio/etc.) and use a safe fallback like "unknown" for unhandled types.
Comment on lines
+540
to
+548
| # Compute auto length if not provided. | ||
| var computed_length: float = float(params.get("length", 0.0)) | ||
| if computed_length <= 0.0: | ||
| for spec in tweens: | ||
| var end_time: float = float(spec.get("delay", 0.0)) + float(spec.get("duration", 0.0)) | ||
| if end_time > computed_length: | ||
| computed_length = end_time | ||
| if computed_length <= 0.0: | ||
| computed_length = 1.0 |
There was a problem hiding this comment.
create_simple() treats length <= 0 as “omit and auto-compute”, but the tool/docs describe auto-length only when length is omitted. If a client explicitly passes 0/negative, this silently changes semantics instead of rejecting invalid input. Consider: if params.has("length") and the value is <= 0, return INVALID_PARAMS; otherwise auto-compute only when the key is absent/null.
Suggested change
| # Compute auto length if not provided. | |
| var computed_length: float = float(params.get("length", 0.0)) | |
| if computed_length <= 0.0: | |
| for spec in tweens: | |
| var end_time: float = float(spec.get("delay", 0.0)) + float(spec.get("duration", 0.0)) | |
| if end_time > computed_length: | |
| computed_length = end_time | |
| if computed_length <= 0.0: | |
| computed_length = 1.0 | |
| # Compute auto length only when length is omitted or null. | |
| var has_length := params.has("length") and params.get("length") != null | |
| var computed_length: float = 0.0 | |
| if has_length: | |
| computed_length = float(params.get("length")) | |
| if computed_length <= 0.0: | |
| return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, | |
| "'length' must be > 0 when provided") | |
| else: | |
| for spec in tweens: | |
| var end_time: float = float(spec.get("delay", 0.0)) + float(spec.get("duration", 0.0)) | |
| if end_time > computed_length: | |
| computed_length = end_time | |
| if computed_length <= 0.0: | |
| computed_length = 1.0 |
There was a problem hiding this comment.
Yes — resolved in 0dff069. create_simple() now only auto-computes length when length is omitted/null, and explicitly rejects provided length <= 0 with INVALID_PARAMS (covered by test_create_simple_rejects_zero_length and test_create_simple_rejects_negative_length).
- Flip "Polished game-production extensions have started" to in-progress now that ui_*, theme_*, and animation_* families are shipped - Bump test counts to current totals (373 Python + 307 GDScript = 680) in implementation-plan.md and CLAUDE.md https://claude.ai/code/session_01EZM3nFsQL4eV9atno59x9e
Four correctness fixes surfaced by Copilot's PR review:
1. Use `_NAMED_TRANSITIONS` constant in `_parse_transition` — the constant
was defined but unused; hard-coded duplicates elsewhere invited drift.
2. Honor library-qualified animation names ("moves/idle") in `_resolve_animation`.
`list_animations` returns that form for non-default libraries; round-tripping
it back through `animation_get` used to fall through to INTERNAL_ERROR
because the per-library scan checked the qualified string against bare
clip names. Now split on the first "/" and look up the library directly.
3. Map all Animation.TrackType enum values explicitly in `get_animation`.
The previous `"value" if ... else "method"` mislabeled bezier/audio/3D
tracks as "method". New helper returns the honest label or "unknown".
4. Reject explicit `length <= 0` in `animation_create_simple`. The docs
say length is auto-computed when omitted; silently auto-computing for
0 or negative values hid client bugs. Now rejects with INVALID_PARAMS
and only auto-computes when the key is absent or null.
Adds four GDScript tests covering explicit-zero/negative length rejection,
library-qualified name round-trip, distinct value/method labeling, and
the `_track_type_to_string` helper itself.
https://claude.ai/code/session_01EZM3nFsQL4eV9atno59x9e
The previous round's bezier-label assertion reached into a private static helper via the class name (`AnimationHandler._track_type_to_string`), and the value/method distinctness test used a property (`modulate`) that doesn't exist on the default Node3D scene root. Both paths tripped up Godot-side CI across all three platforms. Rewrite both tests so they exercise the public read path: - Add a real Animation.TYPE_BEZIER track directly to the clip and assert it comes back labeled "bezier" (exercises `_track_type_to_string` end-to-end) - Use `position` (present on Node3D) and `queue_free` for the value/method keyframes so coercion and validation both resolve Also tighten the library-qualified name test: use StringName literals for library / clip keys to match Godot's internal types, drop the redundant `has_animation` pre-assert. https://claude.ai/code/session_01EZM3nFsQL4eV9atno59x9e
`var key := v.to_lower()` failed static type inference because `v` is typed `Variant` and GDScript 4.6 can't resolve the call's return type. Cast to String explicitly. Fixes the compile failure that blocked plugin load on Linux/macOS/Windows Godot test jobs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… gating
Three follow-ups from PR review:
1. `_coerce_value_for_track` now returns `{ok|error}`. Callers
pre-coerce keyframe values and surface failures as INVALID_PARAMS.
Previously, passing an unparseable Color string silently stored the
raw text as the keyframe, producing garbage at play time. The
authoring-time fallbacks are preserved: if the target node isn't in
the scene yet, or the target exists but the property isn't present
(retargeting case — user may set AnimationPlayer.root_node later),
the raw value passes through unchanged. Errors only fire when the
property resolves to a known type and the value can't be parsed.
2. `animation_play` / `animation_stop` no longer call `require_writable`.
They're transient preview ops ("Runtime playback state — not saved
with scene") and belong on the read side alongside `animation_list`
and `animation_get`.
3. `animation_create_simple` now delegates per-track insertion to the
shared `_do_add_property_track` helper — the inline track-building
block is gone. Coercion logic now has a single caller path.
Tests: two Python unit tests lock in play/stop-as-read-only; one GDScript
test exercises the new error path (unparseable color on a Sprite2D's
modulate). Four existing tests updated to use Vector3 dicts (`{x,y,z}`)
instead of Vector2 dicts for `.:position` on the Node3D scene root —
they passed before only because the old coercer silently fell through.
Live smoke test: all 312 GDScript tests pass, all 375 Python tests pass,
ruff clean. Exercised coerce → play → stop → unparseable-color rejection
→ create_simple(color strings) → get roundtrip against the live editor.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
dsarno
added a commit
that referenced
this pull request
Apr 16, 2026
Nine follow-ups from a retroactive code review of the animation_* tool family (#17) and the cyberpunk HUD friction fixes (#21). All 388 Python + 345 GDScript tests pass; smoke-tested live against the editor. 1. node_create scene_path now uses PackedScene.GEN_EDIT_STATE_INSTANCE so the editor treats the result as a real scene instance (foldout icon, .tscn stores a reference, not an exploded subtree). Removes the _set_owner_recursive call that was running outside the undo action and also broke the instance link by forcing descendant ownership. 2. Animation track undo no longer caches the baseline track_count at do time - it resolves the index via anim.find_track(path, type) at undo time. Robust to any history interleaving. New test stresses this path. 3. _coerce_value_for_track now errors with INVALID_PARAMS when the target node exists but the property doesnt - previously the raw value passed through and produced garbage keyframes at playback. Matches the handlers own docstring. 4. add_method_track validates keyframe args is an Array and method is a non-empty string. Previously a typed-assign crash or silent garbage. 5. animation_delete now uses _resolve_animation to locate the owning library, matching the read-side symmetry with animation_get / animation_play. Delete from non-default libraries works end-to-end. 6. theme_set_stylebox_flat flattened from 27 parameters to a nested-dict API: border / corners / margins / shadow each take {all, sides} with side-specific keys overriding all. Unknown keys fail loudly. Breaking change - tool was only merged in #21 and has no external clients yet. 7. project_stop replaces the fixed 150ms sleep with a bounded poll on session.readiness. Reaches truth as soon as the existing readiness_changed event arrives (~17ms) instead of blanket-sleeping 150ms, with a 1s timeout for a hung play process. 8. ui_build_layout pseudo-property tests now read back via get_theme_*_override (not the fallback get_theme_* getters), per the CLAUDE.md "assert on the stored Variant" rule. Added missing theme_override_styles/ test. 9. signal_connect distinguishes declared-but-uninstantiated autoloads from genuinely missing nodes, with a clear error pointing users at @onready/runtime-connect as the workaround. Previous "not found" error masked the real reason most runtime-only autoloads cant be connected at edit time. Also updates CLAUDE.md with reference patterns for find_track-at-undo, GEN_EDIT_STATE_INSTANCE, and get_theme_*_override readback tests. Deferred to a follow-up PR: plugin dispatcher coroutine support + readiness Option C (plugin-side await). That refactor touches the hot path every command flows through and deserves its own bisect-friendly landing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This was referenced Apr 16, 2026
dsarno
added a commit
that referenced
this pull request
Apr 16, 2026
* Address retroactive review of PRs #17 and #21 Nine follow-ups from a retroactive code review of the animation_* tool family (#17) and the cyberpunk HUD friction fixes (#21). All 388 Python + 345 GDScript tests pass; smoke-tested live against the editor. 1. node_create scene_path now uses PackedScene.GEN_EDIT_STATE_INSTANCE so the editor treats the result as a real scene instance (foldout icon, .tscn stores a reference, not an exploded subtree). Removes the _set_owner_recursive call that was running outside the undo action and also broke the instance link by forcing descendant ownership. 2. Animation track undo no longer caches the baseline track_count at do time - it resolves the index via anim.find_track(path, type) at undo time. Robust to any history interleaving. New test stresses this path. 3. _coerce_value_for_track now errors with INVALID_PARAMS when the target node exists but the property doesnt - previously the raw value passed through and produced garbage keyframes at playback. Matches the handlers own docstring. 4. add_method_track validates keyframe args is an Array and method is a non-empty string. Previously a typed-assign crash or silent garbage. 5. animation_delete now uses _resolve_animation to locate the owning library, matching the read-side symmetry with animation_get / animation_play. Delete from non-default libraries works end-to-end. 6. theme_set_stylebox_flat flattened from 27 parameters to a nested-dict API: border / corners / margins / shadow each take {all, sides} with side-specific keys overriding all. Unknown keys fail loudly. Breaking change - tool was only merged in #21 and has no external clients yet. 7. project_stop replaces the fixed 150ms sleep with a bounded poll on session.readiness. Reaches truth as soon as the existing readiness_changed event arrives (~17ms) instead of blanket-sleeping 150ms, with a 1s timeout for a hung play process. 8. ui_build_layout pseudo-property tests now read back via get_theme_*_override (not the fallback get_theme_* getters), per the CLAUDE.md "assert on the stored Variant" rule. Added missing theme_override_styles/ test. 9. signal_connect distinguishes declared-but-uninstantiated autoloads from genuinely missing nodes, with a clear error pointing users at @onready/runtime-connect as the workaround. Previous "not found" error masked the real reason most runtime-only autoloads cant be connected at edit time. Also updates CLAUDE.md with reference patterns for find_track-at-undo, GEN_EDIT_STATE_INSTANCE, and get_theme_*_override readback tests. Deferred to a follow-up PR: plugin dispatcher coroutine support + readiness Option C (plugin-side await). That refactor touches the hot path every command flows through and deserves its own bisect-friendly landing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs: bump test counts and reflect PR #28 API reshape - CLAUDE.md: 387 → 388 Python tests - implementation-plan.md: 387+333=720 → 388+345=733 total; theme_* blurb now describes the nested border/corners/margins/shadow dicts and animation undo-robust-to-history-interleaving Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * simplify: hoist property-type resolution + use get_running_loop 1. animation_handler.gd: _coerce_value_for_track split into _resolve_track_prop_context (per-track) + _coerce_with_context (per-key). add_property_track resolves the target property type once before the keyframe loop instead of re-walking get_property_list() for every keyframe. Dense clips (100+ keys) avoid 100x the work. Existing API (_coerce_value_for_track) preserved for create_simple, which still resolves per-tween since target/property change per tween. 2. project.py: replace asyncio.get_event_loop() with get_running_loop() (former is deprecated in Python 3.12+ inside async functions). All 388 Python + 345 GDScript tests pass. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This was referenced May 5, 2026
dsarno
added a commit
that referenced
this pull request
May 30, 2026
Routing check_uv_version / _find_system_install through McpCliExec uses OS.execute_with_pipe, whose output capture behaves differently on Godot 4.3 (the documented reason the cli_exec capture tests are skipped on < 4.4). On the 4.3 CI canary, `which godot-ai` came back with empty captured output, so _find_system_install returned "", get_server_command() resolved to nothing, and test_uvx_server_command_uses_exact_pin_not_tilde hit the zero-assertion guard (get_server_command() was empty, so its assert loop never ran). 4.6 captured fine, which is why only the 4.3 canary failed. Finding #17 was a low-severity defensive tweak (bound a hung uv/which); not worth breaking the minimum-supported engine. Restore the original blocking OS.execute, which captures reliably on all versions. client_configurator.gd is now byte-identical to its pre-audit baseline. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
dsarno
added a commit
that referenced
this pull request
May 30, 2026
…s, docs) (#494) * Fix read-only audit findings across plugin, server, tests, and scripts A read-only audit surfaced 28 actionable findings; this commit fixes them. Correctness: - theme set_constant/set_font_size reject non-numeric values instead of silently coercing garbage to 0 (validate before int(); int("abc")==0 never tripped the null guard). - signal_handler type-checks path/signal/target/method before calling .is_empty(), so a non-string param returns WRONG_TYPE instead of crashing the dispatcher as an opaque "malformed result". - script find_symbols now detects `static func` declarations. - server_lifecycle.stop_server no longer re-appends an already-tracked PID (produced duplicate kill candidates). Fragility: - material apply_to_node falls back to the in-memory material when the post-save reload returns null (was clearing the slot / crashing get_class()). - project/session godot:// resources route through safe_payload(_sync) like every other resource module. - ci-game-capture-smoke cleanup calls project_manage(op=stop); project_stop is not a named tool, so the game subprocess was never stopped. - serve-this-worktree guards `--port` with no following value under set -u. - test_cli_reload no longer leaks GODOT_AI_DEV_* env vars past teardown. Efficiency: - client_configurator check_uv_version / _find_system_install use the bounded McpCliExec instead of blocking OS.execute on the main thread. Dead code: - remove unused _resolve_or_create_player and _snapshot_curve, drop the no-op particle alias identity-map, and prefix unused scene_root locals with _. Test quality: - fix a vacuous readiness-gate assertion (asserted a command name never sent). - assert returned values, not just key presence, in logs/resource read tests. - assert color components in test_set_param_color_dict. - convert a bare return to skip() in test_project. - rename misnamed integration "pagination" tests to round-trip (real pagination is covered in test_mcp_tools.py and unit test_pagination.py). Docs/comments: - correct stale telemetry milestone/emit-point comment and mark the reserved record/milestone types; fix plugin.gd logger-build comment, batch history docstring, and the tools core-tool docstring; rename debugger _session_id (it is read in the stale-session guard). Repo hygiene: - remove orphaned test_telemetry_setting.gd.uid. Verified: ruff check clean; full pytest (1096 passed, 4 skipped); GDScript suite 0 failures on Godot 4.6.3 (1366 passed) and 4.3 (1332 passed); 0 parse errors on both engines. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Revert finding #17 (bounded McpCliExec) — regressed Godot 4.3 Routing check_uv_version / _find_system_install through McpCliExec uses OS.execute_with_pipe, whose output capture behaves differently on Godot 4.3 (the documented reason the cli_exec capture tests are skipped on < 4.4). On the 4.3 CI canary, `which godot-ai` came back with empty captured output, so _find_system_install returned "", get_server_command() resolved to nothing, and test_uvx_server_command_uses_exact_pin_not_tilde hit the zero-assertion guard (get_server_command() was empty, so its assert loop never ran). 4.6 captured fine, which is why only the 4.3 canary failed. Finding #17 was a low-severity defensive tweak (bound a hung uv/which); not worth breaking the minimum-supported engine. Restore the original blocking OS.execute, which captures reliably on all versions. client_configurator.gd is now byte-identical to its pre-audit baseline. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Address Copilot review: theme int hint + static-func find_symbols test - theme set_constant/set_font_size invalid-value errors now append an "expected an integer" hint instead of _COLOR_HINT ("hex #rrggbb…"), which was misleading for numeric slots (Copilot finding). - Add a `static func` to the find_symbols test fixture and assert its name is detected, locking in the static-func parsing path so it can't silently regress (Copilot finding). 0 GDScript parse errors on Godot 4.6.3 and 4.3. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ships 10 MCP tools for AnimationPlayer authoring: animation_player_create,
animation_create, animation_add_property_track, animation_add_method_track,
animation_set_autoplay, animation_play, animation_stop, animation_list,
animation_get, and animation_create_simple (high-level one-call composer).
Enables hover pulses, slide-in menus, fade transitions, and damage shake
for the roguelite benchmark. Animations live in-scene (save with .tscn);
all mutating operations are fully undoable. AnimationTree, bezier/audio
tracks, and preset helpers (fade/slide_in/shake/pulse_loop) are deferred.
with full EditorUndoRedoManager integration, value coercion (Color/Vector2
from dict/string), and baseline-track-count undo pattern for track inserts
All 373 Python tests pass; ruff clean.
https://claude.ai/code/session_01EZM3nFsQL4eV9atno59x9e