godotengine / godot-proposals

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

Add a warning when using signals in unusual contexts #6666

Open L4Vo5 opened 1 year ago

L4Vo5 commented 1 year ago

Describe the project you are working on

UI, but should apply to anything

Describe the problem or limitation you are having in your project

GDScript allows using signals in probably-undesired contexts without really complaining. For example, if you have a button variable, it's easy to accidentally type if button.pressed: when what you really meant to type was if button.button_pressed: If you make that error, godot won't complain, it'll just always evaluate to true and leave the user confused (I assume it's checking that the signal isn't null or something? but that's irrelevant to the issue). I'm not sure how much of a problem this actually is for most other nodes, but for buttons specifically I've been confused more than once. Of course, there'd need to be more people who run into this before it's really worth fixing. I think beginners specially, who don't really know where to look for errors, might get stuck for a while before they figure out what's going on.

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

There should be a warning emitted when signals are used in unusual contexts. I'm not sure how far-reaching it should be, as for example typing var is_pressed = button.pressed might not do what the user expects, but generally things like var my_signal = button.pressed are valid and might have their use cases. At the very least, I think evaluating the truthiness of signals should be what's warned against. If you're doing it on purpose you can always be explicit with my_signal != null or other such methods, in which case the warning shouldn't trigger.

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

I don't care for the exact name and wording, but the warning could be something like: signal_evaluation: "Object signal is evaluated in boolean expression. Were you trying to access a property with a similar name?"

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

It can be worked around by not making the mistake, but then again, so can every other warning

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

Warnings are for things the user may not know or remember. Nobody's gonna hunt down an add-on for this before they even learn about it, and after they do, they likely still wouldn't bother.

dalexeev commented 1 year ago

Note that in 4.0, properties, constants, methods, and signals can no longer have the same name.