Skip to content

refactor!: remove network variable settings, network behaviour cleanup #1097

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.

Sign up for GitHub

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 10 commits into from
Aug 26, 2021
Merged

refactor!: remove network variable settings, network behaviour cleanup #1097

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
4e3e196
refactor!: remove network variable settings, network behaviour cleanup
mattwalsh-unity Aug 25, 2021
abfcfc0
remove ensure initialized
mattwalsh-unity Aug 26, 2021
a049539
moved init, removed unused var
mattwalsh-unity Aug 26, 2021
c1ed83b
Merge branch 'develop' into feature/remove_netvar_settings
mattwalsh-unity Aug 26, 2021
705ec49
refactor!: remove network variable settings, network behaviour cleanup
mattwalsh-unity Aug 25, 2021
764ae72
remove ensure initialized
mattwalsh-unity Aug 26, 2021
0e84dd3
moved init, removed unused var
mattwalsh-unity Aug 26, 2021
08f38f6
whitespace / include fixes
mattwalsh-unity Aug 26, 2021
a14a89b
merge fix
mattwalsh-unity Aug 26, 2021
f0eb64c
remove unnecessary requires
mattwalsh-unity Aug 26, 2021
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
68 changes: 35 additions & 33 deletions com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ internal void InitializeVariables()
sortedFields[i].SetValue(this, instance);
}

instance.NetworkBehaviour = this;
instance.Initialize(this);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't want NetworkBehaviour to be exposed, want tighter control over this


var instanceNameProperty = fieldType.GetProperty(nameof(NetworkVariableBase.Name));
var sanitizedVariableName = sortedFields[i].Name.Replace("<", string.Empty).Replace(">k__BackingField", string.Empty);
Expand Down Expand Up @@ -624,15 +624,15 @@ private bool CouldHaveDirtyNetworkVariables()
return false;
}

