fix: if-guard NetworkManager.__rpc_name_table access
#1056
Conversation
| // RuntimeAccessModifiersILPP will make this `public` | ||
| internal static readonly Dictionary<uint, string> __rpc_name_table = new Dictionary<uint, string>(); | ||
| #else // !(UNITY_EDITOR || DEVELOPMENT_BUILD) | ||
| // RuntimeAccessModifiersILPP will make this `public` | ||
| internal static readonly Dictionary<uint, string> __rpc_name_table = null; // not needed on release builds |
There was a problem hiding this comment.
we kept this field for the old ILPP implementation (pre-2020.3 ILPP, targeting 2019.4) because we needed it for nameof(NetworkManager.__rpc_name_table) code.
however, ILPP code is refactored (cleaned) on 2020.3+ and now we don't actually require this field to be there in the middle of ILPP.
so, let's get rid of it and wrap lines using __rpc_name_table with #if DEVELOPMENT_BUILD || UNITY_EDITOR everywhere!
0xFA11
changed the title
NetworkManager.__rpc_name_tableNetworkManager.__rpc_name_table access
| @@ -99,10 +99,12 @@ internal void __endSendServerRpc(NetworkSerializer serializer, uint rpcMethodId, | |||
| ? NetworkManager.MessageQueueContainer.EndAddQueueItemToFrame(serializer.Writer, MessageQueueHistoryFrame.QueueFrameType.Inbound, serverRpcParams.Send.UpdateStage) | |||
| : NetworkManager.MessageQueueContainer.EndAddQueueItemToFrame(serializer.Writer, MessageQueueHistoryFrame.QueueFrameType.Outbound, NetworkUpdateStage.PostLateUpdate); | |||
|
|
|||
| #if DEVELOPMENT_BUILD || UNITY_EDITOR | |||
There was a problem hiding this comment.
This definitely fixes it for me.
I'm assuming it's the NetworkManager.NetworkMetrics is null?
What I don't know is if the tools team indeed intended this to be dev / editor only. I dunno, maybe they would want to be able to track RPCs in a production app? @becksebenius-unity ?
There was a problem hiding this comment.
We compromised on this one because there was a fairly long-winded discussion about whether the additional memory footprint of rpc names should exist in release builds - and since the Profiler (the only tool being released with this version) is only enabled in Development builds, we decided to close the conversation and go this route. It does seem fairly unlikely that we would need the names of RPCs reported in any tool other than the profiler, but that remains to be seen.
There was a problem hiding this comment.
we might bring them back once we justify their use-cases and even then, we might follow a different approach to reduce memory footprint (thinking aggressive inlining here).
|
What is this PR trying to achieve? |
Fatih answered my question, didn't realize there was an actual bug here. I'm good with this cahnge |
…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
fixes MTT-1058