[Scummvm-devel] ATTN Porters: SaveFileManager API Adjustments

[Johannes Schickel]

On 03/17/2016 09:53 AM, Marcus Comstedt wrote:
> Johannes Schickel <lordhoto at scummvm.org> writes:
>
>> Basically, it means that you can access save files (and other engine
>> written files) without worrying about case issues on the underlying
>> hardware.
> I never had to worry about it before.  The worry starts here...

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.

>
>>> That openForSaving("foo") should remove "Foo" and "FOO" if they exist?
>> I wouldn't necessarily make an open call remove files.
> But if it doesn't remove them the number of "conflicts" increase.
> If it later does openForLoading("foo") it might get one of the other
> files.

Sorry I misread this as "openForLoading" in the first place. Certainly 
it should use one of the present "Foo" or "FOO" files in this case. 
Thus, this would not introduce the number of conflicts.

>
>> Casing in target names is one possibility where different cases can
>> originate from. We have (semi) case insensitive target name handling,
>> i.e. the case of the target doesn't matter when you run
>> games.
> I assume you mean when the user inputs a target name on the command
> line?  That's never going to happen on the Dreamcast.
>

Valid point, but remember this is a API which affects behavior across 
all ports.

>> Synching files between devices and/or manually inserting save files
>> (from bug reports etc.) is another source for different cases.
> Copying a file between memory cards on the Dreamcast preserves the
> filename verbatim.  There is no UI to rename a file.  If you have a
> Nexus card that allows you to extract files from the card to other
> systems, you will get the in the form of VMI files, which also
> preserve the filename verbatim (the filename of the VMI file itself is
> irrelevant).

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.

>
>> Last but not least, all other engine facing high level file APIs are
>> case insensitive, i.e. file opening etc., this change makes
>> SaveFileManager fit in.
> Ok, I don't want to sound negative here, but I don't really fancy
> adding slow and complicated code to all savefile operations to make an
> API "fit in".  We have separate APIs for separate things because they
> have different requirements (from both engine and platform sides);
> adding things which are not based in a technical requirement (and
> might actually counteract other requirements) to make the savefiles
> "fit in" with other files seems counterproductive.  Savegames are a
> separate API because, on some platforms at least, they do _not_ "fit
> in" with other files, and may in fact be something completely
> different from other files.

In practice we would either need to do this on engine side or on backend 
side. Either way someone has to worry about case issues. This is a 
requirement because otherwise we are stuck in an inconsistent state.
If we worry about this on engine side, we will need to adapt all 
relevant places of ScummVM to be prepared for case sensitivity, i.e. 
target names (their use in save file naming), all uses of 
SaveFileManager for non-save files by the engines, etc.
If we worry about this on backend side, we will need to adapt all 
backends. 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.
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.

So, basically where was a choice in officially making it case sensitive 
or case insensitive.
Case insensitive matches the practical usage of engines, thus does not 
force additional complexity on the client side. It has the benefit of 
being in line with our File API, thus reducing the risk of confusion on 
developer side.
Case sensitive makes it hard for backend developers on case sensitive 
platforms. However, on the bright side it limits the complexity at a few 
places in the code.

>> They could of course
>> worry about providing consistent file names but I went in favor of
>> having a consistent API.
> Each API should be internally consistent, but if there are technical
> reasons for API 1 to do something differently from API 2, I think that
> is more important than cross-API consistency.
>

Well, I do agree with you in general on this one. However, the 
SaveFileManager API is a really good candidate to have a defined 
behavior on this level. It is not a low-level API like FSNode, where it 
matching the actual file system (and thus undefined for a user) is a 
good choice and a requirement to work well for its purpose.

Funny note: Before this explicit statement that it should be case 
insenstive our default implementation was inconsistent, it was partly 
case sensitive and partly case insensitive.

// Johannes