Search Unity

[SyncVar] Hook Not Called on Client Immediately After Scene Change

Discussion in 'Multiplayer' started by Zullar, Aug 8, 2016.

  1. Zullar

    Zullar

    Joined:
    May 21, 2013
    Posts:
    651
    I bug reported but in case anybody else has this issue...

    I have a [SyncVar] byte on a persistent DontDestroyOnLoad UNET object. When it's value is changed on the Host immediately after a scene change for some reason the hook is not called on the client... even though the value is changed on the client. As you can see from the output log below the SyncVar value on the client is changed from 0 to 2 prior to scene change and the SyncVar hook is called, but after the scene change when I change the value back from 2 to 0 the hook is not called... even though the value changes.

    I haven't pinpointed the exact scenario... but it seems SyncVar hooks are unreliable in the time around scene changes initiated by NetworkManager.ServerChangeScene().

    Code (csharp):
    1.  
    2. mySyncVarByte = 0
    3. mySyncVarByte.Hook called: oldVal=0 newVal=2 //SyncVar hook properly called when value is changed from 0 to 2 prior to scene change
    4. mySyncVarByte = 2
    5. Client.OnLevelWasLoaded
    6. mySyncVarByte.FeatMeleeBasic Color = 2
    7. mySyncVarByte.FeatMeleeBasic Color = 0 //The SyncVar's value is changed on the client but NO SYNCVAR HOOK IS CALLED
    8.  
     
  2. PhilSA

    PhilSA

    Joined:
    Jul 11, 2013
    Posts:
    1,926
    While I agree hooks should fire at all times, this might actually be a "feature" :rolleyes:
    http://docs.unity3d.com/Manual/UNetStateSync.html
    Chances are, when loading a new scene, all serialization is counted as being in "initialState", which means the hook doesn't fire.

    I've encountered a problem similar to this when I was trying to sync the entire state of the game when a client connects mid-game. Knowing SyncVars are supposed to get synchronized automatically when a new client is ready, I figured I could add hooks to syncvars to do all the necessary initialization (for example, have a synced NetworkInstanceId to an object, and my hook would actually go find that object and store the real reference somewhere). But it turns out the hook doesn't get called during player initialization even if the SyncVar value does get properly updated.

    So my solution ended up being that I manually call my hook function for all the necessary SyncVars on OnStartClient(). I've confirmed that all SyncVars are properly updated before OnStartClient gets called. All that's left to do is to find out if OnStartClient gets called when loading a new scene, or if there's some kind of equivalent callback you can use
     
    Last edited: Aug 8, 2016
    Zullar likes this.
  3. Zullar

    Zullar

    Joined:
    May 21, 2013
    Posts:
    651
    I follow what you are saying. There are 2 separate issues from what I understand.
    A: On new objects or for late connecting players the SyncVars are not called even if the value has been changed from it's default value. As you point out this is intended and gives the programmer control over how to handle initialization... and I do the same thing that you do by manually calling the Hook during initialization. My understanding is that when new objects are spawned (or a later player connects) all SyncVar data is sent to the clients... but this data is special and does not call hooks.
    B: Persistent DontDestroyOnLoad objects that have been around for a long time (many seconds) do not always have the Hooks called when values are changed if the values are changed around a SceneChange. This is what this post is about. The object isn't new and any SyncVar changes *should* call the hook from my understanding. So I think this is a bug.
     
  4. PhilSA

    PhilSA

    Joined:
    Jul 11, 2013
    Posts:
    1,926
    ah, I see. Well, just to be sure, I'd try to see if this isn't due to "initialState" first. Maybe the system forces those persistent objects to serialize as initialState after a scene is changed? You could quickly confirm this with this code both in the persistent object's networkBehaviour and the player's networkBehaviour:

    Code (CSharp):
    1.  
    2.     public override bool OnSerialize(NetworkWriter writer, bool initialState)
    3.     {
    4.         if (initialState)
    5.         {
    6.             Debug.Log(this.gameObject.name + " serialized as initialState");
    7.         }
    8.     }
    9.     public override bool OnDeserialize(NetworkWriter writer, bool initialState)
    10.     {
    11.         if (initialState)
    12.         {
    13.             Debug.Log(this.gameObject.name + " deserialized as initialState");
    14.         }
    15.     }
    16.  
    If either one of those outputs a debug log when changing scenes, I'd suspect this would be why the hook doesn't get called. Otherwise... I guess this really would be a bug, or maybe a problem due to the obscure execution order of things (like, what if hooks can only be called after a client is "ready", and changing scenes temporarily sets clients to "not ready", and SyncVars get synced in that time window, etc, etc......)
     
    Zullar likes this.
  5. Zullar

    Zullar

    Joined:
    May 21, 2013
    Posts:
    651
    Those are all good ideas. I'll check it out.
     
  6. Zullar

    Zullar

    Joined:
    May 21, 2013
    Posts:
    651
    OK I can't figure this out. Adding either of these codes to my object causes all sorts of errors like this.
    IndexOutOfRangeException: NetworkReader:ReadByte out of range:NetBuf sz:151 pos:151

    Code (csharp):
    1.  
    2. public override bool OnSerialize(NetworkWriter networkWriter, bool initialState)
    3. {
    4.     return base.OnSerialize(networkWriter, initialState);
    5. }
    6.  
    or...
    Code (csharp):
    1.  
    2. public override void OnDeserialize(NetworkReader networkReader, bool initialState)
    3. {
    4.     base.OnDeserialize(networkReader, initialState);
    5. }
    6.  
    Why would they have any effect? I've commented/uncommented them 3 times and it turns errors on/off. Am I missing something?
     
  7. PhilSA

    PhilSA

    Joined:
    Jul 11, 2013
    Posts:
    1,926
    Oh yeah, that's probably because overriding these functions messes with the code that would normally be generated by SyncVars there. You should try making this test with a really bare-bones networkBehaviour that contains no syncvars
     
    Zullar likes this.
  8. Zullar

    Zullar

    Joined:
    May 21, 2013
    Posts:
    651
    OK I did some testing and you are right. Persistent networked DontDestroyOnLoad objects are OnSerialize'd and sent when scenes are changed... regardless if if any SyncVars are changed/dirty. The scene change OnSerialize() initialState is true for these persistent objects ... even though it is not their initial state (bug).

    It appears UNET is not properly handling persistent networked objects from a previous scene. It is treating them as if they are new scene objects and serializing and sending ALL syncvar data with the initialState=true (and therefore not calling hooks).

    So 99% of the time this is not a problem. Re-sending unnecessary syncVar data on a scene change for persistent networked objects will only use a little extra network bandwidth and typically not harm anything. However, in the rare case a SyncVar is changed/dirty during a scene change then the new SyncVar value is sent... but because initialState=true for this OnSerialize event the SyncVar hook is not called. This is how you end up with SyncVar values that changed without their hooks being called.

    Thanks for helping me understand. I bug reported it. What do you think the best work-around is?

    Can you help me understand why this code generates bugs? It is my understanding that calling the base method should essentially have no effect... but this isn't true. What am I missing?
    Code (csharp):
    1.  
    2. public override bool OnSerialize(NetworkWriter networkWriter, bool initialState)
    3. {
    4.     return base.OnSerialize(networkWriter, initialState);
    5. }
    6.  
     
  9. Oshroth

    Oshroth

    Joined:
    Apr 28, 2014
    Posts:
    99
    UNetWeaver generates OnDe/Serialize functions when you have syncvars in your class, however if you make your own implementation, it won't add the generated ones. Its probably expecting a value that isn't getting serialised causing a buffer overrun
     
    isidro02139 and Zullar like this.
  10. isidro02139

    isidro02139

    Joined:
    Jul 31, 2012
    Posts:
    72
    Oh my lordy I think this might be connected to the horrible bug I have been experiencing over the last 48 hours... (I will recap it below, hopefully either @Oshroth @PhilSA or @Zullar could confirm my thinking is correct!)

    I am trying to assign a color to the NetworkPlayer via a LobbyHook. I want to be able to have this color available by the time OnStartClient() or Start() is called on each client, and indeed on the surface things seem to be working but what I noticed when I repeatedly tested starting a battle was that the Hook function I have assigned to my NetworkPlayer’s color SyncVar is not called ~15% of the time! :eek:

    I am at work now so I can't test, but I will confirm once I go home that @PhilSA's solution (manually calling all hook functions in OnStartClient() and passing in the SyncVars) eliminates the bug.

    Thank you guys! Also, if you didn't know about it and are interested, I have recieved a lot of great unet support from the Slack community at unitydevs.slack.com (#network) – please do join if you are interested :D

    Best wishes to all,
    Arun
     
  11. Zullar

    Zullar

    Joined:
    May 21, 2013
    Posts:
    651
    I'm a little curious why the hook intermittently does not work. In any case @PhilSA 's solution should work (calling hooks during OnStartClient)

    I should add that SyncVar hooks are *intentionally* not called during OnStartClient by design to allow the user to differentiate between initial setting of SyncVars and runtime changing of SyncVars.
     
  12. YondernautsGames

    YondernautsGames

    Joined:
    Nov 24, 2014
    Posts:
    353
    Hey all

    Was there ever anything more uncovered about this or a bug report submitted? I've just run into this problem now.

    I have persistent player objects, and I'm finding that some of the variables don't sync after a scene change. For example, I have a "GameMode" object in the scenes that sets the player syncvar "player.isInGame" to true in OnStartServer. The player syncvar has a hook where I log the value it changes to. I also call the hook in the player OnStartClient for initial state. My game uses tournament style matches and I reload the current scene between every round to reset the level.

    If I build and run, and use that instance to join a server hosted from the editor then everything works fine. The hook is called on both players (client and client/host). However, if I host the server on the instance and join from the editor then whenever I change scenes, that hook is never called on the client (but is for the player on the host). It's really weird and I haven't found a reason at all. The problem in the first post here sounds like exactly what I'm experiencing.

    Btw, I'm using 5.6.0f3 currently and hesitant to upgrade as the game is for an asset store template and I wanted to aim for 5.6 as my minimum supported version. I will if I have to though.

    Cheers
     
  13. Zullar

    Zullar

    Joined:
    May 21, 2013
    Posts:
    651
    tldr;
    Yes it's been bug reported. Yes it's been replicated. No there is no fix.

    The root cause of the issue is that NetworkIdentity.observers are removed and re-added during scene transition. This causes all RPC's and and SyncVars and such to not work briefly during scene transition. When the new scene is loaded the objects are basically "re-spawned" setting SyncVars to the latest value, but hooks are not called because OnDeserialize initialState == true. Unity QA did replicate this bug, but there is no fix in the works because I was told it requires a major change to UNET.

    If you want to bandaid after each OnDeserialize you can check to see if the SyncVar has changed. This works, but it requires duplication of code because you must have a separate field/property corresponding to each SyncVar.

    At this point I'm completely moving away from UNET due to the bugs and lack of support with the HLAPI. I think I've bug reported ~20 UNET HLAPI bugs, QA is good at replicating and they have reproduced most of the bugs I've submitted. However, zero of these bugs have been fixed in the last ~2 years. The rest of Unity is great, it's just UNET that I haven't been able to use.
     
  14. YondernautsGames

    YondernautsGames

    Joined:
    Nov 24, 2014
    Posts:
    353
    Aye, I have to admit that I'm very nervous about releasing an asset that relies on Unet as even if I get it working for my needs, I'm them forcing users to work with it as well. Fine if it does the job, but if they have to jump through all these hoops too then it will just end up making me look bad. Still. It'll take me months to make the switch to an alternative, and I don't see Photon as hugely viable since that requires users to pay for another asset to use mine. Rock and a hard place o_0

    Anyhow, you mention using a check after OnDeserialize? How would I go about that? My understanding was that implementing anything in OnDeserialize was a no-go if you use SyncVars as UNetWeaver wants to do its own thing.

    If I can't get around this specific issue, I can always make my player objects non-persistent and use static variables server side to carry relevant info over between levels. That would fit my needs currently, but does limit me with future plans.

    Thanks for getting back to me
     
  15. Zullar

    Zullar

    Joined:
    May 21, 2013
    Posts:
    651
    Oh you are right. Custom OnSerialize/Deserialize + SyncVars are mutually exclusive due to UNET weaver. So your UNET options might be
    1: Check every Update to see if value has changed (if you aren't concerned about overhead or a slight delay).
    2: Create your own CustomSyncVars. Then implement your own OnSerialize/OnDeserialize. This wont' fix the scene change issue where NetworkIdentity.observers are dropped, re-added, and the object is re-spawned. But it *will* fix the bug where hooks aren't called if the value is changed upon re-spawn.
    3: Look into HLAPI Pro. I haven't used it but I see it gets a lot of positive feedback. Some guy is fixing up the UNET HLAPI.
    4: Use MessageBase or LLAPI to send data (instead of HLAPI script serialization).

    #1 would be extremely easy, although very bad practice (duplication of fields, added overhead)
    I've done a combination of #2 and #4 to get around the UNET scene change hook failure bug and it works well. But this is basically making your own HLAPI and took me a long time.

    My big issue (that I don't know how to get around) is with UNET scene objects not spawning properly. Simply attaching a NetworkIdentity to an objects screws up all sorts of stuff like Animator initialization. It's random (I have 100 identical prefabs and when I load the scene 20 work OK and 80 do not... even though they are all the same). Not sure how to work around this.
     
  16. YondernautsGames

    YondernautsGames

    Joined:
    Nov 24, 2014
    Posts:
    353
    Thanks. Yeah, I think if it was purely for me then I would go with option #2 or even look at doing more with the LLAPI. I've done a bit with messages to completely phase out the use of SyncLists (they just seemed to break everything around them), but I've not really had a chance to dig into the LLAPI.

    Anyhow, I've actually decided to drop multiplayer from the initial release of my asset. I can't imagine releasing by the end of the year if I stick with it, plus I'd just be forcing any users who want to extend the multiplayer to go through the same learning curve and workarounds I (or we) have. After release I'll be looking at building up from LLAPI or alternative libraries such as forge remastered. Ideally I want something I can include in the asset so people aren't forced to buy other assets and I done have to constantly be reacting to other asset updates.

    HLAPI Pro does look interesting though.

    BTW, I've not hit your issue with object spawning, but mainly since I hadn't reached that point in my implementation yet. Good look sorting it. The only thing I can think of that sounds similar was I had an intermittent (very hard to repro) issue with virtual syncvar hooks not behaving properly (almost like they were non-virtual). That would affect random objects or every 10th time i ran the game, etc. I might have been doing something stupid, but I switched to enforcing non-virtual hooks that call the virtual "OnSyncX" methods and I've not had a problem since.
     
  17. kylebrain

    kylebrain

    Joined:
    Jun 15, 2016
    Posts:
    2
    I have the same issue. It's really pisses me off because the official Unity lobby asset uses DontDestroyOnLoad for all lobby player and networked objects and I've been following the structure of that example. Everything works great, but if you want to go back to the lobby scene from the game scene, sorry, everything is broken. I've tried to create my own "hooks" by updating values through Rpc's and Commands but sometimes those break. And when I move everything to destroy on load, my game player prefab doesn't spawn, but after reading this thread it looks like that's what I'll have to do.