Classes, namespaces, singletons

Started by
0 comments, last by frob 8 years, 3 months ago

I recently went through source code of a certain MMO game, that was shut down some time ago, in a process of learning how "big guys" do things. I did so with several open sourced MMOs like Planeshift or Ryzom, but this time my brain melted due to some coding concept I encountered there. I will return to this shortly, but first I will describe my case:

In last few years I worked on an online open world game (no, not massive, just more than usual multiplayer mode for normal games). As it's a way more complex project than usual Tetris game, this means it contains a lot, and I mean a lot of various systems and subsystems that talk to each other. In this project the biggest challenge for me so far is designing classes and their interaction so it makes sense and doesn't create monolithic mess that can be found even in small projects.

And more and more I notice, that I'm mostly making a class-to-be-instantiated-just-once that solves some problem as a system, and this class operates on few or more other classes that are actually instantiated in bigger amount. I bet that 80% of my code consists of classes that are never instantiated more than once (systems, services, graphics, game core etc.). I read tens of articles about singletons, service locators and all that, and I'm still confused what is really the right way to do this in really big ass project. And then I see this MMO code, easily thousands of classes, server/client/shared code. I look at the client initialization code and see chain of calls like this:


Graphics::init()
Sound::init()
Terrain::init()
Threading::init()
SkeletalAnimation::init()

... and so on, sometimes with parameters, sometimes not. These systems follow the same chain with their subsystems and deeper and deeper. I look at some code of such subsystem and it turns out it's not a real class, not even a singleton which I expected in the first place (usually syntax like Class::init() smells like some singleton initialization). All its methods are static, and it doesn't contain any members but it defines a namespace like TerrainNamespace inside class file where it puts some variables and accesses them. There is no object instantiation for these "classes", just some initialization and later shutdown, and of course hundreds of static functions that do things (and sometimes alter the state variables contained in namespace). Below some conceptual example of this:


MySystem.h

class MySystem
{
   public:
 
   static void Install();
   static vector& GetData();
   static void SetFlag(bool flag);

   private:

   static void LoadMyData();
}


MySystem.cpp

namespace MySystemNamespace
{
    int someVar;
    bool someOtherVar;
    std::vector someData;
}

using namespace MySystemNamespace;

void MySystem::Install()
{
    LoadMyData(); // loads data into someData vector
}

vector& MySystem::GetData()
{
    return someData;
}

void MySystem::SetFlag(bool flag)
{
    someOtherVar = flag;
}

so this is used like this:


void Loop()
{
    vector data = MySystem::GetData();
    doSomething(data);

    MySystem::SetFlag(1)
}

main()
{
    MySystem::Install();

    while (running)
      Loop();
}

I'm really shattered, because I've never expected to see something like this (and to such extent and of this magnitude) in one of the biggest gamedev efforts which is making a commercial MMO game.

And the point of my post is, hopefully, stirring some discussion about this solution and finding an answer what is the correct way to design and develop big games with client-server architecture that have really a lot of things to do.

I admit, I put that singleton in title intentionally ;) But this isn't really about singletons, but basic concepts of programming - the way above code works - it allows access to every functionalit of every subsystems without any guards. You want something done - just include a file and SomeSystem::doSomething() and it will be done. It seems tempting, because with so many interactions between systems, it starts to be daunting task to separate concerns, avoid coupling and so on. And here, it's all at my hand. I'm not even sure this can still be called OO-programming.

I wouldn't probably be so shocked if it was used in one or two places, but after exploring that code I'm convinced all systems that follow the rule of "probably no more than one instance of this class is ever needed in a program" are made in this manner.

Hope to get some answers that will put me back on track, because right now I'm really confused.


Where are we and when are we and who are we?
How many people in how many places at how many times?
Advertisement
There are best practices but in the end you need to ship a product.

Singletons -- the pattern of "there can only be one instance ever, you cannot create another instance under any circumstances", is generally bad. It causes all kinds of issues. This is what most people think of when it comes to the Singleton pattern. I've never seen any good come from this pattern, only heartache and late nights hunting bugs. Don't do it.

The static instance pattern, meaning "we provide an instance accessible through a static instance variable" is generally bad, but usually the negative side effects can be dealt with through careful engineering. The static instance gets constructed at some point before main(), and generally you cannot control the object's lifetime. It can suffer effects of shared state that need to be dealt with by policy rather than enforced by the compiler. When you encounter problems of shared state they will be nasty to debug.

The pattern of "we provide a well-known instance accessible through a global pointer". It is slightly less bad than the pattern just above because you can control the object's lifetime. That is likely what the above code is doing with the ns::Init() functions. It can still suffer nasty bugs due to shared state.

When programs follow the pattern they are increasing coupling and introducing hidden dependencies in the code. They add some convenience in some situations since they allow easier access to subsystems, but they also add a large number of risks. Shared state is by far the biggest risk, since bugs from accidental modification of shared state are hard. The hidden dependencies can cause problems for automated tests, but in games that is a business decision needing a good cost/benefit analysis. The hidden dependencies can simplify some game logic but makes code more brittle against a single system rather than allowing change and customization by passing in alternate values other than the hidden dependency value.


Much of what you've got there is static values that are wrapped in a namespace. Basically they're just globals or well-known data. A bad habit to be sure, and you can be certain they had bugs due to the decision, but still something they can decide to do. Bad practices are still practices. In the end you need to ship a product and customers look at the game on their screen, not the source code.

If you have good policies and enforce them strictly, if you understand the consequences and either know they can be minimized or don't really apply to your situation, then choosing to make well-known instances inside a namespace's shared pointer can make sense as a decision. You will need to strictly document the times when the elements are mutable and immutable, and what bounds will be placed on them to ensure validity at all times. You need to know the policy will be strictly enforced, and you need to have a constant monitoring by QA to ensure issues are not introduced, and you should probably add watchdog systems to ensure shared state remains as expected. Even with all that, some bugs are likely to stem from the decision and they can have costs to diagnose and correct. You need to be especially concerned about concurrent use in multiprocessing/multithreaded environments.

It is not a decision to make lightly and the cost of bugs can be high, but as a tradeoff in simplifying some aspects of coding it can make sense for some business environments, especially when the system does not have long-tail maintenence.

This topic is closed to new replies.

Advertisement