25 replies to this topic

[Subotron]

I'm currently designing what I call an engine, but is basically a framework for my upcoming game project. I'm having some serious issues concerning access of modules by different modules. My engine base class is the EngineApp, which consists of a number of modules, like a texture manager, window manager, renderer, error manager, etc. Now, this EngineApp should, in my opinion, be globally accessible because each module might need to access different modules. For example, if something goes wrong in the texture manager because the renderer tries to access a non-existant texture, the error manager should handle the error, which in turn may mean it needs to tell the window manager to exit. Since these modules are all defined in EngineApp, the simplest thing to do (to me) is to make the engine app a singleton class. However, in a game, it would be useful to derive a class from the EngineApp (say GameApp). However, if I make the EngineApp singleton, the GameApp class will call the functions from EngineApp (so GameApp::GetSingletonPtr()->Run(); would actually execute EngineApp::GetSingletonPtr->Run();). This is not intended obviously. Is there a way, with or without singleton usage, to fix these global access problems? I could obviously pass pointers to all modules that are needed when working in some module, but that doesn't seem like a very good idea to me. Now I know how most people think about singletons, and I try not to use them if I can do without, but for now I don't see another solution that works equally well. Who can help me? Edit: not that I think it matters, but for the record, the engine code should ultimately be put in a lib or dll, while the game code (and thus also the GameApp class) is in another project that uses the library.

[OrangyTang]

Quote:

Original post by Subotron I could obviously pass pointers to all modules that are needed when working in some module, but that doesn't seem like a very good idea to me.

Why doesn't this seem like a good idea?

[Gage64]

Quote:

For example, if something goes wrong in the texture manager because the renderer tries to access a non-existant texture, the error manager should handle the error, which in turn may mean it needs to tell the window manager to exit.

This way the error manager will have to know about all the modules, and know how they want to handle errors. It would be much better if the error manager would fire an event saying there was an error (with the relevant information), and the other modules would respond accordingly. That is, the error notification should be completely separate from the error handling (in fact, that's exactly what exceptions and error codes are for).

[Subotron]

Because for the described scenario, the renderer would need a pointer to the texture manager, error manager, and as the engine expands probably a lot more. That would give messy constructors with 10 parameters that will always have the same value. Engines I used in the past (not my own) don't seem to take this approach either, and I was always very happy about that :p

[Subotron]

Quote:

Original post by Gage64 Quote: For example, if something goes wrong in the texture manager because the renderer tries to access a non-existant texture, the error manager should handle the error, which in turn may mean it needs to tell the window manager to exit. This way the error manager will have to know about all the modules, and know how they want to handle errors. It would be much better if the error manager would fire an event saying there was an error (with the relevant information), and the other modules would respond accordingly. That is, the error notification should be completely separate from the error handling (in fact, that's exactly what exceptions and error codes are for).

I guess you are right. You kind of caught me, I don't do error _handling_ yet, mostly just logging of the error and quitting. But either way, the problem still exists that the module where the error occurs should call the error handler. I could just pass a pointer to the error handler for every module, but that really doesn't seem like an elegant solution to me. Is this just me?

[Gage64]

Quote:

That would give messy constructors with 10 parameters that will always have the same value.

This suggests that there isn't enough abstraction/indirection in your design. You might want to look at the Facade Pattern for a simple example of this. Also, some of the articles in the first link in my sig might also be helpful.

[OrangyTang]

Quote:

Original post by Subotron Because for the described scenario, the renderer would need a pointer to the texture manager, error manager, and as the engine expands probably a lot more. That would give messy constructors with 10 parameters that will always have the same value. Engines I used in the past (not my own) don't seem to take this approach either, and I was always very happy about that :p

And lo, we find the real problem - too many cross-dependancies between systems. Or, more likely, you think theres too many dependancies between systems. Often when you actually start passing systems as parameters then you'll find you have much less cross talk than you initially thought. Give it a go and see. If you still have too many dependancies then take a look and see which ones are reasonable and which ones are inappropriate and see how you can remove or reduce them.

[Gage64]

Quote:

the problem still exists that the module where the error occurs should call the error handler.

Not really. The module should only report the error.

I don't know the best way to implement this, but for example, your error manager could allow other modules to register handlers for specific events. When a module generates an error that might interest other modules, it will tell the error manager about it, and the error manager will in turn iterate through the list of registered handlers for that error and call each one with the relevant error information. Note that the error manager doesn't know anything about the modules that are registering handlers (the error "listeners"). It simply calls the handlers, not knowing to who they belong.

Again, I don't know if this is a good implementation (in fact, I think it's not...), but it demonstrates the indirection I was talking about earlier.

[Subotron]

Quote:

Original post by Gage64 Quote: That would give messy constructors with 10 parameters that will always have the same value. This suggests that there isn't enough abstraction/indirection in your design. You might want to look at the Facade Pattern for a simple example of this. Also, some of the articles in the first link in my sig might also be helpful.

I'm not quite sure if I understood, but would the EngineApp class in this case be the facade? Basically this simply means that this Facade hides the nasty code with pointer passing and such so that the end user doesn't notice it happening?

[Gage64]

Quote:

I'm not quite sure if I understood, but would the EngineApp class in this case be the facade? Basically this simply means that this Facade hides the nasty code with pointer passing and such so that the end user doesn't notice it happening?

I'm not sure that's a good example. I think the subsystems in the engine are too varied to allow you to give them one interface (at least not one that isn't composed of 300 functions...).

One example that comes to mind is the rendering system. It has many internal components that are conceptually separate (the transformation pipeline, the clipper, the lighting processor, the texture mapper, the rasterizer, ...), but you can have one interface that simplifies the interaction with all these systems and even hides some of them from you (sorry if this is a bad example, maybe someone can give a better one).

[rip-off]

You can make modules unaware of each other using signals and slots:

class CriticalErrorHandler {
public:
    virtual ~CriticalErrorHandler(){}
    virtual void handle() = 0;
};

class ErrorHandler {
public:
   void addCriticalErrorHandler( CriticalErrorHandler * c )
   {
        criticals.push_back©;
   }

   void onCriticalError()
   {
std::for_each(criticals.begin(),criticals.end(),std::mem_fun(&CriticalErrorHandler::handle));
   }
private:
   std::vector<CriticalErrorHandler *> criticals;
};

class Window {
public:
    void close(){ std::cout << "closing window!" << std::endl; }
};

class CloseWindowOnCriticalError : public CriticalErrorHandler {
public:
    CloseWindowOnCriticalError( Window *window ) : window(window) {}
    virtual void handle()
    {
        window->close();
    }
private:
    Window *window;
};

int main()
{
    Window window;
    ErrorHandler errors;

    errors.addCriticalErrorHandler( new CloseWindowOnCriticalError(&window) );

    errors.onCriticalError();
}

Note how the Window knows nothing about the ErrorHandler and the ErrorHandler knows nothing of the window. Using boost we could clean it up with smart pointers etc. This is a simplified example. Boost has signals and slots stuff too, but I haven't really used them (most of the stuff I do with events like that is in Lua, so everything is just a Lua function).

[Spoonbender]

Quote:

Original post by Subotron Because for the described scenario, the renderer would need a pointer to the texture manager, error manager, and as the engine expands probably a lot more. That would give messy constructors with 10 parameters that will always have the same value.

You mentioned two so far. What are the other 8?
My point is, obviously, that you can't really design your code around "probably". Perhaps one day you'd need to pass in 800 references. Or perhaps not. All you can do is look at what you have now, and what you're going to need. Which 10 components is the renderer going to need access to? And why?

So far, your renderer needs access to two components (texture and error). Why couldn't those be passed in the constructor?

And what is the alternative? Having it access the same 10 different components, *without getting references to them*? That's just as messy, but now you've hidden the dependencies behind a singleton. Now you don't even *know* which components depend on what. That is messy.

Of course, to tidy things up, you can always bundle a few parameters together in a struct before passing them to the renderer. Give it a struct containing references to texture manager and other graphics-related components all in one neat package. Without using singletons or other kinds of glorified globals.

[Gage64]

Quote:

Original post by rip-off You can make modules unaware of each other using signals and slots: ...

Shouldn't Window know about ErrorHandler? How else will it notify that there was an error? (and who calls onCriticalError()?)

[rip-off]

Quote:

Original post by Gage64 Quote: Original post by rip-off You can make modules unaware of each other using signals and slots: ... Shouldn't Window know about ErrorHandler? How else will it notify that there was an error? (and who calls onCriticalError()?)

Well, someone needs a reference to the error handler to trigger the error. Personally I think the error handler is a poor example, I use exceptions and in my code the window will be closed if the exception stack unwinds past its scope. But I worked with the example I was given.

[Subotron]

Ok, what if an end user would want to load a model and add it to the scene manager (or whatever). The user would do something like

smart_ptr<Model> = new Model( fileName );
SceneMgr->Insert( model );

at least, that's what I'd like it to be like. Now suppose something goes wrong while creating the model (say, the file doesn't exist). This error would have to be logged and handled, but for this to work the model instance would need access to the error handler. This would change my call to something like this:

smart_ptr<Model> model = new Model( fileName, errorHandlerPtr );
SceneMgr->Insert( model );

The same would go for any other call to an engine function done by the end user. Now, instead of using the constructor of the model I could also write an Init function that returns an error code if something goes wrong, which could then be sent to the error handler, but that would only make things worse code-wise:

Model* model = new Model();

int ret = model->Init ( fileName );

if ( ret > 0 )
{
    ErrorHandler->Handle( ret );
}
else
{
    SceneMgr->Insert( model );
}

What I'd want is the constructor to initialise the object, and be able to notify the engine if some error occured, and if possible I'd stay clear of having to pass the error handler for object that is created. Also, in my engine managing objects (memory) is done by the manager, so preferably an object always gets inserted into the manager, but when something goes wrong while creating it the manager deletes the object, thereby making sure there are no memory leaks. Or, to state it differently, I'd like to pass ownership of objects to their respective managers asap so I dont have to worry about the memory usage anymore.

[ToohrVyk]

People often say that one's design should be modular. Sadly, many people take this as meaning «use modules.» Having modules in a program does not mean that the program is modular. This is generally the point where I whip out the strong coupling and zero-dependency diagrams and beat your brain into submission, but my law school exams have been going pretty well so I'll try a nicer approach today.

You know what's modular in the real world? Condoms. They can be used as a contraceptive, they can be used to prevent STDs, they can be used as a barrel plug on paintball guns, they can be used to protect a live gun barrel from moisture when wading through rivers, they can be used to smuggle liquids or powders, and so on. And what makes condoms so modular in the first place?

• Easy to set up. Rip the pack open, unfold, perform task. You should nt have to go through a dozen hoops and fill ten forms to get a working object.
  
  
• Simple and general concept. It's an elastic, impermeable, transparent pocket. Although its primary use (the one it's optimized for) is prevention of insemination, it is not restricted to doing only that. In the same way, when you design a portion of your game, determine if the code you are writing is not overly restrictive. Perhaps it could be used to do other similar things in other places?
  
  
• High availabiliy. Condoms are extremely cheap and easy to get your hands on, which proves they have not been designed by your average software engineer. I can see it now:
  
  «The user will only need one condom to get his thing on, right?
  — Right. Singleton?
  — Deal.»
  
  The fundamental point to condoms is that you can get as many as you need. If you design your objects in terms of "you only need one" then by design you are restricting reuse.
  
  
• Standalone. It doesn't require other things to work. You don't have to plug it in. You don't have to use an Adapter pattern to fill it with distilled vodka. You don't have to set up a render-target window and you don't have to run a network thread in the background. Have your objects work on their own, like a condom: a condom needs itself, and an object to be placed around. Your modules should need themselves, and a target task to be placed around.

[Subotron]

in response to my last post, would it be a good idea to combine the error handler with exceptions here to avoid having to pass the error handler? So something like this:

// Function in the engine that is overridden by the end user to load models
virtual CustomInit();

Engine::Init()
{
    try
    {
        CustomInit();
    }
    catch ( Exception )
    {
        ErrorHandler->Handle( Exception );
    }
}

And just throw exceptions in the constructor of a model if something goes wrong?

[ToohrVyk]

Quote:

Original post by Subotron in response to my last post, would it be a good idea to combine the error handler with exceptions here to avoid having to pass the error handler?

I would rather use a functional approach. Caml pseudocode:

(* Unhandled code *)
let _ = frobnicate(42)

(* Handler *)
let handle_not_found f =
  try 
    f ()
  with
    | Not_found -> 91

(* Handled code *)

let _ = handle_not_found (fun () -> frobnicate(42))

Other than that, I'd say "handle the error" is a bad abstraction to add yourself, because it's already provided by exceptions.

[Subotron]

Thanks for responding, but that pseudocode doesn't really clear things up. Probably because I don't know Caml.

I think I do get what you mean by it being a bad thing to add the abstraction ErrorHandler->Handle( Exception ). Handling it can be done locally in such a case, but the error still needs to be reported. So I made a small change in the code below.

From what I read the past hours, about RAII and other stuff, I understand my proposed method would in fact be a good way to go. If the function looks like this:

Engine::Init()
{
    try
    {
        CustomInit();
    }
    catch ( Exception )
    {
        Logger->Write( Exception );

        if ( Exception.level == ERR_CRITICAL )
            Exit();
    }
}

// Override done by the end user
Engine::CustomInit()
{
    boost::shared_ptr<Model> mdl = new Model ( file ); // If an exception is thrown, the model cleans itself up and re-throws the error
    SceneMgr->Insert ( mdl );
}

In case of everything loading fine, no problems. In case of errors, the model never gets added into the scene manager because an exception is thrown. The model will be properly deleted due to the smart pointer, and without having to pass the logger (or error handler) the engine is still notified of the error, and might respond to it if necessary.

This would mean I only have to pass the error handler/logger to modules, and just put other end-user related code that might throw engine exceptions inside a try { ... } catch { ... } block to catch the errors. End users could add custom exceptions and ways to handle them, so there's no restrictions there.

Is there anything wrong with this approach? Will it suffice if the CustomInit function does more than just load one model, or should those different actions be surrounded by additional try - catch blocks? I mean, say I load 3 models, and the second one throws an exception. This doesn't necessarily mean I can't keep using the 1st (and 3rd) models, but it might. As far as I see, catching the exception this way might ruin things for me because everything executed after throwing the exception never actually gets executed. What if some not-so-serious error occures at the first step of CustomInit, and I still want to execute the remainder of the function?

Thanks for all the help so far, I think I already got a better understanding but I want to get this right, since it is the basis for most of the engine.

[MJP]

For your constructor initialization problem...I'm not sure if it's necessarily the best way to solve it, but you could use some sort of factory function for instantiation of your Model class. This would allow you hide the details of your error/exception handling from the code that's using your framework, and give you an opportunity to pass it references to modules that are internal to your frame work (texture managers, error handlers). This approach also gives you some other interesting options, such as using a custom memory allocator to allocate memory for the models. Or you can have your function return an abstract interface class, if you want to completely hide the public interface from the implementation.

Just trying to give you some ideas to work with, at any rate. [smile]