Send SceneInstanceReady only once per scene#11002
Send SceneInstanceReady only once per scene#11002alice-i-cecile merged 1 commit intobevyengine:mainfrom
SceneInstanceReady only once per scene#11002Conversation
|
Welcome, new contributor! Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨ |
|
Did you notice receiving several times this event? It should already be sent only once |
|
Yes, I received it multiple times, exactly as many times actually as I had entities in the scene. I'm not familiar with Bevy's codebase, but from how I understand the code it seems obvious that it calls EDIT: I only tested in v0.12.1 actually, should I test it on |
|
I'm on main and seeing this event only once, but this code shouldn't have changed since the 0.12.1 Could you share the scene you're using? I tried with a few gltf files. Your change should be OK, just trying to understand why you're having this issue here |
6387ea5 to
23afd0e
Compare
I added a unit test. Without the change in this PR the unit test will fail. |
97fd191 to
8aa4b88
Compare
fc3c346 to
ad6fd09
Compare
|
Rebased on main after #11003. |
|
nice test, thanks! unlike scenes from gltfs, yours has two root entities, which explains what you're seeing 👍 |
ad6fd09 to
b19f4ea
Compare
|
Rebased after cfcb688. |
jdm
left a comment
There was a problem hiding this comment.
I appreciate the unit test for this change!
# Objective Send `SceneInstanceReady` only once per scene. ## Solution I assume that this was not intentional. So I just changed it to only be sent once per scene. --- ## Changelog ### Fixed - Fixed `SceneInstanceReady` being emitted for every `Entity` in a scene.
# Objective Send `SceneInstanceReady` only once per scene. ## Solution I assume that this was not intentional. So I just changed it to only be sent once per scene. --- ## Changelog ### Fixed - Fixed `SceneInstanceReady` being emitted for every `Entity` in a scene.
# 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` `Option`al. - 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> }`.
Objective
Send
SceneInstanceReadyonly once per scene.Solution
I assume that this was not intentional.
So I just changed it to only be sent once per scene.
Changelog
Fixed
SceneInstanceReadybeing emitted for everyEntityin a scene.