Skip to content

Type-safe signals from Godot-defined structs #1108

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

Closed
atollk opened this issue Mar 30, 2025 · 5 comments · Fixed by #1111
Closed

Type-safe signals from Godot-defined structs #1108

atollk opened this issue Mar 30, 2025 · 5 comments · Fixed by #1111
Labels
c: engine Godot classes (nodes, resources, ...) feature Adds functionality to the library
Milestone

Comments

@atollk
Copy link

atollk commented Mar 30, 2025

I just saw that type-safe signals have recently been added to godot-rust, including a chapter in the book. That chapter solely focuses on signals defined in custom classes. Is there a way to use signals defined by the Godot library in types like Node in a type-safe way? Is this still work in progress (considering it's just on the master branch, not a released version)?

#[godot_api]
impl IArea2D for Player {
    fn ready(&mut self) {
        self.base().signals().body_entered().connect(/* ... */);
    }
}
@Bromeon Bromeon added feature Adds functionality to the library c: engine Godot classes (nodes, resources, ...) labels Mar 30, 2025
@Bromeon
Copy link
Member

Bromeon commented Mar 30, 2025

I just implemented this, see linked PR 🙂

@Bromeon Bromeon added this to the 0.3 milestone Mar 30, 2025
@atollk
Copy link
Author

atollk commented Mar 30, 2025

Awesome!
I noticed that it does work for some structs now but it still doesn't for some others. So, I understand that this is still in very active development then and it probably isn't a good idea to use signals for now?

@Bromeon
Copy link
Member

Bromeon commented Mar 31, 2025

I noticed that it does work for some structs now but it still doesn't for some others.

Can you be specific? So far, signals are only defined on the classes that declare them, not derived ones.
But what "doesn't work"?

So, I understand that this is still in very active development then and it probably isn't a good idea to use signals for now?

Yes, signal implementation is an incremental process, and 1 PR being merged doesn't mean "every feature one can ever wish for is now there" 😀 see also #1112 (comment).

Whether it's a good idea depends on your use case. I don't expect major breaking changes, and in several places it's already much better than the alternative (untyped signals). But of course, it still needs ironing out, and suggestions on how to implement things are welcome.


Fun coincidence: 😎

111111

@atollk
Copy link
Author

atollk commented Mar 31, 2025

Whether it's a good idea depends on your use case.

I meant more like, because it's an incremental process, when I start using type-safe signals now, I will probably run into missing or buggy functionality, at which point it is unclear to me if I should open another issue or if that's already known, just not implemented.

But what "doesn't work"?

Specifically, the Button struct does not implement WithSignals, so it cannot be used with the new interface.

@Bromeon
Copy link
Member

Bromeon commented Mar 31, 2025

Specifically, the Button struct does not implement WithSignals, so it cannot be used with the new interface.

That's because Button itself does not expose any signals, these are available on BaseButton.

Part of #1112 is to come up with a mechanism that allows inheritance of signals, to give the illusion that Button itself provides those signals (like in GDScript).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: engine Godot classes (nodes, resources, ...) feature Adds functionality to the library
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants