Elaborate on C# Connect/Disconnect #8812
Conversation
| @@ -30,8 +36,13 @@ In addition, you can always access signal names associated with a node type thro | |||
| .. warning:: | |||
|
|
|||
| While all engine signals connected as events are automatically disconnected when nodes are freed, custom | |||
| signals aren't. Meaning that: you will need to manually disconnect (using ``-=``) all the custom signals you | |||
| connected as C# events (using ``+=``). | |||
| signals connected using ``+=`` aren't. Meaning that: you will need to manually disconnect (using ``-=``) | |||
There was a problem hiding this comment.
The "as events" on the line above already implies the "connected using +=" for me, that's why it was worded like this.
There was a problem hiding this comment.
I think this is ambiguous:
While all engine signals connected as events are automatically disconnected when nodes are freed, custom signals aren't.
While all (engine signals connected as events) are automatically disconnected [...], (custom signals) aren't.
vs.
While all (engine signals) (connected as events) are automatically disconnected [...], (custom signals) (connected as events [implied]) aren't.
I don't know if that's enough to convey what I mean, but I didn't even consider the second interpretation--I thought this line was entirely mistaken when I first read it. 😄
There was a problem hiding this comment.
Yeah, the whole sentence is too complicated for me either way, so I don't really have a strong opinion. I hate we don't have a simple way to convey +=.
There was a problem hiding this comment.
I'll take another look later with fresh eyes... yeah, pulling apart the complexity a bit is probably the right move.
And yeah, this is all already a nightmare in casual convos, getting it right in a doc seems like it may be tough. 😅
There was a problem hiding this comment.
After playing with the behavior a bit, I found a whole new chapter to this about capturing variables, so ended up writing a decent amount at #8815.
Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> Co-authored-by: Paul Joannon <437025+paulloz@users.noreply.github.com>
raulsntos
added
enhancement
topic:dotnet
area:manual
There was a problem hiding this comment.
As you say I did imagine it wouldn't take too long to fix and didn't want to discourage users from the event syntax. Unfortunately, there's problems with the proposed fix (godotengine/godot#73730) so it may still take a while. Therefore, I'm more than willing to correct the documentation now (as it should've been done long ago).
Thank you for contributing this PR, it looks good to me so I'll approve but let's wait for @paulloz approval too.
|
It’s fine by me. And (re)documenting the calls to `Connect` and `Disconnect` is a good thing.
… |
|
Thanks! |
|
Cherry-picked to 4.2 in #9648. |
The C# signal page mentions the
ConnectandDisconnectAPIs (defined onGodotObject), but not when you might want to use them, or how. When looking at the class doc, there's technically an example in there, but it seems limited by only translating the code examples rather than having a C#-specific description. It's never mentioned why you might still want/need to use these. I added some examples and some details about this.This also clarifies the note about godotengine/godot#70414. Right now it doesn't link to the active issue, so I added a link, and it also doesn't mention that using
.Connectis another way around it. Depending on how much a particular dev hates manual cleanup, this might be significantly more appealing.I imagine the note might have been kept brief rather than mentioning more about it because this was imagined at the time to perhaps be a short-lived problem. But there doesn't seem to me to be a clear, consensus path forward at godotengine/godot#73730, so I think it's worth the detail.
/cc @paulloz (#8538)