Registering Global Script Classes under Mono

[kalysti]

Registering Editor Plugins, Custom Resources and CustomNodes to _global_script_classes like in all other Scripting Languages (GD, Native..).

This PR is neccesary to create as example Custom Nodes from type "VisualShaderNodeCustom" to use them inside the Visual Editor under Mono. The reason is because the Visual Shadar Editor is doing a lookup for custom nodes by the attribute "to _global_script_classes" of the "project.godot" file. This pr is also usefull for godot 3.

Example:

C# Script for VisualShaderNodeCustom

Visual Shader Node List

VisualShaderNodeCustom attached to the Visual Editor

Project file with Extensions written in C#

This PR is not related to any other

Btw this issue fix also a problem with custom resources which binded under mono to an export array.

[kalysti]

@aaronfranke Its not a enhancement, i blv its a bugfix?

[aaronfranke]

@sboron No, it's a feature addition.

[kalysti]

@aaronfranke @reduz should the possibility of creating extensions for "VisualScriptNodes" or "VisualShaderNodes" be removed in godot 4 or taken out of the core? I haven't read anything about it. To repair an existing module with the aim that it works properly in all script languages offered, I think it is not a feature but a bugfix or do I not understand something right?

[akien-mga]

There's no point bikeshedding over our issue triage. The label is for documentation purposes and doesn't change anything to how this PR will be handled.

The fact that Mono doesn't support global classes yet is a missing feature. You may consider that this missing feature is a bug, but then all missing features can be seen as bugs and there's no point making a distinction.

Here you're implementing a missing feature (global classes support for Mono) which happens to also solve a bug (VisualShaderNodes rely on global classes to be extended and therefore they can't be extended from C#). That bug could also be solved differently (e.g. by allowing extending VisualShaderNodes with another system than global classes, e.g. through the EditorPlugin API). The feature and that bug are not the same thing.

So here you go, have your bug label. But please don't ping a project leader to contest the decision of a core contributor, that's rude.

[kalysti]

Sorry for the communication problem. Nobody was rude here. I asked if the project manager could provide more information to define the PR more precisely and to speed up the process. Standard procedure. It could be that a change is unwanted or unnecessary. But I got it. Setting the respective label has no relevance or influence on the review process.

[akien-mga]

Besides, I ask myself why someone else answers for the project manager and my question remains open.

I am the project manager...

As for your question, is that "should the possibility of creating extensions for "VisualScriptNodes" or "VisualShaderNodes" be removed in godot 4 or taken out of the core?" ?

I don't see how that relates to the labels being assigned to this issue.

[kalysti]

Again. there were two different questions. is my "bugfix / feature / enhancement"
absolutely necessary. maybe there are other plans e.g. the module is to be removed or completely rewritten in the future. Because there are other problems that occur under mono and visual nodes.

And the second question was which label should get it (but now i know that this is not important for the review process ).

[akien-mga]

there were two different questions. is my "bugfix / feature / enhancement"
absolutely necessary. maybe there are other plans e.g. the module is to be removed or completely rewritten in the future. Because there are other problems that occur under mono and visual nodes.

I believe that change is necessary/wanted, yes, IIUC it implements and solves #26875.

As you can see there, and infer from your own changes, this is a general change that implements a missing feature in Mono/C# for parity with GDScript. Since there's no aim to remove support for global classes in the engine, having it implemented for C# is good.

It incidentally fixes an issue with writing custom nodes for VisualScript and VisualShader using C#. There might be other ways to solve it (like changing VisualScript and VisualShader to not rely only on global classes but also have a dedicated API to register new nodes). But if solving #26875 is a sufficient way to address the issue, while also enabling all C# users from benefiting from global classes (which is a feature with a much broader scope than just creating VS nodes), further changes to VS extensibility might not be needed.

[kalysti]

Besides, I ask myself why someone else answers for the project manager and my question remains open.

I am the project manager...

As for your question, is that "should the possibility of creating extensions for "VisualScriptNodes" or "VisualShaderNodes" be removed in godot 4 or taken out of the core?" ?

I don't see how that relates to the labels being assigned to this issue.

@akien-mga okai interesting. then i will fix the other known problems with visual shader nodes as well. I'll put it on my todo list. Thanks for the quick response and sorry for a that missunderstandniss on our communication above.

[willnationsdev]

