# Objective
The `NestedLoader` API as it stands right now is somewhat lacking:
- It consists of several types `NestedLoader`, `UntypedNestedLoader`,
`DirectNestedLoader`, and `UntypedDirectNestedLoader`, where a typestate
pattern on `NestedLoader` would be make it more obvious what it does,
and allow centralising the documentation
- The term "untyped" in the asset loader code is overloaded. It can mean
either:
- we have literally no idea what the type of this asset will be when we
load it (I dub this "unknown type")
- we know what type of asset it will be, but we don't know it statically
- we only have a TypeId (I dub this "dynamic type" / "erased")
- There is no way to get an `UntypedHandle` (erased) given a `TypeId`
## Solution
Changes `NestedLoader` into a type-state pattern, adding two type
params:
- `T` determines the typing
- `StaticTyped`, the default, where you pass in `A` statically into `fn
load<A>() -> ..`
- `DynamicTyped`, where you give a `TypeId`, giving you a
`UntypedHandle`
- `UnknownTyped`, where you have literally no idea what type of asset
you're loading, giving you a `Handle<LoadedUntypedAsset>`
- `M` determines the "mode" (bikeshedding TBD, I couldn't come up with a
better name)
- `Deferred`, the default, won't load the asset when you call `load`,
but it does give you a `Handle` to it (this is nice since it can be a
sync fn)
- `Immediate` will load the asset as soon as you call it, and give you
access to it, but you must be in an async context to call it
Changes some naming of internals in `AssetServer` to fit the new
definitions of "dynamic type" and "unknown type". Note that I didn't do
a full pass over this code to keep the diff small. That can probably be
done in a new PR - I think the definiton I laid out of unknown type vs.
erased makes it pretty clear where each one applies.
<details>
<summary>Old issue</summary>
The only real problem I have with this PR is the requirement to pass in
`type_name` (from `core::any::type_name`) into Erased. Users might not
have that type name, only the ID, and it just seems sort of weird to
*have* to give an asset type name. However, the reason we need it is
because of this:
```rs
pub(crate) fn get_or_create_path_handle_erased(
&mut self,
path: AssetPath<'static>,
type_id: TypeId,
type_name: &str,
loading_mode: HandleLoadingMode,
meta_transform: Option<MetaTransform>,
) -> (UntypedHandle, bool) {
let result = self.get_or_create_path_handle_internal(
path,
Some(type_id),
loading_mode,
meta_transform,
);
// it is ok to unwrap because TypeId was specified above
unwrap_with_context(result, type_name).unwrap()
}
pub(crate) fn unwrap_with_context<T>(
result: Result<T, GetOrCreateHandleInternalError>,
type_name: &str,
) -> Option<T> {
match result {
Ok(value) => Some(value),
Err(GetOrCreateHandleInternalError::HandleMissingButTypeIdNotSpecified) => None,
Err(GetOrCreateHandleInternalError::MissingHandleProviderError(_)) => {
panic!("Cannot allocate an Asset Handle of type '{type_name}' because the asset type has not been initialized. \
Make sure you have called app.init_asset::<{type_name}>()")
}
}
}
```
This `unwrap_with_context` is literally the only reason we need the
`type_name`. Potentially, this can be turned into an `impl
Into<Option<&str>>`, and output a different error message if the type
name is missing. Since if we are loading an asset where we only know the
type ID, by definition we can't output that error message, since we
don't have the type name. I'm open to suggestions on this.
</details>
## Testing
Not sure how to test this, since I kept most of the actual NestedLoader
logic the same. The only new API is loading an `UntypedHandle` when in
the `DynamicTyped, Immediate` state.
## Migration Guide
Code which uses `bevy_asset`'s `LoadContext::loader` / `NestedLoader`
will see some naming changes:
- `untyped` is replaced by `with_unknown_type`
- `with_asset_type` is replaced by `with_static_type`
- `with_asset_type_id` is replaced by `with_dynamic_type`
- `direct` is replaced by `immediate` (the opposite of "immediate" is
"deferred")
# Objective
- Fixes#6370
- Closes#6581
## Solution
- Added the following lints to the workspace:
- `std_instead_of_core`
- `std_instead_of_alloc`
- `alloc_instead_of_core`
- Used `cargo +nightly fmt` with [item level use
formatting](https://rust-lang.github.io/rustfmt/?version=v1.6.0&search=#Item%5C%3A)
to split all `use` statements into single items.
- Used `cargo clippy --workspace --all-targets --all-features --fix
--allow-dirty` to _attempt_ to resolve the new linting issues, and
intervened where the lint was unable to resolve the issue automatically
(usually due to needing an `extern crate alloc;` statement in a crate
root).
- Manually removed certain uses of `std` where negative feature gating
prevented `--all-features` from finding the offending uses.
- Used `cargo +nightly fmt` with [crate level use
formatting](https://rust-lang.github.io/rustfmt/?version=v1.6.0&search=#Crate%5C%3A)
to re-merge all `use` statements matching Bevy's previous styling.
- Manually fixed cases where the `fmt` tool could not re-merge `use`
statements due to conditional compilation attributes.
## Testing
- Ran CI locally
## Migration Guide
The MSRV is now 1.81. Please update to this version or higher.
## Notes
- This is a _massive_ change to try and push through, which is why I've
outlined the semi-automatic steps I used to create this PR, in case this
fails and someone else tries again in the future.
- Making this change has no impact on user code, but does mean Bevy
contributors will be warned to use `core` and `alloc` instead of `std`
where possible.
- This lint is a critical first step towards investigating `no_std`
options for Bevy.
---------
Co-authored-by: François Mockers <francois.mockers@vleue.com>
# Objective
- Bevy currently has lot of invalid intra-doc links, let's fix them!
- Also make CI test them, to avoid future regressions.
- Helps with #1983 (but doesn't fix it, as there could still be explicit
links to docs.rs that are broken)
## Solution
- Make `cargo r -p ci -- doc-check` check fail on warnings (could also
be changed to just some specific lints)
- Manually fix all the warnings (note that in some cases it was unclear
to me what the fix should have been, I'll try to highlight them in a
self-review)
# Objective
The `AssetReader` trait allows customizing the behavior of fetching
bytes for an `AssetPath`, and expects implementors to return `dyn
AsyncRead + AsyncSeek`. This gives implementors of `AssetLoader` great
flexibility to tightly integrate their asset loading behavior with the
asynchronous task system.
However, almost all implementors of `AssetLoader` don't use the async
functionality at all, and just call `AsyncReadExt::read_to_end(&mut
Vec<u8>)`. This is incredibly inefficient, as this method repeatedly
calls `poll_read` on the trait object, filling the vector 32 bytes at a
time. At my work we have assets that are hundreds of megabytes which
makes this a meaningful overhead.
## Solution
Turn the `Reader` type alias into an actual trait, with a provided
method `read_to_end`. This provided method should be more efficient than
the existing extension method, as the compiler will know the underlying
type of `Reader` when generating this function, which removes the
repeated dynamic dispatches and allows the compiler to make further
optimizations after inlining. Individual implementors are able to
override the provided implementation -- for simple asset readers that
just copy bytes from one buffer to another, this allows removing a large
amount of overhead from the provided implementation.
Now that `Reader` is an actual trait, I also improved the ergonomics for
implementing `AssetReader`. Currently, implementors are expected to box
their reader and return it as a trait object, which adds unnecessary
boilerplate to implementations. This PR changes that trait method to
return a pseudo trait alias, which allows implementors to return `impl
Reader` instead of `Box<dyn Reader>`. Now, the boilerplate for boxing
occurs in `ErasedAssetReader`.
## Testing
I made identical changes to my company's fork of bevy. Our app, which
makes heavy use of `read_to_end` for asset loading, still worked
properly after this. I am not aware if we have a more systematic way of
testing asset loading for correctness.
---
## Migration Guide
The trait method `bevy_asset::io::AssetReader::read` (and `read_meta`)
now return an opaque type instead of a boxed trait object. Implementors
of these methods should change the type signatures appropriately
```rust
impl AssetReader for MyReader {
// Before
async fn read<'a>(&'a self, path: &'a Path) -> Result<Box<Reader<'a>>, AssetReaderError> {
let reader = // construct a reader
Box::new(reader) as Box<Reader<'a>>
}
// After
async fn read<'a>(&'a self, path: &'a Path) -> Result<impl Reader + 'a, AssetReaderError> {
// create a reader
}
}
```
`bevy::asset::io::Reader` is now a trait, rather than a type alias for a
trait object. Implementors of `AssetLoader::load` will need to adjust
the method signature accordingly
```rust
impl AssetLoader for MyLoader {
async fn load<'a>(
&'a self,
// Before:
reader: &'a mut bevy::asset::io::Reader,
// After:
reader: &'a mut dyn bevy::asset::io::Reader,
_: &'a Self::Settings,
load_context: &'a mut LoadContext<'_>,
) -> Result<Self::Asset, Self::Error> {
}
```
Additionally, implementors of `AssetReader` that return a type
implementing `futures_io::AsyncRead` and `AsyncSeek` might need to
explicitly implement `bevy::asset::io::Reader` for that type.
```rust
impl bevy::asset::io::Reader for MyAsyncReadAndSeek {}
```
# Objective
Supercedes #12881 . Added a simple implementation that allows the user
to react to multiple asset loads both synchronously and asynchronously.
## Solution
Added `load_acquire`, that holds an item and drops it when loading is
finished or failed.
When used synchronously
Hold an `Arc<()>`, check for `Arc::strong_count() == 1` when all loading
completed.
When used asynchronously
Hold a `SemaphoreGuard`, await on `acquire_all` for completion.
This implementation has more freedom than the original in my opinion.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Zachary Harrold <zac@harrold.com.au>
