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

Storage paths can now have space characters #387

Merged
merged 2 commits into from
Mar 26, 2024

Conversation

Vitorgus
Copy link
Contributor

@Vitorgus Vitorgus commented Mar 20, 2024

I was trying to create a new file in the Firebase Storage with space characters in the file name, e.g:

Firebase.Storage.ref("path with space/image.png").put_file("res://image.png")

But it would fail, even if Firebase allows paths to have spaces. Turns out it was because the space character wasn't being encoded in the url parameter, only the forward slash / was.

Using the uri_encode() http_escape, it should encode both the slash and space characters, as well as any other character.

The target is set to main, but can switch to 4.x if that's the correct branch.

EDIT: my base branch was created from main, so updated to use the godot 3 function instead

Copy link
Collaborator

@WolfgangSenff WolfgangSenff left a comment

Choose a reason for hiding this comment

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

Yes, you'll need to change it to 4.x. If you want to also make this change for main, make sure you check what the correct call is - is it url_encode()? - for Godot 3.x. If you prefer to let us make the change, we'll figure it out after merging your change. Good find! Once the base is changed, I'll go ahead and merge.

@Vitorgus Vitorgus changed the base branch from main to 4.x March 22, 2024 17:33
@Vitorgus Vitorgus changed the base branch from 4.x to main March 22, 2024 17:35
@Vitorgus
Copy link
Contributor Author

When switching to the target branch 4.0, a lot of new diff appeared. I created my fix branch originally from main, so to avoid merging stuff from main (that seems to be the godot 3 version) into 4.0 (the godot 4 version), I replaced the function with the godot 3 equivalent http_escape(). Will open a new PR for the 4.0 properly.

Copy link
Collaborator

@WolfgangSenff WolfgangSenff left a comment

Choose a reason for hiding this comment

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

Looks good! I'll be merging the other one shortly as well.

@WolfgangSenff WolfgangSenff merged commit 62e0dad into GodotNuts:main Mar 26, 2024
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