Skip to content
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

Send SceneInstanceReady when spawning any kind of scene #11741

Merged
merged 1 commit into from
Jul 6, 2024

Conversation

daxpedda
Copy link
Contributor

@daxpedda daxpedda commented Feb 6, 2024

Objective

  • Emit an event regardless of scene type (Scene and DynamicScene).
  • Also send the InstanceId along.

Follow-up to #11002.
Fixes #2218.

Solution

  • Send SceneInstanceReady regardless of scene type.
  • Make SceneInstanceReady::parent Optional.
  • Add SceneInstanceReady::id.

Changelog

Changed

  • SceneInstanceReady is now sent for Scene as well. SceneInstanceReady::parent is an Option and SceneInstanceReady::id, an InstanceId, is added to identify the corresponding Scene.

Migration Guide

  • SceneInstanceReady { parent: Entity } is now SceneInstanceReady { id: InstanceId, parent: Option<Entity> }.

@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior A-Scenes Serialized ECS data stored on the disk C-Usability A targeted quality-of-life change that makes Bevy easier to use M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide and removed C-Bug An unexpected or incorrect behavior labels Feb 6, 2024
@ethereumdegen
Copy link
Contributor

ethereumdegen commented Mar 2, 2024

This seems like a nice way to handle this 🙌

It doesnt seem like it would break the ways i use the event. I am now using this event to detect when a GLTF scene is fully attached to an entity [with a specific component] . i loop through the child nodes and act upon them by name. Such as registering the child to an entity linker , swapping the materials, or replacing the child mesh with a xpbd collider. This functionality is critical to multiple parts of my game. It effectively lets me name nodes in blender to give them super powers in bevy. This change seems like it will not break that flow while also addressing the issue 2218

@daxpedda daxpedda force-pushed the scene-instance-ready branch from 00cf571 to 0013fa2 Compare July 5, 2024 17:20
@alice-i-cecile alice-i-cecile added the S-Needs-Review Needs reviewer attention (from anyone!) to move forward label Jul 5, 2024
Copy link
Contributor

@torsteingrindvik torsteingrindvik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code reads well- the tests really help build confidence here.

@daxpedda daxpedda force-pushed the scene-instance-ready branch from 0013fa2 to d4c6966 Compare July 6, 2024 11:41
@mockersf mockersf added this pull request to the merge queue Jul 6, 2024
Merged via the queue into bevyengine:main with commit ea2a7e5 Jul 6, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Scenes Serialized ECS data stored on the disk C-Usability A targeted quality-of-life change that makes Bevy easier to use M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Needs-Review Needs reviewer attention (from anyone!) to move forward
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Emit an event when a scene has finished being spawned
5 participants