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

Make bevy extension crates opt in #128

Open
PPakalns opened this issue Mar 17, 2023 · 4 comments
Open

Make bevy extension crates opt in #128

PPakalns opened this issue Mar 17, 2023 · 4 comments

Comments

@PPakalns
Copy link

bevy_log, bevy_math, bevy_asset and other crates and their type registrations aren't always needed if partial functionality, minimal functionality of bevy is used.

As all crates by default are included, project compilation has large overhead, as these types are registered in type system I believe that part of them are not optimized out and causes overhead during runtime.

  1. Make type registration not strict, at least now when registering new ui value representations, if type is not registered, it raises panic. Strict validation causes that all plugins need to be added to bevy app for this library to work.

  2. Make additional bevy libraries and their type registrations optional to reduce compilation time overhead.
    For default use case, extension libraries can be added to default-features.

Thanks! This is a very nice library.

@jakobhellermann
Copy link
Owner

image

As far as I know, the only crate that can actually be a bottleneck is bevy_pbr, which is already optional.

bevy_render -> bevy_egui -> bevy_inspector_egui is a dependency chain that can't be broken up. bevy_render depends on bevy_asset and bevy_log so those are free. bevy_math is even earlier as it is in the bevy_math -> bevy_reflect -> bevy_ecs chain.

@jakobhellermann
Copy link
Owner

What I could do, is not depend on bevy_pbr. Instead of accessing e.g. bevy_pbr::AmbientLight by type id to insert the options, I would need to do a more fragile type name lookup to find the registration and then insert the options. But this isn't great because the type name isn't stable, a type can be moved in a non-breaking release.

@PPakalns
Copy link
Author

I think the problem is not with bevy crates themselves, but dependencies that they depends on.

For example bevy_math depends on large glam math crate.

In couple of days I will probably try to integrate this crate in my partial bevy usage case again and will try to prepare minimal PR where the different bevy crates and their type addition are under #[cfg(feature="bevy_*")] and type addition for inspector for unregistered types doesn't cause error.

@jakobhellermann
Copy link
Owner

I'm open to making every crate optional that can be entirely removed from the dependency tree (so making bevy_asset optional wouldn't make sense).

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

No branches or pull requests

2 participants