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

Add support for documenting most editor settings in the class reference #48548

Merged

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented May 7, 2021

Settings defined in editor plugins are missing (about 100 of them), but all other settings (about 200 of them) are now documented in the EditorSettings class.

This PR can be remade for the 3.x branch once we reach an agreement on its design.

This closes godotengine/godot-proposals#1339 and closes godotengine/godot-docs#395. See also #49524 and #49525 (can be merged independently).

Edit: This PR no longer includes the actual documentation for easier reviewing. Settings were actually documented in #63870.

TODO

  • Write descriptions for all editor settings currently present in the XML.
    • Will be split to a second PR.
  • Fix segfault at the end of --doctool . run (the XML is still generated fine).

In a future PR, I'll move editor setting default declarations to be located as much as possible in editor/editor_settings.cpp, so they can all be picked up by doctool and then documented.

Comment on lines 260 to 374

if (name == "EditorSettings") {
// Special case for editor settings, so they can be documented.
// Force the use of default editor settings to prevent the user's settings
// (and list of projects) from being included the list.
EditorSettings::create(true);
EditorSettings::get_singleton()->get_property_list(&properties);
own_properties = properties;
Copy link
Contributor

@EricEzaM EricEzaM May 8, 2021

Choose a reason for hiding this comment

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

Settings which are defined inside editor constructors (e.g. EditorNode EDITOR_DEF("run/output/always_clear_output_on_play", true);) will not be picked up on since the editor is not instantiated. This is also true for some project settings which do not get picked up by the doctool, e.g. editor/main_run_args, see my comment here: godotengine/godot-proposals#1339 (comment)

@Calinou Calinou force-pushed the editor-help-add-editor-settings branch 5 times, most recently from 1a77f87 to 45769b0 Compare June 10, 2021 18:01
@Calinou Calinou force-pushed the editor-help-add-editor-settings branch 3 times, most recently from c5ae78b to 24db120 Compare June 11, 2021 13:56
@Calinou
Copy link
Member Author

Calinou commented Jun 11, 2021

All settings present in the XML are now documented! That said, this doesn't mean all editor settings are now documented. Doing so would require significant refactoring to move editor setting declarations to a centralized location, so I'd prefer leaving this to a future PR.

Unfortunately, I didn't mange to fix the segfault issue when running --doctool. Could anyone lend me a hand on this?

Edit: Fixed by destroying the EditorSettings instance after using it. Thanks to @pycbouh for the advice 🙂

gdb backtrace

(gdb) bt
#0  OS::print_error (this=0x0, p_function=0x1c98122 "unref", 
    p_file=0x1d45cdd "core/string/string_name.cpp", p_line=94, p_code=0x1d5a38a "BUG!", 
    p_rationale=0x1e0f53e "", p_type=Logger::ERR_ERROR) at core/os/os.cpp:109
#1  0x0000000008f16395 in _err_print_error (p_function=0x1c98122 "unref", 
    p_file=0x1d45cdd "core/string/string_name.cpp", p_line=94, p_error=0x1d5a38a "BUG!", 
    p_message=0x1e0f53e "", p_type=ERR_HANDLER_ERROR) at core/error/error_macros.cpp:77
#2  0x0000000008f1634a in _err_print_error (p_function=0x1c98122 "unref", 
    p_file=0x1d45cdd "core/string/string_name.cpp", p_line=94, p_error=0x1d5a38a "BUG!", 
    p_type=ERR_HANDLER_ERROR) at core/error/error_macros.cpp:69
#3  0x0000000008eca5f3 in StringName::unref (this=0xbd2d5a0) at core/string/string_name.cpp:94
#4  0x0000000008ecf1a5 in StringName::~StringName (this=0xbd2d5a0) at core/string/string_name.cpp:378
#5  0x000000000668920e in EditorSettings::~EditorSettings (this=0xbd2d420)
    at editor/editor_settings.cpp:1746
#6  0x0000000006598b2f in memdelete<EditorSettings> (p_class=0xbd2d420) at ./core/os/memory.h:115
#7  0x0000000006598ae3 in Ref<EditorSettings>::unref (this=0x99ba7e8 <EditorSettings::singleton>)
    at ./core/object/reference.h:221
#8  0x0000000006598a85 in Ref<EditorSettings>::~Ref (this=0x99ba7e8 <EditorSettings::singleton>)
    at ./core/object/reference.h:233
#9  0x00007ffff7a78af7 in __run_exit_handlers () from /lib64/libc.so.6
#10 0x00007ffff7a78ca0 in exit () from /lib64/libc.so.6
#11 0x00007ffff7a60b7c in __libc_start_main () from /lib64/libc.so.6
#12 0x000000000486194e in _start ()

@connorshea
Copy link

This'd be great to have, I looked around for an issue/PR for this after trying to mess with some settings and not understanding what they do. Is there anything blocking this atm?

@Calinou
Copy link
Member Author

Calinou commented Jul 25, 2021

This'd be great to have, I looked around for an issue/PR for this after trying to mess with some settings and not understanding what they do. Is there anything blocking this atm?

This needs to be tested again once #50864 is merged, to see whether documentation generation still works in that case (it probably does).

@akien-mga
Copy link
Member

akien-mga commented Jan 7, 2022

Needs a rebase.

If some extensive review is needed from @godotengine/documentation for the added descriptions, I would suggest splitting this in two PRs: one to add the possibility to generate and use editor settings docstrings, and sync the empty templates. And one that fills the templates.

@Calinou Calinou force-pushed the editor-help-add-editor-settings branch from 3db8fa7 to d705b17 Compare January 7, 2022 19:43
@Calinou
Copy link
Member Author

Calinou commented Jan 7, 2022

Rebased, with descriptions added for new settings.

I'm getting this error on CI, but I forgot where you force "fake" default values for doctool to use:

-		<member name="filesystem/directories/default_project_path" type="String" setter="" getter="" default="&quot;/home/hugo&quot;">
+		<member name="filesystem/directories/default_project_path" type="String" setter="" getter="" default="&quot;/home/runner&quot;">

@Calinou Calinou force-pushed the editor-help-add-editor-settings branch from d705b17 to 7478d7c Compare January 7, 2022 19:55
@YuriSizov
Copy link
Contributor

I'm getting this error on CI, but I forgot where you force "fake" default values for doctool to use:

I suspect we only have something for project settings, but not editor settings. The *_NOVAL_* macros, that is.

@Calinou Calinou force-pushed the editor-help-add-editor-settings branch 2 times, most recently from c36f160 to d28f847 Compare February 28, 2022 01:35
@Calinou Calinou changed the title Document most editor settings in the class reference Add support for documenting most editor settings in the class reference Apr 2, 2022
@Calinou Calinou force-pushed the editor-help-add-editor-settings branch from d28f847 to d0e6d9a Compare April 3, 2022 00:35
Settings defined in editor plugins are missing (about 100 of them),
but all other settings (about 200 of them) can now be documented in the
EditorSettings class.

Co-authored-by: Rémi Verschelde <[email protected]>
@akien-mga akien-mga force-pushed the editor-help-add-editor-settings branch from d0e6d9a to 63ce655 Compare July 29, 2022 20:07
@akien-mga akien-mga added the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Jul 29, 2022
@akien-mga
Copy link
Member

I pushed an update which makes the solution a lot less hacky. I think it's good to go now :)

@akien-mga akien-mga merged commit 9094262 into godotengine:master Jul 29, 2022
@akien-mga
Copy link
Member

Thanks!

@akien-mga
Copy link
Member

Cherry-picked for 3.6.

@akien-mga akien-mga removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Aug 24, 2022
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.

Add descriptions/tooltips to editor settings Add an EditorSettings reference
6 participants