Skip to content

feat(playtest): add the character playtest workbench - #131

Merged
nighca merged 10 commits into
1024XEngineer:mainfrom
huyanxius:feat/playtest-workbench
Aug 6, 2026
Merged

feat(playtest): add the character playtest workbench#131
nighca merged 10 commits into
1024XEngineer:mainfrom
huyanxius:feat/playtest-workbench

Conversation

@huyanxius

@huyanxius huyanxius commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

实现 /playtest/:characterId/:outfitId,替换 #70 留下的 13 行占位页。

Closes #130

Why

角色资产做完之后,main 上没有任何地方能验证它「在游戏里是不是这样」。资产库看得到静态帧,看不到帧按各自时长播起来、按方向键走起来是什么样。帧序错了、时长错了、脚底线飘了,静态列表里都看不出来,动起来一眼就知道。

这是导出前的最后一道人工确认。08-03 导师会点名 Playtest 与外部交互最少,适合独立成 issue 单独推进,本 PR 兑现它。

Change Description

  • 新增 pages/playtest/,源码 6 个文件、测试 6 个文件、README 1 个文件,全部落在页面目录内,未改动 appentitiesfeaturesshared 任何一行。
  • 路由无需改动:app.tsx/playtest/:characterId/:outfitId 已存在,本 PR 只替换它指向的实现。
  • 10 条原子提交:适配器、状态机、输入接线、舞台、工作台、页面取数、边界测试、文档各一条,外加两条修复(精灵居中、播放循环的边界情况)。
  • 仓库不新增任何二进制素材。

Implementation Approach

  • 页面自己取数。 页面直接调 characterApis.get,不接受 app 层注入的服务——frontend-architecture-v3.md 第 2 节与 app/app.tsx 的注释都写明 app 只做启动和路由。写法与 character-detailasset-library 一致。
  • 播放顺序按 Frame.index,不按数组下标。 后端整棵下发资产树,数组顺序没有契约保证;照数组播的话顺序一变动画就乱,而且乱得不报错。排序在适配器里做一次,运行时之后只按下标推进。
  • 一次性动作播完停在末帧。 Action.loop 带进运行时,loop: false 的攻击、跳跃走到末帧就停,不回首帧变成假循环。
  • 满幅布局用 flex 分配高度,不减魔数。 底部操控胶囊贴着舞台内沿,舞台只要比视口高一点,胶囊就落到折叠线以下——页面看着是好的,操控要滚动才找得到。
  • 错误分两句。 ApiError 的业务码 404(HTTP 200 信封内)与传输失败分别显示「角色不存在」和「角色读取失败」。这一点比 character-detailasset-library 细:那两个页面把所有失败收成一句,而 Playtest 是靠带两个 ID 的深链进来的,角色被删或 ID 抄错是常见情况,跟后端挂了不是一回事。
  • 测试不新增假数据。 全部复用 src/test/project-assets-backend.tsfeat(projects): implement project center and asset library #119 引入的 HTTP 替身)。那份 fixture 里 walk 的后端帧序恰好是 2、0、1,正好把排序这条钉死。
  • 精灵的居中与位移合并在一处 transform。 从 07-31 快照搬来的写法同时用了 Tailwind 的 -translate-x-1/2 和内联 transform。Tailwind 4 起 translate 工具类编译到独立的 translate 属性而非 transform,两者叠加而非覆盖,静止位置因此左偏半个精灵宽,measureBounds 依赖「静止即居中」算出的左右边界也跟着错位。已去掉工具类,实测精灵中心与舞台中心重合。
  • 播放循环里的帧时长有下限。 advanceRuntime 按「攒够当前帧时长就翻帧、并从已攒时间里扣掉它」推进。帧时长为 0 时扣掉的是 0,判断条件又恒成立,循环动作会在 requestAnimationFrame 回调里转不出来,表现为标签页卡死。生产路径由 model.ts 的 clamp 兜住,但 advanceRuntime 是导出的纯函数、测试已在绕过 model 直接构造数据,运行时安全不该依赖另一个文件里的隐式不变量,因此在循环内也 clamp 到 1ms。
  • 复位只保留一处。 页面原先同时用 key 强制 remount 和 hook 内的复位 effect 做同一件事,remount 还更贵——丢掉已测量的 stage 边界、拆键盘监听、重跑帧预载。现在只留 effect。
  • 边界测试把相对路径归一化后再判层。 只匹配 @/features 字面量拦不住 ../../../features/workflow-controller——两种写法解析到同一个目录。已用一个违规探针文件验证过:加上测试红,删掉转绿。

