-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Fill out some missing docs for bevy_assets #17829
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,6 +24,7 @@ pub struct EmbeddedWatcher { | |
} | ||
|
||
impl EmbeddedWatcher { | ||
/// Creates a new `EmbeddedWatcher` that watches for changes to the embedded assets in the given `dir`. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't really think you need this. The comment above the struct is better. I guess you might want it for the future lints though. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah this is lint-proofing. |
||
pub fn new( | ||
dir: Dir, | ||
root_paths: Arc<RwLock<HashMap<Box<Path>, PathBuf>>>, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,8 @@ use std::path::{Path, PathBuf}; | |
#[cfg(feature = "embedded_watcher")] | ||
use alloc::borrow::ToOwned; | ||
|
||
/// The name of the `embedded` [`AssetSource`], | ||
/// as stored in the [`AssetSourceBuilders`] resource. | ||
pub const EMBEDDED: &str = "embedded"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What does "embedding" an asset in this context mean? I looked at the example and it didn't really explain it either. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It means putting the asset inside the game executable file, in contrast to other assets that are normally loaded from disk to memory when needed. It's normally only used for small engine-related assets so you can use them without having them in your own asset directory. It sounds like the example could use a line or 2 describing that. |
||
|
||
/// A [`Resource`] that manages "rust source files" in a virtual in memory [`Dir`], which is intended | ||
|
@@ -77,6 +79,7 @@ impl EmbeddedAssetRegistry { | |
self.dir.remove_asset(full_path) | ||
} | ||
|
||
/// Registers the [`EMBEDDED`] [`AssetSource`] with the given [`AssetSourceBuilders`]. | ||
pub fn register_source(&self, sources: &mut AssetSourceBuilders) { | ||
let dir = self.dir.clone(); | ||
let processed_dir = self.dir.clone(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -132,26 +132,34 @@ impl<'a> PartialEq for AssetSourceId<'a> { | |
/// 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>>, | ||
/// 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. | ||
pub watcher: Option< | ||
Box< | ||
dyn FnMut(crossbeam_channel::Sender<AssetSourceEvent>) -> Option<Box<dyn AssetWatcher>> | ||
+ Send | ||
+ Sync, | ||
>, | ||
>, | ||
/// The [`ErasedAssetReader`] to use for processed assets. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might be worth linking to this somewhere? That way people know what processing is. https://github.com/bevyengine/bevy/blob/main/crates/bevy_asset/src/processor/mod.rs |
||
pub processed_reader: Option<Box<dyn FnMut() -> Box<dyn ErasedAssetReader> + Send + Sync>>, | ||
/// The [`ErasedAssetWriter`] to use for processed assets. | ||
pub processed_writer: | ||
Option<Box<dyn FnMut(bool) -> Option<Box<dyn ErasedAssetWriter>> + Send + Sync>>, | ||
/// The [`AssetWatcher`] to use for processed assets, if any. | ||
pub processed_watcher: Option< | ||
Box< | ||
dyn FnMut(crossbeam_channel::Sender<AssetSourceEvent>) -> Option<Box<dyn AssetWatcher>> | ||
+ Send | ||
+ Sync, | ||
>, | ||
>, | ||
/// The warning message to display when watching an unprocessed asset fails. | ||
pub watch_warning: Option<&'static str>, | ||
/// The warning message to display when watching a processed asset fails. | ||
pub processed_watch_warning: Option<&'static str>, | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,8 +32,10 @@ pub struct ProcessorTransactionLog { | |
/// An error that occurs when reading from the [`ProcessorTransactionLog`] fails. | ||
#[derive(Error, Debug)] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also might be worth linking to this above the ProcessorTransactionLog |
||
pub enum ReadLogError { | ||
/// An invalid log line was encountered, consisting of the contained string. | ||
#[error("Encountered an invalid log line: '{0}'")] | ||
InvalidLine(String), | ||
/// A file-system-based error occurred while reading the log file. | ||
#[error("Failed to read log file: {0}")] | ||
Io(#[from] futures_io::Error), | ||
} | ||
|
@@ -51,21 +53,27 @@ pub struct WriteLogError { | |
/// An error that occurs when validating the [`ProcessorTransactionLog`] fails. | ||
#[derive(Error, Debug)] | ||
pub enum ValidateLogError { | ||
/// An error that could not be recovered from. All assets will be reprocessed. | ||
#[error("Encountered an unrecoverable error. All assets will be reprocessed.")] | ||
UnrecoverableError, | ||
/// A [`ReadLogError`]. | ||
#[error(transparent)] | ||
ReadLogError(#[from] ReadLogError), | ||
/// Duplicated process asset transactions occurred. | ||
#[error("Encountered a duplicate process asset transaction: {0:?}")] | ||
EntryErrors(Vec<LogEntryError>), | ||
} | ||
|
||
/// An error that occurs when validating individual [`ProcessorTransactionLog`] entries. | ||
#[derive(Error, Debug)] | ||
pub enum LogEntryError { | ||
/// A duplicate process asset transaction occurred for the given asset path. | ||
#[error("Encountered a duplicate process asset transaction: {0}")] | ||
DuplicateTransaction(AssetPath<'static>), | ||
/// A transaction was ended that never started for the given asset path. | ||
#[error("A transaction was ended that never started {0}")] | ||
EndedMissingTransaction(AssetPath<'static>), | ||
/// An asset started processing but never finished at the given asset path. | ||
#[error("An asset started processing but never finished: {0}")] | ||
UnfinishedTransaction(AssetPath<'static>), | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment sent me down a giant rabbit whole lol.
I get that opaque means that there is some data known to the compiler but not to the user. I don't really know what unstable means in this context though.
Maybe it's worth linking to the AssetIndex struct for users to read more?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opaque means you can't "see inside" it, or e.g. do arithmetic with it, but only use it by giving it to APIs. The compiler helps us enforce this but that's not a requirement for using the term.
Unstable means you can't expect to get the same index between restarts of the application, so it doesn't make sense to serialize it for example.
In all cases I can think of where doc links are rendered you'd also have the type right there, so a link is probably redundant.