Skip to content

Commit b5f761c

Browse files
jeffreyrainy0xFA11
jeffreyrainy
and
0xFA11
authored
fix: mtt-857 GitHub issue 915 (#1099)
* wip * fix: issues 915, MTT-857, Clients are receiving data from objects not visible to them * fix: issues 915, MTT-857, Clients are receiving data from objects not visible to them. performance (bandwidth). coding style * fix: compile issue after merge * fix: proper detection of hidden/shown state * style: reverting whitespace changes * fix: adjusting metrics to get the rigth client ids * style: applying code review suggestion Co-authored-by: M. Fatih MAR <mfatihmar@gmail.com> * style: coding standards * fix: reducing memory allocations by keeping a List<ulong> as a member Co-authored-by: M. Fatih MAR <mfatihmar@gmail.com>
1 parent ced4138 commit b5f761c

File tree

5 files changed

+214
-6
lines changed

5 files changed

+214
-6
lines changed

‎com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviourUpdater.cs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,13 @@ internal void NetworkBehaviourUpdate(NetworkManager networkManager)
2828
m_Touched.UnionWith(spawnedObjs);
2929
foreach (var sobj in spawnedObjs)
3030
{
31-
// Sync just the variables for just the objects this client sees
32-
for (int k = 0; k < sobj.ChildNetworkBehaviours.Count; k++)
31+
if (sobj.IsNetworkVisibleTo(client.ClientId))
3332
{
34-
sobj.ChildNetworkBehaviours[k].VariableUpdate(client.ClientId);
33+
// Sync just the variables for just the objects this client sees
34+
for (int k = 0; k < sobj.ChildNetworkBehaviours.Count; k++)
35+
{
36+
sobj.ChildNetworkBehaviours[k].VariableUpdate(client.ClientId);
37+
}
3538
}
3639
}
3740
}

‎com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@ internal NetworkSpawnManager(NetworkManager networkManager)
3535
internal readonly Queue<ReleasedNetworkId> ReleasedNetworkObjectIds = new Queue<ReleasedNetworkId>();
3636
private ulong m_NetworkObjectIdCounter;
3737

38+
// A list of target ClientId, use when sending despawn commands. Kept as a member to reduce memory allocations
39+
private List<ulong> m_TargetClientIds = new List<ulong>();
40+
3841
internal ulong GetNetworkObjectId()
3942
{
4043
if (ReleasedNetworkObjectIds.Count > 0 && NetworkManager.NetworkConfig.RecycleNetworkIds && (Time.unscaledTime - ReleasedNetworkObjectIds.Peek().ReleaseTime) >= NetworkManager.NetworkConfig.NetworkIdRecycleDelay)
@@ -625,7 +628,6 @@ internal void OnDespawnObject(NetworkObject networkObject, bool destroyGameObjec
625628
}
626629
}
627630

628-
networkObject.IsSpawned = false;
629631
networkObject.InvokeBehaviourNetworkDespawn();
630632

631633
if (NetworkManager != null && NetworkManager.IsServer)
@@ -653,11 +655,22 @@ internal void OnDespawnObject(NetworkObject networkObject, bool destroyGameObjec
653655
// As long as we have any remaining clients, then notify of the object being destroy.
654656
if (NetworkManager.ConnectedClientsList.Count > 0)
655657
{
658+
m_TargetClientIds.Clear();
659+
660+
// We keep only the client for which the object is visible
661+
// as the other clients have them already despawned
662+
foreach (var clientId in NetworkManager.ConnectedClientsIds)
663+
{
664+
if (networkObject.IsNetworkVisibleTo(clientId))
665+
{
666+
m_TargetClientIds.Add(clientId);
667+
}
668+
}
656669

657670
ulong[] clientIds = NetworkManager.ConnectedClientsIds;
658671
var context = messageQueueContainer.EnterInternalCommandContext(
659672
MessageQueueContainer.MessageType.DestroyObject, NetworkChannel.Internal,
660-
clientIds, NetworkUpdateStage.PostLateUpdate);
673+
m_TargetClientIds.ToArray(), NetworkUpdateStage.PostLateUpdate);
661674
if (context != null)
662675
{
663676
using var nonNullContext = (InternalCommandContext)context;
@@ -667,13 +680,14 @@ internal void OnDespawnObject(NetworkObject networkObject, bool destroyGameObjec
667680
nonNullContext.NetworkWriter.WriteUInt64Packed(networkObject.NetworkObjectId);
668681

669682
var size = bufferSizeCapture.StopMeasureSegment();
670-
NetworkManager.NetworkMetrics.TrackObjectDestroySent(clientIds, networkObject.NetworkObjectId, networkObject.name, size);
683+
NetworkManager.NetworkMetrics.TrackObjectDestroySent(m_TargetClientIds, networkObject.NetworkObjectId, networkObject.name, size);
671684
}
672685
}
673686
}
674687
}
675688
}
676689
}
690+
networkObject.IsSpawned = false;
677691

