Skip to content

Commit

Permalink
[assets] fix Assets being set as 'changed' each frame (bevyengine#2280)
Browse files Browse the repository at this point in the history
## Objective
- Fixes: bevyengine#2275 
- `Assets` were being flagged as 'changed' each frame regardless of if the assets were actually being updated. 

## Solution
- Only have `Assets` change detection be triggered when the collection is actually modified. 
- This includes utilizing `ResMut` further down the stack instead of a `&mut Assets` directly.
  • Loading branch information
NathanSWard authored and ostwilkens committed Jul 27, 2021
1 parent 6bfd26c commit bdb2e63
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 5 deletions.
6 changes: 4 additions & 2 deletions crates/bevy_asset/src/asset_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::{
RefChange, RefChangeChannel, SourceInfo, SourceMeta,
};
use anyhow::Result;
use bevy_ecs::system::Res;
use bevy_ecs::system::{Res, ResMut};
use bevy_log::warn;
use bevy_tasks::TaskPool;
use bevy_utils::{HashMap, Uuid};
Expand Down Expand Up @@ -466,7 +466,9 @@ impl AssetServer {
}
}

pub(crate) fn update_asset_storage<T: Asset>(&self, assets: &mut Assets<T>) {
// Note: this takes a `ResMut<Assets<T>>` to ensure change detection does not get
// triggered unless the `Assets` collection is actually updated.
pub(crate) fn update_asset_storage<T: Asset>(&self, mut assets: ResMut<Assets<T>>) {
let asset_lifecycles = self.server.asset_lifecycles.read();
let asset_lifecycle = asset_lifecycles.get(&T::TYPE_UUID).unwrap();
let mut asset_sources_guard = None;
Expand Down
6 changes: 5 additions & 1 deletion crates/bevy_asset/src/assets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,11 @@ impl<T: Asset> Assets<T> {
mut events: EventWriter<AssetEvent<T>>,
mut assets: ResMut<Assets<T>>,
) {
events.send_batch(assets.events.drain())
// Check if the events are empty before calling `drain`.
// As `drain` triggers change detection.
if !assets.events.is_empty() {
events.send_batch(assets.events.drain())
}
}

pub fn len(&self) -> usize {
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_asset/src/loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ impl<T: Component> Default for AssetLifecycleChannel<T> {
/// Updates the [Assets] collection according to the changes queued up by [AssetServer].
pub fn update_asset_storage_system<T: Asset + AssetDynamic>(
asset_server: Res<AssetServer>,
mut assets: ResMut<Assets<T>>,
assets: ResMut<Assets<T>>,
) {
asset_server.update_asset_storage(&mut assets);
asset_server.update_asset_storage(assets);
}
23 changes: 23 additions & 0 deletions crates/bevy_ecs/src/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,12 @@ impl<T: Component> Events<T> {
self.events_b.clear();
}

/// Returns true if there are no events in this collection.
#[inline]
pub fn is_empty(&self) -> bool {
self.events_a.is_empty() && self.events_b.is_empty()
}

/// Creates a draining iterator that removes all events.
pub fn drain(&mut self) -> impl Iterator<Item = T> + '_ {
self.reset_start_event_count();
Expand Down Expand Up @@ -557,4 +563,21 @@ mod tests {
.iter(&events)
.eq([TestEvent { i: 0 }, TestEvent { i: 1 }].iter()));
}

#[test]
fn test_events_empty() {
let mut events = Events::<TestEvent>::default();
assert!(events.is_empty());

events.send(TestEvent { i: 0 });
assert!(!events.is_empty());

events.update();
assert!(!events.is_empty());

// events are only empty after the second call to update
// due to double buffering.
events.update();
assert!(events.is_empty());
}
}

0 comments on commit bdb2e63

Please sign in to comment.