SyncVar hooks and Sync[Collection] handlers firing during initial spawn

[MrGadget1024]

Currrently for initial spawn payload this happens:

1. OnObjectSpawnStarted
2. SyncVar hooks fire as objects are spawned
3. SyncList / SyncDictionary / etc. callbacks are fired as objects are spawned
4. OnObjectSpawnFinished

Would be better to defer the calling of hooks and callbacks until OnObjectSpawnFinished instead of firing them randomly during the spawn phase.

Additionally, in OnObjectSpawnFinished, we have this, but I think CheckForLocalPlayer should be first for any given object, not last.

foreach (NetworkIdentity identity in spawned.Values.OrderBy(uv => uv.netId))
{
    identity.NotifyAuthority();
    identity.OnStartClient();
    CheckForLocalPlayer(identity);
}

And finally, it would be ideal if we waited until end-of-frame before running OnObjectSpawnFinished so that Awake on everything we just spawned has had a chance to fire, where callback handlers can get wired up before we invoke them.

public readonly SyncList<SomeStruct> structData = new SyncList<SomeStruct>();

void Awake()
{
    // this needs to fire before OnObjectSpawnFinished runs
    structData.Callback += OnDataUpdated;
}

One more thing...

Now that we have Client To Server, SyncVars don't fire the hook on the owner. It may be that the owner gets the update, but since it already matches the value owner has, the hook would be suppressed as "not changed". I think if Client To Server is set and hasAuthority is true, we should fire the hook on the owner. Otherwise we need to document that it won't fire so users know what to expect.

[miwarnec]

there are a few options later.
mostly after we moved OnDeserialize to C#.

• we could have a list of Actions for the hook. invoke them all after spawn. only issue here is the parameters. casting to object would allocate
• we could have list of actions and byte[] writers. then deserialize for the hook.
• OR: call all OnDeserialize only after all were spawned. but then they wouldn't have the correct data by the time the hook gets called. i.e. A.hook() might try to access B, which didn't have deserialize called yet

[David548219]

This issue introduces all kinds of undesirable side effects. For example, when hooks are fired for initial spawn isClient & isServer variables are not set and will always be false, which is not very intuitive.

Current workaround I use to fix execution order and all side effects (perhaps it will help while this is not fixed):

[SyncVar(hook = nameof(TestHook))] private int hookedSyncVar = 0;

private bool muteHooks = true;

private void TestHook(int oldVal, int newVal) {
    if (muteHooks) return;
    
    // Hook logic goes here
}

public override void OnStartClient() {
    base.OnStartClient();
    
    muteHooks = false;
    if (hookedSyncVar != 0)
        TestHook(0, hookedSyncVar);
}

All of the observations and suggested workaround are from version 66.0.9.

[miwarnec]

#3961