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

Revamp how documentation tooltips work #82051

Conversation

YeldhamDev
Copy link
Member

This PR does the following this:

  • Add a set of get_*_description() functions to EditorHelpBit, that fetches the doc description of various components, and then caches them.
  • Create EditorHelpTooltip, that handles the doc tooltips that before were scattered across multiple places. It internally uses the functions above.
  • As a bonus, prettify a little some simple uses of EditorHelpBit I found while making this PR.

This PR is necessary for #81284 (see #81284 (comment)).

@YeldhamDev YeldhamDev added this to the 4.2 milestone Sep 21, 2023
@YeldhamDev YeldhamDev requested review from a team and YuriSizov September 21, 2023 14:18
@YeldhamDev YeldhamDev force-pushed the i_just_wanted_to_add_tooltips_to_theme_items_man branch from 50cf640 to ea80f45 Compare September 21, 2023 14:40
editor/editor_help.cpp Outdated Show resolved Hide resolved
@YeldhamDev YeldhamDev force-pushed the i_just_wanted_to_add_tooltips_to_theme_items_man branch from ea80f45 to 7d71021 Compare September 22, 2023 03:29
@YuriSizov YuriSizov mentioned this pull request Sep 26, 2023
75 tasks
editor/editor_help.cpp Outdated Show resolved Hide resolved
editor/editor_help.cpp Outdated Show resolved Hide resolved
editor/connections_dialog.cpp Show resolved Hide resolved
editor/connections_dialog.cpp Outdated Show resolved Hide resolved
editor/connections_dialog.cpp Outdated Show resolved Hide resolved
@YeldhamDev YeldhamDev force-pushed the i_just_wanted_to_add_tooltips_to_theme_items_man branch 2 times, most recently from 29ed403 to da43d87 Compare September 26, 2023 20:25
@YeldhamDev
Copy link
Member Author

@dalexeev Changes made.

editor/editor_help.cpp Outdated Show resolved Hide resolved
editor/editor_help.cpp Outdated Show resolved Hide resolved
editor/editor_help.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@YuriSizov YuriSizov left a comment

Choose a reason for hiding this comment

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

Some changes are still required, but overall this should be a great improvement.

We could probably reduce code duplication even more around the whole "No description available for %s." self-modulation construction. But this can be done in a follow-up.

@akien-mga
Copy link
Member

akien-mga commented Oct 2, 2023

Tested this PR briefly while trying to assess whether it would conflict with #76147.

I noticed this bug when you hover a category (@export_category):

ERROR: Invalid tooltip formatting. The expect string should be formatted as 'type|class|property|args'.
   at: parse_tooltip (./editor/editor_help.cpp:2847)

This happens whether the category has a doc hint or not, for example:

extends Node2D

## The script is described here.

## Hello some int
@export var test: int = 6

## My categories are exquisite.
@export_category("Yo cat")

## This is a plop
@export var plop: int = 42

For the record, in 4.2-dev5 categories don't seem to support doc hints in the tooltip, they always show the category's name as a tooltip. In this PR, it shows an empty tooltip + the error mentioned above.

Ideally it would be good for categories to support doc hints too, but that's probably for a separate PR.

Tested @export_subgroup too for good measure, those seem to work like before this PR (no support for doc hints, but shows the group name in the tooltip).

@YeldhamDev YeldhamDev force-pushed the i_just_wanted_to_add_tooltips_to_theme_items_man branch from e6d6b7d to 54a7521 Compare October 3, 2023 18:21
@YeldhamDev YeldhamDev force-pushed the i_just_wanted_to_add_tooltips_to_theme_items_man branch from 54a7521 to ae91644 Compare October 3, 2023 20:41
Copy link
Contributor

@YuriSizov YuriSizov left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for taking a crack at this refactor

@akien-mga akien-mga merged commit bb30c83 into godotengine:master Oct 4, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@YeldhamDev YeldhamDev deleted the i_just_wanted_to_add_tooltips_to_theme_items_man branch October 4, 2023 13:54
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.

4 participants