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

[Editor] Expose more editor settings to documentation #93427

Merged
merged 1 commit into from
Sep 13, 2024

Conversation

AThousandShips
Copy link
Member

@AThousandShips AThousandShips commented Jun 21, 2024

Found quite a few editor settings that weren't declared in a way that allowed the doctool to find them. Haven't fully confirmed that moving these won't break anything, but it shouldn't.

Needing some help with the description for a number of the settings as I don't know what they do, but I've tried to fill them in as best I could.

Some of the settings I left where they were and added a comment, as I didn't know how to move them without possibly breaking them

The settings were copied over as closely as possible, converting to _initial_set and similar, should match correctly (I'm not 100% sure on the exact syntax to use in each case)

@@ -562,6 +568,10 @@ void EditorSettings::_load_defaults(Ref<ConfigFile> p_extra_config) {
_initial_set("filesystem/on_save/compress_binary_resources", true);
_initial_set("filesystem/on_save/safe_save_on_backup_then_rename", true);

// File server
_initial_set("filesystem/file_server/port", 6010);
Copy link
Member Author

Choose a reason for hiding this comment

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

Should this have a range?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes but with no_slider.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah there should probably be a range if there's a min and max for valid ports. I looked through the code a bit and we don't seem to do any validation for the port range currently in EditorFileServer/TCPServer/NetSocketPosix before passing it to the OS APIs.

CC @Faless @mhilbrunner


// Script list
_initial_set("text_editor/script_list/show_members_overview", true);
_initial_set("text_editor/script_list/sort_members_outline_alphabetically", false);
_initial_set("text_editor/script_list/script_temperature_enabled", true);
_initial_set("text_editor/script_list/script_temperature_history_size", 15);
Copy link
Member Author

Choose a reason for hiding this comment

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

Should this have a range?

Copy link
Member

@akien-mga akien-mga Sep 10, 2024

Choose a reason for hiding this comment

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

I wouldn't bother here, unless very high numbers brick the editor.

EDITOR_SETTING_USAGE(Variant::COLOR, PROPERTY_HINT_NONE, "editors/3d_gizmos/gizmo_colors/skeleton", Color(1, 0.8, 0.4), "", PROPERTY_USAGE_DEFAULT | PROPERTY_USAGE_RESTART_IF_CHANGED)
EDITOR_SETTING_USAGE(Variant::COLOR, PROPERTY_HINT_NONE, "editors/3d_gizmos/gizmo_colors/selected_bone", Color(0.8, 0.3, 0.0), "", PROPERTY_USAGE_DEFAULT | PROPERTY_USAGE_RESTART_IF_CHANGED)
EDITOR_SETTING_USAGE(Variant::COLOR, PROPERTY_HINT_NONE, "editors/3d_gizmos/gizmo_colors/csg", Color(0.0, 0.4, 1, 0.15), "", PROPERTY_USAGE_DEFAULT | PROPERTY_USAGE_RESTART_IF_CHANGED)
_initial_set("editors/3d_gizmos/gizmo_settings/bone_axis_length", (float)0.1);
Copy link
Member Author

Choose a reason for hiding this comment

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

Should this have a range?

EDITOR_SETTING_USAGE(Variant::COLOR, PROPERTY_HINT_NONE, "editors/3d_gizmos/gizmo_colors/csg", Color(0.0, 0.4, 1, 0.15), "", PROPERTY_USAGE_DEFAULT | PROPERTY_USAGE_RESTART_IF_CHANGED)
_initial_set("editors/3d_gizmos/gizmo_settings/bone_axis_length", (float)0.1);
EDITOR_SETTING(Variant::INT, PROPERTY_HINT_ENUM, "editors/3d_gizmos/gizmo_settings/bone_shape", 1, "Wire,Octahedron");
EDITOR_SETTING_USAGE(Variant::FLOAT, PROPERTY_HINT_NONE, "editors/3d_gizmos/gizmo_settings/path3d_tilt_disk_size", 0.8, "", PROPERTY_USAGE_DEFAULT | PROPERTY_USAGE_RESTART_IF_CHANGED)
Copy link
Member Author

Choose a reason for hiding this comment

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

Should this have a range?

Comment on lines 824 to 825
_initial_set("editors/grid_map/palette_min_width", 230);
_initial_set("editors/grid_map/preview_size", 64);
Copy link
Member Author

Choose a reason for hiding this comment

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

Should these have ranges?

The path to the text editor executable used to edit text files if [member text_editor/external/use_external_editor] is [code]true[/code].
</member>
<member name="text_editor/external/use_external_editor" type="bool" setter="" getter="">
If [code]true[/code] opens an external editor instead of the built-in script editor. See also [member text_editor/external/exec_path] and [member text_editor/external/exec_flags].
Copy link
Member Author

Choose a reason for hiding this comment

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

Depending on the desired behavior from:

This might need clarification

@@ -4596,9 +4596,6 @@ SceneTreeDock::SceneTreeDock(Node *p_scene_root, EditorSelection *p_editor_selec
set_process_input(true);
set_process(true);

EDITOR_DEF("interface/editors/show_scene_tree_root_selection", true);
EDITOR_DEF("interface/editors/derive_script_globals_by_name", true);
EDITOR_DEF("docks/scene_tree/ask_before_deleting_related_animation_tracks", true);
EDITOR_DEF("_use_favorites_root_selection", false);
Copy link
Member Author

Choose a reason for hiding this comment

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

Left this as it looks internal

Comment on lines +743 to +764
_initial_set("editors/3d_gizmos/gizmo_settings/bone_axis_length", (float)0.1);
EDITOR_SETTING(Variant::INT, PROPERTY_HINT_ENUM, "editors/3d_gizmos/gizmo_settings/bone_shape", 1, "Wire,Octahedron");
Copy link
Member Author

Choose a reason for hiding this comment

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

Should these be restart required? Don't know how they work

@KoBeWi
Copy link
Member

KoBeWi commented Jun 21, 2024

As I once mentioned in godotengine/godot-proposals#4771 (comment), I think it might be better to find a way to define properties in where they belong, but still allow doctools to find them.

Though the current solution, for the sake of exposing the properties to documentation, is fine for now.

@AThousandShips AThousandShips added this to the 4.4 milestone Jul 7, 2024
Copy link
Contributor

@Mickeon Mickeon left a comment

Choose a reason for hiding this comment

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

My reviews above still stand, unless you disagree intentionally of course.

@AThousandShips
Copy link
Member Author

Will go over them, just pushed to fix conflicts

@AThousandShips AThousandShips force-pushed the editor_setting_doc branch 2 times, most recently from b024e76 to cd6e0a8 Compare September 5, 2024 16:16
@akien-mga akien-mga requested a review from Calinou September 5, 2024 17:30
Copy link
Contributor

@Mickeon Mickeon left a comment

Choose a reason for hiding this comment

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

Some of these descriptions still need to be filled but I wouldn't spend further time on this current PR doing that.

@@ -852,6 +852,7 @@ void GDScriptSyntaxHighlighter::_update_cache() {
comment_marker_colors[COMMENT_MARKER_NOTICE] = Color(0.24, 0.54, 0.09);
}

// TODO: Move to editor_settings.cpp
Copy link
Member

Choose a reason for hiding this comment

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

@AThousandShips
Copy link
Member Author

Will add the last suggestions later today and some of the descriptions can be elaborated on in future dedicated PRs by people with specific knowledge

@AThousandShips AThousandShips force-pushed the editor_setting_doc branch 3 times, most recently from 8d0fcd0 to abf0b65 Compare September 9, 2024 16:34
Co-authored-by: Tomasz Chabora <kobewi4e@gmail.com>
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Let's merge this as a good first step, and we can iterate further on adding ranges or improving descriptions in later PRs.

@akien-mga akien-mga merged commit 04456cf into godotengine:master Sep 13, 2024
20 checks passed
@akien-mga
Copy link
Member

Thanks!

@AThousandShips AThousandShips deleted the editor_setting_doc branch September 13, 2024 09:29
@AThousandShips
Copy link
Member Author

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants