fix: errors about visible and transform when setting

[RGCHN]

• fix: correct the mean of visible
• feat: add setRotation、setScale and setPosition method in class Composition

Summary by CodeRabbit

• New Features

  • Introduced new methods for more intuitive 3D transformations: setPosition, setRotation, and setScale.
  • Added conditional visibility control for meshes in various plugins, enhancing the rendering logic.

• Bug Fixes

  • Corrected the visibility logic in ThreeMesh to directly reflect the provided values.
  • Fixed the conditional rendering flow in GLRenderer to proceed only when meshes are not visible.

• Refactor

  • Improved visibility handling in ParticleVFXItem and SpriteVFXItem by removing direct manipulation of visibility properties.
  • Streamlined the initialization process in TextItem by removing redundant texture updates.
  • Enhanced the SpineVFXItem class with a new setScale method and refined visibility checks.

• Documentation

  • Updated comments to reflect changes in visibility handling across various classes.

• Style

  • Adjusted code style for consistency in visibility-related methods across the codebase.

• Tests

  • Modified unit tests to align with the new visibility control logic and 3D transformation methods.

[coderabbitai]

Important

Auto Review Skipped

Auto reviews are disabled on base/target branches other than the default branch. Please add the base/target branch pattern to the list of additional branches to be reviewed in the settings.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository.

To trigger a single review, invoke the @coderabbitai review command.

Walkthrough

The changes across various packages suggest a significant refactoring in the handling of visibility and 3D transformations in a graphics/effects library. New methods for setting position, rotation, and scale directly have been introduced, while visibility handling has been centralized and made more consistent. Access control for certain properties has been relaxed to allow for extended class functionality, and default visibility states have been adjusted.

Changes

File Path	Change Summary
packages/effects-core/src/.../composition.ts	Introduced setPosition, setRotation, and setScale methods.
packages/effects-core/src/.../calculate-item.ts	Changed vfxItem property from private to protected.
packages/effects-core/src/.../particle-system.ts	Updated visibility control logic.
packages/effects-core/src/.../particle-vfx-item.ts	Altered visibility handling and update logic.
packages/effects-core/src/.../sprite-item.ts	Modified visibility assignment logic.
packages/effects-core/src/.../sprite-mesh.ts	Inverted visibility logic for mesh display.
packages/effects-core/src/.../sprite-vfx-item.ts	Removed visibility handling methods and updated lifecycle logic.
packages/effects-core/src/.../text-item.ts	Removed initialization logic and updated text processing.
packages/effects-core/src/.../text-loader.ts	Modified visibility control and added applyChange method.
packages/effects-core/src/.../text-vfx-item.ts	Updated lifecycle methods and visibility handling.
packages/effects-core/src/.../mesh.ts	Changed default visibility state.
packages/effects-core/src/.../vfx-item.ts	Updated visibility state management.
packages/effects-threejs/src/.../three-mesh.ts	Corrected visibility methods logic.
packages/effects-webgl/src/.../gl-renderer.ts	Inverted visibility check in conditional.
plugin-packages/spine/src/.../spine-loader.ts	Added visibility check before rendering.
plugin-packages/spine/src/.../spine-vfx-item.ts	Refactored visibility and scale handling, updated lifecycle methods.
web-packages/demo/src/single.ts	Removed variable assignment and commented out a property.
web-packages/test/unit/src/.../plugin.spec.ts	Updated test assertions and control flow.

🐇✨
In the realm of code where the pixels play,
A rabbit hopped through, refactoring away.
Visibility tweaked, with a hop and a bound,
Transformations set, in the code they are found. 🌟🎩

---

Tips

Chat with CodeRabbit Bot (@coderabbitai)

