search

Unity-Technologies / com.unity.netcode.gameobjects

Netcode for GameObjects is a high-level netcode SDK that provides networking capabilities to GameObject/MonoBehaviour workflows within Unity and sits on top of underlying transport layer.
MIT License
2.16k stars 435 forks source link

Client ID is not mapped to sequential value prior to OnClientDisconnectCallback invocation, causing callback to return incorrect value #1440

Closed kjohnston-ooi closed 2 years ago

kjohnston-ooi commented 3 years ago

Describe the bug When NetworkManager.OnClientDisconnectCallback is triggered, the wrong value is being returned for the client ID. Specifically, when using the Unity Transport, the value returned is the original randomized ID rather than the sequential IDs that are now used everywhere else.

To Reproduce Steps to reproduce the behavior:

  1. Start a server
  2. Connect a client
  3. Observe the client ID sent in the OnClientConnectedCallback has a value of 1
  4. Disconnect the client

Actual outcome The value returned in the OnClientDisconnectCallback will not match the value from the connected callback. In the case of the Unity Transport, it will be a randomized ID such as 8589934592.

Expected outcome The value returned in the disconnect callback would match the value from the connected callback. In my case we are handling player objects ourselves rather than having NGO automatically destroy them on disconnect, so without the correct value being returned here we were unable to determine which player actually disconnected and handle their data accordingly.

Additional notes I believe this issue originates in NetworkManager.HandleRawTransportPoll - for the NetworkEvent.Disconnect case of the switch statement, the line clientId = TransportIdToClientId(clientId); is made after OnClientDisconnectCallback?.Invoke(clientId);, thus the transport (randomized) ID is returned rather than the sequential client ID. In the two other cases handled by that switch statement, the mapping happens before the client ID is used elsewhere. Making this change on my end, I can confirm the disconnect callback returns the correct value.

Environment (please complete the following information):

JayPeet commented 3 years ago

This problem occurs on both the UNET transport and the Ruffles transports, with a couple differences in which IDs you get (because as @kjohnston-ooi says, the TransportIdToClientId call appears to be in the wrong place.)

As far as I can tell from working on a fix on my fork (PR: https://github.com/Unity-Technologies/com.unity.netcode.gameobjects/pull/1474) doing the TransportIdToClientId call should have no side-effects

ShadauxCat commented 2 years ago

Fixed by #1477