From acbed6040eeae99199657e44a9668772738ac344 Mon Sep 17 00:00:00 2001 From: charlotte Date: Mon, 14 Oct 2024 21:21:53 -0700 Subject: [PATCH] Attempt to remove component from render world if not extracted. (#15904) # 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` 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. --- crates/bevy_render/src/extract_component.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/crates/bevy_render/src/extract_component.rs b/crates/bevy_render/src/extract_component.rs index a2cd3b1655a8a..da027ebf0f9ae 100644 --- a/crates/bevy_render/src/extract_component.rs +++ b/crates/bevy_render/src/extract_component.rs @@ -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 @@ -205,6 +206,8 @@ fn extract_components( for (entity, query_item) in &query { if let Some(component) = C::extract_component(query_item) { values.push((entity, component)); + } else { + commands.entity(entity).remove::(); } } *previous_len = values.len(); @@ -222,6 +225,8 @@ fn extract_visible_components( if view_visibility.get() { if let Some(component) = C::extract_component(query_item) { values.push((entity, component)); + } else { + commands.entity(entity).remove::(); } } }