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

Feature: re-usable component bundles #193

Closed
kvark opened this issue Sep 17, 2021 · 8 comments · Fixed by #194
Closed

Feature: re-usable component bundles #193

kvark opened this issue Sep 17, 2021 · 8 comments · Fixed by #194
Labels
enhancement New feature or request

Comments

@kvark
Copy link

kvark commented Sep 17, 2021

I'd like to efficiently re-use component bundles that aren't known statically.
Implementing DynamicBundle myself seems painful and not intended.
Re-using can only happen if all of the components are Clone.
Ideally, we'd have impl DynamicBundle for &'a EntityBuilder if we know that all of the components are cloneable. This would allow me to just keep an EntityBuilder around and use it as a bundle.

@Ralith Ralith added the enhancement New feature or request label Sep 17, 2021
@Ralith
Copy link
Owner

Ralith commented Sep 17, 2021

I think this is widely useful, so I'm interested in having it. In particular, it makes it easy to repeatedly spawn individual entities based on a dynamically-loaded prototype with minimal boilerplate. I believe this is the second time I've received this specific request. However, it represents a significant API design challenge.

As a baseline, this could be accomplished by duplicating most of EntityBuilder, with the addition of requiring Clone for each component type and storing a function pointer that clones into supplied memory, to be invoked from the DynamicBundle::put implementation. Of course, we'd rather not duplicate that logic, or even duplicate the interface in a newtype around a shared structure.

What we need is some way to abstract EntityBuilder over the presence/absence of Clone, and carry around additional metadata necessary to satisfy those bounds dynamically. My first idea is a marker type parameter indicating whether the Clone requirement is present, with available methods differing based on which marker type is supplied. I'll try to evaluate whether this approach is viable tonight.

@Ralith
Copy link
Owner

Ralith commented Sep 17, 2021

As an aside, a somewhat inefficient but easy, stable, and safe workaround for this issue could look something like Vec<Box<dyn Fn(&mut EntityBuilder)>>, populated with closures that insert a component into a builder.

@ten3roberts
Copy link
Contributor

The EntityBuilderClone is extremely useful.

Although it is not published on crates.io. Is it possible to include it in a non breaking minor release?

@Ralith
Copy link
Owner

Ralith commented Nov 27, 2021

Glad to hear it's helpful! Is there some specific breakage you're concerned about? I was planning on cutting 0.7 very soon; I can backport to 0.6 if necessary, but I'd be surprised if the upgrade presented a problem.

@ten3roberts
Copy link
Contributor

Not specifically. I am developing a game engine using hecs and making a template system, which is why reusable builders would be phenomenal.

I am also planning on writing a pipelining crate for hecs to allow for multithreaded schedules with systems. This also links back to issue #19 and weather I should include a commanbuffer provided by the scheduler, or if the world should supply one.

@Ralith
Copy link
Owner

Ralith commented Nov 27, 2021

I've now published 0.7.0. Sounds like you might be interested in the new CommandBuffer helper as well! #19 wouldn't be great for multithreading, since it requires an &mut World.

@ten3roberts
Copy link
Contributor

Absolutely zero breakages in https://github.com/ten3roberts/ivy, and one compile time breakage in https://github.com/ten3roberts/hecs-hierarchy relating to Entity::from_bits returning Option, which is fixed and published.

Splendid work 👍

@Ralith
Copy link
Owner

Ralith commented Nov 28, 2021

Great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants