Modularize OSystem some more

[fingolfin]

This pull request moves the savefile manager, timer manager and fsfactory "slots" from ModularBackend over to OSystem, along with the default accessor methods. It also changes all backends to use the savefile & timer manager slots; a few also use the fsfactory slot now.

This is similar to what I have done before with the audiocd & event manager. The goal is to make it somewhat simpler for backend authors to setup and manage all these managers (the stats say it all: 156 additions and 263 deletions). Think of it as a slow move towards "modularizing" more backends, only instead of trying to migrate them to use ModularBackend, I am migrating ModularBackend (and its "subsystem slots") into OSystem, as that is easier.

[lordhoto]

Definitly the right way. ModularBackend has to die due all its indirection, i.e. we should rather use a "graphics manager" API than to have some internal graphics manager with virtual functions + the osystem virtual function calls on top. Of course special care has to be taken here, since in the case of the graphics maanger we could have changes of it runtime, like when switching between the SDL and the OpenGL SDL graphics manager in the SDL backend.

[fingolfin]

Yeah... well, I did not touch the graphics manager code here for a good reason ;).

BTW, I also didn't touch the Mixer code, because backends currently always need some methods from MixerImpl to actually use it... this is a bit similar with TimerManager, actually, were backends need to cast _timerManager to a DefaultTimerManager pointer, so they can invoke the handler method. That's slightly ugly, but didn't seem that bad. Ideas for "nicer" solutions that are not overly complex are welcome.

[lordhoto]

Yeah... well, I did not touch the graphics manager code here for a good reason ;).

I know, I was just saying for future changes ;-).

That's slightly ugly, but didn't seem that bad. Ideas for "nicer" solutions that are not overly complex are welcome.

You could have a member in the subclasses with the correct type and set both the OSystem and your member to the instance and only use the one of your subclass in your code. Could need some comment that it is destroyed by OSystem though.

[fingolfin]

The SDL backend solves this by using a TimerManager subclass; resp. by using SdlMixerManager, which keeps an Audio::MixerImpl pointer internally.

But I don't think it's particularly elegant to force all backends to use a TimerManager subclass wich performs the Timer handler hookup.

Similarly: Of course one can keep a pointer of the right type (i.e. the implementation type, like DefaultTimerManager and Audio::MixerImpl) in the backends, but that partially undoes the benefit of not having to have such a pointer in every backend. Of course this drawback is not that bad.

Finally, we could simply (ab)use the fact that currently all backends use DefaultTimerManager resp. Audio::MixerImpl, and just change the corresponding OSystem members / "slots" to those types. The only drawback here is that we make it a bit harder to use implementations that do not subclass these. But that's a fairly hypothetical restriction, isn't it? So maybe that's what we should do?

[fingolfin]

Correction: All backends use DefaultTimerManager resp. Audio::MixerImpl, or subclasses thereof. Which is good enough, I guess :)

[zeldin]

Or you could make handler() a virtual method in the base class which does nothing. Then it can be called without casting, although at the penalty of an indirection.

[fingolfin]

Sure, could do that, too. Not sure how bad that indirection cost would be, I'd hope it won't make a difference...

The other thing I slightly dislike about this is that it puts an implementation detail into the base class: strictly speaking, it's hypothetically possible for a TimerManager to not work using a single handler callback method. Alas, this is indeed a rather hypothetical concern, and it is certainly a lot bet than changing the TimerManager pointer to a DefaultTimerManager pointer.

So, I'd suggest to follow Marcus' suggestion -- unless the indirection due to "virtual" concerns anybody?

[zeldin]

There's no real problem with a subclass that does not work with a single handler callback. It can just implement some other mechanism and ignore the handler() method, it would just be a method that does nothing and is never called. That was why I didn't suggest it to be pure virtual in the base class.

Alternatively, the default implementation could call abort() or something...