Not logged inOpenClonk Forum
Up Topic Development / Developer's Corner / C4Game.h
- - By Günther [de] Date 2009-06-07 20:22
At the moment, class C4Game is a singleton and includes many other singleton instances of other classes. At first glance, this is nice, orderly design. At second glance, this brings out the worst aspect of C++: Every user of one of these classes has to include C4Game.h. If one changes something in any of the included headers, about one half of the engine has to be recompiled. I'm proposing to move all of these instances to the top level. This can mostly be done automatically by replacing Game.Foo with ::Foo. Does anybody have a better idea?

Also, splitting C4Game in (de)initializing and miscellaneous helper functions is probably warranted.
Reply
Parent - - By PeterW [de] Date 2009-06-10 11:28 Edited 2009-06-10 11:31
I don't think it's especially "nice" that C4Game is singleton. Even if that may sound pretty far-fetched, it's not impossible we might want to have a couple of games running in parallel in the same engine at some point in the future (say, for dedicated servers sharing the resources or proper scenario sections (yeah, still plotting to kill them ;] )).

After all that indoctrination at my software engineering lectures, I actually believe we might be in need for some sort of component framework. We just need a central authority that manages subsystems and returns references on-demand. Each component queries all needed pointers at initialization (we might have to seperate that from construction at some places, but that's actually a good thing to make explicit, I think).

The interesting question is wether this might cause noticeable performance hits. The trivial design will move all components from static allocation to the heap - potentially contributing to more memory fragmentation. Calling classes by pointer might also be a bit slower than static stuff in some (rare?) inlining cases.

... plus we would need to restructure a good chunk of code. Killing most of C4Wrappers.h in the process, for example.
Parent - - By Günther [de] Date 2009-06-10 12:31
There are hundreds or even thousands of references to Game.Foo. Changing that will require lots and lots of work. And it won't even help for "proper" scenario sections because those will have to share most of the instances in C4Game anyway. And I bet that nobody will ever want to run multiple games in one process, because that will never be as robust as one process per game, because one process per game is what everybody else is testing.

Also, splitting C4Game up will make it easier to split the engine in smaller parts which will make refactoring easier. I bet those software engineering classes didn't teach you to make everything one big heap where everything can access everything else ;-)
Reply
Parent - - By PeterW [de] Date 2009-06-10 17:06 Edited 2009-06-10 17:11
Replacing "Game.Landscape" by "Registry.get<C4Landscape>()" isn't that unrealistic. We only need to save back the pointers where we need the performance.

And it will help proper scenario sections if we put some thought into instance management. In this case, you would have to specify which landscape engine you mean, for example, whereas the definition list component could be shared by everyone. I was thinking about using an arbitrary pointer denoting the "container component" (like a C4Game object - or C4Application).

I'm not sure you understand what I mean. I want to decompose C4Game - that's what components are all about. Maybe I have to code my proposal.
Parent - - By Günther [de] Date 2009-06-10 19:44
Okay, but replacing ::Landscape with Registry.get<C4Landscape>() is also just search&replace. So I'll go ahead with splitting up C4Game the simple way, and try to untangle the engine.
Reply
Parent - By Günther [de] Date 2009-06-28 22:14

> So I'll go ahead with splitting up C4Game the simple way, and try to untangle the engine.


I pushed the result of this work, along with some other cleanups. Actually taking advantage of this by enforcing some separation will have to wait until I'm done with this university term.
Reply
Parent - By Sven2 [us] Date 2009-06-11 23:23
If you want to convert it into pointers, you could just replace all members of C4Game with pointers. This would eliminate all dependencies of C4Game as well.
Parent - By Günther [de] Date 2009-06-15 23:11
Hm, should I move the definition of those globals to C4WinMain.cpp as is done with Game, Application, Config, etc., or to the files containing their implementation?
Reply
Parent - - By Nachtschatten Date 2009-09-02 17:26
Excuse me for resurrecting this old topic, but one question bugs me.

About using singletons, you wrote:

> At first glance, this is nice, orderly design.


However, from what I've read, and from my recent experiences in programming, I disagree. In my opinion, singletons are rarely useful at all. Especially for unit testing, a singleton that's not replaceable with a dummy object can really be a pain.

So I'd like to hear your opinion on the matter. Why do you think singletons are good design, and how comes the Clonk engine makes extensive use of them?
Reply
Parent - - By Sven2 [de] Date 2009-09-03 04:56
Because otherwise, you would either need to pass tons of parameters to every function (C4Object::Execute(Game, Landscape, ...)), or keep track of some ownership structure (this->GetGame()->GetLandscape()). Besides, there are no unit tests.
Parent - - By Nachtschatten Date 2009-09-06 17:22

> or keep track of some ownership structure


Yes, that's the way I got rid of singletons in the project I worked on recently. The object in question got the objects it needed as parameters in its constructor. While there were only two of those, keeping track of ownership was a non-issue anyway, as the code was written in Java, and thus the garbage collection saved us the work in that regard.

Still, your post sounds like singletons are used for almost everything. Is it really that much? And was saving work (parameters, ownership) the only (or the only major) reason for using them?
Reply
Parent - By Sven2 [de] Date 2009-09-06 19:54
If you want to know the historic reason, you should ask matthes. It has been like that forever.

Having used both approaches, I do not like the ownership idea very much. You are adding a lot of bookkeeping work for no specific gain - especially if you have to add a trillion checks against zero pointers because not every element of the parent/child structure is assigned during initialization yet.

To me, it does make sense only if you can actually have to instances. E.g., if you want to implement two landscapes being loaded at the same time.
Parent - - By Günther [de] Date 2009-09-03 13:03
I meant the "C4Game encompasses everything there is in the game" aspect, especially given the fact that there is more than one game during the lifetime of the process. Ending a game with a simple Game.Clear()-Call, which only has to care about C4Game members, is nice from an object-oriented view.
Reply
Parent - - By Nachtschatten Date 2009-09-06 16:38
Oh, I see. Yes, that's a nice feature indeed.
Reply
Parent - By Günther [de] Date 2009-09-07 00:13
Fortunately, it doesn't matter very much whether the objects are members of C4Game or live in the global namespace - They can still be initialized and cleared from C4Game, but we can start disentangling the dependency graph. Unfortunately, I fear that a lot of the interdependencies arise from the nature of a game. Lots of code deals with simulating the game, and everything affects everything else. And separating that cleanly from the other parts (user interface, graphics, sound, network) is a huge effort in API design.
Reply
Up Topic Development / Developer's Corner / C4Game.h

Powered by mwForum 2.29.7 © 1999-2015 Markus Wichitill