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! 👍

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 😁 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.

[Houtamelo]

Anyone working on this?
I can make a prototype if not.

[Bromeon]

Would be appreciated! Also, don't hesitate to further discuss the design before spending a lot of time on implementation (or whichever approach works best for you 🙂 ). I won't have that much time to review in the next 2 weeks, so no need to rush.

[Houtamelo]

I've been trying a few designs now that I finally got time to work on this.

Based on what's discussed above, these are my goals:

1. • Signals should not rely on proc-macros to work. Users should be able to reasonably build and use signals on their own.
2. • Type safety: emit/connect should use Rust's type system to enforce correct types at compile time.
3. • Emit/Connect should not use tuples as arguments, instead they should have methods that take any number of arguments depending on the signal.
4. • Ideally, Emit/Connect should have properly named parameters.

After some hours of trying different things, I ended up with 2 solutions, and neither meets all the points above.

The 1st solution satisfies: 1, 2 and 3.
The 2nd solution satisfies: 2, 3 and 4.

---

Shared

I'll start with what's shared between both solutions: the definition of the signal struct and most of the methods provided in GDScript: https://docs.godotengine.org/en/stable/classes/class_signal.html

#[derive(Debug, Clone)]
pub struct Signal<T> {
	obj: Gd<Object>,
	name: StringName,
	#[doc(hidden)]
	_pd: PhantomData<T>,
}

impl<S> Signal<S> {
	pub fn new<Class>(obj: &Gd<Class>, name: impl Into<StringName>) -> Self
		where Class: Inherits<Object>
	{
		Self {
			obj: obj.clone().upcast(),
			name: name.into(),
			_pd: PhantomData,
		}
	}
	
	pub fn connect(&mut self, func: impl Into<Callable>, flags: ConnectFlags) {
		self.obj
			.connect_ex(self.name.clone(), func.into())
			.flags(flags.ord() as u32)
			.done();
	}

	pub fn connect_deferred(&mut self, func: impl Into<Callable>) {
		self.obj
			.connect_ex(self.name.clone(), func.into())
			.flags(ConnectFlags::DEFERRED.ord() as u32)
			.done();
	}
	
	pub fn get_connections(&self) -> Array<Dictionary> {
		self.obj.get_signal_connection_list(self.name.clone())
	}
	
	pub fn get_name(&self) -> StringName {
		self.name.clone()
	}
	
	pub fn get_object(&self) -> Gd<Object> {
		self.obj.clone()
	}
	
	pub fn get_object_id(&self) -> InstanceId {
		self.obj.instance_id()
	}
	
	pub fn is_connected(&self, func: impl Into<Callable>) -> bool {
		self.obj.is_connected(self.name.clone(), func.into())
	}
	
	pub fn is_null(&self) -> bool {
		!self.obj.is_instance_valid() || !self.obj.has_signal(self.name.clone())
	}
}

impl<T> PartialEq for Signal<T> {
	fn eq(&self, other: &Self) -> bool {
		if !self.obj.is_instance_valid() || !other.obj.is_instance_valid() {
			return false;
		}
		
		self.obj.instance_id() == other.obj.instance_id() && self.name == other.name
	}
}

Note that emit isn't present, each solution handles it in a different way.

Both solutions would share the same proc-macro input, which is exactly the same as the current implementation:

#[derive(GodotClass)]
#[class(no_init, base = Object)]
struct MyClass {
	base: Base<Object>,
}

#[godot_api]
impl MyClass {
	#[signal]
	fn child_entered(child: Gd<Node>) {}
}

---

1st solution - Use declarative macros to provide method impls that convert generic tuples into separate arguments

impl<S> Signal<S> {
	pub fn connect(&mut self, func: impl Into<Callable>, flags: ConnectFlags) {
		self.obj.connect_ex(self.name.clone(), func.into())
			.flags(flags.ord() as u32)
			.done();
	}

	pub fn connect_deferred(&mut self, func: impl Into<Callable>) {
		self.obj.connect_ex(self.name.clone(), func.into())
			.flags(ConnectFlags::DEFERRED.ord() as u32)
			.done();
	}
}

