[break] feat(animation-graph): enrich state and motion inspector forms - #2
Conversation
…eferences - derive pose node enter info (state-machine / animation-blend / stash) without the editor-only `getEnterInfo` prototype methods - add `queryPoseGraphAssetDragHandlers` and the `create-pose-node-on-asset-drag` command to create motion pose nodes from dropped assets - add `queryStateMachineComponentTypes` for state machine component menus - count layer stash references via `visitStashReferences` - add `set-transition-condition-binding-class` and `set-transition-event-binding` commands with transition condition binding dumps and event binding fields Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
- merge speedMultiplierEnabled into a combined speedMultiplier property - publish blend 2D variables as variable-select fields and hide constant values - add Animation Clip Motion group metadata and rename UI markers - copy scalar createArg fields onto created pose nodes (variableName, stashName) - propagate ui attributes through property dumps for inspector overrides Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
…PI and MCP (cocos#893) * feat(scene): support PolygonCollider2D point regeneration * feat(mcp): expose PolygonCollider2D point regeneration
…on-graph-inspector
…t, atlas and converted image resolution) - P2-1: speedMultiplier combined property uses a full default factory so Inspector Reset restores both enabled and multiplier (was silently ignored) - P2-2: resolve SpriteFrame image source through the original texture reference (atlas plist frames and dynamic atlases), falling back to the frame uuid - P2-3: centralize decodable-file resolution in image-processing (resolveImagePixelSource): converted formats read the importer library PNG, others read the source; extractImagePixels uses it instead of assetInfo.file - add regression tests: set → reset → save/reload round-trip, texture uuid resolution cases, library png / missing product / plain source selection - include motion preview data query and preview service wiring (WIP continuation) Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
Review 问题修复结果(de104e43)针对 review 提出的 3 个 P2 已全部修复并推送(87d45f7f → de104e4): P2-1 · Reset 静默失效 ✅
P2-2 · 图集子帧解析错误 ✅
P2-3 · 绕过导入转换产物 ✅
验证
遗留说明
|
looopmax
left a comment
There was a problem hiding this comment.
Review scope
基于 5d07a6f7abff9f71508388b16b02ae9bb4d2b28d → 921d92a4e9289926dc6247f1b614ca640ad65765 的完整 diff 审查(31 files,+5067/-38),并检查了现有讨论、提交快照与远端 checks。当前 PR head 与本地工作树一致;本次未修改代码。
Standards
- Standards:按类型契约、资源导入后的真实运行路径、错误/边界处理、测试可复现性和最小改动原则审查。
- Spec:PR 描述覆盖了 inspector 表单字段、Motion Preview 和 PolygonCollider2D 等实现,但没有给出转换图片、SpriteAtlas/动态合图、薄精灵轮廓、DTS 快照和真实资源 fixture 的完整验收矩阵。
Findings
- [P1] DTS 快照仍声明
void,而generate:dts产物已变为Promise<void>,本地 dts-snapshot 失败 3 个 suite,远端check-dts也失败。请更新提交快照并重新跑兼容性检查。 - [P1]
extractImagePixels对根资源直接读取asset.source,没有选择 importer 的 library 图片。TGA/BMP/PSD/HDR/EXR/ZNT 等转换格式仍可能把原始文件交给 Sharp,导致 PolygonCollider2D 像素读取失败。请复用统一且带存在性校验的图片源解析。 - [P1] SpriteAtlas 子帧的 UUID 前缀是 SpriteAtlas/plist,而不是图片纹理 UUID;动态合图还需要优先 original texture。当前逻辑会把 plist 交给像素读取,图集子帧无法得到像素。请解析原始/当前纹理 UUID,再走统一图片源解析。
- [P1] 闭环起点在 contour 循环回到 start 时已经 push 一次,循环结束后又 push 一次;默认 threshold=1 下,1x1/1xN/2x1 等合法薄精灵会折叠为重复端点,最终少于 3 个点而无法 regenerate。请只保留一个闭环端点并补薄图回归。
- [P2]
queryMotionPreviewData的 JSDoc 写的是目标不存在返回motion: null,但实现会抛TARGET_NOT_FOUND,新增测试也明确断言 reject。请统一公开契约与实现。
减法复核
当前 PR 的标题和主体主要描述 inspector 表单增强,但完整 diff 同时加入 PolygonCollider2D 像素/轮廓能力、Motion Preview facade/runtime,以及 HTML+Markdown 数据结构文档(合计 31 个文件)。这扩大了审查和回归边界;建议拆分为相互独立的变更,或至少在 PR 描述中补齐每条能力的范围、失败路径和 fixture/test matrix。
验证
- PASS:
npx tsc --noEmit -p tsconfig.json。 - PASS:PolygonCollider2D、regenerate API、prefab UI handling 定向测试 24/24。
- FAIL:
npm run generate:dts:ci生成的hideMotion/playMotion/setMotionTime/setMotionVariable/setMotionParameter为Promise<void>,而提交快照为void;本地 dts-snapshot 定向运行 3 个 suite 失败、8 个通过。 - FAIL:远端
check-dts(run 34010901472 / job 101426429190)报告相同的 DTS API breaking/signature mismatch。 - 未覆盖:完整构建、Windows/macOS 运行时、真实 TGA/PSD/HDR/EXR/ZNT/TIFF、TexturePacker/dynamic atlas fixture,以及 Motion Preview UI/runtime 回归。
Verdict
本次以 COMMENT 发布(GitHub 不允许 PR 作者对自己的 PR 使用 REQUEST_CHANGES)。审查结论仍为需要修改:4 个 P1 问题需要在合并前修复——DTS 快照、转换图片源解析、图集/动态合图纹理解析、薄精灵闭环端点;同时请修正 Motion Preview 的 JSDoc 契约,并在修复后重新验证远端 checks 与真实资源路径。
animation-graph-data-structure docs are no longer referenced or maintained; delete the stale docs/zh markdown and generated HTML export to avoid doc drift.
PR #2 修复与验证汇总当前 head 验证结果:
剩余红灯是 |
f9967fc
into
Changdw:feature/animation-graph-inspector
* feat(assets): add animation graph inspector API * feat(assets): add animation graph inspector reset and create * feat(assets): align animation graph editing with Creator * feat(assets): expose pose graph enter info, drag handlers and stash references - derive pose node enter info (state-machine / animation-blend / stash) without the editor-only `getEnterInfo` prototype methods - add `queryPoseGraphAssetDragHandlers` and the `create-pose-node-on-asset-drag` command to create motion pose nodes from dropped assets - add `queryStateMachineComponentTypes` for state machine component menus - count layer stash references via `visitStashReferences` - add `set-transition-condition-binding-class` and `set-transition-event-binding` commands with transition condition binding dumps and event binding fields Co-authored-by: CommandCodeBot <noreply@commandcode.ai> * fix(animation-graph): create typed motion states * [break] feat(animation-graph): enrich state and motion inspector forms (#2) * feat(assets): expose pose graph enter info, drag handlers and stash references - derive pose node enter info (state-machine / animation-blend / stash) without the editor-only `getEnterInfo` prototype methods - add `queryPoseGraphAssetDragHandlers` and the `create-pose-node-on-asset-drag` command to create motion pose nodes from dropped assets - add `queryStateMachineComponentTypes` for state machine component menus - count layer stash references via `visitStashReferences` - add `set-transition-condition-binding-class` and `set-transition-event-binding` commands with transition condition binding dumps and event binding fields Co-authored-by: CommandCodeBot <noreply@commandcode.ai> * fix(animation-graph): create typed motion states * feat(animation-graph): enrich state and motion inspector forms - merge speedMultiplierEnabled into a combined speedMultiplier property - publish blend 2D variables as variable-select fields and hide constant values - add Animation Clip Motion group metadata and rename UI markers - copy scalar createArg fields onto created pose nodes (variableName, stashName) - propagate ui attributes through property dumps for inspector overrides Co-authored-by: CommandCodeBot <noreply@commandcode.ai> * feat(scene): support PolygonCollider2D point regeneration via Scene API and MCP (cocos#893) * feat(scene): support PolygonCollider2D point regeneration * feat(mcp): expose PolygonCollider2D point regeneration * fix(animation-graph): address PR #2 review issues (reset contract, atlas and converted image resolution) - P2-1: speedMultiplier combined property uses a full default factory so Inspector Reset restores both enabled and multiplier (was silently ignored) - P2-2: resolve SpriteFrame image source through the original texture reference (atlas plist frames and dynamic atlases), falling back to the frame uuid - P2-3: centralize decodable-file resolution in image-processing (resolveImagePixelSource): converted formats read the importer library PNG, others read the source; extractImagePixels uses it instead of assetInfo.file - add regression tests: set → reset → save/reload round-trip, texture uuid resolution cases, library png / missing product / plain source selection - include motion preview data query and preview service wiring (WIP continuation) Co-authored-by: CommandCodeBot <noreply@commandcode.ai> * feat(scene): expose animation graph motion preview facade via scene module * feat(scene): support generic motion preview service * fix(scene): harden motion preview lifecycle * fix: address PR #2 review issues * fix: preserve sprite frame UUID fallback * docs: remove obsolete animation graph data structure docs animation-graph-data-structure docs are no longer referenced or maintained; delete the stale docs/zh markdown and generated HTML export to avoid doc drift. --------- Co-authored-by: CommandCodeBot <noreply@commandcode.ai> Co-authored-by: ChiaNing <sun.cloud.mo@gmail.com> --------- Co-authored-by: looopmax <shao.x@outlook.com> Co-authored-by: CommandCodeBot <noreply@commandcode.ai> Co-authored-by: SHAO LIANG <128127821+looopmax@users.noreply.github.com> Co-authored-by: ChiaNing <sun.cloud.mo@gmail.com>
Summary
speedMultiplierEnabledinto a combinedspeedMultiplierproperty ({ enabled, multiplier }) rendered as checkbox + variable selectvariableX/variableYas variable-select fields (FLOAT variables) and hide constantvalueX/valueYAnimation Clip Motiongroup metadata anddisplayNamefor clipvariableName,stashName)uiattributes through property dumps for inspector custom UI overridesTest plan
npx jest animation-graph-service— 14/14 passed