Make building asset sources infallible. (#21721)

# Objective

- Creating an asset source without a reader results in entirely skipping
the asset source.

## Solution

- Make the `AssetSourceBuilder` take a reader builder fn in a `new`
method.
- Remove `AssetSource::build` - users should just call the builder
directly.

## Testing

- Tests still pass.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Greeble <166992735+greeble-dev@users.noreply.github.com>

Author: 3 people

crates/bevy_asset/src/io/embedded/mod.rs

@@ -6,7 +6,7 @@ pub use embedded_watcher::*;
 
 use crate::io::{
     memory::{Dir, MemoryAssetReader, Value},
-    AssetSource, AssetSourceBuilders,
+    AssetSourceBuilder, AssetSourceBuilders,
 };
 use crate::AssetServer;
 use alloc::boxed::Box;
@@ -19,7 +19,7 @@ use std::path::{Path, PathBuf};
 #[cfg(feature = "embedded_watcher")]
 use alloc::borrow::ToOwned;
 
-/// The name of the `embedded` [`AssetSource`],
+/// The name of the `embedded` [`AssetSource`](crate::io::AssetSource),
 /// as stored in the [`AssetSourceBuilders`] resource.
 pub const EMBEDDED: &str = "embedded";
 
@@ -38,8 +38,8 @@ pub struct EmbeddedAssetRegistry {
 impl EmbeddedAssetRegistry {
     /// Inserts a new asset. `full_path` is the full path (as [`file`] would return for that file, if it was capable of
     /// running in a non-rust file). `asset_path` is the path that will be used to identify the asset in the `embedded`
-    /// [`AssetSource`]. `value` is the bytes that will be returned for the asset. This can be _either_ a `&'static [u8]`
-    /// or a [`Vec<u8>`](alloc::vec::Vec).
+    /// [`AssetSource`](crate::io::AssetSource). `value` is the bytes that will be returned for the asset. This can be
+    /// _either_ a `&'static [u8]` or a [`Vec<u8>`](alloc::vec::Vec).
     #[cfg_attr(
         not(feature = "embedded_watcher"),
         expect(
@@ -58,8 +58,8 @@ impl EmbeddedAssetRegistry {
 
     /// Inserts new asset metadata. `full_path` is the full path (as [`file`] would return for that file, if it was capable of
     /// running in a non-rust file). `asset_path` is the path that will be used to identify the asset in the `embedded`
-    /// [`AssetSource`]. `value` is the bytes that will be returned for the asset. This can be _either_ a `&'static [u8]`
-    /// or a [`Vec<u8>`](alloc::vec::Vec).
+    /// [`AssetSource`](crate::io::AssetSource). `value` is the bytes that will be returned for the asset. This can be _either_
+    /// a `&'static [u8]` or a [`Vec<u8>`](alloc::vec::Vec).
     #[cfg_attr(
         not(feature = "embedded_watcher"),
         expect(
@@ -83,7 +83,7 @@ impl EmbeddedAssetRegistry {
         self.dir.remove_asset(full_path)
     }
 
-    /// Registers the [`EMBEDDED`] [`AssetSource`] with the given [`AssetSourceBuilders`].
+    /// Registers the [`EMBEDDED`] [`AssetSource`](crate::io::AssetSource) with the given [`AssetSourceBuilders`].
     pub fn register_source(&self, sources: &mut AssetSourceBuilders) {
         let dir = self.dir.clone();
         let processed_dir = self.dir.clone();
@@ -95,18 +95,18 @@ impl EmbeddedAssetRegistry {
                 reason = "Variable is only mutated when `embedded_watcher` feature is enabled."
             )
         )]
-        let mut source = AssetSource::build()
-            .with_reader(move || Box::new(MemoryAssetReader { root: dir.clone() }))
-            .with_processed_reader(move || {
-                Box::new(MemoryAssetReader {
-                    root: processed_dir.clone(),
+        let mut source =
+            AssetSourceBuilder::new(move || Box::new(MemoryAssetReader { root: dir.clone() }))
+                .with_processed_reader(move || {
+                    Box::new(MemoryAssetReader {
+                        root: processed_dir.clone(),
+                    })
                 })
-            })
-            // Note that we only add a processed watch warning because we don't want to warn
-            // noisily about embedded watching (which is niche) when users enable file watching.
-            .with_processed_watch_warning(
-                "Consider enabling the `embedded_watcher` cargo feature.",
-            );
+                // Note that we only add a processed watch warning because we don't want to warn
+                // noisily about embedded watching (which is niche) when users enable file watching.
+                .with_processed_watch_warning(
+                    "Consider enabling the `embedded_watcher` cargo feature.",
+                );
 
         #[cfg(feature = "embedded_watcher")]
         {
@@ -262,7 +262,7 @@ pub fn _embedded_asset_path(
 }
 
 /// Creates a new `embedded` asset by embedding the bytes of the given path into the current binary
-/// and registering those bytes with the `embedded` [`AssetSource`].
+/// and registering those bytes with the `embedded` [`AssetSource`](crate::io::AssetSource).
 ///
 /// This accepts the current [`App`] as the first parameter and a path `&str` (relative to the current file) as the second.
 ///
@@ -306,7 +306,7 @@ pub fn _embedded_asset_path(
 /// ```
 ///
 /// Some things to note in the path:
-/// 1. The non-default `embedded://` [`AssetSource`]
+/// 1. The non-default `embedded://` [`AssetSource`](crate::io::AssetSource)
 /// 2. `src` is trimmed from the path
 ///
 /// The default behavior also works for cargo workspaces. Pretend the `bevy_rock` crate now exists in a larger workspace in

crates/bevy_asset/src/io/source.rs

@@ -116,10 +116,9 @@ impl<'a> PartialEq for AssetSourceId<'a> {
 
 /// Metadata about an "asset source", such as how to construct the [`AssetReader`](crate::io::AssetReader) and [`AssetWriter`](crate::io::AssetWriter) for the source,
 /// and whether or not the source is processed.
-#[derive(Default)]
 pub struct AssetSourceBuilder {
     /// The [`ErasedAssetReader`] to use on the unprocessed asset.
-    pub reader: Option<Box<dyn FnMut() -> Box<dyn ErasedAssetReader> + Send + Sync>>,
+    pub reader: Box<dyn FnMut() -> Box<dyn ErasedAssetReader> + Send + Sync>,
     /// The [`ErasedAssetWriter`] to use on the unprocessed asset.
     pub writer: Option<Box<dyn FnMut(bool) -> Option<Box<dyn ErasedAssetWriter>> + Send + Sync>>,
     /// The [`AssetWatcher`] to use for unprocessed assets, if any.
@@ -150,15 +149,31 @@ pub struct AssetSourceBuilder {
 }
 
 impl AssetSourceBuilder {
+    /// Creates a new builder, starting with the provided reader.
+    pub fn new(
+        reader: impl FnMut() -> Box<dyn ErasedAssetReader> + Send + Sync + 'static,
+    ) -> AssetSourceBuilder {
+        Self {
+            reader: Box::new(reader),
+            writer: None,
+            watcher: None,
+            processed_reader: None,
+            processed_writer: None,
+            processed_watcher: None,
+            watch_warning: None,
+            processed_watch_warning: None,
+        }
+    }
+
     /// Builds a new [`AssetSource`] with the given `id`. If `watch` is true, the unprocessed source will watch for changes.
     /// If `watch_processed` is true, the processed source will watch for changes.
     pub fn build(
         &mut self,
         id: AssetSourceId<'static>,
         watch: bool,
         watch_processed: bool,
-    ) -> Option<AssetSource> {
-        let reader = self.reader.as_mut()?();
+    ) -> AssetSource {
+        let reader = self.reader.as_mut()();
         let writer = self.writer.as_mut().and_then(|w| w(false));
         let processed_writer = self.processed_writer.as_mut().and_then(|w| w(true));
         let mut source = AssetSource {
@@ -202,15 +217,15 @@ impl AssetSourceBuilder {
                 }
             }
         }
-        Some(source)
+        source
     }
 
     /// Will use the given `reader` function to construct unprocessed [`AssetReader`](crate::io::AssetReader) instances.
     pub fn with_reader(
         mut self,
         reader: impl FnMut() -> Box<dyn ErasedAssetReader> + Send + Sync + 'static,
     ) -> Self {
-        self.reader = Some(Box::new(reader));
+        self.reader = Box::new(reader);
         self
     }
 
@@ -281,8 +296,7 @@ impl AssetSourceBuilder {
     /// For most platforms, this will use [`FileAssetReader`](crate::io::file::FileAssetReader) / [`FileAssetWriter`](crate::io::file::FileAssetWriter),
     /// but some platforms (such as Android) have their own default readers / writers / watchers.
     pub fn platform_default(path: &str, processed_path: Option<&str>) -> Self {
-        let default = Self::default()
-            .with_reader(AssetSource::get_default_reader(path.to_string()))
+        let default = Self::new(AssetSource::get_default_reader(path.to_string()))
             .with_writer(AssetSource::get_default_writer(path.to_string()))
             .with_watcher(AssetSource::get_default_watcher(
                 path.to_string(),
@@ -341,21 +355,20 @@ impl AssetSourceBuilders {
     pub fn build_sources(&mut self, watch: bool, watch_processed: bool) -> AssetSources {
         let mut sources = <HashMap<_, _>>::default();
         for (id, source) in &mut self.sources {
-            if let Some(data) = source.build(
+            let source = source.build(
                 AssetSourceId::Name(id.clone_owned()),
                 watch,
                 watch_processed,
-            ) {
-                sources.insert(id.clone_owned(), data);
-            }
+            );
+            sources.insert(id.clone_owned(), source);
         }
 
         AssetSources {
             sources,
             default: self
                 .default
                 .as_mut()
-                .and_then(|p| p.build(AssetSourceId::Default, watch, watch_processed))
+                .map(|p| p.build(AssetSourceId::Default, watch, watch_processed))
                 .expect(MISSING_DEFAULT_SOURCE),
         }
     }
@@ -382,11 +395,6 @@ pub struct AssetSource {
 }
 
 impl AssetSource {
-    /// Starts building a new [`AssetSource`].
-    pub fn build() -> AssetSourceBuilder {
-        AssetSourceBuilder::default()
-    }
-
     /// Returns this source's id.
     #[inline]
     pub fn id(&self) -> AssetSourceId<'static> {

crates/bevy_asset/src/io/web.rs

@@ -1,5 +1,7 @@
+#[cfg(any(feature = "http", feature = "https"))]
+use crate::io::AssetSourceBuilder;
+use crate::io::PathStream;
 use crate::io::{AssetReader, AssetReaderError, Reader};
-use crate::io::{AssetSource, PathStream};
 use crate::{AssetApp, AssetPlugin};
 use alloc::boxed::Box;
 use bevy_app::{App, Plugin};
@@ -71,16 +73,14 @@ impl Plugin for WebAssetPlugin {
         #[cfg(feature = "http")]
         app.register_asset_source(
             "http",
-            AssetSource::build()
-                .with_reader(move || Box::new(WebAssetReader::Http))
+            AssetSourceBuilder::new(move || Box::new(WebAssetReader::Http))
                 .with_processed_reader(move || Box::new(WebAssetReader::Http)),
         );
 
         #[cfg(feature = "https")]
         app.register_asset_source(
             "https",
-            AssetSource::build()
-                .with_reader(move || Box::new(WebAssetReader::Https))
+            AssetSourceBuilder::new(move || Box::new(WebAssetReader::Https))
                 .with_processed_reader(move || Box::new(WebAssetReader::Https)),
         );
     }

crates/bevy_asset/src/lib.rs

@@ -713,7 +713,7 @@ mod tests {
         io::{
             gated::{GateOpener, GatedReader},
             memory::{Dir, MemoryAssetReader},
-            AssetReader, AssetReaderError, AssetSource, AssetSourceEvent, AssetSourceId,
+            AssetReader, AssetReaderError, AssetSourceBuilder, AssetSourceEvent, AssetSourceId,
             AssetWatcher, Reader,
         },
         loader::{AssetLoader, LoadContext},
@@ -906,7 +906,7 @@ mod tests {
         let (gated_memory_reader, gate_opener) = GatedReader::new(MemoryAssetReader { root: dir });
         app.register_asset_source(
             AssetSourceId::Default,
-            AssetSource::build().with_reader(move || Box::new(gated_memory_reader.clone())),
+            AssetSourceBuilder::new(move || Box::new(gated_memory_reader.clone())),
         )
         .add_plugins((
             TaskPoolPlugin::default(),
@@ -1845,7 +1845,7 @@ mod tests {
         let mut app = App::new();
         app.register_asset_source(
             "unstable",
-            AssetSource::build().with_reader(move || Box::new(unstable_reader.clone())),
+            AssetSourceBuilder::new(move || Box::new(unstable_reader.clone())),
         )
         .add_plugins((TaskPoolPlugin::default(), AssetPlugin::default()))
         .init_asset::<CoolText>()
@@ -1915,8 +1915,7 @@ mod tests {
 
         app.register_asset_source(
             AssetSourceId::Default,
-            AssetSource::build()
-                .with_reader(move || Box::new(MemoryAssetReader { root: dir.clone() })),
+            AssetSourceBuilder::new(move || Box::new(MemoryAssetReader { root: dir.clone() })),
         )
         .add_plugins((TaskPoolPlugin::default(), AssetPlugin::default()));
 
@@ -2039,7 +2038,7 @@ mod tests {
         let memory_reader = MemoryAssetReader { root: dir };
         app.register_asset_source(
             AssetSourceId::Default,
-            AssetSource::build().with_reader(move || Box::new(memory_reader.clone())),
+            AssetSourceBuilder::new(move || Box::new(memory_reader.clone())),
         )
         .add_plugins((
             TaskPoolPlugin::default(),
@@ -2177,7 +2176,7 @@ mod tests {
         let reader = MemoryAssetReader { root: dir.clone() };
         app.register_asset_source(
             AssetSourceId::Default,
-            AssetSource::build().with_reader(move || Box::new(reader.clone())),
+            AssetSourceBuilder::new(move || Box::new(reader.clone())),
         )
         .add_plugins((TaskPoolPlugin::default(), AssetPlugin::default()));
 
@@ -2233,7 +2232,7 @@ mod tests {
         let reader = MemoryAssetReader { root: dir.clone() };
         app.register_asset_source(
             AssetSourceId::Default,
-            AssetSource::build().with_reader(move || Box::new(reader.clone())),
+            AssetSourceBuilder::new(move || Box::new(reader.clone())),
         )
         .add_plugins((TaskPoolPlugin::default(), AssetPlugin::default()));
 
@@ -2299,12 +2298,12 @@ mod tests {
 
         app.register_asset_source(
             AssetSourceId::Default,
-            AssetSource::build()
-                .with_reader(move || Box::new(memory_reader.clone()))
-                .with_watcher(move |sender| {
+            AssetSourceBuilder::new(move || Box::new(memory_reader.clone())).with_watcher(
+                move |sender| {
                     sender_sender.send(sender).unwrap();
                     Some(Box::new(FakeWatcher))
-                }),
+                },
+            ),
         )
         .add_plugins((
             TaskPoolPlugin::default(),
@@ -2487,8 +2486,8 @@ mod tests {
         let dir = Dir::default();
         dir.insert_asset(Path::new("test.u8"), &[]);
 
-        let asset_source = AssetSource::build()
-            .with_reader(move || Box::new(MemoryAssetReader { root: dir.clone() }));
+        let asset_source =
+            AssetSourceBuilder::new(move || Box::new(MemoryAssetReader { root: dir.clone() }));
 
         // Set up the app.
 
@@ -2551,8 +2550,8 @@ mod tests {
         let dir = Dir::default();
         dir.insert_asset(Path::new("test.txt"), &[]);
 
-        let asset_source = AssetSource::build()
-            .with_reader(move || Box::new(MemoryAssetReader { root: dir.clone() }));
+        let asset_source =
+            AssetSourceBuilder::new(move || Box::new(MemoryAssetReader { root: dir.clone() }));
 
         app.register_asset_source(AssetSourceId::Default, asset_source)
             .add_plugins((TaskPoolPlugin::default(), AssetPlugin::default()))
@@ -2603,8 +2602,8 @@ mod tests {
         let dir = Dir::default();
         dir.insert_asset(Path::new("test.txt"), &[]);
 
-        let asset_source = AssetSource::build()
-            .with_reader(move || Box::new(MemoryAssetReader { root: dir.clone() }));
+        let asset_source =
+            AssetSourceBuilder::new(move || Box::new(MemoryAssetReader { root: dir.clone() }));
 
         app.register_asset_source(AssetSourceId::Default, asset_source)
             .add_plugins((TaskPoolPlugin::default(), AssetPlugin::default()))

crates/bevy_asset/src/processor/tests.rs

@@ -25,8 +25,8 @@ use bevy_tasks::BoxedFuture;
 use crate::{
     io::{
         memory::{Dir, MemoryAssetReader, MemoryAssetWriter},
-        AssetReader, AssetReaderError, AssetSource, AssetSourceEvent, AssetSourceId, AssetWatcher,
-        PathStream, Reader,
+        AssetReader, AssetReaderError, AssetSourceBuilder, AssetSourceEvent, AssetSourceId,
+        AssetWatcher, PathStream, Reader,
     },
     processor::{
         AssetProcessor, LoadTransformAndSave, LogEntry, ProcessorState, ProcessorTransactionLog,
@@ -146,8 +146,7 @@ fn create_app_with_asset_processor(extra_sources: &[String]) -> AppWithProcessor
 
         app.register_asset_source(
             source_id,
-            AssetSource::build()
-                .with_reader(move || Box::new(source_memory_reader.clone()))
+            AssetSourceBuilder::new(move || Box::new(source_memory_reader.clone()))
                 .with_watcher(move |sender: async_channel::Sender<AssetSourceEvent>| {
                     source_event_sender_sender.send_blocking(sender).unwrap();
                     Some(Box::new(FakeWatcher))