Forward animation_handler split onto beta (#344 → beta)

[dsarno]

Summary

Forwards the animation_handler split from PR #344 (which I incorrectly merged to main instead of beta) onto beta, so the audit-stack history stays consistent with the umbrella's branching policy in #297. The maintainer-owned beta → main promotion will see the same patch on both sides.

Cherry-pick of d915c4d (#344) onto beta, with a one-file conflict resolved against #337's "scene-absolute target_path" changes:

• _resolve_preset_target (now in animation_presets.gd) accepts both scene-absolute (/Main/Foo) and root_node-relative (World/Foo) target_path shapes, returns {node, kind, track_path_root}. Each preset_* consumes track_path_root (not the raw target_path) when building its track string.
• The _player_root_node helper Accept scene-absolute target_path in animation presets (closes #328) #337 introduced is now AnimationValues.player_root_node (same logic, same call sites: validate_animation, resolve_track_prop_context, _resolve_preset_target).

Why this exists separately

I merged #344 to main directly. Per the umbrella issue's branching policy ("All audit follow-up PRs should target beta by default, not main"), it should have flowed through beta. This PR keeps beta consistent so the eventual beta → main promotion isn't the first time beta sees the split. The patch on both branches is functionally identical — git will collapse them at promotion time.

Validated locally

• script/ci-check-gdscript — clean
• ruff check src/ tests/ — clean
• pytest -q — 767 passed (vs 722 on main; beta has more tests including the 6 test_preset_*_accepts_scene_absolute_target cases Accept scene-absolute target_path in animation presets (closes #328) #337 added)
• SceneTree smoke — confirms handler instantiates its submodules, WeakRef back-pointers resolve, every static helper in AnimationValues round-trips, AnimationPresets._direction_offset returns the right axis convention for 2D and 3D, all 7 proxy methods exist on the handler

Test plan

• CI green on Linux / macOS / Windows
• test_run suite=animation via MCP against a live editor — full pass, including the scene-absolute-target tests added in Accept scene-absolute target_path in animation presets (closes #328) #337

Refs #297 (audit umbrella, finding #13) — #342 (split tracker) — #344 (the version that landed on main).

https://claude.ai/code/session_011QxzADbf9zHwfZicjzdvCw

---

Generated by Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[Copilot]

Pull request overview

This PR forwards the animation handler split (presets + read/value helpers extracted into submodules) onto beta, keeping the animation_handler public surface stable via thin proxy methods while aligning with the existing *_handler.gd + *_presets.gd + *_values.gd pattern used elsewhere in the plugin.

Changes:

• Extract animation preset logic into animation_presets.gd and wire it into animation_handler.gd via _presets proxies.
• Extract read-only animation ops + shared coercion/serialization helpers into animation_values.gd and wire it via _values proxies / static helpers.
• Update the handler to call the new AnimationValues.* statics for transition parsing and value coercion.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
plugin/addons/godot_ai/handlers/animation_handler.gd	Instantiates _presets/_values, routes preset/read APIs through proxies, and switches coercion/transition helpers to AnimationValues.
plugin/addons/godot_ai/handlers/animation_presets.gd	New preset module containing preset_* implementations and _resolve_preset_target.
plugin/addons/godot_ai/handlers/animation_values.gd	New values/read module containing list/get/validate plus shared coercion and serialization helpers.
plugin/addons/godot_ai/handlers/animation_presets.gd.uid	New UID for the presets script.
plugin/addons/godot_ai/handlers/animation_values.gd.uid	New UID for the values script.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[dsarno]

Genuine bug — confirmed by reading the code. Target:modulate:a (the shape preset_fade produces, plus any position:y etc. subpath track) does get the property+subpath glued onto the node part by rfind(":"), so validate_animation reports those tracks as broken even when the target node exists.

But this is pre-existing on main and beta (the original animation_handler.gd:604 already uses rfind(":")), and PR #367 is a faithful cherry-pick of #344 onto beta — fixing it here would mean a behavior change inside what's supposed to be a 1:1 forward of the merge that should have gone to beta in the first place.

I'll open a separate follow-up against beta with the one-character fix (rfind → find) plus a validate_animation test that creates a Target:modulate:a track and asserts valid_count > 0. Better diff/review story than smuggling it into the cherry-pick.

---

Generated by Claude Code