Sign in to follow this  
3Dgonewild

Code design

Recommended Posts

I think my "engine" project (its more like a framework..) is ...kinda messed , and im looking for a way to link all seperated modules together. I've been thinking to use polymorphism , but , im not sure if it will work well. Please tell me your opinion.. Example structure:
class Engine
{
public
Engine() ,~Engine();
virtual functions... eg :
virtual void proccessEvents() = 0;
virtual void present();
}

class Event : public Engine
{
	...
void proccessEvents();
}

class SDLCommon : public Engine
{
SDLCommon()
{
start sdl..etc
}
}

class GLRenderer : public Engine
{
setMatrix..
getMatrix..
present()
clearScreen()
putTexture( string& file... ,string name, unsigned int flags )
callTexture(string name)

}

class ConfigUtility : public Engine
{
	...........
}


Im not sure if that's the best way , that's why im posting here. So , what you guys think ? im in the right path...or...{..}?

Share this post


Link to post
Share on other sites
Here are a few basic rules to get your design straight:

  • Keep everything as simple as possible, stay within the bounds of the language as much as possible.
  • Modules should interact through a small set of semantically rich patterns.
  • You should use modules to build other modules.
  • Always think in terms of usage: first determine how a given module will be used.


In your situation, the design seems too complex (everything inherits from Engine for no apparent reason), and the usage is not obvious at all. So, the first question you should be asking yourself is, how should your modules be used?. Write some example code that uses the modules (even if the features are not implemented yet). This will let you determine whether the interface of the modules is easy enough to use.

Share this post


Link to post
Share on other sites
Quote:
Original post by 3Dgonewild
I think my "engine" project (its more like a framework..) is ...kinda messed , and im looking for a way to link all seperated modules together.

Having your separate modules loosely coupled is a good thing. Of course at some point they're going to need to talk to one another and so there are a host of design patterns that allow them to do so whilst also maintaining that loose coupling.

Quote:
I've been thinking to use polymorphism , but , im not sure if it will work well.

Is having a common base class for all those things useful to you? I would generally think it isn't.

Share this post


Link to post
Share on other sites
Quote:

Write some example code that uses the modules (even if the features are not implemented yet).
This will let you determine whether the interface of the modules is easy enough to use.


Wow , great idea.


Hmm , well , lets see...


A simple demo :




//init..
if(!Engine->Init("config_file.cfg")) DIE..;


//how the audio module should work..
Engine->Resource->Audio->Add(AUDIOTYPE_MUSIC,"sample","bgm.ogg");

//texture cache..
Engine->Resource->Textures->Add("cube","cube.tga",maybe some flags..);

Engine->Start();


while(Engine->Running())
{

Engine->Renderer->Clear();
Engine->Renderer->Translate(..);

if Engine->Events->KeyDown(escape key) Engine->Shutdown();
if Engine->Events->KeyDown(up key) Engine->Resource->Audio->PlayByName("sample");
Engine->Resource->Textures->CallByName("cube");
Engine->Renderer->DrawQuad(..);

Engine->Renderer->Present();
}

Engine->Resource->FreeAll();






Now i will have to think how to implement this system........lol


@dmater:
I think im starting to understand how important is to have seperated modules.

Btw , that site looks EVIL ...lol

Share this post


Link to post
Share on other sites
Small tip: You can cleanup the code a bit by using references (or pointers):


//init..
if(!Engine->Init("config_file.cfg")) DIE..;

// Save references
Renderer &renderer = Engine->Renderer;
Resource &resource = Engine->Resource;
Audio &audio = resource.Audio;
Textures &textures = resource.Textures;
Events &events = Engine->Events;

//how the audio module should work..
audio.Add(AUDIOTYPE_MUSIC,"sample","bgm.ogg");

//texture cache..
textures.Add("cube","cube.tga",maybe some flags..);

Engine->Start();


while(Engine->Running())
{

renderer.Clear();
renderer.Translate(..);

if events.KeyDown(escape key) Engine->Shutdown();
if events.KeyDown(up key) audio.PlayByName("sample");
textures.CallByName("cube");
renderer.DrawQuad(..);

renderer.Present();
}

resource.FreeAll();


Share this post


Link to post
Share on other sites
Quote:
Original post by 3Dgonewild
Now i will have to think how to implement this system........lol


Wait. Before you do that, don't forget to simplify your design to the core. Here's some examples:

Quote:
if(!Engine->Init("config_file.cfg")) DIE..;
Too complex. Initialization is already a language-provided feature (in terms of constructors). You can simplify this line to a constructor call (which will throw an appropriate exception if it fails). Also, you need to decouple configuration-loading functionality to a distinct module:
Engine engine( Config("config_file.cfg") );


Quote:
Engine->Resource->Audio->Add(AUDIOTYPE_MUSIC,"sample","bgm.ogg");
Engine->Resource->Textures->Add("cube","cube.tga",maybe some flags..);
First, why pointers? Unless you want to be able to return "null" in any of these situations, you should be using references (or plain old values instead). Also, using a flag instead of a distinct function creates issues as soon as loading music requires different arguments than non-music. So, use a specific function for loading music. Again, separate filenames from actual filename data to account for potential path rewriting. Also, apply a hash function to your strings to make it faster to handle than plain old strings:
engine.resources.add_music( key["sample"], path("bgm.ogg") );
engine.resources.add_texture( key["cube"], path("cube.tga") );


Quote:
Engine->Start();
while(Engine->Running())
{
Engine->Renderer->Clear();
Engine->Renderer->Translate(..);
Engine->Renderer->Present();
}
I strongly suspect that the above will be happening in almost every single game using the engine. Therefore, it should be encapsulated away. I would suggest using a function that runs until the engine decides it should stop, and which calls a provided function on each render frame (and is responsible for setting up the next frame). To allow for time-based game management, I would further provide a time argument to that function. For now, let's suppose that this function is defined elsewhere. The end result is:

struct local
{
bool game_logic(Engine &engine, float time) { /* Game logic here */ }
};

engine.run(local::game_logic);


Quote:
if Engine->Events->KeyDown(escape key)  Engine->Shutdown();
if Engine->Events->KeyDown(up key)
Engine->Resource->Audio->PlayByName("sample");
This is globally correct. However, in order to avoid shutting down the engine then still doing things, I will decide that instead of calling a shutdown function, simply having the game logic function return false will shut down the engine. As for the music, I would separate the "get music" and "play music" parts from one another, perhaps moving the "play music functionality" to an adapted module such as a playlist. Therefore, I would add to the game logic function:

if (engine.input.keydown(KEY_ESCAPE)) return false;
if (engine.input.keydown(KEY_UP))
engine.playlist.start(
engine.resources.get_music( key["sample"] ) );

/* Rendering code here */

return true;


Quote:
Engine->Resource->Textures->CallByName("cube");
Engine->Renderer->DrawQuad(..);
This is a bad idea, because it requires you to work in terms of state. Instead, you can perform the entire quad rendering in a single statement. That statement can of course delay the rendering to a later time, for instance to group quads by texture or to perform geometry instancing.

engine.render.draw_quad( 
transform,
engine.resources.get_texture( key["cube"] ) );


Quote:
Engine->Resource->FreeAll();
Use a destructor.

The end result:
Engine engine( Config("config_file.cfg") );

engine.resources.add_music( key["sample"], path("bgm.ogg") );
engine.resources.add_texture( key["cube"], path("cube.tga") );

struct local
{
bool game_logic(Engine &engine, float time)
{
if (engine.input.keydown(KEY_ESCAPE)) return false;
if (engine.input.keydown(KEY_UP))
engine.playlist.start(
engine.resources.get_music( key["sample"] ) );

engine.render.draw_quad(
transform,
engine.resources.get_texture( key["cube"] ) );

return true;
}
};

engine.run(local::game_logic);

Share this post


Link to post
Share on other sites
Quote:
Original post by ToohrVyk
you need to decouple configuration-loading functionality to a distinct module:
Engine engine( Config("config_file.cfg") );


Small question - would it be worse if you just pass the file name and internally the engine's constructor created a Config object? This will make the constructor call shorter so I'm wandering why you chose to do it this way.

Share this post


Link to post
Share on other sites
Quote:
Original post by Gage64
Quote:
Original post by ToohrVyk
you need to decouple configuration-loading functionality to a distinct module:
Engine engine( Config("config_file.cfg") );


Small question - would it be worse if you just pass the file name and internally the engine's constructor created a Config object? This will make the constructor call shorter so I'm wandering why you chose to do it this way.


When unit testing, I want to be able to build the Config object myself, instead of having to load it from a file. Also, it helps reduce coupling (now, the engine doesn't need to know how the configuration is loaded or created).

Also, I don't want to give my configuration an implicit constructor, which means I'd have a constructor like this:

Engine::Engine(const std::string &filename) :
resources( /* ? */ ), input( /* ? */ ), renderer( /* ? */ )
{
Config config(filename);
}


So the configuration object would only be created after I had made the calls to the constructors which need subsections of said configuration object! C++ takes the middle way between O'Caml and C# here: like C#, it isn't expressive enough to create things "before" the constructor call and, like O'Caml, it does not allow calling a constructor from another. In the end, it's annoying like hell.

Share this post


Link to post
Share on other sites
Quote:
Original post by ToohrVyk
When unit testing, I want to be able to build the Config object myself, instead of having to load it from a file. Also, it helps reduce coupling (now, the engine doesn't need to know how the configuration is loaded or created).


I assumed that the Config object will always be created from a file, but I guess that this is needlessly limiting my options?

Quote:
Also, I don't want to give my configuration an implicit constructor, which means I'd have a constructor like this:

Engine::Engine(const std::string &filename) :
resources( /* ? */ ), input( /* ? */ ), renderer( /* ? */ )
{
Config config(filename);
}


So the configuration object would only be created after I had made the calls to the constructors which need subsections of said configuration object!


Ah I didn't think about that! [smile]

BTW, what do you mean by implicit constructor?

Share this post


Link to post
Share on other sites
Quote:
Original post by Gage64
BTW, what do you mean by implicit constructor?

An implicit constructor is a regular constructor taking a single argument which makes that class implicitly convertible from another type.

When implicit construction is not what you want then you can guard against it with the explicit modifier to make explicit constructors.

Share this post


Link to post
Share on other sites
Er? I'm not aware of anything preventing you from using an explicit constructor in an initialization list? The items of the list are basically (explicit) constructor calls, aren't they?

Share this post


Link to post
Share on other sites

@ToohrVyK:

Thanks for your great help(its not the first time , lol).
I think im ready to start cleaning up this mess.

Quote:

Also, apply a hash function to your strings to make it faster to handle than plain old strings


I was going to use std::hash_map , but for some reason all the stl headers
that i downloaded , simply , messed up the compiler(code blocks),so
i decided to use std::map , and SDBM hash function.

Just now , i did a quick search and i found this
site:
http://marknelson.us/2007/08/01/memoization/

Which explains how std::hash_map can be used on gcc compilers.
I think i'll go with hash maps for all resources..

Share this post


Link to post
Share on other sites
Quote:
Original post by Zahlman
Er? I'm not aware of anything preventing you from using an explicit constructor in an initialization list? The items of the list are basically (explicit) constructor calls, aren't they?

There is nothing preventing you from doing so generally.

The Config object isn't a member attribute, as it's only needed in the constructor, so it cannot be used in the initialisation list. If you want the Engine constructible from a filename string then either the Config needs an implicit constructor or one needs instantiating after the initialiser list - neither is option particularly creamy.

Quote:
Original post by 3Dgonewild
I was going to use std::hash_map , but for some reason all the stl headers
that i downloaded , simply , messed up the compiler(code blocks),so
i decided to use std::map , and SDBM hash function.

To clarify, hash_map isn't in the standard C++ library. I know that SGI have one in their STL implementation though, which leads to continuous confusion with people thinking it is there as standard.

Share this post


Link to post
Share on other sites
@dmatter:
Quote:

To clarify, hash_map isn't in the standard C++ library.
I know that SGI have one in their STL implementation though,
which leads to continuous confusion with people thinking it is there as standard.


Hmm..so , should i implement something on my own with std::map & a custom hash function?


Something like that:




Texture cache - example - :

class Texture
{
public:
Texture(const unsigned int udata,const unsigned int tid,const int tw,const int th)
:data(tdata),hashedID(tid),w(tw),h(th){}
unsigned int data,hashedID;
int w,h;
};

//id(hash) //texture
std::map<unsigned int,Texture> textures;

addResouce-Texture(const std string& name,const std::string& file)
{
Texture t(.....);
map.add<make pair buildHash(name) , t)
}

call texture(const std string& name)
{
unsigned int id = buildHash(name)
map.find(id)
bind / etc..
}

Share this post


Link to post
Share on other sites
Quote:
Original post by Zahlman
Er? I'm not aware of anything preventing you from using an explicit constructor in an initialization list? The items of the list are basically (explicit) constructor calls, aren't they?


Consider this code:

Engine::Engine(const Config &conf) : renderer(conf), resources(conf), playlist(conf) {}
Engine engine(Config("filename"));


How can you rewrite this function so that Config does not have an implicit constructor, yet the following code is equivalent to the code above in terms of functionality?
Engine engine("filename"));


In terms of OCaml, I'm saying that C++ does not support this:
class engine(filename) = 
let config = new config(filename) in
object
val renderer = new renderer(config)
val resources = new resources(config)
val playlist = new playlist(config)
end

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this