From cb928522f36cf26ce84400a30415ebce0d1f0465 Mon Sep 17 00:00:00 2001 From: MalekiRe Date: Fri, 23 Feb 2024 18:22:40 -0800 Subject: [PATCH 1/7] asset hooks --- crates/bevy_asset/src/lib.rs | 6 +++ crates/bevy_asset/src/loader.rs | 20 +++---- crates/bevy_asset/src/server/mod.rs | 82 +++++++++++++++++++++++++++-- examples/3d/load_gltf.rs | 3 ++ 4 files changed, 96 insertions(+), 15 deletions(-) diff --git a/crates/bevy_asset/src/lib.rs b/crates/bevy_asset/src/lib.rs index 41d6ef834f0fc..1416bc27ec5ed 100644 --- a/crates/bevy_asset/src/lib.rs +++ b/crates/bevy_asset/src/lib.rs @@ -285,6 +285,7 @@ impl VisitAssetDependencies for Vec { pub trait AssetApp { /// Registers the given `loader` in the [`App`]'s [`AssetServer`]. fn register_asset_loader(&mut self, loader: L) -> &mut Self; + fn register_asset_hook(&mut self, hook: impl FnMut(&mut A) -> () + Send + Sync + 'static) -> &mut Self; /// Registers the given `processor` in the [`App`]'s [`AssetProcessor`]. fn register_asset_processor(&mut self, processor: P) -> &mut Self; /// Registers the given [`AssetSourceBuilder`] with the given `id`. @@ -326,6 +327,11 @@ impl AssetApp for App { self } + fn register_asset_hook(&mut self, hook: impl FnMut(&mut A) -> () + Send + Sync + 'static) -> &mut Self { + self.world.resource::().register_asset_hook(hook); + self + } + fn register_asset_processor(&mut self, processor: P) -> &mut Self { if let Some(asset_processor) = self.world.get_resource::() { asset_processor.register_processor(processor); diff --git a/crates/bevy_asset/src/loader.rs b/crates/bevy_asset/src/loader.rs index 627b286482a5d..209a958b9393d 100644 --- a/crates/bevy_asset/src/loader.rs +++ b/crates/bevy_asset/src/loader.rs @@ -1,13 +1,7 @@ -use crate::{ - io::{AssetReaderError, MissingAssetSourceError, MissingProcessedAssetReaderError, Reader}, - meta::{ - loader_settings_meta_transform, AssetHash, AssetMeta, AssetMetaDyn, ProcessedInfoMinimal, - Settings, - }, - path::AssetPath, - Asset, AssetLoadError, AssetServer, AssetServerMode, Assets, Handle, LoadedUntypedAsset, - UntypedAssetId, UntypedHandle, -}; +use crate::{io::{AssetReaderError, MissingAssetSourceError, MissingProcessedAssetReaderError, Reader}, meta::{ + loader_settings_meta_transform, AssetHash, AssetMeta, AssetMetaDyn, ProcessedInfoMinimal, + Settings, +}, path::AssetPath, Asset, AssetLoadError, AssetServer, AssetServerMode, Assets, Handle, LoadedUntypedAsset, UntypedAssetId, UntypedHandle, AssetHooks}; use bevy_ecs::world::World; use bevy_utils::{BoxedFuture, CowArc, HashMap, HashSet}; use downcast_rs::{impl_downcast, Downcast}; @@ -234,6 +228,8 @@ impl ErasedLoadedAsset { pub trait AssetContainer: Downcast + Any + Send + Sync + 'static { fn insert(self: Box, id: UntypedAssetId, world: &mut World); fn asset_type_name(&self) -> &'static str; + + fn load_hook(&mut self, asset_hooks: &mut AssetHooks); } impl_downcast!(AssetContainer); @@ -246,6 +242,10 @@ impl AssetContainer for A { fn asset_type_name(&self) -> &'static str { std::any::type_name::() } + + fn load_hook(&mut self, asset_hooks: &mut AssetHooks) { + asset_hooks.trigger::(self); + } } /// An error that occurs when attempting to call [`LoadContext::load_direct`] diff --git a/crates/bevy_asset/src/server/mod.rs b/crates/bevy_asset/src/server/mod.rs index 5fda4c55e5f3d..3c81e389d140e 100644 --- a/crates/bevy_asset/src/server/mod.rs +++ b/crates/bevy_asset/src/server/mod.rs @@ -20,7 +20,7 @@ use crate::{ use bevy_ecs::prelude::*; use bevy_log::{error, info}; use bevy_tasks::IoTaskPool; -use bevy_utils::{CowArc, HashSet}; +use bevy_utils::{CowArc, hashbrown, HashSet}; use crossbeam_channel::{Receiver, Sender}; use futures_lite::StreamExt; use info::*; @@ -58,6 +58,62 @@ pub(crate) struct AssetServerData { sources: AssetSources, mode: AssetServerMode, meta_check: AssetMetaCheck, + pub(crate) hooks: RwLock, +} + +struct TypeMap(hashbrown::HashMap>); + +impl TypeMap { + pub fn set(&mut self, t: T) { + self.0.insert(TypeId::of::(), Box::new(t)); + } + pub fn has(&self) -> bool { + self.0.contains_key(&TypeId::of::()) + } + + pub fn get_mut(&mut self) -> Option<&mut T> { + self.0.get_mut(&TypeId::of::()).map(|t| { + t.downcast_mut::().unwrap() + }) + } +} + +// pub trait AssetHook = FnMut(&mut A) -> () + Send + 'static; + +type AssetHookVec = Vec () + Send + Sync + 'static>>; + +pub struct AssetHooks(TypeMap); + +impl Default for AssetHooks { + fn default() -> Self { + Self { + 0: TypeMap(hashbrown::HashMap::new()), + } + } +} + +impl AssetHooks { + fn add_asset_hook(&mut self, f: F) + where + A: Asset, + F: FnMut(&mut A) -> () + Send + Sync + 'static, + { + if !self.0.has::>() { + self.0.set::>(Vec::new()); + } + + let hooks = self.0.get_mut::>().unwrap(); + hooks.push(Box::new(f)); + } + pub(crate) fn trigger(&mut self, asset: &mut A) + where + A: Asset + { + let Some(hooks) = self.0.get_mut::>() else { return }; + for hook in hooks { + hook(asset); + } + } } /// The "asset mode" the server is currently in. @@ -118,6 +174,7 @@ impl AssetServer { asset_event_receiver, loaders, infos: RwLock::new(infos), + hooks: RwLock::new(AssetHooks::default()), }), } } @@ -140,6 +197,10 @@ impl AssetServer { self.data.loaders.write().push(loader); } + pub fn register_asset_hook(&self, hook: impl FnMut(&mut A) -> () + Send + Sync + 'static) { + self.data.hooks.write().add_asset_hook(hook); + } + /// Registers a new [`Asset`] type. [`Asset`] types must be registered before assets of that type can be loaded. pub fn register_asset(&self, assets: &Assets) { self.register_handle_provider(assets.get_handle_provider()); @@ -589,7 +650,10 @@ impl AssetServer { } pub(crate) fn load_asset(&self, asset: impl Into>) -> Handle { - let loaded_asset: LoadedAsset = asset.into(); + let mut loaded_asset: LoadedAsset = asset.into(); + + self.data.hooks.write().trigger(&mut loaded_asset.value); + let erased_loaded_asset: ErasedLoadedAsset = loaded_asset.into(); self.load_asset_untyped(None, erased_loaded_asset) .typed_debug_checked() @@ -601,7 +665,9 @@ impl AssetServer { path: Option>, asset: impl Into, ) -> UntypedHandle { - let loaded_asset = asset.into(); + let mut loaded_asset = asset.into(); + loaded_asset.value.load_hook(&mut self.data.hooks.write()); + let handle = if let Some(path) = path { let (handle, _) = self.data.infos.write().get_or_create_path_handle_untyped( path, @@ -1023,13 +1089,19 @@ impl AssetServer { let asset_path = asset_path.clone_owned(); let load_context = LoadContext::new(self, asset_path.clone(), load_dependencies, populate_hashes); - loader.load(reader, meta, load_context).await.map_err(|e| { + let mut asset = loader.load(reader, meta, load_context).await.map_err(|e| { AssetLoadError::AssetLoaderError { path: asset_path.clone_owned(), loader_name: loader.type_name(), error: e.into(), } - }) + }); + + if let Ok(asset) = asset.as_mut() { + asset.value.load_hook(&mut self.data.hooks.write()); + } + + asset } } diff --git a/examples/3d/load_gltf.rs b/examples/3d/load_gltf.rs index c26c8f4e4fc31..0a27d535127aa 100644 --- a/examples/3d/load_gltf.rs +++ b/examples/3d/load_gltf.rs @@ -12,6 +12,9 @@ fn main() { .add_plugins(DefaultPlugins) .add_systems(Startup, setup) .add_systems(Update, animate_light_direction) + .register_asset_hook::(|gltf: &mut bevy_internal::gltf::Gltf| { + info!("number of scenes in gltf: {}", gltf.scenes.len()); + }) .run(); } From 087938399c2f34a6d006e337038449699d168050 Mon Sep 17 00:00:00 2001 From: MalekiRe Date: Fri, 23 Feb 2024 18:26:09 -0800 Subject: [PATCH 2/7] formatting --- crates/bevy_asset/src/lib.rs | 14 +++++++++++--- crates/bevy_asset/src/loader.rs | 14 ++++++++++---- crates/bevy_asset/src/server/mod.rs | 23 ++++++++++++++--------- 3 files changed, 35 insertions(+), 16 deletions(-) diff --git a/crates/bevy_asset/src/lib.rs b/crates/bevy_asset/src/lib.rs index 1416bc27ec5ed..8e64bbf92cfcd 100644 --- a/crates/bevy_asset/src/lib.rs +++ b/crates/bevy_asset/src/lib.rs @@ -285,7 +285,10 @@ impl VisitAssetDependencies for Vec { pub trait AssetApp { /// Registers the given `loader` in the [`App`]'s [`AssetServer`]. fn register_asset_loader(&mut self, loader: L) -> &mut Self; - fn register_asset_hook(&mut self, hook: impl FnMut(&mut A) -> () + Send + Sync + 'static) -> &mut Self; + fn register_asset_hook( + &mut self, + hook: impl FnMut(&mut A) -> () + Send + Sync + 'static, + ) -> &mut Self; /// Registers the given `processor` in the [`App`]'s [`AssetProcessor`]. fn register_asset_processor(&mut self, processor: P) -> &mut Self; /// Registers the given [`AssetSourceBuilder`] with the given `id`. @@ -327,8 +330,13 @@ impl AssetApp for App { self } - fn register_asset_hook(&mut self, hook: impl FnMut(&mut A) -> () + Send + Sync + 'static) -> &mut Self { - self.world.resource::().register_asset_hook(hook); + fn register_asset_hook( + &mut self, + hook: impl FnMut(&mut A) -> () + Send + Sync + 'static, + ) -> &mut Self { + self.world + .resource::() + .register_asset_hook(hook); self } diff --git a/crates/bevy_asset/src/loader.rs b/crates/bevy_asset/src/loader.rs index 209a958b9393d..1415e33876740 100644 --- a/crates/bevy_asset/src/loader.rs +++ b/crates/bevy_asset/src/loader.rs @@ -1,7 +1,13 @@ -use crate::{io::{AssetReaderError, MissingAssetSourceError, MissingProcessedAssetReaderError, Reader}, meta::{ - loader_settings_meta_transform, AssetHash, AssetMeta, AssetMetaDyn, ProcessedInfoMinimal, - Settings, -}, path::AssetPath, Asset, AssetLoadError, AssetServer, AssetServerMode, Assets, Handle, LoadedUntypedAsset, UntypedAssetId, UntypedHandle, AssetHooks}; +use crate::{ + io::{AssetReaderError, MissingAssetSourceError, MissingProcessedAssetReaderError, Reader}, + meta::{ + loader_settings_meta_transform, AssetHash, AssetMeta, AssetMetaDyn, ProcessedInfoMinimal, + Settings, + }, + path::AssetPath, + Asset, AssetHooks, AssetLoadError, AssetServer, AssetServerMode, Assets, Handle, + LoadedUntypedAsset, UntypedAssetId, UntypedHandle, +}; use bevy_ecs::world::World; use bevy_utils::{BoxedFuture, CowArc, HashMap, HashSet}; use downcast_rs::{impl_downcast, Downcast}; diff --git a/crates/bevy_asset/src/server/mod.rs b/crates/bevy_asset/src/server/mod.rs index 3c81e389d140e..33ebd4fb4678f 100644 --- a/crates/bevy_asset/src/server/mod.rs +++ b/crates/bevy_asset/src/server/mod.rs @@ -20,7 +20,7 @@ use crate::{ use bevy_ecs::prelude::*; use bevy_log::{error, info}; use bevy_tasks::IoTaskPool; -use bevy_utils::{CowArc, hashbrown, HashSet}; +use bevy_utils::{hashbrown, CowArc, HashSet}; use crossbeam_channel::{Receiver, Sender}; use futures_lite::StreamExt; use info::*; @@ -72,9 +72,9 @@ impl TypeMap { } pub fn get_mut(&mut self) -> Option<&mut T> { - self.0.get_mut(&TypeId::of::()).map(|t| { - t.downcast_mut::().unwrap() - }) + self.0 + .get_mut(&TypeId::of::()) + .map(|t| t.downcast_mut::().unwrap()) } } @@ -95,8 +95,8 @@ impl Default for AssetHooks { impl AssetHooks { fn add_asset_hook(&mut self, f: F) where - A: Asset, - F: FnMut(&mut A) -> () + Send + Sync + 'static, + A: Asset, + F: FnMut(&mut A) -> () + Send + Sync + 'static, { if !self.0.has::>() { self.0.set::>(Vec::new()); @@ -107,9 +107,11 @@ impl AssetHooks { } pub(crate) fn trigger(&mut self, asset: &mut A) where - A: Asset + A: Asset, { - let Some(hooks) = self.0.get_mut::>() else { return }; + let Some(hooks) = self.0.get_mut::>() else { + return; + }; for hook in hooks { hook(asset); } @@ -197,7 +199,10 @@ impl AssetServer { self.data.loaders.write().push(loader); } - pub fn register_asset_hook(&self, hook: impl FnMut(&mut A) -> () + Send + Sync + 'static) { + pub fn register_asset_hook( + &self, + hook: impl FnMut(&mut A) -> () + Send + Sync + 'static, + ) { self.data.hooks.write().add_asset_hook(hook); } From c4d3510c01c670ab5a9ac0a5e9b431b5a35df77b Mon Sep 17 00:00:00 2001 From: MalekiRe Date: Fri, 23 Feb 2024 20:11:55 -0800 Subject: [PATCH 3/7] Changed it to use the &mut world and take in a system for far better ergonomics and total functionality. --- crates/bevy_asset/src/lib.rs | 9 ++--- crates/bevy_asset/src/loader.rs | 6 ++-- crates/bevy_asset/src/server/mod.rs | 53 +++++++++++++++++++++-------- examples/3d/load_gltf.rs | 12 +++++-- 4 files changed, 55 insertions(+), 25 deletions(-) diff --git a/crates/bevy_asset/src/lib.rs b/crates/bevy_asset/src/lib.rs index 8e64bbf92cfcd..86a5c669b1124 100644 --- a/crates/bevy_asset/src/lib.rs +++ b/crates/bevy_asset/src/lib.rs @@ -47,6 +47,7 @@ use crate::{ processor::{AssetProcessor, Process}, }; use bevy_app::{App, First, MainScheduleOrder, Plugin, PostUpdate}; +use bevy_ecs::prelude::IntoSystem; use bevy_ecs::{ reflect::AppTypeRegistry, schedule::{IntoSystemConfigs, IntoSystemSetConfigs, ScheduleLabel, SystemSet}, @@ -285,9 +286,9 @@ impl VisitAssetDependencies for Vec { pub trait AssetApp { /// Registers the given `loader` in the [`App`]'s [`AssetServer`]. fn register_asset_loader(&mut self, loader: L) -> &mut Self; - fn register_asset_hook( + fn register_asset_hook( &mut self, - hook: impl FnMut(&mut A) -> () + Send + Sync + 'static, + hook: impl IntoSystem<&'static mut A, Out, Marker> + Sync + Send + 'static + Clone, ) -> &mut Self; /// Registers the given `processor` in the [`App`]'s [`AssetProcessor`]. fn register_asset_processor(&mut self, processor: P) -> &mut Self; @@ -330,9 +331,9 @@ impl AssetApp for App { self } - fn register_asset_hook( + fn register_asset_hook( &mut self, - hook: impl FnMut(&mut A) -> () + Send + Sync + 'static, + hook: impl IntoSystem<&'static mut A, Out, Marker> + Sync + Send + 'static + Clone, ) -> &mut Self { self.world .resource::() diff --git a/crates/bevy_asset/src/loader.rs b/crates/bevy_asset/src/loader.rs index 1415e33876740..8caba5df89101 100644 --- a/crates/bevy_asset/src/loader.rs +++ b/crates/bevy_asset/src/loader.rs @@ -235,7 +235,7 @@ pub trait AssetContainer: Downcast + Any + Send + Sync + 'static { fn insert(self: Box, id: UntypedAssetId, world: &mut World); fn asset_type_name(&self) -> &'static str; - fn load_hook(&mut self, asset_hooks: &mut AssetHooks); + fn load_hook(&mut self, asset_hooks: &mut AssetHooks, world: &mut World); } impl_downcast!(AssetContainer); @@ -249,8 +249,8 @@ impl AssetContainer for A { std::any::type_name::() } - fn load_hook(&mut self, asset_hooks: &mut AssetHooks) { - asset_hooks.trigger::(self); + fn load_hook(&mut self, asset_hooks: &mut AssetHooks, world: &mut World) { + asset_hooks.trigger::(self, world); } } diff --git a/crates/bevy_asset/src/server/mod.rs b/crates/bevy_asset/src/server/mod.rs index 33ebd4fb4678f..f4a47eb8959b2 100644 --- a/crates/bevy_asset/src/server/mod.rs +++ b/crates/bevy_asset/src/server/mod.rs @@ -80,7 +80,10 @@ impl TypeMap { // pub trait AssetHook = FnMut(&mut A) -> () + Send + 'static; -type AssetHookVec = Vec () + Send + Sync + 'static>>; +type AssetHookVec = + Vec () + Send + Sync + 'static>>; + +// type AssetHookVec = Vec () + Send + S>> pub struct AssetHooks(TypeMap); @@ -93,19 +96,37 @@ impl Default for AssetHooks { } impl AssetHooks { - fn add_asset_hook(&mut self, f: F) + fn add_asset_hook(&mut self, f: F) where A: Asset, - F: FnMut(&mut A) -> () + Send + Sync + 'static, + F: IntoSystem<&'static mut A, Out, Marker> + Sync + Send + 'static + Clone, + //F: FnMut(&mut A, &mut bevy_ecs::world::World) -> () + Send + Sync + 'static, { if !self.0.has::>() { self.0.set::>(Vec::new()); } let hooks = self.0.get_mut::>().unwrap(); - hooks.push(Box::new(f)); + hooks.push(Box::new(move |asset, world| { + // This uses unsafe code to get a raw pointer in order to fuffill a static + // Lifetime requirement. This is actually okay in this particular case. + // This is because we re-create the system everytime this is called + // Because of this, it means the `In` system param can never be + // Persisted past this closure. + // Combine this with the fact that the In system parameter does not + // Persist in the world struct, it means there is no place that + // The `&'static mut A` could be held past the call of this closure. + // Therefore, it's lifetime being 'static does not break the ailsiing rules + // Because there will never be two references to it. + let mut system: F::System = IntoSystem::into_system(f.clone()); + system.initialize(world); + unsafe { + system.run((asset as *mut A).as_mut().unwrap(), world); + } + system.apply_deferred(world); + })); } - pub(crate) fn trigger(&mut self, asset: &mut A) + pub(crate) fn trigger(&mut self, asset: &mut A, world: &mut World) where A: Asset, { @@ -113,7 +134,7 @@ impl AssetHooks { return; }; for hook in hooks { - hook(asset); + hook(asset, world); } } } @@ -199,9 +220,9 @@ impl AssetServer { self.data.loaders.write().push(loader); } - pub fn register_asset_hook( + pub fn register_asset_hook( &self, - hook: impl FnMut(&mut A) -> () + Send + Sync + 'static, + hook: impl IntoSystem<&'static mut A, Out, Marker> + Sync + Send + 'static + Clone, ) { self.data.hooks.write().add_asset_hook(hook); } @@ -657,7 +678,7 @@ impl AssetServer { pub(crate) fn load_asset(&self, asset: impl Into>) -> Handle { let mut loaded_asset: LoadedAsset = asset.into(); - self.data.hooks.write().trigger(&mut loaded_asset.value); + //self.data.hooks.write().trigger(&mut loaded_asset.value); let erased_loaded_asset: ErasedLoadedAsset = loaded_asset.into(); self.load_asset_untyped(None, erased_loaded_asset) @@ -671,7 +692,7 @@ impl AssetServer { asset: impl Into, ) -> UntypedHandle { let mut loaded_asset = asset.into(); - loaded_asset.value.load_hook(&mut self.data.hooks.write()); + //loaded_asset.value.load_hook(&mut self.data.hooks.write()); let handle = if let Some(path) = path { let (handle, _) = self.data.infos.write().get_or_create_path_handle_untyped( @@ -1102,10 +1123,6 @@ impl AssetServer { } }); - if let Ok(asset) = asset.as_mut() { - asset.value.load_hook(&mut self.data.hooks.write()); - } - asset } } @@ -1117,7 +1134,13 @@ pub fn handle_internal_asset_events(world: &mut World) { let mut untyped_failures = vec![]; for event in server.data.asset_event_receiver.try_iter() { match event { - InternalAssetEvent::Loaded { id, loaded_asset } => { + InternalAssetEvent::Loaded { + id, + mut loaded_asset, + } => { + loaded_asset + .value + .load_hook(&mut server.data.hooks.write(), world); infos.process_asset_load( id, loaded_asset, diff --git a/examples/3d/load_gltf.rs b/examples/3d/load_gltf.rs index 0a27d535127aa..d08f759c2159b 100644 --- a/examples/3d/load_gltf.rs +++ b/examples/3d/load_gltf.rs @@ -4,6 +4,8 @@ use bevy::{ pbr::{CascadeShadowConfigBuilder, DirectionalLightShadowMap}, prelude::*, }; +use bevy_internal::ecs::system::RunSystemOnce; +use bevy_internal::gltf::GltfMesh; use std::f32::consts::*; fn main() { @@ -12,9 +14,13 @@ fn main() { .add_plugins(DefaultPlugins) .add_systems(Startup, setup) .add_systems(Update, animate_light_direction) - .register_asset_hook::(|gltf: &mut bevy_internal::gltf::Gltf| { - info!("number of scenes in gltf: {}", gltf.scenes.len()); - }) + .register_asset_hook::( + |In(gltf): In<&mut bevy_internal::gltf::Gltf>, meshes: Res>| { + for (name, mesh) in gltf.named_meshes.iter() { + println!("name: {}, mesh: {:#?}", name, meshes.get(mesh).unwrap()) + } + }, + ) .run(); } From 535ec1121f39e135a2b718927851a872d7a9bfef Mon Sep 17 00:00:00 2001 From: MalekiRe Date: Fri, 23 Feb 2024 20:13:15 -0800 Subject: [PATCH 4/7] Formatting --- crates/bevy_asset/src/server/mod.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/crates/bevy_asset/src/server/mod.rs b/crates/bevy_asset/src/server/mod.rs index f4a47eb8959b2..0544cc2a51f4e 100644 --- a/crates/bevy_asset/src/server/mod.rs +++ b/crates/bevy_asset/src/server/mod.rs @@ -77,14 +77,9 @@ impl TypeMap { .map(|t| t.downcast_mut::().unwrap()) } } - -// pub trait AssetHook = FnMut(&mut A) -> () + Send + 'static; - type AssetHookVec = Vec () + Send + Sync + 'static>>; -// type AssetHookVec = Vec () + Send + S>> - pub struct AssetHooks(TypeMap); impl Default for AssetHooks { From 55be783d9cf37f5141b9e43aa6e8b6061af65fb7 Mon Sep 17 00:00:00 2001 From: MalekiRe Date: Fri, 23 Feb 2024 22:09:59 -0800 Subject: [PATCH 5/7] Fixed nasal demons --- crates/bevy_asset/src/lib.rs | 4 +- crates/bevy_asset/src/server/mod.rs | 71 +++++++++++++++++++---------- examples/3d/load_gltf.rs | 5 +- 3 files changed, 51 insertions(+), 29 deletions(-) diff --git a/crates/bevy_asset/src/lib.rs b/crates/bevy_asset/src/lib.rs index 86a5c669b1124..da5fc7b79994e 100644 --- a/crates/bevy_asset/src/lib.rs +++ b/crates/bevy_asset/src/lib.rs @@ -288,7 +288,7 @@ pub trait AssetApp { fn register_asset_loader(&mut self, loader: L) -> &mut Self; fn register_asset_hook( &mut self, - hook: impl IntoSystem<&'static mut A, Out, Marker> + Sync + Send + 'static + Clone, + hook: impl IntoSystem, Out, Marker> + Sync + Send + 'static + Clone, ) -> &mut Self; /// Registers the given `processor` in the [`App`]'s [`AssetProcessor`]. fn register_asset_processor(&mut self, processor: P) -> &mut Self; @@ -333,7 +333,7 @@ impl AssetApp for App { fn register_asset_hook( &mut self, - hook: impl IntoSystem<&'static mut A, Out, Marker> + Sync + Send + 'static + Clone, + hook: impl IntoSystem, Out, Marker> + Sync + Send + 'static + Clone, ) -> &mut Self { self.world .resource::() diff --git a/crates/bevy_asset/src/server/mod.rs b/crates/bevy_asset/src/server/mod.rs index 0544cc2a51f4e..d5241c6e4966b 100644 --- a/crates/bevy_asset/src/server/mod.rs +++ b/crates/bevy_asset/src/server/mod.rs @@ -26,7 +26,9 @@ use futures_lite::StreamExt; use info::*; use loaders::*; use parking_lot::RwLock; +use std::ops::{Deref, DerefMut}; use std::path::PathBuf; +use std::sync::atomic::{AtomicBool, Ordering}; use std::{any::TypeId, path::Path, sync::Arc}; use thiserror::Error; @@ -90,12 +92,44 @@ impl Default for AssetHooks { } } +impl AssetWrapper { + fn new(asset: &mut A) -> Self { + Self { + ptr: asset as *mut A, + flag: Default::default(), + } + } +} +/// Safe wrapper around asset pointers to allow for passing into systems. +pub struct AssetWrapper { + ptr: *mut A, + flag: Arc, +} + +impl Deref for AssetWrapper { + type Target = A; + fn deref(&self) -> &A { + unsafe { &*self.ptr } + } +} + +impl DerefMut for AssetWrapper { + fn deref_mut(&mut self) -> &mut A { + unsafe { &mut *self.ptr } + } +} + +impl Drop for AssetWrapper { + fn drop(&mut self) { + self.flag.store(true, Ordering::Release); + } +} + impl AssetHooks { fn add_asset_hook(&mut self, f: F) where A: Asset, - F: IntoSystem<&'static mut A, Out, Marker> + Sync + Send + 'static + Clone, - //F: FnMut(&mut A, &mut bevy_ecs::world::World) -> () + Send + Sync + 'static, + F: IntoSystem, Out, Marker> + Sync + Send + 'static + Clone, { if !self.0.has::>() { self.0.set::>(Vec::new()); @@ -103,20 +137,13 @@ impl AssetHooks { let hooks = self.0.get_mut::>().unwrap(); hooks.push(Box::new(move |asset, world| { - // This uses unsafe code to get a raw pointer in order to fuffill a static - // Lifetime requirement. This is actually okay in this particular case. - // This is because we re-create the system everytime this is called - // Because of this, it means the `In` system param can never be - // Persisted past this closure. - // Combine this with the fact that the In system parameter does not - // Persist in the world struct, it means there is no place that - // The `&'static mut A` could be held past the call of this closure. - // Therefore, it's lifetime being 'static does not break the ailsiing rules - // Because there will never be two references to it. let mut system: F::System = IntoSystem::into_system(f.clone()); system.initialize(world); - unsafe { - system.run((asset as *mut A).as_mut().unwrap(), world); + let asset_wrapper = AssetWrapper::new(asset); + let flag = asset_wrapper.flag.clone(); + system.run(asset_wrapper, world); + if !flag.load(Ordering::Acquire) { + panic!("tried to hold asset pointer past lifetime"); } system.apply_deferred(world); })); @@ -217,7 +244,7 @@ impl AssetServer { pub fn register_asset_hook( &self, - hook: impl IntoSystem<&'static mut A, Out, Marker> + Sync + Send + 'static + Clone, + hook: impl IntoSystem, Out, Marker> + Sync + Send + 'static + Clone, ) { self.data.hooks.write().add_asset_hook(hook); } @@ -671,9 +698,7 @@ impl AssetServer { } pub(crate) fn load_asset(&self, asset: impl Into>) -> Handle { - let mut loaded_asset: LoadedAsset = asset.into(); - - //self.data.hooks.write().trigger(&mut loaded_asset.value); + let loaded_asset: LoadedAsset = asset.into(); let erased_loaded_asset: ErasedLoadedAsset = loaded_asset.into(); self.load_asset_untyped(None, erased_loaded_asset) @@ -686,9 +711,7 @@ impl AssetServer { path: Option>, asset: impl Into, ) -> UntypedHandle { - let mut loaded_asset = asset.into(); - //loaded_asset.value.load_hook(&mut self.data.hooks.write()); - + let loaded_asset = asset.into(); let handle = if let Some(path) = path { let (handle, _) = self.data.infos.write().get_or_create_path_handle_untyped( path, @@ -1110,15 +1133,13 @@ impl AssetServer { let asset_path = asset_path.clone_owned(); let load_context = LoadContext::new(self, asset_path.clone(), load_dependencies, populate_hashes); - let mut asset = loader.load(reader, meta, load_context).await.map_err(|e| { + loader.load(reader, meta, load_context).await.map_err(|e| { AssetLoadError::AssetLoaderError { path: asset_path.clone_owned(), loader_name: loader.type_name(), error: e.into(), } - }); - - asset + }) } } diff --git a/examples/3d/load_gltf.rs b/examples/3d/load_gltf.rs index d08f759c2159b..6e195d3b68edc 100644 --- a/examples/3d/load_gltf.rs +++ b/examples/3d/load_gltf.rs @@ -4,7 +4,7 @@ use bevy::{ pbr::{CascadeShadowConfigBuilder, DirectionalLightShadowMap}, prelude::*, }; -use bevy_internal::ecs::system::RunSystemOnce; +use bevy_internal::asset::AssetWrapper; use bevy_internal::gltf::GltfMesh; use std::f32::consts::*; @@ -15,7 +15,8 @@ fn main() { .add_systems(Startup, setup) .add_systems(Update, animate_light_direction) .register_asset_hook::( - |In(gltf): In<&mut bevy_internal::gltf::Gltf>, meshes: Res>| { + |In(gltf): In>, + meshes: Res>| { for (name, mesh) in gltf.named_meshes.iter() { println!("name: {}, mesh: {:#?}", name, meshes.get(mesh).unwrap()) } From 91eaaa0002065281ad9627f43bd7a551dfc720d2 Mon Sep 17 00:00:00 2001 From: MalekiRe Date: Fri, 23 Feb 2024 22:18:38 -0800 Subject: [PATCH 6/7] Clippy fixes and general fixes --- crates/bevy_asset/src/server/mod.rs | 9 ++++++--- examples/3d/load_gltf.rs | 12 ++++++------ 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/crates/bevy_asset/src/server/mod.rs b/crates/bevy_asset/src/server/mod.rs index d5241c6e4966b..77079879dd963 100644 --- a/crates/bevy_asset/src/server/mod.rs +++ b/crates/bevy_asset/src/server/mod.rs @@ -79,6 +79,7 @@ impl TypeMap { .map(|t| t.downcast_mut::().unwrap()) } } +#[allow(clippy::unused_unit)] type AssetHookVec = Vec () + Send + Sync + 'static>>; @@ -86,9 +87,7 @@ pub struct AssetHooks(TypeMap); impl Default for AssetHooks { fn default() -> Self { - Self { - 0: TypeMap(hashbrown::HashMap::new()), - } + Self(TypeMap(hashbrown::HashMap::new())) } } @@ -109,12 +108,16 @@ pub struct AssetWrapper { impl Deref for AssetWrapper { type Target = A; fn deref(&self) -> &A { + // SAFETY: This is safe because we ensure that we flag right after the system that gets the asset + // that it has dropped the AssetWrapper. If it hasn't we panic. unsafe { &*self.ptr } } } impl DerefMut for AssetWrapper { fn deref_mut(&mut self) -> &mut A { + // SAFETY: This is safe because we ensure that we flag right after the system that gets the asset + // that it has dropped the AssetWrapper. If it hasn't we panic. unsafe { &mut *self.ptr } } } diff --git a/examples/3d/load_gltf.rs b/examples/3d/load_gltf.rs index 6e195d3b68edc..e71870470b73d 100644 --- a/examples/3d/load_gltf.rs +++ b/examples/3d/load_gltf.rs @@ -1,11 +1,12 @@ //! Loads and renders a glTF file as a scene. use bevy::{ + asset::AssetWrapper, + gltf::Gltf, + gltf::GltfMesh, pbr::{CascadeShadowConfigBuilder, DirectionalLightShadowMap}, prelude::*, }; -use bevy_internal::asset::AssetWrapper; -use bevy_internal::gltf::GltfMesh; use std::f32::consts::*; fn main() { @@ -14,11 +15,10 @@ fn main() { .add_plugins(DefaultPlugins) .add_systems(Startup, setup) .add_systems(Update, animate_light_direction) - .register_asset_hook::( - |In(gltf): In>, - meshes: Res>| { + .register_asset_hook::( + |In(gltf): In>, meshes: Res>| { for (name, mesh) in gltf.named_meshes.iter() { - println!("name: {}, mesh: {:#?}", name, meshes.get(mesh).unwrap()) + println!("name: {}, mesh: {:#?}", name, meshes.get(mesh).unwrap()); } }, ) From 462f1fb3536b7806dcd9ce5f1a8231e4c6dcde34 Mon Sep 17 00:00:00 2001 From: MalekiRe Date: Fri, 23 Feb 2024 22:23:15 -0800 Subject: [PATCH 7/7] Added doc comments --- crates/bevy_asset/src/lib.rs | 3 +++ crates/bevy_asset/src/loader.rs | 1 + 2 files changed, 4 insertions(+) diff --git a/crates/bevy_asset/src/lib.rs b/crates/bevy_asset/src/lib.rs index da5fc7b79994e..9f90c41840eaf 100644 --- a/crates/bevy_asset/src/lib.rs +++ b/crates/bevy_asset/src/lib.rs @@ -286,6 +286,9 @@ impl VisitAssetDependencies for Vec { pub trait AssetApp { /// Registers the given `loader` in the [`App`]'s [`AssetServer`]. fn register_asset_loader(&mut self, loader: L) -> &mut Self; + /// Registers a hook that gives a &mut Asset to the user, along with any arbitrary system + /// parameters. This runs before the asset is loaded into the world. You can register multiple + /// hooks. fn register_asset_hook( &mut self, hook: impl IntoSystem, Out, Marker> + Sync + Send + 'static + Clone, diff --git a/crates/bevy_asset/src/loader.rs b/crates/bevy_asset/src/loader.rs index 8caba5df89101..589390d2ef1b7 100644 --- a/crates/bevy_asset/src/loader.rs +++ b/crates/bevy_asset/src/loader.rs @@ -235,6 +235,7 @@ pub trait AssetContainer: Downcast + Any + Send + Sync + 'static { fn insert(self: Box, id: UntypedAssetId, world: &mut World); fn asset_type_name(&self) -> &'static str; + /// Exists such that we can determine the type statically for the loading hook from an asset that has an erased type. fn load_hook(&mut self, asset_hooks: &mut AssetHooks, world: &mut World); }