Skip to content

Commit

Permalink
Fix get_asset_paths not properly deleting empty folders (& recursive …
Browse files Browse the repository at this point in the history
…async functions) (bevyengine#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 bevyengine#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.
  • Loading branch information
ArthurBrussee authored Mar 23, 2024
1 parent 72c51cd commit 34c8778
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 64 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ license = "MIT OR Apache-2.0"
readme = "README.md"
repository = "https://github.com/bevyengine/bevy"
documentation = "https://docs.rs/bevy"
rust-version = "1.76.0"
rust-version = "1.77.0"

[workspace]
exclude = [
Expand Down
78 changes: 39 additions & 39 deletions crates/bevy_asset/src/processor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use crate::{
use bevy_ecs::prelude::*;
use bevy_tasks::IoTaskPool;
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_lite::{AsyncReadExt, AsyncWriteExt, StreamExt};
use parking_lot::RwLock;
Expand Down Expand Up @@ -435,27 +435,25 @@ impl AssetProcessor {

#[allow(unused)]
#[cfg(all(not(target_arch = "wasm32"), feature = "multi-threaded"))]
fn process_assets_internal<'scope>(
async fn process_assets_internal<'scope>(
&'scope self,
scope: &'scope bevy_tasks::Scope<'scope, '_, ()>,
source: &'scope AssetSource,
path: PathBuf,
) -> BoxedFuture<'scope, Result<(), AssetReaderError>> {
Box::pin(async move {
if source.reader().is_directory(&path).await? {
let mut path_stream = source.reader().read_directory(&path).await?;
while let Some(path) = path_stream.next().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;
});
) -> Result<(), AssetReaderError> {
if source.reader().is_directory(&path).await? {
let mut path_stream = source.reader().read_directory(&path).await?;
while let Some(path) = path_stream.next().await {
Box::pin(self.process_assets_internal(scope, 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) {
Expand Down Expand Up @@ -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
/// folders when they are discovered.
fn get_asset_paths<'a>(
async fn get_asset_paths<'a>(
reader: &'a dyn ErasedAssetReader,
clean_empty_folders_writer: Option<&'a dyn ErasedAssetWriter>,
path: PathBuf,
paths: &'a mut Vec<PathBuf>,
) -> BoxedFuture<'a, Result<bool, AssetReaderError>> {
Box::pin(async move {
if reader.is_directory(&path).await? {
let mut path_stream = reader.read_directory(&path).await?;
let mut contains_files = false;
while let Some(child_path) = path_stream.next().await {
contains_files =
get_asset_paths(reader, clean_empty_folders_writer, child_path, paths)
.await?
&& contains_files;
}
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;
}
) -> Result<bool, AssetReaderError> {
if reader.is_directory(&path).await? {
let mut path_stream = reader.read_directory(&path).await?;
let mut contains_files = false;

while let Some(child_path) = path_stream.next().await {
contains_files |= Box::pin(get_asset_paths(
reader,
clean_empty_folders_writer,
child_path,
paths,
))
.await?;
}
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)
}
})
Ok(contains_files)
} else {
paths.push(path);
Ok(true)
}
}

for source in self.sources().iter_processed() {
Expand Down
52 changes: 28 additions & 24 deletions crates/bevy_asset/src/server/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -658,38 +658,42 @@ impl AssetServer {
}

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

let path = path.into_owned();
Expand Down

0 comments on commit 34c8778

Please sign in to comment.