• If you reply to a review comment from CodeRabbit, the bot will automatically respond.
• To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
• Note: Review comments are made on code diffs or files, not on the PR overview.
• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Commands (invoked as PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai help to get help.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• The JSON schema for the configuration file is available here.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

[zheeeng]

@coderabbitai review

[coderabbitai]

Review Status

Actionable comments generated: 3

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between a46794a and aa5da90.

Files selected for processing (18)

• packages/effects-core/src/composition.ts (1 hunks)
• packages/effects-core/src/plugins/cal/calculate-item.ts (1 hunks)
• packages/effects-core/src/plugins/particle/particle-system.ts (1 hunks)
• packages/effects-core/src/plugins/particle/particle-vfx-item.ts (1 hunks)
• packages/effects-core/src/plugins/sprite/sprite-item.ts (1 hunks)
• packages/effects-core/src/plugins/sprite/sprite-mesh.ts (2 hunks)
• packages/effects-core/src/plugins/sprite/sprite-vfx-item.ts (1 hunks)
• packages/effects-core/src/plugins/text/text-item.ts (2 hunks)
• packages/effects-core/src/plugins/text/text-loader.ts (2 hunks)
• packages/effects-core/src/plugins/text/text-vfx-item.ts (2 hunks)
• packages/effects-core/src/render/mesh.ts (1 hunks)
• packages/effects-core/src/vfx-item.ts (4 hunks)
• packages/effects-threejs/src/three-mesh.ts (2 hunks)
• packages/effects-webgl/src/gl-renderer.ts (1 hunks)
• plugin-packages/spine/src/spine-loader.ts (1 hunks)
• plugin-packages/spine/src/spine-vfx-item.ts (2 hunks)
• web-packages/demo/src/single.ts (3 hunks)
• web-packages/test/unit/src/effects-core/composition/plugin.spec.ts (1 hunks)

Additional comments: 39

packages/effects-core/src/composition.ts (5)

• 853-894: The addition of setPosition, setRotation, and setScale methods to the Composition class aligns with the PR objectives to provide more control over 3D transformations. These methods set the position, rotation, and scale relative to the origin, expanding the functionality of the class. Ensure that the implementation of these methods in the CompVFXItem class correctly handles the transformations as intended.

• 858-860: The translate method remains unchanged and continues to delegate the transformation to the content object. This is consistent with the user-provided facts and the AI-generated summary, which do not mention any changes to this method.

• 872-874: The rotate method also remains unchanged and continues to delegate the transformation to the content object. This is consistent with the user-provided facts and the AI-generated summary, which do not mention any changes to this method.

• 885-887: The scale method remains unchanged and continues to delegate the transformation to the content object. This is consistent with the user-provided facts and the AI-generated summary, which do not mention any changes to this method.

• 853-894: The summary I generated incorrectly states that the translate, rotate, and scale methods were altered. However, the hunks show that these methods have not been changed. They still call the corresponding methods on the content object without any modifications to their behavior.

packages/effects-core/src/plugins/cal/calculate-item.ts (1)

• 97-97: The change from private to protected for the vfxItem property aligns with the PR objectives to adjust access control and inheritance.

packages/effects-core/src/plugins/particle/particle-system.ts (1)

• 479-479: The change from vfxItem.getVisible() to vfxItem.contentVisible is consistent with the PR objectives to correct the meaning of the visible property within the affected classes.

packages/effects-core/src/plugins/particle/particle-vfx-item.ts (2)

• 57-61: The logic in the onItemUpdate method correctly updates the visibility of this.content based on the hide variable, and calls onUpdate when the content is visible. This is a change from the previous behavior where onUpdate was called when hide was true, which was incorrect.

• 57-61: The logic in the onItemUpdate method correctly updates the visibility of this.content based on the hide variable, and calls onUpdate when the content is visible. This is a change from the previous behavior where onUpdate was called when hide was true, which was incorrect.

packages/effects-core/src/plugins/sprite/sprite-item.ts (2)

• 297-297: The change from this.visible to this.vfxItem.contentVisible aligns with the PR objectives to standardize visibility control across various classes.

• 299-299: The startSize property is set in the getRenderData method, which was not mentioned in the summary. Please confirm if this is an intentional change and aligns with the PR objectives.

packages/effects-core/src/plugins/sprite/sprite-mesh.ts (2)

• 93-96: The change to the visibility logic in setItems method correctly reflects the PR objectives to address visibility issues. The mesh is now set to invisible when there are no items, which seems to be the intended behavior.

• 157-161: The visibility of the mesh is now being set based on the draw count of the geometry, which is a significant change from the previous logic. This change should be carefully tested to ensure that it behaves as expected in all cases where SpriteMesh is used.

packages/effects-core/src/plugins/sprite/sprite-vfx-item.ts (2)

• 27-35: The changes to onLifetimeBegin and onItemRemoved methods align with the PR objectives of adjusting visibility handling. Ensure that the removal of direct visibility manipulation and the new resource cleanup logic in onItemRemoved are consistent with the intended behavior across the application.

• 38-39: The update to onItemUpdate method, which now updates the time of the content, seems appropriate and should be verified to ensure it works as expected within the application's update cycle.

packages/effects-core/src/plugins/text/text-item.ts (2)

• 64-66: The summary indicates that the updateTexture method was removed, but it is still present in the final state of the code. Please verify if the method should indeed be removed or if the summary is incorrect.

• 286-287: The change to handle null or undefined values for this.text by splitting an empty string is a good defensive programming practice to prevent potential runtime errors.

packages/effects-core/src/plugins/text/text-loader.ts (3)

• 13-13: The summary indicates a modification to the meshes array declaration, but the provided hunk does not show any changes to this declaration. This could be an inconsistency in the summary.

• 27-42: The changes to the onCompositionUpdate method introduce conditional visibility handling and texture updating for TextVFXItem instances. This aligns with the PR objectives to correct visibility handling.

• 15-16: The onCompositionDestroyed method has been updated to conditionally dispose of meshes based on the reusable property of the composition. This is a logical enhancement to the disposal process.

packages/effects-core/src/plugins/text/text-vfx-item.ts (3)

• 25-28: The summary mentions the addition of this.content.updateTexture(); in the onLifetimeBegin method, but this line is not present in the hunk. Please verify if the summary is outdated or if the code change was omitted.

• 23-43: The summary states that the handleVisibleChanged method has been completely removed, but the hunks do not show this method or its removal. Please verify if the summary is outdated or if the code change was omitted.

• 107-113: The summary does not mention the addition of spMesh.mesh.setVisible(true); in the createWireframeMesh method, but this change is present in the hunk. Please verify if the summary is incomplete or if the code change was not meant to be included.

packages/effects-core/src/render/mesh.ts (1)

• 57-57: The change in the default visibility state of Mesh instances from false to true is noted and aligns with the PR objectives. Ensure to verify the impact of this change on any code that creates Mesh instances, as it may affect the initial visibility of these objects.

packages/effects-core/src/vfx-item.ts (5)

• 325-327: The addition of _contentVisible property assignment in the createContent method aligns with the PR objectives to correct visibility handling.

• 370-370: The assignment of _contentVisible to false in the endBehavior condition block is consistent with the PR objectives to correct visibility handling.

• 619-622: The assignment of _contentVisible to false in the reset method is consistent with the PR objectives to correct visibility handling.

• 33-36: The PR summary mentions the removal of visibility handling methods like handleVisibleChanged, but the method is still present in the code. Please verify if this method should be removed or if the summary needs to be updated.

• 33-36: The introduction of new methods setRotation, setScale, and setPosition is consistent with the PR objectives to provide more control over 3D transformations.

packages/effects-threejs/src/three-mesh.ts (2)

• 70-74: The changes to the setVisible method correctly align with the PR objectives to address the handling of the visible property.

• 81-83: The changes to the getVisible method are also in line with the PR objectives and ensure that the visibility state is returned directly.

plugin-packages/spine/src/spine-loader.ts (1)

• 189-195: The addition of the visibility check mesh.getVisible() before adding meshes to the default render pass is a good practice to ensure that only visible meshes are rendered. This change aligns with the PR objectives and summary provided.

plugin-packages/spine/src/spine-vfx-item.ts (4)

• 147-158: Visibility handling in onItemUpdate method appears to be correctly implemented, ensuring that mesh visibility is updated only when content and mesh groups are present.

• 161-165: The addition of a null check for this.state in the onEnd method is a good practice to prevent potential null reference errors.

• 386-389: The setScale method correctly multiplies the new scale factors with the existing scale values, ensuring that the scale is compounded rather than replaced.

• 392-396: The additional visibility check in getBounds method is a logical optimization to avoid unnecessary calculations when the content is not visible.

web-packages/demo/src/single.ts (2)

• 2-7: The summary mentions the removal of an assignment to the item variable, but this change is not visible in the provided hunks. If this change was made, ensure that any code depending on the item variable is updated accordingly to prevent runtime errors.

• 21-27: The summary indicates that the env property in the createPlayer function's configuration object has been commented out, but this change is not visible in the provided hunks. If the env property is indeed commented out, verify that this does not affect the expected behavior of the createPlayer function, especially if the env property is used elsewhere in the code.

web-packages/test/unit/src/effects-core/composition/plugin.spec.ts (1)

• 283-301: The test case reflects the changes to visibility control by using the contentVisible property and the setVisible method. The assertions correctly check the visibility state and the hide spy is called as expected when visibility is changed. This aligns with the PR objectives and the summary of changes.