Opened 3 years ago

Closed 2 years ago

#12766 closed defect (fixed)

SKY: Savegame 0 produces write permission error on Android

Reported by: puetsch Owned by: antoniou79
Priority: high Component: Engine: Sky
Version: Keywords:
Cc: antoniou79 Game: Beneath a Steel Sky

Description

On Android, I'm getting "Failed to save game (Write permission denied)" in BASS when trying to save to game slot 0. Other game slots work fine.

ScummVM 2.2.1pre
Android 10

Change History (11)

comment:1 by antoniou79, 3 years ago

This should not be specific to the Android port. It applies to ScummVM 2.2.0 and above.

This is an intentional fix to a situation that was causing a segmentation fault. The relevant commits are these:

https://github.com/scummvm/scummvm/commit//55136a8f7e2ae07e0937464b359f48a6ddc98263
https://github.com/scummvm/scummvm/commit//18ba1461e03a10b7c92f1089a5c9d8fc5e381b5e

One could possibly implement another approach for this, that would allow manually saving in slot 0 and still not cause issues with the native load code nor seg faults, but at the time that I worked on that fix, this seemed like the quickest and yet effective solution.

comment:2 by antoniou79, 3 years ago

Cc: antoniou79 added
Component: --Unset--Engine: Sky
Summary: Savegame 0 produces write permission error on AndroidSKY: Savegame 0 produces write permission error on Android

comment:3 by puetsch, 3 years ago

Thank you. I wonder if it would be possible to create a clearer error message and/or add this to the FAQ and README?
Getting a generic "write permission denied" made me think it's a permissioning error, and I spent a lot of time trying to Google a solution, trying different paths etc.

comment:4 by antoniou79, 3 years ago

I agree. At the very least I'll see if I can add something to the documentation somewhere.

I did bring up this issue in Discord a while ago (when I committed the fix), but I don't think there was any solution suggested for it, and I moved on to something else.
https://discord.com/channels/581224060529148060/581224061091446795/735130522522747001

The problem is that this engine function SkyEngine::saveGameState() has to return a Common::Error, and of the available ones none is exactly fitting with this situation. There's little room to maneuver here, since this save method is called from the common engine code (Engine::saveGameDialog()) -- it's not specific to the SKY engine.

So we either add a new Error (along with a description) in the common code (common\error.h, common\error.cpp) which would be specific to this situation, or I guess we could disable writing to slot 0 (making it greyed out or something)?

I'm leaving the ticket open until I or someone else addresses this issue.

Version 1, edited 3 years ago by antoniou79 (previous) (next) (diff)

comment:5 by sev-, 3 years ago

Priority: normalhigh

This would be nice to get fixed before the release.

comment:6 by sev-, 3 years ago

Summary: SKY: Savegame 0 produces write permission error on AndroidBACKENDS: ANDROID: SKY: Savegame 0 produces write permission error on Android

comment:7 by antoniou79, 3 years ago

I've issued a PR for a potential way to address this issue.

It adds an extra error (along with a message for it) for the situation where the game engine does not handle or allow saving manually to particular slot(s).

The Sky engine (Beneath a Steel Sky) makes use of this new error.

https://github.com/scummvm/scummvm/pull/3369

comment:8 by antoniou79, 3 years ago

Summary: BACKENDS: ANDROID: SKY: Savegame 0 produces write permission error on AndroidSKY: Savegame 0 produces write permission error on Android

comment:9 by antoniou79, 3 years ago

Changed the ticket title back to "SKY: Savegame 0 produces write permission error on Android" since, as I've written above, this is not an Android specific issue.

comment:10 by antoniou79, 3 years ago

Implemented a different approach for this. The autosave slot is now write protected (AGI engine does this too, as was pointed out to me), so the GMM UI won't let the user rename it or overwrite it.

Relevant commit:
https://github.com/scummvm/scummvm/commit/1712cceb205e0fcc94e8327406b15279e8a5e227

comment:11 by bluegr, 2 years ago

Owner: set to antoniou79
Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.