Skip to content

Better hide internal properties from users #87381

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jan 29, 2024
Merged

Better hide internal properties from users #87381

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 9 additions & 1 deletion editor/editor_help.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2862,9 +2862,17 @@ void EditorHelpTooltip::parse_tooltip(const String &p_text) {
const String &property_name = slices[2];
const String &property_args = slices[3];

String formatted_text;

// Exclude internal properties, they are not documented.
if (type == "internal_property") {
formatted_text = "[i]" + TTR("This property can only be set in the Inspector.") + "[/i]";
set_text(formatted_text);
return;
}

String title;
String description;
String formatted_text;

if (type == "class") {
title = class_name;
Expand Down
18 changes: 15 additions & 3 deletions editor/editor_inspector.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,10 @@ void EditorProperty::set_doc_path(const String &p_doc_path) {
doc_path = p_doc_path;
}

void EditorProperty::set_internal(bool p_internal) {
internal = p_internal;
}

void EditorProperty::update_property() {
GDVIRTUAL_CALL(_update_property);
}
Expand Down Expand Up @@ -748,10 +752,10 @@ void EditorProperty::shortcut_input(const Ref<InputEvent> &p_event) {
if (ED_IS_SHORTCUT("property_editor/copy_value", p_event)) {
menu_option(MENU_COPY_VALUE);
accept_event();
} else if (ED_IS_SHORTCUT("property_editor/paste_value", p_event) && !is_read_only()) {
} else if (!is_read_only() && ED_IS_SHORTCUT("property_editor/paste_value", p_event)) {
menu_option(MENU_PASTE_VALUE);
accept_event();
} else if (ED_IS_SHORTCUT("property_editor/copy_property_path", p_event)) {
} else if (!internal && ED_IS_SHORTCUT("property_editor/copy_property_path", p_event)) {
menu_option(MENU_COPY_PROPERTY_PATH);
accept_event();
}
Expand Down Expand Up @@ -1036,6 +1040,8 @@ void EditorProperty::_update_popup() {
menu->add_icon_shortcut(get_editor_theme_icon(SNAME("ActionPaste")), ED_GET_SHORTCUT("property_editor/paste_value"), MENU_PASTE_VALUE);
menu->add_icon_shortcut(get_editor_theme_icon(SNAME("CopyNodePath")), ED_GET_SHORTCUT("property_editor/copy_property_path"), MENU_COPY_PROPERTY_PATH);
menu->set_item_disabled(MENU_PASTE_VALUE, is_read_only());
menu->set_item_disabled(MENU_COPY_PROPERTY_PATH, internal);

if (!pin_hidden) {
menu->add_separator();
if (can_pin) {
Expand Down Expand Up @@ -3329,14 +3335,20 @@ void EditorInspector::update_tree() {
if (use_doc_hints) {
// `|` separator used in `EditorHelpTooltip` for formatting.
if (theme_item_name.is_empty()) {
ep->set_tooltip_text("property|" + classname + "|" + property_prefix + p.name + "|");
if (p.usage & PROPERTY_USAGE_INTERNAL) {
ep->set_tooltip_text("internal_property|" + classname + "|" + property_prefix + p.name + "|");
} else {
ep->set_tooltip_text("property|" + classname + "|" + property_prefix + p.name + "|");
}
} else {
ep->set_tooltip_text("theme_item|" + classname + "|" + theme_item_name + "|");
}
ep->has_doc_tooltip = true;
}

ep->set_doc_path(doc_path);
ep->set_internal(p.usage & PROPERTY_USAGE_INTERNAL);

ep->update_property();
ep->_update_pin_flags();
ep->update_editor_property_status();
Expand Down
2 changes: 2 additions & 0 deletions editor/editor_inspector.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ class EditorProperty : public Container {
StringName property;
String property_path;
String doc_path;
bool internal = false;
bool has_doc_tooltip = false;

int property_usage;
Expand Down Expand Up @@ -156,6 +157,7 @@ class EditorProperty : public Container {
EditorInspector *get_parent_inspector() const;

void set_doc_path(const String &p_doc_path);
void set_internal(bool p_internal);

virtual void update_property();
void update_editor_property_status();
Expand Down
12 changes: 9 additions & 3 deletions modules/gdscript/gdscript_editor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1135,7 +1135,7 @@ static void _find_identifiers_in_base(const GDScriptCompletionIdentifier &p_base
List<PropertyInfo> members;
scr->get_script_property_list(&members);
for (const PropertyInfo &E : members) {
if (E.usage & (PROPERTY_USAGE_CATEGORY | PROPERTY_USAGE_GROUP | PROPERTY_USAGE_SUBGROUP)) {
if (E.usage & (PROPERTY_USAGE_CATEGORY | PROPERTY_USAGE_GROUP | PROPERTY_USAGE_SUBGROUP | PROPERTY_USAGE_INTERNAL)) {
Copy link
Member

Choose a reason for hiding this comment

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

These flags are repeated a couple of times. It could be constant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should definitely define one for PROPERTY_USAGE_CATEGORY | PROPERTY_USAGE_GROUP | PROPERTY_USAGE_SUBGROUP. I'll try to look into it some other time though.

continue;
}
if (E.name.contains("/")) {
Expand Down Expand Up @@ -1210,7 +1210,7 @@ static void _find_identifiers_in_base(const GDScriptCompletionIdentifier &p_base
List<PropertyInfo> pinfo;
ClassDB::get_property_list(type, &pinfo);
for (const PropertyInfo &E : pinfo) {
if (E.usage & (PROPERTY_USAGE_CATEGORY | PROPERTY_USAGE_GROUP | PROPERTY_USAGE_SUBGROUP)) {
if (E.usage & (PROPERTY_USAGE_CATEGORY | PROPERTY_USAGE_GROUP | PROPERTY_USAGE_SUBGROUP | PROPERTY_USAGE_INTERNAL)) {
continue;
}
if (E.name.contains("/")) {
Expand Down Expand Up @@ -1273,7 +1273,7 @@ static void _find_identifiers_in_base(const GDScriptCompletionIdentifier &p_base
}

for (const PropertyInfo &E : members) {
if (E.usage & (PROPERTY_USAGE_CATEGORY | PROPERTY_USAGE_GROUP | PROPERTY_USAGE_SUBGROUP)) {
if (E.usage & (PROPERTY_USAGE_CATEGORY | PROPERTY_USAGE_GROUP | PROPERTY_USAGE_SUBGROUP | PROPERTY_USAGE_INTERNAL)) {
continue;
}
if (!String(E.name).contains("/")) {
Expand Down Expand Up @@ -3514,6 +3514,12 @@ static Error _lookup_symbol_from_base(const GDScriptParser::DataType &p_base, co
}

if (ClassDB::has_property(class_name, p_symbol, true)) {
PropertyInfo prop_info;
ClassDB::get_property_info(class_name, p_symbol, &prop_info, true);
if (prop_info.usage & PROPERTY_USAGE_INTERNAL) {
return ERR_CANT_RESOLVE;
}

r_result.type = ScriptLanguage::LOOKUP_RESULT_CLASS_PROPERTY;
r_result.class_name = base_type.native_type;
r_result.class_member = p_symbol;
Expand Down