[Scummvm-devel] File handling revamp

Johannes Schickel lordhoto at gmail.com
Tue Jul 29 17:29:27 CEST 2008


On Tuesday 29 July 2008 16:03:10 Max Horn wrote:
> > 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.
> 
> You are completely right.
> 
> <RANT>But I wish people would point this out immediately when they run  
> into it, instead of silently suffering or using other approaches ...</ 
> RANT>

Well they do usually ask on IRC about it. Since that's where they hope to get quick answers.

> So, this is an issue. Luckily, one that shouldn't be hard to fix. E.g.  
> by augmenting the FSNode API to support case insensitive filenames, or  
> by providing simple wrapper which hides these details from engines.  
> Tell me where and how you need the case insensitivity, and we'll  
> implement it ASAP :-).

Well for engine usage as in Parallaction:

_partDir = _baseDir.getChild(name);
_aniDir = _partDir.getChild("ani");
/* ... */
(engines/parallaction/disc_br.cpp ll106-116)

and

Common::String path(name);
FilesystemNode node = _talDir.getChild(path);
if (!node.exists()) {
	path += ".tal";
	node = _talDir.getChild(path);
	if (!node.exists())
		errorFileNotFound(_talDir, path);
}
(engines/parallacction/disc_br.cpp ll136-143)

The code suffers from case sensitivty issues on Linux for example. A user with the files in uppercase would of course get some file not found error messages.

> > but it's less work to
> > do: addDefaultDirectory("foo"); addDefaultDirectory("FOO"); than using
> > the FSNode code to detect what case the file has.
> 
> Well, you simply don't want to have code detecting that, I guess. All  
> you want is to say "Gimme the dir 'foo'", and then you don't care  
> about the case. In 99% of cases, there is only one dir foo or FOO, and  
> you should just get that (either via an overhauled/added FSNode API,  
> or suitable wrapper code). Personally, I'd tend to say: "Screw the  
> folks who keep both a "resource" and a "RESOURCE" folder in their game  
> data copy). Seriously, I don't think anybody should have that. Of  
> course we could also add ways to compensate for that, but I don't  
> think it's worth the effort.

Agreed.

> > 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
> > :-).
> 
> Depends on what you mean...
> Iterating over an FSList using a case insensitive comparator is  
> precisely what most of our code walking dir contents currently does.  
> It seems to be the most natural thing to do when you want to, you  
> know, try to search a list, not caring for case... :-).

Well of course it's meant for opening files later on in the engine. Using FSLists to 'workaround' the case sensitivity would be utterly hacked :-). In other cases it is of course required and nice to use FSList.

> However I am talking about game detection code here, which is looking  
> for multiple files at once. If you just want to open (during play  
> time) a single file "bar.dat" inside some directory "foo", then of  
> course you should not be forced to walk an FSList, agreed. You just  
> want to do something like
>    f.open("foo/bar.dat");
> or
>    f.open(FSNode("foo/bar.dat"));
> or
>    f.open(FSNode("foo").getChild("bar.dat"));
> or
>    f.open("bar.dat", mySearchPaths);
> or even just
>    f.open("bar.dat");	// Search in the global list of default seach  
> paths

Little note from my side: Example 3 will of course not be working currently, unless 'foo' is an absolute path or in the cwd on some systems. (Known since ages too).

> The first two simply represent convenience accessors, easy to  
> implement by writing a simple parser (relative) for POSIX style  
> passes, which would then internally create the corresponding FSNodes,  
> in a case insensitive fashion. I.e. they would both amount to  
> something like the third example, which in turn of course requires us  
> to enhance FSNode accordingly. Should be no big deal, though (note: If  
> dir "foo" does not exist, FSNode("foo") represents an invalid node,  
> getChild would then also return an invalid node, and File::open would  
> finally fail because of this, as expected).

Well for the parser we could just use Common::StringTokenizer from common/util.h. Should be trivial to implement with that.

Also I'm not sure if using getChild on an illegal FSNode will not just crash with an assert. At least things like that happened in the past. Looking at the current code it may happen it fails with an assert for example if getChild is used on an FSNode, which is no directory. Of course FSNode internal implementations could assert out on different cases. That behavior is of course not really nice, since it would just crash for the user without a good error message.

> The alternative (or maybe not alternative, but rather complement) is  
> to use search dirs, as discussed below. And yeah, we could still have  
> some globaly active search dirs. But one of my points is that I think  
> it's better to not jumble *all* the files in possibly a dozen dirs  
> into one big list, but rather try to be a more bit selective about  
> it :).
> 
> Depending on the requirements of the client code, each of these code  
> have its merits. Personally, I would think that for client code  
> writers the following are the most important possibilities:
> 1) f.open("foo/bar.dat");
> 2) f.open("bar.dat");
> 3) f.open("bar.dat", mySearchPaths);
> What is unclear to me is whether the first one should only look under  
> PATH/foo/bar.dat; should look in all search paths for a dir foo with a  
> file bar.dat.

All three examples look like what it should be for the user later on. Of course we should have to take care of File::exists etc. too. And I'm personally for searching *all* global search pathes in example 1), since it would follow the sames rules as example 2) with that.

> > 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.
> 
> Sadly it was not known to me :/. I recommend writing about such  
> problems to this list for discussion. Talking about it on IRC only  
> reaches a limited audience.

Well people seemed to accept that FSNode is case sensitive and wrote code to detect the case on their own instead. (At least that's what I'm thinking KoM is doing currently, I didn't look at the code since some time though). Anyway I wouldn't blame them that they asked on IRC for help/information on that.

// Johannes






More information about the Scummvm-devel mailing list