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 type hints for signal parameters #2557

Open
nathanfranke opened this issue Apr 6, 2021 · 8 comments · May be fixed by godotengine/godot#71952
Open

Add type hints for signal parameters #2557

nathanfranke opened this issue Apr 6, 2021 · 8 comments · May be fixed by godotengine/godot#71952

Comments

@nathanfranke
Copy link
Contributor

See godotengine/godot#26045

Describe the project you are working on

Godot

Describe the problem or limitation you are having in your project

Signals don't have type hints, meaning helper docs are bad and runtime exceptions are more probable

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

Type hint syntax for signal declarations, similar to function signatures

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

signal released(player: Player)

image

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

This will be used often.

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

Signals are part of core

@Calinou Calinou changed the title Type hints for signal parameters Add type hints for signal parameters Apr 6, 2021
@Calinou Calinou added this to the 4.1 milestone Apr 6, 2021
@Calinou
Copy link
Member

Calinou commented Apr 6, 2021

Quoting vnen from godotengine/godot#26045 (comment):

On Godot 4.0 there's already too much stuff and I don't really have time to tinker about this one, so the answer is: no, this is won't available in Godot 4.0.

It's still a similar issue: signals don't really have a type in core so those are never checked. Even with the stringless syntax, without the core enforcing this all the time it may break type expectations. Could be done as only a compile-time hint but I prefer to implement things properly.

@YuriSizov
Copy link
Contributor

YuriSizov commented Apr 6, 2021

It's still a similar issue: signals don't really have a type in core so those are never checked.

I wouldn't mind types in signals in core either. As I understand, everything is a Variant now, which makes some value passthrough impossible (like enums instead of ints, or everything complex defaulting to Object). Which results in boilerplate code where none is really needed, ideally.

@dalexeev
Copy link
Member

dalexeev commented Apr 8, 2021

Even if, for some reason, strict checking cannot be done, it would be nice to add the ability to at least nominally specify types so that they are automatically copied into the callback signature, as is the case with built-in signals.

extends Button

# Before:
signal my_signal(my_arg) # my_arg: int
# I indicate the type in the comment, for myself.

# After:
signal my_signal(my_arg: int)
# Even if the correspondence of a value to the specified type
# is not checked either statically or dynamically (in emit_signal),
# the type is specified simply as metadata for the editor.


# For built-in signals, everything works correctly:
func _on_Button_toggled(button_pressed: bool) -> void:
    pass # Replace with function body.


# Before:
func _on_Button_my_signal(my_arg) -> void:
    pass # Replace with function body.
# I have to manually specify the type.

# After:
func _on_Button_my_signal(my_arg: int) -> void:
    pass # Replace with function body.
# The type is specified automatically.

We can do this for both 4.0 and 3.x, and add full type checking in 4.1.

Also type hints are useful in user class documentation.

@me2beats
Copy link

me2beats commented Apr 12, 2021

Such syntax should be allowed at least (and I wish it could be implemented in 3.4):

signal released(player: Player)

This could do/mean nothing (at first), just be syntactically allowed (no errors).

@Calinou
Copy link
Member

Calinou commented Jun 1, 2021

This could do/mean nothing (at first), just be syntactically allowed (no errors).

I once asked about that and vnen wasn't really in favor of it, as it could be misleading. All other type hints in GDScript have some kind of "semantic" meaning. If we add type hints with the same syntax that don't do anything, it could end up being confusing.

@TylerGubala
Copy link

It would really be nice to have this feature even as just a linter feature where the GDScript editor would just make a warning because it helps to document your code and sometimes its tough coming back into something only to find that you are passing a signal wrong because you had a mistaken assumption.

@Calinou Calinou linked a pull request Feb 1, 2023 that will close this issue
5 tasks
@voithos
Copy link

voithos commented Dec 21, 2023

Was this fixed recently? I just tried this in 4.2.1 (because I had enabled more GDScript type warnings, and was getting a warning that some signal params were untyped), and it seemed to just work.

@dalexeev
Copy link
Member

Was this fixed recently?

Well, signals do have type hints. They are taken into account in the user documentation, in the Signals dock, when generating a callback, with the UNTYPED_DECLARATION warning.

However, signals are still untyped. You can pass any number of arguments of any type to emit(), and this will not cause an error (only in callbacks). However, native signals also have the problem, so it cannot be said that GDScript behaves worse than the core in this regard.

Considering that type hints and typed signals are different things, we could close this proposal and open a new one. I didn't find a proposal for typed signals for core/GDScript, only for C# (#7891).

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

Successfully merging a pull request may close this issue.

9 participants