Skip to content

Commit

Permalink
Don't preemptively parse the label
Browse files Browse the repository at this point in the history
  • Loading branch information
@Selene-Amanita
Selene-Amanita committed Nov 9, 2023
1 parent 312799c commit 52bd527
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 108 deletions.
24 changes: 14 additions & 10 deletions crates/bevy_asset/src/loader.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,13 @@ pub trait AssetLoader: Send + Sync + 'static {
/// Returns a list of extensions supported by this asset loader, without the preceding dot.
fn extensions(&self) -> &[&str];
/// Returns the type name of the sub-asset type with the given `label`.
fn label_type_name(&self, label_type: &str) -> Option<&'static str>;
fn label_type_name(&self, #[allow(unused_variables)] label: &str) -> Option<&'static str> {
None
}
/// Returns the [`TypeId`] of the sub-asset type with the given `label`.
fn label_type_id(&self, label_type: &str) -> Option<TypeId>;
fn label_type_id(&self, #[allow(unused_variables)] label: &str) -> Option<TypeId> {
None
}
}

/// Provides type-erased access to an [`AssetLoader`].
Expand Down Expand Up @@ -73,9 +77,9 @@ pub trait ErasedAssetLoader: Send + Sync + 'static {
/// Returns the [`TypeId`] of the top-level [`Asset`] loaded by the [`AssetLoader`].
fn asset_type_id(&self) -> TypeId;
/// Returns the type name of the sub-asset type with the given `label`.
fn label_type_name(&self, label_type: Option<&str>) -> Option<&'static str>;
fn label_type_name(&self, label: Option<&str>) -> Option<&'static str>;
/// Returns the [`TypeId`] of the sub-asset type with the given `label`.
fn label_type_id(&self, label_type: Option<&str>) -> Option<TypeId>;
fn label_type_id(&self, label: Option<&str>) -> Option<TypeId>;
}

impl<L> ErasedAssetLoader for L
Expand Down Expand Up @@ -135,17 +139,17 @@ where
TypeId::of::<L::Asset>()
}

fn label_type_name(&self, label_type: Option<&str>) -> Option<&'static str> {
match label_type {
fn label_type_name(&self, label: Option<&str>) -> Option<&'static str> {
match label {
None => Some(self.asset_type_name()),
Some(label_type) => <L as AssetLoader>::label_type_name(self, label_type),
Some(label) => <L as AssetLoader>::label_type_name(self, label),
}
}

fn label_type_id(&self, label_type: Option<&str>) -> Option<TypeId> {
match label_type {
fn label_type_id(&self, label: Option<&str>) -> Option<TypeId> {
match label {
None => Some(self.asset_type_id()),
Some(label_type) => <L as AssetLoader>::label_type_id(self, label_type),
Some(label) => <L as AssetLoader>::label_type_id(self, label),
}
}
}
Expand Down
94 changes: 11 additions & 83 deletions crates/bevy_asset/src/path.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,22 +52,11 @@ use thiserror::Error;
/// This means that the common case of `asset_server.load("my_scene.scn")` when it creates and
/// clones internal owned [`AssetPaths`](AssetPath).
/// This also means that you should use [`AssetPath::parse`] in cases where `&str` is the explicit type.
#[derive(Eq, Hash, Clone, Default)]
#[derive(Eq, PartialEq, Hash, Clone, Default)]
pub struct AssetPath<'a> {
source: AssetSourceId<'a>,
path: CowArc<'a, Path>,
label: Option<CowArc<'a, str>>,
/// Last part of the asset label.
/// This is given as a convenience only when parsing an asset path string,
/// but it is not set when a `label` is set directly.
/// Should not be used for comparison.
pub(crate) label_type: Option<CowArc<'a, str>>,
}

impl<'a> PartialEq for AssetPath<'a> {
fn eq(&self, rhs: &Self) -> bool {
self.source == rhs.source && self.path == rhs.path && self.label == rhs.label
}
}

