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.

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

Allow disjoint mutable world access via EntityMut #9419

Merged
merged 43 commits into from
Aug 29, 2023
Merged
Show file tree
Hide file tree
Changes from 42 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
54a1047
add EntityBorrow and EntityBorrowMut
JoJoJet Aug 11, 2023
cd5224e
improve documentation for EntityRef/EntityMut
JoJoJet Aug 11, 2023
df2938d
add methods to `EntityBorrow`
JoJoJet Aug 11, 2023
23ca396
rename a conversion function
JoJoJet Aug 11, 2023
62b9abc
add methods to `EntityBorrowMut`
JoJoJet Aug 11, 2023
e61946a
add `EntityMut` -> `EntityBorrow`
JoJoJet Aug 11, 2023
ccd86b8
condense impl blocks
JoJoJet Aug 11, 2023
0ae781a
elaborate documentation
JoJoJet Aug 11, 2023
b77e5ea
implement `WorldQuery` for `EntityBorrow`
JoJoJet Aug 11, 2023
04390f6
implement `WorldQuery` for `EntityBorrowMut`
JoJoJet Aug 11, 2023
5ef6f65
Update access.rs
JoJoJet Aug 11, 2023
82fd7a3
add a test for disjoint accesses
JoJoJet Aug 11, 2023
40c9411
document an open question
JoJoJet Aug 11, 2023
4a16906
add more conversions
JoJoJet Aug 11, 2023
4137fb8
add `EntityBorrowMut::reborrow`
JoJoJet Aug 11, 2023
7cdcc43
add `iter_entities_mut`
JoJoJet Aug 11, 2023
947f2d8
add examples
JoJoJet Aug 11, 2023
e6deca5
add more unit tests
JoJoJet Aug 11, 2023
fc4fc7a
add `get_entities_mut`
JoJoJet Aug 11, 2023
b809cba
add borrow types to the prelude
JoJoJet Aug 11, 2023
48dc15a
add an example to `get_entities_mut`
JoJoJet Aug 11, 2023
5797136
add `many_entities_mut`
JoJoJet Aug 11, 2023
606c7cc
add read-only `get_many` methods
JoJoJet Aug 11, 2023
4ee4ab7
fix doctests
JoJoJet Aug 11, 2023
80b59ca
obey clippy
JoJoJet Aug 11, 2023
45b6dc0
Apply suggestions from code review
JoJoJet Aug 11, 2023
e00ce62
remove unsound WorldQuery impl for EntityRef
JoJoJet Aug 11, 2023
aac9532
update documentation referring to EntityRef
JoJoJet Aug 12, 2023
89323f5
update test names
JoJoJet Aug 12, 2023
f57d93f
improve style for a doc example
JoJoJet Aug 12, 2023
7683507
add a test for iter_entities_mut
JoJoJet Aug 12, 2023
2e01d88
add failing unit tests
JoJoJet Aug 12, 2023
204e652
add incompatible unit tests for EntityBorrow
JoJoJet Aug 12, 2023
3abc6da
remove some branches in methods
JoJoJet Aug 12, 2023
b6b85ed
fix a unit test
JoJoJet Aug 12, 2023
1e7956f
merge `EntityBorrow` into `EntityRef`
JoJoJet Aug 12, 2023
538ffe7
move `EntityBorrowMut` into `entity_ref.rs`
JoJoJet Aug 13, 2023
f0980b9
restore some tests
JoJoJet Aug 13, 2023
d8e51df
reorder `EntityBorrowMut`
JoJoJet Aug 13, 2023
c00d421
rename `EntityMut` -> `EntityWorldMut`
JoJoJet Aug 13, 2023
61bd29a
rename `EntityBorrowMut` to `EntityMut`
JoJoJet Aug 13, 2023
a9c498a
Update some tests
JoJoJet Aug 13, 2023
a8c029b
Merge branch 'main' into entity-borrow
JoJoJet Aug 28, 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
24 changes: 12 additions & 12 deletions crates/bevy_ecs/src/archetype.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,12 +158,12 @@ pub struct Edges {

impl Edges {
/// Checks the cache for the target archetype when adding a bundle to the
/// source archetype. For more information, see [`EntityMut::insert`].
/// source archetype. For more information, see [`EntityWorldMut::insert`].
///
/// If this returns `None`, it means there has not been a transition from
/// the source archetype via the provided bundle.
///
/// [`EntityMut::insert`]: crate::world::EntityMut::insert
/// [`EntityWorldMut::insert`]: crate::world::EntityWorldMut::insert
#[inline]
pub fn get_add_bundle(&self, bundle_id: BundleId) -> Option<ArchetypeId> {
self.get_add_bundle_internal(bundle_id)
Expand All @@ -177,9 +177,9 @@ impl Edges {
}

/// Caches the target archetype when adding a bundle to the source archetype.
/// For more information, see [`EntityMut::insert`].
/// For more information, see [`EntityWorldMut::insert`].
///
/// [`EntityMut::insert`]: crate::world::EntityMut::insert
/// [`EntityWorldMut::insert`]: crate::world::EntityWorldMut::insert
#[inline]
pub(crate) fn insert_add_bundle(
&mut self,
Expand All @@ -197,24 +197,24 @@ impl Edges {
}

/// Checks the cache for the target archetype when removing a bundle to the
/// source archetype. For more information, see [`EntityMut::remove`].
/// source archetype. For more information, see [`EntityWorldMut::remove`].
///
/// If this returns `None`, it means there has not been a transition from
/// the source archetype via the provided bundle.
///
/// If this returns `Some(None)`, it means that the bundle cannot be removed
/// from the source archetype.
///
/// [`EntityMut::remove`]: crate::world::EntityMut::remove
/// [`EntityWorldMut::remove`]: crate::world::EntityWorldMut::remove
#[inline]
pub fn get_remove_bundle(&self, bundle_id: BundleId) -> Option<Option<ArchetypeId>> {
self.remove_bundle.get(bundle_id).cloned()
}

/// Caches the target archetype when removing a bundle to the source archetype.
/// For more information, see [`EntityMut::remove`].
/// For more information, see [`EntityWorldMut::remove`].
///
/// [`EntityMut::remove`]: crate::world::EntityMut::remove
/// [`EntityWorldMut::remove`]: crate::world::EntityWorldMut::remove
#[inline]
pub(crate) fn insert_remove_bundle(
&mut self,
Expand All @@ -225,21 +225,21 @@ impl Edges {
}

/// Checks the cache for the target archetype when removing a bundle to the
/// source archetype. For more information, see [`EntityMut::remove`].
/// source archetype. For more information, see [`EntityWorldMut::remove`].
///
/// If this returns `None`, it means there has not been a transition from
/// the source archetype via the provided bundle.
///
/// [`EntityMut::remove`]: crate::world::EntityMut::remove
/// [`EntityWorldMut::remove`]: crate::world::EntityWorldMut::remove
#[inline]
pub fn get_take_bundle(&self, bundle_id: BundleId) -> Option<Option<ArchetypeId>> {
self.take_bundle.get(bundle_id).cloned()
}

/// Caches the target archetype when removing a bundle to the source archetype.
/// For more information, see [`EntityMut::take`].
/// For more information, see [`EntityWorldMut::take`].
///
/// [`EntityMut::take`]: crate::world::EntityMut::take
/// [`EntityWorldMut::take`]: crate::world::EntityWorldMut::take
#[inline]
pub(crate) fn insert_take_bundle(
&mut self,
Expand Down
14 changes: 7 additions & 7 deletions crates/bevy_ecs/src/entity/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@
//! |Spawn an entity with components|[`Commands::spawn`]|[`World::spawn`]|
//! |Spawn an entity without components|[`Commands::spawn_empty`]|[`World::spawn_empty`]|
//! |Despawn an entity|[`EntityCommands::despawn`]|[`World::despawn`]|
//! |Insert a component, bundle, or tuple of components and bundles to an entity|[`EntityCommands::insert`]|[`EntityMut::insert`]|
//! |Remove a component, bundle, or tuple of components and bundles from an entity|[`EntityCommands::remove`]|[`EntityMut::remove`]|
//! |Insert a component, bundle, or tuple of components and bundles to an entity|[`EntityCommands::insert`]|[`EntityWorldMut::insert`]|
//! |Remove a component, bundle, or tuple of components and bundles from an entity|[`EntityCommands::remove`]|[`EntityWorldMut::remove`]|
//!
//! [`World`]: crate::world::World
//! [`Commands::spawn`]: crate::system::Commands::spawn
Expand All @@ -33,8 +33,8 @@
//! [`World::spawn`]: crate::world::World::spawn
//! [`World::spawn_empty`]: crate::world::World::spawn_empty
//! [`World::despawn`]: crate::world::World::despawn
//! [`EntityMut::insert`]: crate::world::EntityMut::insert
//! [`EntityMut::remove`]: crate::world::EntityMut::remove
//! [`EntityWorldMut::insert`]: crate::world::EntityWorldMut::insert
//! [`EntityWorldMut::remove`]: crate::world::EntityWorldMut::remove
mod map_entities;

pub use map_entities::*;
Expand Down Expand Up @@ -72,7 +72,7 @@ type IdCursor = isize;
/// # Usage
///
/// This data type is returned by iterating a `Query` that has `Entity` as part of its query fetch type parameter ([learn more]).
/// It can also be obtained by calling [`EntityCommands::id`] or [`EntityMut::id`].
/// It can also be obtained by calling [`EntityCommands::id`] or [`EntityWorldMut::id`].
///
/// ```
/// # use bevy_ecs::prelude::*;
Expand All @@ -84,7 +84,7 @@ type IdCursor = isize;
/// }
///
/// fn exclusive_system(world: &mut World) {
/// // Calling `spawn` returns `EntityMut`.
/// // Calling `spawn` returns `EntityWorldMut`.
/// let entity = world.spawn(SomeComponent).id();
/// }
/// #
Expand All @@ -111,7 +111,7 @@ type IdCursor = isize;
///
/// [learn more]: crate::system::Query#entity-id-access
/// [`EntityCommands::id`]: crate::system::EntityCommands::id
/// [`EntityMut::id`]: crate::world::EntityMut::id
/// [`EntityWorldMut::id`]: crate::world::EntityWorldMut::id
/// [`EntityCommands`]: crate::system::EntityCommands
/// [`Query::get`]: crate::system::Query::get
/// [`World`]: crate::world::World
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ pub mod prelude {
Commands, Deferred, In, IntoSystem, Local, NonSend, NonSendMut, ParallelCommands,
ParamSet, Query, ReadOnlySystem, Res, ResMut, Resource, System, SystemParamFunction,
},
world::{EntityRef, FromWorld, World},
world::{EntityMut, EntityRef, FromWorld, World},
};
}

Expand Down
58 changes: 52 additions & 6 deletions crates/bevy_ecs/src/query/access.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,12 @@ pub struct Access<T: SparseSetIndex> {
reads_and_writes: FixedBitSet,
/// The exclusively-accessed elements.
writes: FixedBitSet,
/// Is `true` if this has access to all elements in the collection?
/// Is `true` if this has access to all elements in the collection.
/// This field is a performance optimization for `&World` (also harder to mess up for soundness).
reads_all: bool,
/// Is `true` if this has mutable access to all elements in the collection.
/// If this is true, then `reads_all` must also be true.
writes_all: bool,
marker: PhantomData<T>,
}

Expand All @@ -68,6 +71,7 @@ impl<T: SparseSetIndex + fmt::Debug> fmt::Debug for Access<T> {
)
.field("writes", &FormattedBitSet::<T>::new(&self.writes))
.field("reads_all", &self.reads_all)
.field("writes_all", &self.writes_all)
.finish()
}
}
Expand All @@ -83,6 +87,7 @@ impl<T: SparseSetIndex> Access<T> {
pub const fn new() -> Self {
Self {
reads_all: false,
writes_all: false,
reads_and_writes: FixedBitSet::new(),
writes: FixedBitSet::new(),
marker: PhantomData,
Expand Down Expand Up @@ -116,36 +121,54 @@ impl<T: SparseSetIndex> Access<T> {
self.reads_all || self.reads_and_writes.contains(index.sparse_set_index())
}

/// Returns `true` if this can access anything.
pub fn has_any_read(&self) -> bool {
self.reads_all || !self.reads_and_writes.is_clear()
}

/// Returns `true` if this can exclusively access the element given by `index`.
pub fn has_write(&self, index: T) -> bool {
self.writes.contains(index.sparse_set_index())
self.writes_all || self.writes.contains(index.sparse_set_index())
}

/// Returns `true` if this accesses anything mutably.
pub fn has_any_write(&self) -> bool {
!self.writes.is_clear()
self.writes_all || !self.writes.is_clear()
}

/// Sets this as having access to all indexed elements (i.e. `&World`).
pub fn read_all(&mut self) {
self.reads_all = true;
}

/// Sets this as having mutable access to all indexed elements (i.e. `EntityMut`).
pub fn write_all(&mut self) {
self.reads_all = true;
self.writes_all = true;
}

/// Returns `true` if this has access to all indexed elements (i.e. `&World`).
pub fn has_read_all(&self) -> bool {
self.reads_all
}

/// Returns `true` if this has write access to all indexed elements (i.e. `EntityMut`).
pub fn has_write_all(&self) -> bool {
self.writes_all
}

/// Removes all accesses.
pub fn clear(&mut self) {
self.reads_all = false;
self.writes_all = false;
self.reads_and_writes.clear();
self.writes.clear();
}

/// Adds all access from `other`.
pub fn extend(&mut self, other: &Access<T>) {
self.reads_all = self.reads_all || other.reads_all;
self.writes_all = self.writes_all || other.writes_all;
self.reads_and_writes.union_with(&other.reads_and_writes);
self.writes.union_with(&other.writes);
}
Expand All @@ -155,13 +178,20 @@ impl<T: SparseSetIndex> Access<T> {
/// [`Access`] instances are incompatible if one can write
/// an element that the other can read or write.
pub fn is_compatible(&self, other: &Access<T>) -> bool {
// Only systems that do not write data are compatible with systems that operate on `&World`.
if self.writes_all {
return !other.has_any_read();
}

if other.writes_all {
return !self.has_any_read();
}

if self.reads_all {
return other.writes.count_ones(..) == 0;
return !other.has_any_write();
}

if other.reads_all {
return self.writes.count_ones(..) == 0;
return !self.has_any_write();
}

self.writes.is_disjoint(&other.reads_and_writes)
Expand All @@ -172,12 +202,23 @@ impl<T: SparseSetIndex> Access<T> {
pub fn get_conflicts(&self, other: &Access<T>) -> Vec<T> {
let mut conflicts = FixedBitSet::default();
if self.reads_all {
// QUESTION: How to handle `other.writes_all`?
conflicts.extend(other.writes.ones());
}

if other.reads_all {
// QUESTION: How to handle `self.writes_all`.
conflicts.extend(self.writes.ones());
}

if self.writes_all {
conflicts.extend(other.reads_and_writes.ones());
}

if other.writes_all {
conflicts.extend(self.reads_and_writes.ones());
}

conflicts.extend(self.writes.intersection(&other.reads_and_writes));
conflicts.extend(self.reads_and_writes.intersection(&other.writes));
conflicts
Expand Down Expand Up @@ -377,6 +418,11 @@ impl<T: SparseSetIndex> FilteredAccess<T> {
pub fn read_all(&mut self) {
self.access.read_all();
}

/// Sets the underlying unfiltered access as having mutable access to all indexed elements.
pub fn write_all(&mut self) {
self.access.write_all();
}
}

#[derive(Clone, Eq, PartialEq)]
Expand Down
Loading
Loading