Opened 3 years ago

Closed 16 months ago

Last modified 16 months ago

#12363 closed defect (fixed)

MYST3: Manually overwriting Autosave file disables autosaving.

Reported by: macca8 Owned by: bluegr
Priority: normal Component: Engine: Myst3
Version: Keywords:
Cc: Game: Myst 3: Exile

Description (last modified by macca8)

The Myst3 save system, supported by its own save dialog and dedicated prompt-to-save feature, has always allowed the option of manually overwriting the Autosave file when prompted by the game, in addition to maintaining support for scheduled autosaving.

However, commit https://github.com/residualvm/residualvm/commit/b333691cc2 - Implement the new autosave system - changed the way in which the Autosave file was being monitored in Myst3, by adding an isAutosave flag to every save file’s metadata. Previously, the file’s identity was established exclusively by testing its name.

The value of this flag is implemented by adding an isAutosave parameter to the Myst3Engine’s saveGameState(), which is passed directly to GameState::Save() for updating the file’s metadata.

The issue here is that the Autosave file’s isAutosave flag is changed to false when the user initiates a manual overwrite of the file, because all manual saves (including to autosave) are passed an isAutosave parameter of false.

For scheduled autosaves, the autosave system determines the status of the file occupying the designated autosave slot by checking the value of its isAutosave flag (as _saveType), and only falls back to a name test if the flag hasn’t been set (refer SaveStateDescriptor::isAutosave() in /engines/savestate.cpp).

This test currently fails if the user has initiated a manual overwrite of the Autosave file, because the autosave system incorrectly assumes that the autosave slot is occupied by a user created save file, which prevents the scheduled autosave from occurring.

What’s needed is an appropriate name test to be included in /engines/myst3/myst3.cpp/Myst3Engine::saveGameState(desc, thumbnail, isAutosave) to reset the isAutosave variable to true, before it’s passed on to _state->save().

I’d suggest something similar to the following (it’s only needed if the parameter is false), placing it immediately after the removal of any extension from saveName (& prior to the assignment of the fileName variable) ...

if !isAutosave && saveName.equalsIgnoreCase(_(“Autosave”)) {
	isAutosave = true;
}

With regard to the name test comparison, I’m not sure if case is an issue or not, because all save names (including AUTOSAVE) appear in uppercase in the Myst3 Save dialog, and all save file names (except Autosave.m3s) are also in uppercase. Alternatively, if case isn't considered an issue, then the name test could be simplified to: saveName == _("Autosave").

Current daily build: 2.3.0git14891-gb5804e1257 (Apr 3 2021)
Platform: macOS (10.6.8 & 10.11.6)
Game Version: 4-CD English

Change History (9)

comment:1 by macca8, 3 years ago

Description: modified (diff)

comment:2 by macca8, 3 years ago

Description: modified (diff)

comment:3 by macca8, 3 years ago

Description: modified (diff)

comment:4 by sev-, 3 years ago

Priority: highnormal

comment:5 by macca8, 3 years ago

Owner: set to macca8
Resolution: fixed
Status: newclosed

The changes made to saveAutosaveIfEnabled() in PR3261 have satisfactorily resolved this issue without requiring user intervention.
Thank you.

comment:6 by macca8, 17 months ago

Resolution: fixed
Status: closednew

This bug has been reinstated in Myst III following the change in 9b05c206, which reverted the autosave test in saveAutosaveIfEnabled() from hasAutosaveName() back to isAutosave().
Also applies to any other game which has write access in the Autosave slot, and which sets the _saveType value based on the autosave flag (in my case, this also includes Riven).

Currently, if a game specifically sets the _saveType value, there's no way to access the name test from within isAutosave(), and so in this case, the autosave file is incorrectly detected as a regular save (and triggers the in-game warning dialog), whereas previously, the same file was correctly detected as the autosave file, by using the name test exclusively.

I have proposed a solution in #13842 which resolves this issue, by ensuring that if the autosave file fails either test (name or saveType), then the alternate test is also considered, thus avoiding what is in reality a false positive.

I'm reopening this ticket, pending one of the team implementing #13842 or similar.

comment:7 by macca8, 16 months ago

After a few false starts trying to create the PR, I've taken the punt and submitted PR4350 in the hope of resolving this before the 2.6.1 release.

comment:8 by bluegr, 16 months ago

Owner: changed from macca8 to bluegr
Resolution: fixed
Status: newclosed

PR4350 has been merged, so this can be closed now

comment:9 by macca8, 16 months ago

Just a heads up for anyone encountering this issue.
This fix missed the cut for the 2.6.1 maintenance release, so you can either fall back to the 2.6.0 release (which predates the bug), or try any of the latest 2.7.0git master builds.

Note: See TracTickets for help on using tickets.