diff --git a/CHANGELOG.md b/CHANGELOG.md index 407633736..fbb6d1c19 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,10 @@ ### Changed +- `all-is-cubes` library: + - `block::BlockAttributes` is now a non-exhaustive struct. + `BlockAttributes` literals should be replaced with `block::Builder::build_attributes()`. + - `all-is-cubes-ui` library: - `apps::Session::settings()` replaces `graphics_options_mut()`. diff --git a/all-is-cubes-content/src/city.rs b/all-is-cubes-content/src/city.rs index 220f34b3f..e74dd0bf9 100644 --- a/all-is-cubes-content/src/city.rs +++ b/all-is-cubes-content/src/city.rs @@ -7,7 +7,8 @@ use itertools::Itertools; use rand::seq::SliceRandom as _; use rand::{Rng, SeedableRng as _}; -use all_is_cubes::block::{self, text::Font, BlockAttributes, Resolution::*, AIR}; +use all_is_cubes::arcstr::literal; +use all_is_cubes::block::{self, text::Font, Resolution::*, AIR}; use all_is_cubes::character::Spawn; use all_is_cubes::color_block; use all_is_cubes::content::palette; @@ -569,11 +570,10 @@ fn place_one_exhibit( bounds_for_info_voxels, universe.insert_anonymous(exhibit_info_space), info_resolution, - [BlockAttributes { - display_name: "Exhibit Name".into(), - ..Default::default() - } - .into()], + [block::Block::builder() + .display_name(literal!("Exhibit Name")) + .build_attributes() + .into()], ))), ], }); diff --git a/all-is-cubes-content/src/city/exhibits/knot.rs b/all-is-cubes-content/src/city/exhibits/knot.rs index 9c87dd708..30e8414c8 100644 --- a/all-is-cubes-content/src/city/exhibits/knot.rs +++ b/all-is-cubes-content/src/city/exhibits/knot.rs @@ -69,10 +69,11 @@ fn KNOT(ctx: Context<'_>) { resolution, txn.insert_anonymous(drawing_space), &mut |block| { - block.with_modifier(BlockAttributes { - display_name: ctx.exhibit.name.into(), - ..BlockAttributes::default() - }) + block.with_modifier( + Block::builder() + .display_name(ctx.exhibit.name) + .build_attributes(), + ) }, )?; Ok((space, txn)) diff --git a/all-is-cubes-content/src/city/exhibits/prelude.rs b/all-is-cubes-content/src/city/exhibits/prelude.rs index e7b68dce0..b54522a16 100644 --- a/all-is-cubes-content/src/city/exhibits/prelude.rs +++ b/all-is-cubes-content/src/city/exhibits/prelude.rs @@ -12,8 +12,7 @@ pub(super) use rand::SeedableRng as _; pub(super) use all_is_cubes::arcstr::{self, literal}; pub(super) use all_is_cubes::block::{ - self, space_to_blocks, text, Block, BlockAttributes, BlockCollision, Composite, - CompositeOperator, Move, + self, space_to_blocks, text, Block, BlockCollision, Composite, CompositeOperator, Move, Resolution::{self, *}, RotationPlacementRule, Zoom, AIR, }; diff --git a/all-is-cubes/src/block/attributes.rs b/all-is-cubes/src/block/attributes.rs index 02c81e607..1b9b4f2af 100644 --- a/all-is-cubes/src/block/attributes.rs +++ b/all-is-cubes/src/block/attributes.rs @@ -149,8 +149,7 @@ macro_rules! attribute_builder_method { /// `BlockAttributes::default()` will produce a reasonable set of defaults for “ordinary” /// blocks. #[derive(Clone, Eq, Hash, PartialEq)] -#[expect(clippy::exhaustive_structs)] -// TODO: Make this non_exhaustive but give users a way to construct it easily, possibly via block::Builder. +#[non_exhaustive] #[macro_rules_attribute::derive(derive_attribute_helpers!)] pub struct BlockAttributes { /// The name that should be displayed to players. diff --git a/all-is-cubes/src/block/builder.rs b/all-is-cubes/src/block/builder.rs index d2fea8a71..54e21d7be 100644 --- a/all-is-cubes/src/block/builder.rs +++ b/all-is-cubes/src/block/builder.rs @@ -15,9 +15,13 @@ use crate::universe::{Handle, Name, Universe, UniverseTransaction}; /// Tool for constructing [`Block`] values conveniently. /// +/// It can also be used to construct [`BlockAttributes`] values. +/// /// To create one, call [`Block::builder()`]. /// ([`Builder::default()`] is also available.) /// +/// # Example +/// /// ``` /// use all_is_cubes::block::{Block, EvaluatedBlock}; /// use all_is_cubes::math::Rgba; @@ -40,9 +44,11 @@ use crate::universe::{Handle, Name, Universe, UniverseTransaction}; #[derive(Clone, Debug, Eq, PartialEq)] #[must_use] pub struct Builder { - // public so that `BlockAttributes`'s macros can define methods for us + /// public so that `BlockAttributes`'s macros can define methods for us pub(in crate::block) attributes: BlockAttributes, + primitive_builder: P, + modifiers: Vec, /// If this is a [`UniverseTransaction`], then it must be produced for the caller to execute. @@ -66,6 +72,21 @@ impl Builder { transaction: (), } } + + /// Returns a [`BlockAttributes`] instead of building a block with those attributes. + /// + /// Panics if any modifiers were added to the builder. + #[track_caller] + pub fn build_attributes(self) -> BlockAttributes { + let Self { + attributes, + primitive_builder: NeedsPrimitive, + modifiers, + transaction: (), + } = self; + assert_eq!(modifiers, []); + attributes + } } impl Builder { diff --git a/all-is-cubes/tests/alloc.rs b/all-is-cubes/tests/alloc.rs index 0f5ed051b..6047ad7ae 100644 --- a/all-is-cubes/tests/alloc.rs +++ b/all-is-cubes/tests/alloc.rs @@ -11,28 +11,30 @@ use all_is_cubes::universe::Universe; #[test] fn clone_block_attributes() { - let original = BlockAttributes { + // TODO: Ideally this would be a struct literal with every field, + // but we define BlockAttributes as #[non_exhaustive]. Find a way to prove this is complete. + let original: BlockAttributes = block::Block::builder() + // // These fields are refcounted or `Copy` and will not allocate when cloned - display_name: arcstr::literal!("hello"), - selectable: true, - animation_hint: block::AnimationHint::UNCHANGING, - - placement_action: Some(block::PlacementAction { + .display_name(arcstr::literal!("hello")) + .selectable(true) + .animation_hint(block::AnimationHint::UNCHANGING) + .placement_action(block::PlacementAction { operation: Operation::Become(block::AIR), in_front: false, - }), - tick_action: Some(block::TickAction::from(Operation::Become(block::AIR))), - activation_action: Some(Operation::Become(block::AIR)), - + }) + .tick_action(block::TickAction::from(Operation::Become(block::AIR))) + .activation_action(Operation::Become(block::AIR)) + // // TODO(inventory): This field will allocate when cloned if it is nonempty, // and we should fix that and test it. - inventory: inv::InvInBlock::default(), - + .inventory_config(inv::InvInBlock::default()) + // // These fields currently will never allocate when cloned - rotation_rule: block::RotationPlacementRule::Never, - }; - let mut clone = None; + .rotation_rule(block::RotationPlacementRule::Never) + .build_attributes(); + let mut clone = None; assert_no_alloc(|| { clone = Some(original.clone()); });