From 6114347bc4718c5b9421f93f5085a1ee25fceb98 Mon Sep 17 00:00:00 2001 From: Martin Dickopp Date: Sun, 29 Dec 2024 23:45:17 +0100 Subject: [PATCH] Fix confusing comment in pbr example (#16996) # Objective After a recent fix for a panic in the pbr example (#16976), the code contains the following comment: ```rust // This system relies on system parameters that are not available at start // Ignore parameter failures so that it will run when possible .add_systems(Update, environment_map_load_finish.never_param_warn()) ``` However, this explanation is incorrect. `EnvironmentMapLabel` is available at start. The real issue is that it is no longer available once it has been removed by `environment_map_load_finish`. ## Solution - Remove confusing/incorrect comment and `never_param_warn()`. - Make `Single>` optional in `environment_map_load_finish`, and check that the entity has not yet been despawned. Since it is expected that an entity is no longer there once it has been despawned, it seems better to me to handle this case in `environment_map_load_finish`. ## Testing Ran `cargo run --example pbr`. --- examples/3d/pbr.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/examples/3d/pbr.rs b/examples/3d/pbr.rs index c7ba9f59df..12922db03b 100644 --- a/examples/3d/pbr.rs +++ b/examples/3d/pbr.rs @@ -7,9 +7,7 @@ fn main() { App::new() .add_plugins(DefaultPlugins) .add_systems(Startup, setup) - // This system relies on system parameters that are not available at start - // Ignore parameter failures so that it will run when possible - .add_systems(Update, environment_map_load_finish.never_param_warn()) + .add_systems(Update, environment_map_load_finish) .run(); } @@ -130,7 +128,7 @@ fn environment_map_load_finish( mut commands: Commands, asset_server: Res, environment_map: Single<&EnvironmentMapLight>, - label_entity: Single>, + label_entity: Option>>, ) { if asset_server .load_state(&environment_map.diffuse_map) @@ -139,7 +137,10 @@ fn environment_map_load_finish( .load_state(&environment_map.specular_map) .is_loaded() { - commands.entity(*label_entity).despawn(); + // Do not attempt to remove `label_entity` if it has already been removed. + if let Some(label_entity) = label_entity { + commands.entity(*label_entity).despawn(); + } } }