From 58f29444ac234c285ce0e3a729ade2b52a303f58 Mon Sep 17 00:00:00 2001 From: HampusM Date: Wed, 21 Aug 2024 20:18:37 +0200 Subject: refactor(ecs): fix clippy lints --- ecs/src/actions.rs | 2 +- ecs/src/component/storage.rs | 37 +++++++++------------- ecs/src/event.rs | 1 + ecs/src/event/component.rs | 6 ++-- ecs/src/extension.rs | 4 +-- ecs/src/lib.rs | 75 ++++++++++++++++++-------------------------- ecs/src/lock.rs | 5 +-- ecs/src/query.rs | 8 ++--- ecs/src/relationship.rs | 30 ++++++++++++++++-- ecs/src/util.rs | 6 ++-- 10 files changed, 92 insertions(+), 82 deletions(-) (limited to 'ecs') diff --git a/ecs/src/actions.rs b/ecs/src/actions.rs index 32fcfb9..5cd7b00 100644 --- a/ecs/src/actions.rs +++ b/ecs/src/actions.rs @@ -66,7 +66,7 @@ impl<'world> Actions<'world> fn new(action_queue: &'world Arc) -> Self { Self { - action_queue: &*action_queue, + action_queue, action_queue_weak: Arc::downgrade(action_queue), } } diff --git a/ecs/src/component/storage.rs b/ecs/src/component/storage.rs index 1bb7fb5..00ba330 100644 --- a/ecs/src/component/storage.rs +++ b/ecs/src/component/storage.rs @@ -38,12 +38,12 @@ impl Storage components_metadata.sort_by_key_b(|component_metadata| component_metadata.id); let archetype_id = ArchetypeId::from_components_metadata( - components_metadata.borrow().into_iter().cloned(), + components_metadata.borrow().iter().cloned(), ); // This looks stupid but the borrow checker complains otherwise if self.archetype_lookup.borrow().contains_key(&archetype_id) { - return self.iter_archetypes_by_lookup(&archetype_id); + return self.iter_archetypes_by_lookup(archetype_id); } let comp_ids_set = create_non_opt_component_id_set(components_metadata.borrow()); @@ -69,7 +69,7 @@ impl Storage }, ); - self.iter_archetypes_by_lookup(&archetype_id) + self.iter_archetypes_by_lookup(archetype_id) } pub fn get_entity_archetype(&self, entity_uid: EntityUid) -> Option<&Archetype> @@ -150,8 +150,7 @@ impl Storage let contains_component_already = components .iter() - .find(|component| archetype.component_ids.contains_key(&component.id())) - .is_some(); + .any(|component| archetype.component_ids.contains_key(&component.id())); if contains_component_already { let component_cnt = components.len(); @@ -162,8 +161,7 @@ impl Storage entity_uid, components .iter() - .map(|component| [component.type_name(), ", "]) - .flatten() + .flat_map(|component| [component.type_name(), ", "]) .enumerate() .take_while(|(index, _)| { *index < (component_cnt * 2) - 1 }) .map(|(_, component)| component) @@ -179,7 +177,7 @@ impl Storage .components .into_iter() .map(|component| component.component.into_inner()) - .chain(components.into_iter().map(|component| component.into())) + .chain(components) .collect(), ); @@ -234,7 +232,7 @@ impl Storage continue; } - if lookup_entry.component_ids.is_subset(&comp_ids_set) { + if lookup_entry.component_ids.is_subset(comp_ids_set) { lookup_entry.archetype_indices.push(archetype_index); } } @@ -293,13 +291,11 @@ impl Storage archetype.has_entity(entity_uid) }) - .map(|archetype_index| *archetype_index) + .copied() } - fn iter_archetypes_by_lookup( - &self, - archetype_id: &ArchetypeId, - ) -> ArchetypeRefIter<'_> + fn iter_archetypes_by_lookup(&self, archetype_id: ArchetypeId) + -> ArchetypeRefIter<'_> { let archetype_lookup = self.archetype_lookup.borrow(); @@ -307,15 +303,15 @@ impl Storage // iterations are nested. The panic happens because the archetype_lookup RefCell // is borrowed and it tries to mutably borrow it let archetype_indices = archetype_lookup - .get(archetype_id) + .get(&archetype_id) .unwrap() .archetype_indices .clone(); - return ArchetypeRefIter { + ArchetypeRefIter { indices: archetype_indices.into_iter(), archetypes: &self.archetypes, - }; + } } } @@ -396,10 +392,7 @@ impl Archetype { self.entities.push(ArchetypeEntity { uid: entity_uid, - components: components - .into_iter() - .map(|component| component.into()) - .collect(), + components: components.into_iter().map(Into::into).collect(), }); } @@ -488,7 +481,7 @@ impl<'archetype> Iterator for EntityIter<'archetype> } } -fn create_non_opt_component_id_set<'a, Item>( +fn create_non_opt_component_id_set( component_metadata_iter: impl IntoIterator, ) -> HashSet where diff --git a/ecs/src/event.rs b/ecs/src/event.rs index 0caa56f..1a4edcc 100644 --- a/ecs/src/event.rs +++ b/ecs/src/event.rs @@ -9,6 +9,7 @@ pub mod component; pub trait Event: Debug + 'static { /// Returns the ID of this event. + #[must_use] fn id() -> Id where Self: Sized, diff --git a/ecs/src/event/component.rs b/ecs/src/event/component.rs index 4628cb1..62e7c4d 100644 --- a/ecs/src/event/component.rs +++ b/ecs/src/event/component.rs @@ -97,20 +97,22 @@ where } } +#[must_use] pub fn create_added_id(component_id: ComponentId) -> Id { Id::new::, _>(Some(component_id)) } +#[must_use] pub fn create_removed_id(component_id: ComponentId) -> Id { Id::new::, _>(Some(component_id)) } -pub struct ComponentToAddedEvent; +pub struct TypeTransformComponentsToAddedEvents; impl - TupleReduceElement for ComponentT + TupleReduceElement for ComponentT where Accumulator: TupleWith>, { diff --git a/ecs/src/extension.rs b/ecs/src/extension.rs index 99320cb..a945d89 100644 --- a/ecs/src/extension.rs +++ b/ecs/src/extension.rs @@ -1,5 +1,5 @@ use crate::component::Sequence as ComponentSequence; -use crate::event::component::ComponentToAddedEvent; +use crate::event::component::TypeTransformComponentsToAddedEvents; use crate::event::{Event, Sequence as EventSequence}; use crate::sole::Sole; use crate::system::System; @@ -40,7 +40,7 @@ impl<'world> Collector<'world> /// Adds a entity to the [`World`]. pub fn add_entity(&mut self, components: Comps) where - Comps: ComponentSequence + TupleReduce, + Comps: ComponentSequence + TupleReduce, Comps::Out: EventSequence, { self.world.create_entity(components); diff --git a/ecs/src/lib.rs b/ecs/src/lib.rs index e298919..40b2021 100644 --- a/ecs/src/lib.rs +++ b/ecs/src/lib.rs @@ -15,12 +15,12 @@ use crate::entity::Uid as EntityUid; use crate::event::component::{ create_added_id as create_component_added_event_id, create_removed_id as create_component_removed_event_id, - ComponentToAddedEvent, + TypeTransformComponentsToAddedEvents, }; use crate::event::start::Start as StartEvent; use crate::event::{Event, Id as EventId, Ids, Sequence as EventSequence}; use crate::extension::{Collector as ExtensionCollector, Extension}; -use crate::lock::Lock; +use crate::lock::{Lock, WriteGuard}; use crate::query::options::Options as QueryOptions; use crate::sole::Sole; use crate::system::{System, TypeErased as TypeErasedSystem}; @@ -69,7 +69,7 @@ impl World /// Will panic if mutable internal lock cannot be acquired. pub fn create_entity(&mut self, components: Comps) -> EntityUid where - Comps: ComponentSequence + TupleReduce, + Comps: ComponentSequence + TupleReduce, Comps::Out: EventSequence, { let (_, entity_uid) = self @@ -145,7 +145,7 @@ impl World Comps: ComponentSequence, OptionsT: QueryOptions, { - Query::new(&self) + Query::new(self) } /// Peforms the actions that have been queued up using [`Actions`]. @@ -172,10 +172,7 @@ impl World for action in active_action_queue.drain(..) { match action { Action::Spawn(components) => { - let mut component_storage_lock = - self.data.component_storage.write_nonblock().expect( - "Failed to acquire read-write component storage lock", - ); + let mut component_storage_lock = self.lock_component_storage_rw(); let component_ids = components .iter() @@ -188,15 +185,7 @@ impl World drop(component_storage_lock); if !has_swapped_active_queue { - let mut active_queue = - self.data.action_queue.active_queue.borrow_mut(); - - *active_queue = match *active_queue { - ActiveActionQueue::A => ActiveActionQueue::B, - ActiveActionQueue::B => ActiveActionQueue::A, - }; - - has_swapped_active_queue = true; + self.swap_event_queue(&mut has_swapped_active_queue); } for component_id in component_ids { @@ -206,10 +195,7 @@ impl World } } Action::AddComponents(entity_uid, components) => { - let mut component_storage_lock = - self.data.component_storage.write_nonblock().expect( - "Failed to acquire read-write component storage lock", - ); + let mut component_storage_lock = self.lock_component_storage_rw(); let component_ids = components .iter() @@ -222,15 +208,7 @@ impl World drop(component_storage_lock); if !has_swapped_active_queue { - let mut active_queue = - self.data.action_queue.active_queue.borrow_mut(); - - *active_queue = match *active_queue { - ActiveActionQueue::A => ActiveActionQueue::B, - ActiveActionQueue::B => ActiveActionQueue::A, - }; - - has_swapped_active_queue = true; + self.swap_event_queue(&mut has_swapped_active_queue); } for component_id in component_ids { @@ -240,10 +218,7 @@ impl World } } Action::RemoveComponents(entity_uid, components_metadata) => { - let mut component_storage_lock = - self.data.component_storage.write_nonblock().expect( - "Failed to acquire read-write component storage lock", - ); + let mut component_storage_lock = self.lock_component_storage_rw(); component_storage_lock.remove_components_from_entity( entity_uid, @@ -255,15 +230,7 @@ impl World drop(component_storage_lock); if !has_swapped_active_queue { - let mut active_queue = - self.data.action_queue.active_queue.borrow_mut(); - - *active_queue = match *active_queue { - ActiveActionQueue::A => ActiveActionQueue::B, - ActiveActionQueue::B => ActiveActionQueue::A, - }; - - has_swapped_active_queue = true; + self.swap_event_queue(&mut has_swapped_active_queue); } for component_metadata in components_metadata { @@ -314,10 +281,30 @@ impl World // SAFETY: The world lives long enough unsafe { - system.run(&self); + system.run(self); } } } + + fn swap_event_queue(&self, has_swapped_active_queue: &mut bool) + { + let mut active_queue = self.data.action_queue.active_queue.borrow_mut(); + + *active_queue = match *active_queue { + ActiveActionQueue::A => ActiveActionQueue::B, + ActiveActionQueue::B => ActiveActionQueue::A, + }; + + *has_swapped_active_queue = true; + } + + fn lock_component_storage_rw(&self) -> WriteGuard<'_, ComponentStorage> + { + self.data + .component_storage + .write_nonblock() + .expect("Failed to acquire read-write component storage lock") + } } #[derive(Debug, Default)] diff --git a/ecs/src/lock.rs b/ecs/src/lock.rs index 77213c9..fbc6842 100644 --- a/ecs/src/lock.rs +++ b/ecs/src/lock.rs @@ -1,6 +1,6 @@ use std::mem::transmute; use std::ops::{Deref, DerefMut}; -use std::sync::{RwLock, RwLockReadGuard, RwLockWriteGuard, TryLockError}; +use std::sync::{PoisonError, RwLock, RwLockReadGuard, RwLockWriteGuard, TryLockError}; use crate::type_name::TypeName; @@ -62,7 +62,7 @@ where { self.inner .into_inner() - .unwrap_or_else(|err| err.into_inner()) + .unwrap_or_else(PoisonError::into_inner) } } @@ -90,6 +90,7 @@ where /// # Safety /// The returned `ReadGuard` must **NOT** be used for longer than the original /// lifetime. + #[must_use] pub unsafe fn upgrade_lifetime<'new>(self) -> ReadGuard<'new, Value> { unsafe { transmute(self) } diff --git a/ecs/src/query.rs b/ecs/src/query.rs index dea3039..944deb1 100644 --- a/ecs/src/query.rs +++ b/ecs/src/query.rs @@ -53,7 +53,7 @@ where entities: self .component_storage .find_entities(Comps::metadata()) - .map((|archetype| archetype.entities()) as ComponentIterMapFn) + .map(Archetype::entities as ComponentIterMapFn) .flatten() .filter(|entity| OptionsT::entity_filter(entity.components())), comps_pd: PhantomData, @@ -61,13 +61,13 @@ where } /// Returns the UID of the entity at the given query iteration index. + #[must_use] pub fn entity_uid(&self, entity_index: usize) -> Option { Some( self.component_storage .find_entities(Comps::metadata()) - .map(|archetype| archetype.entities()) - .flatten() + .flat_map(|archetype| archetype.entities()) .filter(|entity| OptionsT::entity_filter(entity.components())) .nth(entity_index)? .uid(), @@ -123,7 +123,7 @@ where world: &'world World, ) -> Self { - Self::new(&world) + Self::new(world) } fn is_compatible>() -> bool diff --git a/ecs/src/relationship.rs b/ecs/src/relationship.rs index 1a8c842..0fe776e 100644 --- a/ecs/src/relationship.rs +++ b/ecs/src/relationship.rs @@ -26,6 +26,7 @@ where ComponentT: Component, { /// Creates a new `Relationship` with a single target. + #[must_use] pub fn new(entity_uid: EntityUid) -> Self { Self { @@ -35,6 +36,7 @@ where } /// Creates a new `Relationship` with multiple targets. + #[must_use] pub fn new_multiple(entity_uids: impl IntoIterator) -> Self { Self { @@ -132,6 +134,11 @@ where ComponentT: Component, { /// Returns the component of the target at the specified index. + /// + /// # Panics + /// Will panic if the entity does not exist in the archetype it belongs to. This + /// should hopefully never happend. + #[must_use] pub fn get(&self, index: usize) -> Option> { let target = self.get_target(index)?; @@ -162,22 +169,24 @@ where } /// Returns a reference to the target at the specified index. + #[must_use] pub fn get_target(&self, index: usize) -> Option<&EntityUid> { match &self.relationship_comp.entity_uid { SingleOrMultiple::Single(entity_uid) if index == 0 => Some(entity_uid), SingleOrMultiple::Multiple(entity_uids) => entity_uids.get(index), - _ => None, + SingleOrMultiple::Single(_) => None, } } /// Returns a mutable reference to the target at the specified index. + #[must_use] pub fn get_target_mut(&mut self, index: usize) -> Option<&mut EntityUid> { match &mut self.relationship_comp.entity_uid { SingleOrMultiple::Single(entity_uid) if index == 0 => Some(entity_uid), SingleOrMultiple::Multiple(entity_uids) => entity_uids.get_mut(index), - _ => None, + SingleOrMultiple::Single(_) => None, } } @@ -215,6 +224,7 @@ where } } + #[must_use] pub fn target_count(&self) -> usize { match &self.relationship_comp.entity_uid { @@ -224,12 +234,28 @@ where } /// Returns a iterator of the components of the targets of this relationship. + #[must_use] pub fn iter(&self) -> TargetComponentIter<'_, 'rel_comp, Kind, ComponentT> { TargetComponentIter { relation: self, index: 0 } } } +impl<'relationship, 'rel_comp, Kind, ComponentT> IntoIterator + for &'relationship Relation<'rel_comp, Kind, ComponentT> +where + 'relationship: 'rel_comp, + ComponentT: Component, +{ + type IntoIter = TargetComponentIter<'relationship, 'rel_comp, Kind, ComponentT>; + type Item = ComponentRefMut<'rel_comp, ComponentT>; + + fn into_iter(self) -> Self::IntoIter + { + self.iter() + } +} + /// Iterator of the components of the targets of a relationship. pub struct TargetComponentIter<'relationship, 'rel_comp, Kind, ComponentT> where diff --git a/ecs/src/util.rs b/ecs/src/util.rs index f4f8632..4480fc8 100644 --- a/ecs/src/util.rs +++ b/ecs/src/util.rs @@ -17,7 +17,7 @@ impl Sortable for [Item] Func: FnMut(&Self::Item) -> Key, Key: Ord, { - self.sort_by_key(func) + self.sort_by_key(func); } } @@ -30,7 +30,7 @@ impl Sortable for [Item; LENGTH] Func: FnMut(&Self::Item) -> Key, Key: Ord, { - self.sort_by_key(func) + self.sort_by_key(func); } } @@ -43,6 +43,6 @@ impl Sortable for Vec Func: FnMut(&Self::Item) -> Key, Key: Ord, { - self.sort_by_key(func) + self.sort_by_key(func); } } -- cgit v1.2.3-18-g5258