Remove or shrink bevy_utils #11478

doonv · 2024-01-22T18:22:39Z

alice-i-cecile · 2024-01-22T18:25:43Z

Yes please: I want this crate eliminated!

alice-i-cecile · 2024-01-22T18:27:53Z

I think we may want to spin up new crates in the Bevy org for some of the stand-alone utilities.

@cart, would that make sense to you? Things like the default function crate would be good to split out, and will change slowly enough that I don't have concerns about moving it out of the monorepo.

That said, I know you're reluctant about increasing crate number, just because it's a popular misguided punching bag for "dependency bloat".

doonv · 2024-01-22T18:42:07Z

I'm curious whats the compile-time performance penalty of switching between compiling different crates vs. the compile-time performance penalty of compiling unused parts of a crate.

hymm · 2024-01-22T18:52:32Z

label is used in bevy_ecs to define ScheduleLabel trait and SystemSet trait. So maybe should be there.

get_short_name is used in bevy_asset in a Debug impl, bevy_ecs for some ambiguity reporting, and bevy_heirarchy for a warning. So I don't think it makes sense in bevy_reflect which is an optional dependency for those crates. Not sure where it should go though.

alice-i-cecile · 2024-01-22T19:27:04Z

get_short_name feels like a general purpose Rust utility, or maybe something that belongs in bevy_reflect for working with types and paths.

Use `TypeIdMap<T>` instead of `HashMap<TypeId, T>` - ~~`TypeIdMap` was in `bevy_ecs`. I've kept it there because of #11478~~ - ~~I haven't swapped `bevy_reflect` over because it doesn't depend on `bevy_ecs`, but I'd also be happy with moving `TypeIdMap` to `bevy_utils` and then adding a dependency to that~~ - ~~this is a slight change in the public API of `DrawFunctionsInternal`, does this need to go in the changelog?~~ ## Changelog - moved `TypeIdMap` to `bevy_utils` - changed `DrawFunctionsInternal::indices` to `TypeIdMap` ## Migration Guide - `TypeIdMap` now lives in `bevy_utils` - `DrawFunctionsInternal::indices` now uses a `TypeIdMap`. --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>

Use `TypeIdMap<T>` instead of `HashMap<TypeId, T>` - ~~`TypeIdMap` was in `bevy_ecs`. I've kept it there because of bevyengine#11478~~ - ~~I haven't swapped `bevy_reflect` over because it doesn't depend on `bevy_ecs`, but I'd also be happy with moving `TypeIdMap` to `bevy_utils` and then adding a dependency to that~~ - ~~this is a slight change in the public API of `DrawFunctionsInternal`, does this need to go in the changelog?~~ ## Changelog - moved `TypeIdMap` to `bevy_utils` - changed `DrawFunctionsInternal::indices` to `TypeIdMap` ## Migration Guide - `TypeIdMap` now lives in `bevy_utils` - `DrawFunctionsInternal::indices` now uses a `TypeIdMap`. --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>

# Objective Reduce the size of `bevy_utils` (#11478) ## Solution Move `EntityHash` related types into `bevy_ecs`. This also allows us access to `Entity`, which means we no longer need `EntityHashMap`'s first generic argument. --- ## Changelog - Moved `bevy::utils::{EntityHash, EntityHasher, EntityHashMap, EntityHashSet}` into `bevy::ecs::entity::hash` . - Removed `EntityHashMap`'s first generic argument. It is now hardcoded to always be `Entity`. ## Migration Guide - Uses of `bevy::utils::{EntityHash, EntityHasher, EntityHashMap, EntityHashSet}` now have to be imported from `bevy::ecs::entity::hash`. - Uses of `EntityHashMap` no longer have to specify the first generic parameter. It is now hardcoded to always be `Entity`.

# Objective Reduce the size of `bevy_utils` (bevyengine/bevy#11478) ## Solution Move `EntityHash` related types into `bevy_ecs`. This also allows us access to `Entity`, which means we no longer need `EntityHashMap`'s first generic argument. --- ## Changelog - Moved `bevy::utils::{EntityHash, EntityHasher, EntityHashMap, EntityHashSet}` into `bevy::ecs::entity::hash` . - Removed `EntityHashMap`'s first generic argument. It is now hardcoded to always be `Entity`. ## Migration Guide - Uses of `bevy::utils::{EntityHash, EntityHasher, EntityHashMap, EntityHashSet}` now have to be imported from `bevy::ecs::entity::hash`. - Uses of `EntityHashMap` no longer have to specify the first generic parameter. It is now hardcoded to always be `Entity`.

