Delay asset hot reloading (bevyengine#8503)

# Objective

- Fix bevyengine#5631 

## Solution

- Wait 50ms (configurable) after the last modification event before
reloading an asset.

---

## Changelog

- `AssetPlugin::watch_for_changes` is now a `ChangeWatcher` instead of a
`bool`
- Fixed bevyengine#5631

## Migration Guide
- Replace `AssetPlugin::watch_for_changes: true` with e.g.
`ChangeWatcher::with_delay(Duration::from_millis(200))`

---------

Co-authored-by: François <[email protected]>

crates/bevy_asset/src/asset_server.rs

@@ -82,10 +82,11 @@ pub struct AssetServerInternal {
 /// ```
 /// # use bevy_asset::*;
 /// # use bevy_app::*;
+/// # use bevy_utils::Duration;
 /// # let mut app = App::new();
 /// // The asset plugin can be configured to watch for asset changes.
 /// app.add_plugin(AssetPlugin {
-///     watch_for_changes: true,
+///     watch_for_changes: ChangeWatcher::with_delay(Duration::from_millis(200)),
 ///     ..Default::default()
 /// });
 /// ```
@@ -702,7 +703,7 @@ mod test {
     fn setup(asset_path: impl AsRef<Path>) -> AssetServer {
         use crate::FileAssetIo;
         IoTaskPool::init(Default::default);
-        AssetServer::new(FileAssetIo::new(asset_path, false))
+        AssetServer::new(FileAssetIo::new(asset_path, &None))
     }
 
     #[test]

crates/bevy_asset/src/debug_asset_server.rs

@@ -5,14 +5,15 @@
 use bevy_app::{App, Plugin, Update};
 use bevy_ecs::{prelude::*, system::SystemState};
 use bevy_tasks::{IoTaskPool, TaskPoolBuilder};
-use bevy_utils::HashMap;
+use bevy_utils::{Duration, HashMap};
 use std::{
     ops::{Deref, DerefMut},
     path::Path,
 };
 
 use crate::{
-    Asset, AssetEvent, AssetPlugin, AssetServer, Assets, FileAssetIo, Handle, HandleUntyped,
+    Asset, AssetEvent, AssetPlugin, AssetServer, Assets, ChangeWatcher, FileAssetIo, Handle,
+    HandleUntyped,
 };
 
 /// A helper [`App`] used for hot reloading internal assets, which are compiled-in to Bevy plugins.
@@ -72,7 +73,7 @@ impl Plugin for DebugAssetServerPlugin {
         let mut debug_asset_app = App::new();
         debug_asset_app.add_plugin(AssetPlugin {
             asset_folder: "crates".to_string(),
-            watch_for_changes: true,
+            watch_for_changes: ChangeWatcher::with_delay(Duration::from_millis(200)),
         });
         app.insert_non_send_resource(DebugAssetApp(debug_asset_app));
         app.add_systems(Update, run_debug_asset_app);

crates/bevy_asset/src/filesystem_watcher.rs

@@ -1,8 +1,10 @@
-use bevy_utils::{default, HashMap, HashSet};
+use bevy_utils::{default, Duration, HashMap, HashSet};
 use crossbeam_channel::Receiver;
 use notify::{Event, RecommendedWatcher, RecursiveMode, Result, Watcher};
 use std::path::{Path, PathBuf};
 
+use crate::ChangeWatcher;
+
 /// Watches for changes to files on the local filesystem.
 ///
 /// When hot-reloading is enabled, the [`AssetServer`](crate::AssetServer) uses this to reload
@@ -11,10 +13,11 @@ pub struct FilesystemWatcher {
     pub watcher: RecommendedWatcher,
     pub receiver: Receiver<Result<Event>>,
     pub path_map: HashMap<PathBuf, HashSet<PathBuf>>,
+    pub delay: Duration,
 }
 
-impl Default for FilesystemWatcher {
-    fn default() -> Self {
+impl FilesystemWatcher {
+    pub fn new(configuration: &ChangeWatcher) -> Self {
         let (sender, receiver) = crossbeam_channel::unbounded();
         let watcher: RecommendedWatcher = RecommendedWatcher::new(
             move |res| {
@@ -27,11 +30,10 @@ impl Default for FilesystemWatcher {
             watcher,
             receiver,
             path_map: default(),
+            delay: configuration.delay,
         }
     }
-}
 
-impl FilesystemWatcher {
     /// Watch for changes recursively at the provided path.
     pub fn watch<P: AsRef<Path>>(&mut self, to_watch: P, to_reload: PathBuf) -> Result<()> {
         self.path_map

crates/bevy_asset/src/io/android_asset_io.rs

@@ -1,4 +1,4 @@
-use crate::{AssetIo, AssetIoError, Metadata};
+use crate::{AssetIo, AssetIoError, ChangeWatcher, Metadata};
 use anyhow::Result;
 use bevy_utils::BoxedFuture;
 use std::{
@@ -59,7 +59,7 @@ impl AssetIo for AndroidAssetIo {
         Ok(())
     }
 
-    fn watch_for_changes(&self) -> Result<(), AssetIoError> {
+    fn watch_for_changes(&self, _configuration: &ChangeWatcher) -> Result<(), AssetIoError> {
         bevy_log::warn!("Watching for changes is not supported on Android");
         Ok(())
     }

crates/bevy_asset/src/io/file_asset_io.rs

@@ -1,12 +1,12 @@
 #[cfg(feature = "filesystem_watcher")]
 use crate::{filesystem_watcher::FilesystemWatcher, AssetServer};
-use crate::{AssetIo, AssetIoError, Metadata};
+use crate::{AssetIo, AssetIoError, ChangeWatcher, Metadata};
 use anyhow::Result;
 #[cfg(feature = "filesystem_watcher")]
-use bevy_ecs::system::Res;
+use bevy_ecs::system::{Local, Res};
 use bevy_utils::BoxedFuture;
 #[cfg(feature = "filesystem_watcher")]
-use bevy_utils::{default, HashSet};
+use bevy_utils::{default, HashMap, Instant};
 #[cfg(feature = "filesystem_watcher")]
 use crossbeam_channel::TryRecvError;
 use fs::File;
@@ -35,13 +35,13 @@ impl FileAssetIo {
     /// watching for changes.
     ///
     /// See `get_base_path` below.
-    pub fn new<P: AsRef<Path>>(path: P, watch_for_changes: bool) -> Self {
+    pub fn new<P: AsRef<Path>>(path: P, watch_for_changes: &Option<ChangeWatcher>) -> Self {
         let file_asset_io = FileAssetIo {
             #[cfg(feature = "filesystem_watcher")]
             filesystem_watcher: default(),
             root_path: Self::get_base_path().join(path.as_ref()),
         };
-        if watch_for_changes {
+        if let Some(configuration) = watch_for_changes {
             #[cfg(any(
                 not(feature = "filesystem_watcher"),
                 target_arch = "wasm32",
@@ -52,7 +52,7 @@ impl FileAssetIo {
                 wasm32 / android targets"
             );
             #[cfg(feature = "filesystem_watcher")]
-            file_asset_io.watch_for_changes().unwrap();
+            file_asset_io.watch_for_changes(configuration).unwrap();
         }
         file_asset_io
     }
@@ -143,10 +143,10 @@ impl AssetIo for FileAssetIo {
         Ok(())
     }
 
-    fn watch_for_changes(&self) -> Result<(), AssetIoError> {
+    fn watch_for_changes(&self, configuration: &ChangeWatcher) -> Result<(), AssetIoError> {
         #[cfg(feature = "filesystem_watcher")]
         {
-            *self.filesystem_watcher.write() = Some(default());
+            *self.filesystem_watcher.write() = Some(FilesystemWatcher::new(configuration));
         }
         #[cfg(not(feature = "filesystem_watcher"))]
         bevy_log::warn!("Watching for changes is not supported when the `filesystem_watcher` feature is disabled");
@@ -174,22 +174,26 @@ impl AssetIo for FileAssetIo {
     feature = "filesystem_watcher",
     all(not(target_arch = "wasm32"), not(target_os = "android"))
 ))]
-pub fn filesystem_watcher_system(asset_server: Res<AssetServer>) {
+pub fn filesystem_watcher_system(
+    asset_server: Res<AssetServer>,
+    mut changed: Local<HashMap<PathBuf, Instant>>,
+) {
     let asset_io =
         if let Some(asset_io) = asset_server.server.asset_io.downcast_ref::<FileAssetIo>() {
             asset_io
         } else {
             return;
         };
     let watcher = asset_io.filesystem_watcher.read();
+
     if let Some(ref watcher) = *watcher {
-        let mut changed = HashSet::<&PathBuf>::default();
         loop {
             let event = match watcher.receiver.try_recv() {
                 Ok(result) => result.unwrap(),
                 Err(TryRecvError::Empty) => break,
                 Err(TryRecvError::Disconnected) => panic!("FilesystemWatcher disconnected."),
             };
+
             if let notify::event::Event {
                 kind: notify::event::EventKind::Modify(_),
                 paths,
@@ -199,13 +203,22 @@ pub fn filesystem_watcher_system(asset_server: Res<AssetServer>) {
                 for path in &paths {
                     let Some(set) = watcher.path_map.get(path) else {continue};
                     for to_reload in set {
-                        if !changed.contains(to_reload) {
-                            changed.insert(to_reload);
-                            let _ = asset_server.load_untracked(to_reload.as_path().into(), true);
-                        }
+                        // When an asset is modified, note down the timestamp (overriding any previous modification events)
+                        changed.insert(to_reload.to_owned(), Instant::now());
                     }
                 }
             }
         }
+
+        // Reload all assets whose last modification was at least 50ms ago.
+        //
+        // When changing and then saving a shader, several modification events are sent in short succession.
+        // Unless we wait until we are sure the shader is finished being modified (and that there will be no more events coming),
+        // we will sometimes get a crash when trying to reload a partially-modified shader.
+        for (to_reload, _) in
+            changed.drain_filter(|_, last_modified| last_modified.elapsed() >= watcher.delay)
+        {
+            let _ = asset_server.load_untracked(to_reload.as_path().into(), true);
+        }
     }
 }

crates/bevy_asset/src/io/mod.rs

@@ -25,6 +25,8 @@ use std::{
 };
 use thiserror::Error;
 
+use crate::ChangeWatcher;
+
 /// Errors that occur while loading assets.
 #[derive(Error, Debug)]
 pub enum AssetIoError {
@@ -81,7 +83,7 @@ pub trait AssetIo: Downcast + Send + Sync + 'static {
     ) -> Result<(), AssetIoError>;
 
     /// Enables change tracking in this asset I/O.
-    fn watch_for_changes(&self) -> Result<(), AssetIoError>;
+    fn watch_for_changes(&self, configuration: &ChangeWatcher) -> Result<(), AssetIoError>;
 
     /// Returns `true` if the path is a directory.
     fn is_dir(&self, path: &Path) -> bool {

crates/bevy_asset/src/io/wasm_asset_io.rs

@@ -1,4 +1,4 @@
-use crate::{AssetIo, AssetIoError, Metadata};
+use crate::{AssetIo, AssetIoError, ChangeWatcher, Metadata};
 use anyhow::Result;
 use bevy_utils::BoxedFuture;
 use js_sys::Uint8Array;
@@ -64,7 +64,7 @@ impl AssetIo for WasmAssetIo {
         Ok(())
     }
 
-    fn watch_for_changes(&self) -> Result<(), AssetIoError> {
+    fn watch_for_changes(&self, _configuration: &ChangeWatcher) -> Result<(), AssetIoError> {
         bevy_log::warn!("Watching for changes is not supported in WASM");
         Ok(())
     }

crates/bevy_asset/src/lib.rs

@@ -49,6 +49,7 @@ pub use reflect::*;
 
 use bevy_app::{prelude::*, MainScheduleOrder};
 use bevy_ecs::schedule::ScheduleLabel;
+use bevy_utils::Duration;
 
 /// Asset storages are updated.
 #[derive(Debug, Hash, PartialEq, Eq, Clone, ScheduleLabel)]
@@ -57,6 +58,30 @@ pub struct LoadAssets;
 #[derive(Debug, Hash, PartialEq, Eq, Clone, ScheduleLabel)]
 pub struct AssetEvents;
 
+/// Configuration for hot reloading assets by watching for changes.
+#[derive(Debug, Clone)]
+pub struct ChangeWatcher {
+    /// Minimum delay after which a file change will trigger a reload.
+    ///
+    /// The change watcher will wait for this duration after a file change before reloading the
+    /// asset. This is useful to avoid reloading an asset multiple times when it is changed
+    /// multiple times in a short period of time, or to avoid reloading an asset that is still
+    /// being written to.
+    ///
+    /// If you have a slow hard drive or expect to reload large assets, you may want to increase
+    /// this value.
+    pub delay: Duration,
+}
+
+impl ChangeWatcher {
+    /// Enable change watching with the given delay when a file is changed.
+    ///
+    /// See [`Self::delay`] for more details on how this value is used.
+    pub fn with_delay(delay: Duration) -> Option<Self> {
+        Some(Self { delay })
+    }
+}
+
 /// Adds support for [`Assets`] to an App.
 ///
 /// Assets are typed collections with change tracking, which are added as App Resources. Examples of
@@ -67,14 +92,14 @@ pub struct AssetPlugin {
     pub asset_folder: String,
     /// Whether to watch for changes in asset files. Requires the `filesystem_watcher` feature,
     /// and cannot be supported on the wasm32 arch nor android os.
-    pub watch_for_changes: bool,
+    pub watch_for_changes: Option<ChangeWatcher>,
 }
 
 impl Default for AssetPlugin {
     fn default() -> Self {
         Self {
             asset_folder: "assets".to_string(),
-            watch_for_changes: false,
+            watch_for_changes: None,
         }
     }
 }
@@ -86,7 +111,7 @@ impl AssetPlugin {
     /// delegate to the default `AssetIo` for the platform.
     pub fn create_platform_default_asset_io(&self) -> Box<dyn AssetIo> {
         #[cfg(all(not(target_arch = "wasm32"), not(target_os = "android")))]
-        let source = FileAssetIo::new(&self.asset_folder, self.watch_for_changes);
+        let source = FileAssetIo::new(&self.asset_folder, &self.watch_for_changes);
         #[cfg(target_arch = "wasm32")]
         let source = WasmAssetIo::new(&self.asset_folder);
         #[cfg(target_os = "android")]

examples/asset/custom_asset_io.rs

@@ -3,7 +3,7 @@
 //! It does not know anything about the asset formats, only how to talk to the underlying storage.
 
 use bevy::{
-    asset::{AssetIo, AssetIoError, Metadata},
+    asset::{AssetIo, AssetIoError, ChangeWatcher, Metadata},
     prelude::*,
     utils::BoxedFuture,
 };
@@ -39,9 +39,9 @@ impl AssetIo for CustomAssetIo {
         self.0.watch_path_for_changes(to_watch, to_reload)
     }
 
-    fn watch_for_changes(&self) -> Result<(), AssetIoError> {
+    fn watch_for_changes(&self, configuration: &ChangeWatcher) -> Result<(), AssetIoError> {
         info!("watch_for_changes()");
-        self.0.watch_for_changes()
+        self.0.watch_for_changes(configuration)
     }
 
     fn get_metadata(&self, path: &Path) -> Result<Metadata, AssetIoError> {

examples/asset/hot_asset_reloading.rs

@@ -2,13 +2,13 @@
 //! running. This lets you immediately see the results of your changes without restarting the game.
 //! This example illustrates hot reloading mesh changes.
 
-use bevy::prelude::*;
+use bevy::{asset::ChangeWatcher, prelude::*, utils::Duration};
 
 fn main() {
     App::new()
         .add_plugins(DefaultPlugins.set(AssetPlugin {
             // Tell the asset server to watch for asset changes on disk:
-            watch_for_changes: true,
+            watch_for_changes: ChangeWatcher::with_delay(Duration::from_millis(200)),
             ..default()
         }))
         .add_systems(Startup, setup)

examples/scene/scene.rs

@@ -1,13 +1,13 @@
 //! This example illustrates loading scenes from files.
-use bevy::{prelude::*, tasks::IoTaskPool, utils::Duration};
+use bevy::{asset::ChangeWatcher, prelude::*, tasks::IoTaskPool, utils::Duration};
 use std::{fs::File, io::Write};
 
 fn main() {
     App::new()
         .add_plugins(DefaultPlugins.set(AssetPlugin {
             // This tells the AssetServer to watch for changes to assets.
             // It enables our scenes to automatically reload in game when we modify their files.
-            watch_for_changes: true,
+            watch_for_changes: ChangeWatcher::with_delay(Duration::from_millis(200)),
             ..default()
         }))
         .register_type::<ComponentA>()

examples/shader/post_processing.rs

@@ -6,6 +6,7 @@
 //! This is a fairly low level example and assumes some familiarity with rendering concepts and wgpu.
 
 use bevy::{
+    asset::ChangeWatcher,
     core_pipeline::{
         clear_color::ClearColorConfig, core_3d,
         fullscreen_vertex_shader::fullscreen_shader_vertex_state,
@@ -29,13 +30,14 @@ use bevy::{
         view::{ExtractedView, ViewTarget},
         RenderApp,
     },
+    utils::Duration,
 };
 
 fn main() {
     App::new()
         .add_plugins(DefaultPlugins.set(AssetPlugin {
             // Hot reloading the shader works correctly
-            watch_for_changes: true,
+            watch_for_changes: ChangeWatcher::with_delay(Duration::from_millis(200)),
             ..default()
         }))
         .add_plugin(PostProcessPlugin)