Skip to content
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

Add more tools to conveniently work with hierarchies, especially in the presence of ghost nodes #15609

Open
alice-i-cecile opened this issue Oct 3, 2024 · 3 comments
Labels
A-Hierarchy Parent-child entity hierarchies A-UI Graphical user interfaces, styles, layouts, and widgets C-Feature A new feature, making something new possible D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!

Comments

@alice-i-cecile
Copy link
Member

What problem does this solve or what need does it fill?

Following the introduction of #15341, some users have expressed concerns around difficulties migrating to account for ghost nodes in their hierarchies.

This is a great opportunity to add more tools for traversing hierarchies in the spirit of iter_descendants.

What solution would you like?

There are two stages to this:

  1. Add more methods to HierarchyQueryExt to capture other common tasks.
  2. Add a ghost-node aware version of this trait, which lives in bevy_ui and takes a Has<GhostNode> argument as well. These methods should be named iter_node_descendants and so on.

In theory these can be done in either order, or in a single PR. Your choice!

In terms of additional tasks to cover:

  1. root_parent: the entity that is highest up the hierarchy tree, if any.
  2. leaf_children: an iterator over the children that have no children of their own.
  3. siblings: other entities that are in direct children of the same parent.
  4. parent: a convenience method that looks for the parent (much more useful when working with ghost nodes).
  5. children: as above, but for children.

What alternative(s) have you considered?

One day (🥺), this will get rolled into #3742. It's nice to ship helpful APIs to users sooner though, and these methods will be easier to migrate since they have semantic meaning.

@alice-i-cecile alice-i-cecile added A-Hierarchy Parent-child entity hierarchies A-UI Graphical user interfaces, styles, layouts, and widgets C-Feature A new feature, making something new possible D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! labels Oct 3, 2024
@alice-i-cecile alice-i-cecile added this to the 0.15 milestone Oct 3, 2024
@alice-i-cecile
Copy link
Member Author

The UiChildren SystemParam tackles a good chunk of task 2. I'm going to start on task 1, get that merged, and then beef up that API to match it.

github-merge-queue bot pushed a commit that referenced this issue Oct 7, 2024
# Objective

- Working with hierarchies in Bevy is far too tedious due to a lack of
helper functions.
- This is the first half of #15609. 

## Solution

Extend
[`HierarchyQueryExt`](https://docs.rs/bevy/latest/bevy/hierarchy/trait.HierarchyQueryExt)
with the following methods:

- `parent`
- `children`
- `root_parent`
- `iter_leaves`
- `iter_siblings`
- `iter_descendants_depth_first`

I've opted to make both `iter_leaves` and `iter_siblings` collect the
list of matching Entities for now, rather that operate by reference like
the existing `iter_descendants`. This was simpler, and in the case of
`iter_siblings` especially, the number of matching entities is likely to
be much smaller.

I've kept the generics in the type signature however, so we can go back
and optimize that freely without a breaking change whenever we want.

## Testing

I've added some basic testing, but they're currently failing. If you'd
like to help, I'd welcome suggestions or a PR to my PR over the weekend
<3

---------

Co-authored-by: Viktor Gustavsson <[email protected]>
Co-authored-by: poopy <[email protected]>
Co-authored-by: Christian Hughes <[email protected]>
@villor
Copy link
Contributor

villor commented Oct 12, 2024

With the first half of this issue is merged (#15627), we can focus on the second half: UI tree.

The second half

For the second half of this issue, we would want to make UIChildren have feature parity with HierarchyQueryExt.

I propose we also change name of UIChildren to UiTree to better convey what it is. This could also serve as a good place to document the rules of the tree (e.g. has to be all Node/GhostNode up to the root, non-UI entities beneath the tree are allowed/ignored)

I looked over the APIs and summarized the current state along with proposed UiTree APIs:

HierarchyQueryExt (current) UiChildren (current) UiTree (proposal) Notes
parent get_parent parent
children iter_ui_children children Alternative name iter_children to indicate it is an iterator rather than a slice.
root_ancestor missing root_ancestor
iter_leaves missing iter_leaves
iter_siblings missing iter_siblings
iter_descendants missing iter_descendants
iter_descendants_depth_first missing iter_descendants_depth_first
iter_ancestors missing iter_ancestors
missing is_changed children_is_changed Could be added to hierarchy with #15670
missing missing parent_is_changed Could be added to hierarchy with #15670
irrelevant iter_ghost_nodes iter_ghost_nodes

Bonus utilities

In addition to the above, we could also add more utilities that are useful in UI, here is a list inspired by CSS psuedo-classes:

is_root No parent or only ghost node ancestors
is_empty No children
child_index The index of a child among its siblings
is_nth_child child_index(entity) == n
is_nth_last_child child_index(entity) == children().len() - n
is_first_child child_index(entity) == 0
is_last_child child_index(entity) == children().len() - 1
is_odd_child (child_index(entity) + 1) % 2 == 1
is_even_child (child_index(entity) + 1) % 2 == 0

Maybe something for a follow-up though, and I'm not sure how much bloat we want in the API.

@alice-i-cecile
Copy link
Member Author

Excellent investigation. Of the bonus APIs, I like is_root, is_empty (renamed to is_leaf) and child_index. The rest don't feel useful to me.

@alice-i-cecile alice-i-cecile removed this from the 0.15 milestone Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Hierarchy Parent-child entity hierarchies A-UI Graphical user interfaces, styles, layouts, and widgets C-Feature A new feature, making something new possible D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!
Projects
None yet
Development

No branches or pull requests

2 participants