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

Tooltips: Improve code clarity and docs #43280

Merged
merged 1 commit into from
Nov 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 30 additions & 9 deletions doc/classes/Control.xml
Original file line number Diff line number Diff line change
Expand Up @@ -75,15 +75,17 @@
</description>
</method>
<method name="_make_custom_tooltip" qualifiers="virtual">
<return type="Object">
<return type="Control">
</return>
<argument index="0" name="for_text" type="String">
</argument>
<description>
Virtual method to be implemented by the user. Returns a [Control] node that should be used as a tooltip instead of the default one. Use [code]for_text[/code] parameter to determine what text the tooltip should contain (likely the contents of [member hint_tooltip]).
The returned node must be of type [Control] or Control-derieved. It can have child nodes of any type. It is freed when the tooltip disappears, so make sure you always provide a new instance, not e.g. a node from scene. When [code]null[/code] or non-Control node is returned, the default tooltip will be used instead.
Virtual method to be implemented by the user. Returns a [Control] node that should be used as a tooltip instead of the default one. The [code]for_text[/code] includes the contents of the [member hint_tooltip] property.
The returned node must be of type [Control] or Control-derived. It can have child nodes of any type. It is freed when the tooltip disappears, so make sure you always provide a new instance (if you want to use a pre-existing node from your scene tree, you can duplicate it and pass the duplicated instance).When [code]null[/code] or a non-Control node is returned, the default tooltip will be used instead.
The returned node will be added as child to a [PopupPanel], so you should only provide the contents of that panel. That [PopupPanel] can be themed using [method Theme.set_stylebox] for the type [code]"TooltipPanel"[/code] (see [member hint_tooltip] for an example).
[b]Note:[/b] The tooltip is shrunk to minimal size. If you want to ensure it's fully visible, you might want to set its [member rect_min_size] to some non-zero value.
Example of usage with custom-constructed node:
[b]Note:[/b] The node (and any relevant children) should be [member CanvasItem.visible] when returned, otherwise the viewport that instantiates it will not be able to calculate its minimum size reliably.
Example of usage with a custom-constructed node:
[codeblocks]
[gdscript]
func _make_custom_tooltip(for_text):
Expand All @@ -92,26 +94,26 @@
return label
[/gdscript]
[csharp]
public override Godot.Object _MakeCustomTooltip(String forText)
public override Godot.Control _MakeCustomTooltip(String forText)
{
var label = new Label();
label.Text = forText;
return label;
}
[/csharp]
[/codeblocks]
Example of usage with custom scene instance:
Example of usage with a custom scene instance:
[codeblocks]
[gdscript]
func _make_custom_tooltip(for_text):
var tooltip = preload("SomeTooltipScene.tscn").instance()
var tooltip = preload("res://SomeTooltipScene.tscn").instance()
tooltip.get_node("Label").text = for_text
return tooltip
[/gdscript]
[csharp]
public override Godot.Object _MakeCustomTooltip(String forText)
public override Godot.Control _MakeCustomTooltip(String forText)
{
Node tooltip = ResourceLoader.Load&lt;PackedScene&gt;("SomeTooltipScene.tscn").Instance();
Node tooltip = ResourceLoader.Load&lt;PackedScene&gt;("res://SomeTooltipScene.tscn").Instance();
tooltip.GetNode&lt;Label&gt;("Label").Text = forText;
return tooltip;
}
Expand Down Expand Up @@ -993,6 +995,25 @@
</member>
<member name="hint_tooltip" type="String" setter="set_tooltip" getter="_get_tooltip" default="&quot;&quot;">
Changes the tooltip text. The tooltip appears when the user's mouse cursor stays idle over this control for a few moments, provided that the [member mouse_filter] property is not [constant MOUSE_FILTER_IGNORE]. You can change the time required for the tooltip to appear with [code]gui/timers/tooltip_delay_sec[/code] option in Project Settings.
The tooltip popup will use either a default implementation, or a custom one that you can provide by overriding [method _make_custom_tooltip]. The default tooltip includes a [PopupPanel] and [Label] whose theme properties can be customized using [Theme] methods with the [code]"TooltipPanel"[/code] and [code]"TooltipLabel"[/code] respectively. For example:
[codeblocks]
[gdscript]
var style_box = StyleBoxFlat.new()
style_box.set_bg_color(Color(1, 1, 0))
style_box.set_border_width_all(2)
# We assume here that the `theme` property has been assigned a custom Theme beforehand.
theme.set_stylebox("panel", "TooltipPanel", style_box)
theme.set_color("font_color", "TooltipLabel", Color(0, 1, 1))
[/gdscript]
[csharp]
var styleBox = new StyleBoxFlat();
styleBox.SetBgColor(new Color(1, 1, 0));
styleBox.SetBorderWidthAll(2);
// We assume here that the `Theme` property has been assigned a custom Theme beforehand.
Theme.SetStyleBox("panel", "TooltipPanel", styleBox);
Theme.SetColor("font_color", "TooltipLabel", new Color(0, 1, 1));
[/csharp]
Comment on lines +1008 to +1015
Copy link
Member Author

