Fix animation time error because of speed and animatorParameter rename bug #2273
Conversation
WalkthroughThe recent updates to the animation system significantly enhance the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Animator
participant AnimatorController
User->>Animator: Trigger Animation
Animator->>AnimatorController: Check Parameter
AnimatorController->>AnimatorController: Validate Parameter Name
Animator->>Animator: Calculate Transition Time
Animator->>User: Play Animation
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev/1.3 #2273 +/- ##
===========================================
+ Coverage 68.27% 69.17% +0.90%
===========================================
Files 468 523 +55
Lines 24353 27386 +3033
Branches 3632 4099 +467
===========================================
+ Hits 16626 18945 +2319
- Misses 6400 6955 +555
- Partials 1327 1486 +159 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- packages/core/src/animation/Animator.ts (8 hunks)
- packages/core/src/animation/AnimatorController.ts (1 hunks)
- packages/core/src/animation/AnimatorControllerParameter.ts (1 hunks)
- tests/src/core/Animator.test.ts (8 hunks)
Additional context used
GitHub Check: codecov/patch
packages/core/src/animation/AnimatorControllerParameter.ts
[warning] 24-24: packages/core/src/animation/AnimatorControllerParameter.ts#L24
Added line #L24 was not covered by testspackages/core/src/animation/AnimatorController.ts
[warning] 73-74: packages/core/src/animation/AnimatorController.ts#L73-L74
Added lines #L73 - L74 were not covered by tests
Biome
packages/core/src/animation/Animator.ts
[error] 698-698: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 867-867: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
Additional comments not posted (11)
packages/core/src/animation/AnimatorControllerParameter.ts (3)
13-13: LGTM!The
_onNameChangedcallback is correctly declared as a nullable function.
15-15: LGTM!The
_namevariable is correctly declared as a private string variable.
18-29: LGTM! But verify the references toname.The getter and setter for
nameare well-implemented, ensuring encapsulation and controlled updates with a callback mechanism.However, ensure that all references to
namein the codebase are updated correctly.Tools
GitHub Check: codecov/patch
[warning] 24-24: packages/core/src/animation/AnimatorControllerParameter.ts#L24
Added line #L24 was not covered by testspackages/core/src/animation/AnimatorController.ts (1)
72-75: LGTM! But verify the_parametersMapupdates.The
_onNameChangedfunction is well-implemented, ensuring the parameter map remains consistent when a name changes.However, ensure that the
_parametersMapupdates correctly in all scenarios.Verification successful
Verified: The
_parametersMapupdates are handled correctly.The
_parametersMapis consistently used for managing parameters, including adding, deleting, and retrieving them by name.
packages/core/src/animation/AnimatorController.ts: Lines involving_parametersMapmanagement.packages/core/src/animation/Animator.ts: Lines involving_parametersMapaccess.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the `_parametersMap` updates correctly in all scenarios. # Test: Search for references to `_parametersMap`. Expect: Correct handling of name changes. rg --type typescript -A 5 $'_parametersMap'Length of output: 77
Script:
#!/bin/bash # Description: Verify the `_parametersMap` updates correctly in all scenarios. # Test: Search for references to `_parametersMap`. Expect: Correct handling of name changes. rg --glob '*.ts' -A 5 '_parametersMap'Length of output: 3334
Tools
GitHub Check: codecov/patch
[warning] 73-74: packages/core/src/animation/AnimatorController.ts#L73-L74
Added lines #L73 - L74 were not covered by teststests/src/core/Animator.test.ts (5)
328-328: LGTM!The
idleSpeedvariable is correctly declared and set to 2.
329-329: LGTM!The
idleState.speedis correctly set toidleSpeed.
349-351: LGTM!The
idleToWalkTimecalculation is correctly adjusted to incorporateidleSpeed.
377-377: LGTM!The
walkToIdleTimecalculation is correctly adjusted to incorporateidleSpeed.
402-402: LGTM!The
anyToIdleTimecalculation is correctly adjusted to incorporateidleSpeed.packages/core/src/animation/Animator.ts (2)
26-26: Import statement forMathUtilis necessary.The import of
MathUtilis required for the updated cross-fade weight calculations.
610-612: Refined calculation ofremainDeltaTime.The adjustment to incorporate
actualSpeedensures more accurate timing during state transitions.
| @@ -863,7 +864,7 @@ export class Animator extends Component { | |||
|
|
|||
| const duration = state._getDuration() * layerData.crossFadeTransition.duration; | |||
| let crossWeight = Math.abs(destPlayData.frameTime) / duration; | |||
| (crossWeight >= 1.0 || duration === 0) && (crossWeight = 1.0); | |||
| (crossWeight >= 1.0 - MathUtil.zeroTolerance || duration === 0) && (crossWeight = 1.0); | |||
There was a problem hiding this comment.
Improved precision in cross-fade calculations.
The update to use MathUtil.zeroTolerance enhances the precision of cross-fade weight calculations.
However, avoid assignments within expressions for better readability.
- (crossWeight >= 1.0 - MathUtil.zeroTolerance || duration === 0) && (crossWeight = 1.0);
+ if (crossWeight >= 1.0 - MathUtil.zeroTolerance || duration === 0) {
+ crossWeight = 1.0;
+ }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.
| (crossWeight >= 1.0 - MathUtil.zeroTolerance || duration === 0) && (crossWeight = 1.0); | |
| if (crossWeight >= 1.0 - MathUtil.zeroTolerance || duration === 0) { | |
| crossWeight = 1.0; | |
| } |
Tools
Biome
[error] 867-867: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- packages/core/src/animation/Animator.ts (10 hunks)
- packages/core/src/animation/AnimatorControllerParameter.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/core/src/animation/AnimatorControllerParameter.ts
Additional comments not posted (6)
packages/core/src/animation/Animator.ts (6)
26-26: Import statement forMathUtilis approved.The import of
MathUtilis required for precision improvements in the animation calculations.
724-724: Verify handling of updated parameters in_fireAnimationEventsAndCallScripts.Ensure that the updated parameters are correctly handled in the function implementation.
730-730: Enhanced precision in remaining delta time calculation.The adjustment to incorporate
costTimeensures more accurate timing when switching layer states.However, ensure that all function calls to
_updateStatecorrectly handle the newremainDeltaTimelogic.
715-715: Verify handling of updated parameters in_fireAnimationEventsAndCallScripts.Ensure that the updated parameters are correctly handled in the function implementation.
Verification successful
Verified handling of updated parameters in
_fireAnimationEventsAndCallScripts.The function
_fireAnimationEventsAndCallScriptscorrectly handles the updated parameters.
packages/core/src/animation/Animator.tslines 715 and the function implementation.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of `_fireAnimationEventsAndCallScripts` to ensure correct handling of updated parameters. # Test: Search for the function implementation. Expect: Correct handling of updated parameters. rg --type ts -A 20 $'_fireAnimationEventsAndCallScripts'Length of output: 6716
610-611: Enhanced precision in remaining delta time calculation.The adjustment to incorporate
actualSpeedensures more accurate timing when switching layer states.However, ensure that all function calls to
_updateStatecorrectly handle the newremainDeltaTimelogic.
706-706: Improved precision in cross-fade weight calculations.The update to use
crossWeightenhances the precision of cross-fade calculations.However, ensure that all function calls to
_evaluateCrossFadeStatecorrectly handle the newcrossWeightparameter.Verification successful
Improved precision in cross-fade weight calculations.
The update to use
crossWeightenhances the precision of cross-fade calculations. All function calls to_evaluateCrossFadeStatecorrectly handle the newcrossWeightparameter.
packages/core/src/animation/Animator.ts: The function call and definition both correctly include thecrossWeightparameter.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `_evaluateCrossFadeState` correctly handle the new `crossWeight` parameter. # Test: Search for the function usage. Expect: Correct handling of the new `crossWeight` parameter. rg --type ts -A 5 $'_evaluateCrossFadeState'Length of output: 926
Script:
#!/bin/bash # Description: Verify all instances of `_evaluateCrossFadeState` in the codebase to ensure the `crossWeight` parameter is correctly handled. # Test: Search for all instances of `_evaluateCrossFadeState` in the TypeScript files. ast-grep --lang typescript --pattern '_evaluateCrossFadeState($_$$$)'Length of output: 71
Script:
#!/bin/bash # Description: Verify all instances of `_evaluateCrossFadeState` in the codebase to ensure the `crossWeight` parameter is correctly handled. # Test: Search for all instances of `_evaluateCrossFadeState` in the TypeScript files. rg --type ts '_evaluateCrossFadeState'Length of output: 269
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/core/src/animation/Animator.ts (10 hunks)
Additional context used
Biome
packages/core/src/animation/Animator.ts
[error] 698-698: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 822-822: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
Additional comments not posted (4)
packages/core/src/animation/Animator.ts (4)
26-26: Import addition approved.The
MathUtilimport is necessary for precision improvements in cross-fade calculations.
610-611: Change approved.The calculation and conditional check for
remainDeltaTimeensure that any remaining delta time is processed correctly.
Line range hint
706-715: Change approved.The addition of the
crossWeightparameter to_evaluateCrossFadeStateimproves the precision of cross-fade calculations.Tools
Biome
[error] 698-698: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
Line range hint
730-739: Change approved.The calculation and conditional check for
remainDeltaTimeensure that any remaining delta time is processed correctly.
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range, codebase verification and nitpick comments (3)
packages/core/src/animation/Animator.ts (3)
540-541: Enhance readability by renaming variables.Consider renaming
playSpeedtoadjustedPlaySpeedfor clarity.- const playSpeed = state.speed * this.speed; + const adjustedPlaySpeed = state.speed * this.speed;
663-665: Enhance readability by renaming variables.Consider renaming
srcPlaySpeedanddstPlaySpeedtosourcePlaySpeedanddestinationPlaySpeedfor clarity.- const srcPlaySpeed = srcState.speed * speed; - const dstPlaySpeed = destState.speed * speed; + const sourcePlaySpeed = srcState.speed * speed; + const destinationPlaySpeed = destState.speed * speed;
792-793: Enhance readability by renaming variables.Consider renaming
playSpeedtoadjustedPlaySpeedfor clarity.- const playSpeed = state.speed * this.speed; + const adjustedPlaySpeed = state.speed * this.speed;
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/core/src/animation/Animator.ts (12 hunks)
Additional context used
Biome
packages/core/src/animation/Animator.ts
[error] 667-667: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 668-668: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 699-699: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 821-821: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
Additional comments not posted (5)
packages/core/src/animation/Animator.ts (5)
548-548: Ensure accurate transition application.Verify that the transition logic correctly handles edge cases for timing.
740-740: Ensure crossWeight is within valid range.Verify that
crossWeightis always within the range [0, 1] before using it in calculations.
818-818: Ensure crossWeight is within valid range.Verify that
crossWeightis always within the range [0, 1] before using it in calculations.
852-852: Ensure crossWeight is within valid range.Verify that
crossWeightis always within the range [0, 1] before using it in calculations.
1318-1318: Ensure accurate event firing.Verify that the event firing logic correctly handles edge cases for timing.
| srcPlayData && srcPlayData.updateOrientation(srcPlaySpeed * deltaTime); | ||
| destPlayData && destPlayData.updateOrientation(dstPlayDeltaTime); |
There was a problem hiding this comment.
Use optional chaining for safety.
The static analysis tool suggests using optional chaining for better safety.
- srcPlayData && srcPlayData.updateOrientation(srcPlaySpeed * deltaTime);
- destPlayData && destPlayData.updateOrientation(dstPlayDeltaTime);
+ srcPlayData?.updateOrientation(srcPlaySpeed * deltaTime);
+ destPlayData?.updateOrientation(dstPlayDeltaTime);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.
| srcPlayData && srcPlayData.updateOrientation(srcPlaySpeed * deltaTime); | |
| destPlayData && destPlayData.updateOrientation(dstPlayDeltaTime); | |
| srcPlayData?.updateOrientation(srcPlaySpeed * deltaTime); | |
| destPlayData?.updateOrientation(dstPlayDeltaTime); |
Tools
Biome
[error] 667-667: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 668-668: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
| this._evaluatePlayingState(srcPlayData, weight, additive, aniUpdate); | ||
| this._fireAnimationEventsAndCallScripts(layerIndex, srcPlayData, state, lastClipTime, lastPlayState, costTime); | ||
| this._fireAnimationEventsAndCallScripts(layerIndex, srcPlayData, state, lastClipTime, lastPlayState, playCostTime); | ||
|
|
||
| if (needSwitchLayerState) { |
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/core/src/animation/Animator.ts (11 hunks)
Additional context used
Biome
packages/core/src/animation/Animator.ts
[error] 664-664: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 665-665: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 696-696: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 818-818: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
Additional comments not posted (6)
packages/core/src/animation/Animator.ts (6)
26-26: LGTM! The import statement is correct.The
MathUtilimport is necessary for the changes in the file.
540-548: LGTM! The changes improve precision in animation timing.The use of
playSpeedandplayDeltaTimeenhances the accuracy of animation state updates.
670-693: LGTM! The changes improve precision in cross-fade calculations.The use of
srcPlaySpeed,dstPlaySpeed, anddstPlayDeltaTimeenhances the accuracy of cross-fade state updates.
737-737: LGTM! The addition of thecrossWeightparameter improves precision.Including
crossWeightenhances the accuracy of cross-fade calculations.
789-792: LGTM! The changes improve precision in cross-fade from pose state calculations.The use of
playSpeedandplayDeltaTimeenhances the accuracy of cross-fade from pose state updates.
849-849: LGTM! The addition of thecrossWeightparameter improves precision.Including
crossWeightenhances the accuracy of cross-fade from pose state calculations.
| const remainDeltaTime = deltaTime - costTime; | ||
| this._updateState(layerIndex, layerData, layer, remainDeltaTime, aniUpdate); | ||
| if (transition) { | ||
| // actualCostTime = playCostTime / playSpeed |
There was a problem hiding this comment.
// Remove speed factor, use actual time
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- packages/core/src/animation/Animator.ts (12 hunks)
- packages/core/src/animation/AnimatorController.ts (1 hunks)
- packages/core/src/animation/AnimatorControllerParameter.ts (1 hunks)
- tests/src/core/Animator.test.ts (9 hunks)
Files skipped from review as they are similar to previous changes (3)
- packages/core/src/animation/AnimatorController.ts
- packages/core/src/animation/AnimatorControllerParameter.ts
- tests/src/core/Animator.test.ts
Additional context used
Biome
packages/core/src/animation/Animator.ts
[error] 664-664: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 665-665: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 696-696: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 818-818: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
Additional comments not posted (7)
packages/core/src/animation/Animator.ts (7)
26-26: Import addition approved.The addition of
MathUtilimport is necessary for the subsequent changes in the file.
540-548: Playback speed calculation approved.The changes ensure that the playback speed is correctly factored into the delta time.
573-600: Transition timing calculation approved.The changes ensure that the animation transitions are more accurately timed.
606-609: Remaining delta time calculation approved.The changes ensure that any remaining time is properly accounted for.
660-665: Use optional chaining for safety.The use of optional chaining improves safety and readability.
- srcPlayData && srcPlayData.updateOrientation(srcPlaySpeed * deltaTime); - destPlayData && destPlayData.updateOrientation(dstPlayDeltaTime); + srcPlayData?.updateOrientation(srcPlaySpeed * deltaTime); + destPlayData?.updateOrientation(dstPlayDeltaTime);Tools
Biome
[error] 664-664: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 665-665: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
789-818: Cross-fade logic and precision improvements approved.The changes ensure accurate calculation of
playDeltaTime, proper updating ofdestPlayData, and improved precision in cross-fade logic usingMathUtil.zeroTolerance.Tools
Biome
[error] 818-818: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
849-849: Function signature update approved.The addition of
crossWeightto the function signature is necessary for the updated cross-fade logic.
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- packages/core/src/animation/Animator.ts (12 hunks)
- packages/core/src/animation/AnimatorController.ts (1 hunks)
- packages/loader/src/AnimatorControllerLoader.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/core/src/animation/AnimatorController.ts
Additional context used
Biome
packages/core/src/animation/Animator.ts
[error] 664-664: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 665-665: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 696-696: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 818-818: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
Additional comments not posted (6)
packages/loader/src/AnimatorControllerLoader.ts (1)
103-103: LGTM! Simplified parameter addition process.The change improves code readability and efficiency by directly calling
addParameterwithout instantiatingAnimatorControllerParameter.However, ensure that all usages of
addParameterare consistent with this change.Verification successful
The changes to
addParameterare consistent across the codebase.All usages of
addParametermatch the new method signature, confirming that the changes are correctly integrated.
tests/src/core/Animator.test.tspackages/core/src/animation/AnimatorController.tspackages/loader/src/AnimatorControllerLoader.tse2e/case/animator-stateMachine.tsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all usages of `addParameter` to ensure consistency with the new method signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type typescript -A 5 $'addParameter'Length of output: 75
Script:
#!/bin/bash # Description: Verify all usages of `addParameter` to ensure consistency with the new method signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg -A 5 $'addParameter'Length of output: 3567
packages/core/src/animation/Animator.ts (5)
540-548: Improved precision in animation timing.The update to use
playDeltaTimeenhances the precision of animation timing.
737-737: Enhanced precision in cross-fade calculations.The addition of
crossWeightimproves the precision of cross-fade calculations.
789-792: Improved precision in animation timing.The update to use
playDeltaTimeenhances the precision of animation timing.
849-849: Enhanced precision in cross-fade calculations.The addition of
crossWeightimproves the precision of cross-fade calculations.
1315-1323: Simplified animation event firing logic.The removal of the speed factor and the use of
deltaTimedirectly simplifies the logic and reduces potential sources of error.
|
|
||
| let crossWeight = Math.abs(destPlayData.frameTime) / transitionDuration; | ||
| (crossWeight >= 1.0 || transitionDuration === 0) && (crossWeight = 1.0); | ||
| (crossWeight >= 1.0 - MathUtil.zeroTolerance || transitionDuration === 0) && (crossWeight = 1.0); |
There was a problem hiding this comment.
Avoid assignments within expressions.
The use of assignments within expressions can be confusing. Refactor the code for better readability.
- (crossWeight >= 1.0 - MathUtil.zeroTolerance || transitionDuration === 0) && (crossWeight = 1.0);
+ if (crossWeight >= 1.0 - MathUtil.zeroTolerance || transitionDuration === 0) {
+ crossWeight = 1.0;
+ }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.
| (crossWeight >= 1.0 - MathUtil.zeroTolerance || transitionDuration === 0) && (crossWeight = 1.0); | |
| if (crossWeight >= 1.0 - MathUtil.zeroTolerance || transitionDuration === 0) { | |
| crossWeight = 1.0; | |
| } |
Tools
Biome
[error] 696-696: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
|
|
||
| let crossWeight = Math.abs(destPlayData.frameTime) / transitionDuration; | ||
| (crossWeight >= 1.0 || transitionDuration === 0) && (crossWeight = 1.0); | ||
| (crossWeight >= 1.0 - MathUtil.zeroTolerance || transitionDuration === 0) && (crossWeight = 1.0); |
There was a problem hiding this comment.
Avoid assignments within expressions.
The use of assignments within expressions can be confusing. Refactor the code for better readability.
- (crossWeight >= 1.0 - MathUtil.zeroTolerance || transitionDuration === 0) && (crossWeight = 1.0);
+ if (crossWeight >= 1.0 - MathUtil.zeroTolerance || transitionDuration === 0) {
+ crossWeight = 1.0;
+ }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.
| (crossWeight >= 1.0 - MathUtil.zeroTolerance || transitionDuration === 0) && (crossWeight = 1.0); | |
| if (crossWeight >= 1.0 - MathUtil.zeroTolerance || transitionDuration === 0) { | |
| crossWeight = 1.0; | |
| } |
Tools
Biome
[error] 818-818: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
| const srcPlaySpeed = srcState.speed * speed; | ||
| const dstPlaySpeed = destState.speed * speed; | ||
| const dstPlayDeltaTime = dstPlaySpeed * deltaTime; | ||
|
|
||
| srcPlayData && srcPlayData.updateOrientation(actualSrcSpeed * deltaTime); | ||
| destPlayData && destPlayData.updateOrientation(actualDestDeltaTime); | ||
| srcPlayData && srcPlayData.updateOrientation(srcPlaySpeed * deltaTime); | ||
| destPlayData && destPlayData.updateOrientation(dstPlayDeltaTime); |
There was a problem hiding this comment.
Use optional chaining for safety.
The static analysis tool suggests using optional chaining for better safety.
- srcPlayData && srcPlayData.updateOrientation(srcPlaySpeed * deltaTime);
- destPlayData && destPlayData.updateOrientation(dstPlayDeltaTime);
+ srcPlayData?.updateOrientation(srcPlaySpeed * deltaTime);
+ destPlayData?.updateOrientation(dstPlayDeltaTime);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.
| const srcPlaySpeed = srcState.speed * speed; | |
| const dstPlaySpeed = destState.speed * speed; | |
| const dstPlayDeltaTime = dstPlaySpeed * deltaTime; | |
| srcPlayData && srcPlayData.updateOrientation(actualSrcSpeed * deltaTime); | |
| destPlayData && destPlayData.updateOrientation(actualDestDeltaTime); | |
| srcPlayData && srcPlayData.updateOrientation(srcPlaySpeed * deltaTime); | |
| destPlayData && destPlayData.updateOrientation(dstPlayDeltaTime); | |
| const srcPlaySpeed = srcState.speed * speed; | |
| const dstPlaySpeed = destState.speed * speed; | |
| const dstPlayDeltaTime = dstPlaySpeed * deltaTime; | |
| srcPlayData?.updateOrientation(srcPlaySpeed * deltaTime); | |
| destPlayData?.updateOrientation(dstPlayDeltaTime); |
Tools
Biome
[error] 664-664: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 665-665: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
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
New Features
Bug Fixes