macro_rules! impl_variadic_args_methods {
    ($($arg: ident: $T: ident),*) => {
	    #[allow(clippy::too_many_arguments)]
	    #[allow(unused)]
	    impl<$($T: FromGodot + ToGodot),*> Signal<($($T,)*)> {
		    pub fn connect_fn<F>(&mut self, mut func: F, flags: ConnectFlags)
		        where F: FnMut($($T),*) + 'static + Send + Sync
		    {
			    let callable = Callable::from_fn("lambda", move |args| {
				    let mut idx = 0;
				    
				    $(
				        let $arg = args[idx].to::<$T>();
				        idx += 1;
				    )*
				    
				    func($($arg,)*);
				    Ok(Variant::nil())
			    });
			    
			    self.connect(callable, flags);
		    }
		    
		    pub fn connect_fn_deferred<F>(&mut self, func: F)
		        where F: FnMut($($T),*) + 'static
		    {
			    let mut func = AssertThreadSafe(func);
			    
			    let callable = Callable::from_fn("lambda", move |args: &[&Variant]| {
				    let mut idx = 0;
				    
				    $(
				        let $arg = args[idx].to::<$T>();
				        idx += 1;
				    )*
				    
				    func($($arg,)*);
				    Ok(Variant::nil())
			    });
			    
			    self.connect_deferred(callable);
		    }
		    
		    pub fn emit(&mut self, $($arg: $T),*) {
			    self.obj.emit_signal(self.name.clone(), &[$($arg.to_variant()),*]);
		    }
	    }
    };
}

impl_variadic_args_methods!();
impl_variadic_args_methods!(arg: T);
impl_variadic_args_methods!(arg_1: T1, arg_2: T2);
impl_variadic_args_methods!(arg_1: T1, arg_2: T2, arg_3: T3);
impl_variadic_args_methods!(arg_1: T1, arg_2: T2, arg_3: T3, arg_4: T4);
impl_variadic_args_methods!(arg_1: T1, arg_2: T2, arg_3: T3, arg_4: T4, arg_5: T5);
impl_variadic_args_methods!(arg_1: T1, arg_2: T2, arg_3: T3, arg_4: T4, arg_5: T5, arg_6: T6);
impl_variadic_args_methods!(arg_1: T1, arg_2: T2, arg_3: T3, arg_4: T4, arg_5: T5, arg_6: T6, arg_7: T7);
impl_variadic_args_methods!(arg_1: T1, arg_2: T2, arg_3: T3, arg_4: T4, arg_5: T5, arg_6: T6, arg_7: T7, arg_8: T8);

What happens here is that for each combination of tuple sizes, we create method impls that "unwrap" those tuples into separate arguments.

We can't properly name arguments because those are inherent impls that are written in Godot-Rust.

Code generated by proc macro:

pub trait MyClass_child_entered {
	#[must_use]
	fn child_entered(&mut self) -> Signal<(Gd<Node>,)>;
}

impl MyClass_child_entered for MyClass {
	fn child_entered(&mut self) -> Signal<(Gd<Node>,)> {
		Signal::new(&self.to_gd(), "child_entered")
	}
}

impl MyClass_child_entered for Gd<MyClass> {
	fn child_entered(&mut self) -> Signal<(Gd<Node>,)> {
		Signal::new(self, "child_entered")
	}
}

// Which can be used like this:

fn use_signal(my_class: &mut Gd<MyClass>) {
	my_class.child_entered()
		.connect_fn_deferred(|node| {
			godot_print!("Child entered: {:?}", node.get_name());
		});
}

I used a trait because I wanted to allow fetching the signal from both T and Gd<T>, inherent impls would be fine if we weren't cucked by orphan rules.

If fetching the signal from Gd<T> isn't desired, we can scrap the trait and just provide an inherent impl for T.

---

2nd solution - Proc-macros + zero-sized types

All code specific to this solution would be proc-macro generated:

pub struct child_entered;

#[allow(unused)]
pub trait Signal_child_entered {
	fn connect_fn<F>(&mut self, f: F, flags: ConnectFlags) where F: FnMut(Gd<Node>) + 'static + Send + Sync;
	
	fn connect_fn_deferred<F>(&mut self, f: F) where F: FnMut(Gd<Node>) + 'static;
	
	fn emit(&mut self, child: Gd<Node>);
}

#[allow(unused)]
impl Signal_child_entered for Signal<child_entered> {
	fn connect_fn<F>(&mut self, mut f: F, flags: ConnectFlags)
		where F: FnMut(Gd<Node>) + 'static + Send + Sync
	{
		self.connect(Callable::from_fn("lambda", move |args: &[&Variant]| {
			let mut idx = 0;
			
			let child = args[idx].to::<Gd<Node>>();
			idx += 1;
			
			f(child);
			Ok(Variant::nil())
		}), flags);
	}

