Sign in to follow this  
iLoveGameProgramming

Structure of classes in good Game Engine?

Recommended Posts

Hello everyone :)

Well I'm making a game with a friend, and we're having small problem with organizing our classes.
What we're doing is having main engine class, which creates all classes we use in it, then we pass the
classes into constructor that we want to use.. eg:
[source lang="cpp"]gameWorld = new ChunkManager(LogSys, gfxSystem);
gameCamera = new Camera(ScreenWidth, ScreenHeight, 0.0f, 0.0f, 0.0f, SDL_Key, events,joystick1, LogSys);
Time = new GlobalTime(LogSys);
Menu = new SDL_Menu(this,gameCamera,events,Time,LogSys);[/source]
The thing is, we pretty much pass in LogSys class to EVERYTHING!
Is there any easier way to distribute it to pretty much everything?
I was thinking of inheriting it to everything, but i heard inheritance is bad in some cases to game engines..?


Well thanks for help in advance, cant wait to see the answers! :)

Share this post


Link to post
Share on other sites
Inheritance is not bad. It can be bad if you do it wrong or too much. Inheritance is very good when you want substitutable behavior. Take a look at the C++ Faq on inheritance:

[url="http://www.parashift.com/c++-faq-lite/proper-inheritance.html"]http://www.parashift.com/c++-faq-lite/proper-inheritance.html[/url]

On your LogSys case, I would go for a global as MajorTom suggested. Take a look at your sentence "[color=#282828][font=helvetica, arial, verdana, tahoma, sans-serif][size=3][left][background=rgb(250, 251, 252)]Is there any easier way to distribute it to pretty much everything?" [size=4]well, globals are for eveything everywhere.[/size][/background][/left][/size][/font][/color]

Share this post


Link to post
Share on other sites
Debug utilities (ie logging) is one of the very few things which I consider suitable for global access (ie global variable, global function, singleton, ..). If you have other stuff you need everywhere, that might indicate a code design/architecture problem.

Share this post


Link to post
Share on other sites
Personally I wouldn't bother with either.

There are already a bajillion existing logger systems out there, most are far more comprehensive than an individual would ever use. The better systems integrate with your debugger, allow multiple simultaneous output methods including logging across networks, provide graphical visualizations when comparing bulk logs, and do much more than simply output individual text blobs.

Why re-invent the wheel?

Share this post


Link to post
Share on other sites
[quote name='ApochPiQ' timestamp='1340052689' post='4950376']
Keep in mind that your logging system probably doesn't need to be a class at all. Consider using a simple free function instead, possibly in a namespace.
[/quote]

This.

If you still decide it needs to be a class, ask yourself: does it need to be instantiated? What benefit do I get for having two or more instances of logger class? If there's none, then you don't need a class.

Share this post


Link to post
Share on other sites
You can do something like this

[CODE]
class Log {
public:
static Log* Get() {
if( !m_pInstance )
m_pInstance = this;
return m_pInstance;
}
protected:
Log* m_pInstance;
};

Then use the log like this within your classes
Log* pLog = Log::Get();
pLog->Write( "Me and the Cap't make it happen!" );
[/CODE]

Share this post


Link to post
Share on other sites
[quote name='bglanzer' timestamp='1340055837' post='4950394']
You can do something like this

[CODE]
class Log {
public:
static Log* Get() {
if( !m_pInstance )
m_pInstance = this;
return m_pInstance;
}
protected:
Log* m_pInstance;
};

Then use the log like this within your classes
Log* pLog = Log::Get();
pLog->Write( "Me and the Cap't make it happen!" );
[/CODE]
[/quote]


This is the "singleton pattern" or as I like to refer to it "the greatest sin perpetuated by programmers with good intentions."

The singleton thing has been debated to death, so if you're interested, I invite you to go search around for any of the dozens of threads on the subject. For now, though, I will leave it at this: please, don't use that code. Ever.

Share this post


Link to post
Share on other sites
I often times see Sington's used within references. For loggers particularly. I think its used that way in the Hieroglyph Engine from the book Practical Rendering & Computation. They have thier place but like ApochPiQ said they really shouldn't be used. They have the same issues as global variables so you have to take the same precautions.

The other way I've seen loggers done is passing a pointer to your logger on creation just like you are currently doing. I think that Method is used in 3d Game Engine Architecture. This is probably the method I would recommend.

Share this post


