How can I layout a decoupled class structure for a simple game?

Question

Right now I have six classes:

1. Listener - manages socket connections
2. World - a collection of entities and tasks
3. Ticker - coordinates updating the world
4. MessageProcessor - receives commands from players
5. Intelligence - governs the behavior of non-player characters
6. Tasks - tracking and execution of tasks

But they are like spaghetti with reference to each other all over the place... The World is a data model which the MessageProcessor, Intelligence, and Tasks classes modify. The Ticker coordiates those three classes updating the World. The Listener is used by the MessageProcessor for incomming messages, and by the other classes to push updates.

How can I improve this situation?

Answer 1

I gave a related answer not long ago. The subject was on improving the testability of code, for which the general solution is to loosen coupling. The main focus on that previous answer was on decoupling networking related code from the world and it's logic, because networking code is not unit testable and is a pain to mock too.

The solution given there was to use an interface for incoming messages, such that you decouple the MessageProcessor (named Handler in other post) from the network code, and similarly, decouple the UpdateNotifier from the World.

The dashed line is just an indirect reference handled either by an interface or delegate. There exists no direct relation between the World and networking component now, making it testable. This is really just an application of the Model View Adapter pattern.

This doesn't seem dissimilar to the design you've described having, except perhaps you are missing a few interfaces. With this pattern of interface based UpdateNotifiers used to push updates, I essentially reuse the same architecture for handling NPCs, tasks, or anything else which is processed elsewhere. You cherry pick the event notifiers you need for a particular area, and implement a concrete Notifier class for them, such that you have multiple adapters on the same model.

And that really only looks more complicated than it is. The World object has no direct dependencies on anything else, and each other class has at most one direct dependency. You can also isolate the timer from the World, as it probably isn't needed there - but perhaps one of the biggest hurdles is handling synchronization between the different adapters.

Answer 2

Well, I'm not sure I have a full picture of what the issues you are having are, but I have a few possibilities from what you have laid out so far. (I may be actually suggesting some things that are already done since I'm not sure I have enough from the one-liner descriptions to understand fully.

The Model

I would say from what you've described, the main thing that strikes me is that you'll want to start implementing common functionality in a class model; you will want either interfaces or base classes that you can use to derive your high-level objects from.

This way you can handle things consistently with little extra effort. I think the idea of "architectural layers" can be useful as a first cut of how to think about it, (e.g. low-level hardware stuff, socket handling etc., then middle-layers stuff like what kind of things happen in your game, and the details behind how game mechanics work, etc., and high-level stuff like what can the PC or NPCs do, what's the environment doing, etc.. and also the idea that you never want to "jump" layers). However, when it comes down to it the important thing is to just find the right abstractions for your game, and keep everything organized in such a way as you never feel like the bit of code you're working on is doing two completely different kinds of things.

So, first, let's take the fact that it sounds like (naturally) there are a lot of things interacting with world state. For something like this, it's probably advantageous to factor a lot of the 'stuff' out into a single class, and then mostly only have the one class doing that job. Ideally you implement, say, event communication/message passing, in it's own little group, so that there's no need to pollute your higher-level objects with the nitty-gritty of handling stuff.

e.g., you want to focus on what things are doing at a high level in the higher-level objects: in an AI perhaps "begin movement toward a location", "set my haste", "stop movement"; and in an environment subsystem do "start raining", "increase windspeed", "dim lights"; in a user class "fire weapon", "sleep", "cast spell". But I wouldn't want any of my high-level classes to even know about things like "send message to world", or "reset thirst timer", or "receive socket data", or "health cycle tick". (These are all just elucidations, not suggestions. ;D)

Events

For instance, I think it may be valuable to keep one object in charge of dispatching events to the World, that way you no longer have everyone talking to everyone. I would likely just create a set of stuff to handle events in general. So, maybe EventMain and an enumEvents that you use so that each type of event has a special ID. And then use Event as the base class for particular events that need extra functionality. (I have both the ID as well as a derivation model in mind, so that things thing like the Dispatcher which likely only need to know very basic things about the event don't have to also know about the derived classes. For instance, the dispatcher could take an event in and send it out without ever having to know the internals of a derived event. This may or may not turn out to be useful, but it's good to have the options.) You could also have an EventDispatcher that has a queue of events to be send to other subsystems.

You will want something common for recieving and sending events. You could do EventSourcer and EventSinker standalone classes that can be set up in any class that is generating or receiving events. Or, you could instead do IEventSource and IEventSink so that you could implement a common interface on multiple classes, or perhaps a common class EventSourceAndSink that implements both, and which is part of your base class model, so that anything that might need to handle events can just derive from it.

I would probably make ProtocolEncoder and ProtocolDecoder classes. You can always combine them into a single object, but it may be valuable, and shouldn't cause any issues if done adequately, to have them be two separate pieces of code. You could also have a ProtocolHelper class that factors out anything in common between the two. The encoders sole job is to receive messages from the network and turn them into events for your game, which it will then pass on to the EventDispatcher. The decoder class will take events from the dispatcher that need to go out to the network, and it will take the data from them and send it out.

How to Get Where You're Going

Since you do have working code, I would recommend that you just start doing it wherever seems natural. That could be things that are bogging you down, or things you've noticed to be very similar in different places that you could make regular with a class or some other type of abstraction, then pull out the old and put in the new. Once you have figured out a workable first cut of a class model, that should give you ideas based on what you already have and as you go be constantly reconsidering your model, fixing the things that are a problem, lather, rinse, repeat.

It doesn't have to be a lot of work, in fact, some of the most gratifying moments I've had working on code was when I was able to do a neat refactor that left a formerly hideous mess in much better shape, removing a lot of hard-to-understand code, and replacing it with something that's easier to understand, in fewer lines of code, and that opened up a path toward my next addition being a pleasure instead of another "zomg I don't have to touch that code again do I?" moment.

Best of luck, follows is a nominal guide to the things I was talking about; the first bit is more detailed because the main event class is one of the more important concepts, and then I try to just give a nominal overview of the classes and how they interact. I'm sure I could spend even more hours on this, but at this point I'll just say: ask me if you have questions and I'll do what I can to give you a good answer :)

Ideas in Code

Oh, one more thing of note is I didn't deal at all with the complexities added if you have multiple threads; there are things ranging from simple to intricate to manage it all if you do, but that's another exercise. :)

