[Scummvm-devel] File handling revamp

[Max Horn]

Hi Marcus!

Am 29.07.2008 um 13:09 schrieb Marcus Comstedt:

>
> Max Horn <max at quendi.de> writes:
>
>> -> Solution: Make it possible to provide alternate File-class
>> implementations, just like for FSNodes. One way would be by adding a
>> FileManager class, with internal methods like
>>  SeekableReadStream *openFileForReading(String name);
>>  SeekableReadStream *openFileForReading(FSNode);
>>  WriteStream *openFileForWriting(String name);
>> and then the File class simply calls these in open(), and otherwise
>> calls-through to the stream object it obtained.
>
> Wouldn't it be simpler to just have
>
>  SeekableReadStream *openFileForReading();
>  WriteStream *openFileForWriting();
>
> in the FSNode class?  Since FSNodes can already be provided by the
> backend, the FSNode can return port-specific file objects.

Yeah, good idea. I am not married to that FileManager business (the  
idea only came to me while writing the mail yesterday).
You are right, in many ways those methods would make sense in FSNode.

However, we still would need a separate place for the config file  
stream, but that could be done by adding openConfigForReading/ 
Writing() methods to OSystem.

>
>
>
>> * The more I think about it, the more I like the idea of having a
>> pure virtual FileManager class (to be implemented by ports, with a
>> default implementation based on FILE), which provides some
>> minimalistic core API for creating File/DumpFile/ConfigFile/SaveFile
>> streams. The point being that a port could use the exact same
>> implementation for all these types of files, or use a completely
>> custom one for each.
>
> If the implementation is different for each one, then it becomes even
> more clear that this needs to be integrated with ths fs code, not
> separate from it.  I mean, if File, ConfigFile and SaveFile have
> completely different implementations, they will more likely than not
> have different namespaces for their filenames as well (this is
> definitiely the case with game files and savefiles on the Dreamcast).

Exactly. There are savestates (which only have a name, and no path),  
managed by the SaveFileManager; There are game data files (and dump  
files), which you access using FSNodes; and then there is "the" config  
file, which is a single unique file and would be loaded/saved via  not- 
yet-existing API.

Side note: For desktop ports, you also want to support config files in  
arbitrary locations; they simply use FSNodes for that, then, as there  
is no distinction between the various files kinds on the desktop. Non- 
desktop ports do not offer the possibility to use custom config files,  
hence no need for a separate config file "namespace".

>
> So if you create a FSNode("foo"), and do ->exists() in it, should it
> check whether there exists a "foo" in the namespace of Files, in the
> namespace of ConfigFiles, or the namespace of SaveFiles?

FSNodes are only for data files (and maybe dump files, for writing),  
they are *never* to be used for SaveFiles and ConfigFiles. That's one  
of the major points driving my desire to revamp the file handling  
business :).

> In short, the FilesystemFactory needs to be in on the whole deal.

For data & dump files, sure.


> Btw, if we are going to redo the fs, I'd again like to propose having
> different calls for creating a file node and a directory node, rather
> than having a common node that you can ask if it is a directory.
> I think it would result in simpler, more robust code (both for
> implementers and for users).

Sadly, I must have missed that suggestion or maybe did not understand  
it correctly :/. So, do I get you right: You propose that instead of a  
single FSNode class, we have say a FSDirNode and an FSFileNode class.  
Then, you do:
   FSDirNode dir("foo");
   FSFileNode theFile = dir.getChild("bar.dat");

So, the main advantage of this would be that we kind of give the  
FSNode constructor a flag which tells it whether we mean to refer to a  
file, or a directory. Hm, I think I can see some merit to that, fully  
agreed.

An alternative approach would be to keep the single FSNode, and just  
add a flag to the constructor "bool isDir".
That would achieve roughly the same effect, wouldn't it? Maybe I am  
missing something, but as usual I am sure you won't be shy to point it  
out to me (I actually appreciate that ;).

Of course,
   FSDirNode dir("foo");
is more suggestive than
   FSNode dir("foo", true);
but that could in turn be solved via some simple C++ tricks, using  
syntax like
   FSNode dir = FSNode::makeDirNode("foo");



> The only downside is that getChildren
> would have to fill two lists (one with files and one with
> directories),

Well, that's not a major downside, of course. My alternate suggestion  
avoids this extra complication, though.

BTW, a mostly unrelated thing: I feel that it's bad that there is no  
way to get back from an FSList to the corresponding FSNode; that is  
useful when you want to e.g. print out a debug / error message. Like,  
our detectors sometimes want to print a message like "Unknown game  
version detected in dir FOOBAR", but with FSLists, they can't that.

So, FSLists probably should store a ref to the FSNode they are coming  
from..


> but I figure that in the places you use it, you'd
> typically want to separate the directories from the files anyway
> (e.g. file browsers usually like to display all subdirectories
> first).

They can achieve *that* using a simple sort, so this is not really an  
argument in my eyes, neither pro nor contra :).


Cheers,
Max