impl<'a> Debug for AssetPath<'a> {
Expand Down Expand Up @@ -97,8 +86,6 @@ pub enum ParseAssetPathError {
MissingSource,
#[error("Asset label must be at least one character. Either specify the label after the '#' or remove the '#'")]
MissingLabel,
#[error("Asset label shouldn't end with a '/', either remove it or add a sub-label after it")]
MissingEndLabel,
}

impl<'a> AssetPath<'a> {
Expand Down Expand Up @@ -128,26 +115,24 @@ impl<'a> AssetPath<'a> {
///
/// This will return a [`ParseAssetPathError`] if `asset_path` is in an invalid format.
pub fn try_parse(asset_path: &'a str) -> Result<AssetPath<'a>, ParseAssetPathError> {
let (source, path, label, label_type) = Self::parse_internal(asset_path).unwrap();
let (source, path, label) = Self::parse_internal(asset_path).unwrap();
Ok(Self {
source: match source {
Some(source) => AssetSourceId::Name(CowArc::Borrowed(source)),
None => AssetSourceId::Default,
},
path: CowArc::Borrowed(path),
label: label.map(CowArc::Borrowed),
label_type: label_type.map(CowArc::Borrowed),
})
}

fn parse_internal(
asset_path: &str,
) -> Result<(Option<&str>, &Path, Option<&str>, Option<&str>), ParseAssetPathError> {
) -> Result<(Option<&str>, &Path, Option<&str>), ParseAssetPathError> {
let mut chars = asset_path.char_indices();
let mut source_range = None;
let mut path_range = 0..asset_path.len();
let mut label_range = None;
let mut label_type_range = None;
while let Some((index, char)) = chars.next() {
match char {
':' => {
Expand All @@ -169,17 +154,7 @@ impl<'a> AssetPath<'a> {
'#' => {
path_range.end = index;
label_range = Some(index + 1..asset_path.len());
label_type_range = label_range.clone();
}
'/' => {
if label_range.is_some() {
label_type_range = Some(index + 1..asset_path.len());
}
}
'0' | '1' | '2' | '3' | '4' | '5' | '6' | '7' | '8' | '9' => {
if let Some(ref mut label_type_range) = label_type_range {
label_type_range.end = index;
}
break;
}
_ => {}
}
Expand All @@ -203,18 +178,9 @@ impl<'a> AssetPath<'a> {
}
None => None,
};
let label_type = match label_type_range {
Some(label_type_range) => {
if label_type_range.is_empty() {
return Err(ParseAssetPathError::MissingEndLabel);
}
Some(&asset_path[label_type_range])
}
None => None,
};

let path = Path::new(&asset_path[path_range]);
Ok((source, path, label, label_type))
Ok((source, path, label))
}

/// Creates a new [`AssetPath`] from a [`Path`].
Expand All @@ -224,7 +190,6 @@ impl<'a> AssetPath<'a> {
path: CowArc::Borrowed(path),
source: AssetSourceId::Default,
label: None,
label_type: None,
}
}

Expand All @@ -247,18 +212,6 @@ impl<'a> AssetPath<'a> {
self.label.clone()
}

/// Gets the "sub-asset label type".
#[inline]
pub fn label_type(&self) -> Option<&str> {
self.label_type.as_deref()
}

/// Gets the "sub-asset label type".
#[inline]
pub fn label_type_cow(&self) -> Option<CowArc<'a, str>> {
self.label_type.clone()
}

/// Gets the path to the asset in the "virtual filesystem".
#[inline]
pub fn path(&self) -> &Path {
Expand All @@ -272,21 +225,18 @@ impl<'a> AssetPath<'a> {
source: self.source.clone(),
path: self.path.clone(),
label: None,
label_type: None,
}
}

/// Removes a "sub-asset label" from this [`AssetPath`], if one was set.
#[inline]
pub fn remove_label(&mut self) {
self.label = None;
self.label_type = None;
}

/// Takes the "sub-asset label" from this [`AssetPath`], if one was set.
#[inline]
pub fn take_label(&mut self) -> Option<CowArc<'a, str>> {
self.label_type = None;
self.label.take()
}

Expand All @@ -298,7 +248,6 @@ impl<'a> AssetPath<'a> {
source: self.source,
path: self.path,
label: Some(label.into()),
label_type: None,
}
}

Expand All @@ -310,7 +259,6 @@ impl<'a> AssetPath<'a> {
source: source.into(),
path: self.path,
label: self.label,
label_type: self.label_type,
}
}

Expand All @@ -324,7 +272,6 @@ impl<'a> AssetPath<'a> {
Some(AssetPath {
source: self.source.clone(),
label: None,
label_type: None,
path,
})
}
Expand All @@ -339,7 +286,6 @@ impl<'a> AssetPath<'a> {
source: self.source.into_owned(),
path: self.path.into_owned(),
label: self.label.map(|l| l.into_owned()),
label_type: self.label_type.map(|l| l.into_owned()),
}
}

