Reframe old "scene" terminology as "world serialization"#23630
Reframe old "scene" terminology as "world serialization"#23630cart wants to merge 3 commits intobevyengine:mainfrom
Conversation
alice-i-cecile
added
X-Blessed
|
I like these names a lot: I think the framing is a lot clearer, and it should help users both differentiate it from bevy_scene and figure out useful ways to use these types. |
|
You added a new feature but didn't update the readme. Please run |
urben1680
left a comment
urben1680
left a comment
There was a problem hiding this comment.
I never worked with scenes and did not attempt to learn them and the logic in the code with this review. I basically just made my browser mark the word "scene" so I notice when there are mentions outside of changed lines. Because of that I obviously only noticed nearby code needing adjustments.
Two things generally:
- Are the changes too many to do a deprecation cycle where old names are available for 0.19?
- There are soo many things still named with "scene" in the gltf part that I think this is intentional because a bns port is planned this cycle?
| /// | ||
| /// Note: This was recently renamed from `WorldAssetRoot`, in the interest of giving "scene" terminology to | ||
| /// Bevy's next generation scene system, available in `bevy_scene`. |
There was a problem hiding this comment.
Can't wait to read this in 5 years. 😄 How are things usually tracked that should be removed later on? a //todo comment, an issue?
| let registry = world.resource::<AppTypeRegistry>().clone(); | ||
| self.write_to_world_with(world, entity_map, ®istry.read()) | ||
| } | ||
|
|
||
| // TODO: move to AssetSaver when it is implemented | ||
| /// Serialize this dynamic scene into the official Bevy scene format (`.scn` / `.scn.ron`). |
There was a problem hiding this comment.
it was all a lie, so much about this being the "official bevy scene format"!
| @@ -37,15 +37,15 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) { | |||
| // Spawn the scene as a child of this entity at the given transform | |||
There was a problem hiding this comment.
This and many other comments in examples (including their filenames) still speak of "scenes", is this intentional?
| @@ -203,9 +203,9 @@ pub struct AnimationSystems; | |||
| #[derive(Debug, Hash, PartialEq, Eq, Clone, SystemSet)] | |||
| pub enum SceneSpawnerSystems { | |||
There was a problem hiding this comment.
The enum name might have to be renamed too.
| /// through reflection. If the `world` is the "real" world (e.g., not a world in a | ||
| /// [`Scene`](crate::Scene)), the `world` will contain the registry, which can be acquired using | ||
| /// [`WorldAsset`](crate::WorldAsset)), the `world` will contain the registry, which can be acquired using | ||
| /// `world.resource::<AppTypeRegistry>().read()`. For extracting from "scene worlds", you |
There was a problem hiding this comment.
"scene worlds" should be "worlds from serialization"? "serialized worlds"?
| // update them to the entities in the world. | ||
| // If this component references entities in the source world, | ||
| // update them to the entities in the destination world. | ||
| SceneEntityMapper::world_scope(entity_map, world, |world, mapper| { |
There was a problem hiding this comment.
Should SceneEntityMapper also be renamed?
| @@ -71,7 +71,7 @@ impl InstanceId { | |||
| /// - [`despawn_queued_scenes`](Self::despawn_queued_scenes) | |||
| /// - [`despawn_queued_instances`](Self::despawn_queued_instances) | |||
There was a problem hiding this comment.
Quite some things still called scenes here.
EDIT: Note I marked L60-72, the two lines shown here coincidentally are okay.
There was a problem hiding this comment.
I found a handful of other unchanged "scene" vocabulary in this file, you might want to ctrl+f them.
| )); | ||
| } | ||
|
|
||
| fn spawn_scene( |
There was a problem hiding this comment.
This and some other things in this test are named with "scene".
3 participants
Part 2 of #23619
In Bevy 0.19 we are landing a subset of Bevy's Next Generation Scene system (often known as BSN), which now lives in the
bevy_scene/bevy::scenecrate. However the oldbevy_scenesystem still needs to stick around for a bit longer, as it provides some features that Bevy's Next Generation Scene system doesn't (yet!):For this reason, we have renamed the old
bevy_scenecrate tobevy_world_serialization. If you were referencingbevy_scene::*orbevy::scene::*types, rename those paths tobevy_world_serialization::*andbevy::world_serialization::*respectively.Additionally, to avoid confusion / conflicts with the new scene system, all "scene" terminology / types have been reframed as "world serialization":
Scene->WorldAsset(as this was always just a World wrapper)SceneRoot->WorldAssetRootDynamicScene->DynamicWorldDynamicScene::from_scene->DynamicWorld::from_world_assetDynamicSceneBuilder->DynamicWorldBuilderDynamicSceneRoot->DynamicWorldRootSceneInstanceReady->WorldInstanceReadySceneLoader->WorldAssetLoaderScenePlugin->WorldSerializationPluginSceneRootTemplate->WorldAssetRootTemplateSceneSpawner->WorldInstanceSpawnerSceneFilter->WorldFilterSceneLoaderError->WorldAssetLoaderErrorSceneSpawnError->WorldInstanceSpawnErrorNote that I went with
bevy_world_serializationoverbevy_ecs_serialization, as that is what all of the internal features described themselves as. I think it is both more specific and does a better job of making itself decoupled frombevy_ecsproper.