diff --git a/crates/bevy_asset/src/io/android.rs b/crates/bevy_asset/src/io/android.rs index e57f3925b9f77..fd6a71d219894 100644 --- a/crates/bevy_asset/src/io/android.rs +++ b/crates/bevy_asset/src/io/android.rs @@ -1,7 +1,4 @@ -use crate::io::{ - get_meta_path, AssetReader, AssetReaderError, PathStream, Reader, ReaderRequiredFeatures, - VecReader, -}; +use crate::io::{get_meta_path, AssetReader, AssetReaderError, PathStream, Reader, VecReader}; use alloc::{borrow::ToOwned, boxed::Box, ffi::CString, vec::Vec}; use futures_lite::stream; use std::path::Path; @@ -19,11 +16,7 @@ use std::path::Path; pub struct AndroidAssetReader; impl AssetReader for AndroidAssetReader { - async fn read<'a>( - &'a self, - path: &'a Path, - _required_features: ReaderRequiredFeatures, - ) -> Result { + async fn read<'a>(&'a self, path: &'a Path) -> Result { let asset_manager = bevy_android::ANDROID_APP .get() .expect("Bevy must be setup with the #[bevy_main] macro on Android") diff --git a/crates/bevy_asset/src/io/file/file_asset.rs b/crates/bevy_asset/src/io/file/file_asset.rs index 6e9b54ccdf7bb..78fee97cd8fea 100644 --- a/crates/bevy_asset/src/io/file/file_asset.rs +++ b/crates/bevy_asset/src/io/file/file_asset.rs @@ -1,6 +1,6 @@ use crate::io::{ get_meta_path, AssetReader, AssetReaderError, AssetWriter, AssetWriterError, PathStream, - Reader, ReaderRequiredFeatures, Writer, + Reader, ReaderNotSeekableError, SeekableReader, Writer, }; use async_fs::{read_dir, File}; use futures_lite::StreamExt; @@ -10,14 +10,14 @@ use std::path::Path; use super::{FileAssetReader, FileAssetWriter}; -impl Reader for File {} +impl Reader for File { + fn seekable(&mut self) -> Result<&mut dyn SeekableReader, ReaderNotSeekableError> { + Ok(self) + } +} impl AssetReader for FileAssetReader { - async fn read<'a>( - &'a self, - path: &'a Path, - _required_features: ReaderRequiredFeatures, - ) -> Result { + async fn read<'a>(&'a self, path: &'a Path) -> Result { let full_path = self.root_path.join(path); File::open(&full_path).await.map_err(|e| { if e.kind() == std::io::ErrorKind::NotFound { diff --git a/crates/bevy_asset/src/io/file/sync_file_asset.rs b/crates/bevy_asset/src/io/file/sync_file_asset.rs index 28bba3d8e0b2c..cd0b0ab668f5f 100644 --- a/crates/bevy_asset/src/io/file/sync_file_asset.rs +++ b/crates/bevy_asset/src/io/file/sync_file_asset.rs @@ -3,7 +3,7 @@ use futures_lite::Stream; use crate::io::{ get_meta_path, AssetReader, AssetReaderError, AssetWriter, AssetWriterError, AsyncSeek, - PathStream, Reader, ReaderRequiredFeatures, Writer, + PathStream, Reader, ReaderNotSeekableError, SeekableReader, Writer, }; use alloc::{borrow::ToOwned, boxed::Box, vec::Vec}; @@ -48,6 +48,10 @@ impl Reader for FileReader { { stackfuture::StackFuture::from(async { self.0.read_to_end(buf) }) } + + fn seekable(&mut self) -> Result<&mut dyn SeekableReader, ReaderNotSeekableError> { + Ok(self) + } } struct FileWriter(File); @@ -95,11 +99,7 @@ impl Stream for DirReader { } impl AssetReader for FileAssetReader { - async fn read<'a>( - &'a self, - path: &'a Path, - _required_features: ReaderRequiredFeatures, - ) -> Result { + async fn read<'a>(&'a self, path: &'a Path) -> Result { let full_path = self.root_path.join(path); match File::open(&full_path) { Ok(file) => Ok(FileReader(file)), diff --git a/crates/bevy_asset/src/io/gated.rs b/crates/bevy_asset/src/io/gated.rs index 3bb0a10924956..6ce2b65b7cc2f 100644 --- a/crates/bevy_asset/src/io/gated.rs +++ b/crates/bevy_asset/src/io/gated.rs @@ -1,4 +1,4 @@ -use crate::io::{AssetReader, AssetReaderError, PathStream, Reader, ReaderRequiredFeatures}; +use crate::io::{AssetReader, AssetReaderError, PathStream, Reader}; use alloc::{boxed::Box, sync::Arc}; use async_channel::{Receiver, Sender}; use bevy_platform::{collections::HashMap, sync::RwLock}; @@ -55,11 +55,7 @@ impl GatedReader { } impl AssetReader for GatedReader { - async fn read<'a>( - &'a self, - path: &'a Path, - required_features: ReaderRequiredFeatures, - ) -> Result { + async fn read<'a>(&'a self, path: &'a Path) -> Result { let receiver = { let mut gates = self.gates.write().unwrap_or_else(PoisonError::into_inner); let gates = gates @@ -68,7 +64,7 @@ impl AssetReader for GatedReader { gates.1.clone() }; receiver.recv().await.unwrap(); - let result = self.reader.read(path, required_features).await?; + let result = self.reader.read(path).await?; Ok(result) } diff --git a/crates/bevy_asset/src/io/memory.rs b/crates/bevy_asset/src/io/memory.rs index dd0437af4c1d6..89fe32179ce8e 100644 --- a/crates/bevy_asset/src/io/memory.rs +++ b/crates/bevy_asset/src/io/memory.rs @@ -1,6 +1,6 @@ use crate::io::{ AssetReader, AssetReaderError, AssetWriter, AssetWriterError, PathStream, Reader, - ReaderRequiredFeatures, + ReaderNotSeekableError, SeekableReader, }; use alloc::{borrow::ToOwned, boxed::Box, sync::Arc, vec, vec::Vec}; use bevy_platform::{ @@ -354,14 +354,14 @@ impl Reader for DataReader { ) -> stackfuture::StackFuture<'a, std::io::Result, { super::STACK_FUTURE_SIZE }> { crate::io::read_to_end(self.data.value(), &mut self.bytes_read, buf) } + + fn seekable(&mut self) -> Result<&mut dyn SeekableReader, ReaderNotSeekableError> { + Ok(self) + } } impl AssetReader for MemoryAssetReader { - async fn read<'a>( - &'a self, - path: &'a Path, - _required_features: ReaderRequiredFeatures, - ) -> Result { + async fn read<'a>(&'a self, path: &'a Path) -> Result { self.root .get_asset(path) .map(|data| DataReader { diff --git a/crates/bevy_asset/src/io/mod.rs b/crates/bevy_asset/src/io/mod.rs index 220b0126b56c4..16e4a8cd0d03c 100644 --- a/crates/bevy_asset/src/io/mod.rs +++ b/crates/bevy_asset/src/io/mod.rs @@ -43,11 +43,6 @@ use thiserror::Error; /// Errors that occur while loading assets. #[derive(Error, Debug, Clone)] pub enum AssetReaderError { - #[error( - "A reader feature was required, but this AssetReader does not support that feature: {0}" - )] - UnsupportedFeature(#[from] UnsupportedReaderFeature), - /// Path not found. #[error("Path not found: {}", _0.display())] NotFound(PathBuf), @@ -68,9 +63,6 @@ impl PartialEq for AssetReaderError { #[inline] fn eq(&self, other: &Self) -> bool { match (self, other) { - (Self::UnsupportedFeature(feature), Self::UnsupportedFeature(other_feature)) => { - feature == other_feature - } (Self::NotFound(path), Self::NotFound(other_path)) => path == other_path, (Self::Io(error), Self::Io(other_error)) => error.kind() == other_error.kind(), (Self::HttpError(code), Self::HttpError(other_code)) => code == other_code, @@ -87,43 +79,6 @@ impl From for AssetReaderError { } } -/// An error for when a particular feature in [`ReaderRequiredFeatures`] is not supported. -#[derive(Error, Debug, Clone, PartialEq, Eq)] -pub enum UnsupportedReaderFeature { - /// The caller requested to be able to seek any way (forward, backward, from start/end), but - /// this is not supported by the [`AssetReader`]. - #[error("the reader cannot seek in any direction")] - AnySeek, -} - -/// The required features for a `Reader` that an `AssetLoader` may use. -/// -/// This allows the asset loader to communicate with the asset source what features of the reader it -/// will use. This allows the asset source to return an error early (if a feature is unsupported), -/// or use a different reader implementation based on the required features to optimize reading -/// (e.g., using a simpler reader implementation if some features are not required). -/// -/// These features **only** apply to the asset itself, and not any nested loads - those loaders will -/// request their own required features. -#[derive(Clone, Copy, Default)] -pub struct ReaderRequiredFeatures { - /// The kind of seek that the reader needs to support. - pub seek: SeekKind, -} - -/// The kind of seeking that the reader supports. -#[derive(Clone, Copy, Default)] -pub enum SeekKind { - /// The reader can only seek forward. - /// - /// Seeking forward is always required, since at the bare minimum, the reader could choose to - /// just read that many bytes and then drop them (effectively seeking forward). - #[default] - OnlyForward, - /// The reader can seek forward, backward, seek from the start, and seek from the end. - AnySeek, -} - /// The maximum size of a future returned from [`Reader::read_to_end`]. /// This is large enough to fit ten references. // Ideally this would be even smaller (ReadToEndFuture only needs space for two references based on its definition), @@ -139,22 +94,7 @@ pub use stackfuture::StackFuture; /// This is essentially a trait alias for types implementing [`AsyncRead`] and [`AsyncSeek`]. /// The only reason a blanket implementation is not provided for applicable types is to allow /// implementors to override the provided implementation of [`Reader::read_to_end`]. -/// -/// # Reader features -/// -/// This trait includes super traits. However, this **does not** mean that your type needs to -/// support every feature of those super traits. If the caller never uses that feature, then a dummy -/// implementation that just returns an error is sufficient. -/// -/// The caller can request a compatible [`Reader`] using [`ReaderRequiredFeatures`] (when using the -/// [`AssetReader`] trait). This allows the caller to state which features of the reader it will -/// use, avoiding cases where the caller uses a feature that the reader does not support. -/// -/// For example, the caller may set [`ReaderRequiredFeatures::seek`] to -/// [`SeekKind::AnySeek`] to indicate that they may seek backward, or from the start/end. A reader -/// implementation may choose to support that, or may just detect those kinds of seeks and return an -/// error. -pub trait Reader: AsyncRead + AsyncSeek + Unpin + Send + Sync { +pub trait Reader: AsyncRead + Unpin + Send + Sync { /// Reads the entire contents of this reader and appends them to a vec. /// /// # Note for implementors @@ -168,8 +108,47 @@ pub trait Reader: AsyncRead + AsyncSeek + Unpin + Send + Sync { let future = futures_lite::AsyncReadExt::read_to_end(self, buf); StackFuture::from(future) } + + /// Casts this [`Reader`] as a [`SeekableReader`], which layers on [`AsyncSeek`] functionality. + /// Returns [`Ok`] if this [`Reader`] supports seeking. Otherwise returns [`Err`]. + /// + /// Implementers of [`Reader`] are highly encouraged to provide this functionality, as it makes the + /// reader compatible with "seeking" [`AssetLoader`](crate::AssetLoader) implementations. + /// + /// [`AssetLoader`](crate::AssetLoader) implementations that call this are encouraged to provide fallback behavior + /// when it fails, such as reading into a seek-able [`Vec`] (or [`AsyncSeek`]-able [`VecReader`]): + /// + /// ``` + /// # use bevy_asset::{io::{VecReader, Reader}, AsyncSeekExt}; + /// # use std::{io::SeekFrom, vec::Vec}; + /// # async { + /// # let mut original_reader = VecReader::new(Vec::new()); + /// # let reader: &mut dyn Reader = &mut original_reader; + /// let mut fallback_reader; + /// let reader = match reader.seekable() { + /// Ok(seek) => seek, + /// Err(_) => { + /// fallback_reader = VecReader::new(Vec::new()); + /// reader.read_to_end(&mut fallback_reader.bytes).await.unwrap(); + /// &mut fallback_reader + /// } + /// }; + /// reader.seek(SeekFrom::Start(10)).await.unwrap(); + /// # }; + /// ``` + fn seekable(&mut self) -> Result<&mut dyn SeekableReader, ReaderNotSeekableError>; } +pub trait SeekableReader: Reader + AsyncSeek {} + +impl SeekableReader for T {} + +#[derive(Error, Debug, Copy, Clone)] +#[error( + "The `Reader` returned by the current `AssetReader` does not support `AsyncSeek` behavior." +)] +pub struct ReaderNotSeekableError; + impl Reader for Box { fn read_to_end<'a>( &'a mut self, @@ -177,6 +156,10 @@ impl Reader for Box { ) -> StackFuture<'a, std::io::Result, STACK_FUTURE_SIZE> { (**self).read_to_end(buf) } + + fn seekable(&mut self) -> Result<&mut dyn SeekableReader, ReaderNotSeekableError> { + (**self).seekable() + } } /// A future that returns a value or an [`AssetReaderError`] @@ -204,37 +187,15 @@ where pub trait AssetReader: Send + Sync + 'static { /// Returns a future to load the full file data at the provided path. /// - /// # Required Features - /// - /// The `required_features` allows the caller to request that the returned reader implements - /// certain features, and consequently allows this trait to decide how to react to that request. - /// Namely, the implementor could: - /// - /// * Return an error if the caller requests an unsupported feature. This can give a nicer error - /// message to make it clear that the caller (e.g., an asset loader) can't be used with this - /// reader. - /// * Use a different implementation of a reader to ensure support of a feature (e.g., reading - /// the entire asset into memory and then providing that buffer as a reader). - /// * Ignore the request and provide the regular reader anyway. Practically, if the caller never - /// actually uses the feature, it's fine to continue using the reader. However the caller - /// requesting a feature is a **strong signal** that they will use the given feature. - /// - /// The recommendation is to simply return an error for unsupported features. Callers can - /// generally work around this and have more understanding of their constraints. For example, - /// an asset loader may know that it will only load "small" assets, so reading the entire asset - /// into memory won't consume too much memory, and so it can use the regular [`AsyncRead`] API - /// to read the whole asset into memory. If this were done by this trait, the loader may - /// accidentally be allocating too much memory for a large asset without knowing it! - /// /// # Note for implementors /// The preferred style for implementing this method is an `async fn` returning an opaque type. /// /// ```no_run /// # use std::path::Path; - /// # use bevy_asset::{prelude::*, io::{AssetReader, PathStream, Reader, AssetReaderError, ReaderRequiredFeatures}}; + /// # use bevy_asset::{prelude::*, io::{AssetReader, PathStream, Reader, AssetReaderError}}; /// # struct MyReader; /// impl AssetReader for MyReader { - /// async fn read<'a>(&'a self, path: &'a Path, required_features: ReaderRequiredFeatures) -> Result { + /// async fn read<'a>(&'a self, path: &'a Path) -> Result { /// // ... /// # let val: Box = unimplemented!(); Ok(val) /// } @@ -245,11 +206,7 @@ pub trait AssetReader: Send + Sync + 'static { /// # async fn read_meta_bytes<'a>(&'a self, path: &'a Path) -> Result, AssetReaderError> { unimplemented!() } /// } /// ``` - fn read<'a>( - &'a self, - path: &'a Path, - required_features: ReaderRequiredFeatures, - ) -> impl AssetReaderFuture; + fn read<'a>(&'a self, path: &'a Path) -> impl AssetReaderFuture; /// Returns a future to load the full file data at the provided path. fn read_meta<'a>(&'a self, path: &'a Path) -> impl AssetReaderFuture; /// Returns an iterator of directory entry names at the provided path. @@ -284,7 +241,6 @@ pub trait ErasedAssetReader: Send + Sync + 'static { fn read<'a>( &'a self, path: &'a Path, - required_features: ReaderRequiredFeatures, ) -> BoxedFuture<'a, Result, AssetReaderError>>; /// Returns a future to load the full file data at the provided path. fn read_meta<'a>( @@ -313,10 +269,9 @@ impl ErasedAssetReader for T { fn read<'a>( &'a self, path: &'a Path, - required_features: ReaderRequiredFeatures, ) -> BoxedFuture<'a, Result, AssetReaderError>> { Box::pin(async move { - let reader = Self::read(self, path, required_features).await?; + let reader = Self::read(self, path).await?; Ok(Box::new(reader) as Box) }) } @@ -640,7 +595,7 @@ pub trait AssetWatcher: Send + Sync + 'static {} /// An [`AsyncRead`] implementation capable of reading a [`Vec`]. pub struct VecReader { - bytes: Vec, + pub bytes: Vec, bytes_read: usize, } @@ -685,6 +640,10 @@ impl Reader for VecReader { ) -> StackFuture<'a, std::io::Result, STACK_FUTURE_SIZE> { read_to_end(&self.bytes, &mut self.bytes_read, buf) } + + fn seekable(&mut self) -> Result<&mut dyn SeekableReader, ReaderNotSeekableError> { + Ok(self) + } } /// An [`AsyncRead`] implementation capable of reading a [`&[u8]`]. @@ -730,6 +689,10 @@ impl Reader for SliceReader<'_> { ) -> StackFuture<'a, std::io::Result, STACK_FUTURE_SIZE> { read_to_end(self.bytes, &mut self.bytes_read, buf) } + + fn seekable(&mut self) -> Result<&mut dyn SeekableReader, ReaderNotSeekableError> { + Ok(self) + } } /// Performs a read from the `slice` into `buf`. diff --git a/crates/bevy_asset/src/io/processor_gated.rs b/crates/bevy_asset/src/io/processor_gated.rs index 47b11b34fdbb6..05e110097b4e7 100644 --- a/crates/bevy_asset/src/io/processor_gated.rs +++ b/crates/bevy_asset/src/io/processor_gated.rs @@ -1,6 +1,7 @@ use crate::{ io::{ - AssetReader, AssetReaderError, AssetSourceId, PathStream, Reader, ReaderRequiredFeatures, + AssetReader, AssetReaderError, AssetSourceId, PathStream, Reader, ReaderNotSeekableError, + SeekableReader, }, processor::{ProcessStatus, ProcessingState}, AssetPath, @@ -9,10 +10,10 @@ use alloc::{borrow::ToOwned, boxed::Box, sync::Arc, vec::Vec}; use async_lock::RwLockReadGuardArc; use core::{pin::Pin, task::Poll}; use futures_io::AsyncRead; -use std::{io::SeekFrom, path::Path}; +use std::path::Path; use tracing::trace; -use super::{AsyncSeek, ErasedAssetReader}; +use super::ErasedAssetReader; /// An [`AssetReader`] that will prevent asset (and asset metadata) read futures from returning for a /// given path until that path has been processed by [`AssetProcessor`]. @@ -40,11 +41,7 @@ impl ProcessorGatedReader { } impl AssetReader for ProcessorGatedReader { - async fn read<'a>( - &'a self, - path: &'a Path, - required_features: ReaderRequiredFeatures, - ) -> Result { + async fn read<'a>(&'a self, path: &'a Path) -> Result { let asset_path = AssetPath::from(path.to_path_buf()).with_source(self.source.clone()); trace!("Waiting for processing to finish before reading {asset_path}"); let process_result = self @@ -62,7 +59,7 @@ impl AssetReader for ProcessorGatedReader { .processing_state .get_transaction_lock(&asset_path) .await?; - let asset_reader = self.reader.read(path, required_features).await?; + let asset_reader = self.reader.read(path).await?; let reader = TransactionLockedReader::new(asset_reader, lock); Ok(reader) } @@ -141,16 +138,6 @@ impl AsyncRead for TransactionLockedReader<'_> { } } -impl AsyncSeek for TransactionLockedReader<'_> { - fn poll_seek( - mut self: Pin<&mut Self>, - cx: &mut core::task::Context<'_>, - pos: SeekFrom, - ) -> Poll> { - Pin::new(&mut self.reader).poll_seek(cx, pos) - } -} - impl Reader for TransactionLockedReader<'_> { fn read_to_end<'a>( &'a mut self, @@ -158,4 +145,8 @@ impl Reader for TransactionLockedReader<'_> { ) -> stackfuture::StackFuture<'a, std::io::Result, { super::STACK_FUTURE_SIZE }> { self.reader.read_to_end(buf) } + + fn seekable(&mut self) -> Result<&mut dyn SeekableReader, ReaderNotSeekableError> { + self.reader.seekable() + } } diff --git a/crates/bevy_asset/src/io/wasm.rs b/crates/bevy_asset/src/io/wasm.rs index cc62017d67058..cd8f66bdf415e 100644 --- a/crates/bevy_asset/src/io/wasm.rs +++ b/crates/bevy_asset/src/io/wasm.rs @@ -1,6 +1,5 @@ use crate::io::{ - get_meta_path, AssetReader, AssetReaderError, EmptyPathStream, PathStream, Reader, - ReaderRequiredFeatures, VecReader, + get_meta_path, AssetReader, AssetReaderError, EmptyPathStream, PathStream, Reader, VecReader, }; use alloc::{borrow::ToOwned, boxed::Box, format}; use js_sys::{Uint8Array, JSON}; @@ -93,11 +92,7 @@ impl HttpWasmAssetReader { } impl AssetReader for HttpWasmAssetReader { - async fn read<'a>( - &'a self, - path: &'a Path, - _required_features: ReaderRequiredFeatures, - ) -> Result { + async fn read<'a>(&'a self, path: &'a Path) -> Result { let path = self.root_path.join(path); self.fetch_bytes(path).await } diff --git a/crates/bevy_asset/src/io/web.rs b/crates/bevy_asset/src/io/web.rs index 1f5aa0da9fdb9..63f3dced90065 100644 --- a/crates/bevy_asset/src/io/web.rs +++ b/crates/bevy_asset/src/io/web.rs @@ -1,6 +1,4 @@ -use crate::io::{ - AssetReader, AssetReaderError, AssetSourceBuilder, PathStream, Reader, ReaderRequiredFeatures, -}; +use crate::io::{AssetReader, AssetReaderError, AssetSourceBuilder, PathStream, Reader}; use crate::{AssetApp, AssetPlugin}; use alloc::boxed::Box; use bevy_app::{App, Plugin}; @@ -193,7 +191,6 @@ impl AssetReader for WebAssetReader { fn read<'a>( &'a self, path: &'a Path, - _required_features: ReaderRequiredFeatures, ) -> impl ConditionalSendFuture, AssetReaderError>> { get(self.make_uri(path)) } diff --git a/crates/bevy_asset/src/lib.rs b/crates/bevy_asset/src/lib.rs index 1c1a5a72dcc2e..01df322f4a958 100644 --- a/crates/bevy_asset/src/lib.rs +++ b/crates/bevy_asset/src/lib.rs @@ -192,7 +192,7 @@ use bevy_diagnostic::{Diagnostic, DiagnosticsStore, RegisterDiagnostic}; pub use direct_access_ext::DirectAssetAccessExt; pub use event::*; pub use folder::*; -pub use futures_lite::{AsyncReadExt, AsyncWriteExt}; +pub use futures_lite::{AsyncReadExt, AsyncSeekExt, AsyncWriteExt}; pub use handle::*; pub use id::*; pub use loader::*; @@ -712,7 +712,7 @@ mod tests { gated::{GateOpener, GatedReader}, memory::{Dir, MemoryAssetReader}, AssetReader, AssetReaderError, AssetSourceBuilder, AssetSourceEvent, AssetSourceId, - AssetWatcher, Reader, ReaderRequiredFeatures, + AssetWatcher, Reader, }, loader::{AssetLoader, LoadContext}, Asset, AssetApp, AssetEvent, AssetId, AssetLoadError, AssetLoadFailedEvent, AssetPath, @@ -868,11 +868,7 @@ mod tests { ) -> Result { self.memory_reader.read_meta(path).await } - async fn read<'a>( - &'a self, - path: &'a Path, - required_features: ReaderRequiredFeatures, - ) -> Result { + async fn read<'a>(&'a self, path: &'a Path) -> Result { let attempt_number = { let mut attempt_counters = self.attempt_counters.lock().unwrap(); if let Some(existing) = attempt_counters.get_mut(path) { @@ -900,7 +896,7 @@ mod tests { .await; } - self.memory_reader.read(path, required_features).await + self.memory_reader.read(path).await } } diff --git a/crates/bevy_asset/src/loader.rs b/crates/bevy_asset/src/loader.rs index 0d5841e9af1d8..53d779ec29344 100644 --- a/crates/bevy_asset/src/loader.rs +++ b/crates/bevy_asset/src/loader.rs @@ -1,8 +1,5 @@ use crate::{ - io::{ - AssetReaderError, MissingAssetSourceError, MissingProcessedAssetReaderError, Reader, - ReaderRequiredFeatures, - }, + io::{AssetReaderError, MissingAssetSourceError, MissingProcessedAssetReaderError, Reader}, loader_builders::{Deferred, NestedLoader, StaticTyped}, meta::{AssetHash, AssetMeta, AssetMetaDyn, ProcessedInfo, ProcessedInfoMinimal, Settings}, path::AssetPath, @@ -47,11 +44,6 @@ pub trait AssetLoader: TypePath + Send + Sync + 'static { load_context: &mut LoadContext, ) -> impl ConditionalSendFuture>; - /// Returns the required features of the reader for this loader. - fn reader_required_features(_settings: &Self::Settings) -> ReaderRequiredFeatures { - ReaderRequiredFeatures::default() - } - /// Returns a list of extensions supported by this [`AssetLoader`], without the preceding dot. /// Note that users of this [`AssetLoader`] may choose to load files with a non-matching extension. fn extensions(&self) -> &[&str] { @@ -69,9 +61,6 @@ pub trait ErasedAssetLoader: Send + Sync + 'static { load_context: LoadContext<'a>, ) -> BoxedFuture<'a, Result>; - /// Returns the required features of the reader for this loader. - // Note: This takes &self just to be dyn compatible. - fn reader_required_features(&self, settings: &dyn Settings) -> ReaderRequiredFeatures; /// Returns a list of extensions supported by this asset loader, without the preceding dot. fn extensions(&self) -> &[&str]; /// Deserializes metadata from the input `meta` bytes into the appropriate type (erased as [`Box`]). @@ -110,13 +99,6 @@ where }) } - fn reader_required_features(&self, settings: &dyn Settings) -> ReaderRequiredFeatures { - let settings = settings - .downcast_ref::() - .expect("AssetLoader settings should match the loader type"); - ::reader_required_features(settings) - } - fn extensions(&self) -> &[&str] { ::extensions(self) } @@ -502,9 +484,7 @@ impl<'a> LoadContext<'a> { AssetServerMode::Unprocessed => source.reader(), AssetServerMode::Processed => source.processed_reader()?, }; - let mut reader = asset_reader - .read(path.path(), ReaderRequiredFeatures::default()) - .await?; + let mut reader = asset_reader.read(path.path()).await?; let hash = if self.populate_hashes { // NOTE: ensure meta is read while the asset bytes reader is still active to ensure transactionality // See `ProcessorGatedReader` for more info diff --git a/crates/bevy_asset/src/meta.rs b/crates/bevy_asset/src/meta.rs index bf51a3391d8a9..8870dabe380a5 100644 --- a/crates/bevy_asset/src/meta.rs +++ b/crates/bevy_asset/src/meta.rs @@ -223,10 +223,6 @@ impl AssetLoader for () { unreachable!(); } - fn reader_required_features(_settings: &Self::Settings) -> crate::io::ReaderRequiredFeatures { - unreachable!(); - } - fn extensions(&self) -> &[&str] { unreachable!(); } diff --git a/crates/bevy_asset/src/processor/mod.rs b/crates/bevy_asset/src/processor/mod.rs index 565b234aeddb7..7a07c24ab8a2c 100644 --- a/crates/bevy_asset/src/processor/mod.rs +++ b/crates/bevy_asset/src/processor/mod.rs @@ -48,7 +48,6 @@ use crate::{ io::{ AssetReaderError, AssetSource, AssetSourceBuilders, AssetSourceEvent, AssetSourceId, AssetSources, AssetWriterError, ErasedAssetReader, MissingAssetSourceError, - ReaderRequiredFeatures, }, meta::{ get_asset_hash, get_full_asset_hash, AssetAction, AssetActionMinimal, AssetHash, AssetMeta, @@ -458,7 +457,6 @@ impl AssetProcessor { let reader = source.reader(); match reader.read_meta_bytes(path.path()).await { Ok(_) => return Err(WriteDefaultMetaError::MetaAlreadyExists), - Err(AssetReaderError::UnsupportedFeature(feature)) => panic!("reading the meta file never requests a feature, but the following feature is unsupported: {feature}"), Err(AssetReaderError::NotFound(_)) => { // The meta file couldn't be found so just fall through. } @@ -559,10 +557,6 @@ impl AssetProcessor { } Err(err) => { match err { - // There is never a reason for a path check to return an - // `UnsupportedFeature` error. This must be an incorrectly programmed - // `AssetReader`, so just panic to make this clearly unsupported. - AssetReaderError::UnsupportedFeature(feature) => panic!("checking whether a path is a file or folder resulted in unsupported feature: {feature}"), AssetReaderError::NotFound(_) => { // if the path is not found, a processed version does not exist } @@ -634,12 +628,6 @@ impl AssetProcessor { } } Err(err) => match err { - // There is never a reason for a directory read to return an `UnsupportedFeature` - // error. This must be an incorrectly programmed `AssetReader`, so just panic to - // make this clearly unsupported. - AssetReaderError::UnsupportedFeature(feature) => { - panic!("reading a directory resulted in unsupported feature: {feature}") - } AssetReaderError::NotFound(_err) => { // The processed folder does not exist. No need to update anything } @@ -1110,10 +1098,7 @@ impl AssetProcessor { let new_hash = { // Create a reader just for computing the hash. Keep this scoped here so that we drop it // as soon as the hash is computed. - let mut reader_for_hash = reader - .read(path, ReaderRequiredFeatures::default()) - .await - .map_err(reader_err)?; + let mut reader_for_hash = reader.read(path).await.map_err(reader_err)?; get_asset_hash(&meta_bytes, &mut reader_for_hash) .await @@ -1172,7 +1157,6 @@ impl AssetProcessor { // `AssetAction::Process` (which includes its settings). let settings = source_meta.process_settings().unwrap(); - let reader_features = processor.reader_required_features(settings)?; // Create a reader just for the actual process. Note: this means that we're performing // two reads for the same file (but we avoid having to load the whole file into memory). // For some sources (like local file systems), this is not a big deal, but for other @@ -1182,10 +1166,7 @@ impl AssetProcessor { // it's not likely to be too big a deal. If in the future, we decide we want to avoid // this repeated read, we could "ask" the asset source if it prefers avoiding repeated // reads or not. - let reader_for_process = reader - .read(path, reader_features) - .await - .map_err(reader_err)?; + let reader_for_process = reader.read(path).await.map_err(reader_err)?; let mut writer = processed_writer.write(path).await.map_err(writer_err)?; let mut processed_meta = { @@ -1225,10 +1206,7 @@ impl AssetProcessor { .map_err(writer_err)?; } else { // See the reasoning for processing why it's ok to do a second read here. - let mut reader_for_copy = reader - .read(path, ReaderRequiredFeatures::default()) - .await - .map_err(reader_err)?; + let mut reader_for_copy = reader.read(path).await.map_err(reader_err)?; let mut writer = processed_writer.write(path).await.map_err(writer_err)?; futures_lite::io::copy(&mut reader_for_copy, &mut writer) .await diff --git a/crates/bevy_asset/src/processor/process.rs b/crates/bevy_asset/src/processor/process.rs index d4398af78c540..bd32b31ad5ae9 100644 --- a/crates/bevy_asset/src/processor/process.rs +++ b/crates/bevy_asset/src/processor/process.rs @@ -1,8 +1,7 @@ use crate::{ io::{ AssetReaderError, AssetWriterError, MissingAssetWriterError, - MissingProcessedAssetReaderError, MissingProcessedAssetWriterError, Reader, - ReaderRequiredFeatures, Writer, + MissingProcessedAssetReaderError, MissingProcessedAssetWriterError, Reader, Writer, }, meta::{AssetAction, AssetMeta, AssetMetaDyn, ProcessDependencyInfo, ProcessedInfo, Settings}, processor::AssetProcessor, @@ -43,11 +42,6 @@ pub trait Process: TypePath + Send + Sync + Sized + 'static { ) -> impl ConditionalSendFuture< Output = Result<::Settings, ProcessError>, >; - - /// Gets the features of the reader required to process the asset. - fn reader_required_features(_settings: &Self::Settings) -> ReaderRequiredFeatures { - ReaderRequiredFeatures::default() - } } /// A flexible [`Process`] implementation that loads the source [`Asset`] using the `L` [`AssetLoader`], then transforms @@ -213,10 +207,6 @@ where .map_err(|error| ProcessError::AssetSaveError(error.into()))?; Ok(output_settings) } - - fn reader_required_features(settings: &Self::Settings) -> ReaderRequiredFeatures { - Loader::reader_required_features(&settings.loader_settings) - } } /// A type-erased variant of [`Process`] that enables interacting with processor implementations without knowing @@ -229,16 +219,6 @@ pub trait ErasedProcessor: Send + Sync { settings: &'a dyn Settings, writer: &'a mut Writer, ) -> BoxedFuture<'a, Result, ProcessError>>; - /// Type-erased variant of [`Process::reader_required_features`]. - // Note: This takes &self just to be dyn compatible. - #[expect( - clippy::result_large_err, - reason = "this is only an error here because this isn't a future" - )] - fn reader_required_features( - &self, - settings: &dyn Settings, - ) -> Result; /// Deserialized `meta` as type-erased [`AssetMeta`], operating under the assumption that it matches the meta /// for the underlying [`Process`] impl. fn deserialize_meta(&self, meta: &[u8]) -> Result, DeserializeMetaError>; @@ -265,14 +245,6 @@ impl ErasedProcessor for P { }) } - fn reader_required_features( - &self, - settings: &dyn Settings, - ) -> Result { - let settings = settings.downcast_ref().ok_or(ProcessError::WrongMetaType)?; - Ok(P::reader_required_features(settings)) - } - fn deserialize_meta(&self, meta: &[u8]) -> Result, DeserializeMetaError> { let meta: AssetMeta<(), P> = ron::de::from_bytes(meta)?; Ok(Box::new(meta)) diff --git a/crates/bevy_asset/src/processor/tests.rs b/crates/bevy_asset/src/processor/tests.rs index 3bf573613fc96..614068867626c 100644 --- a/crates/bevy_asset/src/processor/tests.rs +++ b/crates/bevy_asset/src/processor/tests.rs @@ -26,7 +26,7 @@ use crate::{ io::{ memory::{Dir, MemoryAssetReader, MemoryAssetWriter}, AssetReader, AssetReaderError, AssetSourceBuilder, AssetSourceBuilders, AssetSourceEvent, - AssetSourceId, AssetWatcher, PathStream, Reader, ReaderRequiredFeatures, + AssetSourceId, AssetWatcher, PathStream, Reader, }, processor::{ AssetProcessor, GetProcessorError, LoadTransformAndSave, LogEntry, Process, ProcessContext, @@ -195,13 +195,9 @@ impl LockGatedReader { } impl AssetReader for LockGatedReader { - async fn read<'a>( - &'a self, - path: &'a Path, - required_features: ReaderRequiredFeatures, - ) -> Result { + async fn read<'a>(&'a self, path: &'a Path) -> Result { let _guard = self.gate.read().await; - self.reader.read(path, required_features).await + self.reader.read(path).await } async fn read_meta<'a>(&'a self, path: &'a Path) -> Result { diff --git a/crates/bevy_asset/src/server/loaders.rs b/crates/bevy_asset/src/server/loaders.rs index 6e0aeb1c58a11..11e9d83c5af0d 100644 --- a/crates/bevy_asset/src/server/loaders.rs +++ b/crates/bevy_asset/src/server/loaders.rs @@ -15,7 +15,6 @@ use tracing::warn; #[cfg(feature = "trace")] use { - crate::io::ReaderRequiredFeatures, alloc::string::ToString, bevy_tasks::ConditionalSendFuture, tracing::{info_span, instrument::Instrument}, @@ -338,10 +337,6 @@ impl AssetLoader for InstrumentedAssetLoader { self.0.load(reader, settings, load_context).instrument(span) } - fn reader_required_features(settings: &Self::Settings) -> ReaderRequiredFeatures { - T::reader_required_features(settings) - } - fn extensions(&self) -> &[&str] { self.0.extensions() } diff --git a/crates/bevy_asset/src/server/mod.rs b/crates/bevy_asset/src/server/mod.rs index 293291f5860dc..9e175fb71f4c4 100644 --- a/crates/bevy_asset/src/server/mod.rs +++ b/crates/bevy_asset/src/server/mod.rs @@ -1522,11 +1522,7 @@ impl AssetServer { let meta = loader.default_meta(); (meta, loader) }; - let required_features = - loader.reader_required_features(meta.loader_settings().expect("meta specifies load")); - let reader = asset_reader - .read(asset_path.path(), required_features) - .await?; + let reader = asset_reader.read(asset_path.path()).await?; Ok((meta, loader, reader)) } @@ -1713,7 +1709,6 @@ impl AssetServer { let reader = source.reader(); match reader.read_meta_bytes(path.path()).await { Ok(_) => return Err(WriteDefaultMetaError::MetaAlreadyExists), - Err(AssetReaderError::UnsupportedFeature(feature)) => panic!("reading the meta file never requests a feature, but the following feature is unsupported: {feature}"), Err(AssetReaderError::NotFound(_)) => { // The meta file couldn't be found so just fall through. } diff --git a/examples/asset/custom_asset_reader.rs b/examples/asset/custom_asset_reader.rs index a2422bac3329c..d95abb6a6b650 100644 --- a/examples/asset/custom_asset_reader.rs +++ b/examples/asset/custom_asset_reader.rs @@ -5,7 +5,7 @@ use bevy::{ asset::io::{ AssetReader, AssetReaderError, AssetSource, AssetSourceBuilder, AssetSourceId, - ErasedAssetReader, PathStream, Reader, ReaderRequiredFeatures, + ErasedAssetReader, PathStream, Reader, }, prelude::*, }; @@ -15,13 +15,9 @@ use std::path::Path; struct CustomAssetReader(Box); impl AssetReader for CustomAssetReader { - async fn read<'a>( - &'a self, - path: &'a Path, - required_features: ReaderRequiredFeatures, - ) -> Result { + async fn read<'a>(&'a self, path: &'a Path) -> Result { info!("Reading {}", path.display()); - self.0.read(path, required_features).await + self.0.read(path).await } async fn read_meta<'a>(&'a self, path: &'a Path) -> Result { self.0.read_meta(path).await diff --git a/release-content/migration-guides/reader_required_features.md b/release-content/migration-guides/reader_required_features.md deleted file mode 100644 index c9fda72f2986c..0000000000000 --- a/release-content/migration-guides/reader_required_features.md +++ /dev/null @@ -1,40 +0,0 @@ ---- -title: The `AssetReader` trait now takes a `ReaderRequiredFeatures` argument. -pull_requests: [] ---- - -The `AssetReader::read` method now takes an additional `ReaderRequiredFeatures` argument. If -previously you had: - -```rust -struct MyAssetReader; - -impl AssetReader for MyAssetReader { - async fn read<'a>( - &'a self, - path: &'a Path, - ) -> Result { - todo!() - } - - // more stuff... -} -``` - -Change this to: - -```rust -struct MyAssetReader; - -impl AssetReader for MyAssetReader { - async fn read<'a>( - &'a self, - path: &'a Path, - _required_features: ReaderRequiredFeatures, - ) -> Result { - todo!() - } - - // more stuff... -} -``` diff --git a/release-content/migration-guides/readers_impl_async_seek.md b/release-content/migration-guides/readers_impl_async_seek.md deleted file mode 100644 index d2957efc6a220..0000000000000 --- a/release-content/migration-guides/readers_impl_async_seek.md +++ /dev/null @@ -1,53 +0,0 @@ ---- -title: Implementations of `Reader` now must implement `AsyncSeek`, and `AsyncSeekForward` is deleted. -pull_requests: [] ---- - -The `Reader` trait no longer requires implementing `AsyncSeekForward` and instead requires -implementing `AsyncSeek`. Each reader will have its own unique implementation so implementing this -will be case specific. The simplest implementation is to simply reject these seeking cases like so: - -```rust -impl AsyncSeek for MyReader { - fn poll_seek( - self: Pin<&mut Self>, - _cx: &mut core::task::Context<'_>, - pos: SeekFrom, - ) -> Poll> { - let forward = match pos { - SeekFrom::Current(curr) if curr >= 0 => curr as u64, - _ => return std::io::Error::new( - std::io::ErrorKind::InvalidInput, - "invalid seek mode", - ), - }; - - // Do whatever your previous `AsyncSeekForward` implementation did... - } -} -``` - -In addition, the `AssetReader` trait now includes a `ReaderRequiredFeatures` argument which can be -used to return an error early for invalid requests. For example: - -```rust -impl AssetReader for MyAssetReader { - async fn read<'a>( - &'a self, - path: &'a Path, - required_features: ReaderRequiredFeatures, - ) -> Result { - match required_features.seek { - SeekKind::Forward => {} - SeekKind::AnySeek => return Err(UnsupportedReaderFeature::AnySeek), - } - - // Do whatever your previous `AssetReader` implementation did, like... - Ok(MyReader) - } -} -``` - -Since we now just use the `AsyncSeek` trait, we've deleted the `AsyncSeekForward` trait. Users of -this trait can migrate by calling the `AsyncSeek::poll_seek` method with -`SeekFrom::Current(offset)`, or the `AsyncSeekExt::seek` method. diff --git a/release-content/migration-guides/seekable_readers.md b/release-content/migration-guides/seekable_readers.md new file mode 100644 index 0000000000000..80a38027c93f2 --- /dev/null +++ b/release-content/migration-guides/seekable_readers.md @@ -0,0 +1,28 @@ +--- +title: Implementations of `Reader` now must implement `Reader::seekable`, and `AsyncSeekForward` is deleted. +pull_requests: [22182] +--- + +The `Reader` trait no longer requires implementing `AsyncSeekForward` and instead requires +implementing `Reader::seekable`, which will cast the `Reader` to `&mut dyn SeekableReader` if it +supports `AsyncSeek` (`SeekableReader: Reader + AsyncSeek`). + +```rust +// If MyReader implements `AsyncSeek` +impl Reader for MyReader { + fn seekable(&mut self) -> Result<&mut dyn SeekableReader, ReaderNotSeekableError> { + Ok(self) + } +} + +// If MyReader does not implement `AsyncSeek` +impl Reader for MyReader { + fn seekable(&mut self) -> Result<&mut dyn SeekableReader, ReaderNotSeekableError> { + None + } +} +``` + +Since we now just use the `AsyncSeek` trait, we've deleted the `AsyncSeekForward` trait. Users of +this trait can migrate by calling the `AsyncSeek::poll_seek` method with +`SeekFrom::Current(offset)`, or the `AsyncSeekExt::seek` method. diff --git a/release-content/release-notes/optional_asset_reader_seek.md b/release-content/release-notes/optional_asset_reader_seek.md index cd92446174289..e0d77ea32d7bb 100644 --- a/release-content/release-notes/optional_asset_reader_seek.md +++ b/release-content/release-notes/optional_asset_reader_seek.md @@ -1,7 +1,7 @@ --- title: The `AssetReader` trait can now (optionally) support seeking any direction. -authors: ["@andriyDev"] -pull_requests: [] +authors: ["@andriyDev", "@cart"] +pull_requests: [22182] --- In Bevy 0.15, we replaced the `AsyncSeek` super trait on `Reader` with `AsyncSeekForward`. This @@ -9,17 +9,11 @@ allowed our `Reader` trait to apply to more cases (e.g., it could allow cases li which may not support seeking backwards). However, it also meant that we could no longer use seeking fully where it was available. -To resolve this issue, we now allow `AssetLoader`s to provide a `ReaderRequiredFeatures` to the -`AssetReader`. The `AssetReader` can then choose how to handle those required features. For example, -it can return an error to indicate that the feature is not supported, or it can choose to use a -different `Reader` implementation to fallback in order to continue to support the feature. +To resolve this issue, we've added support for the `Reader` passed into `AssetLoader` to try casting into `SeekableReader`: -This allowed us to bring back the "requirement" the `Reader: AsyncSeek`, but with a more relaxed -policy: the `Reader` may choose to avoid supporting certain features (corresponding to fields in -`ReaderRequiredFeatures`). +```rust +let seekable_reader = reader.seekable()?; +seekable_reader.seek(SeekFrom::Start(10)).await?; +``` -Our general recommendation is that if your `Reader` implementation does not support a feature, make -your `AssetReader` just return an error for that feature. Usually, an `AssetLoader` can implement a -fallback itself (e.g., reading all the data into memory and then loading from that), and loaders can -be selected using `.meta` files (allowing for fine-grained opt-in in these cases). However if there -is some reasonable implementation you can provide (even if not optimal), feel free to provide one! +`Reader` implementations that support seeking (such as the filesystem `AssetReader`) will cast successfully. If the cast fails, `AssetLoader` implementors can choose to either fail, or implement fallback behavior (such as reading all asset bytes into a `Vec`, which is seekable).