Hmm. I have similar changes, and more, in #44879. The only reason I wrote it all for 3.2 rather than 4.0 was because I was told that the script class system was going to be rewritten for 4.0 and that I shouldn't bother with implementing the features until the GDNative 2.0 refactoring was done. At which point, I was just going to rebase my feature branch onto 4.0 and re-submit under a different PR. So, certainly this PR is subject to the same GDNative 2.0 waiting list, right?

Also, I recall running into problems with recompiling C# scripts causing problems in global class recognition. Also has similar problems when closing and re-opening the editor. Technically, until the scripts are recompiled, the EditorFileSystem will read stale data, even if the content itself is changed, and that can have an effect on the tooling feedback. I had to add new stuff to the ScriptLanguage interface to specifically distinguish compiled languages (that compile to a separate binary) from those which depend only on the file alone and then do selective, delayed re-scanning of type information for files associated with those languages.

You could just parse through all the C# / ScriptLanguage / EditorFileSystem code from my PR and try to mimic it in this PR to account for those issues, but I'm not sure why we wouldn't just refactor all of #44879 while we're at it in place of merging this one if we're going to go that far.

akien-mga do we know what the status is with the whole GDNative 2.0 situation?

[aaronfranke]

@willnationsdev: So, certainly this PR is subject to the same GDNative 2.0 waiting list, right?

I don't see why. This PR only affects Mono, not GDNative. We can't merge your ultimate PR that implements global script classes everywhere at once, but we can still merge a PR that just implements it in C#, so either this can be reviewed or your PR can be rebased (and likely reworked to just add it to C# and maybe VisualScript and not GDNative for now).

[willnationsdev]

If there isn't actually any blocker, then I'd prefer to rebase my stuff w/o GDScript/GDNative changes, since it's more battle-tested thus far, and just make it compatible with Godot 4. That should simplify the review process for that PR too. And then GDScript and GDNative can be updated later.

[kalysti]

Unfortunately, I don't understand the discussion. My PR has absolutely nothing to do with the PR mentioned here. Extensions in Mono, like in GDScript and other languages, must be registered in GlobalScriptClasses. Regardless of whether the PR solves the above-mentioned problem with VisualShaderNodes. Because it solves a lot more than just the problem described above. Whether the module for VisualShaderNodes (I don't see any indication that this was changed with the PR mentioned here) was then rewritten does not matter here.

[willnationsdev]

@sboron Your PR adds global class name support for the C# programming language. The mentioned PR does as well, here. It also accounts for the staleness of script changes vs. compiled assemblies in the EditorFileSystem's file cache for the global class data by allowing languages to declare that they are compiled, registering C# as a compiled language, and re-scanning compiled languages' class names on load.

[kalysti]

@sboron Your PR adds global class name support for the C# programming language. The mentioned PR does as well, here. It also accounts for the staleness of script changes vs. compiled assemblies in the EditorFileSystem's file cache for the global class data by allowing languages to declare that they are compiled, registering C# as a compiled language, and re-scanning compiled languages' class names on load.

Okai mby i was checking it to fast.

I just saw there are two commits. My mistake. The changes look good, and make more sense in the long run. Good job.

[kalysti]

After checking the mentioned pr here, i blv it will be a better solution to go forward with my pr. The mentioned pr looks well, but needs a lot of changes to run under godot 4 and it contains also a part of my fix. Currently the most of my test and dev tools (also for physics and shader stuff) are using exported variables and global script classes which currently not full supported under mono (here is another issue #48197). To go forward we can use this pr, which is solving a two issues at the same time. I can reimplement the mentioned pr to godot 4 step by step in this pr.

[willnationsdev]

I'm already about 80% done with my 4.0 port, including adding the ScriptServer, handling compiled language script classes, and adding VisualScript and C# script classes. All that's left is to apply the changes to the editor code and then I can push the new PR.

[kalysti]

I'm already about 80% done with my 4.0 port, including adding the ScriptServer, handling compiled language script classes, and adding VisualScript and C# script classes. All that's left is to apply the changes to the editor code and then I can push the new PR.

Okai thats good to know. You were diligent 👍🏻

But I still think that this PR does not stand in the way of that. A rollback is possible at any time, and it does not damage the project. Take your time, you work on difficult things on different modules and also required a lot of tests.

[YuriSizov]

This will be superseded by #72619 in 4.1. Thanks for your contribution nonetheless!