[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