C#: Add [ScriptMethodExclude]. An attribute that excluding method generation from source generators

[kkjinping]

C#: Add [ScriptMethodExclude] .
An attribute that excluding method generation from source generators.

[paulloz]

Hello, and thank you for the contribution 😀
Can you explain a bit more the use case behind this? I understand the finality of it (avoid generating metadata for the annotated methods), but I'm not sure that I understand the necessity (why one would need to avoid this small-ish part of the source generation). I am not fundamentally against the idea of giving more control over the generation process, but I'd like to understand the reasoning. Thanks.

[kkjinping]

Hello, and thank you for the contribution 😀 Can you explain a bit more the use case behind this? I understand the finality of it (avoid generating metadata for the annotated methods), but I'm not sure that I understand the necessity (why one would need to avoid this small-ish part of the source generation). I am not fundamentally against the idea of giving more control over the generation process, but I'd like to understand the reasoning. Thanks.

I apologize for taking so long to get back to you.

Here are my reasons for submitting this PR:

1. When there are too many methods, the generated code has a lot of if and equals, this can cause some performance impact.
2. Private methods are automatically exposed, allowing call, however the developer has no control over this.

[paulloz]

No need to apologize, there's no rush at all.

I don't think impact on performance is in consideration here. The amount of code we'd have to generate for this to actually matter would be massive. I do agree that automatically exposing non-public members to the Call / Set / Get API is something we could do without, though.

Can I ask if you have a concrete example as to what prompted you to open this PR?
Sorry if it feels like I'm pushing against the change (I'm not). But, even if this is a small change, it would introduce a new attribute that we'd eventually need to deprecate, since the long-term goal is to make the bindings entirely opt-in (opposed to the current no control / opt-out model). So I'm trying to assess where the short-term benefits lie against the long-term drawbacks.

[Delsin-Yu]

I actually have a use case for this attribute,
I have worked on a C# Wrapper Generator for GDExtension, and one specific GDExtension, Steam, have produced a single class with 732 methods in it, which generates this MEGA 2200 lines of if and else statement that even bricks the compiler: Error CS8078,

This is what that compiler-breaking method looks like:

......

Compile my included C# wrapper file will sometimes cause the following compiler issue:

Translate to: An expression is too long or complex to compile.

I wish we could have this attribute that I can add for each wrapper method, to at least make it compile (as these methods shouldn't be called by others anyway).

[paulloz]

I think this is actually a good addition to the API. Arguably, I'd make it [GodotIgnore], so it could be used in other generators. One possible use case: I want to implement the access to static properties across interop, and it would probably be good to avoid exposing stuff like GodotSharpEditor.Instance if we can.

cc @raulsntos

[raulsntos]

The .NET team has discussed this PR and agree that this is a good addition, but as suggested by @paulloz we'd like to extend the attribute to work for properties as well.

We also bikeshed a bit about the name:

• We decided the attribute should not have a Godot prefix, to be consistent with the rest of our attributes.
• We considered the word Ignore and Exclude, and settled on Ignore because it matches the [JsonIgnore] attribute.
• We considered adding a Member suffix (i.e.: [IgnoreMember]), because the word Ignore alone may be too vague.

In the end, we think both [Ignore] and [IgnoreMember] are acceptable options.

We also intend to merge this PR for 4.4, so I'm adjusting the milestone accordingly. Keep in mind this is not a guarantee that it will be merged for 4.4, and may still be pushed to a later milestone if we can't finish it in time.

[kkjinping]

The .NET team has discussed this PR and agree that this is a good addition, but as suggested by @paulloz we'd like to extend the attribute to work for properties as well.

We also bikeshed a bit about the name:

• We decided the attribute should not have a Godot prefix, to be consistent with the rest of our attributes.
• We considered the word Ignore and Exclude, and settled on Ignore because it matches the [JsonIgnore] attribute.
• We considered adding a Member suffix (i.e.: [IgnoreMember]), because the word Ignore alone may be too vague.

In the end, we think both [Ignore] and [IgnoreMember] are acceptable options.

We also intend to merge this PR for 4.4, so I'm adjusting the milestone accordingly. Keep in mind this is not a guarantee that it will be merged for 4.4, and may still be pushed to a later milestone if we can't finish it in time.

I rather like[IgnoreMember].