internal static void HandleNetworkVariableDeltas(List<NetworkVariableBase> networkVariableList, Stream stream, ulong clientId, NetworkBehaviour logInstance, NetworkManager networkManager)
internal void HandleNetworkVariableDeltas(Stream stream, ulong clientId)
{
using (var reader = PooledNetworkReader.Get(stream))
{
for (int i = 0; i < networkVariableList.Count; i++)
for (int i = 0; i < NetworkVariableFields.Count; i++)
{
ushort varSize = 0;

if (networkManager.NetworkConfig.EnsureNetworkVariableLengthSafety)
if (NetworkManager.NetworkConfig.EnsureNetworkVariableLengthSafety)
{
varSize = reader.ReadUInt16Packed();

Expand All @@ -649,15 +649,15 @@ internal static void HandleNetworkVariableDeltas(List<NetworkVariableBase> netwo
}
}

if (networkManager.IsServer && !networkVariableList[i].CanClientWrite(clientId))
if (NetworkManager.IsServer && !NetworkVariableFields[i].CanClientWrite(clientId))
{
// we are choosing not to fire an exception here, because otherwise a malicious client could use this to crash the server
if (networkManager.NetworkConfig.EnsureNetworkVariableLengthSafety)
if (NetworkManager.NetworkConfig.EnsureNetworkVariableLengthSafety)
{
if (NetworkLog.CurrentLogLevel <= LogLevel.Normal)
{
NetworkLog.LogWarning($"Client wrote to {typeof(NetworkVariable<>).Name} without permission. => {(logInstance != null ? ($"{nameof(NetworkObjectId)}: {logInstance.NetworkObjectId} - {nameof(NetworkObject.GetNetworkBehaviourOrderIndex)}(): {logInstance.NetworkObject.GetNetworkBehaviourOrderIndex(logInstance)} - VariableIndex: {i}") : string.Empty)}");
NetworkLog.LogError($"[{networkVariableList[i].GetType().Name}]");
NetworkLog.LogWarning($"Client wrote to {typeof(NetworkVariable<>).Name} without permission. => {nameof(NetworkObjectId)}: {NetworkObjectId} - {nameof(NetworkObject.GetNetworkBehaviourOrderIndex)}(): {NetworkObject.GetNetworkBehaviourOrderIndex(this)} - VariableIndex: {i}");
NetworkLog.LogError($"[{NetworkVariableFields[i].GetType().Name}]");
}

stream.Position += varSize;
Expand All @@ -673,32 +673,33 @@ internal static void HandleNetworkVariableDeltas(List<NetworkVariableBase> netwo
// - TwoTen
if (NetworkLog.CurrentLogLevel <= LogLevel.Error)
{
NetworkLog.LogError($"Client wrote to {typeof(NetworkVariable<>).Name} without permission. No more variables can be read. This is critical. => {(logInstance != null ? ($"{nameof(NetworkObjectId)}: {logInstance.NetworkObjectId} - {nameof(NetworkObject.GetNetworkBehaviourOrderIndex)}(): {logInstance.NetworkObject.GetNetworkBehaviourOrderIndex(logInstance)} - VariableIndex: {i}") : string.Empty)}");
NetworkLog.LogError($"[{networkVariableList[i].GetType().Name}]");
NetworkLog.LogError($"Client wrote to {typeof(NetworkVariable<>).Name} without permission. No more variables can be read. This is critical. => {nameof(NetworkObjectId)}: {NetworkObjectId} - {nameof(NetworkObject.GetNetworkBehaviourOrderIndex)}(): {NetworkObject.GetNetworkBehaviourOrderIndex(this)} - VariableIndex: {i}");
NetworkLog.LogError($"[{NetworkVariableFields[i].GetType().Name}]");
}

return;
}
long readStartPos = stream.Position;

networkVariableList[i].ReadDelta(stream, networkManager.IsServer);
networkManager.NetworkMetrics.TrackNetworkVariableDeltaReceived(
NetworkVariableFields[i].ReadDelta(stream, NetworkManager.IsServer);
NetworkManager.NetworkMetrics.TrackNetworkVariableDeltaReceived(
clientId,
logInstance.NetworkObjectId,
logInstance.name,
networkVariableList[i].Name,
logInstance.__getTypeName(),
NetworkObjectId,
name,
NetworkVariableFields[i].Name,
__getTypeName(),
stream.Length);

(stream as NetworkBuffer).SkipPadBits();

if (networkManager.NetworkConfig.EnsureNetworkVariableLengthSafety)
if (NetworkManager.NetworkConfig.EnsureNetworkVariableLengthSafety)
{
if (stream.Position > (readStartPos + varSize))
{
if (NetworkLog.CurrentLogLevel <= LogLevel.Normal)
{
NetworkLog.LogWarning($"Var delta read too far. {stream.Position - (readStartPos + varSize)} bytes. => {(logInstance != null ? ($"{nameof(NetworkObjectId)}: {logInstance.NetworkObjectId} - {nameof(NetworkObject.GetNetworkBehaviourOrderIndex)}(): {logInstance.NetworkObject.GetNetworkBehaviourOrderIndex(logInstance)} - VariableIndex: {i}") : string.Empty)}");
NetworkLog.LogWarning(
$"Var delta read too far. {stream.Position - (readStartPos + varSize)} bytes. => {nameof(NetworkObjectId)}: {NetworkObjectId} - {nameof(NetworkObject.GetNetworkBehaviourOrderIndex)}(): {NetworkObject.GetNetworkBehaviourOrderIndex(this)} - VariableIndex: {i}");
}

stream.Position = readStartPos + varSize;
Expand All @@ -707,7 +708,8 @@ internal static void HandleNetworkVariableDeltas(List<NetworkVariableBase> netwo
{
if (NetworkLog.CurrentLogLevel <= LogLevel.Normal)
{
NetworkLog.LogWarning($"Var delta read too little. {(readStartPos + varSize) - stream.Position} bytes. => {(logInstance != null ? ($"{nameof(NetworkObjectId)}: {logInstance.NetworkObjectId} - {nameof(NetworkObject.GetNetworkBehaviourOrderIndex)}(): {logInstance.NetworkObject.GetNetworkBehaviourOrderIndex(logInstance)} - VariableIndex: {i}") : string.Empty)}");
NetworkLog.LogWarning(
$"Var delta read too little. {(readStartPos + varSize) - stream.Position} bytes. => {nameof(NetworkObjectId)}: {NetworkObjectId} - {nameof(NetworkObject.GetNetworkBehaviourOrderIndex)}(): {NetworkObject.GetNetworkBehaviourOrderIndex(this)} - VariableIndex: {i}");
}

stream.Position = readStartPos + varSize;
Expand All @@ -717,20 +719,20 @@ internal static void HandleNetworkVariableDeltas(List<NetworkVariableBase> netwo
}
}

internal static void WriteNetworkVariableData(List<NetworkVariableBase> networkVariableList, Stream stream, ulong clientId, NetworkManager networkManager)
internal void WriteNetworkVariableData(Stream stream, ulong clientId)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same story here, no good reason to make this static

{
if (networkVariableList.Count == 0)
if (NetworkVariableFields.Count == 0)
{
return;
}

using (var writer = PooledNetworkWriter.Get(stream))
{
for (int j = 0; j < networkVariableList.Count; j++)
for (int j = 0; j < NetworkVariableFields.Count; j++)
{
bool canClientRead = networkVariableList[j].CanClientRead(clientId);
bool canClientRead = NetworkVariableFields[j].CanClientRead(clientId);

if (networkManager.NetworkConfig.EnsureNetworkVariableLengthSafety)
if (NetworkManager.NetworkConfig.EnsureNetworkVariableLengthSafety)
{
if (!canClientRead)
{
Expand All @@ -744,11 +746,11 @@ internal static void WriteNetworkVariableData(List<NetworkVariableBase> networkV

if (canClientRead)
{
if (networkManager.NetworkConfig.EnsureNetworkVariableLengthSafety)
if (NetworkManager.NetworkConfig.EnsureNetworkVariableLengthSafety)
{
using (var varBuffer = PooledNetworkBuffer.Get())
{
networkVariableList[j].WriteField(varBuffer);
NetworkVariableFields[j].WriteField(varBuffer);
varBuffer.PadBuffer();

writer.WriteUInt16Packed((ushort)varBuffer.Length);
Expand All @@ -757,28 +759,28 @@ internal static void WriteNetworkVariableData(List<NetworkVariableBase> networkV
}
else
{
networkVariableList[j].WriteField(stream);
NetworkVariableFields[j].WriteField(stream);
writer.WritePadBits();
}
}
}
}
}

internal static void SetNetworkVariableData(List<NetworkVariableBase> networkVariableList, Stream stream, NetworkManager networkManager)
internal void SetNetworkVariableData(Stream stream)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same story here, cleaner as instance method

{
if (networkVariableList.Count == 0)
if (NetworkVariableFields.Count == 0)
{
return;
}

using (var reader = PooledNetworkReader.Get(stream))
{
for (int j = 0; j < networkVariableList.Count; j++)
for (int j = 0; j < NetworkVariableFields.Count; j++)
{
ushort varSize = 0;

if (networkManager.NetworkConfig.EnsureNetworkVariableLengthSafety)
if (NetworkManager.NetworkConfig.EnsureNetworkVariableLengthSafety)
{
varSize = reader.ReadUInt16Packed();

Expand All @@ -797,10 +799,10 @@ internal static void SetNetworkVariableData(List<NetworkVariableBase> networkVar

long readStartPos = stream.Position;

networkVariableList[j].ReadField(stream);
NetworkVariableFields[j].ReadField(stream);
reader.SkipPadBits();

if (networkManager.NetworkConfig.EnsureNetworkVariableLengthSafety)
if (NetworkManager.NetworkConfig.EnsureNetworkVariableLengthSafety)
{
if (stream is NetworkBuffer networkBuffer)
{
Expand Down
14 changes: 8 additions & 6 deletions com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ public void NetworkShow(ulong clientId)

Observers.Add(clientId);

NetworkManager.SpawnManager.SendSpawnCallForObject(clientId, OwnerClientId, this);
NetworkManager.SpawnManager.SendSpawnCallForObject(clientId, this);
}

/// <summary>
Expand Down Expand Up @@ -499,7 +499,7 @@ private void SpawnInternal(bool destroyWithScene, ulong? ownerClientId, bool pla
{
if (Observers.Contains(NetworkManager.ConnectedClientsList[i].ClientId))
{
NetworkManager.SpawnManager.SendSpawnCallForObject(NetworkManager.ConnectedClientsList[i].ClientId, ownerId, this);
NetworkManager.SpawnManager.SendSpawnCallForObject(NetworkManager.ConnectedClientsList[i].ClientId, this);
}
}
}
Expand Down Expand Up @@ -872,17 +872,19 @@ internal void WriteNetworkVariableData(Stream stream, ulong clientId)
{
for (int i = 0; i < ChildNetworkBehaviours.Count; i++)
{
ChildNetworkBehaviours[i].InitializeVariables();
NetworkBehaviour.WriteNetworkVariableData(ChildNetworkBehaviours[i].NetworkVariableFields, stream, clientId, NetworkManager);
var behavior = ChildNetworkBehaviours[i];
behavior.InitializeVariables();
behavior.WriteNetworkVariableData(stream, clientId);
}
}

internal void SetNetworkVariableData(Stream stream)
{
for (int i = 0; i < ChildNetworkBehaviours.Count; i++)
{
ChildNetworkBehaviours[i].InitializeVariables();
NetworkBehaviour.SetNetworkVariableData(ChildNetworkBehaviours[i].NetworkVariableFields, stream, NetworkManager);
var behaviour = ChildNetworkBehaviours[i];
behaviour.InitializeVariables();
behaviour.SetNetworkVariableData(stream);
}
}

Expand Down
6 changes: 3 additions & 3 deletions com.unity.netcode.gameobjects/Runtime/Messaging/InternalMessageHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,9 +211,9 @@ public void HandleNetworkVariableDelta(ulong clientId, Stream stream)

if (NetworkManager.SpawnManager.SpawnedObjects.TryGetValue(networkObjectId, out NetworkObject networkObject))
{
NetworkBehaviour instance = networkObject.GetNetworkBehaviourAtOrderIndex(networkBehaviourIndex);
NetworkBehaviour behaviour = networkObject.GetNetworkBehaviourAtOrderIndex(networkBehaviourIndex);

if (instance == null)
if (behaviour == null)
{
if (NetworkLog.CurrentLogLevel <= LogLevel.Normal)
{
Expand All @@ -222,7 +222,7 @@ public void HandleNetworkVariableDelta(ulong clientId, Stream stream)
}
else
{
NetworkBehaviour.HandleNetworkVariableDeltas(instance.NetworkVariableFields, stream, clientId, instance, NetworkManager);
behaviour.HandleNetworkVariableDeltas(stream, clientId);
}
}
else if (NetworkManager.IsServer)
Expand Down
2 changes: 1 addition & 1 deletion com.unity.netcode.gameobjects/Runtime/Metrics/NetworkObjectProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#if MULTIPLAYER_TOOLS
#if MULTIPLAYER_TOOLS
using Unity.Multiplayer.Tools;
using UnityEngine;

Expand Down
8 changes: 4 additions & 4 deletions com.unity.netcode.gameobjects/Runtime/NetworkVariable/ClientNetworkVariable.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,16 @@ public class ClientNetworkVariable<T> : NetworkVariable<T> where T : unmanaged
{
public ClientNetworkVariable() { }

public ClientNetworkVariable(NetworkVariableSettings settings) : base(settings) { }
public ClientNetworkVariable(NetworkVariableReadPermission readPerm) : base(readPerm) { }

public override bool CanClientWrite(ulong clientId)
{
return NetworkBehaviour.OwnerClientId == clientId;
return m_NetworkBehaviour.OwnerClientId == clientId;
}

public override bool ShouldWrite(ulong clientId, bool isServer)
{
return m_IsDirty && !isServer && NetworkBehaviour.IsOwner;
return m_IsDirty && !isServer && m_NetworkBehaviour.IsOwner;
}

/// <summary>
Expand All @@ -40,7 +40,7 @@ public override T Value

// Also, note this is not really very water-tight, if you are running as a host
// we cannot tell if a ClientNetworkVariable write is happening inside server-ish code
if (NetworkBehaviour.NetworkManager.IsServer)
if (m_NetworkBehaviour.NetworkManager.IsServer)
{
throw new InvalidOperationException("Server not allowed to write to ClientNetworkVariables");
}
Expand Down
Loading