Skip to content

Commit

Permalink
Attempt to remove component from render world if not extracted. (#15904)
Browse files Browse the repository at this point in the history
# Objective

Ensure that components that are conditionally extracted do not linger in
the render world when not extracted from the main world.

## Solution

If the `ExtractComponent` returns `None`, we'll remove the render world
component. I think this is the most sensible behavior here. In the
future if there really is a use case for keeping the previous render
component around, we could add a `Option<Self::Out>` parameter for the
previous render component to the method, or something similar. I think
that this follows the principle of least surprise here relative to what
`None` would suggest and the way that render nodes are typically
written. The alternative would be to add an `enabled` field to pretty
much every camera settings component, or duplicate the extraction
condition as #15856 does.

## Testing

`transmission` no longer crashes.

## Migration Guide

Components that implement `ExtractComponent` and return `None` will
cause the extracted component to be removed from the render world.
  • Loading branch information
tychedelia authored Oct 15, 2024
1 parent c1bb4b2 commit acbed60
Showing 1 changed file with 8 additions and 3 deletions.
11 changes: 8 additions & 3 deletions crates/bevy_render/src/extract_component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,10 @@ pub trait ExtractComponent: Component {

/// The output from extraction.
///
/// Returning `None` based on the queried item can allow early optimization,
/// for example if there is an `enabled: bool` field on `Self`, or by only accepting
/// values within certain thresholds.
/// Returning `None` based on the queried item will remove the component from the entity in
/// the render world. This can be used, for example, to conditionally extract camera settings
/// in order to disable a rendering feature on the basis of those settings, without removing
/// the component from the entity in the main world.
///
/// The output may be different from the queried component.
/// This can be useful for example if only a subset of the fields are useful
Expand Down Expand Up @@ -205,6 +206,8 @@ fn extract_components<C: ExtractComponent>(
for (entity, query_item) in &query {
if let Some(component) = C::extract_component(query_item) {
values.push((entity, component));
} else {
commands.entity(entity).remove::<C>();
}
}
*previous_len = values.len();
Expand All @@ -222,6 +225,8 @@ fn extract_visible_components<C: ExtractComponent>(
if view_visibility.get() {
if let Some(component) = C::extract_component(query_item) {
values.push((entity, component));
} else {
commands.entity(entity).remove::<C>();
}
}
}
Expand Down

0 comments on commit acbed60

Please sign in to comment.