fix(physics): preserve kinematic sync semantics#3041
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
GuoLei1990
left a comment
There was a problem hiding this comment.
首轮 review @ b84a476d4(stacked PR 2/3,--comment)
Stacked 关系:本 PR base =
fix/physics-shaderlab-split(#3025 的 contact event gate),其上 #3042 再 stack 本 PR。已确认dev/2.0与fix/physics-shaderlab-split均不含这套 kinematic sync 机制(grep_syncEntityTransformToNative/KinematicTransformSyncMode/_pendingReenter全空),故对 dev/2.0 主线这是净新增行为(PR 标题"preserve"指的是相对其它已有这套改动的分支保持语义,非 revert 主线既有 fix)。本轮按其自身 diff 审查。
总结
把 kinematic collider 的 transform 同步拆成两条语义清晰的路径:swept target(setKinematicTarget,PhysX 视为帧间运动、产生扫掠接触)与 teleport(setWorldTransform/setGlobalPose,瞬移不带速度)。日常逐帧同步走 target,re-enter / clone-重建 这类"native actor 停在陈旧 pose"的场景强制 teleport,避免从陈旧位姿扫掠拖拽 + 误触发接触事件。方向正确、抽象干净,与 PhysX 文档语义对齐。
设计核对(均通过)
- 两路 dispatch 落点对(Collider.ts:124-131):
_pendingReenterTeleport || _updateFlag.flag分流到_teleportToEntityTransformvs_syncEntityTransformToNative,DynamicCollider override 后者在isKinematic && mode===Target时走move(),否则 super teleport。CharacterController override 前者用setWorldPosition(其 native 只支持位置,_worldRotation加下划线标记刻意丢弃 —— CCT 胶囊不经 transform 旋转,正确)。 - re-enter teleport 的 per-instance 状态正确收口(Collider.ts:39-41,149-155,169):
_pendingReenterTeleport/_enteredScene均@ignoreClone;首次_onEnableInScene只置_enteredScene=true走 subclass 语义(kinematic 仍move),再次 enable 才置 teleport;消费后立即=false(:128)。clone 经_cloneTo置target._pendingReenterTeleport=true让克隆体首帧 teleport 到正确位姿。dirty/teleport 信号挂在 owner 自身、消费后复位,无残留。 - CCD 状态机正确(PhysXDynamicCollider.ts):PhysX 对 kinematic actor 写 CCD flag 会 warn+忽略。
setCollisionDetectionMode缓存 intent,仅非 kinematic 时即时 apply;setIsKinematic(true)先降 Discrete 再置 eKINEMATIC,setIsKinematic(false)先撤 eKINEMATIC 再回放缓存的_collisionDetectionMode。_isKinematic本地镜像跟踪。逻辑自洽。 move()四象限 normalize 正确(PhysXDynamicCollider.ts):带 rotation 的两条路径都copyFrom(rotation).normalize()再喂setKinematicTarget(PhysX 要求 normalized quat,否则 actor 旋转错乱/assert);position-only 复用从 PhysX 读回的当前 rotation(已 normalized)。addForce/addTorquekinematic 提前 return:PhysX 本就对 kinematic no-op + warn,提前 return 是干净优化(省 WASM 跨界 + 抑 warn),_isKinematic一旦在setIsKinematic(false)翻假即恢复 —— 被applyForce after kinematic→dynamic switch测试守住。- lite
move()实现为 teleport(LiteDynamicCollider.ts:62-70,commitb84a476d4):无 PhysX 后端下 setKinematicTarget 无意义,落成setWorldTransform的 teleport 语义,与三参分支正确路由,符合 PR 标题"keep as teleport"。
测试粒度(链路测试,合格)
CCD 两个测试(CCD mode survives kinematic toggle / ...defers native CCD flag application)从公开 collisionDetectionMode/isKinematic setter 驱动,断言 PhysX 原生 _pxActor.getRigidBodyFlags(eENABLE_CCD) —— 读 native 真值而非引擎侧镜像,落点对(CCD flag 本就是 native 关切,@ts-ignore 取 native actor 是唯一验证途径,可接受)。teleports kinematic target collider on re-enable instead of sweeping 守住 re-enter teleport 这条链。
JSDoc 合规(已核对,通过)
公开 kinematicTransformSyncMode getter/setter 有多行 @remarks(Target=swept、Teleport=直写 pose),带句号;新增 DynamicColliderKinematicTransformSyncMode 枚举两个成员各有单行 JSDoc 带句号,并从 physics/index.ts 正确导出。
唯一 [P3](不阻塞)
kinematicTransformSyncMode 的 setter 是裸赋值(DynamicCollider.ts:342-344)。它只影响下一次 _syncEntityTransformToNative 的分支选择,不需要即时同步 native,故无副作用、设计正确 —— 但与同类 setter 一致性上值得一提:若用户在 kinematic 运行中途从 Teleport 切回 Target,切换在下一帧逐帧同步时才生效(无即时重置),这是符合预期的惰性语义,无需改。仅记录,非问题。
结论
无 P0/P1/P2,--comment。 stacked PR,建议随 #3025 → 本 PR → #3042 的栈顺序合并;本 PR 自身 diff 干净、抽象正确、测试守得住。
Summary
setKinematicTargetwhile keeping re-enter/clone stale-pose recovery as teleport.DynamicColliderKinematicTransformSyncModefor callers that need target vs teleport semantics.Split Scope
This is PR 2/3 from the former broad physics split and is stacked on #3025:
DynamicCollider.applyForceAtPositionis intentionally out of scope; the long-term PhysX binding path is handled by #3039.Verification
pnpm -F @galacean/engine-core run b:typespnpm -F @galacean/engine-physics-lite run b:typespnpm -F @galacean/engine-physics-physx run b:typesgit diff --checkTargeted local Vitest collection is currently blocked in this worktree by
packages/galacean/src/index.tsregistering browser polyfills under the Node runner (window is not defined); stacked PR CI currently only runs labeler until the stack is retargeted/merged.