Godot: Add the ability to pass any objects as signal binds, not just built-in types (Mono)

[xfactor2000]

Godot version:
3.0.stable.official.mono

OS/device including version:
Manjaro Linux 64 bit (rolling release)

Issue description:
I am unable to pass custom classes as signal binds, getting exception. I understand this is expected behavior, but I would like to be able to pass any objects alongside my signals.

It is very inconvenient for me as developer to pass data through built-in types like strings, it's time consuming and error prone. On the other hand, being able to define a class and then use it on both sending and receiving side of the signal would save me a lot of effort.

Steps to reproduce:

See the attached project. When clicked, the button generates an exception:

0:00:00:0782 - Attempted to convert an unmarshallable managed type to Variant. Name: 'CustomClass' Encoding: 18

Attempted to convert an unmarshallable managed type to Variant. Name: 'CustomClass' Encoding: 18
Time: 0:00:00:0782
C Error: Method/Function Failed, returning: Variant()
C Source: modules/mono/mono_gd/gd_mono_marshal.cpp:593
C Function: mono_object_to_variant

Minimal reproduction project:
TestProjectBindCustomClass.zip

Please note that the weird .Connect function syntax (object[] within object[] is due to this bug:

16311

[Duehok]

This feature would be very very nice to have.

The delegate/event/eventArgs pattern of C# can be used for an equivalent but is more complex, not integrated in the editor (and harder to integrate than the signals) and I have not yet tested how it interacts with free_queue. (EDIT: works 100% fine, I was afraid of memory leaks but everything is OK).

What's worse is that the error is a runtime error instead of a compile-time error.

[xfactor2000]

@Duehok, your delegate solution seems valid although I need to try it. However, I still believe objects in signal binds should be implemented at language-agnostic level.

[vnen]

The problem is that Godot is not aware of anything that doesn't inherit its Object class (in fact, signals are a property of the Godot objects). It seems inconvenient but it's intended to be used as a script language so all your game classes should be Godot objects (which internally can call anything offered by the language).

Maybe @neikeq can find some workaround, but IMO this doesn't need to work this way.

[xfactor2000]

@vnen I tried your approach by extending CustomClass from Node or Object classes, then I get another error:

ERROR: emit_signal: Error calling method from signal 'pressed': 'Node2D(game.cs):: _on_Btn_pressed': Method not found.
At: core/object.cpp:1202.

Seems like there's a bug, possibly related to #16311

[vnen]

@xfactor2000 yeah, that one is probably a bug.

[xfactor2000]

@vnen So should I do something, like poking somebody?

[KellyThomas]

@xfactor2000

ERROR: emit_signal: Error calling method from signal 'pressed': 'Node2D(game.cs):: _on_Btn_pressed': >Method not found.
At: core/object.cpp:1202.

Is caused by a leading space in the string passed as third parameter:

btn.Connect("pressed",this," _on_Btn_pressed",new object[]{new object[]{new CustomClass()}});

---

I'm not able to test any of the stable releases (my PC is running a different version of mono) but with a recent build from master this is working as expected:

using Godot;

public class Counter : Godot.Object 
{
    private int _counter;

    public override string ToString()
    {
        return $"{_counter++}";
    }
}

public class game : Godot.Node2D
{
    public override void _Ready()
    {
        var btn = GetNode("Btn");
        btn.Connect("pressed", this, "_on_Btn_pressed", new object[] {new Counter()});
    }

    private void _on_Btn_pressed(Counter p1)
    {
        GD.Print($"{p1}");
    }
}

[xfactor2000]

@KellyThomas works! Thanks a lot!

[Fido789]

Godot's inability to pass custom objects to signals is really limitating. A workaround to this issue I have found is to use a custom wrapper class for signal parameters. Unfortunately generic wrapper doesn't work, so it must be something like this:

public class ParameterWrapper : Godot.Object
    {
        private readonly object value;

        public ParameterWrapper(object value)
        {
            this.value = value;
        }

        public T GetValue<T>()
        {
            return (T)this.value;
        }
    }

Could any of core developers confirm, that this is a valid solution and will not produce memory leaks or something?

[xfactor2000]

Nope, in fact it works just fine, it's just that your custom objects have
to inherit from Node. See the discussion above.

On Tue, Jun 5, 2018, 10:11 Fido789 [e-mail] wrote:

Godot's inability to pass custom objects to signals is really limitating.
A workaround to this issue I have found is to use a custom wrapper class
for signal parameters. Unfortunatelly generic wrapper doesn't work, so it
must be something like this:

public class ParameterWrapper : Godot.Object
{
private readonly object value;

    public ParameterWrapper(object value)
    {
        this.value = value;
    }

    public T GetValue<T>()
    {
        return (T)this.value;
    }
}

Could any of core developers confirm, that this is a valid solution and
will not produce memory leaks or something?

—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
https://github.com/godotengine/godot/issues/16706#issuecomment-394605337,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AEwqYVuzmEH91sPpU0NoGhop1erDQarNks5t5i83gaJpZM4SF2uo
.

[Fido789]

That's great. So in that case couldn't be such a wrapper implemented at engine's level? Just checking for the type and if is is not built-in type or Godot.Object, wrap/unwrap it?

[xfactor2000]

Just pass a custom class that inherits from Node inside binds array, and
you will receive it as function parameter.

On Tue, Jun 5, 2018, 11:08 Fido789 [e-mail] wrote:

That's great. So in that case couln't be such a wrapper implemented at
engine's level? Just checking for the type and if is is not built-in type
or Godot.Object, wrap/unwrap it?

—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
https://github.com/godotengine/godot/issues/16706#issuecomment-394620044,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AEwqYfOFwz8y-OkTTCFAen_L9SMd1EtDks5t5jxkgaJpZM4SF2uo
.

[Fido789]

Sure, I can do that, but a lot of my objects doesn't derive from Node/Godot.Object. For example my Level class. It would be very convenient if I could pass it as a parameter to level buttons. I can pass it wrapped in a wrapper above, but if it can be easily wrapped at the engine level, why not to do it?

[xfactor2000]

Why not make your level class inherit from Node? That's what I did, never
bothered me.

On Tue, Jun 5, 2018, 11:16 Fido789 [e-mail] wrote:

Sure, I can do that, but a lot of my objects doesn't derive from
Node/Godot.Object. For example my Level class. It would be very convenient
if I could pass it as a parameter to level buttons. I can pass it wrapped
in a wrapper above, but if it can be easily wrapped at the engine level,
why not to do it?

—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
https://github.com/godotengine/godot/issues/16706#issuecomment-394622483,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AEwqYYGOe62hMpDhCB1z5bHD7HUXBx3Uks5t5j5mgaJpZM4SF2uo
.

[Fido789]

Well there are many reasons, like memory consumption, inability to derive from anything else, easy transfer to another engine (right now Unity -> Godot), unit testing and many other. If I would do that, the cure would be much worse than symptoms.

[xfactor2000]

You can make your base class derive from Node and then derive from it. As
for memory consumption - why is it an issue? Node is meant to be a basic
building block of Godot, so it makes sense to inherit from it often. If it
was not optimized for wide usage, Godot wouldn't work.

On Tue, Jun 5, 2018, 11:27 Fido789 [e-mail] wrote:

Well there are many reasons, like memory consumption, inability to derive
from anything else, easy transfer to another engine (right now Unity ->
Godot), unit testing and many other. If I would do that, the cure would be
much worse than symptoms.

—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
https://github.com/godotengine/godot/issues/16706#issuecomment-394625798,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AEwqYe9BvM9QKKE7IcD1w4-bk2jWjmUCks5t5kENgaJpZM4SF2uo
.

[Fido789]

It is not that memory consumption would be huge or something, it is just unnecessary. For now I will stick to the wrapper.

[eddex]

@Fido789 thanks for the solution with the wrapper! This just made my day 😄

[rsoulard]

I am using the wrapper currently, but the reason I cannot just inherit from Godot.Object or Node is because the classes used come from a shared library that is used by other applications that do not "know" about Godot.