Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Destroy a spawned NetworkObject on a non-host client is not valid error if a server spawns NetworkObjects while a client connects to the already running game #2706

Open
ximael opened this issue Sep 19, 2023 · 6 comments · May be fixed by #2735
Assignees
Labels
stat:imported Issue is tracked internally at Unity type:bug Bug Report

Comments

@ximael
Copy link

ximael commented Sep 19, 2023

Description

If a server spawns NetworkObjects while a client connects to the already running game, I get this error:

Client error:
[Netcode] Destroy a spawned NetworkObject on a non-host client is not valid. Call Destroy or Despawn on the server/host instead.
...
(at ./Library/PackageCache/com.unity.netcode.gameobjects@1.6.0/Runtime/Core/NetworkObject.cs:556)

Server error:
[Netcode-Server Sender=1] Destroy a spawned NetworkObject on a non-host client is not valid. Call Destroy or Despawn on the server/host instead.

Sample project:
https://www.dropbox.com/scl/fi/sw7xgbyhxrz1vcchojnkc/NetcodeTest.zip?rlkey=jhkbq0w9k803p3al5ife9iz7p&dl=0
In the game project we do not spawn a lot of network objects, so the error is really rare. But in the test project it is to detect the problem.

Unity forum thread:
https://forum.unity.com/threads/destroy-a-spawned-networkobject-on-a-non-host-client-is-not-valid.1493204/

Reproduce Steps

  1. Host
  2. Join
  3. See error

Environment

  • OS: [Win 10]
  • Unity Version: [2022.3.7f1 LTS]
  • Netcode Version: [1.6]
@ximael ximael added stat:awaiting triage Status - Awaiting triage from the Netcode team. type:bug Bug Report labels Sep 19, 2023
@chrispope chrispope added the stat:imported Issue is tracked internally at Unity label Oct 2, 2023
@fluong6 fluong6 added stat:import stat:imported Issue is tracked internally at Unity and removed stat:imported Issue is tracked internally at Unity stat:import stat:awaiting triage Status - Awaiting triage from the Netcode team. labels Oct 4, 2023
@NoelStephensUnity
Copy link
Contributor

NoelStephensUnity commented Oct 10, 2023

@ximael

There is something you could do on your end to resolve the issue(s) you are experiencing:

  • Update your actual project so:
    • The server-host is configured to use additive client synchronization.
    • The clients set post synchronization scene unloading to true.
    • Depending upon how many NetworkObjects you will have spawned at any given time, I would make sure your maximum payload size is larger than the default which really isn't larger enough (IMO).
      • I updated your NetcodeTest project to a very high value (like 1.5MB) since you were spawning objects at a pretty fast rate, but you might think about anywhere between 64KB to 128KB (i.e. like you never have more than 100 objects spawned at one time).

I went ahead and attached an updated version of your netcode test project (thank you for this repro project! It greatly helps in many ways)
UpdatedNetcodeTest.zip

The NetworkSceneManager related changes:

    public void StartHost()
    {
        Debug.Log("NetManager.StartHost Begin");
        state = State.Connecting;
        if (NetworkManager.Singleton.StartHost())
        {
            _connStartTime = Time.time;
            NetworkManager.Singleton.SceneManager.OnLoadComplete += OnLoadComplete;
            /// Set client synchronization to additive
            /// For more information <see href="https://docs-multiplayer.unity3d.com/netcode/current/basics/scenemanagement/client-synchronization-mode/"/>
            NetworkManager.Singleton.SceneManager.SetClientSynchronizationMode(LoadSceneMode.Additive); // <-------- Prevents initial issue
            NetworkManager.Singleton.SceneManager.LoadScene(gameSceneName, LoadSceneMode.Single);
        }
        else Debug.LogError("SteamManager.StartHost ERROR");
    }
    
    public void StartClient()
    {
        Debug.Log($"NetManager.StartClient Begin");
        state = State.Connecting;
        
        if (NetworkManager.Singleton.StartClient())
        {
            _connStartTime = Time.time;
            /// Clients should unload any scenes not used during the synchronization process
            /// (i.e. if the server didn't tell the client to load the scene then it will be unloaded)
            /// You can control what scenes a client unloads by subscribing to <see cref="NetworkSceneManager.VerifySceneBeforeUnloading"/> on
            /// the client or server side (depending on which side it is needed)
            NetworkManager.Singleton.SceneManager.PostSynchronizationSceneUnloading = true; // <-------- Unloads any scenes not used
            NetworkManager.Singleton.SceneManager.OnLoadComplete += OnLoadComplete;
        }
        else Debug.LogError("SteamManager.StartClient ERROR");
    }

