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

Remove error messages caused by Save on Focus feature #91555

Merged
merged 1 commit into from May 8, 2024

Conversation

AlexanderFarkas
Copy link
Contributor

@AlexanderFarkas AlexanderFarkas commented May 4, 2024

Fixes #73765

To clarify: fixing it that way covers most of the problems caused by the underlying issue (showing dialogs on the root Window during notification processing).

Another issue could be created to track the underlying problem.

editor/editor_node.cpp Outdated Show resolved Hide resolved
editor/editor_node.cpp Outdated Show resolved Hide resolved
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, it works as expected (Windows 11). I no longer get errors when Save on Focus Loss kicks in.

The downside of this approach is that if saving takes a long time (e.g. due to a huge binary resource being serialized as text in a scene file), the editor will appear to freeze for a few seconds if you alt-tab back in quickly. Still, I consider this is preferable to displaying an error every time the editor loses focus.

Eventually, the scene saving UX should be reworked to use a visually non-blocking method that doesn't need to spawn a dialog. This will alleviate the issue entirely while being less intrusive when you press Ctrl + S or when saving before running a scene.

@AlexanderFarkas
Copy link
Contributor Author

AlexanderFarkas commented May 8, 2024

Tested locally, it works as expected (Windows 11). I no longer get errors when Save on Focus Loss kicks in.

The downside of this approach is that if saving takes a long time (e.g. due to a huge binary resource being serialized as text in a scene file), the editor will appear to freeze for a few seconds if you alt-tab back in quickly. Still, I consider this is preferable to displaying an error every time the editor loses focus.

Eventually, the scene saving UX should be reworked to use a visually non-blocking method that doesn't need to spawn a dialog. This will alleviate the issue entirely while being less intrusive when you press Ctrl + S or when saving before running a scene.

For me the editor freezes even without changes in this PR. It could be especially noticeable when you alt-tab between the editor and the running game. It's probably not related, since alt-tab'ing to the running game always lags for a couple of seconds (even if I don't alt-tab from the editor).

@akien-mga
Copy link
Member

Looks good. Could you amend the commit message (with git commit --amend, then git push --force to update the PR) to use a more explicit message?

E.g.:

Remove error messages caused by Save on Focus feature

Fixes #73765.

@akien-mga akien-mga merged commit b8255b1 into godotengine:master May 8, 2024
15 checks passed
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

@akien-mga akien-mga added the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label May 8, 2024
@AlexanderFarkas
Copy link
Contributor Author

Awesome!
Thank you for the guidance you have provided me with.
Hope to make another PR soon (at least as a draft) :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release topic:editor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Parent node is busy setting up children" on focus loss when save_on_focus_loss option is enabled
3 participants