From eb09695e6326136c2af052e83f0a6a8957490234 Mon Sep 17 00:00:00 2001 From: focustense Date: Sat, 21 Oct 2023 11:25:53 -0400 Subject: [PATCH 1/3] Normalize resolved asset paths using `path_clean`. Makes all paths relative to the `assets` root. Ensures that LDtk-linked assets are shared with assets loaded by `AssetServer`, [bevy_asset_loader](https://github.com/NiklasEi/bevy_asset_loader) and so on, as long as normalized paths are used for those (as they typically are), especially when the LDtk project is in a subdirectory. Fixes #240 --- Cargo.toml | 1 + src/assets/ldtk_project.rs | 29 ++++++++++++++++++++++++++++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index b6dce372..923830b0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,6 +26,7 @@ anyhow = "1.0" thiserror = "1.0" paste = "1.0" derive_more = "0.99.17" +path-clean = "1.0.1" [dev-dependencies] bevy = "0.11" diff --git a/src/assets/ldtk_project.rs b/src/assets/ldtk_project.rs index dd842d0e..2e30ab71 100644 --- a/src/assets/ldtk_project.rs +++ b/src/assets/ldtk_project.rs @@ -15,6 +15,7 @@ use bevy::{ use derive_getters::Getters; use derive_more::From; use std::collections::HashMap; +use path_clean::PathClean; use thiserror::Error; #[cfg(feature = "internal_levels")] @@ -24,7 +25,12 @@ use crate::assets::InternalLevels; use crate::assets::{ExternalLevelMetadata, ExternalLevels}; fn ldtk_path_to_asset_path<'b>(ldtk_path: &Path, rel_path: &str) -> AssetPath<'b> { - ldtk_path.parent().unwrap().join(Path::new(rel_path)).into() + ldtk_path + .parent() + .unwrap() + .join(Path::new(rel_path)) + .clean() + .into() } /// Main asset for loading LDtk project data. @@ -365,6 +371,27 @@ mod tests { } } + #[test] + fn normalizes_asset_paths() { + let resolve_path = |project_path, rel_path| { + let asset_path = ldtk_path_to_asset_path(Path::new(project_path), rel_path); + asset_path.path().to_owned() + }; + + assert_eq!( + resolve_path("project.ldtk", "images/tiles.png"), + Path::new("images/tiles.png")); + assert_eq!( + resolve_path("projects\\sub/project.ldtk", "../images/tiles.png"), + Path::new("projects/images/tiles.png")); + assert_eq!( + resolve_path("projects\\sub/project.ldtk", "../../images/tiles.png"), + Path::new("images/tiles.png")); + assert_eq!( + resolve_path("projects/sub\\project.ldtk", "../../tiles.png"), + Path::new("tiles.png")); + } + #[cfg(feature = "internal_levels")] mod internal_levels { use crate::{ From b837d1211c6900f00a791e8451e19c93e0675ba7 Mon Sep 17 00:00:00 2001 From: focustense Date: Sun, 22 Oct 2023 14:57:36 -0400 Subject: [PATCH 2/3] Make Windows-specific path normalization tests conditional. On Linux, the backslash is treated as part of the directory name and the tests will fail. However, we still want the Windows-specific tests because that is what the non-normalized paths can actually look like in a real app. #240 --- src/assets/ldtk_project.rs | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/src/assets/ldtk_project.rs b/src/assets/ldtk_project.rs index 2e30ab71..86c41c84 100644 --- a/src/assets/ldtk_project.rs +++ b/src/assets/ldtk_project.rs @@ -14,8 +14,8 @@ use bevy::{ }; use derive_getters::Getters; use derive_more::From; -use std::collections::HashMap; use path_clean::PathClean; +use std::collections::HashMap; use thiserror::Error; #[cfg(feature = "internal_levels")] @@ -380,16 +380,37 @@ mod tests { assert_eq!( resolve_path("project.ldtk", "images/tiles.png"), - Path::new("images/tiles.png")); + Path::new("images/tiles.png") + ); + assert_eq!( + resolve_path("projects/sub/project.ldtk", "../images/tiles.png"), + Path::new("projects/images/tiles.png") + ); + assert_eq!( + resolve_path("projects/sub/project.ldtk", "../../tiles.png"), + Path::new("tiles.png") + ); + } + + #[cfg(target_os = "windows")] + fn normalizes_windows_asset_paths() { + let resolve_path = |project_path, rel_path| { + let asset_path = ldtk_path_to_asset_path(Path::new(project_path), rel_path); + asset_path.path().to_owned() + }; + assert_eq!( resolve_path("projects\\sub/project.ldtk", "../images/tiles.png"), - Path::new("projects/images/tiles.png")); + Path::new("projects/images/tiles.png") + ); assert_eq!( resolve_path("projects\\sub/project.ldtk", "../../images/tiles.png"), - Path::new("images/tiles.png")); + Path::new("images/tiles.png") + ); assert_eq!( resolve_path("projects/sub\\project.ldtk", "../../tiles.png"), - Path::new("tiles.png")); + Path::new("tiles.png") + ); } #[cfg(feature = "internal_levels")] From 03cb66d4599ab9dab289190deb5376601abf46dd Mon Sep 17 00:00:00 2001 From: focustense Date: Mon, 23 Oct 2023 23:47:31 -0400 Subject: [PATCH 3/3] Add missing `#[test]` attribute for Windows-specific path-clean test. --- src/assets/ldtk_project.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/assets/ldtk_project.rs b/src/assets/ldtk_project.rs index 86c41c84..63a5439c 100644 --- a/src/assets/ldtk_project.rs +++ b/src/assets/ldtk_project.rs @@ -393,6 +393,7 @@ mod tests { } #[cfg(target_os = "windows")] + #[test] fn normalizes_windows_asset_paths() { let resolve_path = |project_path, rel_path| { let asset_path = ldtk_path_to_asset_path(Path::new(project_path), rel_path);