From f64fcb81423eec060c3cf8cbf625428252c66b70 Mon Sep 17 00:00:00 2001 From: mischa <16416509+vis2k@users.noreply.github.com> Date: Tue, 18 Oct 2022 11:10:22 +0200 Subject: [PATCH] feature: SyncDirection to easily support client auth components without extra Rpcs/Cmds. previously OnSerialize was only server-to-client direction. (#3232) * feature: SyncDirection * broadcast without global sendInterval * comments * TODO * disconnect moved up --- .../NetworkTransformBase.cs | 2 +- Assets/Mirror/Core/NetworkBehaviour.cs | 42 ++++- Assets/Mirror/Core/NetworkClient.cs | 65 ++++++- Assets/Mirror/Core/NetworkIdentity.cs | 178 +++++++++++++++--- Assets/Mirror/Core/NetworkServer.cs | 40 +++- .../Editor/NetworkBehaviourInspector.cs | 10 +- .../NetworkIdentitySerializationTests.cs | 99 +++++++++- .../Tests/Editor/NetworkIdentityTests.cs | 10 + .../Tests/Editor/SyncVarAttributeTest.cs | 4 +- .../Tests/Runtime/NetworkIdentityTests.cs | 4 +- 10 files changed, 412 insertions(+), 42 deletions(-) diff --git a/Assets/Mirror/Components/NetworkTransform2k/NetworkTransformBase.cs b/Assets/Mirror/Components/NetworkTransform2k/NetworkTransformBase.cs index fe6bebe8a..205a1235c 100644 --- a/Assets/Mirror/Components/NetworkTransform2k/NetworkTransformBase.cs +++ b/Assets/Mirror/Components/NetworkTransform2k/NetworkTransformBase.cs @@ -26,7 +26,7 @@ namespace Mirror { public abstract class NetworkTransformBase : NetworkBehaviour { - // TODO SyncDirection { CLIENT_TO_SERVER, SERVER_TO_CLIENT } is easier? + // TODO SyncDirection { ClientToServer, ServerToClient } is easier? [Header("Authority")] [Tooltip("Set to true if moves come from owner client, set to false if moves always come from server")] public bool clientAuthority; diff --git a/Assets/Mirror/Core/NetworkBehaviour.cs b/Assets/Mirror/Core/NetworkBehaviour.cs index 34d372a3a..40ffa8725 100644 --- a/Assets/Mirror/Core/NetworkBehaviour.cs +++ b/Assets/Mirror/Core/NetworkBehaviour.cs @@ -9,12 +9,25 @@ namespace Mirror // SyncMode decides if a component is synced to all observers, or only owner public enum SyncMode { Observers, Owner } + // SyncDirection decides if a component is synced from: + // * server to all clients + // * owner client, to server, to all other clients + // + // naming: 'ClientToServer' etc. instead of 'ClientAuthority', because + // that wouldn't be accurate. server's OnDeserialize can still validate + // client data before applying. it's really about direction, not authority. + public enum SyncDirection { ServerToClient, ClientToServer } + /// Base class for networked components. [AddComponentMenu("")] [RequireComponent(typeof(NetworkIdentity))] [HelpURL("https://mirror-networking.gitbook.io/docs/guides/networkbehaviour")] public abstract class NetworkBehaviour : MonoBehaviour { + /// Sync direction for OnSerialize. ServerToClient by default. ClientToServer for client authority. + [Tooltip("Server Authority calls OnSerialize on the server and syncs it to clients.\n\nClient Authority calls OnSerialize on the owning client, syncs it to server, which then broadcasts it to all other clients.\n\nUse server authority for cheat safety.")] + [HideInInspector] public SyncDirection syncDirection = SyncDirection.ServerToClient; + /// sync mode for OnSerialize // hidden because NetworkBehaviourInspector shows it only if has OnSerialize. [Tooltip("By default synced data is sent from the server to all Observers of the object.\nChange this to Owner to only have the server update the client that has ownership authority for this object")] @@ -53,6 +66,24 @@ public abstract class NetworkBehaviour : MonoBehaviour [Obsolete(".hasAuthority was renamed to .isOwned. This is easier to understand and prepares for SyncDirection, where there is a difference betwen isOwned and authority.")] // 2022-10-13 public bool hasAuthority => isOwned; + /// authority is true if we are allowed to modify this component's state. On server, it's true if SyncDirection is ServerToClient. On client, it's true if SyncDirection is ClientToServer and(!) if this object is owned by the client. + // on the client: if owned and if clientAuthority sync direction + // on the server: if serverAuthority sync direction + // + // for example, NetworkTransform: + // client may modify position if ClientAuthority mode and owned + // server may modify position only if server authority + // + // note that in original Mirror, hasAuthority only meant 'isOwned'. + // there was no syncDirection to check. + // + // also note that this is a per-NetworkBehaviour flag. + // another component may not be client authoritative, etc. + public bool authority => + isClient + ? syncDirection == SyncDirection.ClientToServer && isOwned + : syncDirection == SyncDirection.ServerToClient; + /// The unique network Id of this object (unique at runtime). public uint netId => netIdentity.netId; @@ -1118,8 +1149,13 @@ internal static int ErrorCorrection(int size, byte safety) return (int)(cleared | safety); } - internal void Deserialize(NetworkReader reader, bool initialState) + // returns false in case of errors. + // server needs to know in order to disconnect on error. + internal bool Deserialize(NetworkReader reader, bool initialState) { + // detect errors, but attempt to correct before returning + bool result = true; + // read 1 byte length hash safety & capture beginning for size check byte safety = reader.ReadByte(); int chunkStart = reader.Position; @@ -1140,6 +1176,7 @@ internal void Deserialize(NetworkReader reader, bool initialState) $" * Are the server and client the exact same project?\n" + $" * Maybe this OnDeserialize call was meant for another GameObject? The sceneIds can easily get out of sync if the Hierarchy was modified only in the client OR the server. Try rebuilding both.\n\n" + $"Exception {e}"); + result = false; } // compare bytes read with length hash @@ -1157,7 +1194,10 @@ internal void Deserialize(NetworkReader reader, bool initialState) // see test: SerializationSizeMismatch. int correctedSize = ErrorCorrection(size, safety); reader.Position = chunkStart + correctedSize; + result = false; } + + return result; } internal void ResetSyncObjects() diff --git a/Assets/Mirror/Core/NetworkClient.cs b/Assets/Mirror/Core/NetworkClient.cs index 2bb580436..07f7e1444 100644 --- a/Assets/Mirror/Core/NetworkClient.cs +++ b/Assets/Mirror/Core/NetworkClient.cs @@ -1031,7 +1031,7 @@ internal static void ApplySpawnPayload(NetworkIdentity identity, SpawnMessage me { using (NetworkReaderPooled payloadReader = NetworkReaderPool.Get(message.payload)) { - identity.Deserialize(payloadReader, true); + identity.DeserializeClient(payloadReader, true); } } @@ -1284,7 +1284,7 @@ static void OnEntityStateMessage(EntityStateMessage message) if (spawned.TryGetValue(message.netId, out NetworkIdentity identity) && identity != null) { using (NetworkReaderPooled reader = NetworkReaderPool.Get(message.payload)) - identity.Deserialize(reader, false); + identity.DeserializeClient(reader, false); } else Debug.LogWarning($"Did not find target for sync message for {message.netId} . Note: this can be completely normal because UDP messages may arrive out of order, so this message might have arrived after a Destroy message."); } @@ -1408,6 +1408,54 @@ static void DestroyObject(uint netId) //else Debug.LogWarning($"Did not find target for destroy message for {netId}"); } + // broadcast /////////////////////////////////////////////////////////// + // make sure Broadcast() is only called every sendInterval. + // calling it every update() would require too much bandwidth. + static void Broadcast() + { + // joined the world yet? + if (!connection.isReady) return; + + // nothing to do in host mode. server already knows the state. + if (NetworkServer.active) return; + + // for each entity that the client owns + foreach (NetworkIdentity identity in connection.owned) + { + // make sure it's not null or destroyed. + // (which can happen if someone uses + // GameObject.Destroy instead of + // NetworkServer.Destroy) + if (identity != null) + { + using (NetworkWriterPooled writer = NetworkWriterPool.Get()) + { + // get serialization for this entity viewed by this connection + // (if anything was serialized this time) + identity.SerializeClient(writer); + if (writer.Position > 0) + { + // send state update message + EntityStateMessage message = new EntityStateMessage + { + netId = identity.netId, + payload = writer.ToArraySegment() + }; + Send(message); + + // reset dirty bits so it's not resent next time. + identity.ClearDirtyComponentsDirtyBits(); + } + } + } + // spawned list should have no null entries because we + // always call Remove in OnObjectDestroy everywhere. + // if it does have null then someone used + // GameObject.Destroy instead of NetworkServer.Destroy. + else Debug.LogWarning($"Found 'null' entry in observing list for connectionId={connection.connectionId}. Please call NetworkServer.Destroy to destroy networked objects. Don't use GameObject.Destroy."); + } + } + // update ////////////////////////////////////////////////////////////// // NetworkEarlyUpdate called before any Update/FixedUpdate // (we add this to the UnityEngine in NetworkLoop) @@ -1425,6 +1473,19 @@ internal static void NetworkEarlyUpdate() // (we add this to the UnityEngine in NetworkLoop) internal static void NetworkLateUpdate() { + // broadcast ClientToServer components while active + // note that Broadcast() runs every update. + // on clients with 120 Hz, this will run 120 times per second. + // however, Broadcast only checks .owned, which usually aren't many. + // + // we could use a .sendInterval, but it would also put a minimum + // limit to every component's sendInterval automatically. + if (active) + { + Broadcast(); + } + + // update connections to flush out messages _after_ broadcast // local connection? if (connection is LocalConnectionToServer localConnection) { diff --git a/Assets/Mirror/Core/NetworkIdentity.cs b/Assets/Mirror/Core/NetworkIdentity.cs index 6084f6488..3b33425f7 100644 --- a/Assets/Mirror/Core/NetworkIdentity.cs +++ b/Assets/Mirror/Core/NetworkIdentity.cs @@ -884,9 +884,9 @@ internal void OnStopAuthority() } } - // build dirty mask for owner & observer (= all dirty components). + // build dirty mask for server owner & observers (= all dirty components). // faster to do it in one iteration instead of iterating separately. - (ulong, ulong) DirtyMasks(bool initialState) + (ulong, ulong) ServerDirtyMasks(bool initialState) { ulong ownerMask = 0; ulong observerMask = 0; @@ -896,21 +896,50 @@ internal void OnStopAuthority() { NetworkBehaviour component = components[i]; - // check if dirty. - // for owner, it's always included if dirty. - // for observers, it's only included if dirty AND syncmode to observers. - bool ownerDirty = initialState || component.IsDirty(); - bool observerDirty = ownerDirty && component.syncMode == SyncMode.Observers; + // initially consider all. afterwards only ServerToClient comps. + // see explanation in SerializeServer(). + // see test: Serialize_ClientToServer_ServerOnlySendsInitial(). + if (initialState || component.syncDirection == SyncDirection.ServerToClient) + { + // check if dirty. + // for owner, it's always included if dirty. + // for observers, it's only included if dirty AND syncmode to observers. + bool ownerDirty = (initialState || component.IsDirty()); + bool observerDirty = ownerDirty && component.syncMode == SyncMode.Observers; - // set the n-th bit. - // shifting from small to large numbers is varint-efficient. - ownerMask |= (ulong)(ownerDirty ? 1 : 0) << i; - observerMask |= (ulong)(observerDirty ? 1 : 0) << i; + // set the n-th bit if dirty. + // shifting from small to large numbers is varint-efficient. + if (ownerDirty) ownerMask |= (1u << i); + if (observerDirty) observerMask |= (1u << i); + } } return (ownerMask, observerMask); } + // build dirty mask for client. + // server always knows initialState, so we don't need it here. + ulong ClientDirtyMask() + { + ulong mask = 0; + + NetworkBehaviour[] components = NetworkBehaviours; + for (int i = 0; i < components.Length; ++i) + { + NetworkBehaviour component = components[i]; + + // on client, only consider owned components with SyncDirection to server + if (isOwned && component.syncDirection == SyncDirection.ClientToServer) + { + // set the n-th bit if dirty + // shifting from small to large numbers is varint-efficient. + if (component.IsDirty()) mask |= (1u << i); + } + } + + return mask; + } + // check if n-th component is dirty. // in other words, if it has the n-th bit set in the dirty mask. [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -920,11 +949,11 @@ internal static bool IsDirty(ulong mask, int index) return (mask & nthBit) != 0; } - // serialize all components using dirtyComponentsMask + // serialize components into writer on the server. // check ownerWritten/observersWritten to know if anything was written // We pass dirtyComponentsMask into this function so that we can check // if any Components are dirty before creating writers - internal void Serialize(bool initialState, NetworkWriter ownerWriter, NetworkWriter observersWriter) + internal void SerializeServer(bool initialState, NetworkWriter ownerWriter, NetworkWriter observersWriter) { // ensure NetworkBehaviours are valid before usage ValidateComponents(); @@ -933,7 +962,7 @@ internal void Serialize(bool initialState, NetworkWriter ownerWriter, NetworkWri // instead of writing a 1 byte index per component, // we limit components to 64 bits and write one ulong instead. // the ulong is also varint compressed for minimum bandwidth. - (ulong ownerMask, ulong observerMask) = DirtyMasks(initialState); + (ulong ownerMask, ulong observerMask) = ServerDirtyMasks(initialState); // varint compresses the mask to 1 byte in most cases. // instead of writing an 8 byte ulong. @@ -953,6 +982,24 @@ internal void Serialize(bool initialState, NetworkWriter ownerWriter, NetworkWri { for (int i = 0; i < components.Length; ++i) { + // on the server, we need to consider different sync scenarios: + // + // ServerToClient SyncDirection: + // always serialize for owner. + // serialize for observers only if SyncMode == Observers. + // + // ClientToServer SyncDirection: + // only serialize 'initial' for spawn data. + // skip if not initial, as it comes from the client. + // for example, the server sets the initial spawn position. + // if we wouldn't sync to the owning client too, then it + // would not have any spawn data and always spawn at (0,0,0). + // serialize for observers only if SyncMode == Observers. + // + // since we always send to authority owners too, we end up with + // the same behaviour for both authority modes. + // this makes the code a lot easier & better for branch prediction. + NetworkBehaviour comp = components[i]; // is this component dirty? @@ -990,7 +1037,95 @@ internal void Serialize(bool initialState, NetworkWriter ownerWriter, NetworkWri } } - internal void Deserialize(NetworkReader reader, bool initialState) + // serialize components into writer on the client. + internal void SerializeClient(NetworkWriter writer) + { + // ensure NetworkBehaviours are valid before usage + ValidateComponents(); + NetworkBehaviour[] components = NetworkBehaviours; + + // instead of writing a 1 byte index per component, + // we limit components to 64 bits and write one ulong instead. + // the ulong is also varint compressed for minimum bandwidth. + ulong dirtyMask = ClientDirtyMask(); + + // varint compresses the mask to 1 byte in most cases. + // instead of writing an 8 byte ulong. + // 7 components fit into 1 byte. (previously 7 bytes) + // 11 components fit into 2 bytes. (previously 11 bytes) + // 16 components fit into 3 bytes. (previously 16 bytes) + // TODO imer: server knows amount of comps, write N bytes instead + + // if nothing dirty, then don't even write the mask. + // otherwise, every unchanged object would send a 1 byte dirty mask! + if (dirtyMask != 0) Compression.CompressVarUInt(writer, dirtyMask); + + // serialize all components + // perf: only iterate if dirty mask has dirty bits. + if (dirtyMask != 0) + { + // serialize all components + for (int i = 0; i < components.Length; ++i) + { + // on the client, we need to consider different sync scenarios: + // + // ServerToClient SyncDirection: + // do nothing. + // ClientToServer SyncDirection: + // serialize only if owned. + + NetworkBehaviour comp = components[i]; + + // is this component dirty? + // reuse the mask instead of calling comp.IsDirty() again here. + if (IsDirty(dirtyMask, i)) + // if (isOwned && component.syncDirection == SyncDirection.ClientToServer) + { + // serialize into writer. + // server always knows initialState, we never need to send it + comp.Serialize(writer, false); + } + } + } + } + + // deserialize components from the client on the server. + // there's no 'initialState'. server always knows the initial state. + internal bool DeserializeServer(NetworkReader reader) + { + // ensure NetworkBehaviours are valid before usage + ValidateComponents(); + NetworkBehaviour[] components = NetworkBehaviours; + + // first we deserialize the varinted dirty mask + ulong mask = Compression.DecompressVarUInt(reader); + + // now deserialize every dirty component + for (int i = 0; i < components.Length; ++i) + { + // was this one dirty? + if (IsDirty(mask, i)) + { + NetworkBehaviour comp = components[i]; + + // safety check to ensure clients can only modify their own + // ClientToServer components, nothing else. + if (comp.syncDirection == SyncDirection.ClientToServer) + { + // deserialize this component + // server always knows the initial state (initial=false) + // disconnect if failed, to prevent exploits etc. + if (!comp.Deserialize(reader, false)) return false; + } + } + } + + // successfully deserialized everything + return true; + } + + // deserialize components from server on the client. + internal void DeserializeClient(NetworkReader reader, bool initialState) { // ensure NetworkBehaviours are valid before usage ValidateComponents(); @@ -1011,9 +1146,10 @@ internal void Deserialize(NetworkReader reader, bool initialState) } } - // get cached serialization for this tick (or serialize if none yet) - // IMPORTANT: int tick avoids floating point inaccuracy over days/weeks - internal NetworkIdentitySerialization GetSerializationAtTick(int tick) + // get cached serialization for this tick (or serialize if none yet). + // IMPORTANT: int tick avoids floating point inaccuracy over days/weeks. + // calls SerializeServer, so this function is to be called on server. + internal NetworkIdentitySerialization GetServerSerializationAtTick(int tick) { // only rebuild serialization once per tick. reuse otherwise. // except for tests, where Time.frameCount never increases. @@ -1032,9 +1168,9 @@ internal NetworkIdentitySerialization GetSerializationAtTick(int tick) lastSerialization.observersWriter.Position = 0; // serialize - Serialize(false, - lastSerialization.ownerWriter, - lastSerialization.observersWriter); + SerializeServer(false, + lastSerialization.ownerWriter, + lastSerialization.observersWriter); // clear dirty bits for the components that we serialized. // previously we did this in NetworkServer.BroadcastToConnection diff --git a/Assets/Mirror/Core/NetworkServer.cs b/Assets/Mirror/Core/NetworkServer.cs index 40b7f94ca..f44ba312f 100644 --- a/Assets/Mirror/Core/NetworkServer.cs +++ b/Assets/Mirror/Core/NetworkServer.cs @@ -134,6 +134,7 @@ internal static void RegisterMessageHandlers() RegisterHandler(OnClientReadyMessage); RegisterHandler(OnCommandMessage); RegisterHandler(NetworkTime.OnServerPing, false); + RegisterHandler(OnEntityStateMessage, true); } /// Starts server and listens to incoming connections with max connections limit. @@ -985,6 +986,41 @@ static void OnCommandMessage(NetworkConnectionToClient conn, CommandMessage msg, identity.HandleRemoteCall(msg.componentIndex, msg.functionHash, RemoteCallType.Command, networkReader, conn); } + // client to server broadcast ////////////////////////////////////////// + // for client's owned ClientToServer components. + static void OnEntityStateMessage(NetworkConnectionToClient connection, EntityStateMessage message) + { + // need to validate permissions carefully. + // an attacker may attempt to modify a not-owned or not-ClientToServer component. + + // valid netId? + if (spawned.TryGetValue(message.netId, out NetworkIdentity identity) && identity != null) + { + // owned by the connection? + if (identity.connectionToClient == connection) + { + using (NetworkReaderPooled reader = NetworkReaderPool.Get(message.payload)) + { + // DeserializeServer checks permissions internally. + // failure to deserialize disconnects to prevent exploits. + if (!identity.DeserializeServer(reader)) + { + Debug.LogWarning($"Server failed to deserialize client state for {identity.name} with netId={identity.netId}, Disconnecting."); + connection.Disconnect(); + } + } + } + // an attacker may attempt to modify another connection's entity + else + { + Debug.LogWarning($"Connection {connection.connectionId} attempted to modify {identity} which is not owned by the connection. Disconnecting the connection."); + connection.Disconnect(); + } + } + // no warning. don't spam server logs. + // else Debug.LogWarning($"Did not find target for sync message for {message.netId} . Note: this can be completely normal because UDP messages may arrive out of order, so this message might have arrived after a Destroy message."); + } + // spawning //////////////////////////////////////////////////////////// static ArraySegment CreateSpawnMessagePayload(bool isOwner, NetworkIdentity identity, NetworkWriterPooled ownerWriter, NetworkWriterPooled observersWriter) { @@ -996,7 +1032,7 @@ static ArraySegment CreateSpawnMessagePayload(bool isOwner, NetworkIdentit // serialize all components with initialState = true // (can be null if has none) - identity.Serialize(true, ownerWriter, observersWriter); + identity.SerializeServer(true, ownerWriter, observersWriter); // convert to ArraySegment to avoid reader allocations // if nothing was written, .ToArraySegment returns an empty segment. @@ -1592,7 +1628,7 @@ static NetworkWriter GetEntitySerializationForConnection(NetworkIdentity identit { // get serialization for this entity (cached) // IMPORTANT: int tick avoids floating point inaccuracy over days/weeks - NetworkIdentitySerialization serialization = identity.GetSerializationAtTick(Time.frameCount); + NetworkIdentitySerialization serialization = identity.GetServerSerializationAtTick(Time.frameCount); // is this entity owned by this connection? bool owned = identity.connectionToClient == connection; diff --git a/Assets/Mirror/Editor/NetworkBehaviourInspector.cs b/Assets/Mirror/Editor/NetworkBehaviourInspector.cs index 54b3ae7f3..7cae54f6e 100644 --- a/Assets/Mirror/Editor/NetworkBehaviourInspector.cs +++ b/Assets/Mirror/Editor/NetworkBehaviourInspector.cs @@ -85,7 +85,15 @@ protected void DrawDefaultSyncSettings() EditorGUILayout.Space(); EditorGUILayout.LabelField("Sync Settings", EditorStyles.boldLabel); - EditorGUILayout.PropertyField(serializedObject.FindProperty("syncMode")); + // sync direction + SerializedProperty syncDirection = serializedObject.FindProperty("syncDirection"); + EditorGUILayout.PropertyField(syncDirection); + + // sync mdoe: only show for ServerToClient components + if (syncDirection.enumValueIndex == (int)SyncDirection.ServerToClient) + EditorGUILayout.PropertyField(serializedObject.FindProperty("syncMode")); + + // sync interval EditorGUILayout.PropertyField(serializedObject.FindProperty("syncInterval")); // apply diff --git a/Assets/Mirror/Tests/Editor/NetworkIdentitySerializationTests.cs b/Assets/Mirror/Tests/Editor/NetworkIdentitySerializationTests.cs index bb15f0d87..a1718e74b 100644 --- a/Assets/Mirror/Tests/Editor/NetworkIdentitySerializationTests.cs +++ b/Assets/Mirror/Tests/Editor/NetworkIdentitySerializationTests.cs @@ -49,11 +49,11 @@ public void SerializeAndDeserializeAll() serverComp2.value = "42"; // serialize server object - serverIdentity.Serialize(true, ownerWriter, observersWriter); + serverIdentity.SerializeServer(true, ownerWriter, observersWriter); // deserialize client object with OWNER payload NetworkReader reader = new NetworkReader(ownerWriter.ToArray()); - clientIdentity.Deserialize(reader, true); + clientIdentity.DeserializeClient(reader, true); Assert.That(clientComp1.value, Is.EqualTo(42)); Assert.That(clientComp2.value, Is.EqualTo("42")); @@ -63,7 +63,7 @@ public void SerializeAndDeserializeAll() // deserialize client object with OBSERVERS payload reader = new NetworkReader(observersWriter.ToArray()); - clientIdentity.Deserialize(reader, true); + clientIdentity.DeserializeClient(reader, true); Assert.That(clientComp1.value, Is.EqualTo(42)); // observers mode should be in data Assert.That(clientComp2.value, Is.EqualTo(null)); // owner mode shouldn't be in data } @@ -95,13 +95,13 @@ public void SerializationException() // serialize server object // should work even if compExc throws an exception. // error log because of the exception is expected. - serverIdentity.Serialize(true, ownerWriter, observersWriter); + serverIdentity.SerializeServer(true, ownerWriter, observersWriter); // deserialize client object with OWNER payload // should work even if compExc throws an exception // error log because of the exception is expected NetworkReader reader = new NetworkReader(ownerWriter.ToArray()); - clientIdentity.Deserialize(reader, true); + clientIdentity.DeserializeClient(reader, true); Assert.That(clientComp2.value, Is.EqualTo("42")); // reset component values @@ -111,7 +111,7 @@ public void SerializationException() // should work even if compExc throws an exception // error log because of the exception is expected reader = new NetworkReader(observersWriter.ToArray()); - clientIdentity.Deserialize(reader, true); + clientIdentity.DeserializeClient(reader, true); Assert.That(clientComp2.value, Is.EqualTo(null)); // owner mode should be in data // restore error checks @@ -186,13 +186,13 @@ public void SerializationMismatch() serverComp.value = "42"; // serialize server object - serverIdentity.Serialize(true, ownerWriter, observersWriter); + serverIdentity.SerializeServer(true, ownerWriter, observersWriter); // deserialize on client // ignore warning log because of serialization mismatch LogAssert.ignoreFailingMessages = true; NetworkReader reader = new NetworkReader(ownerWriter.ToArray()); - clientIdentity.Deserialize(reader, true); + clientIdentity.DeserializeClient(reader, true); LogAssert.ignoreFailingMessages = false; // the mismatch component will fail, but the one before and after @@ -205,7 +205,7 @@ public void SerializationMismatch() // 0-dirty-mask. instead, we need to ensure it writes nothing. // too easy to miss, with too significant bandwidth implications. [Test] - public void Serialize_NotInitial_NotDirty_WritesNothing() + public void SerializeServer_NotInitial_NotDirty_WritesNothing() { // create spawned so that isServer/isClient is set properly CreateNetworkedAndSpawn( @@ -218,9 +218,88 @@ public void Serialize_NotInitial_NotDirty_WritesNothing() // serialize server object. // 'initial' would write everything. // instead, try 'not initial' with 0 dirty bits - serverIdentity.Serialize(false, ownerWriter, observersWriter); + serverIdentity.SerializeServer(false, ownerWriter, observersWriter); Assert.That(ownerWriter.Position, Is.EqualTo(0)); Assert.That(observersWriter.Position, Is.EqualTo(0)); } + + [Test] + public void SerializeClient_NotInitial_NotDirty_WritesNothing() + { + // create spawned so that isServer/isClient is set properly + CreateNetworkedAndSpawn( + out _, out NetworkIdentity serverIdentity, out SerializeTest1NetworkBehaviour serverComp1, out SerializeTest2NetworkBehaviour serverComp2, + out _, out NetworkIdentity clientIdentity, out SerializeTest1NetworkBehaviour clientComp1, out SerializeTest2NetworkBehaviour clientComp2); + + // change nothing + // clientComp.value = "42"; + + // serialize client object + serverIdentity.SerializeClient(ownerWriter); + Assert.That(ownerWriter.Position, Is.EqualTo(0)); + } + + // serialize -> deserialize. multiple components to be sure. + // one for Owner, one for Observer + // one ServerToClient, one ClientToServer + [Test] + public void SerializeAndDeserialize_ClientToServer_NOT_OWNED() + { + CreateNetworked(out GameObject _, out NetworkIdentity identity, + out SerializeTest1NetworkBehaviour comp1, + out SerializeTest2NetworkBehaviour comp2); + + // set to CLIENT with some unique values + // and set connection to server to pretend we are the owner. + identity.isOwned = false; + identity.connectionToServer = null; // NOT OWNED + comp1.syncDirection = SyncDirection.ServerToClient; + comp1.value = 12345; + comp2.syncDirection = SyncDirection.ClientToServer; + comp2.value = "67890"; + + // serialize all + identity.SerializeClient(ownerWriter); + + // shouldn't sync anything. because even though it's ClientToServer, + // we don't own this one so we shouldn't serialize & sync it. + Assert.That(ownerWriter.Position, Is.EqualTo(0)); + } + + // even for ClientToServer components, server still sends initial state. + // however, after initial it should never send anything anymore. + // otherwise for components like NetworkTransform, it would waste bandwidth. + [Test] + public void Serialize_ClientToServer_ServerOnlySendsInitial() + { + CreateNetworked(out GameObject _, out NetworkIdentity identity, + out SyncVarTest1NetworkBehaviour comp); + + // pretend to be owned + identity.isOwned = true; + comp.syncInterval = 0; + + // set to CLIENT with some unique values + // and set connection to server to pretend we are the owner. + comp.syncDirection = SyncDirection.ClientToServer; + comp.value = 12345; + + // serialize server with 'initial'. should write something + identity.SerializeServer(true, ownerWriter, observersWriter); + Assert.That(ownerWriter.Position, Is.GreaterThan(0)); + Assert.That(observersWriter.Position, Is.GreaterThan(0)); + Debug.Log("ownerWriter: " + ownerWriter.ToArray()); + Debug.Log("observerWriter: " + ownerWriter.ToArray()); + + // serialize after 'initial' shouldn't write anything. + ++comp.value; // change something + ownerWriter.Position = 0; + observersWriter.Position = 0; + identity.SerializeServer(false, ownerWriter, observersWriter); + Assert.That(ownerWriter.Position, Is.EqualTo(0)); + Assert.That(observersWriter.Position, Is.EqualTo(0)); + Debug.Log("ownerWriter: " + ownerWriter.ToArray()); + Debug.Log("observerWriter: " + ownerWriter.ToArray()); + } } } diff --git a/Assets/Mirror/Tests/Editor/NetworkIdentityTests.cs b/Assets/Mirror/Tests/Editor/NetworkIdentityTests.cs index 903c83ba9..ef82b49d1 100644 --- a/Assets/Mirror/Tests/Editor/NetworkIdentityTests.cs +++ b/Assets/Mirror/Tests/Editor/NetworkIdentityTests.cs @@ -153,6 +153,16 @@ public override void OnDeserialize(NetworkReader reader, bool initialState) } } + class SyncVarTest1NetworkBehaviour : NetworkBehaviour + { + [SyncVar] public int value; + } + + class SyncVarTest2NetworkBehaviour : NetworkBehaviour + { + [SyncVar] public string value; + } + class SerializeExceptionNetworkBehaviour : NetworkBehaviour { public override void OnSerialize(NetworkWriter writer, bool initialState) diff --git a/Assets/Mirror/Tests/Editor/SyncVarAttributeTest.cs b/Assets/Mirror/Tests/Editor/SyncVarAttributeTest.cs index a1d934590..96b00e760 100644 --- a/Assets/Mirror/Tests/Editor/SyncVarAttributeTest.cs +++ b/Assets/Mirror/Tests/Editor/SyncVarAttributeTest.cs @@ -407,14 +407,14 @@ public void TestSyncingAbstractNetworkBehaviour() NetworkWriter ownerWriter = new NetworkWriter(); // not really used in this Test NetworkWriter observersWriter = new NetworkWriter(); - serverIdentity.Serialize(true, ownerWriter, observersWriter); + serverIdentity.SerializeServer(true, ownerWriter, observersWriter); // set up a "client" object CreateNetworked(out _, out NetworkIdentity clientIdentity, out SyncVarAbstractNetworkBehaviour clientBehaviour); // apply all the data from the server object NetworkReader reader = new NetworkReader(ownerWriter.ToArray()); - clientIdentity.Deserialize(reader, true); + clientIdentity.DeserializeClient(reader, true); // check that the syncvars got updated Assert.That(clientBehaviour.monster1, Is.EqualTo(serverBehaviour.monster1), "Data should be synchronized"); diff --git a/Assets/Mirror/Tests/Runtime/NetworkIdentityTests.cs b/Assets/Mirror/Tests/Runtime/NetworkIdentityTests.cs index e92433b73..f47870427 100644 --- a/Assets/Mirror/Tests/Runtime/NetworkIdentityTests.cs +++ b/Assets/Mirror/Tests/Runtime/NetworkIdentityTests.cs @@ -66,10 +66,10 @@ public IEnumerator TestSerializationWithLargeTimestamps() // 14 * 24 hours per day * 60 minutes per hour * 60 seconds per minute = 14 days // NOTE: change this to 'float' to see the tests fail int tick = 14 * 24 * 60 * 60; - NetworkIdentitySerialization serialization = identity.GetSerializationAtTick(tick); + NetworkIdentitySerialization serialization = identity.GetServerSerializationAtTick(tick); // advance tick ++tick; - NetworkIdentitySerialization serializationNew = identity.GetSerializationAtTick(tick); + NetworkIdentitySerialization serializationNew = identity.GetServerSerializationAtTick(tick); // if the serialization has been changed the tickTimeStamp should have moved Assert.That(serialization.tick == serializationNew.tick, Is.False);