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

Expose signals and callbacks as C# events #15558

Closed
mysticfall opened this issue Jan 10, 2018 · 10 comments
Closed

Expose signals and callbacks as C# events #15558

mysticfall opened this issue Jan 10, 2018 · 10 comments

Comments

@mysticfall
Copy link
Contributor

Godot version:
master / 2ac2760

OS/device including version:
Manjaro Linux 17.1-rc2

Issue description: (This issue is a feature request)
Currently, in order to receive various callback events, like when a node is added to the tree or receives an input, one needs to override relevant methods (i.e. _EnterTree()).

However, in case of C# projects, some might prefer accessing them as events since it's more of an idiomatic way to implement an observer pattern. And the same can be said of signals like Button.pressed, for example.

Maybe we can add additional events along with existing callback methods and signals to provide an alternative method for those who prefer a more idiomatic approach. Or, we might let users to choose whether to enable such a feature by using a conditional compilation symbol.

@27thLiz 27thLiz added this to the 3.1 milestone Jan 10, 2018
@PJB3005
Copy link
Contributor

PJB3005 commented Jan 11, 2018

Small note: not all signals are also emitted as virtual calls and vice versa. You can still register to signals using Godot.Object.Connect(). Though yes, the type safety and such from C# events would definitely be nice, along with allowing things like arbitrary delegates.

@akien-mga
Copy link
Member

@neikeq Isn't this done by now?

@neikeq
Copy link
Contributor

neikeq commented Sep 15, 2018

No, this is not done yet. I would really like it to happen, but there are still some problems to settle first.

Supposing we don't change the way signals and connections work

The situation would be like this. We have the following "signal" event:

delegate void NameChangedDelegate(string name);

[Signal]
event NameChangedDelegate NameChanged;

Raising this event should not only call its delegates, but also EmitSignal with the event name. This is so that the engine can also subscribe to events from this signal.
For this to happen, the engine would have to add a special delegate to the event after constructing the object. This raises a few problems like what if the user clears all delegates or sets the event to null.
We likely also want EmitSignal to raise the signal event, this way it can work like any other signal. This introduces the biggest. First the delegates will get called twice, and second:

  • raise event -> EmitSignal
  • EmitSignal -> raise event
  • raise event -> Emit Signal
  • ...
  • Stack overflow at some point

This can probably be fixed by making EmitSignal aware it's being called from a raised signal event, so that it doesn't raise the signal event again.

We also want get_signal_connection_list to include the delegates added to the event.

The other option is to extend the way signals and connections.

Connecting a signal from the engine would actually add a delegate to the event. I think it would fix all some of the problems without introducing new ones (except the get_signal_connection_list problem, for delegates added from C# with += instead of Connect), but I haven't thought much about this yet.

Edit: One possible problem introduced. What about connections added before the script instance is created.

@neikeq
Copy link
Contributor

neikeq commented Sep 15, 2018

One thing we can definitely do in the meantime is add an overload of Connect that replaces the target object and method name with a delegate.

@akien-mga akien-mga modified the milestones: 3.1, 3.2 Jan 9, 2019
@akien-mga akien-mga modified the milestones: 3.2, 4.0 Nov 14, 2019
@Calinou
Copy link
Member

Calinou commented Apr 11, 2020

This seems to be now implemented judging from this progress report. Can you confirm this, @neikeq?

@neikeq
Copy link
Contributor

neikeq commented Apr 12, 2020

Yes, this is implemented for signals in 6a85cdf.
This is not going to happen for callbacks, as they don't necessarily to translate to events. In any case, I suggest requesting callbacks to be replaced (or complemented) with signals where it makes sense. The _EnterTree callback for example already has the equivalent TreeEntered signal as an event.

Note: The problem I described in my first comment was not resolved yet. Right now we still need EmitSignal calls.

@neikeq neikeq closed this as completed Apr 12, 2020
@AldieNightStar
Copy link

AldieNightStar commented Apr 23, 2020

Hey! Would be cool to have some option to pass Functions as an parameters into arguments of the Signals. For now there is Marshaling error

@majdisorder
Copy link

awesome. Is there any documentation on how to implement this? the official docs still show the SignalAttribute + delegate method.

@neikeq
Copy link
Contributor

neikeq commented Sep 7, 2020

@dimi3tron It's just an event with the Signal attribute, e.g.:

[Signal]
event Action Pressed;

Currently you need to use EmitSignal instead of raising/invoking the event, otherwise connections made via Connect won't be called. That may be solved in the future.

Keep in mind this is only available in 4.0 which is not released yet.

@majdisorder
Copy link

Thx. For some reason I was under the impression it was already in the current build. Great to see these kinds of improvements, really makes Godot feel more like home for long time c# developers like myself.

Excited for 4.0!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants