rename limit/limitX/Y/Z to speed/speedX/Y/Z in LimitVelocit…#2928
Merged
cptbtptpbcptdtptp merged 8 commits intogalacean:dev/2.0from Mar 19, 2026
Merged
rename limit/limitX/Y/Z to speed/speedX/Y/Z in LimitVelocit…#2928cptbtptpbcptdtptp merged 8 commits intogalacean:dev/2.0from
cptbtptpbcptdtptp merged 8 commits intogalacean:dev/2.0from
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughPublic API renamed from Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/src/core/particle/LimitVelocityOverLifetime.test.ts (1)
95-108:⚠️ Potential issue | 🟡 MinorTest description references old API names.
The test description says "limitX/Y/Z properties" but should say "speedX/Y/Z properties" to match the renamed API.
📝 Suggested fix
- it("limitX/Y/Z properties", function () { + it("speedX/Y/Z properties", function () {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/src/core/particle/LimitVelocityOverLifetime.test.ts` around lines 95 - 108, Update the test description string to reflect the renamed API: change the it() description from "limitX/Y/Z properties" to "speedX/Y/Z properties" in the test that exercises particleRenderer.generator.limitVelocityOverLifetime and its speedX, speedY, speedZ property assignments so the human-readable test name matches the actual API being tested.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/src/core/particle/LimitVelocityOverLifetime.test.ts`:
- Around line 87-93: Update the test description string to reflect the renamed
API: change the "limit property (alias for limitX)" description to "speed
property (alias for speedX)" in the test that accesses
particleRenderer.generator.limitVelocityOverLifetime and asserts on lvl.speed
and lvl.speedX (using ParticleCompositeCurve in that test) so the description
matches the actual assertions and API names.
---
Outside diff comments:
In `@tests/src/core/particle/LimitVelocityOverLifetime.test.ts`:
- Around line 95-108: Update the test description string to reflect the renamed
API: change the it() description from "limitX/Y/Z properties" to "speedX/Y/Z
properties" in the test that exercises
particleRenderer.generator.limitVelocityOverLifetime and its speedX, speedY,
speedZ property assignments so the human-readable test name matches the actual
API being tested.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 72394700-f33f-4241-a90b-72043c1604a3
📒 Files selected for processing (3)
e2e/case/particleRenderer-limitVelocity.tspackages/core/src/particle/modules/LimitVelocityOverLifetimeModule.tstests/src/core/particle/LimitVelocityOverLifetime.test.ts
GuoLei1990
approved these changes
Mar 19, 2026
github-actions
bot
added
the
documentation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev/2.0 #2928 +/- ##
========================================
Coverage 77.43% 77.44%
========================================
Files 896 896
Lines 98245 98246 +1
Branches 9685 9686 +1
========================================
+ Hits 76080 76088 +8
+ Misses 21999 21993 -6
+ Partials 166 165 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/core/src/particle/modules/LimitVelocityOverLifetimeModule.ts (1)
103-154: Consider keeping deprecatedlimit*aliases for compatibilityThe rename to
speed*is clear, but removinglimit,limitX,limitY,limitZoutright can break existing integrations immediately. Adding deprecated alias accessors would make migration safer.Compatibility shim example
+ /** `@deprecated` Use `speed` instead. */ + get limit(): ParticleCompositeCurve { + return this.speed; + } + /** `@deprecated` Use `speed` instead. */ + set limit(value: ParticleCompositeCurve) { + this.speed = value; + } + + /** `@deprecated` Use `speedX` instead. */ + get limitX(): ParticleCompositeCurve { + return this.speedX; + } + /** `@deprecated` Use `speedX` instead. */ + set limitX(value: ParticleCompositeCurve) { + this.speedX = value; + } + + /** `@deprecated` Use `speedY` instead. */ + get limitY(): ParticleCompositeCurve { + return this.speedY; + } + /** `@deprecated` Use `speedY` instead. */ + set limitY(value: ParticleCompositeCurve) { + this.speedY = value; + } + + /** `@deprecated` Use `speedZ` instead. */ + get limitZ(): ParticleCompositeCurve { + return this.speedZ; + } + /** `@deprecated` Use `speedZ` instead. */ + set limitZ(value: ParticleCompositeCurve) { + this.speedZ = value; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/particle/modules/LimitVelocityOverLifetimeModule.ts` around lines 103 - 154, Add deprecated alias accessors named limit, limitX, limitY, and limitZ that forward to the new speed, speedX, speedY, and speedZ getters/setters so existing code keeps working; implement each getter to return the corresponding _limitX/_limitY/_limitZ (or speed getter for overall) and each setter to delegate to the corresponding speed setter (or set _limitX/_limitY/_limitZ and call _onCompositeCurveChange(lastValue, value) exactly as the speedX/speedY/speedZ setters do), and annotate them with a JSDoc `@deprecated` note so consumers see the migration guidance while preserving identical behavior using the same underlying fields (_limitX/_limitY/_limitZ) and _onCompositeCurveChange.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/particle/modules/LimitVelocityOverLifetimeModule.ts`:
- Around line 215-218: The doc comment for the space property in
LimitVelocityOverLifetimeModule is inaccurate; update the remark to reflect that
space is uploaded and applied for both scalar and separate-axes modes rather
than only when separateAxes is enabled. Edit the JSDoc for the space
field/property (and any related comment block in the
LimitVelocityOverLifetimeModule class) to remove or reword the "Only takes
effect when 'separateAxes' is enabled" phrase and instead state that space
controls whether limits are in local or world space and is honored in both
scalar and separateAxes code paths where the property is uploaded and used.
- Line 79: The change to the private field _dampen in
LimitVelocityOverLifetimeModule changed the default from full damping to zero,
which silently alters runtime behavior; revert the default back to the original
(full damping) value by setting _dampen to the previous default (e.g., 1) in the
LimitVelocityOverLifetimeModule declaration and ensure any related
comments/tests reflect that default so the shader's effectiveDampen produces the
intended clamping behavior.
---
Nitpick comments:
In `@packages/core/src/particle/modules/LimitVelocityOverLifetimeModule.ts`:
- Around line 103-154: Add deprecated alias accessors named limit, limitX,
limitY, and limitZ that forward to the new speed, speedX, speedY, and speedZ
getters/setters so existing code keeps working; implement each getter to return
the corresponding _limitX/_limitY/_limitZ (or speed getter for overall) and each
setter to delegate to the corresponding speed setter (or set
_limitX/_limitY/_limitZ and call _onCompositeCurveChange(lastValue, value)
exactly as the speedX/speedY/speedZ setters do), and annotate them with a JSDoc
`@deprecated` note so consumers see the migration guidance while preserving
identical behavior using the same underlying fields (_limitX/_limitY/_limitZ)
and _onCompositeCurveChange.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0f9e991f-f903-4700-9aaf-547b0e1fa264
📒 Files selected for processing (3)
e2e/case/particleRenderer-limitVelocity.tspackages/core/src/particle/modules/LimitVelocityOverLifetimeModule.tstests/src/core/particle/LimitVelocityOverLifetime.test.ts
✅ Files skipped from review due to trivial changes (1)
- e2e/case/particleRenderer-limitVelocity.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/src/core/particle/LimitVelocityOverLifetime.test.ts
| @deepClone | ||
| private _limitZ: ParticleCompositeCurve; | ||
| private _dampen: number = 1; | ||
| private _dampen: number = 0; |
There was a problem hiding this comment.
Default dampen change silently alters module behavior
Line 79 changes the default from full damping to no damping. With the current shader formula, dampen = 0 yields effectiveDampen = 0, so excess velocity is never reduced by default. This is a functional behavior change, not just a rename.
Proposed fix
- private _dampen: number = 0;
+ private _dampen: number = 1;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private _dampen: number = 0; | |
| private _dampen: number = 1; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/particle/modules/LimitVelocityOverLifetimeModule.ts` at
line 79, The change to the private field _dampen in
LimitVelocityOverLifetimeModule changed the default from full damping to zero,
which silently alters runtime behavior; revert the default back to the original
(full damping) value by setting _dampen to the previous default (e.g., 1) in the
LimitVelocityOverLifetimeModule declaration and ensure any related
comments/tests reflect that default so the shader's effectiveDampen produces the
intended clamping behavior.
Comment on lines
215
to
218
| /** | ||
| * Specifies if the velocity limits are in local space or world space. | ||
| * @remarks Only takes effect when 'separateAxes' is enabled. | ||
| */ |
There was a problem hiding this comment.
space remark is inaccurate
Line 217 says space only works when separateAxes is enabled, but space is uploaded and used in both scalar and separate-axes paths. This doc can mislead users.
Proposed doc fix
/**
* Specifies if the velocity limits are in local space or world space.
- * `@remarks` Only takes effect when 'separateAxes' is enabled.
+ * `@remarks` Applies whether `separateAxes` is enabled or not.
*/📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /** | |
| * Specifies if the velocity limits are in local space or world space. | |
| * @remarks Only takes effect when 'separateAxes' is enabled. | |
| */ | |
| /** | |
| * Specifies if the velocity limits are in local space or world space. | |
| * `@remarks` Applies whether `separateAxes` is enabled or not. | |
| */ |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/particle/modules/LimitVelocityOverLifetimeModule.ts` around
lines 215 - 218, The doc comment for the space property in
LimitVelocityOverLifetimeModule is inaccurate; update the remark to reflect that
space is uploaded and applied for both scalar and separate-axes modes rather
than only when separateAxes is enabled. Edit the JSDoc for the space
field/property (and any related comment block in the
LimitVelocityOverLifetimeModule class) to remove or reword the "Only takes
effect when 'separateAxes' is enabled" phrase and instead state that space
controls whether limits are in local or world space and is honored in both
scalar and separateAxes code paths where the property is uploaded and used.
There was a problem hiding this comment.
♻️ Duplicate comments (2)
packages/core/src/particle/modules/LimitVelocityOverLifetimeModule.ts (2)
79-79:⚠️ Potential issue | 🟠 MajorDefault
_dampenchange from1to0alters runtime behavior.This changes the default behavior from full damping to no damping. Per the comment on line 158, "0 means no damping, 1 means full damping." This is a functional breaking change, not just a rename. If intentional, it should be documented as a breaking change; otherwise, revert to
1.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/particle/modules/LimitVelocityOverLifetimeModule.ts` at line 79, The default value for the private property _dampen in LimitVelocityOverLifetimeModule was changed from 1 to 0 which breaks existing behavior (0 = no damping, 1 = full damping); revert the default back to 1 on the _dampen declaration in the LimitVelocityOverLifetimeModule class (or, if the change is intentional, update the module's public docs and changelog to declare this breaking change) so runtime behavior remains consistent.
215-218:⚠️ Potential issue | 🟡 MinorDocumentation for
spaceproperty is inaccurate.Line 217 states that
space"Only takes effect when 'separateAxes' is enabled," but looking at_updateShaderData()(line 299),spaceis uploaded unconditionally when the module is enabled, regardless ofseparateAxesstate.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/particle/modules/LimitVelocityOverLifetimeModule.ts` around lines 215 - 218, The doc comment for the `space` property in LimitVelocityOverLifetimeModule is incorrect about it only taking effect when `separateAxes` is enabled; update the comment to accurately reflect that `space` is used whenever the module is enabled (see `_updateShaderData()` which uploads `space` unconditionally), or alternatively change `_updateShaderData()` to only upload/apply `space` when `separateAxes` is true—use the `space` property name and `_updateShaderData()` as references when making the fix.
🧹 Nitpick comments (4)
packages/core/src/particle/modules/LimitVelocityOverLifetimeModule.ts (4)
46-51: Internal field naming inconsistency with the rename.The fields
_limitRand,_limitMinConstantVec, and_limitMaxConstantVecstill use the old "limit" naming convention while the rest of the module has been renamed to use "speed". Consider renaming these for consistency:
_limitRand→_speedRand_limitMinConstantVec→_speedMinConstantVec_limitMaxConstantVec→_speedMaxConstantVec🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/particle/modules/LimitVelocityOverLifetimeModule.ts` around lines 46 - 51, Rename the internal fields to match the "speed" naming: change _limitRand → _speedRand, _limitMinConstantVec → _speedMinConstantVec, and _limitMaxConstantVec → _speedMaxConstantVec, then update every reference in this module (constructor, methods like any initializers, serializers, clones, and usages in LimitVelocityOverLifetimeModule methods) to use the new names so compilation and behavior remain identical; ensure decorators like `@ignoreClone` remain on the renamed fields and update any comments or test references that mention the old names.
372-437: Method name inconsistency:_uploadSeparateAxisLimitsshould be_uploadSeparateAxisSpeeds.This method now uploads speed-related shader data for separate axes but retains the old "limit" naming. Consider renaming for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/particle/modules/LimitVelocityOverLifetimeModule.ts` around lines 372 - 437, The method _uploadSeparateAxisLimits is misnamed given it uploads speed shader data; rename the method to _uploadSeparateAxisSpeeds and update all internal references/calls to this method (including any unit tests or callers within LimitVelocityOverLifetimeModule and places that invoke it), keeping the implementation and returned shape unchanged and preserving references to symbols like LimitVelocityOverLifetimeModule._speedCurveModeMacro and _speedIsRandomMacro; ensure the new name is used consistently where the old name was referenced to avoid unresolved symbol errors.
268-277: Method name inconsistency:_isLimitRandomModeshould be_isSpeedRandomMode.The method now operates on
_speedX,_speedY,_speedZfields but retains the old "limit" naming. Consider renaming for consistency with the rest of the refactor.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/particle/modules/LimitVelocityOverLifetimeModule.ts` around lines 268 - 277, Rename the method _isLimitRandomMode to _isSpeedRandomMode and update all references/call sites in this class so callers use _isSpeedRandomMode; keep the body logic unchanged (it inspects _speedX/_speedY/_speedZ and ParticleCurveMode.TwoConstants/TwoCurves). Also update any tests, bindings, or other methods referring to _isLimitRandomMode to the new name to maintain consistency with the speed-related fields.
344-370: Method name inconsistency:_uploadScalarLimitshould be_uploadScalarSpeed.This method now uploads speed-related shader data but retains the old "limit" naming. Consider renaming for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/particle/modules/LimitVelocityOverLifetimeModule.ts` around lines 344 - 370, The method name _uploadScalarLimit is inconsistent with its purpose; rename it to _uploadScalarSpeed and update all internal references and callers in LimitVelocityOverLifetimeModule to use the new name (e.g., where _uploadScalarLimit(...) is invoked). Ensure the method signature and return type remain the same and keep existing references to symbols used inside (speedX, LimitVelocityOverLifetimeModule._speedCurveModeMacro, _speedIsRandomMacro, _speedMaxCurveProperty, _speedMinCurveProperty, _speedMaxConstProperty, _speedMinConstProperty) intact so only the identifier changes; run build/tests to catch any missed usages.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/core/src/particle/modules/LimitVelocityOverLifetimeModule.ts`:
- Line 79: The default value for the private property _dampen in
LimitVelocityOverLifetimeModule was changed from 1 to 0 which breaks existing
behavior (0 = no damping, 1 = full damping); revert the default back to 1 on the
_dampen declaration in the LimitVelocityOverLifetimeModule class (or, if the
change is intentional, update the module's public docs and changelog to declare
this breaking change) so runtime behavior remains consistent.
- Around line 215-218: The doc comment for the `space` property in
LimitVelocityOverLifetimeModule is incorrect about it only taking effect when
`separateAxes` is enabled; update the comment to accurately reflect that `space`
is used whenever the module is enabled (see `_updateShaderData()` which uploads
`space` unconditionally), or alternatively change `_updateShaderData()` to only
upload/apply `space` when `separateAxes` is true—use the `space` property name
and `_updateShaderData()` as references when making the fix.
---
Nitpick comments:
In `@packages/core/src/particle/modules/LimitVelocityOverLifetimeModule.ts`:
- Around line 46-51: Rename the internal fields to match the "speed" naming:
change _limitRand → _speedRand, _limitMinConstantVec → _speedMinConstantVec, and
_limitMaxConstantVec → _speedMaxConstantVec, then update every reference in this
module (constructor, methods like any initializers, serializers, clones, and
usages in LimitVelocityOverLifetimeModule methods) to use the new names so
compilation and behavior remain identical; ensure decorators like `@ignoreClone`
remain on the renamed fields and update any comments or test references that
mention the old names.
- Around line 372-437: The method _uploadSeparateAxisLimits is misnamed given it
uploads speed shader data; rename the method to _uploadSeparateAxisSpeeds and
update all internal references/calls to this method (including any unit tests or
callers within LimitVelocityOverLifetimeModule and places that invoke it),
keeping the implementation and returned shape unchanged and preserving
references to symbols like LimitVelocityOverLifetimeModule._speedCurveModeMacro
and _speedIsRandomMacro; ensure the new name is used consistently where the old
name was referenced to avoid unresolved symbol errors.
- Around line 268-277: Rename the method _isLimitRandomMode to
_isSpeedRandomMode and update all references/call sites in this class so callers
use _isSpeedRandomMode; keep the body logic unchanged (it inspects
_speedX/_speedY/_speedZ and ParticleCurveMode.TwoConstants/TwoCurves). Also
update any tests, bindings, or other methods referring to _isLimitRandomMode to
the new name to maintain consistency with the speed-related fields.
- Around line 344-370: The method name _uploadScalarLimit is inconsistent with
its purpose; rename it to _uploadScalarSpeed and update all internal references
and callers in LimitVelocityOverLifetimeModule to use the new name (e.g., where
_uploadScalarLimit(...) is invoked). Ensure the method signature and return type
remain the same and keep existing references to symbols used inside (speedX,
LimitVelocityOverLifetimeModule._speedCurveModeMacro, _speedIsRandomMacro,
_speedMaxCurveProperty, _speedMinCurveProperty, _speedMaxConstProperty,
_speedMinConstProperty) intact so only the identifier changes; run build/tests
to catch any missed usages.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7ddcd065-8340-4a75-8201-20ca270a36b6
⛔ Files ignored due to path filters (1)
packages/core/src/shaderlib/particle/limit_velocity_over_lifetime_module.glslis excluded by!**/*.glsl
📒 Files selected for processing (1)
packages/core/src/particle/modules/LimitVelocityOverLifetimeModule.ts
- Rename public API: limit/limitX/Y/Z → speed/speedX/Y/Z - Rename internal fields: _limitX/Y/Z → _speedX/Y/Z - Rename shader uniforms: LVLLimit → LVLSpeed - Rename shader macros: RENDERER_LVL_LIMIT_* → RENDERER_LVL_SPEED_* - Change default dampen from 1 to 0 - Update e2e case and tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Collaborator
|
从这次改名的目标来看，我觉得还可以再统一一下语义。 现在公共属性已经切成了 但当前实现里语义还没有完全收口：
如果这次改名的目标是把 |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
cptbtptpbcptdtptp
added
ignore for release
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…yOverLifetimeModule
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
What is the current behavior? (You can also link to an open issue here)
What is the new behavior (if this is a feature change)?
Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
Other information:
Summary by CodeRabbit
Refactor
Documentation
Tests