Make bevy_math optional in bevy_core

[CAD97]

Notably, this makes bevy_core, bevy_diagnostic, and bevy_asset no longer require pulling in bevy_math. None of these crates directly depend on the math primitives, so this is a net positive for use outside of bevy, which can now use these crates without pulling in bevy_math and glam.

Objective

I'm building my own micro-engine off of the bevy primitives for learning purposes, and as part of that, I'm using my own simplistic math library rather than using bevy_math/glam. (Basically, I would like to have everything up to but not including rendering and math.) I was looking at pulling in bevy_asset to handle asset loading (an ugly task I have no interest in re-solving), but noticed it was pulling in bevy_math indirectly.

Cue surprise that bevy_math isn't actually used in any fashion, just to eagerly .register_type in CorePlugin. Putting that functionality behind features = ["bevy"] (as is done in bevy_reflect) is a very minor change that enables me to use bevy_asset without needlessly pulling in glam.

Solution

• Gate CorePlugin's registering of bevy_math types for reflection behind an optional bevy_math dependency
• Add a bevy feature to bevy_core which enables bevy_math and bevy_reflect's bevy feature
• Enable bevy_core's bevy feature via bevy_internal
  • This is the decision which is most arbitrary, but doing so felt most natural; the feature is necessarily only used by the consumer of CorePlugin, which is the bevy end-user.

[alice-i-cecile]

Looks good! Any detangling of our dependency bush is a win IMO.

The code changes here are very simple.

[inodentry]

Yes, this seems like an easy win for bevy's modularity and usefulness in other projects, without significant drawback for use inside bevy. I like this. :)

[cart]

This feels slightly "off" to me, probably because the context of bevy_core is already "bevy". You've arbitrarily decided to put the bevy reflection feature and math into a "bevy" feature bucket based on your project's specific needs. What happens when somebody else doesn't want bevy_tasks / DefaultTaskPool registration and wants to make them optional. Will we put them under the bevy feature? What if your project needs bevy_tasks?

[CAD97]

I think my argument comes down to two main things:

• bevy_math isn't core (ha) to what bevy_core/diagnostic/asset are doing. Rather, the inclusion is just for the effect of pre-registering the bevy_math types for reflection as part of CorePlugin.
• The label/name/time modules' types are very useful building blocks to reuse to anyone building on bevy_ecs.

My "goal" here is to use bevy_asset in my own project that isn't using bevy_math or anything above it in the dependency tree. It's sort of an arbitrary cutoff, but it's also the level I'm interested in working at.

