Improve handling of generic C# types

[raulsntos]

trafficstars

Improve handling of generic C# types

• Create CSharpScript for generic C# types.
  • ScriptPathAttributeGenerator registers the path for the generic type definition.
  • ScriptManagerBridge lookup uses the generic type definition that was registered by the generator.
  • Constructed generic types use a virtual csharp:// path so they can be registered in the map and loaded as if there was a different file for each constructed type, even though they all share the same real path.
    • Thanks so much to @Delsin-Yu for the idea.
  • This allows getting the base type for a C# type that derives from a generic type.
• Shows base scripts in the Add Node and Create Resource dialogs even when they are generic types.
  • get_global_class_name implementation was moved to C# and now always returns the base type even if the script is not a global class (this behavior matches GDScript).
• Create CSharpScript::TypeInfo struct to hold all the type information about the C# type that corresponds to the CSharpScript, and use it as the parameter in UpdateScriptClassInfo to avoid adding more parameters.
  • The fields of the struct, as well as other CSharpScript fields, are also now documented :slightly_smiling_face:.
• Fixes https://github.com/godotengine/godot/issues/87877

There's also a little bit of background on these previously merged PRs:

• https://github.com/godotengine/godot/pull/55597.
• https://github.com/godotengine/godot/pull/70511.

[zaevi]

I met the exception when selecting a node with generic type inherited:

  C:/Users/zaevi/source/Godot/godot-source/modules/mono/glue/GodotSharp/GodotSharp/Core/NativeInterop/ExceptionUtils.cs:113 - System.IO.IOException: 文件名、目录名或卷标语法不正确。 : 'C:\Users\zaevi\source\Godot\temp\crash_test\csharp:\generic\Base.cs:Base`1[System.Int32].cs'
     at System.IO.FileSystem.GetAttributeData(String fullPath, Boolean returnErrorOnNotFound)
     at System.IO.File.GetLastWriteTime(String path)
     at GodotTools.Utils.File.GetLastWriteTime(String path) in C:\Users\zaevi\source\Godot\godot-source\modules\mono\editor\GodotTools\GodotTools\Utils\File.cs:line 25
     at GodotTools.Inspector.InspectorPlugin._ParseBegin(GodotObject godotObject) in C:\Users\zaevi\source\Godot\godot-source\modules\mono\editor\GodotTools\GodotTools\Inspector\InspectorPlugin.cs:line 27
     at Godot.EditorInspectorPlugin.InvokeGodotClassMethod(godot_string_name& method, NativeVariantPtrArgs args, godot_variant& ret) in C:\Users\zaevi\source\Godot\godot-source\modules\mono\glue\GodotSharp\GodotSharpEditor\Generated\GodotObjects\EditorInspectorPlugin.cs:line 156
     at GodotTools.Inspector.InspectorPlugin.InvokeGodotClassMethod(godot_string_name& method, NativeVariantPtrArgs args, godot_variant& ret) in C:\Users\zaevi\source\Godot\godot-source\modules\mono\editor\GodotTools\GodotTools\Godot.SourceGenerators\Godot.SourceGenerators.ScriptMethodsGenerator\GodotTools.Inspector.InspectorPlugin_ScriptMethods.generated.cs:line 50
     at Godot.Bridge.CSharpInstanceBridge.Call(IntPtr godotObjectGCHandle, godot_string_name* method, godot_variant** args, Int32 argCount, godot_variant_call_error* refCallError, godot_variant* ret) in C:\Users\zaevi\source\Godot\godot-source\modules\mono\glue\GodotSharp\GodotSharp\Core\Bridge\CSharpInstanceBridge.cs:line 24

Seems it tries to check LastWriteTime using that virtual path.

[raulsntos]

@zaevi Interesting, it doesn't throw in Linux so I didn't notice. Should be fixed now.

[paulloz]

I'm having weird behaviours with export updates + the error I'm talking about above. See video attached.

