-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Add directory related functions to AndroidAssetReader
#11495
Changes from all commits
46762ef
95d8f08
dc8db07
2e0ba1d
58d120d
3704a72
c07ee19
0d13ee1
0238a1c
3971912
573a6f1
ad68a0f
fb3fb73
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 |
---|---|---|
@@ -1,16 +1,15 @@ | ||
use crate::io::{ | ||
get_meta_path, AssetReader, AssetReaderError, EmptyPathStream, PathStream, Reader, VecReader, | ||
}; | ||
use alloc::ffi::CString; | ||
use crate::io::{get_meta_path, AssetReader, AssetReaderError, PathStream, Reader, VecReader}; | ||
use bevy_utils::tracing::error; | ||
use std::path::Path; | ||
use futures_lite::stream; | ||
use std::{ffi::CString, path::Path}; | ||
|
||
/// [`AssetReader`] implementation for Android devices, built on top of Android's [`AssetManager`]. | ||
/// | ||
/// Implementation details: | ||
/// | ||
/// - [`load_path`](AssetIo::load_path) uses the [`AssetManager`] to load files. | ||
/// - [`read_directory`](AssetIo::read_directory) always returns an empty iterator. | ||
/// - All functions use the [`AssetManager`] to load files. | ||
/// - [`is_directory`](AssetReader::is_directory) tries to open the path | ||
/// as a normal file and treats an error as if the path is a directory. | ||
/// - Watching for changes is not supported. The watcher method will do nothing. | ||
/// | ||
/// [AssetManager]: https://developer.android.com/reference/android/content/res/AssetManager | ||
|
@@ -46,15 +45,53 @@ impl AssetReader for AndroidAssetReader { | |
|
||
async fn read_directory<'a>( | ||
&'a self, | ||
_path: &'a Path, | ||
path: &'a Path, | ||
) -> Result<Box<PathStream>, AssetReaderError> { | ||
let stream: Box<PathStream> = Box::new(EmptyPathStream); | ||
error!("Reading directories is not supported with the AndroidAssetReader"); | ||
Ok(stream) | ||
let asset_manager = bevy_winit::ANDROID_APP | ||
.get() | ||
.expect("Bevy must be setup with the #[bevy_main] macro on Android") | ||
.asset_manager(); | ||
let opened_assets_dir = asset_manager | ||
.open_dir(&CString::new(path.to_str().unwrap()).unwrap()) | ||
.ok_or(AssetReaderError::NotFound(path.to_path_buf()))?; | ||
|
||
let mapped_stream = opened_assets_dir | ||
.filter_map(move |f| { | ||
let file_path = path.join(Path::new(f.to_str().unwrap())); | ||
// filter out meta files as they are not considered assets | ||
if let Some(ext) = file_path.extension().and_then(|e| e.to_str()) { | ||
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.
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 agree, but I think it might be better to just extract out the "meta" file detection and filtering. I am worried that something like Do you think that would make sense? |
||
if ext.eq_ignore_ascii_case("meta") { | ||
return None; | ||
} | ||
} | ||
Some(file_path.to_owned()) | ||
}) | ||
.collect::<Vec<_>>(); | ||
|
||
let read_dir: Box<PathStream> = Box::new(stream::iter(mapped_stream)); | ||
Ok(read_dir) | ||
} | ||
|
||
async fn is_directory<'a>(&'a self, _path: &'a Path) -> Result<bool, AssetReaderError> { | ||
error!("Reading directories is not supported with the AndroidAssetReader"); | ||
Ok(false) | ||
async fn is_directory<'a>( | ||
&'a self, | ||
path: &'a Path, | ||
) -> std::result::Result<bool, AssetReaderError> { | ||
let asset_manager = bevy_winit::ANDROID_APP | ||
.get() | ||
.expect("Bevy must be setup with the #[bevy_main] macro on Android") | ||
.asset_manager(); | ||
// HACK: `AssetManager` does not provide a way to check if path | ||
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. Fair enough |
||
// points to a directory or a file | ||
// `open_dir` succeeds for both files and directories and will only | ||
// fail if the path does not exist at all | ||
// `open` will fail for directories, but it will work for files | ||
// The solution here was to first use `open_dir` to eliminate the case | ||
// when the path does not exist at all, and then to use `open` to | ||
// see if that path is a file or a directory | ||
let cpath = CString::new(path.to_str().unwrap()).unwrap(); | ||
let _ = asset_manager | ||
.open_dir(&cpath) | ||
.ok_or(AssetReaderError::NotFound(path.to_path_buf()))?; | ||
Ok(asset_manager.open(&cpath).is_none()) | ||
} | ||
} |
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.
Can we add this expectation to the documentation?
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 is a message that is also used in
bevy_winit
, so it is just reused here. I agree it would be good to add to the docs. I can add it separately to theandroid
page in the book (which is currently hidden, but I guess it is good to have it there so it is not forgotten once it gets published).