Custom Packets being replayed to other players?

[Kreag]

Hey All,
I'm running into something strange.  I created custom packets for Login/Registration.  When one player logs in and gets the response (sent only to that player), the next player seems to be loading my next scene after the main menu without logging in (and stays as "guest").

Here is my server code that checks the login and sends the response to the originating client:

1. case Packet.LoginRequest:
2.             {
3.                 string email = reader.ReadString();
4.                 string password = reader.ReadString();
5.  
6.                 Task loginTask = new Task(() =>
7.                     {
8.                         DBRepository db = new DBRepository();
9.                         var result = db.Login(email, password);
10.  
11.                         //check to see if this player is already logged in
12.                         foreach (var user in mPlayers)
13.                         {
14.                             if (user.name == result.Name)
15.                             {
16.                                 result.LoggedIn = false;
17.                                 result.Name = "Already Logged In";
18.                             }
19.                         }
20.  
21.                         player.name = result.Name;
22.  
23.                         BinaryWriter writer = BeginSend(Packet.LoginResponse);
24.                         writer.Write(result.LoggedIn);
25.                         writer.Write(result.Name);
26.                         EndSend(true, player);
27.                         Console.WriteLine("Login Result: " + result.LoggedIn.ToString() + " : " + result.Name);
28.                     });
29.                 loginTask.Start();
30.                 break;
31.             }

The player that receives the response grabs the player name then loads the main game scene.  Why would subsequent players load the next scene immediately?

Here is my client code for receiving the response for completeness sake:

1. void LoginResult(Packet result, BinaryReader reader, IPEndPoint source)
2.     {
3.         bool loggedin = reader.ReadBoolean();
4.         string name = reader.ReadString();
5.         if (loggedin)
6.         {
7.             serverMessage.text ="Login Success, loading...";
8.             TNManager.player.name = name;
9.             TNManager.LoadLevel("World1");
10.         }
11.         else
12.         {
13.             serverMessage.text = name;
14.         }
15.     }

[ArenMook]

Well, first a word of warning: you are creating a task there -- so basically a new thread. What you're doing then happens sometime later rather than right away. TNet is not locked in the background while you are doing this, so your BeginSend/EndSend will conflict with what TNet's thread is doing as well. Your code isn't thread safe.

Bottom line is, don't create tasks there.

Now onto your question -- when you do TNManager.LoadLevel, it switches the channel's level, so the next person to join will load this new level instead. If you don't want the new player to load a new level, then use different channels.

[Kreag]

This is good information, Thanks Aren!
So I'll have to switch to another channel prior to loading the new level, that will work fine.

As for the custom packet, the reason I'm creating a task is that I don't really want the packets to stop processing while I access the database.  I have to take your word for it that TNet could be conflicted when I send the result of the database action back to the player.  Is there a way to get that packet back into a queue to be sent?  I don't really care how long it takes.  Is there another pattern you would suggest?

[ArenMook]

All packets are processed in TNet in one place -- TNet.GameServer's ThreadFunction(). This should be the only place where packets are sent out as well. Note that there is no thread locking happening, because this is the only place where sending and receiving can occur. Since you are trying to do it from a separate thread, you would have to add some kind of thread locking. It might be easier to add a queue instead, and when you want to modify the queue you'd lock it first. You would use the same queue inside ThreadFunction() -- if it's not empty, lock it, grab the first item, process it (and send out the packet), continue until it's empty. This kind of a question is really outside TNet's scope at this point -- it's a C# multi-threading question.

[Kreag]

Thanks again Aren,
So If I put something like this in the Threading function (near the end just before you test for received).

1. // Process Database send queue
2.             if (mDatabaseQueue.Count > 0)
3.             {
4.                 for (int i = 0; i < mDatabaseQueue.Count; )
5.                 {
6.                     lock (mDatabaseQueue)
7.                     {
8.                         var sendItem = mDatabaseQueue.Dequeue();
9.                         if (sendItem.Player.isConnected)
10.                         {
11.                             BinaryWriter writer = BeginSend(sendItem.PacketType);
12.                             foreach (var item in sendItem.SendItems)
13.                             {
14.                                 writer.Write(item.ToString());
15.                             }
16.                             EndSend(true, sendItem.Player);
17.                         }
18.                     }
19.                 }
20.             }

I'm getting the nasty "endofstreamexception".  I'm not sure why.. I'm doing the same thing that I would if I was in the big packet case statement.  Any insight into that would be helpful.

[ArenMook]

You never increment 'i', so it would make sense to put it in a "while (mDatabaseQueue.Count != 0)" instead. Locking something also only makes it thread-safe if you do it in all places where the queue would be accessed.

[Kreag]

bahaha! I can't believe I did that.. lol thanks Aren.

[Kreag]

All that is true, but I don't think that explains the endofstreamexception?  (I am locking the queue everywhere I use it)
I also changed that for loop to a while (good eye).

[ArenMook]

Not sure about end of stream, that's something on the socket side. Perhaps the socket was already closed. Did you try to narrow it down with try/catch blocks to see exactly where it happens?

[Kreag]

Yeah, I did a try/catch.  I've debug/logged the exception and also read the inner exception.  It's complaining about a null/reference exception.  Odd thing is that if I send the same packet using the non-thread safe method (with only one guy connected), it works fine.  I'm not sure what is different.  The Processplayerpacket function does the same thing I am doing and in almost the same spot that I am doing it.  Also, if I hook up a debugger session to monodevelop (I normally use VS2013), the client can't connect initially (Is that a known issue?).

[Kreag]

Here is the exception on the client:

System.IO.EndOfStreamException: Failed to read past end of stream.
  at System.IO.BinaryReader.FillBuffer (Int32 numBytes) [0x0005b] in /Users/builduser/buildslave/monoAndRuntimeClassLibs/build/mcs/class/corlib/System.IO/BinaryReader.cs:119
  at System.IO.BinaryReader.ReadString () [0x00059] in /Users/builduser/buildslave/monoAndRuntimeClassLibs/build/mcs/class/corlib/System.IO/BinaryReader.cs:486
  at MainMenu.LoginResult (Packet result, System.IO.BinaryReader reader, System.Net.IPEndPoint source) [0x00007] in E:\Dev\Unity\SpaceUnityTNet\Assets\Scripts\MainMenu.cs:76
UnityEngine.Debug:Log(Object)
MainMenu:LoginResult(Packet, BinaryReader, IPEndPoint) (at Assets/Scripts/MainMenu.cs:91)
TNet.GameClient:ProcessPacket(Buffer, IPEndPoint) (at Assets/TNet/Client/TNGameClient.cs:766)
TNet.GameClient:ProcessPackets() (at Assets/TNet/Client/TNGameClient.cs:719)

[ArenMook]

Unfortunately that doesn't tell me much as the error happens in the System class. All kinds of things can go wrong with a multi-threaded approach, which is why proper thread-safe programming is quite challenging.