Ignoring class methods with underscore prefix cause issue with some properties

[touilleMan]

Godot version

4.0-rc13

System information

n/a

Issue description

Currently the extension_api.json generation code consider in class methods/properties starting with an underscore as hidden and ignore it:

godot/core/extension/extension_api_dump.cpp

Lines 748 to 749 in eda6800

	} else if (F.name.begins_with("_")) {
	//hidden method, ignore

godot/core/extension/extension_api_dump.cpp

Lines 846 to 848 in eda6800

	if (F.name.begins_with("_")) {
	continue; //hidden property
	}

However _ as a prefix is not always used to indicate a private method, for instance considering the anchor_* properties in Control:

godot/scene/gui/control.cpp

Lines 3355 to 3358 in eda6800

	ADD_PROPERTYI(PropertyInfo(Variant::FLOAT, "anchor_left", PROPERTY_HINT_RANGE, "0,1,0.001,or_lesser,or_greater"), "_set_anchor", "get_anchor", SIDE_LEFT);
	ADD_PROPERTYI(PropertyInfo(Variant::FLOAT, "anchor_top", PROPERTY_HINT_RANGE, "0,1,0.001,or_lesser,or_greater"), "_set_anchor", "get_anchor", SIDE_TOP);
	ADD_PROPERTYI(PropertyInfo(Variant::FLOAT, "anchor_right", PROPERTY_HINT_RANGE, "0,1,0.001,or_lesser,or_greater"), "_set_anchor", "get_anchor", SIDE_RIGHT);
	ADD_PROPERTYI(PropertyInfo(Variant::FLOAT, "anchor_bottom", PROPERTY_HINT_RANGE, "0,1,0.001,or_lesser,or_greater"), "_set_anchor", "get_anchor", SIDE_BOTTOM);

Those properties use _set_anchor as setter, this is because Control also expose a set_anchor method (this one behind higher level designed to be directly used)

godot/scene/gui/control.cpp

Line 3189 in eda6800

ClassDB::bind_method(D_METHOD("set_anchor", "side", "anchor", "keep_offset", "push_opposite_anchor"), &Control::set_anchor, DEFVAL(false), DEFVAL(true));

So my guess is we should stop considering the underscore prefix as a private marker (and I'm saying this as a Python lover 😄 ) and just dump all methods/properties.

As a matter of fact I even wonder if there is such thing as class method declared in ClassDB that should be private (the whole point of GDExtension being to provide to external tools the same access GDScript have)

Steps to reproduce

n/a

Minimal reproduction project

No response

[Splizard]

The _ prefix for methods indicates that the method is virtual and can be overridden by a custom implementation.

[touilleMan]

@Splizard Thanks for the feedback

I have the felling this "underscore for virtual method" is very error prone given the is no unittests preventing us to do a bad naming. So we should not rely in that to filter out methods.

On top of that the comment in extension_api_dump.cpp is talking about hidden methods which is a wrong if it stands for virtual

[akien-mga]

A leading underscore is typically used for one of two meanings:

• A virtual method, which is flagged as virtual and this information should be available to GDExtension, it doesn't need to be guessed based on naming.
• A method that needs to be exposed but which we don't want visible in the API. That's typically for methods which need to be called by string (e.g. for UndoRedo). This also used to include all signal callbacks in 3.x but now most of those are using callable_mp and no longer need to be bound. Those are the methods meant to be "hidden" because while they're exposed and public, they're not meant to be in theory.

[touilleMan]

A method that needs to be exposed but which we don't want visible in the API.

Don't we need a dedicated flag for this kind of job ? (just like we have the virtual flag as you say)

Without a specific private flag, that means the test for private is:.

is_private = F.name.begins_with("_") && !(F.flags & METHOD_FLAG_VIRTUAL)

This seems error prone and hard to understand :/

[akien-mga]

We could maybe add one, yeah.

This kind of check is indeed done e.g. in editor/doc_tools.cpp to hide those methods from the class reference:

if (E.name.is_empty() || (E.name[0] == '_' && !(E.flags & METHOD_FLAG_VIRTUAL))) {
	continue; //hidden, don't count
}

Maybe is_internal or something like this. It's not private in the common sense because even though we hide those where we can, you can call these methods.

[reduz]

The general rule is that, anything with starts with '_' is either private or protected. Virtual functions are protected, neither can be accessed from outside the class.

If you are creating a binder, the logic should be:

• If your language supports properties do not expose the setters and getters to it, only the properties.
• If your language does not support properties, ignore them and expose any non private method.
• Virtual methods are protected, hence always exposed.

[AwesomeJuniorMiss]

So that's why we can't call super._process(delta) from GDScript that extends GDExtension class? But in many other engines, like Unreal, it's commonly used and can be very useful.

[dsnopek]

So that's why we can't call super._process(delta) from GDScript that extends GDExtension class? But in many other engines, like Unreal, it's commonly used and can be very useful.

No, I think that's unrelated. The problem is that we don't really have virtual methods defined in GDExtension yet, and super in GDScript can only really refer to methods on parent GDScript classes - I don't think it works with native classes either. A bunch of additional work around virtual methods would need to be done to allow super in GDScript to work across languages.