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.13k stars 430 forks source link

Client-authoritative NetworkTransform spawns in wrong position #2531

Closed Maclay74 closed 1 year ago

Maclay74 commented 1 year ago

Description

When spawning a NetworkObject using SpawnAsPlayerObject method, it always appear in 0,0,0, ignoring the position it had when it was instantiated.

Reproduce Steps

  1. Create a simple ClientNetworkTransform component that overrides OnIsServerAuthoritative method to return false
  2. Assign this component to a prefab
  3. Spawn prefab on server in some place, other than origin

Actual Outcome

Spawned prefab teleports immediately to scene's origin

Expected Outcome

Spawned prefab should remain the same position it had before spawning over the network.

Screenshots

If applicable, add screenshots to help explain your problem.

Environment

Additional Context

Please take a look at this video.

https://user-images.githubusercontent.com/5504685/234705013-b02f6e37-c108-40d0-bfdd-57b9fecc20e7.mp4

The way I instantiate and spawn the prefab:

public class SceneManager : NetworkBehaviour {

    public GameObject playerPrefab;
    public Transform spawnPosition;

    public void Start() {
        NetworkManager.Singleton.OnClientConnectedCallback += OnClientConnected;
    }
    void OnClientConnected(ulong clientId) {
        if (IsHost) {
            var newPlayer = Instantiate(playerPrefab, spawnPosition.position + spawnOffset, Quaternion.identity);
            newPlayer.GetComponent<NetworkObject>().SpawnAsPlayerObject(clientId);
        }
    }
}

Thanks a lot!

Maclay74 commented 1 year ago

Oh and before opening the ticket, I asked this in the official Discord and I was told that this issue was resolved, so I think that I probably do something wrong here and spawning a client-authoritative network object requires some additional effort.

NoelStephensUnity commented 1 year ago

@Maclay74 The issue is primarily focused around the fact that the client (owner) needs to dictate its spawned position since you are using the owner authoritative model. What is happening is that the server instantiates with the offset, but the client (owner) is what defines the initial value.
So, there are really a couple of ways to handle spawning a specific object for a player, but for your project (since I am familiar with it) I might suggest making the following modifications:

Step 1: Default the player prefab's CharacterController to "disabled

image

Step 2: Replace your SceneManager with this:

    public class SceneManager : NetworkBehaviour
    {
        public static SceneManager Instance { get; internal set; }
        public GameObject PlayerPrefab;
        public Transform SpawnPosition;
        public Vector3 SpawnOffset;

        private void Awake()
        {
            Instance = this;
        }

        public override void OnNetworkSpawn()
        {
            if (IsHost)
            {
                // Host spawns itself
                SpawnPlayer(NetworkManager.ServerClientId);
                // Host then registers for client's connecting
                NetworkManager.OnClientConnectedCallback += OnClientConnected;
            }
            base.OnNetworkSpawn();
        }

        public override void OnNetworkDespawn()
        {
            if (IsHost)
            {
                NetworkManager.OnClientConnectedCallback -= OnClientConnected;
            }
            base.OnNetworkDespawn();
        }

        void OnClientConnected(ulong clientId)
        {
            if (IsHost)
            {
                SpawnPlayer(clientId);
            }
        }

        private void SpawnPlayer(ulong clientId)
        {
            var newPlayer = Instantiate(PlayerPrefab);
            newPlayer.GetComponent<NetworkObject>().SpawnWithOwnership(clientId);
        }

        public Vector3 GetSpawnPoint()
        {
            return SpawnPosition.position + SpawnOffset;
        }

        public Quaternion GetFacing()
        {
            return Quaternion.identity;
        }
    }

Step 3: Replace your ClientNetworkTransform with this:

    public class ClientNetworkTransform: NetworkTransform {
        protected override bool OnIsServerAuthoritative() => false;
        public override void OnNetworkSpawn()
        {
            if (IsOwner)
            {
                transform.position = SceneManager.Instance.GetSpawnPoint();
                transform.rotation = SceneManager.Instance.GetFacing();
                GetComponent<CharacterController>().enabled = true;
            }
            base.OnNetworkSpawn();
        }
    }

Explanation:

The CharacterController will collide with things if it starts off enabled. So, you can prevent any collisions that could offset your initial spawn point by making the default for it disabled and then only enabling it for the owner/client.

