Migrate UI bundles to required components
Almost all of our bundle types have been migrated to required components after #14791. However, UI bundles have not been done yet.
#15550 will conflict with that, but since that PR needs more time to bake, we should press on with this work for the 0.15 release.
The strategy to do so is laid out here: https://hackmd.io/@bevy/required_components/%2FpuAA8c18TzeMhjclo36vEQ#Combined-Proposal-1-Conservative-Port-no-major-reworks-Selected
Ah I'm stupid I was worried this was going to be terrible but I hadn't realised required components were transitive until now 😅
I think though that Style might be better a choice instead of Node for the primary component. I've been thinking of a few constructions where it might be useful to have unstyled Nodes. We might want to support alternative layout algorithms as well and it'd be easier if they could output Nodes and use the existing UI extraction setup.
I think though that Style might be better a choice instead of Node for the primary component. I've been thinking of a few constructions where it might be useful to have unstyled Nodes. We might want to support alternative layout algorithms as well and it'd be easier if they could output Nodes and use the existing UI extraction setup.
I think most of Style's fields should be migrated to Node, the rest should be moved elsewhere, and Style should be removed. So in a way, I agree.
I think most of Style's fields should be migrated to Node, the rest should be moved elsewhere, and Style should be removed. So in a way, I agree.
Do you want to do that now, or wait for 0.16? I firmly agree about the ultimate action though.
Do you mean that Style and Node should be consolidated into one component containing both the layout constraints and the resolved layout values? Or is it more like Style being renamed to Node and the resolved layout values being moved somewhere else.
I'd prefer it if we had more granular UI components anyway.
I also prefer more granular UI components, i.e., keeping Node and Style separated. I'm waiting for the community to reach a consensus before migrating the examples in the PR (there are about 300 instances that need to be migrated in the examples).
Do you want to do that now, or wait for 0.16? I firmly agree about the ultimate action though.
I think we should do the conservative port as outlined in the hackmd and do more involved changes in a later release.
Do you mean that Style and Node should be consolidated into one component containing both the layout constraints and the resolved layout values? Or is it more like Style being renamed to Node and the resolved layout values being moved somewhere else.
I think Node should be the "driving" public facing interface containing all of the "styleable" properties inherent to a UI node. For computed values (such as calculated_size), we should move those to one or more separate components.
There should be no more generic Style component. You "style" fields on specific components, you don't set Style.
I think Node should be the "driving" public facing interface containing all of the "styleable" properties inherent to a UI node. For computed values (such as calculated_size), we should move those to one or more separate components.
That sounds good. I've never liked how we keep the relatively non-distinct geometric layout constraints in a component called Style
It still bothers me though that the ouput component containing the computed layout is going to be the primary defining component, even if it's only intended to be temporary it doesn't feel very pleasant. Couldn't we do a renaming with Style becoming Node (and the primary component) and Node becoming ResolvedNode (or whatever) to go along with the required components migration?
I'd really try to reduce breaking changes for right now: this migration is already going to be really painful.
If we don't want breaking changes then Style could be the central component for now, using Node (without changes) doesn't make sense with the new require api. Required components is meant to make it unnecessary to explicitly spawn essential nonconfigurable components but since it's relatively uncommon to spawn completely unstyled UI nodes VitalyAnkh's migration PR is full of code like:
commands
.spawn((
Node::default(),
Style {
width: Val::Percent(100.),
height: Val::Percent(100.),
..default()
}
))
which seems obviously wrong to me. I realise this really is not all that important though and it will be fixed in 0.16. I'll leave this alone now and go and fix some bugs.
Last thing, we could put require on both Style and Node?
It's frustrating to see numerous instances of Node::default() following a Style in the examples. But this approach makes migration to Bevy 0.15 more intuitive for users.
@ickshonpe Are you proposing that we make Style require Node and use Style as the public interface? If we pursue this approach—moving most of Style's components into Node and using Node as the public UI interface in 0.16—we'd be creating an awkward transition (Node -> Style in 0.15, then Style -> Node in 0.16).
I suggest we either make no changes beyond the required component migration, or implement a comprehensive overhaul. A middle ground approach might lead to unnecessary complexity.
@ickshonpe Are you proposing that we make
StylerequireNodeand useStyleas the public interface? If we pursue this approach—moving most ofStyle's components intoNodeand usingNodeas the public UI interface in 0.16—we'd be creating an awkward transition (Node->Stylein 0.15, thenStyle->Nodein 0.16).
I don't think it makes the transition any more awkward or unituitive. It isn't Node -> Style, it's NodeBundle to Style + required components. Bevy users are much more familiar with Style than Node which is normally only used internally. For the 0.16 transition Style gets removed and the fields are moved to Node which would just mean a change from:
commands.spawn(
Style {
width: Val::Percent(100.),
height: Val::Percent(100.),
..default()
}
);
to
commands.spawn(
Node {
width: Val::Percent(100.),
height: Val::Percent(100.),
..default()
}
);
I guess? Or maybe we'll have bsn in by then, I'm not sure about the timeline.
@ickshonpe You're right. I totally agree with you. I will update the PR. Thanks!