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

NamedHierachy for find entities #1830

Closed
wants to merge 4 commits into from

Conversation

lassade
Copy link
Contributor

@lassade lassade commented Apr 5, 2021

Required by #1429, used to find witch entities to animate given a set of named entities;

@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Apr 5, 2021
/// this function takes an vec of entities defined by their parent index
/// (on the same vec) and name.
///
/// Any root entity should be indexed using `I::MAX_VALUE` or `I::MAX`.
Copy link
Member

@mockersf mockersf Apr 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you add a const on NamedHierarchy to help this?

    const ROOT_ENTITY: I = I::MAX_VALUE;

and then document to use NamedHierarchy::ROOT_ENTITY instead of I::MAX_VALUE

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I liked this suggestion, but I used the NO_PARENT, the wording of this docs wasn't right. It's not the index of the root entity but the index of the root parent entity

@alice-i-cecile alice-i-cecile added the A-Animation Make things move and change over time label Apr 7, 2021
lassade added a commit to lassade/bevy that referenced this pull request Apr 11, 2021
@cart cart added the S-Pre-Relicense This PR was made before Bevy added the Apache license. Cannot be merged or used for other work label Jul 23, 2021
@mockersf mockersf removed the S-Pre-Relicense This PR was made before Bevy added the Apache license. Cannot be merged or used for other work label Jul 24, 2021
@alice-i-cecile alice-i-cecile added the S-Needs-Design-Doc This issue or PR is particularly complex, and needs an approved design doc before it can be merged label Sep 22, 2021
@alice-i-cecile alice-i-cecile added the X-Controversial There is active debate or serious implications around merging this PR label Apr 25, 2022
@james7132 james7132 closed this Mar 16, 2023
@james7132
Copy link
Member

This has unfortunately drifted a bit too far from our current animation implementation, and I have concerns about it's global nature. There may be use in the future for such a cached hierarchy for name queries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Animation Make things move and change over time A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Needs-Design-Doc This issue or PR is particularly complex, and needs an approved design doc before it can be merged 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.

5 participants