Skip to content

Commit 6caebf3

Browse files
cartqohalice-i-cecile
authored
BSN Syntax Improvements (#25318)
# Objective BSN currently treats passed in variables and inline functions as "scenes", and requires `template_value()` to pass in a component `Template` value: ```rust fn widget(node: Node, contents: impl Scene) -> impl Scene { bsn! { Widget template_value(node) contents } } ``` This is a bit unexpected, as `node` is a "peer" of `Widget`, from a placement perspective. `template_value` comes up _a lot_, and results in significant confusion and noise. Other "scene inclusions", such as `@SceneComponent {}` and `:"scene.bsn"` have explicit visual indicators that they are scenes. This all results in making `bsn!` harder to write and read than it should be. We can do better. Additionally, this pattern shows up a lot: ```rust bsn! { template_value(Transform::from_xyz(-2.5, 4.5, 9.0).looking_at(Vec3::ZERO, Vec3::Y)) } ``` As does this: ```rust bsn! { template_value(foo.bar()) } ``` The `template_value` wrapper is required because `bsn!` doesn't currently support expressions like `foo.bar()`. `template_value` fixes this by breaking out of `bsn!`'s syntax / reading the Rust expression directly. We should support this pattern directly and it should behave as expected! Additionally, enums (especially enums outside of developers control) are annoying and weird to work with in BSN, as they require `VariantDefaults` to support "per variant patching". This is a nice feature, but it hasn't really come up in practice, and the cost it incurs is way too high from a usability perspective. Enums that don't implement VariantDefaults (or implicitly via FromTemplate) require using template_value: ```rust bsn! { template_value(Team::Blue) } ``` Additionally, things like `NumberInputValue::F32` required `template_value`, as it is ambiguous whether it is an enum variant an or "associated const": ```rust bsn! { template_value(NumberInputValue::F32(1.0)) } ``` So many annoyances! ## Solution - Variables, functions, and expressions in "BSN entry" position are now interpreted as templates. - All scene inclusions now use `@` syntax, which is now considered to be the "uncached scene" syntax. - BSN entries now support `foo.bar()`-style expressions. These are always interpreted as "template values" - I have removed `{}` from "BSN entry position", as `Widget {}` and `Widget { foo }` is ambiguous. This also provides clarity in the "scene list" case, as `{}` is now only used in list item position. - I've implemented Template for the `Propagate` component so it can be used directly - Enums now require specifying every field value. This means any enum that implement Default and Clone can now be used with BSN, just like other types. Structs defined inside enums therefore _also_ require specifying every field, as they are being written on top. However these still have "implicit default". - Thanks to the new enum behavior, the `Type::F32` ambiguity (is it an enum or an associated const) is no longer a problem. These can both parse to the same AST and resolve to the same expression. - "struct update" syntax is now supported - BSN "codegen" has been refactored for improved clarity and terseness - Code and examples have been ported - Added support for `~{TEMPLATE_EXPRESSION}` syntax, which supports arbitrary rust-code template expressions. - `template_value` has been deprecated. In 99.99% of cases, BSN code can remove this wrapper. On the off chance you need an arbitrary template expression that isn't BSN compatible (ex: multiple statements), you can now use `~{TEMPLATE_EXPRESSION}` syntax. This is how it looks now: ```rust fn widget(node: Node, contents: impl Scene) -> impl Scene { bsn! { Widget // component template node // component template variable node.clone() // function that returns a component template @contents // uncached Scene variable @{contents} // uncached Scene expression @scene_function() // uncached Scene function @SceneComponent // uncached Scene Component // An expression that returns a component template Transform::from_xyz(-2.5, 4.5, 9.0).looking_at(Vec3::ZERO, Vec3::Y) } } ``` Ultimately I think we should consider deprecating `template_value`, but we need to keep it around for a bit longer, as it is currently used in one "corner case" in an example. --------- Co-authored-by: qoh <1732901+qoh@users.noreply.github.com> Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
1 parent 34acd90 commit 6caebf3

84 files changed

Lines changed: 2187 additions & 1414 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
---
2+
title: BSN Syntax Improvements.
3+
pull_requests: [25318]
4+
---
5+
6+
BSN landed with a few idiosyncrasies that caused friction in practice. We made some changes to BSN's syntax this cycle in the interest of improving its ergonomics and clarity.
7+
8+
All scene references now require `@` prefixes:
9+
10+
```rust
11+
// Before
12+
bsn! {
13+
scene_variable
14+
scene_function()
15+
{scene_expression}
16+
}
17+
18+
// After
19+
bsn! {
20+
@scene_variable
21+
@scene_function()
22+
@{scene_expression}
23+
}
24+
```
25+
26+
This freed us up to make component values _much_ easier to work with.
27+
28+
```rust
29+
// Before
30+
bsn! {
31+
template_value(component_variable)
32+
template_value(component_function())
33+
}
34+
35+
// After
36+
bsn! {
37+
component_variable
38+
component_function()
39+
}
40+
```
41+
42+
Enums no longer require `VariantDefaults` or `FromTemplate`, provided they implement `Default` and `Clone`:
43+
44+
```rust
45+
// Before
46+
#[derive(Component, Default, Clone, VariantDefaults)]
47+
enum Foo {
48+
A { x: u32, y: u32 },
49+
#[default]
50+
B,
51+
}
52+
53+
bsn! {
54+
Foo::B
55+
}
56+
57+
// After
58+
#[derive(Component, Default, Clone)]
59+
enum Foo {
60+
A { x: u32, y: u32 },
61+
#[default]
62+
B,
63+
}
64+
65+
bsn! {
66+
Foo::B
67+
}
68+
```
69+
70+
If you were using an enum that didn't support `VariantDefaults`, you can remove the `template_value` wrapper:
71+
72+
```rust
73+
// Before
74+
bsn! {
75+
template_value(Foo::A)
76+
}
77+
// After
78+
bsn! {
79+
Foo::A
80+
}
81+
```
82+
83+
The "variant defaults" pattern, which relied on defining individual "default" constructors for each variant is what allowed "individual enum field value patching" (ex: `VariantDefaults` and `FromTemplate` would define `Foo::a_default()` and `Foo::b_default()` in the example above). This is no longer supported, as the weirdness factor (and Rust ecosystem compatibility challenges) were too costly. When working with enums in BSN, you must now specify each field in the enum, just like you would in normal Rust (which doesn't have support for individual enum variant defaults).
84+
85+
```rust
86+
// Before (y field is initialized to its default value)
87+
bsn! {
88+
Foo::A { x: 1 }
89+
}
90+
91+
// After (y field must be manually specified)
92+
bsn! {
93+
Foo::A { x: 1, y: 0 }
94+
}
95+
```
96+
97+
The "builder pattern" previously required a `template_value` wrapper. This can now be removed:
98+
99+
```rust
100+
// Before
101+
bsn! {
102+
template_value(Transform::from_xyz(-2.5, 4.5, 9.0).looking_at(Vec3::ZERO, Vec3::Y))
103+
}
104+
// After
105+
bsn! {
106+
Transform::from_xyz(-2.5, 4.5, 9.0).looking_at(Vec3::ZERO, Vec3::Y)
107+
}
108+
```
109+
110+
Additionally, you can now remove the `template_value` wrapper in cases like this:
111+
112+
```rust
113+
// Before
114+
bsn! {
115+
template_value(node.clone())
116+
}
117+
// After
118+
bsn! {
119+
node.clone()
120+
}
121+
```
122+
123+
In general, you should now be able to remove all `template_value` instances from your BSN declarations!
Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
---
2+
title: BSN Syntax Improvements.
3+
pull_requests: [25318]
4+
---
5+
6+
BSN landed with a few idiosyncrasies that caused friction in practice. We made some changes to BSN's syntax this cycle in the interest of improving its ergonomics and clarity.
7+
8+
All scene references now require `@` prefixes:
9+
10+
```rust
11+
// Before
12+
bsn! {
13+
scene_variable
14+
scene_function()
15+
{scene_expression}
16+
}
17+
18+
// After
19+
bsn! {
20+
@scene_variable
21+
@scene_function()
22+
@{scene_expression}
23+
}
24+
```
25+
26+
This freed us up to make component values _much_ easier to work with.
27+
28+
```rust
29+
// Before
30+
bsn! {
31+
template_value(component_variable)
32+
template_value(component_function())
33+
}
34+
35+
// After
36+
bsn! {
37+
component_variable
38+
component_function()
39+
}
40+
```
41+
42+
Enums no longer require `VariantDefaults` or `FromTemplate`, provided they implement `Default` and `Clone`:
43+
44+
```rust
45+
// Before
46+
#[derive(Component, Default, Clone, VariantDefaults)]
47+
enum Foo {
48+
A { x: u32, y: u32 },
49+
#[default]
50+
B,
51+
}
52+
53+
bsn! {
54+
Foo::B
55+
}
56+
57+
// After
58+
#[derive(Component, Default, Clone)]
59+
enum Foo {
60+
A { x: u32, y: u32 },
61+
#[default]
62+
B,
63+
}
64+
65+
bsn! {
66+
Foo::B
67+
}
68+
```
69+
70+
If you were using an enum that didn't support `VariantDefaults`, you can remove the `template_value` wrapper:
71+
72+
```rust
73+
// Before
74+
bsn! {
75+
template_value(Foo::A)
76+
}
77+
// After
78+
bsn! {
79+
Foo::A
80+
}
81+
```
82+
83+
The "variant defaults" pattern, which relied on defining individual "default" constructors for each variant is what allowed "individual enum field value patching" (ex: `VariantDefaults` and `FromTemplate` would define `Foo::a_default()` and `Foo::b_default()` in the example above). This is no longer supported, as the weirdness factor (and Rust ecosystem compatibility challenges) were too costly. When working with enums in BSN, you must now specify each field in the enum, just like you would in normal Rust (which doesn't have support for individual enum variant defaults).
84+
85+
```rust
86+
// Before (y field is initialized to its default value)
87+
bsn! {
88+
Foo::A { x: 1 }
89+
}
90+
91+
// After (y field must be manually specified)
92+
bsn! {
93+
Foo::A { x: 1, y: 0 }
94+
}
95+
```
96+
97+
The "builder pattern" previously required a `template_value` wrapper. This can now be removed:
98+
99+
```rust
100+
// Before
101+
bsn! {
102+
template_value(Transform::from_xyz(-2.5, 4.5, 9.0).looking_at(Vec3::ZERO, Vec3::Y))
103+
}
104+
// After
105+
bsn! {
106+
Transform::from_xyz(-2.5, 4.5, 9.0).looking_at(Vec3::ZERO, Vec3::Y)
107+
}
108+
```
109+
110+
Additionally, you can now remove the `template_value` wrapper in cases like this:
111+
112+
```rust
113+
// Before
114+
bsn! {
115+
template_value(node.clone())
116+
}
117+
// After
118+
bsn! {
119+
node.clone()
120+
}
121+
```
122+
123+
In general, you should now be able to remove all `template_value` instances from your BSN declarations!

benches/benches/bevy_scene/spawn.rs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -194,16 +194,16 @@ fn ui() -> impl Scene {
194194
bsn! {
195195
Node
196196
Children [
197-
(button() Node { width: Val::Px(200.) }),
198-
(button() Node { width: Val::Px(200.) }),
199-
(button() Node { width: Val::Px(200.) }),
200-
(button() Node { width: Val::Px(200.) }),
201-
(button() Node { width: Val::Px(200.) }),
202-
(button() Node { width: Val::Px(200.) }),
203-
(button() Node { width: Val::Px(200.) }),
204-
(button() Node { width: Val::Px(200.) }),
205-
(button() Node { width: Val::Px(200.) }),
206-
(button() Node { width: Val::Px(200.) }),
197+
(@button() Node { width: Val::Px(200.) }),
198+
(@button() Node { width: Val::Px(200.) }),
199+
(@button() Node { width: Val::Px(200.) }),
200+
(@button() Node { width: Val::Px(200.) }),
201+
(@button() Node { width: Val::Px(200.) }),
202+
(@button() Node { width: Val::Px(200.) }),
203+
(@button() Node { width: Val::Px(200.) }),
204+
(@button() Node { width: Val::Px(200.) }),
205+
(@button() Node { width: Val::Px(200.) }),
206+
(@button() Node { width: Val::Px(200.) }),
207207
]
208208
}
209209
}

crates/bevy_app/src/propagate.rs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use bevy_ecs::{
1616
relationship::{Relationship, RelationshipTarget},
1717
schedule::{IntoScheduleConfigs, ScheduleLabel, SystemSet},
1818
system::{Commands, Local, Query},
19+
template::{FromTemplate, Template},
1920
};
2021
#[cfg(feature = "bevy_reflect")]
2122
use bevy_reflect::Reflect;
@@ -75,6 +76,34 @@ impl<C: Component + Clone + PartialEq, F: QueryFilter, R: Relationship>
7576
)]
7677
pub struct Propagate<C: Component + Clone + PartialEq>(pub C);
7778

79+
/// The [`Template`] for [`Propagate`].
80+
pub struct PropagateTemplate<T>(pub T);
81+
82+
impl<T: Default> Default for PropagateTemplate<T> {
83+
fn default() -> Self {
84+
Self(Default::default())
85+
}
86+
}
87+
88+
impl<C: FromTemplate + Component + Clone + PartialEq> FromTemplate for Propagate<C> {
89+
type Template = PropagateTemplate<C::Template>;
90+
}
91+
92+
impl<C: Template<Output: Component + Clone + PartialEq>> Template for PropagateTemplate<C> {
93+
type Output = Propagate<C::Output>;
94+
95+
fn build_template(
96+
&self,
97+
context: &mut bevy_ecs::template::TemplateContext,
98+
) -> bevy_ecs::error::Result<Self::Output> {
99+
Ok(Propagate(self.0.build_template(context)?))
100+
}
101+
102+
fn clone_template(&self) -> Self {
103+
PropagateTemplate(self.0.clone_template())
104+
}
105+
}
106+
78107
/// Stops the output component being added to this entity.
79108
/// Relationship targets will still inherit the component from this entity or its parents.
80109
///

crates/bevy_camera/src/visibility/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ pub use render_layers::*;
4545
use bevy_app::{Plugin, PostUpdate, ValidateParentHasComponentPlugin};
4646
use bevy_asset::prelude::AssetChanged;
4747
use bevy_asset::{AssetEventSystems, Assets};
48-
use bevy_ecs::{prelude::*, VariantDefaults};
48+
use bevy_ecs::prelude::*;
4949
use bevy_reflect::{std_traits::ReflectDefault, Reflect};
5050
use bevy_transform::{components::GlobalTransform, TransformSystems};
5151
use bevy_utils::{Parallel, TypeIdHashMap};
@@ -77,7 +77,7 @@ pub struct NoCpuCulling;
7777
///
7878
/// To read the visibility of an entity, query for its [`InheritedVisibility`] instead.
7979
/// For more information, see [module level documentation](self#what-is-the-difference-between-visibility-components).
80-
#[derive(Component, Clone, Copy, Reflect, Debug, PartialEq, Eq, Default, VariantDefaults)]
80+
#[derive(Component, Clone, Copy, Reflect, Debug, PartialEq, Eq, Default)]
8181
#[reflect(Component, Default, Debug, PartialEq, Clone)]
8282
#[require(InheritedVisibility, ViewVisibility)]
8383
pub enum Visibility {

crates/bevy_ecs/macros/src/lib.rs

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ mod query_data;
1010
mod query_filter;
1111
mod resource;
1212
mod template;
13-
mod variant_defaults;
1413
mod world_query;
1514

1615
use crate::{query_data::derive_query_data_impl, query_filter::derive_query_filter_impl};
@@ -951,12 +950,3 @@ pub fn derive_from_world(input: TokenStream) -> TokenStream {
951950
pub fn derive_from_template(input: TokenStream) -> TokenStream {
952951
template::derive_from_template(input)
953952
}
954-
955-
/// Derives a `default_<name>` for each branch of an `enum`
956-
/// for use with [`Template`].
957-
///
958-
/// [`Template`]: template/trait.Template.html
959-
#[proc_macro_derive(VariantDefaults)]
960-
pub fn derive_variant_defaults(input: TokenStream) -> TokenStream {
961-
variant_defaults::derive_variant_defaults(input)
962-
}

0 commit comments

Comments
 (0)