Keeping code in a client-server game organized

Question

Background: I help develop a multiplayer game, written mostly in C++, that uses a standard client-server architecture. The server can be compiled by itself, and the client is compiled with the server so you can host games.

Problem

The game combines both client and server code into the same classes, and this is starting to be very cumbersome.

For example, the following is a small sample of something you may see in a common class:

// Server + client
Point Ship::calcPosition()
{
  // Do position calculations; actual (server) and predictive (client)
}

// Server only
void Ship::explode()
{
  // Communicate to the client that this ship has died
}

// Client only
#ifndef SERVER_ONLY
void Ship::renderExplosion()
{
  // Renders explosion graphics and sound effects
}
#endif

And the header:

class Ship
{
    // Server + client
    Point calcPosition();

    // Server only
    void explode();

    // Client only
    #ifndef SERVER_ONLY
    void renderExplosion();
    #endif
}

As you can see, when compiling the server only, preprocessor definitions are used to exclude the graphics and sound code (which seems ugly).

Question:

What are some of the best practices for keeping code in a client-server architecture organized and clean?

Thanks!

Edit: Examples of open source projects that use good organization are also welcome :)

Answer 1

I would consider using a Strategy design pattern whereby you would have a Ship class with functionality common to both client and server, then create another class hierarchy called something like ShipSpecifics that would be an attribute of Ship. The ShipSpecifics would be created with either a server or client concrete derived class and injected into Ship.

It could look something like this:

class ShipSpecifics
{
    // create appropriate methods here, possibly virtual or pure virtual
    // they must be common to both client and server
};

class Ship
{
public:
    Ship() : specifics_(NULL) {}

    Point calcPosition();
    // put more common methods/attributes here

    ShipSpecifics *getSpecifics() { return specifics_; }
    void setSpecifics(ShipSpecifics *s) { specifics_ = s; }

private:
    ShipSpecifics *specifics_;
};

class ShipSpecificsClient : public ShipSpecifics 
{
    void renderExplosion();
    // more client stuff here
};

class ShipSpecificsServer : public ShipSpecifics 
{
    void explode();
    // more server stuff here
};

The classes Ship and ShipSpecifics would be in the code base common to both client and server, and the ShipSpecificsServer and ShipSpecificsClient classes would obviously be in the server and client code bases respectively.

The usage could be something like the following:

// client usage
int main(int argc, argv)
{
    Ship *theShip = new Ship();
    ShipSpecificsClient *clientSpecifics = new ShipSpecificsClient();

    theShip->setSpecifics(clientSpecifics);

    // everything else...
}

// server usage
int main(int argc, argv)
{
    Ship *theShip = new Ship();
    ShipSpecificsServer *serverSpecifics = new ShipSpecificsServer();

    theShip->setSpecifics(serverSpecifics);

    // everything else...
}

Answer 2

Why not take a simple approach? Provide a single header describing what the Ship class will do, with comments but no ifdefs. Then provide the client implementation inside an ifdef as you did in your question, but provide an alternative set of (empty) implementations that will be used when the client is not being compiled in.

It strikes me that if you are clear in your comments and code structure, this approach will be much easier to read and understand than the more "sophisticated" solutions proposed.

This approach has the additional advantage that if the shared code, here calcPosition(), needs to take a slightly different execution path on the client vs. the server, and the client code needs to call an otherwise client-only function (see example below), you will not encounter build complications.

Header:

class Ship
{
    // Server + client
    Point calcPosition();

    // Server only
    void explode();
    Point calcServerActualPosition();

    // Client only
    void renderExplosion();
    Point calcClientPredicitedPosition();
}

Body:

// Server + client
Point Ship::calcPosition()
{
  // Do position calculations; actual (server) and predictive     (client)
   return isClient ? calcClientPredicitedPosition() :
                     calcServerActualPosition();
}

// Server only
void Ship::explode()
{
  // Communicate to the client that this ship has died
}

Point Ship::calcServerActualPosition()
{
  // Returns ship's official position
}


// Client only
#ifndef SERVER_ONLY

void Ship::renderExplosion()
{
  // Renders explosion graphics and sound effects
}

Point Ship::calcClientPredicitedPosition() 
{
  // Returns client's predicted position
}

#else

// Empty stubs for functions not used on server
void  Ship::renderExplosion()              { }
Point Ship::calcClientPredicitedPosition() { return Point(); }

#endif

This code seems quite readable (aside from the cognitive dissonance introduced by the Client only / #ifndef SERVER_ONLY bit, fixable with different names), especially if the pattern is repeated throughout the application.

The only drawback I see is that you will need to repeat your client-only function signatures twice, but if you mess up, it will be obvious and trivial to fix once you see the compiler error.

Answer 3

Define a client stub class that has the client API.

Define the server class that implements the server.

Define a server stub that maps incoming message to server calls.

The stub class has no implementation except to proxy commands to the server via what ever protocol you are using.

You can now change protocols without changing your design.

or

Use a library like MACE-RPC to automatically generate the client and server stubs from the server API.