Link to post
Share on other sites
I don't want to address the logger, but I would like to address your hate for globals. I admittedly don't have a ton of experience with games in particular, but I do have a lot of application development experience, and I have worked on some very performance sensitive applications. The one thing I will say is, having a global "Application" instance is not a bad thing. Most programs need this sort of thing. This "Application" class will typically hold references to various subsystems of your application. This class HAS to be a singleton. It is an error for there to be more than one. If you are familiar with iOS development, it is the "AppDelegate" class. Obviously global variables all over your code are a terrible idea because it increases coupling and makes debugging a mess. But having a single class that represents your application (which can be represented as a singleton or a global variable declared in your main class. I'm fairly certain the compiler doesn't care, and will treat them both the same, though I could be wrong on that) is not a bad thing, and will save you a lot of headaches especially in a large application like a game.

On that note, your logger (if you choose to keep it as a class) could be a class member of your global Application class, so that you don't feel all dirty making it a global, and it still can remain at your disposal anywhere in the code.

I will say this though as parting words: if you find yourself passing an instance of a class to every single constructor or function call, the class is already tightly coupled with your code. There's no point in trying to fight it. It is meant to be a global.

As a note to future posters, like I said, I'm not pro on the subject of game dev, I'm just learning it myself, so I welcome criticism of my response from that perspective.

Share this post


Link to post
Share on other sites
Wouldn't a free function require opening and closing of the log file? Would that cause any type of slowdown compared to a class which opens the file on creation and closes before destruction?

It would make it alot easier to not have to pass the Log to each class that requires it.

Share this post


Link to post
Share on other sites
[quote name='ApochPiQ' timestamp='1340059000' post='4950408']
[quote name='bglanzer' timestamp='1340055837' post='4950394']
You can do something like this

[CODE]
class Log {
public:
static Log* Get() {
if( !m_pInstance )
m_pInstance = this;
return m_pInstance;
}
protected:
Log* m_pInstance;
};

Then use the log like this within your classes
Log* pLog = Log::Get();
pLog->Write( "Me and the Cap't make it happen!" );
[/CODE]
[/quote]


This is the "singleton pattern" or as I like to refer to it "the greatest sin perpetuated by programmers with good intentions."

The singleton thing has been debated to death, so if you're interested, I invite you to go search around for any of the dozens of threads on the subject. For now, though, I will leave it at this: please, don't use that code. Ever.
[/quote]

Singletons used incorrectly are a bad idea, but Singletons do have their place. There are times when having two instances of a class is an error and may/will cause your program to malfunction. I am well aware of the downsides (increased strong coupling, problems with unit testing, debugging headaches), but there are times when having just one instance of a class is needed. I will say though, that a logger class is not one of those cases, having two logger classes shouldn't break your application. There are situations though where a singleton pattern makes sense, and not only can be used, but should.

However, it goes without saying that code is crap =) I don't think the poster was intending to provide copy/paste code (at least I hope not!) Edited by metsfan

Share this post


Link to post
Share on other sites
[quote name='ApochPiQ' timestamp='1340065928' post='4950435']That still raises the obvious question: why is your logger an object/class to begin with?[/quote]Because it has state. An overly generic logger will likely have at least a file-pointer as state, perhaps also a "warning level" to ignore, a verbose flag, etc... You might even have one part of your app plugged into a network logger and another part plugged into a local file logger.

Though I know this isn't what you're getting at [img]http://public.gamedev.net//public/style_emoticons/default/wink.png[/img] You're getting at the point that a game's logger [i]doesn't need to have state[/i].

In my new engine core ([url="http://code.google.com/p/eight/source/browse/src/core/core.cpp"]here[/url] and [url="http://code.google.com/p/eight/source/browse/include/eight/core/debug.h"]here[/url]), I follow ApochPiQ's advice and only provide logging via a global/free function. Despite that, I can still filter out different engine sub-systems, and later I can still add features like network/disk logging, browser-display/formatting, etc...
[code]eiInfoGroup( FooBar, true );//declare and enable log filter "FooBar"
eiInfo(FooBar, "couldn't frobnicate %d", foo );//Print to log, or don't depending on filters.[/code]
[quote name='metsfan' timestamp='1340068197' post='4950445']
Singletons used incorrectly are a bad idea, but Singletons do have their place. There are times when having two instances of a class is an error and may/will cause your program to malfunction.[/quote]Can you name one? I've been trying for a while to find a justification, but my current engine still has zero. I know that in theory, singletons or monostates are the solution to the problem of "[i]2 instances are always an error[/i]", but I'm not sure that this problem actually exists in the real world...
[quote name='bglanzer' timestamp='1340067880' post='4950443']Wouldn't a free function require opening and closing of the log file? Would that cause any type of slowdown compared to a class which opens the file on creation and closes before destruction?[/quote]Not saying you should, but free-functions can still have hidden state. e.g. in the CPP that implements the function, you can have a file-static variable that holds the file pointer. From the outside, it acts like a free function, but internally, it's basically a singleton with it's instantiation policy hidden.
N.B. I've seen some loggers continuously open/close the file on purpose, because it ensures that if your program crashes, the log is always written out correctly ([i]normally, the latest log data would be in a buffer, and maybe not yet on disk[/i]).