Kanabenki · 2024-02-29T11:24:13Z

Regarding the tracing utiliities in bevy_utils, I checked whether that could be easily moved to bevy_log, but that creates a deps cycle since it already depends on bevy_app and bevy_ecs because of the LogPlugin. Would it make sense to have a bevy_tracing crate just for re-exporting tracing + the few tracing utilities?

# Objective Make bevy_utils less of a compilation bottleneck. Tackle #11478. ## Solution * Move all of the directly reexported dependencies and move them to where they're actually used. * Remove the UUID utilities that have gone unused since `TypePath` took over for `TypeUuid`. * There was also a extraneous bytemuck dependency on `bevy_core` that has not been used for a long time (since `encase` became the primary way to prepare GPU buffers). * Remove the `all_tuples` macro reexport from bevy_ecs since it's accessible from `bevy_utils`. --- ## Changelog Removed: Many of the reexports from bevy_utils (petgraph, uuid, nonmax, smallvec, and thiserror). Removed: bevy_core's reexports of bytemuck. ## Migration Guide bevy_utils' reexports of petgraph, uuid, nonmax, smallvec, and thiserror have been removed. bevy_core' reexports of bytemuck's types has been removed. Add them as dependencies in your own crate instead.

…e#12313) # Objective Make bevy_utils less of a compilation bottleneck. Tackle bevyengine#11478. ## Solution * Move all of the directly reexported dependencies and move them to where they're actually used. * Remove the UUID utilities that have gone unused since `TypePath` took over for `TypeUuid`. * There was also a extraneous bytemuck dependency on `bevy_core` that has not been used for a long time (since `encase` became the primary way to prepare GPU buffers). * Remove the `all_tuples` macro reexport from bevy_ecs since it's accessible from `bevy_utils`. --- ## Changelog Removed: Many of the reexports from bevy_utils (petgraph, uuid, nonmax, smallvec, and thiserror). Removed: bevy_core's reexports of bytemuck. ## Migration Guide bevy_utils' reexports of petgraph, uuid, nonmax, smallvec, and thiserror have been removed. bevy_core' reexports of bytemuck's types has been removed. Add them as dependencies in your own crate instead.

# Objective - Attempts to solve two items from #11478. ## Solution - Moved `intern` module from `bevy_utils` into `bevy_ecs` crate and updated all relevant imports. - Moved `label` module from `bevy_utils` into `bevy_ecs` crate and updated all relevant imports. --- ## Migration Guide - Replace `bevy_utils::define_label` imports with `bevy_ecs::define_label` imports. - Replace `bevy_utils::label::DynEq` imports with `bevy_ecs::label::DynEq` imports. - Replace `bevy_utils::label::DynHash` imports with `bevy_ecs::label::DynHash` imports. - Replace `bevy_utils::intern::Interned` imports with `bevy_ecs::intern::Interned` imports. - Replace `bevy_utils::intern::Internable` imports with `bevy_ecs::intern::Internable` imports. - Replace `bevy_utils::intern::Interner` imports with `bevy_ecs::intern::Interner` imports. --------- Co-authored-by: James Liu <contact@jamessliu.com>

mnmaita · 2024-04-26T12:57:51Z

label and intern modules were moved into bevy_ecs. You can tick those whenever you can. I might work on some other items on this list later. Thanks!

mnmaita · 2024-05-23T14:10:41Z

FloatOrd can be ticked as it was moved here.

Regarding stuff like CowArc and OnDrop, does it make sense to have these as crates within the bevy repo, or would it make more sense to have them as standalone repositories within the org?

bushrat011899 · 2024-12-19T07:05:48Z

Based on a Discord message from Cart, here are some actionable items that seem to be generally agreeable:

`default` and `assert_object_safe`

These should just be a couple of micro-crates.

What should bevy_utils::default's crate be called? just-default?
What should bevy_utils::assert_object_safe's crate be called? assert-dyn-compatible? Likely should be renamed since object safe is supposed to now be referred to as dyn compatible according to some other Bevy documentation.

Once we have names/repos for these two crates, moving to them should be very straightforward, just like disqualified for get_short_name.

`async`/`Future`

These should be moved into bevy_tasks, since that is our fundamental async crate at this point in time. Downstream creates like bevy_render can freely import bevy_tasks without increasing compile times since it's already a dependency of bevy_ecs, which is about as fundamental as it gets in Bevy.

