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

Scenes do not hot reload #3759

Open
nicopap opened this issue Jan 24, 2022 · 10 comments
Open

Scenes do not hot reload #3759

nicopap opened this issue Jan 24, 2022 · 10 comments
Labels
A-Assets Load files from disk to use for things like images, models, and sounds C-Bug An unexpected or incorrect behavior

Comments

@nicopap
Copy link
Contributor

nicopap commented Jan 24, 2022

Using the hot reloading file watcher on a gltf scene with more than one mesh leads to unexpected results. The loading of the updated file seems to only partially replace the original one. Some of the original entity properties seem to persist between hot reloads.

Bevy version: 0.6
OS & graphic stack: ArchLinux kernel 5.16.2; mesa/vulkan-radeon lib: v21.3.4; GPU: AMD ATI Radeon RX 5600

Reproduction steps

  • Replace the monkey scene at assets/models/Monkey.gltf by the bevy-debug-scene-1.gltf scene I prepared
  • Run the bevy hot_asset_reloading example
  • Replace Monkey.gltf with bevy-debug-scene-2.gltf.
  • Observe scene, pay attention to shape color and orientation
  • Close the example and re-run it keeping Monkey.gltf a copy of bevy-debug-scene-2.gltf
  • Notice how the scene is now different from before you closed it (notably the color and rotation of meshes do not match)

files:

  • bevy-debug-scene-1.gltf
  • bevy-debug-scene-2.gltf

I've updated my personal website, and now the scenes are gone. What you should try is swap between two scenes that have multiple entities.

Analysis

This was originally titled hot reloading of GLTF files with multiple meshes only works partially, however, after further investigation, it turns out that Scene simply never update.

The reason it seems to work in assets/hot_asset_reloading.rs is that the AssetLoader supports loading multiple asset types as "dependencies", typically when modifying the gltf, you'll modify the mesh and material, which will correctly update. However, scenes also describe the position of entities, which entities there are etc. Turns out this does not work at all. Bevy only implement scene hot reloading for DynamicScenes (as demonstrated in the scene/scene.rs example), not Scenes.

@nicopap nicopap added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Jan 24, 2022
@alice-i-cecile alice-i-cecile added A-Assets Load files from disk to use for things like images, models, and sounds and removed S-Needs-Triage This issue needs to be labelled labels Jan 24, 2022
@superdump
Copy link
Contributor

I wonder if this is also racey.

@superdump
Copy link
Contributor

@nicopap if you move the watch for changes call before the asset_server.load(), does it work properly then?

@nicopap
Copy link
Contributor Author

nicopap commented Jan 27, 2022

Doesn't help. When hot-reloading the shapes stay straight and colored red/orange. While the bevy-debug-scene-2.gltf shapes should be yellow/purple and slanted. Regardless of the call ordering, tested with each of those configuration:

  • watch_for_change before load,
  • watch_for_change after load but before spawn_scene
  • watch_for_change after spawn_scene.

@superdump
Copy link
Contributor

I'm theory-crafting as I go so hopefully this isn't noise. I realised that spawn_scene only happens once in setup. So while setting watch_for_changes, and then calling load will watch for changes on the gltf file and every asset file it references (I hope), the Scene asset that it populates is only updated in the Assets<Scene> but it isn't spawned into the main world. spawn_scene takes everything from the Scene world and puts it into the main world.

I think a possible way to solve this from the outside is to do something like spawn the scene as a child of an entity so that it can be despawned recursively. Add a system that watches the Handle<Scene> for changes, and when it sees a change and sees that it's loaded, despawn recursive the old scene and spawn the new one as a child of an entity.

I imagine this kind of thing will be addressed when @cart looks into assets as having to do this manually for any loaded scenes/prefabs or whatever would be painful.

@superdump
Copy link
Contributor

superdump commented Jan 28, 2022

I was informed by @cart that there is knowledge of the entities that were spawned from a scene in the scene spawner. I’d missed that. Maybe my suggestion still works as a workaround for the underlying bug though.

@nicopap
Copy link
Contributor Author

nicopap commented Apr 19, 2022

Turns out the color difference was because the colored point lights being at different positions. I'm now convinced this is strictly a question of either the Transform or GlobalTransform not updating correctly on scene refresh.

