refactor: composition component set children render order using track order

[wumaolinmaoan]

… order

Summary by CodeRabbit

• Refactor
  • Optimized internal timeline instance initialization through lazy loading patterns.
  • Improved internal state management for composition components.
  • Restructured internal field accessibility within timeline components to support enhanced extensibility.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

The changes introduce lazy initialization of TimelineInstance in CompositionComponent, deferring creation until timelineAsset exists. Additionally, masterTrackInstances field in TimelineInstance is exposed as public with @internal annotation, enabling direct access to track instances for render order management.

Changes

Cohort / File(s)	Summary
Lazy Timeline Initialization packages/effects-core/src/components/composition-component.ts	Introduced private getter for timelineInstance with lazy initialization. Made timelineAsset nullable, removed eager creation in onStart, and updated onUpdate and setChildrenRenderOrder to conditionally operate on timelineInstance. Switched TimelineAsset to type-only import.
Track Instance Visibility packages/effects-core/src/plugins/timeline/timeline-asset.ts	Changed masterTrackInstances from private to public field with @internal JSDoc annotation in TimelineInstance class, enabling controlled access to track instances.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A timeline reborn, now lazy and wise,
Waiting for assets before it must rise,
Track instances unveiled, yet marked internal true,
Optimization hops forth—much faster to brew!

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main refactoring change: composition component's children render order is now set using track order via the reworked setChildrenRenderOrder method.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch refactor/composition-set-render-order

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 350e586 and de4982c.

📒 Files selected for processing (2)

• packages/effects-core/src/components/composition-component.ts
• packages/effects-core/src/plugins/timeline/timeline-asset.ts

🧰 Additional context used

🧠 Learnings (2)

📚 Learning: 2024-10-18T09:15:08.038Z

Learnt from: wumaolinmaoan
Repo: galacean/effects-runtime PR: 691
File: packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts:39-42
Timestamp: 2024-10-18T09:15:08.038Z
Learning: In `packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts`, when accumulating color components, avoid using `clone()` methods because it can cause additional garbage collection overhead.

Applied to files:

• packages/effects-core/src/plugins/timeline/timeline-asset.ts
• packages/effects-core/src/components/composition-component.ts

📚 Learning: 2025-08-05T07:51:18.728Z

Learnt from: ChengYi996
Repo: galacean/effects-runtime PR: 1118
File: packages/effects-core/src/components/ffd-component.ts:162-164
Timestamp: 2025-08-05T07:51:18.728Z
Learning: In packages/effects-core/src/components/ffd-component.ts, the //ts-expect-error usage for accessing siblingComponent.splits (around lines 162-164) is temporary debugging code that will be properly implemented later, as confirmed by ChengYi996.

Applied to files:

• packages/effects-core/src/components/composition-component.ts

🧬 Code graph analysis (1)

packages/effects-core/src/plugins/timeline/timeline-asset.ts (1)

packages/effects-core/src/plugins/timeline/track-instance.ts (1)

• TrackInstance (9-29)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Analyze (javascript-typescript)

🔇 Additional comments (6)

packages/effects-core/src/plugins/timeline/timeline-asset.ts (1)

61-64: Visibility change enables render order logic.

Making masterTrackInstances public with @internal annotation allows CompositionComponent to access track instances for render ordering. The @internal JSDoc signals this is not part of the public API, though it's not runtime-enforced.

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

52-54: LGTM - Simplified initialization.

Removing eager TimelineInstance creation from onStart aligns with the lazy initialization pattern and simplifies the method.

---

68-77: LGTM - Safe guarded access.

The null check before accessing timelineInstance correctly handles the lazy initialization pattern.

---

6-7: Type-only import for TimelineAsset is correct.

TimelineAsset is used only as a type annotation and is appropriately imported as type-only for build optimization. No value-based usages (e.g., instanceof, new) detected.

---

44-50: The lazy initialization pattern here is sound. The timelineAsset field is marked with @serialize() and has no setter methods—it's populated only during object deserialization and remains immutable afterward. The stale instance scenario described is not possible, as timelineAsset cannot be reassigned after initialization.

Likely an incorrect or invalid review comment.

---

199-220: Verify whether all scene items must have track bindings for render order assignment.

The refactored method now assigns render order exclusively to items bound to master tracks via sceneBindings. This is correct for the PR objective of using track order, but confirm:

1. Is it a design requirement that all items in this.items have track bindings?
2. Should unbound items receive a render order, or are they intentionally excluded?
3. Should validation or warnings be added to catch unbound items during development?

If some items legitimately don't require track bindings, document this contract clearly to prevent future bugs.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}