Skip to content

Commit

Permalink
cleanup: move observer HashSet rebuild logic to IM (#3383)
Browse files Browse the repository at this point in the history
This cleanup step prepares for InterestManagement classes being able to customize the rebuild logic, but for that we should move it out of the NetworkServer class
  • Loading branch information
imerr authored Feb 16, 2023
1 parent e0e2626 commit c2d8343
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 104 deletions.
100 changes: 100 additions & 0 deletions Assets/Mirror/Core/InterestManagement.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// interest management component for custom solutions like
// distance based, spatial hashing, raycast based, etc.

using System.Collections.Generic;
using UnityEngine;

Expand All @@ -9,6 +10,10 @@ namespace Mirror
[HelpURL("https://mirror-networking.gitbook.io/docs/guides/interest-management")]
public abstract class InterestManagement : MonoBehaviour
{
// allocate newObservers helper HashSet
readonly HashSet<NetworkConnectionToClient> newObservers =
new HashSet<NetworkConnectionToClient>();

// Awake configures InterestManagement in NetworkServer/Client
// Do NOT check for active server or client here.
// Awake must always set the static aoi references.
Expand Down Expand Up @@ -96,5 +101,100 @@ public virtual void OnSpawned(NetworkIdentity identity) {}
// (useful for 'only rebuild if changed' interest management algorithms)
[ServerCallback]
public virtual void OnDestroyed(NetworkIdentity identity) {}

public void Rebuild(NetworkIdentity identity, bool initialize)
{
// clear newObservers hashset before using it
newObservers.Clear();

// not force hidden?
if (identity.visible != Visibility.ForceHidden)
{
OnRebuildObservers(identity, newObservers);
}

// IMPORTANT: AFTER rebuilding add own player connection in any case
// to ensure player always sees himself no matter what.
// -> OnRebuildObservers might clear observers, so we need to add
// the player's own connection AFTER. 100% fail safe.
// -> fixes https://github.com/vis2k/Mirror/issues/692 where a
// player might teleport out of the ProximityChecker's cast,
// losing the own connection as observer.
if (identity.connectionToClient != null)
{
newObservers.Add(identity.connectionToClient);
}

bool changed = false;

// add all newObservers that aren't in .observers yet
foreach (NetworkConnectionToClient conn in newObservers)
{
// only add ready connections.
// otherwise the player might not be in the world yet or anymore
if (conn != null && conn.isReady)
{
if (initialize || !identity.observers.ContainsKey(conn.connectionId))
{
// new observer
conn.AddToObserving(identity);
// Debug.Log($"New Observer for {gameObject} {conn}");
changed = true;
}
}
}

// remove all old .observers that aren't in newObservers anymore
foreach (NetworkConnectionToClient conn in identity.observers.Values)
{
if (!newObservers.Contains(conn))
{
// removed observer
conn.RemoveFromObserving(identity, false);
// Debug.Log($"Removed Observer for {gameObject} {conn}");
changed = true;
}
}

// copy new observers to observers
if (changed)
{
identity.observers.Clear();
foreach (NetworkConnectionToClient conn in newObservers)
{
if (conn != null && conn.isReady)
identity.observers.Add(conn.connectionId, conn);
}
}

// special case for host mode: we use SetHostVisibility to hide
// NetworkIdentities that aren't in observer range from host.
// this is what games like Dota/Counter-Strike do too, where a host
// does NOT see all players by default. they are in memory, but
// hidden to the host player.
//
// this code is from UNET, it's a bit strange but it works:
// * it hides newly connected identities in host mode
// => that part was the intended behaviour
// * it hides ALL NetworkIdentities in host mode when the host
// connects but hasn't selected a character yet
// => this only works because we have no .localConnection != null
// check. at this stage, localConnection is null because
// StartHost starts the server first, then calls this code,
// then starts the client and sets .localConnection. so we can
// NOT add a null check without breaking host visibility here.
// * it hides ALL NetworkIdentities in server-only mode because
// observers never contain the 'null' .localConnection
// => that was not intended, but let's keep it as it is so we
// don't break anything in host mode. it's way easier than
// iterating all identities in a special function in StartHost.
if (initialize)
{
if (!newObservers.Contains(NetworkServer.localConnection))
{
SetHostVisibility(identity, false);
}
}
}
}
}
105 changes: 2 additions & 103 deletions Assets/Mirror/Core/NetworkServer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,6 @@ public static void Shutdown()
connections.Clear();
connectionsCopy.Clear();
handlers.Clear();
newObservers.Clear();

// destroy all spawned objects, _then_ set inactive.
// make sure .active is still true before calling this.
Expand Down Expand Up @@ -1531,14 +1530,10 @@ static void DestroyObject(NetworkIdentity identity, DestroyMode mode)
}

