Opened 4 months ago

Closed 4 months ago

#11399 closed defect (fixed)

MOHAWK: RIVEN: Return to Launcher & Quit don’t autosave if GMM opened by F5.

Reported by: macca8 Owned by: macca8
Priority: normal Component: Engine: Mohawk
Keywords: Cc:
Game: Riven

Description (last modified by macca8)

Players can now access the GMM by either pressing F5 or the globally available Ctrl+F5.

For the most part, the GMM functions the same regardless of how it’s accessed. However, there are exceptions, and that’s when selecting Return to Launcher or Quit.

Normally, selecting RTL or Quit triggers an autosave-on-exit event, and it still does… but only if the GMM was opened by the globally available Ctrl+F5.

If the GMM is opened by pressing the game-specific F5, selecting RTL or Quit performs its normal function, but doesn’t call for an autosave.

Returning to the Launcher or quitting should always trigger an autosave. It shouldn’t matter how the GMM is accessed.

Current Daily Build: 2.2.0git4158-g7bf520a76e (30 March 2020)
Game Version: English, 5-CD (contains v1.02 patch files)
Platform: Intel Mac (OS X 10.6.8, 10.11.6)

Change History (9)

comment:1 by macca8, 4 months ago

Description: modified (diff)
Summary: MOHAWK: RIVEN: Return to Launcher doesn’t autosave if GMM opened by F5.MOHAWK: RIVEN: Return to Launcher & Quit don’t autosave if GMM opened by F5.

comment:2 by macca8, 4 months ago

Description: modified (diff)

comment:3 by macca8, 4 months ago

Unfortunately, the attempt to allow easy access to Save/Load functions for users - which I believe has already been adequately addressed through the exposure of keymaps - has only succeeded in offering a buggy alternative GMM and a rather odd looking Riven keymap (relative to the CD & DVD versions).

I seriously recommend that you reconsider the decision to rebind F5 to open the GMM, and revert it to its former binding of showing the Options Dialog, and rely on the default Ctrl+F5 for accessing the GMM.

Ctrl+F5 is the recognised default for accessing the GMM (and it’s listed in the keymaps for all to see), and it’s been the cornerstone for creating saves since Riven was added to ScummVM, so there really isn’t any value in trying to establish a Riven-specific binding to a globally available GMM.

Now that the Riven Options Menu has been added to the GMM, reverting F5 realigns the relevant Riven keymap actions to their GMM equivalents (Save, Load, Options), allowing users to bypass the GMM, if they so desire.
It also allows for removal from the Riven keymap of the bogus Open Main Menu action - which doesn’t apply to CD & DVD versions (the GMM is NOT the Riven Main Menu, it only provides support).

More importantly, accessing the GMM by its default Ctrl+F5 provides bug-free performance, whereas using F5 introduces multiple issues, in addition to the autosave issue listed above.

These include:

  • Pressing F5 during a cutscene won’t reveal the GMM, preventing the user from exiting the game. It’s also one of the known catalysts for the next three function issues when the GMM is next opened…
  • Load: screen opens with Autosave preselected and no other saves accessible.
  • Save: selecting a slot doesn’t prompt user for a name.
  • RTL: subsequently returning to the game freezes the app, requiring a Force Quit.

The Load, Save & RTL issues can also be triggered if the user uses the space bar for both actions when pausing & resuming the game.

Obviously, not all users will encounter these issues, but if they do, they must reset the GMM by either reopening the GMM using Ctrl+F5 (which defeats the purpose of using F5 in the first place) or quit.

Of course, it’s up to you what should be done, and I don’t mean to offend, but I felt the need to express my point of view.

comment:4 by bgK, 4 months ago

Owner: set to bgK
Resolution: fixed
Status: newclosed

Ok, thank you for your report. I have made the following changes based on your feedback:

  • Allowed opening the GMM using F5 during cutscenes.
  • Fixed autosaving when quitting when the GMM is opened using F5.

I very well knew I could not satisfy everyone with this change, which is why I waited until after offering the ability to rebind the key shortcuts. So, if you don't like the changes you can revert to the previous behavior by binding the options dialog to F5.

I have not been able to reproduce the other issues you mention related to the GMM. It would be nice if you could open another bug report with detailed instructions.

comment:5 by macca8, 4 months ago

Resolution: fixed
Status: closednew

In an unexpected change of behaviour, exiting during a cutscene now creates an autosave that advances the player to the end of the cutscene.

The GMM correctly shows the Save/Load buttons as disabled, so I wouldn't expect an autosave to be performed.

This behaviour is consistent, regardless of how the GMM is accessed, or if using Cmd+q.

I’m reopening this ticket to seek clarification as to whether this change is intentional or a bug, then I'll post a new report if necessary.

By the way, the issues you couldn't reproduce appear to be restricted to the 32bit build, at least on macOS. I've prepared a new bug report for that (refer #11417).

Last edited 4 months ago by macca8 (previous) (diff)

comment:6 by bgK, 4 months ago

Autosaving while quitting is indeed unreliable when the game is not interactive.. Previously the autosave would not be performed, now it happens after the video is skipped. Honestly, I think doing an autosave on quit was a mistake to begin with... I don't want to have to support something this fragile.
I'd rather warn that unsaved progress will be lost like in the 25th anniversary edition main menu. However, doing that properly requires some work in the ScummVM core systems.

comment:7 by macca8, 4 months ago

From your reply, it appears that the change was unintentional. As it is, this behaviour could deny a player the chance to view the cutscene, particularly if that player is dependent on the autosave feature. For this reason, I don't think we can afford to leave things as they are.

Having said that, I have no objection to autosave-on-exit being removed if there's no other reliable option to disable it. At worst, a player can fall back to the previous time-based autosave.

In my opinion, a warning on exit is the way to go, but I also acknowledge that it's not readily available in the ScummVM core systems. We actually discussed this briefly back in #10133 (now #10134).

With that in mind, as a potential workaround for the player, would it be possible to update ScummVM's confirmation dialog (for RTL & Quit exits) by adding 'All unsaved progress will be lost." to the existing message, which would match your 25th anniversary dialog?

The player then has the option of manually adding confirm_exit=true to the config file to automatically generate a warning on Quit & RTL. I used this method myself prior to autosave-on-exit being available and it worked quite well in all situations at that time.

Perhaps a supporting note could be added to the Riven notes in the ReadMe file to alert the player.

Thanks for the fix for #11417. I'll be keen to see the results tomorrow.

Last edited 4 months ago by macca8 (previous) (diff)

comment:8 by macca8, 4 months ago

My assessment that autosaves created by RTL were somehow leading to freezes when reloaded was incorrect. Please accept my apology if I’ve caused you unnecessary frustration in trying to resolve this matter.

Further testing revealed that the freezes were due to a variation of the known SDL 1.2 bug affecting macOS 10.9 & later, which causes a freeze after multiple switches between windowed mode & fullscreen in the same session.
The autosaves created are perfectly fine, with the minor issues all resolved by the fixes applied in this report and #11417.

With regard to the pending issue of an autosave now being created when exiting during a cutscene, I was wondering if repositioning the saveAutosaveIfEnabled() call might fix the issue?

The discussion about user warnings highlights the temporary nature of autosaving in general, and isn’t directly relevant to the subject of this report.

I’m closing this ticket in favour of #11440, to put this issue into its proper perspective.

Last edited 4 months ago by macca8 (previous) (diff)

comment:9 by macca8, 4 months ago

Owner: changed from bgK to macca8
Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.