Fix get_asset_paths not properly deleting empty folders (& recursive async functions) (#12638)

# Objective

get_asset_paths tries to check whether a folder is empty, and if so
delete it. However rather than checking whether any subfolder contains
files it checks whether _all_ subfolders have files.

Also cleanup various BoxedFutures in async recursive functions like
these, rust 1.77 now allows recursive async functions (albeit still by
boxing), hurray! This is a followup to #12550 (sorta). More BoxedFuture
stuff can be removed now that rust 1.77 is out, which can use async
recursive functions! This is mainly just cleaner code wise - the
recursion still boxes the future so not much to win there.

PR is mainly whitespace changes so do disable whitespace diffs for
easier review.
This commit is contained in:
Arthur Brussee 2024-03-23 03:35:51 +00:00 committed by GitHub
parent 72c51cdab9
commit 34c8778bf0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 69 additions and 65 deletions

View File

@ -11,7 +11,7 @@ license = "MIT OR Apache-2.0"
readme = "README.md" readme = "README.md"
repository = "https://github.com/bevyengine/bevy" repository = "https://github.com/bevyengine/bevy"
documentation = "https://docs.rs/bevy" documentation = "https://docs.rs/bevy"
rust-version = "1.76.0" rust-version = "1.77.0"
[workspace] [workspace]
exclude = [ exclude = [

View File

@ -20,7 +20,7 @@ use crate::{
use bevy_ecs::prelude::*; use bevy_ecs::prelude::*;
use bevy_tasks::IoTaskPool; use bevy_tasks::IoTaskPool;
use bevy_utils::tracing::{debug, error, trace, warn}; use bevy_utils::tracing::{debug, error, trace, warn};
use bevy_utils::{BoxedFuture, HashMap, HashSet}; use bevy_utils::{HashMap, HashSet};
use futures_io::ErrorKind; use futures_io::ErrorKind;
use futures_lite::{AsyncReadExt, AsyncWriteExt, StreamExt}; use futures_lite::{AsyncReadExt, AsyncWriteExt, StreamExt};
use parking_lot::RwLock; use parking_lot::RwLock;
@ -435,27 +435,25 @@ impl AssetProcessor {
#[allow(unused)] #[allow(unused)]
#[cfg(all(not(target_arch = "wasm32"), feature = "multi-threaded"))] #[cfg(all(not(target_arch = "wasm32"), feature = "multi-threaded"))]
fn process_assets_internal<'scope>( async fn process_assets_internal<'scope>(
&'scope self, &'scope self,
scope: &'scope bevy_tasks::Scope<'scope, '_, ()>, scope: &'scope bevy_tasks::Scope<'scope, '_, ()>,
source: &'scope AssetSource, source: &'scope AssetSource,
path: PathBuf, path: PathBuf,
) -> BoxedFuture<'scope, Result<(), AssetReaderError>> { ) -> Result<(), AssetReaderError> {
Box::pin(async move { if source.reader().is_directory(&path).await? {
if source.reader().is_directory(&path).await? { let mut path_stream = source.reader().read_directory(&path).await?;
let mut path_stream = source.reader().read_directory(&path).await?; while let Some(path) = path_stream.next().await {
while let Some(path) = path_stream.next().await { Box::pin(self.process_assets_internal(scope, source, path)).await?;
self.process_assets_internal(scope, source, path).await?;
}
} else {
// Files without extensions are skipped
let processor = self.clone();
scope.spawn(async move {
processor.process_asset(source, path).await;
});
} }
Ok(()) } else {
}) // Files without extensions are skipped
let processor = self.clone();
scope.spawn(async move {
processor.process_asset(source, path).await;
});
}
Ok(())
} }
async fn try_reprocessing_queued(&self) { async fn try_reprocessing_queued(&self) {
@ -514,34 +512,36 @@ impl AssetProcessor {
/// Retrieves asset paths recursively. If `clean_empty_folders_writer` is Some, it will be used to clean up empty /// Retrieves asset paths recursively. If `clean_empty_folders_writer` is Some, it will be used to clean up empty
/// folders when they are discovered. /// folders when they are discovered.
fn get_asset_paths<'a>( async fn get_asset_paths<'a>(
reader: &'a dyn ErasedAssetReader, reader: &'a dyn ErasedAssetReader,
clean_empty_folders_writer: Option<&'a dyn ErasedAssetWriter>, clean_empty_folders_writer: Option<&'a dyn ErasedAssetWriter>,
path: PathBuf, path: PathBuf,
paths: &'a mut Vec<PathBuf>, paths: &'a mut Vec<PathBuf>,
) -> BoxedFuture<'a, Result<bool, AssetReaderError>> { ) -> Result<bool, AssetReaderError> {
Box::pin(async move { if reader.is_directory(&path).await? {
if reader.is_directory(&path).await? { let mut path_stream = reader.read_directory(&path).await?;
let mut path_stream = reader.read_directory(&path).await?; let mut contains_files = false;
let mut contains_files = false;
while let Some(child_path) = path_stream.next().await { while let Some(child_path) = path_stream.next().await {
contains_files = contains_files |= Box::pin(get_asset_paths(
get_asset_paths(reader, clean_empty_folders_writer, child_path, paths) reader,
.await? clean_empty_folders_writer,
&& contains_files; child_path,
} paths,
if !contains_files && path.parent().is_some() { ))
if let Some(writer) = clean_empty_folders_writer { .await?;
// it is ok for this to fail as it is just a cleanup job.
let _ = writer.remove_empty_directory(&path).await;
}
}
Ok(contains_files)
} else {
paths.push(path);
Ok(true)
} }
}) if !contains_files && path.parent().is_some() {
if let Some(writer) = clean_empty_folders_writer {
// it is ok for this to fail as it is just a cleanup job.
let _ = writer.remove_empty_directory(&path).await;
}
}
Ok(contains_files)
} else {
paths.push(path);
Ok(true)
}
} }
for source in self.sources().iter_processed() { for source in self.sources().iter_processed() {

View File

@ -658,38 +658,42 @@ impl AssetServer {
} }
pub(crate) fn load_folder_internal(&self, id: UntypedAssetId, path: AssetPath) { pub(crate) fn load_folder_internal(&self, id: UntypedAssetId, path: AssetPath) {
fn load_folder<'a>( async fn load_folder<'a>(
source: AssetSourceId<'static>, source: AssetSourceId<'static>,
path: &'a Path, path: &'a Path,
reader: &'a dyn ErasedAssetReader, reader: &'a dyn ErasedAssetReader,
server: &'a AssetServer, server: &'a AssetServer,
handles: &'a mut Vec<UntypedHandle>, handles: &'a mut Vec<UntypedHandle>,
) -> bevy_utils::BoxedFuture<'a, Result<(), AssetLoadError>> { ) -> Result<(), AssetLoadError> {
Box::pin(async move { let is_dir = reader.is_directory(path).await?;
let is_dir = reader.is_directory(path).await?; if is_dir {
if is_dir { let mut path_stream = reader.read_directory(path.as_ref()).await?;
let mut path_stream = reader.read_directory(path.as_ref()).await?; while let Some(child_path) = path_stream.next().await {
while let Some(child_path) = path_stream.next().await { if reader.is_directory(&child_path).await? {
if reader.is_directory(&child_path).await? { Box::pin(load_folder(
load_folder(source.clone(), &child_path, reader, server, handles) source.clone(),
.await?; &child_path,
} else { reader,
let path = child_path.to_str().expect("Path should be a valid string."); server,
let asset_path = AssetPath::parse(path).with_source(source.clone()); handles,
match server.load_untyped_async(asset_path).await { ))
Ok(handle) => handles.push(handle), .await?;
// skip assets that cannot be loaded } else {
Err( let path = child_path.to_str().expect("Path should be a valid string.");
AssetLoadError::MissingAssetLoaderForTypeName(_) let asset_path = AssetPath::parse(path).with_source(source.clone());
| AssetLoadError::MissingAssetLoaderForExtension(_), match server.load_untyped_async(asset_path).await {
) => {} Ok(handle) => handles.push(handle),
Err(err) => return Err(err), // skip assets that cannot be loaded
} Err(
AssetLoadError::MissingAssetLoaderForTypeName(_)
| AssetLoadError::MissingAssetLoaderForExtension(_),
) => {}
Err(err) => return Err(err),
} }
} }
} }
Ok(()) }
}) Ok(())
} }
let path = path.into_owned(); let path = path.into_owned();