godot icon indicating copy to clipboard operation
godot copied to clipboard

Implement C# interface export

Open poohcom1 opened this issue 1 year ago • 6 comments

Implements https://github.com/godotengine/godot-proposals/issues/8722

Demo

https://github.com/godotengine/godot/assets/74857873/8a85fc80-68aa-4472-892c-e348ee110787

Base class code
public partial class Exporter : Node
{
    [Export]
    public InterfaceA interfaceAExport; // should allow resources and nodes that implement InterfaceA 

    [Export]
    public MyNamespace.InterfaceB interfaceBExport; // should properly handle namespaces

    [Export]
    public ResourceExporter resourceExporterExport; // testing nested resource
}

[GlobalClass]
public partial class ResourceExporter : Resource {
    [Export]
    public InterfaceA interfaceAExport; // should only allow resource exports when base type is a resource
}

Test Repository

https://github.com/poohcom1/godot-interface-export-demo

(see poohcom1/godot release for built Godot binary and nuget packages)

Changes overview

  • Add the ability to export interfaces in C#, allowing users to select resources (and nodes if the edited object is a node) that implement the specified interfaces.
  • Add PROPERTY_HINT_INTERFACE as a hint for interfaces.
  • Updated base script class, C# source generators, and property editor to facilitate interfaces.
  • If GDScript gets a trait system (https://github.com/godotengine/godot-proposals/issues/6416), this should also be easily integrated with GDScript.
Technical Details

The concept of interfaces doesn't exist in Godot core engine currently, so the PR will work on the following assumptions:

  • Interfaces are a script-only concept
    • Scripts will inform the editor of the interfaces they implement and the interfaces of exported property; the editor will only need to handle the hint string
  • Exported interfaces are either Resources or Nodes
  • Interfaces can only be implemented by scripts of the same language
    • Cross-language interfaces will only work with structurally typed languages like Typescript (and possibly GDScript depending on how traits are implemented). With GDScript and C# being Godot's main language, the likelihood of this being useful is extremely low.

Here is the breakdown all the changes:

Core

  • Added virtual function Script::get_interfaces with default implementation
    • Implemented by CSharpScript
  • Added PROPERTY_HINT_INTERFACE
    • The format of the hint string is language_name,interface1,interface2.
    • The first element is always the language name to prevent interface name collision between languages (say if a third-party Java binding was created and happens to have the same namespaces as C#). Although cross-language interface name collisions are unlikely, I don't want any edge cases here.
    • Interface name string depends on the language. For C#, it's the fully qualified name of the interface. For GDScript, it could be the trait resource file. What matters is that it matches what is returned from Script::get_interfaces.
    • This also assumes that there won't be cross-language interface implementation.
    • I would've preferred a format like language_name:interface1,interface2 to make the element's purpose more explicit, but I've gone with a comma-separated list to avoid conflict with PROPERTY_HINT_TYPE_STRING which also uses colon in its hint string. All of this is internal so we can easily change this without compatability issues.

Mono

  • Added new internal attribute ScriptInterfaceAttribute
  • Added new source generator file: *_ScriptInterface.generated
  • Updated property source generator and marshal utilities to correctly generator properties that are interface and add PROPERTY_HINT_INTERFACE and the correct hint strings. Most of the marshaling changes are detecting interfaces and casting the object to a GodotObject, as interfaces will either be Resources or Nodes.

Editor

Interface exports require the ability to export either Resources or Nodes. To minimize the changes required, I modified the resource property EditorPropertyResource and created EditorInterfacePicker (extends EditorResourcePicker) which just adds the ability to select nodes as well. The resulting change in the resource picker is a bit hacky since it needs to coordinate either the base edited_resoruce or the new edited_node, but this is better than recreating an entirely new property interface and resource picker which is just a combination of EditorPropertyResource and EditorPropertyNodePath.

  • Updated EditorInspectorDefaultPlugin to use EditorPropertyResource for PROPERTY_HINT_INTERFACE`.
  • Updated the following components to have a interface_hint_string field and apply them to their resource picker accordingly:
    • EditorPropertyResource
    • SceneTreeEditor
    • EditorQuickOpen
  • Added EditorData::script_object_implements_interface and EditorData::script_class_implements_interface to facilitate checking interface implementations. Since the hint string is comma-separated and contains both language names and interface names, I've decided for these functions to accept the entire hint string rather than individual interface names to minimize errors from processing beforehand. This is the reason the editor components just hold an interface_hint_string field rather than a list of interface names.

Follow-up implementations

  • [Feature] Interface array support: Right now the source generator will choke if you attempt to export an array of C# interfaces. I've already started implementing this, but since it deals a lot with C# marshaling and type conversion, I'm concerned about its performance. Since this PR is already large enough, I'll create a separate PR for that so reviewing is easier.
  • [Feature] Node drag and drop: The interface resource picker supports drag and drop for resources as expected, but I haven't implemented drag and drop for nodes to have feature parity with node path exports. This requires exposing a few more functions from the base resource picker, so I hesitate to complicate this PR further. Likewise, I'll create a separate PR for this if there's demand.
  • [Bug] C# resource sub-type cannot be assigned to parent type: A major part of this PR is to solve the multiple inheritance issue, allowing exported properties to be restricted by an interface but still extend different parent classes. During testing, I found a bug that only occurs on C# resources: a Resource subclass is not considered a Resource class. For example, if I have a C# custom resource that extends Texture2D, I cannot assign it to a property that is exported as a Resource type. This makes it so that interface exports won't recognize C# classes that don't directly extend Resource, which hinders the point of interfaces. I haven't thoroughly tested this, but I'll create an issue for it if needed.

poohcom1 avatar Jan 08 '24 02:01 poohcom1

PR looking good :)

Random rambling related to cross language interface support that came to my mind. This uses languageId,inLanguageInterfaceId as the hint to serialize the object, good! If we ever want to add cross language interfaces, this is most likely compatible.

PereViader avatar Jan 09 '24 17:01 PereViader

PR #87890 (4.3-dev4) changed way of detecting script object on nodes, after merging this PR with master, only interfaces in resources detection seems to work properly

poglad-ds avatar Mar 23 '24 12:03 poglad-ds

PR #87890 (4.3-dev4) changed way of detecting script object on nodes, after merging this PR with master, only interfaces in resources detection seems to work properly

I've rebased from master to fix this, and it seems to be working fine now. However, I've noticed frequent crashing on launch, and I'm not sure if it's related to my changes or because of https://github.com/godotengine/godot/issues/89610. I'll wait until this is fixed to test again.

poohcom1 avatar Mar 24 '24 05:03 poohcom1

I am bumping this issue as I would really love to see this feature added. Is there any help I could provide for this feature to be merged faster?

jokil123 avatar Apr 29 '24 10:04 jokil123

This is a game changer. GDScript also needs composition/interfaces ASAP!

Servail avatar May 17 '24 20:05 Servail

@jokil123 Godot 4.3 has entered feature freeze, so at the earliest this PR could be considered for 4.4.

I'll be maintaining a fork of Godot with this PR (+follow up changes) for use with my current project, you can help out by testing it and reporting bugs/issues.

poohcom1 avatar May 17 '24 21:05 poohcom1