#8194 closed patch
Quit Confirmation Dialog (feature request #642721)
| Reported by: | Deledrius | Owned by: | fingolfin |
|---|---|---|---|
| Priority: | normal | Component: | GUI |
| Version: | Keywords: | ||
| Cc: | Game: |
Description
Here's a patch to add an optional exit confirmation dialog (off by default). This is my first time using this system, so I hope I performed and upload the diff correctly.
Ticket imported from: #715991. Ticket imported from: patches/299.
Attachments (3)
Change History (15)
by , 21 years ago
| Attachment: | confirm.diff added |
|---|
comment:1 by , 21 years ago
Ignore the first one. It had a minor typo in it, and I don't seem to have permission to remove my own submission.
comment:2 by , 21 years ago
Please submit patches in th unified diff format ("-u" option).
This patch will have to be resubmitted, in theunnified format and against latest CVS. The problem is that it won't apply cleanly; worse since it contains no context information, patch will write it into semi- random places instead of erroring out.
I don't think a command line option ("-i" in the patch) makes much sense here. IMHO it's sufficient if this is in the config file (and in the future in the in-game prefs dialog).
Thank you!
comment:3 by , 21 years ago
| Owner: | set to |
|---|
comment:4 by , 21 years ago
Okay, here it is with -u against today's CVS. The version of WinCVS I was using didn't have the option before, but I've since updated.
I've also removed the -i command line code. I wasn't sure if it was needed before, and only provided it in the name of completeness. Given the current shortage of letters, I agree it's not very useful.
I hope this diff is properly usable now. I also cleaned up a place or two in the code I submitted before which deviated slightly from the coding standards.
by , 21 years ago
| Attachment: | quitdialog.diff added |
|---|
Patch against 2003-05-03 CVS to add Exit Confirmation
comment:5 by , 21 years ago
Some problem with that patch:
* there is no need for your custom getResult methon in the dialog. We already implement such a feature, look at Diaog::setResult() and the return value of runModal()
* for consistency with the rest of the code, _confirm_on_exit should be renamed to something like _confirmExit
* don't call shutDown. Nor are TODO's refering to shutDown appropriate.
* you modified the SDL backend... and you map the Mac Cmd-Q to Alt-X. Well, OK, but the code only seems to handle Ctrl-Z for quit, so that doesn't do anything. My suggestion: handle Alt-X not Ctrl-z in scummvm.cpp; and also modify the code in sdl-common.cpp to always "fake" an alt-x keyboard even, even for Ctrl-z. Of course the proper solution on the long run might be a new event like "quit requested event"
*
comment:6 by , 21 years ago
| Status: | new → pending |
|---|
comment:8 by , 21 years ago
| Status: | pending → new |
|---|
comment:9 by , 21 years ago
Here ya go. Hope it's all done right this time. Sorry about the delay.
* Fixed this. Didn't see the return value from runDialog and runModal.
* Fixed this. There seemed to be a few inconsistencies that confused the matter when I was deciding what to use.
* Didn't see what was wrong with this, but consider it fixed.
Since there's now a quit event and shutdown works, it's
moot. That makes things easier, too...
* Didn't even need to touch this now. Since all quit-request key-combos return a quit event to the game loop, I no longer had to fudge one in. :-)
BTW, I never had a problem with ctrl-z not working on mine.
Maybe I missed something with the version I diffed. Oh well,
I know *this* one works. Let me know if you have any more
comments on this.
by , 21 years ago
| Attachment: | quitdialog.2.diff added |
|---|
Patch against 2003-07-21 CVS Snapshot to add Exit Confirmation
comment:10 by , 21 years ago
Yes, this looks much much better. Not sure if I can get this in before 0.5.0, but once that is release we can get it in.
Some things that I still think could be tweaked (but which I can also do once it's checked in):
* use 0 and 1 for setResult, not 'y' and 'n' -> simplifies code a little, and is "language agnostic (i.e. maybe one day we want to localize that dialog to non-english languages)
* why not use a 'real' dialog with a "yes" and a "no" button". Buttons can have keyboard shortcuts, after all ('y' and 'n' in this case). The setResult call then would be inside a handleCommand method
Again, those are more cosmetical changes, and can be done later :-)
comment:11 by , 21 years ago
| Status: | new → closed |
|---|
comment:12 by , 5 years ago
| Component: | → GUI |
|---|
Patch against 2003-04-05 CVS to add Exit Confirmation