Fix evaluate value error when switch layer state && get target value error in stateMachine callback and animation event #2258
Conversation
luzhuang
added
bug
|
Warning Rate limit exceeded@luzhuang has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 42 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe recent updates enhance the Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant Engine
participant AnimatorController
Developer->>Engine: Create instance of Engine
Developer->>AnimatorController: Instantiate AnimatorController(engine)
Engine->>AnimatorController: Provide engine context
AnimatorController->>AnimationSystem: Manage animations with engine capabilities
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (2)
docs/zh/animation/animatorController.md (1)
138-138: Clarify the need for theengineparameter.The documentation should explain why the
AnimatorControllernow requires anengineparameter for instantiation. This will help developers understand the change and its implications.animator.animatorController = new AnimatorController(engine);docs/en/animation/animatorController.md (1)
140-140: Clarify the need for theengineparameter.The documentation should explain why the
AnimatorControllernow requires anengineparameter for instantiation. This will help developers understand the change and its implications.animator.animatorController = new AnimatorController(engine);
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (10)
- docs/en/animation/animatorController.md (1 hunks)
- docs/zh/animation/animatorController.md (1 hunks)
- e2e/case/animator-customAnimationClip.ts (1 hunks)
- e2e/case/animator-customBlendShape.ts (1 hunks)
- e2e/case/animator-multiSubMeshBlendShape.ts (1 hunks)
- examples/animation-customAnimationClip.ts (3 hunks)
- examples/animation-sprite.ts (5 hunks)
- packages/core/src/animation/AnimatorController.ts (2 hunks)
- packages/loader/src/AnimatorControllerLoader.ts (1 hunks)
- packages/loader/src/gltf/parser/GLTFAnimatorControllerParser.ts (1 hunks)
Files skipped from review due to trivial changes (2)
- examples/animation-customAnimationClip.ts
- examples/animation-sprite.ts
Additional comments not posted (9)
packages/loader/src/gltf/parser/GLTFAnimatorControllerParser.ts (2)
18-18: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
_createAnimatorControllermatch the new signature.Verification successful
Function usage verified and correct.
The
_createAnimatorControllerfunction is called with the appropriate parameters (contextandanimations) as per the new signature.
packages/loader/src/gltf/parser/GLTFAnimatorControllerParser.tsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `_createAnimatorController` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type typescript -A 5 $'_createAnimatorController'Length of output: 88
Script:
#!/bin/bash # Description: Verify all function calls to `_createAnimatorController` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --glob '*.ts' -A 5 '_createAnimatorController'Length of output: 1275
23-26: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
_createAnimatorControllermatch the new signature.Verification successful
Function usage verified.
The
_createAnimatorControllermethod is used correctly with the new signature within theGLTFAnimatorControllerParser.tsfile.
packages/loader/src/gltf/parser/GLTFAnimatorControllerParser.ts: Lines 23-26Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `_createAnimatorController` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type typescript -A 5 $'_createAnimatorController'Length of output: 88
Script:
#!/bin/bash # Description: Verify all function calls to `_createAnimatorController` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type ts -A 5 $'_createAnimatorController'Length of output: 1271
e2e/case/animator-customBlendShape.ts (1)
75-75: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all instantiations of
AnimatorControlleruse theengineparameter.Verification successful
All instances of
AnimatorControllercorrectly use theengineparameter.
- examples/animation-sprite.ts
- examples/animation-customAnimationClip.ts
- packages/loader/src/AnimatorControllerLoader.ts
- packages/loader/src/gltf/parser/GLTFAnimatorControllerParser.ts
- e2e/case/animator-customBlendShape.ts
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all instantiations of `AnimatorController` use the `engine` parameter. # Test: Search for the instantiation. Expect: Only occurrences with the `engine` parameter. rg --type typescript -A 5 $'new AnimatorController'Length of output: 87
Script:
#!/bin/bash # Description: Verify all instantiations of `AnimatorController` use the `engine` parameter. # Test: Search for the instantiation. Expect: Only occurrences with the `engine` parameter. rg --type ts -A 5 $'new AnimatorController'Length of output: 8678
e2e/case/animator-multiSubMeshBlendShape.ts (1)
52-52: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all instantiations of
AnimatorControlleruse theengineparameter.Verification successful
All instantiations of
AnimatorControllercorrectly use theengineparameter.The code changes are consistent across the codebase.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all instantiations of `AnimatorController` use the `engine` parameter. # Test: Search for the instantiation. Expect: Only occurrences with the `engine` parameter. rg --type typescript -A 5 $'new AnimatorController'Length of output: 87
Script:
#!/bin/bash # Description: Verify all instantiations of `AnimatorController` use the `engine` parameter. # Test: Search for the instantiation. Expect: Only occurrences with the `engine` parameter. rg --glob '*.ts' -A 5 'new AnimatorController'Length of output: 8682
packages/core/src/animation/AnimatorController.ts (3)
5-6: Imports verified.The
ReferResourceandEnginemodules are necessary for the new functionality.
11-11: Class extension verified.The
AnimatorControllerclass correctly extendsReferResource.
37-39: Constructor implementation verified.The constructor correctly initializes the class and calls the superclass constructor if an engine is provided.
packages/loader/src/AnimatorControllerLoader.ts (1)
27-27: Instantiation verified.The
AnimatorControlleris correctly instantiated withresourceManager.engine.e2e/case/animator-customAnimationClip.ts (1)
71-71: Instantiation verified.The
AnimatorControlleris correctly instantiated withengine.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- packages/core/src/animation/Animator.ts (23 hunks)
- packages/core/src/animation/internal/AnimatorStatePlayData.ts (1 hunks)
- tests/src/core/Animator.test.ts (3 hunks)
Additional comments not posted (35)
packages/core/src/animation/internal/AnimatorStatePlayData.ts (1)
52-52: Ensure the change in the condition is intentional.The condition has been changed from
Math.abs(time) > durationtoMath.abs(time) >= duration, which means that the state will now be marked asFinishedwhentimeis exactly equal toduration. Verify that this change is intentional and does not introduce any unintended behavior.tests/src/core/Animator.test.ts (2)
Line range hint
2-17:
Imports look good.The new imports are necessary for the added test case.
560-602: New test case implementation looks good.The new test case "change state in one update" is well-implemented and verifies the functionality of state transitions within the animator system. It sets up an
AnimatorControllerwith two states and corresponding animation clips, and checks the correct application of rotation transformations over time.packages/core/src/animation/Animator.ts (32)
481-481: Method signature update looks good.The method signature of
_updateStatehas been updated to replace thestateMachineparameter withlayerand add a new boolean parameteraniUpdate.
507-509: Method signature update looks good.The method signature of
_updateStatehas been updated to replace thestateMachineparameter withlayerand add a new boolean parameteraniUpdate.
513-525: Method calls updated to reflect new parameters.The calls to various methods within
_updateStatehave been updated to reflect the new parameters.
534-536: Method signature update looks good.The method signature of
_updatePlayingStatehas been updated to replace thestateMachineparameter withlayerand add a new boolean parameteraniUpdate.
Line range hint
539-563:
Method calls updated to reflect new parameters.The calls to various methods within
_updatePlayingStatehave been updated to reflect the new parameters.
567-574: Method calls updated to reflect new parameters.The calls to various methods within
_applyStateTransitionshave been updated to reflect the new parameters.
Line range hint
602-632:
Ensure logic correctness for state transitions.The logic for determining
needChangeStateand updating the state has been modified. Ensure that this change is intentional and does not introduce any unintended behavior.
670-672: Method signature update looks good.The method signature of
_updateCrossFadeStatehas been updated to replace thestateMachineparameter withlayerand add a new boolean parameteraniUpdate.
718-722: Ensure logic correctness for cross-fade state transitions.The logic for determining
needChangeStateand updating the state has been modified. Ensure that this change is intentional and does not introduce any unintended behavior.
811-813: Method signature update looks good.The method signature of
_updateCrossFadeFromPoseStatehas been updated to replace thestateMachineparameter withlayerand add a new boolean parameteraniUpdate.
853-856: Ensure logic correctness for cross-fade from pose state transitions.The logic for determining
needChangeStateand updating the state has been modified. Ensure that this change is intentional and does not introduce any unintended behavior.
931-933: Method signature update looks good.The method signature of
_updateFinishedStatehas been updated to replace thestateMachineparameter withlayerand add a new boolean parameteraniUpdate.
946-953: Method calls updated to reflect new parameters.The calls to various methods within
_updateFinishedStatehave been updated to reflect the new parameters.
957-964: Method calls updated to reflect new parameters.The calls to various methods within
_applyStateTransitionshave been updated to reflect the new parameters.
1034-1041: Method signature update looks good.The method signature of
_applyStateTransitionshas been updated to replace thestateMachineparameter withlayerand add a new boolean parameteraniUpdate.
1053-1058: Method calls updated to reflect new parameters.The calls to various methods within
_applyStateTransitionshave been updated to reflect the new parameters.
1065-1070: Method calls updated to reflect new parameters.The calls to various methods within
_applyStateTransitionshave been updated to reflect the new parameters.
1077-1082: Method calls updated to reflect new parameters.The calls to various methods within
_applyStateTransitionshave been updated to reflect the new parameters.
1090-1095: Method calls updated to reflect new parameters.The calls to various methods within
_applyStateTransitionshave been updated to reflect the new parameters.
1102-1107: Method calls updated to reflect new parameters.The calls to various methods within
_applyStateTransitionshave been updated to reflect the new parameters.
1114-1119: Method calls updated to reflect new parameters.The calls to various methods within
_applyStateTransitionshave been updated to reflect the new parameters.
1130-1135: Method signature update looks good.The method signature of
_checkSubTransitionhas been updated to replace thestateMachineparameter withlayerand add a new boolean parameteraniUpdate.
1150-1150: Method calls updated to reflect new parameters.The calls to various methods within
_checkSubTransitionhave been updated to reflect the new parameters.
1164-1169: Method signature update looks good.The method signature of
_checkBackwardsSubTransitionhas been updated to replace thestateMachineparameter withlayerand add a new boolean parameteraniUpdate.
1184-1184: Method calls updated to reflect new parameters.The calls to various methods within
_checkBackwardsSubTransitionhave been updated to reflect the new parameters.
1198-1201: Method signature update looks good.The method signature of
_applyTransitionsByConditionhas been updated to replace thestateMachineparameter withlayerand add a new boolean parameteraniUpdate.
1206-1206: Method calls updated to reflect new parameters.The calls to various methods within
_applyTransitionsByConditionhave been updated to reflect the new parameters.
1236-1238: Method signature update looks good.The method signature of
_applyTransitionhas been updated to replace thestateMachineparameter withlayerand add a new boolean parameteraniUpdate.
1243-1243: Method calls updated to reflect new parameters.The calls to various methods within
_applyTransitionhave been updated to reflect the new parameters.
1455-1457: Method signature update looks good.The method signature of
_checkAnyAndEntryStatehas been updated to replace thestateMachineparameter withlayerand add a new boolean parameteraniUpdate.
1465-1470: Method calls updated to reflect new parameters.The calls to various methods within
_checkAnyAndEntryStatehave been updated to reflect the new parameters.
1474-1479: Method calls updated to reflect new parameters.The calls to various methods within
_checkAnyAndEntryStatehave been updated to reflect the new parameters.
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 (22 hunks)
- tests/src/core/Animator.test.ts (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- tests/src/core/Animator.test.ts
Additional context used
Biome
packages/core/src/animation/Animator.ts
[error] 758-758: 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] 908-908: 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)
482-482: Verify the correctness of the new parameter usage.The new parameter
aniUpdateis added to control whether certain updates should occur. Ensure that its usage is consistent and correct throughout the function.
540-542: Verify the correctness of the new parameter usage.The new parameter
aniUpdateis added to control whether certain updates should occur. Ensure that its usage is consistent and correct throughout the function.
567-569: Verify the correctness of the new parameter usage.The new parameter
aniUpdateis added to control whether certain updates should occur. Ensure that its usage is consistent and correct throughout the function.
1258-1261: Verify the correctness of the new parameter usage.The new parameter
aniUpdateis added to control whether certain updates should occur. Ensure that its usage is consistent and correct throughout the function.
713-715: Verify the correctness of the new parameter usage.The new parameter
aniUpdateis added to control whether certain updates should occur. Ensure that its usage is consistent and correct throughout the function.
997-999: Verify the correctness of the new parameter usage.The new parameter
aniUpdateis added to control whether certain updates should occur. Ensure that its usage is consistent and correct throughout the function.
1514-1516: Verify the correctness of the new parameter usage.The new parameter
aniUpdateis added to control whether certain updates should occur. Ensure that its usage is consistent and correct throughout the function.
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
packages/core/src/animation/Animator.ts (1)
1251-1261: Function calls to_applyTransitionsByConditiondo not match the new signature.The following calls to
_applyTransitionsByConditionare missing theaniUpdateparameter:
packages/core/src/animation/Animator.ts:1251packages/core/src/animation/Animator.ts:1257Please update these calls to include the
aniUpdateparameter.Analysis chain
LGTM! But verify the function usage in the codebase.
The changes to the
_applyTransitionsByConditionfunction are approved.However, ensure that all function calls to
_applyTransitionsByConditionmatch the new signature.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `_applyTransitionsByCondition` match the new signature. # Test: Search for the function usage. Expect: Only occurances of the new signature. rg --type python -A 5 $'_applyTransitionsByCondition'Length of output: 83
Script:
#!/bin/bash # Description: Verify all function calls to `_applyTransitionsByCondition` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type ts -A 5 '_applyTransitionsByCondition'Length of output: 2155
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/core/src/animation/Animator.ts (23 hunks)
Additional context used
Biome
packages/core/src/animation/Animator.ts
[error] 747-747: 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] 900-900: 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)
Line range hint
553-648:
LGTM! But verify the function usage in the codebase.The changes to the
_updatePlayingStatefunction are approved.However, ensure that all function calls to
_updatePlayingStatematch the new signature.Verification successful
All function calls to
_updatePlayingStatematch the new signature.
- The function is called with the parameters
layerIndex,layerData,layer,deltaTime, andaniUpdateas expected.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `_updatePlayingState` match the new signature. # Test: Search for the function usage. Expect: Only occurances of the new signature. rg --type python -A 5 $'_updatePlayingState'Length of output: 74
Script:
#!/bin/bash # Description: Verify all function calls to `_updatePlayingState` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type ts -A 5 '_updatePlayingState'Length of output: 963
Line range hint
697-785:
LGTM! But verify the function usage in the codebase.The changes to the
_updateCrossFadeStatefunction are approved.However, ensure that all function calls to
_updateCrossFadeStatematch the new signature.Verification successful
Function calls to
_updateCrossFadeStatematch the new signature.The changes to the
_updateCrossFadeStatefunction are correctly reflected in the codebase.
packages/core/src/animation/Animator.ts:
- Line 738:
this._updateCrossFadeState(layerIndex, layerData, layer, deltaTime, aniUpdate);- Line 799: Function definition with the new signature.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `_updateCrossFadeState` match the new signature. # Test: Search for the function usage. Expect: Only occurances of the new signature. rg --type python -A 5 $'_updateCrossFadeState'Length of output: 76
Script:
#!/bin/bash # Description: Verify all function calls to `_updateCrossFadeState` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type ts -A 5 '_updateCrossFadeState'Length of output: 961
Tools
Biome
[error] 747-747: 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
479-515:
LGTM! But verify the function usage in the codebase.The changes to the
_updateLayerfunction are approved.However, ensure that all function calls to
_updateLayermatch the new signature.Verification successful
Verified: The function
_updateLayeris correctly called with the new signature in the codebase.
- The function call in
packages/core/src/animation/Animator.tsmatches the updated signature, including theaniUpdateparameter.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `_updateLayer` match the new signature. # Test: Search for the function usage. Expect: Only occurances of the new signature. rg --type python -A 5 $'_updateLayer'Length of output: 67
Script:
#!/bin/bash # Description: Verify all function calls to `_updateLayer` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type typescript -A 5 '_updateLayer'Length of output: 75
Script:
#!/bin/bash # Description: Verify all function calls to `_updateLayer` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg '_updateLayer' -A 5Length of output: 786
Line range hint
526-548:
LGTM! But verify the function usage in the codebase.The changes to the
_updateStatefunction are approved.However, ensure that all function calls to
_updateStatematch the new signature.Verification successful
Function usage verified
All calls to
_updateStatematch the new function signature.
packages/core/src/animation/Animator.ts: All instances of_updateStatehave been updated correctly.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `_updateState` match the new signature. # Test: Search for the function usage. Expect: Only occurances of the new signature. rg --type python -A 5 $'_updateState'Length of output: 67
Script:
#!/bin/bash # Description: Verify all function calls to `_updateState` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type typescript -A 5 '_updateState'Length of output: 75
Script:
#!/bin/bash # Description: Verify all function calls to `_updateState` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type ts -A 5 '_updateState'Length of output: 3459
990-1027: LGTM! But verify the function usage in the codebase.The changes to the
_updateFinishedStatefunction are approved.However, ensure that all function calls to
_updateFinishedStatematch the new signature.Verification successful
All function calls to
_updateFinishedStatematch the new signature.The changes to the
_updateFinishedStatefunction are correctly implemented and consistent throughout the codebase.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `_updateFinishedState` match the new signature. # Test: Search for the function usage. Expect: Only occurances of the new signature. rg --type python -A 5 $'_updateFinishedState'Length of output: 75
Script:
#!/bin/bash # Description: Verify all function calls to `_updateFinishedState` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type js -A 5 '_updateFinishedState'Length of output: 40
Script:
#!/bin/bash # Description: Verify all function calls to `_updateFinishedState` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type js,ts -A 5 '_updateFinishedState'Length of output: 73
Script:
#!/bin/bash # Description: Verify all function calls to `_updateFinishedState` match the new signature. # Test: Search for the function usage in JavaScript files. Expect: Only occurrences of the new signature. rg --type js -A 5 '_updateFinishedState' # Test: Search for the function usage in TypeScript files. Expect: Only occurrences of the new signature. rg --type ts -A 5 '_updateFinishedState'Length of output: 1018
Line range hint
1087-1172:
LGTM! But verify the function usage in the codebase.The changes to the
_applyStateTransitionsfunction are approved.However, ensure that all function calls to
_applyStateTransitionsmatch the new signature.Verification successful
All calls to
_applyStateTransitionsmatch the new signature.The changes to the
_applyStateTransitionsfunction are correctly reflected in all instances where it is called within theAnimator.tsfile.
- All calls include the
aniUpdateparameter.- The function signature is consistent across the codebase.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `_applyStateTransitions` match the new signature. # Test: Search for the function usage. Expect: Only occurances of the new signature. rg --type python -A 5 $'_applyStateTransitions'Length of output: 77
Script:
#!/bin/bash # Description: Verify all function calls to `_applyStateTransitions` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type typescript -A 5 '_applyStateTransitions'Length of output: 85
Script:
#!/bin/bash # Description: Verify all function calls to `_applyStateTransitions` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type ts -A 5 '_applyStateTransitions'Length of output: 1216
Script:
#!/bin/bash # Description: Verify all function calls to `_applyStateTransitions` match the new signature. # Test: Search for the function usage and ensure the full parameter list is captured. rg '_applyStateTransitions' packages/core/src/animation/Animator.ts -A 10Length of output: 891
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 (22 hunks)
- tests/src/core/Animator.test.ts (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- tests/src/core/Animator.test.ts
Additional context used
Biome
packages/core/src/animation/Animator.ts
[error] 727-727: 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] 879-879: 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)
969-971: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
_updateFinishedStatematch the new signature.Verification successful
Verified: All function calls to
_updateFinishedStatematch the new signature.
packages/core/src/animation/Animator.tsline 969: The function_updateFinishedStateis called with the new parameterslayer,deltaTime, andaniUpdate.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `_updateFinishedState` match the new signature. # Test: Search for the function usage. Expect: Only occurances of the new signature. rg --type ts -A 5 $'_updateFinishedState'Length of output: 977
677-679: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
_updateCrossFadeStatematch the new signature.Verification successful
All function calls to
_updateCrossFadeStatematch the new signature.
- Verified that the function
_updateCrossFadeStateis called with the correct parameters in all instances.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `_updateCrossFadeState` match the new signature. # Test: Search for the function usage. Expect: Only occurances of the new signature. rg --type ts -A 5 $'_updateCrossFadeState'Length of output: 961
1230-1233: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
_applyTransitionsByConditionmatch the new signature.Verification successful
Function usage matches the new signature.
The function
_applyTransitionsByConditionis used consistently with the new signature across the codebase.
packages/core/src/animation/Animator.tsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `_applyTransitionsByCondition` match the new signature. # Test: Search for the function usage. Expect: Only occurances of the new signature. rg --type ts -A 5 $'_applyTransitionsByCondition'Length of output: 2155
1486-1488: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
_checkAnyAndEntryStatematch the new signature.Verification successful
All function calls to
_checkAnyAndEntryStatematch the new signature.The function
_checkAnyAndEntryStatehas been correctly updated, and all its usages in the codebase conform to the new signature.
packages/core/src/animation/Animator.ts: Lines 1486-1488Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `_checkAnyAndEntryState` match the new signature. # Test: Search for the function usage. Expect: Only occurances of the new signature. rg --type ts -A 5 $'_checkAnyAndEntryState'Length of output: 1345
506-508: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
_updateStatematch the new signature.Verification successful
Verification complete: All function calls to
_updateStatematch the new signature.
- The function
_updateStateis consistently called with the parameterslayerIndex,layerData,layer,deltaTime, andaniUpdatethroughout theAnimator.tsfile.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `_updateState` match the new signature. # Test: Search for the function usage. Expect: Only occurances of the new signature. rg --type ts -A 5 $'_updateState'Length of output: 3459
1066-1073: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
_applyStateTransitionsmatch the new signature.Verification successful
Verified: All function calls to
_applyStateTransitionsmatch the new signature, including thelayerandaniUpdateparameters.
packages/core/src/animation/Animator.tslines where_applyStateTransitionsis called:
- Line 1066
- Line 1073
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `_applyStateTransitions` match the new signature. # Test: Search for the function usage. Expect: Only occurances of the new signature. rg --type ts -A 5 $'_applyStateTransitions'Length of output: 1216
Script:
#!/bin/bash # Description: Verify all function calls to `_applyStateTransitions` match the new signature with complete parameters. # Test: Search for the function usage and display complete function calls. rg --type ts -A 10 $'_applyStateTransitions'Length of output: 2181
533-535: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
_updatePlayingStatematch the new signature.Verification successful
Let's verify all instances of
_updatePlayingStateto ensure they match the new signature.
All function calls to
_updatePlayingStatematch the new signature.The verification confirms that the function calls are consistent with the updated signature.
packages/core/src/animation/Animator.ts: Line 533Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `_updatePlayingState` match the new signature. # Test: Search for the function usage. Expect: Only occurances of the new signature. rg --type ts -A 5 $'_updatePlayingState'Length of output: 963
Script:
#!/bin/bash # Description: Verify all function calls to `_updatePlayingState` match the new signature. # Test: Search for all instances of the function usage. Expect: Only occurrences of the new signature. rg --type ts '_updatePlayingState'Length of output: 231
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 (22 hunks)
Additional context used
Biome
packages/core/src/animation/Animator.ts
[error] 727-727: 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] 882-882: 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)
506-508: LGTM! The method signature update is appropriate.The addition of
layerandaniUpdateparameters enhances the method's flexibility.
533-535: LGTM! The method signature update is appropriate.The addition of
layerandaniUpdateparameters enhances the method's flexibility.
676-678: LGTM! The method signature update is appropriate.The addition of
layerandaniUpdateparameters enhances the method's flexibility.
972-974: LGTM! The method signature update is appropriate.The addition of
layerandaniUpdateparameters enhances the method's flexibility.
1069-1076: LGTM! The method signature update is appropriate.The addition of
layerandaniUpdateparameters enhances the method's flexibility.
1233-1236: LGTM! The method signature update is appropriate.The addition of
layerandaniUpdateparameters enhances the method's flexibility.
1489-1491: LGTM! The method signature update is appropriate.The addition of
layerandaniUpdateparameters enhances the method's flexibility.
github-actions
bot
added
the
documentation
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- packages/core/src/animation/Animator.ts (26 hunks)
- packages/core/src/animation/AnimatorController.ts (3 hunks)
- packages/core/src/animation/AnimatorControllerParameter.ts (1 hunks)
- tests/src/core/Animator.test.ts (7 hunks)
Files skipped from review due to trivial changes (1)
- packages/core/src/animation/AnimatorControllerParameter.ts
Files skipped from review as they are similar to previous changes (1)
- tests/src/core/Animator.test.ts
Additional context used
Biome
packages/core/src/animation/Animator.ts
[error] 755-755: 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] 910-910: 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 (22)
packages/core/src/animation/AnimatorController.ts (4)
5-6: LGTM! New imports are necessary.The new imports for
ReferResourceandEngineare appropriate and necessary for the changes made to the class inheritance and constructor.
11-11: LGTM! Class inheritance change is appropriate.The
AnimatorControllerclass correctly extendsReferResource, aligning with the PR objectives.
44-44: LGTM! Method signature change is appropriate.The parameter name change from
valuetoinitialValueinaddParametermethod clarifies its purpose.
37-39: Verify the constructor usage in the codebase.The constructor change is approved.
However, ensure that all instantiations of
AnimatorControllercorrectly pass theengineparameter if required.Verification successful
All instantiations of
AnimatorControllercorrectly pass theengineparameter.The constructor change is verified, and all instances in the codebase adhere to the new signature.
examples/animation-customAnimationClip.tsexamples/animation-sprite.tse2e/case/animator-customAnimationClip.tse2e/case/animator-customBlendShape.tse2e/case/animator-multiSubMeshBlendShape.tspackages/loader/src/gltf/parser/GLTFAnimatorControllerParser.tspackages/loader/src/AnimatorControllerLoader.tstests/src/core/Animator.test.tsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all instantiations of `AnimatorController` match the new constructor signature. # Test: Search for the class instantiation. Expect: Only occurances of the new constructor signature. rg --type ts -A 5 $'new AnimatorController'Length of output: 9254
packages/core/src/animation/Animator.ts (18)
12-12: LGTM! New import is necessary.The new import for
AnimatorControllerParameteris appropriate and necessary for the changes made in the file.
508-508: LGTM! Method signature change is appropriate.The method
_updateStatenow correctly acceptslayerandaniUpdateparameters, enhancing flexibility and control.
534-536: LGTM! Method signature change is appropriate.The method
_updateStatenow correctly acceptslayerandaniUpdateparameters, enhancing flexibility and control.
561-563: LGTM! Method signature change is appropriate.The method
_updatePlayingStatenow correctly acceptslayerandaniUpdateparameters, enhancing flexibility and control.
704-706: LGTM! Method signature change is appropriate.The method
_updateCrossFadeStatenow correctly acceptslayerandaniUpdateparameters, enhancing flexibility and control.
867-869: LGTM! Method signature change is appropriate.The method
_updateCrossFadeFromPoseStatenow correctly acceptslayerandaniUpdateparameters, enhancing flexibility and control.
1000-1002: LGTM! Method signature change is appropriate.The method
_updateFinishedStatenow correctly acceptslayerandaniUpdateparameters, enhancing flexibility and control.
1097-1104: LGTM! Method signature change is appropriate.The method
_applyStateTransitionsnow correctly acceptslayerandaniUpdateparameters, enhancing flexibility and control.
1261-1264: LGTM! Method signature change is appropriate.The method
_applyTransitionsByConditionnow correctly acceptslayerandaniUpdateparameters, enhancing flexibility and control.
540-543: LGTM! Control flow change is appropriate.The control flow for managing animation state updates has been correctly refined to incorporate the new parameters.
578-588: LGTM! Control flow change is appropriate.The control flow for managing animation state updates has been correctly refined to incorporate the new parameters.
627-643: LGTM! Control flow change is appropriate.The control flow for managing animation state updates has been correctly refined to incorporate the new parameters.
748-769: LGTM! Control flow change is appropriate.The control flow for managing animation state updates has been correctly refined to incorporate the new parameters.
Tools
Biome
[error] 755-755: 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
806-861:
LGTM! Control flow change is appropriate.The control flow for managing animation state updates has been correctly refined to incorporate the new parameters.
908-924: LGTM! Control flow change is appropriate.The control flow for managing animation state updates has been correctly refined to incorporate the new parameters.
Tools
Biome
[error] 910-910: 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
949-994:
LGTM! Control flow change is appropriate.The control flow for managing animation state updates has been correctly refined to incorporate the new parameters.
1004-1037: LGTM! Control flow change is appropriate.The control flow for managing animation state updates has been correctly refined to incorporate the new parameters.
1518-1542: LGTM! Control flow change is appropriate.The control flow for managing animation state updates has been correctly refined to incorporate the new parameters.
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- packages/core/src/animation/Animator.ts (28 hunks)
- packages/core/src/animation/AnimatorCondition.ts (2 hunks)
- packages/core/src/animation/AnimatorController.ts (3 hunks)
- packages/core/src/animation/AnimatorControllerParameter.ts (1 hunks)
- packages/core/src/animation/AnimatorStateTransition.ts (3 hunks)
Files skipped from review as they are similar to previous changes (2)
- packages/core/src/animation/AnimatorController.ts
- packages/core/src/animation/AnimatorControllerParameter.ts
Additional context used
Biome
packages/core/src/animation/Animator.ts
[error] 764-764: 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] 919-919: 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/AnimatorCondition.ts (2)
1-1: Update import statement.The import statement has been updated to import
AnimatorControllerParameterValueinstead ofAnimatorControllerParameterValueType. Ensure that this change is consistent with the rest of the codebase.
13-13: Update type ofthresholdproperty.The type of the
thresholdproperty has been changed fromAnimatorControllerParameterValueTypetoAnimatorControllerParameterValue. This change should be verified for consistency with other parts of the codebase.packages/core/src/animation/AnimatorStateTransition.ts (3)
1-1: Update import statement.The import statement has been updated to import
AnimatorControllerParameterValueinstead ofAnimatorControllerParameterValueType. Ensure that this change is consistent with the rest of the codebase.
64-64: Update type ofthresholdparameter inaddConditionmethod.The type of the
thresholdparameter in theaddConditionmethod has been changed fromAnimatorControllerParameterValueTypetoAnimatorControllerParameterValue. This change should be verified for consistency with other parts of the codebase.
76-76: Update type ofthresholdparameter in overloadedaddConditionmethod.The type of the
thresholdparameter in the overloadedaddConditionmethod has been changed fromAnimatorControllerParameterValueTypetoAnimatorControllerParameterValue. This change should be verified for consistency with other parts of the codebase.packages/core/src/animation/Animator.ts (1)
12-12: Update import statement.The import statement has been updated to import
AnimatorControllerParameterandAnimatorControllerParameterValue. Ensure that this change is consistent with the rest of the codebase.
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 (28 hunks)
- packages/core/src/animation/AnimatorController.ts (3 hunks)
- packages/loader/src/AnimatorControllerLoader.ts (4 hunks)
Additional context used
Biome
packages/core/src/animation/Animator.ts
[error] 510-510: 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] 733-733: 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] 890-890: 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 (15)
packages/core/src/animation/AnimatorController.ts (5)
11-11: LGTM! Class inheritance change approved.The
AnimatorControllerclass now correctly extendsReferResource.
55-70: LGTM! Method parameter name change approved.The
addParametermethod now usesdefaultValueinstead ofvaluefor better clarity.
65-70: LGTM! Method update approved.The
addParametermethod update is consistent and improves clarity.
Line range hint
101-103:
LGTM! Method approved.The
_registerChangeFlagmethod is straightforward and does not require any changes.
37-50: Verify the constructor change.The constructor now accepts an optional
engineparameter and calls the superclass constructor if provided.Verification successful
Constructor change verification successful.
All instances of
AnimatorControllercorrectly use the new constructor signature with the optionalengineparameter.
examples/animation-sprite.tsexamples/animation-customAnimationClip.tse2e/case/animator-customBlendShape.tse2e/case/animator-multiSubMeshBlendShape.tse2e/case/animator-customAnimationClip.tspackages/loader/src/AnimatorControllerLoader.tspackages/loader/src/gltf/parser/GLTFAnimatorControllerParser.tstests/src/core/Animator.test.tsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all instantiations of `AnimatorController` match the new constructor signature. # Test: Search for the class instantiation. Expect: Only occurrences of the new constructor signature. rg --type typescript -A 5 $'new AnimatorController'Length of output: 87
Script:
#!/bin/bash # Description: Verify all instantiations of `AnimatorController` match the new constructor signature. # Test: Search for the class instantiation. Expect: Only occurrences of the new constructor signature. rg --type ts -A 5 $'new AnimatorController'Length of output: 9282
packages/loader/src/AnimatorControllerLoader.ts (2)
27-27: LGTM! Instantiation change approved.The
AnimatorControlleris now correctly instantiated withresourceManager.engine.
105-105: LGTM! Parameter update approved.The
parameter.valuehas been correctly replaced withparameter.defaultValue.packages/core/src/animation/Animator.ts (8)
500-526: LGTM! Method update approved.The
_updateStatemethod now acceptslayerandaniUpdateparameters, enhancing flexibility.Tools
Biome
[error] 510-510: 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
553-643:
LGTM! Method update approved.The
_updatePlayingStatemethod now acceptslayerandaniUpdateparameters, enhancing flexibility.
Line range hint
681-793:
LGTM! Method update approved.The
_updateCrossFadeStatemethod now acceptslayerandaniUpdateparameters, enhancing flexibility.Tools
Biome
[error] 733-733: 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
844-932:
LGTM! Method update approved.The
_updateCrossFadeFromPoseStatemethod now acceptslayerandaniUpdateparameters, enhancing flexibility.Tools
Biome
[error] 890-890: 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)
980-1021: LGTM! Method update approved.The
_updateFinishedStatemethod now acceptslayerandaniUpdateparameters, enhancing flexibility.
Line range hint
1081-1166:
LGTM! Method update approved.The
_applyStateTransitionsmethod now acceptslayerandaniUpdateparameters, enhancing flexibility.
1245-1255: LGTM! Method update approved.The
_applyTransitionsByConditionmethod now acceptslayerandaniUpdateparameters, enhancing flexibility.
1501-1525: LGTM! Method update approved.The
_checkAnyAndEntryStatemethod now acceptslayerandaniUpdateparameters, enhancing flexibility.
luzhuang
changed the title
luzhuang
changed the title
GuoLei1990
changed the title
GuoLei1990
changed the title
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
AnimatorControllernow requires anengineparameter during instantiation, enhancing its integration with the rendering engine and potentially improving animation management.Documentation
AnimatorControllerinstantiation, providing clarity on the new requirement for theengineparameter.Bug Fixes
AnimatorControllerconstructor, preventing potential errors in existing implementations.Tests