using System;
using System.Collections;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Windows.Forms;

    // this is internal to the project namespace, say, TimsWorld_o_Hurt
    // I'm now resisting calling everything Xxxx_o_Hurt :)

    // examples o' hurt
using EventHandlingLibrary;

namespace EventHandlingLibrary
{
    // this will provide the base class for all the events, and can
    // also have static methods like factory methods, destination 
    // lookups etc. 

    // I have the enums set to protected with the intent being that
    // specific factory functions should be called by other classes.
    // You should change this if it turns out to be too cumbersome.
    public class EventOfHurt
    {
        #region Event Definitions
            protected enum EEventType
            {
                // System Events
                SystemInitializing,
                SubsystemInitComplete,
                FatalErrorNotification,
                SubsystemPingReponse,
                SubsystemPingRequest,

                // Network Events
                FrameRateError,
                ThroughputData,
                ServerTimeout,
                ServerPingRequest,
                ServerPingResponse,

                // User Events
                WeaponsFire,
                MovementNotification,
                FatigueUpdate

                // and so forth
            }

            protected enum ESubsystem
            {
                System,
                Dispatcher,
                TickerTimer,
                WorldEntity,
                WorldTaskManager,
                UserMessageProcessor,
                NetworkListener,
                NetworkTransmitter,
                ProtocolEncoder,
                ProtocolDecoder,
                PlayerCharacter,
                NonPlayerCharacter,
                EventSink,
                EventSource

                // and such
            }
        #endregion

        #region Event Information
            public Guid EventId { get; protected set; }
            public EEventType EventType { get; protected set; }
            public ESubsystem SourceSubsystem { get; protected set; }
            public ESubsystem DestSubsystem { get; protected set; }

            private List<Tuple<EventOfHurt, DateTime>> 
                myEventReferences;

            // the event(s) that triggered it, if any, and when rec'd
            public Tuple<EventOfHurt, DateTime>[] 
                EventReferences 
            { 
                get { return myEventReferences.ToArray(); } 
            }

            public DateTime Timestamp { get; private set; }
        #endregion

        // we'll be using factor methods to create events
        // so keep constructors private; no default constructor
        private EventOfHurt(
            EEventType evt,
            ESubsystem src, 
            ESubsystem dest = ESubsystem.Dispatcher
        )
        {
            EventType = evt;
            SourceSubsystem = src;
            DestSubsystem =  dest;

            EventId = Guid.NewGuid();
            Timestamp = DateTime.UtcNow;
        }

