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

Add resource path string literals in gdscript #9512

Open
Gnumaru opened this issue Apr 13, 2024 · 5 comments
Open

Add resource path string literals in gdscript #9512

Gnumaru opened this issue Apr 13, 2024 · 5 comments

Comments

@Gnumaru
Copy link

Gnumaru commented Apr 13, 2024

Describe the project you are working on

more than one godot project

Describe the problem or limitation you are having in your project

String literals for resource paths in gdscript are error prone. A project refactoring can easily break a lot of things since the now invalid resource path string literals will lead to runtime errors instead or parse time errors and may only be noticed a lot of time after the refactoring. The preload function issues a parser error while trying to preload an inexistent resource path, but not the load function and neither FileAccess.open or any other function dealing with file/dir paths which may be resource paths.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Create a new string literal similar to the raw string literal, preceded by 'respath' (or, if it really needs to be a single character, 'p') which does in parse time the exact same validation that already happen on preload. This way the same safety that preload has can be used in any other place, like in load or in FileAccess.open

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

using a string delimiter preceded by respath

respath"res://asdf.tres"

or res to be shorter

res"res://asdf.tres"

or even p if we need it to be a single character

p"res://asdf.tres"

The parser will, at parse time, check for the existence of that path (regardless if is a directory or file). If the path does exist, the parse will be successful. If the path does not exist, it will issue a parser error identical to the one that already exists in the preload function

If this enhancement will not be used often, can it be worked around with a few lines of script?

This is a gdscript parser feature. The easiest way to work around it is to use a tool or external script to procedurally generate a fake enum (a named class with a bunch of string constants) with one entry for every dir/file path in the project, and never again use string literals, just the constants in the enum. This way any project refactoring (as long as followed by the regeneration of the enum) will lead to compilation errors due to scripts using inexistent constants in the enum

Is there a reason why this should be core and not an add-on in the asset library?

gdscript itself is part of core and no changes to the parser can be made an add-on.

@AThousandShips
Copy link
Member

@Gnumaru
Copy link
Author

Gnumaru commented Apr 13, 2024

See also:

This other proposal is similar yet different.

My whole point is to have the editor to yield gdscript parser errors whenever a string literal for an nonexistent resource path is used. It does not seem feasible or even desirable to do this for any kind of string literal, so I proposed a special prefixed string literal similar to the raw string.

With this we could not only refer do directories instead of files, but also refer to non importable files, like for example a xlsx file that get's exported as-is along with the project. Even for edit time only usage, we can refer inside tool scripts to files and folders that only exist at "edit time" so that the tool scrips can do certain automation using these files.

@AThousandShips
Copy link
Member

This other proposal is similar yet different.

Hence why I didn't close this as a duplicate

But also consider using UID paths to retain stability

@dalexeev
Copy link
Member

I think this is too specific a thing to add to the language. If we do decide to add something similar, I would prefer it to be a function-like keyword, since there is preload(). Let's say respath(path: String) -> String, which works as you described, but also supports constant expressions like respath(MY_DIR + "/file.txt").

A more general approach was to add compile-time functions to the language so that you could implement it yourself. But this is a difficult task that is not planned at the moment.

Another approach is to create an EditorScript that will scan project files and warn about invalid paths:

Example
@tool
extends EditorScript

var _res_path_regex: RegEx = RegEx.create_from_string(r"res://[\w/.]+")

func _run() -> void:
    _check_dir(EditorInterface.get_resource_filesystem().get_filesystem())

func _check_dir(dir: EditorFileSystemDirectory) -> void:
    var dir_path: String = dir.get_path()
    if dir_path == "res://addons/":
        return

    for i: int in dir.get_subdir_count():
        _check_dir(dir.get_subdir(i))

    for i: int in dir.get_file_count():
        var file_name: String = dir.get_file(i)
        var file_path: String = dir.get_file_path(i)
        var file_extension: String = file_name.get_extension().to_lower()

        if not dir.get_file_import_is_valid(i):
            printerr('File "%s" import is invalid. Skipped.' % file_path)
            continue

        var file_content: String
        const EXCLUDED_EXTENSIONS: Array[String] = ["png", "svg"]
        if not EXCLUDED_EXTENSIONS.has(file_extension):
            file_content = FileAccess.get_file_as_string(file_path)

        for dependency_path: String in _get_dependencies(file_path, file_content):
            if not DirAccess.dir_exists_absolute(dependency_path) \
                    and not ResourceLoader.exists(dependency_path):
                printerr('File "%s" does not exist!' % dependency_path)

func _get_dependencies(file_path: String, file_content: String) -> Array[String]:
    var result: Array[String] = []
    for dependency: String in ResourceLoader.get_dependencies(file_path):
        result.append(dependency.get_slice("::", 2))
    if not file_content.is_empty():
        for regex_match: RegExMatch in _res_path_regex.search_all(file_content):
            var dependency_path: String = regex_match.get_string()
            if not result.has(dependency_path):
                result.append(dependency_path)
    return result

@KoBeWi
Copy link
Member

KoBeWi commented Apr 14, 2024

A project refactoring can easily break a lot of things since the now invalid resource path string literals will lead to runtime errors instead or parse time errors and may only be noticed a lot of time after the refactoring.

This can be easily avoided by doing search and replace for the file you just moved.

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

No branches or pull requests

4 participants