Skip to content

Commit

Permalink
refactor: observers is now a set of connections (#74)
Browse files Browse the repository at this point in the history 
This is just a small step towards removing connection id,
but it is fully self contained

BREAKING CHANGE: observers is now a set of connections, not a dictionary
  • Loading branch information
@paulpach
paulpach committed Mar 15, 2020
1 parent 6148d6a commit 4848920
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 47 deletions.
4 changes: 2 additions & 2 deletions Assets/Mirror/Editor/NetworkInformationPreview.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,9 +157,9 @@ public override void OnPreviewGUI(Rect r, GUIStyle background)
observerRect.x += 20; // indent names
observerRect.y += observerRect.height;

foreach (KeyValuePair<int, NetworkConnection> kvp in identity.observers)
foreach ( NetworkConnection kvp in identity.observers)
{
GUI.Label(observerRect, kvp.Value.address + ":" + kvp.Value, styles.ComponentName);
GUI.Label(observerRect, kvp.address + ":" + kvp, styles.ComponentName);
observerRect.y += observerRect.height;
lastY = observerRect.y;
}
Expand Down
2 changes: 1 addition & 1 deletion Assets/Mirror/Examples/AdditiveScenes/Scripts/ShootingTankBehaviour.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ void ShootNearestPlayer()
GameObject target = null;
float distance = 100f;

foreach (NetworkConnection networkConnection in netIdentity.observers.Values)
foreach (NetworkConnection networkConnection in netIdentity.observers)
{
GameObject tempTarget = networkConnection.identity.gameObject;
float tempDistance = Vector3.Distance(tempTarget.transform.position, transform.position);
Expand Down
19 changes: 9 additions & 10 deletions Assets/Mirror/Runtime/NetworkIdentity.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public sealed class NetworkIdentity : MonoBehaviour
/// The set of network connections (players) that can see this object.
/// <para>null until OnStartServer was called. this is necessary for SendTo* to work properly in server-only mode.</para>
/// </summary>
public Dictionary<int, NetworkConnection> observers;
public HashSet<NetworkConnection> observers;

/// <summary>
/// Unique identifier for this particular object instance, used for tracking objects between networked clients and the server.
Expand Down Expand Up @@ -235,7 +235,7 @@ internal void SetClientOwner(NetworkConnection conn)
// this is used when a connection is destroyed, since the "observers" property is read-only
internal void RemoveObserverInternal(NetworkConnection conn)
{
observers?.Remove(conn.connectionId);
observers?.Remove(conn);
}

void Awake()
Expand Down Expand Up @@ -505,7 +505,7 @@ internal void OnStartServer()
}

netId = GetNextNetworkId();
observers = new Dictionary<int, NetworkConnection>();
observers = new HashSet<NetworkConnection>();

if (LogFilter.Debug) Debug.Log("OnStartServer " + this + " NetId:" + netId + " SceneId:" + sceneId);

Expand Down Expand Up @@ -888,7 +888,7 @@ internal void ClearObservers()
{
if (observers != null)
{
foreach (NetworkConnection conn in observers.Values)
foreach (NetworkConnection conn in observers)
{
conn.RemoveFromVisList(this, true);
}
Expand All @@ -904,16 +904,15 @@ internal void AddObserver(NetworkConnection conn)
return;
}

if (observers.ContainsKey(conn.connectionId))
if (observers.Contains(conn))
{
// if we try to add a connectionId that was already added, then
// we may have generated one that was already in use.
return;
}

if (LogFilter.Debug) Debug.Log("Added observer " + conn.address + " added for " + gameObject);

observers[conn.connectionId] = conn;
observers.Add(conn);
conn.AddToVisList(this);
}

Expand Down Expand Up @@ -1000,7 +999,7 @@ public void RebuildObservers(bool initialize)
// otherwise the player might not be in the world yet or anymore
if (conn != null && conn.isReady)
{
if (initialize || !observers.ContainsKey(conn.connectionId))
if (initialize || !observers.Contains(conn))
{
// new observer
conn.AddToVisList(this);
Expand All @@ -1011,7 +1010,7 @@ public void RebuildObservers(bool initialize)
}

// remove all old .observers that aren't in newObservers anymore
foreach (NetworkConnection conn in observers.Values)
foreach (NetworkConnection conn in observers)
{
if (!newObservers.Contains(conn))
{
Expand All @@ -1028,7 +1027,7 @@ public void RebuildObservers(bool initialize)
foreach (NetworkConnection conn in newObservers)
{
if (conn != null && conn.isReady)
observers.Add(conn.connectionId, conn);
observers.Add(conn);
}
}

Expand Down
20 changes: 10 additions & 10 deletions Assets/Mirror/Runtime/NetworkServer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,14 +234,14 @@ internal void ActivateHostScene()
// -> makes code more complicated, but is HIGHLY worth it to
// avoid allocations, allow for multicast, etc.
connectionIdsCache.Clear();
foreach (KeyValuePair<int, NetworkConnection> kvp in identity.observers)
foreach ( NetworkConnection kvp in identity.observers)
{
// use local connection directly because it doesn't send via transport
if (kvp.Value is ULocalConnectionToClient)
kvp.Value.Send(segment);
if (kvp is ULocalConnectionToClient)
kvp.Send(segment);
// gather all internet connections
else
connectionIdsCache.Add(kvp.Key);
connectionIdsCache.Add(kvp.connectionId);
}

// send to all internet connections at once
Expand Down Expand Up @@ -331,19 +331,19 @@ internal void ActivateHostScene()
connectionIdsCache.Clear();
bool result = true;
int count = 0;
foreach (KeyValuePair<int, NetworkConnection> kvp in identity.observers)
foreach ( NetworkConnection kvp in identity.observers)
{
bool isOwner = kvp.Value == identity.connectionToClient;
if ((!isOwner || includeOwner) && kvp.Value.isReady)
bool isOwner = kvp == identity.connectionToClient;
if ((!isOwner || includeOwner) && kvp.isReady)
{
count++;

// use local connection directly because it doesn't send via transport
if (kvp.Value is ULocalConnectionToClient)
result &= kvp.Value.Send(segment);
if (kvp is ULocalConnectionToClient)
result &= kvp.Send(segment);
// gather all internet connections
else
connectionIdsCache.Add(kvp.Key);
connectionIdsCache.Add(kvp.connectionId);
}
}

Expand Down
39 changes: 15 additions & 24 deletions Assets/Mirror/Tests/Editor/NetworkIdentityTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ public void RemoveObserverInternal()

// add an observer connection
var connection = new NetworkConnectionToClient(42);
identity.observers[connection.connectionId] = connection;
identity.observers.Add(connection);

// RemoveObserverInternal with invalid connection should do nothing
identity.RemoveObserverInternal(new NetworkConnectionToClient(43));
Expand Down Expand Up @@ -835,20 +835,11 @@ public void AddObserver()
// call AddObservers
identity.AddObserver(connection1);
identity.AddObserver(connection2);
Assert.That(identity.observers.Count, Is.EqualTo(2));
Assert.That(identity.observers.ContainsKey(connection1.connectionId));
Assert.That(identity.observers[connection1.connectionId], Is.EqualTo(connection1));
Assert.That(identity.observers.ContainsKey(connection2.connectionId));
Assert.That(identity.observers[connection2.connectionId], Is.EqualTo(connection2));
Assert.That(identity.observers, Is.EquivalentTo(new[] { connection1, connection2 }));

// adding a duplicate connectionId shouldn't overwrite the original
var duplicate = new NetworkConnectionToClient(connection1.connectionId);
identity.AddObserver(duplicate);
Assert.That(identity.observers.Count, Is.EqualTo(2));
Assert.That(identity.observers.ContainsKey(connection1.connectionId));
Assert.That(identity.observers[connection1.connectionId], Is.EqualTo(connection1));
Assert.That(identity.observers.ContainsKey(connection2.connectionId));
Assert.That(identity.observers[connection2.connectionId], Is.EqualTo(connection2));
identity.AddObserver(connection1);
Assert.That(identity.observers, Is.EquivalentTo(new[] { connection1, connection2 }));
}

[Test]
Expand All @@ -858,8 +849,8 @@ public void ClearObservers()
identity.OnStartServer();

// add some observers
identity.observers[42] = new NetworkConnectionToClient(42);
identity.observers[43] = new NetworkConnectionToClient(43);
identity.observers.Add(new NetworkConnectionToClient(42));
identity.observers.Add(new NetworkConnectionToClient(43));

// call ClearObservers
identity.ClearObservers();
Expand All @@ -875,7 +866,7 @@ public void Reset()
uint netId = identity.netId;
identity.connectionToClient = new NetworkConnectionToClient(1);
identity.connectionToServer = new NetworkConnectionToServer();
identity.observers[43] = new NetworkConnectionToClient(2);
identity.observers.Add( new NetworkConnectionToClient(2));

// calling reset shouldn't do anything unless it was marked for reset
identity.Reset();
Expand Down Expand Up @@ -946,7 +937,7 @@ public void ServerUpdate()
{
{ MessagePacker.GetId<UpdateVarsMessage>(), (conn, msg, channel) => ++observerCalled }
});
identity.observers[observer.connectionId] = observer;
identity.observers.Add(observer);

// set components dirty again
compA.SetDirtyBit(ulong.MaxValue);
Expand Down Expand Up @@ -1006,9 +997,11 @@ public void GetNewObserversFalseIfNoComponents()
[Test]
public void AddAllReadyServerConnectionsToObservers()
{
var connection1 = new NetworkConnectionToClient(12) { isReady = true };
var connection2 = new NetworkConnectionToClient(13) { isReady = false };
// add some server connections
server.connections[12] = new NetworkConnectionToClient(12){isReady = true};
server.connections[13] = new NetworkConnectionToClient(13){isReady = false};
server.connections[12] = connection1;
server.connections[13] = connection2;

// add a host connection
var localConnection = new ULocalConnectionToClient
Expand All @@ -1023,9 +1016,7 @@ public void AddAllReadyServerConnectionsToObservers()

// add all to observers. should have the two ready connections then.
identity.AddAllReadyServerConnectionsToObservers();
Assert.That(identity.observers.Count, Is.EqualTo(2));
Assert.That(identity.observers.ContainsKey(12));
Assert.That(identity.observers.ContainsKey(server.localConnection.connectionId));
Assert.That(identity.observers, Is.EquivalentTo(new [] { connection1, localConnection }));

// clean up
server.RemoveLocalConnection();
Expand All @@ -1052,7 +1043,7 @@ public void RebuildObserversAddsOwnReadyPlayer()

// rebuild should at least add own ready player
identity.RebuildObservers(true);
Assert.That(identity.observers.ContainsKey(identity.connectionToClient.connectionId));
Assert.That(identity.observers, Does.Contain(identity.connectionToClient));
}

// RebuildObservers should always add the own ready connection
Expand All @@ -1074,7 +1065,7 @@ public void RebuildObserversOnlyAddsOwnPlayerIfReady()

// rebuild shouldn't add own player because conn wasn't set ready
identity.RebuildObservers(true);
Assert.That(!identity.observers.ContainsKey(identity.connectionToClient.connectionId));
Assert.That(identity.observers, Does.Not.Contains(identity.connectionToClient));
}

}
Expand Down

0 comments on commit 4848920

Please sign in to comment.