Unify Swift and Godot Signal Syntax

[samdeane]

See also #586, which describes the problem I was trying to solve.

This PR builds on the changes in #587 - which should be tested and merged first.

Motivation

The #signal macro defines the signal as a static property on the class that contains it.

This means that, given a class with a signal:

@Godot public class SBActor: Object {
    #signal("weapons_changed")
    // ...etc
}

when using the signal in SwiftGodot code, the current spelling to connect to it is:

let actor = SBActor()
actor.connect(signal: SBActor.weaponsChanged, to: self, method: "updateWeapons")

This isn't as intuitive as the GDScript spelling, and it's different from the way we treat signals that are defined in Godot and bridged over to Swift, eg:

myNode.ready.connect {
    print ("The node is ready")
}

Implementation

This PR adds a new #nusignal macro expansion which generates a computed property on the class, using GenericSignal. This is the same approach that is used to implement the signals imported from the Godot API.

We also add an emit() method to GenericSignal, which is of course essential to support emitting signals defined in Swift.

This allows the swift connect call to be written as:

actor.weaponsChanged.connect { print("do something here"); }

It can also be written as:

actor.weaponsChanged.connect(handleWeaponsChanged)

func handleWeaponsChanged() { print("do something here") }

To Do

I added a new macro to ensure that this isn't a breaking change.

Instead of the #xxx style macro, it might be possible to make an attachment-style@Signal, which could be cleaner

If this change is adopted, it's probably worth updating the documentation and deprecating the old macro.

[migueldeicaza]

We have an existing idiom to map Godot signal into Swift idioms, have you seen it?

I wonder if we can reuse that, so we do not end up with an additional idiom - check the generated code we have.

[samdeane]

I guess you're talking about support for the syntax described here, generated by ClassGen.generateSignalType?

It would indeed make sense to try to use the same underlying mechanism for signals that are defined by the user on the Swift side. It confused me that the syntax in the documentation worked for pre-existing signals declared as part of the generated API, but not for signals we declare using #signal. It would be great to fix that, and in the process we could probably eliminate most of the code inSignalRegistration.swift.

There is also a big potential optimisation I think with generateSignalType. Currently it makes inner classes for each signal, eg:

// Signals 
    /// Signal support.
    /// 
    /// 
    /// 
    /// Use the ``Signal1/connect(flags:_:)`` method to connect to the signal on the container object, and ``Signal1/disconnect(_:)`` to drop the connection.
    /// 
    /// You can also await the ``Signal1/emitted`` property for waiting for a single emission of the signal.
    /// 
    public class Signal1 {
        var target: Object
        var signalName: StringName
        init (target: Object, signalName: StringName) {
            self.target = target
            self.signalName = signalName
        }
        
        /// Connects the signal to the specified callback
        /// 
        /// 
        /// 
        /// To disconnect, call the disconnect method, with the returned token on success
        /// 
        /// - Parameters:
        /// 
        /// - callback: the method to invoke when this signal is raised
        /// 
        /// - flags: Optional, can be also added to configure the connection's behavior (see ``Object/ConnectFlags`` constants).
        /// 
        /// - Returns: an object token that can be used to disconnect the object from the target on success, or the error produced by Godot.
        /// 
        @discardableResult /* Signal1 */
        public func connect (flags: Object.ConnectFlags = [], _ callback: @escaping (_ bodyRid: RID, _ body: Node2D, _ bodyShapeIndex: Int64, _ localShapeIndex: Int64) -> ()) -> Object {
            let signalProxy = SignalProxy()
            signalProxy.proxy =  {
                args in
                let arg_0 = RID (args [0])!
                var ptr_1: UnsafeMutableRawPointer?
                args [1].toType (Variant.GType.object, dest: &ptr_1)
                let arg_1 = lookupLiveObject (handleAddress: ptr_1!) as? Node2D ?? Node2D (nativeHandle: ptr_1!)
                let arg_2 = Int64 (args [2])!
                let arg_3 = Int64 (args [3])!
                
                callback (arg_0, arg_1, arg_2, arg_3)
            }
            
            let callable = Callable(object: signalProxy, method: SignalProxy.proxyName)
            let r = target.connect(signal: signalName, callable: callable, flags: UInt32 (flags.rawValue))
            if r != .ok { print ("Warning, error connecting to signal, code: \(r)") }
            return signalProxy
        }
        
        /// Disconnects a signal that was previously connected, the return value from calling ``connect(flags:_:)``
        public func disconnect (_ token: Object) {
            target.disconnect(signal: signalName, callable: Callable (object: token, method: SignalProxy.proxyName))
        }
        
        /// You can await this property to wait for the signal to be emitted once
        public var emitted: Void  {
            get async {
                await withCheckedContinuation {
                    c in
                    connect (flags: .oneShot) { _, _, _, _ in c.resume () }
                }
                
            }
            
        }
        
    }

in Area2D, which is only used for a single property:

    public var bodyShapeEntered: Signal1 { Signal1 (target: self, signalName: "body_shape_entered") }

