scummvm/scummvm — pull request #126 by fingolfin (2011-12-06 19:29:20+00:00)
Title: Dreamweb refactor

Currently, the DreamGenContext class derived from the Context class, and the latter is a kind of "x86 CPU emulator".

This pull requests adds a second base class called "DreamBase" (yeah, I am not very creative today ;) to DreamGenContext. The idea is that as the conversion proceeds, more and more methods of DreamGenContext actually do not need access to the "Context" part (with the possible exception of the "data" segment). Indeed, it is usually helpful to know whether a given method call may modify any of the "CPU registers" in the Context parent or not. Due to complicated call graphs, this can be tricky.

So from now on, whenever a function becomes truly independent from "Context", it can simply be moved into class DreamBase (and its entry in stubs.h can be moved to dreambase.h). This then guarantees that neither the method nor anything it calls can modify the context.

Well, almost... in theory such a method could call a method from DreamWeb, which then could call a DreamGenContext method... but it's a start. The next step then would be change the DreamWeb::_context member to be of type DreamBase to prevent this problem.

Another caveat of this approach is that if a moved function tries to use some of the constants declared in the body of DreamGenContext. After moving the method to DreamBase, access to such constants needs to be prefixed by "DreamGenContext::". I personally would suggest solving this for now by moving those context out of class DreamGenContext; it should be safe enough to have them simply inside namespace DreamGen. Once the conversion is complete, they could be moved again to whatever final resting place seems most sensible.

Lastly, there is an annoyance with overloaded methods that provide both a "clean" version and one callable from not yet converted code. Example:

void printMessage(uint16 x, uint16 y, uint8 index, uint8 maxWidth, bool centered);
void printMessage();

Now, assume for the sake of the argument that the "clean" method (the first one) was ready to be moved to DreamBase. Then this would cause a problem, as we can't move the unclean version (due to it accessing register variables directly). But overloading does not work well across subclasses: The "clean" printMessage in DreamBase would be hidden by the overloaded version.

An easy way out is to simple not move such functions for the time being. Another one is to move it, but to add a thin wrapper in stubs.h for the time being, i.e.
void printMessage(uint16 x, uint16 y, uint8 index, uint8 maxWidth, bool centered) {
DreamBase::printMessage(x, y, index, maxWidth, centered);
}