Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C#: Make internal properties and property accessors public (but hidden) #90002

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 18 additions & 8 deletions modules/mono/editor/bindings_generator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2581,6 +2581,10 @@ Error BindingsGenerator::_generate_cs_property(const BindingsGenerator::TypeInte
p_output.append("\")]");
}

if (p_iprop.is_hidden) {
p_output.append(MEMBER_BEGIN "[EditorBrowsable(EditorBrowsableState.Never)]");
}

p_output.append(MEMBER_BEGIN "public ");

if (prop_allowed_inherited_member_hiding.has(p_itype.proxy_name + "." + p_iprop.proxy_name)) {
Expand Down Expand Up @@ -2840,7 +2844,7 @@ Error BindingsGenerator::_generate_cs_method(const BindingsGenerator::TypeInterf
p_output.append("\")]");
}

if (p_imethod.is_compat) {
if (p_imethod.is_hidden) {
p_output.append(MEMBER_BEGIN "[EditorBrowsable(EditorBrowsableState.Never)]");
}

Expand Down Expand Up @@ -3654,11 +3658,16 @@ bool BindingsGenerator::_populate_object_type_interfaces() {
iprop.setter = ClassDB::get_property_setter(type_cname, iprop.cname);
iprop.getter = ClassDB::get_property_getter(type_cname, iprop.cname);

if (iprop.setter != StringName()) {
accessor_methods[iprop.setter] = iprop.cname;
}
if (iprop.getter != StringName()) {
accessor_methods[iprop.getter] = iprop.cname;
// If the property is internal hide it; otherwise, hide the getter and setter.
if (property.usage & PROPERTY_USAGE_INTERNAL) {
iprop.is_hidden = true;
} else {
if (iprop.setter != StringName()) {
accessor_methods[iprop.setter] = iprop.cname;
}
if (iprop.getter != StringName()) {
accessor_methods[iprop.getter] = iprop.cname;
}
}

bool valid = false;
Expand Down Expand Up @@ -3860,10 +3869,10 @@ bool BindingsGenerator::_populate_object_type_interfaces() {

HashMap<StringName, StringName>::Iterator accessor = accessor_methods.find(imethod.cname);
if (accessor) {
// We only make internal an accessor method if it's in the same class as the property.
// We only hide an accessor method if it's in the same class as the property.
// It's easier this way, but also we don't know if an accessor method in a different class
// could have other purposes, so better leave those untouched.
imethod.is_internal = true;
imethod.is_hidden = true;
}

if (itype.class_doc) {
Expand Down Expand Up @@ -3892,6 +3901,7 @@ bool BindingsGenerator::_populate_object_type_interfaces() {
// after all the non-compat methods have been added. The compat methods are added in
// reverse so the most recently added ones take precedence over older compat methods.
if (imethod.is_compat) {
imethod.is_hidden = true;
compat_methods.push_front(imethod);
continue;
}
Expand Down
16 changes: 16 additions & 0 deletions modules/mono/editor/bindings_generator.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,14 @@ class BindingsGenerator {
StringName setter;
StringName getter;

/**
* Determines if the property will be hidden with the [EditorBrowsable(EditorBrowsableState.Never)]
* attribute.
* We do this for propertyies that have the PROPERTY_USAGE_INTERNAL flag, because they are not meant
* to be exposed to scripting but we can't remove them to prevent breaking compatibility.
*/
bool is_hidden = false;

const DocData::PropertyDoc *prop_doc;

bool is_deprecated = false;
Expand Down Expand Up @@ -179,6 +187,14 @@ class BindingsGenerator {
*/
bool is_internal = false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might want to update the comment on this, it speaks of setters and getters but are not internal any longer

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this still applies, it refers to property getters and setters when the method starts with an underscore and are not virtual. For example: AnimationLibrary::_set_data.


/**
* Determines if the method will be hidden with the [EditorBrowsable(EditorBrowsableState.Never)]
* attribute.
* We do this for methods that we don't want to expose but need to be public to prevent breaking
* compat (i.e: methods with 'is_compat' set to true.)
*/
bool is_hidden = false;

/**
* Determines if the method is a compatibility method added to avoid breaking binary compatibility.
* These methods will be generated but hidden and are considered deprecated.
Expand Down
Loading