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

Ability to add custom signals to nodes from the editor #3655

Closed
me2beats opened this issue Dec 11, 2021 · 17 comments
Closed

Ability to add custom signals to nodes from the editor #3655

me2beats opened this issue Dec 11, 2021 · 17 comments

Comments

@me2beats
Copy link

me2beats commented Dec 11, 2021

Describe the project you are working on

Plugins

Describe the problem or limitation you are having in your project

I have cases when I would like to add my own signals to any node in a scene from the editor (not from a script).
For example I created a scene called InputButton

input button

When I press the button, the ConfirmationDialog with LineEdit pops up.
I find this controls combination quite frequent in my code so I decided to make it a scene so that I could easily reuse it (add instances of the scene anywhere).

In InputButton node I want to create a signal confirmed, that would be emitted when I type something in LineEdit and press Enter or press ConfirmationDialog Ok button.

The problem is I don't want to create the signal from a script attached to InputButton, because perhaps I will set another script on it (in order to create another abstraction); I don't want to use script inheritance in this case.
The signal should be "static" (?), I should be able to use it from the editor.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

An ability to add custom signals to nodes from the editor would solve the problem.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

There is Node dock with Signals tab.
From there one could add signals to any node of any scene, without the need to create a script for it.

If this enhancement will not be used often, can it be worked around with a few lines of script?

it can't

Is there a reason why this should be core and not an add-on in the asset library?

I don't think this is possible with a plugin

@KoBeWi
Copy link
Member

KoBeWi commented Dec 11, 2021

I don't want to use script inheritance in this case.

Why? This is perfect case to use inheritance.

@me2beats
Copy link
Author

Why? This is perfect case to use inheritance.

This can make the code more confusing and increase the refactoring time.
This is still an option but I'd like to avoid script inheritance when possible.

@Zireael07
Copy link

Even if OP's use case can be solved the other way, there is a ton of use cases where this would be very helpful.

@KoBeWi
Copy link
Member

KoBeWi commented Dec 12, 2021

This can make the code more confusing and increase the refactoring time.

How is 3-line script confusing and increases refactoring time? (you don't need tool btw, unless you use it in editor plugin)

Stand-alone signal is useless without a code that will emit it. In your example, you can't connect text_entered directly to emit_signal, because it sends the entered text. You need some intermediate script that will handle your actions and emit the signal. You can of course emit it from another node, but this is wrong usage of signals (it's sometimes needed, but very rarely) and the emitter would need to be outside your InputButton instance, which kills the purpose of instances.

there is a ton of use cases where this would be very helpful.

Name one that doesn't require any kind of script.

@Zireael07
Copy link

Obviously all of those require a script, but being able to connect things without having to type a laundry list of blah.connect(ble) would be wonderful.
In my space game, I have a script that has 15-20 of such lines since it needs to react to a lot of things (I forgot if it's the ships or the planets)

@KoBeWi
Copy link
Member

KoBeWi commented Dec 12, 2021

Unless I misunderstand something, the proposal is about being able to declare a signal without using a script. Any signal you add to script will appear in the connection dock and you can connect it from editor. You can't emit the signal without script (unless you connect other signal to emit_signal, which is not always possible), so not sure what's the problem here.

being able to connect things without having to type a laundry list of blah.connect(ble) would be wonderful.

But you can do that already...

@Zireael07
Copy link

But you can do that already...

If I can, either that wasn't the case in 3.3 or it was bugged. (I haven't used 3.4 much so I might have missed an improvement there)

@KoBeWi
Copy link
Member

KoBeWi commented Dec 12, 2021

extends Node2D
signal my_signal

image
You can connect it normally like any other signal. It works like that at least since 3.0. Or you are referring to something else?

@Zireael07
Copy link

Ah. Never noticed custom signals showed there, but there is one case when you can't connect them this way - when the source and target are in different scenes (can happen in large enough projects)

@KoBeWi
Copy link
Member

KoBeWi commented Dec 12, 2021

there is one case when you can't connect them this way - when the source and target are in different scenes (can happen in large enough projects)

That's unrelated to this proposal though.

@Zireael07
Copy link

Should I open a new proposal then? To me, it is sorta related...

@KoBeWi
Copy link
Member

KoBeWi commented Dec 12, 2021

This proposal is about creating signals in connections dialog without needing to use a script.

Yours is different. Also no idea how you imagine it would work, because if you have 2 completely separate scenes, there is no way for one scene to know about the other one, so you can't connect them. Unless you instance both of them in a parent scene, in which case connecting from editor is already possible. But maybe you have some idea, so feel free to open a new proposal.

@me2beats
Copy link
Author

How is 3-line script confusing and increases refactoring time?

If I only want to create a signal, why do I need to create a script( = file) for each node?

Stand-alone signal is useless without a code that will emit it.

Like groups, isn't it?
They also useless without code, but nodes can be added to them from editor.

You can't emit the signal without script (unless you connect other signal to emit_signal, which is not always possible)

For example, a node has signals s1, s2, s3 and I want s1 and s2 to trigger/emit s3.
I don't need a script to do this
I can configure calling emit_signal("s3") from the editor.
this is already possible in Godot 3

You can of course emit it from another node, but this is wrong usage of signals.

I agree it may look strange and confusing, but what makes it wrong?
I think it wouldn't be bad to emit that signal from another node (script) of the scene (from a direct child for example)

@KoBeWi
Copy link
Member

KoBeWi commented Dec 12, 2021

If I only want to create a signal, why do I need to create a script( = file) for each node?

The editor will look up signals in the node's classes and script is part of that. Adding a signal to node without a script is like adding a method or a variable without a script.

Like groups, isn't it?
but what makes it wrong?

Signals are designed to be emitted by the node that declares them. It helps decoupling nodes. Doing otherwise is technically not wrong, but just goes against their design.

Also groups are entirely different concept. You can add a group that will be used only by other nodes. Actually, if you plan to add a signal to a node, but only emit it from other nodes, so that other connected nodes can receive it, you could as well use a group.

For example, a node has signals s1, s2, s3 and I want s1 and s2 to trigger/emit s3.
I don't need a script to do this

How exactly are you planning to emit s1 and s2 without a script? There is always a script somewhere that emits the signal. Except for the built-in signals, but as I said, it won't work in all cases. You can't connect e.g. Button.toggled to emit_signal, because it passes additional argument.

@me2beats
Copy link
Author

How exactly are you planning to emit s1 and s2 without a script?

Yea, connect to emit_signal("s1"/"s2") 4

You can't connect e.g. Button.toggled to emit_signal, because it passes additional argument.

Will #762 solve it?

@KoBeWi
Copy link
Member

KoBeWi commented Dec 12, 2021

Will #762 solve it?

Not really, you can't both bind and unbind arguments.

@me2beats
Copy link
Author

me2beats commented Jun 11, 2022

closing in favor of inheritance and due to implementation difficulties

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

4 participants