Choose a reason for hiding this comment

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

I haven't tested my C# conversion yet. If anyone wants to give it a try and confirm that it works, that'd be welcome.

Note that you have to have a valid Theme, which can presumably be created with:

Theme = new Theme();
Theme.CopyDefaultTheme();

[/codeblocks]
</member>
<member name="margin_bottom" type="float" setter="set_margin" getter="get_margin" default="0.0">
Distance between the node's bottom edge and its parent control, based on [member anchor_bottom].
Expand Down
4 changes: 3 additions & 1 deletion scene/gui/control.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2738,7 +2738,9 @@ void Control::_bind_methods() {

BIND_VMETHOD(MethodInfo(Variant::BOOL, "can_drop_data", PropertyInfo(Variant::VECTOR2, "position"), PropertyInfo(Variant::NIL, "data")));
BIND_VMETHOD(MethodInfo("drop_data", PropertyInfo(Variant::VECTOR2, "position"), PropertyInfo(Variant::NIL, "data")));
BIND_VMETHOD(MethodInfo(Variant::OBJECT, "_make_custom_tooltip", PropertyInfo(Variant::STRING, "for_text")));
BIND_VMETHOD(MethodInfo(
PropertyInfo(Variant::OBJECT, "control", PROPERTY_HINT_RESOURCE_TYPE, "Control"),
"_make_custom_tooltip", PropertyInfo(Variant::STRING, "for_text")));
BIND_VMETHOD(MethodInfo(Variant::BOOL, "_clips_input"));

ADD_GROUP("Anchor", "anchor_");
Expand Down
82 changes: 46 additions & 36 deletions scene/main/viewport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,11 @@ ViewportTexture::~ViewportTexture() {

/////////////////////////////////////

// Aliases used to provide custom styles to tooltips in the default
// theme and editor theme.
// TooltipPanel is also used for custom tooltips, while TooltipLabel
// is only relevant for default tooltips.

class TooltipPanel : public PopupPanel {
GDCLASS(TooltipPanel, PopupPanel);

Expand All @@ -175,6 +180,8 @@ class TooltipLabel : public Label {
TooltipLabel() {}
};

/////////////////////////////////////

Viewport::GUI::GUI() {
embed_subwindows_hint = false;
embedding_subwindows = false;
Expand All @@ -188,7 +195,7 @@ Viewport::GUI::GUI() {
mouse_over = nullptr;
drag_mouse_over = nullptr;

tooltip = nullptr;
tooltip_control = nullptr;
tooltip_popup = nullptr;
tooltip_label = nullptr;
}
Expand Down Expand Up @@ -1465,7 +1472,7 @@ void Viewport::_gui_sort_roots() {
}

void Viewport::_gui_cancel_tooltip() {
gui.tooltip = nullptr;
gui.tooltip_control = nullptr;
gui.tooltip_timer = -1;
if (gui.tooltip_popup) {
gui.tooltip_popup->queue_delete();
Expand All @@ -1474,21 +1481,23 @@ void Viewport::_gui_cancel_tooltip() {
}
}

String Viewport::_gui_get_tooltip(Control *p_control, const Vector2 &p_pos, Control **r_which) {
String Viewport::_gui_get_tooltip(Control *p_control, const Vector2 &p_pos, Control **r_tooltip_owner) {
Vector2 pos = p_pos;
String tooltip;

while (p_control) {
tooltip = p_control->get_tooltip(pos);

if (r_which) {
*r_which = p_control;
if (r_tooltip_owner) {
*r_tooltip_owner = p_control;
}

if (tooltip != String()) {
// If we found a tooltip, we stop here.
if (!tooltip.empty()) {
break;
}
pos = p_control->get_transform().xform(pos);

// Otherwise, we check parent controls unless some conditions prevent it.

if (p_control->data.mouse_filter == Control::MOUSE_FILTER_STOP) {
break;
Expand All @@ -1497,41 +1506,50 @@ String Viewport::_gui_get_tooltip(Control *p_control, const Vector2 &p_pos, Cont
break;
}

// Transform cursor pos for parent control.
pos = p_control->get_transform().xform(pos);

p_control = p_control->get_parent_control();
}

return tooltip;
}

void Viewport::_gui_show_tooltip() {
if (!gui.tooltip) {
if (!gui.tooltip_control) {
return;
}

Control *which = nullptr;
String tooltip = _gui_get_tooltip(gui.tooltip, gui.tooltip->get_global_transform().xform_inv(gui.last_mouse_pos), &which);
tooltip = tooltip.strip_edges();
if (tooltip.length() == 0) {
return; // bye
// Get the Control under cursor and the relevant tooltip text, if any.
Control *tooltip_owner = nullptr;
String tooltip_text = _gui_get_tooltip(
Copy link
Contributor

Choose a reason for hiding this comment

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

Very nice clean up on this one, especially the confusing variable names. I wasn't sure when tooltip refers to the control and when to the string when I was looking at that code. Since you are making changes to that function, I'd also rename the which variable to something more descriptive. Perhaps tooltip_text_source? Or tooltip_text_source_node, since tooltip_text_source_control would be a bit confusing, haha.

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought about renaming which but since it's only used in local code (and a helper method) I didn't feel it was so problematic. But here goes, I renamed it to tooltip_owner.

gui.tooltip_control,
gui.tooltip_control->get_global_transform().xform_inv(gui.last_mouse_pos),
&tooltip_owner);
tooltip_text.strip_edges();
Copy link
Member Author

Choose a reason for hiding this comment

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

I introduced a bug here, this doesn't do anything as it's not assigned.

if (tooltip_text.empty()) {
return; // Nothing to show.
}

// Remove previous popup if we change something.
if (gui.tooltip_popup) {
memdelete(gui.tooltip_popup);
gui.tooltip_popup = nullptr;
gui.tooltip_label = nullptr;
}

if (!which) {
if (!tooltip_owner) {
return;
}

Control *rp = which;

Control *base_tooltip = which->make_custom_tooltip(tooltip);
// Controls can implement `make_custom_tooltip` to provide their own tooltip.
// This should be a Control node which will be added as child to a TooltipPanel.
Control *base_tooltip = tooltip_owner->make_custom_tooltip(tooltip_text);

// If no custom tooltip is given, use a default implementation.
if (!base_tooltip) {
gui.tooltip_label = memnew(TooltipLabel);
gui.tooltip_label->set_text(tooltip);
gui.tooltip_label->set_text(tooltip_text);
base_tooltip = gui.tooltip_label;
}

Expand All @@ -1545,10 +1563,7 @@ void Viewport::_gui_show_tooltip() {

gui.tooltip_popup = panel;

rp->add_child(gui.tooltip_popup);

//if (gui.tooltip) // Avoids crash when rapidly switching controls.
// gui.tooltip_popup->set_scale(gui.tooltip->get_global_transform().get_scale());
tooltip_owner->add_child(gui.tooltip_popup);

Point2 tooltip_offset = ProjectSettings::get_singleton()->get("display/mouse_cursor/tooltip_position_offset");
Rect2 r(gui.tooltip_pos + tooltip_offset, gui.tooltip_popup->get_contents_minimum_size());
Expand Down Expand Up @@ -1897,8 +1912,6 @@ void Viewport::_gui_input_event(Ref<InputEvent> p_event) {
}

_gui_cancel_tooltip();
//gui.tooltip_popup->hide();

} else {
if (gui.drag_data.get_type() != Variant::NIL && mb->get_button_index() == BUTTON_LEFT) {
if (gui.drag_mouse_over) {
Expand Down Expand Up @@ -2052,8 +2065,8 @@ void Viewport::_gui_input_event(Ref<InputEvent> p_event) {
bool is_tooltip_shown = false;

if (gui.tooltip_popup) {
if (can_tooltip && gui.tooltip) {
String tooltip = _gui_get_tooltip(over, gui.tooltip->get_global_transform().xform_inv(mpos));
if (can_tooltip && gui.tooltip_control) {
String tooltip = _gui_get_tooltip(over, gui.tooltip_control->get_global_transform().xform_inv(mpos));

if (tooltip.length() == 0) {
_gui_cancel_tooltip();
Expand All @@ -2078,14 +2091,12 @@ void Viewport::_gui_input_event(Ref<InputEvent> p_event) {
}

if (can_tooltip && !is_tooltip_shown) {
gui.tooltip = over;
gui.tooltip_pos = over->get_screen_transform().xform(pos); //(parent_xform * get_transform()).affine_inverse().xform(pos);
gui.tooltip_control = over;
gui.tooltip_pos = over->get_screen_transform().xform(pos);
gui.tooltip_timer = gui.tooltip_delay;
}
}

//pos = gui.focus_inv_xform.xform(pos);

mm->set_position(pos);

Control::CursorShape cursor_shape = Control::CURSOR_ARROW;
Expand Down Expand Up @@ -2436,7 +2447,7 @@ void Viewport::_gui_hide_control(Control *p_control) {
if (gui.drag_mouse_over == p_control) {
gui.drag_mouse_over = nullptr;
}
if (gui.tooltip == p_control) {
if (gui.tooltip_control == p_control) {
_gui_cancel_tooltip();
}
}
Expand All @@ -2459,8 +2470,8 @@ void Viewport::_gui_remove_control(Control *p_control) {
if (gui.drag_mouse_over == p_control) {
gui.drag_mouse_over = nullptr;
}
if (gui.tooltip == p_control) {
gui.tooltip = nullptr;
if (gui.tooltip_control == p_control) {
gui.tooltip_control = nullptr;
}
}

Expand Down Expand Up @@ -3579,14 +3590,13 @@ Viewport::Viewport() {

disable_input = false;

//window tooltip
// Window tooltip.
gui.tooltip_timer = -1;

//gui.tooltip_timer->force_parent_owned();
gui.tooltip_delay = GLOBAL_DEF("gui/timers/tooltip_delay_sec", 0.5);
ProjectSettings::get_singleton()->set_custom_property_info("gui/timers/tooltip_delay_sec", PropertyInfo(Variant::FLOAT, "gui/timers/tooltip_delay_sec", PROPERTY_HINT_RANGE, "0,5,0.01,or_greater")); // No negative numbers

gui.tooltip = nullptr;
gui.tooltip_control = nullptr;
gui.tooltip_label = nullptr;
gui.drag_preview = nullptr;
gui.drag_attempted = false;
Expand Down
4 changes: 2 additions & 2 deletions scene/main/viewport.h
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ class Viewport : public Node {
Control *mouse_over;
Control *drag_mouse_over;
Vector2 drag_mouse_over_pos;
Control *tooltip;
Control *tooltip_control;
Window *tooltip_popup;
Label *tooltip_label;
Point2 tooltip_pos;
Expand Down Expand Up @@ -382,7 +382,7 @@ class Viewport : public Node {

void _gui_remove_root_control(List<Control *>::Element *RI);

String _gui_get_tooltip(Control *p_control, const Vector2 &p_pos, Control **r_which = nullptr);
String _gui_get_tooltip(Control *p_control, const Vector2 &p_pos, Control **r_tooltip_owner = nullptr);
void _gui_cancel_tooltip();
void _gui_show_tooltip();

Expand Down