Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(C#) .Bind missing on the Callable object #66106

Closed
TandersT opened this issue Sep 19, 2022 · 16 comments
Closed

(C#) .Bind missing on the Callable object #66106

TandersT opened this issue Sep 19, 2022 · 16 comments

Comments

@TandersT
Copy link

TandersT commented Sep 19, 2022

Godot version

Godot Engine v4.0.beta1.mono.official.4ba934bf3

System information

Windows 11, Vulkan API 1.2.0 - Using Vulkan Device #0: AMD - AMD Radeon RX 5700

Issue description

The method .Bind does not exists in the current beta, along with some of the docuemented code such as the .Connect on a delegate. I'm unsure if this is due to the fact that the documentation is regarding latest, and not beta, but as of now I can not see a way to connect a signal with additional paramteres.

Steps to reproduce

Create a new callable, and try to call .Bind, or any other form of passing a variable on build in signal

Minimal reproduction project

No response

@KoBeWi KoBeWi added this to the 4.0 milestone Sep 19, 2022
@KoBeWi KoBeWi moved this from To Assess to Todo in 4.x Priority Issues Sep 19, 2022
@KoBeWi KoBeWi moved this to To Assess in 4.x Priority Issues Sep 19, 2022
@KoBeWi KoBeWi moved this from Todo to To Assess in 4.x Priority Issues Sep 19, 2022
@raulsntos
Copy link
Member

This part of the documentation is still outdated, I have an open PR (see #64930) to fix this but it's still a draft pending some changes to Callable.

For now, you can use lambdas to connect to signals passing additional parameters:

public class MyNode : Node
{
	[Signal]
	public delegate void MySignalEventHandler(int a, int b, int c);
	
	public override void _Ready()
	{
		MySignal += MySignalCallback;
		MySignal += (a, b, c) => MySignalCallbackWithAdditionalParams(a, b, c, 1, 2, 3);
	}

	private void MySignalCallback(int a, int b, int c) {}
	private void MySignalCallbackWithAdditionalParams(int a, int b, int c, int d, int e, int f) {}
}

About ".Connect on a delegate", this is not possible in C#, I apologize for having invalid syntax in the documentation. My recommendation is to use the generated events instead as shown in the example above. Otherwise, you'll have to use the .Connect method on the node and pass the name of the signal and a Callable instance:

public class MyNode : Node
{
	[Signal]
	public delegate void MySignalEventHandler(int a, int b, int c);
	
	public override void _Ready()
	{
		// This way of constructing Callables will change in the future
		var callable = new Callable(MySignalCallback);
		Connect(SignalName.MySignal, callable);
	}

	private void MySignalCallback(int a, int b, int c) {}
}

@raulsntos raulsntos moved this from To Assess to Todo in 4.x Priority Issues Sep 19, 2022
@raulsntos raulsntos moved this from Todo to To Assess in 4.x Priority Issues Sep 19, 2022
@TandersT
Copy link
Author

TandersT commented Sep 19, 2022

That makes sense, the documentation was a bit odd. Your code works fine for signals that I create, but if I were to try the same with a built-in signal, such as Timeout, it does not accept additional parameters. Is there any way around this for now?

E.g.

public partial class Node2ds : Node2D
{
    [Signal]
    public delegate void MySignalEventHandler(int a, int b, int c);
    public override void _Ready()
    {
        var _t = GetTree().CreateTimer(1);
        MySignal += (a, b, c) => MySignalCallback(a, b, c); // all good
        _t.Timeout += (a, b, c) => MySignalCallback(a, b, c); // no good
    }
    private void MySignalCallback(int a, int b, int c) { }
}

@Zireael07
Copy link
Contributor

@TandersT: You can't add additional parameters to default signals, it's the same in GDScript

@TandersT
Copy link
Author

TandersT commented Sep 19, 2022

Okay, so as of now is there any way to achieve something similar to this, in which the variable _fire, is passed with the signal?

timer.Connect("timeout", this, nameof(FreeFire), new Godot.Collections.Array { _fire });

@raulsntos
Copy link
Member

@TandersT I think you misunderstood my example, you can add additional parameters by using closures, see this example:

var _t = GetTree().CreateTimer(1);
_t.Timeout += () => MySignalCallback(1, 2, 3);
// Or capturing variables:
int a = 1;
int b = 2;
int c = 3;
_t.Timeout += () => MySignalCallback(a, b, c); // a, b, c are captured by the closure

To answer your last comment:

timer.Timeout += () => FreeFire(_fire); // The _fire variable is captured by the closure

@Zireael07 I'm pretty sure you can use bind/unbind with GDScript for default signals, unless I misunderstood what you meant:

func _ready() -> void:
    var t := get_tree().create_timer(1)
    t.timeout.connect(_on_timeout.bind(1, 2, 3))


func _on_timeout(a: int, b: int, c: int) -> void:
    pass

@TandersT
Copy link
Author

Thank you, that does the trick! That's on my end, I see I misunderstood :)

@Zireael07
Copy link
Contributor

@raulsntos Wow, TIL - this trick is worth documenting in the docs!

@paulloz
Copy link
Member

paulloz commented Sep 20, 2022

this trick is worth documenting in the docs!

It is used in the documentation, e.g. in Tween.

@Zireael07
Copy link
Contributor

Should also be documented in Callable documentation itself, and possibly signals

@paulloz
Copy link
Member

paulloz commented Sep 20, 2022

@Zireael07 Sure. I just added C# examples where there were already GDScript ones. The Callable doc page doesn't provide examples for bind in any language at the moment. And pages outside the class ref are outdated on so many levels...

@raulsntos
Copy link
Member

It's currently documented in Object's connect method (a bit outdated, see #64930):

<description>
Connects a [param signal] to a [param callable]. Use [param flags] to set deferred or one-shot connections. See [enum ConnectFlags] constants.
A signal can only be connected once to a [Callable]. It will print an error if already connected, unless the signal was connected with [constant CONNECT_REFERENCE_COUNTED]. To avoid this, first, use [method is_connected] to check for existing connections.
If the callable's target is destroyed in the game's lifecycle, the connection will be lost.
[b]Examples with recommended syntax:[/b]
Connecting signals is one of the most common operations in Godot and the API gives many options to do so, which are described further down. The code block below shows the recommended approach for both GDScript and C#.
[codeblocks]
[gdscript]
func _ready():
var button = Button.new()
# `button_down` here is a Signal object, and we thus call the Signal.connect() method,
# not Object.connect(). See discussion below for a more in-depth overview of the API.
button.button_down.connect(_on_button_down)
# This assumes that a `Player` class exists which defines a `hit` signal.
var player = Player.new()
# We use Signal.connect() again, and we also use the Callable.bind() method which
# returns a new Callable with the parameter binds.
player.hit.connect(_on_player_hit.bind("sword", 100))
func _on_button_down():
print("Button down!")
func _on_player_hit(weapon_type, damage):
print("Hit with weapon %s for %d damage." % [weapon_type, damage])
[/gdscript]
[csharp]
public override void _Ready()
{
var button = new Button();
// C# supports passing signals as events, so we can use this idiomatic construct:
button.ButtonDown += OnButtonDown;
// This assumes that a `Player` class exists which defines a `Hit` signal.
var player = new Player();
// Signals as events (`player.Hit += OnPlayerHit;`) do not support argument binding. You have to use:
player.Hit.Connect(OnPlayerHit, new Godot.Collections.Array {"sword", 100 });
}
private void OnButtonDown()
{
GD.Print("Button down!");
}
private void OnPlayerHit(string weaponType, int damage)
{
GD.Print(String.Format("Hit with weapon {0} for {1} damage.", weaponType, damage));
}
[/csharp]
[/codeblocks]
[b][code]Object.connect()[/code] or [code]Signal.connect()[/code]?[/b]
As seen above, the recommended method to connect signals is not [method Object.connect]. The code block below shows the four options for connecting signals, using either this legacy method or the recommended [method Signal.connect], and using either an implicit [Callable] or a manually defined one.
[codeblocks]
[gdscript]
func _ready():
var button = Button.new()
# Option 1: Object.connect() with an implicit Callable for the defined function.
button.connect("button_down", _on_button_down)
# Option 2: Object.connect() with a constructed Callable using a target object and method name.
button.connect("button_down", Callable(self, "_on_button_down"))
# Option 3: Signal.connect() with an implicit Callable for the defined function.
button.button_down.connect(_on_button_down)
# Option 4: Signal.connect() with a constructed Callable using a target object and method name.
button.button_down.connect(Callable(self, "_on_button_down"))
func _on_button_down():
print("Button down!")
[/gdscript]
[csharp]
public override void _Ready()
{
var button = new Button();
// Option 1: Object.Connect() with an implicit Callable for the defined function.
button.Connect("button_down", OnButtonDown);
// Option 2: Object.connect() with a constructed Callable using a target object and method name.
button.Connect("button_down", new Callable(self, nameof(OnButtonDown)));
// Option 3: Signal.connect() with an implicit Callable for the defined function.
button.ButtonDown.Connect(OnButtonDown);
// Option 3b: In C#, we can use signals as events and connect with this more idiomatic syntax:
button.ButtonDown += OnButtonDown;
// Option 4: Signal.connect() with a constructed Callable using a target object and method name.
button.ButtonDown.Connect(new Callable(self, nameof(OnButtonDown)));
}
private void OnButtonDown()
{
GD.Print("Button down!");
}
[/csharp]
[/codeblocks]
While all options have the same outcome ([code]button[/code]'s [signal BaseButton.button_down] signal will be connected to [code]_on_button_down[/code]), option 3 offers the best validation: it will print a compile-time error if either the [code]button_down[/code] signal or the [code]_on_button_down[/code] callable are undefined. On the other hand, option 2 only relies on string names and will only be able to validate either names at runtime: it will print a runtime error if [code]"button_down"[/code] doesn't correspond to a signal, or if [code]"_on_button_down"[/code] is not a registered method in the object [code]self[/code]. The main reason for using options 1, 2, or 4 would be if you actually need to use strings (e.g. to connect signals programmatically based on strings read from a configuration file). Otherwise, option 3 is the recommended (and fastest) method.
[b]Parameter bindings and passing:[/b]
For legacy or language-specific reasons, there are also several ways to bind parameters to signals. One can pass a [code]binds[/code] [Array] to [method Object.connect] or [method Signal.connect], or use the recommended [method Callable.bind] method to create a new callable from an existing one, with the given parameter binds.
One can also pass additional parameters when emitting the signal with [method emit_signal]. The examples below show the relationship between those two types of parameters.
[codeblocks]
[gdscript]
func _ready():
# This assumes that a `Player` class exists which defines a `hit` signal.
var player = Player.new()
# Option 1: Using Callable.bind().
player.hit.connect(_on_player_hit.bind("sword", 100))
# Option 2: Using a `binds` Array in Signal.connect() (same syntax for Object.connect()).
player.hit.connect(_on_player_hit, ["sword", 100])
# Parameters added when emitting the signal are passed first.
player.emit_signal("hit", "Dark lord", 5)
# Four arguments, since we pass two when emitting (hit_by, level)
# and two when connecting (weapon_type, damage).
func _on_player_hit(hit_by, level, weapon_type, damage):
print("Hit by %s (level %d) with weapon %s for %d damage." % [hit_by, level, weapon_type, damage])
[/gdscript]
[csharp]
public override void _Ready()
{
// This assumes that a `Player` class exists which defines a `Hit` signal.
var player = new Player();
// Option 1: Using Callable.Bind(). This way we can still use signals as events.
player.Hit += OnPlayerHit.Bind("sword", 100);
// Option 2: Using a `binds` Array in Signal.Connect() (same syntax for Object.Connect()).
player.Hit.Connect(OnPlayerHit, new Godot.Collections.Array{ "sword", 100 });
// Parameters added when emitting the signal are passed first.
player.EmitSignal("hit", "Dark lord", 5);
}
// Four arguments, since we pass two when emitting (hitBy, level)
// and two when connecting (weaponType, damage).
private void OnPlayerHit(string hitBy, int level, string weaponType, int damage)
{
GD.Print(String.Format("Hit by {0} (level {1}) with weapon {2} for {3} damage.", hitBy, level, weaponType, damage));
}
[/csharp]
[/codeblocks]
</description>

But it should probably be documented in Callable too.

@paulloz
Copy link
Member

paulloz commented Sep 20, 2022

Missed that one while reading through the docs. I can update those if you need me to 🙂

@raulsntos
Copy link
Member

I already update it in #64930, marked as a draft pending some changes to Callable on C#'s side.

@wp2000x
Copy link

wp2000x commented Nov 23, 2022

@TandersT I think you misunderstood my example, you can add additional parameters by using closures, see this example:

var _t = GetTree().CreateTimer(1);
_t.Timeout += () => MySignalCallback(1, 2, 3);
// Or capturing variables:
int a = 1;
int b = 2;
int c = 3;
_t.Timeout += () => MySignalCallback(a, b, c); // a, b, c are captured by the closure

To answer your last comment:

timer.Timeout += () => FreeFire(_fire); // The _fire variable is captured by the closure

@Zireael07 I'm pretty sure you can use bind/unbind with GDScript for default signals, unless I misunderstood what you meant:

func _ready() -> void:
    var t := get_tree().create_timer(1)
    t.timeout.connect(_on_timeout.bind(1, 2, 3))


func _on_timeout(a: int, b: int, c: int) -> void:
    pass

How to remove such a lambda bind from the signal after it was added?

@raulsntos
Copy link
Member

How to remove such a lambda bind from the signal after it was added?

Do you mean disconnect the lambda from the event? It works like it does in C#, you need to store the lambda:

// Store the lambda in a variable
var myCallback = () => MySignalCallback(1, 2, 3);

// Connect the callback to the signal
MySignal += myCallback;

// Disconnect the callback from the signal
MySignal -= myCallback;

Keep in mind, the events we generate with source generators don't generally need to be disconnected because on disposing the object we will automatically disconnect all signals, if that's what you are worried about:

void CSharpInstance::mono_object_disposed(GCHandleIntPtr p_gchandle_to_free) {
// Must make sure event signals are not left dangling
disconnect_event_signals();

@TandersT
Copy link
Author

The method described here fixed my issues, and since this Callable seems to have its docs updated, hence closing this issues.

@github-project-automation github-project-automation bot moved this from Todo to Done in 4.x Priority Issues Feb 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

6 participants