godotengine / godot-proposals

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

Add an warning or error when trying to use the toggled signal with button with toggle mode off #10875

Open leandro-benedet-garcia opened 1 week ago

leandro-benedet-garcia commented 1 week ago

Describe the project you are working on

Just as couple games that uses toggle buttons.

Describe the problem or limitation you are having in your project

Not a limitation, just a Quality of Life improvement.

it would be interesting to have a warning on the toggled(toggled_on: bool), that tells you that the button is not in toggle mode.

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

it helps by not wasting time with debugging. That is something that I got caught trying to debug twice and spending more than 30 minutes, and once in a while I see people in Discord with same issues, and that is something that is EASILY over-sighted. And the problem is that the default behavior when the code is executed and toggle mode is off, the signal does nothing.

In my opinion it should raise an error or an warning at least.

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

I am unsure if signals tab is capable of having warnings, but my proposal kinda proposes the hability to do so if is not implemented.

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

If you override the node button, maybe, but it is a common issue that people just, forget is a thing.

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

It affects everyone. And the problem is that even if YOU KNOW that it needs to be on toggle mode, you can easily forget and slip out from your mind and then can waste an hour trying to debug. So much the second time it happened I only figured out the issue when I was going to manually re implement the signal using button_down

Also, I will mention one thing that I know will be mentioned.

"It is documented and tells you if you mouse over"

Here's the thing.

The signal is straight forward to use. And very obvious from the name alone what it is supposed to do. Get the state from variable and then do whatever you want with it.

But when suddenly it does nothing, you go trough multiple steps before considering to check the manual or that it is intended behaviour.

leandro-benedet-garcia commented 1 week ago

Another alternative is to separate the regular button and create a new ToggleButton node.

AThousandShips commented 1 week ago

There's no way to detect this unless we add a whole new system for it, there's no system for running checks when a signal is connected, further this might be correct behaviour and someone uses the signal only some times, enabling and disabling toggle as needed, so then this warning would be confusing and wrong

KoBeWi commented 1 week ago

There is a way to check connections to a specific signal, so we could add a debug-only check e.g. in READY.

leandro-benedet-garcia commented 1 week ago

There is a way to check connections to a specific signal, so we could add a debug-only check e.g. in READY.

That would be helpful, if there were a warning like this image, it would be useful too: image

KoBeWi commented 1 week ago

Displaying it in the editor is problematic, because it can't refresh right after connecting (there is no signal or callback for that). It would only appear after you reload the scene.

leandro-benedet-garcia commented 1 week ago

Displaying it in the editor is problematic, because it can't refresh right after connecting (there is no signal or callback for that). It would only appear after you reload the scene.

how difficult would be to create such a callback? May I create another proposal for it? It sounds useful for other things beyond just warnings.

KoBeWi commented 1 week ago

It would need to happen when connecting any signal, so it might impact performance. Although it could be editor-only I guess, but then it would be tricky to use it in Button. It's doable with some hacky call().

leandro-benedet-garcia commented 1 week ago

It would need to happen when connecting any signal, so it might impact performance. Although it could be editor-only I guess, but then it would be tricky to use it in Button. It's doable with some hacky call().

Would it decrease that much? And would it impact things overall? Adding signals runtime doesn't sound something that is frequently made.

I only do that in instancing of objects, and I believe I only ever instanced a bunch of objects with projectiles, even then there are workaround to not get hit with performance.

KoBeWi commented 1 week ago

The engine can connect various signals internally. But if it's editor-only (i.e. only when connecting from editor) then it wouldn't be a problem.