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

Non-blocking load_untyped using a wrapper asset #10198

Merged
merged 5 commits into from
Oct 26, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions crates/bevy_asset/src/assets.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use crate::{Asset, AssetEvent, AssetHandleProvider, AssetId, AssetServer, Handle};
use crate as bevy_asset;
use crate::{Asset, AssetEvent, AssetHandleProvider, AssetId, AssetServer, Handle, UntypedHandle};
use bevy_ecs::{
prelude::EventWriter,
system::{Res, ResMut, Resource},
};
use bevy_reflect::{Reflect, Uuid};
use bevy_reflect::{Reflect, TypePath, Uuid};
use bevy_utils::HashMap;
use crossbeam_channel::{Receiver, Sender};
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -74,6 +75,15 @@ impl AssetIndexAllocator {
}
}

/// A "loaded asset" containing the untyped handle for an asset stored in a given [`AssetPath`].
///
/// [`AssetPath`]: crate::AssetPath
#[derive(Asset, TypePath)]
pub struct LoadedUntypedAsset {
#[dependency]
pub handle: UntypedHandle,
}

// PERF: do we actually need this to be an enum? Can we just use an "invalid" generation instead
#[derive(Default)]
enum Entry<A: Asset> {
Expand Down
1 change: 1 addition & 0 deletions crates/bevy_asset/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ impl Plugin for AssetPlugin {
}
app.insert_resource(embedded)
.init_asset::<LoadedFolder>()
.init_asset::<LoadedUntypedAsset>()
.init_asset::<()>()
.configure_sets(
UpdateAssets,
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_asset/src/processor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,7 @@ impl AssetProcessor {
source: &AssetSource,
asset_path: &AssetPath<'static>,
) -> Result<ProcessResult, ProcessError> {
// TODO: The extension check was removed now tht AssetPath is the input. is that ok?
// TODO: The extension check was removed now that AssetPath is the input. is that ok?
// TODO: check if already processing to protect against duplicate hot-reload events
debug!("Processing {:?}", asset_path);
let server = &self.server;
Expand Down
61 changes: 60 additions & 1 deletion crates/bevy_asset/src/server/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use crate::{
},
path::AssetPath,
Asset, AssetEvent, AssetHandleProvider, AssetId, Assets, DeserializeMetaError,
ErasedLoadedAsset, Handle, UntypedAssetId, UntypedHandle,
ErasedLoadedAsset, Handle, LoadedUntypedAsset, UntypedAssetId, UntypedHandle,
};
use bevy_ecs::prelude::*;
use bevy_log::{error, info, warn};
Expand Down Expand Up @@ -288,6 +288,65 @@ impl AssetServer {
self.load_internal(None, path, false, None).await
}

/// Load an asset without knowing it's type. The method returns a handle to a [`LoadedUntypedAsset`].
///
/// Once the [`LoadedUntypedAsset`] is loaded, an untyped handle for the requested path can be
/// retrieved from it.
///
/// ```
/// use bevy_asset::{Assets, Handle, LoadedUntypedAsset};
/// use bevy_ecs::system::{Res, Resource};
///
/// #[derive(Resource)]
/// struct LoadingUntypedHandle(Handle<LoadedUntypedAsset>);
///
/// fn resolve_loaded_untyped_handle(loading_handle: Res<LoadingUntypedHandle>, loaded_untyped_assets: Res<Assets<LoadedUntypedAsset>>) {
/// if let Some(loaded_untyped_asset) = loaded_untyped_assets.get(&loading_handle.0) {
/// let handle = loaded_untyped_asset.handle.clone();
/// // continue working with `handle` which points to the asset at the originally requested path
/// }
/// }
/// ```
///
/// This indirection enables a non blocking load of an untyped asset, since I/O is
/// required to figure out the asset type before a handle can be created.
#[must_use = "not using the returned strong handle may result in the unexpected release of the assets"]
pub fn load_untyped<'a>(&self, path: impl Into<AssetPath<'a>>) -> Handle<LoadedUntypedAsset> {
let path = path.into().into_owned();
let (handle, should_load) = self
.data
.infos
.write()
.get_or_create_path_handle::<LoadedUntypedAsset>(
path.clone().with_label("untyped"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't safely use labels here:

// this will internally be referenced as "a.gltf#untyped"
assets.load_untyped("a.gltf#Foo")

// this will internally be referenced as "a.gltf#untyped"
assets.load_untyped("a.gltf#Bar")

This will also be ambiguous with any asset labeled untyped in the asset, which is unlikely, but still possible and likely to occur eventually.

Instead, I think we should create a reasonably unique asset source name (generated from the real asset source). This will create a brand new namespace that will never step on actual assets (and can still use labels to differentiate).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just pushed a fix!

HandleLoadingMode::Request,
None,
);
if !should_load {
return handle;
}
let id = handle.id().untyped();

let server = self.clone();
IoTaskPool::get()
.spawn(async move {
match server.load_untyped_async(path).await {
Ok(handle) => server.send_asset_event(InternalAssetEvent::Loaded {
id,
loaded_asset: LoadedAsset::new_with_dependencies(
LoadedUntypedAsset { handle },
None,
)
.into(),
}),
Err(_) => server.send_asset_event(InternalAssetEvent::Failed { id }),
}
})
.detach();

handle
}

/// Performs an async asset load.
///
/// `input_handle` must only be [`Some`] if `should_load` was true when retrieving `input_handle`. This is an optimization to
Expand Down