From 88ce316371fab257647a00d76ee9db3b74c07380 Mon Sep 17 00:00:00 2001 From: Kanabenki Date: Mon, 2 Oct 2023 20:43:15 +0200 Subject: [PATCH] Make builder types take and return `Self` --- crates/bevy_ecs/src/storage/table.rs | 13 +- crates/bevy_scene/src/dynamic_scene.rs | 18 +-- .../bevy_scene/src/dynamic_scene_builder.rs | 145 ++++++++++-------- crates/bevy_scene/src/scene_filter.rs | 50 +++--- crates/bevy_scene/src/serde.rs | 8 +- 5 files changed, 123 insertions(+), 111 deletions(-) diff --git a/crates/bevy_ecs/src/storage/table.rs b/crates/bevy_ecs/src/storage/table.rs index 48276ce3b3344c..6da4504d40add4 100644 --- a/crates/bevy_ecs/src/storage/table.rs +++ b/crates/bevy_ecs/src/storage/table.rs @@ -526,13 +526,16 @@ impl TableBuilder { } } - pub fn add_column(&mut self, component_info: &ComponentInfo) { + #[must_use] + pub fn add_column(mut self, component_info: &ComponentInfo) -> Self { self.columns.insert( component_info.id(), Column::with_capacity(component_info, self.capacity), ); + self } + #[must_use] pub fn build(self) -> Table { Table { columns: self.columns.into_immutable(), @@ -865,7 +868,7 @@ impl Tables { .or_insert_with(|| { let mut table = TableBuilder::with_capacity(0, component_ids.len()); for component_id in component_ids { - table.add_column(components.get_info_unchecked(*component_id)); + table = table.add_column(components.get_info_unchecked(*component_id)); } tables.push(table.build()); (component_ids.to_vec(), TableId::new(tables.len() - 1)) @@ -929,9 +932,9 @@ mod tests { let mut storages = Storages::default(); let component_id = components.init_component::>(&mut storages); let columns = &[component_id]; - let mut builder = TableBuilder::with_capacity(0, columns.len()); - builder.add_column(components.get_info(component_id).unwrap()); - let mut table = builder.build(); + let mut table = TableBuilder::with_capacity(0, columns.len()) + .add_column(components.get_info(component_id).unwrap()) + .build(); let entities = (0..200).map(Entity::from_raw).collect::>(); for entity in &entities { // SAFETY: we allocate and immediately set data afterwards diff --git a/crates/bevy_scene/src/dynamic_scene.rs b/crates/bevy_scene/src/dynamic_scene.rs index dee1e4ae0f3207..9200f7ffec944f 100644 --- a/crates/bevy_scene/src/dynamic_scene.rs +++ b/crates/bevy_scene/src/dynamic_scene.rs @@ -51,12 +51,10 @@ impl DynamicScene { /// Create a new dynamic scene from a given world. pub fn from_world(world: &World) -> Self { - let mut builder = DynamicSceneBuilder::from_world(world); - - builder.extract_entities(world.iter_entities().map(|entity| entity.id())); - builder.extract_resources(); - - builder.build() + DynamicSceneBuilder::from_world(world) + .extract_entities(world.iter_entities().map(|entity| entity.id())) + .extract_resources() + .build() } /// Write the resources, the dynamic entities, and their corresponding components to the given world. @@ -210,10 +208,10 @@ mod tests { // We then write this relationship to a new scene, and then write that scene back to the // world to create another parent and child relationship - let mut scene_builder = DynamicSceneBuilder::from_world(&world); - scene_builder.extract_entity(original_parent_entity); - scene_builder.extract_entity(original_child_entity); - let scene = scene_builder.build(); + let scene = DynamicSceneBuilder::from_world(&world) + .extract_entity(original_parent_entity) + .extract_entity(original_child_entity) + .build(); let mut entity_map = HashMap::default(); scene.write_to_world(&mut world, &mut entity_map).unwrap(); diff --git a/crates/bevy_scene/src/dynamic_scene_builder.rs b/crates/bevy_scene/src/dynamic_scene_builder.rs index 62e5a3ed1f348b..c46128e422f927 100644 --- a/crates/bevy_scene/src/dynamic_scene_builder.rs +++ b/crates/bevy_scene/src/dynamic_scene_builder.rs @@ -75,13 +75,15 @@ impl<'w> DynamicSceneBuilder<'w> { } /// Specify a custom component [`SceneFilter`] to be used with this builder. - pub fn with_filter(&mut self, filter: SceneFilter) -> &mut Self { + #[must_use] + pub fn with_filter(mut self, filter: SceneFilter) -> Self { self.component_filter = filter; self } /// Specify a custom resource [`SceneFilter`] to be used with this builder. - pub fn with_resource_filter(&mut self, filter: SceneFilter) -> &mut Self { + #[must_use] + pub fn with_resource_filter(mut self, filter: SceneFilter) -> Self { self.resource_filter = filter; self } @@ -92,8 +94,9 @@ impl<'w> DynamicSceneBuilder<'w> { /// /// This is the inverse of [`deny`](Self::deny). /// If `T` has already been denied, then it will be removed from the denylist. - pub fn allow(&mut self) -> &mut Self { - self.component_filter.allow::(); + #[must_use] + pub fn allow(mut self) -> Self { + self.component_filter = self.component_filter.allow::(); self } @@ -103,8 +106,9 @@ impl<'w> DynamicSceneBuilder<'w> { /// /// This is the inverse of [`allow`](Self::allow). /// If `T` has already been allowed, then it will be removed from the allowlist. - pub fn deny(&mut self) -> &mut Self { - self.component_filter.deny::(); + #[must_use] + pub fn deny(mut self) -> Self { + self.component_filter = self.component_filter.deny::(); self } @@ -113,7 +117,8 @@ impl<'w> DynamicSceneBuilder<'w> { /// This is useful for resetting the filter so that types may be selectively [denied]. /// /// [denied]: Self::deny - pub fn allow_all(&mut self) -> &mut Self { + #[must_use] + pub fn allow_all(mut self) -> Self { self.component_filter = SceneFilter::allow_all(); self } @@ -123,7 +128,8 @@ impl<'w> DynamicSceneBuilder<'w> { /// This is useful for resetting the filter so that types may be selectively [allowed]. /// /// [allowed]: Self::allow - pub fn deny_all(&mut self) -> &mut Self { + #[must_use] + pub fn deny_all(mut self) -> Self { self.component_filter = SceneFilter::deny_all(); self } @@ -134,8 +140,9 @@ impl<'w> DynamicSceneBuilder<'w> { /// /// This is the inverse of [`deny_resource`](Self::deny_resource). /// If `T` has already been denied, then it will be removed from the denylist. - pub fn allow_resource(&mut self) -> &mut Self { - self.resource_filter.allow::(); + #[must_use] + pub fn allow_resource(mut self) -> Self { + self.resource_filter = self.resource_filter.allow::(); self } @@ -145,8 +152,9 @@ impl<'w> DynamicSceneBuilder<'w> { /// /// This is the inverse of [`allow_resource`](Self::allow_resource). /// If `T` has already been allowed, then it will be removed from the allowlist. - pub fn deny_resource(&mut self) -> &mut Self { - self.resource_filter.deny::(); + #[must_use] + pub fn deny_resource(mut self) -> Self { + self.resource_filter = self.resource_filter.deny::(); self } @@ -155,7 +163,8 @@ impl<'w> DynamicSceneBuilder<'w> { /// This is useful for resetting the filter so that types may be selectively [denied]. /// /// [denied]: Self::deny_resource - pub fn allow_all_resources(&mut self) -> &mut Self { + #[must_use] + pub fn allow_all_resources(mut self) -> Self { self.resource_filter = SceneFilter::allow_all(); self } @@ -165,7 +174,8 @@ impl<'w> DynamicSceneBuilder<'w> { /// This is useful for resetting the filter so that types may be selectively [allowed]. /// /// [allowed]: Self::allow_resource - pub fn deny_all_resources(&mut self) -> &mut Self { + #[must_use] + pub fn deny_all_resources(mut self) -> Self { self.resource_filter = SceneFilter::deny_all(); self } @@ -174,6 +184,7 @@ impl<'w> DynamicSceneBuilder<'w> { /// /// To make sure the dynamic scene doesn't contain entities without any components, call /// [`Self::remove_empty_entities`] before building the scene. + #[must_use] pub fn build(self) -> DynamicScene { DynamicScene { resources: self.extracted_resources.into_values().collect(), @@ -184,14 +195,16 @@ impl<'w> DynamicSceneBuilder<'w> { /// Extract one entity from the builder's [`World`]. /// /// Re-extracting an entity that was already extracted will have no effect. - pub fn extract_entity(&mut self, entity: Entity) -> &mut Self { + #[must_use] + pub fn extract_entity(self, entity: Entity) -> Self { self.extract_entities(std::iter::once(entity)) } /// Despawns all entities with no components. /// /// These were likely created because none of their components were present in the provided type registry upon extraction. - pub fn remove_empty_entities(&mut self) -> &mut Self { + #[must_use] + pub fn remove_empty_entities(mut self) -> Self { self.extracted_scene .retain(|_, entity| !entity.components.is_empty()); @@ -222,16 +235,17 @@ impl<'w> DynamicSceneBuilder<'w> { /// # let _entity = world.spawn(MyComponent).id(); /// let mut query = world.query_filtered::>(); /// - /// let mut builder = DynamicSceneBuilder::from_world(&world); - /// builder.extract_entities(query.iter(&world)); - /// let scene = builder.build(); + /// let scene = DynamicSceneBuilder::from_world(&world) + /// .extract_entities(query.iter(&world)) + /// .build(); /// ``` /// /// Note that components extracted from queried entities must still pass through the filter if one is set. /// /// [`allow`]: Self::allow /// [`deny`]: Self::deny - pub fn extract_entities(&mut self, entities: impl Iterator) -> &mut Self { + #[must_use] + pub fn extract_entities(mut self, entities: impl Iterator) -> Self { let type_registry = self.original_world.resource::().read(); for entity in entities { @@ -295,14 +309,14 @@ impl<'w> DynamicSceneBuilder<'w> { /// # world.init_resource::(); /// world.insert_resource(MyResource); /// - /// let mut builder = DynamicSceneBuilder::from_world(&world); - /// builder.extract_resources(); + /// let mut builder = DynamicSceneBuilder::from_world(&world).extract_resources(); /// let scene = builder.build(); /// ``` /// /// [`allow_resource`]: Self::allow_resource /// [`deny_resource`]: Self::deny_resource - pub fn extract_resources(&mut self) -> &mut Self { + #[must_use] + pub fn extract_resources(mut self) -> Self { let type_registry = self.original_world.resource::().read(); for (component_id, _) in self.original_world.storages().resources.iter() { @@ -374,9 +388,9 @@ mod tests { let entity = world.spawn((ComponentA, ComponentB)).id(); - let mut builder = DynamicSceneBuilder::from_world(&world); - builder.extract_entity(entity); - let scene = builder.build(); + let scene = DynamicSceneBuilder::from_world(&world) + .extract_entity(entity) + .build(); assert_eq!(scene.entities.len(), 1); assert_eq!(scene.entities[0].entity, entity); @@ -394,10 +408,10 @@ mod tests { let entity = world.spawn((ComponentA, ComponentB)).id(); - let mut builder = DynamicSceneBuilder::from_world(&world); - builder.extract_entity(entity); - builder.extract_entity(entity); - let scene = builder.build(); + let scene = DynamicSceneBuilder::from_world(&world) + .extract_entity(entity) + .extract_entity(entity) + .build(); assert_eq!(scene.entities.len(), 1); assert_eq!(scene.entities[0].entity, entity); @@ -419,9 +433,9 @@ mod tests { let entity = world.spawn((ComponentA, ComponentB)).id(); - let mut builder = DynamicSceneBuilder::from_world(&world); - builder.extract_entity(entity); - let scene = builder.build(); + let scene = DynamicSceneBuilder::from_world(&world) + .extract_entity(entity) + .build(); assert_eq!(scene.entities.len(), 1); assert_eq!(scene.entities[0].entity, entity); @@ -441,12 +455,11 @@ mod tests { let entity_c = world.spawn_empty().id(); let entity_d = world.spawn_empty().id(); - let mut builder = DynamicSceneBuilder::from_world(&world); - // Insert entities out of order - builder.extract_entity(entity_b); - builder.extract_entities([entity_d, entity_a].into_iter()); - builder.extract_entity(entity_c); + let builder = DynamicSceneBuilder::from_world(&world) + .extract_entity(entity_b) + .extract_entities([entity_d, entity_a].into_iter()) + .extract_entity(entity_c); let mut entities = builder.build().entities.into_iter(); @@ -474,9 +487,9 @@ mod tests { let _entity_b = world.spawn(ComponentB).id(); let mut query = world.query_filtered::>(); - let mut builder = DynamicSceneBuilder::from_world(&world); - builder.extract_entities(query.iter(&world)); - let scene = builder.build(); + let scene = DynamicSceneBuilder::from_world(&world) + .extract_entities(query.iter(&world)) + .build(); assert_eq!(scene.entities.len(), 2); let mut scene_entities = vec![scene.entities[0].entity, scene.entities[1].entity]; @@ -495,10 +508,10 @@ mod tests { let entity_a = world.spawn(ComponentA).id(); let entity_b = world.spawn(ComponentB).id(); - let mut builder = DynamicSceneBuilder::from_world(&world); - builder.extract_entities([entity_a, entity_b].into_iter()); - builder.remove_empty_entities(); - let scene = builder.build(); + let scene = DynamicSceneBuilder::from_world(&world) + .extract_entities([entity_a, entity_b].into_iter()) + .remove_empty_entities() + .build(); assert_eq!(scene.entities.len(), 1); assert_eq!(scene.entities[0].entity, entity_a); @@ -514,9 +527,9 @@ mod tests { world.insert_resource(ResourceA); - let mut builder = DynamicSceneBuilder::from_world(&world); - builder.extract_resources(); - let scene = builder.build(); + let scene = DynamicSceneBuilder::from_world(&world) + .extract_resources() + .build(); assert_eq!(scene.resources.len(), 1); assert!(scene.resources[0].represents::()); @@ -532,10 +545,10 @@ mod tests { world.insert_resource(ResourceA); - let mut builder = DynamicSceneBuilder::from_world(&world); - builder.extract_resources(); - builder.extract_resources(); - let scene = builder.build(); + let scene = DynamicSceneBuilder::from_world(&world) + .extract_resources() + .extract_resources() + .build(); assert_eq!(scene.resources.len(), 1); assert!(scene.resources[0].represents::()); @@ -557,11 +570,10 @@ mod tests { let entity_a = world.spawn(ComponentA).id(); let entity_b = world.spawn(ComponentB).id(); - let mut builder = DynamicSceneBuilder::from_world(&world); - builder + let scene = DynamicSceneBuilder::from_world(&world) .allow::() - .extract_entities([entity_a_b, entity_a, entity_b].into_iter()); - let scene = builder.build(); + .extract_entities([entity_a_b, entity_a, entity_b].into_iter()) + .build(); assert_eq!(scene.entities.len(), 3); assert!(scene.entities[0].components[0].represents::()); @@ -585,11 +597,10 @@ mod tests { let entity_a = world.spawn(ComponentA).id(); let entity_b = world.spawn(ComponentB).id(); - let mut builder = DynamicSceneBuilder::from_world(&world); - builder + let scene = DynamicSceneBuilder::from_world(&world) .deny::() - .extract_entities([entity_a_b, entity_a, entity_b].into_iter()); - let scene = builder.build(); + .extract_entities([entity_a_b, entity_a, entity_b].into_iter()) + .build(); assert_eq!(scene.entities.len(), 3); assert!(scene.entities[0].components[0].represents::()); @@ -612,9 +623,10 @@ mod tests { world.insert_resource(ResourceA); world.insert_resource(ResourceB); - let mut builder = DynamicSceneBuilder::from_world(&world); - builder.allow_resource::().extract_resources(); - let scene = builder.build(); + let scene = DynamicSceneBuilder::from_world(&world) + .allow_resource::() + .extract_resources() + .build(); assert_eq!(scene.resources.len(), 1); assert!(scene.resources[0].represents::()); @@ -635,9 +647,10 @@ mod tests { world.insert_resource(ResourceA); world.insert_resource(ResourceB); - let mut builder = DynamicSceneBuilder::from_world(&world); - builder.deny_resource::().extract_resources(); - let scene = builder.build(); + let scene = DynamicSceneBuilder::from_world(&world) + .deny_resource::() + .extract_resources() + .build(); assert_eq!(scene.resources.len(), 1); assert!(scene.resources[0].represents::()); diff --git a/crates/bevy_scene/src/scene_filter.rs b/crates/bevy_scene/src/scene_filter.rs index a5076e5d9d668b..1bdfdfa871a453 100644 --- a/crates/bevy_scene/src/scene_filter.rs +++ b/crates/bevy_scene/src/scene_filter.rs @@ -70,7 +70,8 @@ impl SceneFilter { /// [`Denylist`]: SceneFilter::Denylist /// [`Unset`]: SceneFilter::Unset /// [`Allowlist`]: SceneFilter::Allowlist - pub fn allow(&mut self) -> &mut Self { + #[must_use] + pub fn allow(self) -> Self { self.allow_by_id(TypeId::of::()) } @@ -84,10 +85,11 @@ impl SceneFilter { /// [`Denylist`]: SceneFilter::Denylist /// [`Unset`]: SceneFilter::Unset /// [`Allowlist`]: SceneFilter::Allowlist - pub fn allow_by_id(&mut self, type_id: TypeId) -> &mut Self { - match self { + #[must_use] + pub fn allow_by_id(mut self, type_id: TypeId) -> Self { + match &mut self { Self::Unset => { - *self = Self::Allowlist(HashSet::from([type_id])); + self = Self::Allowlist(HashSet::from([type_id])); } Self::Allowlist(list) => { list.insert(type_id); @@ -109,7 +111,8 @@ impl SceneFilter { /// [`Allowlist`]: SceneFilter::Allowlist /// [`Unset`]: SceneFilter::Unset /// [`Denylist`]: SceneFilter::Denylist - pub fn deny(&mut self) -> &mut Self { + #[must_use] + pub fn deny(self) -> Self { self.deny_by_id(TypeId::of::()) } @@ -123,9 +126,10 @@ impl SceneFilter { /// [`Allowlist`]: SceneFilter::Allowlist /// [`Unset`]: SceneFilter::Unset /// [`Denylist`]: SceneFilter::Denylist - pub fn deny_by_id(&mut self, type_id: TypeId) -> &mut Self { - match self { - Self::Unset => *self = Self::Denylist(HashSet::from([type_id])), + #[must_use] + pub fn deny_by_id(mut self, type_id: TypeId) -> Self { + match &mut self { + Self::Unset => self = Self::Denylist(HashSet::from([type_id])), Self::Allowlist(list) => { list.remove(&type_id); } @@ -231,27 +235,21 @@ mod tests { #[test] fn should_set_list_type_if_none() { - let mut filter = SceneFilter::Unset; - filter.allow::(); + let filter = SceneFilter::Unset.allow::(); assert!(matches!(filter, SceneFilter::Allowlist(_))); - let mut filter = SceneFilter::Unset; - filter.deny::(); + let filter = SceneFilter::Unset.deny::(); assert!(matches!(filter, SceneFilter::Denylist(_))); } #[test] fn should_add_to_list() { - let mut filter = SceneFilter::default(); - filter.allow::(); - filter.allow::(); + let filter = SceneFilter::default().allow::().allow::(); assert_eq!(2, filter.len()); assert!(filter.is_allowed::()); assert!(filter.is_allowed::()); - let mut filter = SceneFilter::default(); - filter.deny::(); - filter.deny::(); + let filter = SceneFilter::default().deny::().deny::(); assert_eq!(2, filter.len()); assert!(filter.is_denied::()); assert!(filter.is_denied::()); @@ -259,18 +257,18 @@ mod tests { #[test] fn should_remove_from_list() { - let mut filter = SceneFilter::default(); - filter.allow::(); - filter.allow::(); - filter.deny::(); + let filter = SceneFilter::default() + .allow::() + .allow::() + .deny::(); assert_eq!(1, filter.len()); assert!(filter.is_allowed::()); assert!(!filter.is_allowed::()); - let mut filter = SceneFilter::default(); - filter.deny::(); - filter.deny::(); - filter.allow::(); + let filter = SceneFilter::default() + .deny::() + .deny::() + .allow::(); assert_eq!(1, filter.len()); assert!(filter.is_denied::()); assert!(!filter.is_denied::()); diff --git a/crates/bevy_scene/src/serde.rs b/crates/bevy_scene/src/serde.rs index 515728cae00fae..4a8bf87bfeb602 100644 --- a/crates/bevy_scene/src/serde.rs +++ b/crates/bevy_scene/src/serde.rs @@ -592,10 +592,10 @@ mod tests { world.insert_resource(MyResource { foo: 123 }); - let mut builder = DynamicSceneBuilder::from_world(&world); - builder.extract_entities([a, b, c].into_iter()); - builder.extract_resources(); - let scene = builder.build(); + let scene = DynamicSceneBuilder::from_world(&world) + .extract_entities([a, b, c].into_iter()) + .extract_resources() + .build(); let expected = r#"( resources: {