C# script does not list non-exported properties unlike GDScript

[Ryan-000]

Tested versions

v4.1.3.stable.mono, All 4.2.2 release candidates, 4.2.2.stable.mono

System information

Godot v4.2.2.stable.mono - Desktop

Issue description

Non exported C# properties do not show up in GDScript autocomplete.
They can be set and get, just fine though.

This is a problem for a few reasons:

1. It's tedious to have to add export to every member I want to be added to autocomplete.
2. I often have to use @warning_ignore("unsafe_property_access") on non-exported properties (for no good reason)
3. When I have C# scripts that extend RefCounted, I cannot even put [Export] on Node properties for autocomplete due to "Error GD0107 : Types not derived from Node should not export Node properties"

Which is why non C# exported properties should be included in GDScript autocomplete.

Steps to reproduce

Have a C# script like this

using Godot;

[GlobalClass]
public partial class ACSharpScript : RefCounted
{
	[Export] public int SomeExportedProperty { get; set; }
	public int SomeProperty { get; set; }

	[Export] public int someExportedField;
	public int someField;

	// "Error GD0107 : Types not derived from Node should not export Node properties"
	// [Export] public Node SomeExportedNode { get; set; }
	public Node SomeNode { get; set; }
}

Only exported properties (and fields) show up in autocomplete

Minimal reproduction project (MRP)

MRP.zip

[dalexeev]

I think this is a C# issue, GDScript does not check if the property is exported (PROPERTY_USAGE_DEFAULT). I haven't checked, but most likely C# does not add non-exported properties to get_property_list() and/or get_script_property_list(), unlike GDScript, which adds all variables. I think the inconsistency allows us to classify this as a bug. CC @raulsntos @paulloz

[raulsntos]

Yes, C# only adds exported properties to get_script_property_list(). We do collect non-exported properties too, so as you've noticed they can still be get / set, they're just hidden.

My only concern with changing this is that it will expose everything, even properties that the user didn't intend to expose, since Godot doesn't have a concept of private.

[dalexeev]

GDScript already collects all non-static variables, and this works well. For C#, I guess we should collect all public properties except the internal ones, if such a mark exists. I think if a property exists for set()/get() then get_property_list() should include it.

[paulloz]

I think if we have to change something, we should at least should be consistent with how this works for methods. And for methods, we don't care about accessibility. It would be weirdly inconsistent to expose private string GetMyString() { ... } but not private string MyString { get; } IMHO.

But I'm not a huge fan of the idea (neither am I that we expose private methods to begin with).

[Ryan-000]

In my opinion, private methods and members shouldn't be exposed by default, because it makes it harder to tell if a member is used due to the source generators.

public partial class SomeScript : RefCounted {
	string someUnusedField;

	void SomeUnusedPrivateMethod()
	{
            // Method intentionally left empty.
        }
}

Without the source generators, I get the proper warnings, but I have to remove the partial keyword every time I want to check.

[paulloz]

In my opinion, private methods and members shouldn't be exposed by default

This is not something we can change back easily now, because it'd break compatibility for anyone currently using members exposed that way.

[preslavnpetrov]

Hey, this is something I ran into and brought up in the contributor chat, glad to see there's an issue open for discussion here. I do agree that private methods shouldn't have been exposed in the first place but we're sadly past the point where we could change that. In my opinion we can remedy this by adding cproj defines that the source generator will check for how to expose properties and methods. Example:

<GodotExposeProperties>All/Public/Exported</GodotExposeProperties> where Exported work like it does right now, Public exposes all of the exported ones plus all of the public ones, and All exports everything Variant compatible.
<GodotExposeMethods>All/Public</GodotExposeMethods> same as above but no Export option as that doesn't make sense for methods.

We then add these to the current SDK as defaults that will keep the behaviour the same as it is currently to not break compatibility, but people will be able to opt into the more sane method of exporting public.

This would require CSharp scripts to have a third member variable with the exposed properties on the engine code side, as neither script->exported_members_cache nor script->member_info should change to make sure the get/set behaviour works as it does currently.

[Ryan-000]

I don't think adding all public C# properties to GDScript autocomplete would break compatibility (correct me if I'm wrong), as they are already readable and writeable because they are exposed by the source generators. In my opinion, making them appear in the GDScript autocomplete by default would be a good solution, but we could also add @preslavnpetrov's soultion with "GodotExposeProperties" for those who don't want it (or the other way around if it breaks compatibility).

It also would be nice to have a way to "export" properties specifically for autocomplete without affecting their editor or storage behavior, essentially using PROPERTY_USAGE_NONE. I know GDScript recently got @export_custom (#72912) which allows for this, but it doesn't look like C# has an equivalent.

Would like to hear thoughts on this.

[paulloz]

I'm personally not a fan of the idea of introducing that kind of GodotExposeProperties. If we want to give more control about what is exposed, I would be more inclined to something akin to what is discussed in #90687.

Basically I tend to agree with the following. And having a specific thing to allow opting-out would alleviate my gripes with the fact we over-expose.

I think if a property exists for set()/get() then get_property_list() should include it.

---

About PROPERTY_USAGE_NONE, it would probably be better to have that in its own issue.

[preslavnpetrov]

Yeah I mean either works. I'm a fan of opt-in more than opt-out and setting it as a general rule rather than specific but that's just code preference. I'd be happy in either case, as long as we can start having more direct, manual control over what is exposed and what isn't.

An [Ignore] or [GodotIgnore] attribute would be good, and the name is generic enough that we can apply it to both properties and methods.

On the topic of properties, I would prefer that we only expose these, even at the cost of some inconsistency with methods:

• Variant compatible, [Export]ed properties of any encapsulation (private, internal, protected and public)
• Variant compatible, public properties

Private properties feel like they're gonna be too much clutter to me, and slapping an [Ignore] on all of them would be a bit ugly.