[Scummvm-devel] New File(Manager) API

[Max Horn]

Am 11.04.2004 um 07:28 schrieb Pawel Kolodziejski:

[...]

>> There are three parts to this:
>>
>> 1) ReadStream & WriteStream interfaces and subclasses which implement
>> them.
>>    These are meant to abstract away reading/writing from "streams",
>> which can be files, or memory buffers, or anything else you might come
>> up with. The File class implements both. And there is a XORReadStream
>> which wraps around any ReadStream, adding "XOR encryption" to that
>> stream (which can be used by the SCUMM frontend, and allows us to get
>> rid of the SCUMM specific XOR encryption code in the File class)
>> These are actually mostly independent of my other changes. I'd like to
>> see those (or a variation of them) in whether we do the rest or not.
>> They'll allow us to unify the code in sound/voc.cpp, for example.
>
> that XXXReadStream will be inside/transparent to read() func ?
>
> it will select stream while creating File class constructor ?

No, that wasn't the plan. Rather you can wrap the XORStream around any 
other ReadStream (including File's). So you can have code like this:

   class ScummEngine {
     File *_dataFile;
     ReadStream *_dataStream;
     ...
   };
   ...
   _dataFile = ...;
   if (useEncryption)
     _dataStream = new XORStream(_dataFile);
   else
     _dataStream = _dataFile;

   ...
   _dataFile.seek(pos);
   int32 data = _dataStream.readSInt32();
   ...


[...]

>> The thing which is a bit more complicated is that you can't create 
>> File
>> instances directly on the stack anymore. If that is bothering people a
>> lot, it wouldn't be too hard to "Fix" this, though. Just tell me.
>
> That would be usefull when there is much more 'ifs' in func with 
> closing
> file while something goes wrong. at stack it will be only once when 
> func
> finish.

Yes, that is my main concern, too. However, to allow stack instances of 
the File class  would require us to make the File class a lot more 
complicated (we'd have to introduce a new internal class 
FileImplementation and use some voodoo, etc).

The easy solution, IMO, would be to introduce something similar to the 
Std C++ libs auto_ptr class. An auto_ptr is a bit similar to our 
StackLock in concept: it's a template class which holds a pointer to a 
C++ object; and it's destructor deletes that pointer. I think a simple 
example highlights what I mean better than many words:

bool dummyFunc() {
   FilePath filepath("answer");
   AutoPtr<File> myFile = filepath.makeFile();
   myFile->seek(42);
   if (myFile->readByte() != 42)
     return false;
   theAnswer = 42;
   return true;
}

This function has to exit points, yet there is no leak. Because when 
exiting the scope, myFile's destructor executes, which deletes the File 
object returned my filepath.makeFile().

The added bonus is that we can use this autoptr thing for other 
purposes, too. I.e. whenever you have a temporarily allocated pointer 
sitting around.

>
>> Search path handling would also be very easy, I provided a convenience
>> method tryOpenFile(), which does it. You can use it like this:
>>
>>    PathList searchPaths;
>>    const char *dirs[] = { "", "video", "VIDEO", "data/", "DATA/", 
>> ..., 0
>> };
>>    int i = 0;
>>    while (dirs[i]) {
>>      FilePath path(dirs[i]);
>>      if (path.exists() && path.isDirectory()) {
>>        searchPaths.push_back(path);
>>      }
>>    }
>>    ...
>>    File *myFile = tryOpen("MONKEY.000", searchPaths, kFileReadMode);
>
> is it client code ?

Yes, this would be typical client code. The above could be an excerpt 
of the SCUMM engine.

> in previous example instance is created from filepath , now this is
> created    from what instance ?

I assume you mean: how does tryOpen() create the File pointer? Look at 
newfile.h, the implementation of tryOpen() is given there. I'll paste 
it here again for reference:

File *tryOpenFile(String name, PathList searchPaths, File::AccessMode 
mode = kFileReadMode) {
	for (PathList::Iterator iter = searchPaths.begin(); iter != 
searchPaths.end(); ++iter) {
		// TODO: Maybe also try case variations of the given 'name', i.e. also
		// try with 'name' converted to lower/upper case.
		FilePath path(*iter, name);
		if (path.exists() && path.isFile() && path.isReadable())
			return path.makeFile(mode);
	}
	return 0;
}

  Essentially, it loops over all paths in searchPaths, then checks 
whether a readable file with the specified name exists there, and if 
so, returns a File pointer to it. Otherwise, 0 is returned.


>
>>    if (!myFile)
>>      error("Couldn't open MONKEY.000");
>>    byte firstByte = myFile->readByte();
>>    ...
>>
>> Of course the computation of "searchPaths" only has to be done once.
>
> Does that search path will be initialized at engine initialization ?

For example, yes. But it's completely up to the client code how this is 
done. I.e. nothing stops us from having multiple "searchPaths" (e.g. 
one for game data files, one for video, etc.). This is IMO a very nice 
improvement over the current hard-coded list of "search paths" in 
file.cpp. Moreover, the path list is of course not restricted to 
pointing at subdirs of one directory. If we want to fulfill that one 
feature request and add a "secondary_path" config variable (so you can 
e.g. share the music files between your two versions of The Dig, which 
would be nice e.g. for me :-), then supporting that is trivial in the 
new setup.

>
>>
>> Cheers,
>>
>> Max
>>
>
> This draft explain only few area of code.
> Can you draft full example of implementation ? not necessary C++ :) too
> have full image of implementation.
>
If you mean whether I can implement the specs outlined in the header 
file? Of course I can, but it's of course quite some work, and I first 
want to gauge how people perceive the draft. I.e. no fun in 
implementing this if I am then told it's all crap and have to throw 
away my work :-)

Although so far the two replies I got (by you, Pawel, and by Sev) where 
(in my impression) more positive than negative :-).

If you mean you would like to see more examples of how this new code 
would be used: I tried to give some more in this email, and if you want 
to see more, just ask me about specific things, and I can write an 
example demonstrating how you'd do it.


Cheers,

Max