godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.1k stars 69 forks source link

Warning/error if a receiver method doesn't exist #8982

Open carlrthomas opened 6 months ago

carlrthomas commented 6 months ago

Describe the project you are working on

A post-apocalyptic feel-good, bright pixel-art 2D beat-em-up/shoot-em-up

Describe the problem or limitation you are having in your project

It is really easy to accidentally make a typo somewhere on my code and not notice it. This includes making a typo in the name of a receiver method.

  1. Connect a signal to a receiver method using the editor
  2. Change the name of the receiver method in the code (without re-connecting)
  3. The code will run, and you will never be made aware that the signal no longer has a receiver method to call.

I had this happen by accident where on my laptop I accidentally tapped the name of a receiver and I guess pressed a button thinking I was in another window. This changed the name of the receiver method, and nothing indicated to me that the signal didn't have a matching method anymore.

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

Add at least a runtime error/warning noting that the receiver method doesn't exist. Just spent a kinda embarrassing amount of time trying to diagnose a silly issue... and I'd made a typo in a signal method, but there were no errors or warnings to indicate this. While of course, I ended up realizing through Git diff comparisons that I had done this, it would've been nice to accelerate this because I didn't even know this was related to the problem as I hadn't intentionally made the change.

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

Runtime error/warning at the very least. Preferably, though, if a signal is connected in the editor (not programmatically in code), there should be a warning/error in the output console or next to a given signal connection in the Signals tab indicating the method is missing.

An alternative could maybe be the addition of an annotation @connect (or something signal-related) that would force the editor to verify it's connected to a signal or highlight the function w/ an error (this could still have the limitation of only working with editor-connected ones).

@connect func _on_timer_timeout():
    pass

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

Would probably be quite challenging to, but perhaps some kind of unit test-ish plugin could loop through and verify?

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

Feels like a basic editor feature, akin to the syntax errors that do get highlighted as-is. The fact that methods show a little connection icon next to them suggests on the other hand that they should inform if they aren't if they are intended for that purpose (an especially good argument for the @connect annotation). And for programmatically connected ones, it already gives an error when the Callable provided doesn't exist.

Mickeon commented 6 months ago

Could you be a little clearer? Where did this issue stem from? The description of the problem is extremely vague. I can look for a bit more context, but not everyone knows or has the time.

Zireael07 commented 6 months ago

They made a signal and typo'ed the name of the method to connect to. They want a warning if a signal is connected to nonexistent method

carlrthomas commented 6 months ago

Could you be a little clearer? Where did this issue stem from? The description of the problem is extremely vague. I can look for a bit more context, but not everyone knows or has the time.

Made some edits I hope clarify things.

FabriceCastel commented 6 months ago

Logging warnings every time you emit to a signal that has no callbacks is probably going to get very noisy. There's a lot of situations where it's totally expected for a signal not to have any listeners, so I don't think that logging errors should be the default behaviour.

That said, I think it would be valuable to have an option to toggle this behaviour either globally (probably easier to implement) or as an optional bit of syntax at the signal level, eg.

non_empty_signal some_signal(foo: int, bar: bool)

That logs a warning/error whenever the signal gets emitted but invokes no callbacks

Not the most thought out feedback, just some scattered thoughts

sancho2 commented 5 months ago

Logging warnings every time you emit to a signal

Sure, but I think we would want this warning when connecting rather than emitting.

amarraff commented 3 weeks ago

I would also like to see this be improved. I've experienced this issue multiple times, specifically with signal parameters; if a connected receiver function doesn't contain the exact parameters (or lack thereof) a signal is expecting, no warning or error will appear. It just won't work.