Skip to content

Commit

Permalink
Replace World::read_change_ticks with World::change_ticks within …
Browse files Browse the repository at this point in the history
…`bevy_ecs` crate (bevyengine#6816)

# Objective

- Fixes bevyengine#6812.

## Solution

- Replaced `World::read_change_ticks` with `World::change_ticks` within `bevy_ecs` crate in places where `World` references were mutable.

---
  • Loading branch information
mnmaita authored and alradish committed Jan 22, 2023
1 parent fff3a6c commit 0eb5d15
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 49 deletions.
58 changes: 17 additions & 41 deletions crates/bevy_ecs/src/query/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,15 +276,9 @@ impl<Q: WorldQuery, F: ReadOnlyWorldQuery> QueryState<Q, F> {
entity: Entity,
) -> Result<Q::Item<'w>, QueryEntityError> {
self.update_archetypes(world);
let change_tick = world.change_tick();
// SAFETY: query has unique world access
unsafe {
self.get_unchecked_manual(
world,
entity,
world.last_change_tick(),
world.read_change_tick(),
)
}
unsafe { self.get_unchecked_manual(world, entity, world.last_change_tick(), change_tick) }
}

/// Returns the query results for the given array of [`Entity`].
Expand Down Expand Up @@ -333,15 +327,11 @@ impl<Q: WorldQuery, F: ReadOnlyWorldQuery> QueryState<Q, F> {
) -> Result<[Q::Item<'w>; N], QueryEntityError> {
self.update_archetypes(world);

let change_tick = world.change_tick();
// SAFETY: method requires exclusive world access
// and world has been validated via update_archetypes
unsafe {
self.get_many_unchecked_manual(
world,
entities,
world.last_change_tick(),
world.read_change_tick(),
)
self.get_many_unchecked_manual(world, entities, world.last_change_tick(), change_tick)
}
}

Expand Down Expand Up @@ -525,10 +515,11 @@ impl<Q: WorldQuery, F: ReadOnlyWorldQuery> QueryState<Q, F> {
/// Returns an [`Iterator`] over the query results for the given [`World`].
#[inline]
pub fn iter_mut<'w, 's>(&'s mut self, world: &'w mut World) -> QueryIter<'w, 's, Q, F> {
let change_tick = world.change_tick();
// SAFETY: query has unique world access
unsafe {
self.update_archetypes(world);
self.iter_unchecked_manual(world, world.last_change_tick(), world.read_change_tick())
self.iter_unchecked_manual(world, world.last_change_tick(), change_tick)
}
}

Expand Down Expand Up @@ -611,14 +602,11 @@ impl<Q: WorldQuery, F: ReadOnlyWorldQuery> QueryState<Q, F> {
&'s mut self,
world: &'w mut World,
) -> QueryCombinationIter<'w, 's, Q, F, K> {
let change_tick = world.change_tick();
// SAFETY: query has unique world access
unsafe {
self.update_archetypes(world);
self.iter_combinations_unchecked_manual(
world,
world.last_change_tick(),
world.read_change_tick(),
)
self.iter_combinations_unchecked_manual(world, world.last_change_tick(), change_tick)
}
}

Expand Down Expand Up @@ -665,14 +653,10 @@ impl<Q: WorldQuery, F: ReadOnlyWorldQuery> QueryState<Q, F> {
EntityList::Item: Borrow<Entity>,
{
self.update_archetypes(world);
let change_tick = world.change_tick();
// SAFETY: Query has unique world access.
unsafe {
self.iter_many_unchecked_manual(
entities,
world,
world.last_change_tick(),
world.read_change_tick(),
)
self.iter_many_unchecked_manual(entities, world, world.last_change_tick(), change_tick)
}
}

Expand Down Expand Up @@ -797,15 +781,11 @@ impl<Q: WorldQuery, F: ReadOnlyWorldQuery> QueryState<Q, F> {
/// `iter_mut()` method, but cannot be chained like a normal [`Iterator`].
#[inline]
pub fn for_each_mut<'w, FN: FnMut(Q::Item<'w>)>(&mut self, world: &'w mut World, func: FN) {
let change_tick = world.change_tick();
// SAFETY: query has unique world access
unsafe {
self.update_archetypes(world);
self.for_each_unchecked_manual(
world,
func,
world.last_change_tick(),
world.read_change_tick(),
);
self.for_each_unchecked_manual(world, func, world.last_change_tick(), change_tick);
}
}

Expand Down Expand Up @@ -873,6 +853,7 @@ impl<Q: WorldQuery, F: ReadOnlyWorldQuery> QueryState<Q, F> {
batch_size: usize,
func: FN,
) {
let change_tick = world.change_tick();
// SAFETY: query has unique world access
unsafe {
self.update_archetypes(world);
Expand All @@ -881,7 +862,7 @@ impl<Q: WorldQuery, F: ReadOnlyWorldQuery> QueryState<Q, F> {
batch_size,
func,
world.last_change_tick(),
world.read_change_tick(),
change_tick,
);
}
}
Expand Down Expand Up @@ -1195,14 +1176,9 @@ impl<Q: WorldQuery, F: ReadOnlyWorldQuery> QueryState<Q, F> {
) -> Result<Q::Item<'w>, QuerySingleError> {
self.update_archetypes(world);

let change_tick = world.change_tick();
// SAFETY: query has unique world access
unsafe {
self.get_single_unchecked_manual(
world,
world.last_change_tick(),
world.read_change_tick(),
)
}
unsafe { self.get_single_unchecked_manual(world, world.last_change_tick(), change_tick) }
}

/// Returns a query result when there is exactly one entity matching the query.
Expand Down Expand Up @@ -1293,7 +1269,7 @@ mod tests {

// These don't matter for the test
let last_change_tick = world.last_change_tick();
let change_tick = world.read_change_tick();
let change_tick = world.change_tick();

// It's best to test get_many_unchecked_manual directly,
// as it is shared and unsafe
Expand Down
7 changes: 2 additions & 5 deletions crates/bevy_ecs/src/world/entity_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -930,15 +930,12 @@ pub(crate) unsafe fn get_mut_by_id(
location: EntityLocation,
component_id: ComponentId,
) -> Option<MutUntyped> {
let change_tick = world.change_tick();
// SAFETY: world access is unique, entity location and component_id required to be valid
get_component_and_ticks(world, component_id, entity, location).map(|(value, ticks)| {
MutUntyped {
value: value.assert_unique(),
ticks: Ticks::from_tick_cells(
ticks,
world.last_change_tick(),
world.read_change_tick(),
),
ticks: Ticks::from_tick_cells(ticks, world.last_change_tick(), change_tick),
}
})
}
Expand Down
6 changes: 3 additions & 3 deletions crates/bevy_ecs/src/world/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1457,14 +1457,14 @@ impl World {
self.validate_non_send_access_untyped(info.name());
}

let change_tick = self.change_tick();

let (ptr, ticks) = self.get_resource_with_ticks(component_id)?;

// SAFETY: This function has exclusive access to the world so nothing aliases `ticks`.
// - index is in-bounds because the column is initialized and non-empty
// - no other reference to the ticks of the same row can exist at the same time
let ticks = unsafe {
Ticks::from_tick_cells(ticks, self.last_change_tick(), self.read_change_tick())
};
let ticks = unsafe { Ticks::from_tick_cells(ticks, self.last_change_tick(), change_tick) };

Some(MutUntyped {
// SAFETY: This function has exclusive access to the world so nothing aliases `ptr`.
Expand Down

0 comments on commit 0eb5d15

Please sign in to comment.