From 61dfcf1ba2049bf0375821652e49b0e4c4147623 Mon Sep 17 00:00:00 2001 From: HampusM Date: Fri, 29 Mar 2024 14:20:21 +0100 Subject: feat(ecs): make World unwind safe --- Cargo.lock | 2 + ecs/Cargo.toml | 5 ++ ecs/src/actions.rs | 3 +- ecs/src/component.rs | 36 ++++++++----- ecs/src/lib.rs | 94 +++++++++++++++++++++++++++----- ecs/src/lock.rs | 130 +++++++++++++++++++++++++++++++++++++++++++++ ecs/src/system.rs | 21 ++++---- ecs/src/system/stateful.rs | 17 +++--- ecs/src/type_name.rs | 5 ++ 9 files changed, 266 insertions(+), 47 deletions(-) create mode 100644 ecs/src/lock.rs create mode 100644 ecs/src/type_name.rs diff --git a/Cargo.lock b/Cargo.lock index d0f7732..3b32a48 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -125,6 +125,8 @@ dependencies = [ "ecs-macros", "paste", "seq-macro", + "thiserror", + "tracing", "util-macros", ] diff --git a/ecs/Cargo.toml b/ecs/Cargo.toml index 44f1b24..ad0f373 100644 --- a/ecs/Cargo.toml +++ b/ecs/Cargo.toml @@ -3,9 +3,14 @@ name = "ecs" version = "0.1.0" edition = "2021" +[features] +debug = ["dep:tracing"] + [dependencies] seq-macro = "0.3.5" paste = "1.0.14" +thiserror = "1.0.49" +tracing = { version = "0.1.39", optional = true } ecs-macros = { path = "../ecs-macros" } util-macros = { path = "../util-macros" } diff --git a/ecs/src/actions.rs b/ecs/src/actions.rs index edfee55..4507e71 100644 --- a/ecs/src/actions.rs +++ b/ecs/src/actions.rs @@ -19,7 +19,8 @@ impl<'world> Actions<'world> { self.world_data .action_queue - .borrow_mut() + .write_nonblock() + .expect("Failed to aquire read-write action queue lock") .push(Action::Spawn(components.into_vec())); } } diff --git a/ecs/src/component.rs b/ecs/src/component.rs index 07701c8..120ba30 100644 --- a/ecs/src/component.rs +++ b/ecs/src/component.rs @@ -1,19 +1,20 @@ use std::any::{Any, TypeId}; -use std::cell::{RefCell, RefMut}; use std::fmt::Debug; use std::ops::{Deref, DerefMut}; use seq_macro::seq; +use crate::lock::{Lock, WriteGuard}; use crate::system::{ ComponentRefMut, Input as SystemInput, Param as SystemParam, System, }; +use crate::type_name::TypeName; use crate::WorldData; -pub trait Component: SystemInput + Any +pub trait Component: SystemInput + Any + TypeName { #[doc(hidden)] fn as_any_mut(&mut self) -> &mut dyn Any; @@ -48,6 +49,14 @@ impl Debug for dyn Component } } +impl TypeName for Box +{ + fn type_name(&self) -> &'static str + { + self.as_ref().type_name() + } +} + /// A sequence of components. pub trait Sequence { @@ -59,7 +68,9 @@ pub trait Sequence fn type_ids() -> Vec; - fn from_components(components: &[RefCell>]) -> Self::Refs<'_>; + fn from_components<'component>( + components: impl Iterator>>, + ) -> Self::Refs<'component>; } macro_rules! inner { @@ -83,18 +94,18 @@ macro_rules! inner { ] } - fn from_components( - components: &[RefCell>] - ) -> Self::Refs<'_> + fn from_components<'component>( + components: impl Iterator>>, + ) -> Self::Refs<'component> { #( - let mut comp_~I: Option>> = None; + let mut comp_~I: Option>> = None; )* for comp in components { - let Ok(comp_ref) = comp.try_borrow_mut() else { - continue; - }; + let comp_ref = comp + .write_nonblock() + .expect("Failed to acquire read-write component lock"); #( if comp_ref.is::() { @@ -106,10 +117,7 @@ macro_rules! inner { (#( ComponentRefMut::new( - RefMut::filter_map( - comp_~I.unwrap(), - |component| component.downcast_mut::() - ).expect("Failed to downcast component") + comp_~I.unwrap(), ), )*) } diff --git a/ecs/src/lib.rs b/ecs/src/lib.rs index 18e7381..11e67f7 100644 --- a/ecs/src/lib.rs +++ b/ecs/src/lib.rs @@ -1,15 +1,17 @@ #![deny(clippy::all, clippy::pedantic)] -use std::any::{Any, TypeId}; -use std::cell::{Ref, RefCell}; +use std::any::{type_name, Any, TypeId}; use std::collections::{HashMap, HashSet}; use std::fmt::Debug; use std::marker::PhantomData; +use std::ops::RangeBounds; use std::slice::Iter as SliceIter; +use std::vec::Drain; use crate::actions::Action; use crate::component::{Component, Sequence as ComponentSequence}; use crate::event::{Event, Id as EventId}; +use crate::lock::{Lock, ReadGuard}; use crate::system::{ NoInitParamFlag as NoInitSystemParamFlag, Param as SystemParam, @@ -17,19 +19,28 @@ use crate::system::{ TypeErased as TypeErasedSystem, }; use crate::tuple::FilterExclude as TupleFilterExclude; +use crate::type_name::TypeName; pub mod actions; pub mod component; pub mod event; +pub mod lock; pub mod system; pub mod tuple; +pub mod type_name; pub use ecs_macros::Component; #[derive(Debug, Default)] struct Entity { - components: Vec>>, + components: Vec, +} + +#[derive(Debug)] +struct EntityComponent +{ + component: Lock>, } #[derive(Debug, Default)] @@ -53,13 +64,14 @@ impl World { self.data .component_storage - .borrow_mut() + .write_nonblock() + .expect("Failed to acquire read-write component storage lock") .entities .push(Entity { components: components .into_vec() .into_iter() - .map(RefCell::new) + .map(|component| EntityComponent { component: Lock::new(component) }) .collect(), }); } @@ -110,17 +122,26 @@ impl World /// Peforms the actions that have been queued up using [`Actions`]. pub fn perform_queued_actions(&self) { - for action in self.data.action_queue.borrow_mut().drain(..) { + for action in self + .data + .action_queue + .write_nonblock() + .expect("Failed to aquire read-write action queue lock") + .drain(..) + { match action { Action::Spawn(components) => { self.data .component_storage - .borrow_mut() + .write_nonblock() + .expect("Failed to acquire read-write component storage lock") .entities .push(Entity { components: components .into_iter() - .map(RefCell::new) + .map(|component| EntityComponent { + component: Lock::new(component), + }) .collect(), }); } @@ -133,8 +154,35 @@ impl World pub struct WorldData { events: HashMap>, - component_storage: RefCell, - action_queue: RefCell>, + component_storage: Lock, + action_queue: Lock, +} + +#[derive(Debug, Default)] +struct ActionQueue +{ + queue: Vec, +} + +impl ActionQueue +{ + fn push(&mut self, action: Action) + { + self.queue.push(action); + } + + fn drain(&mut self, range: impl RangeBounds) -> Drain + { + self.queue.drain(range) + } +} + +impl TypeName for ActionQueue +{ + fn type_name(&self) -> &'static str + { + type_name::() + } } #[derive(Debug)] @@ -142,7 +190,7 @@ pub struct Query<'world, Comps> where Comps: ComponentSequence, { - component_storage: Ref<'world, ComponentStorage>, + component_storage: ReadGuard<'world, ComponentStorage>, comps_pd: PhantomData, } @@ -164,7 +212,10 @@ where fn new(world_data: &'world WorldData) -> Self { Self { - component_storage: world_data.component_storage.borrow(), + component_storage: world_data + .component_storage + .read_nonblock() + .expect("Failed to acquire read-only component storage lock"), comps_pd: PhantomData, } } @@ -268,7 +319,12 @@ where let matching_entity = find_entity_with_components(&mut self.entity_iter, &self.component_type_ids)?; - Some(Comps::from_components(&matching_entity.components)) + Some(Comps::from_components( + matching_entity + .components + .iter() + .map(|component| &component.component), + )) } } @@ -283,7 +339,9 @@ fn find_entity_with_components<'world>( let entity_components = entity .components .iter() - .filter_map(|component| Some(component.try_borrow().ok()?.as_ref().type_id())) + .filter_map(|component| { + Some(component.component.read_nonblock().ok()?.as_ref().type_id()) + }) .collect::>(); if component_type_ids @@ -302,3 +360,11 @@ pub struct ComponentStorage { entities: Vec, } + +impl TypeName for ComponentStorage +{ + fn type_name(&self) -> &'static str + { + type_name::() + } +} diff --git a/ecs/src/lock.rs b/ecs/src/lock.rs new file mode 100644 index 0000000..ff46761 --- /dev/null +++ b/ecs/src/lock.rs @@ -0,0 +1,130 @@ +use std::ops::{Deref, DerefMut}; +use std::sync::{RwLock, RwLockReadGuard, RwLockWriteGuard, TryLockError}; + +use crate::type_name::TypeName; + +#[derive(Debug, Default)] +pub struct Lock +where + Value: TypeName, +{ + inner: RwLock, +} + +impl Lock +where + Value: TypeName, +{ + pub fn new(value: Value) -> Self + { + Self { inner: RwLock::new(value) } + } + + pub fn read_nonblock(&self) -> Result, Error> + { + let guard = self.inner.try_read().or_else(|err| match err { + TryLockError::WouldBlock => Err(Error::Unavailable), + TryLockError::Poisoned(poison_err) => Ok(poison_err.into_inner()), + })?; + + #[cfg(feature = "debug")] + tracing::trace!("Acquired lock to value of type {}", guard.type_name()); + + Ok(ReadGuard { inner: guard }) + } + + pub fn write_nonblock(&self) -> Result, Error> + { + let guard = self.inner.try_write().or_else(|err| match err { + TryLockError::WouldBlock => Err(Error::Unavailable), + TryLockError::Poisoned(poison_err) => Ok(poison_err.into_inner()), + })?; + + #[cfg(feature = "debug")] + tracing::trace!( + "Acquired mutable lock to value of type {}", + guard.type_name() + ); + + Ok(WriteGuard { inner: guard }) + } +} + +#[derive(Debug, thiserror::Error)] +pub enum Error +{ + #[error("Lock is unavailable")] + Unavailable, +} + +#[derive(Debug)] +pub struct ReadGuard<'guard, Value> +where + Value: TypeName, +{ + inner: RwLockReadGuard<'guard, Value>, +} + +impl<'guard, Value> Deref for ReadGuard<'guard, Value> +where + Value: TypeName, +{ + type Target = Value; + + fn deref(&self) -> &Self::Target + { + &self.inner + } +} + +impl<'guard, Value> Drop for ReadGuard<'guard, Value> +where + Value: TypeName, +{ + fn drop(&mut self) + { + #[cfg(feature = "debug")] + tracing::trace!("Dropped lock to value of type {}", self.type_name()); + } +} + +#[derive(Debug)] +pub struct WriteGuard<'guard, Value> +where + Value: TypeName, +{ + inner: RwLockWriteGuard<'guard, Value>, +} + +impl<'guard, Value> Deref for WriteGuard<'guard, Value> +where + Value: TypeName, +{ + type Target = Value; + + fn deref(&self) -> &Self::Target + { + &self.inner + } +} + +impl<'guard, Value> DerefMut for WriteGuard<'guard, Value> +where + Value: TypeName, +{ + fn deref_mut(&mut self) -> &mut Self::Target + { + &mut self.inner + } +} + +impl<'guard, Value> Drop for WriteGuard<'guard, Value> +where + Value: TypeName, +{ + fn drop(&mut self) + { + #[cfg(feature = "debug")] + tracing::trace!("Dropped mutable lock to value of type {}", self.type_name()); + } +} diff --git a/ecs/src/system.rs b/ecs/src/system.rs index 76db2b5..3440a57 100644 --- a/ecs/src/system.rs +++ b/ecs/src/system.rs @@ -1,13 +1,15 @@ use std::any::Any; -use std::cell::RefMut; use std::convert::Infallible; use std::fmt::Debug; +use std::marker::PhantomData; use std::ops::{Deref, DerefMut}; +use std::panic::{RefUnwindSafe, UnwindSafe}; use std::ptr::addr_of; use seq_macro::seq; use crate::component::Component; +use crate::lock::WriteGuard; use crate::system::util::check_params_are_compatible; use crate::tuple::{FilterElement as TupleFilterElement, With as TupleWith}; use crate::WorldData; @@ -45,7 +47,7 @@ macro_rules! impl_system { impl<'world, Func, #(TParam~I,)*> System<'world, fn(#(TParam~I,)*)> for Func where - Func: Fn(#(TParam~I,)*) + Copy + 'static, + Func: Fn(#(TParam~I,)*) + Copy + RefUnwindSafe + UnwindSafe + 'static, #(TParam~I: Param<'world, Flags = NoInitParamFlag>,)* { type Input = Infallible; @@ -125,7 +127,7 @@ pub trait Into pub struct TypeErased { - data: Box, + data: Box, func: Box, } @@ -153,7 +155,7 @@ impl Debug for TypeErased } /// Function in [`TypeErased`] used to run the system. -type TypeErasedFunc = dyn Fn(&dyn Any, &WorldData); +type TypeErasedFunc = dyn Fn(&dyn Any, &WorldData) + RefUnwindSafe + UnwindSafe; /// A parameter to a [`System`]. /// @@ -194,14 +196,15 @@ where #[derive(Debug)] pub struct ComponentRefMut<'a, ComponentT: Component> { - inner: RefMut<'a, ComponentT>, + inner: WriteGuard<'a, Box>, + _ph: PhantomData, } impl<'a, ComponentT: Component> ComponentRefMut<'a, ComponentT> { - pub(crate) fn new(inner: RefMut<'a, ComponentT>) -> Self + pub(crate) fn new(inner: WriteGuard<'a, Box>) -> Self { - Self { inner } + Self { inner, _ph: PhantomData } } } @@ -211,7 +214,7 @@ impl<'a, ComponentT: Component> Deref for ComponentRefMut<'a, ComponentT> fn deref(&self) -> &Self::Target { - &self.inner + self.inner.downcast_ref().unwrap() } } @@ -219,6 +222,6 @@ impl<'a, ComponentT: Component> DerefMut for ComponentRefMut<'a, ComponentT> { fn deref_mut(&mut self) -> &mut Self::Target { - &mut self.inner + self.inner.downcast_mut().unwrap() } } diff --git a/ecs/src/system/stateful.rs b/ecs/src/system/stateful.rs index 7ce87fa..8d56a13 100644 --- a/ecs/src/system/stateful.rs +++ b/ecs/src/system/stateful.rs @@ -1,10 +1,11 @@ use std::any::{type_name, Any, TypeId}; -use std::cell::{RefCell, RefMut}; use std::collections::HashMap; +use std::panic::{RefUnwindSafe, UnwindSafe}; use seq_macro::seq; use crate::component::Component; +use crate::lock::Lock; use crate::system::util::check_params_are_compatible; use crate::system::{ComponentRefMut, Into as IntoSystem, Param, System, TypeErased}; use crate::tuple::{ @@ -20,7 +21,7 @@ use crate::WorldData; pub struct Stateful { func: Func, - local_components: HashMap>>, + local_components: HashMap>>, } macro_rules! impl_system { @@ -29,7 +30,7 @@ macro_rules! impl_system { impl<'world, Func, #(TParam~I,)*> System<'world, fn(&'world (), #(TParam~I,)*)> for Stateful where - Func: Fn(#(TParam~I,)*) + Copy + 'static, + Func: Fn(#(TParam~I,)*) + Copy + RefUnwindSafe + UnwindSafe + 'static, #(TParam~I: Param<'world>,)* #(TParam~I::Input: 'static,)* (#(TParam~I::Input,)*): TupleFilter, @@ -119,12 +120,10 @@ macro_rules! impl_system { { let local_component = self.local_components .get(&TypeId::of::())? - .borrow_mut(); + .write_nonblock() + .expect("Failed to aquire read-write local component lock"); - Some(ComponentRefMut::new(RefMut::filter_map( - local_component, - |local_component| local_component.downcast_mut() - ).ok()?)) + Some(ComponentRefMut::new(local_component)) } fn set_local_component( @@ -135,7 +134,7 @@ macro_rules! impl_system { self.local_components .insert( TypeId::of::(), - RefCell::new(Box::new(local_component)) + Lock::new(Box::new(local_component)) ); } } diff --git a/ecs/src/type_name.rs b/ecs/src/type_name.rs new file mode 100644 index 0000000..5892c6f --- /dev/null +++ b/ecs/src/type_name.rs @@ -0,0 +1,5 @@ +pub trait TypeName +{ + /// Returns the name of this type. + fn type_name(&self) -> &'static str; +} -- cgit v1.2.3-18-g5258