Page MenuHome

Confirm Discard Changes [WIP]
ClosedPublic

Authored by Jacques Lucke (JacquesLucke) on May 8 2019, 4:44 PM.

Details

Summary

I'm still not sure about the best architecture for
multi-step user interactions within the current
operator system. I've tried wrapping the operator
with another, but that becomes quite complicated.

The best option so far seems to be to use a finite
state machine within the operator. For that I use
an additional property that stores the current state.
The operator might then call itself in different states.
Generally, this approach is quite scalable, when more
states are needed. There is a bit of overhead compared
to normal operators, but most things could be abstracted
away behind a generic API.

Note: I intentionally don't use the invoke method anymore,
because having two entry points makes it more complicated.

I also made display_file_selector and display_discard_warning
explicit properties of the operator. That means that
the caller has to set those if non-default behavior is
needed. Personally, I prefer this approach a lot over
having to "hack" certain behaviors by setting the
operator context to e.g. INVOKE_DEFAULT in Python.

Any comments on the approach?
As I said, I'm still not sure if there are better
ways to do this currently.

Diff Detail

Repository
rB Blender

Event Timeline

Brecht Van Lommel (brecht) requested changes to this revision.May 10 2019, 4:50 PM

It's still important for invoke and exec to both be available, even if there is additional control. exec must not do user interaction, we rely on that. If it has to do here when some hidden state property is set that's ok. But with default properties exec and invoke should do the typical thing I think.

That state things is weird, but I don't have good ideas for it besides making changes to the operator system.

source/blender/windowmanager/intern/wm_files.c
2079

Should also have PROP_HIDDEN.

2272

The default for this should be true.

This revision now requires changes to proceed.May 10 2019, 4:50 PM
  • minor updates to address code comments

I'll check how I can change it so that invoke and exec still work as before.
Where do we rely on exec not doing any user interaction? The limitation makes sense, I'm just not sure where we rely on it. It seems to work well. Should there be an assert when exec does not finish or cancel or pass through?

When exec is not allowed to do user interaction, we probably have to call invoke more than once or use modal somehow. In any case, the file selector and confirm dialog have to be updated, so that they don't always call exec.

  • support calling invoke from confirm dialog
  • improve naming
  • fix operator contexts
  • make operator state dispatch generic
  • remove "display_discard_warning"

This brings back the old behavior when just exec is used.
There is one limitation (which is not a problem here):
Currently the file selector always calls exec.
That means that there cannot be user-interaction-steps
afterwards. Fortunately, we don't need that here.

I'll check how I can change it so that invoke and exec still work as before.
Where do we rely on exec not doing any user interaction? The limitation makes sense, I'm just not sure where we rely on it. It seems to work well. Should there be an assert when exec does not finish or cancel or pass through?

The automated tests and render farm scripts assume exec has no user interaction. There could be an assert when exec returns running modal.

Seems fine given the current limitations of the operator API.

This revision is now accepted and ready to land.May 13 2019, 7:22 PM

Good, should I commit this in the current state?

Possible improvements that could be made before (or after) the commit:

  1. Use a blocking dialog similar to the confirm quit dialog.
  2. Move the generic state management somewhere else.

Blocking dialog
I talked about that with @William Reynish (billreynish), yesterday. We think it would be good to have Cancel, Don't Save and Save fields. When the file has not been saved before, a file browser will open to select the save location. In that case, the new file won't be opened afterwards. The user would have to restart the file-opening-procedure. This is because it could be very confusing (and currently technically impossible) to open two separate file browsers with the same operator (as part of a multi-step-interaction).

One issue I'm not sure how to solve yet is how to forward the operator settings. I think I'd have to make a copy of the IDProperties * and pass it as argument to the blocking dialog. The problem is that I don't see how this would be freed afterwards.

State management
If we need this mainly comes down to two questions:

  • Do we expect that more operators need similar multi-step-interactions in the future?
  • Do we expect that the operator system will be updated before we need this functionality in more places? Note: I'm don't know how the ideal operator system would look like to support this functionality natively.

Here's how the dialog could look:

Basically it should be identical with the one you see when you quit.

For the images thing, we could add a string to clarify that it also affects X amount of image files, if that is the case:

Here I have removed the icons in the buttons and made the 'active' button more prominent, but those are just stylistic changes. The important part is that it should be consistent with the Save Prompt that appears when quitting.

Edit: even more terse version:

I think it's fine to commit in this state and continue improving.

For the images there could be a checkbox, so you can choose to save them or not.

A version with a checkbox for saving images:

Personally I have a slight preference for this to just be a preference in the Save & Load Preferences, and that the dialog just tells users what will happen. Saving the images is almost always what you'd want anyway, and this way the text string can be more informative about the number of images to be saved, while keeping the dialog as simple as possible.

So, this very same dialog will be the one that will appear when you do File->New, or when opening a blend file on top of an unsaved project, right?
If not, why not? This seems to be the perfect dialog for that. The current "ok dialog" is not efficient and it's very easy to miss.