Gino Valente d96933ad9c 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
..
scene.rs	bevy_scene: Add SceneFilter (#6793)	2023-07-06 21:04:26 +00:00