Skip to content

feat!: network variables - client auth, permission cleanup, containers #1074

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 29 commits into from
Aug 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
8990cb4
refactor!: convert NetworkTransform.NetworkState to `struct`
0xFA11 Aug 17, 2021
a304dcc
feat!: remove client network permissions [MTT-1019]
mattwalsh-unity Aug 13, 2021
28eff19
feat: client auth experiment
mattwalsh-unity Aug 20, 2021
b5c8041
removed unneeded function
mattwalsh-unity Aug 20, 2021
63cbc7a
NetworkList is now a derived class
mattwalsh-unity Aug 20, 2021
efd612a
dict & set
mattwalsh-unity Aug 20, 2021
51159b1
de-interface checkpoint
mattwalsh-unity Aug 20, 2021
b85b7b3
lint fixes
mattwalsh-unity Aug 20, 2021
c6da3c7
removed valueRef
mattwalsh-unity Aug 23, 2021
acc1db2
cleanup
mattwalsh-unity Aug 24, 2021
0d7555b
added host tests
mattwalsh-unity Aug 24, 2021
ae45477
promoto tmp netstate to be a legit member
0xFA11 Aug 24, 2021
7726238
hrm channel
mattwalsh-unity Aug 24, 2021
e205ded
fix NetworkVarBufferCopyTest
mattwalsh-unity Aug 24, 2021
fb50bdd
fix networktransformstatetests
0xFA11 Aug 24, 2021
dec3707
merge branch 'develop' into 'experimental/client_auth'
0xFA11 Aug 24, 2021
2b9a477
more container cleanup
mattwalsh-unity Aug 24, 2021
26dab82
Merge branch 'experimental/client_auth' of github.com:Unity-Technolog…
mattwalsh-unity Aug 24, 2021
2e0b565
put meta file back
0xFA11 Aug 24, 2021
f1aed5a
Merge branch 'experimental/client_auth' of github.com:Unity-Technolog…
mattwalsh-unity Aug 24, 2021
aad72e0
channel cleanup
mattwalsh-unity Aug 24, 2021
5674c62
fixed client perm, more tests, cleanup
mattwalsh-unity Aug 24, 2021
5fd4b51
INetworkVariable -> NetworkVariableBase
mattwalsh-unity Aug 24, 2021
d42a641
comment cleanup
mattwalsh-unity Aug 24, 2021
7cc4fd5
fix compile error: put NetworkVariableBase.cs.meta in
0xFA11 Aug 25, 2021
690bcd9
minor polish/cleanup
0xFA11 Aug 25, 2021
78be4fe
remove NetworkVariableBase.SetNetworkBehaviour()
0xFA11 Aug 25, 2021
fdbccbc
fix tools project tests
0xFA11 Aug 25, 2021
22792cc
Merge branch 'develop' into experimental/client_auth
mattwalsh-unity Aug 25, 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
11 changes: 3 additions & 8 deletions com.unity.netcode.gameobjects/Editor/NetworkBehaviourEditor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -58,20 +58,15 @@ private void RenderNetworkVariable(int index)
if (value == null)
{
var fieldType = m_NetworkVariableFields[m_NetworkVariableNames[index]].FieldType;
var networkVariable = (INetworkVariable)Activator.CreateInstance(fieldType, true);
var networkVariable = (NetworkVariableBase)Activator.CreateInstance(fieldType, true);
m_NetworkVariableFields[m_NetworkVariableNames[index]].SetValue(target, networkVariable);
}

var type = m_NetworkVariableFields[m_NetworkVariableNames[index]].GetValue(target).GetType();
var genericType = type.GetGenericArguments()[0];

EditorGUILayout.BeginHorizontal();
if (genericType == typeof(string))
{
var networkVariable = (NetworkVariable<string>)m_NetworkVariableFields[m_NetworkVariableNames[index]].GetValue(target);
networkVariable.Value = EditorGUILayout.TextField(m_NetworkVariableNames[index], networkVariable.Value);
}
else if (genericType.IsValueType)
if (genericType.IsValueType)
{
var method = typeof(NetworkBehaviourEditor).GetMethod("RenderNetworkVariableValueType", BindingFlags.Public | BindingFlags.Instance | BindingFlags.FlattenHierarchy | BindingFlags.NonPublic);
var genericMethod = method.MakeGenericMethod(genericType);
Expand All @@ -86,7 +81,7 @@ private void RenderNetworkVariable(int index)
EditorGUILayout.EndHorizontal();
}