	fn connect_fn_deferred<F>(&mut self, f: F)
		where F: FnMut(Gd<Node>) + 'static
	{
		let mut f = AssertThreadSafe(f);

		let callable = Callable::from_fn("lambda", move |args: &[&Variant]| {
			let mut idx = 0;

			let child = args[idx].to::<Gd<Node>>();
			idx += 1;

			f(child);
			Ok(Variant::nil())
		});

		self.connect(callable, ConnectFlags::DEFERRED);
	}

	fn emit(&mut self, child: Gd<Node>) {
		self.obj.emit_signal(self.name.clone(), &[child.to_variant()]);
	}
}

pub trait MyClass_child_entered {
	#[must_use]
	fn child_entered(&mut self) -> Signal<child_entered>;
}

impl MyClass_child_entered for MyClass {
	fn child_entered(&mut self) -> Signal<child_entered> {
		Signal::new(&self.to_gd(), "child_entered")
	}
}

impl MyClass_child_entered for Gd<MyClass> {
	fn child_entered(&mut self) -> Signal<child_entered> {
		Signal::new(self, "child_entered")
	}
}

---

Both solutions can be used like this, except the 2nd shows the proper parameter name:

fn use_signal(my_class: &mut Gd<MyClass>) {
	my_class.child_entered().connect_fn_deferred(|node| {
		godot_print!("Child entered: {:?}", node.get_name());
	});

	let node = Node::new_alloc();
	my_class.child_entered().emit(node);
}

---

Personally, I'm okay with both, but I prefer the first solution, I don't think parameter names are worth sacrificing proc-macro "independence".

I can start the actual implementation when a design is agreed upon.

[Houtamelo]

Did some tinkering again today, and I realized something: we can combine both solutions presented in my post above.

We can keep the tuple implementations from the 1st, but use the 2nd to generate code with the proc macro.

The result:

• Users will still be able to easily define their own signals, but their hand-made ones won't have parameters properly named.
• The proc-macro generated signals will have parameters with proper names, we can also use it to generate all the godot's built-in signals.

[fpdotmonkey]

