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

Move methods on Children that mutate to EntityCommands #15270

Open
ItsDoot opened this issue Sep 17, 2024 · 1 comment
Open

Move methods on Children that mutate to EntityCommands #15270

ItsDoot opened this issue Sep 17, 2024 · 1 comment
Labels
A-ECS Entities, components, systems, and events C-Code-Quality A section of code that is hard to understand or change D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Design This issue requires design work to think about how it would best be accomplished X-Contentious There are nontrivial implications that should be thought through

Comments

@ItsDoot
Copy link
Contributor

ItsDoot commented Sep 17, 2024

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

All of our hierarchy management controls happen through Commands, except methods of sorting children for a given entity. These methods should be changed to Commands like the rest of the hierarchy management. This might be required when changing Children to a relation, anyways.

What solution would you like?

Replace the following methods with commands:

Ideally:

impl EntityCommands {
    pub fn sort_children_by(&mut self, compare: impl FnMut(Entity, Entity) -> Ordering) -> &mut Self;
    pub fn sort_children_by_key<K: Ord>(&mut self, compare: impl FnMut(Entity) -> K) -> &mut Self;
    // etc...
}
@ItsDoot ItsDoot added A-ECS Entities, components, systems, and events C-Code-Quality A section of code that is hard to understand or change S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! A-Hierarchy D-Straightforward Simple bug fixes and API improvements, docs, test and examples labels Sep 17, 2024
@alice-i-cecile
Copy link
Member

alice-i-cecile commented Sep 17, 2024

This can't be an impl block on EntityCommands directly, since bevy_ecs doesn't know about children and you can't have foreign impl blocks. That said, a trait extension method will work just fine here.

That said, this is a UX regression: commands are non-immediate, but these methods can do direct mutation (for now). I'm not sold on the idea as a result.

@alice-i-cecile alice-i-cecile added S-Needs-Design This issue requires design work to think about how it would best be accomplished X-Contentious There are nontrivial implications that should be thought through and removed S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! labels Sep 17, 2024
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 C-Code-Quality A section of code that is hard to understand or change D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Design This issue requires design work to think about how it would best be accomplished X-Contentious There are nontrivial implications that should be thought through
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants