Add a connection flag to prevent firing signals from editor

[KoBeWi]

Describe the project you are working on

Some projects in Godot 4.0

Describe the problem or limitation you are having in your project

Godot 4.0 recently got a feature that allows you to unbind signal arguments when connecting them, so you can discard excessive arguments when connecting from the editor. This is great, I can finally connect animation_finished to queue_free and it works.

Well, almost XD The signal is emitted in the editor (without any tool script etc) and will crash the editor if you decide to free the root node. Some old issue: https://github.com/godotengine/godot/issues/13394

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

To prevent such cases, we could block the method from being called. To do this, I suggest adding CONNECT_NO_EDITOR connection flag. When there's a connection with this flag, the target method will not be called when a node receives this signal. Editor would automatically add this flag when connecting from the Node dock. This way the users can avoid unexpected crashes or scene modifications.

(making this flag optional could be useful for plugins though)

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

Add CONNECT_NO_EDITOR connection flag. When connecting signal from the editor, this flag would automatically be added. The code that disables calling method could be wrapped in #ifdef TOOLS_ENABLED, so it doesn't affect non-editor builds.

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

The workaround is to add methods (or sometimes scripts that weren't necessary) to avoid the editor crashing, but this is meh.

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

Must be core.

[YuriSizov]

I feel like there should be another solution that requires less user interaction and awkward flags. Because having this flag on connections in scripts which aren't tool is very awkward.

(making this flag optional could be useful for plugins though)

Wouldn't really help plugins, as you need to work on them without breaking them too. It's the same old problem of the dual purpose of the tool mode. The only real solution here would be to split the tool context into two, one for adding interactivity to edited scenes and another for running plugins.

[KoBeWi]

another solution that requires less user interaction

But this solution requires 0 user interaction. The flag would be set automatically (unless maybe disabled).

Wouldn't really help plugins, as you need to work on them without breaking them too.

But the things this flag would prevent don't usually happen in plugins. My particular problem is AnimationPlayer causing queue_free() to be called and crashing the editor; this doesn't happen in plugins, because you are not freeing nodes with an animation there.

[YuriSizov]

But this solution requires 0 user interaction. The flag would be set automatically (unless maybe disabled).

For people using scene-based connections. And for the rest of us you need to mark every connection as NO_EDITOR in code.

this doesn't happen in plugins, because you are not freeing nodes with an animation there.

Why not? :eyes: Your plugin can have any kind of scene, so it can have a scene that removes something after an animation is finished too.

A new CONNECT_NO_EDITOR flag would be useless for plugins, because it cannot be used in any plugin scene without breaking its logic. But the underlying cause for your crash can still happen in plugin scenes as you edit them. And having this flag by default for all scene-based connections means that you can no longer use scene-based connections for plugin scenes, because their connection logic won't work when running the plugin. So you'd need a toggle...

[KoBeWi]

And for the rest of us you need to mark every connection as NO_EDITOR in code.

Well, with code connections you don't usually run in such problems. And you don't need to use the flag at all if you are fine with the standard behavior.

Why not?

I mean it just doesn't apply to the vast majority of the plugins. Freeing root node from animation implies some temporary scene, e.g. explosion, pickup effect etc. Sure, plugins can have these too, but pretty sure they don't in 99.9% cases.

So you'd need a toggle...

Yeah.

[YuriSizov]

Well, with code connections you don't usually run in such problems. And you don't need to use the flag at all if you are fine with the standard behavior.

You're saying that it crashes the editor. That's not the standard behavior anyone would be fine with. I don't see how a way of connecting signals is important here, unless the reason it crashes is because it's a scene-based connection. Then it's a bug that needs to be fixed, and the fix is unlikely to be a special flag.

[KoBeWi]

I mean, it's what we have currently. And you can't really prevent it - the signal is emitted in the editor, received in the editor, the method is called because it's a C++ method, so it bypasses the need for tool keyword which is in scripts. If you have a better solution than explicitly blocking connections in the editor I'm all ears 👂👂

The problem was there for a long time, but it wasn't as visible, because there aren't many commonly used signals that you could connect. Like, you can e.g. connect tree_entered to queue_free to make the scene unable to be opened.

But now we have more options for connecting nodes due to unbind functionality, so solving this is a bit more important.

[YuriSizov]

the signal is emitted in the editor, received in the editor, the method is called because it's a C++ method, so it bypasses the need for tool keyword which is in scripts.

The tool keyword is needed for the script to be loaded in the editor environment. So the fact that a non-tool script is loaded and its methods are executed sounds like a different kind of problem that should probably be solved, superseding this specific issue and your proposed solution 🤔

Normally, you can use non-tool scripts in the editor context only by loading them directly from another tool script. Or, I guess, this encapsulation also breaks with built-ins, because they always load with the owner scene? Since you use them a lot, I assume this is also the case for when you experience the crash? Would it crash with a discrete script?

PS. I naturally agree that the crash condition needs to be fixed, I'm just not sure about this solution.

[KoBeWi]

No this is working as intended. The signal calls a core method in a node, it can happen without any script. I don't think we can reasonably block calling non-script methods.

[YuriSizov]

Ah, right, there is no script at all. So it's either this flag or some way to mark the scene itself as tool to block some of its logic from running in the editor (like signal connection handling, but maybe something else too). And plugins are screwed either way without a proper solution for splitting execution contexts...

Oh well, I guess a flag for scene-based connections is fine. With a UI toggle to allow it to be run in editor (I think it should be opt-in, not opt-out, at least in the editor's UI).

[SysError99]

This feature is also very viable in 3.x stable branch. I need to look forward on bringing this into 3.x

[therealeldaria]

As a newbie Godot user trying to follow a course I could not understand why my Editor would crash just because I enabled a one shot on a particle effect. I opened a bug and it was tagged to another issues that I could follow to another issue and a suggested fix, then to this discussion. I find it really odd that the editor would react to a queue_free when not in runtime mode. As it is now I have to go and remove the signal while playing around with the parameters, then reconnect it before running the game. And curse that I again forgot it when the editor crashes.