KEMBAR78
TETRAEDGE: Do not call quitGame() when receiving a QUIT or RTL event by criezy · Pull Request #4827 · scummvm/scummvm · GitHub
Skip to content

Conversation

@criezy
Copy link
Member

@criezy criezy commented Mar 20, 2023

The only thing that quitGame() does is to send a new QUIT event, but we already have one. So this is unnecessary. Furthermore this causes an infinite loop in the case of a RTL event if the option to return to the launcher on quit is on. Indeed DefaultEventManager::pushEvent() has a check in the case of a EVENT_QUIT to ignore it if it already received one, which avoids the infinite loop, unless the EVENT_QUIT are converted to RTL events.

This fixes bug #14328 and #14329.

I checked that the games still quits properly with all known way to quit a game:

  • The in-game menu.
  • Using CMD+Q (on macOS).
  • Using the Quit ScummVM menu item in the ScummVM menu (on macOS).
  • Using the close button in the Window title bar.
  • Using the GMM.

This pull request also includes a separate commit to not show the ScummVM quit confirmation dialog if Ask for confirmation on exit is on in the ScummVM options. Indeed that option does not work with at least Syberia and Syberia 2 (and furthermore Syberia has its own confirmation dialog).

criezy added 2 commits March 20, 2023 21:27
Syberia has its own confirmation dialog. Syberia 2 does not seem
to ask for confirmation, but using the GUI confirmation does not
work anyway as if the user decides not to quit after all, we end
up with a black screen.
The only thing that quitGame() does is to send a new QUIT event, but
we already have one. So this is unnecessary. Furthermore this causes
an infinite loop in the case of a RTL event if the option to return
to the launcher on quit is on. Indeed  DefaultEventManager::pushEvent()
has a check in the case of a QUIT_EVENT to ignore it if it already
received one, which avoids the infinite loop, unless the QUIT_EVENT
are converted to RTL events.

This fixes bug #14328 and #14329.
@criezy criezy requested a review from mduggan March 20, 2023 22:03
@mduggan
Copy link
Contributor

mduggan commented Mar 20, 2023

Thanks for fixing!

@mduggan mduggan merged commit 5eed5c6 into scummvm:master Mar 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants