Page MenuHome

Close T61679: Always prompt quit when there are unsaved changes
AbandonedPublic

Authored by Jacques Lucke (JacquesLucke) on Feb 25 2019, 3:50 PM.

Diff Detail

Repository
rB Blender
Branch
always-quit-prompt (branched from master)
Build Status
Buildable 2994
Build 2994: arc lint + arc unit

Event Timeline

As I mentioned in T61679, this isn't something you can typically disable with artist-oriented apps. The only reason this was an option was that it was very cautiously added very late in Blender's lifetime. I see no reason to have this as an option for end-users.

The real egregious issue is that this dialog isn't supported when doing File > New or File > Open (!!!)

It also does not ask to save unsaved images if you forget to save image data before quitting.

This revision is now accepted and ready to land.Feb 25 2019, 4:00 PM

Agree it's fine to remove this.

As a developer it can be convenient not to ask for confirmation if you open/close Blender hundreds of times a day, but I'm not sure if anyone is using it for that reason still.

source/blender/makesdna/DNA_userdef_types.h
893

Cleared means it's cleared in the versioning code so that a future settings can reuse this flag. That was not done here, so you can leave out that comment.

Unless there are artists in the studio or long time users who can rationalize why this is useful, fine to remove it.

I'll probably setup my own key binding for force-quit, since I like this feature when checking bug reports.

Before, superusers could use bpy.ops.wm.quit_blender('EXEC_DEFAULT') to bypass the save quit dialog.
After the implementation of the new quit dialog this no longer worked, however turning off "Prompt Quit" remedied this.

Now that this also will be gone, what are our options to force quit Blender?

Good point, I guess exec should not ask for confirmation, only invoke.

  • fix flag deprecation
  • only show prompt when invoke is called

I personally open/close Blender so often that I use the feature, but as long as there is a way to make our own operator skip the dialog via invoke then it's fine.

Also got very used to just do Ctrl+Q > D, since the keys are very close. If devs are good with it then +1!

Note that there are 3 people who like to have the ability to quit quickly, I'm still OK to remove it but we don't _have_ to remove if enough people find it useful.

Editing the keymap gets you some of the way there, but still doesn't work if the window manager asks the window to close - so it's not exactly a replacement.

We could provide a Force Quit operator that is hard to trigger by accident except when someone created a keymap entry for it. The normal Quit operator would always show the dialog then.

This way we can also get rid of the "hacks" that people seem to have developed: bpy.ops.wm.quit_blender('EXEC_DEFAULT').

Even if we have a non-hacky code-path for force quit, it could still be a preference.

Bonus points if it doesn't save quit.blend, was testing with very large files recently and quitting was making my computer hang, eg - if you have >2gb file open w/ gzip compression.

Thinking there may be reasons a quit dialog isn't useful, when using Blender as an interactive tool for eg you might not want it (admittedly we don't have interactive mode yet, just saying).

Patch which keeps the preference but removes the confirmation menu:

