Comet Map System

[Tyrantosaurus]

Hi all,

I've started tinkering around with Spirited's Comet source and have started with implementing a "Map System", and I'd love some feedback on my approach.

Following reading Spirited's blog post on Multi-Threaded Game Server Design, my thoughts are like this:

A WorldProcessor with a ConcurrentDictionary linking a map ID to a MapProcessor. The world processor will handle IMapEvents and pass them to the appropriate MapProcessor based on the event's map ID. This allows for loading maps on the fly, as well as unloading maps as needed.

The MapProcessor is a BackgroundService utilizing a task channel as a queue that processes IMapEvents (maybe multiple channels, one for each entity type [player, mob, etc]?). This is similar to the existing PacketProcessor.

With this system, instead of packets being handled directly, they will call WorldProcessor with an event, ensuring packets are handled in order.

Thanks

[Spirited]

Hi Tyrantosaurus! That's a fun name. Your proposal is pretty solid, with the exception of using multiple channels per entity type. The reason why you potentially wouldn't want two channels for players and monsters is because that could cause locks on shared resources or race conditions during combat. But the idea of having a MapProcessor per map id is in-line with what I talked about in that article. An optimization might be to have multiple maps in a MapProcessor, but I wouldn't worry about that unless you start having problems with high parallelism. Good luck, and let me know if you want any additional feedback.

[Tyrantosaurus]

Thanks for the quick reply Spirited! Thank you for the compliment, I also thought it was fun.

Good call on the multiple channels, I will definitely avoid that. I like the optimization idea of combining multiple maps into a single processor. I wonder if I could experiment with having a single MapProcessor and split it out once it reaches a certain size or something? But I won't worry about that for now.

I think I've gotten a pretty decent system down so far. I'm just getting back into C# so there's probably some improvements to be made, but here's what I got:

namespace Comet.Game.MapSystem
{
   using Comet.Game.MapSystem.Events;
   using System.Collections.Concurrent;
   using System.Threading;

   internal class WorldProcessor
   {
       private readonly ConcurrentDictionary<uint, MapProcessor> _keyValuePairs = new();

       public void Queue(uint mapID, params IMapEvent[] events)
       {
           MapProcessor mapProcessor;
           while (!_keyValuePairs.TryGetValue(mapID, out mapProcessor))
               if (_keyValuePairs.TryAdd(mapID, mapProcessor = new MapProcessor(mapID)))
               {
                   mapProcessor.Enqueue(new MetaInitMap());
                   mapProcessor.StartAsync(new CancellationToken()).ConfigureAwait(false);
                   break;
               }

           foreach (var @event in events)
               mapProcessor.Enqueue(@event);
       }
   }
}

namespace Comet.Game.MapSystem
{
   using Microsoft.Extensions.Hosting;
   using System;
   using System.Linq;
   using System.Threading;
   using System.Threading.Channels;
   using System.Threading.Tasks;
   using Comet.Map;
   using Comet.Game.MapSystem.Events;
   using System.Collections.Concurrent;
   using System.Collections.Generic;

   public class MapProcessor : BackgroundService
   {
       private readonly Channel<IMapEvent> _channel = Channel.CreateUnbounded<IMapEvent>();
       private readonly ConcurrentDictionary<uint, IMapEntity> _entities = new();
       private readonly uint _mapID;
       private Map? _map;

       private CancellationToken _stoppingToken;

       public Map Map => _map.Value;
       public uint MapID => _mapID;

       public MapProcessor(uint mapID)
       {
           _mapID = mapID;
       }

       public void LoadMap()
       {
           if (_map.HasValue) return;

           _map = MapLoader.LoadMap(Program.Config.Meta.ConquerDirectory, _mapID);
       }

       protected override async Task ExecuteAsync(CancellationToken stoppingToken)
       {
           _stoppingToken = stoppingToken;

           while (!stoppingToken.IsCancellationRequested)
           {
               var @event = await _channel.Reader.ReadAsync(stoppingToken);

               await ProcessAsync(@event);
           }
       }

       public void Enqueue(IMapEvent @event)
       {
           if (_stoppingToken.IsCancellationRequested) return;

           _channel.Writer.TryWrite(@event);
       }

