Adds #signal Macro

[PadraigK]

Adds a new freestanding macro that can be used inside a @Godot class to register a signal with Godot.

Signal arguments can be defined and their types will be enforced on new emit(signal:_argument) methods.

@Godot 
class Player: Node2D {
    #signal("game_started")
    #signal("lives_changed", argument: ["new_lives_count": Int.self])

    func startGame() {
       emit(Player.gameStarted)
       emit(Player.livesChanged, 5)
    }
}

class Level: Area2D {
    func _ready() { 
       player.connect(Player.gameStarted, to: self, method: "game_started")
    }
    @Callable func game_started() { 
       GD.print("got game started signal!")
    }
}

This all works nicely with the Godot editor too — signals are exposed there and can be connected to GDScripts and well use the argument names provided in the #signal invocation.

---

I did have to change GodotVariant's init() requirement to be an unwrap so that subclasses of Godot objects could be made to conform to it. This makes the PR a bit more wide-reaching, but it means that it is possible to send a Node object as an argument to a @Callable now.

[migueldeicaza]

I love this change.

Here are some thoughts that I have:

• I am a bit sad that the constructor requirement is dropped, because it was so convenient to create objects from a variant. On the other hand, I do not think we can afford to have two sets of public methods (because Windows has a limit on public methods, and I just spent a lot of time reducing our surface). Perhaps we could just change the method name from unwrap(variant:) to something shorter. Not sure.

• Now that we are blessing public Swift types with methods from the extension, I think we might need to change gType to be godotType across the board.

Here are the Api changes for this PR:

https://gist.github.com/migueldeicaza/7c0624034196453574075d375a51ec2e

[alicerunsonfedora]

Perhaps this might be me, but I feel that the initializer was more appropriate in this context, and not specifically for type casting reasons. Rather, because we are constructing something with that variant. unwrap would imply to me that Variant is actually a representation of an optional, which, I think, might be a little confusing.

[alicerunsonfedora]

Is there a particular reason why the closing parens was moved here?

[PadraigK]

Oh, just a personal code formatting habit that leaked in. I'd love to start using something like swiftformat on this project some day, we've got a lot of styles mixing here.

For now I'll revert this part because it doesn't belong in the PR.

[alicerunsonfedora]

I'm noticing that SignalWithNArguments feels ergonomically similar to that of SwiftUI's TupleView. Perhaps we can glean from that, so that we're not manually writing all these cases out?

[PadraigK]

On the surface, I don't see a way that can work and still give us the nice emit functions that enforce the types, but if you can say more about what you have in mind I can take another look.

[PadraigK]

I love this change.

Here are some thoughts that I have:

• I am a bit sad that the constructor requirement is dropped, because it was so convenient to create objects from a variant. On the other hand, I do not think we can afford to have two sets of public methods (because Windows has a limit on public methods, and I just spent a lot of time reducing our surface). Perhaps we could just change the method name from unwrap(variant:) to something shorter. Not sure.

Yeah I think you and @alicerunsonfedora are right to call this out. I'm not super happy with unwrap either. I have a feeling that GodotVariant is now be attempting to be too many things. I will think about it some more and explore alternatives.

• Now that we are blessing public Swift types with methods from the extension, I think we might need to change gType to be godotType across the board.

Good point, I can definitely change this.

Thanks for the feedback!

[migueldeicaza]

Apologies folks, I accidentally merged this patch, and I have now undone the push.

I think we can reopen this.

[PadraigK]

I'll repost soon.

Noting some other feedback from a friend: a member macro might be more appropriate that a freestanding one, so I'll also look into that.

[migueldeicaza]

One small observation, the unwrap method is not calling the init(value:) method for builtins like it did before:

diff -ruN SwiftGodot-REF/generated-builtin/Basis.swift SwiftGodot-DEBUG/generated-builtin/Basis.swift
--- SwiftGodot-REF/generated-builtin/Basis.swift        2023-10-21 12:11:09
+++ SwiftGodot-DEBUG/generated-builtin/Basis.swift      2023-10-21 12:12:40
@@ -13,15 +13,13 @@
 /// For more information, read the "Matrices and transforms" documentation article.
 /// 
 public struct Basis: Equatable, Hashable, GodotVariant {
+    public static var gType: Variant.GType { .basis }
     /// Creates a new instance from the given variant if it contains a Basis
-    public init? (_ from: Variant) {
-        guard from.gtype == .basis else {
-            return nil
-        }
-        
-        var v = Basis()
-        from.toType(.basis, dest: &v)
-        self.init (from: v)
+    public static func unwrap(variant: Variant) -> Self? {
+        guard variant.gtype == .basis else { return nil }
+        var value = Basis()
+        variant.toType(.basis, dest: &value)
+        return value
     }

[migueldeicaza]

Another question, you mentioned:

I did have to change GodotVariant's init() requirement to be an unwrap so that subclasses of Godot objects could be made to conform to it. This makes the PR a bit more wide-reaching, but it means that it is possible to send a Node object as an argument to a @callable now.

I could not figure out how Object or Wrapper are made to conform to GodotVariant on this patch, did I miss something?

There are no conformances to it that I could find.

[PadraigK]

One small observation, the unwrap method is not calling the init(value:) method for builtins like it did before:

 public struct Basis: Equatable, Hashable, GodotVariant {
+    public static var gType: Variant.GType { .basis }
     /// Creates a new instance from the given variant if it contains a Basis
-    public init? (_ from: Variant) {
-        guard from.gtype == .basis else {
-            return nil
-        }
-        
-        var v = Basis()
-        from.toType(.basis, dest: &v)
-        self.init (from: v)
+    public static func unwrap(variant: Variant) -> Self? {
+        guard variant.gtype == .basis else { return nil }
+        var value = Basis()
+        variant.toType(.basis, dest: &value)
+        return value
     }

Hmm, I see the difference but I don't quite follow why it should call the initializer?

1. We've made a new instance with var value = Basis()
2. Then copied the variant's storage into our instance with variant.toType(.basis, dest: &value)

If we now call init(value:) we will
3. Create a second new Basis()
4. Call the constructor to copy the storage over again.

[PadraigK]

Another question, you mentioned:

I did have to change GodotVariant's init() requirement to be an unwrap so that subclasses of Godot objects could be made to conform to it. This makes the PR a bit more wide-reaching, but it means that it is possible to send a Node object as an argument to a @callable now.

I could not figure out how Object or Wrapper are made to conform to GodotVariant on this patch, did I miss something?

There are no conformances to it that I could find.

I added these manually in my game project when they were needed. But I'll look at this again as part of re-considering unwrap() and see if there's a way to not need to conform in the game project.