Screenshots

Beforemain 上的占位页:

before

After — 待机,左侧列出该造型下全部有帧的动作:

after-idle

After — 切到行走:

after-walk

截图取自本地 dev server,配一个只存在于本机、按 API_CONTRACT.md 返回固定角色的桩服务(素材为 asset-lab 的 samurai,36 帧待机 / 行走 / 跳跃)。桩服务与素材都不进仓库。

Testing

本地执行了 CI 工作流中的全部步骤:

  • npm run format:check — 通过(66 个文件)
  • npm run lint — 通过,无告警
  • npm run typecheck — 通过
  • npm run test — 通过(17 个文件 / 69 个用例,其中 playtest 7 个文件 / 30 个用例)
  • npm run build — 通过(284.16 kB / gzip 89.63 kB)

浏览器手工核对:帧按时长播放、点击切换动作、A/D 与方向键操控、松开回到待机、窗口缩放后移动范围重算、底部操控胶囊在一屏内不需滚动。

Follow-ups

  • 后端接口尚未挂载。 GET /characters/{id}main 上只有数据模型与抽象接口,web/api/ 下没有对应路由(属 feat(backend): 后端全栈实现——基础设施 + 领域模块 + 用户认证 + API 契约 #75 范围)。字段契约已逐字段对齐,接口挂载后本页面无需改动。在此之前正式路径显示「角色读取失败」。
  • 无演示入口:本 PR 有意不包含演示路由与随仓库提交的素材,因此在 feat(backend): 后端全栈实现——基础设施 + 领域模块 + 用户认证 + API 契约 #75 合入前页面上看不到内容。
  • preloadActionFrames 预载当前造型下全部动作的帧,动作多、帧数大时首次进入的网络开销未做上限。
  • 精灵居中那个缺陷通过了 typecheck、lint 与当时全部 65 个用例,是机器人评审发现的。补的 stage.test.tsx 只能钉住成因(transform 字符串完整、元素上不出现 translate 工具类),jsdom 不排版,偏移本身测不出来。同类的布局问题在本仓库目前只能靠人眼或浏览器实测。
  • advanceRuntime 在结果无变化时返回原引用,但只有非循环动作停在末帧且角色未移动时会命中。循环动作播放期间 frameElapsedMs 每帧真的在变,仍然每帧重渲染一次。要消除这部分开销需要把 frameElapsedMs 移出 state,本 PR 不做。
  • 键盘监听挂在 window 上,Playtest 路由内没有其他会抢按键的输入控件;将来页面上出现输入框时需要复核 isTypingTarget 的覆盖面。

Related

@vercel

vercel Bot commented Aug 6, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
windup Ready Ready Preview Aug 6, 2026 9:19am

fennoai[bot]

This comment was marked as outdated.

@huyanxius

Copy link
Copy Markdown
Collaborator Author

@fennoai /review

@fennoai

fennoai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Review Result

Found 1 issue.

  • P2 frontend/src/pages/playtest/workbench/stage.tsx:74: the sprite is horizontally translated twice. The image has Tailwind -translate-x-1/2, which builds to the CSS translate property, and it also has inline transform: translate3d(calc(-50% + ${x}px), 0, 0) scaleX(...) on line 77. Modern browsers apply both transform longhands and the transform shorthand, so the neutral position is offset by another half sprite width instead of centered on the stage line. This skews the playtest baseline and movement bounds, especially for wider sprites. Remove the Tailwind translate utility or move all centering/movement into one transform path.

Verified:

  • npm run test -- src/pages/playtest passed: 6 files / 26 tests.
  • npm run typecheck passed.
  • npm run lint passed.
  • npm run format:check passed.
  • npm run build passed.
  • git diff --check 795b51ea05464e3cbd3ba23832089c8b61631ac8...c53f4997a15038525681d4490ea23f3282fe4a10 passed.

Note: npm ci was needed because frontend/node_modules was absent. It completed with a Node engine warning: sandbox Node is v20.20.2, while react-router@8.3.0 declares >=22.22.0.

View job run

@huyanxius

Copy link
Copy Markdown
Collaborator Author

Confirmed and fixed in 6840a08.

Measured on a running page before the fix: the sprite resolved to translate: -50% and transform: matrix(1, 0, 0, 1, -128, 0) at the same time. With a 256px sprite on a 1456px stage, the sprite centre sat at 592 while the stage centre was at 720 — off by exactly half the sprite width, so its right edge was resting on the centre line instead of its centre.

Root cause is the Tailwind 4 upgrade: translate utilities now compile to the standalone translate property rather than into transform. Individual transform properties are applied before transform, so the two declarations stack instead of one overriding the other. This code came from a 2026-07-31 snapshot where the same markup was still correct under Tailwind 3.

Second-order effect you did not mention, and the reason this was worth fixing before merge rather than after: measureBounds derives the horizontal movement limits from (stageWidth - characterWidth) / 2, which assumes the resting position is centred. With the offset the character walked past the left edge and stopped short on the right.

Fix drops -translate-x-1/2 and keeps centring and movement in the single inline transform. After the change the sprite centre and the stage centre both measure 720.

Added workbench/stage.test.tsx as a regression guard. jsdom does not lay out, so the offset itself is not measurable in a test; the assertions pin the cause instead — the full transform string, and the absence of any translate utility on the element. Worth noting that this defect passed typecheck, lint and all 65 existing tests, so nothing in the suite would have caught it.

@minorcell

Copy link
Copy Markdown
Member

/review -claude

@minorcell

Copy link
Copy Markdown
Member

@fennoai /review

需要 review 的话,直接输入"/review" 这个 slash 指令即可,这个 slash 会走专用的 review skill、subagents

@fennoai

fennoai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Command Clarification

明白。这个触发内容是在说明用法,不是直接请求我重新 review,所以本次没有启动 PR review。

需要我走专用 review 流程时,请直接发送:

/review

View job run

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Playtest Workbench 审查

整体质量高:纯运行时逻辑与 React 胶合层 分离干净、rAF 用 ref + 函数式 setRuntime 正确避开了闭包陷阱、架构边界测试(别名与相对路径都折算)是很强的护栏。安全面无问题——路由参数经 encodeURIComponent,错误信息已脱敏,后端 imageUrl 只进 <img src>(非脚本 sink)。

以下为具体发现,已按位置放置为行内评论:

重要

  • runtime.ts advanceRuntime:循环动作 + 非正 durationMs 会导致 while 死循环,当前仅靠 model.ts 的远端 Math.max(1,…) clamp 兜底,而 advanceRuntime 已导出、测试直接手搓 action 绕过该保护。
  • runtime.ts / use-playtest-runtime.ts:rAF 每帧都 { ...runtime } 返回新引用,即使角色静止/停在末帧也会 ~60fps 全树重渲染。建议在 advanceRuntime 无变化时返回原引用,让 useState bail out。
  • index.tsx:key={initialActionId ?? ''} 的整树 remount 与 usePlaytestRuntime 内部基于 initialActionId 的 reset effect 重复,且更重(丢失 bounds、重装监听、重跑预加载)。二选一。

次要

  • index.tsx:fetch effect 依赖 [characterId, outfitId],但 body 只用 characterId;同角色换 outfit 会多余重拉整份 Character。
  • README.md:「按住切 walk / 松开切 idle」表述为无条件,但 setDirectionInput 在缺少对应 action 时回退到当前 action(仍移动)。属优雅降级边界,建议补注「存在 walk/idle 动作时」。
  • README.md:两者都缺失时的 100ms 硬默认(DEFAULT_FRAME_DURATION_MS)未在文档中提及。

View job run

Comment thread frontend/src/pages/playtest/workbench/runtime/runtime.ts Outdated
Comment thread frontend/src/pages/playtest/workbench/runtime/runtime.ts
Comment thread frontend/src/pages/playtest/index.tsx Outdated
Comment thread frontend/src/pages/playtest/index.tsx
Comment thread frontend/src/pages/playtest/README.md Outdated
@huyanxius

Copy link
Copy Markdown
Collaborator Author

@fennoai 先问一个前置问题,请只回答,暂时不要做任何改动。

本 PR 的 head 分支在 fork 上:huyanxius/Windup:feat/playtest-workbench,不在 1024XEngineer/Windup 主仓。PR 的 maintainer_can_modify 为 true。

你有没有权限把提交推到这个 fork 分支?

如果有,我接下来会把要修的几条发给你,你在这个分支上改。
如果没有,也请直接说,并说明你在这种情况下的默认行为是什么。是另开一个对着 main 的新 PR,还是只把改动内容贴在评论里由我本地应用。不要自行另开 PR。

@fennoai

fennoai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Permission Answer

有权限判断上可以推:我检查了 huyanxius/Windup,它是 public fork,触发用户 huyanxius 在该仓库上的权限是 admin。在这个前提下,如果你接下来明确要求我修改,我可以在当前 PR 的 fork head 分支 huyanxius/Windup:feat/playtest-workbench 上尝试提交并推送。

需要说明一点:真正执行 git push 时仍取决于 GitHub 是否允许当前 FennoAI runtime credential 写入该 fork 分支。若之后实际 push 被 GitHub 拒绝,我的默认行为是停止并在评论里说明失败原因;不会自行另开对 main 的新 PR。除非你明确要求,我也不会把它改成另一个 PR 流程。

本次只回答问题,没有改代码、没有提交、没有推送。

View job run

@huyanxius

Copy link
Copy Markdown
Collaborator Author

@fennoai 那好,对于你提出的问题反馈如下,四条采纳,一条按下述范围采纳。请在本 PR 分支上修复。
1. runtime.ts advanceRuntime 的死循环(采纳)
把循环里读到的帧时长下限提到 1,与 model.ts 的 frameDuration 保持同一个下限:Math.max(1, action.frames[frameIndex]?.durationMs ?? 1),条件判断和递减两处都要用同一个值。补一条测试:循环动作的帧 durationMs 为 0 时 advanceRuntime 能正常返回。

2. index.tsx 的 key 与 hook 内复位重复(采纳)
去掉 key={initialActionId ?? ''},保留 usePlaytestRuntime 里依赖 [actions, initialActionId] 的复位 effect 作为唯一权威。

3. index.tsx 的 outfitId 冗余依赖(采纳)
fetch effect 的依赖收窄为 [characterId],effect 内不再引用 outfitId;路由参数完整性由渲染期已有的守卫负责。

4. README 表述绝对化(采纳)
把「按住时切到首个 walk 动作」「松开后切回首个 idle 动作」限定为「存在对应动作时」,并补一句:找不到时保持当前动作、移动照常。同时补上第三级兜底 DEFAULT_FRAME_DURATION_MS 的 100ms。

5. advanceRuntime 每帧新对象(按缩小的范围采纳)
可以加你建议的相等判断并在无变化时返回原引用。但请不要在提交信息或 PR 描述里写成「消除空闲帧重渲染」:循环动作播放时 frameElapsedMs 每帧确实在变,相等判断不会命中,实际生效的只有非循环动作停在末帧且角色未移动的情况。如需彻底消除循环动画的每帧重渲染,需要把 frameElapsedMs 移出 state,本 PR 不做。

约束

  • 只改 frontend/src/pages/playtest/ 下的文件,不要动 .github/ 或其它目录。
  • 提交信息用 Conventional Commits,例如 fix(playtest): clamp frame duration inside the playback loop。一条或多条都可以,每条都要合规。
  • 改完请跑 format:check、lint、typecheck、test、build 五项并回报结果。
  • 若 push 被 GitHub 拒绝,按你说的停下并说明原因。

@fennoai

fennoai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Completed

已在本 PR fork 分支 huyanxius/Windup:feat/playtest-workbench 上修复并推送。

Commit: c97c641 fix(playtest): harden playback runtime edge cases

改动范围保持在 frontend/src/pages/playtest/ 下:

  • advanceRuntime 播放循环内统一把帧时长下限 clamp 到 1ms,并补了 durationMs: 0 的循环动作测试。
  • 移除页面层 PlaytestWorkbench 的重复 key 复位。
  • 将角色读取 effect 依赖收窄为 [characterId],不再读取 outfitId
  • 更新 README,限定 walk/idle 动作存在时才切换,并补充 100ms 第三级兜底。
  • 为运行时无变化场景返回原引用,范围按你描述的非循环动作停在末帧场景处理。

验证结果:

  • npm run format:check passed
  • npm run lint passed
  • npm run typecheck passed
  • npm run test passed: 17 files / 69 tests
  • npm run build passed

Push succeeded; workspace is clean.

View job run

@huyanxius

Copy link
Copy Markdown
Collaborator Author

@fennoai /compact

@fennoai

fennoai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Compact Summary

Current PR context:

  • PR: 1024XEngineer/Windup#131
  • Branch: huyanxius/Windup:feat/playtest-workbench into 1024XEngineer/Windup:main
  • Current head SHA: c97c641d73bbb1cb8830d6cca8dbbe1ea38274cc
  • Last FennoAI commit pushed: c97c641 fix(playtest): harden playback runtime edge cases

Recent completed work:

  • Fixed advanceRuntime to clamp frame durations inside the playback loop to at least 1ms.
  • Added runtime tests for zero-duration looping frames and unchanged stopped one-shot runtime references.
  • Removed duplicate key reset from PlaytestWorkbench rendering.
  • Narrowed the character fetch effect dependency to [characterId].
  • Updated frontend/src/pages/playtest/README.md to qualify walk/idle fallbacks and document the 100ms default duration fallback.
  • Scope stayed under frontend/src/pages/playtest/.

Verification from the last implementation run:

  • npm run format:check passed
  • npm run lint passed
  • npm run typecheck passed
  • npm run test passed: 17 files / 69 tests
  • npm run build passed
  • Push to huyanxius/Windup:feat/playtest-workbench succeeded

No repository changes were made for this /compact request.

View job run

@huyanxius

Copy link
Copy Markdown
Collaborator Author

@fennoai 长期记忆:在本仓库中所有的 Github comments 都对我使用中文。

@fennoai

fennoai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

已记录

已写入长期记忆:在 1024XEngineer/Windup 仓库中,FennoAI 发给你的所有 GitHub comments 都使用中文。

本次没有改动仓库代码。

查看任务运行

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: 这种一般不用提交,边界通过文档/注释/良好的目录组织一般就能体现了,不用单独写测试代码来保证

@nighca
nighca merged commit 830708b into 1024XEngineer:main Aug 6, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: 实现 Playtest 核验台页面

3 participants