ValksGodotTools / Multiplayer-2023-Old

Client + server template
MIT License
8 stars 2 forks source link

"[Client] 0 does not exist in Players" #8

Closed valkyrienyanko closed 1 year ago

valkyrienyanko commented 1 year ago

Issue

The error spam happens when the 3rd client joins.

Steps to Reproduce

  1. Godot > Debug > Run Multiple Instances -> Run 3 Instances
  2. Start the server and client in one of the instances
  3. Start the other 2 clients in the other 2 instances
  4. Notice how the error appears when the 3rd client connects

Where the issue happens in the code

https://github.com/Valks-Games/Multiplayer-Template/blob/1635b43c5010af6fb23b77570d292b4be1d1bfa0/Scripts/GameMaster.cs#L47

Preview

Untitled

valkyrienyanko commented 1 year ago

Test Plan

  1. Temporarily disable position packets for both client and server
  2. Verify that the player list is the same on every client and the server
  3. Test this for 2 instances then 3 instances then 4 instances
  4. Re-enable position packets
  5. If still having troubles consider making another debug UI bound to F10 to give a visual of the data in each client / server. No idea how this UI will look. It would be more dedicated to watching specific values that were printed with .PrintFull() whenever they were updated unlike the console that just spams every log message.
valkyrienyanko commented 1 year ago

Untitled

Testing with 3 clients. Position packets have been disabled.

The server has all 3 clients accounted for.

Client 0 has clients 1 and 2. Client 1 has clients 0 and 2. Client 2 only has client 1 <--- this is where the issue lies

Client 2 receives client id 1 twice which is what is causing the same key added error.

E 0:01:15:0446   :0 @ void System.ThrowHelper.ThrowAddingDuplicateWithKeyArgumentException<T >(T ): System.ArgumentException: An item with the same key has already been added. Key: 1
  <C++ Error>    System.ArgumentException
  <C++ Source>   :0 @ void System.ThrowHelper.ThrowAddingDuplicateWithKeyArgumentException<T >(T )
  <Stack Trace>  :0 @ void System.ThrowHelper.ThrowAddingDuplicateWithKeyArgumentException<T >(T )
                 :0 @ Boolean System.Collections.Generic.Dictionary`2.TryInsert(TKey , TValue , System.Collections.Generic.InsertionBehavior )
                 :0 @ void System.Collections.Generic.Dictionary`2.Add(TKey , TValue )
                 GameMaster.cs:47 @ void Sandbox2.GameMaster.AddPlayer(UInt32 , Godot.Vector2 )
                 SPacketPlayerJoin.cs:22 @ void Sandbox2.SPacketPlayerJoin.Handle()
                 ENetClient.cs:114 @ void GodotUtils.Netcode.Client.ENetClient.HandlePackets()
                 GameMaster.cs:18 @ void Sandbox2.GameMaster._PhysicsProcess(Double )
                 Node.cs:1771 @ Boolean Godot.Node.InvokeGodotClassMethod(Godot.NativeInterop.godot_string_name& , Godot.NativeInterop.NativeVariantPtrArgs , Godot.NativeInterop.godot_variant& )
                 Sandbox2.GameMaster_ScriptMethods.generated.cs:33 @ Boolean Sandbox2.GameMaster.InvokeGodotClassMethod(Godot.NativeInterop.godot_string_name& , Godot.NativeInterop.NativeVariantPtrArgs , Godot.NativeInterop.godot_variant& )
                 CSharpInstanceBridge.cs:24 @ Godot.NativeInterop.godot_bool Godot.Bridge.CSharpInstanceBridge.Call(IntPtr , Godot.NativeInterop.godot_string_name* , Godot.NativeInterop.godot_variant** , Int32 , Godot.NativeInterop.godot_variant_call_error* , Godot.NativeInterop.godot_variant* )

So why does it work for the first 2 clients but not the 3rd?

valkyrienyanko commented 1 year ago

I have checked the server side and it is indeed sending all the correct data!

Untitled

valkyrienyanko commented 1 year ago

I found the issue. Putting things back to the way they were fixed this. Instead of enqueing the packet to the Godot queue and handling it on the Godot thread. I handle it like I did before, on the ENet-Client thread. And now everything fixed. But why what was causing such a big issue??

            /*
             * Instead of packets being handled client-side, they are handled
             * on the Godot thread.
             * //handlePacket.Handle();
             */
            handlePacket.Handle();
            //GodotPackets.Enqueue(handlePacket);
            packetReader.Dispose();
valkyrienyanko commented 1 year ago

After more research into this I have found out that ConcurrentQueue.TryDequeue(...) can contain duplicate data! I never knew this was a thing!

https://www.google.com/search?q=C%23+concurrentqueue+trydequeue+duplicate+data

I have tried switching to BlockingCollection and ConcurrentBag but neither have worked. I'm not really sure what I'm doing.

I'm going to be trying to do ConcurrentDictionary next.

Someone answering some elses question on stackoverflow said they needed to deep clone before enqueuing the instance. I have found a newtonsoft util function that deep clones classes. But it did not work because my instance is from a abstract class. And I'm not even sure if deep cloning is the play here...

valkyrienyanko commented 1 year ago

ConcurrentDictionary did not work either...

I have no idea what I am doing anymore. Nothing I am trying is working...

valkyrienyanko commented 1 year ago

No matter what I do, there is always the duplicate data on the other end 100% of the time.

valkyrienyanko commented 1 year ago

Untitled

Solution is to send over packet reader and handle packet instead of just handle packet.

No idea why this fixes everything...

Some black magic I'd say.