https://github.com/godotengine/godot/assets/437025/4f89f9e6-e957-49f5-91f4-c8ae29de5a35

[zaevi]

I'm having weird behaviours with export updates + the error I'm talking about above. See video attached.

godot.windows.editor.dev.x86_64.mono_6ur3hBmru1.mp4

Could be caused by the cache. It's ok after I delete the .godot folder (and rebuild), but updating the exported property and reloading the project throws the exception again.

[raulsntos]

@paulloz Thanks, I can reproduce following the steps in the video. I'm having the same behavior in 4.2.1-stable. The inspector plugin does not seem to be called at all when modifying generic scripts, but it doesn't seem to be introduced by this PR.

Oh I see, for generic scripts like res://Foo.cs we end up creating an empty CSharpScript so it's always valid = false, and since it's invalid it never refreshes the inspector:

https://github.com/godotengine/godot/blob/d3352813ea44447bfbf135efdec23acc4d1d3f89/modules/mono/csharp_script.cpp#L2223

We would not be able to update the exports anyway, since it's a generic type definition and we can't call C# methods on an unbound generic. Not sure how we could fix this so I guess I'll leave it for the future since it's a pre-existing issue.

I'm also able to reproduce the other error you mentioned, I don't know why I didn't encounter it before.

[paulloz]

The inspector plugin does not seem to be called at all when modifying generic scripts, but it doesn't seem to be introduced by this PR.

You are right about the inspector plugin, but the exports themselves should appear after a build. They currently do on master:

https://github.com/godotengine/godot/assets/437025/0de8d69e-a48a-4276-a006-6b6291794b32

We would not be able to update the exports anyway, since it's a generic type definition and we can't call C# methods on an unbound generic.

But at that point the generic type is fully bound, since it's the once in the class hierarchy of a valid node.

[raulsntos]

You are right about the inspector plugin, but the exports themselves should appear after a build. They currently do on master

Oh, you are right. I feel like I tested this and it wasn't working. Thank you for testing.

But at that point the generic type is fully bound, since it's the one in the class hierarchy of a valid node.

I mean the empty CSharpScript with the path res://Foo.cs, not the CSharpScript with the virtual path csharp://Foo.cs:Foo`1[System.String].cs that is part of the Bar hierarchy.

I have checked and it seems _update_exports is never called on Foo<string> because AddScriptBridge returns false since it can't find the virtual path in the _pathTypeBiMap at this point of the reloading. This is probably because of the reload order, Foo<string> comes first since it's the base script but the virtual path won't be added until we construct it when reloading Bar which comes afterwards. I'm starting to think the complexity of these virtual paths may be too high.

[Delsin-Yu]

Back to the reloading order again... Is it possible to somehow refractor the reloading logic to ensure this editor reserialization call comes after a complete iteration of to_reloads? This will remove the need to bother with script reloading order any more.

[raulsntos]

Is it possible to somehow refactor the reloading logic to ensure this editor reserialization call comes after a complete iteration of to_reloads?

That sounds like it require drastic changes to the reload logic which risk introducing bugs, I'm not sure it's worth doing such a big change since we'll be moving away from scripts in the future.

---

the exports themselves should appear after a build. They currently do on master

I was curious why it was working in master since we also don't have a path to the script there and found out that scripts without paths actually go through a different path when reloading assemblies.

So I made scripts with csharp:// paths be reloaded in the same way (as if they had no path) and it seems the exports update now.

Edit: Also rebased to fix the conflict with InspectorPlugin.

[akien-mga]

Thanks!

[EduardoLemos567]

That sounds like it require drastic changes to the reload logic which risk introducing bugs, I'm not sure it's worth doing such a big change since we'll be moving away from scripts in the future.

@raulsntos

Could you, please, elaborate a bit on "we'll be moving away from scripts in the future" ? :) I'm very interested on the future of C# in Godot.

Also if there's any roadmap for C# features in the future, could you or anyone point me the link(s) ?

Thank you.