However, for the last few years, no game engine that I've worked on has logged straight to disk like this. The last ~3 engines I've used, all have a "companion" developer application that you run alongside the game -- this usually just sits down in your system tray, and communicates with the game over a socket. When the game needs to perform any dev duties, like logging, the game sends the data over the socket to this companion app. The game doesn't need to know how to rebuild assets or write logs, it just needs to know how to ask the companion/dev app how to do these things.

[quote name='iLoveGameProgramming' timestamp='1340048818' post='4950354']
The thing is, we pretty much pass in LogSys class to EVERYTHING!
[/quote]As above, I think logging should be implemented a a simple global function -- but if this was anything other that logging, then you're doing it correctly.
For emphasis, 99.9% of the time, I don't allow people to use [font=courier new,courier,monospace]new[/font]/[font=courier new,courier,monospace]malloc[/font] in my engine -- they've got to be passed an allocator object if they want to allocate memory. Imagine all the classes which I've got to pass around allocators between! Yet, I still believe this approach is more correct, as it makes your code slightly more functional and have no hidden dependencies/side-effects. Edited by Hodgman

Share this post


Link to post
Share on other sites
[quote name='Hodgman' timestamp='1340072749' post='4950460']

However, for the last few years, no game engine that I've worked on has logged straight to disk like this. The last ~3 engines I've used, all have a "companion" developer application that you run alongside the game -- this usually just sits down in your system tray, and communicates with the game over a socket. When the game needs to perform any dev duties, like logging, the game sends the data over the socket to this companion app. The game doesn't need to know how to rebuild assets or write logs, it just needs to know how to ask the companion/dev app how to do these things.
[/quote]

So those game engines are incapable of enabling any logging in production code? They likely still need state in any case, unless they're opening and closing a socket or pipe with every log request.

I would really just stick to using a global for the logger. Overuse of globals is a bad thing, but bending over backwards for ideological purity and compromising the complexity of your code for the sake of not using any globals is much worse in my opinion. The goal of development is to produce a product, not a human rights treaty.

As long as you follow a good practice of explicitly initializing and cleaning up the state of your globals in discrete locations (i.e. in your main()) instead of relying on constructors and destructors that may be called before or after it is safe to do so, there is no problem with using globals, or singletons either, really.

You can use a free function that will access the global logger object to do the actual logging if you want, to avoid littering your code with calls to global variables. Personally I like to use macros to call my logging and debugging constructs. That way I can implicitly pass things like __FILE__ and __LINE__ to the logger, and I can easily replace whatever logging/debugging implementation I want to use later if necessary.

Share this post


Link to post
Share on other sites
[quote]Can you name one? I've been trying for a while to find a justification, but my current engine still has zero. I know that in theory, singletons or monostates are the solution to the problem of "[i]2 instances are always an error[/i]", but I'm not sure that this problem actually exists in the real world...[/quote]

In the case of PC game engines I doubt there are very many cases where a "2 instances are always an error" situation will exist, but there are many cases in the real world. Particularly when you have classes that instance third-party libraries and/or drivers that can't handle multiple interfaces in the same process. i.e. a while back I was working on an application that interfaces to control modules on vehicles, and I had to use a vendor-supplied DLL to communicate with the control modules through another piece of vendor-supplied hardware. Any attempt to instantiate more than one interface to the vendor-supplied hardware from the same process could cause problems, up to and including damaging the control module and rendering the vehicle useless until the control module was replaced or repaired. While 2 instances of a class that can interface to the vendor's DLL isn't necessarily an error, it is a huge financial risk, which is even better ;)

Share this post


Link to post
Share on other sites
[quote name='Hodgman' timestamp='1340072749' post='4950460']
Can you name one? I've been trying for a while to find a justification, but my current engine still has zero. I know that in theory, singletons or monostates are the solution to the problem of "2 instances are always an error", but I'm not sure that this problem actually exists in the real world...
[/quote]

The best example I can think of is the one I mentioned, which is having a singleton to represent the application itself. An example would be the UIApplication class in iOS. UIApplication is implemented as a singleton because the class represents the application itself. Since there is only one application, using a singleton class to represent our application makes sense. I'm not saying this is the only way to represent an application, but it is a very intuitive and well established way of doing so.

Share this post


Link to post
Share on other sites
[quote name='krippy2k8' timestamp='1340075563' post='4950467']So those game engines are incapable of enabling any logging in production code? They likely still need state in any case, unless they're opening and closing a socket or pipe with every log request.[/quote]They have to use a [i]different[/i] logger implementation for retail builds. It could just swallow/ignore log entries ([i]and probably would ignore/disable a lot of logging channels[/i]), or pipe them to the game's console ([i]maybe colouring channels[/i]), or a text file inside [font=courier new,courier,monospace]%appdata%[/font], etc... You don't need to inherit an interface or anything to do this, a simple [font=courier new,courier,monospace]ifdef[/font] to select different implementations is often good enough.
[quote name='metsfan' timestamp='1340076839' post='4950475']The best example I can think of is the one I mentioned, which is having a singleton to represent the application itself. [i]Since there is only one application, using a singleton class to represent our application makes sense[/i][/quote]
It makes it [i]convenient[/i], but doesn't necessarily make sense. What is the [i]one application[/i] that's running? Isn't this just a grouping of global variables into another (global) structure called 'application'? You could just make a few more globals instead and get rid of the grouping, or not even make them global ;) What if I want to launch a new process in the background, or send a message to another app -- in those cases there is more than one application in the problem domain..?[quote]An example would be the UIApplication class in iOS. UIApplication is implemented as a singleton because the class represents the application itself. I'm not saying this is the only way to represent an application, but it is a very intuitive and well established way of doing so.[/quote]I'm not familiar with iOS, but from scanning the [url="http://developer.apple.com/library/ios/#DOCUMENTATION/UIKit/Reference/UIApplication_Class/Reference/Reference.html"]UIApplication[/url] docs with, it seems to be written in the kind of OOP style where you dump a whole bunch of useful functions into a class and then inherit from it. This style also often uses singletons to make it easy to get at those functions. That's fine, each style to it's own, but this is still an arbitrary choice by the API designer to use a singleton as a restriction on the end-user developers usage. You could replace the [font=courier new,courier,monospace]UIApplication[/font] singleton by, inside [font=courier new,courier,monospace]main[/font], making local variables for a window, an event queue, a URL fetcher, etc... and then having the choice weather to make them global variables ([i]e.g. via a singleton[/i]) or not, [i]ourselves (i.e. the end-user of the API)[/i]. Edited by Hodgman

Share this post


Link to post
Share on other sites
[quote name='Hodgman' timestamp='1340081228' post='4950487']
They have to use a different logger implementation for retail builds. It could just swallow/ignore log entries (and probably would ignore/disable a lot of logging channels), or pipe them to the game's console (maybe colouring channels), or a text file inside %appdata%, etc... You don't need to inherit an interface or anything to do this, a simple ifdef to select different implementations is often good enough.
[/quote]

I prefer a single logging object with multiple logging channels, where each channel can be of a different type. i.e. a FileLoggingChannel for writing to file, a ConsoleLoggingChannel for writing to the console, a SocketLoggingChannel for sending the logs over the network to an external logging application/server. Each channel type just implements a simple interface like:

[CODE]
class ILogChannel
{
public:
virtual void open(){}
virtual void close(){}
virtual void log( const LogMessage& msg ) = 0;
};

[/CODE]

and is responsible only for outputting the log message object. The main logging object handles adding file, line, function info, process and thread IDs, timestamps, possibly even call stacks when logging exceptions/errors, etc, to the message object. All about flexibility. If you want to use an external debugging tool, the game/application can even detect the presence of the tool when it starts up and add a SocketLoggingChannel if the tool is running, or a FileLoggingChannel if it's not. Then you can pass the same build around to different machines without having to recompile or distribute the debugging tool (or just use a config file).

I would also like to point out that opening/closing files on every log entry as a way of flushing the buffers is a little silly. Unless you're using something like std::fstream that uses it's own internal stream buffer, you usually don't need to worry about a process crash; OS disk buffering is typically done via a system write-through cache that exists outside of the process space, so only a hard system crash/power failure would compromise the buffered data (and opening/closing a file does not force the data from the cache to the device anyway.) Even in cases where there is no system write-through cache, you can typically flush a buffer with much less overhead than open/close on every write. Edited by krippy2k8

