Registering custom signals with multiple arguments

Hi everyone, I've been looking through the information trying to see how to create signals with multiple arguments, and I was wondering how to go about creating signals with arguments in Nim?

I know that something like

gdobj CharacterStats of Node:
  method init*() =
    self.addUserSignal("health_changed")

is somewhat the equivalent of the following GDScript:

extends Node

signal health_changed

I'm still unsure how to write the equivalent GDScript:

extends Node
signal health_changed(amount)

Thanks for all the help so far!

I haven't done this before but according to : https://pragmagic.github.io/godot-nim/v0.5.4/godotapi/objects.html#addUserSignal,Object,string,Array
proc addUserSignal*(self: Object; signal: string; arguments: Array) {..}

you should be able to do or something like this atleast:

gdobj CharacterStats of Node:
  method init*() =
    let amount = 5
    self.addUserSignal("health_changed", newArray(amount.toVariant))

That's what I figured and hoping wouldn't be the case. I was hoping that at least you could do something like

self.addUserSignal("health_changed", newArray(VariantType.Int))

I'm still unsure whether or not the newArray passed to addUserSignal would contain elements such as 0.toVariant or whether the number passed into the newArray would really just be the numerical representation of the VariantType enum, like VariantType.Int = 1

I actually don't know much about this either and googling this didnt get me far.
But: https://docs.godotengine.org/en/stable/tutorials/plugins/gdnative/gdnative-cpp-example.html#signals says:

Here we see a nice improvement in the latest version of godot-cpp 
where our register_signal method can be a single call first taking the signals name, 
then having pairs of values specifying the parameter name and type of each parameter 
we'll send along with this signal.

However I cannot read C++ haha.

It looks like this would require a change in the genType function to use Nativescript 1.1 and register signals along with registering methods

godot-nim/godot/nim/godotmacros.nim

Line 440 in f25c3c7

proc genType(obj: ObjectDecl): NimNode {.compileTime.} =

It would also likely require a change to parseType to parse custom signal declarations like this:

signal healthChanged(amount: int)

This is how you can do it:

self.addUserSignal("health_changed", newArray([{"amount": TYPE_INT}.toTable()]))

TYPE_xxx constants can be found in global_constants.nim. A small improvement could be to implement a toVariant/fromVariant conversion for a seq of pairs. Then the .toTable() part won't be necessary.

self.addUserSignal("health_changed", newArray([{"amount": TYPE_INT}.toTable()]))

hmm, I tried using this, and I get a type mismatch error for newArray since there isn't a proc that accepts Table.

This is how I've added a signal in the past. But it's pretty verbose:

  import godotapi / [global_constants]
  var arg0 = newDictionary()
  arg0["name".toVariant] = "amount".toVariant
  arg0["type".toVariant] = TYPE_INT.toVariant
  var args = newArray(arg0.toVariant) # each dictionary adds an argument
  self.addUserSignal("health_changed", args)

Trying changing array to sequence (add @). This should work:

self.addUserSignal("health_changed", newArray(@[{"amount": TYPE_INT}.toTable()]))

@endragor Does the register_signals method in the C++ version register them so that the signals are viewable in the Godot editor? If so, that's what I was alluding to and was wondering whether that would be a better way to go about it?

Trying changing array to sequence (add @). This should work:
self.addUserSignal("health_changed", newArray(@[{"amount": TYPE_INT}.toTable()]))

Yeah, that's not working either. Here's the truncated error.

Error: type mismatch: got <seq[Table[system.string, system.int64]]>
but expected one of:
proc newArray(arr: GodotArray): Array
  first type mismatch at position: 1
  required type for arr: GodotArray
  but expression '@[toTable([("amount", 2'i64)])]' is of type: seq[Table[system.string, system.int64]]
...

Looking at the code in arrays.nim, I don't see how newArray accepts a seq or a Table. The only proc that would make sense is for openarray[Variant], but the arguments would need to have toVariant called on it right?