       private async Task ProcessAsync(IMapEvent @event)
       {
           try
           {
               await (@event.Type switch
               {
                   MapEventType.MetaInitMap => MetaInitMap.ProcessAsync(this, @event as MetaInitMap),
                   MapEventType.SpawnEvent => SpawnEvent.ProcessAsync(this, @event as SpawnEvent),
                   MapEventType.JumpEvent => JumpEvent.ProcessAsync(this, @event as JumpEvent),
                   MapEventType.WalkEvent => WalkEvent.ProcessAsync(this, @event as WalkEvent),
                   _ => throw new NotImplementedException(string.Format("Unhandled Map Event Type: {0}", @event.Type))
               });
           }
           catch (NotImplementedException ex)
           {
               Console.WriteLine(ex.Message);
           }
           catch (Exception ex)
           {
               Console.WriteLine(ex.Message);
               if (ex.InnerException != null) Console.WriteLine(ex.InnerException.Message);
           }
       }

       public void AddEntity(IMapEntity entity)
       {
           _entities.TryAdd(entity.ID, entity);
       }

       public void RemoveEntity(IMapEntity entity)
       {
           _entities.TryRemove(entity.ID, out _);
       }

       public IEnumerable<IMapEntity> Entities()
       {
           return _entities.Values;
       }
   }
}

[Tyrantosaurus]

After doing some more pondering, I think I am going to refactor the packet processor and essentially merge it with the map processor.

So the partition IDs will become the character's map ID, or 0 before the MsgConnect packet is sent. This way I can handle packets directly since they are already part of that map's queue.

It simplifies the code as I no longer need packet handlers and event handlers, since they would be one and the same.

If I have an event that is not generated by the client, I can just queue a packet to that client directly. Custom packets could be created as needed too.

What do you think of this approach?

[Spirited]

Hm... so the original design of Comet was to prototype for another project. That other project utilizes a proxy and separate map servers. That's the reason why they're separated out. If you feel having a single processor is more beneficial for a single executable server, then it sounds reasonable. Though, I'm not sure if all actions can / need to be written to the map channels. For example, MsgRegister and MsgConnect. If you do combine it, then you might want to consider a backup for these types of packets.

[Konichu]

Hm... so the original design of Comet was to prototype for another project. That other project utilizes a proxy and separate map servers. That's the reason why they're separated out. If you feel having a single processor is more beneficial for a single executable server, then it sounds reasonable. Though, I'm not sure if all actions can / need to be written to the map channels. For example, MsgRegister and MsgConnect. If you do combine it, then you might want to consider a backup for these types of packets.

I'm filtering some packets that doesn't change world states and they're processed in the socket processor. The rest of them are queued into the world processor.

[Tyrantosaurus]

Little update on this.

I've went back to the event system, it scales better and works with events that originate outside of the client. For instance, anything that occurs after a set amount of time, like items despawning. There was no way to make this work without a client attached to the packet processor queue.

I'm filtering some packets that doesn't change world states and they're processed in the socket processor. The rest of them are queued into the world processor.

Interesting, which packets for example? I have every packet (except for MsgRegister, MsgConnect) handled through the map event queue, it ensures that any data being processed is in sync.

[Spirited]

Little update on this.

I've went back to the event system, it scales better and works with events that originate outside of the client. For instance, anything that occurs after a set amount of time, like items despawning. There was no way to make this work without a client attached to the packet processor queue.

I'm filtering some packets that doesn't change world states and they're processed in the socket processor. The rest of them are queued into the world processor.

Interesting, which packets for example? I have every packet (except for MsgRegister, MsgConnect) handled through the map event queue, it ensures that any data being processed is in sync.

Hm... I'd have to look into the complexity a bit more, but the idea was to have those events also queue on the channel. Things like monster movement, monster attacks, item usage, and item dropping/despawning would all be queued with player actions. If you were to take a multi-threaded approach, you could have a player thread and item despawn thread race to either pick up or despawn an item. Depending on how you handle those conditions, you could accidentally create a null object that crashes the server, or in the best case a poor player experience (where in the channel system, it would favor the player).

[Tyrantosaurus]

Hm... I'd have to look into the complexity a bit more, but the idea was to have those events also queue on the channel. Things like monster movement, monster attacks, item usage, and item dropping/despawning would all be queued with player actions. If you were to take a multi-threaded approach, you could have a player thread and item despawn thread race to either pick up or despawn an item. Depending on how you handle those conditions, you could accidentally create a null object that crashes the server, or in the best case a poor player experience (where in the channel system, it would favor the player).

Yeah that is how I have it. Essentially everything that happens on a map is queued in that map's single channel.

Each map has a tick that queues a "Tick" event, which does things like removing status effects after they have expired, or removing items from the map.