Skip to content

Conversation

@webbertakken
Copy link
Contributor

Changes

  • Allows usage cross platform syntax that results from auto-completing paths to asset files.
  • Added tests for the regex to ensure it doesn't break for anyone.

For example, in powershell an autocomplete always adds the .\ regardless of whether you typed it or not.

collider .\assets\textures\arenas\church_ctf_1\container-1.png

resulting in

Couldn't find the file .\assets\textures\arenas\church_ctf_1\container-1.png

Tests:

image

@CleanCut

This comment was marked as resolved.

@webbertakken
Copy link
Contributor Author

Both outputs are the same in this example.

== WINDOWS ==
Components(
    [
        CurDir,
        Normal(
            "assets",
        ),
        Normal(
            "sprite",
        ),
        Normal(
            "sprite.png",
        ),
    ],
)


== POSIX ==
Components(
    [
        CurDir,
        Normal(
            "assets",
        ),
        Normal(
            "sprite",
        ),
        Normal(
            "sprite.png",
        ),
    ],
)

This was meant to be a quick and solid fix. I'm still too much in the learning curve (after your two excellent courses, thank you very much) to be contributing to low level packages already. Would you mind placing the follow-up out of scope for this PR?

@CleanCut
Copy link
Owner

(after your two excellent courses, thank you very much)

You are most welcome. 😄

Both outputs are the same in this example.

WHAT!?!? If the component detection is working with a .\ path, then assets should have been stripped off of your path, and everything should have worked.

Can you please run this and post the output?

use std::path::PathBuf;

fn main() {
    let orig = r".\assets\textures\arenas\church_ctf_1\container-1.png";
    println!("Original path is: {}", orig);
    let mut path = PathBuf::from(orig);
    println!("Parsed path is: {:#?}", path);
    if path.starts_with("assets") {
        println!("Yes, detected that path starts with assets.");
        path = path.strip_prefix("assets").unwrap().to_path_buf();
        println!("Stripped path is: {:#?}", path);
    } else {
        println!("No, did not detect that path starts with assets.");
    }
    let rejoined_path = PathBuf::from("assets").join(&path);
    println!("Rejoined path is: {:#?}", rejoined_path);
}

@webbertakken
Copy link
Contributor Author

Output:

Original path is: .\assets\textures\arenas\church_ctf_1\container-1.png
Parsed path is: ".\\assets\\textures\\arenas\\church_ctf_1\\container-1.png"
No, did not detect that path starts with assets.
Rejoined path is: "assets\\.\\assets\\textures\\arenas\\church_ctf_1\\container-1.png"

@CleanCut
Copy link
Owner

🤦🏻‍♂️ Ah, okay, Path::starts_with returns false if the path starts with a current directory on all platforms. That's my bug, not Rust's.

I hit the same problem when I try cargo run --example collider ./assets/sprite/racing/barrel_blue.png on macOS. Mystery solved!

So, all we really need to do is to strip ./ or .\, and then the existing code should do the rest. I'll push up a commit to simplify things.

@webbertakken
Copy link
Contributor Author

webbertakken commented Nov 14, 2022

Actually I would argue that my fix simplified your code more than doing two separate replace actions and abstracting a simple replace into a separate function.

Multiple slashes should also be considered a valid path.

Recommend reverting to 36118a320b3740bbe7ffb4fe475f01bfc391d943.

@CleanCut
Copy link
Owner

Actually I would argue that my fix simplified your code more than doing two separate replace actions and abstracting a simple replace into a separate function.

I believe I've found an even simpler solution! Standard library to the rescue. 😄

Multiple slashes should also be considered a valid path.

Canonicalization is handled deeper within Bevy/std already.

Copy link
Owner

@CleanCut CleanCut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉 Nice. I like where this ended up.

@CleanCut CleanCut merged commit 8567aa7 into CleanCut:main Nov 15, 2022
@webbertakken webbertakken deleted the allow-using-tab branch November 15, 2022 19:14
@CleanCut
Copy link
Owner

Included in rusty_engine v5.2.1 (just released). 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants