-
-
Notifications
You must be signed in to change notification settings - Fork 21k
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
Ask before closing with unsaved resources #65504
Conversation
@@ -2657,22 +2657,36 @@ void EditorNode::_menu_option_confirm(int p_option, bool p_confirmed) { | |||
case FILE_CLOSE_ALL_AND_RELOAD_CURRENT_PROJECT: { | |||
if (!p_confirmed) { | |||
tab_closing_idx = _next_unsaved_scene(false); | |||
_scene_tab_changed(tab_closing_idx); | |||
if (tab_closing_idx == -1) { | |||
tab_closing_idx = -2; // Only external resources are unsaved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could maybe use a local bool instead of using a new magic number in an unrelated index?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can't be a local bool, because the value is also used elsewhere. Re-using the variable was simpler to do and is less error-prone than adding a new variable and making sure it's used where necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But outside this scope, does it serve a purpose and is it properly handled that tab_closing_idx
stays -2
?
What I meant with a local bool would be this:
tab_closing_idx = _next_unsaved_scene(false);
bool ext_res_only = false;
if (tab_closing_idx == -1) {
ext_res_only = true;
} else {
_scene_tab_changed(tab_closing_idx);
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, here:
Lines 1958 to 1960 in 8b79a19
if (scene_idx != -1) { | |
_discard_changes(); | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's pretty smelly :(
But well, I guess this can be revisited in the future.
Thanks! |
Fixes #45612
If none of the scenes have unsaved changed, but the global history is unsaved, a unique dialog appears: