4
\$\begingroup\$

As part of a small game engine I'm currently working on, I have written a "NetLib".

NetLib should manage different types of packages through PacketStreams which are based on TcpClients. I'm quite unsure if my implementation of PacketStream and related classes is reasonable or whether I'm overdoing it.

public class PacketStream : IDisposable
    {
        public bool Connected { get; private set; } = true;
        public IAvailabilityStream Stream { get { return _stream; } }
        private readonly PacketStreamReader _reader;
        private readonly PacketStreamWriter _writer;
        private readonly IAvailabilityStream _stream;
        private readonly Dictionary<int, Func<PacketStreamReader, PacketBase>> _readerStrategies = new Dictionary<int, Func<PacketStreamReader, PacketBase>>();
        private readonly Dictionary<int, Action<PacketStreamWriter, PacketBase>> _writerStrategies = new Dictionary<int, Action<PacketStreamWriter, PacketBase>>();

        private readonly PacketBase _emptyPacket = new EmptyPacket();

        internal PacketStream(IAvailabilityStream stream)
        {
            _stream = stream;
            _reader = new PacketStreamReader(stream, Encoding.UTF8);
            _writer = new PacketStreamWriter(stream, Encoding.UTF8);
        }

        public void Write(PacketBase packet)
        {
            if (!_writerStrategies.ContainsKey(packet.PacketTypeId))
            {
                throw new InvalidOperationException($"Packet with ID {packet.PacketTypeId} was not found in WriterStrategies.");
            }
            try
            {
                _writer.Write(packet.PacketTypeId);
                _writerStrategies[packet.PacketTypeId](_writer, packet);
            }
            catch (Exception e)
            {
                Connected = false;
            }
        }

        public async Task WriteAsyc(PacketBase packet)
        {
            if (!_writerStrategies.ContainsKey(packet.PacketTypeId))
            {
                throw new InvalidOperationException($"Packet with ID {packet.PacketTypeId} was not found in WriterStrategies.");
            }
            await _writer.WriteAsync(packet.PacketTypeId);
            _writerStrategies[packet.PacketTypeId](_writer, packet);
        }

        public PacketBase Read()
        {
            if (!_stream.DataAvailable)
                return _emptyPacket;

            var packetTypeId = _reader.ReadInt32();

            if (!_readerStrategies.ContainsKey(packetTypeId))
            {
                throw new InvalidOperationException($"Packet with ID {packetTypeId} was not found in ReaderStrategies.");
            }
            return _readerStrategies[packetTypeId](_reader);
        }

        public async Task<PacketBase> ReadAsync()
        {
            if (!_stream.DataAvailable)
                return _emptyPacket;

            var packetTypeId = await _reader.ReadInt32Async();

            if (!_readerStrategies.ContainsKey(packetTypeId))
            {
                throw new InvalidOperationException($"Packet with ID {packetTypeId} was not found in ReaderStrategies.");
            }
            return _readerStrategies[packetTypeId](_reader);
        }

        public void Register<T>(int identifier, Func<PacketStreamReader, T, T> reader, Action<PacketStreamWriter, T> writer) where T : PacketBase, new()
        {
            if (_readerStrategies.ContainsKey(identifier) || _writerStrategies.ContainsKey(identifier))
            {
                throw new InvalidOperationException();
            }
            _readerStrategies.Add(identifier, (binaryReader) => reader(_reader, new T()));
            _writerStrategies.Add(identifier, (binaryReader, packetBase) => writer(_writer, (T)packetBase));
        }

        public void Dispose()
        {
            _reader.Dispose();
            _writer.Dispose();
            _stream.Dispose();
        }
    }

PacketStreamFactory manages package registration. You create different types of streams by adding read/write strategies inside PacketStreamFactory.

