-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
force parenthesis for elements in the children! macro list #20760
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
30f1b2c
0bf285a
e3ff6d2
44ee1b0
a42d573
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -665,14 +665,14 @@ mod linear_gradient { | |
angle: LinearGradient::TO_RIGHT, | ||
stops: stops.clone(), | ||
}), | ||
children![ | ||
children![( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel like this was probably intended to be spawned as a single bundle, as having separate entities with just a If so, then I believe this highlights the value of this PR ;) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the CI confirms this https://pixel-eagle.com/project/b04f67c0-c054-4a6f-92ec-f599fec2fd1d/run/22845/compare/22830?screenshot=testbed_ui/screenshot-LinearGradient.png (what an amazing setup BTW!). I feel like we probably want to use different font settings as this makes the output less readable. |
||
Node { | ||
position_type: PositionType::Absolute, | ||
..default() | ||
}, | ||
TextFont::from_font_size(10.), | ||
bevy::ui::widget::Text(format!("{color_space:?}")), | ||
] | ||
bevy::ui::widget::Text(format!("{color_space:?}")) | ||
),] | ||
)], | ||
)); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This choice introduces significant syntactic noise. I personally still think the "parentheses are optional" rule is preferable.
The the intuitive read of this list is correct:
Rust trains us to read this as "this is a list of the children A, B, and C".
The grammar is "this is a list of children: if you want a child to have more than one component, add parenthesis". That is not so hard to learn that it justifies the added labor of changing it to "this is a list of children, each child must have parenthesis".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wherever this lands, the resulting choice needs to be in sync with BSN
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mm, I really understand where you're coming from, and if I didn't experience this confusion myself a couple times, I would probably agree with you.
I think the confusion comes from the fact that conceptually bundles are also lists. Given that lists of lists of components are not a common pattern anywhere else, it's super easy to miss the extra nesting. This in itself would again not be so bad if the resulting bugs are usually not immediately obvious and quite difficult to diagnose.
Not to sound too melodramatic, but I fear that a novice making this mistake by accident could be inclined to write Bevy off as unrealible. It really doesnt take long for a new user to run into the children macro, especially when doing ui related stuff.
Finally, I was also never a huge fan of the parenthesis solution, would you maybe have something else in mind?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the main here for me is how often are you creating children with a single component like
children![A, B, C]
? As a user of bevy I don't think I've ever done this, usually its a group of components I want to add.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's quite common when building ui's to have multiple children with just a
Node
or just aText
.Conversely, since required components I often have just a
Sprite
initially. Then later when you want to add some logic and slap a marker component on it, you have to be very carefully to remember the parenthesis.