Platform Support

SyncUnsafeCell and SyncCell are unstable APIs that Bevy is providing a first-party implementation of. The hashbrown, FixedHasher, web_time, and other similar items represent platform compatibility items. Additionally, the no_std efforts have brought in critical-section, portable-atomic-util, and spin in a similar capacity. Instead of spreading these dependencies (and the techniques to work with them on the appropriate platform) across Bevy, let's consolidate them into a focussed crate, bevy_platform_support (or similar name) who's only job is to provide consistent APIs for all platforms.

`tracing`/`log`

Things like log_once should be moved into bevy_log, while the re-export should be removed entirely. Importing these crates directly where needed flattens the dependency graph, which helps with compile times.

BenjaminBrienen · 2024-12-19T09:45:21Z

If each of these are turned into its own issue, it might be easier to organize the work and make it clear if one of the tasks has been picked up.

alice-i-cecile · 2024-12-19T19:11:17Z

Yeah, spin up micro-issues please!

mnmaita · 2024-12-19T19:48:32Z

I can take the moving into bevy_tasks one.

cart · 2024-12-19T21:51:35Z

For default() I like something descriptive like short_default. Also a strong proponent of snake_case crate names / we should use that convention in any crates we make.

# Objective Fixes #16104 ## Solution I removed all instances of `:?` and put them back one by one where it caused an error. I removed some bevy_utils helper functions that were only used in 2 places and don't add value. See: #11478 ## Testing CI should catch the mistakes ## Migration Guide `bevy::utils::{dbg,info,warn,error}` were removed. Use `bevy::utils::tracing::{debug,info,warn,error}` instead. --------- Co-authored-by: SpecificProtagonist <vincentjunge@posteo.net>