private void RenderNetworkVariableValueType<T>(int index) where T : struct
private void RenderNetworkVariableValueType<T>(int index) where T : unmanaged
{
var networkVariable = (NetworkVariable<T>)m_NetworkVariableFields[m_NetworkVariableNames[index]].GetValue(target);
var type = typeof(T);
Expand Down
19 changes: 8 additions & 11 deletions com.unity.netcode.gameobjects/Prototyping/NetworkTransform.cs
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,6 @@ public void NetworkSerialize(NetworkSerializer serializer)
}
}

/// <summary>
/// The network channel to use send updates
/// </summary>
[Tooltip("The network channel to use send updates")]
public NetworkChannel Channel = NetworkChannel.NetworkVariable;

/// <summary>
/// Sets whether this transform should sync in local space or in world space.
/// This is important to set since reparenting this transform could have issues,
Expand All @@ -156,6 +150,7 @@ public void NetworkSerialize(NetworkSerializer serializer)
public float FixedSendsPerSecond = 30f;

private Transform m_Transform; // cache the transform component to reduce unnecessary bounce between managed and native
internal NetworkState LocalNetworkState;
internal readonly NetworkVariable<NetworkState> ReplNetworkState = new NetworkVariable<NetworkState>(new NetworkState());
internal NetworkState PrevNetworkState;

Expand Down Expand Up @@ -364,10 +359,6 @@ private void OnNetworkStateChanged(NetworkState oldState, NetworkState newState)
private void Awake()
{
m_Transform = transform;

ReplNetworkState.Settings.SendNetworkChannel = Channel;
ReplNetworkState.Settings.SendTickrate = FixedSendsPerSecond;

ReplNetworkState.OnValueChanged += OnNetworkStateChanged;
}

Expand All @@ -385,7 +376,13 @@ private void FixedUpdate()

