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

[WIP] Added tooltip delay setting #27942

Closed
wants to merge 1 commit into from

Conversation

Jummit
Copy link
Contributor

@Jummit Jummit commented Apr 11, 2019

closes #27879

@Jummit Jummit requested a review from reduz as a code owner April 11, 2019 20:39
@Jummit
Copy link
Contributor Author

Jummit commented Apr 11, 2019

The only thing I have to do now is to replace the isEngine() with a check if the viewport is in the engine editor, then it is good to go.

@Jummit Jummit changed the title Added tooltip delay setting [WIP] Added tooltip delay setting Apr 11, 2019
@Jummit Jummit force-pushed the pr-tooltip-setting branch from 9120972 to c1562b3 Compare April 11, 2019 21:45
@Chaosus Chaosus added this to the 3.2 milestone Apr 15, 2019
@@ -3222,7 +3223,12 @@ Viewport::Viewport() {
gui.tooltip_timer = -1;

//gui.tooltip_timer->force_parent_owned();
gui.tooltip_delay = GLOBAL_DEF("gui/timers/tooltip_delay_sec", 0.5);
if (isEngine()) {
Copy link
Member

Choose a reason for hiding this comment

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

This method doesn't exist...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, this is a placeholder for a test if the code is run in the editor. How can I do that?

Copy link
Member

Choose a reason for hiding this comment

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

Engine::get_singleton()->is_editor_hint()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I will change that!

Copy link
Member

Choose a reason for hiding this comment

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

That code would be problematic though, as when running in the editor you would no longer define gui/timers/tooltip_delay_sec, so it won't appear in the Project Settings to set for the running game. And the call to set_custom_property_info afterwards would fail/run on an undefined value.

So you should keep this GLOBAL_DEF even when running in the editor, but you can then define the gui.tooltip_delay to the EditorSettings parameter when Engine::get_singleton()->is_editor_hint().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that sounds like a far cleaner approach.

Copy link
Contributor Author

@Jummit Jummit May 24, 2019

Choose a reason for hiding this comment

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

When I have

#ifdef TOOLS_ENABLED
if (Engine::get_singleton()->is_editor_hint()) {
	gui.tooltip_delay = EditorSettings::get_singleton()->get("interface/editor/tooltip_delay");
}
#endif

in viewport.cpp, how can I make it not crash in the project selection? There are no other instances where code is run in the project manager that uses EditorSettings.
Even if I use if Main::is_project_manager(), it still crashes:

godot/main/main.cpp

Lines 148 to 155 in 71c784f

/* Helper methods */
// Used by Mono module, should likely be registered in Engine singleton instead
// FIXME: This is also not 100% accurate, `project_manager` is only true when it was requested,
// but not if e.g. we fail to load and project and fallback to the manager.
bool Main::is_project_manager() {
return project_manager;
}

@@ -50,6 +50,7 @@
#include "scene/resources/mesh.h"
#include "scene/scene_string_names.h"
#include "servers/physics_2d_server.h"
#include "editor/editor_settings.h"
Copy link
Member

Choose a reason for hiding this comment

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

Any editor-specific code in scene should be enclosed in #ifdef TOOLS_ENABLED, otherwise the export template can't build.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, this should not be allowed at all as per #29730.

@akien-mga akien-mga removed the request for review from reduz April 30, 2019 12:27
@Calinou
Copy link
Member

Calinou commented Jul 21, 2019

@Jummit Any plans to update this pull request? 🙂

@akien-mga
Copy link
Member

Closing as it's not in a usable state and not getting updated.

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
4 participants