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

Add a tooltip delay setting for editor #35806

Closed

Conversation

Fermats-Fish
Copy link

@Fermats-Fish Fermats-Fish commented Jan 31, 2020

Resolves #27879 and resolves #9030.

Adds an editor setting which changes the tooltip delay for tooltips inside the editor.

A restart is required for this change to be applied, however the code could be extended so that this is not required.

This is my first time contributing to a major open source project, and my first time using C++, so let me know if I should have done anything differently.

@Fermats-Fish Fermats-Fish force-pushed the tooltip-delay-setting branch 2 times, most recently from 4ec9680 to 17705d4 Compare January 31, 2020 23:47
@YeldhamDev
Copy link
Member

Would be nice to have this as an exposed property in Viewport for developers.

@Fermats-Fish
Copy link
Author

@YeldhamDev Do you mean using ADD_PROPERTY instead of gui.tooltip_delay so that game developers can change the tooltip delay for each viewport object?

@YeldhamDev
Copy link
Member

@Fermats-Fish Nevermind, completely forgot about the project setting.

@Fermats-Fish
Copy link
Author

@YeldhamDev No problem.

@groud
Copy link
Member

groud commented Feb 1, 2020

Honestly I am not sure this is worth it. It's yet another almost pointless editor settings...
Making it configurable for games makes sense though.

@Fermats-Fish
Copy link
Author

@groud At the very least it makes sense to make it so that changing the in game tooltip delay via project settings does not change the tooltip delay in the editor. I could always modify this code to just fix that issue without adding the editor setting.

@groud
Copy link
Member

groud commented Feb 1, 2020

Sure ! My single point was about the editor settings, but the rest of the change look fine to me :)

Comment on lines 3318 to 3319
float Viewport::editor_tooltip_delay = 0.5f;
float Viewport::regular_tooltip_delay = 0.5f;
Copy link
Member

Choose a reason for hiding this comment

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

Since the master branch now accepts C++17 code, you can initialize those variables directly in the header file.

Also, is static needed here? We only make variables static if we absolutely need it.

@@ -305,7 +308,7 @@ class Viewport : public Node {
Variant drag_data;
Control *drag_preview;
float tooltip_timer;
float tooltip_delay;
float *tooltip_delay;
Copy link
Member

Choose a reason for hiding this comment

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

Out of curiosity, why was this * added?

@Calinou Calinou added this to the 4.0 milestone Jan 5, 2021
@YuriSizov YuriSizov requested a review from a team September 1, 2021 20:35
@Mickeon
Copy link
Contributor

Mickeon commented Nov 15, 2022

I was looking for this setting and I was astounded I could not find it. This may be salvaged, probably...

@akien-mga akien-mga modified the milestones: 4.0, 4.x Jan 19, 2023
@Calinou Calinou mentioned this pull request Jul 31, 2023
75 tasks
@rsubtil
Copy link
Contributor

rsubtil commented Sep 25, 2023

+1 for salvaging; we want to have near instant (<0.05s) tooltips in our project, but we found out this property propagates to the editor, and with very low times it makes mouse scroll extremely finicky and annoying.

Is there any suggested workflow to salvage PRs? I assume I'd need to open another PR, would marking the current PR owner as Co-authored-by: be a good practice?

@Calinou
Copy link
Member

Calinou commented Sep 25, 2023

Is there any suggested workflow to salvage PRs? I assume I'd need to open another PR, would marking the current PR owner as Co-authored-by: be a good practice?

Sure 🙂 This is what I do personally.

@Fermats-Fish
Copy link
Author

Hi everyone. If the community is interested in getting this feature merged now, I'm happy to have a go at fixing up my commit :).

Resolves godotengine#27879 and godotengine#9030.

Adds an editor setting which changes the tooltip delay for tooltips
inside the editor.

A restart is required for this change to be applied, however the
code could be extended so that this is not required.
@Fermats-Fish Fermats-Fish force-pushed the tooltip-delay-setting branch from 17705d4 to 4923b81 Compare October 14, 2023 11:30
@Fermats-Fish Fermats-Fish requested a review from a team as a code owner October 14, 2023 11:30
@Fermats-Fish
Copy link
Author

Finally got some time to have a go at this.

I've fixed the merge conflicts, changed to the new system for creating editor settings, fixed the unnecessary pointer, and simplified the code by removing an unnecessary static variable.

The code still requires a restart every time you change the editor tooltip delay however. When I next get some time I can try to figure out a way to resolve this.

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Tested locally (rebased on top of master 9050ee1), there's an issue with how gui/timers/tooltip_delay_sec is registered. Also, the editor setting doesn't have any affect even after restarting the editor (I tried a tooltip delay of 0.05 and tooltips still take half a second to appear).

Since GLOBAL_DEF() is only reached when not running in the editor, this means the project setting is never registered in the editor, so you can't edit its value. It also won't appear in the generated class reference.

The GLOBAL_DEF() line should be moved to this location:

GLOBAL_DEF(PropertyInfo(Variant::INT, "gui/timers/incremental_search_max_interval_msec", PROPERTY_HINT_RANGE, "0,10000,1,or_greater"), 2000);

After doing this, replace the call to GLOBAL_DEF() in viewport.cpp with GLOBAL_GET().

@AThousandShips AThousandShips changed the title Added tooltip delay setting for editor Add a tooltip delay setting for editor Feb 13, 2024
@akien-mga
Copy link
Member

Superseded by #85678. Thanks for the contribution!

@akien-mga akien-mga closed this Apr 26, 2024
@akien-mga akien-mga removed this from the 4.x milestone Apr 26, 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.

Allow changing of Editor tooltip delay Changing the tooltip delay in project settings affects the editor
9 participants