public class PacketStreamFactory
    {
        public static PacketStream CreateLoginServerPacketStream(IAvailabilityStream stream)
        {
            var packetStream = new PacketStream(stream);
            packetStream.Register<HeartbeatPacket>((int)PacketType.HEARTBEAT, Read, Write);
            packetStream.Register<LoginPacket>((int)PacketType.LOGIN, Read, Write);
            packetStream.Register<AuthSuccessPacket>((int)PacketType.AUTH_SUCCESS, Read, Write);
            packetStream.Register<AuthFailurePacket>((int)PacketType.AUTH_FAILURE, Read, Write);
            return packetStream;
        }

        public static PacketStream CreateLobbyPacketStream(IAvailabilityStream stream)
        {
            var packetStream = new PacketStream(stream);
            packetStream.Register<HeartbeatPacket>((int)PacketType.HEARTBEAT, Read, Write);
            packetStream.Register<ChatMessagePacket>((int)PacketType.CHAT_MESSAGE, Read, Write);
            packetStream.Register<CreateRoomPacket>((int)PacketType.CREATE_ROOM, Read, Write);
            packetStream.Register<CreateRoomResponsePacket>((int)PacketType.CREATE_ROOM_RESPONSE, Read, Write);
            packetStream.Register<RoomPresencePacket>((int)PacketType.ROOM_PRESENCE, Read, Write);
            packetStream.Register<RosterItemPacket>((int)PacketType.ROSTER_ITEM, Read, Write);
            packetStream.Register<PresencePacket>((int)PacketType.PRESENCE, Read, Write);
            packetStream.Register<StatusPacket>((int)PacketType.STATUS, Read, Write);
            return packetStream;
        }

        #region LOGIN
        private static void Write(PacketStreamWriter binaryWriter, LoginPacket loginPacket)
        {
            binaryWriter.Write(loginPacket.Username);
            binaryWriter.Write(loginPacket.Password);
        }

        private static LoginPacket Read(PacketStreamReader binaryReader, LoginPacket loginPacket)
        {
            loginPacket.Username = binaryReader.ReadString();
            loginPacket.Password = binaryReader.ReadString();
            return loginPacket;
        }
        #endregion
    }

Is this a reasonable approach or am I cluttering my code with unnecessary patterns?

Edit

In regards to Pieter Witvoet's comment: The idea behind this design is the restriction of packets at times I don't expect them. For example during the login state I would only expect my clients to send LoginPackets to my server and not ChatMessagePackets.

So the basic workflow would be:

  • Client application starts and uses the PacketStream returned by CreateLoginServerPacketStream to authenticate.
  • As soon as the client recieves a succesful login response he converts his LoginServerPacketStream to LobbyPacketStream which enables him to send chatmessages, roomcreation requests and so on.

The same thing happens in the server application as they both share this library.

I also thought about creating a plugin based SDK where other developers could simply add other games by registering their PacketStreams read/write methods and implement some kind of generic game interface.

\$\endgroup\$
2
  • 1
    \$\begingroup\$ What are your design goals for this? Why do streams need to know how to serialize/deserialize packets, and why should one stream be able to read/write a certain packet type but not another? Do you need streams to be reconfigurable at run-time? \$\endgroup\$ – Pieter Witvoet May 10 '17 at 12:44
  • \$\begingroup\$ Your code looks very nice, but I wouldn't have known what a PacketStream does unless I read your post. You can add xml-doc comments to your methods, and it should make things clearer to other developers. \$\endgroup\$ – alexyorke Aug 23 '17 at 0:41
2
\$\begingroup\$

This approach adds a very good first glance at what will happen during code execution. If you look at PacketStreamFactoryyou can already see all the packets that the program is able to process.


Just a couple of tips:

Use enums when possible and cast int to enum not vice versa.

Currently:

Dictionary<int, object> _readerStrategies;
_readerStrategies.Add((int)PacketType.HEARTBEAT, new object());
_readerStrategies[packet.PacketTypeId];

Better:

Dictionary<PacketType, object> _readerStrategies;
_readerStrategies.Add(PacketType.HEARTBEAT, new object());
_readerStrategies[(PacketType)packet.PacketTypeId];

This is more readable as the cast to int does not confuse during packet registration.

If you want PacketStream to be reusable, you could introduce a type parameter for the package key.

PacketStream<T> and PacketBase<T>

Check your IDisposable implementation

IDisposable is not as simple as it looks.

See: MSDN - Disposable pattern

Naming Conventions:

Your enum member name are all uppercase and separated by underscores. This is not a common pattern in C#. (Of course this always depends on taste)

See: MSDN - Framework Design Guidelines

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.