Add support for registering global script classes (class_name) using C#

[seadra]

Godot version:

3.1rc1

OS/device including version:

Linux

Issue description:

Class name and icon features don't seem to be working with C#/Mono.

Steps to reproduce:
I tried adding functions GetClassName() and GetScriptClassIconPath() to a C# script but didn't work (is this how it's supposed to work?).

Minimal reproduction project:

[neikeq]

Where do GetClassName() and GetScriptClassIconPath() come from?

[seadra]

Just functions I added to a script, again, I don't know if this is the right way so let me rephrase it.

Is there a way to use the class_name functionality of GDScript in C#?

[neikeq]

No, I will add that after 3.1 (it's too late to risk introducing regressions). I still don't understand were those methods come from though. I can't find any get_class_name equivalent for GDScript.

[akien-mga]

I still don't understand were those methods come from though. I can't find any get_class_name equivalent for GDScript.

They were just trying a shot in the dark I think, to see if by chance those would have been the virtual methods used to expose this feature.

In GDScript, the syntax is:

extends Node
class_name MyNode
# or, with icon:
class_name MyNode, "res://path/to/icon.png"

[shartte]

It's implemented in gdscript_parser.cpp. Look for TK_PR_CLASS_NAME.
I have to dig deeper to understand how a GDScript "global class" translates to C#, where classes are global anyway due to there being only one project assembly.
The icon is probably for the editor, and I suppose having that would be nice :)

[seadra]

@shartte

C# classes don't appear as a node type in add node dialogue, which is the main use case of class_name keyword.

[shartte]

Ah that explains that then. Lots of different ways of solving this though.
I.e. one could either have all C# classes that somehow inherit from Node show up, and simply use their name as the global class name. Or, one could use a [GlobalClass("thename", "res://the_icon")] attribute on the class to mark it as a global class, which seems more sensible.

[neikeq]

It would be [GlobalClass("res://icon_path")]. The class name is redundant as you can take it from the type.

[shartte]

Only if you use fully qualified names. Is the class name supposed to be used only for display purposes or is that actually stored somewhere?

[neikeq]

I think the class name is for display only. The script is actually referenced with the path, since that's how Godot does scripts.
Now that I think of it, this will cause problems if you want to use this on classes that cannot be referenced as a file. But that's more the subject of #15661.

[akien-mga]

I think the class name is for display only. The script is actually referenced with the path, since that's how Godot does scripts.

The class name is also registered as a global class. You can do node is MyNode or var node = MyNode.new().

[shartte]

Hm so that means you won't likely be able to use the fully qualified class name since periods in the name would break it?
I'd vote for at least allowing devs to set the desired name in the attribute then... (and fallback to C# class name if unspecified).

[neikeq]

The class name is also registered as a global class. You can do node is MyNode or var node = MyNode.new().

I'm pretty sure the MyNode.new() syntax translates to something similar to preload(script_path).new() internally.

Also the is operator in GDScript cannot be used with C# scripts.

[aaronfranke]

Since Godot already has another way to create custom node types which works with any language, and since class_name is only a GDScript feature, perhaps we should consider deprecating class_name in favor of creating custom node types through addons, or perhaps make it have different functionality...? I don't really see why we need two ways to create custom node types, especially if one only works with GDScript. It's fine I guess.

You can do node is MyNode

See: #16863 (comment). For the different functionality idea, we could have it be classified as a different type when compared using get_class.

Another idea, it would be useful if we could create custom structs in GDScript, we could use class_name and not extend Node, and refer to them like data types. I ran into some problems here and here (the example I gave is general, what I was actually trying to make is Basis25D).

[seadra]

I don't think those are the same. AFAIK, class_name can be used with ordinary scripts which are part of a plugin. class_name is very useful because not every custom type needs to be a plugin, so please don't deprecate it.

[willnationsdev]

@neikeq

The class name is also registered as a global class. You can do node is MyNode or var node = MyNode.new().
I'm pretty sure the MyNode.new() syntax translates to something similar to preload(script_path).new() internally.

Iirc, the GDScriptLanguage class has a Vector that it uses to store a collection of global variables for the language and a string->int map (can't recall if HashMap or Map) that registers the script class names to the corresponding indexes. During its initialization, it runs through the list of types defined in the ScriptServer, loads all of the file paths, and adds them all as global variables.

So, if you had a FQN stored as the script class name in the ScriptServer, it would likely just end up creating a global variable that couldn't be accessed (since no valid GDScript identifier would match the string key in the map that provides the corresponding global variable vector index). The CSharpScripts would get turned into inaccessible globals.

Also the is operator in GDScript cannot be used with C# scripts.

That sounds more like the is keyword implementation needs to be updated to support arbitrary scripted types.

@aaronfranke

Since Godot already has another way to create custom node types which works with any language, and since class_name is only a GDScript feature,
It's fine I guess.

Actually, the main difference is that script class names can be accessed at runtime while custom types can only be accessed from the editor. Furthermore, in order to define a custom type, you must register it via an EditorPlugin whereas script classes are defined directly from the script's file.

The EditorFileSystem that scans the project every time you save in the ScriptEditor will also attempt to update script class names. It iterates over all script files, figures out which ScriptLanguage instance to grab, and then calls get_global_class_name on the script using the ScriptLanguage. Its implementation then returns the name of the script file (and any potential icon) which it then caches in Dictionaries inside the project.godot file. This file is then used to initialize the ScriptServer's global ClassName->ScriptPath map which is accessible to scripting languages at runtime.

Rather than deprecating the highly specific script classes in favor of the generic custom types, the better approach would be to add support for script classes to all scripting languages and then completely deprecate the custom type system since it doesn't support as wide of a feature set. GDScript and NativeScript already have support (although NativeScript doesn't generate any global variables or anything).

@seadra

The only reason I didn't add support to CSharpScript initially was because you needed the get_base_script() method to return the proper inherited script in order for the CreateDialog to understand their inheritance hierarchies correctly (at the time, don't think that's changed either), and when I asked about why it hadn't been implemented. no one even knew what it was for. And I couldn't seem to figure out how to make sense of the CSharpScript implementation or what the MonoObject relationship was to CSharpScript, Script, etc.

I haven't added support to VisualScript cause, at the time, I would've needed to define a GUI and I was running out of time since the 3.1 release was so close. And I just haven't come back to the problem yet. All you have to do to add support is grant some way for a Script to define a class name and/or icon, and then have the ScriptLanguage implementation return that data when get_global_class_name is called with the script's path. And then, if you want, you can create some means of making other language's scripts accessible by name. The ScriptServer only has the one namespace though. reduz hasn't wanted to change that (I already asked a long time ago).

[aaronfranke]

While I would love to see this done as part of godotengine/godot-proposals#22, there's no way that this is going to be done for 3.2. I suggest bumping to the 4.0 milestone.

[willnationsdev]

@aaronfranke Yeah, this won't be done till 4.0 at the earliest. In fact, I already have a partial implementation done, but I am having trouble getting the ScriptClass attribute detected by the CSharpScript. Waiting to hear back from @neikeq for assistance on figuring out why my code isn't working.

[PauloVBettio]

Does the milestone still checks out or this will be postponed for 4.1 or other future version?