ERROR: Class 'VisualScriptEditor' already exists

Gallilus commented 3 years ago

Godot version

4.0.dev 19-08-2021

System information

Windows 10

Issue description

Can not open old project's and a new project gives this error when opening a .vs file

Steps to reproduce

Open a new project and open a VisualScript

Minimal reproduction project

Temppppp.zip

fire commented 3 years ago

@aaronfranke

aaronfranke commented 3 years ago

Not sure why you pinged me. @mhilbrunner

mhilbrunner commented 3 years ago

Yeah, https://github.com/godotengine/godot/pull/51627 may be the cause of this, will investigate. Thanks for reporting!

mhilbrunner commented 3 years ago

I can confirm this doesn't happen with the commit prior to the above mentioned PR, but does with it, so it seems to be the cause.

mhilbrunner commented 3 years ago

After some debugging, first thoughts:

This happens because both VisualScriptEditor and _VisualScriptEditor (now vs_bind::VisualScriptEditor) are exposed to GDScript.

Now after the above PR, both try to register as VisualScriptEditor, which collides and fails. This isn't an issue for all other underscored classes, as only one was ever exposed (e.g. _OS was exposed, not the wrapped OS).

Both are marked with GDCLASS, but only _VisualScriptEditor is registered via GDREGISTER_CLASS.

VisualScriptEditor isn't registered via GDREGISTER_CLASS, so it only gets registered and exposed at runtime when the first instance is created because of GDCLASS, which eventually calls initialize_class on construction, wich calls ClassDB_add_class.

As this only happens on runtime once a VS is opened, this wasn't caught during build time, CI or manual testing.

It seems a bug or maybe unintentional that both VisualScriptEditor classes are exposed. The latter (the _VisualScriptEditor one) also gets registered as a singleton under the name VisualScriptEditor, confusing things even further. It seems it just allows to add custom nodes.

We could solve this by renaming _VisualScriptEditor/vs_bind::VisualScriptEditor, as having a VisualScriptEditor singleton that just has the following methods:

create_node_custom(name)
add_custom_node(name, category,cript)
remove_custom_node(name, category)

... and doesn't have anything to do with the proper VisualScriptEditor seems a little werid. It doesn't even itself interact with the VS script editor directly.

Maybe we could just rename it VisualScriptCustomNodes, and keep the singleton named VisualScriptEditor. Or should we rename the singleton to VisualScript? IMO VisualScript.add_custom_node() would make sense? Or name the singleton VisualScriptCustomNodes?

cc @fire

Gallilus commented 3 years ago

create_node_custom(name)?

fire commented 3 years ago

@Gallilus

Does VisualScript.add_custom_node() make sense to you?

Gallilus commented 3 years ago

What would VisualScript.add_custom_node() do? Create custom node from Gdscript? Nope no sense to me

mhilbrunner commented 3 years ago

This was fixed in https://github.com/godotengine/godot/pull/51916 by un-exposing the internal editor class.

Please let us know if you still run into this issue with current master. Thanks again for reporting!

Gallilus commented 3 years ago

@mhilbrunner

Now I am able to open the editor. But it is not usable. The only node I'm able to add is using rmb-click

And the menu is all phunky

mhilbrunner commented 3 years ago

Thats... unfun. Thanks for the feedback, will check tomorrow.

mhilbrunner commented 3 years ago

@Gallilus Thanks again for taking the time with this. Would you be willing to build and test it with this PR? https://github.com/godotengine/godot/pull/52023

It hopefully should be a better fix :)

fire commented 3 years ago

Can confirm these operations:

Can spawn new node
Left sidebar showing functions and variables work
Dragging a new now can now open the auto complete.

godotengine / godot