Skip to content

Commit

Permalink
explicitly mention component in methods on DynamicSceneBuilder (#…
Browse files Browse the repository at this point in the history
…15210)

# Objective

The method names on `DynamicSceneBuilder` are misleading. Specifically,
`deny_all` and `allow_all` implies everything will be denied/allowed,
including all components and resources. In reality, these methods only
apply to components (which is mentioned in the docs).

## Solution

- change `deny_all` and `allow_all` to `deny_all_components` and
`allow_all_components`
- also, change the remaining methods to mention component where it makes
sense

We could also add the `deny_all` and `allow_all` methods back later,
only this time, they would deny/allow both resources and components.

## Showcase

### Before
```rust
let builder = DynamicSceneBuilder::from_world(world)
    .deny_all()
    .deny_all_resources()
    .allow::<MyComponent>();
```

### After
```rust
let builder = DynamicSceneBuilder::from_world(world)
    .deny_all_components()
    .deny_all_resources()
    .allow_component::<MyComponent>();
```

## Migration Guide

the following invocations on `DynamicSceneBuilder` should be changed by
users
- `with_filter` -> `with_component_filter`
- `allow` -> `allow_component`
- `deny` -> `deny_component`
- `allow_all` -> `allow_all_components`
- `deny_all` -> `deny_all_components`
  • Loading branch information
atornity authored Sep 15, 2024
1 parent 2ea51fc commit 2ea8d35
Showing 1 changed file with 15 additions and 15 deletions.
30 changes: 15 additions & 15 deletions crates/bevy_scene/src/dynamic_scene_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ use std::collections::BTreeMap;
///
/// 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_filter) or by explicitly
/// [allowing](DynamicSceneBuilder::allow)/[denying](DynamicSceneBuilder::deny) certain components.
/// 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.
///
/// Extraction happens immediately and uses the filter as it exists during the time of extraction.
///
Expand Down Expand Up @@ -76,7 +76,7 @@ impl<'w> DynamicSceneBuilder<'w> {

/// Specify a custom component [`SceneFilter`] to be used with this builder.
#[must_use]
pub fn with_filter(mut self, filter: SceneFilter) -> Self {
pub fn with_component_filter(mut self, filter: SceneFilter) -> Self {
self.component_filter = filter;
self
}
Expand All @@ -92,10 +92,10 @@ impl<'w> DynamicSceneBuilder<'w> {
///
/// This method may be called multiple times for any number of components.
///
/// This is the inverse of [`deny`](Self::deny).
/// This is the inverse of [`deny_component`](Self::deny_component).
/// If `T` has already been denied, then it will be removed from the denylist.
#[must_use]
pub fn allow<T: Component>(mut self) -> Self {
pub fn allow_component<T: Component>(mut self) -> Self {
self.component_filter = self.component_filter.allow::<T>();
self
}
Expand All @@ -104,10 +104,10 @@ impl<'w> DynamicSceneBuilder<'w> {
///
/// This method may be called multiple times for any number of components.
///
/// This is the inverse of [`allow`](Self::allow).
/// This is the inverse of [`allow_component`](Self::allow_component).
/// If `T` has already been allowed, then it will be removed from the allowlist.
#[must_use]
pub fn deny<T: Component>(mut self) -> Self {
pub fn deny_component<T: Component>(mut self) -> Self {
self.component_filter = self.component_filter.deny::<T>();
self
}
Expand All @@ -116,9 +116,9 @@ impl<'w> DynamicSceneBuilder<'w> {
///
/// This is useful for resetting the filter so that types may be selectively [denied].
///
/// [denied]: Self::deny
/// [denied]: Self::deny_component
#[must_use]
pub fn allow_all(mut self) -> Self {
pub fn allow_all_components(mut self) -> Self {
self.component_filter = SceneFilter::allow_all();
self
}
Expand All @@ -127,9 +127,9 @@ impl<'w> DynamicSceneBuilder<'w> {
///
/// This is useful for resetting the filter so that types may be selectively [allowed].
///
/// [allowed]: Self::allow
/// [allowed]: Self::allow_component
#[must_use]
pub fn deny_all(mut self) -> Self {
pub fn deny_all_components(mut self) -> Self {
self.component_filter = SceneFilter::deny_all();
self
}
Expand Down Expand Up @@ -242,8 +242,8 @@ impl<'w> DynamicSceneBuilder<'w> {
///
/// Note that components extracted from queried entities must still pass through the filter if one is set.
///
/// [`allow`]: Self::allow
/// [`deny`]: Self::deny
/// [`allow`]: Self::allow_component
/// [`deny`]: Self::deny_component
#[must_use]
pub fn extract_entities(mut self, entities: impl Iterator<Item = Entity>) -> Self {
let type_registry = self.original_world.resource::<AppTypeRegistry>().read();
Expand Down Expand Up @@ -582,7 +582,7 @@ mod tests {
let entity_b = world.spawn(ComponentB).id();

let scene = DynamicSceneBuilder::from_world(&world)
.allow::<ComponentA>()
.allow_component::<ComponentA>()
.extract_entities([entity_a_b, entity_a, entity_b].into_iter())
.build();

Expand All @@ -609,7 +609,7 @@ mod tests {
let entity_b = world.spawn(ComponentB).id();

let scene = DynamicSceneBuilder::from_world(&world)
.deny::<ComponentA>()
.deny_component::<ComponentA>()
.extract_entities([entity_a_b, entity_a, entity_b].into_iter())
.build();

Expand Down

0 comments on commit 2ea8d35

Please sign in to comment.