(Side note: you could even make an argument against bevy_core turning on the bevy_reflect/bevy feature; it's enabled again at the bevy_internal level.)

I understand there's a need to at some point in the tree just say that external use without part of the crate just isn't a supported use case. I'm just trying to make a case to make bevy_asset not require bevy_math, because the level of "required for functionality" for using bevy_asset is functionally zero. (You use bevy_asset without registering CorePlugin.)

If there's a better way to achieve this (e.g. invert the bevy_asset -> bevy_diagnostic dependency or make that dependency optional), I'm happy to do that instead. Or I'm honestly fine keeping this patch locally (what I'm playing with likely won't be a published crate).

[CAD97]

I think the core-ish conflict here is that bevy_core is sort of doing two, maybe three distinct jobs. It's providing the name and time modules' types for use in an ECS, label module for use with bevy_ecs, the Bytes interface for GPU buffer serialization, and the CorePlugin for bevy_app for initializing the above resources, initializing bevy_task, and registering primitive and bevy_math types for bevy_serialization.

The "problem" then comes from pulling one necessarily pulling the others. bevy_diagnostic only uses the time types, and bevy_asset even only optionally reports any diagnostics.

My "issue" isn't that bevy_core assumes bevy_math is desirable, it's more that bevy_asset, which is quite individually useful, has a dep noticeably fatter tree than required for standalone use with bevy_app/bevy_ecs but not the whole of bevy.

It's fine if this is a modularity border that the bevy project doesn't see value in setting -- after all, the difference is just cold compile times for downstream using bevy piecewise, since the unused code/crates will trivially optimize out -- but I personally think that this is a border worth putting at least a little bit of "as-is" support into, thus filing this PR.

But, again, it's perfectly understandable if the bevy project doesn't want to carve this out this way. And if there is a better way that you'd like to provide bevy_asset for lighterweight use, I'd be happy to implement that approach as well.

Trimmed, annotated dep tree of bevy_asset

bevy_asset v0.5.0 (a60fe30)
├── bevy_app v0.5.0 (a60fe30)
│   ├── bevy_derive v0.5.0 (proc-macro) (a60fe30) (...)
│   ├── bevy_ecs v0.5.0 (a60fe30) (...)
│   ├── bevy_reflect v0.5.0 (a60fe30) (*)
│   └── bevy_utils v0.5.0 (a60fe30) (*)
├── bevy_diagnostic v0.5.0 (a60fe30) (?)
│   ├── bevy_app v0.5.0 (a60fe30) (*)
│   ├── bevy_core v0.5.0 (a60fe30)
│   │   ├── bevy_app v0.5.0 (a60fe30) (*)
│   │   ├── bevy_derive v0.5.0 (proc-macro) (a60fe30) (*)
│   │   ├── bevy_ecs v0.5.0 (a60fe30) (*)
│   │   ├── bevy_math v0.5.0 (a60fe30) (..!)
│   │   ├── bevy_reflect v0.5.0 (a60fe30) (*)
│   │   ├── bevy_tasks v0.5.0 (a60fe30) (*)
│   │   ├── bevy_utils v0.5.0 (a60fe30) (*)
│   │   └── ... (*)
│   ├── bevy_ecs v0.5.0 (a60fe30) (*)
│   ├── bevy_log v0.5.0 (a60fe30)
│   └── bevy_utils v0.5.0 (a60fe30) (*)
├── bevy_ecs v0.5.0 (a60fe30) (*)
├── bevy_log v0.5.0 (a60fe30) (*)
├── bevy_reflect v0.5.0 (a60fe30) (*)
├── bevy_tasks v0.5.0 (a60fe30) (*)
├── bevy_utils v0.5.0 (a60fe30) (*)
└── ...

Symbol key: *: included previously; ...: omitted (transitive) deps; ?: questionable/optional/minimal/noncore use (to root); !: basically unused (to root)

[cart]

I do agree that this is worth discussing and I think you've zero-ed in on the core issue: bevy_core currently serves a number of different roles:

• type registrations: math / std / ecs / time / misc
• time
• names / labels
• FloatOrd
• Byte conversion
• task pool setup

It is the "glue" that ties miscellaneous things without their own crates into bevy apps, often when the actual crates cannot do the registering themselves due to circular dependencies (ex: bevy_tasks is used in bevy_ecs, so we can't handle plugin stuff in bevy_tasks). I do think that it is worth revisiting bevy_core. I object to the current impl in this pr for the reasons I called out above, but I think we can probably find a solution that satisfies everyone:

• Byte conversion will be deprecated in favor of bytemuck and crevice once the new renderer gets merged
• bevy_math might actually be able to define its own plugin / take a dependency on bevy_app. this forces bevy_math consumers to also consume bevy_app / bevy_ecs / etc, but maybe thats ok.
• time stuff could be broken out into bevy_time
• FloatOrd could be moved to bevy_utils (although its type registration cannot)
• Type registrations could be moved somewhere else (maybe even the top level bevy_internal crate)
• We could consider doing a task pool setup plugin in bevy_ecs or bevy_internal

In short, we might be able to do away with bevy_core entirely. I'd prefer to start doing this stuff after the new renderer is merged to avoid conflicts / allow us to remove the Bytes stuff entirely. These are all straightforward changes that won't take any real time investment.

[CAD97]

Filed #2931 to track the enhancement properly.