Skip to content

fix: Not sending ChangeOwnership messages to clients that have an object hidden #2251

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Oct 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions com.unity.netcode.gameobjects/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ Additional documentation and release notes are available at [Multiplayer Documen

### Fixed

- Fixed issue caused when changing ownership of objects hidden to some clients (#2242)
- Fixed issue where an in-scene placed NetworkObject would not invoke NetworkBehaviour.OnNetworkSpawn if the GameObject was disabled when it was despawned. (#2239)
- Fixed issue where clients were not rebuilding the `NetworkConfig` hash value for each unique connection request. (#2226)
- Fixed the issue where player objects were not taking the `DontDestroyWithOwner` property into consideration when a client disconnected. (#2225)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -278,11 +278,14 @@ internal void ChangeOwnership(NetworkObject networkObject, ulong clientId)
NetworkObjectId = networkObject.NetworkObjectId,
OwnerClientId = networkObject.OwnerClientId
};
var size = NetworkManager.SendMessage(ref message, NetworkDelivery.ReliableSequenced, NetworkManager.ConnectedClientsIds);

foreach (var client in NetworkManager.ConnectedClients)
{
NetworkManager.NetworkMetrics.TrackOwnershipChangeSent(client.Key, networkObject, size);
if (networkObject.IsNetworkVisibleTo(client.Value.ClientId))
{
var size = NetworkManager.SendMessage(ref message, NetworkDelivery.ReliableSequenced, client.Value.ClientId);
NetworkManager.NetworkMetrics.TrackOwnershipChangeSent(client.Key, networkObject, size);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,13 @@ public IEnumerator TrackOwnershipChangeSentMetric()
var ownershipChangeSent = metricValues.First();
Assert.AreEqual(networkObject.NetworkObjectId, ownershipChangeSent.NetworkId.NetworkId);
Assert.AreEqual(Server.LocalClientId, ownershipChangeSent.Connection.Id);
Assert.AreEqual(0, ownershipChangeSent.BytesCount);

// The first metric is to the server(self), so its size is now correctly reported as 0.
// Let's check the last one instead, to have a valid value
ownershipChangeSent = metricValues.Last();
Assert.AreEqual(networkObject.NetworkObjectId, ownershipChangeSent.NetworkId.NetworkId);
Assert.AreEqual(Client.LocalClientId, ownershipChangeSent.Connection.Id);
Assert.AreEqual(FastBufferWriter.GetWriteSize<ChangeOwnershipMessage>() + k_MessageHeaderSize, ownershipChangeSent.BytesCount);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -306,5 +306,37 @@ public IEnumerator NetworkHideDespawnTest()

LogAssert.NoUnexpectedReceived();
}

[UnityTest]
public IEnumerator NetworkHideChangeOwnership()
{
ShowHideObject.ClientTargetedNetworkObjects.Clear();
ShowHideObject.ClientIdToTarget = m_ClientNetworkManagers[1].LocalClientId;
ShowHideObject.Silent = true;

var spawnedObject1 = SpawnObject(m_PrefabToSpawn, m_ServerNetworkManager);
m_NetSpawnedObject1 = spawnedObject1.GetComponent<NetworkObject>();

m_NetSpawnedObject1.GetComponent<ShowHideObject>().MyNetworkVariable.Value++;
// Hide an object to a client
m_NetSpawnedObject1.NetworkHide(m_ClientNetworkManagers[1].LocalClientId);

yield return WaitForConditionOrTimeOut(() => ShowHideObject.ClientTargetedNetworkObjects.Count == 0);

// Change ownership while the object is hidden to some
m_NetSpawnedObject1.ChangeOwnership(m_ClientNetworkManagers[0].LocalClientId);

// The two-second wait is actually needed as there's a potential warning of unhandled message after 1 second
yield return new WaitForSeconds(1.25f);

LogAssert.NoUnexpectedReceived();

// Show the object again to check nothing unexpected happens
m_NetSpawnedObject1.NetworkShow(m_ClientNetworkManagers[1].LocalClientId);

yield return WaitForConditionOrTimeOut(() => ShowHideObject.ClientTargetedNetworkObjects.Count == 1);

Assert.True(ShowHideObject.ClientTargetedNetworkObjects[0].OwnerClientId == m_ClientNetworkManagers[0].LocalClientId);
}
}
}