Expand Down Expand Up @@ -429,7 +375,7 @@ impl<'a> AssetPath<'a> {
// It's a label only
Ok(self.clone_owned().with_label(label.to_owned()))
} else {
let (source, rpath, rlabel, rlabel_type) = AssetPath::parse_internal(path)?;
let (source, rpath, rlabel) = AssetPath::parse_internal(path)?;
let mut base_path = PathBuf::from(self.path());
if replace && !self.path.to_str().unwrap().ends_with('/') {
// No error if base is empty (per RFC 1808).
Expand Down Expand Up @@ -482,7 +428,6 @@ impl<'a> AssetPath<'a> {
},
path: CowArc::Owned(result_path.into()),
label: rlabel.map(|l| CowArc::Owned(l.into())),
label_type: rlabel_type.map(|l| CowArc::Owned(l.into())),
})
}
}
Expand Down Expand Up @@ -510,12 +455,11 @@ impl<'a> AssetPath<'a> {
impl From<&'static str> for AssetPath<'static> {
#[inline]
fn from(asset_path: &'static str) -> Self {
let (source, path, label, label_type) = Self::parse_internal(asset_path).unwrap();
let (source, path, label) = Self::parse_internal(asset_path).unwrap();
AssetPath {
source: source.into(),
path: CowArc::Static(path),
label: label.map(CowArc::Static),
label_type: label_type.map(CowArc::Static),
}
}
}
Expand All @@ -541,7 +485,6 @@ impl From<&'static Path> for AssetPath<'static> {
source: AssetSourceId::Default,
path: CowArc::Static(path),
label: None,
label_type: None,
}
}
}
Expand All @@ -553,7 +496,6 @@ impl From<PathBuf> for AssetPath<'static> {
source: AssetSourceId::Default,
path: path.into(),
label: None,
label_type: None,
}
}
}
Expand Down Expand Up @@ -751,33 +693,19 @@ mod tests {
#[test]
fn parse_asset_path() {
let result = AssetPath::parse_internal("a/b.test");
assert_eq!(result, Ok((None, Path::new("a/b.test"), None, None)));
assert_eq!(result, Ok((None, Path::new("a/b.test"), None)));

let result = AssetPath::parse_internal("http://a/b.test");
assert_eq!(
result,
Ok((Some("http"), Path::new("a/b.test"), None, None))
);
assert_eq!(result, Ok((Some("http"), Path::new("a/b.test"), None)));

let result = AssetPath::parse_internal("http://a/b.test#Foo");
assert_eq!(
result,
Ok((
Some("http"),
Path::new("a/b.test"),
Some("Foo"),
Some("Foo")
))
);

let result = AssetPath::parse_internal("a#Foo0/Bar1");
assert_eq!(
result,
Ok((None, Path::new("a"), Some("Foo0/Bar1"), Some("Bar")))
Ok((Some("http"), Path::new("a/b.test"), Some("Foo")))
);

let result = AssetPath::parse_internal("http://");
assert_eq!(result, Ok((Some("http"), Path::new(""), None, None)));
assert_eq!(result, Ok((Some("http"), Path::new(""), None)));

let result = AssetPath::parse_internal("://x");
assert_eq!(result, Err(crate::ParseAssetPathError::MissingSource));
Expand Down
15 changes: 6 additions & 9 deletions crates/bevy_asset/src/server/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -386,15 +386,13 @@ impl AssetServer {
}
None => {
let mut infos = self.data.infos.write();
let label_type = path.label_type();
let (Some(type_id), Some(type_name)) = (
loader.label_type_id(label_type),
loader.label_type_name(label_type),
) else {
let label = path.label();
let (Some(type_id), Some(type_name)) =
(loader.label_type_id(label), loader.label_type_name(label))
else {
return Err(AssetLoadError::WrongLabel {
base_path: path.without_label().into_owned(),
label: path.label().unwrap_or_default().to_owned(),
label_type: label_type.unwrap_or_default().to_owned(),
label: label.unwrap_or_default().to_owned(),
});
};
infos.get_or_create_path_handle_untyped(
Expand Down Expand Up @@ -1064,11 +1062,10 @@ pub enum AssetLoadError {
base_path: AssetPath<'static>,
label: String,
},
#[error("The type of the file at '{base_path}' does not support the label type '{label_type}' at the end of the label '{label}'.")]
#[error("The type of the file at '{base_path}' does not support the label format '{label}'.")]
WrongLabel {
base_path: AssetPath<'static>,
label: String,
label_type: String,
},
}

Expand Down
Loading

0 comments on commit 52bd527

Please sign in to comment.