In anycase, I just followed the docs for https://docs.godotengine.org/en/stable/classes/class_object.html#class-object-method-add-user-signal and that's how I arrived at the code that uses a Dictionary per argument with "name" and "type" keys. It would be nice to have a cleaner have a cleaner way to set this up, otherwise I'll write a macro.

@geekrelief this is the same error that I'm running into atm, which is partially why I suggested looking into calling register_signal in the same genType type call that registers the methods and the variables to be exported. I could be wrong, but it seems like that might be more idiomatic Godot thing to do.

@arundilipan Yeah, I hear. I've considered modifying stuff in godotmacros.nim for other things like allowing let variables for instance or other macros. But I've resorted to creating my own macros to call with methods/procs. We would need to modify parseType and genType to allow things other than var, proc, and methods.

I'll try playing around with it tomorrow.

@geekrelief any code you could share I think it could come in handy for future people looking into these stuff.

EDIT: nvm I see the gdnim repo.

@zetashift Feel free to create issues on the gdnim repo if you have questions or ping me on discord. Been looking to share, but I'm not sure what's the best/appropriate way to reach out to our tiny godot-nim community.

Anyone have a suggestion on how to get nim to recognize this syntax?

signal my_signal(a:bool = true)

Nim is erroring at the '=' thinking my_signal is using an Object construction syntax when what we want is a proc syntax for default arguments. And it would be nice to have that for default arguments for signals to match gdscript.

signal my_signal(a:bool)

works fine gut no default arguments.

signal my_signal(a:bool):
  a = true

Works too, but not as pretty.

Edit:

signal my_signal(a = true, b:int)

Is another option, it produces an AST I can work with.

@geekrelief how did you get Nim to recognize that syntax?

@arundilipan Not sure which syntax are you talking about. But if you do a

dumpAstGen:
  signal my_signal(a=true, b:int)

You get:

nnkStmtList.newTree(
  nnkCommand.newTree(
    newIdentNode("signal"),
    nnkCall.newTree(
      newIdentNode("my_signal"),
      nnkExprEqExpr.newTree(
        newIdentNode("a"),
        newIdentNode("true")
      ),
      nnkExprColonExpr.newTree(
        newIdentNode("b"),
        newIdentNode("int")
      )
    )
  )
)

@geekrelief how are you trying to go about parsing that syntax? Are you trying to do it through the genType function or are you making a separate macro called signal?

Going through parseType/genType is the only way. Right now parseType throws away anything that isn't a var, proc, or method. So I'm modifying parseType to use the above signal syntax that allows for default arguments.

Edit: FYI, I'm dealing with some plumbing issues right now, so bear with me if I'm slow to respond. :)

So I got word that the proc syntax is only possible if the proc keyword is used. https://forum.nim-lang.org/t/7148

Do you think default arguments are necessary? I'm thinking about implementing two versions of signal, where one accepts a body that will initialize any default arguments.

signal my_signal(a:bool)
# or
signal my_signal(a:bool):
  a = true

Thoughts?
Anyway, going to work on the one with out default args first.

Edit:
What do you think of the proc syntax?

proc my_signal(a:bool = true) {.signal.}

My personal vote is for:

signal my_signal(a:bool): a = true

I think the proc version would be more error prone or confusing if you forget the .signal. pragma.

My personal vote is for:
signal my_signal(a:bool): a = true
I think the proc version would be more error prone or confusing if you forget the .signal. pragma.

I like this version fwiw!

My personal vote is for:
signal my_signal(a:bool): a = true
I think the proc version would be more error prone or confusing if you forget the .signal. pragma.

I second this syntax for sure. I think the problem with using proc is that semantically, Godot signals aren't functions, they're data. It definitely makes more sense to use that syntax because it's very similar to GDScript. If we wanted to use a pragma, we could something like this:

type mySignal {.signal.} = object
  a: bool
  b: bool

where {.signal.} modifies the type declaration to adhere to some concept Signal that allows us to call registerSignals or addUserSignal.

@geekrelief I tried working on that solution, any progress? I'm not great with nim macros so that's probably why I'm still struggling

@arundilipan I had to deal with some stuff, and fighting a cold too. So I haven't had a whole lot of time to tackle the issue. Thanks for attempting it. Like you I'm somewhat new to nim / godot and macros too. Learning as I go. I did take a closer look at things last night, and it's turned out to be a little trickier than I thought cause I was trying to mimic the godot-nim code base of using templates for registering stuff with godot, but I think I'll just go with a macro and slap something together to get it working first and clean it up afterwards based on feedback.

@geekrelief Get well soon!

Got the most basic signal working

  signal test_sig()
  method ready() =
    discard self.connect("test_sig", self, "on_test_sig")
    self.emitSignal("test_sig")

  proc on_test_sig() {.gdExport.} =
    print "got test_sig"

next working on parameters

So with parameters I'm trying to think of an elegant way to go from the parameter type to VariantType. Say a signal has parameters:

signal my_signal(a:bool, b:Vector2, c:PoolByteArray)

I guess for now I'll just match the type strings to VariantType enum in a big case statement. And circle back later to make it more elegant. Going to take a break and circle back to this later tonight.

@geekrelief First of all, I hope you get well soon! Second, that's exactly what my macro was trying to do. Basically it takes the type definitions of the declaration and it matches against the VariantType enum for right now. I think for most cases that is plenty, because I haven't figured out how to create converters for Variants

Code isn't pretty but I got parameters working. Tomorrow I'll work on default arguments and clean things up.

  signal test_sig(a_bool:bool, a_int8:int8, a_string:string)

  method ready() =
    discard self.connect("test_sig", self, "on_test_sig")
    self.emitSignal("test_sig", true.toVariant, 123.toVariant, "hello".toVariant)
    self.emitSignal("test_sig", false.toVariant, (-128).toVariant, "world".toVariant)

  proc on_test_sig(a_bool:bool, a_int8:int8, a_string:string) {.gdExport.} =
    print &"got test_sig {a_bool = } {a_int8 = } {a_string = }"

output:

got test_sig a_bool = true a_int8 = 123 a_string = hello
got test_sig a_bool = false a_int8 = -128 a_string = world

Edit: So, yes it works, but the generated code isn't passing the signal arguments correctly. See below.

oh wow, I just discovered, that you can register a signal without any arguments, call emitSignal with arguments and still receive them! So all you have to do is specify the signal name. WTF? bug is a feature?

Hmm I'm running into an issue passing the GodotSignalArguments. I create an array of GodotSignalArgument and pass the address of the first element to GodotSignal's args member, but godot crashes when trying to access the second GodotSignalArgument.

This is the output from expandMacros:

var sigArgs_369099033 = [GodotSignalArgument(name: toGodotString("a_bool"), typ: 1), 
                         GodotSignalArgument(name: toGodotString("a2_bool"), typ: 1)]
var godotSignal`gensym10 = GodotSignal(name: toGodotString("test_sig"),
                                       numArgs: 2, args: addr(sigArgs_369099033[0]))
nativeScriptRegisterSignal(getNativeLibHandle(), "TestComp",
                           godotSignal`gensym10)

Running in the debugger, looks like the next entry points to garbage. Anyone have a clue what's up?

I wonder if it's related to this godotengine/godot-cpp#473

I'm getting a crash here: https://github.com/godotengine/godot/blob/3.2/modules/gdnative/nativescript/godot_nativescript.cpp#L155

