[Scummvm-devel] File handling revamp

Johannes Schickel lordhoto at gmail.com
Tue Jul 29 03:30:08 CEST 2008


> * The overly convoluted, complicated, under-documented and abused
> File::addDefaultDirectory() system (The blame is on me to a big extent, of
> course :). Semantics are unclear here, too. Right now, we blindly add every
> directory in our reach and it's friends, and then just hope the files we
> want are included somewhere there. And that there are no name clashes. No
> way of saying "game data files are in these dirs, GUI theme stuff please
> only from that dir". One specific problem: The AdvancedDetector's
> detectGame() function needs to support taking either a FSList param, or
> alternatively uses File::open() to locate files. Besides complicating the
> code, this causes a disparity between detecting and running games, which can
> lead to subtle bugs.

Well there's one problem all engine developers have to address when
using FSNode for opening files in directories: case (in-)sensivity.
That blows the client code unnecessarily IMHO. Of course
addDefaultDirectory is case sensitive too AFAIK, but it's less work to
do: addDefaultDirectory("foo"); addDefaultDirectory("FOO"); than using
the FSNode code to detect what case the file has. Also for getting
FSNode objects to files inside that path FSNode one has to handle
possible cases too, or creating an FSList and compare names via an
case insensitive comparator, which would be utterly stupid and hacky
:-).

That 'issue' is known since some time though and the KoM engine and
Parallaction engine had/have problems with it. Peres told me
Parallaction doesn't handle case sensitivity at all (yet). It only
affects BRA currently as far as I got it though, so nothing we have to
worry about for our next release.

> -> Solution: Scrap it! And replace it with something better. Right now I am
> not sure what *exactly* that means; we first will have to compile a list of
> requirements. I.e., what exactly are those search paths used for?. Then come
> up with a design proposal based on that.
> One approach might be to introduce a SearchDirectory class, which is inited
> with a FSNode, and could cache the list of all files in it (and optionally
> of its subdirs, recursively). Then either File::open (optionally) takes (a
> list of) SearchDirectory object(s), and/or a global set of active
> SearchDirectories could be stored. This approach makes it easy to
> dynamically add/remove dirs from the search list (just dump the
> SearchDirectory object); or to search for files only in a subset of all
> search dirs. Not to mention that we get proper OO-style code encapsulation.

A SearchDirectory object sounds like a good idea for a first thought.
It should of course handle all case sensitivity problems internally
and then thus would take care of what I mentioned above. So basically
the idea sounds fine to me.

> * Path separators (slashes) are silently creeping into File::open calls,
> decreasing portability in a fashion that currently is hard to detect.
> -> Solution: Change the code to use FSNodes as it should, or implement the
> SearchDirectory proposal above, or something else which makes it possible to
> conveniently load files from a (set of) dirs.
> Also: add a check to File::open() that warns (ultimately: errors out) when a
> slash is used in a file name.

Well it would be of course good to use FSNode for that, but see the
problems with FSNode mentioned above. That's why I'm currently using
paths in File::open in my Lands of Lore patch on the patch tracker
too.

> That's all for now. But I guess the list is long enough. Though I am sure I
> forgot stuff. Porters, speak up if you can think of something else that'd
> help! Engine maintainers and everybody else, speak up if you have concerns
> missing on this list!

I only answered to the, in my opinion, most important points in this
mail. I hope nobody thinks I do not care about the others, but I'm
getting a bit tired so maybe that will follow later.

// Johannes




More information about the Scummvm-devel mailing list