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

@export_file need to allow you to keep the old behavior of setting it as res path instead of uid if desired (breaking change!) #104379

Open
geekley opened this issue Mar 19, 2025 · 4 comments

Comments

@geekley
Copy link

geekley commented Mar 19, 2025

Tested versions

System information

Godot v4.4.stable (4c311cb) - Freedesktop SDK 24.08 (Flatpak runtime) on X11 - X11 display driver, Multi-window, 2 monitors - OpenGL 3 (Compatibility) - Mesa Intel(R) HD Graphics 5500 (BDW GT2) - Intel(R) Core(TM) i5-5300U CPU @ 2.30GHz (4 threads)

Issue description

The change to how @export_file works is a breaking change!
I was relying on the res path structure (changing .ogg extension of an audio file to an external .mp3 on web platform, which I'm playing via JS to avoid the audio issues on single thread and to stream it out of the pck).

This is bad because there's no indication that the actual path is an uid now unless you inspect the tscn -- the inspector still shows it as a res path, and since it's a string type, of course you'd expect it to retain its previous behavior consistent with @export_dir.
I can understand the reasoning for it is to update on refactoring, but I wish this was handled differently (e.g. keep track of it somehow, e.g. by adding a uid "metadata" instead of replacing it with a uid).

Since that's how it is now, please at least do these:

1. Document it properly

Docs for @export_file still say it's a "path to a file". An uid is not what one would expect from this wording.
More importantly, please add this as a breaking change in Upgrading from Godot 4.3 to Godot 4.4 docs page.

2. Please let us decide if we want an uid or path

There's valid use cases for preferring a res path too!
If possible, I suggest adding a parameter on @export_file to keep the old behavior if desired. E.g:

@export_file(as_uid: bool, filter: String = "", ...) vararg # if false, export by path; if true export by uid
@export_file(filter: String = "", ...) vararg # use a default from project settings

3. Add a project setting so we can choose the default mode

This would apply when @export_file doesn't specify the parameter, so we can keep the old behavior when upgrading to Godot 4.4.
You can make it by res path by default (for upgrading projects) but set it to uid on new projects.

4. Please add clarity in the inspector

Add some UI in the inspector so we can know if its a uid or res path.
E.g. a checkbox so we can toggle between uid or res path.
Or if it's not toggleable directly, an icon that shows when it's actually an uid.
Or maybe show it as the uid and show the inferred res path on hover in a tooltip or below the field.

Steps to reproduce

Just make a script with @export_file and set a project file in the inspector.
Look at the tscn and notice it's actually a uid, even though the inspector say it's a res path.
If you were relying on the previous behavior, your project will break on 4.4.

Example use cases that break:

  • Changing extension of a file to play a different external audio on web, that you need out of the pck.
  • Get resources dynamically relying on path structure e.g. in a custom localization solution where you may want to add/change a locale folder dynamically in the res path.
  • Any behavior that changes dynamically based on an expected consistent path structure.

Minimal reproduction project (MRP)

N/A

@etherealxx
Copy link
Contributor

I agree that this is a breaking change. While it's still possible to get the path from the UID, it's counterintuitive since the Inspector UI display the path when it's actually an uid path. Some of my functions that expect string path is broken.

Also if you're having an exported array of file paths from a 4.3 project, and upgrading to 4.4, the existing array items will still be file path, while if you add a new one from inspector, it'll be an UID path. So there's two different type of path in a single exported variable.

My suggested solution (aside from that new parameter, if it's possible) would be a project setting that decide the default behavior of @export_file, either using UID path or the regular string path. And probably make a new one (@export_uid or something) that always return UID path for those who prefers that

@dalexeev
Copy link
Member

In my opinion, we should restore the old behavior (for compatibility and consistency reasons) and not introduce an annotation parameter (since it is incompatible) or a project setting (since it is error-prone). Instead, we could add a new dedicated export hint and annotation @export_file_as_uid, @export_file_uid or @export_uid.

@etherealxx
Copy link
Contributor

@dalexeev but what about those who are new with 4.4 and already used to the new method? That's another breaking change for them, if the change is supposed to be in a patch (4.4.x) update.

@dalexeev
Copy link
Member

I think it's better to break for a small number of new users (4.4 was released very recently) and it's unlikely that UID -> path will break anyone's expectations as much as path -> UID. Excessive configurability is bad, in my opinion the behavior of @export_file should be the same for all projects, regardless of the project settings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Unassessed
Status: For team assessment
Development

No branches or pull requests

4 participants