Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Merged by Bors] - Move all logic to UnsafeWorldCell #7381

Closed
wants to merge 2 commits into from
Closed

[Merged by Bors] - Move all logic to UnsafeWorldCell #7381

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
76e77f6
do it all
BoxyUwU Feb 6, 2023
609f38e
use `world_metadata` instead of `unsafe_world`
BoxyUwU Feb 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions crates/bevy_ecs/src/system/system_param.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,7 @@ unsafe impl<'a, T: Resource> SystemParam for Res<'a, T> {
change_tick: u32,
) -> Self::Item<'w, 's> {
let (ptr, ticks) = world
.as_unsafe_world_cell_migration_internal()
.get_resource_with_ticks(component_id)
.unwrap_or_else(|| {
panic!(
Expand Down Expand Up @@ -486,6 +487,7 @@ unsafe impl<'a, T: Resource> SystemParam for Option<Res<'a, T>> {
change_tick: u32,
) -> Self::Item<'w, 's> {
world
.as_unsafe_world_cell_migration_internal()
.get_resource_with_ticks(component_id)
.map(|(ptr, ticks)| Res {
value: ptr.deref(),
Expand Down Expand Up @@ -540,7 +542,7 @@ unsafe impl<'a, T: Resource> SystemParam for ResMut<'a, T> {
) -> Self::Item<'w, 's> {
let value = world
.as_unsafe_world_cell_migration_internal()
.get_resource_mut_with_id(component_id)
.get_resource_mut_by_id(component_id)
.unwrap_or_else(|| {
panic!(
"Resource requested by {} does not exist: {}",
Expand All @@ -549,7 +551,7 @@ unsafe impl<'a, T: Resource> SystemParam for ResMut<'a, T> {
)
});
ResMut {
value: value.value,
value: value.value.deref_mut::<T>(),
ticks: TicksMut {
added: value.ticks.added,
changed: value.ticks.changed,
Expand Down Expand Up @@ -578,9 +580,9 @@ unsafe impl<'a, T: Resource> SystemParam for Option<ResMut<'a, T>> {
) -> Self::Item<'w, 's> {
world
.as_unsafe_world_cell_migration_internal()
.get_resource_mut_with_id(component_id)
.get_resource_mut_by_id(component_id)
.map(|value| ResMut {
value: value.value,
value: value.value.deref_mut::<T>(),
ticks: TicksMut {
added: value.ticks.added,
changed: value.ticks.changed,
Expand Down Expand Up @@ -889,6 +891,7 @@ unsafe impl<'a, T: 'static> SystemParam for NonSend<'a, T> {
change_tick: u32,
) -> Self::Item<'w, 's> {
let (ptr, ticks) = world
.as_unsafe_world_cell_migration_internal()
.get_non_send_with_ticks(component_id)
.unwrap_or_else(|| {
panic!(
Expand Down Expand Up @@ -927,6 +930,7 @@ unsafe impl<T: 'static> SystemParam for Option<NonSend<'_, T>> {
change_tick: u32,
) -> Self::Item<'w, 's> {
world
.as_unsafe_world_cell_migration_internal()
.get_non_send_with_ticks(component_id)
.map(|(ptr, ticks)| NonSend {
value: ptr.deref(),
Expand Down Expand Up @@ -979,6 +983,7 @@ unsafe impl<'a, T: 'static> SystemParam for NonSendMut<'a, T> {
change_tick: u32,
) -> Self::Item<'w, 's> {
let (ptr, ticks) = world
.as_unsafe_world_cell_migration_internal()
.get_non_send_with_ticks(component_id)
.unwrap_or_else(|| {
panic!(
Expand Down Expand Up @@ -1011,6 +1016,7 @@ unsafe impl<'a, T: 'static> SystemParam for Option<NonSendMut<'a, T>> {
change_tick: u32,
) -> Self::Item<'w, 's> {
world
.as_unsafe_world_cell_migration_internal()
.get_non_send_with_ticks(component_id)
.map(|(ptr, ticks)| NonSendMut {
value: ptr.assert_unique().deref_mut(),
Expand Down
113 changes: 17 additions & 96 deletions crates/bevy_ecs/src/world/entity_ref.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
use crate::{
archetype::{Archetype, ArchetypeId, Archetypes},
bundle::{Bundle, BundleInfo},
change_detection::{MutUntyped, TicksMut},
component::{
Component, ComponentId, ComponentStorage, ComponentTicks, Components, StorageType,
},
change_detection::MutUntyped,
component::{Component, ComponentId, ComponentTicks, Components, StorageType},
entity::{Entities, Entity, EntityLocation},
removal_detection::RemovedComponentEvents,
storage::Storages,
Expand Down Expand Up @@ -79,12 +77,14 @@ impl<'w> EntityRef<'w> {

#[inline]
pub fn contains_id(&self, component_id: ComponentId) -> bool {
contains_component_with_id(self.world, component_id, self.location)
self.as_unsafe_world_cell_readonly()
.contains_id(component_id)
}

#[inline]
pub fn contains_type_id(&self, type_id: TypeId) -> bool {
contains_component_with_type(self.world, type_id, self.location)
self.as_unsafe_world_cell_readonly()
.contains_type_id(type_id)
}

#[inline]
Expand Down Expand Up @@ -209,12 +209,14 @@ impl<'w> EntityMut<'w> {

#[inline]
pub fn contains_id(&self, component_id: ComponentId) -> bool {
contains_component_with_id(self.world, component_id, self.location)
self.as_unsafe_world_cell_readonly()
.contains_id(component_id)
}

#[inline]
pub fn contains_type_id(&self, type_id: TypeId) -> bool {
contains_component_with_type(self.world, type_id, self.location)
self.as_unsafe_world_cell_readonly()
.contains_type_id(type_id)
}

#[inline]
Expand Down Expand Up @@ -600,19 +602,10 @@ impl<'w> EntityMut<'w> {
/// which is only valid while the [`EntityMut`] is alive.
#[inline]
pub fn get_by_id(&self, component_id: ComponentId) -> Option<Ptr<'_>> {
let info = self.world.components().get_info(component_id)?;
// SAFETY:
// - entity_location is valid
// - component_id is valid as checked by the line above
// - the storage type is accurate as checked by the fetched ComponentInfo
unsafe {
self.world.get_component(
component_id,
info.storage_type(),
self.entity,
self.location,
)
}
// - `&self` ensures that no mutable references exist to this entity's components.
// - `as_unsafe_world_cell_readonly` gives read only permission for all components on this entity
unsafe { self.as_unsafe_world_cell_readonly().get_by_id(component_id) }
}

/// Gets a [`MutUntyped`] of the component of the given [`ComponentId`] from the entity.
Expand All @@ -625,32 +618,13 @@ impl<'w> EntityMut<'w> {
/// which is only valid while the [`EntityMut`] is alive.
#[inline]
pub fn get_mut_by_id(&mut self, component_id: ComponentId) -> Option<MutUntyped<'_>> {
self.world.components().get_info(component_id)?;
// SAFETY: entity_location is valid, component_id is valid as checked by the line above
unsafe { get_mut_by_id(self.world, self.entity, self.location, component_id) }
}
}

pub(crate) fn contains_component_with_type(
world: &World,
type_id: TypeId,
location: EntityLocation,
) -> bool {
if let Some(component_id) = world.components.get_id(type_id) {
contains_component_with_id(world, component_id, location)
} else {
false
// SAFETY:
// - `&mut self` ensures that no references exist to this entity's components.
// - `as_unsafe_world_cell` gives mutable permission for all components on this entity
unsafe { self.as_unsafe_world_cell().get_mut_by_id(component_id) }
}
}

pub(crate) fn contains_component_with_id(
world: &World,
component_id: ComponentId,
location: EntityLocation,
) -> bool {
world.archetypes[location.archetype_id].contains(component_id)
}

/// Removes a bundle from the given archetype and returns the resulting archetype (or None if the
/// removal was invalid). in the event that adding the given bundle does not result in an Archetype
/// change. Results are cached in the Archetype Graph to avoid redundant work.
Expand Down Expand Up @@ -768,59 +742,6 @@ fn sorted_remove<T: Eq + Ord + Copy>(source: &mut Vec<T>, remove: &[T]) {
});
}

// SAFETY: EntityLocation must be valid
#[inline]
pub(crate) unsafe fn get_mut<T: Component>(
world: &mut World,
entity: Entity,
location: EntityLocation,
) -> Option<Mut<'_, T>> {
let change_tick = world.change_tick();
let last_change_tick = world.last_change_tick();
// SAFETY:
// - world access is unique
// - entity location is valid
// - returned component is of type T
world
.get_component_and_ticks_with_type(
TypeId::of::<T>(),
T::Storage::STORAGE_TYPE,
entity,
location,
)
.map(|(value, ticks)| Mut {
// SAFETY:
// - world access is unique and ties world lifetime to `Mut` lifetime
// - `value` is of type `T`
value: value.assert_unique().deref_mut::<T>(),
ticks: TicksMut::from_tick_cells(ticks, last_change_tick, change_tick),
})
}

// SAFETY: EntityLocation must be valid, component_id must be valid
#[inline]
pub(crate) unsafe fn get_mut_by_id(
world: &mut World,
entity: Entity,
location: EntityLocation,
component_id: ComponentId,
) -> Option<MutUntyped<'_>> {
let change_tick = world.change_tick();
// SAFETY: component_id is valid
let info = world.components.get_info_unchecked(component_id);
// SAFETY:
// - world access is unique
// - entity location is valid
// - returned component is of type T
world
.get_component_and_ticks(component_id, info.storage_type(), entity, location)
.map(|(value, ticks)| MutUntyped {
// SAFETY: world access is unique and ties world lifetime to `MutUntyped` lifetime
value: value.assert_unique(),
ticks: TicksMut::from_tick_cells(ticks, world.last_change_tick(), change_tick),
})
}

/// Moves component data out of storage.
///
/// This function leaves the underlying memory unchanged, but the component behind
Expand Down
Loading