Fix transform parent dirty error when added as root entity #2311
Conversation
WalkthroughThe recent changes enhance the Changes
Sequence Diagram(s)sequenceDiagram
participant Parent as Parent Entity
participant Child as Child Entity
participant Scene as Scene Manager
Parent->>Child: Add as child
Parent->>Parent: Mark as dirty
Parent->>Child: Call _setTransformDirty
Child->>Scene: Check world position
Scene-->>Child: Update position based on Parent
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- packages/core/src/Entity.ts (2 hunks)
- packages/core/src/Scene.ts (1 hunks)
- tests/src/core/Transform.test.ts (3 hunks)
Additional comments not posted (4)
tests/src/core/Transform.test.ts (2)
15-16: Verify scene assignment logic.The scene is now assigned from
engine.sceneManager.scenes[0]instead ofengine.sceneManager.activeScene. Ensure that this change is intentional and does not affect other tests relying on the active scene.
29-48: LGTM! Consider adding more assertions.The test case effectively validates the transformation propagation. Consider adding more assertions to cover edge cases or different transformation scenarios.
packages/core/src/Scene.ts (1)
366-366: LGTM! Verify the impact on performance.The addition of
_setTransformDirtyfor non-root entities ensures accurate transformation updates. Verify that this change does not introduce any performance regressions, especially in scenes with a large number of entities.packages/core/src/Entity.ts (1)
589-600: LGTM! Ensure unit tests cover this method.The method
_setTransformDirtyensures accurate propagation of transformation changes. Ensure that unit tests cover this method to validate its functionality.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/core/src/Scene.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/core/src/Scene.ts
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
Tests