godot icon indicating copy to clipboard operation
godot copied to clipboard

C#: Document generated members

Open raulsntos opened this issue 2 years ago • 1 comments

Documents generated members and tries to discourage users from calling/overriding internal methods that only exist to be used by the engine.

  • Fixes https://github.com/godotengine/godot/issues/65996.

raulsntos avatar Jul 09 '23 12:07 raulsntos

net8 adds an Experimental attribute for things like this: dotnet/designs@main/accepted/2023/preview-apis/preview-apis.md which we could use for these methods once that is released

I don't feel like the Experimental attribute fits these APIs, since they are not preview features. I plan to use this attribute for Godot API that we mark is_experimental such as Navigation APIs.

Or alternatively we could mark these things Obsolete to generate a warning right now when using them.

Some of these methods are used by source generators so this would generate warnings for users that don't use them directly. Also, it would generate warnings for us in the GodotSharp project.

Should even properly document them in xml docs then or should we just have the warning not to use them and whatever is is necessary to suppress the warnings?

I think documenting them is useful, because it can be helpful for contributors. I don't think it's that important for users not to call these methods, just letting them know that they are not mean to and hiding them so they don't accidentally stumble upon them is enough.

raulsntos avatar Jul 10 '23 08:07 raulsntos

With the Obsolete / Experimental I was mostly thinking about future changes´to these methods, eg we explicitly and clearly signal that we may end up changing these methods if future improvements require it. (Unless this is interpretations is completely wrong and they are supposed to be proper members of the public interface, and if it was possible to make them inaccessible they would still be public)

RedworkDE avatar Jul 10 '23 19:07 RedworkDE

My concern is that since some of these methods are used by source generators, those warnings would show up in user projects even if they are not using them directly.

Also, technically we should avoid breaking compat in these methods too, because an external library built against Godot 4.0 could be using them. For example, if a library implements a Node type that the user derives from in their game project, then I think the generated InvokeGodotClassMethod method will throw a MissingMethodException when calling the base method implementation.

And I see what you mean now by using Obsolete / Experimental to mark that these methods may change in the future, but I still feel it's weird to mark the API as obsolete or experimental for this. It's just a matter of semantics for me.

raulsntos avatar Jul 11 '23 10:07 raulsntos

Thanks!

YuriSizov avatar Jul 25 '23 19:07 YuriSizov