bevy/crates/bevy_scene/src/dynamic_scene_builder.rs

734 lines
26 KiB
Rust
Raw Normal View History

bevy_scene: Add `SceneFilter` (#6793) # Objective Currently, `DynamicScene`s extract all components listed in the given (or the world's) type registry. This acts as a quasi-filter of sorts. However, it can be troublesome to use effectively and lacks decent control. For example, say you need to serialize only the following component over the network: ```rust #[derive(Reflect, Component, Default)] #[reflect(Component)] struct NPC { name: Option<String> } ``` To do this, you'd need to: 1. Create a new `AppTypeRegistry` 2. Register `NPC` 3. Register `Option<String>` If we skip Step 3, then the entire scene might fail to serialize as `Option<String>` requires registration. Not only is this annoying and easy to forget, but it can leave users with an impossible task: serializing a third-party type that contains private types. Generally, the third-party crate will register their private types within a plugin so the user doesn't need to do it themselves. However, this means we are now unable to serialize _just_ that type— we're forced to allow everything! ## Solution Add the `SceneFilter` enum for filtering components to extract. This filter can be used to optionally allow or deny entire sets of components/resources. With the `DynamicSceneBuilder`, users have more control over how their `DynamicScene`s are built. To only serialize a subset of components, use the `allow` method: ```rust let scene = builder .allow::<ComponentA>() .allow::<ComponentB>() .extract_entity(entity) .build(); ``` To serialize everything _but_ a subset of components, use the `deny` method: ```rust let scene = builder .deny::<ComponentA>() .deny::<ComponentB>() .extract_entity(entity) .build(); ``` Or create a custom filter: ```rust let components = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(components); // let filter = SceneFilter::Denylist(components); let scene = builder .with_filter(Some(filter)) .extract_entity(entity) .build(); ``` Similar operations exist for resources: <details> <summary>View Resource Methods</summary> To only serialize a subset of resources, use the `allow_resource` method: ```rust let scene = builder .allow_resource::<ResourceA>() .extract_resources() .build(); ``` To serialize everything _but_ a subset of resources, use the `deny_resource` method: ```rust let scene = builder .deny_resource::<ResourceA>() .extract_resources() .build(); ``` Or create a custom filter: ```rust let resources = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(resources); // let filter = SceneFilter::Denylist(resources); let scene = builder .with_resource_filter(Some(filter)) .extract_resources() .build(); ``` </details> ### Open Questions - [x] ~~`allow` and `deny` are mutually exclusive. Currently, they overwrite each other. Should this instead be a panic?~~ Took @soqb's suggestion and made it so that the opposing method simply removes that type from the list. - [x] ~~`DynamicSceneBuilder` extracts entity data as soon as `extract_entity`/`extract_entities` is called. Should this behavior instead be moved to the `build` method to prevent ordering mixups (e.g. `.allow::<Foo>().extract_entity(entity)` vs `.extract_entity(entity).allow::<Foo>()`)? The tradeoff would be iterating over the given entities twice: once at extraction and again at build.~~ Based on the feedback from @Testare it sounds like it might be better to just keep the current functionality (if anything we can open a separate PR that adds deferred methods for extraction, so the choice/performance hit is up to the user). - [ ] An alternative might be to remove the filter from `DynamicSceneBuilder` and have it as a separate parameter to the extraction methods (either in the existing ones or as added `extract_entity_with_filter`-type methods). Is this preferable? - [x] ~~Should we include constructors that include common types to allow/deny? For example, a `SceneFilter::standard_allowlist` that includes things like `Parent` and `Children`?~~ Consensus suggests we should. I may split this out into a followup PR, though. - [x] ~~Should we add the ability to remove types from the filter regardless of whether an allowlist or denylist (e.g. `filter.remove::<Foo>()`)?~~ See the first list item - [x] ~~Should `SceneFilter` be an enum? Would it make more sense as a struct that contains an `is_denylist` boolean?~~ With the added `SceneFilter::None` state (replacing the need to wrap in an `Option` or rely on an empty `Denylist`), it seems an enum is better suited now - [x] ~~Bikeshed: Do we like the naming convention? Should we instead use `include`/`exclude` terminology?~~ Sounds like we're sticking with `allow`/`deny`! - [x] ~~Does this feature need a new example? Do we simply include it in the existing one (maybe even as a comment?)? Should this be done in a followup PR instead?~~ Example will be added in a followup PR ### Followup Tasks - [ ] Add a dedicated `SceneFilter` example - [ ] Possibly add default types to the filter (e.g. deny things like `ComputedVisibility`, allow `Parent`, etc) --- ## Changelog - Added the `SceneFilter` enum for filtering components and resources when building a `DynamicScene` - Added methods: - `DynamicSceneBuilder::with_filter` - `DynamicSceneBuilder::allow` - `DynamicSceneBuilder::deny` - `DynamicSceneBuilder::allow_all` - `DynamicSceneBuilder::deny_all` - `DynamicSceneBuilder::with_resource_filter` - `DynamicSceneBuilder::allow_resource` - `DynamicSceneBuilder::deny_resource` - `DynamicSceneBuilder::allow_all_resources` - `DynamicSceneBuilder::deny_all_resources` - Removed methods: - `DynamicSceneBuilder::from_world_with_type_registry` - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference ## Migration Guide - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let dynamic_scene = DynamicScene::from_world(&world, registry); // let dynamic_scene = DynamicScene::from_scene(&scene, registry); // NEW let dynamic_scene = DynamicScene::from_world(&world); // let dynamic_scene = DynamicScene::from_scene(&scene); ``` - Removed `DynamicSceneBuilder::from_world_with_type_registry`. Now the registry is automatically taken from the given world: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let builder = DynamicSceneBuilder::from_world_with_type_registry(&world, registry); // NEW let builder = DynamicSceneBuilder::from_world(&world); ```
2023-07-06 21:04:26 +00:00
use crate::{DynamicEntity, DynamicScene, SceneFilter};
Add `core` and `alloc` over `std` Lints (#15281) # Objective - Fixes #6370 - Closes #6581 ## Solution - Added the following lints to the workspace: - `std_instead_of_core` - `std_instead_of_alloc` - `alloc_instead_of_core` - Used `cargo +nightly fmt` with [item level use formatting](https://rust-lang.github.io/rustfmt/?version=v1.6.0&search=#Item%5C%3A) to split all `use` statements into single items. - Used `cargo clippy --workspace --all-targets --all-features --fix --allow-dirty` to _attempt_ to resolve the new linting issues, and intervened where the lint was unable to resolve the issue automatically (usually due to needing an `extern crate alloc;` statement in a crate root). - Manually removed certain uses of `std` where negative feature gating prevented `--all-features` from finding the offending uses. - Used `cargo +nightly fmt` with [crate level use formatting](https://rust-lang.github.io/rustfmt/?version=v1.6.0&search=#Crate%5C%3A) to re-merge all `use` statements matching Bevy's previous styling. - Manually fixed cases where the `fmt` tool could not re-merge `use` statements due to conditional compilation attributes. ## Testing - Ran CI locally ## Migration Guide The MSRV is now 1.81. Please update to this version or higher. ## Notes - This is a _massive_ change to try and push through, which is why I've outlined the semi-automatic steps I used to create this PR, in case this fails and someone else tries again in the future. - Making this change has no impact on user code, but does mean Bevy contributors will be warned to use `core` and `alloc` instead of `std` where possible. - This lint is a critical first step towards investigating `no_std` options for Bevy. --------- Co-authored-by: François Mockers <francois.mockers@vleue.com>
2024-09-27 00:59:59 +00:00
use alloc::collections::BTreeMap;
(De) serialize resources in scenes (#6846) # Objective Co-Authored-By: davier [bricedavier@gmail.com](mailto:bricedavier@gmail.com) Fixes #3576. Adds a `resources` field in scene serialization data to allow de/serializing resources that have reflection enabled. ## Solution Most of this code is taken from a previous closed PR: https://github.com/bevyengine/bevy/pull/3580. Most of the credit goes to @Davier , what I did was mostly getting it to work on the latest main branch of Bevy, along with adding a few asserts in the currently existing tests to be sure everything is working properly. This PR changes the scene format to include resources in this way: ``` ( resources: { // List of resources here, keyed by resource type name. }, entities: [ // Previous scene format here ], ) ``` An example taken from the tests: ``` ( resources: { "bevy_scene::serde::tests::MyResource": ( foo: 123, ), }, entities: { // Previous scene format here }, ) ``` For this, a `resources` fields has been added on the `DynamicScene` and the `DynamicSceneBuilder` structs. The latter now also has a method named `extract_resources` to properly extract the existing resources registered in the local type registry, in a similar way to `extract_entities`. --- ## Changelog Added: Reflect resources registered in the type registry used by dynamic scenes will now be properly de/serialized in scene data. ## Migration Guide Since the scene format has been changed, the user may not be able to use scenes saved prior to this PR due to the `resources` scene field being missing. ~~To preserve backwards compatibility, I will try to make the `resources` fully optional so that old scenes can be loaded without issue.~~ ## TODOs - [x] I may have to update a few doc blocks still referring to dynamic scenes as mere container of entities, since they now include resources as well. - [x] ~~I want to make the `resources` key optional, as specified in the Migration Guide, so that old scenes will be compatible with this change.~~ Since this would only be trivial for ron format, I think it might be better to consider it in a separate PR/discussion to figure out if it could be done for binary serialization too. - [x] I suppose it might be a good idea to add a resources in the scene example so that users will quickly notice they can serialize resources just like entities. --------- Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2023-03-20 21:17:02 +00:00
use bevy_ecs::{
component::{Component, ComponentId},
(De) serialize resources in scenes (#6846) # Objective Co-Authored-By: davier [bricedavier@gmail.com](mailto:bricedavier@gmail.com) Fixes #3576. Adds a `resources` field in scene serialization data to allow de/serializing resources that have reflection enabled. ## Solution Most of this code is taken from a previous closed PR: https://github.com/bevyengine/bevy/pull/3580. Most of the credit goes to @Davier , what I did was mostly getting it to work on the latest main branch of Bevy, along with adding a few asserts in the currently existing tests to be sure everything is working properly. This PR changes the scene format to include resources in this way: ``` ( resources: { // List of resources here, keyed by resource type name. }, entities: [ // Previous scene format here ], ) ``` An example taken from the tests: ``` ( resources: { "bevy_scene::serde::tests::MyResource": ( foo: 123, ), }, entities: { // Previous scene format here }, ) ``` For this, a `resources` fields has been added on the `DynamicScene` and the `DynamicSceneBuilder` structs. The latter now also has a method named `extract_resources` to properly extract the existing resources registered in the local type registry, in a similar way to `extract_entities`. --- ## Changelog Added: Reflect resources registered in the type registry used by dynamic scenes will now be properly de/serialized in scene data. ## Migration Guide Since the scene format has been changed, the user may not be able to use scenes saved prior to this PR due to the `resources` scene field being missing. ~~To preserve backwards compatibility, I will try to make the `resources` fully optional so that old scenes can be loaded without issue.~~ ## TODOs - [x] I may have to update a few doc blocks still referring to dynamic scenes as mere container of entities, since they now include resources as well. - [x] ~~I want to make the `resources` key optional, as specified in the Migration Guide, so that old scenes will be compatible with this change.~~ Since this would only be trivial for ron format, I think it might be better to consider it in a separate PR/discussion to figure out if it could be done for binary serialization too. - [x] I suppose it might be a good idea to add a resources in the scene example so that users will quickly notice they can serialize resources just like entities. --------- Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2023-03-20 21:17:02 +00:00
prelude::Entity,
reflect::{AppTypeRegistry, ReflectComponent, ReflectResource},
system::Resource,
(De) serialize resources in scenes (#6846) # Objective Co-Authored-By: davier [bricedavier@gmail.com](mailto:bricedavier@gmail.com) Fixes #3576. Adds a `resources` field in scene serialization data to allow de/serializing resources that have reflection enabled. ## Solution Most of this code is taken from a previous closed PR: https://github.com/bevyengine/bevy/pull/3580. Most of the credit goes to @Davier , what I did was mostly getting it to work on the latest main branch of Bevy, along with adding a few asserts in the currently existing tests to be sure everything is working properly. This PR changes the scene format to include resources in this way: ``` ( resources: { // List of resources here, keyed by resource type name. }, entities: [ // Previous scene format here ], ) ``` An example taken from the tests: ``` ( resources: { "bevy_scene::serde::tests::MyResource": ( foo: 123, ), }, entities: { // Previous scene format here }, ) ``` For this, a `resources` fields has been added on the `DynamicScene` and the `DynamicSceneBuilder` structs. The latter now also has a method named `extract_resources` to properly extract the existing resources registered in the local type registry, in a similar way to `extract_entities`. --- ## Changelog Added: Reflect resources registered in the type registry used by dynamic scenes will now be properly de/serialized in scene data. ## Migration Guide Since the scene format has been changed, the user may not be able to use scenes saved prior to this PR due to the `resources` scene field being missing. ~~To preserve backwards compatibility, I will try to make the `resources` fully optional so that old scenes can be loaded without issue.~~ ## TODOs - [x] I may have to update a few doc blocks still referring to dynamic scenes as mere container of entities, since they now include resources as well. - [x] ~~I want to make the `resources` key optional, as specified in the Migration Guide, so that old scenes will be compatible with this change.~~ Since this would only be trivial for ron format, I think it might be better to consider it in a separate PR/discussion to figure out if it could be done for binary serialization too. - [x] I suppose it might be a good idea to add a resources in the scene example so that users will quickly notice they can serialize resources just like entities. --------- Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2023-03-20 21:17:02 +00:00
world::World,
};
Use `FromReflect` when extracting entities in dynamic scenes (#15174) # Objective Fix #10284. ## Solution When `DynamicSceneBuilder` extracts entities, they are cloned via `PartialReflect::clone_value`, making them into dynamic versions of the original components. This loses any custom `ReflectSerialize` type data. Dynamic scenes are deserialized with the original types, not the dynamic versions, and so any component with a custom serialize may fail. In this case `Rect` and `Vec2`. The dynamic version includes the field names 'x' and 'y' but the `Serialize` impl doesn't, hence the "expect float" error. The solution here: Instead of using `clone_value` to clone the components, `FromReflect` clones and retains the original information needed to serialize with any custom `Serialize` impls. I think using something like `reflect_clone` from (https://github.com/bevyengine/bevy/pull/13432) might make this more efficient. I also did the same when deserializing dynamic scenes to appease some of the round-trip tests which use `ReflectPartialEq`, which requires the types be the same and not a unique/proxy pair. I'm not sure it's otherwise necessary. Maybe this would also be more efficient when spawning dynamic scenes with `reflect_clone` instead of `FromReflect` again? An alternative solution would be to fall back to the dynamic version when deserializing `DynamicScene`s if the custom version fails. I think that's possible. Or maybe simply always deserializing via the dynamic route for dynamic scenes? ## Testing This example is similar to the original test case in #10284: ``` rust #![allow(missing_docs)] use bevy::{prelude::*, scene::SceneInstanceReady}; fn main() { App::new() .add_plugins(DefaultPlugins) .add_systems(Startup, (save, load).chain()) .observe(check) .run(); } static SAVEGAME_SAVE_PATH: &str = "savegame.scn.ron"; fn save(world: &mut World) { let entity = world.spawn(OrthographicProjection::default()).id(); let scene = DynamicSceneBuilder::from_world(world) .extract_entity(entity) .build(); if let Some(registry) = world.get_resource::<AppTypeRegistry>() { let registry = registry.read(); let serialized_scene = scene.serialize(&registry).unwrap(); // println!("{}", serialized_scene); std::fs::write(format!("assets/{SAVEGAME_SAVE_PATH}"), serialized_scene).unwrap(); } world.entity_mut(entity).despawn_recursive(); } fn load(mut commands: Commands, asset_server: Res<AssetServer>) { commands.spawn(DynamicSceneBundle { scene: asset_server.load(SAVEGAME_SAVE_PATH), ..default() }); } fn check(_trigger: Trigger<SceneInstanceReady>, query: Query<&OrthographicProjection>) { dbg!(query.single()); } ``` ## Migration Guide The `DynamicScene` format is changed to use custom serialize impls so old scene files will need updating: Old: ```ron ( resources: {}, entities: { 4294967299: ( components: { "bevy_render::camera::projection::OrthographicProjection": ( near: 0.0, far: 1000.0, viewport_origin: ( x: 0.5, y: 0.5, ), scaling_mode: WindowSize(1.0), scale: 1.0, area: ( min: ( x: -1.0, y: -1.0, ), max: ( x: 1.0, y: 1.0, ), ), ), }, ), }, ) ``` New: ```ron ( resources: {}, entities: { 4294967299: ( components: { "bevy_render::camera::projection::OrthographicProjection": ( near: 0.0, far: 1000.0, viewport_origin: (0.5, 0.5), scaling_mode: WindowSize(1.0), scale: 1.0, area: ( min: (-1.0, -1.0), max: (1.0, 1.0), ), ), }, ), }, ) ``` --------- Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
2024-09-15 14:33:39 +00:00
use bevy_reflect::{PartialReflect, ReflectFromReflect};
use bevy_utils::default;
(De) serialize resources in scenes (#6846) # Objective Co-Authored-By: davier [bricedavier@gmail.com](mailto:bricedavier@gmail.com) Fixes #3576. Adds a `resources` field in scene serialization data to allow de/serializing resources that have reflection enabled. ## Solution Most of this code is taken from a previous closed PR: https://github.com/bevyengine/bevy/pull/3580. Most of the credit goes to @Davier , what I did was mostly getting it to work on the latest main branch of Bevy, along with adding a few asserts in the currently existing tests to be sure everything is working properly. This PR changes the scene format to include resources in this way: ``` ( resources: { // List of resources here, keyed by resource type name. }, entities: [ // Previous scene format here ], ) ``` An example taken from the tests: ``` ( resources: { "bevy_scene::serde::tests::MyResource": ( foo: 123, ), }, entities: { // Previous scene format here }, ) ``` For this, a `resources` fields has been added on the `DynamicScene` and the `DynamicSceneBuilder` structs. The latter now also has a method named `extract_resources` to properly extract the existing resources registered in the local type registry, in a similar way to `extract_entities`. --- ## Changelog Added: Reflect resources registered in the type registry used by dynamic scenes will now be properly de/serialized in scene data. ## Migration Guide Since the scene format has been changed, the user may not be able to use scenes saved prior to this PR due to the `resources` scene field being missing. ~~To preserve backwards compatibility, I will try to make the `resources` fully optional so that old scenes can be loaded without issue.~~ ## TODOs - [x] I may have to update a few doc blocks still referring to dynamic scenes as mere container of entities, since they now include resources as well. - [x] ~~I want to make the `resources` key optional, as specified in the Migration Guide, so that old scenes will be compatible with this change.~~ Since this would only be trivial for ron format, I think it might be better to consider it in a separate PR/discussion to figure out if it could be done for binary serialization too. - [x] I suppose it might be a good idea to add a resources in the scene example so that users will quickly notice they can serialize resources just like entities. --------- Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2023-03-20 21:17:02 +00:00
/// A [`DynamicScene`] builder, used to build a scene from a [`World`] by extracting some entities and resources.
///
bevy_scene: Add `SceneFilter` (#6793) # Objective Currently, `DynamicScene`s extract all components listed in the given (or the world's) type registry. This acts as a quasi-filter of sorts. However, it can be troublesome to use effectively and lacks decent control. For example, say you need to serialize only the following component over the network: ```rust #[derive(Reflect, Component, Default)] #[reflect(Component)] struct NPC { name: Option<String> } ``` To do this, you'd need to: 1. Create a new `AppTypeRegistry` 2. Register `NPC` 3. Register `Option<String>` If we skip Step 3, then the entire scene might fail to serialize as `Option<String>` requires registration. Not only is this annoying and easy to forget, but it can leave users with an impossible task: serializing a third-party type that contains private types. Generally, the third-party crate will register their private types within a plugin so the user doesn't need to do it themselves. However, this means we are now unable to serialize _just_ that type— we're forced to allow everything! ## Solution Add the `SceneFilter` enum for filtering components to extract. This filter can be used to optionally allow or deny entire sets of components/resources. With the `DynamicSceneBuilder`, users have more control over how their `DynamicScene`s are built. To only serialize a subset of components, use the `allow` method: ```rust let scene = builder .allow::<ComponentA>() .allow::<ComponentB>() .extract_entity(entity) .build(); ``` To serialize everything _but_ a subset of components, use the `deny` method: ```rust let scene = builder .deny::<ComponentA>() .deny::<ComponentB>() .extract_entity(entity) .build(); ``` Or create a custom filter: ```rust let components = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(components); // let filter = SceneFilter::Denylist(components); let scene = builder .with_filter(Some(filter)) .extract_entity(entity) .build(); ``` Similar operations exist for resources: <details> <summary>View Resource Methods</summary> To only serialize a subset of resources, use the `allow_resource` method: ```rust let scene = builder .allow_resource::<ResourceA>() .extract_resources() .build(); ``` To serialize everything _but_ a subset of resources, use the `deny_resource` method: ```rust let scene = builder .deny_resource::<ResourceA>() .extract_resources() .build(); ``` Or create a custom filter: ```rust let resources = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(resources); // let filter = SceneFilter::Denylist(resources); let scene = builder .with_resource_filter(Some(filter)) .extract_resources() .build(); ``` </details> ### Open Questions - [x] ~~`allow` and `deny` are mutually exclusive. Currently, they overwrite each other. Should this instead be a panic?~~ Took @soqb's suggestion and made it so that the opposing method simply removes that type from the list. - [x] ~~`DynamicSceneBuilder` extracts entity data as soon as `extract_entity`/`extract_entities` is called. Should this behavior instead be moved to the `build` method to prevent ordering mixups (e.g. `.allow::<Foo>().extract_entity(entity)` vs `.extract_entity(entity).allow::<Foo>()`)? The tradeoff would be iterating over the given entities twice: once at extraction and again at build.~~ Based on the feedback from @Testare it sounds like it might be better to just keep the current functionality (if anything we can open a separate PR that adds deferred methods for extraction, so the choice/performance hit is up to the user). - [ ] An alternative might be to remove the filter from `DynamicSceneBuilder` and have it as a separate parameter to the extraction methods (either in the existing ones or as added `extract_entity_with_filter`-type methods). Is this preferable? - [x] ~~Should we include constructors that include common types to allow/deny? For example, a `SceneFilter::standard_allowlist` that includes things like `Parent` and `Children`?~~ Consensus suggests we should. I may split this out into a followup PR, though. - [x] ~~Should we add the ability to remove types from the filter regardless of whether an allowlist or denylist (e.g. `filter.remove::<Foo>()`)?~~ See the first list item - [x] ~~Should `SceneFilter` be an enum? Would it make more sense as a struct that contains an `is_denylist` boolean?~~ With the added `SceneFilter::None` state (replacing the need to wrap in an `Option` or rely on an empty `Denylist`), it seems an enum is better suited now - [x] ~~Bikeshed: Do we like the naming convention? Should we instead use `include`/`exclude` terminology?~~ Sounds like we're sticking with `allow`/`deny`! - [x] ~~Does this feature need a new example? Do we simply include it in the existing one (maybe even as a comment?)? Should this be done in a followup PR instead?~~ Example will be added in a followup PR ### Followup Tasks - [ ] Add a dedicated `SceneFilter` example - [ ] Possibly add default types to the filter (e.g. deny things like `ComputedVisibility`, allow `Parent`, etc) --- ## Changelog - Added the `SceneFilter` enum for filtering components and resources when building a `DynamicScene` - Added methods: - `DynamicSceneBuilder::with_filter` - `DynamicSceneBuilder::allow` - `DynamicSceneBuilder::deny` - `DynamicSceneBuilder::allow_all` - `DynamicSceneBuilder::deny_all` - `DynamicSceneBuilder::with_resource_filter` - `DynamicSceneBuilder::allow_resource` - `DynamicSceneBuilder::deny_resource` - `DynamicSceneBuilder::allow_all_resources` - `DynamicSceneBuilder::deny_all_resources` - Removed methods: - `DynamicSceneBuilder::from_world_with_type_registry` - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference ## Migration Guide - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let dynamic_scene = DynamicScene::from_world(&world, registry); // let dynamic_scene = DynamicScene::from_scene(&scene, registry); // NEW let dynamic_scene = DynamicScene::from_world(&world); // let dynamic_scene = DynamicScene::from_scene(&scene); ``` - Removed `DynamicSceneBuilder::from_world_with_type_registry`. Now the registry is automatically taken from the given world: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let builder = DynamicSceneBuilder::from_world_with_type_registry(&world, registry); // NEW let builder = DynamicSceneBuilder::from_world(&world); ```
2023-07-06 21:04:26 +00:00
/// # Component Extraction
///
/// By default, all components registered with [`ReflectComponent`] type data in a world's [`AppTypeRegistry`] will be extracted.
/// (this type data is added automatically during registration if [`Reflect`] is derived with the `#[reflect(Component)]` attribute).
/// This can be changed by [specifying a filter](DynamicSceneBuilder::with_component_filter) or by explicitly
/// [allowing](DynamicSceneBuilder::allow_component)/[denying](DynamicSceneBuilder::deny_component) certain components.
bevy_scene: Add `SceneFilter` (#6793) # Objective Currently, `DynamicScene`s extract all components listed in the given (or the world's) type registry. This acts as a quasi-filter of sorts. However, it can be troublesome to use effectively and lacks decent control. For example, say you need to serialize only the following component over the network: ```rust #[derive(Reflect, Component, Default)] #[reflect(Component)] struct NPC { name: Option<String> } ``` To do this, you'd need to: 1. Create a new `AppTypeRegistry` 2. Register `NPC` 3. Register `Option<String>` If we skip Step 3, then the entire scene might fail to serialize as `Option<String>` requires registration. Not only is this annoying and easy to forget, but it can leave users with an impossible task: serializing a third-party type that contains private types. Generally, the third-party crate will register their private types within a plugin so the user doesn't need to do it themselves. However, this means we are now unable to serialize _just_ that type— we're forced to allow everything! ## Solution Add the `SceneFilter` enum for filtering components to extract. This filter can be used to optionally allow or deny entire sets of components/resources. With the `DynamicSceneBuilder`, users have more control over how their `DynamicScene`s are built. To only serialize a subset of components, use the `allow` method: ```rust let scene = builder .allow::<ComponentA>() .allow::<ComponentB>() .extract_entity(entity) .build(); ``` To serialize everything _but_ a subset of components, use the `deny` method: ```rust let scene = builder .deny::<ComponentA>() .deny::<ComponentB>() .extract_entity(entity) .build(); ``` Or create a custom filter: ```rust let components = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(components); // let filter = SceneFilter::Denylist(components); let scene = builder .with_filter(Some(filter)) .extract_entity(entity) .build(); ``` Similar operations exist for resources: <details> <summary>View Resource Methods</summary> To only serialize a subset of resources, use the `allow_resource` method: ```rust let scene = builder .allow_resource::<ResourceA>() .extract_resources() .build(); ``` To serialize everything _but_ a subset of resources, use the `deny_resource` method: ```rust let scene = builder .deny_resource::<ResourceA>() .extract_resources() .build(); ``` Or create a custom filter: ```rust let resources = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(resources); // let filter = SceneFilter::Denylist(resources); let scene = builder .with_resource_filter(Some(filter)) .extract_resources() .build(); ``` </details> ### Open Questions - [x] ~~`allow` and `deny` are mutually exclusive. Currently, they overwrite each other. Should this instead be a panic?~~ Took @soqb's suggestion and made it so that the opposing method simply removes that type from the list. - [x] ~~`DynamicSceneBuilder` extracts entity data as soon as `extract_entity`/`extract_entities` is called. Should this behavior instead be moved to the `build` method to prevent ordering mixups (e.g. `.allow::<Foo>().extract_entity(entity)` vs `.extract_entity(entity).allow::<Foo>()`)? The tradeoff would be iterating over the given entities twice: once at extraction and again at build.~~ Based on the feedback from @Testare it sounds like it might be better to just keep the current functionality (if anything we can open a separate PR that adds deferred methods for extraction, so the choice/performance hit is up to the user). - [ ] An alternative might be to remove the filter from `DynamicSceneBuilder` and have it as a separate parameter to the extraction methods (either in the existing ones or as added `extract_entity_with_filter`-type methods). Is this preferable? - [x] ~~Should we include constructors that include common types to allow/deny? For example, a `SceneFilter::standard_allowlist` that includes things like `Parent` and `Children`?~~ Consensus suggests we should. I may split this out into a followup PR, though. - [x] ~~Should we add the ability to remove types from the filter regardless of whether an allowlist or denylist (e.g. `filter.remove::<Foo>()`)?~~ See the first list item - [x] ~~Should `SceneFilter` be an enum? Would it make more sense as a struct that contains an `is_denylist` boolean?~~ With the added `SceneFilter::None` state (replacing the need to wrap in an `Option` or rely on an empty `Denylist`), it seems an enum is better suited now - [x] ~~Bikeshed: Do we like the naming convention? Should we instead use `include`/`exclude` terminology?~~ Sounds like we're sticking with `allow`/`deny`! - [x] ~~Does this feature need a new example? Do we simply include it in the existing one (maybe even as a comment?)? Should this be done in a followup PR instead?~~ Example will be added in a followup PR ### Followup Tasks - [ ] Add a dedicated `SceneFilter` example - [ ] Possibly add default types to the filter (e.g. deny things like `ComputedVisibility`, allow `Parent`, etc) --- ## Changelog - Added the `SceneFilter` enum for filtering components and resources when building a `DynamicScene` - Added methods: - `DynamicSceneBuilder::with_filter` - `DynamicSceneBuilder::allow` - `DynamicSceneBuilder::deny` - `DynamicSceneBuilder::allow_all` - `DynamicSceneBuilder::deny_all` - `DynamicSceneBuilder::with_resource_filter` - `DynamicSceneBuilder::allow_resource` - `DynamicSceneBuilder::deny_resource` - `DynamicSceneBuilder::allow_all_resources` - `DynamicSceneBuilder::deny_all_resources` - Removed methods: - `DynamicSceneBuilder::from_world_with_type_registry` - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference ## Migration Guide - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let dynamic_scene = DynamicScene::from_world(&world, registry); // let dynamic_scene = DynamicScene::from_scene(&scene, registry); // NEW let dynamic_scene = DynamicScene::from_world(&world); // let dynamic_scene = DynamicScene::from_scene(&scene); ``` - Removed `DynamicSceneBuilder::from_world_with_type_registry`. Now the registry is automatically taken from the given world: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let builder = DynamicSceneBuilder::from_world_with_type_registry(&world, registry); // NEW let builder = DynamicSceneBuilder::from_world(&world); ```
2023-07-06 21:04:26 +00:00
///
/// Extraction happens immediately and uses the filter as it exists during the time of extraction.
///
/// # Resource Extraction
///
/// By default, all resources registered with [`ReflectResource`] type data in a world's [`AppTypeRegistry`] will be extracted.
/// (this type data is added automatically during registration if [`Reflect`] is derived with the `#[reflect(Resource)]` attribute).
/// This can be changed by [specifying a filter](DynamicSceneBuilder::with_resource_filter) or by explicitly
/// [allowing](DynamicSceneBuilder::allow_resource)/[denying](DynamicSceneBuilder::deny_resource) certain resources.
///
/// Extraction happens immediately and uses the filter as it exists during the time of extraction.
///
/// # Entity Order
///
bevy_scene: Add `SceneFilter` (#6793) # Objective Currently, `DynamicScene`s extract all components listed in the given (or the world's) type registry. This acts as a quasi-filter of sorts. However, it can be troublesome to use effectively and lacks decent control. For example, say you need to serialize only the following component over the network: ```rust #[derive(Reflect, Component, Default)] #[reflect(Component)] struct NPC { name: Option<String> } ``` To do this, you'd need to: 1. Create a new `AppTypeRegistry` 2. Register `NPC` 3. Register `Option<String>` If we skip Step 3, then the entire scene might fail to serialize as `Option<String>` requires registration. Not only is this annoying and easy to forget, but it can leave users with an impossible task: serializing a third-party type that contains private types. Generally, the third-party crate will register their private types within a plugin so the user doesn't need to do it themselves. However, this means we are now unable to serialize _just_ that type— we're forced to allow everything! ## Solution Add the `SceneFilter` enum for filtering components to extract. This filter can be used to optionally allow or deny entire sets of components/resources. With the `DynamicSceneBuilder`, users have more control over how their `DynamicScene`s are built. To only serialize a subset of components, use the `allow` method: ```rust let scene = builder .allow::<ComponentA>() .allow::<ComponentB>() .extract_entity(entity) .build(); ``` To serialize everything _but_ a subset of components, use the `deny` method: ```rust let scene = builder .deny::<ComponentA>() .deny::<ComponentB>() .extract_entity(entity) .build(); ``` Or create a custom filter: ```rust let components = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(components); // let filter = SceneFilter::Denylist(components); let scene = builder .with_filter(Some(filter)) .extract_entity(entity) .build(); ``` Similar operations exist for resources: <details> <summary>View Resource Methods</summary> To only serialize a subset of resources, use the `allow_resource` method: ```rust let scene = builder .allow_resource::<ResourceA>() .extract_resources() .build(); ``` To serialize everything _but_ a subset of resources, use the `deny_resource` method: ```rust let scene = builder .deny_resource::<ResourceA>() .extract_resources() .build(); ``` Or create a custom filter: ```rust let resources = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(resources); // let filter = SceneFilter::Denylist(resources); let scene = builder .with_resource_filter(Some(filter)) .extract_resources() .build(); ``` </details> ### Open Questions - [x] ~~`allow` and `deny` are mutually exclusive. Currently, they overwrite each other. Should this instead be a panic?~~ Took @soqb's suggestion and made it so that the opposing method simply removes that type from the list. - [x] ~~`DynamicSceneBuilder` extracts entity data as soon as `extract_entity`/`extract_entities` is called. Should this behavior instead be moved to the `build` method to prevent ordering mixups (e.g. `.allow::<Foo>().extract_entity(entity)` vs `.extract_entity(entity).allow::<Foo>()`)? The tradeoff would be iterating over the given entities twice: once at extraction and again at build.~~ Based on the feedback from @Testare it sounds like it might be better to just keep the current functionality (if anything we can open a separate PR that adds deferred methods for extraction, so the choice/performance hit is up to the user). - [ ] An alternative might be to remove the filter from `DynamicSceneBuilder` and have it as a separate parameter to the extraction methods (either in the existing ones or as added `extract_entity_with_filter`-type methods). Is this preferable? - [x] ~~Should we include constructors that include common types to allow/deny? For example, a `SceneFilter::standard_allowlist` that includes things like `Parent` and `Children`?~~ Consensus suggests we should. I may split this out into a followup PR, though. - [x] ~~Should we add the ability to remove types from the filter regardless of whether an allowlist or denylist (e.g. `filter.remove::<Foo>()`)?~~ See the first list item - [x] ~~Should `SceneFilter` be an enum? Would it make more sense as a struct that contains an `is_denylist` boolean?~~ With the added `SceneFilter::None` state (replacing the need to wrap in an `Option` or rely on an empty `Denylist`), it seems an enum is better suited now - [x] ~~Bikeshed: Do we like the naming convention? Should we instead use `include`/`exclude` terminology?~~ Sounds like we're sticking with `allow`/`deny`! - [x] ~~Does this feature need a new example? Do we simply include it in the existing one (maybe even as a comment?)? Should this be done in a followup PR instead?~~ Example will be added in a followup PR ### Followup Tasks - [ ] Add a dedicated `SceneFilter` example - [ ] Possibly add default types to the filter (e.g. deny things like `ComputedVisibility`, allow `Parent`, etc) --- ## Changelog - Added the `SceneFilter` enum for filtering components and resources when building a `DynamicScene` - Added methods: - `DynamicSceneBuilder::with_filter` - `DynamicSceneBuilder::allow` - `DynamicSceneBuilder::deny` - `DynamicSceneBuilder::allow_all` - `DynamicSceneBuilder::deny_all` - `DynamicSceneBuilder::with_resource_filter` - `DynamicSceneBuilder::allow_resource` - `DynamicSceneBuilder::deny_resource` - `DynamicSceneBuilder::allow_all_resources` - `DynamicSceneBuilder::deny_all_resources` - Removed methods: - `DynamicSceneBuilder::from_world_with_type_registry` - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference ## Migration Guide - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let dynamic_scene = DynamicScene::from_world(&world, registry); // let dynamic_scene = DynamicScene::from_scene(&scene, registry); // NEW let dynamic_scene = DynamicScene::from_world(&world); // let dynamic_scene = DynamicScene::from_scene(&scene); ``` - Removed `DynamicSceneBuilder::from_world_with_type_registry`. Now the registry is automatically taken from the given world: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let builder = DynamicSceneBuilder::from_world_with_type_registry(&world, registry); // NEW let builder = DynamicSceneBuilder::from_world(&world); ```
2023-07-06 21:04:26 +00:00
/// Extracted entities will always be stored in ascending order based on their [index](Entity::index).
/// This means that inserting `Entity(1v0)` then `Entity(0v0)` will always result in the entities
/// being ordered as `[Entity(0v0), Entity(1v0)]`.
///
/// # Example
/// ```
/// # use bevy_scene::DynamicSceneBuilder;
/// # use bevy_ecs::reflect::AppTypeRegistry;
/// # use bevy_ecs::{
/// # component::Component, prelude::Entity, query::With, reflect::ReflectComponent, world::World,
/// # };
/// # use bevy_reflect::Reflect;
/// # #[derive(Component, Reflect, Default, Eq, PartialEq, Debug)]
/// # #[reflect(Component)]
/// # struct ComponentA;
/// # let mut world = World::default();
/// # world.init_resource::<AppTypeRegistry>();
/// # let entity = world.spawn(ComponentA).id();
/// let dynamic_scene = DynamicSceneBuilder::from_world(&world).extract_entity(entity).build();
/// ```
reflect: implement the unique reflect rfc (#7207) # Objective - Implements the [Unique Reflect RFC](https://github.com/nicopap/rfcs/blob/bevy-reflect-api/rfcs/56-better-reflect.md). ## Solution - Implements the RFC. - This implementation differs in some ways from the RFC: - In the RFC, it was suggested `Reflect: Any` but `PartialReflect: ?Any`. During initial implementation I tried this, but we assume the `PartialReflect: 'static` in a lot of places and the changes required crept out of the scope of this PR. - `PartialReflect::try_into_reflect` originally returned `Option<Box<dyn Reflect>>` but i changed this to `Result<Box<dyn Reflect>, Box<dyn PartialReflect>>` since the method takes by value and otherwise there would be no way to recover the type. `as_full` and `as_full_mut` both still return `Option<&(mut) dyn Reflect>`. --- ## Changelog - Added `PartialReflect`. - `Reflect` is now a subtrait of `PartialReflect`. - Moved most methods on `Reflect` to the new `PartialReflect`. - Added `PartialReflect::{as_partial_reflect, as_partial_reflect_mut, into_partial_reflect}`. - Added `PartialReflect::{try_as_reflect, try_as_reflect_mut, try_into_reflect}`. - Added `<dyn PartialReflect>::{try_downcast_ref, try_downcast_mut, try_downcast, try_take}` supplementing the methods on `dyn Reflect`. ## Migration Guide - Most instances of `dyn Reflect` should be changed to `dyn PartialReflect` which is less restrictive, however trait bounds should generally stay as `T: Reflect`. - The new `PartialReflect::{as_partial_reflect, as_partial_reflect_mut, into_partial_reflect, try_as_reflect, try_as_reflect_mut, try_into_reflect}` methods as well as `Reflect::{as_reflect, as_reflect_mut, into_reflect}` will need to be implemented for manual implementors of `Reflect`. ## Future Work - This PR is designed to be followed up by another "Unique Reflect Phase 2" that addresses the following points: - Investigate making serialization revolve around `Reflect` instead of `PartialReflect`. - [Remove the `try_*` methods on `dyn PartialReflect` since they are stop gaps](https://github.com/bevyengine/bevy/pull/7207#discussion_r1083476050). - Investigate usages like `ReflectComponent`. In the places they currently use `PartialReflect`, should they be changed to use `Reflect`? - Merging this opens the door to lots of reflection features we haven't been able to implement. - We could re-add [the `Reflectable` trait](https://github.com/bevyengine/bevy/blob/8e3488c88065a94a4f72199587e59341c9b6553d/crates/bevy_reflect/src/reflect.rs#L337-L342) and make `FromReflect` a requirement to improve [`FromReflect` ergonomics](https://github.com/bevyengine/rfcs/pull/59). This is currently not possible because dynamic types cannot sensibly be `FromReflect`. - Since this is an alternative to #5772, #5781 would be made cleaner. --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
2024-08-12 17:01:41 +00:00
///
/// [`Reflect`]: bevy_reflect::Reflect
pub struct DynamicSceneBuilder<'w> {
reflect: implement the unique reflect rfc (#7207) # Objective - Implements the [Unique Reflect RFC](https://github.com/nicopap/rfcs/blob/bevy-reflect-api/rfcs/56-better-reflect.md). ## Solution - Implements the RFC. - This implementation differs in some ways from the RFC: - In the RFC, it was suggested `Reflect: Any` but `PartialReflect: ?Any`. During initial implementation I tried this, but we assume the `PartialReflect: 'static` in a lot of places and the changes required crept out of the scope of this PR. - `PartialReflect::try_into_reflect` originally returned `Option<Box<dyn Reflect>>` but i changed this to `Result<Box<dyn Reflect>, Box<dyn PartialReflect>>` since the method takes by value and otherwise there would be no way to recover the type. `as_full` and `as_full_mut` both still return `Option<&(mut) dyn Reflect>`. --- ## Changelog - Added `PartialReflect`. - `Reflect` is now a subtrait of `PartialReflect`. - Moved most methods on `Reflect` to the new `PartialReflect`. - Added `PartialReflect::{as_partial_reflect, as_partial_reflect_mut, into_partial_reflect}`. - Added `PartialReflect::{try_as_reflect, try_as_reflect_mut, try_into_reflect}`. - Added `<dyn PartialReflect>::{try_downcast_ref, try_downcast_mut, try_downcast, try_take}` supplementing the methods on `dyn Reflect`. ## Migration Guide - Most instances of `dyn Reflect` should be changed to `dyn PartialReflect` which is less restrictive, however trait bounds should generally stay as `T: Reflect`. - The new `PartialReflect::{as_partial_reflect, as_partial_reflect_mut, into_partial_reflect, try_as_reflect, try_as_reflect_mut, try_into_reflect}` methods as well as `Reflect::{as_reflect, as_reflect_mut, into_reflect}` will need to be implemented for manual implementors of `Reflect`. ## Future Work - This PR is designed to be followed up by another "Unique Reflect Phase 2" that addresses the following points: - Investigate making serialization revolve around `Reflect` instead of `PartialReflect`. - [Remove the `try_*` methods on `dyn PartialReflect` since they are stop gaps](https://github.com/bevyengine/bevy/pull/7207#discussion_r1083476050). - Investigate usages like `ReflectComponent`. In the places they currently use `PartialReflect`, should they be changed to use `Reflect`? - Merging this opens the door to lots of reflection features we haven't been able to implement. - We could re-add [the `Reflectable` trait](https://github.com/bevyengine/bevy/blob/8e3488c88065a94a4f72199587e59341c9b6553d/crates/bevy_reflect/src/reflect.rs#L337-L342) and make `FromReflect` a requirement to improve [`FromReflect` ergonomics](https://github.com/bevyengine/rfcs/pull/59). This is currently not possible because dynamic types cannot sensibly be `FromReflect`. - Since this is an alternative to #5772, #5781 would be made cleaner. --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
2024-08-12 17:01:41 +00:00
extracted_resources: BTreeMap<ComponentId, Box<dyn PartialReflect>>,
Make scene handling of entity references robust (#7335) # Objective - Handle dangling entity references inside scenes - Handle references to entities with generation > 0 inside scenes - Fix a latent bug in `Parent`'s `MapEntities` implementation, which would, if the parent was outside the scene, cause the scene to be loaded into the new world with a parent reference potentially pointing to some random entity in that new world. - Fixes #4793 and addresses #7235 ## Solution - DynamicScenes now identify entities with a `Entity` instead of a u32, therefore including generation - `World` exposes a new `reserve_generations` function that despawns an entity and advances its generation by some extra amount. - `MapEntities` implementations have a new `get_or_reserve` function available that will always return an `Entity`, establishing a new mapping to a dead entity when the entity they are called with is not in the `EntityMap`. Subsequent calls with that same `Entity` will return the same newly created dead entity reference, preserving equality semantics. - As a result, after loading a scene containing references to dead entities (or entities otherwise outside the scene), those references will all point to different generations on a single entity id in the new world. --- ## Changelog ### Changed - In serialized scenes, entities are now identified by a u64 instead of a u32. - In serialized scenes, components with entity references now have those references serialize as u64s instead of structs. ### Fixed - Scenes containing components with entity references will now deserialize and add to a world reliably. ## Migration Guide - `MapEntities` implementations must change from a `&EntityMap` parameter to a `&mut EntityMapper` parameter and can no longer return a `Result`. Finally, they should switch from calling `EntityMap::get` to calling `EntityMapper::get_or_reserve`. --------- Co-authored-by: Nicola Papale <nicopap@users.noreply.github.com>
2023-05-01 15:49:27 +00:00
extracted_scene: BTreeMap<Entity, DynamicEntity>,
bevy_scene: Add `SceneFilter` (#6793) # Objective Currently, `DynamicScene`s extract all components listed in the given (or the world's) type registry. This acts as a quasi-filter of sorts. However, it can be troublesome to use effectively and lacks decent control. For example, say you need to serialize only the following component over the network: ```rust #[derive(Reflect, Component, Default)] #[reflect(Component)] struct NPC { name: Option<String> } ``` To do this, you'd need to: 1. Create a new `AppTypeRegistry` 2. Register `NPC` 3. Register `Option<String>` If we skip Step 3, then the entire scene might fail to serialize as `Option<String>` requires registration. Not only is this annoying and easy to forget, but it can leave users with an impossible task: serializing a third-party type that contains private types. Generally, the third-party crate will register their private types within a plugin so the user doesn't need to do it themselves. However, this means we are now unable to serialize _just_ that type— we're forced to allow everything! ## Solution Add the `SceneFilter` enum for filtering components to extract. This filter can be used to optionally allow or deny entire sets of components/resources. With the `DynamicSceneBuilder`, users have more control over how their `DynamicScene`s are built. To only serialize a subset of components, use the `allow` method: ```rust let scene = builder .allow::<ComponentA>() .allow::<ComponentB>() .extract_entity(entity) .build(); ``` To serialize everything _but_ a subset of components, use the `deny` method: ```rust let scene = builder .deny::<ComponentA>() .deny::<ComponentB>() .extract_entity(entity) .build(); ``` Or create a custom filter: ```rust let components = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(components); // let filter = SceneFilter::Denylist(components); let scene = builder .with_filter(Some(filter)) .extract_entity(entity) .build(); ``` Similar operations exist for resources: <details> <summary>View Resource Methods</summary> To only serialize a subset of resources, use the `allow_resource` method: ```rust let scene = builder .allow_resource::<ResourceA>() .extract_resources() .build(); ``` To serialize everything _but_ a subset of resources, use the `deny_resource` method: ```rust let scene = builder .deny_resource::<ResourceA>() .extract_resources() .build(); ``` Or create a custom filter: ```rust let resources = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(resources); // let filter = SceneFilter::Denylist(resources); let scene = builder .with_resource_filter(Some(filter)) .extract_resources() .build(); ``` </details> ### Open Questions - [x] ~~`allow` and `deny` are mutually exclusive. Currently, they overwrite each other. Should this instead be a panic?~~ Took @soqb's suggestion and made it so that the opposing method simply removes that type from the list. - [x] ~~`DynamicSceneBuilder` extracts entity data as soon as `extract_entity`/`extract_entities` is called. Should this behavior instead be moved to the `build` method to prevent ordering mixups (e.g. `.allow::<Foo>().extract_entity(entity)` vs `.extract_entity(entity).allow::<Foo>()`)? The tradeoff would be iterating over the given entities twice: once at extraction and again at build.~~ Based on the feedback from @Testare it sounds like it might be better to just keep the current functionality (if anything we can open a separate PR that adds deferred methods for extraction, so the choice/performance hit is up to the user). - [ ] An alternative might be to remove the filter from `DynamicSceneBuilder` and have it as a separate parameter to the extraction methods (either in the existing ones or as added `extract_entity_with_filter`-type methods). Is this preferable? - [x] ~~Should we include constructors that include common types to allow/deny? For example, a `SceneFilter::standard_allowlist` that includes things like `Parent` and `Children`?~~ Consensus suggests we should. I may split this out into a followup PR, though. - [x] ~~Should we add the ability to remove types from the filter regardless of whether an allowlist or denylist (e.g. `filter.remove::<Foo>()`)?~~ See the first list item - [x] ~~Should `SceneFilter` be an enum? Would it make more sense as a struct that contains an `is_denylist` boolean?~~ With the added `SceneFilter::None` state (replacing the need to wrap in an `Option` or rely on an empty `Denylist`), it seems an enum is better suited now - [x] ~~Bikeshed: Do we like the naming convention? Should we instead use `include`/`exclude` terminology?~~ Sounds like we're sticking with `allow`/`deny`! - [x] ~~Does this feature need a new example? Do we simply include it in the existing one (maybe even as a comment?)? Should this be done in a followup PR instead?~~ Example will be added in a followup PR ### Followup Tasks - [ ] Add a dedicated `SceneFilter` example - [ ] Possibly add default types to the filter (e.g. deny things like `ComputedVisibility`, allow `Parent`, etc) --- ## Changelog - Added the `SceneFilter` enum for filtering components and resources when building a `DynamicScene` - Added methods: - `DynamicSceneBuilder::with_filter` - `DynamicSceneBuilder::allow` - `DynamicSceneBuilder::deny` - `DynamicSceneBuilder::allow_all` - `DynamicSceneBuilder::deny_all` - `DynamicSceneBuilder::with_resource_filter` - `DynamicSceneBuilder::allow_resource` - `DynamicSceneBuilder::deny_resource` - `DynamicSceneBuilder::allow_all_resources` - `DynamicSceneBuilder::deny_all_resources` - Removed methods: - `DynamicSceneBuilder::from_world_with_type_registry` - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference ## Migration Guide - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let dynamic_scene = DynamicScene::from_world(&world, registry); // let dynamic_scene = DynamicScene::from_scene(&scene, registry); // NEW let dynamic_scene = DynamicScene::from_world(&world); // let dynamic_scene = DynamicScene::from_scene(&scene); ``` - Removed `DynamicSceneBuilder::from_world_with_type_registry`. Now the registry is automatically taken from the given world: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let builder = DynamicSceneBuilder::from_world_with_type_registry(&world, registry); // NEW let builder = DynamicSceneBuilder::from_world(&world); ```
2023-07-06 21:04:26 +00:00
component_filter: SceneFilter,
resource_filter: SceneFilter,
original_world: &'w World,
}
impl<'w> DynamicSceneBuilder<'w> {
/// Prepare a builder that will extract entities and their component from the given [`World`].
pub fn from_world(world: &'w World) -> Self {
Self {
(De) serialize resources in scenes (#6846) # Objective Co-Authored-By: davier [bricedavier@gmail.com](mailto:bricedavier@gmail.com) Fixes #3576. Adds a `resources` field in scene serialization data to allow de/serializing resources that have reflection enabled. ## Solution Most of this code is taken from a previous closed PR: https://github.com/bevyengine/bevy/pull/3580. Most of the credit goes to @Davier , what I did was mostly getting it to work on the latest main branch of Bevy, along with adding a few asserts in the currently existing tests to be sure everything is working properly. This PR changes the scene format to include resources in this way: ``` ( resources: { // List of resources here, keyed by resource type name. }, entities: [ // Previous scene format here ], ) ``` An example taken from the tests: ``` ( resources: { "bevy_scene::serde::tests::MyResource": ( foo: 123, ), }, entities: { // Previous scene format here }, ) ``` For this, a `resources` fields has been added on the `DynamicScene` and the `DynamicSceneBuilder` structs. The latter now also has a method named `extract_resources` to properly extract the existing resources registered in the local type registry, in a similar way to `extract_entities`. --- ## Changelog Added: Reflect resources registered in the type registry used by dynamic scenes will now be properly de/serialized in scene data. ## Migration Guide Since the scene format has been changed, the user may not be able to use scenes saved prior to this PR due to the `resources` scene field being missing. ~~To preserve backwards compatibility, I will try to make the `resources` fully optional so that old scenes can be loaded without issue.~~ ## TODOs - [x] I may have to update a few doc blocks still referring to dynamic scenes as mere container of entities, since they now include resources as well. - [x] ~~I want to make the `resources` key optional, as specified in the Migration Guide, so that old scenes will be compatible with this change.~~ Since this would only be trivial for ron format, I think it might be better to consider it in a separate PR/discussion to figure out if it could be done for binary serialization too. - [x] I suppose it might be a good idea to add a resources in the scene example so that users will quickly notice they can serialize resources just like entities. --------- Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2023-03-20 21:17:02 +00:00
extracted_resources: default(),
extracted_scene: default(),
bevy_scene: Add `SceneFilter` (#6793) # Objective Currently, `DynamicScene`s extract all components listed in the given (or the world's) type registry. This acts as a quasi-filter of sorts. However, it can be troublesome to use effectively and lacks decent control. For example, say you need to serialize only the following component over the network: ```rust #[derive(Reflect, Component, Default)] #[reflect(Component)] struct NPC { name: Option<String> } ``` To do this, you'd need to: 1. Create a new `AppTypeRegistry` 2. Register `NPC` 3. Register `Option<String>` If we skip Step 3, then the entire scene might fail to serialize as `Option<String>` requires registration. Not only is this annoying and easy to forget, but it can leave users with an impossible task: serializing a third-party type that contains private types. Generally, the third-party crate will register their private types within a plugin so the user doesn't need to do it themselves. However, this means we are now unable to serialize _just_ that type— we're forced to allow everything! ## Solution Add the `SceneFilter` enum for filtering components to extract. This filter can be used to optionally allow or deny entire sets of components/resources. With the `DynamicSceneBuilder`, users have more control over how their `DynamicScene`s are built. To only serialize a subset of components, use the `allow` method: ```rust let scene = builder .allow::<ComponentA>() .allow::<ComponentB>() .extract_entity(entity) .build(); ``` To serialize everything _but_ a subset of components, use the `deny` method: ```rust let scene = builder .deny::<ComponentA>() .deny::<ComponentB>() .extract_entity(entity) .build(); ``` Or create a custom filter: ```rust let components = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(components); // let filter = SceneFilter::Denylist(components); let scene = builder .with_filter(Some(filter)) .extract_entity(entity) .build(); ``` Similar operations exist for resources: <details> <summary>View Resource Methods</summary> To only serialize a subset of resources, use the `allow_resource` method: ```rust let scene = builder .allow_resource::<ResourceA>() .extract_resources() .build(); ``` To serialize everything _but_ a subset of resources, use the `deny_resource` method: ```rust let scene = builder .deny_resource::<ResourceA>() .extract_resources() .build(); ``` Or create a custom filter: ```rust let resources = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(resources); // let filter = SceneFilter::Denylist(resources); let scene = builder .with_resource_filter(Some(filter)) .extract_resources() .build(); ``` </details> ### Open Questions - [x] ~~`allow` and `deny` are mutually exclusive. Currently, they overwrite each other. Should this instead be a panic?~~ Took @soqb's suggestion and made it so that the opposing method simply removes that type from the list. - [x] ~~`DynamicSceneBuilder` extracts entity data as soon as `extract_entity`/`extract_entities` is called. Should this behavior instead be moved to the `build` method to prevent ordering mixups (e.g. `.allow::<Foo>().extract_entity(entity)` vs `.extract_entity(entity).allow::<Foo>()`)? The tradeoff would be iterating over the given entities twice: once at extraction and again at build.~~ Based on the feedback from @Testare it sounds like it might be better to just keep the current functionality (if anything we can open a separate PR that adds deferred methods for extraction, so the choice/performance hit is up to the user). - [ ] An alternative might be to remove the filter from `DynamicSceneBuilder` and have it as a separate parameter to the extraction methods (either in the existing ones or as added `extract_entity_with_filter`-type methods). Is this preferable? - [x] ~~Should we include constructors that include common types to allow/deny? For example, a `SceneFilter::standard_allowlist` that includes things like `Parent` and `Children`?~~ Consensus suggests we should. I may split this out into a followup PR, though. - [x] ~~Should we add the ability to remove types from the filter regardless of whether an allowlist or denylist (e.g. `filter.remove::<Foo>()`)?~~ See the first list item - [x] ~~Should `SceneFilter` be an enum? Would it make more sense as a struct that contains an `is_denylist` boolean?~~ With the added `SceneFilter::None` state (replacing the need to wrap in an `Option` or rely on an empty `Denylist`), it seems an enum is better suited now - [x] ~~Bikeshed: Do we like the naming convention? Should we instead use `include`/`exclude` terminology?~~ Sounds like we're sticking with `allow`/`deny`! - [x] ~~Does this feature need a new example? Do we simply include it in the existing one (maybe even as a comment?)? Should this be done in a followup PR instead?~~ Example will be added in a followup PR ### Followup Tasks - [ ] Add a dedicated `SceneFilter` example - [ ] Possibly add default types to the filter (e.g. deny things like `ComputedVisibility`, allow `Parent`, etc) --- ## Changelog - Added the `SceneFilter` enum for filtering components and resources when building a `DynamicScene` - Added methods: - `DynamicSceneBuilder::with_filter` - `DynamicSceneBuilder::allow` - `DynamicSceneBuilder::deny` - `DynamicSceneBuilder::allow_all` - `DynamicSceneBuilder::deny_all` - `DynamicSceneBuilder::with_resource_filter` - `DynamicSceneBuilder::allow_resource` - `DynamicSceneBuilder::deny_resource` - `DynamicSceneBuilder::allow_all_resources` - `DynamicSceneBuilder::deny_all_resources` - Removed methods: - `DynamicSceneBuilder::from_world_with_type_registry` - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference ## Migration Guide - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let dynamic_scene = DynamicScene::from_world(&world, registry); // let dynamic_scene = DynamicScene::from_scene(&scene, registry); // NEW let dynamic_scene = DynamicScene::from_world(&world); // let dynamic_scene = DynamicScene::from_scene(&scene); ``` - Removed `DynamicSceneBuilder::from_world_with_type_registry`. Now the registry is automatically taken from the given world: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let builder = DynamicSceneBuilder::from_world_with_type_registry(&world, registry); // NEW let builder = DynamicSceneBuilder::from_world(&world); ```
2023-07-06 21:04:26 +00:00
component_filter: SceneFilter::default(),
resource_filter: SceneFilter::default(),
original_world: world,
}
}
bevy_scene: Add `SceneFilter` (#6793) # Objective Currently, `DynamicScene`s extract all components listed in the given (or the world's) type registry. This acts as a quasi-filter of sorts. However, it can be troublesome to use effectively and lacks decent control. For example, say you need to serialize only the following component over the network: ```rust #[derive(Reflect, Component, Default)] #[reflect(Component)] struct NPC { name: Option<String> } ``` To do this, you'd need to: 1. Create a new `AppTypeRegistry` 2. Register `NPC` 3. Register `Option<String>` If we skip Step 3, then the entire scene might fail to serialize as `Option<String>` requires registration. Not only is this annoying and easy to forget, but it can leave users with an impossible task: serializing a third-party type that contains private types. Generally, the third-party crate will register their private types within a plugin so the user doesn't need to do it themselves. However, this means we are now unable to serialize _just_ that type— we're forced to allow everything! ## Solution Add the `SceneFilter` enum for filtering components to extract. This filter can be used to optionally allow or deny entire sets of components/resources. With the `DynamicSceneBuilder`, users have more control over how their `DynamicScene`s are built. To only serialize a subset of components, use the `allow` method: ```rust let scene = builder .allow::<ComponentA>() .allow::<ComponentB>() .extract_entity(entity) .build(); ``` To serialize everything _but_ a subset of components, use the `deny` method: ```rust let scene = builder .deny::<ComponentA>() .deny::<ComponentB>() .extract_entity(entity) .build(); ``` Or create a custom filter: ```rust let components = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(components); // let filter = SceneFilter::Denylist(components); let scene = builder .with_filter(Some(filter)) .extract_entity(entity) .build(); ``` Similar operations exist for resources: <details> <summary>View Resource Methods</summary> To only serialize a subset of resources, use the `allow_resource` method: ```rust let scene = builder .allow_resource::<ResourceA>() .extract_resources() .build(); ``` To serialize everything _but_ a subset of resources, use the `deny_resource` method: ```rust let scene = builder .deny_resource::<ResourceA>() .extract_resources() .build(); ``` Or create a custom filter: ```rust let resources = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(resources); // let filter = SceneFilter::Denylist(resources); let scene = builder .with_resource_filter(Some(filter)) .extract_resources() .build(); ``` </details> ### Open Questions - [x] ~~`allow` and `deny` are mutually exclusive. Currently, they overwrite each other. Should this instead be a panic?~~ Took @soqb's suggestion and made it so that the opposing method simply removes that type from the list. - [x] ~~`DynamicSceneBuilder` extracts entity data as soon as `extract_entity`/`extract_entities` is called. Should this behavior instead be moved to the `build` method to prevent ordering mixups (e.g. `.allow::<Foo>().extract_entity(entity)` vs `.extract_entity(entity).allow::<Foo>()`)? The tradeoff would be iterating over the given entities twice: once at extraction and again at build.~~ Based on the feedback from @Testare it sounds like it might be better to just keep the current functionality (if anything we can open a separate PR that adds deferred methods for extraction, so the choice/performance hit is up to the user). - [ ] An alternative might be to remove the filter from `DynamicSceneBuilder` and have it as a separate parameter to the extraction methods (either in the existing ones or as added `extract_entity_with_filter`-type methods). Is this preferable? - [x] ~~Should we include constructors that include common types to allow/deny? For example, a `SceneFilter::standard_allowlist` that includes things like `Parent` and `Children`?~~ Consensus suggests we should. I may split this out into a followup PR, though. - [x] ~~Should we add the ability to remove types from the filter regardless of whether an allowlist or denylist (e.g. `filter.remove::<Foo>()`)?~~ See the first list item - [x] ~~Should `SceneFilter` be an enum? Would it make more sense as a struct that contains an `is_denylist` boolean?~~ With the added `SceneFilter::None` state (replacing the need to wrap in an `Option` or rely on an empty `Denylist`), it seems an enum is better suited now - [x] ~~Bikeshed: Do we like the naming convention? Should we instead use `include`/`exclude` terminology?~~ Sounds like we're sticking with `allow`/`deny`! - [x] ~~Does this feature need a new example? Do we simply include it in the existing one (maybe even as a comment?)? Should this be done in a followup PR instead?~~ Example will be added in a followup PR ### Followup Tasks - [ ] Add a dedicated `SceneFilter` example - [ ] Possibly add default types to the filter (e.g. deny things like `ComputedVisibility`, allow `Parent`, etc) --- ## Changelog - Added the `SceneFilter` enum for filtering components and resources when building a `DynamicScene` - Added methods: - `DynamicSceneBuilder::with_filter` - `DynamicSceneBuilder::allow` - `DynamicSceneBuilder::deny` - `DynamicSceneBuilder::allow_all` - `DynamicSceneBuilder::deny_all` - `DynamicSceneBuilder::with_resource_filter` - `DynamicSceneBuilder::allow_resource` - `DynamicSceneBuilder::deny_resource` - `DynamicSceneBuilder::allow_all_resources` - `DynamicSceneBuilder::deny_all_resources` - Removed methods: - `DynamicSceneBuilder::from_world_with_type_registry` - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference ## Migration Guide - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let dynamic_scene = DynamicScene::from_world(&world, registry); // let dynamic_scene = DynamicScene::from_scene(&scene, registry); // NEW let dynamic_scene = DynamicScene::from_world(&world); // let dynamic_scene = DynamicScene::from_scene(&scene); ``` - Removed `DynamicSceneBuilder::from_world_with_type_registry`. Now the registry is automatically taken from the given world: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let builder = DynamicSceneBuilder::from_world_with_type_registry(&world, registry); // NEW let builder = DynamicSceneBuilder::from_world(&world); ```
2023-07-06 21:04:26 +00:00
/// Specify a custom component [`SceneFilter`] to be used with this builder.
#[must_use]
pub fn with_component_filter(mut self, filter: SceneFilter) -> Self {
bevy_scene: Add `SceneFilter` (#6793) # Objective Currently, `DynamicScene`s extract all components listed in the given (or the world's) type registry. This acts as a quasi-filter of sorts. However, it can be troublesome to use effectively and lacks decent control. For example, say you need to serialize only the following component over the network: ```rust #[derive(Reflect, Component, Default)] #[reflect(Component)] struct NPC { name: Option<String> } ``` To do this, you'd need to: 1. Create a new `AppTypeRegistry` 2. Register `NPC` 3. Register `Option<String>` If we skip Step 3, then the entire scene might fail to serialize as `Option<String>` requires registration. Not only is this annoying and easy to forget, but it can leave users with an impossible task: serializing a third-party type that contains private types. Generally, the third-party crate will register their private types within a plugin so the user doesn't need to do it themselves. However, this means we are now unable to serialize _just_ that type— we're forced to allow everything! ## Solution Add the `SceneFilter` enum for filtering components to extract. This filter can be used to optionally allow or deny entire sets of components/resources. With the `DynamicSceneBuilder`, users have more control over how their `DynamicScene`s are built. To only serialize a subset of components, use the `allow` method: ```rust let scene = builder .allow::<ComponentA>() .allow::<ComponentB>() .extract_entity(entity) .build(); ``` To serialize everything _but_ a subset of components, use the `deny` method: ```rust let scene = builder .deny::<ComponentA>() .deny::<ComponentB>() .extract_entity(entity) .build(); ``` Or create a custom filter: ```rust let components = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(components); // let filter = SceneFilter::Denylist(components); let scene = builder .with_filter(Some(filter)) .extract_entity(entity) .build(); ``` Similar operations exist for resources: <details> <summary>View Resource Methods</summary> To only serialize a subset of resources, use the `allow_resource` method: ```rust let scene = builder .allow_resource::<ResourceA>() .extract_resources() .build(); ``` To serialize everything _but_ a subset of resources, use the `deny_resource` method: ```rust let scene = builder .deny_resource::<ResourceA>() .extract_resources() .build(); ``` Or create a custom filter: ```rust let resources = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(resources); // let filter = SceneFilter::Denylist(resources); let scene = builder .with_resource_filter(Some(filter)) .extract_resources() .build(); ``` </details> ### Open Questions - [x] ~~`allow` and `deny` are mutually exclusive. Currently, they overwrite each other. Should this instead be a panic?~~ Took @soqb's suggestion and made it so that the opposing method simply removes that type from the list. - [x] ~~`DynamicSceneBuilder` extracts entity data as soon as `extract_entity`/`extract_entities` is called. Should this behavior instead be moved to the `build` method to prevent ordering mixups (e.g. `.allow::<Foo>().extract_entity(entity)` vs `.extract_entity(entity).allow::<Foo>()`)? The tradeoff would be iterating over the given entities twice: once at extraction and again at build.~~ Based on the feedback from @Testare it sounds like it might be better to just keep the current functionality (if anything we can open a separate PR that adds deferred methods for extraction, so the choice/performance hit is up to the user). - [ ] An alternative might be to remove the filter from `DynamicSceneBuilder` and have it as a separate parameter to the extraction methods (either in the existing ones or as added `extract_entity_with_filter`-type methods). Is this preferable? - [x] ~~Should we include constructors that include common types to allow/deny? For example, a `SceneFilter::standard_allowlist` that includes things like `Parent` and `Children`?~~ Consensus suggests we should. I may split this out into a followup PR, though. - [x] ~~Should we add the ability to remove types from the filter regardless of whether an allowlist or denylist (e.g. `filter.remove::<Foo>()`)?~~ See the first list item - [x] ~~Should `SceneFilter` be an enum? Would it make more sense as a struct that contains an `is_denylist` boolean?~~ With the added `SceneFilter::None` state (replacing the need to wrap in an `Option` or rely on an empty `Denylist`), it seems an enum is better suited now - [x] ~~Bikeshed: Do we like the naming convention? Should we instead use `include`/`exclude` terminology?~~ Sounds like we're sticking with `allow`/`deny`! - [x] ~~Does this feature need a new example? Do we simply include it in the existing one (maybe even as a comment?)? Should this be done in a followup PR instead?~~ Example will be added in a followup PR ### Followup Tasks - [ ] Add a dedicated `SceneFilter` example - [ ] Possibly add default types to the filter (e.g. deny things like `ComputedVisibility`, allow `Parent`, etc) --- ## Changelog - Added the `SceneFilter` enum for filtering components and resources when building a `DynamicScene` - Added methods: - `DynamicSceneBuilder::with_filter` - `DynamicSceneBuilder::allow` - `DynamicSceneBuilder::deny` - `DynamicSceneBuilder::allow_all` - `DynamicSceneBuilder::deny_all` - `DynamicSceneBuilder::with_resource_filter` - `DynamicSceneBuilder::allow_resource` - `DynamicSceneBuilder::deny_resource` - `DynamicSceneBuilder::allow_all_resources` - `DynamicSceneBuilder::deny_all_resources` - Removed methods: - `DynamicSceneBuilder::from_world_with_type_registry` - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference ## Migration Guide - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let dynamic_scene = DynamicScene::from_world(&world, registry); // let dynamic_scene = DynamicScene::from_scene(&scene, registry); // NEW let dynamic_scene = DynamicScene::from_world(&world); // let dynamic_scene = DynamicScene::from_scene(&scene); ``` - Removed `DynamicSceneBuilder::from_world_with_type_registry`. Now the registry is automatically taken from the given world: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let builder = DynamicSceneBuilder::from_world_with_type_registry(&world, registry); // NEW let builder = DynamicSceneBuilder::from_world(&world); ```
2023-07-06 21:04:26 +00:00
self.component_filter = filter;
self
}
/// Specify a custom resource [`SceneFilter`] to be used with this builder.
#[must_use]
pub fn with_resource_filter(mut self, filter: SceneFilter) -> Self {
bevy_scene: Add `SceneFilter` (#6793) # Objective Currently, `DynamicScene`s extract all components listed in the given (or the world's) type registry. This acts as a quasi-filter of sorts. However, it can be troublesome to use effectively and lacks decent control. For example, say you need to serialize only the following component over the network: ```rust #[derive(Reflect, Component, Default)] #[reflect(Component)] struct NPC { name: Option<String> } ``` To do this, you'd need to: 1. Create a new `AppTypeRegistry` 2. Register `NPC` 3. Register `Option<String>` If we skip Step 3, then the entire scene might fail to serialize as `Option<String>` requires registration. Not only is this annoying and easy to forget, but it can leave users with an impossible task: serializing a third-party type that contains private types. Generally, the third-party crate will register their private types within a plugin so the user doesn't need to do it themselves. However, this means we are now unable to serialize _just_ that type— we're forced to allow everything! ## Solution Add the `SceneFilter` enum for filtering components to extract. This filter can be used to optionally allow or deny entire sets of components/resources. With the `DynamicSceneBuilder`, users have more control over how their `DynamicScene`s are built. To only serialize a subset of components, use the `allow` method: ```rust let scene = builder .allow::<ComponentA>() .allow::<ComponentB>() .extract_entity(entity) .build(); ``` To serialize everything _but_ a subset of components, use the `deny` method: ```rust let scene = builder .deny::<ComponentA>() .deny::<ComponentB>() .extract_entity(entity) .build(); ``` Or create a custom filter: ```rust let components = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(components); // let filter = SceneFilter::Denylist(components); let scene = builder .with_filter(Some(filter)) .extract_entity(entity) .build(); ``` Similar operations exist for resources: <details> <summary>View Resource Methods</summary> To only serialize a subset of resources, use the `allow_resource` method: ```rust let scene = builder .allow_resource::<ResourceA>() .extract_resources() .build(); ``` To serialize everything _but_ a subset of resources, use the `deny_resource` method: ```rust let scene = builder .deny_resource::<ResourceA>() .extract_resources() .build(); ``` Or create a custom filter: ```rust let resources = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(resources); // let filter = SceneFilter::Denylist(resources); let scene = builder .with_resource_filter(Some(filter)) .extract_resources() .build(); ``` </details> ### Open Questions - [x] ~~`allow` and `deny` are mutually exclusive. Currently, they overwrite each other. Should this instead be a panic?~~ Took @soqb's suggestion and made it so that the opposing method simply removes that type from the list. - [x] ~~`DynamicSceneBuilder` extracts entity data as soon as `extract_entity`/`extract_entities` is called. Should this behavior instead be moved to the `build` method to prevent ordering mixups (e.g. `.allow::<Foo>().extract_entity(entity)` vs `.extract_entity(entity).allow::<Foo>()`)? The tradeoff would be iterating over the given entities twice: once at extraction and again at build.~~ Based on the feedback from @Testare it sounds like it might be better to just keep the current functionality (if anything we can open a separate PR that adds deferred methods for extraction, so the choice/performance hit is up to the user). - [ ] An alternative might be to remove the filter from `DynamicSceneBuilder` and have it as a separate parameter to the extraction methods (either in the existing ones or as added `extract_entity_with_filter`-type methods). Is this preferable? - [x] ~~Should we include constructors that include common types to allow/deny? For example, a `SceneFilter::standard_allowlist` that includes things like `Parent` and `Children`?~~ Consensus suggests we should. I may split this out into a followup PR, though. - [x] ~~Should we add the ability to remove types from the filter regardless of whether an allowlist or denylist (e.g. `filter.remove::<Foo>()`)?~~ See the first list item - [x] ~~Should `SceneFilter` be an enum? Would it make more sense as a struct that contains an `is_denylist` boolean?~~ With the added `SceneFilter::None` state (replacing the need to wrap in an `Option` or rely on an empty `Denylist`), it seems an enum is better suited now - [x] ~~Bikeshed: Do we like the naming convention? Should we instead use `include`/`exclude` terminology?~~ Sounds like we're sticking with `allow`/`deny`! - [x] ~~Does this feature need a new example? Do we simply include it in the existing one (maybe even as a comment?)? Should this be done in a followup PR instead?~~ Example will be added in a followup PR ### Followup Tasks - [ ] Add a dedicated `SceneFilter` example - [ ] Possibly add default types to the filter (e.g. deny things like `ComputedVisibility`, allow `Parent`, etc) --- ## Changelog - Added the `SceneFilter` enum for filtering components and resources when building a `DynamicScene` - Added methods: - `DynamicSceneBuilder::with_filter` - `DynamicSceneBuilder::allow` - `DynamicSceneBuilder::deny` - `DynamicSceneBuilder::allow_all` - `DynamicSceneBuilder::deny_all` - `DynamicSceneBuilder::with_resource_filter` - `DynamicSceneBuilder::allow_resource` - `DynamicSceneBuilder::deny_resource` - `DynamicSceneBuilder::allow_all_resources` - `DynamicSceneBuilder::deny_all_resources` - Removed methods: - `DynamicSceneBuilder::from_world_with_type_registry` - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference ## Migration Guide - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let dynamic_scene = DynamicScene::from_world(&world, registry); // let dynamic_scene = DynamicScene::from_scene(&scene, registry); // NEW let dynamic_scene = DynamicScene::from_world(&world); // let dynamic_scene = DynamicScene::from_scene(&scene); ``` - Removed `DynamicSceneBuilder::from_world_with_type_registry`. Now the registry is automatically taken from the given world: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let builder = DynamicSceneBuilder::from_world_with_type_registry(&world, registry); // NEW let builder = DynamicSceneBuilder::from_world(&world); ```
2023-07-06 21:04:26 +00:00
self.resource_filter = filter;
self
}
/// Updates the filter to allow all component and resource types.
///
/// This is useful for resetting the filter so that types may be selectively denied
/// with [`deny_component`](`Self::deny_component`) and [`deny_resource`](`Self::deny_resource`).
pub fn allow_all(mut self) -> Self {
self.component_filter = SceneFilter::allow_all();
self.resource_filter = SceneFilter::allow_all();
self
}
/// Updates the filter to deny all component and resource types.
///
/// This is useful for resetting the filter so that types may be selectively allowed
/// with [`allow_component`](`Self::allow_component`) and [`allow_resource`](`Self::allow_resource`).
pub fn deny_all(mut self) -> Self {
self.component_filter = SceneFilter::deny_all();
self.resource_filter = SceneFilter::deny_all();
self
}
bevy_scene: Add `SceneFilter` (#6793) # Objective Currently, `DynamicScene`s extract all components listed in the given (or the world's) type registry. This acts as a quasi-filter of sorts. However, it can be troublesome to use effectively and lacks decent control. For example, say you need to serialize only the following component over the network: ```rust #[derive(Reflect, Component, Default)] #[reflect(Component)] struct NPC { name: Option<String> } ``` To do this, you'd need to: 1. Create a new `AppTypeRegistry` 2. Register `NPC` 3. Register `Option<String>` If we skip Step 3, then the entire scene might fail to serialize as `Option<String>` requires registration. Not only is this annoying and easy to forget, but it can leave users with an impossible task: serializing a third-party type that contains private types. Generally, the third-party crate will register their private types within a plugin so the user doesn't need to do it themselves. However, this means we are now unable to serialize _just_ that type— we're forced to allow everything! ## Solution Add the `SceneFilter` enum for filtering components to extract. This filter can be used to optionally allow or deny entire sets of components/resources. With the `DynamicSceneBuilder`, users have more control over how their `DynamicScene`s are built. To only serialize a subset of components, use the `allow` method: ```rust let scene = builder .allow::<ComponentA>() .allow::<ComponentB>() .extract_entity(entity) .build(); ``` To serialize everything _but_ a subset of components, use the `deny` method: ```rust let scene = builder .deny::<ComponentA>() .deny::<ComponentB>() .extract_entity(entity) .build(); ``` Or create a custom filter: ```rust let components = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(components); // let filter = SceneFilter::Denylist(components); let scene = builder .with_filter(Some(filter)) .extract_entity(entity) .build(); ``` Similar operations exist for resources: <details> <summary>View Resource Methods</summary> To only serialize a subset of resources, use the `allow_resource` method: ```rust let scene = builder .allow_resource::<ResourceA>() .extract_resources() .build(); ``` To serialize everything _but_ a subset of resources, use the `deny_resource` method: ```rust let scene = builder .deny_resource::<ResourceA>() .extract_resources() .build(); ``` Or create a custom filter: ```rust let resources = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(resources); // let filter = SceneFilter::Denylist(resources); let scene = builder .with_resource_filter(Some(filter)) .extract_resources() .build(); ``` </details> ### Open Questions - [x] ~~`allow` and `deny` are mutually exclusive. Currently, they overwrite each other. Should this instead be a panic?~~ Took @soqb's suggestion and made it so that the opposing method simply removes that type from the list. - [x] ~~`DynamicSceneBuilder` extracts entity data as soon as `extract_entity`/`extract_entities` is called. Should this behavior instead be moved to the `build` method to prevent ordering mixups (e.g. `.allow::<Foo>().extract_entity(entity)` vs `.extract_entity(entity).allow::<Foo>()`)? The tradeoff would be iterating over the given entities twice: once at extraction and again at build.~~ Based on the feedback from @Testare it sounds like it might be better to just keep the current functionality (if anything we can open a separate PR that adds deferred methods for extraction, so the choice/performance hit is up to the user). - [ ] An alternative might be to remove the filter from `DynamicSceneBuilder` and have it as a separate parameter to the extraction methods (either in the existing ones or as added `extract_entity_with_filter`-type methods). Is this preferable? - [x] ~~Should we include constructors that include common types to allow/deny? For example, a `SceneFilter::standard_allowlist` that includes things like `Parent` and `Children`?~~ Consensus suggests we should. I may split this out into a followup PR, though. - [x] ~~Should we add the ability to remove types from the filter regardless of whether an allowlist or denylist (e.g. `filter.remove::<Foo>()`)?~~ See the first list item - [x] ~~Should `SceneFilter` be an enum? Would it make more sense as a struct that contains an `is_denylist` boolean?~~ With the added `SceneFilter::None` state (replacing the need to wrap in an `Option` or rely on an empty `Denylist`), it seems an enum is better suited now - [x] ~~Bikeshed: Do we like the naming convention? Should we instead use `include`/`exclude` terminology?~~ Sounds like we're sticking with `allow`/`deny`! - [x] ~~Does this feature need a new example? Do we simply include it in the existing one (maybe even as a comment?)? Should this be done in a followup PR instead?~~ Example will be added in a followup PR ### Followup Tasks - [ ] Add a dedicated `SceneFilter` example - [ ] Possibly add default types to the filter (e.g. deny things like `ComputedVisibility`, allow `Parent`, etc) --- ## Changelog - Added the `SceneFilter` enum for filtering components and resources when building a `DynamicScene` - Added methods: - `DynamicSceneBuilder::with_filter` - `DynamicSceneBuilder::allow` - `DynamicSceneBuilder::deny` - `DynamicSceneBuilder::allow_all` - `DynamicSceneBuilder::deny_all` - `DynamicSceneBuilder::with_resource_filter` - `DynamicSceneBuilder::allow_resource` - `DynamicSceneBuilder::deny_resource` - `DynamicSceneBuilder::allow_all_resources` - `DynamicSceneBuilder::deny_all_resources` - Removed methods: - `DynamicSceneBuilder::from_world_with_type_registry` - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference ## Migration Guide - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let dynamic_scene = DynamicScene::from_world(&world, registry); // let dynamic_scene = DynamicScene::from_scene(&scene, registry); // NEW let dynamic_scene = DynamicScene::from_world(&world); // let dynamic_scene = DynamicScene::from_scene(&scene); ``` - Removed `DynamicSceneBuilder::from_world_with_type_registry`. Now the registry is automatically taken from the given world: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let builder = DynamicSceneBuilder::from_world_with_type_registry(&world, registry); // NEW let builder = DynamicSceneBuilder::from_world(&world); ```
2023-07-06 21:04:26 +00:00
/// Allows the given component type, `T`, to be included in the generated scene.
///
/// This method may be called multiple times for any number of components.
///
/// This is the inverse of [`deny_component`](Self::deny_component).
bevy_scene: Add `SceneFilter` (#6793) # Objective Currently, `DynamicScene`s extract all components listed in the given (or the world's) type registry. This acts as a quasi-filter of sorts. However, it can be troublesome to use effectively and lacks decent control. For example, say you need to serialize only the following component over the network: ```rust #[derive(Reflect, Component, Default)] #[reflect(Component)] struct NPC { name: Option<String> } ``` To do this, you'd need to: 1. Create a new `AppTypeRegistry` 2. Register `NPC` 3. Register `Option<String>` If we skip Step 3, then the entire scene might fail to serialize as `Option<String>` requires registration. Not only is this annoying and easy to forget, but it can leave users with an impossible task: serializing a third-party type that contains private types. Generally, the third-party crate will register their private types within a plugin so the user doesn't need to do it themselves. However, this means we are now unable to serialize _just_ that type— we're forced to allow everything! ## Solution Add the `SceneFilter` enum for filtering components to extract. This filter can be used to optionally allow or deny entire sets of components/resources. With the `DynamicSceneBuilder`, users have more control over how their `DynamicScene`s are built. To only serialize a subset of components, use the `allow` method: ```rust let scene = builder .allow::<ComponentA>() .allow::<ComponentB>() .extract_entity(entity) .build(); ``` To serialize everything _but_ a subset of components, use the `deny` method: ```rust let scene = builder .deny::<ComponentA>() .deny::<ComponentB>() .extract_entity(entity) .build(); ``` Or create a custom filter: ```rust let components = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(components); // let filter = SceneFilter::Denylist(components); let scene = builder .with_filter(Some(filter)) .extract_entity(entity) .build(); ``` Similar operations exist for resources: <details> <summary>View Resource Methods</summary> To only serialize a subset of resources, use the `allow_resource` method: ```rust let scene = builder .allow_resource::<ResourceA>() .extract_resources() .build(); ``` To serialize everything _but_ a subset of resources, use the `deny_resource` method: ```rust let scene = builder .deny_resource::<ResourceA>() .extract_resources() .build(); ``` Or create a custom filter: ```rust let resources = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(resources); // let filter = SceneFilter::Denylist(resources); let scene = builder .with_resource_filter(Some(filter)) .extract_resources() .build(); ``` </details> ### Open Questions - [x] ~~`allow` and `deny` are mutually exclusive. Currently, they overwrite each other. Should this instead be a panic?~~ Took @soqb's suggestion and made it so that the opposing method simply removes that type from the list. - [x] ~~`DynamicSceneBuilder` extracts entity data as soon as `extract_entity`/`extract_entities` is called. Should this behavior instead be moved to the `build` method to prevent ordering mixups (e.g. `.allow::<Foo>().extract_entity(entity)` vs `.extract_entity(entity).allow::<Foo>()`)? The tradeoff would be iterating over the given entities twice: once at extraction and again at build.~~ Based on the feedback from @Testare it sounds like it might be better to just keep the current functionality (if anything we can open a separate PR that adds deferred methods for extraction, so the choice/performance hit is up to the user). - [ ] An alternative might be to remove the filter from `DynamicSceneBuilder` and have it as a separate parameter to the extraction methods (either in the existing ones or as added `extract_entity_with_filter`-type methods). Is this preferable? - [x] ~~Should we include constructors that include common types to allow/deny? For example, a `SceneFilter::standard_allowlist` that includes things like `Parent` and `Children`?~~ Consensus suggests we should. I may split this out into a followup PR, though. - [x] ~~Should we add the ability to remove types from the filter regardless of whether an allowlist or denylist (e.g. `filter.remove::<Foo>()`)?~~ See the first list item - [x] ~~Should `SceneFilter` be an enum? Would it make more sense as a struct that contains an `is_denylist` boolean?~~ With the added `SceneFilter::None` state (replacing the need to wrap in an `Option` or rely on an empty `Denylist`), it seems an enum is better suited now - [x] ~~Bikeshed: Do we like the naming convention? Should we instead use `include`/`exclude` terminology?~~ Sounds like we're sticking with `allow`/`deny`! - [x] ~~Does this feature need a new example? Do we simply include it in the existing one (maybe even as a comment?)? Should this be done in a followup PR instead?~~ Example will be added in a followup PR ### Followup Tasks - [ ] Add a dedicated `SceneFilter` example - [ ] Possibly add default types to the filter (e.g. deny things like `ComputedVisibility`, allow `Parent`, etc) --- ## Changelog - Added the `SceneFilter` enum for filtering components and resources when building a `DynamicScene` - Added methods: - `DynamicSceneBuilder::with_filter` - `DynamicSceneBuilder::allow` - `DynamicSceneBuilder::deny` - `DynamicSceneBuilder::allow_all` - `DynamicSceneBuilder::deny_all` - `DynamicSceneBuilder::with_resource_filter` - `DynamicSceneBuilder::allow_resource` - `DynamicSceneBuilder::deny_resource` - `DynamicSceneBuilder::allow_all_resources` - `DynamicSceneBuilder::deny_all_resources` - Removed methods: - `DynamicSceneBuilder::from_world_with_type_registry` - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference ## Migration Guide - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let dynamic_scene = DynamicScene::from_world(&world, registry); // let dynamic_scene = DynamicScene::from_scene(&scene, registry); // NEW let dynamic_scene = DynamicScene::from_world(&world); // let dynamic_scene = DynamicScene::from_scene(&scene); ``` - Removed `DynamicSceneBuilder::from_world_with_type_registry`. Now the registry is automatically taken from the given world: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let builder = DynamicSceneBuilder::from_world_with_type_registry(&world, registry); // NEW let builder = DynamicSceneBuilder::from_world(&world); ```
2023-07-06 21:04:26 +00:00
/// If `T` has already been denied, then it will be removed from the denylist.
#[must_use]
pub fn allow_component<T: Component>(mut self) -> Self {
self.component_filter = self.component_filter.allow::<T>();
bevy_scene: Add `SceneFilter` (#6793) # Objective Currently, `DynamicScene`s extract all components listed in the given (or the world's) type registry. This acts as a quasi-filter of sorts. However, it can be troublesome to use effectively and lacks decent control. For example, say you need to serialize only the following component over the network: ```rust #[derive(Reflect, Component, Default)] #[reflect(Component)] struct NPC { name: Option<String> } ``` To do this, you'd need to: 1. Create a new `AppTypeRegistry` 2. Register `NPC` 3. Register `Option<String>` If we skip Step 3, then the entire scene might fail to serialize as `Option<String>` requires registration. Not only is this annoying and easy to forget, but it can leave users with an impossible task: serializing a third-party type that contains private types. Generally, the third-party crate will register their private types within a plugin so the user doesn't need to do it themselves. However, this means we are now unable to serialize _just_ that type— we're forced to allow everything! ## Solution Add the `SceneFilter` enum for filtering components to extract. This filter can be used to optionally allow or deny entire sets of components/resources. With the `DynamicSceneBuilder`, users have more control over how their `DynamicScene`s are built. To only serialize a subset of components, use the `allow` method: ```rust let scene = builder .allow::<ComponentA>() .allow::<ComponentB>() .extract_entity(entity) .build(); ``` To serialize everything _but_ a subset of components, use the `deny` method: ```rust let scene = builder .deny::<ComponentA>() .deny::<ComponentB>() .extract_entity(entity) .build(); ``` Or create a custom filter: ```rust let components = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(components); // let filter = SceneFilter::Denylist(components); let scene = builder .with_filter(Some(filter)) .extract_entity(entity) .build(); ``` Similar operations exist for resources: <details> <summary>View Resource Methods</summary> To only serialize a subset of resources, use the `allow_resource` method: ```rust let scene = builder .allow_resource::<ResourceA>() .extract_resources() .build(); ``` To serialize everything _but_ a subset of resources, use the `deny_resource` method: ```rust let scene = builder .deny_resource::<ResourceA>() .extract_resources() .build(); ``` Or create a custom filter: ```rust let resources = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(resources); // let filter = SceneFilter::Denylist(resources); let scene = builder .with_resource_filter(Some(filter)) .extract_resources() .build(); ``` </details> ### Open Questions - [x] ~~`allow` and `deny` are mutually exclusive. Currently, they overwrite each other. Should this instead be a panic?~~ Took @soqb's suggestion and made it so that the opposing method simply removes that type from the list. - [x] ~~`DynamicSceneBuilder` extracts entity data as soon as `extract_entity`/`extract_entities` is called. Should this behavior instead be moved to the `build` method to prevent ordering mixups (e.g. `.allow::<Foo>().extract_entity(entity)` vs `.extract_entity(entity).allow::<Foo>()`)? The tradeoff would be iterating over the given entities twice: once at extraction and again at build.~~ Based on the feedback from @Testare it sounds like it might be better to just keep the current functionality (if anything we can open a separate PR that adds deferred methods for extraction, so the choice/performance hit is up to the user). - [ ] An alternative might be to remove the filter from `DynamicSceneBuilder` and have it as a separate parameter to the extraction methods (either in the existing ones or as added `extract_entity_with_filter`-type methods). Is this preferable? - [x] ~~Should we include constructors that include common types to allow/deny? For example, a `SceneFilter::standard_allowlist` that includes things like `Parent` and `Children`?~~ Consensus suggests we should. I may split this out into a followup PR, though. - [x] ~~Should we add the ability to remove types from the filter regardless of whether an allowlist or denylist (e.g. `filter.remove::<Foo>()`)?~~ See the first list item - [x] ~~Should `SceneFilter` be an enum? Would it make more sense as a struct that contains an `is_denylist` boolean?~~ With the added `SceneFilter::None` state (replacing the need to wrap in an `Option` or rely on an empty `Denylist`), it seems an enum is better suited now - [x] ~~Bikeshed: Do we like the naming convention? Should we instead use `include`/`exclude` terminology?~~ Sounds like we're sticking with `allow`/`deny`! - [x] ~~Does this feature need a new example? Do we simply include it in the existing one (maybe even as a comment?)? Should this be done in a followup PR instead?~~ Example will be added in a followup PR ### Followup Tasks - [ ] Add a dedicated `SceneFilter` example - [ ] Possibly add default types to the filter (e.g. deny things like `ComputedVisibility`, allow `Parent`, etc) --- ## Changelog - Added the `SceneFilter` enum for filtering components and resources when building a `DynamicScene` - Added methods: - `DynamicSceneBuilder::with_filter` - `DynamicSceneBuilder::allow` - `DynamicSceneBuilder::deny` - `DynamicSceneBuilder::allow_all` - `DynamicSceneBuilder::deny_all` - `DynamicSceneBuilder::with_resource_filter` - `DynamicSceneBuilder::allow_resource` - `DynamicSceneBuilder::deny_resource` - `DynamicSceneBuilder::allow_all_resources` - `DynamicSceneBuilder::deny_all_resources` - Removed methods: - `DynamicSceneBuilder::from_world_with_type_registry` - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference ## Migration Guide - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let dynamic_scene = DynamicScene::from_world(&world, registry); // let dynamic_scene = DynamicScene::from_scene(&scene, registry); // NEW let dynamic_scene = DynamicScene::from_world(&world); // let dynamic_scene = DynamicScene::from_scene(&scene); ``` - Removed `DynamicSceneBuilder::from_world_with_type_registry`. Now the registry is automatically taken from the given world: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let builder = DynamicSceneBuilder::from_world_with_type_registry(&world, registry); // NEW let builder = DynamicSceneBuilder::from_world(&world); ```
2023-07-06 21:04:26 +00:00
self
}
/// Denies the given component type, `T`, from being included in the generated scene.
///
/// This method may be called multiple times for any number of components.
///
/// This is the inverse of [`allow_component`](Self::allow_component).
bevy_scene: Add `SceneFilter` (#6793) # Objective Currently, `DynamicScene`s extract all components listed in the given (or the world's) type registry. This acts as a quasi-filter of sorts. However, it can be troublesome to use effectively and lacks decent control. For example, say you need to serialize only the following component over the network: ```rust #[derive(Reflect, Component, Default)] #[reflect(Component)] struct NPC { name: Option<String> } ``` To do this, you'd need to: 1. Create a new `AppTypeRegistry` 2. Register `NPC` 3. Register `Option<String>` If we skip Step 3, then the entire scene might fail to serialize as `Option<String>` requires registration. Not only is this annoying and easy to forget, but it can leave users with an impossible task: serializing a third-party type that contains private types. Generally, the third-party crate will register their private types within a plugin so the user doesn't need to do it themselves. However, this means we are now unable to serialize _just_ that type— we're forced to allow everything! ## Solution Add the `SceneFilter` enum for filtering components to extract. This filter can be used to optionally allow or deny entire sets of components/resources. With the `DynamicSceneBuilder`, users have more control over how their `DynamicScene`s are built. To only serialize a subset of components, use the `allow` method: ```rust let scene = builder .allow::<ComponentA>() .allow::<ComponentB>() .extract_entity(entity) .build(); ``` To serialize everything _but_ a subset of components, use the `deny` method: ```rust let scene = builder .deny::<ComponentA>() .deny::<ComponentB>() .extract_entity(entity) .build(); ``` Or create a custom filter: ```rust let components = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(components); // let filter = SceneFilter::Denylist(components); let scene = builder .with_filter(Some(filter)) .extract_entity(entity) .build(); ``` Similar operations exist for resources: <details> <summary>View Resource Methods</summary> To only serialize a subset of resources, use the `allow_resource` method: ```rust let scene = builder .allow_resource::<ResourceA>() .extract_resources() .build(); ``` To serialize everything _but_ a subset of resources, use the `deny_resource` method: ```rust let scene = builder .deny_resource::<ResourceA>() .extract_resources() .build(); ``` Or create a custom filter: ```rust let resources = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(resources); // let filter = SceneFilter::Denylist(resources); let scene = builder .with_resource_filter(Some(filter)) .extract_resources() .build(); ``` </details> ### Open Questions - [x] ~~`allow` and `deny` are mutually exclusive. Currently, they overwrite each other. Should this instead be a panic?~~ Took @soqb's suggestion and made it so that the opposing method simply removes that type from the list. - [x] ~~`DynamicSceneBuilder` extracts entity data as soon as `extract_entity`/`extract_entities` is called. Should this behavior instead be moved to the `build` method to prevent ordering mixups (e.g. `.allow::<Foo>().extract_entity(entity)` vs `.extract_entity(entity).allow::<Foo>()`)? The tradeoff would be iterating over the given entities twice: once at extraction and again at build.~~ Based on the feedback from @Testare it sounds like it might be better to just keep the current functionality (if anything we can open a separate PR that adds deferred methods for extraction, so the choice/performance hit is up to the user). - [ ] An alternative might be to remove the filter from `DynamicSceneBuilder` and have it as a separate parameter to the extraction methods (either in the existing ones or as added `extract_entity_with_filter`-type methods). Is this preferable? - [x] ~~Should we include constructors that include common types to allow/deny? For example, a `SceneFilter::standard_allowlist` that includes things like `Parent` and `Children`?~~ Consensus suggests we should. I may split this out into a followup PR, though. - [x] ~~Should we add the ability to remove types from the filter regardless of whether an allowlist or denylist (e.g. `filter.remove::<Foo>()`)?~~ See the first list item - [x] ~~Should `SceneFilter` be an enum? Would it make more sense as a struct that contains an `is_denylist` boolean?~~ With the added `SceneFilter::None` state (replacing the need to wrap in an `Option` or rely on an empty `Denylist`), it seems an enum is better suited now - [x] ~~Bikeshed: Do we like the naming convention? Should we instead use `include`/`exclude` terminology?~~ Sounds like we're sticking with `allow`/`deny`! - [x] ~~Does this feature need a new example? Do we simply include it in the existing one (maybe even as a comment?)? Should this be done in a followup PR instead?~~ Example will be added in a followup PR ### Followup Tasks - [ ] Add a dedicated `SceneFilter` example - [ ] Possibly add default types to the filter (e.g. deny things like `ComputedVisibility`, allow `Parent`, etc) --- ## Changelog - Added the `SceneFilter` enum for filtering components and resources when building a `DynamicScene` - Added methods: - `DynamicSceneBuilder::with_filter` - `DynamicSceneBuilder::allow` - `DynamicSceneBuilder::deny` - `DynamicSceneBuilder::allow_all` - `DynamicSceneBuilder::deny_all` - `DynamicSceneBuilder::with_resource_filter` - `DynamicSceneBuilder::allow_resource` - `DynamicSceneBuilder::deny_resource` - `DynamicSceneBuilder::allow_all_resources` - `DynamicSceneBuilder::deny_all_resources` - Removed methods: - `DynamicSceneBuilder::from_world_with_type_registry` - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference ## Migration Guide - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let dynamic_scene = DynamicScene::from_world(&world, registry); // let dynamic_scene = DynamicScene::from_scene(&scene, registry); // NEW let dynamic_scene = DynamicScene::from_world(&world); // let dynamic_scene = DynamicScene::from_scene(&scene); ``` - Removed `DynamicSceneBuilder::from_world_with_type_registry`. Now the registry is automatically taken from the given world: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let builder = DynamicSceneBuilder::from_world_with_type_registry(&world, registry); // NEW let builder = DynamicSceneBuilder::from_world(&world); ```
2023-07-06 21:04:26 +00:00
/// If `T` has already been allowed, then it will be removed from the allowlist.
#[must_use]
pub fn deny_component<T: Component>(mut self) -> Self {
self.component_filter = self.component_filter.deny::<T>();
bevy_scene: Add `SceneFilter` (#6793) # Objective Currently, `DynamicScene`s extract all components listed in the given (or the world's) type registry. This acts as a quasi-filter of sorts. However, it can be troublesome to use effectively and lacks decent control. For example, say you need to serialize only the following component over the network: ```rust #[derive(Reflect, Component, Default)] #[reflect(Component)] struct NPC { name: Option<String> } ``` To do this, you'd need to: 1. Create a new `AppTypeRegistry` 2. Register `NPC` 3. Register `Option<String>` If we skip Step 3, then the entire scene might fail to serialize as `Option<String>` requires registration. Not only is this annoying and easy to forget, but it can leave users with an impossible task: serializing a third-party type that contains private types. Generally, the third-party crate will register their private types within a plugin so the user doesn't need to do it themselves. However, this means we are now unable to serialize _just_ that type— we're forced to allow everything! ## Solution Add the `SceneFilter` enum for filtering components to extract. This filter can be used to optionally allow or deny entire sets of components/resources. With the `DynamicSceneBuilder`, users have more control over how their `DynamicScene`s are built. To only serialize a subset of components, use the `allow` method: ```rust let scene = builder .allow::<ComponentA>() .allow::<ComponentB>() .extract_entity(entity) .build(); ``` To serialize everything _but_ a subset of components, use the `deny` method: ```rust let scene = builder .deny::<ComponentA>() .deny::<ComponentB>() .extract_entity(entity) .build(); ``` Or create a custom filter: ```rust let components = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(components); // let filter = SceneFilter::Denylist(components); let scene = builder .with_filter(Some(filter)) .extract_entity(entity) .build(); ``` Similar operations exist for resources: <details> <summary>View Resource Methods</summary> To only serialize a subset of resources, use the `allow_resource` method: ```rust let scene = builder .allow_resource::<ResourceA>() .extract_resources() .build(); ``` To serialize everything _but_ a subset of resources, use the `deny_resource` method: ```rust let scene = builder .deny_resource::<ResourceA>() .extract_resources() .build(); ``` Or create a custom filter: ```rust let resources = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(resources); // let filter = SceneFilter::Denylist(resources); let scene = builder .with_resource_filter(Some(filter)) .extract_resources() .build(); ``` </details> ### Open Questions - [x] ~~`allow` and `deny` are mutually exclusive. Currently, they overwrite each other. Should this instead be a panic?~~ Took @soqb's suggestion and made it so that the opposing method simply removes that type from the list. - [x] ~~`DynamicSceneBuilder` extracts entity data as soon as `extract_entity`/`extract_entities` is called. Should this behavior instead be moved to the `build` method to prevent ordering mixups (e.g. `.allow::<Foo>().extract_entity(entity)` vs `.extract_entity(entity).allow::<Foo>()`)? The tradeoff would be iterating over the given entities twice: once at extraction and again at build.~~ Based on the feedback from @Testare it sounds like it might be better to just keep the current functionality (if anything we can open a separate PR that adds deferred methods for extraction, so the choice/performance hit is up to the user). - [ ] An alternative might be to remove the filter from `DynamicSceneBuilder` and have it as a separate parameter to the extraction methods (either in the existing ones or as added `extract_entity_with_filter`-type methods). Is this preferable? - [x] ~~Should we include constructors that include common types to allow/deny? For example, a `SceneFilter::standard_allowlist` that includes things like `Parent` and `Children`?~~ Consensus suggests we should. I may split this out into a followup PR, though. - [x] ~~Should we add the ability to remove types from the filter regardless of whether an allowlist or denylist (e.g. `filter.remove::<Foo>()`)?~~ See the first list item - [x] ~~Should `SceneFilter` be an enum? Would it make more sense as a struct that contains an `is_denylist` boolean?~~ With the added `SceneFilter::None` state (replacing the need to wrap in an `Option` or rely on an empty `Denylist`), it seems an enum is better suited now - [x] ~~Bikeshed: Do we like the naming convention? Should we instead use `include`/`exclude` terminology?~~ Sounds like we're sticking with `allow`/`deny`! - [x] ~~Does this feature need a new example? Do we simply include it in the existing one (maybe even as a comment?)? Should this be done in a followup PR instead?~~ Example will be added in a followup PR ### Followup Tasks - [ ] Add a dedicated `SceneFilter` example - [ ] Possibly add default types to the filter (e.g. deny things like `ComputedVisibility`, allow `Parent`, etc) --- ## Changelog - Added the `SceneFilter` enum for filtering components and resources when building a `DynamicScene` - Added methods: - `DynamicSceneBuilder::with_filter` - `DynamicSceneBuilder::allow` - `DynamicSceneBuilder::deny` - `DynamicSceneBuilder::allow_all` - `DynamicSceneBuilder::deny_all` - `DynamicSceneBuilder::with_resource_filter` - `DynamicSceneBuilder::allow_resource` - `DynamicSceneBuilder::deny_resource` - `DynamicSceneBuilder::allow_all_resources` - `DynamicSceneBuilder::deny_all_resources` - Removed methods: - `DynamicSceneBuilder::from_world_with_type_registry` - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference ## Migration Guide - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let dynamic_scene = DynamicScene::from_world(&world, registry); // let dynamic_scene = DynamicScene::from_scene(&scene, registry); // NEW let dynamic_scene = DynamicScene::from_world(&world); // let dynamic_scene = DynamicScene::from_scene(&scene); ``` - Removed `DynamicSceneBuilder::from_world_with_type_registry`. Now the registry is automatically taken from the given world: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let builder = DynamicSceneBuilder::from_world_with_type_registry(&world, registry); // NEW let builder = DynamicSceneBuilder::from_world(&world); ```
2023-07-06 21:04:26 +00:00
self
}
/// Updates the filter to allow all component types.
///
/// This is useful for resetting the filter so that types may be selectively [denied].
///
/// [denied]: Self::deny_component
#[must_use]
pub fn allow_all_components(mut self) -> Self {
bevy_scene: Add `SceneFilter` (#6793) # Objective Currently, `DynamicScene`s extract all components listed in the given (or the world's) type registry. This acts as a quasi-filter of sorts. However, it can be troublesome to use effectively and lacks decent control. For example, say you need to serialize only the following component over the network: ```rust #[derive(Reflect, Component, Default)] #[reflect(Component)] struct NPC { name: Option<String> } ``` To do this, you'd need to: 1. Create a new `AppTypeRegistry` 2. Register `NPC` 3. Register `Option<String>` If we skip Step 3, then the entire scene might fail to serialize as `Option<String>` requires registration. Not only is this annoying and easy to forget, but it can leave users with an impossible task: serializing a third-party type that contains private types. Generally, the third-party crate will register their private types within a plugin so the user doesn't need to do it themselves. However, this means we are now unable to serialize _just_ that type— we're forced to allow everything! ## Solution Add the `SceneFilter` enum for filtering components to extract. This filter can be used to optionally allow or deny entire sets of components/resources. With the `DynamicSceneBuilder`, users have more control over how their `DynamicScene`s are built. To only serialize a subset of components, use the `allow` method: ```rust let scene = builder .allow::<ComponentA>() .allow::<ComponentB>() .extract_entity(entity) .build(); ``` To serialize everything _but_ a subset of components, use the `deny` method: ```rust let scene = builder .deny::<ComponentA>() .deny::<ComponentB>() .extract_entity(entity) .build(); ``` Or create a custom filter: ```rust let components = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(components); // let filter = SceneFilter::Denylist(components); let scene = builder .with_filter(Some(filter)) .extract_entity(entity) .build(); ``` Similar operations exist for resources: <details> <summary>View Resource Methods</summary> To only serialize a subset of resources, use the `allow_resource` method: ```rust let scene = builder .allow_resource::<ResourceA>() .extract_resources() .build(); ``` To serialize everything _but_ a subset of resources, use the `deny_resource` method: ```rust let scene = builder .deny_resource::<ResourceA>() .extract_resources() .build(); ``` Or create a custom filter: ```rust let resources = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(resources); // let filter = SceneFilter::Denylist(resources); let scene = builder .with_resource_filter(Some(filter)) .extract_resources() .build(); ``` </details> ### Open Questions - [x] ~~`allow` and `deny` are mutually exclusive. Currently, they overwrite each other. Should this instead be a panic?~~ Took @soqb's suggestion and made it so that the opposing method simply removes that type from the list. - [x] ~~`DynamicSceneBuilder` extracts entity data as soon as `extract_entity`/`extract_entities` is called. Should this behavior instead be moved to the `build` method to prevent ordering mixups (e.g. `.allow::<Foo>().extract_entity(entity)` vs `.extract_entity(entity).allow::<Foo>()`)? The tradeoff would be iterating over the given entities twice: once at extraction and again at build.~~ Based on the feedback from @Testare it sounds like it might be better to just keep the current functionality (if anything we can open a separate PR that adds deferred methods for extraction, so the choice/performance hit is up to the user). - [ ] An alternative might be to remove the filter from `DynamicSceneBuilder` and have it as a separate parameter to the extraction methods (either in the existing ones or as added `extract_entity_with_filter`-type methods). Is this preferable? - [x] ~~Should we include constructors that include common types to allow/deny? For example, a `SceneFilter::standard_allowlist` that includes things like `Parent` and `Children`?~~ Consensus suggests we should. I may split this out into a followup PR, though. - [x] ~~Should we add the ability to remove types from the filter regardless of whether an allowlist or denylist (e.g. `filter.remove::<Foo>()`)?~~ See the first list item - [x] ~~Should `SceneFilter` be an enum? Would it make more sense as a struct that contains an `is_denylist` boolean?~~ With the added `SceneFilter::None` state (replacing the need to wrap in an `Option` or rely on an empty `Denylist`), it seems an enum is better suited now - [x] ~~Bikeshed: Do we like the naming convention? Should we instead use `include`/`exclude` terminology?~~ Sounds like we're sticking with `allow`/`deny`! - [x] ~~Does this feature need a new example? Do we simply include it in the existing one (maybe even as a comment?)? Should this be done in a followup PR instead?~~ Example will be added in a followup PR ### Followup Tasks - [ ] Add a dedicated `SceneFilter` example - [ ] Possibly add default types to the filter (e.g. deny things like `ComputedVisibility`, allow `Parent`, etc) --- ## Changelog - Added the `SceneFilter` enum for filtering components and resources when building a `DynamicScene` - Added methods: - `DynamicSceneBuilder::with_filter` - `DynamicSceneBuilder::allow` - `DynamicSceneBuilder::deny` - `DynamicSceneBuilder::allow_all` - `DynamicSceneBuilder::deny_all` - `DynamicSceneBuilder::with_resource_filter` - `DynamicSceneBuilder::allow_resource` - `DynamicSceneBuilder::deny_resource` - `DynamicSceneBuilder::allow_all_resources` - `DynamicSceneBuilder::deny_all_resources` - Removed methods: - `DynamicSceneBuilder::from_world_with_type_registry` - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference ## Migration Guide - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let dynamic_scene = DynamicScene::from_world(&world, registry); // let dynamic_scene = DynamicScene::from_scene(&scene, registry); // NEW let dynamic_scene = DynamicScene::from_world(&world); // let dynamic_scene = DynamicScene::from_scene(&scene); ``` - Removed `DynamicSceneBuilder::from_world_with_type_registry`. Now the registry is automatically taken from the given world: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let builder = DynamicSceneBuilder::from_world_with_type_registry(&world, registry); // NEW let builder = DynamicSceneBuilder::from_world(&world); ```
2023-07-06 21:04:26 +00:00
self.component_filter = SceneFilter::allow_all();
self
}
/// Updates the filter to deny all component types.
///
/// This is useful for resetting the filter so that types may be selectively [allowed].
///
/// [allowed]: Self::allow_component
#[must_use]
pub fn deny_all_components(mut self) -> Self {
bevy_scene: Add `SceneFilter` (#6793) # Objective Currently, `DynamicScene`s extract all components listed in the given (or the world's) type registry. This acts as a quasi-filter of sorts. However, it can be troublesome to use effectively and lacks decent control. For example, say you need to serialize only the following component over the network: ```rust #[derive(Reflect, Component, Default)] #[reflect(Component)] struct NPC { name: Option<String> } ``` To do this, you'd need to: 1. Create a new `AppTypeRegistry` 2. Register `NPC` 3. Register `Option<String>` If we skip Step 3, then the entire scene might fail to serialize as `Option<String>` requires registration. Not only is this annoying and easy to forget, but it can leave users with an impossible task: serializing a third-party type that contains private types. Generally, the third-party crate will register their private types within a plugin so the user doesn't need to do it themselves. However, this means we are now unable to serialize _just_ that type— we're forced to allow everything! ## Solution Add the `SceneFilter` enum for filtering components to extract. This filter can be used to optionally allow or deny entire sets of components/resources. With the `DynamicSceneBuilder`, users have more control over how their `DynamicScene`s are built. To only serialize a subset of components, use the `allow` method: ```rust let scene = builder .allow::<ComponentA>() .allow::<ComponentB>() .extract_entity(entity) .build(); ``` To serialize everything _but_ a subset of components, use the `deny` method: ```rust let scene = builder .deny::<ComponentA>() .deny::<ComponentB>() .extract_entity(entity) .build(); ``` Or create a custom filter: ```rust let components = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(components); // let filter = SceneFilter::Denylist(components); let scene = builder .with_filter(Some(filter)) .extract_entity(entity) .build(); ``` Similar operations exist for resources: <details> <summary>View Resource Methods</summary> To only serialize a subset of resources, use the `allow_resource` method: ```rust let scene = builder .allow_resource::<ResourceA>() .extract_resources() .build(); ``` To serialize everything _but_ a subset of resources, use the `deny_resource` method: ```rust let scene = builder .deny_resource::<ResourceA>() .extract_resources() .build(); ``` Or create a custom filter: ```rust let resources = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(resources); // let filter = SceneFilter::Denylist(resources); let scene = builder .with_resource_filter(Some(filter)) .extract_resources() .build(); ``` </details> ### Open Questions - [x] ~~`allow` and `deny` are mutually exclusive. Currently, they overwrite each other. Should this instead be a panic?~~ Took @soqb's suggestion and made it so that the opposing method simply removes that type from the list. - [x] ~~`DynamicSceneBuilder` extracts entity data as soon as `extract_entity`/`extract_entities` is called. Should this behavior instead be moved to the `build` method to prevent ordering mixups (e.g. `.allow::<Foo>().extract_entity(entity)` vs `.extract_entity(entity).allow::<Foo>()`)? The tradeoff would be iterating over the given entities twice: once at extraction and again at build.~~ Based on the feedback from @Testare it sounds like it might be better to just keep the current functionality (if anything we can open a separate PR that adds deferred methods for extraction, so the choice/performance hit is up to the user). - [ ] An alternative might be to remove the filter from `DynamicSceneBuilder` and have it as a separate parameter to the extraction methods (either in the existing ones or as added `extract_entity_with_filter`-type methods). Is this preferable? - [x] ~~Should we include constructors that include common types to allow/deny? For example, a `SceneFilter::standard_allowlist` that includes things like `Parent` and `Children`?~~ Consensus suggests we should. I may split this out into a followup PR, though. - [x] ~~Should we add the ability to remove types from the filter regardless of whether an allowlist or denylist (e.g. `filter.remove::<Foo>()`)?~~ See the first list item - [x] ~~Should `SceneFilter` be an enum? Would it make more sense as a struct that contains an `is_denylist` boolean?~~ With the added `SceneFilter::None` state (replacing the need to wrap in an `Option` or rely on an empty `Denylist`), it seems an enum is better suited now - [x] ~~Bikeshed: Do we like the naming convention? Should we instead use `include`/`exclude` terminology?~~ Sounds like we're sticking with `allow`/`deny`! - [x] ~~Does this feature need a new example? Do we simply include it in the existing one (maybe even as a comment?)? Should this be done in a followup PR instead?~~ Example will be added in a followup PR ### Followup Tasks - [ ] Add a dedicated `SceneFilter` example - [ ] Possibly add default types to the filter (e.g. deny things like `ComputedVisibility`, allow `Parent`, etc) --- ## Changelog - Added the `SceneFilter` enum for filtering components and resources when building a `DynamicScene` - Added methods: - `DynamicSceneBuilder::with_filter` - `DynamicSceneBuilder::allow` - `DynamicSceneBuilder::deny` - `DynamicSceneBuilder::allow_all` - `DynamicSceneBuilder::deny_all` - `DynamicSceneBuilder::with_resource_filter` - `DynamicSceneBuilder::allow_resource` - `DynamicSceneBuilder::deny_resource` - `DynamicSceneBuilder::allow_all_resources` - `DynamicSceneBuilder::deny_all_resources` - Removed methods: - `DynamicSceneBuilder::from_world_with_type_registry` - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference ## Migration Guide - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let dynamic_scene = DynamicScene::from_world(&world, registry); // let dynamic_scene = DynamicScene::from_scene(&scene, registry); // NEW let dynamic_scene = DynamicScene::from_world(&world); // let dynamic_scene = DynamicScene::from_scene(&scene); ``` - Removed `DynamicSceneBuilder::from_world_with_type_registry`. Now the registry is automatically taken from the given world: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let builder = DynamicSceneBuilder::from_world_with_type_registry(&world, registry); // NEW let builder = DynamicSceneBuilder::from_world(&world); ```
2023-07-06 21:04:26 +00:00
self.component_filter = SceneFilter::deny_all();
self
}
/// Allows the given resource type, `T`, to be included in the generated scene.
///
/// This method may be called multiple times for any number of resources.
///
/// This is the inverse of [`deny_resource`](Self::deny_resource).
/// If `T` has already been denied, then it will be removed from the denylist.
#[must_use]
pub fn allow_resource<T: Resource>(mut self) -> Self {
self.resource_filter = self.resource_filter.allow::<T>();
bevy_scene: Add `SceneFilter` (#6793) # Objective Currently, `DynamicScene`s extract all components listed in the given (or the world's) type registry. This acts as a quasi-filter of sorts. However, it can be troublesome to use effectively and lacks decent control. For example, say you need to serialize only the following component over the network: ```rust #[derive(Reflect, Component, Default)] #[reflect(Component)] struct NPC { name: Option<String> } ``` To do this, you'd need to: 1. Create a new `AppTypeRegistry` 2. Register `NPC` 3. Register `Option<String>` If we skip Step 3, then the entire scene might fail to serialize as `Option<String>` requires registration. Not only is this annoying and easy to forget, but it can leave users with an impossible task: serializing a third-party type that contains private types. Generally, the third-party crate will register their private types within a plugin so the user doesn't need to do it themselves. However, this means we are now unable to serialize _just_ that type— we're forced to allow everything! ## Solution Add the `SceneFilter` enum for filtering components to extract. This filter can be used to optionally allow or deny entire sets of components/resources. With the `DynamicSceneBuilder`, users have more control over how their `DynamicScene`s are built. To only serialize a subset of components, use the `allow` method: ```rust let scene = builder .allow::<ComponentA>() .allow::<ComponentB>() .extract_entity(entity) .build(); ``` To serialize everything _but_ a subset of components, use the `deny` method: ```rust let scene = builder .deny::<ComponentA>() .deny::<ComponentB>() .extract_entity(entity) .build(); ``` Or create a custom filter: ```rust let components = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(components); // let filter = SceneFilter::Denylist(components); let scene = builder .with_filter(Some(filter)) .extract_entity(entity) .build(); ``` Similar operations exist for resources: <details> <summary>View Resource Methods</summary> To only serialize a subset of resources, use the `allow_resource` method: ```rust let scene = builder .allow_resource::<ResourceA>() .extract_resources() .build(); ``` To serialize everything _but_ a subset of resources, use the `deny_resource` method: ```rust let scene = builder .deny_resource::<ResourceA>() .extract_resources() .build(); ``` Or create a custom filter: ```rust let resources = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(resources); // let filter = SceneFilter::Denylist(resources); let scene = builder .with_resource_filter(Some(filter)) .extract_resources() .build(); ``` </details> ### Open Questions - [x] ~~`allow` and `deny` are mutually exclusive. Currently, they overwrite each other. Should this instead be a panic?~~ Took @soqb's suggestion and made it so that the opposing method simply removes that type from the list. - [x] ~~`DynamicSceneBuilder` extracts entity data as soon as `extract_entity`/`extract_entities` is called. Should this behavior instead be moved to the `build` method to prevent ordering mixups (e.g. `.allow::<Foo>().extract_entity(entity)` vs `.extract_entity(entity).allow::<Foo>()`)? The tradeoff would be iterating over the given entities twice: once at extraction and again at build.~~ Based on the feedback from @Testare it sounds like it might be better to just keep the current functionality (if anything we can open a separate PR that adds deferred methods for extraction, so the choice/performance hit is up to the user). - [ ] An alternative might be to remove the filter from `DynamicSceneBuilder` and have it as a separate parameter to the extraction methods (either in the existing ones or as added `extract_entity_with_filter`-type methods). Is this preferable? - [x] ~~Should we include constructors that include common types to allow/deny? For example, a `SceneFilter::standard_allowlist` that includes things like `Parent` and `Children`?~~ Consensus suggests we should. I may split this out into a followup PR, though. - [x] ~~Should we add the ability to remove types from the filter regardless of whether an allowlist or denylist (e.g. `filter.remove::<Foo>()`)?~~ See the first list item - [x] ~~Should `SceneFilter` be an enum? Would it make more sense as a struct that contains an `is_denylist` boolean?~~ With the added `SceneFilter::None` state (replacing the need to wrap in an `Option` or rely on an empty `Denylist`), it seems an enum is better suited now - [x] ~~Bikeshed: Do we like the naming convention? Should we instead use `include`/`exclude` terminology?~~ Sounds like we're sticking with `allow`/`deny`! - [x] ~~Does this feature need a new example? Do we simply include it in the existing one (maybe even as a comment?)? Should this be done in a followup PR instead?~~ Example will be added in a followup PR ### Followup Tasks - [ ] Add a dedicated `SceneFilter` example - [ ] Possibly add default types to the filter (e.g. deny things like `ComputedVisibility`, allow `Parent`, etc) --- ## Changelog - Added the `SceneFilter` enum for filtering components and resources when building a `DynamicScene` - Added methods: - `DynamicSceneBuilder::with_filter` - `DynamicSceneBuilder::allow` - `DynamicSceneBuilder::deny` - `DynamicSceneBuilder::allow_all` - `DynamicSceneBuilder::deny_all` - `DynamicSceneBuilder::with_resource_filter` - `DynamicSceneBuilder::allow_resource` - `DynamicSceneBuilder::deny_resource` - `DynamicSceneBuilder::allow_all_resources` - `DynamicSceneBuilder::deny_all_resources` - Removed methods: - `DynamicSceneBuilder::from_world_with_type_registry` - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference ## Migration Guide - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let dynamic_scene = DynamicScene::from_world(&world, registry); // let dynamic_scene = DynamicScene::from_scene(&scene, registry); // NEW let dynamic_scene = DynamicScene::from_world(&world); // let dynamic_scene = DynamicScene::from_scene(&scene); ``` - Removed `DynamicSceneBuilder::from_world_with_type_registry`. Now the registry is automatically taken from the given world: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let builder = DynamicSceneBuilder::from_world_with_type_registry(&world, registry); // NEW let builder = DynamicSceneBuilder::from_world(&world); ```
2023-07-06 21:04:26 +00:00
self
}
/// Denies the given resource type, `T`, from being included in the generated scene.
///
/// This method may be called multiple times for any number of resources.
///
/// This is the inverse of [`allow_resource`](Self::allow_resource).
/// If `T` has already been allowed, then it will be removed from the allowlist.
#[must_use]
pub fn deny_resource<T: Resource>(mut self) -> Self {
self.resource_filter = self.resource_filter.deny::<T>();
bevy_scene: Add `SceneFilter` (#6793) # Objective Currently, `DynamicScene`s extract all components listed in the given (or the world's) type registry. This acts as a quasi-filter of sorts. However, it can be troublesome to use effectively and lacks decent control. For example, say you need to serialize only the following component over the network: ```rust #[derive(Reflect, Component, Default)] #[reflect(Component)] struct NPC { name: Option<String> } ``` To do this, you'd need to: 1. Create a new `AppTypeRegistry` 2. Register `NPC` 3. Register `Option<String>` If we skip Step 3, then the entire scene might fail to serialize as `Option<String>` requires registration. Not only is this annoying and easy to forget, but it can leave users with an impossible task: serializing a third-party type that contains private types. Generally, the third-party crate will register their private types within a plugin so the user doesn't need to do it themselves. However, this means we are now unable to serialize _just_ that type— we're forced to allow everything! ## Solution Add the `SceneFilter` enum for filtering components to extract. This filter can be used to optionally allow or deny entire sets of components/resources. With the `DynamicSceneBuilder`, users have more control over how their `DynamicScene`s are built. To only serialize a subset of components, use the `allow` method: ```rust let scene = builder .allow::<ComponentA>() .allow::<ComponentB>() .extract_entity(entity) .build(); ``` To serialize everything _but_ a subset of components, use the `deny` method: ```rust let scene = builder .deny::<ComponentA>() .deny::<ComponentB>() .extract_entity(entity) .build(); ``` Or create a custom filter: ```rust let components = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(components); // let filter = SceneFilter::Denylist(components); let scene = builder .with_filter(Some(filter)) .extract_entity(entity) .build(); ``` Similar operations exist for resources: <details> <summary>View Resource Methods</summary> To only serialize a subset of resources, use the `allow_resource` method: ```rust let scene = builder .allow_resource::<ResourceA>() .extract_resources() .build(); ``` To serialize everything _but_ a subset of resources, use the `deny_resource` method: ```rust let scene = builder .deny_resource::<ResourceA>() .extract_resources() .build(); ``` Or create a custom filter: ```rust let resources = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(resources); // let filter = SceneFilter::Denylist(resources); let scene = builder .with_resource_filter(Some(filter)) .extract_resources() .build(); ``` </details> ### Open Questions - [x] ~~`allow` and `deny` are mutually exclusive. Currently, they overwrite each other. Should this instead be a panic?~~ Took @soqb's suggestion and made it so that the opposing method simply removes that type from the list. - [x] ~~`DynamicSceneBuilder` extracts entity data as soon as `extract_entity`/`extract_entities` is called. Should this behavior instead be moved to the `build` method to prevent ordering mixups (e.g. `.allow::<Foo>().extract_entity(entity)` vs `.extract_entity(entity).allow::<Foo>()`)? The tradeoff would be iterating over the given entities twice: once at extraction and again at build.~~ Based on the feedback from @Testare it sounds like it might be better to just keep the current functionality (if anything we can open a separate PR that adds deferred methods for extraction, so the choice/performance hit is up to the user). - [ ] An alternative might be to remove the filter from `DynamicSceneBuilder` and have it as a separate parameter to the extraction methods (either in the existing ones or as added `extract_entity_with_filter`-type methods). Is this preferable? - [x] ~~Should we include constructors that include common types to allow/deny? For example, a `SceneFilter::standard_allowlist` that includes things like `Parent` and `Children`?~~ Consensus suggests we should. I may split this out into a followup PR, though. - [x] ~~Should we add the ability to remove types from the filter regardless of whether an allowlist or denylist (e.g. `filter.remove::<Foo>()`)?~~ See the first list item - [x] ~~Should `SceneFilter` be an enum? Would it make more sense as a struct that contains an `is_denylist` boolean?~~ With the added `SceneFilter::None` state (replacing the need to wrap in an `Option` or rely on an empty `Denylist`), it seems an enum is better suited now - [x] ~~Bikeshed: Do we like the naming convention? Should we instead use `include`/`exclude` terminology?~~ Sounds like we're sticking with `allow`/`deny`! - [x] ~~Does this feature need a new example? Do we simply include it in the existing one (maybe even as a comment?)? Should this be done in a followup PR instead?~~ Example will be added in a followup PR ### Followup Tasks - [ ] Add a dedicated `SceneFilter` example - [ ] Possibly add default types to the filter (e.g. deny things like `ComputedVisibility`, allow `Parent`, etc) --- ## Changelog - Added the `SceneFilter` enum for filtering components and resources when building a `DynamicScene` - Added methods: - `DynamicSceneBuilder::with_filter` - `DynamicSceneBuilder::allow` - `DynamicSceneBuilder::deny` - `DynamicSceneBuilder::allow_all` - `DynamicSceneBuilder::deny_all` - `DynamicSceneBuilder::with_resource_filter` - `DynamicSceneBuilder::allow_resource` - `DynamicSceneBuilder::deny_resource` - `DynamicSceneBuilder::allow_all_resources` - `DynamicSceneBuilder::deny_all_resources` - Removed methods: - `DynamicSceneBuilder::from_world_with_type_registry` - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference ## Migration Guide - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let dynamic_scene = DynamicScene::from_world(&world, registry); // let dynamic_scene = DynamicScene::from_scene(&scene, registry); // NEW let dynamic_scene = DynamicScene::from_world(&world); // let dynamic_scene = DynamicScene::from_scene(&scene); ``` - Removed `DynamicSceneBuilder::from_world_with_type_registry`. Now the registry is automatically taken from the given world: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let builder = DynamicSceneBuilder::from_world_with_type_registry(&world, registry); // NEW let builder = DynamicSceneBuilder::from_world(&world); ```
2023-07-06 21:04:26 +00:00
self
}
/// Updates the filter to allow all resource types.
///
/// This is useful for resetting the filter so that types may be selectively [denied].
///
/// [denied]: Self::deny_resource
#[must_use]
pub fn allow_all_resources(mut self) -> Self {
bevy_scene: Add `SceneFilter` (#6793) # Objective Currently, `DynamicScene`s extract all components listed in the given (or the world's) type registry. This acts as a quasi-filter of sorts. However, it can be troublesome to use effectively and lacks decent control. For example, say you need to serialize only the following component over the network: ```rust #[derive(Reflect, Component, Default)] #[reflect(Component)] struct NPC { name: Option<String> } ``` To do this, you'd need to: 1. Create a new `AppTypeRegistry` 2. Register `NPC` 3. Register `Option<String>` If we skip Step 3, then the entire scene might fail to serialize as `Option<String>` requires registration. Not only is this annoying and easy to forget, but it can leave users with an impossible task: serializing a third-party type that contains private types. Generally, the third-party crate will register their private types within a plugin so the user doesn't need to do it themselves. However, this means we are now unable to serialize _just_ that type— we're forced to allow everything! ## Solution Add the `SceneFilter` enum for filtering components to extract. This filter can be used to optionally allow or deny entire sets of components/resources. With the `DynamicSceneBuilder`, users have more control over how their `DynamicScene`s are built. To only serialize a subset of components, use the `allow` method: ```rust let scene = builder .allow::<ComponentA>() .allow::<ComponentB>() .extract_entity(entity) .build(); ``` To serialize everything _but_ a subset of components, use the `deny` method: ```rust let scene = builder .deny::<ComponentA>() .deny::<ComponentB>() .extract_entity(entity) .build(); ``` Or create a custom filter: ```rust let components = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(components); // let filter = SceneFilter::Denylist(components); let scene = builder .with_filter(Some(filter)) .extract_entity(entity) .build(); ``` Similar operations exist for resources: <details> <summary>View Resource Methods</summary> To only serialize a subset of resources, use the `allow_resource` method: ```rust let scene = builder .allow_resource::<ResourceA>() .extract_resources() .build(); ``` To serialize everything _but_ a subset of resources, use the `deny_resource` method: ```rust let scene = builder .deny_resource::<ResourceA>() .extract_resources() .build(); ``` Or create a custom filter: ```rust let resources = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(resources); // let filter = SceneFilter::Denylist(resources); let scene = builder .with_resource_filter(Some(filter)) .extract_resources() .build(); ``` </details> ### Open Questions - [x] ~~`allow` and `deny` are mutually exclusive. Currently, they overwrite each other. Should this instead be a panic?~~ Took @soqb's suggestion and made it so that the opposing method simply removes that type from the list. - [x] ~~`DynamicSceneBuilder` extracts entity data as soon as `extract_entity`/`extract_entities` is called. Should this behavior instead be moved to the `build` method to prevent ordering mixups (e.g. `.allow::<Foo>().extract_entity(entity)` vs `.extract_entity(entity).allow::<Foo>()`)? The tradeoff would be iterating over the given entities twice: once at extraction and again at build.~~ Based on the feedback from @Testare it sounds like it might be better to just keep the current functionality (if anything we can open a separate PR that adds deferred methods for extraction, so the choice/performance hit is up to the user). - [ ] An alternative might be to remove the filter from `DynamicSceneBuilder` and have it as a separate parameter to the extraction methods (either in the existing ones or as added `extract_entity_with_filter`-type methods). Is this preferable? - [x] ~~Should we include constructors that include common types to allow/deny? For example, a `SceneFilter::standard_allowlist` that includes things like `Parent` and `Children`?~~ Consensus suggests we should. I may split this out into a followup PR, though. - [x] ~~Should we add the ability to remove types from the filter regardless of whether an allowlist or denylist (e.g. `filter.remove::<Foo>()`)?~~ See the first list item - [x] ~~Should `SceneFilter` be an enum? Would it make more sense as a struct that contains an `is_denylist` boolean?~~ With the added `SceneFilter::None` state (replacing the need to wrap in an `Option` or rely on an empty `Denylist`), it seems an enum is better suited now - [x] ~~Bikeshed: Do we like the naming convention? Should we instead use `include`/`exclude` terminology?~~ Sounds like we're sticking with `allow`/`deny`! - [x] ~~Does this feature need a new example? Do we simply include it in the existing one (maybe even as a comment?)? Should this be done in a followup PR instead?~~ Example will be added in a followup PR ### Followup Tasks - [ ] Add a dedicated `SceneFilter` example - [ ] Possibly add default types to the filter (e.g. deny things like `ComputedVisibility`, allow `Parent`, etc) --- ## Changelog - Added the `SceneFilter` enum for filtering components and resources when building a `DynamicScene` - Added methods: - `DynamicSceneBuilder::with_filter` - `DynamicSceneBuilder::allow` - `DynamicSceneBuilder::deny` - `DynamicSceneBuilder::allow_all` - `DynamicSceneBuilder::deny_all` - `DynamicSceneBuilder::with_resource_filter` - `DynamicSceneBuilder::allow_resource` - `DynamicSceneBuilder::deny_resource` - `DynamicSceneBuilder::allow_all_resources` - `DynamicSceneBuilder::deny_all_resources` - Removed methods: - `DynamicSceneBuilder::from_world_with_type_registry` - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference ## Migration Guide - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let dynamic_scene = DynamicScene::from_world(&world, registry); // let dynamic_scene = DynamicScene::from_scene(&scene, registry); // NEW let dynamic_scene = DynamicScene::from_world(&world); // let dynamic_scene = DynamicScene::from_scene(&scene); ``` - Removed `DynamicSceneBuilder::from_world_with_type_registry`. Now the registry is automatically taken from the given world: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let builder = DynamicSceneBuilder::from_world_with_type_registry(&world, registry); // NEW let builder = DynamicSceneBuilder::from_world(&world); ```
2023-07-06 21:04:26 +00:00
self.resource_filter = SceneFilter::allow_all();
self
}
/// Updates the filter to deny all resource types.
///
/// This is useful for resetting the filter so that types may be selectively [allowed].
///
/// [allowed]: Self::allow_resource
#[must_use]
pub fn deny_all_resources(mut self) -> Self {
bevy_scene: Add `SceneFilter` (#6793) # Objective Currently, `DynamicScene`s extract all components listed in the given (or the world's) type registry. This acts as a quasi-filter of sorts. However, it can be troublesome to use effectively and lacks decent control. For example, say you need to serialize only the following component over the network: ```rust #[derive(Reflect, Component, Default)] #[reflect(Component)] struct NPC { name: Option<String> } ``` To do this, you'd need to: 1. Create a new `AppTypeRegistry` 2. Register `NPC` 3. Register `Option<String>` If we skip Step 3, then the entire scene might fail to serialize as `Option<String>` requires registration. Not only is this annoying and easy to forget, but it can leave users with an impossible task: serializing a third-party type that contains private types. Generally, the third-party crate will register their private types within a plugin so the user doesn't need to do it themselves. However, this means we are now unable to serialize _just_ that type— we're forced to allow everything! ## Solution Add the `SceneFilter` enum for filtering components to extract. This filter can be used to optionally allow or deny entire sets of components/resources. With the `DynamicSceneBuilder`, users have more control over how their `DynamicScene`s are built. To only serialize a subset of components, use the `allow` method: ```rust let scene = builder .allow::<ComponentA>() .allow::<ComponentB>() .extract_entity(entity) .build(); ``` To serialize everything _but_ a subset of components, use the `deny` method: ```rust let scene = builder .deny::<ComponentA>() .deny::<ComponentB>() .extract_entity(entity) .build(); ``` Or create a custom filter: ```rust let components = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(components); // let filter = SceneFilter::Denylist(components); let scene = builder .with_filter(Some(filter)) .extract_entity(entity) .build(); ``` Similar operations exist for resources: <details> <summary>View Resource Methods</summary> To only serialize a subset of resources, use the `allow_resource` method: ```rust let scene = builder .allow_resource::<ResourceA>() .extract_resources() .build(); ``` To serialize everything _but_ a subset of resources, use the `deny_resource` method: ```rust let scene = builder .deny_resource::<ResourceA>() .extract_resources() .build(); ``` Or create a custom filter: ```rust let resources = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(resources); // let filter = SceneFilter::Denylist(resources); let scene = builder .with_resource_filter(Some(filter)) .extract_resources() .build(); ``` </details> ### Open Questions - [x] ~~`allow` and `deny` are mutually exclusive. Currently, they overwrite each other. Should this instead be a panic?~~ Took @soqb's suggestion and made it so that the opposing method simply removes that type from the list. - [x] ~~`DynamicSceneBuilder` extracts entity data as soon as `extract_entity`/`extract_entities` is called. Should this behavior instead be moved to the `build` method to prevent ordering mixups (e.g. `.allow::<Foo>().extract_entity(entity)` vs `.extract_entity(entity).allow::<Foo>()`)? The tradeoff would be iterating over the given entities twice: once at extraction and again at build.~~ Based on the feedback from @Testare it sounds like it might be better to just keep the current functionality (if anything we can open a separate PR that adds deferred methods for extraction, so the choice/performance hit is up to the user). - [ ] An alternative might be to remove the filter from `DynamicSceneBuilder` and have it as a separate parameter to the extraction methods (either in the existing ones or as added `extract_entity_with_filter`-type methods). Is this preferable? - [x] ~~Should we include constructors that include common types to allow/deny? For example, a `SceneFilter::standard_allowlist` that includes things like `Parent` and `Children`?~~ Consensus suggests we should. I may split this out into a followup PR, though. - [x] ~~Should we add the ability to remove types from the filter regardless of whether an allowlist or denylist (e.g. `filter.remove::<Foo>()`)?~~ See the first list item - [x] ~~Should `SceneFilter` be an enum? Would it make more sense as a struct that contains an `is_denylist` boolean?~~ With the added `SceneFilter::None` state (replacing the need to wrap in an `Option` or rely on an empty `Denylist`), it seems an enum is better suited now - [x] ~~Bikeshed: Do we like the naming convention? Should we instead use `include`/`exclude` terminology?~~ Sounds like we're sticking with `allow`/`deny`! - [x] ~~Does this feature need a new example? Do we simply include it in the existing one (maybe even as a comment?)? Should this be done in a followup PR instead?~~ Example will be added in a followup PR ### Followup Tasks - [ ] Add a dedicated `SceneFilter` example - [ ] Possibly add default types to the filter (e.g. deny things like `ComputedVisibility`, allow `Parent`, etc) --- ## Changelog - Added the `SceneFilter` enum for filtering components and resources when building a `DynamicScene` - Added methods: - `DynamicSceneBuilder::with_filter` - `DynamicSceneBuilder::allow` - `DynamicSceneBuilder::deny` - `DynamicSceneBuilder::allow_all` - `DynamicSceneBuilder::deny_all` - `DynamicSceneBuilder::with_resource_filter` - `DynamicSceneBuilder::allow_resource` - `DynamicSceneBuilder::deny_resource` - `DynamicSceneBuilder::allow_all_resources` - `DynamicSceneBuilder::deny_all_resources` - Removed methods: - `DynamicSceneBuilder::from_world_with_type_registry` - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference ## Migration Guide - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let dynamic_scene = DynamicScene::from_world(&world, registry); // let dynamic_scene = DynamicScene::from_scene(&scene, registry); // NEW let dynamic_scene = DynamicScene::from_world(&world); // let dynamic_scene = DynamicScene::from_scene(&scene); ``` - Removed `DynamicSceneBuilder::from_world_with_type_registry`. Now the registry is automatically taken from the given world: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let builder = DynamicSceneBuilder::from_world_with_type_registry(&world, registry); // NEW let builder = DynamicSceneBuilder::from_world(&world); ```
2023-07-06 21:04:26 +00:00
self.resource_filter = SceneFilter::deny_all();
self
}
/// Consume the builder, producing a [`DynamicScene`].
///
/// To make sure the dynamic scene doesn't contain entities without any components, call
/// [`Self::remove_empty_entities`] before building the scene.
#[must_use]
pub fn build(self) -> DynamicScene {
DynamicScene {
(De) serialize resources in scenes (#6846) # Objective Co-Authored-By: davier [bricedavier@gmail.com](mailto:bricedavier@gmail.com) Fixes #3576. Adds a `resources` field in scene serialization data to allow de/serializing resources that have reflection enabled. ## Solution Most of this code is taken from a previous closed PR: https://github.com/bevyengine/bevy/pull/3580. Most of the credit goes to @Davier , what I did was mostly getting it to work on the latest main branch of Bevy, along with adding a few asserts in the currently existing tests to be sure everything is working properly. This PR changes the scene format to include resources in this way: ``` ( resources: { // List of resources here, keyed by resource type name. }, entities: [ // Previous scene format here ], ) ``` An example taken from the tests: ``` ( resources: { "bevy_scene::serde::tests::MyResource": ( foo: 123, ), }, entities: { // Previous scene format here }, ) ``` For this, a `resources` fields has been added on the `DynamicScene` and the `DynamicSceneBuilder` structs. The latter now also has a method named `extract_resources` to properly extract the existing resources registered in the local type registry, in a similar way to `extract_entities`. --- ## Changelog Added: Reflect resources registered in the type registry used by dynamic scenes will now be properly de/serialized in scene data. ## Migration Guide Since the scene format has been changed, the user may not be able to use scenes saved prior to this PR due to the `resources` scene field being missing. ~~To preserve backwards compatibility, I will try to make the `resources` fully optional so that old scenes can be loaded without issue.~~ ## TODOs - [x] I may have to update a few doc blocks still referring to dynamic scenes as mere container of entities, since they now include resources as well. - [x] ~~I want to make the `resources` key optional, as specified in the Migration Guide, so that old scenes will be compatible with this change.~~ Since this would only be trivial for ron format, I think it might be better to consider it in a separate PR/discussion to figure out if it could be done for binary serialization too. - [x] I suppose it might be a good idea to add a resources in the scene example so that users will quickly notice they can serialize resources just like entities. --------- Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2023-03-20 21:17:02 +00:00
resources: self.extracted_resources.into_values().collect(),
entities: self.extracted_scene.into_values().collect(),
}
}
/// Extract one entity from the builder's [`World`].
///
/// Re-extracting an entity that was already extracted will have no effect.
#[must_use]
pub fn extract_entity(self, entity: Entity) -> Self {
Add `core` and `alloc` over `std` Lints (#15281) # Objective - Fixes #6370 - Closes #6581 ## Solution - Added the following lints to the workspace: - `std_instead_of_core` - `std_instead_of_alloc` - `alloc_instead_of_core` - Used `cargo +nightly fmt` with [item level use formatting](https://rust-lang.github.io/rustfmt/?version=v1.6.0&search=#Item%5C%3A) to split all `use` statements into single items. - Used `cargo clippy --workspace --all-targets --all-features --fix --allow-dirty` to _attempt_ to resolve the new linting issues, and intervened where the lint was unable to resolve the issue automatically (usually due to needing an `extern crate alloc;` statement in a crate root). - Manually removed certain uses of `std` where negative feature gating prevented `--all-features` from finding the offending uses. - Used `cargo +nightly fmt` with [crate level use formatting](https://rust-lang.github.io/rustfmt/?version=v1.6.0&search=#Crate%5C%3A) to re-merge all `use` statements matching Bevy's previous styling. - Manually fixed cases where the `fmt` tool could not re-merge `use` statements due to conditional compilation attributes. ## Testing - Ran CI locally ## Migration Guide The MSRV is now 1.81. Please update to this version or higher. ## Notes - This is a _massive_ change to try and push through, which is why I've outlined the semi-automatic steps I used to create this PR, in case this fails and someone else tries again in the future. - Making this change has no impact on user code, but does mean Bevy contributors will be warned to use `core` and `alloc` instead of `std` where possible. - This lint is a critical first step towards investigating `no_std` options for Bevy. --------- Co-authored-by: François Mockers <francois.mockers@vleue.com>
2024-09-27 00:59:59 +00:00
self.extract_entities(core::iter::once(entity))
}
/// Despawns all entities with no components.
///
/// These were likely created because none of their components were present in the provided type registry upon extraction.
#[must_use]
pub fn remove_empty_entities(mut self) -> Self {
self.extracted_scene
.retain(|_, entity| !entity.components.is_empty());
self
}
/// Extract entities from the builder's [`World`].
///
/// Re-extracting an entity that was already extracted will have no effect.
///
bevy_scene: Add `SceneFilter` (#6793) # Objective Currently, `DynamicScene`s extract all components listed in the given (or the world's) type registry. This acts as a quasi-filter of sorts. However, it can be troublesome to use effectively and lacks decent control. For example, say you need to serialize only the following component over the network: ```rust #[derive(Reflect, Component, Default)] #[reflect(Component)] struct NPC { name: Option<String> } ``` To do this, you'd need to: 1. Create a new `AppTypeRegistry` 2. Register `NPC` 3. Register `Option<String>` If we skip Step 3, then the entire scene might fail to serialize as `Option<String>` requires registration. Not only is this annoying and easy to forget, but it can leave users with an impossible task: serializing a third-party type that contains private types. Generally, the third-party crate will register their private types within a plugin so the user doesn't need to do it themselves. However, this means we are now unable to serialize _just_ that type— we're forced to allow everything! ## Solution Add the `SceneFilter` enum for filtering components to extract. This filter can be used to optionally allow or deny entire sets of components/resources. With the `DynamicSceneBuilder`, users have more control over how their `DynamicScene`s are built. To only serialize a subset of components, use the `allow` method: ```rust let scene = builder .allow::<ComponentA>() .allow::<ComponentB>() .extract_entity(entity) .build(); ``` To serialize everything _but_ a subset of components, use the `deny` method: ```rust let scene = builder .deny::<ComponentA>() .deny::<ComponentB>() .extract_entity(entity) .build(); ``` Or create a custom filter: ```rust let components = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(components); // let filter = SceneFilter::Denylist(components); let scene = builder .with_filter(Some(filter)) .extract_entity(entity) .build(); ``` Similar operations exist for resources: <details> <summary>View Resource Methods</summary> To only serialize a subset of resources, use the `allow_resource` method: ```rust let scene = builder .allow_resource::<ResourceA>() .extract_resources() .build(); ``` To serialize everything _but_ a subset of resources, use the `deny_resource` method: ```rust let scene = builder .deny_resource::<ResourceA>() .extract_resources() .build(); ``` Or create a custom filter: ```rust let resources = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(resources); // let filter = SceneFilter::Denylist(resources); let scene = builder .with_resource_filter(Some(filter)) .extract_resources() .build(); ``` </details> ### Open Questions - [x] ~~`allow` and `deny` are mutually exclusive. Currently, they overwrite each other. Should this instead be a panic?~~ Took @soqb's suggestion and made it so that the opposing method simply removes that type from the list. - [x] ~~`DynamicSceneBuilder` extracts entity data as soon as `extract_entity`/`extract_entities` is called. Should this behavior instead be moved to the `build` method to prevent ordering mixups (e.g. `.allow::<Foo>().extract_entity(entity)` vs `.extract_entity(entity).allow::<Foo>()`)? The tradeoff would be iterating over the given entities twice: once at extraction and again at build.~~ Based on the feedback from @Testare it sounds like it might be better to just keep the current functionality (if anything we can open a separate PR that adds deferred methods for extraction, so the choice/performance hit is up to the user). - [ ] An alternative might be to remove the filter from `DynamicSceneBuilder` and have it as a separate parameter to the extraction methods (either in the existing ones or as added `extract_entity_with_filter`-type methods). Is this preferable? - [x] ~~Should we include constructors that include common types to allow/deny? For example, a `SceneFilter::standard_allowlist` that includes things like `Parent` and `Children`?~~ Consensus suggests we should. I may split this out into a followup PR, though. - [x] ~~Should we add the ability to remove types from the filter regardless of whether an allowlist or denylist (e.g. `filter.remove::<Foo>()`)?~~ See the first list item - [x] ~~Should `SceneFilter` be an enum? Would it make more sense as a struct that contains an `is_denylist` boolean?~~ With the added `SceneFilter::None` state (replacing the need to wrap in an `Option` or rely on an empty `Denylist`), it seems an enum is better suited now - [x] ~~Bikeshed: Do we like the naming convention? Should we instead use `include`/`exclude` terminology?~~ Sounds like we're sticking with `allow`/`deny`! - [x] ~~Does this feature need a new example? Do we simply include it in the existing one (maybe even as a comment?)? Should this be done in a followup PR instead?~~ Example will be added in a followup PR ### Followup Tasks - [ ] Add a dedicated `SceneFilter` example - [ ] Possibly add default types to the filter (e.g. deny things like `ComputedVisibility`, allow `Parent`, etc) --- ## Changelog - Added the `SceneFilter` enum for filtering components and resources when building a `DynamicScene` - Added methods: - `DynamicSceneBuilder::with_filter` - `DynamicSceneBuilder::allow` - `DynamicSceneBuilder::deny` - `DynamicSceneBuilder::allow_all` - `DynamicSceneBuilder::deny_all` - `DynamicSceneBuilder::with_resource_filter` - `DynamicSceneBuilder::allow_resource` - `DynamicSceneBuilder::deny_resource` - `DynamicSceneBuilder::allow_all_resources` - `DynamicSceneBuilder::deny_all_resources` - Removed methods: - `DynamicSceneBuilder::from_world_with_type_registry` - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference ## Migration Guide - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let dynamic_scene = DynamicScene::from_world(&world, registry); // let dynamic_scene = DynamicScene::from_scene(&scene, registry); // NEW let dynamic_scene = DynamicScene::from_world(&world); // let dynamic_scene = DynamicScene::from_scene(&scene); ``` - Removed `DynamicSceneBuilder::from_world_with_type_registry`. Now the registry is automatically taken from the given world: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let builder = DynamicSceneBuilder::from_world_with_type_registry(&world, registry); // NEW let builder = DynamicSceneBuilder::from_world(&world); ```
2023-07-06 21:04:26 +00:00
/// To control which components are extracted, use the [`allow`] or
/// [`deny`] helper methods.
///
/// This method may be used to extract entities from a query:
/// ```
/// # use bevy_scene::DynamicSceneBuilder;
/// # use bevy_ecs::reflect::AppTypeRegistry;
/// # use bevy_ecs::{
/// # component::Component, prelude::Entity, query::With, reflect::ReflectComponent, world::World,
/// # };
/// # use bevy_reflect::Reflect;
/// #[derive(Component, Default, Reflect)]
/// #[reflect(Component)]
/// struct MyComponent;
///
/// # let mut world = World::default();
/// # world.init_resource::<AppTypeRegistry>();
/// # let _entity = world.spawn(MyComponent).id();
/// let mut query = world.query_filtered::<Entity, With<MyComponent>>();
///
/// let scene = DynamicSceneBuilder::from_world(&world)
/// .extract_entities(query.iter(&world))
/// .build();
/// ```
bevy_scene: Add `SceneFilter` (#6793) # Objective Currently, `DynamicScene`s extract all components listed in the given (or the world's) type registry. This acts as a quasi-filter of sorts. However, it can be troublesome to use effectively and lacks decent control. For example, say you need to serialize only the following component over the network: ```rust #[derive(Reflect, Component, Default)] #[reflect(Component)] struct NPC { name: Option<String> } ``` To do this, you'd need to: 1. Create a new `AppTypeRegistry` 2. Register `NPC` 3. Register `Option<String>` If we skip Step 3, then the entire scene might fail to serialize as `Option<String>` requires registration. Not only is this annoying and easy to forget, but it can leave users with an impossible task: serializing a third-party type that contains private types. Generally, the third-party crate will register their private types within a plugin so the user doesn't need to do it themselves. However, this means we are now unable to serialize _just_ that type— we're forced to allow everything! ## Solution Add the `SceneFilter` enum for filtering components to extract. This filter can be used to optionally allow or deny entire sets of components/resources. With the `DynamicSceneBuilder`, users have more control over how their `DynamicScene`s are built. To only serialize a subset of components, use the `allow` method: ```rust let scene = builder .allow::<ComponentA>() .allow::<ComponentB>() .extract_entity(entity) .build(); ``` To serialize everything _but_ a subset of components, use the `deny` method: ```rust let scene = builder .deny::<ComponentA>() .deny::<ComponentB>() .extract_entity(entity) .build(); ``` Or create a custom filter: ```rust let components = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(components); // let filter = SceneFilter::Denylist(components); let scene = builder .with_filter(Some(filter)) .extract_entity(entity) .build(); ``` Similar operations exist for resources: <details> <summary>View Resource Methods</summary> To only serialize a subset of resources, use the `allow_resource` method: ```rust let scene = builder .allow_resource::<ResourceA>() .extract_resources() .build(); ``` To serialize everything _but_ a subset of resources, use the `deny_resource` method: ```rust let scene = builder .deny_resource::<ResourceA>() .extract_resources() .build(); ``` Or create a custom filter: ```rust let resources = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(resources); // let filter = SceneFilter::Denylist(resources); let scene = builder .with_resource_filter(Some(filter)) .extract_resources() .build(); ``` </details> ### Open Questions - [x] ~~`allow` and `deny` are mutually exclusive. Currently, they overwrite each other. Should this instead be a panic?~~ Took @soqb's suggestion and made it so that the opposing method simply removes that type from the list. - [x] ~~`DynamicSceneBuilder` extracts entity data as soon as `extract_entity`/`extract_entities` is called. Should this behavior instead be moved to the `build` method to prevent ordering mixups (e.g. `.allow::<Foo>().extract_entity(entity)` vs `.extract_entity(entity).allow::<Foo>()`)? The tradeoff would be iterating over the given entities twice: once at extraction and again at build.~~ Based on the feedback from @Testare it sounds like it might be better to just keep the current functionality (if anything we can open a separate PR that adds deferred methods for extraction, so the choice/performance hit is up to the user). - [ ] An alternative might be to remove the filter from `DynamicSceneBuilder` and have it as a separate parameter to the extraction methods (either in the existing ones or as added `extract_entity_with_filter`-type methods). Is this preferable? - [x] ~~Should we include constructors that include common types to allow/deny? For example, a `SceneFilter::standard_allowlist` that includes things like `Parent` and `Children`?~~ Consensus suggests we should. I may split this out into a followup PR, though. - [x] ~~Should we add the ability to remove types from the filter regardless of whether an allowlist or denylist (e.g. `filter.remove::<Foo>()`)?~~ See the first list item - [x] ~~Should `SceneFilter` be an enum? Would it make more sense as a struct that contains an `is_denylist` boolean?~~ With the added `SceneFilter::None` state (replacing the need to wrap in an `Option` or rely on an empty `Denylist`), it seems an enum is better suited now - [x] ~~Bikeshed: Do we like the naming convention? Should we instead use `include`/`exclude` terminology?~~ Sounds like we're sticking with `allow`/`deny`! - [x] ~~Does this feature need a new example? Do we simply include it in the existing one (maybe even as a comment?)? Should this be done in a followup PR instead?~~ Example will be added in a followup PR ### Followup Tasks - [ ] Add a dedicated `SceneFilter` example - [ ] Possibly add default types to the filter (e.g. deny things like `ComputedVisibility`, allow `Parent`, etc) --- ## Changelog - Added the `SceneFilter` enum for filtering components and resources when building a `DynamicScene` - Added methods: - `DynamicSceneBuilder::with_filter` - `DynamicSceneBuilder::allow` - `DynamicSceneBuilder::deny` - `DynamicSceneBuilder::allow_all` - `DynamicSceneBuilder::deny_all` - `DynamicSceneBuilder::with_resource_filter` - `DynamicSceneBuilder::allow_resource` - `DynamicSceneBuilder::deny_resource` - `DynamicSceneBuilder::allow_all_resources` - `DynamicSceneBuilder::deny_all_resources` - Removed methods: - `DynamicSceneBuilder::from_world_with_type_registry` - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference ## Migration Guide - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let dynamic_scene = DynamicScene::from_world(&world, registry); // let dynamic_scene = DynamicScene::from_scene(&scene, registry); // NEW let dynamic_scene = DynamicScene::from_world(&world); // let dynamic_scene = DynamicScene::from_scene(&scene); ``` - Removed `DynamicSceneBuilder::from_world_with_type_registry`. Now the registry is automatically taken from the given world: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let builder = DynamicSceneBuilder::from_world_with_type_registry(&world, registry); // NEW let builder = DynamicSceneBuilder::from_world(&world); ```
2023-07-06 21:04:26 +00:00
///
/// Note that components extracted from queried entities must still pass through the filter if one is set.
///
/// [`allow`]: Self::allow_component
/// [`deny`]: Self::deny_component
#[must_use]
pub fn extract_entities(mut self, entities: impl Iterator<Item = Entity>) -> Self {
bevy_scene: Add `SceneFilter` (#6793) # Objective Currently, `DynamicScene`s extract all components listed in the given (or the world's) type registry. This acts as a quasi-filter of sorts. However, it can be troublesome to use effectively and lacks decent control. For example, say you need to serialize only the following component over the network: ```rust #[derive(Reflect, Component, Default)] #[reflect(Component)] struct NPC { name: Option<String> } ``` To do this, you'd need to: 1. Create a new `AppTypeRegistry` 2. Register `NPC` 3. Register `Option<String>` If we skip Step 3, then the entire scene might fail to serialize as `Option<String>` requires registration. Not only is this annoying and easy to forget, but it can leave users with an impossible task: serializing a third-party type that contains private types. Generally, the third-party crate will register their private types within a plugin so the user doesn't need to do it themselves. However, this means we are now unable to serialize _just_ that type— we're forced to allow everything! ## Solution Add the `SceneFilter` enum for filtering components to extract. This filter can be used to optionally allow or deny entire sets of components/resources. With the `DynamicSceneBuilder`, users have more control over how their `DynamicScene`s are built. To only serialize a subset of components, use the `allow` method: ```rust let scene = builder .allow::<ComponentA>() .allow::<ComponentB>() .extract_entity(entity) .build(); ``` To serialize everything _but_ a subset of components, use the `deny` method: ```rust let scene = builder .deny::<ComponentA>() .deny::<ComponentB>() .extract_entity(entity) .build(); ``` Or create a custom filter: ```rust let components = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(components); // let filter = SceneFilter::Denylist(components); let scene = builder .with_filter(Some(filter)) .extract_entity(entity) .build(); ``` Similar operations exist for resources: <details> <summary>View Resource Methods</summary> To only serialize a subset of resources, use the `allow_resource` method: ```rust let scene = builder .allow_resource::<ResourceA>() .extract_resources() .build(); ``` To serialize everything _but_ a subset of resources, use the `deny_resource` method: ```rust let scene = builder .deny_resource::<ResourceA>() .extract_resources() .build(); ``` Or create a custom filter: ```rust let resources = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(resources); // let filter = SceneFilter::Denylist(resources); let scene = builder .with_resource_filter(Some(filter)) .extract_resources() .build(); ``` </details> ### Open Questions - [x] ~~`allow` and `deny` are mutually exclusive. Currently, they overwrite each other. Should this instead be a panic?~~ Took @soqb's suggestion and made it so that the opposing method simply removes that type from the list. - [x] ~~`DynamicSceneBuilder` extracts entity data as soon as `extract_entity`/`extract_entities` is called. Should this behavior instead be moved to the `build` method to prevent ordering mixups (e.g. `.allow::<Foo>().extract_entity(entity)` vs `.extract_entity(entity).allow::<Foo>()`)? The tradeoff would be iterating over the given entities twice: once at extraction and again at build.~~ Based on the feedback from @Testare it sounds like it might be better to just keep the current functionality (if anything we can open a separate PR that adds deferred methods for extraction, so the choice/performance hit is up to the user). - [ ] An alternative might be to remove the filter from `DynamicSceneBuilder` and have it as a separate parameter to the extraction methods (either in the existing ones or as added `extract_entity_with_filter`-type methods). Is this preferable? - [x] ~~Should we include constructors that include common types to allow/deny? For example, a `SceneFilter::standard_allowlist` that includes things like `Parent` and `Children`?~~ Consensus suggests we should. I may split this out into a followup PR, though. - [x] ~~Should we add the ability to remove types from the filter regardless of whether an allowlist or denylist (e.g. `filter.remove::<Foo>()`)?~~ See the first list item - [x] ~~Should `SceneFilter` be an enum? Would it make more sense as a struct that contains an `is_denylist` boolean?~~ With the added `SceneFilter::None` state (replacing the need to wrap in an `Option` or rely on an empty `Denylist`), it seems an enum is better suited now - [x] ~~Bikeshed: Do we like the naming convention? Should we instead use `include`/`exclude` terminology?~~ Sounds like we're sticking with `allow`/`deny`! - [x] ~~Does this feature need a new example? Do we simply include it in the existing one (maybe even as a comment?)? Should this be done in a followup PR instead?~~ Example will be added in a followup PR ### Followup Tasks - [ ] Add a dedicated `SceneFilter` example - [ ] Possibly add default types to the filter (e.g. deny things like `ComputedVisibility`, allow `Parent`, etc) --- ## Changelog - Added the `SceneFilter` enum for filtering components and resources when building a `DynamicScene` - Added methods: - `DynamicSceneBuilder::with_filter` - `DynamicSceneBuilder::allow` - `DynamicSceneBuilder::deny` - `DynamicSceneBuilder::allow_all` - `DynamicSceneBuilder::deny_all` - `DynamicSceneBuilder::with_resource_filter` - `DynamicSceneBuilder::allow_resource` - `DynamicSceneBuilder::deny_resource` - `DynamicSceneBuilder::allow_all_resources` - `DynamicSceneBuilder::deny_all_resources` - Removed methods: - `DynamicSceneBuilder::from_world_with_type_registry` - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference ## Migration Guide - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let dynamic_scene = DynamicScene::from_world(&world, registry); // let dynamic_scene = DynamicScene::from_scene(&scene, registry); // NEW let dynamic_scene = DynamicScene::from_world(&world); // let dynamic_scene = DynamicScene::from_scene(&scene); ``` - Removed `DynamicSceneBuilder::from_world_with_type_registry`. Now the registry is automatically taken from the given world: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let builder = DynamicSceneBuilder::from_world_with_type_registry(&world, registry); // NEW let builder = DynamicSceneBuilder::from_world(&world); ```
2023-07-06 21:04:26 +00:00
let type_registry = self.original_world.resource::<AppTypeRegistry>().read();
for entity in entities {
Make scene handling of entity references robust (#7335) # Objective - Handle dangling entity references inside scenes - Handle references to entities with generation > 0 inside scenes - Fix a latent bug in `Parent`'s `MapEntities` implementation, which would, if the parent was outside the scene, cause the scene to be loaded into the new world with a parent reference potentially pointing to some random entity in that new world. - Fixes #4793 and addresses #7235 ## Solution - DynamicScenes now identify entities with a `Entity` instead of a u32, therefore including generation - `World` exposes a new `reserve_generations` function that despawns an entity and advances its generation by some extra amount. - `MapEntities` implementations have a new `get_or_reserve` function available that will always return an `Entity`, establishing a new mapping to a dead entity when the entity they are called with is not in the `EntityMap`. Subsequent calls with that same `Entity` will return the same newly created dead entity reference, preserving equality semantics. - As a result, after loading a scene containing references to dead entities (or entities otherwise outside the scene), those references will all point to different generations on a single entity id in the new world. --- ## Changelog ### Changed - In serialized scenes, entities are now identified by a u64 instead of a u32. - In serialized scenes, components with entity references now have those references serialize as u64s instead of structs. ### Fixed - Scenes containing components with entity references will now deserialize and add to a world reliably. ## Migration Guide - `MapEntities` implementations must change from a `&EntityMap` parameter to a `&mut EntityMapper` parameter and can no longer return a `Result`. Finally, they should switch from calling `EntityMap::get` to calling `EntityMapper::get_or_reserve`. --------- Co-authored-by: Nicola Papale <nicopap@users.noreply.github.com>
2023-05-01 15:49:27 +00:00
if self.extracted_scene.contains_key(&entity) {
continue;
}
let mut entry = DynamicEntity {
Make scene handling of entity references robust (#7335) # Objective - Handle dangling entity references inside scenes - Handle references to entities with generation > 0 inside scenes - Fix a latent bug in `Parent`'s `MapEntities` implementation, which would, if the parent was outside the scene, cause the scene to be loaded into the new world with a parent reference potentially pointing to some random entity in that new world. - Fixes #4793 and addresses #7235 ## Solution - DynamicScenes now identify entities with a `Entity` instead of a u32, therefore including generation - `World` exposes a new `reserve_generations` function that despawns an entity and advances its generation by some extra amount. - `MapEntities` implementations have a new `get_or_reserve` function available that will always return an `Entity`, establishing a new mapping to a dead entity when the entity they are called with is not in the `EntityMap`. Subsequent calls with that same `Entity` will return the same newly created dead entity reference, preserving equality semantics. - As a result, after loading a scene containing references to dead entities (or entities otherwise outside the scene), those references will all point to different generations on a single entity id in the new world. --- ## Changelog ### Changed - In serialized scenes, entities are now identified by a u64 instead of a u32. - In serialized scenes, components with entity references now have those references serialize as u64s instead of structs. ### Fixed - Scenes containing components with entity references will now deserialize and add to a world reliably. ## Migration Guide - `MapEntities` implementations must change from a `&EntityMap` parameter to a `&mut EntityMapper` parameter and can no longer return a `Result`. Finally, they should switch from calling `EntityMap::get` to calling `EntityMapper::get_or_reserve`. --------- Co-authored-by: Nicola Papale <nicopap@users.noreply.github.com>
2023-05-01 15:49:27 +00:00
entity,
components: Vec::new(),
};
Make scene handling of entity references robust (#7335) # Objective - Handle dangling entity references inside scenes - Handle references to entities with generation > 0 inside scenes - Fix a latent bug in `Parent`'s `MapEntities` implementation, which would, if the parent was outside the scene, cause the scene to be loaded into the new world with a parent reference potentially pointing to some random entity in that new world. - Fixes #4793 and addresses #7235 ## Solution - DynamicScenes now identify entities with a `Entity` instead of a u32, therefore including generation - `World` exposes a new `reserve_generations` function that despawns an entity and advances its generation by some extra amount. - `MapEntities` implementations have a new `get_or_reserve` function available that will always return an `Entity`, establishing a new mapping to a dead entity when the entity they are called with is not in the `EntityMap`. Subsequent calls with that same `Entity` will return the same newly created dead entity reference, preserving equality semantics. - As a result, after loading a scene containing references to dead entities (or entities otherwise outside the scene), those references will all point to different generations on a single entity id in the new world. --- ## Changelog ### Changed - In serialized scenes, entities are now identified by a u64 instead of a u32. - In serialized scenes, components with entity references now have those references serialize as u64s instead of structs. ### Fixed - Scenes containing components with entity references will now deserialize and add to a world reliably. ## Migration Guide - `MapEntities` implementations must change from a `&EntityMap` parameter to a `&mut EntityMapper` parameter and can no longer return a `Result`. Finally, they should switch from calling `EntityMap::get` to calling `EntityMapper::get_or_reserve`. --------- Co-authored-by: Nicola Papale <nicopap@users.noreply.github.com>
2023-05-01 15:49:27 +00:00
let original_entity = self.original_world.entity(entity);
for component_id in original_entity.archetype().components() {
(De) serialize resources in scenes (#6846) # Objective Co-Authored-By: davier [bricedavier@gmail.com](mailto:bricedavier@gmail.com) Fixes #3576. Adds a `resources` field in scene serialization data to allow de/serializing resources that have reflection enabled. ## Solution Most of this code is taken from a previous closed PR: https://github.com/bevyengine/bevy/pull/3580. Most of the credit goes to @Davier , what I did was mostly getting it to work on the latest main branch of Bevy, along with adding a few asserts in the currently existing tests to be sure everything is working properly. This PR changes the scene format to include resources in this way: ``` ( resources: { // List of resources here, keyed by resource type name. }, entities: [ // Previous scene format here ], ) ``` An example taken from the tests: ``` ( resources: { "bevy_scene::serde::tests::MyResource": ( foo: 123, ), }, entities: { // Previous scene format here }, ) ``` For this, a `resources` fields has been added on the `DynamicScene` and the `DynamicSceneBuilder` structs. The latter now also has a method named `extract_resources` to properly extract the existing resources registered in the local type registry, in a similar way to `extract_entities`. --- ## Changelog Added: Reflect resources registered in the type registry used by dynamic scenes will now be properly de/serialized in scene data. ## Migration Guide Since the scene format has been changed, the user may not be able to use scenes saved prior to this PR due to the `resources` scene field being missing. ~~To preserve backwards compatibility, I will try to make the `resources` fully optional so that old scenes can be loaded without issue.~~ ## TODOs - [x] I may have to update a few doc blocks still referring to dynamic scenes as mere container of entities, since they now include resources as well. - [x] ~~I want to make the `resources` key optional, as specified in the Migration Guide, so that old scenes will be compatible with this change.~~ Since this would only be trivial for ron format, I think it might be better to consider it in a separate PR/discussion to figure out if it could be done for binary serialization too. - [x] I suppose it might be a good idea to add a resources in the scene example so that users will quickly notice they can serialize resources just like entities. --------- Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2023-03-20 21:17:02 +00:00
let mut extract_and_push = || {
let type_id = self
.original_world
.components()
.get_info(component_id)?
.type_id()?;
bevy_scene: Add `SceneFilter` (#6793) # Objective Currently, `DynamicScene`s extract all components listed in the given (or the world's) type registry. This acts as a quasi-filter of sorts. However, it can be troublesome to use effectively and lacks decent control. For example, say you need to serialize only the following component over the network: ```rust #[derive(Reflect, Component, Default)] #[reflect(Component)] struct NPC { name: Option<String> } ``` To do this, you'd need to: 1. Create a new `AppTypeRegistry` 2. Register `NPC` 3. Register `Option<String>` If we skip Step 3, then the entire scene might fail to serialize as `Option<String>` requires registration. Not only is this annoying and easy to forget, but it can leave users with an impossible task: serializing a third-party type that contains private types. Generally, the third-party crate will register their private types within a plugin so the user doesn't need to do it themselves. However, this means we are now unable to serialize _just_ that type— we're forced to allow everything! ## Solution Add the `SceneFilter` enum for filtering components to extract. This filter can be used to optionally allow or deny entire sets of components/resources. With the `DynamicSceneBuilder`, users have more control over how their `DynamicScene`s are built. To only serialize a subset of components, use the `allow` method: ```rust let scene = builder .allow::<ComponentA>() .allow::<ComponentB>() .extract_entity(entity) .build(); ``` To serialize everything _but_ a subset of components, use the `deny` method: ```rust let scene = builder .deny::<ComponentA>() .deny::<ComponentB>() .extract_entity(entity) .build(); ``` Or create a custom filter: ```rust let components = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(components); // let filter = SceneFilter::Denylist(components); let scene = builder .with_filter(Some(filter)) .extract_entity(entity) .build(); ``` Similar operations exist for resources: <details> <summary>View Resource Methods</summary> To only serialize a subset of resources, use the `allow_resource` method: ```rust let scene = builder .allow_resource::<ResourceA>() .extract_resources() .build(); ``` To serialize everything _but_ a subset of resources, use the `deny_resource` method: ```rust let scene = builder .deny_resource::<ResourceA>() .extract_resources() .build(); ``` Or create a custom filter: ```rust let resources = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(resources); // let filter = SceneFilter::Denylist(resources); let scene = builder .with_resource_filter(Some(filter)) .extract_resources() .build(); ``` </details> ### Open Questions - [x] ~~`allow` and `deny` are mutually exclusive. Currently, they overwrite each other. Should this instead be a panic?~~ Took @soqb's suggestion and made it so that the opposing method simply removes that type from the list. - [x] ~~`DynamicSceneBuilder` extracts entity data as soon as `extract_entity`/`extract_entities` is called. Should this behavior instead be moved to the `build` method to prevent ordering mixups (e.g. `.allow::<Foo>().extract_entity(entity)` vs `.extract_entity(entity).allow::<Foo>()`)? The tradeoff would be iterating over the given entities twice: once at extraction and again at build.~~ Based on the feedback from @Testare it sounds like it might be better to just keep the current functionality (if anything we can open a separate PR that adds deferred methods for extraction, so the choice/performance hit is up to the user). - [ ] An alternative might be to remove the filter from `DynamicSceneBuilder` and have it as a separate parameter to the extraction methods (either in the existing ones or as added `extract_entity_with_filter`-type methods). Is this preferable? - [x] ~~Should we include constructors that include common types to allow/deny? For example, a `SceneFilter::standard_allowlist` that includes things like `Parent` and `Children`?~~ Consensus suggests we should. I may split this out into a followup PR, though. - [x] ~~Should we add the ability to remove types from the filter regardless of whether an allowlist or denylist (e.g. `filter.remove::<Foo>()`)?~~ See the first list item - [x] ~~Should `SceneFilter` be an enum? Would it make more sense as a struct that contains an `is_denylist` boolean?~~ With the added `SceneFilter::None` state (replacing the need to wrap in an `Option` or rely on an empty `Denylist`), it seems an enum is better suited now - [x] ~~Bikeshed: Do we like the naming convention? Should we instead use `include`/`exclude` terminology?~~ Sounds like we're sticking with `allow`/`deny`! - [x] ~~Does this feature need a new example? Do we simply include it in the existing one (maybe even as a comment?)? Should this be done in a followup PR instead?~~ Example will be added in a followup PR ### Followup Tasks - [ ] Add a dedicated `SceneFilter` example - [ ] Possibly add default types to the filter (e.g. deny things like `ComputedVisibility`, allow `Parent`, etc) --- ## Changelog - Added the `SceneFilter` enum for filtering components and resources when building a `DynamicScene` - Added methods: - `DynamicSceneBuilder::with_filter` - `DynamicSceneBuilder::allow` - `DynamicSceneBuilder::deny` - `DynamicSceneBuilder::allow_all` - `DynamicSceneBuilder::deny_all` - `DynamicSceneBuilder::with_resource_filter` - `DynamicSceneBuilder::allow_resource` - `DynamicSceneBuilder::deny_resource` - `DynamicSceneBuilder::allow_all_resources` - `DynamicSceneBuilder::deny_all_resources` - Removed methods: - `DynamicSceneBuilder::from_world_with_type_registry` - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference ## Migration Guide - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let dynamic_scene = DynamicScene::from_world(&world, registry); // let dynamic_scene = DynamicScene::from_scene(&scene, registry); // NEW let dynamic_scene = DynamicScene::from_world(&world); // let dynamic_scene = DynamicScene::from_scene(&scene); ``` - Removed `DynamicSceneBuilder::from_world_with_type_registry`. Now the registry is automatically taken from the given world: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let builder = DynamicSceneBuilder::from_world_with_type_registry(&world, registry); // NEW let builder = DynamicSceneBuilder::from_world(&world); ```
2023-07-06 21:04:26 +00:00
let is_denied = self.component_filter.is_denied_by_id(type_id);
if is_denied {
// Component is either in the denylist or _not_ in the allowlist
return None;
}
Use `FromReflect` when extracting entities in dynamic scenes (#15174) # Objective Fix #10284. ## Solution When `DynamicSceneBuilder` extracts entities, they are cloned via `PartialReflect::clone_value`, making them into dynamic versions of the original components. This loses any custom `ReflectSerialize` type data. Dynamic scenes are deserialized with the original types, not the dynamic versions, and so any component with a custom serialize may fail. In this case `Rect` and `Vec2`. The dynamic version includes the field names 'x' and 'y' but the `Serialize` impl doesn't, hence the "expect float" error. The solution here: Instead of using `clone_value` to clone the components, `FromReflect` clones and retains the original information needed to serialize with any custom `Serialize` impls. I think using something like `reflect_clone` from (https://github.com/bevyengine/bevy/pull/13432) might make this more efficient. I also did the same when deserializing dynamic scenes to appease some of the round-trip tests which use `ReflectPartialEq`, which requires the types be the same and not a unique/proxy pair. I'm not sure it's otherwise necessary. Maybe this would also be more efficient when spawning dynamic scenes with `reflect_clone` instead of `FromReflect` again? An alternative solution would be to fall back to the dynamic version when deserializing `DynamicScene`s if the custom version fails. I think that's possible. Or maybe simply always deserializing via the dynamic route for dynamic scenes? ## Testing This example is similar to the original test case in #10284: ``` rust #![allow(missing_docs)] use bevy::{prelude::*, scene::SceneInstanceReady}; fn main() { App::new() .add_plugins(DefaultPlugins) .add_systems(Startup, (save, load).chain()) .observe(check) .run(); } static SAVEGAME_SAVE_PATH: &str = "savegame.scn.ron"; fn save(world: &mut World) { let entity = world.spawn(OrthographicProjection::default()).id(); let scene = DynamicSceneBuilder::from_world(world) .extract_entity(entity) .build(); if let Some(registry) = world.get_resource::<AppTypeRegistry>() { let registry = registry.read(); let serialized_scene = scene.serialize(&registry).unwrap(); // println!("{}", serialized_scene); std::fs::write(format!("assets/{SAVEGAME_SAVE_PATH}"), serialized_scene).unwrap(); } world.entity_mut(entity).despawn_recursive(); } fn load(mut commands: Commands, asset_server: Res<AssetServer>) { commands.spawn(DynamicSceneBundle { scene: asset_server.load(SAVEGAME_SAVE_PATH), ..default() }); } fn check(_trigger: Trigger<SceneInstanceReady>, query: Query<&OrthographicProjection>) { dbg!(query.single()); } ``` ## Migration Guide The `DynamicScene` format is changed to use custom serialize impls so old scene files will need updating: Old: ```ron ( resources: {}, entities: { 4294967299: ( components: { "bevy_render::camera::projection::OrthographicProjection": ( near: 0.0, far: 1000.0, viewport_origin: ( x: 0.5, y: 0.5, ), scaling_mode: WindowSize(1.0), scale: 1.0, area: ( min: ( x: -1.0, y: -1.0, ), max: ( x: 1.0, y: 1.0, ), ), ), }, ), }, ) ``` New: ```ron ( resources: {}, entities: { 4294967299: ( components: { "bevy_render::camera::projection::OrthographicProjection": ( near: 0.0, far: 1000.0, viewport_origin: (0.5, 0.5), scaling_mode: WindowSize(1.0), scale: 1.0, area: ( min: (-1.0, -1.0), max: (1.0, 1.0), ), ), }, ), }, ) ``` --------- Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
2024-09-15 14:33:39 +00:00
let type_registration = type_registry.get(type_id)?;
let component = type_registration
(De) serialize resources in scenes (#6846) # Objective Co-Authored-By: davier [bricedavier@gmail.com](mailto:bricedavier@gmail.com) Fixes #3576. Adds a `resources` field in scene serialization data to allow de/serializing resources that have reflection enabled. ## Solution Most of this code is taken from a previous closed PR: https://github.com/bevyengine/bevy/pull/3580. Most of the credit goes to @Davier , what I did was mostly getting it to work on the latest main branch of Bevy, along with adding a few asserts in the currently existing tests to be sure everything is working properly. This PR changes the scene format to include resources in this way: ``` ( resources: { // List of resources here, keyed by resource type name. }, entities: [ // Previous scene format here ], ) ``` An example taken from the tests: ``` ( resources: { "bevy_scene::serde::tests::MyResource": ( foo: 123, ), }, entities: { // Previous scene format here }, ) ``` For this, a `resources` fields has been added on the `DynamicScene` and the `DynamicSceneBuilder` structs. The latter now also has a method named `extract_resources` to properly extract the existing resources registered in the local type registry, in a similar way to `extract_entities`. --- ## Changelog Added: Reflect resources registered in the type registry used by dynamic scenes will now be properly de/serialized in scene data. ## Migration Guide Since the scene format has been changed, the user may not be able to use scenes saved prior to this PR due to the `resources` scene field being missing. ~~To preserve backwards compatibility, I will try to make the `resources` fully optional so that old scenes can be loaded without issue.~~ ## TODOs - [x] I may have to update a few doc blocks still referring to dynamic scenes as mere container of entities, since they now include resources as well. - [x] ~~I want to make the `resources` key optional, as specified in the Migration Guide, so that old scenes will be compatible with this change.~~ Since this would only be trivial for ron format, I think it might be better to consider it in a separate PR/discussion to figure out if it could be done for binary serialization too. - [x] I suppose it might be a good idea to add a resources in the scene example so that users will quickly notice they can serialize resources just like entities. --------- Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2023-03-20 21:17:02 +00:00
.data::<ReflectComponent>()?
Make scene handling of entity references robust (#7335) # Objective - Handle dangling entity references inside scenes - Handle references to entities with generation > 0 inside scenes - Fix a latent bug in `Parent`'s `MapEntities` implementation, which would, if the parent was outside the scene, cause the scene to be loaded into the new world with a parent reference potentially pointing to some random entity in that new world. - Fixes #4793 and addresses #7235 ## Solution - DynamicScenes now identify entities with a `Entity` instead of a u32, therefore including generation - `World` exposes a new `reserve_generations` function that despawns an entity and advances its generation by some extra amount. - `MapEntities` implementations have a new `get_or_reserve` function available that will always return an `Entity`, establishing a new mapping to a dead entity when the entity they are called with is not in the `EntityMap`. Subsequent calls with that same `Entity` will return the same newly created dead entity reference, preserving equality semantics. - As a result, after loading a scene containing references to dead entities (or entities otherwise outside the scene), those references will all point to different generations on a single entity id in the new world. --- ## Changelog ### Changed - In serialized scenes, entities are now identified by a u64 instead of a u32. - In serialized scenes, components with entity references now have those references serialize as u64s instead of structs. ### Fixed - Scenes containing components with entity references will now deserialize and add to a world reliably. ## Migration Guide - `MapEntities` implementations must change from a `&EntityMap` parameter to a `&mut EntityMapper` parameter and can no longer return a `Result`. Finally, they should switch from calling `EntityMap::get` to calling `EntityMapper::get_or_reserve`. --------- Co-authored-by: Nicola Papale <nicopap@users.noreply.github.com>
2023-05-01 15:49:27 +00:00
.reflect(original_entity)?;
Use `FromReflect` when extracting entities in dynamic scenes (#15174) # Objective Fix #10284. ## Solution When `DynamicSceneBuilder` extracts entities, they are cloned via `PartialReflect::clone_value`, making them into dynamic versions of the original components. This loses any custom `ReflectSerialize` type data. Dynamic scenes are deserialized with the original types, not the dynamic versions, and so any component with a custom serialize may fail. In this case `Rect` and `Vec2`. The dynamic version includes the field names 'x' and 'y' but the `Serialize` impl doesn't, hence the "expect float" error. The solution here: Instead of using `clone_value` to clone the components, `FromReflect` clones and retains the original information needed to serialize with any custom `Serialize` impls. I think using something like `reflect_clone` from (https://github.com/bevyengine/bevy/pull/13432) might make this more efficient. I also did the same when deserializing dynamic scenes to appease some of the round-trip tests which use `ReflectPartialEq`, which requires the types be the same and not a unique/proxy pair. I'm not sure it's otherwise necessary. Maybe this would also be more efficient when spawning dynamic scenes with `reflect_clone` instead of `FromReflect` again? An alternative solution would be to fall back to the dynamic version when deserializing `DynamicScene`s if the custom version fails. I think that's possible. Or maybe simply always deserializing via the dynamic route for dynamic scenes? ## Testing This example is similar to the original test case in #10284: ``` rust #![allow(missing_docs)] use bevy::{prelude::*, scene::SceneInstanceReady}; fn main() { App::new() .add_plugins(DefaultPlugins) .add_systems(Startup, (save, load).chain()) .observe(check) .run(); } static SAVEGAME_SAVE_PATH: &str = "savegame.scn.ron"; fn save(world: &mut World) { let entity = world.spawn(OrthographicProjection::default()).id(); let scene = DynamicSceneBuilder::from_world(world) .extract_entity(entity) .build(); if let Some(registry) = world.get_resource::<AppTypeRegistry>() { let registry = registry.read(); let serialized_scene = scene.serialize(&registry).unwrap(); // println!("{}", serialized_scene); std::fs::write(format!("assets/{SAVEGAME_SAVE_PATH}"), serialized_scene).unwrap(); } world.entity_mut(entity).despawn_recursive(); } fn load(mut commands: Commands, asset_server: Res<AssetServer>) { commands.spawn(DynamicSceneBundle { scene: asset_server.load(SAVEGAME_SAVE_PATH), ..default() }); } fn check(_trigger: Trigger<SceneInstanceReady>, query: Query<&OrthographicProjection>) { dbg!(query.single()); } ``` ## Migration Guide The `DynamicScene` format is changed to use custom serialize impls so old scene files will need updating: Old: ```ron ( resources: {}, entities: { 4294967299: ( components: { "bevy_render::camera::projection::OrthographicProjection": ( near: 0.0, far: 1000.0, viewport_origin: ( x: 0.5, y: 0.5, ), scaling_mode: WindowSize(1.0), scale: 1.0, area: ( min: ( x: -1.0, y: -1.0, ), max: ( x: 1.0, y: 1.0, ), ), ), }, ), }, ) ``` New: ```ron ( resources: {}, entities: { 4294967299: ( components: { "bevy_render::camera::projection::OrthographicProjection": ( near: 0.0, far: 1000.0, viewport_origin: (0.5, 0.5), scaling_mode: WindowSize(1.0), scale: 1.0, area: ( min: (-1.0, -1.0), max: (1.0, 1.0), ), ), }, ), }, ) ``` --------- Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
2024-09-15 14:33:39 +00:00
// Clone via `FromReflect`. Unlike `PartialReflect::clone_value` this
// retains the original type and `ReflectSerialize` type data which is needed to
// deserialize.
let component = type_registration
.data::<ReflectFromReflect>()
.and_then(|fr| fr.from_reflect(component.as_partial_reflect()))
.map(PartialReflect::into_partial_reflect)
.unwrap_or_else(|| component.clone_value());
entry.components.push(component);
(De) serialize resources in scenes (#6846) # Objective Co-Authored-By: davier [bricedavier@gmail.com](mailto:bricedavier@gmail.com) Fixes #3576. Adds a `resources` field in scene serialization data to allow de/serializing resources that have reflection enabled. ## Solution Most of this code is taken from a previous closed PR: https://github.com/bevyengine/bevy/pull/3580. Most of the credit goes to @Davier , what I did was mostly getting it to work on the latest main branch of Bevy, along with adding a few asserts in the currently existing tests to be sure everything is working properly. This PR changes the scene format to include resources in this way: ``` ( resources: { // List of resources here, keyed by resource type name. }, entities: [ // Previous scene format here ], ) ``` An example taken from the tests: ``` ( resources: { "bevy_scene::serde::tests::MyResource": ( foo: 123, ), }, entities: { // Previous scene format here }, ) ``` For this, a `resources` fields has been added on the `DynamicScene` and the `DynamicSceneBuilder` structs. The latter now also has a method named `extract_resources` to properly extract the existing resources registered in the local type registry, in a similar way to `extract_entities`. --- ## Changelog Added: Reflect resources registered in the type registry used by dynamic scenes will now be properly de/serialized in scene data. ## Migration Guide Since the scene format has been changed, the user may not be able to use scenes saved prior to this PR due to the `resources` scene field being missing. ~~To preserve backwards compatibility, I will try to make the `resources` fully optional so that old scenes can be loaded without issue.~~ ## TODOs - [x] I may have to update a few doc blocks still referring to dynamic scenes as mere container of entities, since they now include resources as well. - [x] ~~I want to make the `resources` key optional, as specified in the Migration Guide, so that old scenes will be compatible with this change.~~ Since this would only be trivial for ron format, I think it might be better to consider it in a separate PR/discussion to figure out if it could be done for binary serialization too. - [x] I suppose it might be a good idea to add a resources in the scene example so that users will quickly notice they can serialize resources just like entities. --------- Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2023-03-20 21:17:02 +00:00
Some(())
};
extract_and_push();
}
Make scene handling of entity references robust (#7335) # Objective - Handle dangling entity references inside scenes - Handle references to entities with generation > 0 inside scenes - Fix a latent bug in `Parent`'s `MapEntities` implementation, which would, if the parent was outside the scene, cause the scene to be loaded into the new world with a parent reference potentially pointing to some random entity in that new world. - Fixes #4793 and addresses #7235 ## Solution - DynamicScenes now identify entities with a `Entity` instead of a u32, therefore including generation - `World` exposes a new `reserve_generations` function that despawns an entity and advances its generation by some extra amount. - `MapEntities` implementations have a new `get_or_reserve` function available that will always return an `Entity`, establishing a new mapping to a dead entity when the entity they are called with is not in the `EntityMap`. Subsequent calls with that same `Entity` will return the same newly created dead entity reference, preserving equality semantics. - As a result, after loading a scene containing references to dead entities (or entities otherwise outside the scene), those references will all point to different generations on a single entity id in the new world. --- ## Changelog ### Changed - In serialized scenes, entities are now identified by a u64 instead of a u32. - In serialized scenes, components with entity references now have those references serialize as u64s instead of structs. ### Fixed - Scenes containing components with entity references will now deserialize and add to a world reliably. ## Migration Guide - `MapEntities` implementations must change from a `&EntityMap` parameter to a `&mut EntityMapper` parameter and can no longer return a `Result`. Finally, they should switch from calling `EntityMap::get` to calling `EntityMapper::get_or_reserve`. --------- Co-authored-by: Nicola Papale <nicopap@users.noreply.github.com>
2023-05-01 15:49:27 +00:00
self.extracted_scene.insert(entity, entry);
}
self
}
(De) serialize resources in scenes (#6846) # Objective Co-Authored-By: davier [bricedavier@gmail.com](mailto:bricedavier@gmail.com) Fixes #3576. Adds a `resources` field in scene serialization data to allow de/serializing resources that have reflection enabled. ## Solution Most of this code is taken from a previous closed PR: https://github.com/bevyengine/bevy/pull/3580. Most of the credit goes to @Davier , what I did was mostly getting it to work on the latest main branch of Bevy, along with adding a few asserts in the currently existing tests to be sure everything is working properly. This PR changes the scene format to include resources in this way: ``` ( resources: { // List of resources here, keyed by resource type name. }, entities: [ // Previous scene format here ], ) ``` An example taken from the tests: ``` ( resources: { "bevy_scene::serde::tests::MyResource": ( foo: 123, ), }, entities: { // Previous scene format here }, ) ``` For this, a `resources` fields has been added on the `DynamicScene` and the `DynamicSceneBuilder` structs. The latter now also has a method named `extract_resources` to properly extract the existing resources registered in the local type registry, in a similar way to `extract_entities`. --- ## Changelog Added: Reflect resources registered in the type registry used by dynamic scenes will now be properly de/serialized in scene data. ## Migration Guide Since the scene format has been changed, the user may not be able to use scenes saved prior to this PR due to the `resources` scene field being missing. ~~To preserve backwards compatibility, I will try to make the `resources` fully optional so that old scenes can be loaded without issue.~~ ## TODOs - [x] I may have to update a few doc blocks still referring to dynamic scenes as mere container of entities, since they now include resources as well. - [x] ~~I want to make the `resources` key optional, as specified in the Migration Guide, so that old scenes will be compatible with this change.~~ Since this would only be trivial for ron format, I think it might be better to consider it in a separate PR/discussion to figure out if it could be done for binary serialization too. - [x] I suppose it might be a good idea to add a resources in the scene example so that users will quickly notice they can serialize resources just like entities. --------- Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2023-03-20 21:17:02 +00:00
/// Extract resources from the builder's [`World`].
///
/// Re-extracting a resource that was already extracted will have no effect.
bevy_scene: Add `SceneFilter` (#6793) # Objective Currently, `DynamicScene`s extract all components listed in the given (or the world's) type registry. This acts as a quasi-filter of sorts. However, it can be troublesome to use effectively and lacks decent control. For example, say you need to serialize only the following component over the network: ```rust #[derive(Reflect, Component, Default)] #[reflect(Component)] struct NPC { name: Option<String> } ``` To do this, you'd need to: 1. Create a new `AppTypeRegistry` 2. Register `NPC` 3. Register `Option<String>` If we skip Step 3, then the entire scene might fail to serialize as `Option<String>` requires registration. Not only is this annoying and easy to forget, but it can leave users with an impossible task: serializing a third-party type that contains private types. Generally, the third-party crate will register their private types within a plugin so the user doesn't need to do it themselves. However, this means we are now unable to serialize _just_ that type— we're forced to allow everything! ## Solution Add the `SceneFilter` enum for filtering components to extract. This filter can be used to optionally allow or deny entire sets of components/resources. With the `DynamicSceneBuilder`, users have more control over how their `DynamicScene`s are built. To only serialize a subset of components, use the `allow` method: ```rust let scene = builder .allow::<ComponentA>() .allow::<ComponentB>() .extract_entity(entity) .build(); ``` To serialize everything _but_ a subset of components, use the `deny` method: ```rust let scene = builder .deny::<ComponentA>() .deny::<ComponentB>() .extract_entity(entity) .build(); ``` Or create a custom filter: ```rust let components = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(components); // let filter = SceneFilter::Denylist(components); let scene = builder .with_filter(Some(filter)) .extract_entity(entity) .build(); ``` Similar operations exist for resources: <details> <summary>View Resource Methods</summary> To only serialize a subset of resources, use the `allow_resource` method: ```rust let scene = builder .allow_resource::<ResourceA>() .extract_resources() .build(); ``` To serialize everything _but_ a subset of resources, use the `deny_resource` method: ```rust let scene = builder .deny_resource::<ResourceA>() .extract_resources() .build(); ``` Or create a custom filter: ```rust let resources = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(resources); // let filter = SceneFilter::Denylist(resources); let scene = builder .with_resource_filter(Some(filter)) .extract_resources() .build(); ``` </details> ### Open Questions - [x] ~~`allow` and `deny` are mutually exclusive. Currently, they overwrite each other. Should this instead be a panic?~~ Took @soqb's suggestion and made it so that the opposing method simply removes that type from the list. - [x] ~~`DynamicSceneBuilder` extracts entity data as soon as `extract_entity`/`extract_entities` is called. Should this behavior instead be moved to the `build` method to prevent ordering mixups (e.g. `.allow::<Foo>().extract_entity(entity)` vs `.extract_entity(entity).allow::<Foo>()`)? The tradeoff would be iterating over the given entities twice: once at extraction and again at build.~~ Based on the feedback from @Testare it sounds like it might be better to just keep the current functionality (if anything we can open a separate PR that adds deferred methods for extraction, so the choice/performance hit is up to the user). - [ ] An alternative might be to remove the filter from `DynamicSceneBuilder` and have it as a separate parameter to the extraction methods (either in the existing ones or as added `extract_entity_with_filter`-type methods). Is this preferable? - [x] ~~Should we include constructors that include common types to allow/deny? For example, a `SceneFilter::standard_allowlist` that includes things like `Parent` and `Children`?~~ Consensus suggests we should. I may split this out into a followup PR, though. - [x] ~~Should we add the ability to remove types from the filter regardless of whether an allowlist or denylist (e.g. `filter.remove::<Foo>()`)?~~ See the first list item - [x] ~~Should `SceneFilter` be an enum? Would it make more sense as a struct that contains an `is_denylist` boolean?~~ With the added `SceneFilter::None` state (replacing the need to wrap in an `Option` or rely on an empty `Denylist`), it seems an enum is better suited now - [x] ~~Bikeshed: Do we like the naming convention? Should we instead use `include`/`exclude` terminology?~~ Sounds like we're sticking with `allow`/`deny`! - [x] ~~Does this feature need a new example? Do we simply include it in the existing one (maybe even as a comment?)? Should this be done in a followup PR instead?~~ Example will be added in a followup PR ### Followup Tasks - [ ] Add a dedicated `SceneFilter` example - [ ] Possibly add default types to the filter (e.g. deny things like `ComputedVisibility`, allow `Parent`, etc) --- ## Changelog - Added the `SceneFilter` enum for filtering components and resources when building a `DynamicScene` - Added methods: - `DynamicSceneBuilder::with_filter` - `DynamicSceneBuilder::allow` - `DynamicSceneBuilder::deny` - `DynamicSceneBuilder::allow_all` - `DynamicSceneBuilder::deny_all` - `DynamicSceneBuilder::with_resource_filter` - `DynamicSceneBuilder::allow_resource` - `DynamicSceneBuilder::deny_resource` - `DynamicSceneBuilder::allow_all_resources` - `DynamicSceneBuilder::deny_all_resources` - Removed methods: - `DynamicSceneBuilder::from_world_with_type_registry` - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference ## Migration Guide - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let dynamic_scene = DynamicScene::from_world(&world, registry); // let dynamic_scene = DynamicScene::from_scene(&scene, registry); // NEW let dynamic_scene = DynamicScene::from_world(&world); // let dynamic_scene = DynamicScene::from_scene(&scene); ``` - Removed `DynamicSceneBuilder::from_world_with_type_registry`. Now the registry is automatically taken from the given world: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let builder = DynamicSceneBuilder::from_world_with_type_registry(&world, registry); // NEW let builder = DynamicSceneBuilder::from_world(&world); ```
2023-07-06 21:04:26 +00:00
///
/// To control which resources are extracted, use the [`allow_resource`] or
/// [`deny_resource`] helper methods.
///
(De) serialize resources in scenes (#6846) # Objective Co-Authored-By: davier [bricedavier@gmail.com](mailto:bricedavier@gmail.com) Fixes #3576. Adds a `resources` field in scene serialization data to allow de/serializing resources that have reflection enabled. ## Solution Most of this code is taken from a previous closed PR: https://github.com/bevyengine/bevy/pull/3580. Most of the credit goes to @Davier , what I did was mostly getting it to work on the latest main branch of Bevy, along with adding a few asserts in the currently existing tests to be sure everything is working properly. This PR changes the scene format to include resources in this way: ``` ( resources: { // List of resources here, keyed by resource type name. }, entities: [ // Previous scene format here ], ) ``` An example taken from the tests: ``` ( resources: { "bevy_scene::serde::tests::MyResource": ( foo: 123, ), }, entities: { // Previous scene format here }, ) ``` For this, a `resources` fields has been added on the `DynamicScene` and the `DynamicSceneBuilder` structs. The latter now also has a method named `extract_resources` to properly extract the existing resources registered in the local type registry, in a similar way to `extract_entities`. --- ## Changelog Added: Reflect resources registered in the type registry used by dynamic scenes will now be properly de/serialized in scene data. ## Migration Guide Since the scene format has been changed, the user may not be able to use scenes saved prior to this PR due to the `resources` scene field being missing. ~~To preserve backwards compatibility, I will try to make the `resources` fully optional so that old scenes can be loaded without issue.~~ ## TODOs - [x] I may have to update a few doc blocks still referring to dynamic scenes as mere container of entities, since they now include resources as well. - [x] ~~I want to make the `resources` key optional, as specified in the Migration Guide, so that old scenes will be compatible with this change.~~ Since this would only be trivial for ron format, I think it might be better to consider it in a separate PR/discussion to figure out if it could be done for binary serialization too. - [x] I suppose it might be a good idea to add a resources in the scene example so that users will quickly notice they can serialize resources just like entities. --------- Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2023-03-20 21:17:02 +00:00
/// ```
/// # use bevy_scene::DynamicSceneBuilder;
/// # use bevy_ecs::reflect::AppTypeRegistry;
(De) serialize resources in scenes (#6846) # Objective Co-Authored-By: davier [bricedavier@gmail.com](mailto:bricedavier@gmail.com) Fixes #3576. Adds a `resources` field in scene serialization data to allow de/serializing resources that have reflection enabled. ## Solution Most of this code is taken from a previous closed PR: https://github.com/bevyengine/bevy/pull/3580. Most of the credit goes to @Davier , what I did was mostly getting it to work on the latest main branch of Bevy, along with adding a few asserts in the currently existing tests to be sure everything is working properly. This PR changes the scene format to include resources in this way: ``` ( resources: { // List of resources here, keyed by resource type name. }, entities: [ // Previous scene format here ], ) ``` An example taken from the tests: ``` ( resources: { "bevy_scene::serde::tests::MyResource": ( foo: 123, ), }, entities: { // Previous scene format here }, ) ``` For this, a `resources` fields has been added on the `DynamicScene` and the `DynamicSceneBuilder` structs. The latter now also has a method named `extract_resources` to properly extract the existing resources registered in the local type registry, in a similar way to `extract_entities`. --- ## Changelog Added: Reflect resources registered in the type registry used by dynamic scenes will now be properly de/serialized in scene data. ## Migration Guide Since the scene format has been changed, the user may not be able to use scenes saved prior to this PR due to the `resources` scene field being missing. ~~To preserve backwards compatibility, I will try to make the `resources` fully optional so that old scenes can be loaded without issue.~~ ## TODOs - [x] I may have to update a few doc blocks still referring to dynamic scenes as mere container of entities, since they now include resources as well. - [x] ~~I want to make the `resources` key optional, as specified in the Migration Guide, so that old scenes will be compatible with this change.~~ Since this would only be trivial for ron format, I think it might be better to consider it in a separate PR/discussion to figure out if it could be done for binary serialization too. - [x] I suppose it might be a good idea to add a resources in the scene example so that users will quickly notice they can serialize resources just like entities. --------- Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2023-03-20 21:17:02 +00:00
/// # use bevy_ecs::prelude::{ReflectResource, Resource, World};
/// # use bevy_reflect::Reflect;
/// #[derive(Resource, Default, Reflect)]
/// #[reflect(Resource)]
/// struct MyResource;
///
/// # let mut world = World::default();
/// # world.init_resource::<AppTypeRegistry>();
/// world.insert_resource(MyResource);
///
/// let mut builder = DynamicSceneBuilder::from_world(&world).extract_resources();
(De) serialize resources in scenes (#6846) # Objective Co-Authored-By: davier [bricedavier@gmail.com](mailto:bricedavier@gmail.com) Fixes #3576. Adds a `resources` field in scene serialization data to allow de/serializing resources that have reflection enabled. ## Solution Most of this code is taken from a previous closed PR: https://github.com/bevyengine/bevy/pull/3580. Most of the credit goes to @Davier , what I did was mostly getting it to work on the latest main branch of Bevy, along with adding a few asserts in the currently existing tests to be sure everything is working properly. This PR changes the scene format to include resources in this way: ``` ( resources: { // List of resources here, keyed by resource type name. }, entities: [ // Previous scene format here ], ) ``` An example taken from the tests: ``` ( resources: { "bevy_scene::serde::tests::MyResource": ( foo: 123, ), }, entities: { // Previous scene format here }, ) ``` For this, a `resources` fields has been added on the `DynamicScene` and the `DynamicSceneBuilder` structs. The latter now also has a method named `extract_resources` to properly extract the existing resources registered in the local type registry, in a similar way to `extract_entities`. --- ## Changelog Added: Reflect resources registered in the type registry used by dynamic scenes will now be properly de/serialized in scene data. ## Migration Guide Since the scene format has been changed, the user may not be able to use scenes saved prior to this PR due to the `resources` scene field being missing. ~~To preserve backwards compatibility, I will try to make the `resources` fully optional so that old scenes can be loaded without issue.~~ ## TODOs - [x] I may have to update a few doc blocks still referring to dynamic scenes as mere container of entities, since they now include resources as well. - [x] ~~I want to make the `resources` key optional, as specified in the Migration Guide, so that old scenes will be compatible with this change.~~ Since this would only be trivial for ron format, I think it might be better to consider it in a separate PR/discussion to figure out if it could be done for binary serialization too. - [x] I suppose it might be a good idea to add a resources in the scene example so that users will quickly notice they can serialize resources just like entities. --------- Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2023-03-20 21:17:02 +00:00
/// let scene = builder.build();
/// ```
bevy_scene: Add `SceneFilter` (#6793) # Objective Currently, `DynamicScene`s extract all components listed in the given (or the world's) type registry. This acts as a quasi-filter of sorts. However, it can be troublesome to use effectively and lacks decent control. For example, say you need to serialize only the following component over the network: ```rust #[derive(Reflect, Component, Default)] #[reflect(Component)] struct NPC { name: Option<String> } ``` To do this, you'd need to: 1. Create a new `AppTypeRegistry` 2. Register `NPC` 3. Register `Option<String>` If we skip Step 3, then the entire scene might fail to serialize as `Option<String>` requires registration. Not only is this annoying and easy to forget, but it can leave users with an impossible task: serializing a third-party type that contains private types. Generally, the third-party crate will register their private types within a plugin so the user doesn't need to do it themselves. However, this means we are now unable to serialize _just_ that type— we're forced to allow everything! ## Solution Add the `SceneFilter` enum for filtering components to extract. This filter can be used to optionally allow or deny entire sets of components/resources. With the `DynamicSceneBuilder`, users have more control over how their `DynamicScene`s are built. To only serialize a subset of components, use the `allow` method: ```rust let scene = builder .allow::<ComponentA>() .allow::<ComponentB>() .extract_entity(entity) .build(); ``` To serialize everything _but_ a subset of components, use the `deny` method: ```rust let scene = builder .deny::<ComponentA>() .deny::<ComponentB>() .extract_entity(entity) .build(); ``` Or create a custom filter: ```rust let components = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(components); // let filter = SceneFilter::Denylist(components); let scene = builder .with_filter(Some(filter)) .extract_entity(entity) .build(); ``` Similar operations exist for resources: <details> <summary>View Resource Methods</summary> To only serialize a subset of resources, use the `allow_resource` method: ```rust let scene = builder .allow_resource::<ResourceA>() .extract_resources() .build(); ``` To serialize everything _but_ a subset of resources, use the `deny_resource` method: ```rust let scene = builder .deny_resource::<ResourceA>() .extract_resources() .build(); ``` Or create a custom filter: ```rust let resources = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(resources); // let filter = SceneFilter::Denylist(resources); let scene = builder .with_resource_filter(Some(filter)) .extract_resources() .build(); ``` </details> ### Open Questions - [x] ~~`allow` and `deny` are mutually exclusive. Currently, they overwrite each other. Should this instead be a panic?~~ Took @soqb's suggestion and made it so that the opposing method simply removes that type from the list. - [x] ~~`DynamicSceneBuilder` extracts entity data as soon as `extract_entity`/`extract_entities` is called. Should this behavior instead be moved to the `build` method to prevent ordering mixups (e.g. `.allow::<Foo>().extract_entity(entity)` vs `.extract_entity(entity).allow::<Foo>()`)? The tradeoff would be iterating over the given entities twice: once at extraction and again at build.~~ Based on the feedback from @Testare it sounds like it might be better to just keep the current functionality (if anything we can open a separate PR that adds deferred methods for extraction, so the choice/performance hit is up to the user). - [ ] An alternative might be to remove the filter from `DynamicSceneBuilder` and have it as a separate parameter to the extraction methods (either in the existing ones or as added `extract_entity_with_filter`-type methods). Is this preferable? - [x] ~~Should we include constructors that include common types to allow/deny? For example, a `SceneFilter::standard_allowlist` that includes things like `Parent` and `Children`?~~ Consensus suggests we should. I may split this out into a followup PR, though. - [x] ~~Should we add the ability to remove types from the filter regardless of whether an allowlist or denylist (e.g. `filter.remove::<Foo>()`)?~~ See the first list item - [x] ~~Should `SceneFilter` be an enum? Would it make more sense as a struct that contains an `is_denylist` boolean?~~ With the added `SceneFilter::None` state (replacing the need to wrap in an `Option` or rely on an empty `Denylist`), it seems an enum is better suited now - [x] ~~Bikeshed: Do we like the naming convention? Should we instead use `include`/`exclude` terminology?~~ Sounds like we're sticking with `allow`/`deny`! - [x] ~~Does this feature need a new example? Do we simply include it in the existing one (maybe even as a comment?)? Should this be done in a followup PR instead?~~ Example will be added in a followup PR ### Followup Tasks - [ ] Add a dedicated `SceneFilter` example - [ ] Possibly add default types to the filter (e.g. deny things like `ComputedVisibility`, allow `Parent`, etc) --- ## Changelog - Added the `SceneFilter` enum for filtering components and resources when building a `DynamicScene` - Added methods: - `DynamicSceneBuilder::with_filter` - `DynamicSceneBuilder::allow` - `DynamicSceneBuilder::deny` - `DynamicSceneBuilder::allow_all` - `DynamicSceneBuilder::deny_all` - `DynamicSceneBuilder::with_resource_filter` - `DynamicSceneBuilder::allow_resource` - `DynamicSceneBuilder::deny_resource` - `DynamicSceneBuilder::allow_all_resources` - `DynamicSceneBuilder::deny_all_resources` - Removed methods: - `DynamicSceneBuilder::from_world_with_type_registry` - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference ## Migration Guide - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let dynamic_scene = DynamicScene::from_world(&world, registry); // let dynamic_scene = DynamicScene::from_scene(&scene, registry); // NEW let dynamic_scene = DynamicScene::from_world(&world); // let dynamic_scene = DynamicScene::from_scene(&scene); ``` - Removed `DynamicSceneBuilder::from_world_with_type_registry`. Now the registry is automatically taken from the given world: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let builder = DynamicSceneBuilder::from_world_with_type_registry(&world, registry); // NEW let builder = DynamicSceneBuilder::from_world(&world); ```
2023-07-06 21:04:26 +00:00
///
/// [`allow_resource`]: Self::allow_resource
/// [`deny_resource`]: Self::deny_resource
#[must_use]
pub fn extract_resources(mut self) -> Self {
bevy_scene: Add `SceneFilter` (#6793) # Objective Currently, `DynamicScene`s extract all components listed in the given (or the world's) type registry. This acts as a quasi-filter of sorts. However, it can be troublesome to use effectively and lacks decent control. For example, say you need to serialize only the following component over the network: ```rust #[derive(Reflect, Component, Default)] #[reflect(Component)] struct NPC { name: Option<String> } ``` To do this, you'd need to: 1. Create a new `AppTypeRegistry` 2. Register `NPC` 3. Register `Option<String>` If we skip Step 3, then the entire scene might fail to serialize as `Option<String>` requires registration. Not only is this annoying and easy to forget, but it can leave users with an impossible task: serializing a third-party type that contains private types. Generally, the third-party crate will register their private types within a plugin so the user doesn't need to do it themselves. However, this means we are now unable to serialize _just_ that type— we're forced to allow everything! ## Solution Add the `SceneFilter` enum for filtering components to extract. This filter can be used to optionally allow or deny entire sets of components/resources. With the `DynamicSceneBuilder`, users have more control over how their `DynamicScene`s are built. To only serialize a subset of components, use the `allow` method: ```rust let scene = builder .allow::<ComponentA>() .allow::<ComponentB>() .extract_entity(entity) .build(); ``` To serialize everything _but_ a subset of components, use the `deny` method: ```rust let scene = builder .deny::<ComponentA>() .deny::<ComponentB>() .extract_entity(entity) .build(); ``` Or create a custom filter: ```rust let components = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(components); // let filter = SceneFilter::Denylist(components); let scene = builder .with_filter(Some(filter)) .extract_entity(entity) .build(); ``` Similar operations exist for resources: <details> <summary>View Resource Methods</summary> To only serialize a subset of resources, use the `allow_resource` method: ```rust let scene = builder .allow_resource::<ResourceA>() .extract_resources() .build(); ``` To serialize everything _but_ a subset of resources, use the `deny_resource` method: ```rust let scene = builder .deny_resource::<ResourceA>() .extract_resources() .build(); ``` Or create a custom filter: ```rust let resources = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(resources); // let filter = SceneFilter::Denylist(resources); let scene = builder .with_resource_filter(Some(filter)) .extract_resources() .build(); ``` </details> ### Open Questions - [x] ~~`allow` and `deny` are mutually exclusive. Currently, they overwrite each other. Should this instead be a panic?~~ Took @soqb's suggestion and made it so that the opposing method simply removes that type from the list. - [x] ~~`DynamicSceneBuilder` extracts entity data as soon as `extract_entity`/`extract_entities` is called. Should this behavior instead be moved to the `build` method to prevent ordering mixups (e.g. `.allow::<Foo>().extract_entity(entity)` vs `.extract_entity(entity).allow::<Foo>()`)? The tradeoff would be iterating over the given entities twice: once at extraction and again at build.~~ Based on the feedback from @Testare it sounds like it might be better to just keep the current functionality (if anything we can open a separate PR that adds deferred methods for extraction, so the choice/performance hit is up to the user). - [ ] An alternative might be to remove the filter from `DynamicSceneBuilder` and have it as a separate parameter to the extraction methods (either in the existing ones or as added `extract_entity_with_filter`-type methods). Is this preferable? - [x] ~~Should we include constructors that include common types to allow/deny? For example, a `SceneFilter::standard_allowlist` that includes things like `Parent` and `Children`?~~ Consensus suggests we should. I may split this out into a followup PR, though. - [x] ~~Should we add the ability to remove types from the filter regardless of whether an allowlist or denylist (e.g. `filter.remove::<Foo>()`)?~~ See the first list item - [x] ~~Should `SceneFilter` be an enum? Would it make more sense as a struct that contains an `is_denylist` boolean?~~ With the added `SceneFilter::None` state (replacing the need to wrap in an `Option` or rely on an empty `Denylist`), it seems an enum is better suited now - [x] ~~Bikeshed: Do we like the naming convention? Should we instead use `include`/`exclude` terminology?~~ Sounds like we're sticking with `allow`/`deny`! - [x] ~~Does this feature need a new example? Do we simply include it in the existing one (maybe even as a comment?)? Should this be done in a followup PR instead?~~ Example will be added in a followup PR ### Followup Tasks - [ ] Add a dedicated `SceneFilter` example - [ ] Possibly add default types to the filter (e.g. deny things like `ComputedVisibility`, allow `Parent`, etc) --- ## Changelog - Added the `SceneFilter` enum for filtering components and resources when building a `DynamicScene` - Added methods: - `DynamicSceneBuilder::with_filter` - `DynamicSceneBuilder::allow` - `DynamicSceneBuilder::deny` - `DynamicSceneBuilder::allow_all` - `DynamicSceneBuilder::deny_all` - `DynamicSceneBuilder::with_resource_filter` - `DynamicSceneBuilder::allow_resource` - `DynamicSceneBuilder::deny_resource` - `DynamicSceneBuilder::allow_all_resources` - `DynamicSceneBuilder::deny_all_resources` - Removed methods: - `DynamicSceneBuilder::from_world_with_type_registry` - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference ## Migration Guide - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let dynamic_scene = DynamicScene::from_world(&world, registry); // let dynamic_scene = DynamicScene::from_scene(&scene, registry); // NEW let dynamic_scene = DynamicScene::from_world(&world); // let dynamic_scene = DynamicScene::from_scene(&scene); ``` - Removed `DynamicSceneBuilder::from_world_with_type_registry`. Now the registry is automatically taken from the given world: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let builder = DynamicSceneBuilder::from_world_with_type_registry(&world, registry); // NEW let builder = DynamicSceneBuilder::from_world(&world); ```
2023-07-06 21:04:26 +00:00
let type_registry = self.original_world.resource::<AppTypeRegistry>().read();
(De) serialize resources in scenes (#6846) # Objective Co-Authored-By: davier [bricedavier@gmail.com](mailto:bricedavier@gmail.com) Fixes #3576. Adds a `resources` field in scene serialization data to allow de/serializing resources that have reflection enabled. ## Solution Most of this code is taken from a previous closed PR: https://github.com/bevyengine/bevy/pull/3580. Most of the credit goes to @Davier , what I did was mostly getting it to work on the latest main branch of Bevy, along with adding a few asserts in the currently existing tests to be sure everything is working properly. This PR changes the scene format to include resources in this way: ``` ( resources: { // List of resources here, keyed by resource type name. }, entities: [ // Previous scene format here ], ) ``` An example taken from the tests: ``` ( resources: { "bevy_scene::serde::tests::MyResource": ( foo: 123, ), }, entities: { // Previous scene format here }, ) ``` For this, a `resources` fields has been added on the `DynamicScene` and the `DynamicSceneBuilder` structs. The latter now also has a method named `extract_resources` to properly extract the existing resources registered in the local type registry, in a similar way to `extract_entities`. --- ## Changelog Added: Reflect resources registered in the type registry used by dynamic scenes will now be properly de/serialized in scene data. ## Migration Guide Since the scene format has been changed, the user may not be able to use scenes saved prior to this PR due to the `resources` scene field being missing. ~~To preserve backwards compatibility, I will try to make the `resources` fully optional so that old scenes can be loaded without issue.~~ ## TODOs - [x] I may have to update a few doc blocks still referring to dynamic scenes as mere container of entities, since they now include resources as well. - [x] ~~I want to make the `resources` key optional, as specified in the Migration Guide, so that old scenes will be compatible with this change.~~ Since this would only be trivial for ron format, I think it might be better to consider it in a separate PR/discussion to figure out if it could be done for binary serialization too. - [x] I suppose it might be a good idea to add a resources in the scene example so that users will quickly notice they can serialize resources just like entities. --------- Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2023-03-20 21:17:02 +00:00
for (component_id, _) in self.original_world.storages().resources.iter() {
let mut extract_and_push = || {
let type_id = self
.original_world
.components()
.get_info(component_id)?
.type_id()?;
bevy_scene: Add `SceneFilter` (#6793) # Objective Currently, `DynamicScene`s extract all components listed in the given (or the world's) type registry. This acts as a quasi-filter of sorts. However, it can be troublesome to use effectively and lacks decent control. For example, say you need to serialize only the following component over the network: ```rust #[derive(Reflect, Component, Default)] #[reflect(Component)] struct NPC { name: Option<String> } ``` To do this, you'd need to: 1. Create a new `AppTypeRegistry` 2. Register `NPC` 3. Register `Option<String>` If we skip Step 3, then the entire scene might fail to serialize as `Option<String>` requires registration. Not only is this annoying and easy to forget, but it can leave users with an impossible task: serializing a third-party type that contains private types. Generally, the third-party crate will register their private types within a plugin so the user doesn't need to do it themselves. However, this means we are now unable to serialize _just_ that type— we're forced to allow everything! ## Solution Add the `SceneFilter` enum for filtering components to extract. This filter can be used to optionally allow or deny entire sets of components/resources. With the `DynamicSceneBuilder`, users have more control over how their `DynamicScene`s are built. To only serialize a subset of components, use the `allow` method: ```rust let scene = builder .allow::<ComponentA>() .allow::<ComponentB>() .extract_entity(entity) .build(); ``` To serialize everything _but_ a subset of components, use the `deny` method: ```rust let scene = builder .deny::<ComponentA>() .deny::<ComponentB>() .extract_entity(entity) .build(); ``` Or create a custom filter: ```rust let components = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(components); // let filter = SceneFilter::Denylist(components); let scene = builder .with_filter(Some(filter)) .extract_entity(entity) .build(); ``` Similar operations exist for resources: <details> <summary>View Resource Methods</summary> To only serialize a subset of resources, use the `allow_resource` method: ```rust let scene = builder .allow_resource::<ResourceA>() .extract_resources() .build(); ``` To serialize everything _but_ a subset of resources, use the `deny_resource` method: ```rust let scene = builder .deny_resource::<ResourceA>() .extract_resources() .build(); ``` Or create a custom filter: ```rust let resources = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(resources); // let filter = SceneFilter::Denylist(resources); let scene = builder .with_resource_filter(Some(filter)) .extract_resources() .build(); ``` </details> ### Open Questions - [x] ~~`allow` and `deny` are mutually exclusive. Currently, they overwrite each other. Should this instead be a panic?~~ Took @soqb's suggestion and made it so that the opposing method simply removes that type from the list. - [x] ~~`DynamicSceneBuilder` extracts entity data as soon as `extract_entity`/`extract_entities` is called. Should this behavior instead be moved to the `build` method to prevent ordering mixups (e.g. `.allow::<Foo>().extract_entity(entity)` vs `.extract_entity(entity).allow::<Foo>()`)? The tradeoff would be iterating over the given entities twice: once at extraction and again at build.~~ Based on the feedback from @Testare it sounds like it might be better to just keep the current functionality (if anything we can open a separate PR that adds deferred methods for extraction, so the choice/performance hit is up to the user). - [ ] An alternative might be to remove the filter from `DynamicSceneBuilder` and have it as a separate parameter to the extraction methods (either in the existing ones or as added `extract_entity_with_filter`-type methods). Is this preferable? - [x] ~~Should we include constructors that include common types to allow/deny? For example, a `SceneFilter::standard_allowlist` that includes things like `Parent` and `Children`?~~ Consensus suggests we should. I may split this out into a followup PR, though. - [x] ~~Should we add the ability to remove types from the filter regardless of whether an allowlist or denylist (e.g. `filter.remove::<Foo>()`)?~~ See the first list item - [x] ~~Should `SceneFilter` be an enum? Would it make more sense as a struct that contains an `is_denylist` boolean?~~ With the added `SceneFilter::None` state (replacing the need to wrap in an `Option` or rely on an empty `Denylist`), it seems an enum is better suited now - [x] ~~Bikeshed: Do we like the naming convention? Should we instead use `include`/`exclude` terminology?~~ Sounds like we're sticking with `allow`/`deny`! - [x] ~~Does this feature need a new example? Do we simply include it in the existing one (maybe even as a comment?)? Should this be done in a followup PR instead?~~ Example will be added in a followup PR ### Followup Tasks - [ ] Add a dedicated `SceneFilter` example - [ ] Possibly add default types to the filter (e.g. deny things like `ComputedVisibility`, allow `Parent`, etc) --- ## Changelog - Added the `SceneFilter` enum for filtering components and resources when building a `DynamicScene` - Added methods: - `DynamicSceneBuilder::with_filter` - `DynamicSceneBuilder::allow` - `DynamicSceneBuilder::deny` - `DynamicSceneBuilder::allow_all` - `DynamicSceneBuilder::deny_all` - `DynamicSceneBuilder::with_resource_filter` - `DynamicSceneBuilder::allow_resource` - `DynamicSceneBuilder::deny_resource` - `DynamicSceneBuilder::allow_all_resources` - `DynamicSceneBuilder::deny_all_resources` - Removed methods: - `DynamicSceneBuilder::from_world_with_type_registry` - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference ## Migration Guide - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let dynamic_scene = DynamicScene::from_world(&world, registry); // let dynamic_scene = DynamicScene::from_scene(&scene, registry); // NEW let dynamic_scene = DynamicScene::from_world(&world); // let dynamic_scene = DynamicScene::from_scene(&scene); ``` - Removed `DynamicSceneBuilder::from_world_with_type_registry`. Now the registry is automatically taken from the given world: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let builder = DynamicSceneBuilder::from_world_with_type_registry(&world, registry); // NEW let builder = DynamicSceneBuilder::from_world(&world); ```
2023-07-06 21:04:26 +00:00
let is_denied = self.resource_filter.is_denied_by_id(type_id);
if is_denied {
// Resource is either in the denylist or _not_ in the allowlist
return None;
}
let type_registration = type_registry.get(type_id)?;
let resource = type_registration
(De) serialize resources in scenes (#6846) # Objective Co-Authored-By: davier [bricedavier@gmail.com](mailto:bricedavier@gmail.com) Fixes #3576. Adds a `resources` field in scene serialization data to allow de/serializing resources that have reflection enabled. ## Solution Most of this code is taken from a previous closed PR: https://github.com/bevyengine/bevy/pull/3580. Most of the credit goes to @Davier , what I did was mostly getting it to work on the latest main branch of Bevy, along with adding a few asserts in the currently existing tests to be sure everything is working properly. This PR changes the scene format to include resources in this way: ``` ( resources: { // List of resources here, keyed by resource type name. }, entities: [ // Previous scene format here ], ) ``` An example taken from the tests: ``` ( resources: { "bevy_scene::serde::tests::MyResource": ( foo: 123, ), }, entities: { // Previous scene format here }, ) ``` For this, a `resources` fields has been added on the `DynamicScene` and the `DynamicSceneBuilder` structs. The latter now also has a method named `extract_resources` to properly extract the existing resources registered in the local type registry, in a similar way to `extract_entities`. --- ## Changelog Added: Reflect resources registered in the type registry used by dynamic scenes will now be properly de/serialized in scene data. ## Migration Guide Since the scene format has been changed, the user may not be able to use scenes saved prior to this PR due to the `resources` scene field being missing. ~~To preserve backwards compatibility, I will try to make the `resources` fully optional so that old scenes can be loaded without issue.~~ ## TODOs - [x] I may have to update a few doc blocks still referring to dynamic scenes as mere container of entities, since they now include resources as well. - [x] ~~I want to make the `resources` key optional, as specified in the Migration Guide, so that old scenes will be compatible with this change.~~ Since this would only be trivial for ron format, I think it might be better to consider it in a separate PR/discussion to figure out if it could be done for binary serialization too. - [x] I suppose it might be a good idea to add a resources in the scene example so that users will quickly notice they can serialize resources just like entities. --------- Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2023-03-20 21:17:02 +00:00
.data::<ReflectResource>()?
.reflect(self.original_world)?;
let resource = type_registration
.data::<ReflectFromReflect>()
.and_then(|fr| fr.from_reflect(resource.as_partial_reflect()))
.map(PartialReflect::into_partial_reflect)
.unwrap_or_else(|| resource.clone_value());
self.extracted_resources.insert(component_id, resource);
(De) serialize resources in scenes (#6846) # Objective Co-Authored-By: davier [bricedavier@gmail.com](mailto:bricedavier@gmail.com) Fixes #3576. Adds a `resources` field in scene serialization data to allow de/serializing resources that have reflection enabled. ## Solution Most of this code is taken from a previous closed PR: https://github.com/bevyengine/bevy/pull/3580. Most of the credit goes to @Davier , what I did was mostly getting it to work on the latest main branch of Bevy, along with adding a few asserts in the currently existing tests to be sure everything is working properly. This PR changes the scene format to include resources in this way: ``` ( resources: { // List of resources here, keyed by resource type name. }, entities: [ // Previous scene format here ], ) ``` An example taken from the tests: ``` ( resources: { "bevy_scene::serde::tests::MyResource": ( foo: 123, ), }, entities: { // Previous scene format here }, ) ``` For this, a `resources` fields has been added on the `DynamicScene` and the `DynamicSceneBuilder` structs. The latter now also has a method named `extract_resources` to properly extract the existing resources registered in the local type registry, in a similar way to `extract_entities`. --- ## Changelog Added: Reflect resources registered in the type registry used by dynamic scenes will now be properly de/serialized in scene data. ## Migration Guide Since the scene format has been changed, the user may not be able to use scenes saved prior to this PR due to the `resources` scene field being missing. ~~To preserve backwards compatibility, I will try to make the `resources` fully optional so that old scenes can be loaded without issue.~~ ## TODOs - [x] I may have to update a few doc blocks still referring to dynamic scenes as mere container of entities, since they now include resources as well. - [x] ~~I want to make the `resources` key optional, as specified in the Migration Guide, so that old scenes will be compatible with this change.~~ Since this would only be trivial for ron format, I think it might be better to consider it in a separate PR/discussion to figure out if it could be done for binary serialization too. - [x] I suppose it might be a good idea to add a resources in the scene example so that users will quickly notice they can serialize resources just like entities. --------- Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2023-03-20 21:17:02 +00:00
Some(())
};
extract_and_push();
}
drop(type_registry);
self
}
}
#[cfg(test)]
mod tests {
use bevy_ecs::{
component::Component,
prelude::{Entity, Resource},
query::With,
reflect::{AppTypeRegistry, ReflectComponent, ReflectResource},
world::World,
};
use bevy_reflect::Reflect;
use super::DynamicSceneBuilder;
#[derive(Component, Reflect, Default, Eq, PartialEq, Debug)]
#[reflect(Component)]
struct ComponentA;
(De) serialize resources in scenes (#6846) # Objective Co-Authored-By: davier [bricedavier@gmail.com](mailto:bricedavier@gmail.com) Fixes #3576. Adds a `resources` field in scene serialization data to allow de/serializing resources that have reflection enabled. ## Solution Most of this code is taken from a previous closed PR: https://github.com/bevyengine/bevy/pull/3580. Most of the credit goes to @Davier , what I did was mostly getting it to work on the latest main branch of Bevy, along with adding a few asserts in the currently existing tests to be sure everything is working properly. This PR changes the scene format to include resources in this way: ``` ( resources: { // List of resources here, keyed by resource type name. }, entities: [ // Previous scene format here ], ) ``` An example taken from the tests: ``` ( resources: { "bevy_scene::serde::tests::MyResource": ( foo: 123, ), }, entities: { // Previous scene format here }, ) ``` For this, a `resources` fields has been added on the `DynamicScene` and the `DynamicSceneBuilder` structs. The latter now also has a method named `extract_resources` to properly extract the existing resources registered in the local type registry, in a similar way to `extract_entities`. --- ## Changelog Added: Reflect resources registered in the type registry used by dynamic scenes will now be properly de/serialized in scene data. ## Migration Guide Since the scene format has been changed, the user may not be able to use scenes saved prior to this PR due to the `resources` scene field being missing. ~~To preserve backwards compatibility, I will try to make the `resources` fully optional so that old scenes can be loaded without issue.~~ ## TODOs - [x] I may have to update a few doc blocks still referring to dynamic scenes as mere container of entities, since they now include resources as well. - [x] ~~I want to make the `resources` key optional, as specified in the Migration Guide, so that old scenes will be compatible with this change.~~ Since this would only be trivial for ron format, I think it might be better to consider it in a separate PR/discussion to figure out if it could be done for binary serialization too. - [x] I suppose it might be a good idea to add a resources in the scene example so that users will quickly notice they can serialize resources just like entities. --------- Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2023-03-20 21:17:02 +00:00
#[derive(Component, Reflect, Default, Eq, PartialEq, Debug)]
#[reflect(Component)]
struct ComponentB;
(De) serialize resources in scenes (#6846) # Objective Co-Authored-By: davier [bricedavier@gmail.com](mailto:bricedavier@gmail.com) Fixes #3576. Adds a `resources` field in scene serialization data to allow de/serializing resources that have reflection enabled. ## Solution Most of this code is taken from a previous closed PR: https://github.com/bevyengine/bevy/pull/3580. Most of the credit goes to @Davier , what I did was mostly getting it to work on the latest main branch of Bevy, along with adding a few asserts in the currently existing tests to be sure everything is working properly. This PR changes the scene format to include resources in this way: ``` ( resources: { // List of resources here, keyed by resource type name. }, entities: [ // Previous scene format here ], ) ``` An example taken from the tests: ``` ( resources: { "bevy_scene::serde::tests::MyResource": ( foo: 123, ), }, entities: { // Previous scene format here }, ) ``` For this, a `resources` fields has been added on the `DynamicScene` and the `DynamicSceneBuilder` structs. The latter now also has a method named `extract_resources` to properly extract the existing resources registered in the local type registry, in a similar way to `extract_entities`. --- ## Changelog Added: Reflect resources registered in the type registry used by dynamic scenes will now be properly de/serialized in scene data. ## Migration Guide Since the scene format has been changed, the user may not be able to use scenes saved prior to this PR due to the `resources` scene field being missing. ~~To preserve backwards compatibility, I will try to make the `resources` fully optional so that old scenes can be loaded without issue.~~ ## TODOs - [x] I may have to update a few doc blocks still referring to dynamic scenes as mere container of entities, since they now include resources as well. - [x] ~~I want to make the `resources` key optional, as specified in the Migration Guide, so that old scenes will be compatible with this change.~~ Since this would only be trivial for ron format, I think it might be better to consider it in a separate PR/discussion to figure out if it could be done for binary serialization too. - [x] I suppose it might be a good idea to add a resources in the scene example so that users will quickly notice they can serialize resources just like entities. --------- Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2023-03-20 21:17:02 +00:00
#[derive(Resource, Reflect, Default, Eq, PartialEq, Debug)]
#[reflect(Resource)]
struct ResourceA;
bevy_scene: Add `SceneFilter` (#6793) # Objective Currently, `DynamicScene`s extract all components listed in the given (or the world's) type registry. This acts as a quasi-filter of sorts. However, it can be troublesome to use effectively and lacks decent control. For example, say you need to serialize only the following component over the network: ```rust #[derive(Reflect, Component, Default)] #[reflect(Component)] struct NPC { name: Option<String> } ``` To do this, you'd need to: 1. Create a new `AppTypeRegistry` 2. Register `NPC` 3. Register `Option<String>` If we skip Step 3, then the entire scene might fail to serialize as `Option<String>` requires registration. Not only is this annoying and easy to forget, but it can leave users with an impossible task: serializing a third-party type that contains private types. Generally, the third-party crate will register their private types within a plugin so the user doesn't need to do it themselves. However, this means we are now unable to serialize _just_ that type— we're forced to allow everything! ## Solution Add the `SceneFilter` enum for filtering components to extract. This filter can be used to optionally allow or deny entire sets of components/resources. With the `DynamicSceneBuilder`, users have more control over how their `DynamicScene`s are built. To only serialize a subset of components, use the `allow` method: ```rust let scene = builder .allow::<ComponentA>() .allow::<ComponentB>() .extract_entity(entity) .build(); ``` To serialize everything _but_ a subset of components, use the `deny` method: ```rust let scene = builder .deny::<ComponentA>() .deny::<ComponentB>() .extract_entity(entity) .build(); ``` Or create a custom filter: ```rust let components = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(components); // let filter = SceneFilter::Denylist(components); let scene = builder .with_filter(Some(filter)) .extract_entity(entity) .build(); ``` Similar operations exist for resources: <details> <summary>View Resource Methods</summary> To only serialize a subset of resources, use the `allow_resource` method: ```rust let scene = builder .allow_resource::<ResourceA>() .extract_resources() .build(); ``` To serialize everything _but_ a subset of resources, use the `deny_resource` method: ```rust let scene = builder .deny_resource::<ResourceA>() .extract_resources() .build(); ``` Or create a custom filter: ```rust let resources = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(resources); // let filter = SceneFilter::Denylist(resources); let scene = builder .with_resource_filter(Some(filter)) .extract_resources() .build(); ``` </details> ### Open Questions - [x] ~~`allow` and `deny` are mutually exclusive. Currently, they overwrite each other. Should this instead be a panic?~~ Took @soqb's suggestion and made it so that the opposing method simply removes that type from the list. - [x] ~~`DynamicSceneBuilder` extracts entity data as soon as `extract_entity`/`extract_entities` is called. Should this behavior instead be moved to the `build` method to prevent ordering mixups (e.g. `.allow::<Foo>().extract_entity(entity)` vs `.extract_entity(entity).allow::<Foo>()`)? The tradeoff would be iterating over the given entities twice: once at extraction and again at build.~~ Based on the feedback from @Testare it sounds like it might be better to just keep the current functionality (if anything we can open a separate PR that adds deferred methods for extraction, so the choice/performance hit is up to the user). - [ ] An alternative might be to remove the filter from `DynamicSceneBuilder` and have it as a separate parameter to the extraction methods (either in the existing ones or as added `extract_entity_with_filter`-type methods). Is this preferable? - [x] ~~Should we include constructors that include common types to allow/deny? For example, a `SceneFilter::standard_allowlist` that includes things like `Parent` and `Children`?~~ Consensus suggests we should. I may split this out into a followup PR, though. - [x] ~~Should we add the ability to remove types from the filter regardless of whether an allowlist or denylist (e.g. `filter.remove::<Foo>()`)?~~ See the first list item - [x] ~~Should `SceneFilter` be an enum? Would it make more sense as a struct that contains an `is_denylist` boolean?~~ With the added `SceneFilter::None` state (replacing the need to wrap in an `Option` or rely on an empty `Denylist`), it seems an enum is better suited now - [x] ~~Bikeshed: Do we like the naming convention? Should we instead use `include`/`exclude` terminology?~~ Sounds like we're sticking with `allow`/`deny`! - [x] ~~Does this feature need a new example? Do we simply include it in the existing one (maybe even as a comment?)? Should this be done in a followup PR instead?~~ Example will be added in a followup PR ### Followup Tasks - [ ] Add a dedicated `SceneFilter` example - [ ] Possibly add default types to the filter (e.g. deny things like `ComputedVisibility`, allow `Parent`, etc) --- ## Changelog - Added the `SceneFilter` enum for filtering components and resources when building a `DynamicScene` - Added methods: - `DynamicSceneBuilder::with_filter` - `DynamicSceneBuilder::allow` - `DynamicSceneBuilder::deny` - `DynamicSceneBuilder::allow_all` - `DynamicSceneBuilder::deny_all` - `DynamicSceneBuilder::with_resource_filter` - `DynamicSceneBuilder::allow_resource` - `DynamicSceneBuilder::deny_resource` - `DynamicSceneBuilder::allow_all_resources` - `DynamicSceneBuilder::deny_all_resources` - Removed methods: - `DynamicSceneBuilder::from_world_with_type_registry` - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference ## Migration Guide - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let dynamic_scene = DynamicScene::from_world(&world, registry); // let dynamic_scene = DynamicScene::from_scene(&scene, registry); // NEW let dynamic_scene = DynamicScene::from_world(&world); // let dynamic_scene = DynamicScene::from_scene(&scene); ``` - Removed `DynamicSceneBuilder::from_world_with_type_registry`. Now the registry is automatically taken from the given world: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let builder = DynamicSceneBuilder::from_world_with_type_registry(&world, registry); // NEW let builder = DynamicSceneBuilder::from_world(&world); ```
2023-07-06 21:04:26 +00:00
#[derive(Resource, Reflect, Default, Eq, PartialEq, Debug)]
#[reflect(Resource)]
struct ResourceB;
#[test]
fn extract_one_entity() {
let mut world = World::default();
let atr = AppTypeRegistry::default();
atr.write().register::<ComponentA>();
world.insert_resource(atr);
let entity = world.spawn((ComponentA, ComponentB)).id();
let scene = DynamicSceneBuilder::from_world(&world)
.extract_entity(entity)
.build();
assert_eq!(scene.entities.len(), 1);
Make scene handling of entity references robust (#7335) # Objective - Handle dangling entity references inside scenes - Handle references to entities with generation > 0 inside scenes - Fix a latent bug in `Parent`'s `MapEntities` implementation, which would, if the parent was outside the scene, cause the scene to be loaded into the new world with a parent reference potentially pointing to some random entity in that new world. - Fixes #4793 and addresses #7235 ## Solution - DynamicScenes now identify entities with a `Entity` instead of a u32, therefore including generation - `World` exposes a new `reserve_generations` function that despawns an entity and advances its generation by some extra amount. - `MapEntities` implementations have a new `get_or_reserve` function available that will always return an `Entity`, establishing a new mapping to a dead entity when the entity they are called with is not in the `EntityMap`. Subsequent calls with that same `Entity` will return the same newly created dead entity reference, preserving equality semantics. - As a result, after loading a scene containing references to dead entities (or entities otherwise outside the scene), those references will all point to different generations on a single entity id in the new world. --- ## Changelog ### Changed - In serialized scenes, entities are now identified by a u64 instead of a u32. - In serialized scenes, components with entity references now have those references serialize as u64s instead of structs. ### Fixed - Scenes containing components with entity references will now deserialize and add to a world reliably. ## Migration Guide - `MapEntities` implementations must change from a `&EntityMap` parameter to a `&mut EntityMapper` parameter and can no longer return a `Result`. Finally, they should switch from calling `EntityMap::get` to calling `EntityMapper::get_or_reserve`. --------- Co-authored-by: Nicola Papale <nicopap@users.noreply.github.com>
2023-05-01 15:49:27 +00:00
assert_eq!(scene.entities[0].entity, entity);
assert_eq!(scene.entities[0].components.len(), 1);
assert!(scene.entities[0].components[0].represents::<ComponentA>());
}
#[test]
fn extract_one_entity_twice() {
let mut world = World::default();
let atr = AppTypeRegistry::default();
atr.write().register::<ComponentA>();
world.insert_resource(atr);
let entity = world.spawn((ComponentA, ComponentB)).id();
let scene = DynamicSceneBuilder::from_world(&world)
.extract_entity(entity)
.extract_entity(entity)
.build();
assert_eq!(scene.entities.len(), 1);
Make scene handling of entity references robust (#7335) # Objective - Handle dangling entity references inside scenes - Handle references to entities with generation > 0 inside scenes - Fix a latent bug in `Parent`'s `MapEntities` implementation, which would, if the parent was outside the scene, cause the scene to be loaded into the new world with a parent reference potentially pointing to some random entity in that new world. - Fixes #4793 and addresses #7235 ## Solution - DynamicScenes now identify entities with a `Entity` instead of a u32, therefore including generation - `World` exposes a new `reserve_generations` function that despawns an entity and advances its generation by some extra amount. - `MapEntities` implementations have a new `get_or_reserve` function available that will always return an `Entity`, establishing a new mapping to a dead entity when the entity they are called with is not in the `EntityMap`. Subsequent calls with that same `Entity` will return the same newly created dead entity reference, preserving equality semantics. - As a result, after loading a scene containing references to dead entities (or entities otherwise outside the scene), those references will all point to different generations on a single entity id in the new world. --- ## Changelog ### Changed - In serialized scenes, entities are now identified by a u64 instead of a u32. - In serialized scenes, components with entity references now have those references serialize as u64s instead of structs. ### Fixed - Scenes containing components with entity references will now deserialize and add to a world reliably. ## Migration Guide - `MapEntities` implementations must change from a `&EntityMap` parameter to a `&mut EntityMapper` parameter and can no longer return a `Result`. Finally, they should switch from calling `EntityMap::get` to calling `EntityMapper::get_or_reserve`. --------- Co-authored-by: Nicola Papale <nicopap@users.noreply.github.com>
2023-05-01 15:49:27 +00:00
assert_eq!(scene.entities[0].entity, entity);
assert_eq!(scene.entities[0].components.len(), 1);
assert!(scene.entities[0].components[0].represents::<ComponentA>());
}
#[test]
fn extract_one_entity_two_components() {
let mut world = World::default();
let atr = AppTypeRegistry::default();
{
let mut register = atr.write();
register.register::<ComponentA>();
register.register::<ComponentB>();
}
world.insert_resource(atr);
let entity = world.spawn((ComponentA, ComponentB)).id();
let scene = DynamicSceneBuilder::from_world(&world)
.extract_entity(entity)
.build();
assert_eq!(scene.entities.len(), 1);
Make scene handling of entity references robust (#7335) # Objective - Handle dangling entity references inside scenes - Handle references to entities with generation > 0 inside scenes - Fix a latent bug in `Parent`'s `MapEntities` implementation, which would, if the parent was outside the scene, cause the scene to be loaded into the new world with a parent reference potentially pointing to some random entity in that new world. - Fixes #4793 and addresses #7235 ## Solution - DynamicScenes now identify entities with a `Entity` instead of a u32, therefore including generation - `World` exposes a new `reserve_generations` function that despawns an entity and advances its generation by some extra amount. - `MapEntities` implementations have a new `get_or_reserve` function available that will always return an `Entity`, establishing a new mapping to a dead entity when the entity they are called with is not in the `EntityMap`. Subsequent calls with that same `Entity` will return the same newly created dead entity reference, preserving equality semantics. - As a result, after loading a scene containing references to dead entities (or entities otherwise outside the scene), those references will all point to different generations on a single entity id in the new world. --- ## Changelog ### Changed - In serialized scenes, entities are now identified by a u64 instead of a u32. - In serialized scenes, components with entity references now have those references serialize as u64s instead of structs. ### Fixed - Scenes containing components with entity references will now deserialize and add to a world reliably. ## Migration Guide - `MapEntities` implementations must change from a `&EntityMap` parameter to a `&mut EntityMapper` parameter and can no longer return a `Result`. Finally, they should switch from calling `EntityMap::get` to calling `EntityMapper::get_or_reserve`. --------- Co-authored-by: Nicola Papale <nicopap@users.noreply.github.com>
2023-05-01 15:49:27 +00:00
assert_eq!(scene.entities[0].entity, entity);
assert_eq!(scene.entities[0].components.len(), 2);
assert!(scene.entities[0].components[0].represents::<ComponentA>());
assert!(scene.entities[0].components[1].represents::<ComponentB>());
}
#[test]
fn extract_entity_order() {
let mut world = World::default();
world.init_resource::<AppTypeRegistry>();
// Spawn entities in order
let entity_a = world.spawn_empty().id();
let entity_b = world.spawn_empty().id();
let entity_c = world.spawn_empty().id();
let entity_d = world.spawn_empty().id();
// Insert entities out of order
let builder = DynamicSceneBuilder::from_world(&world)
.extract_entity(entity_b)
.extract_entities([entity_d, entity_a].into_iter())
.extract_entity(entity_c);
let mut entities = builder.build().entities.into_iter();
// Assert entities are ordered
Make scene handling of entity references robust (#7335) # Objective - Handle dangling entity references inside scenes - Handle references to entities with generation > 0 inside scenes - Fix a latent bug in `Parent`'s `MapEntities` implementation, which would, if the parent was outside the scene, cause the scene to be loaded into the new world with a parent reference potentially pointing to some random entity in that new world. - Fixes #4793 and addresses #7235 ## Solution - DynamicScenes now identify entities with a `Entity` instead of a u32, therefore including generation - `World` exposes a new `reserve_generations` function that despawns an entity and advances its generation by some extra amount. - `MapEntities` implementations have a new `get_or_reserve` function available that will always return an `Entity`, establishing a new mapping to a dead entity when the entity they are called with is not in the `EntityMap`. Subsequent calls with that same `Entity` will return the same newly created dead entity reference, preserving equality semantics. - As a result, after loading a scene containing references to dead entities (or entities otherwise outside the scene), those references will all point to different generations on a single entity id in the new world. --- ## Changelog ### Changed - In serialized scenes, entities are now identified by a u64 instead of a u32. - In serialized scenes, components with entity references now have those references serialize as u64s instead of structs. ### Fixed - Scenes containing components with entity references will now deserialize and add to a world reliably. ## Migration Guide - `MapEntities` implementations must change from a `&EntityMap` parameter to a `&mut EntityMapper` parameter and can no longer return a `Result`. Finally, they should switch from calling `EntityMap::get` to calling `EntityMapper::get_or_reserve`. --------- Co-authored-by: Nicola Papale <nicopap@users.noreply.github.com>
2023-05-01 15:49:27 +00:00
assert_eq!(entity_a, entities.next().map(|e| e.entity).unwrap());
assert_eq!(entity_b, entities.next().map(|e| e.entity).unwrap());
assert_eq!(entity_c, entities.next().map(|e| e.entity).unwrap());
assert_eq!(entity_d, entities.next().map(|e| e.entity).unwrap());
}
#[test]
fn extract_query() {
let mut world = World::default();
let atr = AppTypeRegistry::default();
{
let mut register = atr.write();
register.register::<ComponentA>();
register.register::<ComponentB>();
}
world.insert_resource(atr);
let entity_a_b = world.spawn((ComponentA, ComponentB)).id();
let entity_a = world.spawn(ComponentA).id();
let _entity_b = world.spawn(ComponentB).id();
let mut query = world.query_filtered::<Entity, With<ComponentA>>();
let scene = DynamicSceneBuilder::from_world(&world)
.extract_entities(query.iter(&world))
.build();
assert_eq!(scene.entities.len(), 2);
let mut scene_entities = vec![scene.entities[0].entity, scene.entities[1].entity];
scene_entities.sort();
Make scene handling of entity references robust (#7335) # Objective - Handle dangling entity references inside scenes - Handle references to entities with generation > 0 inside scenes - Fix a latent bug in `Parent`'s `MapEntities` implementation, which would, if the parent was outside the scene, cause the scene to be loaded into the new world with a parent reference potentially pointing to some random entity in that new world. - Fixes #4793 and addresses #7235 ## Solution - DynamicScenes now identify entities with a `Entity` instead of a u32, therefore including generation - `World` exposes a new `reserve_generations` function that despawns an entity and advances its generation by some extra amount. - `MapEntities` implementations have a new `get_or_reserve` function available that will always return an `Entity`, establishing a new mapping to a dead entity when the entity they are called with is not in the `EntityMap`. Subsequent calls with that same `Entity` will return the same newly created dead entity reference, preserving equality semantics. - As a result, after loading a scene containing references to dead entities (or entities otherwise outside the scene), those references will all point to different generations on a single entity id in the new world. --- ## Changelog ### Changed - In serialized scenes, entities are now identified by a u64 instead of a u32. - In serialized scenes, components with entity references now have those references serialize as u64s instead of structs. ### Fixed - Scenes containing components with entity references will now deserialize and add to a world reliably. ## Migration Guide - `MapEntities` implementations must change from a `&EntityMap` parameter to a `&mut EntityMapper` parameter and can no longer return a `Result`. Finally, they should switch from calling `EntityMap::get` to calling `EntityMapper::get_or_reserve`. --------- Co-authored-by: Nicola Papale <nicopap@users.noreply.github.com>
2023-05-01 15:49:27 +00:00
assert_eq!(scene_entities, [entity_a_b, entity_a]);
}
#[test]
fn remove_componentless_entity() {
let mut world = World::default();
let atr = AppTypeRegistry::default();
atr.write().register::<ComponentA>();
world.insert_resource(atr);
let entity_a = world.spawn(ComponentA).id();
let entity_b = world.spawn(ComponentB).id();
let scene = DynamicSceneBuilder::from_world(&world)
.extract_entities([entity_a, entity_b].into_iter())
.remove_empty_entities()
.build();
assert_eq!(scene.entities.len(), 1);
Make scene handling of entity references robust (#7335) # Objective - Handle dangling entity references inside scenes - Handle references to entities with generation > 0 inside scenes - Fix a latent bug in `Parent`'s `MapEntities` implementation, which would, if the parent was outside the scene, cause the scene to be loaded into the new world with a parent reference potentially pointing to some random entity in that new world. - Fixes #4793 and addresses #7235 ## Solution - DynamicScenes now identify entities with a `Entity` instead of a u32, therefore including generation - `World` exposes a new `reserve_generations` function that despawns an entity and advances its generation by some extra amount. - `MapEntities` implementations have a new `get_or_reserve` function available that will always return an `Entity`, establishing a new mapping to a dead entity when the entity they are called with is not in the `EntityMap`. Subsequent calls with that same `Entity` will return the same newly created dead entity reference, preserving equality semantics. - As a result, after loading a scene containing references to dead entities (or entities otherwise outside the scene), those references will all point to different generations on a single entity id in the new world. --- ## Changelog ### Changed - In serialized scenes, entities are now identified by a u64 instead of a u32. - In serialized scenes, components with entity references now have those references serialize as u64s instead of structs. ### Fixed - Scenes containing components with entity references will now deserialize and add to a world reliably. ## Migration Guide - `MapEntities` implementations must change from a `&EntityMap` parameter to a `&mut EntityMapper` parameter and can no longer return a `Result`. Finally, they should switch from calling `EntityMap::get` to calling `EntityMapper::get_or_reserve`. --------- Co-authored-by: Nicola Papale <nicopap@users.noreply.github.com>
2023-05-01 15:49:27 +00:00
assert_eq!(scene.entities[0].entity, entity_a);
}
(De) serialize resources in scenes (#6846) # Objective Co-Authored-By: davier [bricedavier@gmail.com](mailto:bricedavier@gmail.com) Fixes #3576. Adds a `resources` field in scene serialization data to allow de/serializing resources that have reflection enabled. ## Solution Most of this code is taken from a previous closed PR: https://github.com/bevyengine/bevy/pull/3580. Most of the credit goes to @Davier , what I did was mostly getting it to work on the latest main branch of Bevy, along with adding a few asserts in the currently existing tests to be sure everything is working properly. This PR changes the scene format to include resources in this way: ``` ( resources: { // List of resources here, keyed by resource type name. }, entities: [ // Previous scene format here ], ) ``` An example taken from the tests: ``` ( resources: { "bevy_scene::serde::tests::MyResource": ( foo: 123, ), }, entities: { // Previous scene format here }, ) ``` For this, a `resources` fields has been added on the `DynamicScene` and the `DynamicSceneBuilder` structs. The latter now also has a method named `extract_resources` to properly extract the existing resources registered in the local type registry, in a similar way to `extract_entities`. --- ## Changelog Added: Reflect resources registered in the type registry used by dynamic scenes will now be properly de/serialized in scene data. ## Migration Guide Since the scene format has been changed, the user may not be able to use scenes saved prior to this PR due to the `resources` scene field being missing. ~~To preserve backwards compatibility, I will try to make the `resources` fully optional so that old scenes can be loaded without issue.~~ ## TODOs - [x] I may have to update a few doc blocks still referring to dynamic scenes as mere container of entities, since they now include resources as well. - [x] ~~I want to make the `resources` key optional, as specified in the Migration Guide, so that old scenes will be compatible with this change.~~ Since this would only be trivial for ron format, I think it might be better to consider it in a separate PR/discussion to figure out if it could be done for binary serialization too. - [x] I suppose it might be a good idea to add a resources in the scene example so that users will quickly notice they can serialize resources just like entities. --------- Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2023-03-20 21:17:02 +00:00
#[test]
fn extract_one_resource() {
let mut world = World::default();
let atr = AppTypeRegistry::default();
atr.write().register::<ResourceA>();
world.insert_resource(atr);
world.insert_resource(ResourceA);
let scene = DynamicSceneBuilder::from_world(&world)
.extract_resources()
.build();
(De) serialize resources in scenes (#6846) # Objective Co-Authored-By: davier [bricedavier@gmail.com](mailto:bricedavier@gmail.com) Fixes #3576. Adds a `resources` field in scene serialization data to allow de/serializing resources that have reflection enabled. ## Solution Most of this code is taken from a previous closed PR: https://github.com/bevyengine/bevy/pull/3580. Most of the credit goes to @Davier , what I did was mostly getting it to work on the latest main branch of Bevy, along with adding a few asserts in the currently existing tests to be sure everything is working properly. This PR changes the scene format to include resources in this way: ``` ( resources: { // List of resources here, keyed by resource type name. }, entities: [ // Previous scene format here ], ) ``` An example taken from the tests: ``` ( resources: { "bevy_scene::serde::tests::MyResource": ( foo: 123, ), }, entities: { // Previous scene format here }, ) ``` For this, a `resources` fields has been added on the `DynamicScene` and the `DynamicSceneBuilder` structs. The latter now also has a method named `extract_resources` to properly extract the existing resources registered in the local type registry, in a similar way to `extract_entities`. --- ## Changelog Added: Reflect resources registered in the type registry used by dynamic scenes will now be properly de/serialized in scene data. ## Migration Guide Since the scene format has been changed, the user may not be able to use scenes saved prior to this PR due to the `resources` scene field being missing. ~~To preserve backwards compatibility, I will try to make the `resources` fully optional so that old scenes can be loaded without issue.~~ ## TODOs - [x] I may have to update a few doc blocks still referring to dynamic scenes as mere container of entities, since they now include resources as well. - [x] ~~I want to make the `resources` key optional, as specified in the Migration Guide, so that old scenes will be compatible with this change.~~ Since this would only be trivial for ron format, I think it might be better to consider it in a separate PR/discussion to figure out if it could be done for binary serialization too. - [x] I suppose it might be a good idea to add a resources in the scene example so that users will quickly notice they can serialize resources just like entities. --------- Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2023-03-20 21:17:02 +00:00
assert_eq!(scene.resources.len(), 1);
assert!(scene.resources[0].represents::<ResourceA>());
}
#[test]
fn extract_one_resource_twice() {
let mut world = World::default();
let atr = AppTypeRegistry::default();
atr.write().register::<ResourceA>();
world.insert_resource(atr);
world.insert_resource(ResourceA);
let scene = DynamicSceneBuilder::from_world(&world)
.extract_resources()
.extract_resources()
.build();
(De) serialize resources in scenes (#6846) # Objective Co-Authored-By: davier [bricedavier@gmail.com](mailto:bricedavier@gmail.com) Fixes #3576. Adds a `resources` field in scene serialization data to allow de/serializing resources that have reflection enabled. ## Solution Most of this code is taken from a previous closed PR: https://github.com/bevyengine/bevy/pull/3580. Most of the credit goes to @Davier , what I did was mostly getting it to work on the latest main branch of Bevy, along with adding a few asserts in the currently existing tests to be sure everything is working properly. This PR changes the scene format to include resources in this way: ``` ( resources: { // List of resources here, keyed by resource type name. }, entities: [ // Previous scene format here ], ) ``` An example taken from the tests: ``` ( resources: { "bevy_scene::serde::tests::MyResource": ( foo: 123, ), }, entities: { // Previous scene format here }, ) ``` For this, a `resources` fields has been added on the `DynamicScene` and the `DynamicSceneBuilder` structs. The latter now also has a method named `extract_resources` to properly extract the existing resources registered in the local type registry, in a similar way to `extract_entities`. --- ## Changelog Added: Reflect resources registered in the type registry used by dynamic scenes will now be properly de/serialized in scene data. ## Migration Guide Since the scene format has been changed, the user may not be able to use scenes saved prior to this PR due to the `resources` scene field being missing. ~~To preserve backwards compatibility, I will try to make the `resources` fully optional so that old scenes can be loaded without issue.~~ ## TODOs - [x] I may have to update a few doc blocks still referring to dynamic scenes as mere container of entities, since they now include resources as well. - [x] ~~I want to make the `resources` key optional, as specified in the Migration Guide, so that old scenes will be compatible with this change.~~ Since this would only be trivial for ron format, I think it might be better to consider it in a separate PR/discussion to figure out if it could be done for binary serialization too. - [x] I suppose it might be a good idea to add a resources in the scene example so that users will quickly notice they can serialize resources just like entities. --------- Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2023-03-20 21:17:02 +00:00
assert_eq!(scene.resources.len(), 1);
assert!(scene.resources[0].represents::<ResourceA>());
}
bevy_scene: Add `SceneFilter` (#6793) # Objective Currently, `DynamicScene`s extract all components listed in the given (or the world's) type registry. This acts as a quasi-filter of sorts. However, it can be troublesome to use effectively and lacks decent control. For example, say you need to serialize only the following component over the network: ```rust #[derive(Reflect, Component, Default)] #[reflect(Component)] struct NPC { name: Option<String> } ``` To do this, you'd need to: 1. Create a new `AppTypeRegistry` 2. Register `NPC` 3. Register `Option<String>` If we skip Step 3, then the entire scene might fail to serialize as `Option<String>` requires registration. Not only is this annoying and easy to forget, but it can leave users with an impossible task: serializing a third-party type that contains private types. Generally, the third-party crate will register their private types within a plugin so the user doesn't need to do it themselves. However, this means we are now unable to serialize _just_ that type— we're forced to allow everything! ## Solution Add the `SceneFilter` enum for filtering components to extract. This filter can be used to optionally allow or deny entire sets of components/resources. With the `DynamicSceneBuilder`, users have more control over how their `DynamicScene`s are built. To only serialize a subset of components, use the `allow` method: ```rust let scene = builder .allow::<ComponentA>() .allow::<ComponentB>() .extract_entity(entity) .build(); ``` To serialize everything _but_ a subset of components, use the `deny` method: ```rust let scene = builder .deny::<ComponentA>() .deny::<ComponentB>() .extract_entity(entity) .build(); ``` Or create a custom filter: ```rust let components = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(components); // let filter = SceneFilter::Denylist(components); let scene = builder .with_filter(Some(filter)) .extract_entity(entity) .build(); ``` Similar operations exist for resources: <details> <summary>View Resource Methods</summary> To only serialize a subset of resources, use the `allow_resource` method: ```rust let scene = builder .allow_resource::<ResourceA>() .extract_resources() .build(); ``` To serialize everything _but_ a subset of resources, use the `deny_resource` method: ```rust let scene = builder .deny_resource::<ResourceA>() .extract_resources() .build(); ``` Or create a custom filter: ```rust let resources = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(resources); // let filter = SceneFilter::Denylist(resources); let scene = builder .with_resource_filter(Some(filter)) .extract_resources() .build(); ``` </details> ### Open Questions - [x] ~~`allow` and `deny` are mutually exclusive. Currently, they overwrite each other. Should this instead be a panic?~~ Took @soqb's suggestion and made it so that the opposing method simply removes that type from the list. - [x] ~~`DynamicSceneBuilder` extracts entity data as soon as `extract_entity`/`extract_entities` is called. Should this behavior instead be moved to the `build` method to prevent ordering mixups (e.g. `.allow::<Foo>().extract_entity(entity)` vs `.extract_entity(entity).allow::<Foo>()`)? The tradeoff would be iterating over the given entities twice: once at extraction and again at build.~~ Based on the feedback from @Testare it sounds like it might be better to just keep the current functionality (if anything we can open a separate PR that adds deferred methods for extraction, so the choice/performance hit is up to the user). - [ ] An alternative might be to remove the filter from `DynamicSceneBuilder` and have it as a separate parameter to the extraction methods (either in the existing ones or as added `extract_entity_with_filter`-type methods). Is this preferable? - [x] ~~Should we include constructors that include common types to allow/deny? For example, a `SceneFilter::standard_allowlist` that includes things like `Parent` and `Children`?~~ Consensus suggests we should. I may split this out into a followup PR, though. - [x] ~~Should we add the ability to remove types from the filter regardless of whether an allowlist or denylist (e.g. `filter.remove::<Foo>()`)?~~ See the first list item - [x] ~~Should `SceneFilter` be an enum? Would it make more sense as a struct that contains an `is_denylist` boolean?~~ With the added `SceneFilter::None` state (replacing the need to wrap in an `Option` or rely on an empty `Denylist`), it seems an enum is better suited now - [x] ~~Bikeshed: Do we like the naming convention? Should we instead use `include`/`exclude` terminology?~~ Sounds like we're sticking with `allow`/`deny`! - [x] ~~Does this feature need a new example? Do we simply include it in the existing one (maybe even as a comment?)? Should this be done in a followup PR instead?~~ Example will be added in a followup PR ### Followup Tasks - [ ] Add a dedicated `SceneFilter` example - [ ] Possibly add default types to the filter (e.g. deny things like `ComputedVisibility`, allow `Parent`, etc) --- ## Changelog - Added the `SceneFilter` enum for filtering components and resources when building a `DynamicScene` - Added methods: - `DynamicSceneBuilder::with_filter` - `DynamicSceneBuilder::allow` - `DynamicSceneBuilder::deny` - `DynamicSceneBuilder::allow_all` - `DynamicSceneBuilder::deny_all` - `DynamicSceneBuilder::with_resource_filter` - `DynamicSceneBuilder::allow_resource` - `DynamicSceneBuilder::deny_resource` - `DynamicSceneBuilder::allow_all_resources` - `DynamicSceneBuilder::deny_all_resources` - Removed methods: - `DynamicSceneBuilder::from_world_with_type_registry` - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference ## Migration Guide - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let dynamic_scene = DynamicScene::from_world(&world, registry); // let dynamic_scene = DynamicScene::from_scene(&scene, registry); // NEW let dynamic_scene = DynamicScene::from_world(&world); // let dynamic_scene = DynamicScene::from_scene(&scene); ``` - Removed `DynamicSceneBuilder::from_world_with_type_registry`. Now the registry is automatically taken from the given world: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let builder = DynamicSceneBuilder::from_world_with_type_registry(&world, registry); // NEW let builder = DynamicSceneBuilder::from_world(&world); ```
2023-07-06 21:04:26 +00:00
#[test]
fn should_extract_allowed_components() {
let mut world = World::default();
let atr = AppTypeRegistry::default();
{
let mut register = atr.write();
register.register::<ComponentA>();
register.register::<ComponentB>();
}
world.insert_resource(atr);
let entity_a_b = world.spawn((ComponentA, ComponentB)).id();
let entity_a = world.spawn(ComponentA).id();
let entity_b = world.spawn(ComponentB).id();
let scene = DynamicSceneBuilder::from_world(&world)
.allow_component::<ComponentA>()
.extract_entities([entity_a_b, entity_a, entity_b].into_iter())
.build();
bevy_scene: Add `SceneFilter` (#6793) # Objective Currently, `DynamicScene`s extract all components listed in the given (or the world's) type registry. This acts as a quasi-filter of sorts. However, it can be troublesome to use effectively and lacks decent control. For example, say you need to serialize only the following component over the network: ```rust #[derive(Reflect, Component, Default)] #[reflect(Component)] struct NPC { name: Option<String> } ``` To do this, you'd need to: 1. Create a new `AppTypeRegistry` 2. Register `NPC` 3. Register `Option<String>` If we skip Step 3, then the entire scene might fail to serialize as `Option<String>` requires registration. Not only is this annoying and easy to forget, but it can leave users with an impossible task: serializing a third-party type that contains private types. Generally, the third-party crate will register their private types within a plugin so the user doesn't need to do it themselves. However, this means we are now unable to serialize _just_ that type— we're forced to allow everything! ## Solution Add the `SceneFilter` enum for filtering components to extract. This filter can be used to optionally allow or deny entire sets of components/resources. With the `DynamicSceneBuilder`, users have more control over how their `DynamicScene`s are built. To only serialize a subset of components, use the `allow` method: ```rust let scene = builder .allow::<ComponentA>() .allow::<ComponentB>() .extract_entity(entity) .build(); ``` To serialize everything _but_ a subset of components, use the `deny` method: ```rust let scene = builder .deny::<ComponentA>() .deny::<ComponentB>() .extract_entity(entity) .build(); ``` Or create a custom filter: ```rust let components = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(components); // let filter = SceneFilter::Denylist(components); let scene = builder .with_filter(Some(filter)) .extract_entity(entity) .build(); ``` Similar operations exist for resources: <details> <summary>View Resource Methods</summary> To only serialize a subset of resources, use the `allow_resource` method: ```rust let scene = builder .allow_resource::<ResourceA>() .extract_resources() .build(); ``` To serialize everything _but_ a subset of resources, use the `deny_resource` method: ```rust let scene = builder .deny_resource::<ResourceA>() .extract_resources() .build(); ``` Or create a custom filter: ```rust let resources = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(resources); // let filter = SceneFilter::Denylist(resources); let scene = builder .with_resource_filter(Some(filter)) .extract_resources() .build(); ``` </details> ### Open Questions - [x] ~~`allow` and `deny` are mutually exclusive. Currently, they overwrite each other. Should this instead be a panic?~~ Took @soqb's suggestion and made it so that the opposing method simply removes that type from the list. - [x] ~~`DynamicSceneBuilder` extracts entity data as soon as `extract_entity`/`extract_entities` is called. Should this behavior instead be moved to the `build` method to prevent ordering mixups (e.g. `.allow::<Foo>().extract_entity(entity)` vs `.extract_entity(entity).allow::<Foo>()`)? The tradeoff would be iterating over the given entities twice: once at extraction and again at build.~~ Based on the feedback from @Testare it sounds like it might be better to just keep the current functionality (if anything we can open a separate PR that adds deferred methods for extraction, so the choice/performance hit is up to the user). - [ ] An alternative might be to remove the filter from `DynamicSceneBuilder` and have it as a separate parameter to the extraction methods (either in the existing ones or as added `extract_entity_with_filter`-type methods). Is this preferable? - [x] ~~Should we include constructors that include common types to allow/deny? For example, a `SceneFilter::standard_allowlist` that includes things like `Parent` and `Children`?~~ Consensus suggests we should. I may split this out into a followup PR, though. - [x] ~~Should we add the ability to remove types from the filter regardless of whether an allowlist or denylist (e.g. `filter.remove::<Foo>()`)?~~ See the first list item - [x] ~~Should `SceneFilter` be an enum? Would it make more sense as a struct that contains an `is_denylist` boolean?~~ With the added `SceneFilter::None` state (replacing the need to wrap in an `Option` or rely on an empty `Denylist`), it seems an enum is better suited now - [x] ~~Bikeshed: Do we like the naming convention? Should we instead use `include`/`exclude` terminology?~~ Sounds like we're sticking with `allow`/`deny`! - [x] ~~Does this feature need a new example? Do we simply include it in the existing one (maybe even as a comment?)? Should this be done in a followup PR instead?~~ Example will be added in a followup PR ### Followup Tasks - [ ] Add a dedicated `SceneFilter` example - [ ] Possibly add default types to the filter (e.g. deny things like `ComputedVisibility`, allow `Parent`, etc) --- ## Changelog - Added the `SceneFilter` enum for filtering components and resources when building a `DynamicScene` - Added methods: - `DynamicSceneBuilder::with_filter` - `DynamicSceneBuilder::allow` - `DynamicSceneBuilder::deny` - `DynamicSceneBuilder::allow_all` - `DynamicSceneBuilder::deny_all` - `DynamicSceneBuilder::with_resource_filter` - `DynamicSceneBuilder::allow_resource` - `DynamicSceneBuilder::deny_resource` - `DynamicSceneBuilder::allow_all_resources` - `DynamicSceneBuilder::deny_all_resources` - Removed methods: - `DynamicSceneBuilder::from_world_with_type_registry` - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference ## Migration Guide - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let dynamic_scene = DynamicScene::from_world(&world, registry); // let dynamic_scene = DynamicScene::from_scene(&scene, registry); // NEW let dynamic_scene = DynamicScene::from_world(&world); // let dynamic_scene = DynamicScene::from_scene(&scene); ``` - Removed `DynamicSceneBuilder::from_world_with_type_registry`. Now the registry is automatically taken from the given world: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let builder = DynamicSceneBuilder::from_world_with_type_registry(&world, registry); // NEW let builder = DynamicSceneBuilder::from_world(&world); ```
2023-07-06 21:04:26 +00:00
assert_eq!(scene.entities.len(), 3);
assert!(scene.entities[0].components[0].represents::<ComponentA>());
assert!(scene.entities[1].components[0].represents::<ComponentA>());
assert_eq!(scene.entities[2].components.len(), 0);
}
#[test]
fn should_not_extract_denied_components() {
let mut world = World::default();
let atr = AppTypeRegistry::default();
{
let mut register = atr.write();
register.register::<ComponentA>();
register.register::<ComponentB>();
}
world.insert_resource(atr);
let entity_a_b = world.spawn((ComponentA, ComponentB)).id();
let entity_a = world.spawn(ComponentA).id();
let entity_b = world.spawn(ComponentB).id();
let scene = DynamicSceneBuilder::from_world(&world)
.deny_component::<ComponentA>()
.extract_entities([entity_a_b, entity_a, entity_b].into_iter())
.build();
bevy_scene: Add `SceneFilter` (#6793) # Objective Currently, `DynamicScene`s extract all components listed in the given (or the world's) type registry. This acts as a quasi-filter of sorts. However, it can be troublesome to use effectively and lacks decent control. For example, say you need to serialize only the following component over the network: ```rust #[derive(Reflect, Component, Default)] #[reflect(Component)] struct NPC { name: Option<String> } ``` To do this, you'd need to: 1. Create a new `AppTypeRegistry` 2. Register `NPC` 3. Register `Option<String>` If we skip Step 3, then the entire scene might fail to serialize as `Option<String>` requires registration. Not only is this annoying and easy to forget, but it can leave users with an impossible task: serializing a third-party type that contains private types. Generally, the third-party crate will register their private types within a plugin so the user doesn't need to do it themselves. However, this means we are now unable to serialize _just_ that type— we're forced to allow everything! ## Solution Add the `SceneFilter` enum for filtering components to extract. This filter can be used to optionally allow or deny entire sets of components/resources. With the `DynamicSceneBuilder`, users have more control over how their `DynamicScene`s are built. To only serialize a subset of components, use the `allow` method: ```rust let scene = builder .allow::<ComponentA>() .allow::<ComponentB>() .extract_entity(entity) .build(); ``` To serialize everything _but_ a subset of components, use the `deny` method: ```rust let scene = builder .deny::<ComponentA>() .deny::<ComponentB>() .extract_entity(entity) .build(); ``` Or create a custom filter: ```rust let components = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(components); // let filter = SceneFilter::Denylist(components); let scene = builder .with_filter(Some(filter)) .extract_entity(entity) .build(); ``` Similar operations exist for resources: <details> <summary>View Resource Methods</summary> To only serialize a subset of resources, use the `allow_resource` method: ```rust let scene = builder .allow_resource::<ResourceA>() .extract_resources() .build(); ``` To serialize everything _but_ a subset of resources, use the `deny_resource` method: ```rust let scene = builder .deny_resource::<ResourceA>() .extract_resources() .build(); ``` Or create a custom filter: ```rust let resources = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(resources); // let filter = SceneFilter::Denylist(resources); let scene = builder .with_resource_filter(Some(filter)) .extract_resources() .build(); ``` </details> ### Open Questions - [x] ~~`allow` and `deny` are mutually exclusive. Currently, they overwrite each other. Should this instead be a panic?~~ Took @soqb's suggestion and made it so that the opposing method simply removes that type from the list. - [x] ~~`DynamicSceneBuilder` extracts entity data as soon as `extract_entity`/`extract_entities` is called. Should this behavior instead be moved to the `build` method to prevent ordering mixups (e.g. `.allow::<Foo>().extract_entity(entity)` vs `.extract_entity(entity).allow::<Foo>()`)? The tradeoff would be iterating over the given entities twice: once at extraction and again at build.~~ Based on the feedback from @Testare it sounds like it might be better to just keep the current functionality (if anything we can open a separate PR that adds deferred methods for extraction, so the choice/performance hit is up to the user). - [ ] An alternative might be to remove the filter from `DynamicSceneBuilder` and have it as a separate parameter to the extraction methods (either in the existing ones or as added `extract_entity_with_filter`-type methods). Is this preferable? - [x] ~~Should we include constructors that include common types to allow/deny? For example, a `SceneFilter::standard_allowlist` that includes things like `Parent` and `Children`?~~ Consensus suggests we should. I may split this out into a followup PR, though. - [x] ~~Should we add the ability to remove types from the filter regardless of whether an allowlist or denylist (e.g. `filter.remove::<Foo>()`)?~~ See the first list item - [x] ~~Should `SceneFilter` be an enum? Would it make more sense as a struct that contains an `is_denylist` boolean?~~ With the added `SceneFilter::None` state (replacing the need to wrap in an `Option` or rely on an empty `Denylist`), it seems an enum is better suited now - [x] ~~Bikeshed: Do we like the naming convention? Should we instead use `include`/`exclude` terminology?~~ Sounds like we're sticking with `allow`/`deny`! - [x] ~~Does this feature need a new example? Do we simply include it in the existing one (maybe even as a comment?)? Should this be done in a followup PR instead?~~ Example will be added in a followup PR ### Followup Tasks - [ ] Add a dedicated `SceneFilter` example - [ ] Possibly add default types to the filter (e.g. deny things like `ComputedVisibility`, allow `Parent`, etc) --- ## Changelog - Added the `SceneFilter` enum for filtering components and resources when building a `DynamicScene` - Added methods: - `DynamicSceneBuilder::with_filter` - `DynamicSceneBuilder::allow` - `DynamicSceneBuilder::deny` - `DynamicSceneBuilder::allow_all` - `DynamicSceneBuilder::deny_all` - `DynamicSceneBuilder::with_resource_filter` - `DynamicSceneBuilder::allow_resource` - `DynamicSceneBuilder::deny_resource` - `DynamicSceneBuilder::allow_all_resources` - `DynamicSceneBuilder::deny_all_resources` - Removed methods: - `DynamicSceneBuilder::from_world_with_type_registry` - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference ## Migration Guide - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let dynamic_scene = DynamicScene::from_world(&world, registry); // let dynamic_scene = DynamicScene::from_scene(&scene, registry); // NEW let dynamic_scene = DynamicScene::from_world(&world); // let dynamic_scene = DynamicScene::from_scene(&scene); ``` - Removed `DynamicSceneBuilder::from_world_with_type_registry`. Now the registry is automatically taken from the given world: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let builder = DynamicSceneBuilder::from_world_with_type_registry(&world, registry); // NEW let builder = DynamicSceneBuilder::from_world(&world); ```
2023-07-06 21:04:26 +00:00
assert_eq!(scene.entities.len(), 3);
assert!(scene.entities[0].components[0].represents::<ComponentB>());
assert_eq!(scene.entities[1].components.len(), 0);
assert!(scene.entities[2].components[0].represents::<ComponentB>());
}
#[test]
fn should_extract_allowed_resources() {
let mut world = World::default();
let atr = AppTypeRegistry::default();
{
let mut register = atr.write();
register.register::<ResourceA>();
register.register::<ResourceB>();
}
world.insert_resource(atr);
world.insert_resource(ResourceA);
world.insert_resource(ResourceB);
let scene = DynamicSceneBuilder::from_world(&world)
.allow_resource::<ResourceA>()
.extract_resources()
.build();
bevy_scene: Add `SceneFilter` (#6793) # Objective Currently, `DynamicScene`s extract all components listed in the given (or the world's) type registry. This acts as a quasi-filter of sorts. However, it can be troublesome to use effectively and lacks decent control. For example, say you need to serialize only the following component over the network: ```rust #[derive(Reflect, Component, Default)] #[reflect(Component)] struct NPC { name: Option<String> } ``` To do this, you'd need to: 1. Create a new `AppTypeRegistry` 2. Register `NPC` 3. Register `Option<String>` If we skip Step 3, then the entire scene might fail to serialize as `Option<String>` requires registration. Not only is this annoying and easy to forget, but it can leave users with an impossible task: serializing a third-party type that contains private types. Generally, the third-party crate will register their private types within a plugin so the user doesn't need to do it themselves. However, this means we are now unable to serialize _just_ that type— we're forced to allow everything! ## Solution Add the `SceneFilter` enum for filtering components to extract. This filter can be used to optionally allow or deny entire sets of components/resources. With the `DynamicSceneBuilder`, users have more control over how their `DynamicScene`s are built. To only serialize a subset of components, use the `allow` method: ```rust let scene = builder .allow::<ComponentA>() .allow::<ComponentB>() .extract_entity(entity) .build(); ``` To serialize everything _but_ a subset of components, use the `deny` method: ```rust let scene = builder .deny::<ComponentA>() .deny::<ComponentB>() .extract_entity(entity) .build(); ``` Or create a custom filter: ```rust let components = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(components); // let filter = SceneFilter::Denylist(components); let scene = builder .with_filter(Some(filter)) .extract_entity(entity) .build(); ``` Similar operations exist for resources: <details> <summary>View Resource Methods</summary> To only serialize a subset of resources, use the `allow_resource` method: ```rust let scene = builder .allow_resource::<ResourceA>() .extract_resources() .build(); ``` To serialize everything _but_ a subset of resources, use the `deny_resource` method: ```rust let scene = builder .deny_resource::<ResourceA>() .extract_resources() .build(); ``` Or create a custom filter: ```rust let resources = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(resources); // let filter = SceneFilter::Denylist(resources); let scene = builder .with_resource_filter(Some(filter)) .extract_resources() .build(); ``` </details> ### Open Questions - [x] ~~`allow` and `deny` are mutually exclusive. Currently, they overwrite each other. Should this instead be a panic?~~ Took @soqb's suggestion and made it so that the opposing method simply removes that type from the list. - [x] ~~`DynamicSceneBuilder` extracts entity data as soon as `extract_entity`/`extract_entities` is called. Should this behavior instead be moved to the `build` method to prevent ordering mixups (e.g. `.allow::<Foo>().extract_entity(entity)` vs `.extract_entity(entity).allow::<Foo>()`)? The tradeoff would be iterating over the given entities twice: once at extraction and again at build.~~ Based on the feedback from @Testare it sounds like it might be better to just keep the current functionality (if anything we can open a separate PR that adds deferred methods for extraction, so the choice/performance hit is up to the user). - [ ] An alternative might be to remove the filter from `DynamicSceneBuilder` and have it as a separate parameter to the extraction methods (either in the existing ones or as added `extract_entity_with_filter`-type methods). Is this preferable? - [x] ~~Should we include constructors that include common types to allow/deny? For example, a `SceneFilter::standard_allowlist` that includes things like `Parent` and `Children`?~~ Consensus suggests we should. I may split this out into a followup PR, though. - [x] ~~Should we add the ability to remove types from the filter regardless of whether an allowlist or denylist (e.g. `filter.remove::<Foo>()`)?~~ See the first list item - [x] ~~Should `SceneFilter` be an enum? Would it make more sense as a struct that contains an `is_denylist` boolean?~~ With the added `SceneFilter::None` state (replacing the need to wrap in an `Option` or rely on an empty `Denylist`), it seems an enum is better suited now - [x] ~~Bikeshed: Do we like the naming convention? Should we instead use `include`/`exclude` terminology?~~ Sounds like we're sticking with `allow`/`deny`! - [x] ~~Does this feature need a new example? Do we simply include it in the existing one (maybe even as a comment?)? Should this be done in a followup PR instead?~~ Example will be added in a followup PR ### Followup Tasks - [ ] Add a dedicated `SceneFilter` example - [ ] Possibly add default types to the filter (e.g. deny things like `ComputedVisibility`, allow `Parent`, etc) --- ## Changelog - Added the `SceneFilter` enum for filtering components and resources when building a `DynamicScene` - Added methods: - `DynamicSceneBuilder::with_filter` - `DynamicSceneBuilder::allow` - `DynamicSceneBuilder::deny` - `DynamicSceneBuilder::allow_all` - `DynamicSceneBuilder::deny_all` - `DynamicSceneBuilder::with_resource_filter` - `DynamicSceneBuilder::allow_resource` - `DynamicSceneBuilder::deny_resource` - `DynamicSceneBuilder::allow_all_resources` - `DynamicSceneBuilder::deny_all_resources` - Removed methods: - `DynamicSceneBuilder::from_world_with_type_registry` - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference ## Migration Guide - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let dynamic_scene = DynamicScene::from_world(&world, registry); // let dynamic_scene = DynamicScene::from_scene(&scene, registry); // NEW let dynamic_scene = DynamicScene::from_world(&world); // let dynamic_scene = DynamicScene::from_scene(&scene); ``` - Removed `DynamicSceneBuilder::from_world_with_type_registry`. Now the registry is automatically taken from the given world: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let builder = DynamicSceneBuilder::from_world_with_type_registry(&world, registry); // NEW let builder = DynamicSceneBuilder::from_world(&world); ```
2023-07-06 21:04:26 +00:00
assert_eq!(scene.resources.len(), 1);
assert!(scene.resources[0].represents::<ResourceA>());
}
#[test]
fn should_not_extract_denied_resources() {
let mut world = World::default();
let atr = AppTypeRegistry::default();
{
let mut register = atr.write();
register.register::<ResourceA>();
register.register::<ResourceB>();
}
world.insert_resource(atr);
world.insert_resource(ResourceA);
world.insert_resource(ResourceB);
let scene = DynamicSceneBuilder::from_world(&world)
.deny_resource::<ResourceA>()
.extract_resources()
.build();
bevy_scene: Add `SceneFilter` (#6793) # Objective Currently, `DynamicScene`s extract all components listed in the given (or the world's) type registry. This acts as a quasi-filter of sorts. However, it can be troublesome to use effectively and lacks decent control. For example, say you need to serialize only the following component over the network: ```rust #[derive(Reflect, Component, Default)] #[reflect(Component)] struct NPC { name: Option<String> } ``` To do this, you'd need to: 1. Create a new `AppTypeRegistry` 2. Register `NPC` 3. Register `Option<String>` If we skip Step 3, then the entire scene might fail to serialize as `Option<String>` requires registration. Not only is this annoying and easy to forget, but it can leave users with an impossible task: serializing a third-party type that contains private types. Generally, the third-party crate will register their private types within a plugin so the user doesn't need to do it themselves. However, this means we are now unable to serialize _just_ that type— we're forced to allow everything! ## Solution Add the `SceneFilter` enum for filtering components to extract. This filter can be used to optionally allow or deny entire sets of components/resources. With the `DynamicSceneBuilder`, users have more control over how their `DynamicScene`s are built. To only serialize a subset of components, use the `allow` method: ```rust let scene = builder .allow::<ComponentA>() .allow::<ComponentB>() .extract_entity(entity) .build(); ``` To serialize everything _but_ a subset of components, use the `deny` method: ```rust let scene = builder .deny::<ComponentA>() .deny::<ComponentB>() .extract_entity(entity) .build(); ``` Or create a custom filter: ```rust let components = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(components); // let filter = SceneFilter::Denylist(components); let scene = builder .with_filter(Some(filter)) .extract_entity(entity) .build(); ``` Similar operations exist for resources: <details> <summary>View Resource Methods</summary> To only serialize a subset of resources, use the `allow_resource` method: ```rust let scene = builder .allow_resource::<ResourceA>() .extract_resources() .build(); ``` To serialize everything _but_ a subset of resources, use the `deny_resource` method: ```rust let scene = builder .deny_resource::<ResourceA>() .extract_resources() .build(); ``` Or create a custom filter: ```rust let resources = HashSet::from([type_id]); let filter = SceneFilter::Allowlist(resources); // let filter = SceneFilter::Denylist(resources); let scene = builder .with_resource_filter(Some(filter)) .extract_resources() .build(); ``` </details> ### Open Questions - [x] ~~`allow` and `deny` are mutually exclusive. Currently, they overwrite each other. Should this instead be a panic?~~ Took @soqb's suggestion and made it so that the opposing method simply removes that type from the list. - [x] ~~`DynamicSceneBuilder` extracts entity data as soon as `extract_entity`/`extract_entities` is called. Should this behavior instead be moved to the `build` method to prevent ordering mixups (e.g. `.allow::<Foo>().extract_entity(entity)` vs `.extract_entity(entity).allow::<Foo>()`)? The tradeoff would be iterating over the given entities twice: once at extraction and again at build.~~ Based on the feedback from @Testare it sounds like it might be better to just keep the current functionality (if anything we can open a separate PR that adds deferred methods for extraction, so the choice/performance hit is up to the user). - [ ] An alternative might be to remove the filter from `DynamicSceneBuilder` and have it as a separate parameter to the extraction methods (either in the existing ones or as added `extract_entity_with_filter`-type methods). Is this preferable? - [x] ~~Should we include constructors that include common types to allow/deny? For example, a `SceneFilter::standard_allowlist` that includes things like `Parent` and `Children`?~~ Consensus suggests we should. I may split this out into a followup PR, though. - [x] ~~Should we add the ability to remove types from the filter regardless of whether an allowlist or denylist (e.g. `filter.remove::<Foo>()`)?~~ See the first list item - [x] ~~Should `SceneFilter` be an enum? Would it make more sense as a struct that contains an `is_denylist` boolean?~~ With the added `SceneFilter::None` state (replacing the need to wrap in an `Option` or rely on an empty `Denylist`), it seems an enum is better suited now - [x] ~~Bikeshed: Do we like the naming convention? Should we instead use `include`/`exclude` terminology?~~ Sounds like we're sticking with `allow`/`deny`! - [x] ~~Does this feature need a new example? Do we simply include it in the existing one (maybe even as a comment?)? Should this be done in a followup PR instead?~~ Example will be added in a followup PR ### Followup Tasks - [ ] Add a dedicated `SceneFilter` example - [ ] Possibly add default types to the filter (e.g. deny things like `ComputedVisibility`, allow `Parent`, etc) --- ## Changelog - Added the `SceneFilter` enum for filtering components and resources when building a `DynamicScene` - Added methods: - `DynamicSceneBuilder::with_filter` - `DynamicSceneBuilder::allow` - `DynamicSceneBuilder::deny` - `DynamicSceneBuilder::allow_all` - `DynamicSceneBuilder::deny_all` - `DynamicSceneBuilder::with_resource_filter` - `DynamicSceneBuilder::allow_resource` - `DynamicSceneBuilder::deny_resource` - `DynamicSceneBuilder::allow_all_resources` - `DynamicSceneBuilder::deny_all_resources` - Removed methods: - `DynamicSceneBuilder::from_world_with_type_registry` - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference ## Migration Guide - `DynamicScene::from_scene` and `DynamicScene::from_world` no longer require an `AppTypeRegistry` reference: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let dynamic_scene = DynamicScene::from_world(&world, registry); // let dynamic_scene = DynamicScene::from_scene(&scene, registry); // NEW let dynamic_scene = DynamicScene::from_world(&world); // let dynamic_scene = DynamicScene::from_scene(&scene); ``` - Removed `DynamicSceneBuilder::from_world_with_type_registry`. Now the registry is automatically taken from the given world: ```rust // OLD let registry = world.resource::<AppTypeRegistry>(); let builder = DynamicSceneBuilder::from_world_with_type_registry(&world, registry); // NEW let builder = DynamicSceneBuilder::from_world(&world); ```
2023-07-06 21:04:26 +00:00
assert_eq!(scene.resources.len(), 1);
assert!(scene.resources[0].represents::<ResourceB>());
}
#[test]
fn should_use_from_reflect() {
#[derive(Resource, Component, Reflect)]
#[reflect(Resource, Component)]
struct SomeType(i32);
let mut world = World::default();
let atr = AppTypeRegistry::default();
{
let mut register = atr.write();
register.register::<SomeType>();
}
world.insert_resource(atr);
world.insert_resource(SomeType(123));
let entity = world.spawn(SomeType(123)).id();
let scene = DynamicSceneBuilder::from_world(&world)
.extract_resources()
.extract_entities(vec![entity].into_iter())
.build();
let component = &scene.entities[0].components[0];
assert!(component
.try_as_reflect()
.expect("component should be concrete due to `FromReflect`")
.is::<SomeType>());
let resource = &scene.resources[0];
assert!(resource
.try_as_reflect()
.expect("resource should be concrete due to `FromReflect`")
.is::<SomeType>());
}
}