[Scummvm-devel] SDL backend modularization remarks

[Max Horn]

Hi Alejandro,

your welcome. I just saw you made some new commits.

Am 03.06.2010 um 01:25 schrieb vgvgf:
[...]

> 
> Regarding the Feature methods, the plan was to have the backend class
> OSystem_SDL call each backend feature methods and then return an OR
> comparison of all the results for example the hasFeature function.
> Going now with the Manager classes, the function names shouldn't be
> dangerous anymore, but if it's desired I'll make specific names for
> each subsystem as you suggest.

If the classes involved are completely independent from each other (seems to be the case to me), then of course there is no need to rename the feature methods for each. However, I still wonder whether we really need to have empty feature check methods in every "Manager" ? I'd prefer adding them only if we determine that we *really* need them ;). In particular in the "mutex manager", I have a hard time to imagine we'd ever need them -- and if we *do* need them, we can still add them, in a way that suits our hypothetical future needs optimally.

Likewise, I am not quite sure what purpose DefaultAudioManager serves, it seems to be an empty class inserted between Audio::MixerImpl and hypothetical subclasses of it. Again, I fail to see the purpose of this -- note, I am not saying there *isn't* one, maybe, I just don't see if, and your code and its comments don't really give one, either :).

But maybe as soon as the rest of your code hits the repository, it will all become crystal clear :).


> Regarding the Audio CD functions, I was only separating the actual sdl
> code before the real refactoring, and I realized that it really
> doesn't belong to the audio subsystem and isn't necessary for many
> ports. As you say, a default empty Manager class for CD funtions would
> work fine, and the OSystem functions should be made pure virtual.

Pure virtual? That would force all subclasses to implement them, the opposite of what we do right now (providing empty default implementations, to make sure no backend needs to override them).
The long term plan would in fact be to get rid of the Audio CD methods in OSystem completely, in favor of a revised AudioCDManager, as described in my previous post :).



Bye,
Max