godotengine / godot

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

`TreeItem` tooltip is repeating the item's text by default. #97154

Open WhalesState opened 19 hours ago

WhalesState commented 19 hours ago

Tested versions

4.x

System information

Windows 10

Issue description

It's just annoying to be visible by default, no Control node in the editor is setting it's tooltip to it's name or text by default, and it's much extra work to override it for each item to be empty "", since we already can see the Item's name in the tree.

Steps to reproduce

Create any TreeItem and hover it.

https://github.com/user-attachments/assets/7926714c-5db8-4717-92f6-bc3ed7d83785

Minimal reproduction project (MRP)

N/A

WhalesState commented 19 hours ago

Bug :).

It's really annoying and redundant!

AThousandShips commented 19 hours ago

Something to discuss :) especially as changing this when it is desired breaks compatibility, just because you don't want or use it doesn't mean others won't

This should be controlled by a boolean property defaulting to true IMO to make transition acceptable

Note that this has been this way since forever, so changing something that was literally always in Godot is a pretty major disruption (and no one else seem to have complained so far)

Also if we change this we would have to change things for ItemList and TabBar as well both of which does this too

WhalesState commented 19 hours ago

Just to make it clear, this shouldn't happen, tooltip text should be empty similar to all Control nodes, It's not helping, it's repeating the Item's text most of the time.

why this should be controlled by a boolean ? if we do need a tooltip, we can use TreeItem.set_tooltip_text(column: int, tooltip: String) and we will provide a useful tooltip rather than just the same Item's text, Imagine whenever you hover any Button or LineEdit in the editor and you see a popup repeating it's text property another time!

Note that this has been this way since forever, so changing something that was literally always in Godot is a pretty major disruption (and no one else seem to have complained so far)

No one have complained doesn't mean it's useful or should be the default behavior or not a bug.

Also if we change this we would have to change things for ItemList and TabBar as well both of which does this too.

It's not used for TabBar, maybe someone have noticed that it's stupid(brilliantly unnecessary) and removed it 😄.

AThousandShips commented 19 hours ago

Since this is original to Godot ever since the beginning of open source this is very much not a bug but intentional behavior, so it'd instead be appropriate to add a property on Tree to control this, the same should be added to other classes that do this, TabBar or ItemList

It is used for TabBar (under specific conditions, but still the same behavior): https://github.com/godotengine/godot/blob/694d3c2930bdfb43fd93e1e6641f66c4f19a5c77/scene/gui/tab_bar.cpp#L326-L337

Removing something completely that is so original is massively disruptive and removing it would be breaking compatibility, regardless if you think this behavior makes sense, this is how bug classification works

Please, having to set a property to false to get the behavior you want is not unreasonable to maintain compatibility

WhalesState commented 19 hours ago

This is redundant and I have never seen any other Control node doing this, and this has nothing to do with compatibility, since you, me and everyone can still see the TreeItem's text clearly visible in the Tree without any issues.

Also by testing TabBar, this didn't happen after adding Tabs and hovering them.

AThousandShips commented 19 hours ago

For TabBar it happens when items are truncated, it happens with ItemList

But I'll make a PR to add an option to Tree to control this

WhalesState commented 19 hours ago

For TabBar it happens when items are truncated, it happens with ItemList

But I'll make a PR to add an option to Tree to control this

Thank You.

akien-mga commented 19 hours ago

Aside from bikeshedding on whether this is a bug or a feature, I would suggest researching why this behavior was implemented like this in the first place, and whether it is used anywhere to add functionality.

For example the Inspector properties are capitalized for display, but show/used to show the raw property name on hover. Maybe there's something related here?

Or maybe it's indeed an oversight we never bothered to correct.

AThousandShips commented 19 hours ago

It has gone through several cycles of updating the tooltip code so it is unlikely to have been completely overlooked but will go through some old PRs and see if there have been any thoughts on it

CC @reduz this is from pre-open source, do you know any origin for this behavior?

WhalesState commented 19 hours ago

For example the Inspector properties are capitalized for display, but show/used to show the raw property name on hover. Maybe there's something related here?

Maybe because it's used in the editor Scene dock, yet it's edited to provide an extra line showing the node's type.

So removing this should may result in editing the editor Scene dock to add both the item's text then the node type in another line, just in case it was just editing the tooltip string and not setting both together.

AThousandShips commented 19 hours ago

If we remove this instead of making it controlled by a property (I have a branch ready but waiting to open a PR until we've discussed a little more) we'd have to go through all the editor code and decide if we want to make the cases use the item text explicitly as a tooltip or not, as there are at least a few cases that don't set any tooltip where a tooltip would be lost, so we'd have to decide on how to treat those cases

WhalesState commented 18 hours ago

Just we should prevent setting the tooltip automatically.

I wonder what will happen if I change the app language in realtime, will it also translate the tooltip whenever the text is translated, ie. changed whenever the Item's text is changed?

If yes, then maybe setting the tooltip of the item will be overriden whenever the item's text is changed too.

If no, then users will either have to translate the tooltip that they didn't create, or to remove the tooltip manually everytime a new item is added.

AThousandShips commented 18 hours ago

It doesn't set the tooltip, it just fetched the item text when no tooltip is assigned, so no worries there's no problem with those things, the tooltip is not assigned automatically ever https://github.com/godotengine/godot/blob/694d3c2930bdfb43fd93e1e6641f66c4f19a5c77/scene/gui/tree.cpp#L5538-L5562

WhalesState commented 18 hours ago

So it means that if i do set every text in the game or app when translation is changed, to be updated in real time, then the Tree and the ItemList will show the same first text everytime the translation is changed.

English - default : Hello will popup Hello Changing to Arabic - أهلا - will popup Hello Changing to Russian - привет will popup Hello

AThousandShips commented 18 hours ago

No it won't, it will get the translated text, unless items aren't translated at all when displayed, as it doesn't do anything with the tooltip assigned

WhalesState commented 18 hours ago

Yes, gladly it do translate the tooltip too, just what's missing is your PR to be able to disable showing the tooltip automatically when it's left empty, Thanks in advance.

Mickeon commented 17 hours ago

This occasionally gets brought up and it seems like we should be avoiding these "useless" tooltips entirely.

The only reason these tooltip exist is to make sure the text is readable in case the text doesn't fit the item's width. But even then, a majority of items with this problem actually display tooltips that are way more meaningful on their own (think of the FileSystem dock for example). It also just negatively affects using Tree in an application built in Godot, as this behaviour does way more than what the user intends it to and it cannot be controlled.