// interest management /////////////////////////////////////////////////

// Helper function to add all server connections as observers.
// This is used if none of the components provides their own
// OnRebuildObservers function.
// allocate newObservers helper HashSet only once
// internal for tests
internal static readonly HashSet<NetworkConnectionToClient> newObservers =
new HashSet<NetworkConnectionToClient>();

// rebuild observers default method (no AOI) - adds all connections
static void RebuildObserversDefault(NetworkIdentity identity, bool initialize)
{
Expand Down Expand Up @@ -1597,106 +1592,10 @@ public static void RebuildObservers(NetworkIdentity identity, bool initialize)
// otherwise let interest management system rebuild
else
{
RebuildObserversCustom(identity, initialize);
aoi.Rebuild(identity, initialize);
}
}

// rebuild observers via interest management system
static void RebuildObserversCustom(NetworkIdentity identity, bool initialize)
{
// clear newObservers hashset before using it
newObservers.Clear();

// not force hidden?
if (identity.visible != Visibility.ForceHidden)
{
aoi.OnRebuildObservers(identity, newObservers);
}

// IMPORTANT: AFTER rebuilding add own player connection in any case
// to ensure player always sees himself no matter what.
// -> OnRebuildObservers might clear observers, so we need to add
// the player's own connection AFTER. 100% fail safe.
// -> fixes https://github.com/vis2k/Mirror/issues/692 where a
// player might teleport out of the ProximityChecker's cast,
// losing the own connection as observer.
if (identity.connectionToClient != null)
{
newObservers.Add(identity.connectionToClient);
}

bool changed = false;

// add all newObservers that aren't in .observers yet
foreach (NetworkConnectionToClient conn in newObservers)
{
// only add ready connections.
// otherwise the player might not be in the world yet or anymore
if (conn != null && conn.isReady)
{
if (initialize || !identity.observers.ContainsKey(conn.connectionId))
{
// new observer
conn.AddToObserving(identity);
// Debug.Log($"New Observer for {gameObject} {conn}");
changed = true;
}
}
}

// remove all old .observers that aren't in newObservers anymore
foreach (NetworkConnectionToClient conn in identity.observers.Values)
{
if (!newObservers.Contains(conn))
{
// removed observer
conn.RemoveFromObserving(identity, false);
// Debug.Log($"Removed Observer for {gameObject} {conn}");
changed = true;
}
}

// copy new observers to observers
if (changed)
{
identity.observers.Clear();
foreach (NetworkConnectionToClient conn in newObservers)
{
if (conn != null && conn.isReady)
identity.observers.Add(conn.connectionId, conn);
}
}

// special case for host mode: we use SetHostVisibility to hide
// NetworkIdentities that aren't in observer range from host.
// this is what games like Dota/Counter-Strike do too, where a host
// does NOT see all players by default. they are in memory, but
// hidden to the host player.
//
// this code is from UNET, it's a bit strange but it works:
// * it hides newly connected identities in host mode
// => that part was the intended behaviour
// * it hides ALL NetworkIdentities in host mode when the host
// connects but hasn't selected a character yet
// => this only works because we have no .localConnection != null
// check. at this stage, localConnection is null because
// StartHost starts the server first, then calls this code,
// then starts the client and sets .localConnection. so we can
// NOT add a null check without breaking host visibility here.
// * it hides ALL NetworkIdentities in server-only mode because
// observers never contain the 'null' .localConnection
// => that was not intended, but let's keep it as it is so we
// don't break anything in host mode. it's way easier than
// iterating all identities in a special function in StartHost.
if (initialize)
{
if (!newObservers.Contains(localConnection))
{
if (aoi != null)
aoi.SetHostVisibility(identity, false);
}
}
}

// broadcasting ////////////////////////////////////////////////////////
// helper function to get the right serialization for a connection
Expand Down
1 change: 0 additions & 1 deletion Assets/Mirror/Tests/Editor/NetworkServerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1155,7 +1155,6 @@ public void ShutdownCleanup()
Assert.That(NetworkServer.connections.Count, Is.EqualTo(0));
Assert.That(NetworkServer.connectionsCopy.Count, Is.EqualTo(0));
Assert.That(NetworkServer.handlers.Count, Is.EqualTo(0));
Assert.That(NetworkServer.newObservers.Count, Is.EqualTo(0));
Assert.That(NetworkServer.spawned.Count, Is.EqualTo(0));

Assert.That(NetworkServer.localConnection, Is.Null);
Expand Down

0 comments on commit c2d8343

Please sign in to comment.