[Scummvm-devel] ATTN Porters: SaveFileManager API Adjustments

[Johannes Schickel]

On 03/17/2016 11:12 PM, Marcus Comstedt wrote:
> Johannes Schickel <lordhoto at scummvm.org> writes:
>
>> That's true. However, if we explictly made SaveFileManager case
>> sensitive (it has been unspecified before), we would start making all
>> engine code worry about this. So, either choice we make there is
>> someone affected. Right now engine code does not worry but just
>> effectively assumes the SaveFileManager is case insensitive.
> To some extent the engine will always need to worry about the
> capabilities of the different backends and cater to the lowest common
> denominator.  If it is unspecified whether savegame filenames are case
> sensitive or not, it means that the engine must abstain from trying to
> create two different savegames whose names differ only in case.  But
> that is still the case even if we say that the names are always case
> insensitive.  In addition, it needs to consider things like maximum
> filename length (12 characters on the Dreamcast for example), and
> characters which are disallowed in filenames (none on the Dreamcast,
> but other backends may forbid characters like '/' and ':').

They will, yeah, but there is a different degree on how much they need 
to worry about behavior on different platforms.

Btw. 12 characters on the Dreamcast is already an issue. We have default 
target names exceeding that limit and since engines often use target 
name + number this is a problem. Not having such limitations documented 
is an issue because this way people won't realize it until they run into 
some issues. (A similar issue was the case issue.)

>
>> In this case it is not possible to introduce conflicts? It sounds like
>> you might just not worry too much about case issues in your backend
>> then? Simply enforcing all lowercase or similar sounds fine right now.
> Lowercasing the filenames would certainly be acceptable
> performance-wise (as opposed to having to perform an extra file
> listing to figure out the "correct" casing, which would be very slow),
> but I worry a bit about creating incompatibitiles with existing
> saves.  Do we know for a fact that no current engines use uppercase
> characters in the filenames?
>
>

I looked at our sources a bit and I'm afraid not. For example, BASS 
saves use all upper-case by default :-/. Some SCUMM gameids (which are 
used as default target names) are mixed case.

>> Let's not forget that we also have case insensitive
>> platforms, trying to implement a case sensitive save file API there is
>> probably going to be some work too.
> Yes, that is not my proposal.  Rather to leave it implementation
> defined and have the engines follow the rules required for both
> variants to work:
>
> A) Always use the same casing when opening for read / deleting as was
>     used when opening for write, or alternatively use a filename
>     returned by listSavefiles() without changing its case (I have no
>     issue with listSavefiles() performing a case insensitive glob match
>     even on case sensitive systems, it needs to retrieve the list of
>     files anyway)

This is behavior you suggest for listSavefiles is exactly the reason why 
I looked into this. Our default implementation thought this is a good 
idea and did it. And that is a real problem, because it results in 
discovered save files which can not be loaded. But it is the only 
sensible way for the engines to figure out whether conflicting cases 
exist on case sensitive platforms and if only for backwards 
compatibility(!).

Basically on a case sensitive file system you always have to worry about 
cases being mixed up, and if only because of user interaction and 
copying save files from other people/and or other devices. Thus, you 
would need to use listSavefiles at some point to figure out what the 
exact filename is (this is basically what you want to avoid in your 
backend AFAICT). Engines can have the possibility to optimize this by 
keeping the filenames around, but so could you in your backend.
One question I have is: can you switch memory cards during run-time on 
Dreamcast? This is something which engines *might* have to worry about 
too, depending on their style of handling it, but you can handle this 
much easier in your backend.

> B) Never try to create two saves with file names which differ only in
>     case

Pretty sure, no engine will intentionally do this.

> The first is required for case sensitive systems, the second for case
> insensitive systems.  You will need to cater to at least one of them,
> and there are simple ways (such as always lowercasing) to do both at
> the same time.

Always lowercasing to try to work around case sensitivity is not an 
option as we already figured out. Because if old savegames (can have a 
different case) are around without this behavior, then they will be missed.

>
>> If we just leave this unspecified then, it's effectively leaving it to
>> the engine side because they can not assume it being either way and
>> need to work fine with both.
> Yup.  I don't think it's very difficult to do though.  On the other
> hand forcing the backend to behave in a specific way may incur serious
> performance penalites, and might not even benefit the engine because
> it would actually have preferred the other behaviour...

The thing is: if you let engines worry about case sensitivity, they will 
need to replicate a very similar behavior like you would need to 
implement in your backend.

> At the very least, I think the engine should declare whether it needs
> this functionality or not.  If it already complies with both A) and
> B), then there is no reason for the backend to try to give it
> something it has no need of, at the expense of user experience (slower
> saves and loads).
>

So, how big would the penalty be for your system?

// Johannes