The host can go ahead and spawn itself since it is really "already connected" to the server and doesn't need to synchronize. To avoid client's triggering the same OnClientConnectedCallback, you should register for that event after the host has spawned its player. From that point forward, when a client connects you just need to spawn an instance of that player (avoid setting position and rotation during instantiation since your are using an owner authoritative model). Finally, we keep the instance of the SceneManager static for all spawned instances of the players to be able to get their respective spawn position and facing.

Within the ClientNetworkTransform, the owner will get and set its spawn position and rotation/facing. Then it enables its CharacterController to start colliding with things (the assumption is the spawn position won't be inside something).

The end result should be that everyone spawns on the "Spawn" object position with the +1 Y offset.

Let me know if this resolves your issue?

Maclay74 commented 1 year ago

Hey @NoelStephensUnity! Thanks for the solution! Yes, I think it resolves my problem!

Just to clarify, If at some moment during the gameplay I need to move a player to another position, I would call a ClientRpc with target position, right?

Maclay74 commented 1 year ago

And about

avoid setting position and rotation during instantiation since your are using an owner authoritative model

I tried without it and a newly spawned player appears in scene's origin for RTT on server, When I instantiate it in desired position, it doesn't happen. Maybe I could keep it?

https://user-images.githubusercontent.com/5504685/234849057-e494f8c9-0011-4d38-bd8a-3025437fa8d1.mp4

NoelStephensUnity commented 1 year ago

Well... hmmm.. sure if it prevents it... but as long as it is set on the owner's side prior to invoking the base OnNetworkSpawn it should automatically teleport to that position without interpolation...so there could be an issue elsewhere.

Maclay74 commented 1 year ago

Yes, it does teleport without interpolation, but the order of actions is next (please correct me if I'm wrong)

  1. Instantiate prefab on server as a regular game object, without setting position and rotation it appears in 0,0,0
  2. Spawn network object, notify clients and owner
  3. Owner sets new position and rotation, sends updated transform to the server
  4. Server broadcasts it to other clients

So, server and clients are going to see updated position in RTT, right? I don't see any issue, but I think I have to instantiate with position and rotation that owner will confirm on their side.

NoelStephensUnity commented 1 year ago

@Maclay74 That is pretty much correct.

That is pretty much the timing layout. What you can do to have precise "upon spawn" placement (if you want to keep your method of spawning) is the following:

This should allow you to keep pretty much the exact same approach with just the adjustments in who is the owner. In the ClientNetworkTransform, you would need to override the "OnGainedOwnership" and enable the CharacterController locally when that is invoked. You would also need to have the special case for the host as to when you enable the CharacterController...sort of like this:

public class ClientNetworkTransform : NetworkTransform
{
    protected override bool OnIsServerAuthoritative() => false;

    private bool HostHasPlayerObject()
    {
        if (!IsHost)
        {
            return false;
        }

        if (NetworkManager.LocalClient != null && NetworkManager.LocalClient.PlayerObject != null)
        {
            var hostCharacterController = NetworkManager.LocalClient.PlayerObject.GetComponent<CharacterController>();
            if (hostCharacterController != null && hostCharacterController.enabled)
            {
                return true;
            }
        }
        return false;
    }

    public override void OnNetworkSpawn()
    {
        if (IsOwner)
        {
            transform.position = SceneManager.Instance.GetSpawnPoint();
            transform.rotation = SceneManager.Instance.GetFacing();
            if (!HostHasPlayerObject())
            {
                GetComponent<CharacterController>().enabled = true;
            }
        }
        base.OnNetworkSpawn();
    }

    public override void OnGainedOwnership()
    {
        if (!IsHost && IsOwner)
        {
            GetComponent<CharacterController>().enabled = true;
        }
        base.OnGainedOwnership();
    }
}

(did not test the above code...it should work...but if it doesn't let me know) This way the host only enables the CharacterController once for its local player and for all other client players it leaves them disabled. Then when the host transfers ownership to the client, the client will enable it locally.

Something along these lines would assure the spawn position and rotation is set when spawned and that there is no latency between the client owner, the host, and the other connected clients.

Maclay74 commented 1 year ago

Got it, now it's crystal clear! Thanks again for your support! I'll try this approach!