        // called to create a non-derived event for simple things; 
        // but keep other classes limited to calling specific factory
        // methods
        protected static EventOfHurt CreateGeneric(
            EEventType evt, ESubsystem src, 
            ESubsystem dest = ESubsystem.Dispatcher,
            Tuple<EventOfHurt, DateTime>[] reasons = null
        )
        {
            EventOfHurt RetVal;

            if (dest == null)
                dest = ESubsystem.Dispatcher;

            List<Tuple<EventOfHurt, DateTime>> ReasonList = 
                new List<Tuple<EventOfHurt,DateTime>>();

            if (reasons != null)
                ReasonList.AddRange(reasons);

            // the initializer after the constructor allows for a 
            // lot more flexibility than e.g., optional params
            RetVal = new EventOfHurt(evt, src) {
                myEventReferences = ReasonList
            };

            return RetVal;
        }

        // some of the specific methods can just return a generic
        // non-derived event
        public static EventOfHurt CreateTickerTimerEvent(
            EEventType evt, ESubsystem dest
        )
        {
            ESubsystem src = ESubsystem.TickerTimer;
            return CreateGeneric(evt, src, dest, null);
        }

        // some may return actual derived classes
        public static EventOfHurt CreatePlayerActionEvent(
            EEventType evt, ESubsystem dest,
            Tuple<EventOfHurt, DateTime>[] reasons
        )
        {
            PlayerEvent PE = new PlayerActionEvent(42);
            return PE;
        }
    }

    // could have some specific info relevant to player 
    // events in this class, world location, etc.
    public class PlayerEvent :
        EventOfHurt
    {
    };

    // and even further speciailzation here, weapon used
    // speed, etc. 
    public class PlayerActionEvent :
        PlayerEvent
    {
        public PlayerActionEvent(int ExtraInfo)
        {
        }
    };
}

namespace EntitiesOfHurt
{
    public class LatchedBool
    {
        private bool myValue = false;
        public bool Value
        {
            get { return myValue; }
            set {
                if (!myValue)
                    myValue = value;
            }
        }
    }

    public class EventOfHurtArgs :
        EventArgs
    {
        public EventOfHurtArgs(EventOfHurt evt)
        {
            myDispatchedEvent = evt;
        }

        private EventOfHurt myDispatchedEvent;
        public EventOfHurt DispatchedEvent
        {
            get { return myDispatchedEvent; }
        }
    }

    public class MultiDispatchEventArgs :
        EventOfHurtArgs
    {
        public MultiDispatchEventArgs(EventOfHurt evt) :
            base(evt)
        {
        }

        public LatchedBool isHandled; 
    }

    public interface IEventSink
    {
        // could do this via methods like this, or by attching to the
        // events in a source
        void MultiDispatchRecieve(object sender, MultiDispatchEventArgs e);
        void EventOfHurt(object sender, EventOfHurtArgs e);

        // to allow attaching an event source and notifying that
        // the events need to be hooked
        void AttachEventSource(IEventSource evtSource);
        void DetachEventSource(IEventSource evtSource);
    }

    // you could hook things up in your app so that most requests
    // go through the Dispatcher
    public interface IEventSource
    {
        // for IEventSinks to map
        event EventHandler<MultiDispatchEventArgs> onMultiDispatchEvent;
        event EventHandler<EventOfHurtArgs> onEventOfHurt;

        void FireEventOfHurt(EventOfHurt newEvent);
        void FireMultiDispatchEvent(EventOfHurt newEvent);

        // to allow attaching an event source and notifying that
        // the events need to be hooked
        void AttachEventSink(IEventSink evtSink);
        void DetachEventSink(IEventSink evtSink);
    }

    // to the extent that it works with your model, I think it likely
    // that you'll want to keep the event flow being mainly just
    // Dispatcher <---> Others and to minimize except where absolutely
    // necessary (e.g., performance) Others <---> Others.

    // DON'T FORGET THREAD SAFETY! :)
    public class Dispatcher : 
        IEventSource, IEventSink
    {
    }

    public class ProtocolDecoder :
        IEventSource
    {
    }

    public class ProtocolEncoder :
        IEventSink
    {
    }

    public class NetworkListener
    {
        // just have these as members, then you can have the
        // functionality of both on the listener, but the 
        // listener will not send or receive events, it will
        // focus on the sockets.

        private ProtocolEncoder myEncoder;
        private ProtocolDecoder myDecoder;
    }

    public class TheWorld :
        IEventSink, IEventSource
    {

    }

    public class Character
    {
    }

    public class NonPlayerCharacter :
        Character,
        IEventSource, IEventSink
    {
    }

    public class PlayerCharacter :
        Character,
        IEventSource, IEventSink
    {
    }
}