Skip to content

Commit

Permalink
feat: Remove DA NetworkVariable modifications (#3155)
Browse files Browse the repository at this point in the history
  • Loading branch information
EmandM authored Dec 6, 2024
1 parent 218a647 commit 3c4f1cc
Show file tree
Hide file tree
Showing 15 changed files with 17 additions and 573 deletions.
2 changes: 2 additions & 0 deletions com.unity.netcode.gameobjects/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ Additional documentation and release notes are available at [Multiplayer Documen

### Fixed

- Fixed issue where an exception was thrown when calling `NetworkManager.Shutdown` after calling `UnityTransport.Shutdown`. (#3118)
- Fixed issue where `NetworkList` properties on in-scene placed `NetworkObject`s could cause small memory leaks when entering playmode. (#3147)
- Fixed in-scene `NertworkObject` synchronization issue when loading a scene with currently connected clients connected to a session created by a `NetworkManager` started as a server (i.e. not as a host). (#3133)
- Fixed issue where a `NetworkManager` started as a server would not add itself as an observer to in-scene placed `NetworkObject`s instantiated and spawned by a scene loading event. (#3133)
Expand All @@ -26,6 +27,7 @@ Additional documentation and release notes are available at [Multiplayer Documen

### Changed

- Optimised `NetworkVariable` and `NetworkTransform` related packets when in Distributed Authority mode.
- The Debug Simulator section of the Unity Transport component was removed. This section was not functional anymore and users are now recommended to use the more featureful [Network Simulator](https://docs-multiplayer.unity3d.com/tools/current/tools-network-simulator/) tool from the Multiplayer Tools package instead. (#3121)

## [2.1.1] - 2024-10-18
Expand Down
42 changes: 6 additions & 36 deletions com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs
Original file line number Diff line number Diff line change
Expand Up @@ -826,7 +826,7 @@ public virtual void OnGainedOwnership() { }
internal void InternalOnGainedOwnership()
{
UpdateNetworkProperties();
// New owners need to assure any NetworkVariables they have write permissions
// New owners need to assure any NetworkVariables they have write permissions
// to are updated so the previous and original values are aligned with the
// current value (primarily for collections).
if (OwnerClientId == NetworkManager.LocalClientId)
Expand Down Expand Up @@ -1181,14 +1181,8 @@ internal void WriteNetworkVariableData(FastBufferWriter writer, ulong targetClie
{
// Create any values that require accessing the NetworkManager locally (it is expensive to access it in NetworkBehaviour)
var networkManager = NetworkManager;
var distributedAuthority = networkManager.DistributedAuthorityMode;
var ensureLengthSafety = networkManager.NetworkConfig.EnsureNetworkVariableLengthSafety;

// Always write the NetworkVariable count even if zero for distributed authority (used by comb server)
if (distributedAuthority)
{
writer.WriteValueSafe((ushort)NetworkVariableFields.Count);
}

// Exit early if there are no NetworkVariables
if (NetworkVariableFields.Count == 0)
Expand All @@ -1203,14 +1197,8 @@ internal void WriteNetworkVariableData(FastBufferWriter writer, ulong targetClie
if (NetworkVariableFields[j].CanClientRead(targetClientId))
{
// Write additional NetworkVariable information when length safety is enabled or when in distributed authority mode
if (ensureLengthSafety || distributedAuthority)
if (ensureLengthSafety)
{
// Write the type being serialized for distributed authority (only for comb-server)
if (distributedAuthority)
{
writer.WriteValueSafe(NetworkVariableFields[j].Type);
}

var writePos = writer.Position;
// Note: This value can't be packed because we don't know how large it will be in advance
// we reserve space for it, then write the data, then come back and fill in the space
Expand Down Expand Up @@ -1261,20 +1249,8 @@ internal void SetNetworkVariableData(FastBufferReader reader, ulong clientId)
{
// Stack cache any values that requires accessing the NetworkManager (it is expensive to access it in NetworkBehaviour)
var networkManager = NetworkManager;
var distributedAuthority = networkManager.DistributedAuthorityMode;
var ensureLengthSafety = networkManager.NetworkConfig.EnsureNetworkVariableLengthSafety;

// Always read the NetworkVariable count when in distributed authority (sanity check if comb-server matches what client has locally)
if (distributedAuthority)
{
reader.ReadValueSafe(out ushort variableCount);
if (variableCount != NetworkVariableFields.Count)
{
Debug.LogError($"[{name}][NetworkObjectId: {NetworkObjectId}][NetworkBehaviourId: {NetworkBehaviourId}] NetworkVariable count mismatch! (Read: {variableCount} vs. Expected: {NetworkVariableFields.Count})");
return;
}
}

// Exit early if nothing else to read
if (NetworkVariableFields.Count == 0)
{
Expand All @@ -1289,14 +1265,8 @@ internal void SetNetworkVariableData(FastBufferReader reader, ulong clientId)
// Distributed Authority: All clients have read permissions, always try to read the value
if (NetworkVariableFields[j].CanClientRead(clientId))
{
if (ensureLengthSafety || distributedAuthority)
if (ensureLengthSafety)
{
// Read the type being serialized and discard it (for now) when in a distributed authority network topology (only used by comb-server)
if (distributedAuthority)
{
reader.ReadValueSafe(out NetworkVariableType _);
}

reader.ReadValueSafe(out varSize);
if (varSize == 0)
{
Expand All @@ -1320,11 +1290,11 @@ internal void SetNetworkVariableData(FastBufferReader reader, ulong clientId)
continue;
}

// Read the NetworkVarible value
// Read the NetworkVariable value
NetworkVariableFields[j].ReadField(reader);

// When EnsureNetworkVariableLengthSafety or DistributedAuthorityMode always do a bounds check
if (ensureLengthSafety || distributedAuthority)
// When EnsureNetworkVariableLengthSafety always do a bounds check
if (ensureLengthSafety)
{
if (reader.Position > (readStartPos + varSize))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,28 +141,6 @@ public NetworkMessageManager(INetworkMessageSender sender, object owner, INetwor
{
RegisterMessageType(type);
}

#if UNITY_EDITOR
if (EnableMessageOrderConsoleLog)
{
// DANGO-TODO: Remove this when we have some form of message type indices stability in place
// For now, just log the messages and their assigned types for reference purposes.
var networkManager = m_Owner as NetworkManager;
if (networkManager != null)
{
if (networkManager.DistributedAuthorityMode)
{
var messageListing = new StringBuilder();
messageListing.AppendLine("NGO Message Index to Type Listing:");
foreach (var message in m_MessageTypes)
{
messageListing.AppendLine($"[{message.Value}][{message.Key.Name}]");
}
Debug.Log(messageListing);
}
}
}
#endif
}
catch (Exception)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ public class NetworkList<T> : NetworkVariableBase where T : unmanaged, IEquatabl
/// The callback to be invoked when the list gets changed
/// </summary>
public event OnListChangedDelegate OnListChanged;
internal override NetworkVariableType Type => NetworkVariableType.NetworkList;

/// <summary>
/// Constructor method for <see cref="NetworkList"/>
Expand Down Expand Up @@ -136,20 +135,6 @@ public override void WriteDelta(FastBufferWriter writer)
/// <inheritdoc />
public override void WriteField(FastBufferWriter writer)
{
if (m_NetworkManager.DistributedAuthorityMode)
{
writer.WriteValueSafe(NetworkVariableSerialization<T>.Serializer.Type);
if (NetworkVariableSerialization<T>.Serializer.Type == NetworkVariableType.Unmanaged)
{
// Write the size of the unmanaged serialized type as it has a fixed size. This allows the CMB runtime to correctly read the unmanged type.
var placeholder = new T();
var startPos = writer.Position;
NetworkVariableSerialization<T>.Serializer.Write(writer, ref placeholder);
var size = writer.Position - startPos;
writer.Seek(startPos);
BytePacker.WriteValueBitPacked(writer, size);
}
}
writer.WriteValueSafe((ushort)m_List.Length);
for (int i = 0; i < m_List.Length; i++)
{
Expand All @@ -161,15 +146,6 @@ public override void WriteField(FastBufferWriter writer)
public override void ReadField(FastBufferReader reader)
{
m_List.Clear();
if (m_NetworkManager.DistributedAuthorityMode)
{
SerializationTools.ReadType(reader, NetworkVariableSerialization<T>.Serializer);
// Collection item type is used by the DA server, drop value here.
if (NetworkVariableSerialization<T>.Serializer.Type == NetworkVariableType.Unmanaged)
{
ByteUnpacker.ReadValueBitPacked(reader, out int _);
}
}
reader.ReadValueSafe(out ushort count);
for (int i = 0; i < count; i++)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,6 @@ public override void OnInitialize()
NetworkVariableSerialization<T>.Duplicate(m_InternalValue, ref m_PreviousValue);
}

internal override NetworkVariableType Type => NetworkVariableType.Value;

/// <summary>
/// Constructor for <see cref="NetworkVariable{T}"/>
/// </summary>
Expand Down Expand Up @@ -92,7 +90,7 @@ public void Reset(T value = default)
// The introduction of standard .NET collections caused an issue with permissions since there is no way to detect changes in the
// collection without doing a full comparison. While this approach does consume more memory per collection instance, it is the
// lowest risk approach to resolving the issue where a client with no write permissions could make changes to a collection locally
// which can cause a myriad of issues.
// which can cause a myriad of issues.
private protected T m_InternalOriginalValue;

private protected T m_PreviousValue;
Expand Down Expand Up @@ -305,7 +303,7 @@ public override void ReadDelta(FastBufferReader reader, bool keepDirtyDelta)
/// This should be always invoked (client & server) to assure the previous values are set
/// !! IMPORTANT !!
/// When a server forwards delta updates to connected clients, it needs to preserve the previous dirty value(s)
/// until it is done serializing all valid NetworkVariable field deltas (relative to each client). This is invoked
/// until it is done serializing all valid NetworkVariable field deltas (relative to each client). This is invoked
/// after it is done forwarding the deltas at the end of the <see cref="NetworkVariableDeltaMessage.Handle(ref NetworkContext)"/> method.
/// </summary>
internal override void PostDeltaRead()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@ public abstract class NetworkVariableBase : IDisposable

private NetworkManager m_InternalNetworkManager;

internal virtual NetworkVariableType Type => NetworkVariableType.Unknown;

internal string GetWritePermissionError()
{
return $"|Client-{m_NetworkManager.LocalClientId}|{m_NetworkBehaviour.name}|{Name}| Write permissions ({WritePerm}) for this client instance is not allowed!";
Expand Down Expand Up @@ -351,7 +349,7 @@ internal ulong OwnerClientId()
/// This should be always invoked (client & server) to assure the previous values are set
/// !! IMPORTANT !!
/// When a server forwards delta updates to connected clients, it needs to preserve the previous dirty value(s)
/// until it is done serializing all valid NetworkVariable field deltas (relative to each client). This is invoked
/// until it is done serializing all valid NetworkVariable field deltas (relative to each client). This is invoked
/// after it is done forwarding the deltas at the end of the <see cref="NetworkVariableDeltaMessage.Handle(ref NetworkContext)"/> method.
/// </summary>
internal virtual void PostDeltaRead()
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,6 @@ namespace Unity.Netcode
/// <typeparam name="T"></typeparam>
internal class FallbackSerializer<T> : INetworkVariableSerializer<T>
{
public NetworkVariableType Type => NetworkVariableType.Unknown;
public bool IsDistributedAuthorityOptimized => true;

private void ThrowArgumentError()
{
throw new ArgumentException($"Serialization has not been generated for type {typeof(T).FullName}. This can be addressed by adding a [{nameof(GenerateSerializationForGenericParameterAttribute)}] to your generic class that serializes this value (if you are using one), adding [{nameof(GenerateSerializationForTypeAttribute)}(typeof({typeof(T).FullName})] to the class or method that is attempting to serialize it, or creating a field on a {nameof(NetworkBehaviour)} of type {nameof(NetworkVariable<T>)}. If this error continues to appear after doing one of those things and this is a type you can change, then either implement {nameof(INetworkSerializable)} or mark it as serializable by memcpy by adding {nameof(INetworkSerializeByMemcpy)} to its interface list to enable automatic serialization generation. If not, assign serialization code to {nameof(UserNetworkVariableSerialization<T>)}.{nameof(UserNetworkVariableSerialization<T>.WriteValue)}, {nameof(UserNetworkVariableSerialization<T>)}.{nameof(UserNetworkVariableSerialization<T>.ReadValue)}, and {nameof(UserNetworkVariableSerialization<T>)}.{nameof(UserNetworkVariableSerialization<T>.DuplicateValue)}, or if it's serializable by memcpy (contains no pointers), wrap it in {typeof(ForceNetworkSerializeByMemcpy<>).Name}.");
Expand Down Expand Up @@ -82,11 +79,6 @@ public void Duplicate(in T value, ref T duplicatedValue)
}
UserNetworkVariableSerialization<T>.DuplicateValue(value, ref duplicatedValue);
}

public void WriteDistributedAuthority(FastBufferWriter writer, ref T value) => ThrowArgumentError();
public void ReadDistributedAuthority(FastBufferReader reader, ref T value) => ThrowArgumentError();
public void WriteDeltaDistributedAuthority(FastBufferWriter writer, ref T value, ref T previousValue) => ThrowArgumentError();
public void ReadDeltaDistributedAuthority(FastBufferReader reader, ref T value) => ThrowArgumentError();
}

// RuntimeAccessModifiersILPP will make this `public`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,28 +2,8 @@

namespace Unity.Netcode
{
/// <summary>
/// Interface used by NetworkVariables to serialize them with additional information for the DA runtime
/// </summary>
///
/// <typeparam name="T"></typeparam>
internal interface IDistributedAuthoritySerializer<T>
{
/// <summary>
/// The Type tells the DA server how to parse this type.
/// The user should never be able to override this value, as it is meaningful for the DA server
/// </summary>
public NetworkVariableType Type { get; }
public bool IsDistributedAuthorityOptimized { get; }
public void WriteDistributedAuthority(FastBufferWriter writer, ref T value);
public void ReadDistributedAuthority(FastBufferReader reader, ref T value);
public void WriteDeltaDistributedAuthority(FastBufferWriter writer, ref T value, ref T previousValue);
public void ReadDeltaDistributedAuthority(FastBufferReader reader, ref T value);
}


/// <typeparam name="T"></typeparam>
internal interface INetworkVariableSerializer<T> : IDistributedAuthoritySerializer<T>
internal interface INetworkVariableSerializer<T>
{
// Write has to be taken by ref here because of INetworkSerializable
// Open Instance Delegates (pointers to methods without an instance attached to them)
Expand Down
Loading

0 comments on commit 3c4f1cc

Please sign in to comment.