Fix ProgressDialog errors during first scan.#108893
Fix ProgressDialog errors during first scan.#108893Repiteo merged 1 commit intogodotengine:masterfrom
ProgressDialog errors during first scan.#108893Conversation
editor/gui/progress_dialog.cpp
Outdated
| Window *current_window = SceneTree::get_singleton()->get_root()->get_last_exclusive_window(); | ||
| ERR_FAIL_NULL(current_window); | ||
| reparent(current_window); | ||
| callable_mp((Control *)this, &Control::reparent).call_deferred(current_window, true); |
There was a problem hiding this comment.
Can't use reparent during first scan.
There was a problem hiding this comment.
I was wondering whether it's worth deferring all the reparent and show calls just to fix an issue that only affects the first scan. These calls work fine with all other tasks.
There was a problem hiding this comment.
If the issue is that current_window is setting up children at the time, you can use is_ready() to check if you can reparent immediately.
editor/editor_node.cpp
Outdated
| command_palette->register_shortcuts_as_command(); | ||
|
|
||
| callable_mp(this, &EditorNode::_begin_first_scan).call_deferred(); | ||
| EditorNode::_begin_first_scan(); |
There was a problem hiding this comment.
First scan should be called as soon as possible after entering tree, delaying it causes issues when flushing the MessageQueue.
There was a problem hiding this comment.
Note: This line was added here #64610, so i will test with editor startup benchmark to make sure it doesn't cause any further issues.
Edit:
Benchmarks still work without any issues.
ProgressDialog error during first scan.
ProgressDialog error during first scan.ProgressDialog errors during first scan.
14ee9bd to
8c3c3cf
Compare
I don't think it's safe to assume that for both issues, but indeed, this is what triggers the error. Specifically, editor checks for Lines 5621 to 5623 in e0603ae And then here for cmdline_mode:Lines 5357 to 5359 in e0603ae This inconsistency is what's causing the errors, but not sure if this is the only way to trigger them. |
editor/gui/progress_dialog.cpp
Outdated
| current_window->set_disable_input(window_is_input_disabled); | ||
|
|
||
| show(); | ||
| callable_mp((CanvasItem *)this, &CanvasItem::show).call_deferred(); |
There was a problem hiding this comment.
To avoid showing the dialog before reparent, is it ok to make it visible first then changing its parent ? I just have tried to avoid any further issues that may happen because of this.
previously the reparent happened before showing the dialog and i tried to keep it the same as before.
There was a problem hiding this comment.
Showing it earlier shouldn't cause problems, but deferring makes sense.
It would be nice if the 2 deferred calls were done together, either just moved to be one after another, or as part of a separate method (like _reparent_and_show() which would then be deferred instead).
I have found that the main issue was because first scan is deferred then |
KoBeWi
left a comment
There was a problem hiding this comment.
Some code comments, but otherwise works correctly.
Co-authored-by: Tomasz Chabora <kobewi4e@gmail.com>
8c3c3cf to
1cc459d
Compare
Applied the changes from the comments and rebased, Thank you. |
|
Thanks! |
|
This PR seems to have broken headless imports: #108990. I'd guess headless exports as well, but I haven't checked. |
ERROR: editor/progress_dialog.cpp:169 - Do not use progress dialog (task) while flushing the message queue or using call_deferred()!#100900Note: On Windows, minimizing the window while loading the project no longer throw errors, other issues can't be reproduced since users have reported that it happens randomly and I think they have minimized the window during loading without noticing it was the main reason.