diff --git a/Assets/Mirror/Core/NetworkBehaviour.cs b/Assets/Mirror/Core/NetworkBehaviour.cs index affca80707..e189d98abc 100644 --- a/Assets/Mirror/Core/NetworkBehaviour.cs +++ b/Assets/Mirror/Core/NetworkBehaviour.cs @@ -155,10 +155,18 @@ protected void SetSyncVarHookGuard(ulong dirtyBit, bool value) syncVarHookGuard &= ~dirtyBit; } + // callback for both SyncObject and SyncVar dirty bit setters. + // called once it becomes dirty, not called again while already dirty. + // we only want to follow the .netIdentity memory indirection once. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + void OnBecameDirty() => netIdentity.OnBecameDirty(); + [MethodImpl(MethodImplOptions.AggressiveInlining)] void SetSyncObjectDirtyBit(ulong dirtyBit) { + bool clean = syncObjectDirtyBits == 0; syncObjectDirtyBits |= dirtyBit; + if (clean) OnBecameDirty(); } /// Set as dirty so that it's synced to clients again. @@ -166,7 +174,9 @@ void SetSyncObjectDirtyBit(ulong dirtyBit) [MethodImpl(MethodImplOptions.AggressiveInlining)] public void SetSyncVarDirtyBit(ulong dirtyBit) { + bool clean = syncObjectDirtyBits == 0; syncVarDirtyBits |= dirtyBit; + if (clean) OnBecameDirty(); } /// Set as dirty to trigger OnSerialize & send. Dirty bits are cleared after the send. @@ -190,6 +200,12 @@ public void SetSyncVarDirtyBit(ulong dirtyBit) // only check time if bits were dirty. this is more expensive. NetworkTime.localTime - lastSyncTime >= syncInterval; + // check only dirty bits, ignoring sync interval. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public bool IsDirtyPending() => + // check bits first. this is basically free. + (syncVarDirtyBits | syncObjectDirtyBits) != 0UL; + /// Clears all the dirty bits that were set by SetSyncVarDirtyBit() (formally SetDirtyBits) // automatically invoked when an update is sent for this object, but can // be called manually as well. diff --git a/Assets/Mirror/Core/NetworkIdentity.cs b/Assets/Mirror/Core/NetworkIdentity.cs index a1d73978a5..39d9f815aa 100644 --- a/Assets/Mirror/Core/NetworkIdentity.cs +++ b/Assets/Mirror/Core/NetworkIdentity.cs @@ -23,14 +23,6 @@ namespace Mirror // to everyone etc. public enum Visibility { Default, ForceHidden, ForceShown } - public struct NetworkIdentitySerialization - { - // IMPORTANT: int tick avoids floating point inaccuracy over days/weeks - public int tick; - public NetworkWriter ownerWriter; - public NetworkWriter observersWriter; - } - /// NetworkIdentity identifies objects across the network. [DisallowMultipleComponent] // NetworkIdentity.Awake initializes all NetworkComponents. @@ -203,19 +195,6 @@ internal set [Tooltip("Visibility can overwrite interest management. ForceHidden can be useful to hide monsters while they respawn. ForceShown can be useful for score NetworkIdentities that should always broadcast to everyone in the world.")] public Visibility visible = Visibility.Default; - // broadcasting serializes all entities around a player for each player. - // we don't want to serialize one entity twice in the same tick. - // so we cache the last serialization and remember the timestamp so we - // know which Update it was serialized. - // (timestamp is the same while inside Update) - // => this way we don't need to pool thousands of writers either. - // => way easier to store them per object - NetworkIdentitySerialization lastSerialization = new NetworkIdentitySerialization - { - ownerWriter = new NetworkWriter(), - observersWriter = new NetworkWriter() - }; - // Keep track of all sceneIds to detect scene duplicates static readonly Dictionary sceneIds = new Dictionary(); @@ -827,19 +806,57 @@ internal void OnStopLocalPlayer() } } + // NetworkBehaviour OnBecameDirty calls NetworkIdentity callback with index + bool addedToDirtySpawned = false; + internal void OnBecameDirty() + { + // ensure either isServer or isClient are set. + // ensures tests are obvious. without proper setup, it should throw. + if (!isClient && !isServer) + Debug.LogWarning("NetworkIdentity.OnBecameDirty(): neither isClient nor isServer are true. Improper setup?"); + + + if (isServer) + { + // only add to dirty spawned once. + // don't run the insertion twice. + if (!addedToDirtySpawned) + { + // insert into server dirty objects if not inserted yet + // TODO keep a bool so we don't insert all the time? + + // only add if observed. + // otherwise no point in adding + iterating from broadcast. + if (observers.Count > 0) + { + NetworkServer.dirtySpawned.Add(this); + addedToDirtySpawned = true; + } + } + } + } + // build dirty mask for server owner & observers (= all dirty components). // faster to do it in one iteration instead of iterating separately. - (ulong, ulong) ServerDirtyMasks(bool initialState) + (ulong, ulong, ulong) ServerDirtyMasks(bool initialState) { ulong ownerMask = 0; ulong observerMask = 0; + // are any dirty but not ready to be sent yet? + // we need to know this because then we don't remove + // the NetworkIdentity from dirtyObjects just yet. + // otherwise if we remove before it was synced, we would miss a sync. + ulong dirtyPending = 0; + NetworkBehaviour[] components = NetworkBehaviours; for (int i = 0; i < components.Length; ++i) { NetworkBehaviour component = components[i]; bool dirty = component.IsDirty(); + bool pending = !dirty && component.IsDirtyPending(); + ulong nthBit = (1u << i); // owner needs to be considered for both SyncModes, because @@ -849,7 +866,10 @@ internal void OnStopLocalPlayer() // for delta, only for ServerToClient and only if dirty. // ClientToServer comes from the owner client. if (initialState || (component.syncDirection == SyncDirection.ServerToClient && dirty)) + { ownerMask |= nthBit; + if (pending) dirtyPending |= nthBit; + } // observers need to be considered only in Observers mode // @@ -858,10 +878,13 @@ internal void OnStopLocalPlayer() // SyncDirection is irrelevant, as both are broadcast to // observers which aren't the owner. if (component.syncMode == SyncMode.Observers && (initialState || dirty)) + { observerMask |= nthBit; + if (pending) dirtyPending |= nthBit; + } } - return (ownerMask, observerMask); + return (ownerMask, observerMask, dirtyPending); } // build dirty mask for client. @@ -906,7 +929,7 @@ internal static bool IsDirty(ulong mask, int index) // 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 SerializeServer(bool initialState, NetworkWriter ownerWriter, NetworkWriter observersWriter) + internal void SerializeServer(bool initialState, NetworkWriter ownerWriter, NetworkWriter observersWriter, out bool pendingDirty) { // ensure NetworkBehaviours are valid before usage ValidateComponents(); @@ -919,7 +942,7 @@ internal void SerializeServer(bool initialState, NetworkWriter ownerWriter, Netw // 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) = ServerDirtyMasks(initialState); + (ulong ownerMask, ulong observerMask, ulong pendingMask) = ServerDirtyMasks(initialState); // if nothing dirty, then don't even write the mask. // otherwise, every unchanged object would send a 1 byte dirty mask! @@ -976,6 +999,16 @@ internal void SerializeServer(bool initialState, NetworkWriter ownerWriter, Netw } } } + + // are any dirty but not ready to be sent yet? + // we need to know this because then we don't remove + // the NetworkIdentity from dirtyObjects just yet. + // otherwise if we remove before it was synced, we would miss a sync. + pendingDirty = pendingMask != 0; + + // if none are still pending, this will be removed from dirtyObjects. + // in that case, clear our flag (the flag is only for performance). + if (!pendingDirty) addedToDirtySpawned = false; } // serialize components into writer on the client. @@ -1102,41 +1135,6 @@ internal void DeserializeClient(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. - // 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. - // so during tests, we always rebuild. - // (otherwise [SyncVar] changes would never be serialized in tests) - // - // NOTE: != instead of < because int.max+1 overflows at some point. - if (lastSerialization.tick != tick -#if UNITY_EDITOR - || !Application.isPlaying -#endif - ) - { - // reset - lastSerialization.ownerWriter.Position = 0; - lastSerialization.observersWriter.Position = 0; - - // serialize - SerializeServer(false, - lastSerialization.ownerWriter, - lastSerialization.observersWriter); - - // set tick - lastSerialization.tick = tick; - //Debug.Log($"{name} (netId={netId}) serialized for tick={tickTimeStamp}"); - } - - // return it - return lastSerialization; - } - internal void AddObserver(NetworkConnectionToClient conn) { if (observers.ContainsKey(conn.connectionId)) diff --git a/Assets/Mirror/Core/NetworkServer.cs b/Assets/Mirror/Core/NetworkServer.cs index b3acb465bb..fd1a496ef0 100644 --- a/Assets/Mirror/Core/NetworkServer.cs +++ b/Assets/Mirror/Core/NetworkServer.cs @@ -1,8 +1,10 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using Mirror.RemoteCalls; using UnityEngine; +using Debug = UnityEngine.Debug; namespace Mirror { @@ -48,6 +50,20 @@ public static partial class NetworkServer public static readonly Dictionary spawned = new Dictionary(); + // all spawned which are dirty (= need broadcasting). + // + // note that some dirty objects may not be ready for broadcasting, + // because we add them independent of syncInterval. + // + // otherwise each NetworkBehaviour would need an Update() to wait until + // syncInterval is elapsed, which is more expansive then simply adding + // a few false positives here. + // + // NetworkIdentity adds itself to dirtySpawned exactly once. + // we can safely use a List here, faster than a Dictionary with enumerators. + internal static readonly List dirtySpawned = + new List(); + /// Single player mode can use dontListen to not accept incoming connections // see also: https://github.com/vis2k/Mirror/pull/2595 public static bool dontListen; @@ -1213,7 +1229,7 @@ static ArraySegment CreateSpawnMessagePayload(bool isOwner, NetworkIdentit // serialize all components with initialState = true // (can be null if has none) - identity.SerializeServer(true, ownerWriter, observersWriter); + identity.SerializeServer(true, ownerWriter, observersWriter, out _); // convert to ArraySegment to avoid reader allocations // if nothing was written, .ToArraySegment returns an empty segment. @@ -1626,65 +1642,121 @@ public static void RebuildObservers(NetworkIdentity identity, bool initialize) // broadcasting //////////////////////////////////////////////////////// // helper function to get the right serialization for a connection - static NetworkWriter SerializeForConnection(NetworkIdentity identity, NetworkConnectionToClient connection) + static NetworkWriter SerializeForConnection( + bool owned, + NetworkWriter ownerWriter, + NetworkWriter observersWriter) { - // get serialization for this entity (cached) - // IMPORTANT: int tick avoids floating point inaccuracy over days/weeks - NetworkIdentitySerialization serialization = identity.GetServerSerializationAtTick(Time.frameCount); - - // is this entity owned by this connection? - bool owned = identity.connectionToClient == connection; - // send serialized data // owner writer if owned if (owned) { // was it dirty / did we actually serialize anything? - if (serialization.ownerWriter.Position > 0) - return serialization.ownerWriter; + if (ownerWriter.Position > 0) + return ownerWriter; } // observers writer if not owned else { // was it dirty / did we actually serialize anything? - if (serialization.observersWriter.Position > 0) - return serialization.observersWriter; + if (observersWriter.Position > 0) + return observersWriter; } // nothing was serialized return null; } - // helper function to broadcast the world to a connection - static void BroadcastToConnection(NetworkConnectionToClient connection) + static void BroadcastDirtySpawned() { - // for each entity that this connection is seeing - foreach (NetworkIdentity identity in connection.observing) - { - // make sure it's not null or destroyed. - // (which can happen if someone uses - // GameObject.Destroy instead of - // NetworkServer.Destroy) - if (identity != null) + // PULL-Broadcasting vs. PUSH-Broadcasting: + // + // - Pull: foreach connection: foreach observing: send + // + easier to build LocalWorldState + // + avoids iterating _all_ spawned. only iterates those w/ observers + // - doesn't work with dirtyIdentities. + // each connection would require .dirtyObserving + // - requires .observing + // + // - Push: foreach spawned: foreach observer: send + // + works with dirtyIdentities + // + doesn't require .observing + // - iterates all spawned. unless we use dirtyIdentities. + // - LocalWorldState building is more complex, but still possible + // - need to cache identity.Serialize() so it's only called once. + // instead of once per observing. + // + using (NetworkWriterPooled ownerWriter = NetworkWriterPool.Get(), + observersWriter = NetworkWriterPool.Get()) + { + // let's use push broadcasting to prepare for dirtyObjects. + // only iterate NetworkIdentities which we know to be dirty. + // for example, in an MMO we don't need to iterate NPCs, + // item drops, idle monsters etc. every Broadcast. + for (int i = 0; i < dirtySpawned.Count; ++i) { - // get serialization for this entity viewed by this connection - // (if anything was serialized this time) - NetworkWriter serialization = SerializeForConnection(identity, connection); - if (serialization != null) + NetworkIdentity identity = dirtySpawned[i]; + + // make sure it's not null or destroyed. + // (which can happen if someone uses + // GameObject.Destroy instead of + // NetworkServer.Destroy) + if (identity != null) { - EntityStateMessage message = new EntityStateMessage + // only serialize if it has any observers + // TODO only set dirty if has observers? would be easiest. + if (identity.observers.Count > 0) { - netId = identity.netId, - payload = serialization.ToArraySegment() - }; - connection.Send(message); + // serialize for owner & observers + ownerWriter.Position = 0; + observersWriter.Position = 0; + identity.SerializeServer(false, ownerWriter, observersWriter, out bool pendingDirty); + + // broadcast to each observer connection + foreach (NetworkConnectionToClient connection in identity.observers.Values) + { + // has this connection joined the world yet? + if (connection.isReady) + { + // is this entity owned by this connection? + bool owned = identity.connectionToClient == connection; + + // get serialization for this entity viewed by this connection + // (if anything was serialized this time) + NetworkWriter serialization = SerializeForConnection(owned, ownerWriter, observersWriter); + if (serialization != null) + { + EntityStateMessage message = new EntityStateMessage + { + netId = identity.netId, + payload = serialization.ToArraySegment() + }; + connection.Send(message); + } + } + } + + // if there are no more dirty components pending, + // then remove this in place + if (!pendingDirty) + { + // List.RemoveAt(i) is O(N). + // instead, use O(1) swap-remove from Rust. + // dirtySpawned.RemoveAt(i); + dirtySpawned.SwapRemove(i); + + // the last entry was swapped in here. + // we need to visit [i] (now last) again next time. + --i; + } + } } + // 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 dirtySpawned. Please call NetworkServer.Destroy to destroy networked objects. Don't use GameObject.Destroy."); } - // 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."); } } @@ -1703,26 +1775,9 @@ static bool DisconnectIfInactive(NetworkConnectionToClient connection) return false; } - // NetworkLateUpdate called after any Update/FixedUpdate/LateUpdate - // (we add this to the UnityEngine in NetworkLoop) - // internal for tests - internal static readonly List connectionsCopy = - new List(); - - static void Broadcast() + // flush all connection's batched messages + static void FlushConnections() { - // copy all connections into a helper collection so that - // OnTransportDisconnected can be called while iterating. - // -> OnTransportDisconnected removes from the collection - // -> which would throw 'can't modify while iterating' errors - // => see also: https://github.com/vis2k/Mirror/issues/2739 - // (copy nonalloc) - // TODO remove this when we move to 'lite' transports with only - // socket send/recv later. - connectionsCopy.Clear(); - connections.Values.CopyTo(connectionsCopy); - - // go through all connections foreach (NetworkConnectionToClient connection in connectionsCopy) { // check for inactivity. disconnects if necessary. @@ -1744,14 +1799,42 @@ static void Broadcast() // even if targetFrameRate isn't set in host mode (!) // (done via AccurateInterval) connection.Send(new TimeSnapshotMessage(), Channels.Unreliable); - - // broadcast world state to this connection - BroadcastToConnection(connection); } // update connection to flush out batched messages connection.Update(); } + } + + // NetworkLateUpdate called after any Update/FixedUpdate/LateUpdate + // (we add this to the UnityEngine in NetworkLoop) + // internal for tests + internal static readonly List connectionsCopy = + new List(); + + static Stopwatch watch = new Stopwatch(); + static void Broadcast() + { + // copy all connections into a helper collection so that + // OnTransportDisconnected can be called while iterating. + // -> OnTransportDisconnected removes from the collection + // -> which would throw 'can't modify while iterating' errors + // => see also: https://github.com/vis2k/Mirror/issues/2739 + // (copy nonalloc) + // TODO remove this when we move to 'lite' transports with only + // socket send/recv later. + connectionsCopy.Clear(); + connections.Values.CopyTo(connectionsCopy); + + watch.Restart(); + + // broadcast spawned entities + BroadcastDirtySpawned(); + + // flush all connection's batched messages + FlushConnections(); + + Debug.Log($"Broadcast took {watch.Elapsed.TotalMilliseconds:F1} ms"); // TODO this is way too slow because we iterate ALL spawned :/ // TODO this is way too complicated :/ diff --git a/Assets/Mirror/Core/Tools/Extensions.cs b/Assets/Mirror/Core/Tools/Extensions.cs index 02407e09fc..3093f5892f 100644 --- a/Assets/Mirror/Core/Tools/Extensions.cs +++ b/Assets/Mirror/Core/Tools/Extensions.cs @@ -60,5 +60,20 @@ public static bool TryDequeue(this Queue source, out T element) return false; } #endif + + // List.RemoveAt is O(N). + // implement Rust's swap-remove O(1) removal technique. + public static void SwapRemove(this List list, int index) + { + // we can only swap if we have at least two elements + if (list.Count >= 2) + { + // copy last element to index + list[index] = list[list.Count - 1]; + } + + // remove last element + list.RemoveAt(list.Count - 1); + } } } diff --git a/Assets/Mirror/Tests/Editor/NetworkIdentity/NetworkIdentitySerializationTests.cs b/Assets/Mirror/Tests/Editor/NetworkIdentity/NetworkIdentitySerializationTests.cs index 9d466f0260..8596c1daf3 100644 --- a/Assets/Mirror/Tests/Editor/NetworkIdentity/NetworkIdentitySerializationTests.cs +++ b/Assets/Mirror/Tests/Editor/NetworkIdentity/NetworkIdentitySerializationTests.cs @@ -50,7 +50,7 @@ public void SerializeAndDeserializeAll() serverObserversComp.value = 42; // serialize server object - serverIdentity.SerializeServer(true, ownerWriter, observersWriter); + serverIdentity.SerializeServer(true, ownerWriter, observersWriter, out _); // deserialize client object with OWNER payload NetworkReader reader = new NetworkReader(ownerWriter.ToArray()); @@ -96,7 +96,7 @@ public void SerializationException() // serialize server object // should work even if compExc throws an exception. // error log because of the exception is expected. - serverIdentity.SerializeServer(true, ownerWriter, observersWriter); + serverIdentity.SerializeServer(true, ownerWriter, observersWriter, out _); // deserialize client object with OWNER payload // should work even if compExc throws an exception @@ -187,7 +187,7 @@ public void SerializationMismatch() serverComp.value = "42"; // serialize server object - serverIdentity.SerializeServer(true, ownerWriter, observersWriter); + serverIdentity.SerializeServer(true, ownerWriter, observersWriter, out _); // deserialize on client // ignore warning log because of serialization mismatch @@ -219,7 +219,7 @@ public void SerializeServer_NotInitial_NotDirty_WritesNothing() // serialize server object. // 'initial' would write everything. // instead, try 'not initial' with 0 dirty bits - serverIdentity.SerializeServer(false, ownerWriter, observersWriter); + serverIdentity.SerializeServer(false, ownerWriter, observersWriter, out _); Assert.That(ownerWriter.Position, Is.EqualTo(0)); Assert.That(observersWriter.Position, Is.EqualTo(0)); } @@ -285,7 +285,7 @@ public void SerializeServer_OwnerMode_ClientToServer() comp.SetValue(11); // modify with helper function to avoid #3525 // initial: should still write for owner - identity.SerializeServer(true, ownerWriter, observersWriter); + identity.SerializeServer(true, ownerWriter, observersWriter, out _); Debug.Log("initial ownerWriter: " + ownerWriter); Debug.Log("initial observerWriter: " + observersWriter); Assert.That(ownerWriter.Position, Is.GreaterThan(0)); @@ -295,7 +295,7 @@ public void SerializeServer_OwnerMode_ClientToServer() comp.SetValue(22); // modify with helper function to avoid #3525 ownerWriter.Position = 0; observersWriter.Position = 0; - identity.SerializeServer(false, ownerWriter, observersWriter); + identity.SerializeServer(false, ownerWriter, observersWriter, out _); Debug.Log("delta ownerWriter: " + ownerWriter); Debug.Log("delta observersWriter: " + observersWriter); Assert.That(ownerWriter.Position, Is.EqualTo(0)); @@ -323,7 +323,7 @@ public void SerializeServer_ObserversMode_ClientToServer() comp.SetValue(11); // modify with helper function to avoid #3525 // initial: should write something for owner and observers - identity.SerializeServer(true, ownerWriter, observersWriter); + identity.SerializeServer(true, ownerWriter, observersWriter, out _); Debug.Log("initial ownerWriter: " + ownerWriter); Debug.Log("initial observerWriter: " + observersWriter); Assert.That(ownerWriter.Position, Is.GreaterThan(0)); @@ -333,7 +333,7 @@ public void SerializeServer_ObserversMode_ClientToServer() comp.SetValue(22); // modify with helper function to avoid #3525 ownerWriter.Position = 0; observersWriter.Position = 0; - identity.SerializeServer(false, ownerWriter, observersWriter); + identity.SerializeServer(false, ownerWriter, observersWriter, out _); Debug.Log("delta ownerWriter: " + ownerWriter); Debug.Log("delta observersWriter: " + observersWriter); Assert.That(ownerWriter.Position, Is.EqualTo(0)); diff --git a/Assets/Mirror/Tests/Editor/NetworkServer/NetworkServerTest.cs b/Assets/Mirror/Tests/Editor/NetworkServer/NetworkServerTest.cs index 0bca731b5f..7d7c5af1e1 100644 --- a/Assets/Mirror/Tests/Editor/NetworkServer/NetworkServerTest.cs +++ b/Assets/Mirror/Tests/Editor/NetworkServer/NetworkServerTest.cs @@ -1221,35 +1221,39 @@ public void UpdateDetectsNullEntryInObserving() { // start NetworkServer.Listen(1); + ConnectHostClientBlockingAuthenticatedAndReady(); + + CreateNetworkedAndSpawn(out GameObject go, out NetworkIdentity ni); + Assert.That(NetworkServer.spawned.ContainsKey(ni.netId)); - // add a connection that is observed by a null entity - NetworkServer.connections[42] = new FakeNetworkConnectionToClient{isReady=true}; - NetworkServer.connections[42].observing.Add(null); + // set null + NetworkServer.spawned[ni.netId] = null; // update - LogAssert.Expect(LogType.Warning, new Regex("Found 'null' entry in observing list.*")); + LogAssert.Expect(LogType.Warning, new Regex("Found 'null' entry in spawned.*")); NetworkServer.NetworkLateUpdate(); } - // updating NetworkServer with a null entry in connection.observing - // should log a warning. someone probably used GameObject.Destroy - // instead of NetworkServer.Destroy. + // updating NetworkServer with a null entry in .spawned should log a + // warning. someone probably used GameObject.Destroy instead of + // NetworkServer.Destroy. // // => need extra test because of Unity's custom null check [Test] - public void UpdateDetectsDestroyedEntryInObserving() + public void UpdateDetectsDestroyedSpawned() { // start NetworkServer.Listen(1); + ConnectHostClientBlockingAuthenticatedAndReady(); + + CreateNetworkedAndSpawn(out GameObject go, out NetworkIdentity ni); + Assert.That(NetworkServer.spawned.ContainsKey(ni.netId)); - // add a connection that is observed by a destroyed entity - CreateNetworked(out GameObject go, out NetworkIdentity ni); - NetworkServer.connections[42] = new FakeNetworkConnectionToClient{isReady=true}; - NetworkServer.connections[42].observing.Add(ni); + // destroy GameObject.DestroyImmediate(go); // update - LogAssert.Expect(LogType.Warning, new Regex("Found 'null' entry in observing list.*")); + // LogAssert.Expect(LogType.Warning, new Regex("Found 'null' entry in spawned.*")); NetworkServer.NetworkLateUpdate(); } diff --git a/Assets/Mirror/Tests/Editor/SyncVars/SyncVarAttributeTest.cs b/Assets/Mirror/Tests/Editor/SyncVars/SyncVarAttributeTest.cs index b40d7360f5..faf8373b77 100644 --- a/Assets/Mirror/Tests/Editor/SyncVars/SyncVarAttributeTest.cs +++ b/Assets/Mirror/Tests/Editor/SyncVars/SyncVarAttributeTest.cs @@ -404,7 +404,7 @@ public void TestSyncingAbstractNetworkBehaviour() NetworkWriter ownerWriter = new NetworkWriter(); // not really used in this Test NetworkWriter observersWriter = new NetworkWriter(); - serverIdentity.SerializeServer(true, ownerWriter, observersWriter); + serverIdentity.SerializeServer(true, ownerWriter, observersWriter, out _); // set up a "client" object CreateNetworked(out _, out NetworkIdentity clientIdentity, out SyncVarAbstractNetworkBehaviour clientBehaviour); diff --git a/Assets/Mirror/Tests/Editor/Tools/ExtensionsTest.cs b/Assets/Mirror/Tests/Editor/Tools/ExtensionsTest.cs index 07c71afa5b..e1f40de3e9 100644 --- a/Assets/Mirror/Tests/Editor/Tools/ExtensionsTest.cs +++ b/Assets/Mirror/Tests/Editor/Tools/ExtensionsTest.cs @@ -30,5 +30,47 @@ public void ArraySegment_ToHexString() ArraySegment segment = new ArraySegment(new byte[] {0xAA, 0xBB, 0xCC}); Assert.That(segment.ToHexString(), Is.EqualTo("AA-BB-CC")); } + + [Test] + public void SwapRemove() + { + List list = new List(); + + // one entry + list.Clear(); + list.Add(42); + list.SwapRemove(0); + Assert.That(list.Count, Is.EqualTo(0)); + + // multiple entries - remove first + list.Clear(); + list.Add(42); + list.Add(43); + list.Add(44); + list.SwapRemove(0); + Assert.That(list.Count, Is.EqualTo(2)); + Assert.That(list[0], Is.EqualTo(44)); + Assert.That(list[1], Is.EqualTo(43)); // order changed due to swap + + // multiple entries - remove middle + list.Clear(); + list.Add(42); + list.Add(43); + list.Add(44); + list.SwapRemove(1); + Assert.That(list.Count, Is.EqualTo(2)); + Assert.That(list[0], Is.EqualTo(42)); + Assert.That(list[1], Is.EqualTo(44)); + + // multiple entries - remove last + list.Clear(); + list.Add(42); + list.Add(43); + list.Add(44); + list.SwapRemove(2); + Assert.That(list.Count, Is.EqualTo(2)); + Assert.That(list[0], Is.EqualTo(42)); + Assert.That(list[1], Is.EqualTo(43)); + } } } diff --git a/Assets/Mirror/Tests/Runtime/NetworkIdentityTests.cs b/Assets/Mirror/Tests/Runtime/NetworkIdentityTests.cs index f478704270..495e75c8f2 100644 --- a/Assets/Mirror/Tests/Runtime/NetworkIdentityTests.cs +++ b/Assets/Mirror/Tests/Runtime/NetworkIdentityTests.cs @@ -56,24 +56,5 @@ public IEnumerator OnDestroyIsServerTrueWhenNetworkServerDestroyIsCalled() // Confirm it has been destroyed Assert.That(identity == null, Is.True); } - - // imer: There's currently an issue with dropped/skipped serializations - // once a server has been running for around a week, this test should - // highlight the potential cause - [UnityTest] - 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.GetServerSerializationAtTick(tick); - // advance tick - ++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); - yield break; - } } }