Skip to content

Commit d87e5c3

Browse files
authored
add helper functions for registering bundles to world (#20932)
This avoids having to create a `ComponentRegistrator` and passing it to Bundles::register_info everywhere it's called, reducing unsafe blocks. There aren't any calls to `register_info` and related functions that don't have access to a `&mut World` where it wouldn't be possible to do this. (part 2 to #20790), #20739 # Objective calling `Bundles::register_info` involves creating a `ComponentRegistrator` from world `components` and `component_ids`, requiring a `&mut World` and a safety comment ensuring the `ComponentRegistrator` belongs to the same world as the `Bundles`. ## Solution create a helper function to the world that calls `register_info` with the correct `ComponentRegistrator`, removing the need for unsafe blocks.
1 parent 39b2543 commit d87e5c3

File tree

5 files changed

+45
-90
lines changed

5 files changed

+45
-90
lines changed

crates/bevy_ecs/src/bundle/insert.rs

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use crate::{
99
},
1010
bundle::{ArchetypeMoveType, Bundle, BundleId, BundleInfo, DynamicBundle, InsertMode},
1111
change_detection::MaybeLocation,
12-
component::{Components, ComponentsRegistrator, StorageType, Tick},
12+
component::{Components, StorageType, Tick},
1313
entity::{Entities, Entity, EntityLocation},
1414
lifecycle::{ADD, INSERT, REPLACE},
1515
observer::Observers,
@@ -37,16 +37,8 @@ impl<'w> BundleInserter<'w> {
3737
archetype_id: ArchetypeId,
3838
change_tick: Tick,
3939
) -> Self {
40-
// SAFETY: These come from the same world. `world.components_registrator` can't be used since we borrow other fields too.
41-
let mut registrator =
42-
unsafe { ComponentsRegistrator::new(&mut world.components, &mut world.component_ids) };
43-
44-
// SAFETY: `registrator`, `world.bundles`, and `world.storages` all come from the same world
45-
let bundle_id = unsafe {
46-
world
47-
.bundles
48-
.register_info::<T>(&mut registrator, &mut world.storages)
49-
};
40+
let bundle_id = world.register_bundle_info::<T>();
41+
5042
// SAFETY: We just ensured this bundle exists
5143
unsafe { Self::new_with_id(world, archetype_id, bundle_id, change_tick) }
5244
}

crates/bevy_ecs/src/bundle/remove.rs

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use crate::{
66
archetype::{Archetype, ArchetypeCreated, ArchetypeId, Archetypes},
77
bundle::{Bundle, BundleId, BundleInfo},
88
change_detection::MaybeLocation,
9-
component::{ComponentId, Components, ComponentsRegistrator, StorageType},
9+
component::{ComponentId, Components, StorageType},
1010
entity::{Entity, EntityLocation},
1111
lifecycle::{REMOVE, REPLACE},
1212
observer::Observers,
@@ -39,16 +39,8 @@ impl<'w> BundleRemover<'w> {
3939
archetype_id: ArchetypeId,
4040
require_all: bool,
4141
) -> Option<Self> {
42-
// SAFETY: These come from the same world. `world.components_registrator` can't be used since we borrow other fields too.
43-
let mut registrator =
44-
unsafe { ComponentsRegistrator::new(&mut world.components, &mut world.component_ids) };
45-
46-
// SAFETY: `registrator`, `world.storages`, and `world.bundles` all come from the same world.
47-
let bundle_id = unsafe {
48-
world
49-
.bundles
50-
.register_info::<T>(&mut registrator, &mut world.storages)
51-
};
42+
let bundle_id = world.register_bundle_info::<T>();
43+
5244
// SAFETY: we initialized this bundle_id in `init_info`, and caller ensures archetype is valid.
5345
unsafe { Self::new_with_id(world, archetype_id, bundle_id, require_all) }
5446
}

crates/bevy_ecs/src/bundle/spawner.rs

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use crate::{
66
archetype::{Archetype, ArchetypeCreated, ArchetypeId, SpawnBundleStatus},
77
bundle::{Bundle, BundleId, BundleInfo, DynamicBundle, InsertMode},
88
change_detection::MaybeLocation,
9-
component::{ComponentsRegistrator, Tick},
9+
component::Tick,
1010
entity::{Entities, Entity, EntityLocation},
1111
lifecycle::{ADD, INSERT},
1212
relationship::RelationshipHookMode,
@@ -26,16 +26,8 @@ pub(crate) struct BundleSpawner<'w> {
2626
impl<'w> BundleSpawner<'w> {
2727
#[inline]
2828
pub fn new<T: Bundle>(world: &'w mut World, change_tick: Tick) -> Self {
29-
// SAFETY: These come from the same world. `world.components_registrator` can't be used since we borrow other fields too.
30-
let mut registrator =
31-
unsafe { ComponentsRegistrator::new(&mut world.components, &mut world.component_ids) };
29+
let bundle_id = world.register_bundle_info::<T>();
3230

33-
// SAFETY: `registrator`, `world.bundles`, and `world.storages` all come from the same world.
34-
let bundle_id = unsafe {
35-
world
36-
.bundles
37-
.register_info::<T>(&mut registrator, &mut world.storages)
38-
};
3931
// SAFETY: we initialized this bundle_id in `init_info`
4032
unsafe { Self::new_with_id(world, bundle_id, change_tick) }
4133
}

crates/bevy_ecs/src/world/entity_ref.rs

Lines changed: 9 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,7 @@ use crate::{
55
InsertMode,
66
},
77
change_detection::{MaybeLocation, MutUntyped},
8-
component::{
9-
Component, ComponentId, ComponentTicks, Components, ComponentsRegistrator, Mutable,
10-
StorageType, Tick,
11-
},
8+
component::{Component, ComponentId, ComponentTicks, Components, Mutable, StorageType, Tick},
129
entity::{
1310
ContainsEntity, Entity, EntityCloner, EntityClonerBuilder, EntityEquivalent,
1411
EntityIdLocation, EntityLocation, OptIn, OptOut,
@@ -2298,16 +2295,7 @@ impl<'w> EntityWorldMut<'w> {
22982295
caller: MaybeLocation,
22992296
) -> &mut Self {
23002297
let location = self.location();
2301-
let storages = &mut self.world.storages;
2302-
let bundles = &mut self.world.bundles;
2303-
// SAFETY: These come from the same world.
2304-
let mut registrator = unsafe {
2305-
ComponentsRegistrator::new(&mut self.world.components, &mut self.world.component_ids)
2306-
};
2307-
2308-
// SAFETY: `storages`, `bundles` and `registrator` come from the same world.
2309-
let bundle_id =
2310-
unsafe { bundles.register_contributed_bundle_info::<T>(&mut registrator, storages) };
2298+
let bundle_id = self.world.register_contributed_bundle_info::<T>();
23112299

23122300
// SAFETY: We just created the bundle, and the archetype is valid, since we are in it.
23132301
let Some(mut remover) = (unsafe {
@@ -2347,21 +2335,10 @@ impl<'w> EntityWorldMut<'w> {
23472335
#[inline]
23482336
pub(crate) fn retain_with_caller<T: Bundle>(&mut self, caller: MaybeLocation) -> &mut Self {
23492337
let old_location = self.location();
2338+
let retained_bundle = self.world.register_bundle_info::<T>();
23502339
let archetypes = &mut self.world.archetypes;
2351-
let storages = &mut self.world.storages;
2352-
// SAFETY: These come from the same world.
2353-
let mut registrator = unsafe {
2354-
ComponentsRegistrator::new(&mut self.world.components, &mut self.world.component_ids)
2355-
};
23562340

2357-
// SAFETY: `storages`, `bundles` and `registrator` come from the same world.
2358-
let retained_bundle = unsafe {
2359-
self.world
2360-
.bundles
2361-
.register_info::<T>(&mut registrator, storages)
2362-
};
2363-
2364-
// SAFETY: `retained_bundle` exists as we just initialized it.
2341+
// SAFETY: `retained_bundle` exists as we just registered it.
23652342
let retained_bundle_info = unsafe { self.world.bundles.get_unchecked(retained_bundle) };
23662343
let old_archetype = &mut archetypes[old_location.archetype_id];
23672344

@@ -2370,10 +2347,11 @@ impl<'w> EntityWorldMut<'w> {
23702347
.components()
23712348
.filter(|c| !retained_bundle_info.contributed_components().contains(c))
23722349
.collect::<Vec<_>>();
2373-
let remove_bundle =
2374-
self.world
2375-
.bundles
2376-
.init_dynamic_info(&mut self.world.storages, &registrator, to_remove);
2350+
let remove_bundle = self.world.bundles.init_dynamic_info(
2351+
&mut self.world.storages,
2352+
&self.world.components,
2353+
to_remove,
2354+
);
23772355

23782356
// SAFETY: We just created the bundle, and the archetype is valid, since we are in it.
23792357
let Some(mut remover) = (unsafe {

crates/bevy_ecs/src/world/mod.rs

Lines changed: 28 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,17 @@ pub mod unsafe_world_cell;
1313
#[cfg(feature = "bevy_reflect")]
1414
pub mod reflect;
1515

16-
pub use crate::{
17-
change_detection::{Mut, Ref, CHECK_TICK_THRESHOLD},
18-
world::command_queue::CommandQueue,
19-
};
2016
use crate::{
17+
bundle::BundleId,
2118
error::{DefaultErrorHandler, ErrorHandler},
2219
event::BufferedEvent,
2320
lifecycle::{ComponentHooks, ADD, DESPAWN, INSERT, REMOVE, REPLACE},
2421
prelude::{Add, Despawn, Insert, Remove, Replace},
2522
};
23+
pub use crate::{
24+
change_detection::{Mut, Ref, CHECK_TICK_THRESHOLD},
25+
world::command_queue::CommandQueue,
26+
};
2627
pub use bevy_ecs_macros::FromWorld;
2728
use bevy_utils::prelude::DebugName;
2829
pub use deferred_world::DeferredWorld;
@@ -2302,15 +2303,7 @@ impl World {
23022303

23032304
self.flush();
23042305
let change_tick = self.change_tick();
2305-
// SAFETY: These come from the same world. `Self.components_registrator` can't be used since we borrow other fields too.
2306-
let mut registrator =
2307-
unsafe { ComponentsRegistrator::new(&mut self.components, &mut self.component_ids) };
2308-
2309-
// SAFETY: `registrator`, `self.bundles`, and `self.storages` all come from this world.
2310-
let bundle_id = unsafe {
2311-
self.bundles
2312-
.register_info::<B>(&mut registrator, &mut self.storages)
2313-
};
2306+
let bundle_id = self.register_bundle_info::<B>();
23142307

23152308
let mut batch_iter = batch.into_iter();
23162309

@@ -2450,15 +2443,7 @@ impl World {
24502443

24512444
self.flush();
24522445
let change_tick = self.change_tick();
2453-
// SAFETY: These come from the same world. `Self.components_registrator` can't be used since we borrow other fields too.
2454-
let mut registrator =
2455-
unsafe { ComponentsRegistrator::new(&mut self.components, &mut self.component_ids) };
2456-
2457-
// SAFETY: `registrator`, `self.bundles`, and `self.storages` all come from this world.
2458-
let bundle_id = unsafe {
2459-
self.bundles
2460-
.register_info::<B>(&mut registrator, &mut self.storages)
2461-
};
2446+
let bundle_id = self.register_bundle_info::<B>();
24622447

24632448
let mut invalid_entities = Vec::<Entity>::new();
24642449
let mut batch_iter = batch.into_iter();
@@ -2470,7 +2455,7 @@ impl World {
24702455
if let Some((first_entity, first_bundle)) = batch_iter.next() {
24712456
if let Some(first_location) = self.entities().get(first_entity) {
24722457
let mut cache = InserterArchetypeCache {
2473-
// SAFETY: we initialized this bundle_id in `register_info`
2458+
// SAFETY: we initialized this bundle_id in `register_bundle_info`
24742459
inserter: unsafe {
24752460
BundleInserter::new_with_id(
24762461
self,
@@ -3075,18 +3060,34 @@ impl World {
30753060
/// component in the bundle.
30763061
#[inline]
30773062
pub fn register_bundle<B: Bundle>(&mut self) -> &BundleInfo {
3063+
let id = self.register_bundle_info::<B>();
3064+
3065+
// SAFETY: We just initialized the bundle so its id should definitely be valid.
3066+
unsafe { self.bundles.get(id).debug_checked_unwrap() }
3067+
}
3068+
3069+
pub(crate) fn register_bundle_info<B: Bundle>(&mut self) -> BundleId {
30783070
// SAFETY: These come from the same world. `Self.components_registrator` can't be used since we borrow other fields too.
30793071
let mut registrator =
30803072
unsafe { ComponentsRegistrator::new(&mut self.components, &mut self.component_ids) };
30813073

30823074
// SAFETY: `registrator`, `self.storages` and `self.bundles` all come from this world.
3083-
let id = unsafe {
3075+
unsafe {
30843076
self.bundles
30853077
.register_info::<B>(&mut registrator, &mut self.storages)
3086-
};
3078+
}
3079+
}
30873080

3088-
// SAFETY: We just initialized the bundle so its id should definitely be valid.
3089-
unsafe { self.bundles.get(id).debug_checked_unwrap() }
3081+
pub(crate) fn register_contributed_bundle_info<B: Bundle>(&mut self) -> BundleId {
3082+
// SAFETY: These come from the same world. `Self.components_registrator` can't be used since we borrow other fields too.
3083+
let mut registrator =
3084+
unsafe { ComponentsRegistrator::new(&mut self.components, &mut self.component_ids) };
3085+
3086+
// SAFETY: `registrator`, `self.bundles` and `self.storages` are all from this world.
3087+
unsafe {
3088+
self.bundles
3089+
.register_contributed_bundle_info::<B>(&mut registrator, &mut self.storages)
3090+
}
30903091
}
30913092

30923093
/// Registers the given [`ComponentId`]s as a dynamic bundle and returns both the required component ids and the bundle id.

0 commit comments

Comments
 (0)