[0] atomic_conditional_increment (C:\godot\geekrelief_godot\core\safe_refcount.cpp:122)
[1] atomic_conditional_increment (C:\godot\geekrelief_godot\core\safe_refcount.cpp:122)
[2] godot_nativescript_register_signal (C:\godot\geekrelief_godot\modules\gdnative\nativescript\godot_nativescript.cpp:155)
[3] NimMain
[4] NimMain
[5] godot_nativescript_thread_exit
[6] NimMain
[7] godot_nativescript_init

Edit: Definitely, seems related. To @o01eg's issue. From godot-nim's side GodotSignalArgument is 56 bytes and on the godot engine side, godot_signal_argument is 52 bytes.

Hmm why is GodotSignalArgument's size 56 when all of its components add up to 52?
diff = 4 = sizeof(GodotSignalArgument) = 56 - ( name = 8 + typ = 4 + hint = 4 + hintString = 8 + usage = 4 + defaultValue = 24)

Fixed the issue. GodotSignalArgument needs a .packed. pragma. I was thinking it was some kind of alignment issue!

Edit: Reminder to self: godot/internal/godotinternaltypes.nim needs change to GodotSignalArgument with .packed. pragma.

Awesome! Got parameters working finally!

FYI, the default arguments seem backward. The engine code is looping through the signals from front to back to get their default value.

	for (int i = 0; i < p_signal->num_default_args; i++) {
		Variant *v;
		godot_signal_argument attrib = p_signal->args[i];

		v = (Variant *)&attrib.default_value;

		default_args.push_back(*v);
	}

So if you have a signal like:

  signal my_signal(a_bool:bool, a_string:string):
    a_string = "hello"

Wouldn't work. You'd have to specify a_bool with the default argument. Is this the same for gdscript?

Going through the docs it doesn't even look like default arguments for a signal are a thing. https://docs.godotengine.org/en/stable/development/file_formats/gdscript_grammar.html The gdscript grammar doesn't allow for it. I'm going to skip implementing default arguments for now.

Here's my code: https://github.com/geekrelief/godot-nim/tree/added_signals I updated the example in the docs for nim/godotmacros.nim.

Try it out and let me know if you have any issues.

Whoops I forgot I removed some old code in those files, I need to add back for older versions of nim.
Edit: patched it up

@geekrelief This is fantastic! I have one question: if you add the compiled nativescript file to a node in Godot, does it show you the signals that have been registered in genType?

@geekrelief This is fantastic! I have one question: if you add the compiled nativescript file to a node in Godot, does it show you the signals that have been registered in genType?

You mean like this?

Ok I figured out why it didn't work for me earlier, @geekrelief, it's cause the signals have to be in snake_case. But man you're the GOAT.

Ok I figured out why it didn't work for me earlier, @geekrelief, it's cause the signals have to be in snake_case.

cool, yeah seems like anything you send to godot should be snake_case 👍

Ok I figured out why it didn't work for me earlier, @geekrelief, it's cause the signals have to be in snake_case. But man you're the GOAT.

Thx, but the real GOAT is @endragor. :)

@arundilipan I wonder is there an elegant way to deal with this case sensitivity? Is it the receiving function that needs to be snake_case or the signal name or both?

So at the moment, the way it works it seems is that when the methods get compiled, the method names get converted into snake_case, so it seems as though if we're registering the signal with a method, the method name has to be in snake_case when we add the signal to the method.

So at the moment, the way it works it seems is that when the methods get compiled, the method names get converted into snake_case, so it seems as though if we're registering the signal with a method, the method name has to be in snake_case when we add the signal to the method.

Cool. Yeah that's "expected" behavior when it comes to godot-nim. I think we're good for now so I put out a PR.

Hey guys, I just pushed a new PR based on endragor's feedback. It fixes a potential, unsafe memory issue when using GC and registering a signal with arguments. Please check it out and let me know if you run into any issues.

Will do. I noticed that that you had a setup where you were able to reload Nim code into Godot relatively easily, and I was wondering if there was a way we could get that working with nake? I'm fairly ignorant of the nim build system so I'm not sure about how we could get it working on Linux or Mac?

