Skip to content

Commit

Permalink
Start asset_v2 migration BROKEN
Browse files Browse the repository at this point in the history
  • Loading branch information
nicopap committed Sep 15, 2023
1 parent fcf8253 commit 10443ce
Show file tree
Hide file tree
Showing 4 changed files with 110 additions and 85 deletions.
143 changes: 84 additions & 59 deletions crates/bevy_asset/src/asset_changed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<HandleId, Tick>,
#[doc(hidden)]
#[derive(Resource)]
pub struct AssetChanges<A: Asset> {
pub(crate) change_ticks: HashMap<AssetId<A>, Tick>,
pub(crate) last_change_tick: Tick,
}
impl<A: Asset> Default for AssetChanges<A> {
fn default() -> Self {
Self {
change_ticks: Default::default(),
last_change_tick: Tick::new(0),
}
}
}

struct AssetChangeCheck<'w, A: Asset> {
handle_change_map: &'w HashMap<AssetId<A>, Tick>,
last_run: Tick,
this_run: Tick,
}
impl<'w> AssetChangeCheck<'w> {
fn new<T: Asset>(changes: &'w AssetChanges<T>, last_run: Tick, this_run: Tick) -> Self {
impl<A: Asset> 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<A: Asset> Copy for AssetChangeCheck<'_, A> {}
impl<'w, A: Asset> AssetChangeCheck<'w, A> {
fn new(changes: &'w AssetChanges<A>, last_run: Tick, this_run: Tick) -> Self {
Self {
handle_change_map: &changes.change_ticks,
last_run,
Expand All @@ -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<T: Asset>(&self, handle: &Handle<T>) -> bool {
fn has_changed(&self, handle: &Handle<A>) -> 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 {
Expand All @@ -38,29 +62,29 @@ impl<'w> AssetChangeCheck<'w> {
}
}

/// A shortcut for the commonly used `Or<(Changed<Handle<T>>, AssetChanged<T>)>`
/// A shortcut for the commonly used `Or<(Changed<Handle<A>>, AssetChanged<A>)>`
/// query.
///
/// If you want to react to changes to `Handle<T>` component of an entity, you have
/// If you want to react to changes to `Handle<A>` component of an entity, you have
/// two cases to worry about:
/// - The `Handle<T>` was changed through a `Query<&mut Handle<T>>`.
/// - The `T` in `Assets<T>` pointed by `Handle<T>` was changed through a
/// `ResMut<Assets<T>>::get_mut`.
/// - The `Handle<A>` was changed through a `Query<&mut Handle<A>>`.
/// - The `A` in `Assets<A>` pointed by `Handle<A>` was changed through a
/// `ResMut<Assets<A>>::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<T> = Or<(Changed<Handle<T>>, AssetChanged<T>)>;
pub type AssetOrHandleChanged<A> = Or<(Changed<Handle<A>>, AssetChanged<A>)>;

/// Query filter to get all entities which `Handle<T>` component underlying asset changed.
/// Query filter to get all entities which `Handle<A>` component underlying asset changed.
///
/// Unlike `Changed<Handle<T>>`, this filter returns entities for which **the underlying `T`
/// Unlike `Changed<Handle<A>>`, 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<Mesh>::get_mut`] method. This does not change the `Handle<T>` that entities have
/// as component, so `Changed<Handle<T>>` **will not do anything**. While with `AssetChanged<T>`,
/// [`Assets<Mesh>::get_mut`] method. This does not change the `Handle<A>` that entities have
/// as component, so `Changed<Handle<A>>` **will not do anything**. While with `AssetChanged<A>`,
/// all entities with the modified mesh will be queried.
///
/// # Notes
/// **performance**: This reads a hashmap once per entity with a `Handle<T>` component, this might
/// **performance**: This reads a hashmap once per entity with a `Handle<A>` 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.
///
Expand All @@ -73,15 +97,15 @@ pub type AssetOrHandleChanged<T> = Or<(Changed<Handle<T>>, AssetChanged<T>)>;
///
/// [`AssetStage::AssetEvents`]: crate::AssetStage::AssetEvents
/// [`Assets<Mesh>::get_mut`]: crate::Assets::get_mut
pub struct AssetChanged<T>(PhantomData<T>);
pub struct AssetChanged<A>(PhantomData<A>);

/// Fetch for [`AssetChanged`].
#[doc(hidden)]
pub struct AssetChangedFetch<'w, T: Asset> {
inner: Option<ReadFetch<'w, Handle<T>>>,
check: AssetChangeCheck<'w>,
pub struct AssetChangedFetch<'w, A: Asset> {
inner: Option<ReadFetch<'w, Handle<A>>>,
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,
Expand All @@ -92,31 +116,31 @@ impl<'w, T: Asset> Clone for AssetChangedFetch<'w, T> {

/// State for [`AssetChanged`].
#[doc(hidden)]
pub struct AssetChangedState<T: Asset> {
pub struct AssetChangedState<A: Asset> {
handle_id: ComponentId,
asset_changes_id: ComponentId,
asset_changes_archetype_id: ArchetypeComponentId,
_asset: PhantomData<fn(T)>,
_asset: PhantomData<fn(A)>,
}

// SAFETY: `ROQueryFetch<Self>` is the same as `QueryFetch<Self>`
unsafe impl<T: Asset> WorldQuery for AssetChanged<T> {
unsafe impl<A: Asset> WorldQuery for AssetChanged<A> {
type ReadOnly = Self;
type Fetch<'w> = AssetChangedFetch<'w, T>;
type State = AssetChangedState<T>;
type Fetch<'w> = AssetChangedFetch<'w, A>;
type State = AssetChangedState<A>;

type Item<'w> = bool;

fn shrink<'wlong: 'wshort, 'wshort>(item: QueryItem<'wlong, Self>) -> QueryItem<'wshort, Self> {
item
}

fn init_state(world: &mut World) -> AssetChangedState<T> {
fn init_state(world: &mut World) -> AssetChangedState<A> {
let asset_changes_resource_id =
if let Some(id) = world.components().resource_id::<AssetChanges<T>>() {
if let Some(id) = world.components().resource_id::<AssetChanges<A>>() {
id
} else {
world.init_resource::<AssetChanges<T>>()
world.init_resource::<AssetChanges<A>>()
};
let asset_changes_archetype_id = world
.storages()
Expand All @@ -125,7 +149,7 @@ unsafe impl<T: Asset> WorldQuery for AssetChanged<T> {
.unwrap()
.id();
AssetChangedState {
handle_id: world.init_component::<Handle<T>>(),
handle_id: world.init_component::<Handle<A>>(),
asset_changes_id: asset_changes_resource_id,
asset_changes_archetype_id,
_asset: PhantomData,
Expand All @@ -146,27 +170,27 @@ unsafe impl<T: Asset> WorldQuery for AssetChanged<T> {
) -> 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::<T>()
)
"AssetChanges<{ty}> was removed, please do not remove AssetChanges<{ty}> when using AssetChanged<{ty}>",
ty = std::any::type_name::<A>()
)
};
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::<T>(changes, last_run, this_run),
check: AssetChangeCheck::new(changes, last_run, this_run),
}
}

const IS_DENSE: bool = <&Handle<T>>::IS_DENSE;
const IS_DENSE: bool = <&Handle<A>>::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<T>>::set_table(inner, &state.handle_id, table);
<&Handle<A>>::set_table(inner, &state.handle_id, table);
}
}

Expand All @@ -177,7 +201,7 @@ unsafe impl<T: Asset> WorldQuery for AssetChanged<T> {
table: &'w Table,
) {
if let Some(inner) = &mut fetch.inner {
<&Handle<T>>::set_archetype(inner, &state.handle_id, archetype, table);
<&Handle<A>>::set_archetype(inner, &state.handle_id, archetype, table);
}
}

Expand All @@ -187,7 +211,7 @@ unsafe impl<T: Asset> WorldQuery for AssetChanged<T> {
table_row: TableRow,
) -> Self::Item<'w> {
fetch.inner.as_mut().map_or(false, |inner| {
let handle = <&Handle<T>>::fetch(inner, entity, table_row);
let handle = <&Handle<A>>::fetch(inner, entity, table_row);
fetch.check.has_changed(handle)
})
}
Expand All @@ -203,7 +227,7 @@ unsafe impl<T: Asset> WorldQuery for AssetChanged<T> {

#[inline]
fn update_component_access(state: &Self::State, access: &mut FilteredAccess<ComponentId>) {
<&Handle<T>>::update_component_access(&state.handle_id, access);
<&Handle<A>>::update_component_access(&state.handle_id, access);
access.add_read(state.asset_changes_id);
}

Expand All @@ -214,17 +238,20 @@ unsafe impl<T: Asset> WorldQuery for AssetChanged<T> {
access: &mut Access<ArchetypeComponentId>,
) {
access.add_read(state.asset_changes_archetype_id);
<&Handle<T>>::update_archetype_component_access(&state.handle_id, archetype, access);
<&Handle<A>>::update_archetype_component_access(&state.handle_id, archetype, access);
}
}

/// SAFETY: read-only access
unsafe impl<T: Asset> ReadOnlyWorldQuery for AssetChanged<T> {}
unsafe impl<A: Asset> ReadOnlyWorldQuery for AssetChanged<A> {}

#[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,
Expand All @@ -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<Marker>(system: impl IntoSystem<(), (), Marker>) {
let mut app = App::new();
app.add_plugins((bevy_core::FrameCountPlugin, crate::AssetPlugin::default()))
.add_asset::<MyAsset>()
app.add_plugins((TaskPoolPlugin::default(), AssetPlugin::default()))
.init_asset::<MyAsset>()
.add_systems(Update, system);
app.run();
app.update();
}

#[test]
Expand Down Expand Up @@ -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();
Expand All @@ -324,12 +350,11 @@ mod tests {

fn update_once(mut assets: ResMut<Assets<MyAsset>>, mut run_count: Local<u32>) {
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 {
Expand All @@ -339,8 +364,8 @@ mod tests {
};
*run_count += 1;
}
app.add_plugins((bevy_core::FrameCountPlugin, crate::AssetPlugin::default()))
.add_asset::<MyAsset>()
app.add_plugins((TaskPoolPlugin::default(), AssetPlugin::default()))
.init_asset::<MyAsset>()
.init_resource::<Counter>()
.add_systems(
Startup,
Expand All @@ -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::<Counter>().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
Expand Down
23 changes: 0 additions & 23 deletions crates/bevy_asset/src/asset_changes.rs

This file was deleted.

25 changes: 22 additions & 3 deletions crates/bevy_asset/src/assets.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -478,7 +481,23 @@ impl<A: Asset> Assets<A> {
/// A system that applies accumulated asset change events to the [`Events`] resource.
///
/// [`Events`]: bevy_ecs::event::Events
pub fn asset_events(mut assets: ResMut<Self>, mut events: EventWriter<AssetEvent<A>>) {
pub fn asset_events(
mut assets: ResMut<Self>,
mut events: EventWriter<AssetEvent<A>>,
asset_changes: Option<ResMut<AssetChanges<A>>>,
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(..));
}
}
Expand Down
Loading

0 comments on commit 10443ce

Please sign in to comment.