if (IsServer)
{
ReplNetworkState.SetDirty(UpdateNetworkState(ref ReplNetworkState.ValueRef));
// try to update local NetworkState
if (UpdateNetworkState(ref LocalNetworkState))
{
// if updated (dirty), change NetVar, mark it dirty
ReplNetworkState.Value = LocalNetworkState;
ReplNetworkState.SetDirty(true);
}
}
// try to update previously consumed NetworkState
// if we have any changes, that means made some updates locally
Expand Down
119 changes: 13 additions & 106 deletions com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ public virtual void OnNetworkObjectParentChanged(NetworkObject parentNetworkObje

private readonly List<HashSet<int>> m_ChannelMappedNetworkVariableIndexes = new List<HashSet<int>>();
private readonly List<NetworkChannel> m_ChannelsForNetworkVariableGroups = new List<NetworkChannel>();
internal readonly List<INetworkVariable> NetworkVariableFields = new List<INetworkVariable>();
internal readonly List<NetworkVariableBase> NetworkVariableFields = new List<NetworkVariableBase>();

private static Dictionary<Type, FieldInfo[]> s_FieldTypes = new Dictionary<Type, FieldInfo[]>();

Expand Down Expand Up @@ -415,19 +415,19 @@ internal void InitializeVariables()
{
Type fieldType = sortedFields[i].FieldType;

if (fieldType.HasInterface(typeof(INetworkVariable)))
if (fieldType.IsSubclassOf(typeof(NetworkVariableBase)))
{
var instance = (INetworkVariable)sortedFields[i].GetValue(this);
var instance = (NetworkVariableBase)sortedFields[i].GetValue(this);

if (instance == null)
{
instance = (INetworkVariable)Activator.CreateInstance(fieldType, true);
instance = (NetworkVariableBase)Activator.CreateInstance(fieldType, true);
sortedFields[i].SetValue(this, instance);
}

instance.SetNetworkBehaviour(this);
instance.NetworkBehaviour = this;

var instanceNameProperty = fieldType.GetProperty(nameof(INetworkVariable.Name));
var instanceNameProperty = fieldType.GetProperty(nameof(NetworkVariableBase.Name));
var sanitizedVariableName = sortedFields[i].Name.Replace("<", string.Empty).Replace(">k__BackingField", string.Empty);
instanceNameProperty?.SetValue(instance, sanitizedVariableName);

Expand All @@ -442,7 +442,7 @@ internal void InitializeVariables()

for (int i = 0; i < NetworkVariableFields.Count; i++)
{
NetworkChannel networkChannel = NetworkVariableFields[i].GetChannel();
var networkChannel = NetworkVariableBase.NetworkVariableChannel;

if (!firstLevelIndex.ContainsKey(networkChannel))
{
Expand Down Expand Up @@ -514,6 +514,7 @@ private void NetworkVariableUpdate(ulong clientId, int behaviourIndex)
{
using (var writer = PooledNetworkWriter.Get(buffer))
{
// TODO: could skip this if no variables dirty, though obsolete w/ Snapshot
writer.WriteUInt64Packed(NetworkObjectId);
writer.WriteUInt16Packed(NetworkObject.GetNetworkBehaviourOrderIndex(this));

Expand All @@ -537,14 +538,9 @@ private void NetworkVariableUpdate(ulong clientId, int behaviourIndex)
continue;
}

bool isDirty =
NetworkVariableFields[k]
.IsDirty(); // cache this here. You never know what operations users will do in the dirty methods

// if I'm dirty AND a client, write (server always has all permissions)
// if I'm dirty AND the server AND the client can read me, send.
bool shouldWrite = isDirty &&
(!IsServer || NetworkVariableFields[k].CanClientRead(clientId));
bool shouldWrite = NetworkVariableFields[k].ShouldWrite(clientId, IsServer);

if (NetworkManager.NetworkConfig.EnsureNetworkVariableLengthSafety)
{
Expand Down Expand Up @@ -628,7 +624,7 @@ private bool CouldHaveDirtyNetworkVariables()
return false;
}

internal static void HandleNetworkVariableDeltas(List<INetworkVariable> networkVariableList, Stream stream, ulong clientId, NetworkBehaviour logInstance, NetworkManager networkManager)
internal static void HandleNetworkVariableDeltas(List<NetworkVariableBase> networkVariableList, Stream stream, ulong clientId, NetworkBehaviour logInstance, NetworkManager networkManager)
{
using (var reader = PooledNetworkReader.Get(stream))
{
Expand All @@ -655,6 +651,7 @@ internal static void HandleNetworkVariableDeltas(List<INetworkVariable> networkV

if (networkManager.IsServer && !networkVariableList[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 (NetworkLog.CurrentLogLevel <= LogLevel.Normal)
Expand All @@ -674,7 +671,6 @@ internal static void HandleNetworkVariableDeltas(List<INetworkVariable> networkV
//A dummy read COULD be added to the interface for this situation, but it's just being too nice.
//This is after all a developer fault. A critical error should be fine.
// - 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)}");
Expand All @@ -683,7 +679,6 @@ internal static void HandleNetworkVariableDeltas(List<INetworkVariable> networkV

return;
}

long readStartPos = stream.Position;

networkVariableList[i].ReadDelta(stream, networkManager.IsServer);
Expand Down Expand Up @@ -722,95 +717,7 @@ internal static void HandleNetworkVariableDeltas(List<INetworkVariable> networkV
}
}

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

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

if (varSize == 0)
{
continue;
}
}
else
{
if (!reader.ReadBool())
{
continue;
}
}

if (networkManager.IsServer && !networkVariableList[i].CanClientWrite(clientId))
{
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)}");
}

stream.Position += varSize;
continue;
}

//This client wrote somewhere they are not allowed. This is critical
//We can't just skip this field. Because we don't actually know how to dummy read
//That is, we don't know how many bytes to skip. Because the interface doesn't have a
//Read that gives us the value. Only a Read that applies the value straight away
//A dummy read COULD be added to the interface for this situation, but it's just being too nice.
//This is after all a developer fault. A critical error should be fine.
// - 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)}");
}

return;
}

long readStartPos = stream.Position;

networkVariableList[i].ReadField(stream);

if (networkManager.NetworkConfig.EnsureNetworkVariableLengthSafety)
{
if (stream is NetworkBuffer networkBuffer)
{
networkBuffer.SkipPadBits();
}

if (stream.Position > (readStartPos + varSize))
{
if (NetworkLog.CurrentLogLevel <= LogLevel.Normal)
{
NetworkLog.LogWarning($"Var update 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)}");
}

stream.Position = readStartPos + varSize;
}
else if (stream.Position < (readStartPos + varSize))
{
if (NetworkLog.CurrentLogLevel <= LogLevel.Normal)
{
NetworkLog.LogWarning($"Var update 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)}");
}

stream.Position = readStartPos + varSize;
}
}
}
}
}


internal static void WriteNetworkVariableData(List<INetworkVariable> networkVariableList, Stream stream, ulong clientId, NetworkManager networkManager)
internal static void WriteNetworkVariableData(List<NetworkVariableBase> networkVariableList, Stream stream, ulong clientId, NetworkManager networkManager)
{
if (networkVariableList.Count == 0)
{
Expand Down Expand Up @@ -858,7 +765,7 @@ internal static void WriteNetworkVariableData(List<INetworkVariable> networkVari
}
}

internal static void SetNetworkVariableData(List<INetworkVariable> networkVariableList, Stream stream, NetworkManager networkManager)
internal static void SetNetworkVariableData(List<NetworkVariableBase> networkVariableList, Stream stream, NetworkManager networkManager)
{
if (networkVariableList.Count == 0)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,12 @@ internal void NetworkBehaviourUpdate(NetworkManager networkManager)
// when client updates the server, it tells it about all its objects
foreach (var sobj in networkManager.SpawnManager.SpawnedObjectsList)
{
for (int k = 0; k < sobj.ChildNetworkBehaviours.Count; k++)
if (sobj.IsOwner)
{
sobj.ChildNetworkBehaviours[k].VariableUpdate(networkManager.ServerClientId);
for (int k = 0; k < sobj.ChildNetworkBehaviours.Count; k++)
{
sobj.ChildNetworkBehaviours[k].VariableUpdate(networkManager.ServerClientId);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,7 @@ internal void ProcessSingleAck(ushort ackSequence, ulong clientId, ClientData cl
/// This will look into all spawned objects
/// </summary>
/// <param name="key">The key to search for</param>
private INetworkVariable FindNetworkVar(VariableKey key)
private NetworkVariableBase FindNetworkVar(VariableKey key)
{
var spawnedObjects = NetworkManager.SpawnManager.SpawnedObjects;

Expand Down Expand Up @@ -895,7 +895,7 @@ internal void Despawn(SnapshotDespawnCommand command)
/// Might not happen for all variable on every frame. Might even happen more than once.
/// </summary>
/// <param name="networkVariable">The NetworkVariable to write, or rather, its INetworkVariable</param>
internal void Store(ulong networkObjectId, int behaviourIndex, int variableIndex, INetworkVariable networkVariable)
internal void Store(ulong networkObjectId, int behaviourIndex, int variableIndex, NetworkVariableBase networkVariable)
{
VariableKey k;
k.NetworkObjectId = networkObjectId;
Expand All @@ -914,7 +914,7 @@ internal void Store(ulong networkObjectId, int behaviourIndex, int variableIndex
WriteVariableToSnapshot(m_Snapshot, networkVariable, pos);
}

private void WriteVariableToSnapshot(Snapshot snapshot, INetworkVariable networkVariable, int index)
private void WriteVariableToSnapshot(Snapshot snapshot, NetworkVariableBase networkVariable, int index)
{
// write var into buffer, possibly adjusting entry's position and Length
using (var varBuffer = PooledNetworkBuffer.Get())
Expand Down
Loading