godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.14k stars 93 forks source link

Enhance Signal Connection UI for Improved User Experience and Flexibility #8828

Open megonemad1 opened 9 months ago

megonemad1 commented 9 months ago

Describe the project you are working on

2D RPG about making tools, to get resources to beat monsters.

Describe the problem or limitation you are having in your project

Signal Connection in the editor is cumbersome and lacking in flexibility. it's the main place where beginners hit signals and doesn't make it obvious how they can be used effectively. Currently, the UI offers bind/unbind and adds arguments behind an advanced checkbox. the bind removes ALL arguments without any fine-grained control. add extra arguments is a list that can't be rearranged after adding elements and is ALWAYS at the end of the parameter list. there is no feedback that the node you are connecting to has an argument passed with it. The below image is the current state of play. image

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

Change "Extra Arguments" to "Arguments" and add the list of arguments provided by the signal in the list. make the list a GD.Collections.Array, and abstract away the bind unbind logic to be determined by the array its elements and its order.

this means a signal that has a Node argument it provides could run a function like AnimationPlayer.play since you can tell it to drop the node and add the string argument required. this also makes signals more explicit and obvious as to what's happening.

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

image image

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

Used often, only real work around is duplicating signals and having one with no args

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

currently, the signal connection UI in the core godot does not clearly indicate what parameters are being used or give the user the flexibility to use the full functionality of signals.

This is in aid of avoiding small gd boilerplate scripts, eg:

func on_signal(node:Node):
    const key = "key1"
    sink_function_1(key,node)

func on_signal(node:Node):
    const key = "key2"
    sink_function_2(key)

Connecting a signal to a function should be as intuitive as connecting cables from one device to another as this is ultimately what is happening.

Calinou commented 9 months ago

This would require modifying the array property editor to allow marking certain indices as read-only (and non-reorderable).

megonemad1 commented 9 months ago

The proposal actually surgesting allowing reordering, since connecting a signal that provides a node to a function that takes a string key and a node would require reordering the list: Signal(Node) + "key" -> sink(string key, node) Currently the above will require glue code to connect despite being the right values given

Secondly if making it read only is problematic making it pre-populated with a placeholder value that represents the one provided by the signal would achieve the same effect.

If the placeholder is changed it would be the same as dropping (unbinding) the signal argument and using only static values

Last note I want to highlight this isn't intended to mutate or proform logic in the signal connection. Instead please think of it as a telephone patch board or an electronics breadboard that allows you to connect the source and sink together in a flexible way

I hope this is clearer but let me know if you can see any more issues with the proposal

dalexeev commented 7 months ago
  • remove unbind (it's not very clear and it's functions are achievable with the above)

Unbind is not about removing bound arguments, it is about removing call arguments. So the field should not be removed without an alternative. (Also, you can bind and unbind at the same time, and the order is important, but I think for the editor this case is not so important.)

megonemad1 commented 6 months ago

Removing call arguments as in removing arguments from the values that will be passed to the called function. Not removing bound arguments which are bound to the signal and are a definition of what that signal will provide when it's called.

This means if you have a signal with onThing that has a string argument that is provided by the signal when it is emitted. You use unbind to drop the string in the signal connection so it never is passed to the argument to the signal handlers parameter.

Is this consistent with your definitions of bound/call arguments?

When I'm using signals in the editor as boundaries between scripts functionality I've found that sometimes I don't care for the specifics of a signal arguments and instead care about the fact the signal happened. For example onHit(float damage) there are use cases where the amount of damage is not needed. Meanwhile the function your connecting to may want a parameter that is constant to the connection. If the signal has no provided arguments then I can give this argument constant to the connection as an extra argument. But in the case of a signal like the onHit above it will not be possible as the damage float can't be dropped and an extra argument given. Similarly in the case of setting functions that take a key and value eg setStat(StringName stat, float value) it wouldn't be possible to connect a signal that provides a float since the additional arguments are always added at the end of the list. Meaning you can't set a StringName in the connection that will then be used as that will be (value StringName) and single the above is a common pattern in library it means you can't just change the function signature without messing with the library.

I hope I've better explained what I'm trying to get across but if it's still unclear let me know what. I don't think this is at its heart a problematic addition I'm just bad at describing it accurately enough.