Yeah there's probably a way to make it work with nake, but I'm not a fan. Nim has a mismash of configuration files, nimscript, nimble and nake. From what I know nake is deprecated and the future is nimscript. And it was really confusing for a newbie like me that was trying to learn nim and godot-nim several months ago.

Besides using nimble to install libraries, I don't see much of an advantage in using nake or nimble/nimscript. You can't use exportc functions from nimscript so checking file modification times isn't possible. Godot-nim uses all three and it was very confusing to wade through all of it. I wanted to try hacking on godot-nim for hot reloading and try out ARC. So I hacked together my own build system which mimics nimble tasks and I'm happy with it. It's super easy to modify and is just plain nim + a little cfg file. I use my build system to do everything from rebuilding the godot engine, generating the godot-nim api, generating and building components, and launching godot.

Anyway, I think zetashift was trying to get gdnim working for Linux. But I'm not sure if he was successful. If you're interested in checking it out, feel free to chat me up on discord if you have further questions. The gdnim README should give you an overview of how it works.

Edit: I forgot to mention, my system won't work without an engine patch, so updating nake alone is insufficent. I tried 5-6 different ways to get things working (lots of crashes and tears) and using reloading dlls via resources was the most robust.

It sounds like you reinvented nake :)

I'm not sure about the current state of dependency management and build tooling in Nim, as we develop our games with an old version (0.18). But, when godot-nim was made, NimScript was too limiting (you couldn't use any Nim library with it, only a subset) and Nimble also used NimScript which resulted in similar limitations. So in godot-nim-stub nimble is used for package management, .nims to specify build flags depending on environment, and nake for custom build tasks. We also use a similar system for our games.

It sounds like you reinvented nake

:) Yes, that was part of the goal. I just wanted to reduce my perimeter of ignorance while working on hot reloading for godot. At the time everything was new.

I'm in a much better position to port things back to using nake now, but my activation energy for it is pretty high. hehe Mostly because hot reloading isn't possible without a godot engine patch. But if you or enough people think it's worth pursuing I can help. Here's a capture of gdnim's workflow: https://drive.google.com/file/d/1mMmh8FSNj5Wd9mm8gVvvjnTmXs_WSnVU/view?usp=sharing

There's also one other thing I wanted to ask regarding signals, and that's how we would handle coroutines? Would we use Nim's async/await to wrap it or would we have to come up with something different to accommodate that?

@arundilipan Coroutines in gdscript are language specific. It's not an API thing, so there's no way to emulate that behavior as far as I know. But you might be able to fake it with an iterator, if you need something coroutine like I think.. Nim has a coroutines package, but I think I tried it a while back and ran into some issues. I know they're definitely not battle tested for gc ARC/ORC.

@geekrelief wait, so if that's the case, how do does C# and Javascript have their equivalent of GDScript's yield? In the ECMAScript module they essentially map the yield functionality to JS's async/await, and so does C# if I'm not mistaken?

You mean as a language construct or actually supporting yield via GDNative? Cause C# and Javascript both have yield as part of their language spec, and both use them with iterators. I don't know how they interact with godot, but there shouldn't be any direct comparisons with C# since it's natively supported (i.e. not through gdnative).

I'm talking about supporting yield via GDNative. Javascript is built as a C++ module, but I think even the godot-rust project was trying to figure out how to get async/await to map to yield in GDNative. Javascript has a function godot.godot_yield that's similar to ToSignal in C# that makes a Promise out of a signal emission, and so like in C# you would use async/await like you would use yield in GDScript.

I only had a cursory look a the Javascript binding and rust bindings, but as I suspected, they're essentially implementing a yield api in their language, meaning if we wanted to do something similar we'd probably have to use nim's async/await or something.

We'll have to implement something that's equivalent to this for nim

