HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Asynchronously accepting multiple client connections without impacting main thread

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
withoutmainimpactingclientacceptingasynchronouslymultiplethreadconnections

Problem

I am implementing a multi-player game in C# and having seen all sorts of projects, from indie developers all the way to large studios struggle to patch networking into existing products in the past, have made the decision to get the networking aspect done first, and build the game on top of that.

I've so far gotten to the stage where my server can accept clients asynchronously, but I have a strong feeling that I've implemented the threading very badly. I think it is possible that I am misunderstanding and/or misusing async/await and Tasks, but having read documentation and similar questions I think I will need practical experience to get my head wrapped around the correct usage of these.

As a result, I am hoping that someone would be generous enough to look over my code and help push me onto the right track. I have excluded 'using' statements and the client code for the purposes of this question, however I can confirm the code does function correctly.

I am not too concerned about issues such as naming, and the fact that I have a bit too much code logic in the Main function right now -- these will be modified and extracted to more appropriate locations respectively as the project progresses.

I have denoted lines I am particularly concerned about with <---- THIS

```
namespace Game_Server {
class ServerMain {
static void Main(string[] args) {
Server server = new Server();

server.Start().Start(); // _clients { get; set; }
public List clients { get {
return _clients.ToList();
} } //();
}

public Task Start() {
server.Start();

return new Task(async () => { // <---- THIS
while (true) {
TcpClient client = await server.AcceptTcpClientAsync();
_clients.Add(client);
Console.WriteLine("Connection on port 6780 from client " + client.Client.RemoteEndPoint);
}

Solution

Some quick remarks:

  • Always add access modifiers.



  • You're not using string[] args, so remove it.



  • I'm not a fan of private properties, certainly not when they could be fields (and thus they should not have getters/setters). private TcpListener server { get; }, private ConcurrentBag _clients { get; set; }



  • Be consistent in naming: _clients starts with an underscore, which is a naming convention usually used for fields, but server doesn't start with an underscore.



  • Properties should be PascalCase: public List clients. I feel the same applies to private ones (previous remark).



  • No underscores in names (with the exception of an underscore prefix for fields, of course): Game_Server.

Context

StackExchange Code Review Q#99298, answer score: 3

Revisions (0)

No revisions yet.