Signals

[Bromeon]

See how we can support signal registration, in the builder API and/or proc-macros.

[lilizoey]

It is probably possible to make signals into futures. allowing us to do self.signal().await in rust too. this could probably be done before we implement coroutines as async methods too, which would limit their use a bit but there are people who use things like tokio alongside gdext.

[0awful]

Is their an appetite for a shorter term solution?

I'd like the #[signal] macro to do the following:

#[signal]
pub fn hit()

to expand to

pub fn hit() {
    self.base.emit_signal("hit".into(), &[])
}

In the case of:

#[signal]
pub fn message(some_message: String) {}

It could expand to

pub fn message(some_message: String) {
  self.base.emit_signal("message".into(), &[some_message])
}

It wouldn't currently allow for await semantics, but it would make the #[signal] macro functional rather than a placeholder.

There's two gotchas that I can see. First being "what if there isn't a base?" I'm imagining that would be solved by trait bounds. The tougher issue is "what if the base isn't the .base param?" I'm sure there's a way to get around that, as that has got to have come up somewhere before.

If you are open to this I'm interested in giving implementing it a go. Just don't want to do stuff up if it isn't a direction you're looking for or something that doesn't line up with your prioritization scheme.

[Dheatly23]

I kinda need signal (it's the only major blocker in my project), so you have my go ahead.

My only suggestion is that the signal method should return a struct. Then more methods can be added like emit() or connect(). In GDScript it's already like that.

With regards to .await, it can be emulated via one-time Callable, passing the future along the way. Not the most ergonomic, but i guess it's fine? Anyways, doing async right now is probably not happening any time soon due to Send constraint.

[Bromeon]

I don't think inherent methods are the right abstraction for signals; there is no meaningful method body the user can provide. They should be fields, possibly ZST. This would also allow connect() in addition to emit() functionality. It can mean some useless field initialization like with PhantomData, but a reasonable Default impl would allow .. syntax in structs.

Ideally this is done in a way that extends naturally to a later builder API; i.e. any sort of magic added by proc-macros should also be possible to implement in a procedural, programmatic way (see #4). Just as a preparation, I would not want to start the builder API yet.

For example, one approach would be to include the signature as a generic argument.

#[signal]
message: Signal<Fn(String) -> ()>

[ambeeeeee]

It's been almost a year and I don't know if any discussion has happened in the discord but I could imagine signals continuing to look like this:

#[derive(GodotClass)]
#[class(init)]
pub struct MyRefCounted {
    base: Base<RefCounted>,
}

#[godot_api]
impl MyRefCounted {
    #[signal]
    fn foo_signal();

    #[signal]
    fn bar_signal(parameter: Gd<Node>) -> String;
}

So from a magic macro DSL standpoint, nothing has changed. However, behind the scenes, the following code would be generated:

#[godot_api]
impl MyRefCounted {
    // ZST definitions for FooSignal and BarSignal generated by macro

    fn foo_signal() -> FooSignal {
        // ...
    }

    fn bar_signal() -> BarSignal {
        // ...
    }
}

The generated types would expose an API as you suggest for the member-based solution:

struct FooSignal;

// or trait impl style macro magic, same difference
impl FooSignal {
    pub fn connect(/* signature is implementation detail, can't remember precise callable logic impl */) { } 

    pub fn emit(source: &mut MyRefCounted) {}
}

So that'd allow you to interact with signals as follows:

#[godot_api]
impl MyRefCounted {
    #[signal]
    fn foo_signal();

    #[func]
    fn callback() {
        // do things here
    }
}

#[godot_api]
impl INode for MyRefCounted {
    fn on_ready(&mut self) {
        self.foo_signal().emit(self);

        self.foo_signal().connect(Callable::from_object_method(correct_value, "callback"));
    }
}

I think this better models the relationship between signals and Godot classes. The way they work is so detached from the struct definition of a godot class in Rust that I'd push for them to continue using the function representation.

That said, this is firmly in the territory of macro magic. We're rewriting a function signature entirely differently than how it exists in the source code, and adding magic syntax to signals. I'd definitely argue that this is justified (although that's a given because I am currently suggesting it).

And as you said, you would like to see things be able to be implemented in both builder patterns and macro magic. This system could also be visualized as a half-built builder. If you were to imagine the generated builder to be a specialized kind of SignalBuilder that abstracts over emit and connect operations given a signal's name and signature.

This could be imagined as follows, but it may not fall in line with your vision for a builder API in the future:

#[signal]
fn bar_signal(parameter: Gd<Node>) -> String;

pub fn bar_signal() -> SignalBuilder<Fn(Gd<Node>) -> String> {
    // Instead of a specialized type, we instead provided a pre-initialized builder
    // that you could have just used without the macro magic
    SignalBuilder::new("bar_signal")
} 

So we'd be able to just re-use the procedural builder and unify the usage of the pattern with this solution, but the #[signal] macro would 100% be "macro magic" and its not a decision to make lightly.

[Bromeon]

Very interesting, thanks a lot for the detailed thoughts! :+1:

I agree that the function syntax of #[signal] is kinda nice and straightforward, especially when coming from GDScript. I'll reflect a bit upon this and see if I can find ways to limit the macro magic while still providing decent UX.

[fpdotmonkey]

I would argue for implementing signals as members of the associated object, e.g.

#[derive(GodotClass)]
#[class(base = Object)]
struct GrassController {
    #[export]
    highlight: Color,
    #[var]
    animation_frame: u32,
    grass_cut: Signal<(/* grass_id */ u32, /* how_cut */ CutKind)>,
    base: Base<Object>,
}

Signal would then have the expected methods. There likely would need to be some code gen like what @ambeeeeee has suggested to make the signal publicly accessible within rust, e.g. GrassController::signal_grass_cut(...). The derive macro would automatically pick up the Signals for code gen in the same way it already picks up Base.

The benefits of this approach:

• The exported attributes of the class are almost all in one place
  • Exceptions are #[func] which are rightly in an impl since they're actual functions with implementations and constants, which can only be in an impl in rust
• Type safety on account of the typed arg tuple
• No stringy signal names
• Syntax is similar to GDScript in how signals are declared with a name and arg tuple and are conventionally declared at the top of the source file along with @exports and vars
• Signal<ArgTuple> and associates could be generated for engine types, which would make the API experience a bit better, too.

The drawbacks of this approach:

• Reasonable implementations would make this a major breaking change, requiring that it be put off for 1.X or a later major version.
• No mechanism for attaching names to the individual args
• Without variadics, it may be challenging to create a good signature for Signal::emit.
  • fn emit(arg_tuple: (...)) is definitely possible and better than the current state, but without arg names, the design intent of the signature is unclear.
• Generating the requisite fn signal_{signal_name}(...) methods may demand that the user have an inherent impl that they wouldn't otherwise.
• It lacks FFI type safety, namely that all arg tuple members must be To/FromGodot, but there's no way to specify that type constraint on Tuples in Rust

[Bromeon]

Great list! Just playing devil's advocate:

The benefits of this approach:

• Type safety on account of the typed arg tuple
• No stringy signal names

#[signal] in impl blocks can generate code that is both type-safe and has distinct types.

But you have a point that a struct-based solution would be closer to a potential builder API. But with proc-macros, this isn't an actual benefit.

---

• Syntax is similar to GDScript in how signals are declared with a name and arg tuple and are conventionally declared at the top of the source file along with @exports and vars

Not sure I agree: function-based declaration syntax is much closer to GDScript, and can use named parameters. These could be used in a generated emit as well.

Place of declaration isn't a strong argument -- we already have fn and const inside impl blocks. The latter are also "conventionally" on top.

---

Reasonable implementations would make this a major breaking change, requiring that it be put off for 1.X or a later major version.

In Cargo-SemVer, 0.x minor versions behave like major versions. So we can make breaking changes in 0.2, 0.3 etc.

This particular change has also been communicated for eternities:

---

Generating the requisite fn signal_{signal_name}(...) methods may demand that the user have an inherent impl that they wouldn't otherwise.

There is already codegen that generates its own impl blocks. Since you can have multiple impls, it should be fine. It puts some restriction on where #[godot_api] appears (in another module can cause trouble).

---

It lacks FFI type safety, namely that all arg tuple members must be To/FromGodot, but there's no way to specify that type constraint on Tuples in Rust

There is, via custom trait -- it's what we do in signature.rs. But it bruteforces all possible tuple lengths (which is anyway needed, due to lack of variadics).

[ambeeeeee]

There's a lot of discussion here, but I'd like to point out that the motivation behind my specific suggestions were to come up with a way to have both #[signal] and the builder coexist, through the macro just simplifying the definition and interaction at the call site.

No stringy signal names

#[signal(rename = "")] syntax like #[func]

Signal and associates could be generated for engine types, which would make the API experience a bit better, too.

This in general brings up the idea that whatever we put into place here should also go into place for engine types as they have plenty of signals associated with their types.

edit after the fact: are there any obvious challenges that would come with injecting signals as struct members in code generated from the engine types? What about serialization codegen, how do we know that the magic signal structs are not needed to be serialized?

Without variadics, it may be challenging to create a good signature for Signal::emit

Well damn that's an oversight and a half on my end.

Some discussion on Signal, this is what it would ultimately look like after making trade-offs for actually getting to express the signal that way:

Signal<(String,)>

Single value tuples are a bit of a weird concept for Rust devs who haven't been familiarized with why we reach for them, and can sometimes be something they gloss over. This can be a problem, but it would be fine with documentation. I really do think losing any documentation on the roles of the individual parameters is something to be afraid of here.

I think whatever we do there is a clear pathway to do something with macros that complements procedural builders, without feeling like we're just duplicating the effort involved.

I personally feel like initializing a user-defined GodotClass can have unique challenges that you won't find anywhere else already, and this would introduce magical members in all of the backing struct types.

The derive macro would automatically pick up the Signals for code gen in the same way it already picks up Base.

I can't believe that a derive macro is picking up on a type in the type system. Isn't that a bad idea?

Looking into how it works, it doesn't really break anything when you trick the macros gdext has into accepting the wrong type. by just naming a random type Base. Obviously since it's passed directly into some codegen that requires it to be the correct type, it works out in the end.

So it wouldn't "break" anything to have the macro pick up on another special type in the type system, but I guess I underestimated how much macro magic is already in gdext.

[StatisMike]

I am more inclined towards including signals in impl rather than as a struct field. Proposal by @ambeeeeee sounds nice, and I have similar reservation for including this as a field: as I've been working on custom resource format for gdext-defined Resources utilizing serde, the somewhat special Base field already brought in some troubles. I havent been able to work around, which for a time being discards signals from them.

Nonetheless, it adds additional special, potentially useless field if the signal ends up not being used (eg. if class is extended in gdscript and signal won't get emitted).

On @fpdotmonkey suggestion, the special field problem could potentially be worked around in such circumstances. If the overhead of creating some Signal structure every time before emitting would be non-negligible, I would deem it a valid concern. Though I don't see readability benefits of the signal being patched onto the object structure, I think it being in the impl is more clear. Keeping impl block cleaned up (eg. consts at the top, then signals, then members) is more on the user side and preference and I don't see a reason to force it in other ways.

[Bromeon]

edit after the fact: are there any obvious challenges that would come with injecting signals as struct members in code generated from the engine types?

This is actually a very good point. We don't have access to the struct definition inside the #[godot_api] macro, so we simply cannot add fields to the struct definition.

As a function

What we could do however is add new methods, and extra state that's dynamically registered in the base field.

// All generated by #[godot_api]:

impl MyClass {
    fn signals(&'a self) -> MyClassSignals<'a>
}

struct MyClassSignals<'a> {}
impl<'a> MyClassSignals<'a> {
     // These would look up some dynamic property behind self.base...
     fn on_created() -> Signal<...>;
     fn on_destroyed() -> Signal<...>;
}

Usage:

self.signals().on_created().emit(...);

As a field

But then again, this would also be possible with fields.

struct MyClass {
    on_created: Signal<...>,
    on_destroyed: Signal<...>,
}

Which, as was mentioned, has the advantage that it's less magic and closer to a procedural approach.

self.on_created.emit(...);

---

Proc-macro inference

I can't believe that a derive macro is picking up on a type in the type system. Isn't that a bad idea?

Looking into how it works, it doesn't really break anything when you trick the macros gdext has into accepting the wrong type. by just naming a random type Base. Obviously since it's passed directly into some codegen that requires it to be the correct type, it works out in the end.

So it wouldn't "break" anything to have the macro pick up on another special type in the type system, but I guess I underestimated how much macro magic is already in gdext.

It's not great as in "most idiomatic" but since the guy who wanted to introduce actual reflection was un-invited from RustConf last year, we likely won't have anything better than proc-macros this decade, so we need to be creative. In practice, detecting types like Base works very well and we even provide #[hint] to help the macro in ambiguous cases. As you say, it's important that actual type checks still happen behind the scenes.

Note that removing an explicit attribute #[base] was a quite popular change. But as listed in that issue, it's not the only place where proc-macros detect something (not just types but presence in general).

I see these things very pragmatic and gladly deviate from best practices™ when it helps ergonomics. Abusing Deref for inheritance already secured us a place in hell 🔥

[fpdotmonkey]

Re: de/serialization of Signal, I would imagine this could be largely mitigated by giving a sensible default impl for serde::{Serialize, Deserialize}. Namely no-op, since my intuition would be that serializing a callback wouldn't often be needed. That said, I'm open to being told I'm wrong.

[fpdotmonkey]

// All generated by #[godot_api]:

impl MyClass {
    fn signals(&'a self) -> MyClassSignals<'a>
}

struct MyClassSignals<'a> {
     // These would look up some dynamic property behind self.base...
     fn on_created() -> Signal<...>;
     fn on_destroyed() -> Signal<...>;
}

This to me seems a very neat compromise, perhaps even without the gen by macros. I could imagine something like this.

#[derive(GodotClass)]
#[class(signals=GrassControllerSignals, ...)]
struct GrassController {
    // no Signal members
}

#[derive(GodotSignals)]  // perhaps a compile error if not all members are the appropriate type
struct GrassControllerSignals<'a> {
    fn grass_cut() -> Signal<...>,
}

my_grass_controller.signals().grass_cut().emit(...);

[Bromeon]

Interesting approach, as well!

You declared a fn inside the struct, did you mean to use a field of type Signal<...>? Edit: I made a mistake with struct vs. impl and was confused seeing this mistake :grin: all good

In practice, this would however require one more block for users (or even two if you declare actual fn inside their impl), which can be a bit heavy...

[fpdotmonkey]

You declared a fn inside the struct, did you mean to use a field of type Signal<...>?

I copy-pasted what you had written. I understood it as a shorthand for a my_signal: Fn() -> Signal<...> or similar, but it certainly could be a Signal.