I understand why it does this (it's simple and it works!), but it's quite wasteful, when quite a lot of signals probably have the same signature.

(admittedly, the signature in the example above is probably unique, but there are quite a lot of animation-related signals for example which take a single animation name as their parameter, and we're making an identical class for each one)

The repeated code will affect compile times, and I'm not sure whether the linker will be smart enough to eliminate the duplication.

There's an optimisation in their for the SimpleSignal no-parameters case, but I can see a couple of possible further optimisations:

1. Generate these helper classes at the top-level, encoding the signature into the class name. That way we can reuse the classes for any signal with the same signature, which will cut down on the amount of generated code. This ought to be relatively easy to do - the generator will just need a way to keep track of a map of signatures->class-names, and to only emit each class once.

2. Implement a single helper class using generics. I think that this might be possible using the generic variadic / parameter pack / etc support. It would be a further optimisation since the code generator would just need to generate the property stubs like:

    public var bodyShapeEntered: GenericSignal<RID,Node2D,Int64,Int64> { GenericSignal<RID, Node2d,Int64,Int64> (target: self, signalName: "body_shape_entered") }

and it would automatically have connect and emit methods with signatures that had RID, Node2D, Int64, Int64 arguments.

I don't know whether the underlying swift compiler would be smart enough to avoid the bloat of repeated expansion of this generic into concrete versions with different signatures... but at least that is someone else's problem 😛.

[samdeane]

Ignoring the optimisations for now, I wonder if I can lift the code from generateSignalType into a place where the #signal macro could also call it.

Then using #signal("foo", Bar.self)

could emit a SignalFoo support class and a computed property:

    public class SignalFoo { /*. generated support for Foo signal here... */ }
    public var foo: SignalFoo { SignalFoo(target: self, signalName: "foo") }

and you could do obj.foo.connect { bar in ... }, or obj.foo.emit(Bar()) like you can for the API signals.

[migueldeicaza]

Correct, it is wasteful, and would love to consolidate, that was filed as #42

I am open to the changes if we can harmonize them with the existing signal setup, would this change break existing code that might have used #signal already?

[samdeane]

Better mockup - this code actually runs and the test passes, although it's obviously just a sketch...

import ChaosTesting
import Testing

@testable import Scratch

@Test func name() async throws {
  struct Foo {
    var foo: GenericSignal<Int> { GenericSignal<Int>(name: "foo") }
  }

  var callbackRan = false
  let f = Foo()
  f.foo.connect { (i: Int) in
    #expect(i == 42)
    callbackRan = true
  }

  f.foo.emit(42)
  #expect(callbackRan)
}

struct GenericSignal<each T> {
  let name: String

  func emit(_ t: repeat each T) {
    emitSignal(name, repeat each t)
  }

  func connect(callback: @escaping (_ t: repeat each T) -> Void) {
    sp.proxy = { args in
      callback(repeat args.pop(as: (each T).self)!)
    }
  }
}

nonisolated(unsafe) var sp = SignalProxy()

/// A packed array of arguments that can be popped off one by one,
/// with type checking.
class Arguments {
  init(_ args: [Any]) {
    self.args = args
  }

  var args: [Any] = []
  func pop<T>(as: T.Type) -> T? {
    let v = args.first as? T
    args.removeFirst()
    return v
  }
}

/// A proxy object that can be registered with a signal.
/// When the signal is emitted, the proxy will be called,
/// and will invoke its callback with the supplied arguments.
struct SignalProxy {
  var proxy: ((Arguments) -> Void)? = nil

  func proxyFunc(args: Arguments) {
    proxy?(args)
  }
}

func emitSignal<each T>(_ name: String, _ args: repeat each T) {
  var a: [Any] = []
  repeat a.append(each args)
  sp.proxyFunc(args: Arguments(a))
}

[samdeane]

would this change break existing code that might have used #signal already?

It might be possible to support some kind of backwards compatibility, but I suspect that a cleaner route could be to implement a new solution and then deprecate the old one.

I am wondering whether the new solution could use @Signal instead.

So you'd do something like:

@Godot class MyClass {
  @Signal var mySignal

and it would expand the body for you, out to:

@Godot class MyClass {
  var mySignal: GenericSignal< > { GenericSignal< >(target: self, signalName: "mySignal")

[samdeane]

So you could also do:

@Godot class MyClass {
  @Signal(args: [String.self, Int.self]) var mySignal

and it would expand the body to:

@Godot class MyClass {
  var mySignal: GenericSignal<String, Int> { GenericSignal<String, Int>(target: self, signalName: "mySignal")

and so on...

[samdeane]

I've added a new issue #586, which is written from the perspective of fixing the disparity between the way we call imported signals and the way we call Swift-declared ones.

I think I could try to implement #42 using my suggested solution, which should be a non-breaking change if it works.
I could then implement #586, using the generic helper class from #42.

[samdeane]

Note that:

• this builds on GenericSignal helper class #587 so potentially it'd be easier to review & ideally merge that first
• this change is intended to be fully additive at the moment and leaves the old syntax alone
• there are additional bits of work that could be done - but I didn't want the scope of this PR to get out of hand
  • deprecate (or remove) the old #signal macro
  • switch to @Signal for the new macro?
  • update the Signal documentation -- I'm happy to do this but wanted to wait on a decision about deprecation etc

[samdeane]

I've cleaned up the formatting - the PR should now just contain the actual changes I've made.

[migueldeicaza]

It looks like the Windows build is broken:

2024-11-01T14:33:56.8202285Z D:\a\SwiftGodot\SwiftGodot\Sources\SwiftGodot\Core\GenericSignal.swift:49:20: error: value pack expansion can only appear inside a function argument list or tuple element

Maybe parameter Packs are not supported on Windows yet?

[samdeane]

Maybe parameter Packs are not supported on Windows yet?

Uh-oh. I'll take a look.