Skip to content

fix: MLAPI spawned objects now sets NetworkManager override #767

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
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.multiplayer.mlapi/Editor/NetworkObjectEditor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ public override void OnInspectorGUI()
EditorGUILayout.TextField(nameof(NetworkObject.IsSceneObject), "null");
}
EditorGUILayout.Toggle(nameof(NetworkObject.DestroyWithScene), m_NetworkObject.DestroyWithScene);
EditorGUILayout.TextField(nameof(NetworkObject.NetworkManager), m_NetworkObject.NetworkManager.gameObject.name);
GUI.enabled = guiEnabled;

if (NetworkManager.Singleton != null && NetworkManager.Singleton.IsServer)
Expand Down
40 changes: 20 additions & 20 deletions com.unity.multiplayer.mlapi/Runtime/Core/NetworkBehaviour.cs
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,14 @@ public NetworkSerializer __beginSendServerRpc(ServerRpcParams serverRpcParams, R
{
PooledNetworkWriter writer;

var rpcQueueContainer = NetworkObject.NetworkManager.RpcQueueContainer;
var rpcQueueContainer = NetworkManager.RpcQueueContainer;
var isUsingBatching = rpcQueueContainer.IsUsingBatching();
var transportChannel = rpcDelivery == RpcDelivery.Reliable ? NetworkChannel.ReliableRpc : NetworkChannel.UnreliableRpc;

if (IsHost)
{
writer = rpcQueueContainer.BeginAddQueueItemToFrame(RpcQueueContainer.QueueItemType.ServerRpc, Time.realtimeSinceStartup, transportChannel,
NetworkObject.NetworkManager.ServerClientId, null, RpcQueueHistoryFrame.QueueFrameType.Inbound, serverRpcParams.Send.UpdateStage);
NetworkManager.ServerClientId, null, RpcQueueHistoryFrame.QueueFrameType.Inbound, serverRpcParams.Send.UpdateStage);

if (!isUsingBatching)
{
Expand All @@ -89,7 +89,7 @@ public NetworkSerializer __beginSendServerRpc(ServerRpcParams serverRpcParams, R
else
{
writer = rpcQueueContainer.BeginAddQueueItemToFrame(RpcQueueContainer.QueueItemType.ServerRpc, Time.realtimeSinceStartup, transportChannel,
NetworkObject.NetworkManager.ServerClientId, null, RpcQueueHistoryFrame.QueueFrameType.Outbound, NetworkUpdateStage.PostLateUpdate);
NetworkManager.ServerClientId, null, RpcQueueHistoryFrame.QueueFrameType.Outbound, NetworkUpdateStage.PostLateUpdate);
if (!isUsingBatching)
{
writer.WriteByte(NetworkConstants.SERVER_RPC); // MessageType
Expand Down Expand Up @@ -120,7 +120,7 @@ public void __endSendServerRpc(NetworkSerializer serializer, ServerRpcParams ser
return;
}

var rpcQueueContainer = NetworkObject.NetworkManager.RpcQueueContainer;
var rpcQueueContainer = NetworkManager.RpcQueueContainer;
if (IsHost)
{
rpcQueueContainer.EndAddQueueItemToFrame(serializer.Writer, RpcQueueHistoryFrame.QueueFrameType.Inbound, serverRpcParams.Send.UpdateStage);
Expand All @@ -146,26 +146,26 @@ public NetworkSerializer __beginSendClientRpc(ClientRpcParams clientRpcParams, R
PooledNetworkWriter writer;

// This will start a new queue item entry and will then return the writer to the current frame's stream
var rpcQueueContainer = NetworkObject.NetworkManager.RpcQueueContainer;
var rpcQueueContainer = NetworkManager.RpcQueueContainer;
var isUsingBatching = rpcQueueContainer.IsUsingBatching();
var transportChannel = rpcDelivery == RpcDelivery.Reliable ? NetworkChannel.ReliableRpc : NetworkChannel.UnreliableRpc;

ulong[] clientIds = clientRpcParams.Send.TargetClientIds ?? NetworkObject.NetworkManager.ConnectedClientsList.Select(c => c.ClientId).ToArray();
ulong[] clientIds = clientRpcParams.Send.TargetClientIds ?? NetworkManager.ConnectedClientsList.Select(c => c.ClientId).ToArray();
if (clientRpcParams.Send.TargetClientIds != null && clientRpcParams.Send.TargetClientIds.Length == 0)
{
clientIds = NetworkObject.NetworkManager.ConnectedClientsList.Select(c => c.ClientId).ToArray();
clientIds = NetworkManager.ConnectedClientsList.Select(c => c.ClientId).ToArray();
}

//NOTES ON BELOW CHANGES:
//The following checks for IsHost and whether the host client id is part of the clients to recieve the RPC
//Is part of a patch-fix to handle looping back RPCs into the next frame's inbound queue.
//!!! This code is temporary and will change (soon) when NetworkSerializer can be configured for mutliple NetworkWriters!!!
var containsServerClientId = clientIds.Contains(NetworkObject.NetworkManager.ServerClientId);
var containsServerClientId = clientIds.Contains(NetworkManager.ServerClientId);
if (IsHost && containsServerClientId)
{
//Always write to the next frame's inbound queue
writer = rpcQueueContainer.BeginAddQueueItemToFrame(RpcQueueContainer.QueueItemType.ClientRpc, Time.realtimeSinceStartup, transportChannel,
NetworkObject.NetworkManager.ServerClientId, null, RpcQueueHistoryFrame.QueueFrameType.Inbound, clientRpcParams.Send.UpdateStage);
NetworkManager.ServerClientId, null, RpcQueueHistoryFrame.QueueFrameType.Inbound, clientRpcParams.Send.UpdateStage);

//Handle sending to the other clients, if so the above notes explain why this code is here (a temporary patch-fix)
if (clientIds.Length > 1)
Expand Down Expand Up @@ -225,17 +225,17 @@ public void __endSendClientRpc(NetworkSerializer serializer, ClientRpcParams cli
return;
}

var rpcQueueContainer = NetworkObject.NetworkManager.RpcQueueContainer;
var rpcQueueContainer = NetworkManager.RpcQueueContainer;

if (IsHost)
{
ulong[] clientIds = clientRpcParams.Send.TargetClientIds ?? NetworkObject.NetworkManager.ConnectedClientsList.Select(c => c.ClientId).ToArray();
ulong[] clientIds = clientRpcParams.Send.TargetClientIds ?? NetworkManager.ConnectedClientsList.Select(c => c.ClientId).ToArray();
if (clientRpcParams.Send.TargetClientIds != null && clientRpcParams.Send.TargetClientIds.Length == 0)
{
clientIds = NetworkObject.NetworkManager.ConnectedClientsList.Select(c => c.ClientId).ToArray();
clientIds = NetworkManager.ConnectedClientsList.Select(c => c.ClientId).ToArray();
}

var containsServerClientId = clientIds.Contains(NetworkObject.NetworkManager.ServerClientId);
var containsServerClientId = clientIds.Contains(NetworkManager.ServerClientId);
if (containsServerClientId && clientIds.Length == 1)
{
rpcQueueContainer.EndAddQueueItemToFrame(serializer.Writer, RpcQueueHistoryFrame.QueueFrameType.Inbound, clientRpcParams.Send.UpdateStage);
Expand Down Expand Up @@ -264,19 +264,19 @@ public void __endSendClientRpc(NetworkSerializer serializer, ClientRpcParams cli
/// <summary>
/// Gets if we are executing as server
/// </summary>
protected bool IsServer => IsRunning && NetworkObject.NetworkManager.IsServer;
protected bool IsServer => IsRunning && NetworkManager.IsServer;

/// <summary>
/// Gets if we are executing as client
/// </summary>
protected bool IsClient => IsRunning && NetworkObject.NetworkManager.IsClient;
protected bool IsClient => IsRunning && NetworkManager.IsClient;

/// <summary>
/// Gets if we are executing as Host, I.E Server and Client
/// </summary>
protected bool IsHost => IsRunning && NetworkObject.NetworkManager.IsHost;
protected bool IsHost => IsRunning && NetworkManager.IsHost;

private bool IsRunning => NetworkObject.NetworkManager != null && NetworkObject.NetworkManager.IsListening;
private bool IsRunning => NetworkManager != null && NetworkManager.IsListening;

/// <summary>
/// Gets Whether or not the object has a owner
Expand Down Expand Up @@ -615,7 +615,7 @@ private void NetworkVariableUpdate(ulong clientId)
if (!m_ChannelMappedNetworkVariableIndexes[j].Contains(k))
{
// This var does not belong to the currently iterating channel group.
if (NetworkObject.NetworkManager.NetworkConfig.EnsureNetworkVariableLengthSafety)
if (NetworkManager.NetworkConfig.EnsureNetworkVariableLengthSafety)
{
writer.WriteUInt16Packed(0);
}
Expand All @@ -633,7 +633,7 @@ private void NetworkVariableUpdate(ulong clientId)
// if I'm dirty AND the server AND the client can read me, send.
bool shouldWrite = isDirty && (!IsServer || NetworkVariableFields[k].CanClientRead(clientId));

if (NetworkObject.NetworkManager.NetworkConfig.EnsureNetworkVariableLengthSafety)
if (NetworkManager.NetworkConfig.EnsureNetworkVariableLengthSafety)
{
if (!shouldWrite)
{
Expand All @@ -653,7 +653,7 @@ private void NetworkVariableUpdate(ulong clientId)
// this will allow lag-compensation
writer.WriteUInt16Packed(NetworkVariableFields[k].RemoteTick);

if (NetworkObject.NetworkManager.NetworkConfig.EnsureNetworkVariableLengthSafety)
if (NetworkManager.NetworkConfig.EnsureNetworkVariableLengthSafety)
{
using (var varBuffer = PooledNetworkBuffer.Get())
{
Expand Down
4 changes: 2 additions & 2 deletions com.unity.multiplayer.mlapi/Runtime/Core/NetworkManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -321,11 +321,11 @@ private void Init(bool server)
SpawnManager = new NetworkSpawnManager(this);

CustomMessagingManager = new CustomMessagingManager(this);

BufferManager = new BufferManager(this);

SceneManager = new NetworkSceneManager(this);

BufferManager = new BufferManager();

if (MessageHandler == null)
{
// Only create this if it's not already set (like in test cases)
Expand Down
14 changes: 10 additions & 4 deletions com.unity.multiplayer.mlapi/Runtime/Core/NetworkObject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,15 @@ private void OnValidate()
/// <summary>
/// Gets the NetworkManager that owns this NetworkObject instance
/// </summary>
public NetworkManager NetworkManager => NetworkManagerTestOverride != null ? NetworkManagerTestOverride : NetworkManager.Singleton;
public NetworkManager NetworkManager => NetworkManagerOwner != null ? NetworkManagerOwner : NetworkManager.Singleton;

internal NetworkManager NetworkManagerTestOverride;
/// <summary>
/// The NetworkManager that owns this NetworkObject.
/// This property controls where this NetworkObject belongs.
/// This property is null by default currently, which means that the above NetworkManager getter will return the Singleton.
/// In the future this is the path where alternative NetworkManagers should be injected for running multi NetworkManagers
/// </summary>
internal NetworkManager NetworkManagerOwner;

/// <summary>
/// Gets the unique Id of this object that is synced across the network
Expand Down Expand Up @@ -218,7 +224,7 @@ public static void NetworkShow(List<NetworkObject> networkObjects, ulong clientI
{
if (networkObjects == null || networkObjects.Count == 0)
{
throw new ArgumentNullException("At least one NetworkObject has to be provided");
throw new ArgumentNullException("At least one " + nameof(NetworkObject) + " has to be provided");
}

NetworkManager networkManager = networkObjects[0].NetworkManager;
Expand Down Expand Up @@ -313,7 +319,7 @@ public static void NetworkHide(List<NetworkObject> networkObjects, ulong clientI
{
if (networkObjects == null || networkObjects.Count == 0)
{
throw new ArgumentNullException("At least one NetworkObject has to be provided");
throw new ArgumentNullException("At least one " + nameof(NetworkObject) + " has to be provided");
}

NetworkManager networkManager = networkObjects[0].NetworkManager;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,13 @@ internal struct BufferedMessage
internal float BufferTime;
}

private NetworkManager m_NetworkManager { get; }

internal BufferManager(NetworkManager networkManager)
{
m_NetworkManager = networkManager;
}

internal Queue<BufferedMessage> ConsumeBuffersForNetworkId(ulong networkId)
{
if (m_BufferQueues.ContainsKey(networkId))
Expand Down Expand Up @@ -77,7 +84,7 @@ internal void CleanBuffer()
#endif
foreach (var pair in m_BufferQueues)
{
while (pair.Value.Count > 0 && Time.realtimeSinceStartup - pair.Value.Peek().BufferTime >= NetworkManager.Singleton.NetworkConfig.MessageBufferTimeout)
while (pair.Value.Count > 0 && Time.realtimeSinceStartup - pair.Value.Peek().BufferTime >= m_NetworkManager.NetworkConfig.MessageBufferTimeout)
{
BufferedMessage message = pair.Value.Dequeue();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,8 @@ internal NetworkObject CreateLocalNetworkObject(bool softCreate, uint prefabHash
// Otherwise, instantiate an instance of the NetworkPrefab linked to the prefabHash
var networkObject = ((position == null && rotation == null) ? UnityEngine.Object.Instantiate(networkPrefabReference) : UnityEngine.Object.Instantiate(networkPrefabReference, position.GetValueOrDefault(Vector3.zero), rotation.GetValueOrDefault(Quaternion.identity))).GetComponent<NetworkObject>();

networkObject.NetworkManagerTestOverride = NetworkManager;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably worth a comment saying something like "by default all network objects are spawned with this singleton, central NM which can be overridden for testing"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as per my comments in #762, I'm still not 100% onboard with this approach yet.


if (parentNetworkObject != null)
{
networkObject.transform.SetParent(parentNetworkObject.transform, true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public void NetworkManagerOverrideTest()
var networkObject = gameObject.AddComponent<NetworkObject>();

// Set override
networkObject.NetworkManagerTestOverride = networkManager;
networkObject.NetworkManagerOwner = networkManager;

Debug.Assert(networkObject.NetworkManager == networkManager);

Expand Down