-
-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
load_folder() panics when given an absolute path but load() does not #9458
Comments
What OS are you on? File system bugs are sometimes OS specific, so it's good to record it. |
I'm on windows but it will have the same behavior on linux. The issue is that The workaround that I found is to set the root directory to the project path; if that's the intended use then maybe it would be better to add a line or two in the docs rather than a code fix. Edit: |
I don't believe this is true. According to the Path::join docs, To follow-up on your example: let a: Path; // "C:\Path\To\Executable\"
let b: Path; // "C:\Absolute\Path\To\Folder\"
let c = a.join(b); // "C:\Absolute\Path\To\Folder\"
let d = c.strip_prefix(&a); // Error: `c` does not start with `a` The minimal change that would resolve this bug would be to construct a relative path from fn read_directory(
&self,
path: &Path,
) -> Result<Box<dyn Iterator<Item = PathBuf>>, AssetIoError> {
let root_path = self.root_path.to_owned();
Ok(Box::new(fs::read_dir(root_path.join(path))?.map(
move |entry| {
let path = entry.unwrap().path();
pathdiff::diff_paths(path, root_path).unwrap()
},
)))
} |
I'm new to rust so I'm not used to a sane(er) standard library; this is nice to learn. |
Fixes #9458. On case-insensitive filesystems (Windows, Mac, NTFS mounted in Linux, etc.), a path can be represented in a multiple ways: - `c:\users\user\rust\assets\hello\world` - `c:/users/user/rust/assets/hello/world` - `C:\USERS\USER\rust\assets\hello\world` If user specifies a path variant that doesn't match asset folder path bevy calculates, `path.strip_prefix()` will fail, as demonstrated below: ```rs dbg!(Path::new("c:/foo/bar/baz").strip_prefix("c:/foo")); // Ok("bar/baz") dbg!(Path::new("c:/FOO/bar/baz").strip_prefix("c:/foo")); // StripPrefixError(()) ``` This commit rewrites the code in question in a way that prefix stripping is no longer necessary. I've tested with the following paths on my computer: ```rs let res = asset_server.load_folder("C:\\Users\\user\\rust\\assets\\foo\\bar"); dbg!(res); let res = asset_server.load_folder("c:\\users\\user\\rust\\assets\\foo\\bar"); dbg!(res); let res = asset_server.load_folder("C:/Users/user/rust/assets/foo/bar"); dbg!(res); ```
…#9490) Fixes bevyengine#9458. On case-insensitive filesystems (Windows, Mac, NTFS mounted in Linux, etc.), a path can be represented in a multiple ways: - `c:\users\user\rust\assets\hello\world` - `c:/users/user/rust/assets/hello/world` - `C:\USERS\USER\rust\assets\hello\world` If user specifies a path variant that doesn't match asset folder path bevy calculates, `path.strip_prefix()` will fail, as demonstrated below: ```rs dbg!(Path::new("c:/foo/bar/baz").strip_prefix("c:/foo")); // Ok("bar/baz") dbg!(Path::new("c:/FOO/bar/baz").strip_prefix("c:/foo")); // StripPrefixError(()) ``` This commit rewrites the code in question in a way that prefix stripping is no longer necessary. I've tested with the following paths on my computer: ```rs let res = asset_server.load_folder("C:\\Users\\user\\rust\\assets\\foo\\bar"); dbg!(res); let res = asset_server.load_folder("c:\\users\\user\\rust\\assets\\foo\\bar"); dbg!(res); let res = asset_server.load_folder("C:/Users/user/rust/assets/foo/bar"); dbg!(res); ```
Bevy version
0.11
The issue is currently present in the main branch as well:
https://github.com/bevyengine/bevy/blob/b6a9d8eba7807b63be525e423911c0217706f13a/crates/bevy_asset/src/io/file_asset_io.rs#L112C4-L123C6
What you did
What went wrong
Additional information
I'm using toml files as project files (like Unreal/Unity/Godot) and want to load and watch the files in the project folder for changes; this folder can be anywhere on the filesystem. The 'read_directory' function assumes that the provided path is relative to self and appends the provided path to the self root path which turns into "C:\Path\To\Executable\C:\Absolute\Path\To\Folder\" and panics.
The
load
function works as expected but theload_folder
variation panics which is inconsistent behavior. I think the best fix would be to auto-detect absolute paths and add the root path if it's not. If that's not in the cards then an optional boolean parameter foris_absolute
could work.The text was updated successfully, but these errors were encountered: