C#: Add `[ScriptMethodExclude]`. An attribute that excluding method generation from source generators
C#: Add [ScriptMethodExclude] .
An attribute that excluding method generation from source generators.
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.
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:
- When there are too many methods, the generated code has a lot of
ifandequals, this can cause some performance impact. - Private methods are automatically exposed, allowing
call, however the developer has no control over this.
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.
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).
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
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
Godotprefix, to be consistent with the rest of our attributes. - We considered the word
IgnoreandExclude, and settled onIgnorebecause it matches the[JsonIgnore]attribute. - We considered adding a
Membersuffix (i.e.:[IgnoreMember]), because the wordIgnorealone 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.