Skip to content
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

Capitalize the drive letter in windows absolute paths #727

Merged
merged 1 commit into from
Sep 25, 2024

Conversation

DaelonSuzuka
Copy link
Collaborator

RE: #726

@KoBeWi please grab the CI build and let me know if this solves your problem

@KoBeWi
Copy link
Member

KoBeWi commented Sep 25, 2024

It solves the problem, but looks like a hack. Why is projectDir like that?

@DaelonSuzuka
Copy link
Collaborator Author

I don't understand the question. Why is projectDir like what?

@KoBeWi
Copy link
Member

KoBeWi commented Sep 25, 2024

Like wrong. Windows always uses capitalized drive letters, so I don't get where the small drive letter came from. That would be the true source of this bug.

@DaelonSuzuka
Copy link
Collaborator Author

DaelonSuzuka commented Sep 25, 2024

path.dirname() is from the Node standard library, and apparently it returns Windows paths with a lower case drive letter. What is it you're expecting me to do about that except fix the return value?

@KoBeWi
Copy link
Member

KoBeWi commented Sep 25, 2024

Ah so it's just some Node weirdness.

What is it you're expecting me to do about that except fix the return value?

I was mostly curious, because it looks like bandaid fix. But if the problem is standard library then it's the only way I guess.

@DaelonSuzuka
Copy link
Collaborator Author

Ah so it's just some Node weirdness.

Pretty much. It preserves the case in the rest of the path, for some reason: ie it doesn't lower the path part of p:/SkyknightsOnline on my machine, just the drive letter.

@DaelonSuzuka DaelonSuzuka merged commit 658270e into godotengine:master Sep 25, 2024
4 checks passed
@DaelonSuzuka DaelonSuzuka deleted the drive-letter-case branch September 25, 2024 21:20
@Calinou Calinou added the bug label Oct 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants