fix: networkmanager destroy on app quit#1011
Conversation
LukeStampfli
requested review from
0xFA11,
NoelStephensUnity and
mattwalsh-unity
|
I remember this issue popping out in our samples. It's great that these changes address that. |
There was a problem hiding this comment.
Hey @LukeStampfli, I have a weird question... I am not sure whether people are supposed to subclass the NetworkManager, but if they do, and they want to override any of those methods, especially Awake, are they able to with this private access modifier + not marking them virtual?
I guess I am more curious about how this code is intended to be used.
| private void OnSceneUnloaded(Scene scene) | ||
| { | ||
| if (scene == gameObject.scene) | ||
| { | ||
| OnDestroy(); | ||
| } | ||
| } |
There was a problem hiding this comment.
so, we're saying if the scene of NetworkManager's scene is being unloaded (therefore it'll also be unloaded as a part of it), we'd manually call OnDestroy() method. wouldn't it be called anyway as a part of scene unloading process? is this block needed?
There was a problem hiding this comment.
It is needed because we want OnDestroy on the NetworkManager to be called before OnDestroy gets called on the NetworkObjects in the scene. And this seems to be the only way we can achieve that. (ScriptExecutionOrder does not apply to OnDestroy)
There was a problem hiding this comment.
oh, now I get it. well, sounds like a band-aid to me :/ but let's get that band-aid until we have a better solution.
There was a problem hiding this comment.
This won't fix the issues with NetworkObjects being destroyed on the client side (when pooled) or if you are transitioning between single loading mode scenes when the NetworkManager is within the DontDestroyOnLoad scene because that gets unloaded last (after all other scenes are unloaded).
There was a problem hiding this comment.
Yes that's correct, we are not fixing those issues here. This only solves application quit and unloading a non DontDestroyOnLoad NetworkManager from a scene.
|
should we get this in? @LukeStampfli |
0xFA11
changed the title
…kManager and then unloading the scene.
NoelStephensUnity
left a comment
NoelStephensUnity
left a comment
There was a problem hiding this comment.
Looks like it should be just fine.
…nsform * develop: (21 commits) test: adding more details to multiprocess readme (#1050) refactor!: convert NetworkTransform.NetworkState to `struct` (#1061) fix: networkmanager destroy on app quit (#1011) feat: snapshot, using unreliable packets, now that the underlying foundation supports it. Only merge after #1062 (#1064) feat: snapshot. Fully integrated despawn, mtt-1092, mtt-1056 (#1062) fix: eliminate bad use-after-free(destroy) pattern (#1068) chore: cleanup meta files for empty dirs (#1067) chore: minor MLAPI to Netcode rename (#1065) feat: report network behaviour name to the profiler (#1058) fix: player movement (#1063) test: Add unit tests for NetworkTime properties (#1053) chore: remove authority & netvar perms from NetworkTransform (#1059) feat: networktransform pos/rot/sca thresholds on state sync (#1055) feat: expose network behaviour type name internally (#1057) chore: remove all the old profiling code (#1048) fix: if-guard `NetworkManager.__rpc_name_table` access (#1056) fix: Disabling fixedupdate portion of SpawnRpcDespawn test because it's failing for known reasons that will be fixed in the IMessage refactor. (#1049) feat: Implement metrics for the new network profiler (#960) chore!: change package name & asmdefs (#1026) feat: per axis networktransform state sync (+bitwise state comp) (#1042) ... # Conflicts: # com.unity.netcode.gameobjects/Prototyping/NetworkTransform.cs
* fix: Ensure that networkmanager is shutdown before GameObjects are being destroyed Co-authored-by: M. Fatih MAR <mfatihmar@gmail.com>
5 participants
A small fix which helps us to shutdown MLAPI correctly. The problem here was that when we unload a scene or exit playmode 'OnDestroy' gets called in an order which we cannot control. We do want the NetworkManager to shutdown first though before running OnDestroy on NetworkObjects so that we can despawn them correctly.