Share this post


Link to post
Share on other sites
Interesting discussion to follow as a Java developer. Are the logging conventions in C++ really that diverse? For Java there is almost a de-facto standard for logging, and in all the cases I've seen it used it's simply through a static member in each class, e.g.:

[CODE]
public class StdPlayerCreator implements PlayerCreator {
public static final Logger LOG = Logger.getLogger(StdPlayerCreator.class);
...

LOG.error("Internal error, failed to find a settlement");
[/CODE]

The parameter to getLogger() is effectively just a hierarchical label. It allows you to configure the logging output threshold on a class or package hierarchy. (This configuration is in a separate XML.)

Yes the above would break if multiple "runs" which should have distinct logs are run in the same JVM (java process). However that is rare and the logger can be instantiated differently for such use cases.

(Sorry for the tangential comment... ;-) )

Share this post


Link to post
Share on other sites
[quote name='ApochPiQ' timestamp='1340065928' post='4950435']
That still raises the obvious question: why is your logger an object/class to begin with?
[/quote]
Specifically, client code wants to log a message, not to "Get" a Log instance, worry about Log references, and still have to call Log methods to do something useful. Free functions are clearly the best fitting API for logging; static variables can also be used with functions and classes can be used behind the scenes, so nothing is lost from a technological standpoint.

Share this post


Link to post
Share on other sites
[quote name='LorenzoGatti' timestamp='1340091216' post='4950509']
Specifically, client code wants to log a message, not to "Get" a Log instance, worry about Log references, and still have to call Log methods to do something useful. Free functions are clearly the best fitting API for logging; static variables can also be used with functions and classes can be used behind the scenes, so nothing is lost from a technological standpoint.
[/quote]

Unless you want to log something other than simple strings. What if you want to log other values as well? You could use a printf-style log function, but then you lose type safety and must spend time generating an appropriate format string, and hope that the type of a variable never changes... with the possibility that your logging will itself cause a crash or bug! Or you would need to generate the string some other way before calling your log function.

Or you could do something like:

[CODE]
Logger::warning(__FILE__,__LINE__) << "foo is out of expected range: " << foo << endlog;
[/CODE]


or use a macro to eliminate some of the dirty work, and just do something like this:
[CODE]WARNING << "foo is out of expected range: " << foo << endlog;[/CODE]

or even (my preference):
[CODE]cs_warn( "foo is out of expected range: ", foo );[/CODE]

which makes adding logging much more pleasant, while maintaining type safety.

You could also generate a whole crapload of templated free functions to accomplish something similar, but that makes things a little more difficult when you need to mix integral types with user-defined types, and is generally quite a bit less flexible overall. Edited by krippy2k8

Share this post


Link to post
Share on other sites
My opinion:[list]
[*]Using globals is always wrong. They create a mess with initialization ordering, threading and dependencies.
[*]A free function would open the door for dozens of globals. Enable/Disable logging to OutputDebugString() - you need a global flag. Write log to a file? You need another global flag and a global variable storing the file handle.
[*]Singletons, global variables in disguise, rob you of all flexibility (like logging networking related stuff in file 1 and graphics related stuff in file 2) and gets in the way of unit testing.
[*]Passing some semi-global application class around is just hugely increasing dependencies since now you don't know which objects a class will look up through the application class (the service locator anti-pattern).
[/list]

The way you're doing it (constructor injection) is quite alright, though I agree that having to pass your logger everywhere you want to create an instance of class X tends to have a negative impact on usability and adds complexity to the interface.

Some other options:[list]
[*]Equip your classes with a SetLogger() method. By default, the logger is NULL, therefore no logging is performed. If you want to log something, you simply assign the logger to the class post construction.
[*]Do not add logging to your classes directly, wrap them in a logging wrapper (eg. a LoggedRenderer around your Renderer). Obviously can only log calls and results and with some effort exceptions leaving the renderer.
[*]Add a Log event (as in signals/slots) to your class. This is just another variant of the SetLogger() idea, of course.
[*]Forfeit logging altogether. I've done this in my code. I'm going all-out with assertions for pedantically checking internal states and exceptions for any usage error. Since the point of logging is to help you find the cause of errors quickly, by not letting any inconsistency, bad return code or missing capability slip under the carpet you remove the need for logging.
[/list] Edited by Cygon

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