godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
88.97k stars 20.18k forks source link

Description missing from theme properties on type variations #94790

Open Giganzo opened 1 month ago

Giganzo commented 1 month ago

Tested versions

Godot v4.3.rc1

System information

Fedora Linux 40 (KDE Plasma)

Issue description

Description missing from theme properties on type variations

Steps to reproduce

Create a theme add a type variation set base type. Hover over any theme property, tooltip says "no description available"

Screenshot_20240726_125625

Screenshot_20240726_125710

Maybe there is a reason for this. If so would it be more helpful with "See base type for description"?

Minimal reproduction project (MRP)

N/A

akien-mga commented 1 month ago

I don't understand what the two screenshots are showing respectively. It seems to be a screenshot of the exact same location but one has the description and the other not? How do you reproduce both cases?

Giganzo commented 1 month ago

I don't understand what the two screenshots are showing respectively. It seems to be a screenshot of the exact same location but one has the description and the other not? How do you reproduce both cases?

First picture is for button. Second image is from a type variation with base type set to button. I would expect both of them to show same description for the default properties.

akien-mga commented 1 month ago

This might be a good first issue for new contributors.

I would suggest starting by looking at the code for EditorHelpBit in editor/editor_help.cpp, notably EditorHelpBit::HelpData EditorHelpBit::_get_theme_item_help_data. Maybe it can be extended to handle theme variations by checking for the description in their base type.

See also editor/plugins/theme_editor_plugin.cpp where it instantiates EditorHelpBit:

Control *ThemeItemLabel::make_custom_tooltip(const String &p_text) const {
        EditorHelpBit *help_bit = memnew(EditorHelpBit(p_text));
        EditorHelpBitTooltip::show_tooltip(help_bit, const_cast<ThemeItemLabel *>(this));
        return memnew(Control); // Make the standard tooltip invisible.
}
JeffreyJu8 commented 1 month ago

Hi, I would like to contribute to this issue.

Dowsley commented 1 month ago

@JeffreyJu8 I was also looking at this issue yesterday. I'll let you know if I find anything :) hopefully it helps

Dowsley commented 1 month ago

Here's a clue:

If I reproduce the steps here, which is adding a type variation, and setting base type via the UI, the description does not show as expected.

button2_creation

button2_description_empty

But if I later create a script with a named class of the same name (in this case, Button2), extending Button, the description appears correctly. Also, the UI updates with the icon for the button, correctly showing it is of type button now.

button2_script_creation

button2_description_shows

I believe this is because inside EditorHelpBit::HelpData EditorHelpBit::_get_theme_item_help_data there is already a mechanism that fetches the helper data from the parent class. But when creating a variation (in that case, Button2) via the UI, it does not register as a class somehow.

I tested that manually by watching the value of the classdoc iterator E, which is initially assigned the value of dd->class_list.find(p_class_name); . Without a named script, it does not find any class with the name in _p_classname. With an empty iterator, it is impossible to find its parent class.

Not sure if I am missing something here. But is this a bug? Or intended behaviour @akien-mga ?

NicoCobb commented 1 month ago

Hey @JeffreyJu8, are you still taking a look at this? I would like to tackle it if not but if you're in process on it I don't want to overstep

JeffreyJu8 commented 1 month ago

Hey @JeffreyJu8, are you still taking a look at this? I would like to tackle it if not but if you're in process on it I don't want to overstep

Hi, I'm currently not working on it so feel free to tackle it!

NicoCobb commented 3 weeks ago

Hey finally got around to attempting to address this, is there someone I should talk to for code review and conducting that process as this is my first contribution? My PR is here at https://github.com/godotengine/godot/pull/95822.

This fixes the documentation issue, but the associated icon does not update currently with it and I'm checking into making that work as well. Tried to implement it in the least disruptive way I could find, but would love notes on what could be improved or if there's anything I missed.

I put in the description that it is associated with this issue, but unsure if there was a better place to link the issue or if I missed anything in the PR guidelines