-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merge origin/main into origin/dev #40
Conversation
build: support CHANGELOG generation script
fix: add id and transform setting from pre-composition item
fix: 修复拖拽问题
fix: hitTest bug in pre-composition
Merge origin/main into origin/feat/1.1
chore: auto tigger bot review for specific branches
fix: errors about visible and transform when setting
…untime into test/fix-case
fix: solve pre composition problem in 3D plugin
test: fix unit and case test problems
test: fix plugin unit test
build: add vite legacy polyfill
Merge origin/feat/1.1 into origin/main
docs: changelog 1.1.0
Co-authored-by: Zheeeng <hi@zheeeng.me>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
docs: update the README and provide the Chinese version
fix: error about delay and disappear in pre-comp item
docs: changelog 1.1.1
WalkthroughThe project has undergone a series of updates to enhance functionality and documentation. A bug related to delays and visibility in a pre-comp item has been fixed. The documentation is now available in Chinese, and the English README has been improved for clarity and usefulness. Development instructions have been consolidated into a new file, and code changes suggest adjustments in the visual effects timing and visibility handling. Changes
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 X ? TipsChat with CodeRabbit Bot (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 1
Configuration used: .coderabbit.yaml
Files ignored due to filter (10)
- packages/effects-core/package.json
- packages/effects-helper/package.json
- packages/effects-threejs/package.json
- packages/effects-webgl/package.json
- packages/effects/package.json
- plugin-packages/alipay-downgrade/package.json
- plugin-packages/editor-gizmo/package.json
- plugin-packages/model/package.json
- plugin-packages/orientation-transformer/package.json
- plugin-packages/spine/package.json
Files selected for processing (7)
- CHANGELOG.md (1 hunks)
- README-zh_CN.md (1 hunks)
- README.md (1 hunks)
- docs/developing.md (1 hunks)
- packages/effects-core/src/comp-vfx-item.ts (1 hunks)
- packages/effects-core/src/plugins/particle/particle-vfx-item.ts (2 hunks)
- packages/effects-core/src/vfx-item.ts (2 hunks)
Files skipped from review due to trivial changes (4)
- CHANGELOG.md
- README-zh_CN.md
- README.md
- docs/developing.md
Additional comments: 4
packages/effects-core/src/comp-vfx-item.ts (2)
88-94: The addition of the
delay
property to theCompVFXItem
constructor is consistent with the PR's objective to modify the behavior of visual effects. Ensure that thedelay
property is properly documented and that its usage is correctly implemented throughout the class.88-94: Verify that the
delay
property is being used where necessary, such as in update calculations or any other time-sensitive logic within theCompVFXItem
class.packages/effects-core/src/vfx-item.ts (2)
402-412: The addition of a conditional block to handle the visibility of elements when a
CompVFXItem
is destroyed seems to be implemented correctly. However, it's important to ensure that this new behavior is documented and that any other parts of the code that depend on the visibility ofCompVFXItem
instances are updated to handle this new logic.685-690: Explicitly setting
_contentVisible
tofalse
during the reset process is in line with the summary provided. This change ensures that the content visibility is correctly reset, which is crucial for the proper functioning of the visual effects when items are reused or disposed of. Ensure that this change does not conflict with any other parts of the codebase that might rely on the previous state of_contentVisible
after a reset.
override onItemUpdate (dt: number, lifetime: number) { | ||
if (this.content) { | ||
let hide = !this.visible; | ||
let hide = !this.contentVisible; | ||
|
||
const parentItem = this.parentId && this.composition?.getItemByID(this.parentId); | ||
|
||
if (!hide && parentItem) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There seems to be a logic error in the onItemUpdate
method. The hide
variable is set to false
if parentData.visible
is false
, which seems counterintuitive. If the parent is not visible, it would make sense for hide
to be true
.
if (parentData) {
this.content.setParentTransform(parentData.transform);
- if (!parentData.visible) {
- hide = false;
+ if (parentData.visible === false) {
+ hide = true;
}
}
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.
override onItemUpdate (dt: number, lifetime: number) { | |
if (this.content) { | |
let hide = !this.visible; | |
let hide = !this.contentVisible; | |
const parentItem = this.parentId && this.composition?.getItemByID(this.parentId); | |
if (!hide && parentItem) { | |
override onItemUpdate (dt: number, lifetime: number) { | |
if (this.content) { | |
let hide = !this.contentVisible; | |
const parentItem = this.parentId && this.composition?.getItemByID(this.parentId); | |
if (parentItem) { | |
this.content.setParentTransform(parentItem.transform); | |
if (parentItem.visible === false) { | |
hide = true; | |
} | |
} |
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor
contentVisible
property and adjusted visibility logic in the particle effects code.