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

fix(NetworkClient): Defer ApplySpawnPayload until OnObjectSpawnFinished #3097 #3961

Merged
merged 3 commits into from
Dec 30, 2024

Conversation

MrGadget1024
Copy link
Collaborator

@MrGadget1024 MrGadget1024 commented Dec 29, 2024

Virtually eliminates null refs with GO/NI/NB SyncVars by getting all objects into spawned dictionary before applying payloads and invoking hooks.

  • No change to normal spawning, only initial spawn
  • Uses and clears a static dictionary, so no garbage allocation

See ticket #3097

Test with PickupsDropsChilds example:

  • Added SyncList to PickupsDropsChilds on player for everything player drops
  • Added SyncVar to SceneObject for owner player object
  • Started with built host client, dropped 3 things, then joined client from editor
  • Depending on spawn order, either the SyncVar hook or the SyncList OnAdd handler would've thrown NRE's
public readonly SyncList<GameObject> drops = new SyncList<GameObject>();

public override void OnStartClient()
{
    drops.OnAdd += OnAdd;
    for (int i = 0; i < drops.Count; i++)
        drops.OnAdd?.Invoke(i);
}

void OnAdd(int index)
{
    Debug.Log($"OnAdd {index} {drops[index].name}");
}

[Command]
void CmdDropItem()
{
    . . .
    // set owner object
    sceneObject.owner = gameObject;
    . . .
    // Spawn the scene object on the network for all to see
    NetworkServer.Spawn(newSceneObject);

    drops.Add(newSceneObject);
}
[ReadOnly, SyncVar(hook = nameof(OnOwnerChanged))]
public GameObject owner;

void OnOwnerChanged(GameObject _, GameObject newOwner)
{
    Debug.Log($"SyncVar hook OnOwnerChanged {owner.name}", owner);
}

Before this PR

NullReferenceException: Object reference not set to an instance of an object
Mirror.Examples.PickupsDropsChilds.PickupsDropsChilds.OnAdd (System.Int32 index) (at Assets/Mirror/Examples/PickupsDropsChilds/Scripts/PickupsDropsChilds.cs:40)
Mirror.Examples.PickupsDropsChilds.PickupsDropsChilds.OnStartClient () (at Assets/Mirror/Examples/PickupsDropsChilds/Scripts/PickupsDropsChilds.cs:35)
Mirror.NetworkIdentity.OnStartClient () (at Assets/Mirror/Core/NetworkIdentity.cs:757)
UnityEngine.Debug:LogException(Exception, Object)
Mirror.NetworkIdentity:OnStartClient() (at Assets/Mirror/Core/NetworkIdentity.cs:761)
Mirror.NetworkClient:InvokeIdentityCallbacks(NetworkIdentity) (at Assets/Mirror/Core/NetworkClient.cs:1398)
Mirror.NetworkClient:BootstrapIdentity(NetworkIdentity) (at Assets/Mirror/Core/NetworkClient.cs:1371)

After the PR, all refs are valid.

image

Virtually eliminates null refs with GO/NI/NB SyncVars by getting all objects into spawned dictionary before applying payloads and invoking hooks.
- No change to normal spawning, only initial spawn
- Uses and clears a static dictionary, so no garbage allocation

See ticket #3097
@miwarnec miwarnec changed the title fix(NetworkClient): Defer ApplySpawnPayload until OnObjectSpawnFinished fix(NetworkClient): Defer ApplySpawnPayload until OnObjectSpawnFinished #3097 Dec 30, 2024
@miwarnec miwarnec merged commit fb59969 into master Dec 30, 2024
@miwarnec miwarnec deleted the DeferSpawnPayload branch December 30, 2024 20:41
miwarnec added a commit that referenced this pull request Jan 4, 2025
@MrGadget1024 MrGadget1024 restored the DeferSpawnPayload branch January 12, 2025 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Review enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants