Engine: allow modules to access eachother (singleton?)

Started by
24 comments, last by Subotron 16 years, 3 months ago
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(c);   }   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).
Advertisement
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.
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()?)
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.
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.
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.
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 modelsvirtual CustomInit();Engine::Init(){    try    {        CustomInit();    }    catch ( Exception )    {        ErrorHandler->Handle( Exception );    }}

And just throw exceptions in the constructor of a model if something goes wrong?
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.
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 userEngine::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.
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]

This topic is closed to new replies.

Advertisement