Let us know if that resolves your issue or if you run into any other issues using those adjusted NetworkSceneManager settings?

We will look into either improving the error message or the possibility of deferring the processing of CreateObjectMessages until after client synchronization has completed (or possibly both).

@NoelStephensUnity
Copy link
Contributor

NoelStephensUnity commented Oct 11, 2023

@ximael
If you do not want to change your client synchronization mode for project specific purposes, the update after the up-and-coming v1.7.0 update will contain a fix (#2735) for your issue.
👍

@lumium-mpaff
Copy link

I encountered the same behaviour with our project. This project is using OpenXR and the XR Interaction Toolkit as well. On connect the client spawns via ServerRpc its own XR rig with client ownership. We encountered this bug when the users are connecting simultaneously, so that the XR rig of other clients is spawned in the unloading scene - we worked around this like @NoelStephensUnity proposed by using additive client synchronization and post synchronization scene loading.

But we still encounter this bug by changing scenes on the server when the server spawns multiple objects (5 objects of the same prefab are spawned correctly, the other 3 to 8 objects of another prefab are spawned and run into this issue) by a NetworkBehaviour's OnNetworkSpawn. Our current workaround is letting the server spawn those network objects after the callback of the LoadEventCompleted scene event. Since this is pretty ugly (some clients can see the scene before the objects are spawned), I don't think that this is the intended behaviour.

I also tried using the fix from your pull request on version 1.6.0 of Netcode, since a checkout of the develop branch makes Unity 2021.3.22f1 continuously throw errors when loaded into the package manager. Using this fix with 1.6.0 makes Unity check whether a Synchronize event is active via IsSceneEventActive, but the SceneEventDataStore on the client is always empty, so it returns false and doesn't defer the objects. I'm not sure why this is the case - we are using the internal SceneManagement of Netcode. I don't want to rule out that I am using that in a wrong way, but everything else seems to be working as intended.

Since the deadline of the project is coming up pretty soon, we'll have to use the workaround - I just wanted to let you know that there seem to be cases when this bug still occurs.

If you need a sample project for this, let me know - but I'll probably won't be able to provide you with this in the next few days because of the project's deadline, that inches closer every minute.

@NoelStephensUnity
Copy link
Contributor

NoelStephensUnity commented Oct 17, 2023

@lumium-mpaff
Hmm, so I am seeing that there could another case where this could be useful during the scene SceneEventType.Load.
However, could you provide additional details regarding how you are handling the scene loading?

  • Are you using SceneLoadMode.Single or SceneLoadMode.Additive when loading?
    • If single mode, then I might need to make an adjustment for that since the PR only handles client synchronization.
    • If additive, then as long as your currently active scene (server side) isn't going to be unloaded you can spawn those objects prior to loading and then begin the loading process.
      • Not sure of the context, but you could default certain components disabled with one NetworkBehaviour that contains a simple bool NetworkVariable that when enabled will enable those components... so basically the the visuals/game-related logic are disabled until the LoadEventComplete event on the server side or alternately either a component in the scene being loaded "enables" the spawned objects or the spawned objects wait for that specific scene to be loaded (i.e. LoadEventComplete event client side) and then they enable themselves...where all NetworkBehaviour components would still need to be enabled initially but could exit early in updates/etc until a specific condition is reached (i.e. scene loaded or NeworkVariable value).

However, I will definitely look into deferring CreateObjectMessages for SceneEventType.Load before that PR is finalized.

@NoelStephensUnity
Copy link
Contributor

NoelStephensUnity commented Oct 17, 2023

@lumium-mpaff

I also tried using the fix from your pull request on version 1.6.0 of Netcode, since a checkout of the develop branch makes Unity 2021.3.22f1 continuously throw errors when loaded into the package manager

If you have time (understanding your deadline), could you provide the errors you are encountering in the editor when using the develop branch?

Not sure if this is how you did it, but you can use a specific branch by replacing your packages/manifest.json file's com.unity.netcode.gameobjects entry to something like this:
"com.unity.netcode.gameobjects": "https://github.com/Unity-Technologies/com.unity.netcode.gameobjects.git?path=com.unity.netcode.gameobjects#fix/networkobject-destroy-during-client-synchronization-MTT-7539",

Where everything after the # is nothing more than the branch name.

You might see if that works out better for you.

@NoelStephensUnity
Copy link
Contributor

@lumium-mpaff
#2735 now includes support for deferring CreateObjectMessage driven instantiation and spawning during a LoadSceneMode.Single scene loading event.

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stat:imported Issue is tracked internally at Unity type:bug Bug Report
Projects
None yet
6 participants