Rename Node to NodeSize

[ickshonpe]

Objective

Rename Node to NodeSize.

Reasons:

• Better semantics.
• Node is ambiguous with taffy::node::Node and bevy_render::render_graph::node::Node.

Changelog

• Renamed Node to NodeSize.

Migration Guide

Node has been renamed to NodeSize.

[github-actions]

It looks like your PR is a breaking change, but you didn't provide a migration guide.

Could you add some context on what users should update when this change get released in a new version of Bevy?
It will be used to help writing the migration guide for the version. Putting it after a ## Migration Guide will help it get automatically picked up by our tooling.

[nicopap]

I'm so in for adding into the type's name Size, because that's what it is. It's difficult to find when you need to access a UI node size and the way of doing it doesn't have word "size" in it.

The "ambiguity" with other types named Node IMO is a weak argument to change the name (prog lang item names are only relevant within certain contexts, and it's fine to have name conflicts for items that are not used in the same context), but the semantics clarity improvement is enough for me.

But… now the name is NodeSize, it does imply we are talking about the size of something else that is a Node, but in reality, Node doesn't exist anymore.

I would suggest this: create a new marker component called Ui (or UiElement, UiNode). use this in places where Node was used but its size not accessed; Rename NodeSize to UiSize or UiNodeSize.

Also, I see a few variables named node: Node, now they are node: NodeSize, I think those variables should be renamed to size.

What do you think of this?

[ickshonpe]

Agreed, except that the taffy::node::Node / Node ambiguity is quite aggravating in the layout module.

Changed the node: NodeSize parameters and variable names where I could find them as you suggested.

I'm not sure about adding a UI marker component. I think I guess that the "something" that makes an entity what it is is the composition of its components and an extra marker component is redundant.

UiNodeSize / UiSize was something I considered too, I prefer it but not sure how other people would see it. I think in the case we did go with a Ui- prefix, NodeBundle should be renamed to UiNodeBundle as well (or just UiBundle even).

Also started thinking that NodeSize doesn't quite capture the correct semantics either. Maybe CalculatedSize or DerivedSize would be better.

[nicoburns]

Agreed, except that the taffy::node::Node / Node ambiguity is quite aggravating in the layout module.

FWIW, taffy::node::Node doesn't exist in Taffy 0.4. It's replacement is called taffy::tree::NodeId.

I'm not sure about adding a UI marker component. I think I guess that the "something" that makes an entity what it is is the composition of its components and an extra marker component is redundant.

I think we definitely need something that marks all entities that the UI systems interact with. I'd be pretty surprised if we aren't already querying for Node in systems (and thus removing such a marker would actually break bevy_ui).

I have mixed feeling regarding splitting out the NodeSize component. One the one hand I think this would make it much more discoverable, and it's something people have struggled to find. On the other hand, it does make some amount of sense to be in the Node component given that it is something that every UI node has. I think it would be less confusing if/when the Node component gained more fields. If we do then I think that CalculatedNodeSize or ComputedNodeSize or UiCalculatedSize/ UiComputedSize woud be a good name.

UiNodeSize / UiSize was something I considered too, I prefer it but not sure how other people would see it. I think in the case we did go with a Ui- prefix, NodeBundle should be renamed to UiNodeBundle as well (or just UiBundle even).

I would definitely be in favour of prefixing the UI types with Ui. So UiNode, UiNodeBundle, UiText. The UI types tend not interoperate with other bevy types, so I think it is good to make that clear. In particular, Node is a very generic name and could be anything. And there are a couple of other Text types which people get confused between quite often.

[nicopap]

the marker component was only a suggestion. I agree with ickshonpe that it's questionable to add one if it hasn't a use independent of another component.

I like the suggestion of Calculated/Derived. Since we already use Calculated in other places, and Size alone is imprecise, I'd suggest CalculatedUiSize (even if I personally usually strongly prefer shorter type/variable names)

I don't see what other fields Node could have. By the nature of ECS, I actually think it's impossible in fact to add fields to Node (since they would be part of another component, due to features (and therefore systems) working orthogonally on components).

I think it might make sense to retire Node. It's a weird conceit, but yeah, there isn't really a "UI" component per se, so why add one? This would need a second pass to remove Node from the bevy ui vocabulary…

I'm also slightly worried about API churn. When I read the release note, there is always the cynic in me that goes "they really don't care about the user, why on earth are they changing over and over those public names?" If there isn't a strong justification for a change like that, I think it's wiser to avoid it.

[djeedai]

I was going to make the same suggestion about using Size in the name. But then I remembered that it's a calculated size. And the calculated size is already available via a Node::size() method, so would be redundant. More importantly, the name Node highlights two important things that are lost with the suggested renames:

1. This is the component that turns this Entity into a UI thing. This is a crucial marker for UI. And weakening its importance via renaming feels like we're blurring the line between UI and non-UI related entities.
2. It's part of a hierarchy, being a "node", and therefore its behavior is dependent on other parent/sibling/children nodes. It's not an isolates object. Although see discussion in Remove `Transform` hierarchy in UI #8941.

So in the end I feel like UiNode would improve consistency with, say, the recent UiRect rename. But I'd leave any "size" concept out of the name. I'd keep this as a marker for the UI system to find all UI entities, and allow shoving any internal calculations as private data on it (and we might add more than size in the future).