my_class.child_entered().connect_fn_deferred(|node| {

To me, it's not obvious that my_class.child_entered() is a constructor for a signal. Of course .connect_fn_deferred() indicates that, but I think it'd be valuable to ensure than the method name is clear. Perhaps making it .signal_child_entered() or putting the constructor methods on an intermediate signal struct, e.g. my_class.signals().child_entered()

let mut func = AssertThreadSafe(func);

Is it not allowed to require Fn types to be Send + Sync? Also is AssertThreadSafe a shorthand or just a function that I couldn't find in a quick search?

pub trait MyClass_child_entered {

Is there be a reason to not make it MyClassSignals? Then, all user-made signals appear in the same section of the docs.

Does the PhantomData do anything?

[Houtamelo]

To me, it's not obvious that my_class.child_entered() is a constructor for a signal. Of course .connect_fn_deferred() indicates that, but I think it'd be valuable to ensure than the method name is clear. Perhaps making it .signal_child_entered() or putting the constructor methods on an intermediate signal struct, e.g. my_class.signals().child_entered()
I think that would add unnecessary bloat:

• As you mentioned, the usage and methods available on the return value of the signal constructor make it very clear that it's a signal.
• The type of the return method is Signal<T>.
• Built-in signals are well documented in Godot's docs.
• Custom signals will all be defined in a single block with #[godot_api], with a very clear attribute macro #[signal]

I don't think this is an issue.

Is it not allowed to require Fn types to be Send + Sync? Also is AssertThreadSafe a shorthand or just a function that I couldn't find in a quick search?

Callable::from_fn does require for F to be Send + Sync, but as mentioned in my latest comment in #588 (comment) , this makes them very limited and not very useful. The type AssertThreadSafe is just a wrapper with a unsafe impl Send + Sync {}, which I made to bypass the current Send + Sync bounds. It probably won't be included in the final PR as Bromeon mentioned about adding a unsafe Callable::from_fn_unchecked that doesn't have the bounds.

Is there be a reason to not make it MyClassSignals? Then, all user-made signals appear in the same section of the docs.

You can't bundle all signals in the same trait because they could have different signatures, you can't have two emit methods with different parameter requirements. Rust doesn't allow overloading.

Does the PhantomData do anything?

Yes, it allows me to put a generic T on the Signal struct, which is used to provide different implementations of traits based on which signal is being used.
Read https://docs.rs/sr-std/latest/sr_std/marker/struct.PhantomData.html for more details.

Edit: fixing formatting.

[Bromeon]

Thanks a lot for the great design, @Houtamelo! 👍

• Signals should not rely on proc-macros to work. Users should be able to reasonably build and use signals on their own.
• Type safety: emit/connect should use Rust's type system to enforce correct types at compile time.
• Emit/Connect should not use tuples as arguments, instead they should have methods that take any number of arguments depending on the signal.
• Ideally, Emit/Connect should have properly named parameters.

I think these are great premises. If we want to get close to GDScript usability, signals should be named and type-safe wherever possible. And the more natural they feel in Rust, the more ergonomic the experience.

---

Regarding obj.child_entered() vs. obj.signals().child_entered():

I think that would add unnecessary bloat:

• As you mentioned, the usage and methods available on the return value of the signal constructor make it very clear that it's a signal.
• The type of the return method is Signal<T>.
• Built-in signals are well documented in Godot's docs.
• Custom signals will all be defined in a single block with #[godot_api], with a very clear attribute macro #[signal]

I don't think this is an issue.

Only 1 of these 4 points are visible at the usage site, and only once the code is already written.

A dedicated signals() "namespace" would have a few advantages:

1. You can IDE-complete all signals of the current class.
2. Rustdocs of the class would allow to separate them from other symbols, but still group them.
3. In GDScript, signals can be differentiated from method calls because they don't have parentheses (my_signal.emit() vs. Rust my_signal().emit()). Granted, the emit() makes it relatively clear in this case, but there are also contexts where you just pass signals as arguments. Additionally, Godot's own signals are not always very obviously recognizable as signals (Node::replacing_by, CanvasItem::draw, ...).
4. It requires a few less generated symbols and simplifies some implementation, see below.

I also don't know if an extra .signals() call is a real issue in practice? This is a common pattern with godot-rust already, see obj.bind().something() or self.base().something()...

---

Is there be a reason to not make it MyClassSignals? Then, all user-made signals appear in the same section of the docs.

You can't bundle all signals in the same trait because they could have different signatures, you can't have two emit methods with different parameter requirements. Rust doesn't allow overloading.

I'm also a bit worried about the number of introduced symbols and the genericity, which both have an impact on compile times and doc complexity. It may be a bit hard to discover, debug and read related compile errors.

But about @fpdotmonkey's suggestions, would something like this not be possible? I tried to use more concrete types where applicable, and reduce the per-signal symbols from

1 tag struct + 2 traits + 3 trait impls

to

1 definition struct + 1 inherent impl + 1 trait impl

It's well possible that I missed something though.

pub struct MyClassSignals {
    pub child_entered: MyClassSignalChildEntered,
    ... // other signals
}

pub struct MyClassSignalAreaEntered { 
    signal: Signal, // actual reference to object + method
}

impl MyClassSignalAreaEntered {
    pub fn connect<F>(&mut self, function: F)
        where F: FnMut(Gd<Area2D>) { ... }

    pub fn emit(&mut self, area: Gd<Area2D>) { ... }
}

// Maybe this API could be hidden, as it's only needed for type erasure/dispatch, or maybe
// it can be useful for users later, too... Name TBD
impl SignalDispatch for MyClassSignalAreaEntered {
    type Params = (...); // some tuple

    fn generic_connect(...) { ... }
    fn generic_emit(...) { ... }
}

trait SignalProvider {
    type SignalGroup;
    fn signals(&self) -> SignalGroup;
}

impl SignalProvider for MyClass {
    type SignalGroup = MyClassSignals;
    pub fn signals(&self) -> MyClassSignals { ... }
}

// Do NOT impl SignalProvider for Gd<MyClass> 

// Inside gdext library -- could this solve the orphan issue?
impl<T> Gd<T> 
where T: SignalProvider {
    pub fn signals(&self) -> T::SignalGroup {
         // Get direct access to instance without bind().
         // Must be unsafe, but works as long as SignalProvider doesn't access Rust-side state of the object.
         let obj: &T = unsafe { self.bind_unchecked() }; // private API
         // Accepting Gd<Self> instead of &self would make the access more elegant, 
         // but at the cost of more indirections/symbols.

         obj.signals()
    }
}

[Houtamelo]

I likely won't be able to pick this up again this year, so anyone else is welcome to move forward with the implementation if they'd like.

[walksanatora]

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) -> ()>

Inorder to make ..default Base would need to be Default (or have some way to make a "dummy" that can be used in default impl

[Bromeon]

I'm currently working on a design based on the above proposals. Will update once I have something tangible 🙂