function godot_yield(target, signal) {
  return new Promise(function(resolve, reject) {
    function callback(...args) {
      const slot_id = GodotAnonymousConnectionManager.get_slot_id(target, signal, GodotAnonymousConnectionManager);
      const method_name = callback[slot_id];
      GodotAnonymousConnectionManager[method_name] = undefined;
      delete GodotAnonymousConnectionManager[method_name];
      delete callback[slot_id];
      resolve(args);
    };
    target.connect(signal, GodotAnonymousConnectionManager, callback, [], godot.Object.CONNECT_ONESHOT);
  });
}

I wonder if this should probably be implemented as part of godot-nim-stub though. At least the way the JS binding is doing it, GodotAnonymouseConnectionManager is created as part of the bindings initialization. So it's acting like more of a framework instead of a pure gdnative binding.

Relevant issue: godotengine/godot#23040
It indeed should be done from Nim's side.

So I did a little digging into it, I'll see what I can do to get started on it.

So what's your strategy for doing this? To be honest I looked at this a while ago, and didn't understand nim's async and godot-nim well enough at the time to move forward.

Here are my thoughts on this so far. I can see how we'd used asynchdispatch's poll in the method process to process Futures. We'd explicitly have to add something like:

import asynchdispatch
gdobj ...
  method process() =
    if hasPendingOperations():
      poll(0)
    ...

So Futures will complete.

Then we'd tag a function with {.async.}, and since that function returns a Future when invoked. Inside that function we would then use await on some function, call it onSignal, that returns a Future used in a callback for connect.

  method ready() =
    # created self.timer
    asyncCheck self.onTimer()

  proc onTimer() {.async.} = 
    await onSignal(self.timer, "timeout") #onSignal returns a Future[void], await on it to block
    print "timer timeout"

What I'm trying to figure out is how to create/reference the callback to be passed to connect when it only accepts a string for the callback? I don't think we can automatically generate the callback inside of onSignal without passing the signal parameter types since a signal api is not part of gdnative. Take for example, Area2D has a area_shape_entered ( int area_id, Area2D area, int area_shape, int self_shape ) signal, we'd need to pass the signal parameters explicitly, or modify the gdnative api to include signals.

  proc somefunc() {.async.} =
    await onSignal(self.area2D, "area_shape_entered", proc(area_id:int, area:Area2D, area_shape:int, self_shape:int))
    print &"{area_shape=} entered"
    ...

So assuming we have all the info we need to generate a callback what are the options for passing it to connect? Currently callbacks are class member procs that need {.gdExport.}, which uses nativeScriptRegisterMethod to register it with godot. But we can't use {.gdExport.} on a callback inside of a proc. So onSignal needs a new macro to process it, call it genSignalCallback, that will genSym a name for the callback, register it with godot, then pass the callback name to connect. But the problem with that is genSignalCallback needs to write to the AST generated by godotmacros gdobj. I don't know if that's even possible "out-of-band", so we'd have to parse procs when gdobj is generating the AST to add the generated callback as another method to be registered. Since only {.async.} functions can contain await onSignal we'd only have to parse those functions.

Hmm so this seems doable. Thoughts?

First let's make another issue on the repo and close this one.

Looking at the way ECMAScript is doing it, they modified the godot.Object.connect function to allow you to pass in anonymous functions instead of strings that reference the object's instance methods. So what I was thinking was let's make another connect where, instead of passing a string that references the instance method to be called, you pass a proc that handles the signal. That may require macros or templates, but the benefit is we can do something like this instead:

method ready() =
  self.connect("signal", self) do (data: string) -> void:
    # do stuff here

If we figure this out, then what we can do is create Future[T]s using asyncdispatch/asyncfutures in a callback function and attach that the signal just in ECMAScript. What do you think?

I'm not sure how creating another connect function that accepts a function fixes the issue of having to register the function, but sure move the discussion to another issue. I'm going to take a closer look at registerGodotMethod to see if there's an easier way to do this.