[Scummvm-devel] ENGINE AUTHORS: case-insensitivy and directory handling

Max Horn max at quendi.de
Sun Aug 3 20:28:06 CEST 2008


Am 01.08.2008 um 15:39 schrieb peres:

> Hi team

Hi Peres, hi all,

before I reply to your post, here's a plea:

Everybody, if you have *any* objections to the plan below, please  
voice them. If you keep silent, that's fine, too, but please don't  
complain afterwards if we make changes you don't like. You've been  
warned :)

[...]

> The recent discussion on the devel mailing list has brought the idea  
> of
> unifying the handling of game files and directories in a single API  
> that
> sits atop of FilesystemNode and deals with case-sensitivity issues  
> for the
> engines. This API would thus replace Common::File in the medium-long  
> term,
> also providing a way to access files in subdirectories.

Well, it would replace Common::File, which is our current "single" (or  
not so single) API which tries to deal with case-sensitivity issues  
and other stuff; except that it doesn't properly sit atop FSNode, and  
has various other shortcomings, but we already discussed that at  
length ;)


> The concept is to model directories instead of a game file, and give  
> the
> engine an instance of the directory where the game is stored. This  
> could
> be asked to open a file, for which it would return a readable  
> stream, or a
> directory, for which it would return another directory object.

This makes sense, aye.

On a sidenote: The ScummFile class in ScummVM might benefit from such  
a change, too: It is a Common::File subclass which allows opening  
"subfiles"; i.e. it is a wrapper around an container/archive files, in  
which various other files are collected together, and this class  
allows transparent access to these files via the same API as  
Common::File provides.

Redux: With a GameDirectory/GameDirectories approach as you propose,  
this container access code in SCUMM could probably be remodeled in a  
more natural fashion. All we would have to do is to allow subclasses  
of GameDirectory to be used, too.

Of course, it would also make it easier to add support for reading  
files from ZIP files in a transparent fashion... For theme data etc.,  
right now we don't want that for game data.



> Going a bit further, this approach also allows more powerful and  
> flexible
> file searches than the current default directory system -  
> implemented as
> static members of Common::File - does. The key is an object that  
> manages a
> set of the directories introduced above, together with an array of
> settings that specify how searches should be performed (per  
> directory!),
> for example how deep a certain directory can be searched, if hidden  
> files
> should be considered, and so on...


Also, these objects should cache the directory contents, making  
lookups faster and relieving backends from the burden of doing such  
caching. Drawback: changes to the FS may not be noticed. My take: if a  
user tinkers with the list of present files while ScummVM is running,  
they shall bear the consequences ;).
But of course we could easily flush the file/dir caches at certain  
points, too (like when returning to the launcher).



> The attachment contains a quick implementation of both classes I  
> sketched
> in this mail: I called them GameDirectory and GameDirectories. It  
> should
> be considered a proof of concept more than an usable piece of code,  
> and
> has shortcomings so far (like what to do with FSList returned by
> matchPattern). Anyway, I would be glad if engine authors would drop  
> their
> considerations about it. I am also posting on the patch tracker so the
> code is easily reachable in the future.

All in all I like the idea. Some of the refinements that come to mind:

* As you already pointed out, of course we wouldn't want to base this  
on Common::File. So, I think we should do as Marcus suggested and add  
FSNode::openFileForReading/Writing() methods. It seems to me that we  
should do that no matter what else we change. So, I'll commit a dummy  
implementation for that ASAP (i.e. one which internally uses  
Common::File, too, for now).

* Caching the contents of the given dir is mandatory. Just keep a  
HashMap of the files. Easy to do (case insensitive) lookups against  
that, too. I think I would lazily create it, the first time somebody  
tries to access a file in the dir.

* GameDirectory::getSubDirectory() doesn't support error handling in  
its current form. I.e. if you try to do it on a non-existing dir.  
Right now it'll just crash, I think. But a typical use case is that we  
might want to also search for files in the "resource" or "RESOURCE"  
subdir, if it exists. That is not really possible right now.
One quick (and dirty?) solution: Return an empty GameDirectory if the  
subdir does not exist.

* In getSubDirectory, I think I wouldn't want to error out on a name  
clash. Just print a warning. Better to just run as good as possible  
(by only using the first of the matching dirs) than to do a full  
crash, isn't it? At least in this case...


* Resource managment: One nice thing about class File is that it  
automatically takes care of closing the file and freeing any used  
memory when it goes out of scope. With GameDirectory::openFile(), we  
loose that nice bit. Well, I guess we can use Common::SharedPtr to  
resolve that. I.e. advice engine authors to do
   SharedPtr<SeekableReadStream> file = dir.openFile("test.dat");
Of course, they would still have to check the returned value to be non- 
zero to detect errors, but that's not worse than current code, I guess.


* Finally, I guess a (pseudo) example of how to *use* GameDirectory  
would help people to understand and comment on your proposal much more  
than any (lengthy) text would :)



> A final word on the way this can be applied to the code base, provided
> there is a consensus around it. Moving engines from Common::File to
> *anything* different is indeed a massive task (there 134 engine files
> referencing Common::File as of today!), but the process can be  
> carried on
> gradually, by having this API in parallel and changing code in steps.

Aye, that's an important thing. Though if we really decide to do that  
switch, I am sure it could all be done within a single week, once the  
new system is in place (should we go with your approach, that is :).

Cheers,
Max




More information about the Scummvm-devel mailing list