Ensure encapsulation and Open/Closed principle with Command pattern?

Question

I'm developing a small multiplayer game. It'll be served by one websockets server and consumed by multiple consumers. As such I need to be cautious about concurrency errors.

The general software architecture I've come up with is: Encapsulate all game state into the highest-level Game class and don't allow other threads to access gamestate objects. Instead, clients (websocket threads on the server) send commands to the Game class where they're added to a threadsafe list and later processed by the Game.Tick method which is called in a separate thread. For example:

Game.AddCommand assigns a command to the list of incoming commands:

    public void AddCommand(Guid id, ICommand command)
    {
        lock (_commandsLock)
        {
            _commands[id] = command;
        }
    }

Game.Tick processes each command and clears the list:

    public void Tick()
    {
        IList<ICommand> commands;
        lock (_commandsLock)
        {
            commands = _commands.Values.ToList();
            _commands = new Dictionary<Guid, ICommand>();
        }

        foreach (var command in commands)
        {
            command.Resolve();
            Observer.Update(_map);
        }
    }

The issue is this: To resolve themselves each Command needs access to the internals of Game. I intend to use a number of services such that a Resolve method can be very few or even a single line, such as movementService.Move(entity, target). If I passed these services into the Resolve method, each Command would become bloated with things that it doesn't need.

Instead, I'd like to use DI. But since each Command is created by a client, they can't access the services that belong to Game. These services need to be exposed, and if the client does something wrong (ie call Resolve themselves) they could change gamestate and cause concurrency errors.

I tried to resolve this by creating the notion of a CommandResolver, in which case a Command would be a simple model with some parameters and nothing more, resolved by an internal CommandResolver which has access to gamestate. But this required elaborate mapping (eg map MoveCommand to MoveCommandResolver- each command is a different type with different dependencies) which defies the Open/Closed principle- you have to modify the mapping, likely a long awkward switch statement, rather than simply extending the functionality with a new Command & CommandResolver.

Now I'm scrapping the resolver and considering one of two possibilities: a) Expose from Game a CommandFactory that can be used to generate commands, prehaps with a syntax like game.GetCommandFor(entity).Move(target). The commands then have access to gamestate and resolve themselves. b) Commands are constructed by clients, but before being resolved an Initialise(IUnityContainer) method is called that gives the Command access to gamestate. This way it only gains access to gamestate once the command is ready to be issued. Of course, the client could still technically hold onto the command and call it at a later date.

Any thoughts on what the best solution is? My primary concerns are easy extensibility while ensuring the integrity of Game's internal state.

Answer 1

It is not possible to create a perfectly safe solution here. Either you abstract over the commands and thus sacrifice flexibility, or allow more flexibility but loosen up the encapsulation of your game state.

The question here is whether the command objects can be trusted to not corrupt the game state. If you can't trust them, using a command factory that effectively whitelists a few known-good commands may be desirable. The factory methods can perform validation to reject illegal commands.

But most of the time, you can trust your own code. Sacrificing a bit of encapsulation in order to get things done is fine. For example, we might have a design like this:

interface IGameCommand
{
  void Resolve(GameState game);
}

Then in Game.Tick():

foreach (var command in commands)
{
  command.Resolve(state);
  ...
}

This does require that your game state exposes the necessary operations and services. This game state object doesn't have to be the same as your Game's ordinary public interface – it can be a separate class. This might be helpful to avoid accidentally calling methods on the game directly instead of queuing a command for later. Technically this could be used to keep a reference beyond the lifetime of the command, but if all of this is your code then simply don't do that.

For a multiplayer game, think carefully about your security model. A message-oriented solution can be a good idea because you can validate the messages, and can exchange messages over a secure connection (for certain values of “secure”). The type system of your programming language is not a security barrier. You cannot execute untrusted code and assume that it can't access private details. Therefore, the messages must not contain code. Instead, you will have to use a resolver approach like you discussed to map the messages to behaviour.