…_tasks` (#16951) # Objective - Related to #11478 ## Solution - Moved `futures.rs`, `ConditionalSend` `ConditionalSendFuture` and `BoxedFuture` from `bevy_utils` to `bevy_tasks`. ## Testing - CI checks ## Migration Guide - Several modules were moved from `bevy_utils` into `bevy_tasks`: - Replace `bevy_utils::futures` imports with `bevy_tasks::futures`. - Replace `bevy_utils::ConditionalSend` with `bevy_tasks::ConditionalSend`. - Replace `bevy_utils::ConditionalSendFuture` with `bevy_tasks::ConditionalSendFuture`. - Replace `bevy_utils::BoxedFuture` with `bevy_tasks::BoxedFuture`.

# Objective - Contributes to #11478 - Contributes to #16877 ## Solution - Removed everything except `Instant` from `bevy_utils::time` ## Testing - CI --- ## Migration Guide If you relied on any of the following from `bevy_utils::time`: - `Duration` - `TryFromFloatSecsError` Import these directly from `core::time` regardless of platform target (WASM, mobile, etc.) If you relied on any of the following from `bevy_utils::time`: - `SystemTime` - `SystemTimeError` Instead import these directly from either `std::time` or `web_time` as appropriate for your target platform. ## Notes `Duration` and `TryFromFloatSecsError` are both re-exports from `core::time` regardless of whether they are used from `web_time` or `std::time`, so there is no value gained from re-exporting them from `bevy_utils::time` as well. As for `SystemTime` and `SystemTimeError`, no Bevy internal crates or examples rely on these types. Since Bevy doesn't have a `Time<Wall>` resource for interacting with wall-time (and likely shouldn't need one), I think removing these from `bevy_utils` entirely and waiting for a use-case to justify inclusion is a reasonable path forward.

# Objective - Contributes to #11478 ## Solution - Made `bevy_utils::tracing` `doc(hidden)` - Re-exported `tracing` from `bevy_log` for end-users - Added `tracing` directly to crates that need it. ## Testing - CI --- ## Migration Guide If you were importing `tracing` via `bevy::utils::tracing`, instead use `bevy::log::tracing`. Note that many items within `tracing` are also directly re-exported from `bevy::log` as well, so you may only need `bevy::log` for the most common items (e.g., `warn!`, `trace!`, etc.). This also applies to the `log_once!` family of macros. ## Notes - While this doesn't reduce the line-count in `bevy_utils`, it further decouples the internal crates from `bevy_utils`, making its eventual removal more feasible in the future. - I have just imported `tracing` as we do for all dependencies. However, a workspace dependency may be more appropriate for version management.

# Objective Fixes bevyengine#16104 ## Solution I removed all instances of `:?` and put them back one by one where it caused an error. I removed some bevy_utils helper functions that were only used in 2 places and don't add value. See: bevyengine#11478 ## Testing CI should catch the mistakes ## Migration Guide `bevy::utils::{dbg,info,warn,error}` were removed. Use `bevy::utils::tracing::{debug,info,warn,error}` instead. --------- Co-authored-by: SpecificProtagonist <vincentjunge@posteo.net>

…_tasks` (bevyengine#16951) # Objective - Related to bevyengine#11478 ## Solution - Moved `futures.rs`, `ConditionalSend` `ConditionalSendFuture` and `BoxedFuture` from `bevy_utils` to `bevy_tasks`. ## Testing - CI checks ## Migration Guide - Several modules were moved from `bevy_utils` into `bevy_tasks`: - Replace `bevy_utils::futures` imports with `bevy_tasks::futures`. - Replace `bevy_utils::ConditionalSend` with `bevy_tasks::ConditionalSend`. - Replace `bevy_utils::ConditionalSendFuture` with `bevy_tasks::ConditionalSendFuture`. - Replace `bevy_utils::BoxedFuture` with `bevy_tasks::BoxedFuture`.

mgi388 · 2025-01-08T23:44:35Z

Hey @bushrat011899 assert_object_safe is only used by bevy_ecs right?

What if we just put it in bevy_ecs?

If another crate needs it, then it can just copy the function in there right?

bushrat011899 · 2025-01-09T00:00:35Z

@mgi388 yeah that's fine. You can see it's not even a function in the traditional sense, just a way to force writing dyn Foo as a "test" of dyn compatibility.

mgi388 · 2025-01-09T00:45:48Z

@mgi388 yeah that's fine. You can see it's not even a function in the traditional sense, just a way to force writing dyn Foo as a "test" of dyn compatibility.

Yep makes sense, it's just a compile time check.

It seems tricky/impossible to keep the doctests for assert_object_safe without making it public in the crate (maybe there's a trick I'm missing, but I've tried a few things). The doctests could get moved to unit tests except for the fact that we really want to test the compile_fail one, which cannot be in a unit test.

Maybe it's fine to remain public in bevy_ecs since it was already public in bevy_utils anyway.

BenjaminBrienen · 2025-01-09T01:14:57Z

assert_object_safe is generally useful and could be its own crate.

mgi388 · 2025-01-09T02:02:34Z

assert_object_safe is generally useful and could be its own crate.

IMO the following applies here since it's just so tiny:

If another crate needs it, then it can just copy the function in there right?

I.e. Just copy the code if you want it. I defer to the maintainers though!

mintlu8 · 2025-01-09T05:09:42Z

assert_object_safe is generally useful and could be its own crate.

FYI it's called dyn compatible now, and it's literally just one line we can just

/// Assert Trait is dyn compatible
const _: Option<Box<dyn Trait>> = None;

alice-i-cecile · 2025-01-09T05:48:13Z

Yeah, no need for an extra crate. We should just move it out.

# Objective - Shrink `bevy_utils` more. - Refs #11478 ## Solution - Removes `assert_object_safe` from `bevy_utils` by using a compile time check instead. ## Testing - CI. --- ## Migration Guide `assert_object_safe` is no longer exported by `bevy_utils`. Instead, you can write a compile time check that your trait is "dyn compatible": ```rust /// Assert MyTrait is dyn compatible const _: Option<Box<dyn MyTrait>> = None; ``` --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>

doonv added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Jan 22, 2024

alice-i-cecile added C-Code-Quality A section of code that is hard to understand or change A-Utils Utility functions and types and removed C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Jan 22, 2024

doonv mentioned this issue Jan 23, 2024

Move EntityHash related types into bevy_ecs #11498

Merged

SpecificProtagonist mentioned this issue Feb 3, 2024

Use TypeIdMap whenever possible #11684

Merged

alice-i-cecile mentioned this issue Feb 5, 2024

Deprecate StableHashMap/StableHashSet #11717

Closed

james7132 mentioned this issue Mar 2, 2024

Inconsistent use of tracing and bevy_log #11298

Closed

alice-i-cecile mentioned this issue Mar 2, 2024

Replace bevy_log's tracing reexport with bevy_utils' #12254

Merged

james7132 mentioned this issue Mar 5, 2024

Disentangle bevy_utils/bevy_core's reexported dependencies #12313

Merged

alice-i-cecile mentioned this issue Mar 25, 2024

Move FloatOrd into bevy_math #12712

Closed

stinkytoe mentioned this issue Mar 26, 2024

reexported image crate from bevy_render as bevy::render::util::image #12724

Closed

mnmaita mentioned this issue Mar 29, 2024

Moves intern and label modules into bevy_ecs #12772

Merged

BenjaminBrienen mentioned this issue Oct 29, 2024

Prefer Display over Debug #16112

Merged

BenjaminBrienen mentioned this issue Nov 10, 2024

Remove bevy_utils::default() and prefer <_>::default() #16325

Closed

mockersf mentioned this issue Nov 20, 2024

remove the detailed_trace macro #16450

Open

This was referenced Dec 18, 2024

Reorganise Platform Support Dependencies #16877

Open

Split up bevy_core #16892

Closed

mnmaita mentioned this issue Dec 24, 2024

Move futures.rs, ConditionalSend and BoxedFuture types to bevy_tasks #16951

Merged

This was referenced Jan 5, 2025

Remove everything except Instant from bevy_utils::time #17158

Merged

Remove tracing re-export from bevy_utils #17161

Merged

BenjaminBrienen added S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes labels Jan 9, 2025

mgi388 mentioned this issue Jan 9, 2025

Add compile-time dyn compatible checks for DynEq, DynHash #17254

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove or shrink bevy_utils #11478

Remove or shrink bevy_utils #11478

doonv commented Jan 22, 2024 •

edited

Loading

alice-i-cecile commented Jan 22, 2024

alice-i-cecile commented Jan 22, 2024

doonv commented Jan 22, 2024

hymm commented Jan 22, 2024

alice-i-cecile commented Jan 22, 2024

Kanabenki commented Feb 29, 2024 •

edited

Loading

mnmaita commented Apr 26, 2024

mnmaita commented May 23, 2024

bushrat011899 commented Dec 19, 2024 •

edited

Loading

BenjaminBrienen commented Dec 19, 2024

alice-i-cecile commented Dec 19, 2024

mnmaita commented Dec 19, 2024

cart commented Dec 19, 2024 •

edited

Loading

mgi388 commented Jan 8, 2025 •

edited

Loading

bushrat011899 commented Jan 9, 2025

mgi388 commented Jan 9, 2025

BenjaminBrienen commented Jan 9, 2025

mgi388 commented Jan 9, 2025 •

edited

Loading

mintlu8 commented Jan 9, 2025 •

edited

Loading

alice-i-cecile commented Jan 9, 2025

Remove or shrink bevy_utils #11478

Remove or shrink bevy_utils #11478

Comments

doonv commented Jan 22, 2024 • edited Loading

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

What solution would you like?

Long list of what to do with each item in bevy_utils

alice-i-cecile commented Jan 22, 2024

alice-i-cecile commented Jan 22, 2024

doonv commented Jan 22, 2024

hymm commented Jan 22, 2024

alice-i-cecile commented Jan 22, 2024

Kanabenki commented Feb 29, 2024 • edited Loading

mnmaita commented Apr 26, 2024

mnmaita commented May 23, 2024

bushrat011899 commented Dec 19, 2024 • edited Loading

default and assert_object_safe

async/Future

Platform Support

tracing/log

BenjaminBrienen commented Dec 19, 2024

alice-i-cecile commented Dec 19, 2024

mnmaita commented Dec 19, 2024

cart commented Dec 19, 2024 • edited Loading

mgi388 commented Jan 8, 2025 • edited Loading

bushrat011899 commented Jan 9, 2025

mgi388 commented Jan 9, 2025

BenjaminBrienen commented Jan 9, 2025

mgi388 commented Jan 9, 2025 • edited Loading

mintlu8 commented Jan 9, 2025 • edited Loading

alice-i-cecile commented Jan 9, 2025

doonv commented Jan 22, 2024 •

edited

Loading

Long list of what to do with each item in `bevy_utils`

Kanabenki commented Feb 29, 2024 •

edited

Loading

bushrat011899 commented Dec 19, 2024 •

edited

Loading

`default` and `assert_object_safe`

`async`/`Future`

`tracing`/`log`

cart commented Dec 19, 2024 •

edited

Loading

mgi388 commented Jan 8, 2025 •

edited

Loading

mgi388 commented Jan 9, 2025 •

edited

Loading

mintlu8 commented Jan 9, 2025 •

edited

Loading