@nicopap
Copy link
Contributor Author

nicopap commented Apr 19, 2022

#3340 does not fix this. Still looking into it.

@nicopap
Copy link
Contributor Author

nicopap commented Apr 19, 2022

I think there is no hot reloading for Scenes. The scene.rs example demonstrate hot reloading for DynamicScene, not Scene. It might just be an accident that it works with gltf files. Is that correct? It seems to update meshes and materials based on the labeled_asset field of the LoadContext, and the fact the asset loading system recursively loads assets added to by an asset loader.

@nicopap
Copy link
Contributor Author

nicopap commented Apr 19, 2022

I cooked up an absolutely atrocious patch that implements hot reloading for Scene. This confirms my suspicion that Scene does not hot-reload. I'm not too hot on sharing the code though. I might clean up it later for review.

@nicopap nicopap changed the title hot reloading of GLTF files with multiple meshes only works partially Scenes do not hot reload Apr 19, 2022
@superdump
Copy link
Contributor

I cooked up an absolutely atrocious patch that implements hot reloading for Scene. This confirms my suspicion that Scene does not hot-reload. I'm not too hot on sharing the code though. I might clean up it later for review.

I'll keep reloading and await the hotness. ;)

nicopap added a commit to nicopap/bevy that referenced this issue Apr 21, 2022
* Merge code in `SceneSpawner` for managing `DynamicScene` and `Scene`.
  * Add back hot reloading for `Scene`s, fixes bevyengine#3759
  * Add ability to despawn `Scene`s
  * Add ability to spawn `DynamicScene` as child of existing entities
* Add documentation to `SceneSpawner` methods.
* Add `write_to_world` method to `Scene` (does the same
  as `DynamicScene::write_to_world`)

This fixes bevyengine#3759 see this commit's merging PR for further details about
this change.
nicopap added a commit to nicopap/bevy that referenced this issue Apr 21, 2022
* Merge code in `SceneSpawner` for managing `DynamicScene` and `Scene`.
  * Add back hot reloading for `Scene`s, fixes bevyengine#3759
  * Add ability to despawn `Scene`s
  * Add ability to spawn `DynamicScene` as child of existing entities
* Add documentation to `SceneSpawner` methods.
* Add `write_to_world` method to `Scene` (does the same
  as `DynamicScene::write_to_world`)

This fixes bevyengine#3759 see this commit's merging PR for further details about
this change.

Merge PR: bevyengine#4552
nicopap added a commit to nicopap/bevy that referenced this issue Apr 27, 2022
* Merge code in `SceneSpawner` for managing `DynamicScene` and `Scene`.
  * Add back hot reloading for `Scene`s, fixes bevyengine#3759
  * Add ability to despawn `Scene`s
  * Add ability to spawn `DynamicScene` as child of existing entities
* Add documentation to `SceneSpawner` methods.
* Add `write_to_world` method to `Scene` (does the same
  as `DynamicScene::write_to_world`)

This fixes bevyengine#3759 see this commit's merging PR for further details about
this change.

Merge PR: bevyengine#4552
nicopap added a commit to nicopap/bevy that referenced this issue Jun 18, 2022
* Merge code in `SceneSpawner` for managing `DynamicScene` and `Scene`.
  * Add back hot reloading for `Scene`s, fixes bevyengine#3759
* Add documentation to `SceneSpawner` methods.
* Add `write_to_world` method to `Scene` (does the same
  as `DynamicScene::write_to_world`)

This fixes bevyengine#3759 see this commit's merging PR for further details about
this change.

Merge PR: bevyengine#4552

A previous version of this change merged `SceneSpawner::*{,_dynamic}`,
it made it too difficult to use the `SceneSpawner` so it was reverted.
slyedoc added a commit to slyedoc/bevy_sly_blender that referenced this issue Jul 20, 2024
…e issue when no blueprints, put my Spawn commands behind "nesting" feature since my child parent logic was messed up, good enough for now, still dont have hotreload for scene, see bevyengine/bevy#3759
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Assets Load files from disk to use for things like images, models, and sounds C-Bug An unexpected or incorrect behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants