Skip to content

Commit b70ebb2

Browse files
greeble-devgreeble-dev
authored
Remove unused parts of AssetLoaders::find (#23703)
## Objective `AssetLoaders::find` maps from various properties to an asset loader: ```rust fn find( &self, type_name: Option<&str>, asset_type_id: Option<TypeId>, extension: Option<&str>, asset_path: Option<&AssetPath<'_>>, ) -> Option<MaybeAssetLoader> { ``` But in practice the `type_name` and `extension` parameters are always `None`, and the `asset_path` parameter is always `Some`. ## Solution This PR removes the unused parts: ```diff fn find( &self, - type_name: Option<&str>, asset_type_id: Option<TypeId>, - extension: Option<&str>, - asset_path: Option<&AssetPath<'_>>, + asset_path: &AssetPath<'_>, ) -> Option<MaybeAssetLoader> { ``` `AssetLoaders::find` is `pub(crate)`, so this should not affect users. I looked back at the [original PR](#11644) and couldn't spot any cases where these parameters were ever used. I'm not sure if that's an oversight or if they were intended for future use. I also made matching changes to `AssetLoaderError::MissingAssetLoaders`. This is `pub` so technically should have a migration guide. I'm not entirely convinced that's necessary for such a trivial change, but happy to add one if requested. ```diff MissingAssetLoader { - loader_name: Option<String>, asset_type_id: Option<TypeId>, - extension: Option<String>, - asset_path: Option<String>, + asset_path: String, }, ``` ## Testing ```sh cargo test -p bevy_asset cargo run --example asset_loading cargo run --example asset_processing --features="file_watcher asset_processor" ```
1 parent 5d03586 commit b70ebb2

2 files changed

Lines changed: 23 additions & 69 deletions

File tree

‎crates/bevy_asset/src/server/loaders.rs‎

Lines changed: 17 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -154,18 +154,11 @@ impl AssetLoaders {
154154
/// Find an [`AssetLoader`] based on provided search criteria
155155
pub(crate) fn find(
156156
&self,
157-
type_name: Option<&str>,
158157
asset_type_id: Option<TypeId>,
159-
extension: Option<&str>,
160-
asset_path: Option<&AssetPath<'_>>,
158+
asset_path: &AssetPath<'_>,
161159
) -> Option<MaybeAssetLoader> {
162-
// If provided the type name of the loader, return that immediately
163-
if let Some(type_name) = type_name {
164-
return self.get_by_name(type_name);
165-
}
166-
167160
// The presence of a label will affect loader choice
168-
let label = asset_path.as_ref().and_then(|path| path.label());
161+
let label = asset_path.label();
169162

170163
// Try by asset type
171164
let candidates = if let Some(type_id) = asset_type_id {
@@ -207,15 +200,8 @@ impl AssetLoaders {
207200
}
208201
};
209202

210-
// Try the provided extension
211-
if let Some(extension) = extension
212-
&& let Some(&index) = try_extension(extension)
213-
{
214-
return self.get_by_index(index);
215-
}
216-
217203
// Try extracting the extension from the path
218-
if let Some(full_extension) = asset_path.and_then(AssetPath::get_full_extension) {
204+
if let Some(full_extension) = asset_path.get_full_extension() {
219205
if let Some(&index) = try_extension(full_extension) {
220206
return self.get_by_index(index);
221207
}
@@ -236,15 +222,15 @@ impl AssetLoaders {
236222
{
237223
Some(loader) => {
238224
warn!(
239-
"Multiple AssetLoaders found for Asset: {:?}; Path: {:?}; Extension: {:?}",
240-
asset_type_id, asset_path, extension
225+
"Multiple AssetLoaders found for Asset: {:?}; Path: {:?};",
226+
asset_type_id, asset_path
241227
);
242228
Some(loader)
243229
}
244230
None => {
245231
warn!(
246-
"No AssetLoader found for Asset: {:?}; Path: {:?}; Extension: {:?}",
247-
asset_type_id, asset_path, extension
232+
"No AssetLoader found for Asset: {:?}; Path: {:?};",
233+
asset_type_id, asset_path
248234
);
249235
None
250236
}
@@ -597,10 +583,8 @@ mod tests {
597583
let loader = block_on(
598584
loaders
599585
.find(
600-
None,
601586
Some(TypeId::of::<A>()),
602-
None,
603-
Some(&AssetPath::from_path(Path::new("asset.a"))),
587+
&AssetPath::from_path(Path::new("asset.a")),
604588
)
605589
.unwrap()
606590
.get(),
@@ -618,10 +602,8 @@ mod tests {
618602
let loader = block_on(
619603
loaders
620604
.find(
621-
None,
622605
Some(TypeId::of::<B>()),
623-
None,
624-
Some(&AssetPath::from_path(Path::new("asset.b"))),
606+
&AssetPath::from_path(Path::new("asset.b")),
625607
)
626608
.unwrap()
627609
.get(),
@@ -639,10 +621,8 @@ mod tests {
639621
let loader = block_on(
640622
loaders
641623
.find(
642-
None,
643624
Some(TypeId::of::<C>()),
644-
None,
645-
Some(&AssetPath::from_path(Path::new("asset.c"))),
625+
&AssetPath::from_path(Path::new("asset.c")),
646626
)
647627
.unwrap()
648628
.get(),
@@ -662,10 +642,8 @@ mod tests {
662642
let loader = block_on(
663643
loaders
664644
.find(
665-
None,
666645
Some(TypeId::of::<C>()),
667-
None,
668-
Some(&AssetPath::from_path(Path::new("asset.a"))),
646+
&AssetPath::from_path(Path::new("asset.a")),
669647
)
670648
.unwrap()
671649
.get(),
@@ -683,10 +661,8 @@ mod tests {
683661
let loader = block_on(
684662
loaders
685663
.find(
686-
None,
687664
Some(TypeId::of::<C>()),
688-
None,
689-
Some(&AssetPath::from_path(Path::new("asset.b"))),
665+
&AssetPath::from_path(Path::new("asset.b")),
690666
)
691667
.unwrap()
692668
.get(),
@@ -706,10 +682,8 @@ mod tests {
706682
let loader = block_on(
707683
loaders
708684
.find(
709-
None,
710685
Some(TypeId::of::<A>()),
711-
None,
712-
Some(&AssetPath::from_path(Path::new("asset.x"))),
686+
&AssetPath::from_path(Path::new("asset.x")),
713687
)
714688
.unwrap()
715689
.get(),
@@ -727,10 +701,8 @@ mod tests {
727701
let loader = block_on(
728702
loaders
729703
.find(
730-
None,
731704
Some(TypeId::of::<A>()),
732-
None,
733-
Some(&AssetPath::from_path(Path::new("asset"))),
705+
&AssetPath::from_path(Path::new("asset")),
734706
)
735707
.unwrap()
736708
.get(),
@@ -766,10 +738,8 @@ mod tests {
766738
let loader = block_on(
767739
loaders
768740
.find(
769-
None,
770741
Some(TypeId::of::<A>()),
771-
None,
772-
Some(&AssetPath::from_path(Path::new("asset.a"))),
742+
&AssetPath::from_path(Path::new("asset.a")),
773743
)
774744
.unwrap()
775745
.get(),
@@ -785,10 +755,8 @@ mod tests {
785755
let loader = block_on(
786756
loaders
787757
.find(
788-
None,
789758
Some(TypeId::of::<A>()),
790-
None,
791-
Some(&AssetPath::from_path(Path::new("asset.x"))),
759+
&AssetPath::from_path(Path::new("asset.x")),
792760
)
793761
.unwrap()
794762
.get(),
@@ -804,10 +772,8 @@ mod tests {
804772
let loader = block_on(
805773
loaders
806774
.find(
807-
None,
808775
Some(TypeId::of::<A>()),
809-
None,
810-
Some(&AssetPath::from_path(Path::new("asset"))),
776+
&AssetPath::from_path(Path::new("asset")),
811777
)
812778
.unwrap()
813779
.get(),

‎crates/bevy_asset/src/server/mod.rs‎

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1615,16 +1615,11 @@ impl AssetServer {
16151615
}
16161616
Err(AssetReaderError::NotFound(_)) => {
16171617
// TODO: Handle error transformation
1618-
let loader = {
1619-
self.read_loaders()
1620-
.find(None, asset_type_id, None, Some(asset_path))
1621-
};
1618+
let loader = { self.read_loaders().find(asset_type_id, asset_path) };
16221619

16231620
let error = || AssetLoadError::MissingAssetLoader {
1624-
loader_name: None,
16251621
asset_type_id,
1626-
extension: None,
1627-
asset_path: Some(asset_path.to_string()),
1622+
asset_path: asset_path.to_string(),
16281623
};
16291624

16301625
let loader = loader.ok_or_else(error)?.get().await.map_err(|_| error())?;
@@ -1635,16 +1630,11 @@ impl AssetServer {
16351630
Err(err) => return Err(err.into()),
16361631
}
16371632
} else {
1638-
let loader = {
1639-
self.read_loaders()
1640-
.find(None, asset_type_id, None, Some(asset_path))
1641-
};
1633+
let loader = { self.read_loaders().find(asset_type_id, asset_path) };
16421634

16431635
let error = || AssetLoadError::MissingAssetLoader {
1644-
loader_name: None,
16451636
asset_type_id,
1646-
extension: None,
1647-
asset_path: Some(asset_path.to_string()),
1637+
asset_path: asset_path.to_string(),
16481638
};
16491639

16501640
let loader = loader.ok_or_else(error)?.get().await.map_err(|_| error())?;
@@ -2375,12 +2365,10 @@ pub enum AssetLoadError {
23752365
actual_asset_name: &'static str,
23762366
loader_name: &'static str,
23772367
},
2378-
#[error("Could not find an asset loader matching: Loader Name: {loader_name:?}; Asset Type: {asset_type_id:?}; Extension: {extension:?}; Path: {asset_path:?};")]
2368+
#[error("Could not find an asset loader matching: Asset Type: {asset_type_id:?}; Path: {asset_path:?};")]
23792369
MissingAssetLoader {
2380-
loader_name: Option<String>,
23812370
asset_type_id: Option<TypeId>,
2382-
extension: Option<String>,
2383-
asset_path: Option<String>,
2371+
asset_path: String,
23842372
},
23852373
#[error(transparent)]
23862374
MissingAssetLoaderForExtension(#[from] MissingAssetLoaderForExtensionError),

0 commit comments

Comments
 (0)