From 10443ce7b376c0ccf643f226aec8c6ce59549167 Mon Sep 17 00:00:00 2001 From: Nicola Papale Date: Fri, 15 Sep 2023 10:06:13 +0200 Subject: [PATCH] Start asset_v2 migration BROKEN --- crates/bevy_asset/src/asset_changed.rs | 143 +++++++++++++++---------- crates/bevy_asset/src/asset_changes.rs | 23 ---- crates/bevy_asset/src/assets.rs | 25 ++++- crates/bevy_asset/src/lib.rs | 4 + 4 files changed, 110 insertions(+), 85 deletions(-) delete mode 100644 crates/bevy_asset/src/asset_changes.rs diff --git a/crates/bevy_asset/src/asset_changed.rs b/crates/bevy_asset/src/asset_changed.rs index bf85361d983256..6994593214af0c 100644 --- a/crates/bevy_asset/src/asset_changed.rs +++ b/crates/bevy_asset/src/asset_changed.rs @@ -4,23 +4,47 @@ use std::marker::PhantomData; use bevy_ecs::{ archetype::{Archetype, ArchetypeComponentId}, component::{ComponentId, Tick}, - prelude::{Changed, Entity, Or, World}, + prelude::{Changed, Entity, Or, Resource, World}, query::{Access, FilteredAccess, QueryItem, ReadFetch, ReadOnlyWorldQuery, WorldQuery}, storage::{Table, TableRow}, world::unsafe_world_cell::UnsafeWorldCell, }; use bevy_utils::HashMap; -use crate::{asset_changes::AssetChanges, Asset, Handle, HandleId}; +use crate::{Asset, AssetId, Handle}; -#[derive(Clone, Copy)] -struct AssetChangeCheck<'w> { - handle_change_map: &'w HashMap, +#[doc(hidden)] +#[derive(Resource)] +pub struct AssetChanges { + pub(crate) change_ticks: HashMap, Tick>, + pub(crate) last_change_tick: Tick, +} +impl Default for AssetChanges { + fn default() -> Self { + Self { + change_ticks: Default::default(), + last_change_tick: Tick::new(0), + } + } +} + +struct AssetChangeCheck<'w, A: Asset> { + handle_change_map: &'w HashMap, Tick>, last_run: Tick, this_run: Tick, } -impl<'w> AssetChangeCheck<'w> { - fn new(changes: &'w AssetChanges, last_run: Tick, this_run: Tick) -> Self { +impl Clone for AssetChangeCheck<'_, A> { + fn clone(&self) -> Self { + AssetChangeCheck { + handle_change_map: self.handle_change_map, + last_run: self.last_run, + this_run: self.this_run, + } + } +} +impl Copy for AssetChangeCheck<'_, A> {} +impl<'w, A: Asset> AssetChangeCheck<'w, A> { + fn new(changes: &'w AssetChanges, last_run: Tick, this_run: Tick) -> Self { Self { handle_change_map: &changes.change_ticks, last_run, @@ -29,7 +53,7 @@ impl<'w> AssetChangeCheck<'w> { } // TODO(perf): some sort of caching? Each check has two levels of indirection, // which is not optimal. - fn has_changed(&self, handle: &Handle) -> bool { + fn has_changed(&self, handle: &Handle) -> bool { if let Some(&handle_change_tick) = self.handle_change_map.get(&handle.id()) { handle_change_tick.is_newer_than(self.last_run, self.this_run) } else { @@ -38,29 +62,29 @@ impl<'w> AssetChangeCheck<'w> { } } -/// A shortcut for the commonly used `Or<(Changed>, AssetChanged)>` +/// A shortcut for the commonly used `Or<(Changed>, AssetChanged)>` /// query. /// -/// If you want to react to changes to `Handle` component of an entity, you have +/// If you want to react to changes to `Handle` component of an entity, you have /// two cases to worry about: -/// - The `Handle` was changed through a `Query<&mut Handle>`. -/// - The `T` in `Assets` pointed by `Handle` was changed through a -/// `ResMut>::get_mut`. +/// - The `Handle` was changed through a `Query<&mut Handle>`. +/// - The `A` in `Assets` pointed by `Handle` was changed through a +/// `ResMut>::get_mut`. /// /// To properly handle both those cases, you need to combine the `Changed` and `AssetChanged` /// filters. This query type does it for you. -pub type AssetOrHandleChanged = Or<(Changed>, AssetChanged)>; +pub type AssetOrHandleChanged = Or<(Changed>, AssetChanged)>; -/// Query filter to get all entities which `Handle` component underlying asset changed. +/// Query filter to get all entities which `Handle` component underlying asset changed. /// -/// Unlike `Changed>`, this filter returns entities for which **the underlying `T` +/// Unlike `Changed>`, this filter returns entities for which **the underlying `A` /// asset** was changed. For example, when modifying a `Mesh`, you would do so through the -/// [`Assets::get_mut`] method. This does not change the `Handle` that entities have -/// as component, so `Changed>` **will not do anything**. While with `AssetChanged`, +/// [`Assets::get_mut`] method. This does not change the `Handle` that entities have +/// as component, so `Changed>` **will not do anything**. While with `AssetChanged`, /// all entities with the modified mesh will be queried. /// /// # Notes -/// **performance**: This reads a hashmap once per entity with a `Handle` component, this might +/// **performance**: This reads a hashmap once per entity with a `Handle` component, this might /// result in slowdown, particularly if there are a lot of such entities. Note that if there was no /// updates, it will simply skip everything. /// @@ -73,15 +97,15 @@ pub type AssetOrHandleChanged = Or<(Changed>, AssetChanged)>; /// /// [`AssetStage::AssetEvents`]: crate::AssetStage::AssetEvents /// [`Assets::get_mut`]: crate::Assets::get_mut -pub struct AssetChanged(PhantomData); +pub struct AssetChanged(PhantomData); /// Fetch for [`AssetChanged`]. #[doc(hidden)] -pub struct AssetChangedFetch<'w, T: Asset> { - inner: Option>>, - check: AssetChangeCheck<'w>, +pub struct AssetChangedFetch<'w, A: Asset> { + inner: Option>>, + check: AssetChangeCheck<'w, A>, } -impl<'w, T: Asset> Clone for AssetChangedFetch<'w, T> { +impl<'w, A: Asset> Clone for AssetChangedFetch<'w, A> { fn clone(&self) -> Self { Self { inner: self.inner, @@ -92,18 +116,18 @@ impl<'w, T: Asset> Clone for AssetChangedFetch<'w, T> { /// State for [`AssetChanged`]. #[doc(hidden)] -pub struct AssetChangedState { +pub struct AssetChangedState { handle_id: ComponentId, asset_changes_id: ComponentId, asset_changes_archetype_id: ArchetypeComponentId, - _asset: PhantomData, + _asset: PhantomData, } // SAFETY: `ROQueryFetch` is the same as `QueryFetch` -unsafe impl WorldQuery for AssetChanged { +unsafe impl WorldQuery for AssetChanged { type ReadOnly = Self; - type Fetch<'w> = AssetChangedFetch<'w, T>; - type State = AssetChangedState; + type Fetch<'w> = AssetChangedFetch<'w, A>; + type State = AssetChangedState; type Item<'w> = bool; @@ -111,12 +135,12 @@ unsafe impl WorldQuery for AssetChanged { item } - fn init_state(world: &mut World) -> AssetChangedState { + fn init_state(world: &mut World) -> AssetChangedState { let asset_changes_resource_id = - if let Some(id) = world.components().resource_id::>() { + if let Some(id) = world.components().resource_id::>() { id } else { - world.init_resource::>() + world.init_resource::>() }; let asset_changes_archetype_id = world .storages() @@ -125,7 +149,7 @@ unsafe impl WorldQuery for AssetChanged { .unwrap() .id(); AssetChangedState { - handle_id: world.init_component::>(), + handle_id: world.init_component::>(), asset_changes_id: asset_changes_resource_id, asset_changes_archetype_id, _asset: PhantomData, @@ -146,27 +170,27 @@ unsafe impl WorldQuery for AssetChanged { ) -> Self::Fetch<'w> { let err_msg = || { panic!( - "AssetChanges<{ty}> was removed, please do not remove AssetChanges<{ty}> when using AssetChanged<{ty}>", - ty = std::any::type_name::() - ) + "AssetChanges<{ty}> was removed, please do not remove AssetChanges<{ty}> when using AssetChanged<{ty}>", + ty = std::any::type_name::() + ) }; let changes: &AssetChanges<_> = world.get_resource().unwrap_or_else(err_msg); let has_updates = changes.last_change_tick.is_newer_than(last_run, this_run); AssetChangedFetch { inner: has_updates .then(|| <&_>::init_fetch(world, &state.handle_id, last_run, this_run)), - check: AssetChangeCheck::new::(changes, last_run, this_run), + check: AssetChangeCheck::new(changes, last_run, this_run), } } - const IS_DENSE: bool = <&Handle>::IS_DENSE; + const IS_DENSE: bool = <&Handle>::IS_DENSE; // This Fetch filters more than just the archetype. const IS_ARCHETYPAL: bool = false; unsafe fn set_table<'w>(fetch: &mut Self::Fetch<'w>, state: &Self::State, table: &'w Table) { if let Some(inner) = &mut fetch.inner { - <&Handle>::set_table(inner, &state.handle_id, table); + <&Handle>::set_table(inner, &state.handle_id, table); } } @@ -177,7 +201,7 @@ unsafe impl WorldQuery for AssetChanged { table: &'w Table, ) { if let Some(inner) = &mut fetch.inner { - <&Handle>::set_archetype(inner, &state.handle_id, archetype, table); + <&Handle>::set_archetype(inner, &state.handle_id, archetype, table); } } @@ -187,7 +211,7 @@ unsafe impl WorldQuery for AssetChanged { table_row: TableRow, ) -> Self::Item<'w> { fetch.inner.as_mut().map_or(false, |inner| { - let handle = <&Handle>::fetch(inner, entity, table_row); + let handle = <&Handle>::fetch(inner, entity, table_row); fetch.check.has_changed(handle) }) } @@ -203,7 +227,7 @@ unsafe impl WorldQuery for AssetChanged { #[inline] fn update_component_access(state: &Self::State, access: &mut FilteredAccess) { - <&Handle>::update_component_access(&state.handle_id, access); + <&Handle>::update_component_access(&state.handle_id, access); access.add_read(state.asset_changes_id); } @@ -214,17 +238,20 @@ unsafe impl WorldQuery for AssetChanged { access: &mut Access, ) { access.add_read(state.asset_changes_archetype_id); - <&Handle>::update_archetype_component_access(&state.handle_id, archetype, access); + <&Handle>::update_archetype_component_access(&state.handle_id, archetype, access); } } /// SAFETY: read-only access -unsafe impl ReadOnlyWorldQuery for AssetChanged {} +unsafe impl ReadOnlyWorldQuery for AssetChanged {} #[cfg(test)] mod tests { - use crate::{AddAsset, Assets}; + use crate::{self as bevy_asset, AssetPlugin}; + + use crate::{AssetApp, Assets}; use bevy_app::{App, AppExit, Startup, Update}; + use bevy_core::TaskPoolPlugin; use bevy_ecs::{ entity::Entity, event::EventWriter, @@ -235,16 +262,15 @@ mod tests { use super::*; - #[derive(bevy_reflect::TypeUuid, TypePath)] - #[uuid = "44115972-f31b-46e5-be5c-2b9aece6a52f"] + #[derive(Asset, TypePath, Debug)] struct MyAsset(u32, &'static str); fn run_app(system: impl IntoSystem<(), (), Marker>) { let mut app = App::new(); - app.add_plugins((bevy_core::FrameCountPlugin, crate::AssetPlugin::default())) - .add_asset::() + app.add_plugins((TaskPoolPlugin::default(), AssetPlugin::default())) + .init_asset::() .add_systems(Update, system); - app.run(); + app.update(); } #[test] @@ -301,7 +327,7 @@ mod tests { #[test] fn asset_change() { - #[derive(Default, Resource)] + #[derive(Default, PartialEq, Debug, Resource)] struct Counter(u32, u32); let mut app = App::new(); @@ -324,12 +350,11 @@ mod tests { fn update_once(mut assets: ResMut>, mut run_count: Local) { let mut update_index = |i| { - let handle = assets + let id = assets .iter() .find_map(|(h, a)| (a.0 == i).then_some(h)) .unwrap(); - let handle = assets.get_handle(handle); - let asset = assets.get_mut(&handle).unwrap(); + let asset = assets.get_mut(id).unwrap(); asset.1 = "new_value"; }; match *run_count { @@ -339,8 +364,8 @@ mod tests { }; *run_count += 1; } - app.add_plugins((bevy_core::FrameCountPlugin, crate::AssetPlugin::default())) - .add_asset::() + app.add_plugins((TaskPoolPlugin::default(), AssetPlugin::default())) + .init_asset::() .init_resource::() .add_systems( Startup, @@ -355,10 +380,10 @@ mod tests { }, ) .add_systems(Update, (count_update, update_once).chain()); - let assert_counter = |app: &App, assert: Counter| { + #[track_caller] + fn assert_counter(app: &App, assert: Counter) { let counter = app.world.get_resource::().unwrap(); - assert_eq!(counter.0, assert.0); - assert_eq!(counter.1, assert.1); + assert_eq!(counter, &assert); }; // First run of the app, `add_systems(Startup…)` runs. app.update(); // run_count == 0 diff --git a/crates/bevy_asset/src/asset_changes.rs b/crates/bevy_asset/src/asset_changes.rs deleted file mode 100644 index 118248d23237a6..00000000000000 --- a/crates/bevy_asset/src/asset_changes.rs +++ /dev/null @@ -1,23 +0,0 @@ -use std::marker::PhantomData; - -use bevy_ecs::{component::Tick, prelude::*}; -use bevy_utils::HashMap; - -use crate::{Asset, HandleId}; - -#[doc(hidden)] -#[derive(Resource)] -pub struct AssetChanges { - pub(crate) change_ticks: HashMap, - pub(crate) last_change_tick: Tick, - _marker: PhantomData, -} -impl Default for AssetChanges { - fn default() -> Self { - Self { - change_ticks: Default::default(), - last_change_tick: Tick::new(0), - _marker: PhantomData, - } - } -} diff --git a/crates/bevy_asset/src/assets.rs b/crates/bevy_asset/src/assets.rs index a01a93c4dd71da..868fa6f5bec91d 100644 --- a/crates/bevy_asset/src/assets.rs +++ b/crates/bevy_asset/src/assets.rs @@ -1,7 +1,10 @@ -use crate::{Asset, AssetEvent, AssetHandleProvider, AssetId, AssetServer, Handle}; +use crate::{ + asset_changed::AssetChanges, Asset, AssetEvent, AssetHandleProvider, AssetId, AssetServer, + Handle, +}; use bevy_ecs::{ prelude::EventWriter, - system::{Res, ResMut, Resource}, + system::{Res, ResMut, Resource, SystemChangeTick}, }; use bevy_reflect::{Reflect, Uuid}; use bevy_utils::HashMap; @@ -478,7 +481,23 @@ impl Assets { /// A system that applies accumulated asset change events to the [`Events`] resource. /// /// [`Events`]: bevy_ecs::event::Events - pub fn asset_events(mut assets: ResMut, mut events: EventWriter>) { + pub fn asset_events( + mut assets: ResMut, + mut events: EventWriter>, + asset_changes: Option>>, + ticks: SystemChangeTick, + ) { + if let Some(mut asset_changes) = asset_changes { + for new_event in &assets.queued_events { + use AssetEvent::{Added, LoadedWithDependencies, Modified, Removed}; + match new_event { + Removed { id } => asset_changes.change_ticks.remove(id), + Added { id } | Modified { id } | LoadedWithDependencies { id } => { + asset_changes.change_ticks.insert(*id, ticks.this_run()) + } + }; + } + } events.send_batch(assets.queued_events.drain(..)); } } diff --git a/crates/bevy_asset/src/lib.rs b/crates/bevy_asset/src/lib.rs index 0dc9d0589b63eb..53fd74b415ac1a 100644 --- a/crates/bevy_asset/src/lib.rs +++ b/crates/bevy_asset/src/lib.rs @@ -4,6 +4,9 @@ pub mod processor; pub mod saver; pub mod prelude { + #[doc(hidden)] + pub use crate::asset_changed::{AssetChanged, AssetOrHandleChanged}; + #[doc(hidden)] pub use crate::{ Asset, AssetApp, AssetEvent, AssetId, AssetPlugin, AssetServer, Assets, Handle, @@ -11,6 +14,7 @@ pub mod prelude { }; } +mod asset_changed; mod assets; mod event; mod folder;