678692
if (SpawnedObjects.Remove(networkObject.NetworkObjectId))
679693
{

‎com.unity.netcode.gameobjects/Tests/Runtime/HiddenVariableTests.cs

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
using System.Collections;
2+
using UnityEngine;
3+
using UnityEngine.TestTools;
4+
5+
namespace Unity.Netcode.RuntimeTests
6+
{
7+
public class HiddenVariableTest : NetworkBehaviour
8+
{
9+
10+
}
11+
12+
public class HiddenVariableObject : NetworkBehaviour
13+
{
14+
public NetworkVariable<int> MyNetworkVariable = new NetworkVariable<int>();
15+
public NetworkList<int> MyNetworkList = new NetworkList<int>();
16+
17+
public static int ChangeCount = 0;
18+
public static int ListChangeCount = 0;
19+
public static int ExpectedSize = 0;
20+
21+
public override void OnNetworkSpawn()
22+
{
23+
Debug.Log($"{nameof(HiddenVariableObject)}.{nameof(OnNetworkSpawn)}()");
24+
25+
MyNetworkVariable.OnValueChanged += Changed;
26+
MyNetworkList.OnListChanged += ListChanged;
27+
28+
base.OnNetworkSpawn();
29+
}
30+
31+
public void Changed(int before, int after)
32+
{
33+
Debug.Log($"Value changed from {before} to {after} on {NetworkManager.LocalClientId}");
34+
ChangeCount++;
35+
}
36+
public void ListChanged(NetworkListEvent<int> listEvent)
37+
{
38+
Debug.Log($"ListEvent received: type {listEvent.Type}, index {listEvent.Index}, value {listEvent.Value}");
39+
ListChangeCount++;
40+
41+
Debug.Assert(ExpectedSize == MyNetworkList.Count);
42+
}
43+
}
44+
45+
public class HiddenVariableTests : BaseMultiInstanceTest
46+
{
47+
protected override int NbClients => 4;
48+
49+
private NetworkObject m_NetSpawnedObject;
50+
private GameObject m_TestNetworkPrefab;
51+
52+
[UnitySetUp]
53+
public override IEnumerator Setup()
54+
{
55+
yield return StartSomeClientsAndServerWithPlayers(useHost: true, nbClients: NbClients,
56+
updatePlayerPrefab: playerPrefab =>
57+
{
58+
var networkTransform = playerPrefab.AddComponent<HiddenVariableTest>();
59+
m_TestNetworkPrefab = PreparePrefab();
60+
});
61+
}
62+
63+
public GameObject PreparePrefab()
64+
{
65+
var prefabToSpawn = new GameObject("MyTestObject");
66+
var networkObjectPrefab = prefabToSpawn.AddComponent<NetworkObject>();
67+
MultiInstanceHelpers.MakeNetworkObjectTestPrefab(networkObjectPrefab);
68+
prefabToSpawn.AddComponent<HiddenVariableObject>();
69+
70+
m_ServerNetworkManager.NetworkConfig.NetworkPrefabs.Add(new NetworkPrefab() { Prefab = prefabToSpawn });
71+
foreach (var clientNetworkManager in m_ClientNetworkManagers)
72+
{
73+
clientNetworkManager.NetworkConfig.NetworkPrefabs.Add(new NetworkPrefab() { Prefab = prefabToSpawn });
74+
}
75+
return prefabToSpawn;
76+
}
77+
78+
public IEnumerator WaitForConnectedCount(int targetCount)
79+
{
80+
var endTime = Time.realtimeSinceStartup + 1.0;
81+
while (m_ServerNetworkManager.ConnectedClientsList.Count < targetCount && Time.realtimeSinceStartup < endTime)
82+
{
83+
yield return new WaitForSeconds(0.01f);
84+
}
85+
}
86+
87+
public IEnumerator WaitForChangeCount(int targetCount)
88+
{
89+
var endTime = Time.realtimeSinceStartup + 1.0;
90+
while ((HiddenVariableObject.ChangeCount != targetCount ||
91+
HiddenVariableObject.ListChangeCount != targetCount) &&
92+
Time.realtimeSinceStartup < endTime)
93+
{
94+
yield return new WaitForSeconds(0.01f);
95+
}
96+
}
97+
98+
[UnityTest]
99+
public IEnumerator HiddenVariableTest()
100+
{
101+
Debug.Log("Running test");
102+
103+
var spawnedObject = Object.Instantiate(m_TestNetworkPrefab);
104+
m_NetSpawnedObject = spawnedObject.GetComponent<NetworkObject>();
105+
m_NetSpawnedObject.NetworkManagerOwner = m_ServerNetworkManager;
106+
yield return WaitForConnectedCount(NbClients);
107+
108+
// Spawn object with ownership on one client
109+
var client = m_ServerNetworkManager.ConnectedClientsList[1];
110+
var otherClient = m_ServerNetworkManager.ConnectedClientsList[2];
111+
m_NetSpawnedObject.SpawnWithOwnership(client.ClientId);
112+
113+
// Set the NetworkVariable value to 2
114+
HiddenVariableObject.ExpectedSize = 1;
115+
HiddenVariableObject.ChangeCount = 0;
116+
HiddenVariableObject.ListChangeCount = 0;
117+
118+
m_NetSpawnedObject.GetComponent<HiddenVariableObject>().MyNetworkVariable.Value = 2;
119+
m_NetSpawnedObject.GetComponent<HiddenVariableObject>().MyNetworkList.Add(2);
120+
121+
yield return WaitForChangeCount(NbClients + 1);
122+
Debug.Assert(HiddenVariableObject.ChangeCount == NbClients + 1);
123+
Debug.Assert(HiddenVariableObject.ListChangeCount == NbClients + 1);
124+
125+
// Hide our object to a different client
126+
HiddenVariableObject.ExpectedSize = 2;
127+
HiddenVariableObject.ChangeCount = 0;
128+
HiddenVariableObject.ListChangeCount = 0;
129+
m_NetSpawnedObject.NetworkHide(otherClient.ClientId);
130+
131+
// Change the NetworkVariable value
132+
// we should get one less notification of value changing and no errors or exception
133+
m_NetSpawnedObject.GetComponent<HiddenVariableObject>().MyNetworkVariable.Value = 3;
134+
m_NetSpawnedObject.GetComponent<HiddenVariableObject>().MyNetworkList.Add(3);
135+
136+
yield return new WaitForSeconds(1.0f);
137+
Debug.Assert(HiddenVariableObject.ChangeCount == NbClients);
138+
Debug.Assert(HiddenVariableObject.ListChangeCount == NbClients);
139+
140+
// Show our object again to this client
141+
HiddenVariableObject.ExpectedSize = 3;
142+
HiddenVariableObject.ChangeCount = 0;
143+
HiddenVariableObject.ListChangeCount = 0;
144+
m_NetSpawnedObject.NetworkShow(otherClient.ClientId);
145+
146+
// Change the NetworkVariable value
147+
// we should get all notifications of value changing and no errors or exception
148+
m_NetSpawnedObject.GetComponent<HiddenVariableObject>().MyNetworkVariable.Value = 4;
149+
m_NetSpawnedObject.GetComponent<HiddenVariableObject>().MyNetworkList.Add(4);
150+
151+
yield return WaitForChangeCount(NbClients + 1);
152+
Debug.Assert(HiddenVariableObject.ChangeCount == NbClients + 1);
153+
Debug.Assert(HiddenVariableObject.ListChangeCount == NbClients + 1);
154+
155+
// Hide our object to that different client again, and then destroy it
156+
m_NetSpawnedObject.NetworkHide(otherClient.ClientId);
157+
yield return new WaitForSeconds(0.2f);
158+
m_NetSpawnedObject.Despawn();
159+
yield return new WaitForSeconds(0.2f);
160+
}
161+
}
162+
}

‎com.unity.netcode.gameobjects/Tests/Runtime/HiddenVariableTests.cs.meta

Lines changed: 11 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎com.unity.netcode.gameobjects/Tests/Runtime/NetworkShowHideTests.cs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,18 @@ public class NetworkShowHideTest : NetworkBehaviour
1414

1515
public class ShowHideObject : NetworkBehaviour
1616
{
17+
public NetworkVariable<int> MyNetworkVariable;
18+
19+
private void Start()
20+
{
21+
MyNetworkVariable = new NetworkVariable<int>();
22+
MyNetworkVariable.OnValueChanged += Changed;
23+
}
24+
25+
public void Changed(int before, int after)
26+
{
27+
Debug.Log($"Value changed from {before} to {after}");
28+
}
1729

1830
}
1931

@@ -187,6 +199,12 @@ public IEnumerator NetworkShowHideTest()
187199
// hide them on one client
188200
Show(mode == 0, false);
189201

202+
yield return new WaitForSeconds(1.0f);
203+
204+
m_NetSpawnedObject1.GetComponent<ShowHideObject>().MyNetworkVariable.Value = 3;
205+
206+
yield return new WaitForSeconds(1.0f);
207+
190208
// verify they got hidden
191209
yield return CheckVisible(false);
192210

0 commit comments

Comments
 (0)