Skip to content

Commit

Permalink
Deprecate get_or_spawn (#15652)
Browse files Browse the repository at this point in the history
# Objective

After merging retained rendering world #15320, we now have a good way of
creating a link between worlds (*HIYAA intensifies*). This means that
`get_or_spawn` is no longer necessary for that function. Entity should
be opaque as the warning above `get_or_spawn` says. This is also part of
#15459.

I'm deprecating `get_or_spawn_batch` in a different PR in order to keep
the PR small in size.

## Solution

Deprecate `get_or_spawn` and replace it with `get_entity` in most
contexts. If it's possible to query `&RenderEntity`, then the entity is
synced and `render_entity.id()` is initialized in the render world.

## Migration Guide

If you are given an `Entity` and you want to do something with it, use
`Commands.entity(...)` or `World.entity(...)`. If instead you want to
spawn something use `Commands.spawn(...)` or `World.spawn(...)`. If you
are not sure if an entity exists, you can always use `get_entity` and
match on the `Option<...>` that is returned.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
  • Loading branch information
Trashtalk217 and alice-i-cecile authored Oct 7, 2024
1 parent 0374648 commit d1bd46d
Show file tree
Hide file tree
Showing 17 changed files with 90 additions and 200 deletions.
38 changes: 0 additions & 38 deletions benches/benches/bevy_ecs/world/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,41 +209,3 @@ pub fn medium_sized_commands(criterion: &mut Criterion) {
pub fn large_sized_commands(criterion: &mut Criterion) {
sized_commands_impl::<SizedCommand<LargeStruct>>(criterion);
}

pub fn get_or_spawn(criterion: &mut Criterion) {
let mut group = criterion.benchmark_group("get_or_spawn");
group.warm_up_time(core::time::Duration::from_millis(500));
group.measurement_time(core::time::Duration::from_secs(4));

group.bench_function("individual", |bencher| {
let mut world = World::default();
let mut command_queue = CommandQueue::default();

bencher.iter(|| {
let mut commands = Commands::new(&mut command_queue, &world);
for i in 0..10_000 {
commands
.get_or_spawn(Entity::from_raw(i))
.insert((Matrix::default(), Vec3::default()));
}
command_queue.apply(&mut world);
});
});

group.bench_function("batched", |bencher| {
let mut world = World::default();
let mut command_queue = CommandQueue::default();

bencher.iter(|| {
let mut commands = Commands::new(&mut command_queue, &world);
let mut values = Vec::with_capacity(10_000);
for i in 0..10_000 {
values.push((Entity::from_raw(i), (Matrix::default(), Vec3::default())));
}
commands.insert_or_spawn_batch(values);
command_queue.apply(&mut world);
});
});

group.finish();
}
1 change: 0 additions & 1 deletion benches/benches/bevy_ecs/world/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ criterion_group!(
zero_sized_commands,
medium_sized_commands,
large_sized_commands,
get_or_spawn,
world_entity,
world_get,
world_query_get,
Expand Down
3 changes: 2 additions & 1 deletion crates/bevy_core_pipeline/src/core_3d/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -590,7 +590,8 @@ pub fn extract_camera_prepass_phase(
live_entities.insert(entity);

commands
.get_or_spawn(entity)
.get_entity(entity)
.expect("Camera entity wasn't synced.")
.insert_if(DepthPrepass, || depth_prepass)
.insert_if(NormalPrepass, || normal_prepass)
.insert_if(MotionVectorPrepass, || motion_vector_prepass)
Expand Down
32 changes: 18 additions & 14 deletions crates/bevy_core_pipeline/src/dof/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -830,20 +830,24 @@ fn extract_depth_of_field_settings(
calculate_focal_length(depth_of_field.sensor_height, perspective_projection.fov);

// Convert `DepthOfField` to `DepthOfFieldUniform`.
commands.get_or_spawn(entity).insert((
*depth_of_field,
DepthOfFieldUniform {
focal_distance: depth_of_field.focal_distance,
focal_length,
coc_scale_factor: focal_length * focal_length
/ (depth_of_field.sensor_height * depth_of_field.aperture_f_stops),
max_circle_of_confusion_diameter: depth_of_field.max_circle_of_confusion_diameter,
max_depth: depth_of_field.max_depth,
pad_a: 0,
pad_b: 0,
pad_c: 0,
},
));
commands
.get_entity(entity)
.expect("Depth of field entity wasn't synced.")
.insert((
*depth_of_field,
DepthOfFieldUniform {
focal_distance: depth_of_field.focal_distance,
focal_length,
coc_scale_factor: focal_length * focal_length
/ (depth_of_field.sensor_height * depth_of_field.aperture_f_stops),
max_circle_of_confusion_diameter: depth_of_field
.max_circle_of_confusion_diameter,
max_depth: depth_of_field.max_depth,
pad_a: 0,
pad_b: 0,
pad_c: 0,
},
));
}
}

Expand Down
3 changes: 2 additions & 1 deletion crates/bevy_core_pipeline/src/taa/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,8 @@ fn extract_taa_settings(mut commands: Commands, mut main_world: ResMut<MainWorld
let has_perspective_projection = matches!(camera_projection, Projection::Perspective(_));
if camera.is_active && has_perspective_projection {
commands
.get_or_spawn(entity.id())
.get_entity(entity.id())
.expect("Camera entity wasn't synced.")
.insert(taa_settings.clone());
taa_settings.reset = false;
}
Expand Down
101 changes: 0 additions & 101 deletions crates/bevy_ecs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1616,107 +1616,6 @@ mod tests {
assert_eq!(0, query_min_size![(&A, &B), Or<(Changed<A>, Changed<B>)>]);
}

#[test]
fn reserve_entities_across_worlds() {
let mut world_a = World::default();
let mut world_b = World::default();

let e1 = world_a.spawn(A(1)).id();
let e2 = world_a.spawn(A(2)).id();
let e3 = world_a.entities().reserve_entity();
world_a.flush_entities();

let world_a_max_entities = world_a.entities().len();
world_b.entities.reserve_entities(world_a_max_entities);
world_b.entities.flush_as_invalid();

let e4 = world_b.spawn(A(4)).id();
assert_eq!(
e4,
Entity::from_raw(3),
"new entity is created immediately after world_a's max entity"
);
assert!(world_b.get::<A>(e1).is_none());
assert!(world_b.get_entity(e1).is_err());

assert!(world_b.get::<A>(e2).is_none());
assert!(world_b.get_entity(e2).is_err());

assert!(world_b.get::<A>(e3).is_none());
assert!(world_b.get_entity(e3).is_err());

world_b.get_or_spawn(e1).unwrap().insert(B(1));
assert_eq!(
world_b.get::<B>(e1),
Some(&B(1)),
"spawning into 'world_a' entities works"
);

world_b.get_or_spawn(e4).unwrap().insert(B(4));
assert_eq!(
world_b.get::<B>(e4),
Some(&B(4)),
"spawning into existing `world_b` entities works"
);
assert_eq!(
world_b.get::<A>(e4),
Some(&A(4)),
"spawning into existing `world_b` entities works"
);

let e4_mismatched_generation =
Entity::from_raw_and_generation(3, NonZero::<u32>::new(2).unwrap());
assert!(
world_b.get_or_spawn(e4_mismatched_generation).is_none(),
"attempting to spawn on top of an entity with a mismatched entity generation fails"
);
assert_eq!(
world_b.get::<B>(e4),
Some(&B(4)),
"failed mismatched spawn doesn't change existing entity"
);
assert_eq!(
world_b.get::<A>(e4),
Some(&A(4)),
"failed mismatched spawn doesn't change existing entity"
);

let high_non_existent_entity = Entity::from_raw(6);
world_b
.get_or_spawn(high_non_existent_entity)
.unwrap()
.insert(B(10));
assert_eq!(
world_b.get::<B>(high_non_existent_entity),
Some(&B(10)),
"inserting into newly allocated high / non-continuous entity id works"
);

let high_non_existent_but_reserved_entity = Entity::from_raw(5);
assert!(
world_b.get_entity(high_non_existent_but_reserved_entity).is_err(),
"entities between high-newly allocated entity and continuous block of existing entities don't exist"
);

let reserved_entities = vec![
world_b.entities().reserve_entity(),
world_b.entities().reserve_entity(),
world_b.entities().reserve_entity(),
world_b.entities().reserve_entity(),
];

assert_eq!(
reserved_entities,
vec![
Entity::from_raw(5),
Entity::from_raw(4),
Entity::from_raw(7),
Entity::from_raw(8),
],
"space between original entities and high entities is used for new entity ids"
);
}

#[test]
fn insert_or_spawn_batch() {
let mut world = World::default();
Expand Down
2 changes: 2 additions & 0 deletions crates/bevy_ecs/src/system/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,8 +325,10 @@ impl<'w, 's> Commands<'w, 's> {
/// [`Commands::spawn`]. This method should generally only be used for sharing entities across
/// apps, and only when they have a scheme worked out to share an ID space (which doesn't happen
/// by default).
#[deprecated(since = "0.15.0", note = "use Commands::spawn instead")]
pub fn get_or_spawn(&mut self, entity: Entity) -> EntityCommands {
self.queue(move |world: &mut World| {
#[allow(deprecated)]
world.get_or_spawn(entity);
});
EntityCommands {
Expand Down
1 change: 1 addition & 0 deletions crates/bevy_ecs/src/world/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -944,6 +944,7 @@ impl World {
/// This method should generally only be used for sharing entities across apps, and only when they have a
/// scheme worked out to share an ID space (which doesn't happen by default).
#[inline]
#[deprecated(since = "0.15.0", note = "use `World::spawn` instead")]
pub fn get_or_spawn(&mut self, entity: Entity) -> Option<EntityWorldMut> {
self.flush();
match self.entities.alloc_at_without_replacement(entity) {
Expand Down
21 changes: 12 additions & 9 deletions crates/bevy_pbr/src/cluster/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -554,14 +554,17 @@ pub fn extract_clusters(
}
}

commands.get_or_spawn(entity.id()).insert((
ExtractedClusterableObjects { data },
ExtractedClusterConfig {
near: clusters.near,
far: clusters.far,
dimensions: clusters.dimensions,
},
));
commands
.get_entity(entity.id())
.expect("Clusters entity wasn't synced.")
.insert((
ExtractedClusterableObjects { data },
ExtractedClusterConfig {
near: clusters.near,
far: clusters.far,
dimensions: clusters.dimensions,
},
));
}
}

Expand Down Expand Up @@ -617,7 +620,7 @@ pub fn prepare_clusters(

view_clusters_bindings.write_buffers(render_device, &render_queue);

commands.get_or_spawn(entity).insert(view_clusters_bindings);
commands.entity(entity).insert(view_clusters_bindings);
}
}

Expand Down
9 changes: 6 additions & 3 deletions crates/bevy_pbr/src/light_probe/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,8 @@ fn gather_environment_map_uniform(
EnvironmentMapUniform::default()
};
commands
.get_or_spawn(view_entity.id())
.get_entity(view_entity.id())
.expect("Environment map light entity wasn't synced.")
.insert(environment_map_uniform);
}
}
Expand Down Expand Up @@ -440,11 +441,13 @@ fn gather_light_probes<C>(
// Record the per-view light probes.
if render_view_light_probes.is_empty() {
commands
.get_or_spawn(entity)
.get_entity(entity)
.expect("View entity wasn't synced.")
.remove::<RenderViewLightProbes<C>>();
} else {
commands
.get_or_spawn(entity)
.get_entity(entity)
.expect("View entity wasn't synced.")
.insert(render_view_light_probes);
}
}
Expand Down
4 changes: 3 additions & 1 deletion crates/bevy_pbr/src/prepass/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -585,7 +585,9 @@ pub fn extract_camera_previous_view_data(
for (entity, camera, maybe_previous_view_data) in cameras_3d.iter() {
if camera.is_active {
let entity = entity.id();
let mut entity = commands.get_or_spawn(entity);
let mut entity = commands
.get_entity(entity)
.expect("Camera entity wasn't synced.");

if let Some(previous_view_data) = maybe_previous_view_data {
entity.insert(previous_view_data.clone());
Expand Down
45 changes: 24 additions & 21 deletions crates/bevy_pbr/src/render/light.rs
Original file line number Diff line number Diff line change
Expand Up @@ -403,27 +403,30 @@ pub fn extract_lights(
}
}

commands.get_or_spawn(entity.id()).insert((
ExtractedDirectionalLight {
color: directional_light.color.into(),
illuminance: directional_light.illuminance,
transform: *transform,
volumetric: volumetric_light.is_some(),
soft_shadow_size: directional_light.soft_shadow_size,
shadows_enabled: directional_light.shadows_enabled,
shadow_depth_bias: directional_light.shadow_depth_bias,
// The factor of SQRT_2 is for the worst-case diagonal offset
shadow_normal_bias: directional_light.shadow_normal_bias
* core::f32::consts::SQRT_2,
cascade_shadow_config: cascade_config.clone(),
cascades: extracted_cascades,
frusta: extracted_frusta,
render_layers: maybe_layers.unwrap_or_default().clone(),
},
CascadesVisibleEntities {
entities: cascade_visible_entities,
},
));
commands
.get_entity(entity.id())
.expect("Light entity wasn't synced.")
.insert((
ExtractedDirectionalLight {
color: directional_light.color.into(),
illuminance: directional_light.illuminance,
transform: *transform,
volumetric: volumetric_light.is_some(),
soft_shadow_size: directional_light.soft_shadow_size,
shadows_enabled: directional_light.shadows_enabled,
shadow_depth_bias: directional_light.shadow_depth_bias,
// The factor of SQRT_2 is for the worst-case diagonal offset
shadow_normal_bias: directional_light.shadow_normal_bias
* core::f32::consts::SQRT_2,
cascade_shadow_config: cascade_config.clone(),
cascades: extracted_cascades,
frusta: extracted_frusta,
render_layers: maybe_layers.unwrap_or_default().clone(),
},
CascadesVisibleEntities {
entities: cascade_visible_entities,
},
));
}
}

Expand Down
3 changes: 2 additions & 1 deletion crates/bevy_pbr/src/ssao/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,8 @@ fn extract_ssao_settings(
}
if camera.is_active {
commands
.get_or_spawn(entity.id())
.get_entity(entity.id())
.expect("SSAO entity wasn't synced.")
.insert(ssao_settings.clone());
}
}
Expand Down
13 changes: 10 additions & 3 deletions crates/bevy_pbr/src/volumetric_fog/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,18 +280,25 @@ pub fn extract_volumetric_fog(
}

for (entity, volumetric_fog) in view_targets.iter() {
commands.get_or_spawn(entity.id()).insert(*volumetric_fog);
commands
.get_entity(entity.id())
.expect("Volumetric fog entity wasn't synced.")
.insert(*volumetric_fog);
}

for (entity, fog_volume, fog_transform) in fog_volumes.iter() {
commands
.get_or_spawn(entity.id())
.get_entity(entity.id())
.expect("Fog volume entity wasn't synced.")
.insert((*fog_volume).clone())
.insert(*fog_transform);
}

for (entity, volumetric_light) in volumetric_lights.iter() {
commands.get_or_spawn(entity.id()).insert(*volumetric_light);
commands
.get_entity(entity.id())
.expect("Volumetric light entity wasn't synced.")
.insert(*volumetric_light);
}
}

Expand Down
Loading

0 comments on commit d1bd46d

Please sign in to comment.