Fix ParticleCompositeCurve zero max overload selection - #3083
Conversation
Walkthrough
ChangesParticle composite curve handling
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
d424af3 to
cfaa3bb
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/src/core/particle/ParticleCurve.test.ts`:
- Around line 19-26: Add a regression test using the Engine
hydration/deserialization entry point with serialized $args [5, 0], rather than
only constructing ParticleCompositeCurve directly. Assert the hydrated object’s
ParticleCurveMode, constantMin, constantMax, and evaluations at 0 and 1, while
retaining the existing constructor test unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3057f391-74dc-49db-8819-39d29613c198
📒 Files selected for processing (2)
packages/core/src/particle/modules/ParticleCompositeCurve.tstests/src/core/particle/ParticleCurve.test.ts
| it("Constructor with zero as the max constant", () => { | ||
| const gradient = new ParticleCompositeCurve(5, 0); | ||
| expect(gradient.mode).to.equal(ParticleCurveMode.TwoConstants); | ||
| expect(gradient.constantMin).to.equal(5); | ||
| expect(gradient.constantMax).to.equal(0); | ||
| expect(gradient.evaluate(undefined, 0)).to.equal(5); | ||
| expect(gradient.evaluate(undefined, 1)).to.equal(0); | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Cover the Engine hydration path in the regression test.
This test validates the core constructor directly, but it cannot catch runtime-v2 hydration converting $args: [5, 0] into Constant 5. Add a test through the actual Engine hydration/deserialization entry point and assert the hydrated object’s mode, endpoints, and evaluation; retain this unit test as well.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/src/core/particle/ParticleCurve.test.ts` around lines 19 - 26, Add a
regression test using the Engine hydration/deserialization entry point with
serialized $args [5, 0], rather than only constructing ParticleCompositeCurve
directly. Assert the hydrated object’s ParticleCurveMode, constantMin,
constantMax, and evaluations at 0 and 1, while retaining the existing
constructor test unchanged.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev/2.0 #3083 +/- ##
===========================================
- Coverage 85.42% 85.40% -0.02%
===========================================
Files 811 811
Lines 94654 94654
Branches 11512 11504 -8
===========================================
- Hits 80854 80839 -15
- Misses 13710 13725 +15
Partials 90 90
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
GuoLei1990
left a comment
GuoLei1990
left a comment
There was a problem hiding this comment.
🫧 尘小沫
结论
阻塞级别：无。已完成 bd34daa45612af8b402cd3be916ff181f21ae742...cfaa3bb4c97136019cce494b2dc33062522c06b9 的完整单提交增量审查；本轮未发现新的 P0/P1/P2，实际 review 动作为 COMMENTED，目标 HEAD 为 cfaa3bb4c97136019cce494b2dc33062522c06b9。CI 的 lint、三平台 build、四组 e2e 与 patch coverage 均通过；codecov/project 仍因整体覆盖率下降 0.02% 为红，patch 覆盖率为 100%。自动 CR 不替代人工 Reviewer 的合入门禁，本 review 不执行 APPROVE。
已关闭问题清单
discussion_r3670582660（要求增加 runtime-v2 hydration 回归）— 不适用。ReflectionParser对$args只逐项_resolveValue后原样执行new Class(...args)，既不按 truthiness 选择重载，也不会过滤0；本次根因和公开契约均由ParticleCompositeCurve构造器单点拥有。新增测试直接从公开构造入口断言mode、两个端点及evaluate，回退本修复后会失败，已经覆盖真实回归点；在本 PR 再复制一条未改动 generic loader 路径不会增加根因保护。
架构、熵增与测试治理
上游 Editor #3813 产出 canonical $args: [constantMin, constantMax]；Engine loader 仅解析参数并原样展开；ParticleCompositeCurve 构造器是重载选择、端点写入和 mode 的唯一 owner；下游 EmissionModule 只通过 burst.count.evaluate(...) 消费该公开状态。改动把 owner 内部的 truthiness 判据替换为参数存在性判据，没有新增 owner、状态、转换、校验、兼容分支、wrapper 或持久化表示，概念净增为 0。
同根因扫描未发现第二套 numeric max 重载判断；相邻 ParticleCompositeGradient 的第二参数是对象，不存在合法 falsy 端点。两曲线分支同步改为存在性判断只统一同一个构造协议，没有引入平行状态机。测试使用公开入口和公开输出，且反转代码即可证伪；没有失效 fixture/mock/snapshot，也没有为了旧测试保留 legacy fallback 或第二条转换路径。
2 participants
Summary
ParticleCompositeCurvetwo-argument overloads by argument presence instead of truthiness0as a valid maximum constantVerification
HEADLESS=true pnpm vitest run tests/src/core/particle/ParticleCurve.test.ts(8 tests passed)pnpm -F @galacean/engine-core b:typespnpm eslint packages/core/src/particle/modules/ParticleCompositeCurve.ts(no errors; 6 pre-existing warnings elsewhere in the file)pnpm prettier --check packages/core/src/particle/modules/ParticleCompositeCurve.ts tests/src/core/particle/ParticleCurve.test.tsCloses #3082
Summary by CodeRabbit
Bug Fixes
0being interpreted incorrectly.Tests
0.Downstream
ParticleCompositeCurve(min, max)runtime-v2 constructor data.2.0.0-alpha.*, Editor will bump its pinned Engine dependencies and restore the API → source-v2 → Builder → Loader → Engine object regression.