From 7846ef33013c3958dc6c82c496a78bbd0d5b885e Mon Sep 17 00:00:00 2001 From: Ryan Date: Mon, 12 Feb 2024 05:48:42 -0800 Subject: [PATCH 1/3] Fix bug in event broadcasting When a component is added or removed from an entity, the system list currently being iterated over may no longer match the target entity. To fix this, events are now only handled once the current event has finished broadcasting. Still not entirely sure that this is the most desirable behavior though. The `EventPtr` and `EventMut` interface also had an update. --- CHANGELOG.md | 6 + README.md | 6 +- src/assert.rs | 37 +- src/blob_vec.rs | 3 - src/event.rs | 191 ++++++---- src/world.rs | 347 +++++++++---------- tutorial/ch03_sending_events_from_systems.md | 74 ++-- 7 files changed, 336 insertions(+), 328 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cf860a7..749ea93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Change Log +## Unreleased + +- Fixed a bug in event propagation. +- The behavior of the event queue has changed. The next event in the queue is handled only once the current event has finished broadcasting. +- Changed `EventPtr` interface and relaxed `Send` and `Sync` bounds for `EventMut`. + ## 0.2.2 - 2024-02-8 - Update documentation diff --git a/README.md b/README.md index 1ef0fed..66f146c 100644 --- a/README.md +++ b/README.md @@ -7,11 +7,11 @@ It aims to have a small but maximally expressive set of features that are easy a ## Features -- In addition to the usual Entities, Components, and Systems, `evenio` introduces _events_ as a first-class citizen. -Rather than restricting systems to run once every frame/update in a fixed order, systems are generalized as _event handlers_. +- In addition to the usual Entities, Components, and Systems, `evenio` introduces events as a first-class citizen. +Rather than restricting systems to run once every frame/update in a fixed order, systems are generalized as event handlers. The control flow of the entire program is then defined by the flow of events between systems. - Structural changes to the world (such as entity despawning, component additions/removals, etc.) are mediated by events, allowing systems to hook into their occurrence. -- _Targeted events_ enable systems to efficiently filter events based on queries. +- Targeted events enable systems to efficiently filter events based on queries. - Component types, event types, and systems are identified with generational indices, allowing them to be added and removed dynamically. - Execute systems in parallel with [Rayon]. - Core of the library does not depend on Rust's type system. diff --git a/src/assert.rs b/src/assert.rs index ddf5f1b..e96cd4e 100644 --- a/src/assert.rs +++ b/src/assert.rs @@ -1,7 +1,7 @@ //! Utilities for runtime and compile-time assertions. -use alloc::vec::Vec; use core::marker::PhantomData; +use core::slice::SliceIndex; use core::{fmt, mem}; use slab::Slab; @@ -16,7 +16,7 @@ const _: () = assert!( /// Extension trait for checked array indexing with checks removed in release /// mode. -pub(crate) trait GetDebugChecked { +pub(crate) trait GetDebugChecked { type Output: ?Sized; /// Gets a reference to the element at the given index. @@ -28,7 +28,7 @@ pub(crate) trait GetDebugChecked { /// /// - `idx` must be in bounds. #[track_caller] - unsafe fn get_debug_checked(&self, idx: usize) -> &Self::Output; + unsafe fn get_debug_checked(&self, idx: Idx) -> &Self::Output; /// Gets a mutable reference to the element at the given index. /// /// If `idx` is not in bounds, a panic occurs in debug mode and Undefined @@ -38,14 +38,16 @@ pub(crate) trait GetDebugChecked { /// /// - `idx` must be in bounds. #[track_caller] - unsafe fn get_debug_checked_mut(&mut self, idx: usize) -> &mut Self::Output; + unsafe fn get_debug_checked_mut(&mut self, idx: Idx) -> &mut Self::Output; } -impl GetDebugChecked for [T] { - type Output = T; +impl GetDebugChecked for [T] +where + I: SliceIndex<[T]>, +{ + type Output = I::Output; - #[inline] - unsafe fn get_debug_checked(&self, idx: usize) -> &Self::Output { + unsafe fn get_debug_checked(&self, idx: I) -> &Self::Output { #[cfg(debug_assertions)] return &self[idx]; @@ -53,8 +55,7 @@ impl GetDebugChecked for [T] { return self.get_unchecked(idx); } - #[inline] - unsafe fn get_debug_checked_mut(&mut self, idx: usize) -> &mut Self::Output { + unsafe fn get_debug_checked_mut(&mut self, idx: I) -> &mut Self::Output { #[cfg(debug_assertions)] return &mut self[idx]; @@ -63,22 +64,8 @@ impl GetDebugChecked for [T] { } } -impl GetDebugChecked for Vec { - type Output = T; - - #[inline] - unsafe fn get_debug_checked(&self, idx: usize) -> &Self::Output { - self.as_slice().get_debug_checked(idx) - } - - #[inline] - unsafe fn get_debug_checked_mut(&mut self, idx: usize) -> &mut Self::Output { - self.as_mut_slice().get_debug_checked_mut(idx) - } -} - // Don't use `Slab::get_unchecked` because there's a panicking branch. https://github.com/tokio-rs/slab/pull/74 -impl GetDebugChecked for Slab { +impl GetDebugChecked for Slab { type Output = T; #[inline] diff --git a/src/blob_vec.rs b/src/blob_vec.rs index c4b58c4..afd3a2f 100644 --- a/src/blob_vec.rs +++ b/src/blob_vec.rs @@ -27,9 +27,6 @@ impl BlobVec { /// - `drop` must be safe to call with elements of this `BlobVec` as /// described by [`DropFn`]'s documentation. pub(crate) unsafe fn new(layout: Layout, drop: DropFn) -> Self { - // // SAFETY: `Layout` guarantees alignment is non-zero. - // let data = NonNull::new(layout.align() as *mut u8).unwrap_debug_checked(); - Self { elem_layout: pad_to_align(&layout), len: 0, diff --git a/src/event.rs b/src/event.rs index b3982a8..5429b57 100644 --- a/src/event.rs +++ b/src/event.rs @@ -1,8 +1,7 @@ //! Types for sending and receiving [`Event`]s. use alloc::borrow::Cow; -use alloc::vec::Vec; -use alloc::{format, vec}; +use alloc::format; use core::alloc::Layout; use core::any::TypeId; use core::marker::PhantomData; @@ -20,7 +19,7 @@ use memoffset::offset_of; use crate::access::Access; use crate::archetype::Archetype; use crate::assert::{ - AssertMutable, AssertTargetedEvent, AssertUntargetedEvent, GetDebugChecked, UnwrapDebugChecked, + AssertMutable, AssertTargetedEvent, AssertUntargetedEvent, GetDebugChecked, UnwrapDebugChecked }; use crate::component::ComponentIdx; use crate::drop::DropFn; @@ -616,12 +615,12 @@ impl EventQueue { } } - pub(crate) unsafe fn get_debug_checked_mut(&mut self, idx: usize) -> &mut EventQueueItem { - self.items.get_debug_checked_mut(idx) + pub(crate) fn pop_front(&mut self) -> Option { + self.items.pop() } #[inline] - pub(crate) unsafe fn push(&mut self, event: E, idx: u32) { + pub(crate) unsafe fn push_front(&mut self, event: E, idx: u32) { let meta = if E::IS_TARGETED { EventMeta::Targeted { idx: TargetedEventIdx(idx), @@ -633,10 +632,19 @@ impl EventQueue { } }; - let event = self.bump.alloc(event) as *mut E as *mut u8; + let event = NonNull::from(self.bump.alloc(event)).cast::(); self.items.push(EventQueueItem { meta, event }); } + /// Reverses elements in the range `from..`. + /// + /// # Safety + /// + /// `from` must be in bounds. + pub(crate) unsafe fn reverse_from(&mut self, from: usize) { + self.items.get_debug_checked_mut(from..).reverse(); + } + pub(crate) fn iter(&self) -> impl Iterator { self.items.iter() } @@ -656,10 +664,6 @@ impl EventQueue { pub(crate) fn is_empty(&self) -> bool { self.len() == 0 } - - pub(crate) unsafe fn set_len(&mut self, new_len: usize) { - self.items.set_len(new_len) - } } // SAFETY: The bump allocator is only accessed behind an exclusive reference to @@ -674,7 +678,7 @@ pub(crate) struct EventQueueItem { pub(crate) meta: EventMeta, /// Type-erased pointer to this event. When null, ownership of the event /// has been transferred and no destructor needs to run. - pub(crate) event: *mut u8, + pub(crate) event: NonNull, } // SAFETY: Events are always Send + Sync. @@ -705,66 +709,44 @@ impl EventMeta { /// Type-erased pointer to an event. Passed to systems in [`System::run`]. /// -/// This is essentially a reference-to-pointer-to-event where the -/// pointer-to-event can be set to null. -/// /// [`System::run`]: crate::system::System::run #[derive(Clone, Copy, Debug)] pub struct EventPtr<'a> { - ptr: NonNull<*mut u8>, + event: NonNull, + // `false` when borrowed, `true` when taken. + ownership_flag: NonNull, _marker: PhantomData<&'a mut u8>, } impl<'a> EventPtr<'a> { - pub(crate) fn new(ptr: NonNull<*mut u8>) -> Self { + pub(crate) fn new(event: NonNull, ownership_flag: NonNull) -> Self { Self { - ptr, + event, + ownership_flag, _marker: PhantomData, } } - /// Reinterprets the pointer as a reference to `E`. - /// - /// # Safety - /// - /// - Must have permission to access the event immutably. - /// - Event data must be safe to reinterpret as an instance of `E`. This - /// implies that layouts of `E` and the event must match. - pub unsafe fn as_event(self) -> &'a E { - &*self.ptr.as_ptr().read().cast::() - } - - /// Reinterprets the pointer as a mutable reference to `E`. - /// - /// # Safety - /// - /// - Must have permission to access the event mutably. - /// - Event data must be safe to reinterpret as an instance of `E`. This - /// implies that layouts of `E` and the event must match. - pub unsafe fn as_event_mut(self) -> EventMut<'a, E> { - EventMut { - ptr: unsafe { &mut *(self.ptr.as_ptr() as *mut *mut E) }, - _marker: PhantomData, - } - } + /// Returns the underlying pointer to the type-erased event. + #[track_caller] + pub fn as_ptr(self) -> NonNull { + let is_owned = unsafe { *self.ownership_flag.as_ptr() }; + debug_assert!(!is_owned, "`as_ptr` cannot be called after the event has been marked as owned"); - /// Gets a reference to the type-erased event pointer. - /// - /// # Safety - /// - /// - Must have permission to access the event immutably. - pub unsafe fn as_ptr(self) -> &'a *const u8 { - &*(self.ptr.as_ptr() as *const *const u8) + self.event } - /// Gets a mutable reference to the type-erased event pointer. The pointer - /// can be set to null to take ownership of the event. + /// Marks the event as owned. It is then the system's responsibility to drop + /// the event. /// /// # Safety /// /// - Must have permission to access the event mutably. - pub unsafe fn as_ptr_mut(self) -> &'a mut *mut u8 { - &mut *self.ptr.as_ptr() + /// - Once the event is set as owned, [`as_ptr`] cannot be called. + /// + /// [`as_ptr`]: Self::as_ptr + pub unsafe fn set_owned(self) { + *self.ownership_flag.as_ptr() = true; } } @@ -773,11 +755,18 @@ impl<'a> EventPtr<'a> { /// To get at `E`, use the [`Deref`] and [`DerefMut`] implementations or /// [`take`](Self::take). pub struct EventMut<'a, E> { - ptr: &'a mut *mut E, + ptr: EventPtr<'a>, _marker: PhantomData<&'a mut E>, } -impl EventMut<'_, E> { +impl<'a, E> EventMut<'a, E> { + fn new(ptr: EventPtr<'a>) -> Self { + Self { + ptr, + _marker: PhantomData, + } + } + /// Takes ownership of the event. Any other systems listening for this event /// will not run. /// @@ -802,25 +791,26 @@ impl EventMut<'_, E> { /// // ownership of the event before the second could run. /// ``` pub fn take(this: Self) -> E { - let res = unsafe { this.ptr.read() }; - - *this.ptr = core::ptr::null_mut(); - + let res = unsafe { this.ptr.as_ptr().as_ptr().cast::().read() }; + unsafe { this.ptr.set_owned() }; res } } +unsafe impl Send for EventMut<'_, E> {} +unsafe impl Sync for EventMut<'_, E> {} + impl Deref for EventMut<'_, E> { type Target = E; fn deref(&self) -> &Self::Target { - unsafe { &**self.ptr } + unsafe { self.ptr.as_ptr().cast::().as_ref() } } } impl DerefMut for EventMut<'_, E> { fn deref_mut(&mut self) -> &mut Self::Target { - unsafe { &mut **self.ptr } + unsafe { self.ptr.as_ptr().cast::().as_mut() } } } @@ -881,7 +871,7 @@ unsafe impl SystemParam for Receiver<'_, E> { // SAFETY: // - We have permission to access the event immutably. // - System was configured to listen for `E`. - event: event_ptr.as_event::(), + event: event_ptr.as_ptr().cast().as_ref(), query: (), } } @@ -929,9 +919,8 @@ unsafe impl SystemParam for Receiver<'_, E, Q> { event_ptr: EventPtr<'a>, world: UnsafeWorldCell<'a>, ) -> Self::Item<'a> { - let event = event_ptr.as_event::(); - assert!(E::IS_TARGETED); + let event = event_ptr.as_ptr().cast::().as_ref(); let target = event.target(); // SAFETY: The target entity is guaranteed to match the query. @@ -998,7 +987,7 @@ unsafe impl SystemParam for ReceiverMut<'_, E> { _world: UnsafeWorldCell<'a>, ) -> Self::Item<'a> { ReceiverMut { - event: event_ptr.as_event_mut(), + event: EventMut::new(event_ptr), query: (), } } @@ -1047,9 +1036,9 @@ unsafe impl SystemParam for ReceiverMut<'_, E, Q> event_ptr: EventPtr<'a>, world: UnsafeWorldCell<'a>, ) -> Self::Item<'a> { - let event = event_ptr.as_event_mut::(); - assert!(E::IS_TARGETED); + + let event = EventMut::::new(event_ptr); let target = event.target(); // SAFETY: The target entity is guaranteed to match the query. @@ -1568,4 +1557,72 @@ mod tests { assert!(world.events().contains(EventId::SPAWN_QUEUED)); assert!(world.remove_event(EventId::SPAWN_QUEUED).is_none()); } + + #[test] + fn change_entity_during_broadcast() { + let mut world = World::new(); + + #[derive(Event)] + struct E(#[event(target)] EntityId); + + #[derive(Component)] + struct C(String); + + world.add_system(|r: Receiver, mut s: Sender>| { + s.remove::(r.event.0); + }); + + world.add_system(|r: Receiver| { + r.query.0.push_str("123"); + }); + + let e = world.spawn(); + world.insert(e, C("abc".into())); + + world.send(E(e)); + } + + #[test] + fn event_order() { + #[derive(Event)] + struct A; + #[derive(Event, Debug)] + struct B(i32); + #[derive(Event, Debug)] + struct C(i32); + + #[derive(Component)] + struct Result(Vec); + + fn get_a_send_b(_: Receiver, mut sender: Sender) { + sender.send(B(0)); + sender.send(B(3)); + } + + fn get_b_send_c(r: Receiver, mut sender: Sender, res: Single<&mut Result>) { + res.0.0.push(r.event.0); + sender.send(C(r.event.0 + 1)); + sender.send(C(r.event.0 + 2)); + } + + fn get_c(r: Receiver, res: Single<&mut Result>) { + res.0.0.push(r.event.0); + } + + let mut world = World::new(); + + let res = world.spawn(); + world.insert(res, Result(vec![])); + + world.add_system(get_a_send_b); + world.add_system(get_b_send_c); + world.add_system(get_c); + + world.send(A); + + assert_eq!( + world.get_component::(res).unwrap().0.as_slice(), + &[0, 1, 2, 3, 4, 5] + ); + } } diff --git a/src/world.rs b/src/world.rs index 6554015..0efceca 100644 --- a/src/world.rs +++ b/src/world.rs @@ -7,7 +7,7 @@ use core::any::{self, TypeId}; use core::cell::UnsafeCell; use core::marker::PhantomData; use core::mem; -use core::ptr::{self, NonNull}; +use core::ptr::NonNull; use crate::archetype::Archetypes; use crate::assert::{AssertMutable, UnwrapDebugChecked}; @@ -517,7 +517,7 @@ impl World { for arch in self.archetypes.iter() { if arch.column_of(component.index()).is_some() { for &entity_id in arch.entity_ids() { - unsafe { self.event_queue.push(Despawn(entity_id), despawn_idx) }; + unsafe { self.event_queue.push_front(Despawn(entity_id), despawn_idx) }; } } } @@ -742,209 +742,190 @@ impl World { /// Send all queued events to systems. The event queue will be empty after /// this call. fn flush_event_queue(&mut self) { - handle_events(0, self); - debug_assert_eq!(self.event_queue.len(), 0); - self.event_queue.clear(); - - fn handle_events(queue_start_idx: usize, world: &mut World) { - 'next_event: for queue_idx in queue_start_idx..world.event_queue.len() { - let item = unsafe { world.event_queue.get_debug_checked_mut(queue_idx) }; - let event_meta = item.meta; - let event_info = unsafe { - world - .events - .get_by_index(event_meta.event_idx()) - .unwrap_debug_checked() - }; - let event_kind = event_info.kind(); - - // Put the event pointer on the stack because pointers into the event queue - // would be invalidated by pushes. - let mut event = EventDropper { - // Set pointer to null so the `World`'s destructor will know we've taken - // ownership of the event. - event: mem::replace(&mut item.event, ptr::null_mut()), - drop: event_info.drop(), - }; - - struct EventDropper { - event: *mut u8, - drop: DropFn, - } + 'next_event: while let Some(item) = self.event_queue.pop_front() { + let event_meta = item.meta; + let event_info = unsafe { + self.events + .get_by_index(event_meta.event_idx()) + .unwrap_debug_checked() + }; + let event_kind = event_info.kind(); + + // In case `System::run` unwinds, we need to drop the event we're holding on the + // stack. The other events in the event queue will be handled by + // `World`'s destructor. + struct EventDropper { + event: NonNull, + drop: DropFn, + ownership_flag: bool, + } - impl EventDropper { - /// Extracts the event pointer and drop fn without running - /// the destructor. - #[inline] - fn unpack(self) -> (*mut u8, DropFn) { - let event = self.event; - let drop = self.drop; - mem::forget(self); + impl EventDropper { + /// Extracts the event pointer and drop fn without running + /// the destructor. + #[inline] + fn unpack(self) -> (NonNull, DropFn) { + let event = self.event; + let drop = self.drop; + mem::forget(self); - (event, drop) - } + (event, drop) } + } - // In case `System::run` or `handle_events` unwinds, we need to drop the event - // we're holding on the stack. The other events in the event queue will be - // handled by `World`'s destructor. - impl Drop for EventDropper { - #[inline] - fn drop(&mut self) { - if let (Some(event), Some(drop)) = (NonNull::new(self.event), self.drop) { - unsafe { drop(event) }; + impl Drop for EventDropper { + #[cold] + fn drop(&mut self) { + if !self.ownership_flag { + if let Some(drop) = self.drop { + unsafe { drop(self.event) }; } } } + } - let system_list = match event_meta { - EventMeta::Untargeted { idx } => unsafe { - world - .systems - .get_untargeted_list(idx) + let mut event = EventDropper { + event: item.event, + drop: event_info.drop(), + ownership_flag: false, + }; + + let system_list = match event_meta { + EventMeta::Untargeted { idx } => unsafe { + self.systems.get_untargeted_list(idx).unwrap_debug_checked() + }, + EventMeta::Targeted { idx, target } => { + let Some(location) = self.entities.get(target) else { + continue; + }; + + let arch = unsafe { + self.archetypes + .get(location.archetype) .unwrap_debug_checked() - }, - EventMeta::Targeted { idx, target } => { - let Some(location) = world.entities.get(target) else { - continue; - }; + }; - let arch = unsafe { - world - .archetypes - .get(location.archetype) - .unwrap_debug_checked() - }; + static EMPTY: SystemList = SystemList::new(); - static EMPTY: SystemList = SystemList::new(); + // Return an empty system list instead of continuing in case this event is + // special. + arch.system_list_for(idx).unwrap_or(&EMPTY) + } + }; - // Return an empty system list instead of continuing in case this event is - // special. - arch.system_list_for(idx).unwrap_or(&EMPTY) - } - }; + let systems: *const [_] = system_list.systems(); - let systems: *const [_] = system_list.systems(); + for info_ptr in unsafe { &*systems } { + let system = unsafe { &mut (*info_ptr.as_ptr()).system }; - for info_ptr in unsafe { &*systems } { - let events_before = world.event_queue.len(); + let info = unsafe { SystemInfo::ref_from_ptr(info_ptr) }; - let system = unsafe { &mut (*info_ptr.as_ptr()).system }; + let event_ptr = + EventPtr::new(event.event, NonNull::from(&mut event.ownership_flag)); - let info = unsafe { SystemInfo::ref_from_ptr(info_ptr) }; + let events_before = self.event_queue.len(); - let event_ptr = EventPtr::new(NonNull::from(&mut event.event)); - let world_cell = world.unsafe_cell_mut(); + let world_cell = self.unsafe_cell_mut(); - unsafe { system.run(info, event_ptr, world_cell) }; + unsafe { system.run(info, event_ptr, world_cell) }; - let events_after = world.event_queue.len(); + unsafe { self.event_queue.reverse_from(events_before) }; + + // Did the system take ownership of the event? + if event.ownership_flag { + event.unpack(); + continue 'next_event; + } + } - if events_before < events_after { - // Eagerly handle any events produced by the system. - handle_events(events_before, world); + match event_kind { + EventKind::Normal => { + // Ordinary event. Run drop fn. + if let (ptr, Some(drop)) = event.unpack() { + unsafe { drop(ptr) }; } + } + EventKind::Insert { + component_idx, + component_offset, + } => { + let entity_id = unsafe { *event.event.as_ptr().cast::() }; + + if let Some(loc) = self.entities.get(entity_id) { + let dst = unsafe { + self.archetypes.traverse_insert( + loc.archetype, + component_idx, + &mut self.components, + &mut self.systems, + ) + }; - debug_assert_eq!(world.event_queue.len(), events_before); + let component_ptr = + unsafe { event.event.as_ptr().add(component_offset as usize) } + .cast_const(); + + unsafe { + self.archetypes.move_entity( + loc, + dst, + [(component_idx, component_ptr)], + &mut self.entities, + ) + }; - // Did the system take ownership of the event? - if event.event.is_null() { - // Event is null; destructor wouldn't do anything. + // Inserted component is owned by the archetype now. We wait to unpack + // in case one of the above functions panics. event.unpack(); - - continue 'next_event; } } + EventKind::Remove { component_idx } => { + // `Remove` doesn't need drop. + let (event, _) = event.unpack(); + + // SAFETY: `Remove` is `repr(transparent)` with the first field being the + // `EntityId`, so we can safely reinterpret this pointer. + let entity_id = unsafe { *event.as_ptr().cast::() }; + + if let Some(loc) = self.entities.get(entity_id) { + let dst = unsafe { + self.archetypes.traverse_remove( + loc.archetype, + component_idx, + &mut self.components, + &mut self.systems, + ) + }; - match event_kind { - EventKind::Normal => { - // Ordinary event. Run event dropper destructor. - } - EventKind::Insert { - component_idx, - component_offset, - } => { - let entity_id = unsafe { *event.event.cast::() }; - - if let Some(loc) = world.entities.get(entity_id) { - let dst = unsafe { - world.archetypes.traverse_insert( - loc.archetype, - component_idx, - &mut world.components, - &mut world.systems, - ) - }; - - let component_ptr = - unsafe { event.event.add(component_offset as usize) }.cast_const(); - - unsafe { - world.archetypes.move_entity( - loc, - dst, - [(component_idx, component_ptr)], - &mut world.entities, - ) - }; - - // Inserted component is owned by the archetype now. We wait to unpack - // in case one of the above functions panics. - event.unpack(); - } - } - EventKind::Remove { component_idx } => { - // `Remove` doesn't need drop. - let (event, _) = event.unpack(); - - // SAFETY: `Remove` is `repr(transparent)` with the first field being the - // `EntityId`, so we can safely reinterpret this pointer. - let entity_id = unsafe { *event.cast::() }; - - if let Some(loc) = world.entities.get(entity_id) { - let dst = unsafe { - world.archetypes.traverse_remove( - loc.archetype, - component_idx, - &mut world.components, - &mut world.systems, - ) - }; - - unsafe { - world - .archetypes - .move_entity(loc, dst, [], &mut world.entities) - }; - } - } - EventKind::SpawnQueued => { - // `SpawnQueued` doesn't need drop. - let _ = event.unpack(); - - // Spawn one entity from the reserved entity queue. - world - .reserved_entities - .spawn_one(&mut world.entities, |id| world.archetypes.spawn(id)); + unsafe { + self.archetypes + .move_entity(loc, dst, [], &mut self.entities) + }; } - EventKind::Despawn => { - // `Despawn` doesn't need drop. - let (event, _) = event.unpack(); + } + EventKind::SpawnQueued => { + // `SpawnQueued` doesn't need drop. + let _ = event.unpack(); + + // Spawn one entity from the reserved entity queue. + self.reserved_entities + .spawn_one(&mut self.entities, |id| self.archetypes.spawn(id)); + } + EventKind::Despawn => { + // `Despawn` doesn't need drop. + let (event, _) = event.unpack(); - let entity_id = unsafe { *event.cast::() }.0; + let entity_id = unsafe { *event.as_ptr().cast::() }.0; - world - .archetypes - .remove_entity(entity_id, &mut world.entities); + self.archetypes.remove_entity(entity_id, &mut self.entities); - // Reset next key iter. - world.reserved_entities.refresh(&world.entities); - } + // Reset next key iter. + self.reserved_entities.refresh(&self.entities); } } - - unsafe { world.event_queue.set_len(queue_start_idx) }; } + + self.event_queue.clear(); } /// Returns a new [`UnsafeWorldCell`] with permission to _read_ all data in @@ -977,16 +958,14 @@ impl Drop for World { // Drop in-flight events still in the event queue. This can happen if a panic // occurs. for item in self.event_queue.iter() { - if let Some(event) = NonNull::new(item.event) { - let info = unsafe { - self.events - .get_by_index(item.meta.event_idx()) - .unwrap_debug_checked() - }; - - if let Some(drop) = info.drop() { - unsafe { drop(event) }; - } + let info = unsafe { + self.events + .get_by_index(item.meta.event_idx()) + .unwrap_debug_checked() + }; + + if let Some(drop) = info.drop() { + unsafe { drop(item.event) }; } } } @@ -1004,7 +983,7 @@ impl Sender<'_> { /// Enqueue an event. pub fn send(&mut self, event: E) { let idx = self.world.add_event::().index().as_u32(); - unsafe { self.world.event_queue.push(event, idx) }; + unsafe { self.world.event_queue.push_front(event, idx) }; } /// Enqueue the spawning of an entity and [`Spawn`] event. Returns the @@ -1014,7 +993,7 @@ impl Sender<'_> { unsafe { self.world .event_queue - .push(SpawnQueued, EventId::SPAWN_QUEUED.index().as_u32()) + .push_front(SpawnQueued, EventId::SPAWN_QUEUED.index().as_u32()) } self.send(Spawn(id)); id @@ -1052,7 +1031,7 @@ impl<'a> UnsafeWorldCell<'a> { /// - Must have permission to access the event queue mutably. /// - Event index must be correct for the given event. pub unsafe fn send_with_index(self, event: E, idx: u32) { - unsafe { (*self.world.as_ptr()).event_queue.push(event, idx) } + unsafe { (*self.world.as_ptr()).event_queue.push_front(event, idx) } } /// # Safety diff --git a/tutorial/ch03_sending_events_from_systems.md b/tutorial/ch03_sending_events_from_systems.md index 6658723..f63cf26 100644 --- a/tutorial/ch03_sending_events_from_systems.md +++ b/tutorial/ch03_sending_events_from_systems.md @@ -36,20 +36,34 @@ got C! In the parameter `Sender<(B, C)>`, the `(B, C)` is the set of events the sender is allowed to send. Attempting to send an event that is not in this set will fail. -Note that [`Sender::send`] does not immediately send the event. It is instead added to an _event queue_, which is flushed once the system returns. - -Finally, note that events are evaluated recursively – events sent by systems will finish broadcasting before the outer event has finished broadcasting. -This is similar to a call stack or pre-order tree traversal. Consider the program: +Note that [`Sender::send`] does not immediately send events, but rather adds them to the front of the **event queue** in reverse order. +The next event in the queue begins broadcasting once all handlers for the current event have finished. ```rust use evenio::prelude::*; #[derive(Event)] struct A; -#[derive(Event)] -struct B; -#[derive(Event)] -struct C; +#[derive(Event, Debug)] +struct B(i32); +#[derive(Event, Debug)] +struct C(i32); + +fn get_a_send_b(_: Receiver, mut sender: Sender) { + sender.send(B(0)); + sender.send(B(3)); + println!("got A, sending B twice!"); +} + +fn get_b_send_c(r: Receiver, mut sender: Sender) { + sender.send(C(r.event.0 + 1)); + sender.send(C(r.event.0 + 2)); + println!("got {:?}, sending C twice!", r.event); +} + +fn get_c(r: Receiver) { + println!("got {:?}!", r.event); +} let mut world = World::new(); @@ -59,50 +73,18 @@ world.add_system(get_c); println!("sending A!"); world.send(A); - -fn get_a_send_b(_: Receiver, mut sender: Sender) { - sender.send(B); - sender.send(B); - println!("got A, sending B twice!"); -} - -fn get_b_send_c(_: Receiver, mut sender: Sender) { - sender.send(C); - sender.send(C); - println!("got B, sending C twice!"); -} - -fn get_c(_: Receiver) { - println!("got C!"); -} ``` Output: ```txt sending A! got A, sending B twice! -got B, sending C twice! -got C! -got C! -got B, sending C twice! -got C! -got C! -``` - -The control flow of the above program can be visualized with the following diagram: - -```txt - ┌─┐ - ┌───┤A├───┐ - │ └─┘ │ - │ │ - ┌▼┐ ┌▼┐ - ┌─┤B├─┐ ┌─┤B├─┐ - │ └─┘ │ │ └─┘ │ - │ │ │ │ -┌▼┐ ┌▼┐ ┌▼┐ ┌▼┐ -│C│ │C│ │C│ │C│ -└─┘ └─┘ └─┘ └─┘ +got B(0), sending C twice! +got C(1)! +got C(2)! +got B(3), sending C twice! +got C(4)! +got C(5)! ``` [`World::send`]: crate::world::World::send From 5a537429ff58be09d4a38b77f7c0b028fe29a9a8 Mon Sep 17 00:00:00 2001 From: Ryan Date: Mon, 12 Feb 2024 05:59:40 -0800 Subject: [PATCH 2/3] fmt --- src/event.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/event.rs b/src/event.rs index 5429b57..771300c 100644 --- a/src/event.rs +++ b/src/event.rs @@ -19,7 +19,7 @@ use memoffset::offset_of; use crate::access::Access; use crate::archetype::Archetype; use crate::assert::{ - AssertMutable, AssertTargetedEvent, AssertUntargetedEvent, GetDebugChecked, UnwrapDebugChecked + AssertMutable, AssertTargetedEvent, AssertUntargetedEvent, GetDebugChecked, UnwrapDebugChecked, }; use crate::component::ComponentIdx; use crate::drop::DropFn; @@ -637,9 +637,9 @@ impl EventQueue { } /// Reverses elements in the range `from..`. - /// + /// /// # Safety - /// + /// /// `from` must be in bounds. pub(crate) unsafe fn reverse_from(&mut self, from: usize) { self.items.get_debug_checked_mut(from..).reverse(); @@ -731,7 +731,10 @@ impl<'a> EventPtr<'a> { #[track_caller] pub fn as_ptr(self) -> NonNull { let is_owned = unsafe { *self.ownership_flag.as_ptr() }; - debug_assert!(!is_owned, "`as_ptr` cannot be called after the event has been marked as owned"); + debug_assert!( + !is_owned, + "`as_ptr` cannot be called after the event has been marked as owned" + ); self.event } @@ -743,7 +746,7 @@ impl<'a> EventPtr<'a> { /// /// - Must have permission to access the event mutably. /// - Once the event is set as owned, [`as_ptr`] cannot be called. - /// + /// /// [`as_ptr`]: Self::as_ptr pub unsafe fn set_owned(self) { *self.ownership_flag.as_ptr() = true; @@ -1600,13 +1603,13 @@ mod tests { } fn get_b_send_c(r: Receiver, mut sender: Sender, res: Single<&mut Result>) { - res.0.0.push(r.event.0); + res.0 .0.push(r.event.0); sender.send(C(r.event.0 + 1)); sender.send(C(r.event.0 + 2)); } fn get_c(r: Receiver, res: Single<&mut Result>) { - res.0.0.push(r.event.0); + res.0 .0.push(r.event.0); } let mut world = World::new(); From 535e2cc74f829d54843a2eded3a4a593b7b87862 Mon Sep 17 00:00:00 2001 From: Ryan Date: Mon, 12 Feb 2024 06:10:57 -0800 Subject: [PATCH 3/3] Compile correctly with --no-default-features --- src/archetype.rs | 6 +++++- src/event.rs | 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/archetype.rs b/src/archetype.rs index fd50516..18a8ac6 100644 --- a/src/archetype.rs +++ b/src/archetype.rs @@ -9,6 +9,7 @@ use core::cmp::Ordering; use core::ptr; use core::ptr::NonNull; +use ahash::RandomState; use slab::Slab; use crate::assert::{assume_debug_checked, GetDebugChecked, UnwrapDebugChecked}; @@ -47,9 +48,12 @@ pub struct Archetypes { impl Archetypes { pub(crate) fn new() -> Self { + let mut map = HashMap::with_hasher(RandomState::new()); + map.insert(vec![].into_boxed_slice(), ArchetypeIdx::EMPTY); + Self { archetypes: Slab::from_iter([(0, Archetype::empty())]), - by_components: HashMap::from_iter([(vec![].into_boxed_slice(), ArchetypeIdx::EMPTY)]), + by_components: map, } } diff --git a/src/event.rs b/src/event.rs index 771300c..d805c27 100644 --- a/src/event.rs +++ b/src/event.rs @@ -1,7 +1,8 @@ //! Types for sending and receiving [`Event`]s. use alloc::borrow::Cow; -use alloc::format; +use alloc::vec::Vec; +use alloc::{format, vec}; use core::alloc::Layout; use core::any::TypeId; use core::marker::PhantomData;