From 70ad6671dba9e7dc93e8a085ad8bb73904ca1fa5 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Thu, 17 Sep 2020 17:16:38 -0700 Subject: [PATCH] ecs: use generational entity ids and other optimizations (#504) ecs: use generational entity ids and other optimizations --- assets/scenes/load_scene_example.scn | 38 +- crates/bevy_app/src/app.rs | 6 +- crates/bevy_ecs/hecs/benches/bench.rs | 2 +- crates/bevy_ecs/hecs/examples/format.rs | 39 -- crates/bevy_ecs/hecs/src/archetype.rs | 172 +++---- crates/bevy_ecs/hecs/src/borrow.rs | 53 +-- crates/bevy_ecs/hecs/src/entities.rs | 371 +++++++++++++-- crates/bevy_ecs/hecs/src/lib.rs | 14 +- crates/bevy_ecs/hecs/src/query.rs | 156 ++++--- crates/bevy_ecs/hecs/src/query_one.rs | 98 +++- crates/bevy_ecs/hecs/src/serde.rs | 2 +- crates/bevy_ecs/hecs/src/world.rs | 438 +++++++++++++----- crates/bevy_ecs/hecs/tests/tests.rs | 41 +- .../bevy_ecs/src/resource/resource_query.rs | 5 +- crates/bevy_ecs/src/resource/resources.rs | 23 +- .../src/schedule/parallel_executor.rs | 40 +- crates/bevy_ecs/src/schedule/schedule.rs | 16 +- crates/bevy_ecs/src/system/commands.rs | 53 +-- crates/bevy_ecs/src/system/into_system.rs | 28 +- crates/bevy_ecs/src/system/query.rs | 187 ++++++-- crates/bevy_ecs/src/system/system.rs | 6 +- crates/bevy_ecs/src/world/world_builder.rs | 8 +- .../impl_property/impl_property_bevy_ecs.rs | 4 +- crates/bevy_render/src/draw.rs | 20 +- .../src/render_graph/nodes/pass_node.rs | 9 +- crates/bevy_scene/src/scene.rs | 4 +- crates/bevy_scene/src/scene_spawner.rs | 120 ++--- crates/bevy_scene/src/serde.rs | 2 +- .../bevy_transform/src/components/parent.rs | 2 +- .../src/hierarchy/child_builder.rs | 15 +- .../bevy_transform/src/hierarchy/hierarchy.rs | 5 +- .../hierarchy/hierarchy_maintenance_system.rs | 1 + .../src/hierarchy/world_child_builder.rs | 11 +- .../src/transform_propagate_system.rs | 1 + examples/scene/scene.rs | 18 +- 35 files changed, 1232 insertions(+), 776 deletions(-) delete mode 100644 crates/bevy_ecs/hecs/examples/format.rs diff --git a/assets/scenes/load_scene_example.scn b/assets/scenes/load_scene_example.scn index 6569df5c1b..a682621972 100644 --- a/assets/scenes/load_scene_example.scn +++ b/assets/scenes/load_scene_example.scn @@ -1,6 +1,24 @@ [ ( - entity: 328997855, + entity: 0, + components: [ + { + "type": "ComponentB", + "map": { + "value": "hello", + }, + }, + { + "type": "ComponentA", + "map": { + "x": 1.0, + "y": 2.0, + }, + }, + ], + ), + ( + entity: 1, components: [ { "type": "ComponentA", @@ -11,22 +29,4 @@ }, ], ), - ( - entity: 404566393, - components: [ - { - "type": "ComponentA", - "map": { - "x": 1.0, - "y": 2.0, - }, - }, - { - "type": "ComponentB", - "map": { - "value": "hello", - }, - }, - ], - ), ] \ No newline at end of file diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index c62cdea8e7..76798d0680 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -57,7 +57,8 @@ impl App { } pub fn update(&mut self) { - self.schedule.initialize(&mut self.resources); + self.schedule + .initialize(&mut self.world, &mut self.resources); self.executor .run(&mut self.schedule, &mut self.world, &mut self.resources); } @@ -69,7 +70,8 @@ impl App { .unwrap_or_else(DefaultTaskPoolOptions::default) .create_default_pools(&mut self.resources); - self.startup_schedule.initialize(&mut self.resources); + self.startup_schedule + .initialize(&mut self.world, &mut self.resources); self.startup_executor.run( &mut self.startup_schedule, &mut self.world, diff --git a/crates/bevy_ecs/hecs/benches/bench.rs b/crates/bevy_ecs/hecs/benches/bench.rs index eb6220addd..90b9634f1e 100644 --- a/crates/bevy_ecs/hecs/benches/bench.rs +++ b/crates/bevy_ecs/hecs/benches/bench.rs @@ -68,7 +68,7 @@ fn iterate_100k(b: &mut Bencher) { world.spawn((Position(-(i as f32)), Velocity(i as f32))); } b.iter(|| { - for (mut pos, vel) in &mut world.query::<(&mut Position, &Velocity)>() { + for (mut pos, vel) in &mut world.query_mut::<(&mut Position, &Velocity)>() { pos.0 += vel.0; } }) diff --git a/crates/bevy_ecs/hecs/examples/format.rs b/crates/bevy_ecs/hecs/examples/format.rs deleted file mode 100644 index 942c4eaa98..0000000000 --- a/crates/bevy_ecs/hecs/examples/format.rs +++ /dev/null @@ -1,39 +0,0 @@ -// modified by Bevy contributors - -//! One way to the contents of an entity, as you might do for debugging. A similar pattern could -//! also be useful for serialization, or other row-oriented generic operations. - -fn format_entity(entity: bevy_hecs::EntityRef<'_>) -> String { - fn fmt( - entity: bevy_hecs::EntityRef<'_>, - ) -> Option { - Some(entity.get::()?.to_string()) - } - - const FUNCTIONS: &[&dyn Fn(bevy_hecs::EntityRef<'_>) -> Option] = - &[&fmt::, &fmt::, &fmt::]; - - let mut out = String::new(); - for f in FUNCTIONS { - if let Some(x) = f(entity) { - if out.is_empty() { - out.push_str("["); - } else { - out.push_str(", "); - } - out.push_str(&x); - } - } - if out.is_empty() { - out.push_str(&"[]"); - } else { - out.push(']'); - } - out -} - -fn main() { - let mut world = bevy_hecs::World::new(); - let e = world.spawn((42, true)); - println!("{}", format_entity(world.entity(e).unwrap())); -} diff --git a/crates/bevy_ecs/hecs/src/archetype.rs b/crates/bevy_ecs/hecs/src/archetype.rs index fcf582f36f..2bc2e66962 100644 --- a/crates/bevy_ecs/hecs/src/archetype.rs +++ b/crates/bevy_ecs/hecs/src/archetype.rs @@ -14,11 +14,12 @@ // modified by Bevy contributors -use crate::alloc::{ - alloc::{alloc, dealloc, Layout}, - boxed::Box, - vec, - vec::Vec, +use crate::{ + alloc::{ + alloc::{alloc, dealloc, Layout}, + vec::Vec, + }, + Entity, }; use bevy_utils::{HashMap, HashMapExt}; use core::{ @@ -37,13 +38,13 @@ use crate::{borrow::AtomicBorrow, query::Fetch, Access, Component, Query}; pub struct Archetype { types: Vec, state: HashMap, - len: u32, - entities: Box<[u128]>, + len: usize, + entities: Vec, // UnsafeCell allows unique references into `data` to be constructed while shared references // containing the `Archetype` exist data: UnsafeCell>, data_size: usize, - grow_size: u32, + grow_size: usize, } impl Archetype { @@ -53,7 +54,7 @@ impl Archetype { } #[allow(missing_docs)] - pub fn with_grow(types: Vec, grow_size: u32) -> Self { + pub fn with_grow(types: Vec, grow_size: usize) -> Self { debug_assert!( types.windows(2).all(|x| x[0] < x[1]), "type info unsorted or contains duplicates" @@ -65,7 +66,7 @@ impl Archetype { Self { state, types, - entities: Box::new([]), + entities: Vec::new(), len: 0, data: UnsafeCell::new(NonNull::dangling()), data_size: 0, @@ -94,6 +95,12 @@ impl Archetype { self.has_dynamic(TypeId::of::()) } + #[allow(missing_docs)] + #[inline] + pub fn has_type(&self, ty: TypeId) -> bool { + self.has_dynamic(ty) + } + pub(crate) fn has_dynamic(&self, id: TypeId) -> bool { self.state.contains_key(&id) } @@ -111,61 +118,21 @@ impl Archetype { #[allow(missing_docs)] #[inline] - pub fn get_with_added(&self) -> Option<(NonNull, NonNull)> { + pub fn get_with_type_state(&self) -> Option<(NonNull, &TypeState)> { let state = self.state.get(&TypeId::of::())?; Some(unsafe { ( NonNull::new_unchecked( (*self.data.get()).as_ptr().add(state.offset).cast::() as *mut T ), - NonNull::new_unchecked(state.added_entities.as_ptr() as *mut bool), + state, ) }) } #[allow(missing_docs)] - #[inline] - pub fn get_with_mutated(&self) -> Option<(NonNull, NonNull)> { - let state = self.state.get(&TypeId::of::())?; - Some(unsafe { - ( - NonNull::new_unchecked( - (*self.data.get()).as_ptr().add(state.offset).cast::() as *mut T - ), - NonNull::new_unchecked(state.mutated_entities.as_ptr() as *mut bool), - ) - }) - } - - #[allow(missing_docs)] - #[inline] - pub fn get_with_added_and_mutated( - &self, - ) -> Option<(NonNull, NonNull, NonNull)> { - let state = self.state.get(&TypeId::of::())?; - Some(unsafe { - ( - NonNull::new_unchecked( - (*self.data.get()).as_ptr().add(state.offset).cast::() as *mut T - ), - NonNull::new_unchecked(state.added_entities.as_ptr() as *mut bool), - NonNull::new_unchecked(state.mutated_entities.as_ptr() as *mut bool), - ) - }) - } - - #[allow(missing_docs)] - #[inline] - pub fn get_mutated(&self) -> Option> { - let state = self.state.get(&TypeId::of::())?; - Some(unsafe { NonNull::new_unchecked(state.mutated_entities.as_ptr() as *mut bool) }) - } - - #[allow(missing_docs)] - #[inline] - pub fn get_added(&self) -> Option> { - let state = self.state.get(&TypeId::of::())?; - Some(unsafe { NonNull::new_unchecked(state.added_entities.as_ptr() as *mut bool) }) + pub fn get_type_state(&self, ty: TypeId) -> Option<&TypeState> { + self.state.get(&ty) } #[allow(missing_docs)] @@ -215,7 +182,7 @@ impl Archetype { #[allow(missing_docs)] #[inline] - pub fn len(&self) -> u32 { + pub fn len(&self) -> usize { self.len } @@ -226,17 +193,17 @@ impl Archetype { } #[allow(missing_docs)] - pub fn iter_entities(&self) -> impl Iterator { - self.entities.iter().take(self.len as usize) + pub fn iter_entities(&self) -> impl Iterator { + self.entities.iter().take(self.len) } #[inline] - pub(crate) fn entities(&self) -> NonNull { + pub(crate) fn entities(&self) -> NonNull { unsafe { NonNull::new_unchecked(self.entities.as_ptr() as *mut _) } } - pub(crate) fn entity_id(&self, index: u32) -> u128 { - self.entities[index as usize] + pub(crate) fn get_entity(&self, index: usize) -> Entity { + self.entities[index] } #[allow(missing_docs)] @@ -250,37 +217,37 @@ impl Archetype { &self, ty: TypeId, size: usize, - index: u32, + index: usize, ) -> Option> { debug_assert!(index < self.len); Some(NonNull::new_unchecked( (*self.data.get()) .as_ptr() - .add(self.state.get(&ty)?.offset + size * index as usize) + .add(self.state.get(&ty)?.offset + size * index) .cast::(), )) } /// # Safety /// Every type must be written immediately after this call - pub unsafe fn allocate(&mut self, id: u128) -> u32 { - if self.len as usize == self.entities.len() { + pub unsafe fn allocate(&mut self, id: Entity) -> usize { + if self.len == self.entities.len() { self.grow(self.len.max(self.grow_size)); } - self.entities[self.len as usize] = id; + self.entities[self.len] = id; self.len += 1; self.len - 1 } - pub(crate) fn reserve(&mut self, additional: u32) { + pub(crate) fn reserve(&mut self, additional: usize) { if additional > (self.capacity() - self.len()) { self.grow(additional - (self.capacity() - self.len())); } } - fn capacity(&self) -> u32 { - self.entities.len() as u32 + fn capacity(&self) -> usize { + self.entities.len() } #[allow(missing_docs)] @@ -290,13 +257,17 @@ impl Archetype { } } - fn grow(&mut self, increment: u32) { + fn grow(&mut self, increment: usize) { unsafe { - let old_count = self.len as usize; - let count = old_count + increment as usize; - let mut new_entities = vec![!0; count].into_boxed_slice(); - new_entities[0..old_count].copy_from_slice(&self.entities[0..old_count]); - self.entities = new_entities; + let old_count = self.len; + let count = old_count + increment; + self.entities.resize( + self.entities.len() + increment, + Entity { + id: u32::MAX, + generation: u32::MAX, + }, + ); for type_state in self.state.values_mut() { type_state.mutated_entities.resize_with(count, || false); @@ -341,7 +312,7 @@ impl Archetype { } /// Returns the ID of the entity moved into `index`, if any - pub(crate) unsafe fn remove(&mut self, index: u32) -> Option { + pub(crate) unsafe fn remove(&mut self, index: usize) -> Option { let last = self.len - 1; for ty in &self.types { let removed = self @@ -360,16 +331,14 @@ impl Archetype { ); let type_state = self.state.get_mut(&ty.id).unwrap(); - type_state.mutated_entities[index as usize] = - type_state.mutated_entities[last as usize]; - type_state.added_entities[index as usize] = - type_state.added_entities[last as usize]; + type_state.mutated_entities[index] = type_state.mutated_entities[last]; + type_state.added_entities[index] = type_state.added_entities[last]; } } self.len = last; if index != last { - self.entities[index as usize] = self.entities[last as usize]; - Some(self.entities[last as usize]) + self.entities[index] = self.entities[last]; + Some(self.entities[last]) } else { None } @@ -378,9 +347,9 @@ impl Archetype { /// Returns the ID of the entity moved into `index`, if any pub(crate) unsafe fn move_to( &mut self, - index: u32, + index: usize, mut f: impl FnMut(*mut u8, TypeId, usize, bool, bool), - ) -> Option { + ) -> Option { let last = self.len - 1; for ty in &self.types { let moved = self @@ -388,8 +357,8 @@ impl Archetype { .unwrap() .as_ptr(); let type_state = self.state.get(&ty.id).unwrap(); - let is_added = type_state.added_entities[index as usize]; - let is_mutated = type_state.mutated_entities[index as usize]; + let is_added = type_state.added_entities[index]; + let is_mutated = type_state.mutated_entities[index]; f(moved, ty.id(), ty.layout().size(), is_added, is_mutated); if index != last { ptr::copy_nonoverlapping( @@ -400,16 +369,14 @@ impl Archetype { ty.layout.size(), ); let type_state = self.state.get_mut(&ty.id).unwrap(); - type_state.added_entities[index as usize] = - type_state.added_entities[last as usize]; - type_state.mutated_entities[index as usize] = - type_state.mutated_entities[last as usize]; + type_state.added_entities[index] = type_state.added_entities[last]; + type_state.mutated_entities[index] = type_state.mutated_entities[last]; } } self.len -= 1; if index != last { - self.entities[index as usize] = self.entities[last as usize]; - Some(self.entities[last as usize]) + self.entities[index] = self.entities[last]; + Some(self.entities[last]) } else { None } @@ -427,16 +394,16 @@ impl Archetype { component: *mut u8, ty: TypeId, size: usize, - index: u32, + index: usize, added: bool, ) { let state = self.state.get_mut(&ty).unwrap(); if added { - state.added_entities[index as usize] = true; + state.added_entities[index] = true; } let ptr = (*self.data.get()) .as_ptr() - .add(state.offset + size * index as usize) + .add(state.offset + size * index) .cast::(); ptr::copy_nonoverlapping(component, ptr, size); } @@ -464,11 +431,12 @@ impl Drop for Archetype { } } +/// Metadata about a type stored in an archetype pub struct TypeState { offset: usize, borrow: AtomicBorrow, - pub mutated_entities: Vec, - pub added_entities: Vec, + mutated_entities: Vec, + added_entities: Vec, } impl TypeState { @@ -490,6 +458,18 @@ impl TypeState { *added = false; } } + + #[allow(missing_docs)] + #[inline] + pub fn mutated(&self) -> NonNull { + unsafe { NonNull::new_unchecked(self.mutated_entities.as_ptr() as *mut bool) } + } + + #[allow(missing_docs)] + #[inline] + pub fn added(&self) -> NonNull { + unsafe { NonNull::new_unchecked(self.added_entities.as_ptr() as *mut bool) } + } } /// Metadata required to store a component diff --git a/crates/bevy_ecs/hecs/src/borrow.rs b/crates/bevy_ecs/hecs/src/borrow.rs index fdd1df86ab..8d1adad892 100644 --- a/crates/bevy_ecs/hecs/src/borrow.rs +++ b/crates/bevy_ecs/hecs/src/borrow.rs @@ -17,7 +17,6 @@ use core::{ fmt::Debug, ops::{Deref, DerefMut}, - ptr::NonNull, sync::atomic::{AtomicUsize, Ordering}, }; @@ -68,7 +67,7 @@ const UNIQUE_BIT: usize = !(usize::max_value() >> 1); #[derive(Clone)] pub struct Ref<'a, T: Component> { archetype: &'a Archetype, - target: NonNull, + target: &'a T, } impl<'a, T: Component> Ref<'a, T> { @@ -77,16 +76,15 @@ impl<'a, T: Component> Ref<'a, T> { /// # Safety /// /// - the index of the component must be valid - pub unsafe fn new(archetype: &'a Archetype, index: u32) -> Result { - let target = NonNull::new_unchecked( - archetype - .get::() - .ok_or_else(MissingComponent::new::)? - .as_ptr() - .add(index as usize), - ); + pub unsafe fn new(archetype: &'a Archetype, index: usize) -> Result { + let target = archetype + .get::() + .ok_or_else(MissingComponent::new::)?; archetype.borrow::(); - Ok(Self { archetype, target }) + Ok(Self { + archetype, + target: &*target.as_ptr().add(index as usize), + }) } } @@ -103,7 +101,7 @@ impl<'a, T: Component> Deref for Ref<'a, T> { type Target = T; fn deref(&self) -> &T { - unsafe { self.target.as_ref() } + self.target } } @@ -119,7 +117,7 @@ where /// Unique borrow of an entity's component pub struct RefMut<'a, T: Component> { archetype: &'a Archetype, - target: NonNull, + target: &'a mut T, modified: &'a mut bool, } @@ -129,24 +127,15 @@ impl<'a, T: Component> RefMut<'a, T> { /// # Safety /// /// - the index of the component must be valid - pub unsafe fn new(archetype: &'a Archetype, index: u32) -> Result { - let target = NonNull::new_unchecked( - archetype - .get::() - .ok_or_else(MissingComponent::new::)? - .as_ptr() - .add(index as usize), - ); + pub unsafe fn new(archetype: &'a Archetype, index: usize) -> Result { + let (target, type_state) = archetype + .get_with_type_state::() + .ok_or_else(MissingComponent::new::)?; archetype.borrow_mut::(); - let modified = archetype - .get_mutated::() - .unwrap() - .as_ptr() - .add(index as usize); Ok(Self { archetype, - target, - modified: &mut *modified, + target: &mut *target.as_ptr().add(index), + modified: &mut *type_state.mutated().as_ptr().add(index), }) } } @@ -164,14 +153,14 @@ impl<'a, T: Component> Deref for RefMut<'a, T> { type Target = T; fn deref(&self) -> &T { - unsafe { self.target.as_ref() } + self.target } } impl<'a, T: Component> DerefMut for RefMut<'a, T> { fn deref_mut(&mut self) -> &mut T { *self.modified = true; - unsafe { self.target.as_mut() } + self.target } } @@ -188,7 +177,7 @@ where #[derive(Copy, Clone)] pub struct EntityRef<'a> { archetype: Option<&'a Archetype>, - index: u32, + index: usize, } impl<'a> EntityRef<'a> { @@ -200,7 +189,7 @@ impl<'a> EntityRef<'a> { } } - pub(crate) unsafe fn new(archetype: &'a Archetype, index: u32) -> Self { + pub(crate) unsafe fn new(archetype: &'a Archetype, index: usize) -> Self { Self { archetype: Some(archetype), index, diff --git a/crates/bevy_ecs/hecs/src/entities.rs b/crates/bevy_ecs/hecs/src/entities.rs index 6affb1c864..bf26575dbd 100644 --- a/crates/bevy_ecs/hecs/src/entities.rs +++ b/crates/bevy_ecs/hecs/src/entities.rs @@ -1,27 +1,45 @@ -// modified by Bevy contributors - -use bevy_utils::HashMap; -use core::fmt; +use alloc::{boxed::Box, vec::Vec}; +use core::{ + convert::TryFrom, + fmt, mem, + sync::atomic::{AtomicU32, Ordering}, +}; #[cfg(feature = "std")] use std::error::Error; /// Lightweight unique ID of an entity /// /// Obtained from `World::spawn`. Can be stored to refer to an entity in the future. -#[derive(Debug, Clone, Copy, Hash, Eq, Ord, PartialEq, PartialOrd)] -pub struct Entity(u128); +#[derive(Clone, Copy, Hash, Eq, Ord, PartialEq, PartialOrd)] +pub struct Entity { + pub(crate) generation: u32, + pub(crate) id: u32, +} -#[allow(clippy::new_without_default)] impl Entity { - #[allow(missing_docs)] - pub fn new() -> Self { - Self(rand::random::()) + /// Creates a new entity reference with a generation of 0 + pub fn new(id: u32) -> Entity { + Entity { id, generation: 0 } } - #[allow(missing_docs)] - #[inline] - pub fn from_id(id: u128) -> Self { - Self(id) + /// Convert to a form convenient for passing outside of rust + /// + /// Only useful for identifying entities within the same instance of an application. Do not use + /// for serialization between runs. + /// + /// No particular structure is guaranteed for the returned bits. + pub fn to_bits(self) -> u64 { + u64::from(self.generation) << 32 | u64::from(self.id) + } + + /// Reconstruct an `Entity` previously destructured with `to_bits` + /// + /// Only useful when applied to results from `to_bits` in the same instance of an application. + pub fn from_bits(bits: u64) -> Self { + Self { + generation: (bits >> 32) as u32, + id: bits as u32, + } } /// Extract a transiently unique identifier @@ -29,65 +47,306 @@ impl Entity { /// No two simultaneously-live entities share the same ID, but dead entities' IDs may collide /// with both live and dead entities. Useful for compactly representing entities within a /// specific snapshot of the world, such as when serializing. - #[inline] - pub fn id(self) -> u128 { - self.0 + pub fn id(self) -> u32 { + self.id + } +} + +impl fmt::Debug for Entity { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}v{}", self.id, self.generation) } } #[derive(Default)] pub(crate) struct Entities { - pub entity_locations: HashMap, + pub meta: Vec, + // Reserved entities outside the range of `meta`, having implicit generation 0, archetype 0, and + // undefined index. Calling `flush` converts these to real entities, which can have a fully + // defined location. + pending: AtomicU32, + // Unused entity IDs below `meta.len()` + free: Vec, + free_cursor: AtomicU32, + // Reserved IDs within `meta.len()` with implicit archetype 0 and undefined index. Should be + // consumed and used to initialize locations to produce real entities after calling `flush`. + reserved: Box<[AtomicU32]>, + reserved_cursor: AtomicU32, } impl Entities { + /// Reserve an entity ID concurrently + /// + /// Storage for entity generation and location is lazily allocated by calling `flush`. Locations + /// can be determined by the return value of `flush` and by iterating through the `reserved` + /// accessors, and should all be written immediately after flushing. + pub fn reserve_entity(&self) -> Entity { + loop { + let index = self.free_cursor.load(Ordering::Relaxed); + match index.checked_sub(1) { + // The freelist is empty, so increment `pending` to arrange for a new entity with a + // predictable ID to be allocated on the next `flush` call + None => { + let n = self.pending.fetch_add(1, Ordering::Relaxed); + return Entity { + generation: 0, + id: u32::try_from(self.meta.len()) + .ok() + .and_then(|x| x.checked_add(n)) + .expect("too many entities"), + }; + } + // The freelist has entities in it, so move the last entry to the reserved list, to + // be consumed by the caller as part of a higher-level flush. + Some(next) => { + // We don't care about memory ordering here so long as we get our slot. + if self + .free_cursor + .compare_exchange_weak(index, next, Ordering::Relaxed, Ordering::Relaxed) + .is_err() + { + // Another thread already consumed this slot, start over. + continue; + } + let id = self.free[next as usize]; + let reservation = self.reserved_cursor.fetch_add(1, Ordering::Relaxed); + self.reserved[reservation as usize].store(id, Ordering::Relaxed); + return Entity { + generation: self.meta[id as usize].generation, + id, + }; + } + } + } + } + + /// Allocate an entity ID directly + /// + /// Location should be written immediately. + pub fn alloc(&mut self) -> Entity { + debug_assert_eq!( + self.pending.load(Ordering::Relaxed), + 0, + "allocator must be flushed before potentially growing" + ); + let index = self.free_cursor.load(Ordering::Relaxed); + match index.checked_sub(1) { + None => { + self.grow(0); + let cursor = self.free_cursor.fetch_sub(1, Ordering::Relaxed); + let id = self.free[(cursor - 1) as usize]; + Entity { + generation: self.meta[id as usize].generation, + id, + } + } + Some(next) => { + // Not racey due to &mut self + self.free_cursor.store(next, Ordering::Relaxed); + let id = self.free[next as usize]; + Entity { + generation: self.meta[id as usize].generation, + id, + } + } + } + } + /// Destroy an entity, allowing it to be reused /// /// Must not be called on reserved entities prior to `flush`. pub fn free(&mut self, entity: Entity) -> Result { - if let Some(location) = self.entity_locations.remove(&entity) { - Ok(location) + let meta = &mut self.meta[entity.id as usize]; + if meta.generation != entity.generation { + return Err(NoSuchEntity); + } + meta.generation += 1; + let loc = mem::replace( + &mut meta.location, + Location { + archetype: 0, + // Guard against bugs in reservation handling + index: usize::max_value(), + }, + ); + let index = self.free_cursor.fetch_add(1, Ordering::Relaxed); // Not racey due to &mut self + self.free[index as usize] = entity.id; + debug_assert!( + loc.index != usize::max_value(), + "free called on reserved entity without flush" + ); + Ok(loc) + } + + /// Ensure `n` at least allocations can succeed without reallocating + pub fn reserve(&mut self, additional: u32) { + debug_assert_eq!( + self.pending.load(Ordering::Relaxed), + 0, + "allocator must be flushed before potentially growing" + ); + let free = self.free_cursor.load(Ordering::Relaxed); + if additional > free { + self.grow(additional - free); + } + } + + pub fn contains(&self, entity: Entity) -> bool { + if entity.id >= self.meta.len() as u32 { + return true; + } + self.meta[entity.id as usize].generation == entity.generation + } + + pub fn clear(&mut self) { + // Not racey due to &mut self + self.free_cursor + .store(self.meta.len() as u32, Ordering::Relaxed); + for (i, x) in self.free.iter_mut().enumerate() { + *x = i as u32; + } + self.pending.store(0, Ordering::Relaxed); + self.reserved_cursor.store(0, Ordering::Relaxed); + } + + /// Access the location storage of an entity + /// + /// Must not be called on pending entities. + pub fn get_mut(&mut self, entity: Entity) -> Result<&mut Location, NoSuchEntity> { + let meta = &mut self.meta[entity.id as usize]; + if meta.generation == entity.generation { + Ok(&mut meta.location) } else { Err(NoSuchEntity) } } - /// Ensure `n` at least allocations can succeed without reallocating - pub fn reserve(&mut self, additional: u32) { - self.entity_locations.reserve(additional as usize) - } - - pub fn contains(&self, entity: Entity) -> bool { - self.entity_locations.contains_key(&entity) - } - - pub fn clear(&mut self) { - self.entity_locations.clear(); - } - - /// Access the location storage of an entity - pub fn get_mut(&mut self, entity: Entity) -> Result<&mut Location, NoSuchEntity> { - self.entity_locations.get_mut(&entity).ok_or(NoSuchEntity) - } - - /// Access the location storage of an entity - pub fn insert(&mut self, entity: Entity, location: Location) { - self.entity_locations.insert(entity, location); - } - + /// Returns `Ok(Location { archetype: 0, index: undefined })` for pending entities pub fn get(&self, entity: Entity) -> Result { - self.entity_locations - .get(&entity) - .cloned() - .ok_or(NoSuchEntity) + if self.meta.len() <= entity.id as usize { + return Ok(Location { + archetype: 0, + index: usize::max_value(), + }); + } + let meta = &self.meta[entity.id as usize]; + if meta.generation != entity.generation { + return Err(NoSuchEntity); + } + if meta.location.archetype == 0 { + return Ok(Location { + archetype: 0, + index: usize::max_value(), + }); + } + Ok(meta.location) + } + + /// Allocate space for and enumerate pending entities + #[allow(clippy::reversed_empty_ranges)] + pub fn flush(&mut self) -> impl Iterator { + let pending = self.pending.load(Ordering::Relaxed); // Not racey due to &mut self + if pending != 0 { + let first = self.meta.len() as u32; + self.grow(0); + first..(first + pending) + } else { + 0..0 + } + } + + // The following three methods allow iteration over `reserved` simultaneous to location + // writes. This is a lazy hack, but we only use it in `World::flush` so the complexity and unsafety + // involved in producing an `impl Iterator` isn't a clear win. + pub fn reserved_len(&self) -> u32 { + self.reserved_cursor.load(Ordering::Relaxed) + } + + pub fn reserved(&self, i: u32) -> u32 { + debug_assert!(i < self.reserved_len()); + self.reserved[i as usize].load(Ordering::Relaxed) + } + + pub fn clear_reserved(&mut self) { + self.reserved_cursor.store(0, Ordering::Relaxed); + } + + /// Expand storage and mark all but the first `pending` of the new slots as free + fn grow(&mut self, increment: u32) { + let pending = self.pending.swap(0, Ordering::Relaxed); + let new_len = (self.meta.len() + pending as usize + increment as usize) + .max(self.meta.len() * 2) + .max(1024); + let mut new_meta = Vec::with_capacity(new_len); + new_meta.extend_from_slice(&self.meta); + new_meta.resize( + new_len, + EntityMeta { + generation: 0, + location: Location { + archetype: 0, + index: usize::max_value(), // dummy value, to be filled in + }, + }, + ); + + let free_cursor = self.free_cursor.load(Ordering::Relaxed); // Not racey due to &mut self + let mut new_free = Vec::with_capacity(new_len); + new_free.extend_from_slice(&self.free[0..free_cursor as usize]); + // Add freshly allocated trailing free slots + new_free.extend(((self.meta.len() as u32 + pending)..new_len as u32).rev()); + debug_assert!(new_free.len() <= new_len); + self.free_cursor + .store(new_free.len() as u32, Ordering::Relaxed); // Not racey due to &mut self + + // Zero-fill + new_free.resize(new_len, 0); + + self.meta = new_meta; + self.free = new_free; + let mut new_reserved = Vec::with_capacity(new_len); + // Not racey due to &mut self + let reserved_cursor = self.reserved_cursor.load(Ordering::Relaxed); + for x in &self.reserved[..reserved_cursor as usize] { + new_reserved.push(AtomicU32::new(x.load(Ordering::Relaxed))); + } + new_reserved.resize_with(new_len, || AtomicU32::new(0)); + self.reserved = new_reserved.into(); + } + + pub fn get_reserver(&self) -> EntityReserver { + // SAFE: reservers use atomics for anything write-related + let entities: &'static Entities = unsafe { mem::transmute(self) }; + EntityReserver { entities } + } +} + +/// Reserves entities in a way that is usable in multi-threaded contexts. +pub struct EntityReserver { + entities: &'static Entities, +} + +impl EntityReserver { + /// Reserves an entity + pub fn reserve_entity(&self) -> Entity { + self.entities.reserve_entity() } } #[derive(Copy, Clone)] -#[allow(missing_docs)] +pub(crate) struct EntityMeta { + pub generation: u32, + pub location: Location, +} + +/// A location of an entity in an archetype +#[derive(Copy, Clone)] pub struct Location { + /// The archetype index pub archetype: u32, - pub index: u32, + + /// The index of the entity in the archetype + pub index: usize, } /// Error indicating that no entity with a particular ID exists @@ -102,3 +361,17 @@ impl fmt::Display for NoSuchEntity { #[cfg(feature = "std")] impl Error for NoSuchEntity {} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn entity_bits_roundtrip() { + let e = Entity { + generation: 0xDEADBEEF, + id: 0xBAADF00D, + }; + assert_eq!(Entity::from_bits(e.to_bits()), e); + } +} diff --git a/crates/bevy_ecs/hecs/src/lib.rs b/crates/bevy_ecs/hecs/src/lib.rs index 728bbfcc74..7191b4358e 100644 --- a/crates/bevy_ecs/hecs/src/lib.rs +++ b/crates/bevy_ecs/hecs/src/lib.rs @@ -33,7 +33,7 @@ //! let a = world.spawn((123, true, "abc")); //! let b = world.spawn((42, false)); //! // Systems can be simple for loops -//! for (id, mut number, &flag) in &mut world.query::<(Entity, &mut i32, &bool)>() { +//! for (id, mut number, &flag) in &mut world.query_mut::<(Entity, &mut i32, &bool)>() { //! if flag { *number *= 2; } //! } //! assert_eq!(*world.get::(a).unwrap(), 246); @@ -75,17 +75,17 @@ mod query_one; mod serde; mod world; -pub use archetype::Archetype; -pub use borrow::{EntityRef, Ref, RefMut}; +pub use archetype::{Archetype, TypeState}; +pub use borrow::{Ref, RefMut}; pub use bundle::{Bundle, DynamicBundle, MissingComponent}; -pub use entities::{Entity, Location, NoSuchEntity}; +pub use entities::{Entity, EntityReserver, Location, NoSuchEntity}; pub use entity_builder::{BuiltEntity, EntityBuilder}; pub use query::{ - Access, Added, BatchedIter, Changed, Mut, Mutated, Or, Query, QueryBorrow, QueryIter, With, - Without, + Access, Added, BatchedIter, Changed, Mut, Mutated, Or, Query, QueryBorrow, QueryIter, + ReadOnlyFetch, With, Without, }; pub use query_one::QueryOne; -pub use world::{ArchetypesGeneration, Component, ComponentError, Iter, SpawnBatchIter, World}; +pub use world::{ArchetypesGeneration, Component, ComponentError, SpawnBatchIter, World}; // Unstable implementation details needed by the macros #[doc(hidden)] diff --git a/crates/bevy_ecs/hecs/src/query.rs b/crates/bevy_ecs/hecs/src/query.rs index 120e392597..62c17c5484 100644 --- a/crates/bevy_ecs/hecs/src/query.rs +++ b/crates/bevy_ecs/hecs/src/query.rs @@ -20,7 +20,7 @@ use core::{ ptr::NonNull, }; -use crate::{archetype::Archetype, Component, Entity}; +use crate::{archetype::Archetype, Component, Entity, MissingComponent}; /// A collection of component types to fetch from a `World` pub trait Query { @@ -28,6 +28,9 @@ pub trait Query { type Fetch: for<'a> Fetch<'a>; } +/// A fetch that is read only. This should only be implemented for read-only fetches. +pub unsafe trait ReadOnlyFetch {} + /// Streaming iterators over contiguous homogeneous ranges of components pub trait Fetch<'a>: Sized { /// Type of value to be fetched @@ -76,7 +79,8 @@ pub enum Access { } #[derive(Copy, Clone, Debug)] -pub struct EntityFetch(NonNull); +pub struct EntityFetch(NonNull); +unsafe impl ReadOnlyFetch for EntityFetch {} impl Query for Entity { type Fetch = EntityFetch; @@ -107,7 +111,7 @@ impl<'a> Fetch<'a> for EntityFetch { unsafe fn next(&mut self) -> Self::Item { let id = self.0.as_ptr(); self.0 = NonNull::new_unchecked(id.add(1)); - Entity::from_id(*id) + *id } } @@ -118,6 +122,8 @@ impl<'a, T: Component> Query for &'a T { #[doc(hidden)] pub struct FetchRead(NonNull); +unsafe impl ReadOnlyFetch for FetchRead {} + impl<'a, T: Component> Fetch<'a> for FetchRead { type Item = &'a T; @@ -161,8 +167,24 @@ impl Query for Option { /// Unique borrow of an entity's component pub struct Mut<'a, T: Component> { - value: &'a mut T, - mutated: &'a mut bool, + pub(crate) value: &'a mut T, + pub(crate) mutated: &'a mut bool, +} + +impl<'a, T: Component> Mut<'a, T> { + /// Creates a new mutable reference to a component. This is unsafe because the index bounds are not checked. + /// + /// # Safety + /// This doesn't check the bounds of index in archetype + pub unsafe fn new(archetype: &'a Archetype, index: usize) -> Result { + let (target, type_state) = archetype + .get_with_type_state::() + .ok_or_else(MissingComponent::new::)?; + Ok(Self { + value: &mut *target.as_ptr().add(index), + mutated: &mut *type_state.mutated().as_ptr().add(index), + }) + } } unsafe impl Send for Mut<'_, T> {} @@ -214,11 +236,11 @@ impl<'a, T: Component> Fetch<'a> for FetchMut { unsafe fn get(archetype: &'a Archetype, offset: usize) -> Option { archetype - .get_with_mutated::() - .map(|(components, mutated)| { + .get_with_type_state::() + .map(|(components, type_state)| { Self( NonNull::new_unchecked(components.as_ptr().add(offset)), - NonNull::new_unchecked(mutated.as_ptr().add(offset)), + NonNull::new_unchecked(type_state.mutated().as_ptr().add(offset)), ) }) } @@ -306,11 +328,11 @@ impl_or_query!(Q1, Q2, Q3, Q4, Q5, Q6, Q7, Q8, Q9, Q10); /// let mut world = World::new(); /// world.spawn((123, true, 1., Some(1))); /// world.spawn((456, false, 2., Some(0))); -/// for mut b in world.query::>().iter().skip(1).take(1) { +/// for mut b in world.query_mut::>().iter().skip(1).take(1) { /// *b += 1; /// } /// let components = world -/// .query::, Mutated, Mutated, Mutated>)>>() +/// .query_mut::, Mutated, Mutated, Mutated>)>>() /// .iter() /// .map(|(b, i, f, o)| (*b, *i)) /// .collect::>(); @@ -361,11 +383,11 @@ impl<'a, T: Component> Fetch<'a> for FetchMutated { unsafe fn get(archetype: &'a Archetype, offset: usize) -> Option { archetype - .get_with_mutated::() - .map(|(components, mutated)| { + .get_with_type_state::() + .map(|(components, type_state)| { Self( NonNull::new_unchecked(components.as_ptr().add(offset)), - NonNull::new_unchecked(mutated.as_ptr().add(offset)), + NonNull::new_unchecked(type_state.mutated().as_ptr().add(offset)), ) }) } @@ -408,6 +430,7 @@ impl<'a, T: Component> Query for Added<'a, T> { #[doc(hidden)] pub struct FetchAdded(NonNull, NonNull); +unsafe impl ReadOnlyFetch for FetchAdded {} impl<'a, T: Component> Fetch<'a> for FetchAdded { type Item = Added<'a, T>; @@ -425,12 +448,14 @@ impl<'a, T: Component> Fetch<'a> for FetchAdded { } unsafe fn get(archetype: &'a Archetype, offset: usize) -> Option { - archetype.get_with_added::().map(|(components, added)| { - Self( - NonNull::new_unchecked(components.as_ptr().add(offset)), - NonNull::new_unchecked(added.as_ptr().add(offset)), - ) - }) + archetype + .get_with_type_state::() + .map(|(components, type_state)| { + Self( + NonNull::new_unchecked(components.as_ptr().add(offset)), + NonNull::new_unchecked(type_state.added().as_ptr().add(offset)), + ) + }) } fn release(archetype: &Archetype) { @@ -471,6 +496,7 @@ impl<'a, T: Component> Query for Changed<'a, T> { #[doc(hidden)] pub struct FetchChanged(NonNull, NonNull, NonNull); +unsafe impl ReadOnlyFetch for FetchChanged {} impl<'a, T: Component> Fetch<'a> for FetchChanged { type Item = Changed<'a, T>; @@ -489,12 +515,12 @@ impl<'a, T: Component> Fetch<'a> for FetchChanged { unsafe fn get(archetype: &'a Archetype, offset: usize) -> Option { archetype - .get_with_added_and_mutated::() - .map(|(components, added, mutated)| { + .get_with_type_state::() + .map(|(components, type_state)| { Self( NonNull::new_unchecked(components.as_ptr().add(offset)), - NonNull::new_unchecked(added.as_ptr().add(offset)), - NonNull::new_unchecked(mutated.as_ptr().add(offset)), + NonNull::new_unchecked(type_state.added().as_ptr().add(offset)), + NonNull::new_unchecked(type_state.mutated().as_ptr().add(offset)), ) }) } @@ -520,6 +546,7 @@ impl<'a, T: Component> Fetch<'a> for FetchChanged { #[doc(hidden)] pub struct TryFetch(Option); +unsafe impl ReadOnlyFetch for TryFetch where T: ReadOnlyFetch {} impl<'a, T: Fetch<'a>> Fetch<'a> for TryFetch { type Item = Option; @@ -574,6 +601,10 @@ impl Query for Without { #[doc(hidden)] pub struct FetchWithout(F, PhantomData); +unsafe impl<'a, T: Component, F: Fetch<'a>> ReadOnlyFetch for FetchWithout where + F: ReadOnlyFetch +{ +} impl<'a, T: Component, F: Fetch<'a>> Fetch<'a> for FetchWithout { type Item = F::Item; @@ -637,6 +668,7 @@ impl Query for With { #[doc(hidden)] pub struct FetchWith(F, PhantomData); +unsafe impl<'a, T: Component, F: Fetch<'a>> ReadOnlyFetch for FetchWith where F: ReadOnlyFetch {} impl<'a, T: Component, F: Fetch<'a>> Fetch<'a> for FetchWith { type Item = F::Item; @@ -706,7 +738,7 @@ impl<'w, Q: Query> QueryBorrow<'w, Q> { /// Like `iter`, but returns child iterators of at most `batch_size` elements /// /// Useful for distributing work over a threadpool. - pub fn iter_batched<'q>(&'q mut self, batch_size: u32) -> BatchedIter<'q, 'w, Q> { + pub fn iter_batched<'q>(&'q mut self, batch_size: usize) -> BatchedIter<'q, 'w, Q> { self.borrow(); BatchedIter { borrow: self, @@ -722,12 +754,7 @@ impl<'w, Q: Query> QueryBorrow<'w, Q> { "called QueryBorrow::iter twice on the same borrow; construct a new query instead" ); } - for x in self.archetypes { - // TODO: Release prior borrows on failure? - if Q::Fetch::access(x) >= Some(Access::Read) { - Q::Fetch::borrow(x); - } - } + self.borrowed = true; } @@ -781,32 +808,20 @@ impl<'w, Q: Query> QueryBorrow<'w, Q> { /// Helper to change the type of the query fn transform(mut self) -> QueryBorrow<'w, R> { - let x = QueryBorrow { + let borrow = QueryBorrow { archetypes: self.archetypes, borrowed: self.borrowed, _marker: PhantomData, }; - // Ensure `Drop` won't fire redundantly + self.borrowed = false; - x + borrow } } unsafe impl<'w, Q: Query> Send for QueryBorrow<'w, Q> {} unsafe impl<'w, Q: Query> Sync for QueryBorrow<'w, Q> {} -impl<'w, Q: Query> Drop for QueryBorrow<'w, Q> { - fn drop(&mut self) { - if self.borrowed { - for x in self.archetypes { - if Q::Fetch::access(x) >= Some(Access::Read) { - Q::Fetch::release(x); - } - } - } - } -} - impl<'q, 'w, Q: Query> IntoIterator for &'q mut QueryBorrow<'w, Q> { type IntoIter = QueryIter<'q, 'w, Q>; type Item = >::Item; @@ -819,7 +834,7 @@ impl<'q, 'w, Q: Query> IntoIterator for &'q mut QueryBorrow<'w, Q> { /// Iterator over the set of entities with the components in `Q` pub struct QueryIter<'q, 'w, Q: Query> { borrow: &'q mut QueryBorrow<'w, Q>, - archetype_index: u32, + archetype_index: usize, iter: Option>, } @@ -834,7 +849,7 @@ impl<'q, 'w, Q: Query> Iterator for QueryIter<'q, 'w, Q> { loop { match self.iter { None => { - let archetype = self.borrow.archetypes.get(self.archetype_index as usize)?; + let archetype = self.borrow.archetypes.get(self.archetype_index)?; self.archetype_index += 1; unsafe { self.iter = Q::Fetch::get(archetype, 0).map(|fetch| ChunkIter { @@ -868,14 +883,14 @@ impl<'q, 'w, Q: Query> ExactSizeIterator for QueryIter<'q, 'w, Q> { .archetypes .iter() .filter(|&x| Q::Fetch::access(x).is_some()) - .map(|x| x.len() as usize) + .map(|x| x.len()) .sum() } } struct ChunkIter { fetch: Q::Fetch, - len: u32, + len: usize, } impl ChunkIter { @@ -900,9 +915,9 @@ impl ChunkIter { /// Batched version of `QueryIter` pub struct BatchedIter<'q, 'w, Q: Query> { borrow: &'q mut QueryBorrow<'w, Q>, - archetype_index: u32, - batch_size: u32, - batch: u32, + archetype_index: usize, + batch_size: usize, + batch: usize, } unsafe impl<'q, 'w, Q: Query> Send for BatchedIter<'q, 'w, Q> {} @@ -913,14 +928,14 @@ impl<'q, 'w, Q: Query> Iterator for BatchedIter<'q, 'w, Q> { fn next(&mut self) -> Option { loop { - let archetype = self.borrow.archetypes.get(self.archetype_index as usize)?; + let archetype = self.borrow.archetypes.get(self.archetype_index)?; let offset = self.batch_size * self.batch; if offset >= archetype.len() { self.archetype_index += 1; self.batch = 0; continue; } - if let Some(fetch) = unsafe { Q::Fetch::get(archetype, offset as usize) } { + if let Some(fetch) = unsafe { Q::Fetch::get(archetype, offset) } { self.batch += 1; return Some(Batch { _marker: PhantomData, @@ -1003,10 +1018,11 @@ macro_rules! tuple_impl { impl<$($name: Query),*> Query for ($($name,)*) { type Fetch = ($($name::Fetch,)*); } + + unsafe impl<$($name: ReadOnlyFetch),*> ReadOnlyFetch for ($($name,)*) {} }; } -//smaller_tuples_too!(tuple_impl, B, A); smaller_tuples_too!(tuple_impl, O, N, M, L, K, J, I, H, G, F, E, D, C, B, A); #[cfg(test)] @@ -1067,31 +1083,31 @@ mod tests { let e3 = world.spawn((A(0), B(0))); world.spawn((A(0), B)); - for (i, mut a) in world.query::>().iter().enumerate() { + for (i, mut a) in world.query_mut::>().iter().enumerate() { if i % 2 == 0 { a.0 += 1; } } - fn get_changed_a(world: &World) -> Vec { + fn get_changed_a(world: &mut World) -> Vec { world - .query::<(Mutated, Entity)>() + .query_mut::<(Mutated, Entity)>() .iter() .map(|(_a, e)| e) .collect::>() }; - assert_eq!(get_changed_a(&world), vec![e1, e3]); + assert_eq!(get_changed_a(&mut world), vec![e1, e3]); // ensure changing an entity's archetypes also moves its mutated state world.insert(e1, (C,)).unwrap(); - assert_eq!(get_changed_a(&world), vec![e3, e1], "changed entities list should not change (although the order will due to archetype moves)"); + assert_eq!(get_changed_a(&mut world), vec![e3, e1], "changed entities list should not change (although the order will due to archetype moves)"); // spawning a new A entity should not change existing mutated state world.insert(e1, (A(0), B)).unwrap(); assert_eq!( - get_changed_a(&world), + get_changed_a(&mut world), vec![e3, e1], "changed entities list should not change" ); @@ -1099,7 +1115,7 @@ mod tests { // removing an unchanged entity should not change mutated state world.despawn(e2).unwrap(); assert_eq!( - get_changed_a(&world), + get_changed_a(&mut world), vec![e3, e1], "changed entities list should not change" ); @@ -1107,7 +1123,7 @@ mod tests { // removing a changed entity should remove it from enumeration world.despawn(e1).unwrap(); assert_eq!( - get_changed_a(&world), + get_changed_a(&mut world), vec![e3], "e1 should no longer be returned" ); @@ -1115,7 +1131,7 @@ mod tests { world.clear_trackers(); assert!(world - .query::<(Mutated, Entity)>() + .query_mut::<(Mutated, Entity)>() .iter() .map(|(_a, e)| e) .collect::>() @@ -1129,16 +1145,16 @@ mod tests { let e2 = world.spawn((A(0), B(0))); world.spawn((A(0), B(0))); - for mut a in world.query::>().iter() { + for mut a in world.query_mut::>().iter() { a.0 += 1; } - for mut b in world.query::>().iter().skip(1).take(1) { + for mut b in world.query_mut::>().iter().skip(1).take(1) { b.0 += 1; } let a_b_changed = world - .query::<(Mutated, Mutated, Entity)>() + .query_mut::<(Mutated, Mutated, Entity)>() .iter() .map(|(_a, _b, e)| e) .collect::>(); @@ -1154,16 +1170,16 @@ mod tests { let _e4 = world.spawn((A(0), B(0))); // Mutate A in entities e1 and e2 - for mut a in world.query::>().iter().take(2) { + for mut a in world.query_mut::>().iter().take(2) { a.0 += 1; } // Mutate B in entities e2 and e3 - for mut b in world.query::>().iter().skip(1).take(2) { + for mut b in world.query_mut::>().iter().skip(1).take(2) { b.0 += 1; } let a_b_changed = world - .query::<(Or<(Mutated, Mutated)>, Entity)>() + .query_mut::<(Or<(Mutated, Mutated)>, Entity)>() .iter() .map(|((_a, _b), e)| e) .collect::>(); diff --git a/crates/bevy_ecs/hecs/src/query_one.rs b/crates/bevy_ecs/hecs/src/query_one.rs index b99124f39e..cd355fb3bf 100644 --- a/crates/bevy_ecs/hecs/src/query_one.rs +++ b/crates/bevy_ecs/hecs/src/query_one.rs @@ -3,15 +3,14 @@ use core::marker::PhantomData; use crate::{ - query::{Fetch, With, Without}, + query::{Fetch, ReadOnlyFetch, With, Without}, Archetype, Component, Query, }; /// A borrow of a `World` sufficient to execute the query `Q` on a single entity pub struct QueryOne<'a, Q: Query> { archetype: &'a Archetype, - index: u32, - borrowed: bool, + index: usize, _marker: PhantomData, } @@ -21,11 +20,10 @@ impl<'a, Q: Query> QueryOne<'a, Q> { /// # Safety /// /// `index` must be in-bounds for `archetype` - pub(crate) unsafe fn new(archetype: &'a Archetype, index: u32) -> Self { + pub(crate) unsafe fn new(archetype: &'a Archetype, index: usize) -> Self { Self { archetype, index, - borrowed: false, _marker: PhantomData, } } @@ -37,13 +35,8 @@ impl<'a, Q: Query> QueryOne<'a, Q> { /// Panics if called more than once or if it would construct a borrow that clashes with another /// pre-existing borrow. pub fn get(&mut self) -> Option<>::Item> { - if self.borrowed { - panic!("called QueryOnce::get twice; construct a new query instead"); - } unsafe { - let mut fetch = Q::Fetch::get(self.archetype, self.index as usize)?; - self.borrowed = true; - Q::Fetch::borrow(self.archetype); + let mut fetch = Q::Fetch::get(self.archetype, self.index)?; Some(fetch.next()) } } @@ -63,26 +56,81 @@ impl<'a, Q: Query> QueryOne<'a, Q> { } /// Helper to change the type of the query - fn transform(mut self) -> QueryOne<'a, R> { - let x = QueryOne { + fn transform(self) -> QueryOne<'a, R> { + QueryOne { archetype: self.archetype, index: self.index, - borrowed: self.borrowed, _marker: PhantomData, - }; - // Ensure `Drop` won't fire redundantly - self.borrowed = false; - x - } -} - -impl Drop for QueryOne<'_, Q> { - fn drop(&mut self) { - if self.borrowed { - Q::Fetch::release(self.archetype); } } } unsafe impl Send for QueryOne<'_, Q> {} unsafe impl Sync for QueryOne<'_, Q> {} + +/// A read only borrow of a `World` sufficient to execute the query `Q` on a single entity +pub struct ReadOnlyQueryOne<'a, Q: Query> { + archetype: &'a Archetype, + index: usize, + _marker: PhantomData, +} + +impl<'a, Q: Query> ReadOnlyQueryOne<'a, Q> +where + Q::Fetch: ReadOnlyFetch, +{ + /// Construct a query accessing the entity in `archetype` at `index` + /// + /// # Safety + /// + /// `index` must be in-bounds for `archetype` + pub(crate) unsafe fn new(archetype: &'a Archetype, index: usize) -> Self { + Self { + archetype, + index, + _marker: PhantomData, + } + } + + /// Get the query result, or `None` if the entity does not satisfy the query + /// + /// Must be called at most once. + /// + /// Panics if called more than once or if it would construct a borrow that clashes with another + /// pre-existing borrow. + pub fn get(&self) -> Option<>::Item> + where + Q::Fetch: ReadOnlyFetch, + { + unsafe { + let mut fetch = Q::Fetch::get(self.archetype, self.index)?; + Some(fetch.next()) + } + } + + /// Transform the query into one that requires a certain component without borrowing it + /// + /// See `QueryBorrow::with` for details. + pub fn with(self) -> QueryOne<'a, With> { + self.transform() + } + + /// Transform the query into one that skips entities having a certain component + /// + /// See `QueryBorrow::without` for details. + pub fn without(self) -> QueryOne<'a, Without> { + self.transform() + } + + /// Helper to change the type of the query + fn transform(self) -> QueryOne<'a, R> { + QueryOne { + archetype: self.archetype, + index: self.index, + _marker: PhantomData, + } + } +} + +unsafe impl Send for ReadOnlyQueryOne<'_, Q> {} +unsafe impl Sync for ReadOnlyQueryOne<'_, Q> {} diff --git a/crates/bevy_ecs/hecs/src/serde.rs b/crates/bevy_ecs/hecs/src/serde.rs index bd3ac57711..cddb1060ad 100644 --- a/crates/bevy_ecs/hecs/src/serde.rs +++ b/crates/bevy_ecs/hecs/src/serde.rs @@ -8,6 +8,6 @@ impl Serialize for Entity { where S: Serializer, { - serializer.serialize_u128(self.id()) + serializer.serialize_u32(self.id()) } } diff --git a/crates/bevy_ecs/hecs/src/world.rs b/crates/bevy_ecs/hecs/src/world.rs index da5a47c40b..efdf627dc4 100644 --- a/crates/bevy_ecs/hecs/src/world.rs +++ b/crates/bevy_ecs/hecs/src/world.rs @@ -14,9 +14,12 @@ // modified by Bevy contributors -use crate::alloc::vec::Vec; +use crate::{ + alloc::vec::Vec, borrow::EntityRef, query::ReadOnlyFetch, query_one::ReadOnlyQueryOne, + EntityReserver, Mut, RefMut, +}; use bevy_utils::{HashMap, HashSet}; -use core::{any::TypeId, convert::TryFrom, fmt, mem, ptr}; +use core::{any::TypeId, fmt, mem, ptr}; #[cfg(feature = "std")] use std::error::Error; @@ -24,8 +27,8 @@ use std::error::Error; use crate::{ archetype::Archetype, entities::{Entities, Location}, - Bundle, DynamicBundle, Entity, EntityRef, MissingComponent, NoSuchEntity, Query, QueryBorrow, - QueryOne, Ref, RefMut, + Bundle, DynamicBundle, Entity, MissingComponent, NoSuchEntity, Query, QueryBorrow, QueryOne, + Ref, }; /// An unordered collection of entities, each having any number of distinctly typed components @@ -80,20 +83,11 @@ impl World { /// let b = world.spawn((456, true)); /// ``` pub fn spawn(&mut self, components: impl DynamicBundle) -> Entity { - let entity = Entity::new(); - self.spawn_as_entity(entity, components); - entity - } + // Ensure all entity allocations are accounted for so `self.entities` can realloc if + // necessary + self.flush(); - /// Create an entity with the given Entity id and the given components - /// - /// Arguments can be tuples, structs annotated with `#[derive(Bundle)]`, or the result of - /// calling `build` on an `EntityBuilder`, which is useful if the set of components isn't - /// statically known. To spawn an entity with only one component, use a one-element tuple like - /// `(x,)`. - /// - /// Any type that satisfies `Send + Sync + 'static` can be used as a component. - pub fn spawn_as_entity(&mut self, entity: Entity, components: impl DynamicBundle) { + let entity = self.entities.alloc(); let archetype_id = components.with_ids(|ids| { self.index.get(ids).copied().unwrap_or_else(|| { let x = self.archetypes.len() as u32; @@ -106,19 +100,18 @@ impl World { let archetype = &mut self.archetypes[archetype_id as usize]; unsafe { - let index = archetype.allocate(entity.id()); + let index = archetype.allocate(entity); components.put(|ptr, ty, size| { archetype.put_dynamic(ptr, ty, size, index, true); true }); - self.entities.insert( - entity, - Location { - archetype: archetype_id, - index, - }, - ); + self.entities.meta[entity.id as usize].location = Location { + archetype: archetype_id, + index, + }; } + + entity } /// Efficiently spawn a large number of entities with the same components @@ -139,11 +132,13 @@ impl World { I: IntoIterator, I::Item: Bundle, { + // Ensure all entity allocations are accounted for so `self.entities` can realloc if + // necessary + self.flush(); + let iter = iter.into_iter(); let (lower, upper) = iter.size_hint(); - let archetype_id = self.reserve_inner::( - u32::try_from(upper.unwrap_or(lower)).expect("iterator too large"), - ); + let archetype_id = self.reserve_inner::(upper.unwrap_or(lower) as u32); SpawnBatchIter { inner: iter, @@ -155,10 +150,12 @@ impl World { /// Destroy an entity and all its components pub fn despawn(&mut self, entity: Entity) -> Result<(), NoSuchEntity> { + self.flush(); + let loc = self.entities.free(entity)?; let archetype = &mut self.archetypes[loc.archetype as usize]; if let Some(moved) = unsafe { archetype.remove(loc.index) } { - self.entities.get_mut(Entity::from_id(moved)).unwrap().index = loc.index; + self.entities.get_mut(moved).unwrap().index = loc.index; } for ty in archetype.types() { let removed_entities = self @@ -175,7 +172,13 @@ impl World { self.reserve_inner::(additional); } + /// Reserves an entity. + pub fn reserve_entity(&self) -> Entity { + self.entities.reserve_entity() + } + fn reserve_inner(&mut self, additional: u32) -> u32 { + self.flush(); self.entities.reserve(additional); let archetype_id = T::with_static_ids(|ids| { @@ -188,7 +191,7 @@ impl World { }) }); - self.archetypes[archetype_id as usize].reserve(additional); + self.archetypes[archetype_id as usize].reserve(additional as usize); archetype_id } @@ -202,7 +205,7 @@ impl World { .removed_components .entry(ty.id()) .or_insert_with(Vec::new); - removed_entities.extend(archetype.iter_entities().map(|id| Entity::from_id(*id))); + removed_entities.extend(archetype.iter_entities().copied()); } archetype.clear(); } @@ -214,6 +217,14 @@ impl World { self.entities.contains(entity) } + /// Returns true if the given entity has a component with the given type id. + pub fn has_component_type(&self, entity: Entity, ty: TypeId) -> bool { + self.get_entity_location(entity) + .map(|location| &self.archetypes[location.archetype as usize]) + .map(|archetype| archetype.has_type(ty)) + .unwrap_or(false) + } + /// Efficiently iterate over all entities that have certain components /// /// Calling `iter` on the returned value yields `(Entity, Q)` tuples, where `Q` is some query @@ -227,17 +238,6 @@ impl World { /// The returned `QueryBorrow` can be further transformed with combinator methods; see its /// documentation for details. /// - /// Iterating a query will panic if it would violate an existing unique reference or construct - /// an invalid unique reference. This occurs when two simultaneously-active queries could expose - /// the same entity. Simultaneous queries can access the same component type if and only if the - /// world contains no entities that have all components required by both queries, assuming no - /// other component borrows are outstanding. - /// - /// Iterating a query yields references with lifetimes bound to the `QueryBorrow` returned - /// here. To ensure those are invalidated, the return value of this method must be dropped for - /// its dynamic borrows from the world to be released. Similarly, lifetime rules ensure that - /// references obtained from a query cannot outlive the `QueryBorrow`. - /// /// # Example /// ``` /// # use bevy_hecs::*; @@ -253,15 +253,86 @@ impl World { /// assert!(entities.contains(&(a, 123, true))); /// assert!(entities.contains(&(b, 456, false))); /// ``` - pub fn query(&self) -> QueryBorrow<'_, Q> { + pub fn query(&self) -> QueryBorrow<'_, Q> + where + Q::Fetch: ReadOnlyFetch, + { + // SAFE: read-only access to world and read only query prevents mutable access + unsafe { self.query_unchecked() } + } + + /// Efficiently iterate over all entities that have certain components + /// + /// Calling `iter` on the returned value yields `(Entity, Q)` tuples, where `Q` is some query + /// type. A query type is `&T`, `&mut T`, a tuple of query types, or an `Option` wrapping a + /// query type, where `T` is any component type. Components queried with `&mut` must only appear + /// once. Entities which do not have a component type referenced outside of an `Option` will be + /// skipped. + /// + /// Entities are yielded in arbitrary order. + /// + /// The returned `QueryBorrow` can be further transformed with combinator methods; see its + /// documentation for details. + /// + /// # Example + /// ``` + /// # use bevy_hecs::*; + /// let mut world = World::new(); + /// let a = world.spawn((123, true, "abc")); + /// let b = world.spawn((456, false)); + /// let c = world.spawn((42, "def")); + /// let entities = world.query::<(Entity, &i32, &bool)>() + /// .iter() + /// .map(|(e, &i, &b)| (e, i, b)) // Copy out of the world + /// .collect::>(); + /// assert_eq!(entities.len(), 2); + /// assert!(entities.contains(&(a, 123, true))); + /// assert!(entities.contains(&(b, 456, false))); + /// ``` + pub fn query_mut(&mut self) -> QueryBorrow<'_, Q> { + // SAFE: unique mutable access + unsafe { self.query_unchecked() } + } + + /// Efficiently iterate over all entities that have certain components + /// + /// Calling `iter` on the returned value yields `(Entity, Q)` tuples, where `Q` is some query + /// type. A query type is `&T`, `&mut T`, a tuple of query types, or an `Option` wrapping a + /// query type, where `T` is any component type. Components queried with `&mut` must only appear + /// once. Entities which do not have a component type referenced outside of an `Option` will be + /// skipped. + /// + /// Entities are yielded in arbitrary order. + /// + /// The returned `QueryBorrow` can be further transformed with combinator methods; see its + /// documentation for details. + /// + /// # Safety + /// This does not check for mutable query correctness. To be safe, make sure mutable queries + /// have unique access to the components they query. + /// + /// # Example + /// ``` + /// # use bevy_hecs::*; + /// let mut world = World::new(); + /// let a = world.spawn((123, true, "abc")); + /// let b = world.spawn((456, false)); + /// let c = world.spawn((42, "def")); + /// let entities = world.query::<(Entity, &i32, &bool)>() + /// .iter() + /// .map(|(e, &i, &b)| (e, i, b)) // Copy out of the world + /// .collect::>(); + /// assert_eq!(entities.len(), 2); + /// assert!(entities.contains(&(a, 123, true))); + /// assert!(entities.contains(&(b, 456, false))); + /// ``` + pub unsafe fn query_unchecked(&self) -> QueryBorrow<'_, Q> { QueryBorrow::new(&self.archetypes) } - /// Prepare a query against a single entity + /// Prepare a read only query against a single entity /// - /// Call `get` on the resulting `QueryOne` to actually execute the query. The `QueryOne` value - /// is responsible for releasing the dynamically-checked borrow made by `get`, so it can't be - /// dropped while references returned by `get` are live. + /// Call `get` on the resulting `QueryOne` to actually execute the query. /// /// Handy for accessing multiple components simultaneously. /// @@ -271,49 +342,128 @@ impl World { /// let mut world = World::new(); /// let a = world.spawn((123, true, "abc")); /// // The returned query must outlive the borrow made by `get` - /// let mut query = world.query_one::<(&mut i32, &bool)>(a).unwrap(); + /// let mut query = world.query_one::<(&i32, &bool)>(a).unwrap(); + /// let (number, flag) = query.get().unwrap(); + /// assert_eq!(*number, 123); + /// ``` + pub fn query_one( + &self, + entity: Entity, + ) -> Result, NoSuchEntity> + where + Q::Fetch: ReadOnlyFetch, + { + let loc = self.entities.get(entity)?; + Ok(unsafe { ReadOnlyQueryOne::new(&self.archetypes[loc.archetype as usize], loc.index) }) + } + + /// Prepare a query against a single entity + /// + /// Call `get` on the resulting `QueryOne` to actually execute the query. + /// + /// Handy for accessing multiple components simultaneously. + /// + /// # Example + /// ``` + /// # use bevy_hecs::*; + /// let mut world = World::new(); + /// let a = world.spawn((123, true, "abc")); + /// // The returned query must outlive the borrow made by `get` + /// let mut query = world.query_one_mut::<(&mut i32, &bool)>(a).unwrap(); /// let (mut number, flag) = query.get().unwrap(); /// if *flag { *number *= 2; } /// assert_eq!(*number, 246); /// ``` - pub fn query_one(&self, entity: Entity) -> Result, NoSuchEntity> { + pub fn query_one_mut( + &mut self, + entity: Entity, + ) -> Result, NoSuchEntity> { + // SAFE: unique mutable access to world + unsafe { self.query_one_mut_unchecked(entity) } + } + + /// Prepare a query against a single entity, without checking the safety of mutable queries + /// + /// Call `get` on the resulting `QueryOne` to actually execute the query. + /// + /// Handy for accessing multiple components simultaneously. + /// + /// # Safety + /// This does not check for mutable query correctness. To be safe, make sure mutable queries + /// have unique access to the components they query. + /// + /// # Example + /// ``` + /// # use bevy_hecs::*; + /// let mut world = World::new(); + /// let a = world.spawn((123, true, "abc")); + /// // The returned query must outlive the borrow made by `get` + /// let mut query = world.query_one_mut::<(&mut i32, &bool)>(a).unwrap(); + /// let (mut number, flag) = query.get().unwrap(); + /// if *flag { *number *= 2; } + /// assert_eq!(*number, 246); + /// ``` + pub unsafe fn query_one_mut_unchecked( + &self, + entity: Entity, + ) -> Result, NoSuchEntity> { let loc = self.entities.get(entity)?; - Ok(unsafe { QueryOne::new(&self.archetypes[loc.archetype as usize], loc.index) }) + Ok(QueryOne::new( + &self.archetypes[loc.archetype as usize], + loc.index, + )) } /// Borrow the `T` component of `entity` - /// - /// Panics if the component is already uniquely borrowed from another entity with the same - /// components. - pub fn get(&self, entity: Entity) -> Result, ComponentError> { - let loc = self.entities.get(entity)?; - if loc.archetype == 0 { - return Err(MissingComponent::new::().into()); + pub fn get(&self, entity: Entity) -> Result<&'_ T, ComponentError> { + unsafe { + let loc = self.entities.get(entity)?; + if loc.archetype == 0 { + return Err(MissingComponent::new::().into()); + } + Ok(&*self.archetypes[loc.archetype as usize] + .get::() + .ok_or_else(MissingComponent::new::)? + .as_ptr() + .add(loc.index as usize)) } - Ok(unsafe { Ref::new(&self.archetypes[loc.archetype as usize], loc.index)? }) } - /// Uniquely borrow the `T` component of `entity` - /// - /// Panics if the component is already borrowed from another entity with the same components. - pub fn get_mut(&self, entity: Entity) -> Result, ComponentError> { - let loc = self.entities.get(entity)?; - if loc.archetype == 0 { - return Err(MissingComponent::new::().into()); - } - Ok(unsafe { RefMut::new(&self.archetypes[loc.archetype as usize], loc.index)? }) + /// Mutably borrow the `T` component of `entity` + pub fn get_mut(&mut self, entity: Entity) -> Result, ComponentError> { + // SAFE: uniquely borrows world + unsafe { self.get_mut_unchecked(entity) } } /// Access an entity regardless of its component types /// /// Does not immediately borrow any component. - pub fn entity(&self, entity: Entity) -> Result, NoSuchEntity> { + pub fn entity(&mut self, entity: Entity) -> Result, NoSuchEntity> { Ok(match self.entities.get(entity)? { Location { archetype: 0, .. } => EntityRef::empty(), loc => unsafe { EntityRef::new(&self.archetypes[loc.archetype as usize], loc.index) }, }) } + /// Borrow the `T` component of `entity` without checking if it can be mutated + /// + /// # Safety + /// This does not check for mutable access correctness. To be safe, make sure this is the only + /// thing accessing this entity's T component. + pub unsafe fn get_mut_unchecked( + &self, + entity: Entity, + ) -> Result, ComponentError> { + let loc = self.entities.get(entity)?; + if loc.archetype == 0 { + return Err(MissingComponent::new::().into()); + } + Ok(Mut::new( + &self.archetypes[loc.archetype as usize], + loc.index, + )?) + } + /// Iterate over all entities in the world /// /// Entities are yielded in arbitrary order. Prefer `World::query` for better performance when @@ -330,7 +480,7 @@ impl World { /// assert!(ids.contains(&a)); /// assert!(ids.contains(&b)); /// ``` - pub fn iter(&self) -> Iter<'_> { + pub fn iter(&mut self) -> Iter<'_> { Iter::new(&self.archetypes, &self.entities) } @@ -364,6 +514,7 @@ impl World { ) -> Result<(), NoSuchEntity> { use std::collections::hash_map::Entry; + self.flush(); let loc = self.entities.get_mut(entity)?; unsafe { // Assemble Vec for the final entity @@ -407,18 +558,18 @@ impl World { loc.archetype as usize, target as usize, ); - let target_index = target_arch.allocate(entity.id()); + let target_index = target_arch.allocate(entity); loc.archetype = target; let old_index = mem::replace(&mut loc.index, target_index); if let Some(moved) = source_arch.move_to(old_index, |ptr, ty, size, is_added, is_mutated| { target_arch.put_dynamic(ptr, ty, size, target_index, false); let type_state = target_arch.get_type_state_mut(ty).unwrap(); - type_state.added_entities[target_index as usize] = is_added; - type_state.mutated_entities[target_index as usize] = is_mutated; + *type_state.added().as_ptr().add(target_index) = is_added; + *type_state.mutated().as_ptr().add(target_index) = is_mutated; }) { - self.entities.get_mut(Entity::from_id(moved)).unwrap().index = old_index; + self.entities.get_mut(moved).unwrap().index = old_index; } components.put(|ptr, ty, size| { @@ -462,6 +613,7 @@ impl World { pub fn remove(&mut self, entity: Entity) -> Result { use std::collections::hash_map::Entry; + self.flush(); let loc = self.entities.get_mut(entity)?; unsafe { let removed = T::with_static_ids(|ids| ids.iter().copied().collect::>()); @@ -490,7 +642,7 @@ impl World { loc.archetype as usize, target as usize, ); - let target_index = target_arch.allocate(entity.id()); + let target_index = target_arch.allocate(entity); loc.archetype = target; loc.index = target_index; let removed_components = &mut self.removed_components; @@ -500,8 +652,8 @@ impl World { if let Some(dst) = target_arch.get_dynamic(ty, size, target_index) { ptr::copy_nonoverlapping(src, dst.as_ptr(), size); let state = target_arch.get_type_state_mut(ty).unwrap(); - state.added_entities[target_index as usize] = is_added; - state.mutated_entities[target_index as usize] = is_mutated; + *state.added().as_ptr().add(target_index) = is_added; + *state.mutated().as_ptr().add(target_index) = is_mutated; } else { let removed_entities = removed_components.entry(ty).or_insert_with(Vec::new); @@ -509,7 +661,7 @@ impl World { } }) { - self.entities.get_mut(Entity::from_id(moved)).unwrap().index = old_index; + self.entities.get_mut(moved).unwrap().index = old_index; } Ok(bundle) } @@ -522,24 +674,75 @@ impl World { self.remove::<(T,)>(entity).map(|(x,)| x) } - /// Borrow the `T` component of `entity` without safety checks - /// - /// Should only be used as a building block for safe abstractions. + /// Borrow the `T` component at the given location, without safety checks /// /// # Safety - /// - /// `entity` must have been previously obtained from this `World`, and no unique borrow of the - /// same component of `entity` may be live simultaneous to the returned reference. - pub unsafe fn get_unchecked(&self, entity: Entity) -> Result<&T, ComponentError> { - let loc = self.entities.get(entity)?; - if loc.archetype == 0 { + /// This does not check that the location is within bounds of the archetype. + pub unsafe fn get_ref_at_location_unchecked( + &self, + location: Location, + ) -> Result, ComponentError> { + if location.archetype == 0 { return Err(MissingComponent::new::().into()); } - Ok(&*self.archetypes[loc.archetype as usize] + Ok(Ref::new( + &self.archetypes[location.archetype as usize], + location.index, + )?) + } + + /// Borrow the `T` component at the given location, without safety checks + /// + /// # Safety + /// This does not check that the location is within bounds of the archetype. + /// It also does not check for mutable access correctness. To be safe, make sure this is the only + /// thing accessing this entity's T component. + pub unsafe fn get_ref_mut_at_location_unchecked( + &self, + location: Location, + ) -> Result, ComponentError> { + if location.archetype == 0 { + return Err(MissingComponent::new::().into()); + } + Ok(RefMut::new( + &self.archetypes[location.archetype as usize], + location.index, + )?) + } + + /// Borrow the `T` component at the given location, without safety checks + /// # Safety + /// This does not check that the location is within bounds of the archetype. + pub unsafe fn get_at_location_unchecked( + &self, + location: Location, + ) -> Result<&T, ComponentError> { + if location.archetype == 0 { + return Err(MissingComponent::new::().into()); + } + Ok(&*self.archetypes[location.archetype as usize] .get::() .ok_or_else(MissingComponent::new::)? .as_ptr() - .add(loc.index as usize)) + .add(location.index as usize)) + } + + /// Borrow the `T` component at the given location, without safety checks + /// # Safety + /// This does not check that the location is within bounds of the archetype. + /// It also does not check for mutable access correctness. To be safe, make sure this is the only + /// thing accessing this entity's T component. + pub unsafe fn get_mut_at_location_unchecked( + &self, + location: Location, + ) -> Result, ComponentError> { + if location.archetype == 0 { + return Err(MissingComponent::new::().into()); + } + Ok(Mut::new( + &self.archetypes[location.archetype as usize], + location.index, + )?) } /// Uniquely borrow the `T` component of `entity` without safety checks @@ -565,6 +768,31 @@ impl World { .add(loc.index as usize)) } + /// Convert all reserved entities into empty entities that can be iterated and accessed + /// + /// Invoked implicitly by `spawn`, `despawn`, `insert`, and `remove`. + pub fn flush(&mut self) { + let arch = &mut self.archetypes[0]; + for entity_id in self.entities.flush() { + self.entities.meta[entity_id as usize].location.index = unsafe { + arch.allocate(Entity { + id: entity_id, + generation: self.entities.meta[entity_id as usize].generation, + }) + }; + } + for i in 0..self.entities.reserved_len() { + let id = self.entities.reserved(i); + self.entities.meta[id as usize].location.index = unsafe { + arch.allocate(Entity { + id, + generation: self.entities.meta[id as usize].generation, + }) + }; + } + self.entities.clear_reserved(); + } + /// Inspect the archetypes that entities are organized into /// /// Useful for dynamically scheduling concurrent queries by checking borrows in advance. Does @@ -608,6 +836,11 @@ impl World { self.removed_components.clear(); } + + /// Gets an entity reserver, which can be used to reserve entity ids in a multi-threaded context. + pub fn get_entity_reserver(&self) -> EntityReserver { + self.entities.get_reserver() + } } unsafe impl Send for World {} @@ -619,7 +852,7 @@ impl Default for World { } } -impl<'a> IntoIterator for &'a World { +impl<'a> IntoIterator for &'a mut World { type IntoIter = Iter<'a>; type Item = (Entity, EntityRef<'a>); @@ -682,7 +915,7 @@ pub struct Iter<'a> { archetypes: core::slice::Iter<'a, Archetype>, entities: &'a Entities, current: Option<&'a Archetype>, - index: u32, + index: usize, } impl<'a> Iter<'a> { @@ -710,23 +943,21 @@ impl<'a> Iterator for Iter<'a> { self.index = 0; } Some(current) => { - if self.index == current.len() as u32 { + if self.index == current.len() { self.current = None; continue; } let index = self.index; self.index += 1; - let id = current.entity_id(index); - return Some((Entity::from_id(id), unsafe { - EntityRef::new(current, index) - })); + let id = current.get_entity(index); + return Some((id, unsafe { EntityRef::new(current, index) })); } } } } fn size_hint(&self) -> (usize, Option) { - (0, Some(self.entities.entity_locations.len())) + (0, Some(self.entities.meta.len())) } } @@ -784,20 +1015,17 @@ where fn next(&mut self) -> Option { let components = self.inner.next()?; - let entity = Entity::new(); + let entity = self.entities.alloc(); unsafe { - let index = self.archetype.allocate(entity.id()); + let index = self.archetype.allocate(entity); components.put(|ptr, ty, size| { self.archetype.put_dynamic(ptr, ty, size, index, true); true }); - self.entities.insert( - entity, - Location { - archetype: self.archetype_id, - index, - }, - ); + self.entities.meta[entity.id as usize].location = Location { + archetype: self.archetype_id, + index, + }; } Some(entity) } diff --git a/crates/bevy_ecs/hecs/tests/tests.rs b/crates/bevy_ecs/hecs/tests/tests.rs index b5f5b30aa0..f3911cb0df 100644 --- a/crates/bevy_ecs/hecs/tests/tests.rs +++ b/crates/bevy_ecs/hecs/tests/tests.rs @@ -164,36 +164,6 @@ fn dynamic_components() { ); } -#[test] -#[should_panic(expected = "already borrowed")] -fn illegal_borrow() { - let mut world = World::new(); - world.spawn(("abc", 123)); - world.spawn(("def", 456)); - - world.query::<(&mut i32, &i32)>().iter(); -} - -#[test] -#[should_panic(expected = "already borrowed")] -fn illegal_borrow_2() { - let mut world = World::new(); - world.spawn(("abc", 123)); - world.spawn(("def", 456)); - - world.query::<(&mut i32, &mut i32)>().iter(); -} - -#[test] -fn disjoint_queries() { - let mut world = World::new(); - world.spawn(("abc", true)); - world.spawn(("def", 456)); - - let _a = world.query::<(&mut &str, &bool)>(); - let _b = world.query::<(&mut &str, &i32)>(); -} - #[test] fn shared_borrow() { let mut world = World::new(); @@ -203,15 +173,6 @@ fn shared_borrow() { world.query::<(&i32, &i32)>(); } -#[test] -#[should_panic(expected = "already borrowed")] -fn illegal_random_access() { - let mut world = World::new(); - let e = world.spawn(("abc", 123)); - let _borrow = world.get_mut::(e).unwrap(); - world.get::(e).unwrap(); -} - #[test] #[cfg(feature = "macros")] fn derived_bundle() { @@ -267,7 +228,7 @@ fn alias() { let mut world = World::new(); world.spawn(("abc", 123)); world.spawn(("def", 456, true)); - let mut q = world.query::<&mut i32>(); + let mut q = world.query_mut::(); let _a = q.iter().collect::>(); let _b = q.iter().collect::>(); } diff --git a/crates/bevy_ecs/src/resource/resource_query.rs b/crates/bevy_ecs/src/resource/resource_query.rs index 8c65551ba9..0b4acf7704 100644 --- a/crates/bevy_ecs/src/resource/resource_query.rs +++ b/crates/bevy_ecs/src/resource/resource_query.rs @@ -266,8 +266,9 @@ impl<'a, T: Resource> FetchResource<'a> for FetchResourceWrite { type Item = ResMut<'a, T>; unsafe fn get(resources: &'a Resources, _system_id: Option) -> Self::Item { - let (value, mutated) = resources.get_unsafe_ref_with_mutated::(ResourceIndex::Global); - ResMut::new(value, mutated) + let (value, type_state) = + resources.get_unsafe_ref_with_type_state::(ResourceIndex::Global); + ResMut::new(value, type_state.mutated()) } fn borrow(resources: &Resources) { diff --git a/crates/bevy_ecs/src/resource/resources.rs b/crates/bevy_ecs/src/resource/resources.rs index 0d166b759f..4b2992bc18 100644 --- a/crates/bevy_ecs/src/resource/resources.rs +++ b/crates/bevy_ecs/src/resource/resources.rs @@ -1,6 +1,6 @@ use super::{FetchResource, ResourceQuery}; use crate::system::SystemId; -use bevy_hecs::{Archetype, Ref, RefMut, TypeInfo}; +use bevy_hecs::{Archetype, Entity, Ref, RefMut, TypeInfo, TypeState}; use bevy_utils::HashMap; use core::any::TypeId; use std::ptr::NonNull; @@ -11,8 +11,8 @@ impl Resource for T {} pub(crate) struct ResourceData { archetype: Archetype, - default_index: Option, - system_id_to_archetype_index: HashMap, + default_index: Option, + system_id_to_archetype_index: HashMap, } pub enum ResourceIndex { @@ -94,7 +94,7 @@ impl Resources { use std::cmp::Ordering; match index.cmp(&archetype.len()) { Ordering::Equal => { - unsafe { archetype.allocate(index as u128) }; + unsafe { archetype.allocate(Entity::new(index as u32)) }; } Ordering::Greater => panic!("attempted to access index beyond 'current_capacity + 1'"), Ordering::Less => (), @@ -177,18 +177,18 @@ impl Resources { #[inline] #[allow(clippy::missing_safety_doc)] - pub unsafe fn get_unsafe_ref_with_mutated( + pub unsafe fn get_unsafe_ref_with_type_state( &self, resource_index: ResourceIndex, - ) -> (NonNull, NonNull) { + ) -> (NonNull, &TypeState) { self.get_resource_data_index::(resource_index) .and_then(|(data, index)| { data.archetype - .get_with_mutated::() - .map(|(resource, mutated)| { + .get_with_type_state::() + .map(|(resource, type_state)| { ( NonNull::new_unchecked(resource.as_ptr().add(index)), - NonNull::new_unchecked(mutated.as_ptr().add(index)), + type_state, ) }) }) @@ -203,9 +203,10 @@ impl Resources { ) -> (NonNull, NonNull) { self.get_resource_data_index::(resource_index) .and_then(|(data, index)| { + let type_state = data.archetype.get_type_state(TypeId::of::())?; Some(( - NonNull::new_unchecked(data.archetype.get_added::()?.as_ptr().add(index)), - NonNull::new_unchecked(data.archetype.get_mutated::()?.as_ptr().add(index)), + NonNull::new_unchecked(type_state.added().as_ptr().add(index)), + NonNull::new_unchecked(type_state.mutated().as_ptr().add(index)), )) }) .unwrap_or_else(|| panic!("Resource does not exist {}", std::any::type_name::())) diff --git a/crates/bevy_ecs/src/schedule/parallel_executor.rs b/crates/bevy_ecs/src/schedule/parallel_executor.rs index 71faf4fd7d..57d26650bf 100644 --- a/crates/bevy_ecs/src/schedule/parallel_executor.rs +++ b/crates/bevy_ecs/src/schedule/parallel_executor.rs @@ -6,8 +6,7 @@ use crate::{ use bevy_hecs::{ArchetypesGeneration, World}; use bevy_tasks::{ComputeTaskPool, CountdownEvent, TaskPool}; use fixedbitset::FixedBitSet; -use parking_lot::Mutex; -use std::{ops::Range, sync::Arc}; +use std::ops::Range; /// Executes each schedule stage in parallel by analyzing system dependencies. /// System execution order is undefined except under the following conditions: @@ -109,7 +108,7 @@ impl ExecutorStage { pub fn prepare_to_next_thread_local( &mut self, world: &World, - systems: &[Arc>>], + systems: &mut [Box], schedule_changed: bool, next_thread_local_index: usize, ) -> Range { @@ -143,8 +142,7 @@ impl ExecutorStage { if schedule_changed || archetypes_generation_changed { // update each system's archetype access to latest world archetypes for system_index in prepare_system_index_range.clone() { - let mut system = systems[system_index].lock(); - system.update_archetype_access(world); + systems[system_index].update_archetype_access(world); // Clear this so that the next block of code that populates it doesn't insert // duplicates @@ -156,7 +154,7 @@ impl ExecutorStage { let mut current_archetype_access = ArchetypeAccess::default(); let mut current_resource_access = TypeAccess::default(); for system_index in prepare_system_index_range.clone() { - let system = systems[system_index].lock(); + let system = &systems[system_index]; let archetype_access = system.archetype_access(); match system.thread_local_execution() { ThreadLocalExecution::NextFlush => { @@ -169,7 +167,7 @@ impl ExecutorStage { for earlier_system_index in prepare_system_index_range.start..system_index { - let earlier_system = systems[earlier_system_index].lock(); + let earlier_system = &systems[earlier_system_index]; // due to how prepare ranges work, previous systems should all be "NextFlush" debug_assert_eq!( @@ -292,7 +290,7 @@ impl ExecutorStage { &self, world: &World, resources: &Resources, - systems: &[Arc>>], + systems: &mut [Box], prepared_system_range: Range, compute_pool: &TaskPool, ) { @@ -300,25 +298,16 @@ impl ExecutorStage { log::trace!("running systems {:?}", prepared_system_range); compute_pool.scope(|scope| { let start_system_index = prepared_system_range.start; - for system_index in prepared_system_range { - let system = systems[system_index].clone(); - + let mut system_index = start_system_index; + for system in &mut systems[prepared_system_range] { log::trace!( "prepare {} {} with {} dependents and {} dependencies", system_index, - system.lock().name(), + system.name(), self.system_dependents[system_index].len(), self.system_dependencies[system_index].count_ones(..) ); - for dependency in self.system_dependencies[system_index].ones() { - log::trace!( - " * system ({}) depends on {}", - system_index, - systems[dependency].lock().name() - ); - } - // This event will be awaited, preventing the task from starting until all // our dependencies finish running let ready_event = &self.ready_events[system_index]; @@ -375,7 +364,6 @@ impl ExecutorStage { // Execute the system - in a scope to ensure the system lock is dropped before // triggering dependents { - let mut system = system.lock(); log::trace!("run {}", system.name()); #[cfg(feature = "profiler")] crate::profiler_start(resources, system.name().clone()); @@ -389,6 +377,7 @@ impl ExecutorStage { trigger_event.decrement(); } }); + system_index += 1; } }); } @@ -397,7 +386,7 @@ impl ExecutorStage { &mut self, world: &mut World, resources: &mut Resources, - systems: &[Arc>>], + systems: &mut [Box], schedule_changed: bool, ) { let start_archetypes_generation = world.archetypes_generation(); @@ -424,7 +413,6 @@ impl ExecutorStage { .resize(systems.len(), Vec::new()); for (system_index, system) in systems.iter().enumerate() { - let system = system.lock(); if system.thread_local_execution() == ThreadLocalExecution::Immediate { self.thread_local_system_indices.push(system_index); } @@ -466,7 +454,7 @@ impl ExecutorStage { self.thread_local_system_indices[next_thread_local_index]; { // if a thread local system is ready to run, run it exclusively on the main thread - let mut system = systems[thread_local_system_index].lock(); + let system = systems[thread_local_system_index].as_mut(); log::trace!("running thread local system {}", system.name()); system.run(world, resources); system.run_thread_local(world, resources); @@ -495,8 +483,7 @@ impl ExecutorStage { } // "flush" - for system in systems.iter() { - let mut system = system.lock(); + for system in systems.iter_mut() { match system.thread_local_execution() { ThreadLocalExecution::NextFlush => system.run_thread_local(world, resources), ThreadLocalExecution::Immediate => { /* already ran */ } @@ -559,6 +546,7 @@ mod tests { schedule.add_system_to_stage("PostArchetypeChange", read.system()); let mut executor = ParallelExecutor::default(); + schedule.initialize(&mut world, &mut resources); executor.run(&mut schedule, &mut world, &mut resources); } diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs index 0f89b185bf..0fca964395 100644 --- a/crates/bevy_ecs/src/schedule/schedule.rs +++ b/crates/bevy_ecs/src/schedule/schedule.rs @@ -4,15 +4,14 @@ use crate::{ }; use bevy_hecs::World; use bevy_utils::{HashMap, HashSet}; -use parking_lot::Mutex; -use std::{borrow::Cow, sync::Arc}; +use std::borrow::Cow; /// An ordered collection of stages, which each contain an ordered list of [System]s. /// Schedules are essentially the "execution plan" for an App's systems. /// They are run on a given [World] and [Resources] reference. #[derive(Default)] pub struct Schedule { - pub(crate) stages: HashMap, Vec>>>>, + pub(crate) stages: HashMap, Vec>>, pub(crate) stage_order: Vec>, pub(crate) system_ids: HashSet, generation: usize, @@ -94,7 +93,7 @@ impl Schedule { ); } self.system_ids.insert(system.id()); - systems.push(Arc::new(Mutex::new(system))); + systems.push(system); self.generation += 1; self @@ -118,7 +117,7 @@ impl Schedule { ); } self.system_ids.insert(system.id()); - systems.insert(0, Arc::new(Mutex::new(system))); + systems.insert(0, system); self.generation += 1; self @@ -128,7 +127,6 @@ impl Schedule { for stage_name in self.stage_order.iter() { if let Some(stage_systems) = self.stages.get_mut(stage_name) { for system in stage_systems.iter_mut() { - let mut system = system.lock(); #[cfg(feature = "profiler")] crate::profiler_start(resources, system.name().clone()); system.update_archetype_access(world); @@ -147,7 +145,6 @@ impl Schedule { // "flush" // NOTE: when this is made parallel a full sync is required here for system in stage_systems.iter_mut() { - let mut system = system.lock(); match system.thread_local_execution() { ThreadLocalExecution::NextFlush => { system.run_thread_local(world, resources) @@ -163,15 +160,14 @@ impl Schedule { } // TODO: move this code to ParallelExecutor - pub fn initialize(&mut self, resources: &mut Resources) { + pub fn initialize(&mut self, world: &mut World, resources: &mut Resources) { if self.last_initialize_generation == self.generation { return; } for stage in self.stages.values_mut() { for system in stage.iter_mut() { - let mut system = system.lock(); - system.initialize(resources); + system.initialize(world, resources); } } diff --git a/crates/bevy_ecs/src/system/commands.rs b/crates/bevy_ecs/src/system/commands.rs index 2082edf142..56e2a684f0 100644 --- a/crates/bevy_ecs/src/system/commands.rs +++ b/crates/bevy_ecs/src/system/commands.rs @@ -1,6 +1,6 @@ use super::SystemId; use crate::resource::{Resource, Resources}; -use bevy_hecs::{Bundle, Component, DynamicBundle, Entity, World}; +use bevy_hecs::{Bundle, Component, DynamicBundle, Entity, EntityReserver, World}; use parking_lot::Mutex; use std::{marker::PhantomData, sync::Arc}; @@ -31,23 +31,6 @@ where } } -pub(crate) struct SpawnAsEntity -where - T: DynamicBundle + Send + Sync + 'static, -{ - entity: Entity, - components: T, -} - -impl WorldWriter for SpawnAsEntity -where - T: DynamicBundle + Send + Sync + 'static, -{ - fn write(self: Box, world: &mut World) { - world.spawn_as_entity(self.entity, self.components); - } -} - pub(crate) struct SpawnBatch where I: IntoIterator, @@ -158,24 +141,19 @@ impl ResourcesWriter for InsertLocalResource { pub struct CommandsInternal { pub commands: Vec, pub current_entity: Option, + pub entity_reserver: Option, } impl CommandsInternal { pub fn spawn(&mut self, components: impl DynamicBundle + Send + Sync + 'static) -> &mut Self { - self.spawn_as_entity(Entity::new(), components) - } - - pub fn spawn_as_entity( - &mut self, - entity: Entity, - components: impl DynamicBundle + Send + Sync + 'static, - ) -> &mut Self { + let entity = self + .entity_reserver + .as_ref() + .expect("entity reserver has not been set") + .reserve_entity(); self.current_entity = Some(entity); self.commands - .push(Command::WriteWorld(Box::new(SpawnAsEntity { - entity, - components, - }))); + .push(Command::WriteWorld(Box::new(Insert { entity, components }))); self } @@ -224,17 +202,9 @@ pub struct Commands { impl Commands { pub fn spawn(&mut self, components: impl DynamicBundle + Send + Sync + 'static) -> &mut Self { - self.spawn_as_entity(Entity::new(), components) - } - - pub fn spawn_as_entity( - &mut self, - entity: Entity, - components: impl DynamicBundle + Send + Sync + 'static, - ) -> &mut Self { { let mut commands = self.commands.lock(); - commands.spawn_as_entity(entity, components); + commands.spawn(components); } self } @@ -348,6 +318,10 @@ impl Commands { phantom: PhantomData, }) } + + pub fn set_entity_reserver(&self, entity_reserver: EntityReserver) { + self.commands.lock().entity_reserver = Some(entity_reserver); + } } #[cfg(test)] @@ -361,6 +335,7 @@ mod tests { let mut world = World::default(); let mut resources = Resources::default(); let mut command_buffer = Commands::default(); + command_buffer.set_entity_reserver(world.get_entity_reserver()); command_buffer.spawn((1u32, 2u64)); command_buffer.insert_resource(3.14f32); command_buffer.apply(&mut world, &mut resources); diff --git a/crates/bevy_ecs/src/system/into_system.rs b/crates/bevy_ecs/src/system/into_system.rs index cf544a8d89..3da8449cac 100644 --- a/crates/bevy_ecs/src/system/into_system.rs +++ b/crates/bevy_ecs/src/system/into_system.rs @@ -11,7 +11,7 @@ pub(crate) struct SystemFn where F: FnMut(&World, &Resources, &ArchetypeAccess, &mut State) + Send + Sync, ThreadLocalF: FnMut(&mut World, &mut Resources, &mut State) + Send + Sync, - Init: FnMut(&mut Resources) + Send + Sync, + Init: FnMut(&mut World, &mut Resources, &mut State) + Send + Sync, SetArchetypeAccess: FnMut(&World, &mut ArchetypeAccess, &mut State) + Send + Sync, State: Send + Sync, { @@ -32,7 +32,7 @@ impl System where F: FnMut(&World, &Resources, &ArchetypeAccess, &mut State) + Send + Sync, ThreadLocalF: FnMut(&mut World, &mut Resources, &mut State) + Send + Sync, - Init: FnMut(&mut Resources) + Send + Sync, + Init: FnMut(&mut World, &mut Resources, &mut State) + Send + Sync, SetArchetypeAccess: FnMut(&World, &mut ArchetypeAccess, &mut State) + Send + Sync, State: Send + Sync, { @@ -65,8 +65,8 @@ where (self.thread_local_func)(world, resources, &mut self.state); } - fn initialize(&mut self, resources: &mut Resources) { - (self.init_func)(resources); + fn initialize(&mut self, world: &mut World, resources: &mut Resources) { + (self.init_func)(world, resources, &mut self.state); } fn id(&self) -> SystemId { @@ -104,21 +104,23 @@ macro_rules! impl_into_foreach_system { name: core::any::type_name::().into(), id, func: move |world, resources, _archetype_access, state| { - <<($($resource,)*) as ResourceQuery>::Fetch as FetchResource>::borrow(&resources); { if let Some(($($resource,)*)) = resources.query_system::<($($resource,)*)>(id) { - for ($($component,)*) in world.query::<($($component,)*)>().iter() { - fn_call!(self, ($($commands, state)*), ($($resource),*), ($($component),*)) + // SAFE: the scheduler has ensured that there is no archetype clashing here + unsafe { + for ($($component,)*) in world.query_unchecked::<($($component,)*)>().iter() { + fn_call!(self, ($($commands, state)*), ($($resource),*), ($($component),*)) + } } } } - <<($($resource,)*) as ResourceQuery>::Fetch as FetchResource>::release(&resources); }, thread_local_func: move |world, resources, state| { state.apply(world, resources); }, - init_func: move |resources| { + init_func: move |world, resources, state| { <($($resource,)*)>::initialize(resources, Some(id)); + state.set_entity_reserver(world.get_entity_reserver()) }, resource_access: <<($($resource,)*) as ResourceQuery>::Fetch as FetchResource>::access(), archetype_access: ArchetypeAccess::default(), @@ -174,7 +176,6 @@ macro_rules! impl_into_query_system { id, name: core::any::type_name::().into(), func: move |world, resources, archetype_access, state| { - <<($($resource,)*) as ResourceQuery>::Fetch as FetchResource>::borrow(&resources); { if let Some(($($resource,)*)) = resources.query_system::<($($resource,)*)>(id) { let mut i = 0; @@ -187,13 +188,14 @@ macro_rules! impl_into_query_system { fn_call!(self, ($($commands, commands)*), ($($resource),*), ($($query),*)) } } - <<($($resource,)*) as ResourceQuery>::Fetch as FetchResource>::release(&resources); }, thread_local_func: move |world, resources, state| { state.commands.apply(world, resources); }, - init_func: move |resources| { + init_func: move |world, resources, state| { <($($resource,)*)>::initialize(resources, Some(id)); + state.commands.set_entity_reserver(world.get_entity_reserver()) + }, resource_access: <<($($resource,)*) as ResourceQuery>::Fetch as FetchResource>::access(), archetype_access: ArchetypeAccess::default(), @@ -317,7 +319,7 @@ where self.run(world, resources); }, func: |_, _, _, _| {}, - init_func: |_| {}, + init_func: |_, _, _| {}, set_archetype_access: |_, _, _| {}, thread_local_execution: ThreadLocalExecution::Immediate, name: core::any::type_name::().into(), diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 353fe14117..172e372e27 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -1,7 +1,7 @@ use crate::ArchetypeAccess; use bevy_hecs::{ - Archetype, Component, ComponentError, Entity, Fetch, Query as HecsQuery, QueryOne, Ref, RefMut, - World, + Archetype, Component, ComponentError, Entity, Fetch, Query as HecsQuery, Ref, RefMut, With, + Without, World, }; use bevy_tasks::ParallelIterator; use std::marker::PhantomData; @@ -33,13 +33,15 @@ impl<'a, Q: HecsQuery> Query<'a, Q> { } #[inline] - pub fn iter(&mut self) -> QueryBorrow<'_, Q> { - QueryBorrow::new(&self.world.archetypes, self.archetype_access) + pub fn iter(&mut self) -> QueryBorrowChecked<'_, Q> { + QueryBorrowChecked::new(&self.world.archetypes, self.archetype_access) } + // TODO: find a way to make `iter`, `get`, `get_mut`, and `entity` safe without using tracking pointers with global locks + /// Gets a reference to the entity's component of the given type. This will fail if the entity does not have /// the given component type or if the given component type does not match this query. - pub fn get(&self, entity: Entity) -> Result, QueryError> { + pub fn get(&self, entity: Entity) -> Result, QueryError> { if let Some(location) = self.world.get_entity_location(entity) { if self .archetype_access @@ -50,7 +52,12 @@ impl<'a, Q: HecsQuery> Query<'a, Q> { .mutable .contains(location.archetype as usize) { - self.world.get(entity).map_err(QueryError::ComponentError) + // SAFE: we have already checked that the entity/component matches our archetype access. and systems are scheduled to run with safe archetype access + unsafe { + self.world + .get_ref_at_location_unchecked(location) + .map_err(QueryError::ComponentError) + } } else { Err(QueryError::CannotReadArchetype) } @@ -59,7 +66,7 @@ impl<'a, Q: HecsQuery> Query<'a, Q> { } } - pub fn entity(&self, entity: Entity) -> Result, QueryError> { + pub fn entity(&mut self, entity: Entity) -> Result, QueryError> { if let Some(location) = self.world.get_entity_location(entity) { if self .archetype_access @@ -70,7 +77,13 @@ impl<'a, Q: HecsQuery> Query<'a, Q> { .mutable .contains(location.archetype as usize) { - Ok(self.world.query_one(entity).unwrap()) + // SAFE: we have already checked that the entity matches our archetype. and systems are scheduled to run with safe archetype access + Ok(unsafe { + QueryOneChecked::new( + &self.world.archetypes[location.archetype as usize], + location.index, + ) + }) } else { Err(QueryError::CannotReadArchetype) } @@ -92,9 +105,12 @@ impl<'a, Q: HecsQuery> Query<'a, Q> { .mutable .contains(location.archetype as usize) { - self.world - .get_mut(entity) - .map_err(QueryError::ComponentError) + // SAFE: RefMut does exclusivity checks and we have already validated the entity + unsafe { + self.world + .get_ref_mut_at_location_unchecked(location) + .map_err(QueryError::ComponentError) + } } else { Err(QueryError::CannotWriteArchetype) } @@ -106,7 +122,7 @@ impl<'a, Q: HecsQuery> Query<'a, Q> { /// Sets the entity's component to the given value. This will fail if the entity does not already have /// the given component type or if the given component type does not match this query. - pub fn set(&self, entity: Entity, component: T) -> Result<(), QueryError> { + pub fn set(&mut self, entity: Entity, component: T) -> Result<(), QueryError> { let mut current = self.get_mut::(entity)?; *current = component; Ok(()) @@ -116,23 +132,18 @@ impl<'a, Q: HecsQuery> Query<'a, Q> { /// A borrow of a `World` sufficient to execute the query `Q` /// /// Note that borrows are not released until this object is dropped. -pub struct QueryBorrow<'w, Q: HecsQuery> { +pub struct QueryBorrowChecked<'w, Q: HecsQuery> { archetypes: &'w [Archetype], archetype_access: &'w ArchetypeAccess, + borrowed: bool, _marker: PhantomData, } -impl<'w, Q: HecsQuery> QueryBorrow<'w, Q> { +impl<'w, Q: HecsQuery> QueryBorrowChecked<'w, Q> { pub(crate) fn new(archetypes: &'w [Archetype], archetype_access: &'w ArchetypeAccess) -> Self { - for index in archetype_access.immutable.ones() { - Q::Fetch::borrow(&archetypes[index]); - } - - for index in archetype_access.mutable.ones() { - Q::Fetch::borrow(&archetypes[index]); - } Self { archetypes, + borrowed: false, archetype_access, _marker: PhantomData, } @@ -143,6 +154,7 @@ impl<'w, Q: HecsQuery> QueryBorrow<'w, Q> { /// Must be called only once per query. #[inline] pub fn iter<'q>(&'q mut self) -> QueryIter<'q, 'w, Q> { + self.borrow(); QueryIter { borrow: self, archetype_index: 0, @@ -164,7 +176,8 @@ impl<'w, Q: HecsQuery> QueryBorrow<'w, Q> { /// each batch could take longer than running the batch. On the other /// hand, a too large batch size risks that one batch is still running /// long after the rest have finished. - pub fn par_iter<'q>(&'q mut self, batch_size: u32) -> ParIter<'q, 'w, Q> { + pub fn par_iter<'q>(&'q mut self, batch_size: usize) -> ParIter<'q, 'w, Q> { + self.borrow(); ParIter { borrow: self, archetype_index: 0, @@ -172,25 +185,45 @@ impl<'w, Q: HecsQuery> QueryBorrow<'w, Q> { batch: 0, } } -} -unsafe impl<'w, Q: HecsQuery> Send for QueryBorrow<'w, Q> {} -unsafe impl<'w, Q: HecsQuery> Sync for QueryBorrow<'w, Q> {} + fn borrow(&mut self) { + if self.borrowed { + panic!( + "called QueryBorrowChecked::iter twice on the same borrow; construct a new query instead" + ); + } -impl<'w, Q: HecsQuery> Drop for QueryBorrow<'w, Q> { - #[inline] - fn drop(&mut self) { for index in self.archetype_access.immutable.ones() { - Q::Fetch::release(&self.archetypes[index]); + Q::Fetch::borrow(&self.archetypes[index]); } for index in self.archetype_access.mutable.ones() { - Q::Fetch::release(&self.archetypes[index]); + Q::Fetch::borrow(&self.archetypes[index]); + } + + self.borrowed = true; + } +} + +unsafe impl<'w, Q: HecsQuery> Send for QueryBorrowChecked<'w, Q> {} +unsafe impl<'w, Q: HecsQuery> Sync for QueryBorrowChecked<'w, Q> {} + +impl<'w, Q: HecsQuery> Drop for QueryBorrowChecked<'w, Q> { + #[inline] + fn drop(&mut self) { + if self.borrowed { + for index in self.archetype_access.immutable.ones() { + Q::Fetch::release(&self.archetypes[index]); + } + + for index in self.archetype_access.mutable.ones() { + Q::Fetch::release(&self.archetypes[index]); + } } } } -impl<'q, 'w, Q: HecsQuery> IntoIterator for &'q mut QueryBorrow<'w, Q> { +impl<'q, 'w, Q: HecsQuery> IntoIterator for &'q mut QueryBorrowChecked<'w, Q> { type IntoIter = QueryIter<'q, 'w, Q>; type Item = >::Item; @@ -202,8 +235,8 @@ impl<'q, 'w, Q: HecsQuery> IntoIterator for &'q mut QueryBorrow<'w, Q> { /// Iterator over the set of entities with the components in `Q` pub struct QueryIter<'q, 'w, Q: HecsQuery> { - borrow: &'q mut QueryBorrow<'w, Q>, - archetype_index: u32, + borrow: &'q mut QueryBorrowChecked<'w, Q>, + archetype_index: usize, iter: Option>, } @@ -252,14 +285,14 @@ impl<'q, 'w, Q: HecsQuery> ExactSizeIterator for QueryIter<'q, 'w, Q> { .archetypes .iter() .filter(|&x| Q::Fetch::access(x).is_some()) - .map(|x| x.len() as usize) + .map(|x| x.len()) .sum() } } struct ChunkIter { fetch: Q::Fetch, - len: u32, + len: usize, } impl ChunkIter { @@ -284,10 +317,10 @@ impl ChunkIter { /// Batched version of `QueryIter` pub struct ParIter<'q, 'w, Q: HecsQuery> { - borrow: &'q mut QueryBorrow<'w, Q>, - archetype_index: u32, - batch_size: u32, - batch: u32, + borrow: &'q mut QueryBorrowChecked<'w, Q>, + archetype_index: usize, + batch_size: usize, + batch: usize, } impl<'q, 'w, Q: HecsQuery> ParallelIterator> for ParIter<'q, 'w, Q> { @@ -295,7 +328,7 @@ impl<'q, 'w, Q: HecsQuery> ParallelIterator> for ParIter<'q, 'w, Q> fn next_batch(&mut self) -> Option> { loop { - let archetype = self.borrow.archetypes.get(self.archetype_index as usize)?; + let archetype = self.borrow.archetypes.get(self.archetype_index)?; let offset = self.batch_size * self.batch; if offset >= archetype.len() { self.archetype_index += 1; @@ -339,3 +372,77 @@ impl<'q, 'w, Q: HecsQuery> Iterator for Batch<'q, Q> { } unsafe impl<'q, Q: HecsQuery> Send for Batch<'q, Q> {} + +/// A borrow of a `World` sufficient to execute the query `Q` on a single entity +pub struct QueryOneChecked<'a, Q: HecsQuery> { + archetype: &'a Archetype, + index: usize, + borrowed: bool, + _marker: PhantomData, +} + +impl<'a, Q: HecsQuery> QueryOneChecked<'a, Q> { + /// Construct a query accessing the entity in `archetype` at `index` + /// + /// # Safety + /// + /// `index` must be in-bounds for `archetype` + pub(crate) unsafe fn new(archetype: &'a Archetype, index: usize) -> Self { + Self { + archetype, + index, + borrowed: false, + _marker: PhantomData, + } + } + + /// Get the query result, or `None` if the entity does not satisfy the query + /// + /// Must be called at most once. + /// + /// Panics if called more than once or if it would construct a borrow that clashes with another + /// pre-existing borrow. + pub fn get(&mut self) -> Option<>::Item> { + unsafe { + let mut fetch = Q::Fetch::get(self.archetype, self.index as usize)?; + self.borrowed = true; + Q::Fetch::borrow(self.archetype); + Some(fetch.next()) + } + } + + /// Transform the query into one that requires a certain component without borrowing it + /// + /// See `QueryBorrow::with` for details. + pub fn with(self) -> QueryOneChecked<'a, With> { + self.transform() + } + + /// Transform the query into one that skips entities having a certain component + /// + /// See `QueryBorrow::without` for details. + pub fn without(self) -> QueryOneChecked<'a, Without> { + self.transform() + } + + /// Helper to change the type of the query + fn transform(self) -> QueryOneChecked<'a, R> { + QueryOneChecked { + archetype: self.archetype, + index: self.index, + borrowed: self.borrowed, + _marker: PhantomData, + } + } +} + +impl Drop for QueryOneChecked<'_, Q> { + fn drop(&mut self) { + if self.borrowed { + Q::Fetch::release(self.archetype); + } + } +} + +unsafe impl Send for QueryOneChecked<'_, Q> {} +unsafe impl Sync for QueryOneChecked<'_, Q> {} diff --git a/crates/bevy_ecs/src/system/system.rs b/crates/bevy_ecs/src/system/system.rs index e5172536cc..87ec57e8c5 100644 --- a/crates/bevy_ecs/src/system/system.rs +++ b/crates/bevy_ecs/src/system/system.rs @@ -12,12 +12,12 @@ pub enum ThreadLocalExecution { } #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)] -pub struct SystemId(pub u32); +pub struct SystemId(pub usize); impl SystemId { #[allow(clippy::new_without_default)] pub fn new() -> Self { - SystemId(rand::random::()) + SystemId(rand::random::()) } } @@ -31,7 +31,7 @@ pub trait System: Send + Sync { fn thread_local_execution(&self) -> ThreadLocalExecution; fn run(&mut self, world: &World, resources: &Resources); fn run_thread_local(&mut self, world: &mut World, resources: &mut Resources); - fn initialize(&mut self, _resources: &mut Resources) {} + fn initialize(&mut self, _world: &mut World, _resources: &mut Resources) {} } /// Provides information about the archetypes a [System] reads and writes diff --git a/crates/bevy_ecs/src/world/world_builder.rs b/crates/bevy_ecs/src/world/world_builder.rs index d0f67fdd85..6430f3f3dd 100644 --- a/crates/bevy_ecs/src/world/world_builder.rs +++ b/crates/bevy_ecs/src/world/world_builder.rs @@ -22,7 +22,7 @@ pub struct WorldBuilder<'a> { impl<'a> WorldBuilder<'a> { pub fn entity(&mut self) -> &mut Self { - self.current_entity = Some(Entity::new()); + self.current_entity = Some(self.world.reserve_entity()); self } @@ -61,10 +61,4 @@ impl<'a> WorldBuilder<'a> { self.current_entity = Some(self.world.spawn(components)); self } - - pub fn spawn_as_entity(&mut self, entity: Entity, components: impl DynamicBundle) -> &mut Self { - self.world.spawn_as_entity(entity, components); - self.current_entity = Some(entity); - self - } } diff --git a/crates/bevy_property/src/impl_property/impl_property_bevy_ecs.rs b/crates/bevy_property/src/impl_property/impl_property_bevy_ecs.rs index ca54770b84..466fb1fd10 100644 --- a/crates/bevy_property/src/impl_property/impl_property_bevy_ecs.rs +++ b/crates/bevy_property/src/impl_property/impl_property_bevy_ecs.rs @@ -8,7 +8,7 @@ impl_property!(Entity, serialize_entity, deserialize_entity); mod private { use serde::{Deserialize, Serialize}; #[derive(Serialize, Deserialize)] - pub(super) struct Entity(pub(super) u128); + pub(super) struct Entity(pub(super) u32); } fn serialize_entity(entity: &Entity) -> Serializable { @@ -20,5 +20,5 @@ fn deserialize_entity( _registry: &PropertyTypeRegistry, ) -> Result, erased_serde::Error> { let entity = private::Entity::deserialize(deserializer)?; - Ok(Box::new(Entity::from_id(entity.0))) + Ok(Box::new(Entity::new(entity.0))) } diff --git a/crates/bevy_render/src/draw.rs b/crates/bevy_render/src/draw.rs index 09695833b6..f1ec867ea9 100644 --- a/crates/bevy_render/src/draw.rs +++ b/crates/bevy_render/src/draw.rs @@ -177,19 +177,21 @@ impl<'a> FetchResource<'a> for FetchDrawContext { unsafe fn get(resources: &'a Resources, _system_id: Option) -> Self::Item { let pipelines = { - let (value, mutated) = resources - .get_unsafe_ref_with_mutated::>(ResourceIndex::Global); - ResMut::new(value, mutated) + let (value, type_state) = resources + .get_unsafe_ref_with_type_state::>( + ResourceIndex::Global, + ); + ResMut::new(value, type_state.mutated()) }; let shaders = { - let (value, mutated) = - resources.get_unsafe_ref_with_mutated::>(ResourceIndex::Global); - ResMut::new(value, mutated) + let (value, type_state) = + resources.get_unsafe_ref_with_type_state::>(ResourceIndex::Global); + ResMut::new(value, type_state.mutated()) }; let pipeline_compiler = { - let (value, mutated) = - resources.get_unsafe_ref_with_mutated::(ResourceIndex::Global); - ResMut::new(value, mutated) + let (value, type_state) = + resources.get_unsafe_ref_with_type_state::(ResourceIndex::Global); + ResMut::new(value, type_state.mutated()) }; DrawContext { diff --git a/crates/bevy_render/src/render_graph/nodes/pass_node.rs b/crates/bevy_render/src/render_graph/nodes/pass_node.rs index d0731533e9..678dd30a34 100644 --- a/crates/bevy_render/src/render_graph/nodes/pass_node.rs +++ b/crates/bevy_render/src/render_graph/nodes/pass_node.rs @@ -12,7 +12,7 @@ use crate::{ }, }; use bevy_asset::{Assets, Handle}; -use bevy_ecs::{HecsQuery, Resources, World}; +use bevy_ecs::{HecsQuery, ReadOnlyFetch, Resources, World}; use std::marker::PhantomData; struct CameraInfo { @@ -108,7 +108,10 @@ impl PassNode { } } -impl Node for PassNode { +impl Node for PassNode +where + Q::Fetch: ReadOnlyFetch, +{ fn input(&self) -> &[ResourceSlotInfo] { &self.inputs } @@ -190,7 +193,7 @@ impl Node for PassNode { // attempt to draw each visible entity let mut draw_state = DrawState::default(); for visible_entity in visible_entities.iter() { - if let Ok(mut query_one) = world.query_one::(visible_entity.entity) { + if let Ok(query_one) = world.query_one::(visible_entity.entity) { if query_one.get().is_none() { // visible entity does not match the Pass query continue; diff --git a/crates/bevy_scene/src/scene.rs b/crates/bevy_scene/src/scene.rs index ddbc0486f5..1db5561e3b 100644 --- a/crates/bevy_scene/src/scene.rs +++ b/crates/bevy_scene/src/scene.rs @@ -11,7 +11,7 @@ pub struct Scene { } pub struct Entity { - pub entity: u128, + pub entity: u32, pub components: Vec, } @@ -23,7 +23,7 @@ impl Scene { for (index, entity) in archetype.iter_entities().enumerate() { if index == entities.len() { entities.push(Entity { - entity: *entity, + entity: entity.id(), components: Vec::new(), }) } diff --git a/crates/bevy_scene/src/scene_spawner.rs b/crates/bevy_scene/src/scene_spawner.rs index 05d759bcca..53c1541db7 100644 --- a/crates/bevy_scene/src/scene_spawner.rs +++ b/crates/bevy_scene/src/scene_spawner.rs @@ -3,12 +3,12 @@ use bevy_app::prelude::*; use bevy_asset::{AssetEvent, Assets, Handle}; use bevy_ecs::{Resources, World}; use bevy_type_registry::TypeRegistry; -use bevy_utils::{HashMap, HashSet}; +use bevy_utils::HashMap; use thiserror::Error; use uuid::Uuid; struct InstanceInfo { - entity_map: HashMap, + entity_map: HashMap, } #[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)] @@ -22,13 +22,11 @@ impl InstanceId { #[derive(Default)] pub struct SceneSpawner { - loaded_scenes: HashSet>, spawned_scenes: HashMap, Vec>, spawned_instances: HashMap, scene_asset_event_reader: EventReader>, - scenes_to_spawn: Vec>, - scenes_to_load: Vec>, - scenes_to_unload: Vec>, + scenes_to_instance: Vec>, + scenes_to_despawn: Vec>, } #[derive(Error, Debug)] @@ -40,30 +38,15 @@ pub enum SceneSpawnError { } impl SceneSpawner { - pub fn instance(&mut self, scene_handle: Handle) { - self.scenes_to_spawn.push(scene_handle); + pub fn spawn(&mut self, scene_handle: Handle) { + self.scenes_to_instance.push(scene_handle); } - pub fn load(&mut self, scene_handle: Handle) { - self.scenes_to_load.push(scene_handle); + pub fn despawn(&mut self, scene_handle: Handle) { + self.scenes_to_despawn.push(scene_handle); } - pub fn unload(&mut self, scene_handle: Handle) { - self.scenes_to_unload.push(scene_handle); - } - - pub fn load_sync( - &mut self, - world: &mut World, - resources: &Resources, - scene_handle: Handle, - ) -> Result<(), SceneSpawnError> { - Self::load_internal(world, resources, scene_handle, None)?; - self.loaded_scenes.insert(scene_handle); - Ok(()) - } - - pub fn unload_sync( + pub fn despawn_sync( &mut self, world: &mut World, scene_handle: Handle, @@ -72,12 +55,11 @@ impl SceneSpawner { for instance_id in instance_ids { if let Some(instance) = self.spawned_instances.get(&instance_id) { for entity in instance.entity_map.values() { - let _ = world.despawn(*entity); // Ignore the result, unload only cares if it exists. + let _ = world.despawn(*entity); // Ignore the result, despawn only cares if it exists. } } } - self.loaded_scenes.remove(&scene_handle); self.spawned_scenes.remove(&scene_handle); } Ok(()) @@ -93,7 +75,7 @@ impl SceneSpawner { let mut instance_info = InstanceInfo { entity_map: HashMap::default(), }; - Self::load_internal(world, resources, scene_handle, Some(&mut instance_info))?; + Self::spawn_internal(world, resources, scene_handle, &mut instance_info)?; self.spawned_instances.insert(instance_id, instance_info); let spawned = self .spawned_scenes @@ -103,11 +85,11 @@ impl SceneSpawner { Ok(()) } - fn load_internal( + fn spawn_internal( world: &mut World, resources: &Resources, scene_handle: Handle, - mut instance_info: Option<&mut InstanceInfo>, + instance_info: &mut InstanceInfo, ) -> Result<(), SceneSpawnError> { let type_registry = resources.get::().unwrap(); let component_registry = type_registry.component.read(); @@ -119,33 +101,21 @@ impl SceneSpawner { })?; for scene_entity in scene.entities.iter() { - let entity = if let Some(ref mut instance_info) = instance_info { - *instance_info - .entity_map - .entry(scene_entity.entity) - .or_insert_with(bevy_ecs::Entity::new) - } else { - bevy_ecs::Entity::from_id(scene_entity.entity) - }; - if world.contains(entity) { - for component in scene_entity.components.iter() { - let component_registration = component_registry - .get_with_name(&component.type_name) - .ok_or_else(|| SceneSpawnError::UnregisteredComponent { - type_name: component.type_name.to_string(), - })?; + let entity = *instance_info + .entity_map + .entry(scene_entity.entity) + .or_insert_with(|| world.reserve_entity()); + for component in scene_entity.components.iter() { + let component_registration = component_registry + .get_with_name(&component.type_name) + .ok_or_else(|| SceneSpawnError::UnregisteredComponent { + type_name: component.type_name.to_string(), + })?; + if world.has_component_type(entity, component_registration.ty) { if component.type_name != "Camera" { component_registration.apply_component_to_entity(world, entity, component); } - } - } else { - world.spawn_as_entity(entity, (1,)); - for component in scene_entity.components.iter() { - let component_registration = component_registry - .get_with_name(&component.type_name) - .ok_or_else(|| SceneSpawnError::UnregisteredComponent { - type_name: component.type_name.to_string(), - })?; + } else { component_registration .add_component_to_entity(world, resources, entity, component); } @@ -164,7 +134,7 @@ impl SceneSpawner { if let Some(spawned_instances) = self.spawned_scenes.get(scene_handle) { for instance_id in spawned_instances.iter() { if let Some(instance_info) = self.spawned_instances.get_mut(instance_id) { - Self::load_internal(world, resources, *scene_handle, Some(instance_info))?; + Self::spawn_internal(world, resources, *scene_handle, instance_info)?; } } } @@ -172,31 +142,11 @@ impl SceneSpawner { Ok(()) } - pub fn load_queued_scenes( - &mut self, - world: &mut World, - resources: &Resources, - ) -> Result<(), SceneSpawnError> { - let scenes_to_load = std::mem::take(&mut self.scenes_to_load); + pub fn despawn_queued_scenes(&mut self, world: &mut World) -> Result<(), SceneSpawnError> { + let scenes_to_despawn = std::mem::take(&mut self.scenes_to_despawn); - for scene_handle in scenes_to_load { - match self.load_sync(world, resources, scene_handle) { - Ok(_) => {} - Err(SceneSpawnError::NonExistentScene { .. }) => { - self.scenes_to_load.push(scene_handle) - } - Err(err) => return Err(err), - } - } - - Ok(()) - } - - pub fn unload_queued_scenes(&mut self, world: &mut World) -> Result<(), SceneSpawnError> { - let scenes_to_unload = std::mem::take(&mut self.scenes_to_unload); - - for scene_handle in scenes_to_unload { - self.unload_sync(world, scene_handle)?; + for scene_handle in scenes_to_despawn { + self.despawn_sync(world, scene_handle)?; } Ok(()) } @@ -206,13 +156,13 @@ impl SceneSpawner { world: &mut World, resources: &Resources, ) -> Result<(), SceneSpawnError> { - let scenes_to_spawn = std::mem::take(&mut self.scenes_to_spawn); + let scenes_to_spawn = std::mem::take(&mut self.scenes_to_instance); for scene_handle in scenes_to_spawn { match self.spawn_sync(world, resources, scene_handle) { Ok(_) => {} Err(SceneSpawnError::NonExistentScene { .. }) => { - self.scenes_to_spawn.push(scene_handle) + self.scenes_to_instance.push(scene_handle) } Err(err) => return Err(err), } @@ -232,17 +182,13 @@ pub fn scene_spawner_system(world: &mut World, resources: &mut Resources) { .iter(&scene_asset_events) { if let AssetEvent::Modified { handle } = event { - if scene_spawner.loaded_scenes.contains(handle) { - scene_spawner.load(*handle); - } if scene_spawner.spawned_scenes.contains_key(handle) { updated_spawned_scenes.push(*handle); } } } - scene_spawner.unload_queued_scenes(world).unwrap(); - scene_spawner.load_queued_scenes(world, resources).unwrap(); + scene_spawner.despawn_queued_scenes(world).unwrap(); scene_spawner.spawn_queued_scenes(world, resources).unwrap(); scene_spawner .update_spawned_scenes(world, resources, &updated_spawned_scenes) diff --git a/crates/bevy_scene/src/serde.rs b/crates/bevy_scene/src/serde.rs index e6f2b1bd2a..9d9e4b7c9b 100644 --- a/crates/bevy_scene/src/serde.rs +++ b/crates/bevy_scene/src/serde.rs @@ -182,7 +182,7 @@ impl<'a, 'de> Visitor<'de> for SceneEntityVisiter<'a> { if id.is_some() { return Err(Error::duplicate_field(ENTITY_FIELD_ENTITY)); } - id = Some(map.next_value::()?); + id = Some(map.next_value::()?); } EntityField::Components => { if components.is_some() { diff --git a/crates/bevy_transform/src/components/parent.rs b/crates/bevy_transform/src/components/parent.rs index 23b4890a61..05f30b4a43 100644 --- a/crates/bevy_transform/src/components/parent.rs +++ b/crates/bevy_transform/src/components/parent.rs @@ -11,7 +11,7 @@ pub struct Parent(pub Entity); // ways to handle cases like this. impl FromResources for Parent { fn from_resources(_resources: &bevy_ecs::Resources) -> Self { - Parent(Entity::from_id(u128::MAX)) + Parent(Entity::new(u32::MAX)) } } diff --git a/crates/bevy_transform/src/hierarchy/child_builder.rs b/crates/bevy_transform/src/hierarchy/child_builder.rs index 1c0e64e30d..5bfb7afd0f 100644 --- a/crates/bevy_transform/src/hierarchy/child_builder.rs +++ b/crates/bevy_transform/src/hierarchy/child_builder.rs @@ -74,16 +74,10 @@ impl WorldWriter for PushChildren { impl<'a> ChildBuilder<'a> { pub fn spawn(&mut self, components: impl DynamicBundle + Send + Sync + 'static) -> &mut Self { - self.spawn_as_entity(Entity::new(), components) - } - - pub fn spawn_as_entity( - &mut self, - entity: Entity, - components: impl DynamicBundle + Send + Sync + 'static, - ) -> &mut Self { - self.commands.spawn_as_entity(entity, components); - self.push_children.children.push(entity); + self.commands.spawn(components); + self.push_children + .children + .push(self.commands.current_entity.unwrap()); self } @@ -216,6 +210,7 @@ mod tests { let mut world = World::default(); let mut resources = Resources::default(); let mut commands = Commands::default(); + commands.set_entity_reserver(world.get_entity_reserver()); let mut parent = None; let mut child1 = None; diff --git a/crates/bevy_transform/src/hierarchy/hierarchy.rs b/crates/bevy_transform/src/hierarchy/hierarchy.rs index b13f665e96..5e340c93b0 100644 --- a/crates/bevy_transform/src/hierarchy/hierarchy.rs +++ b/crates/bevy_transform/src/hierarchy/hierarchy.rs @@ -45,8 +45,8 @@ pub struct DespawnRecursive { fn despawn_with_children_recursive(world: &mut World, entity: Entity) { // first, make the entity's own parent forget about it - if let Ok(parent) = world.get::(entity) { - if let Ok(mut children) = world.get_mut::(parent.0) { + if let Ok(parent) = world.get::(entity).map(|parent| parent.0) { + if let Ok(mut children) = world.get_mut::(parent) { children.retain(|c| *c != entity); } } @@ -98,6 +98,7 @@ mod tests { let mut world = World::default(); let mut resources = Resources::default(); let mut command_buffer = Commands::default(); + command_buffer.set_entity_reserver(world.get_entity_reserver()); command_buffer.spawn((0u32, 0u64)).with_children(|parent| { parent.spawn((0u32, 0u64)); diff --git a/crates/bevy_transform/src/hierarchy/hierarchy_maintenance_system.rs b/crates/bevy_transform/src/hierarchy/hierarchy_maintenance_system.rs index 8a7f163735..a844e886ba 100644 --- a/crates/bevy_transform/src/hierarchy/hierarchy_maintenance_system.rs +++ b/crates/bevy_transform/src/hierarchy/hierarchy_maintenance_system.rs @@ -125,6 +125,7 @@ mod test { // Add parent entities let mut commands = Commands::default(); + commands.set_entity_reserver(world.get_entity_reserver()); let mut parent = None; let mut children = Vec::new(); commands diff --git a/crates/bevy_transform/src/hierarchy/world_child_builder.rs b/crates/bevy_transform/src/hierarchy/world_child_builder.rs index c3ebd39d10..4982df8aa3 100644 --- a/crates/bevy_transform/src/hierarchy/world_child_builder.rs +++ b/crates/bevy_transform/src/hierarchy/world_child_builder.rs @@ -8,22 +8,15 @@ pub struct WorldChildBuilder<'a, 'b> { impl<'a, 'b> WorldChildBuilder<'a, 'b> { pub fn spawn(&mut self, components: impl DynamicBundle + Send + Sync + 'static) -> &mut Self { - self.spawn_as_entity(Entity::new(), components) - } - - pub fn spawn_as_entity( - &mut self, - entity: Entity, - components: impl DynamicBundle + Send + Sync + 'static, - ) -> &mut Self { let parent_entity = self .parent_entities .last() .cloned() .expect("There should always be a parent at this point."); self.world_builder - .spawn_as_entity(entity, components) + .spawn(components) .with_bundle((Parent(parent_entity), PreviousParent(Some(parent_entity)))); + let entity = self.world_builder.current_entity.unwrap(); { let world = &mut self.world_builder.world; let mut added = false; diff --git a/crates/bevy_transform/src/transform_propagate_system.rs b/crates/bevy_transform/src/transform_propagate_system.rs index 9f6622c095..bfc03ae581 100644 --- a/crates/bevy_transform/src/transform_propagate_system.rs +++ b/crates/bevy_transform/src/transform_propagate_system.rs @@ -117,6 +117,7 @@ mod test { // Root entity let mut commands = Commands::default(); + commands.set_entity_reserver(world.get_entity_reserver()); let mut children = Vec::new(); commands .spawn(( diff --git a/examples/scene/scene.rs b/examples/scene/scene.rs index fe86a610d4..9c8c8817c0 100644 --- a/examples/scene/scene.rs +++ b/examples/scene/scene.rs @@ -56,26 +56,18 @@ fn load_scene_system(asset_server: Res, mut scene_spawner: ResMut = { @@ -87,7 +79,7 @@ fn load_scene_right_now_system(world: &mut World, resources: &mut Resources) { }; let mut scene_spawner = resources.get_mut::().unwrap(); scene_spawner - .load_sync(world, resources, scene_handle) + .spawn_sync(world, resources, scene_handle) .unwrap(); }