Quote:Original post by MaulingMonkey
This sounds more like an argument against deep class hierarchies than reference passing -- introducing globals only hides the problem that the tree is unwieldy and opaque to understanding. You don't understand the entire system, you only understand the global -- and since anything can modify globals, you probably only think you understand it! (and what's happening to it)
Isn't that always the case?
One of biggest problems with singleton (as a pattern) is that it mixes implementation detail with design.
In a properly designed system, presence of singleton is not a problem. One way to test whether system depends on singleton is this:
struct Foo { Foo(Bar * bar = BarSingleton::getInstance());};
If the following makes sense, then system is de-coupled to a degree that storage is not mandated.
However, in case of this particular problem, VideoScreen must be Initialize()d - which is a problem, since BarSingleton cannot be auto-constructed as required above (creating a new instance of VideoScreen leaves it in undetermined state), meaning that use of globals is obstructing a bigger problem.
Quote:-> GameApp
--> GamePlayState
---> GameMap
----> Tile
-----> Image // FINALLY gets used here
Hmmm. Warning lights flashing yet?
You've written it down yourself - you hierarchy is not suitable.
Why does image need to know who the VideoScreen is? Image is passive and doesn't do anything to it. Typically, for classes like Image you have something like:
class Image { Image(InputStream * is) // loads from some abstract source { is->readThis(); is->readThat(); is->readSomeMore(); if (not_condition) throw error; } void draw(VideoScreen * vs, int x, int y);};
Why does tile need screen? To know the size of screen? It doesn't need that information. A Tile is dumb.
struct Tile : public Rectangle { // singleton is perfectly fine here in the same way // std::allocator is fine for STL containers // but we are free to choose something else Tile(InputStream * is, ResourceManager * rm = ResourceManager::getSingleton()) { image = rm->loadCached(is->readTileId()); // read more data about what is contained in this tile } Image * image; std::vector<Foo*> contained;};
GameMap doesn't need it either. Game map doesn't care about coordinates available for rendering. GameMap is determined by level designer.
struct GameMap { GameMap(InputStream * is) // load from some abstract source { extents.x = 0; extents.y = 0; extents.w = is->readInt(); extents.h = is->readInt(); for (...) { tiles.push_back(Tile(is, imageCache)); } } void render(VideoScreen * vs, const Rectangle & clip); const Rectangle & getExtents() const { return extents; } ResourceManager imageCache; std::vector<Tile> tiles; Rectangle extents;};
The above example uses typical windowing paradigm, namely view offset is part of VideoScreen (canvas or similar), and render function can query. In addition, we provide clip rectangle to limit what is drawn.
So....
VideoScreen * vs = new OpenGLVS(); // or DXVS() or SDLVS() or WhateverVS() // file, memory, internet, pak, ... whatever source InputStream * is = getReader("levels/level01.map"); GameMap map(is); vs->setViewOffset(50,20); // similar to singleton // unless we really need it, we just use default clip // and redraw everything, but we have the choice of replacing it // to optimize the rendering map.render(vs, vs->getExtents());
So you end up with classes which are independent of each other, yet interoperate in a clear manner.
Note that to create a Tile or Image, you do not need to know anything about GameMap or Game or VideoScreen or anything else. That's not what Image or Tile do - they are dumb and passive and do one thing and one thing only.