Fix error when closing Attach Node Script window

[volzhs]

Fix #34471

[akien-mga]

I guess it could be CONNECT_ONESHOT, and then the disconnect can be removed?

[akien-mga]

Ping.

[akien-mga]

Force pushed an amend that does that.

[akien-mga]

I wonder if it's really needed to do these disconnects at all in the first place. popup_hide is only used here to disconnect the script_created signal, but do we need it disconnected?

The only reason I can think of would be that this ScriptCreateDialog instance is also reused in ScriptEditor, where it's connected but there never disconnected:

editor/plugins/script_editor_plugin.cpp
296:void ScriptEditor::_script_created(Ref<Script> p_script) {
3133:   ClassDB::bind_method("_script_created", &ScriptEditor::_script_created);
3415:   script_create_dialog->connect("script_created", this, "_script_created");

If it works seemlessly to re-connect the signal without having disconnected it first, then we could simply get rid of the disconnect callback.

[akien-mga]

The only reason I can think of would be that this ScriptCreateDialog instance is also reused in ScriptEditor, where it's connected but there never disconnected:

Actually each class has a separate instance of ScriptCreateDialog, so there's no "reconnection" as I mentioned above. So maybe we can just connect the script_created signal in the constructor and then leave it be.

[akien-mga]

Actually each class has a separate instance of ScriptCreateDialog, so there's no "reconnection" as I mentioned above. So maybe we can just connect the script_created signal in the constructor and then leave it be.

It seems that's what the code used to do, but it was changed in #30196 by @LikeLakers2 for the sake of editor plugins that call ScriptCreateDialog themselves. Not sure if that PR was the best approach, but it does make some sense so I guess for now we should just stick to making the popup_hide callback a oneshot.

[LikeLakers2]

@akien-mga I don't remember it ever having multiple instances of ScriptCreateDialog when I searched through the code (although the last I checked was when I made #30738), whether for editor plugins or otherwise. All get_script_create_dialog()s would inevitably point towards SceneTreeDock's dialog. Or at least, that's how the code looked to me.

That said, I would recommend having multiple ScriptCreateDialog instances if we can code it that way, so long as doing so won't clash with the others. The only reason I haven't done it myself yet is because... I kinda felt it might make a mess of the code when all I wanted to do was fix a bug. Plus I didn't feel right doing that sort of big change just to fix a bug. So... :/

Also: If you want to revert my PR (i.e. if it's just easier to implement your own solution than work off of mine) please feel free to do so. Wouldn't bother me.

[akien-mga]

Thanks!