Skip to content

Commit 8ad7118

Browse files
ElliottjPierceElliottjPierce
authored
Only get valid component ids (#19510)
# Objective - #19504 showed a 11x regression in getting component values for unregistered components. This pr should fix that and improve others a little too. - This is some cleanup work from #18173 . ## Solution - Whenever we expect a component value to exist, we only care about fully registered components, not queued to be registered components since, for the value to exist, it must be registered. - So we can use the faster `get_valid_*` instead of `get_*` in a lot of places. - Also found a bug where `valid_*` did not forward to `get_valid_*` properly. That's fixed. ## Testing CI
1 parent 7ac2ae5 commit 8ad7118

File tree

9 files changed

+70
-45
lines changed

9 files changed

+70
-45
lines changed

‎crates/bevy_ecs/src/component.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2400,7 +2400,7 @@ impl Components {
24002400
/// * [`World::component_id()`]
24012401
#[inline]
24022402
pub fn valid_component_id<T: Component>(&self) -> Option<ComponentId> {
2403-
self.get_id(TypeId::of::<T>())
2403+
self.get_valid_id(TypeId::of::<T>())
24042404
}
24052405

24062406
/// Type-erased equivalent of [`Components::valid_resource_id()`].
@@ -2431,7 +2431,7 @@ impl Components {
24312431
/// * [`Components::get_resource_id()`]
24322432
#[inline]
24332433
pub fn valid_resource_id<T: Resource>(&self) -> Option<ComponentId> {
2434-
self.get_resource_id(TypeId::of::<T>())
2434+
self.get_valid_resource_id(TypeId::of::<T>())
24352435
}
24362436

24372437
/// Type-erased equivalent of [`Components::component_id()`].

‎crates/bevy_ecs/src/entity/clone_entities.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -705,7 +705,7 @@ impl<'w> EntityClonerBuilder<'w> {
705705
/// [`deny_all`](`Self::deny_all`) before calling any of the `allow` methods.
706706
pub fn allow_by_type_ids(&mut self, ids: impl IntoIterator<Item = TypeId>) -> &mut Self {
707707
for type_id in ids {
708-
if let Some(id) = self.world.components().get_id(type_id) {
708+
if let Some(id) = self.world.components().get_valid_id(type_id) {
709709
self.filter_allow(id);
710710
}
711711
}
@@ -740,7 +740,7 @@ impl<'w> EntityClonerBuilder<'w> {
740740
/// Extends the list of components that shouldn't be cloned by type ids.
741741
pub fn deny_by_type_ids(&mut self, ids: impl IntoIterator<Item = TypeId>) -> &mut Self {
742742
for type_id in ids {
743-
if let Some(id) = self.world.components().get_id(type_id) {
743+
if let Some(id) = self.world.components().get_valid_id(type_id) {
744744
self.filter_deny(id);
745745
}
746746
}
@@ -762,7 +762,7 @@ impl<'w> EntityClonerBuilder<'w> {
762762
&mut self,
763763
clone_behavior: ComponentCloneBehavior,
764764
) -> &mut Self {
765-
if let Some(id) = self.world.components().component_id::<T>() {
765+
if let Some(id) = self.world.components().valid_component_id::<T>() {
766766
self.entity_cloner
767767
.clone_behavior_overrides
768768
.insert(id, clone_behavior);
@@ -787,7 +787,7 @@ impl<'w> EntityClonerBuilder<'w> {
787787

788788
/// Removes a previously set override of [`ComponentCloneBehavior`] for a component in this builder.
789789
pub fn remove_clone_behavior_override<T: Component>(&mut self) -> &mut Self {
790-
if let Some(id) = self.world.components().component_id::<T>() {
790+
if let Some(id) = self.world.components().valid_component_id::<T>() {
791791
self.entity_cloner.clone_behavior_overrides.remove(&id);
792792
}
793793
self

‎crates/bevy_ecs/src/world/entity_ref.rs

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3279,7 +3279,11 @@ impl<'w> FilteredEntityRef<'w> {
32793279
/// Returns `None` if the entity does not have a component of type `T`.
32803280
#[inline]
32813281
pub fn get<T: Component>(&self) -> Option<&'w T> {
3282-
let id = self.entity.world().components().get_id(TypeId::of::<T>())?;
3282+
let id = self
3283+
.entity
3284+
.world()
3285+
.components()
3286+
.get_valid_id(TypeId::of::<T>())?;
32833287
self.access
32843288
.has_component_read(id)
32853289
// SAFETY: We have read access
@@ -3293,7 +3297,11 @@ impl<'w> FilteredEntityRef<'w> {
32933297
/// Returns `None` if the entity does not have a component of type `T`.
32943298
#[inline]
32953299
pub fn get_ref<T: Component>(&self) -> Option<Ref<'w, T>> {
3296-
let id = self.entity.world().components().get_id(TypeId::of::<T>())?;
3300+
let id = self
3301+
.entity
3302+
.world()
3303+
.components()
3304+
.get_valid_id(TypeId::of::<T>())?;
32973305
self.access
32983306
.has_component_read(id)
32993307
// SAFETY: We have read access
@@ -3305,7 +3313,11 @@ impl<'w> FilteredEntityRef<'w> {
33053313
/// detection in custom runtimes.
33063314
#[inline]
33073315
pub fn get_change_ticks<T: Component>(&self) -> Option<ComponentTicks> {
3308-
let id = self.entity.world().components().get_id(TypeId::of::<T>())?;
3316+
let id = self
3317+
.entity
3318+
.world()
3319+
.components()
3320+
.get_valid_id(TypeId::of::<T>())?;
33093321
self.access
33103322
.has_component_read(id)
33113323
// SAFETY: We have read access
@@ -3637,7 +3649,11 @@ impl<'w> FilteredEntityMut<'w> {
36373649
/// Returns `None` if the entity does not have a component of type `T`.
36383650
#[inline]
36393651
pub fn get_mut<T: Component<Mutability = Mutable>>(&mut self) -> Option<Mut<'_, T>> {
3640-
let id = self.entity.world().components().get_id(TypeId::of::<T>())?;
3652+
let id = self
3653+
.entity
3654+
.world()
3655+
.components()
3656+
.get_valid_id(TypeId::of::<T>())?;
36413657
self.access
36423658
.has_component_write(id)
36433659
// SAFETY: We have write access
@@ -3665,7 +3681,11 @@ impl<'w> FilteredEntityMut<'w> {
36653681
/// - `T` must be a mutable component
36663682
#[inline]
36673683
pub unsafe fn into_mut_assume_mutable<T: Component>(self) -> Option<Mut<'w, T>> {
3668-
let id = self.entity.world().components().get_id(TypeId::of::<T>())?;
3684+
let id = self
3685+
.entity
3686+
.world()
3687+
.components()
3688+
.get_valid_id(TypeId::of::<T>())?;
36693689
self.access
36703690
.has_component_write(id)
36713691
// SAFETY:
@@ -3895,7 +3915,7 @@ where
38953915
C: Component,
38963916
{
38973917
let components = self.entity.world().components();
3898-
let id = components.component_id::<C>()?;
3918+
let id = components.valid_component_id::<C>()?;
38993919
if bundle_contains_component::<B>(components, id) {
39003920
None
39013921
} else {
@@ -3915,7 +3935,7 @@ where
39153935
C: Component,
39163936
{
39173937
let components = self.entity.world().components();
3918-
let id = components.component_id::<C>()?;
3938+
let id = components.valid_component_id::<C>()?;
39193939
if bundle_contains_component::<B>(components, id) {
39203940
None
39213941
} else {
@@ -3995,7 +4015,11 @@ where
39954015
/// detection in custom runtimes.
39964016
#[inline]
39974017
pub fn get_change_ticks<T: Component>(&self) -> Option<ComponentTicks> {
3998-
let component_id = self.entity.world().components().get_id(TypeId::of::<T>())?;
4018+
let component_id = self
4019+
.entity
4020+
.world()
4021+
.components()
4022+
.get_valid_id(TypeId::of::<T>())?;
39994023
let components = self.entity.world().components();
40004024
(!bundle_contains_component::<B>(components, component_id))
40014025
.then(|| {
@@ -4164,7 +4188,7 @@ where
41644188
C: Component<Mutability = Mutable>,
41654189
{
41664190
let components = self.entity.world().components();
4167-
let id = components.component_id::<C>()?;
4191+
let id = components.valid_component_id::<C>()?;
41684192
if bundle_contains_component::<B>(components, id) {
41694193
None
41704194
} else {
@@ -4747,7 +4771,7 @@ mod tests {
47474771
let entity = world.spawn(TestComponent(42)).id();
47484772
let component_id = world
47494773
.components()
4750-
.get_id(core::any::TypeId::of::<TestComponent>())
4774+
.get_valid_id(core::any::TypeId::of::<TestComponent>())
47514775
.unwrap();
47524776

47534777
let entity = world.entity(entity);
@@ -4764,7 +4788,7 @@ mod tests {
47644788
let entity = world.spawn(TestComponent(42)).id();
47654789
let component_id = world
47664790
.components()
4767-
.get_id(core::any::TypeId::of::<TestComponent>())
4791+
.get_valid_id(core::any::TypeId::of::<TestComponent>())
47684792
.unwrap();
47694793

47704794
let mut entity_mut = world.entity_mut(entity);

‎crates/bevy_ecs/src/world/filtered_resource.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ impl<'w, 's> FilteredResources<'w, 's> {
157157
let component_id = self
158158
.world
159159
.components()
160-
.resource_id::<R>()
160+
.valid_resource_id::<R>()
161161
.ok_or(ResourceFetchError::NotRegistered)?;
162162
if !self.access.has_resource_read(component_id) {
163163
return Err(ResourceFetchError::NoResourceAccess(component_id));
@@ -474,7 +474,7 @@ impl<'w, 's> FilteredResourcesMut<'w, 's> {
474474
let component_id = self
475475
.world
476476
.components()
477-
.resource_id::<R>()
477+
.valid_resource_id::<R>()
478478
.ok_or(ResourceFetchError::NotRegistered)?;
479479
// SAFETY: THe caller ensures that there are no conflicting borrows.
480480
unsafe { self.get_mut_by_id_unchecked(component_id) }

‎crates/bevy_ecs/src/world/mod.rs

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -531,7 +531,7 @@ impl World {
531531

532532
/// Retrieves the [required components](RequiredComponents) for the given component type, if it exists.
533533
pub fn get_required_components<C: Component>(&self) -> Option<&RequiredComponents> {
534-
let id = self.components().component_id::<C>()?;
534+
let id = self.components().valid_component_id::<C>()?;
535535
let component_info = self.components().get_info(id)?;
536536
Some(component_info.required_components())
537537
}
@@ -1623,7 +1623,7 @@ impl World {
16231623
/// since the last call to [`World::clear_trackers`].
16241624
pub fn removed<T: Component>(&self) -> impl Iterator<Item = Entity> + '_ {
16251625
self.components
1626-
.get_id(TypeId::of::<T>())
1626+
.get_valid_id(TypeId::of::<T>())
16271627
.map(|component_id| self.removed_with_id(component_id))
16281628
.into_iter()
16291629
.flatten()
@@ -1772,7 +1772,7 @@ impl World {
17721772
/// Removes the resource of a given type and returns it, if it exists. Otherwise returns `None`.
17731773
#[inline]
17741774
pub fn remove_resource<R: Resource>(&mut self) -> Option<R> {
1775-
let component_id = self.components.get_resource_id(TypeId::of::<R>())?;
1775+
let component_id = self.components.get_valid_resource_id(TypeId::of::<R>())?;
17761776
let (ptr, _, _) = self.storages.resources.get_mut(component_id)?.remove()?;
17771777
// SAFETY: `component_id` was gotten via looking up the `R` type
17781778
unsafe { Some(ptr.read::<R>()) }
@@ -1791,7 +1791,7 @@ impl World {
17911791
/// thread than where the value was inserted from.
17921792
#[inline]
17931793
pub fn remove_non_send_resource<R: 'static>(&mut self) -> Option<R> {
1794-
let component_id = self.components.get_resource_id(TypeId::of::<R>())?;
1794+
let component_id = self.components.get_valid_resource_id(TypeId::of::<R>())?;
17951795
let (ptr, _, _) = self
17961796
.storages
17971797
.non_send_resources
@@ -1805,7 +1805,7 @@ impl World {
18051805
#[inline]
18061806
pub fn contains_resource<R: Resource>(&self) -> bool {
18071807
self.components
1808-
.get_resource_id(TypeId::of::<R>())
1808+
.get_valid_resource_id(TypeId::of::<R>())
18091809
.and_then(|component_id| self.storages.resources.get(component_id))
18101810
.is_some_and(ResourceData::is_present)
18111811
}
@@ -1823,7 +1823,7 @@ impl World {
18231823
#[inline]
18241824
pub fn contains_non_send<R: 'static>(&self) -> bool {
18251825
self.components
1826-
.get_resource_id(TypeId::of::<R>())
1826+
.get_valid_resource_id(TypeId::of::<R>())
18271827
.and_then(|component_id| self.storages.non_send_resources.get(component_id))
18281828
.is_some_and(ResourceData::is_present)
18291829
}
@@ -1846,7 +1846,7 @@ impl World {
18461846
/// was called.
18471847
pub fn is_resource_added<R: Resource>(&self) -> bool {
18481848
self.components
1849-
.get_resource_id(TypeId::of::<R>())
1849+
.get_valid_resource_id(TypeId::of::<R>())
18501850
.is_some_and(|component_id| self.is_resource_added_by_id(component_id))
18511851
}
18521852

@@ -1877,7 +1877,7 @@ impl World {
18771877
/// was called.
18781878
pub fn is_resource_changed<R: Resource>(&self) -> bool {
18791879
self.components
1880-
.get_resource_id(TypeId::of::<R>())
1880+
.get_valid_resource_id(TypeId::of::<R>())
18811881
.is_some_and(|component_id| self.is_resource_changed_by_id(component_id))
18821882
}
18831883

@@ -1902,7 +1902,7 @@ impl World {
19021902
/// Retrieves the change ticks for the given resource.
19031903
pub fn get_resource_change_ticks<R: Resource>(&self) -> Option<ComponentTicks> {
19041904
self.components
1905-
.get_resource_id(TypeId::of::<R>())
1905+
.get_valid_resource_id(TypeId::of::<R>())
19061906
.and_then(|component_id| self.get_resource_change_ticks_by_id(component_id))
19071907
}
19081908

@@ -2558,7 +2558,7 @@ impl World {
25582558
let last_change_tick = self.last_change_tick();
25592559
let change_tick = self.change_tick();
25602560

2561-
let component_id = self.components.get_resource_id(TypeId::of::<R>())?;
2561+
let component_id = self.components.get_valid_resource_id(TypeId::of::<R>())?;
25622562
let (ptr, mut ticks, mut caller) = self
25632563
.storages
25642564
.resources
@@ -3750,7 +3750,7 @@ mod tests {
37503750
world.insert_resource(TestResource(42));
37513751
let component_id = world
37523752
.components()
3753-
.get_resource_id(TypeId::of::<TestResource>())
3753+
.get_valid_resource_id(TypeId::of::<TestResource>())
37543754
.unwrap();
37553755

37563756
let resource = world.get_resource_by_id(component_id).unwrap();
@@ -3766,7 +3766,7 @@ mod tests {
37663766
world.insert_resource(TestResource(42));
37673767
let component_id = world
37683768
.components()
3769-
.get_resource_id(TypeId::of::<TestResource>())
3769+
.get_valid_resource_id(TypeId::of::<TestResource>())
37703770
.unwrap();
37713771

37723772
{

‎crates/bevy_ecs/src/world/reflect.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ impl World {
7070
entity: Entity,
7171
type_id: TypeId,
7272
) -> Result<&dyn Reflect, GetComponentReflectError> {
73-
let Some(component_id) = self.components().get_id(type_id) else {
73+
let Some(component_id) = self.components().get_valid_id(type_id) else {
7474
return Err(GetComponentReflectError::NoCorrespondingComponentId(
7575
type_id,
7676
));
@@ -158,7 +158,7 @@ impl World {
158158
));
159159
};
160160

161-
let Some(component_id) = self.components().get_id(type_id) else {
161+
let Some(component_id) = self.components().get_valid_id(type_id) else {
162162
return Err(GetComponentReflectError::NoCorrespondingComponentId(
163163
type_id,
164164
));

0 commit comments

Comments
 (0)