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

A spawn trait for Commands and ChildBuilder #5647

Open
tguichaoua opened this issue Aug 10, 2022 · 2 comments
Open

A spawn trait for Commands and ChildBuilder #5647

tguichaoua opened this issue Aug 10, 2022 · 2 comments
Labels
A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible X-Controversial There is active debate or serious implications around merging this PR

Comments

@tguichaoua
Copy link
Contributor

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

Both Commands and ChildBuilder share the methods spawn and spawn_bundle.

In my project I create a UiExt that add methods to spawn some ui related structure of entity (e.g. for scrolling list) that look like this :

pub trait UiExt<'w, 's> {
    fn spawn<'a>(&'a mut self) -> EntityCommands<'w, 's, 'a>;

    fn spawn_scroll_list<'a>(
        &'a mut self,
        style: Style,
        spawn_items: impl FnOnce(&mut ChildBuilder),
    ) -> EntityCommands<'w, 's, 'a> {
        let mut entity_commands = self.spawn();
        scroll_list::build_scroll_list(&mut entity_commands, style, spawn_items);
        entity_commands
    }
}

impl<'w, 's> UiExt<'w, 's> for Commands<'w, 's> {
    fn spawn<'a>(&'a mut self) -> EntityCommands<'w, 's, 'a> {
        self.spawn()
    }
}

impl<'w, 's, 'a> UiExt<'w, 's> for ChildBuilder<'w, 's, 'a> {
    fn spawn<'b>(&'b mut self) -> EntityCommands<'w, 's, 'b> {
        self.spawn()
    }
}

The methods are the same whatever if self is Commands or ChildBuilder as long as I can get a EntityCommands.
But because spawn is not implemented via a trait I cannot simply use a blanket implementation.

What solution would you like?

Expose spawn via a trait for Commands and ChildBuilder.

Allowing blanket implementation:

impl<'w, 's, T> UiExt<'w, 's> for T
where
    T : SpawnEntityCommands<'w, 's>
{
    fn spawn_scroll_list<'a>(
        &'a mut self,
        style: Style,
        spawn_items: impl FnOnce(&mut ChildBuilder),
    ) -> EntityCommands<'w, 's, 'a> {
        let mut entity_commands = self.spawn();
        scroll_list::build_scroll_list(&mut entity_commands, style, spawn_items);
        entity_commands
    }
}

What alternative(s) have you considered?

Add a spawn method in my UiExt trait and manually implement it for Commands and ChildBuilder.

@tguichaoua tguichaoua added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Aug 10, 2022
@Weibye Weibye added A-ECS Entities, components, systems, and events X-Controversial There is active debate or serious implications around merging this PR and removed S-Needs-Triage This issue needs to be labelled labels Aug 10, 2022
@asafigan
Copy link
Contributor

asafigan commented Sep 1, 2022

It might also be good to add this trait to World.

@hw762
Copy link

hw762 commented Nov 24, 2022

This would also be helpful with spawn_batch function, as ChildBuilder doesn't have it and is significantly hitting my performance.

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-Feature A new feature, making something new possible X-Controversial There is active debate or serious implications around merging this PR
Projects
None yet
Development

No branches or pull requests

4 participants