1diff --git a/release/scripts/startup/bl_ui/space_topbar.py b/release/scripts/startup/bl_ui/space_topbar.py
2index 937cec9eb29..f0ff5e9a29d 100644
3--- a/release/scripts/startup/bl_ui/space_topbar.py
4+++ b/release/scripts/startup/bl_ui/space_topbar.py
5@@ -621,7 +621,7 @@ class TOPBAR_MT_file(Menu):
6 layout.separator()
7
8 layout.operator_context = 'EXEC_AREA'
9- if bpy.data.is_dirty and context.preferences.view.use_quit_dialog:
10+ if bpy.data.is_dirty:
11 layout.operator_context = 'INVOKE_SCREEN' # quit dialog
12 layout.operator("wm.quit_blender", text="Quit", icon='QUIT')
13
14diff --git a/release/scripts/startup/bl_ui/space_userpref.py b/release/scripts/startup/bl_ui/space_userpref.py
15index 58582cacc24..f6373a4b970 100644
16--- a/release/scripts/startup/bl_ui/space_userpref.py
17+++ b/release/scripts/startup/bl_ui/space_userpref.py
18@@ -1262,7 +1262,7 @@ class USERPREF_PT_saveload_blend(PreferencePanel):
19 flow.prop(paths, "use_load_ui")
20 flow.prop(paths, "use_save_preview_images")
21 flow.prop(paths, "use_tabs_as_spaces")
22- flow.prop(view, "use_quit_dialog")
23+ flow.prop(view, "use_save_prompt")
24
25 layout.separator()
26
27diff --git a/source/blender/blenkernel/intern/blender.c b/source/blender/blenkernel/intern/blender.c
28index 2aa585b8080..a5c80ab05a0 100644
29--- a/source/blender/blenkernel/intern/blender.c
30+++ b/source/blender/blenkernel/intern/blender.c
31@@ -305,7 +305,7 @@ void BKE_blender_userdef_app_template_data_swap(UserDef *userdef_a, UserDef *use
32 DATA_SWAP(app_flag);
33
34 /* We could add others. */
35- FLAG_SWAP(uiflag, int, USER_QUIT_PROMPT);
36+ FLAG_SWAP(uiflag, int, USER_SAVE_PROMPT);
37
38 #undef SWAP_TYPELESS
39 #undef DATA_SWAP
40diff --git a/source/blender/makesdna/DNA_userdef_types.h b/source/blender/makesdna/DNA_userdef_types.h
41index d80c0683c08..932377fa718 100644
42--- a/source/blender/makesdna/DNA_userdef_types.h
43+++ b/source/blender/makesdna/DNA_userdef_types.h
44@@ -890,7 +890,7 @@ typedef enum eUserpref_UI_Flag {
45 USER_SPLASH_DISABLE = (1 << 27),
46 USER_HIDE_RECENT = (1 << 28),
47 USER_SHOW_THUMBNAILS = (1 << 29),
48- USER_QUIT_PROMPT = (1 << 30),
49+ USER_SAVE_PROMPT = (1 << 30),
50 USER_HIDE_SYSTEM_BOOKMARKS = (1u << 31),
51 } eUserpref_UI_Flag;
52
53diff --git a/source/blender/makesrna/intern/rna_userdef.c b/source/blender/makesrna/intern/rna_userdef.c
54index b12852220a2..cf79e4eec4b 100644
55--- a/source/blender/makesrna/intern/rna_userdef.c
56+++ b/source/blender/makesrna/intern/rna_userdef.c
57@@ -3843,9 +3843,9 @@ static void rna_def_userdef_view(BlenderRNA *brna)
58 RNA_def_property_ui_text(prop, "Confirm Threshold",
59 "Distance threshold after which selection is made (zero to disable)");
60
61- prop = RNA_def_property(srna, "use_quit_dialog", PROP_BOOLEAN, PROP_NONE);
62- RNA_def_property_boolean_sdna(prop, NULL, "uiflag", USER_QUIT_PROMPT);
63- RNA_def_property_ui_text(prop, "Prompt Quit",
64+ prop = RNA_def_property(srna, "use_save_prompt", PROP_BOOLEAN, PROP_NONE);
65+ RNA_def_property_boolean_sdna(prop, NULL, "uiflag", USER_SAVE_PROMPT);
66+ RNA_def_property_ui_text(prop, "Save Prompt",
67 "Ask for confirmation when quitting with unsaved changes");
68
69 prop = RNA_def_property(srna, "show_column_layout", PROP_BOOLEAN, PROP_NONE);
70diff --git a/source/blender/windowmanager/intern/wm_operators.c b/source/blender/windowmanager/intern/wm_operators.c
71index 3413b7ebab1..bc1dc0eb9ec 100644
72--- a/source/blender/windowmanager/intern/wm_operators.c
73+++ b/source/blender/windowmanager/intern/wm_operators.c
74@@ -1804,18 +1804,19 @@ static void WM_OT_window_fullscreen_toggle(wmOperatorType *ot)
75
76 static int wm_exit_blender_exec(bContext *C, wmOperator *UNUSED(op))
77 {
78- wm_quit_with_optional_confirmation_prompt(C, CTX_wm_window(C));
79+ wm_exit_schedule_delayed(C);
80 return OPERATOR_FINISHED;
81 }
82
83-static int wm_exit_blender_invoke(bContext *C, wmOperator *op, const wmEvent *event)
84+static int wm_exit_blender_invoke(bContext *C, wmOperator *UNUSED(op), const wmEvent *UNUSED(event))
85 {
86- if (U.uiflag & USER_QUIT_PROMPT) {
87- return wm_exit_blender_exec(C, op);
88+ if (U.uiflag & USER_SAVE_PROMPT) {
89+ wm_quit_with_optional_confirmation_prompt(C, CTX_wm_window(C));
90 }
91 else {
92- return WM_operator_confirm(C, op, event);
93+ wm_exit_schedule_delayed(C);
94 }
95+ return OPERATOR_FINISHED;
96 }
97
98 static void WM_OT_quit_blender(wmOperatorType *ot)
99diff --git a/source/blender/windowmanager/intern/wm_window.c b/source/blender/windowmanager/intern/wm_window.c
100index c432dce0757..613f5c58f85 100644
101--- a/source/blender/windowmanager/intern/wm_window.c
102+++ b/source/blender/windowmanager/intern/wm_window.c
103@@ -473,8 +473,13 @@ void wm_quit_with_optional_confirmation_prompt(bContext *C, wmWindow *win)
104 * here (this function gets called outside of normal event handling loop). */
105 CTX_wm_window_set(C, win);
106
107- if ((U.uiflag & USER_QUIT_PROMPT) && !wm->file_saved && !G.background) {
108- wm_confirm_quit(C);
109+ if (U.uiflag & USER_SAVE_PROMPT) {
110+ if (!wm->file_saved && !G.background) {
111+ wm_confirm_quit(C);
112+ }
113+ else {
114+ wm_exit_schedule_delayed(C);
115+ }
116 }
117 else {
118 wm_exit_schedule_delayed(C);

I don't really mind strongly either way, other than the fact that this seems like adding clutter in the code and also the Preferences for an option that artists won't use. We really have a lot of Preferences in Blender, and this seems like one of those things that was made an option just because it was added a later time in development,

We could put all these kinds of things in a Developer section, or could accept the rather neat solution of only showing prompt when invoke is called. This solution avoids the need for a preference.

If we really must keep it, we should probably rename it from Quit Prompt to Save Prompt. It's not a prompt for quitting but a prompt for saving unsaved changes. It should also appear when performing Open or New File commands, which also illustrates that this is not really a Quit Prompt at all.

The real urgent & egregious issue is the File->New and File->Open which misses this save prompt completely.

We could put all these kinds of things in a Developer section, or could accept the rather neat solution of only showing prompt when invoke is called. This solution avoids the need for a preference.

Closing the window via the window-manager will still show the prompt.

Agree reducing unnecessary options in the preferences is worth doing where possible. The clutter in the code is minimal so I wouldn't worry about that in this case. (many other worse offenders in this respect)

@Campbell Barton (campbellbarton) If some developers need to quit quickly and never have real work to save, they could simply use the operator without invoke. Currently there's already a difference between closing the window and doing File > Quit. One uses a system dialog, and the other uses Blender's built-in dialog.

This particular issue is not a big deal either way, but there's a chance it could help a bit with focusing on improving the save dialog if we double down on having one system, rather than the current mishmash of dialogs that appear only on some OS's, and the preferences to use alternative dialogs.

...for an option that artists won't use

I say it from an artist point of view. Opening and closing Blender all the time to try things out in library files, characters, props, which I check and study but don't plan to actually save. Maybe casual artist don't use it but those working in real production do this all the time (the lack of an asset manager and overrides is part of the reason though).

We could put all these kinds of things in a Developer section

It's not developer only.

If we really must keep it, we should probably rename it from Quit Prompt to Save Prompt

+1 . This way the dialog can be re-used when creating/opening a new file as well.

A "Force Quit" operator would still be useful though (if the 'exec_default' feels too hacky). One of the ways we use is when sending files to render, we have a "Submit & Quit" to automatically close Blender after the file is sent to the farm, I can see other type of add-ons/farms using a similar workflow.

@Pablo Vazquez (pablovazquez) This patch essentially already has a force quit option. If you add this to the keymap without invoke, there will be no dialog.

I will defer to others on whether to keep this as a separate preference option, or to do it the way this patch suggests. It all seems to be a bit of bikeshedding when compared the real serious issue of it not appearing for File > New and File > Open.

@Campbell Barton (campbellbarton) If some developers need to quit quickly and never have real work to save, they could simply use the operator without invoke.

Again, this doesn't work when closing from the window-manager (I don't use Blenders's Ctrl-Q to quit).


+1 to rename to "Save Prompt", propose this patch P918.