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: Fix documentation for built-in scripts #92280

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dalexeev
Copy link
Member

@dalexeev dalexeev commented May 23, 2024

Fixes the following bugs:

1. Standard tooltip is displayed instead of doc tooltip

I couldn't reproduce this, but it is apparently caused by the check:

Control *ConnectionsDockTree::make_custom_tooltip(const String &p_text) const {
// If it's not a doc tooltip, fallback to the default one.
if (p_text.contains("::")) {
return nullptr;
}

String path = String(selected_node->get_path_to(target)) + " :: " + cd.method + "()";

2. Doc tooltip is empty for built-in classes and their members

This is because the Script Editor handles editing/saving external and built-in scripts inconsistently. As a result, Script.reload() was never called for built-in scripts (this method causes documentation to be regenerated in GDScript).

I'm not familiar with the Script Editor, so I tried to make minimal changes. I'm not sure about this part, please review and test carefully.

2.1. Documentation missing for built-in scripts

This is the real reason why tooltips didn't work. This fix may be inconvenient for some people, but we cannot make tooltips work without registering the built-in scripts documentation globally. Instead, we might add filters in the future (see godotengine/godot-proposals#4285).

After the fix:

3. EditorHelp does not handle :: in class names correctly

Added parsing instead of get_slice(":", index).

@dalexeev dalexeev added this to the 4.3 milestone May 23, 2024
@dalexeev dalexeev requested a review from a team as a code owner May 23, 2024 12:00
@KoBeWi
Copy link
Member

KoBeWi commented Jul 16, 2024

Does this allow for built-in script documentation? I listed it as a limitation #55287, so resolving it would be nice.

@dalexeev
Copy link
Member Author

Does this allow for built-in script documentation?

Yes, please test if it works correctly. But we should probably move the milestone to 4.4 because of possible regressions.

@akien-mga akien-mga modified the milestones: 4.3, 4.4 Jul 17, 2024
@KoBeWi
Copy link
Member

KoBeWi commented Aug 8, 2024

I tested that it works, but the documentation is lost upon editor restart. If the page is opened on launch, it's empty:
image
You need to save the script/scene while the script is open in the script editor to regenerate the doc.

Though it's fine like that if we don't have doc cache for scripts. It's unreasonable to generate docs for every built-in script in the project. But it would be nice if at least it didn't require saving after opening a scene.

@dalexeev
Copy link
Member Author

dalexeev commented Aug 8, 2024

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.

@export script variables do not display tooltips when they are in built in script of node.
3 participants