Skip to content

GhostNode trait #17901

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

GhostNode trait #17901

wants to merge 11 commits into from

Conversation

ickshonpe
Copy link
Contributor

@ickshonpe ickshonpe commented Feb 17, 2025

Objective

Replace the UI's ghost node implementation with a trait that can be used with any hierarchy, not just UI Nodes.

Solution

Make GhostNode into a trait with an Actual associated type.
GhostNode should be implemented on a marker component that must be inserted on every entity in the collapsible hierarchy.
The flattened view of the hierarchy sees only those nodes with an Actual component.

The UI specfic parts of the old ghost nodes implementation and the feature gate have been moved to a new module named navigation.

The marker component that implements GhostNode is called UiNode and the associated Actual type is Node. Node requires UiNode.

The navigation SystemParams are mostly unchanged except for some renamings:

  • iter_ui_children -> iter_actual_children
  • is_ui_node -> is_actual

Migration Guide

Give every entity in the UI hierarchy a `UiElement` marker component and, if the "ghost_nodes" feature is enabled, implement the `Ghost` trait for `UiElement`
* `Ghost` to `GhostNode`
* `UiElement` to `UiNode`
* `Ghost::Completion` to `GhostNode::Actual`

Both system param impl changes:
* `iter_ui_children` to `iter_actual_actual_children`
* `is_ui_node` to `is_actual_node`

The `experimental` module now only contains the `GhostNode` trait and it's implementation. It works with any hierarchy now, not just UI nodes.

The "ghost_nodes" feature gate and UI specfic navigation implementation has been moved to a new `navigation` module.
@ickshonpe ickshonpe added A-UI Graphical user interfaces, styles, layouts, and widgets X-Controversial There is active debate or serious implications around merging this PR A-ECS Entities, components, systems, and events labels Feb 19, 2025
@janhohenheim
Copy link
Member

Triage: has merge conflicts
@ickshonpe do you want to update this PR or should I tag it S-Needs-Adoption? :)

@janhohenheim janhohenheim added the S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged label May 17, 2025
@ickshonpe ickshonpe added S-Blocked This cannot move forward until something else changes and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels May 19, 2025
@ickshonpe
Copy link
Contributor Author

ickshonpe commented May 19, 2025

Triage: has merge conflicts @ickshonpe do you want to update this PR or should I tag it S-Needs-Adoption? :)

I can't even remember writing these ghost node PRs. I think updating them is pointless right now, need to get the computed node and UI transform changes merged first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events A-UI Graphical user interfaces, styles, layouts, and widgets S-Blocked This cannot move forward until something else changes X-Controversial There is active debate or serious implications around merging this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants