[Scummvm-devel] RANT

Johannes Schickel lordhoto at gmail.com
Mon Jul 24 08:03:05 CEST 2006


Max Horn wrote:
> one last response: While I still think that (ab)using displayName()  
> for file access, is a bad idea, fact is that FilesystemNode is  
> missing an additional name() method that can be used for filename  
> lookup / comparison. This lack caused me to abuse displayName() in  
> the SCUMM game detector, too, BTW (albeit in a lesser fashion -- just  
> for comparing to a list to known filenames, not for use in  
> File::open). The main reason I haven't added FSNode::name() yet  
> (which would be very easy to do, after all) is that I was afraid of  
> it silently getting abused -- by people using it in File::open.

One problem here is that nobody really looks at the documentation of the
File class (if you can call it documentation by the way). I guess if
people would know that there is a Common::File::open which accepts a
FilesystemNode they wouldn't even think of using FilesystemNode::name.
Well I used FilesystemNode::displayName to open a file just because my
code hadn't (and still has not) the possibility to use a FilesystemNode.
That's because of a couple of reasons I'll tell later.

> Alas, I was a fool to act that way in the first place. Sure it'll get  
> abused, but who cares -- likely nobody besides me. Sorry for unjustly  
> flagging your commit, LordHoto, nearly everybody else on the team  
> likely would have written the very same code, after all (and looking  
> through our code, several already have). LordHoto has made  
> outstanding contributions to the project, and I never wanted to paint  
> him black, just to make that clear.

I didn't took it at as you wanted to paint me back. I wonder about what
'outstanding' contributions you talk... :)

> So, a better response would have been to start a discussion on how to  
> improve upon FSNode. But since typically my attempts to start  
> discussion on design of backend/frontend/glue code leads to no  
> response from the Big Black Hole, and in the end I am alone to decide  
> what to do anyway, I prematurely cut that discussion out. Which  
> clearly was not a good idea. :-/

Well I think one reason why nobody would take place in a discussion
about FSNode improvements is that nobody uses it, some maybe knew it
only from their game detector code, which is except Kyra where I use it
also to load all kind of package files with it, the only place there the
engines are using FSNode. I think it has a some reasons. I made some
tests with it and here are my results:

// file in extrapath
FilesystemNode("kyra.dat") creates an invalid node
FilesystemNode("KYRA.DAT") creates an invalid node
FilesystemNode(ConfMan.get("extrapath").c_str() + "kyra.dat") creates a
valid node
FilesystemNode(ConfMan.get("extrapath").c_str() + "KYRA.DAT") creates an
invalid node
Common::File::exists("kyra.dat"): true
Common::File::exists("KYRA.DAT"): true

// file in path
FilesystemNode("dead.vrm") creates an invalid node
FilesystemNode("DEAD.VRM") creates an invalid node
FilesystemNode(ConfMan.get("path") + "dead.vrm") creates a valid node
FilesystemNode(ConfMan.get("path") + "DEAD.VRM") creates an invalid node
Common::File::exists("dead.vrm"): true
Common::File::exists("DEAD.VRM"): true

// file in cwd
FilesystemNode("scummvm") creates a valid node
FilesystemNode("SCUMMVM") creates an invalid node
Common::File::exists("scummvm"): true
Common::File::exists("SCUMMVM"): true

Tested with the POSIX FS implementation. As you can see FSNode lacks
support of case insensitive file access, access to files which are in
the game data path or other special paths added with
Common::File::addDiretory(Recursive), it can just access files in the
CWD (at least with the POSIX FS implementation) without using a explicit
path, so if you would need a valid FilesystemNode object for example on
"KYRA.DAT" you would try all paths, and try "KYRA.DAT" and "kyra.dat" as
filenames for it, which is too much work for something like that. Also
what should the engines do with a FilesystemNode object? Most just
trying to open files or check if they are present. The Common::File
class features this (Common::File::open and Common::File::exists). Some
engines are using addDefaultDirectory(Recursive) too (that one is also
not case insensitive and works just with complete paths, maybe it's
better this way, just for your information). So the FilesystemNode API
is virtually only used in the GUI code (gui/*) and the Plugin code
(base/*), with which normal team members never have anything to do.
So (I think at least) nobody have any needs for improvements to the
FilesystemNode API (except the detectors which *all* were using
FilesystemNode::displayName to check if a file is in the game data path
before FilesystemNode::name was introduced).

> Unfortunately, I still do not feel like starting a discussion on the  
> design here, simply because I strongly believe it will lead to  
> nothing anyway. If somebody *does* want to talk about FSNode and how  
> to improve it, that'd be most welcome, but it won't be me starting  
> such a discussion. Because my experience is that nobody on this list  
> is going to participate in the "discussion" (for whatever reasons --  
> because they "do not feel qualified", lack the time, the interest,  
> because they are scarred of me or feel I'll do whatever I please  
> anyway, whatever...). 

For some reasons why maybe nobody would take part in that specific
discussion read the reasons I listed a bit earlier. The problem is that
sometimes nobody used that API, which you want to change and trying to
start a discussion about, really.

> Only fools keep talking to walls, and since I  
> don't like acting a fool that much, I am not really inclined to keep  
> this up (my other mail regarding String is a last attempt, but I  
> guess it's doomed from the start, too).

If you talk about something important and the others don't listen I
think *they* are the fools and not you. That's the same here, you are
trying to talk about important design aspects of ScummVM but nobody
listens, but then again (some) people are integrating your changes into
their code.

> I realize that adding a new port is much more fun than redesigning  
> OSystem or improving the existing SDL backend to properly cope with  
> errors (during res switching, for example -- think transaction  
> rollbacks), and that adding a new engine is more fun than fixing &  
> improving the savegame system. What I realize now is that I can't  
> force interest in these on anybody. So I'll stop trying, and will  
> turn passive for the time being. Should somebody be interested in  
> working on any of these, I can still offer my help to that person(s).

I would say adding something new is always more fun, but then again
someone has to look at the existing things and improve/fix them.
And in the past this person was always you and I for myself I'm glad
that I never had to touch that code. You did (and I hope you will do it
again in the future) in a perfect way, maybe people didn't care about it
because they had other things to do. I think ScummVM would look a lot
different today if you were not working on that code parts. I really
hope you come back soon, since else I fear nobody will look at those
code parts (in the nearer future at least). Then again I can just tell
from the short time I'm in the ScummVM Team.

> Most everybody seems content with the current  
> state of things anyway, so my efforts seem misplaced.

I always try to live with that's there, but I always tried to look at
all commits (except for other engines), to check if something new what
is helpful is there to integrate it into my code. I have to say that I
never asked for something to be improved by myself, but that's mostly
out of laziness, since a quick hack around missing features is most
times faster done and it also works, that's not the best way to look at
things though.

// Johannes




More information about the Scummvm-devel mailing list