Skip to content

Commit

Permalink
Fix for duplicate OnDisconnected callbacks when ConnectionState.Close…
Browse files Browse the repository at this point in the history
…dByPeer or ConnectionState.ProblemDetectedLocally were followed by ConnectionState.None
  • Loading branch information
andererandre committed Aug 19, 2020
1 parent 4a2eb22 commit f84872b
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 22 deletions.
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
using System;
using System.Linq;
using System.Text;
using System.Collections.Generic;
using System.Threading.Tasks;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Steamworks.Data;
Expand All @@ -14,8 +13,6 @@ private class TestSocketInterface : SocketManager
{
public bool HasFinished = false;

public List<Connection> Connected = new List<Connection>();

public override void OnConnectionChanged( Connection connection, ConnectionInfo data )
{
Console.WriteLine( $"[Socket{Socket}][connection:{connection}][data.Identity:{data.Identity}] [data.State:{data.State}]" );
Expand All @@ -34,8 +31,6 @@ public override void OnConnecting( Connection connection, ConnectionInfo data )
/// </summary>
public override void OnConnected( Connection connection, ConnectionInfo data )
{
Connected.Add( connection );

Console.WriteLine( $"" );
Console.WriteLine( $"Socket -> OnConnected:" );
Console.WriteLine( $" data.Address: {data.Address}" );
Expand All @@ -57,8 +52,6 @@ public override void OnConnected( Connection connection, ConnectionInfo data )
/// </summary>
public override void OnDisconnected( Connection connection, ConnectionInfo data )
{
Connected.Remove( connection );

Console.WriteLine( $" - OnDisconnected" );

base.OnDisconnected( connection, data );
Expand Down
3 changes: 2 additions & 1 deletion Facepunch.Steamworks/Networking/Connection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,11 @@ namespace Steamworks.Data
/// You can override all the virtual functions to turn it into what you
/// want it to do.
/// </summary>
public struct Connection
public struct Connection : IEquatable<Connection>
{
public uint Id { get; set; }

public bool Equals( Connection other ) => Id == other.Id;
public override string ToString() => Id.ToString();
public static implicit operator Connection( uint value ) => new Connection() { Id = value };
public static implicit operator uint( Connection value ) => value.Id;
Expand Down
36 changes: 25 additions & 11 deletions Facepunch.Steamworks/Networking/ConnectionManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,18 +47,40 @@ public virtual void OnConnectionChanged( ConnectionInfo info )
{
ConnectionInfo = info;

//
// Some notes:
// - Update state before the callbacks, in case an exception is thrown
// - ConnectionState.None happens when a connection is destroyed, even if it was already disconnected (ClosedByPeer / ProblemDetectedLocally)
//
switch ( info.State )
{
case ConnectionState.Connecting:
OnConnecting( info );
if ( !Connecting && !Connected )
{
Connecting = true;

OnConnecting( info );
}
break;
case ConnectionState.Connected:
OnConnected( info );
if ( Connecting && !Connected )
{
Connecting = false;
Connected = true;

OnConnected( info );
}
break;
case ConnectionState.ClosedByPeer:
case ConnectionState.ProblemDetectedLocally:
case ConnectionState.None:
OnDisconnected( info );
if ( Connecting || Connected )
{
Connecting = false;
Connected = false;

OnDisconnected( info );
}
break;
}
}
Expand All @@ -69,8 +91,6 @@ public virtual void OnConnectionChanged( ConnectionInfo info )
public virtual void OnConnecting( ConnectionInfo info )
{
Interface?.OnConnecting( info );

Connecting = true;
}

/// <summary>
Expand All @@ -79,9 +99,6 @@ public virtual void OnConnecting( ConnectionInfo info )
public virtual void OnConnected( ConnectionInfo info )
{
Interface?.OnConnected( info );

Connected = true;
Connecting = false;
}

/// <summary>
Expand All @@ -90,9 +107,6 @@ public virtual void OnConnected( ConnectionInfo info )
public virtual void OnDisconnected( ConnectionInfo info )
{
Interface?.OnDisconnected( info );

Connected = false;
Connecting = false;
}

public void Receive( int bufferSize = 32 )
Expand Down
31 changes: 28 additions & 3 deletions Facepunch.Steamworks/Networking/SocketManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ public partial class SocketManager
{
public ISocketManager Interface { get; set; }

public HashSet<Connection> Connecting = new HashSet<Connection>();
public HashSet<Connection> Connected = new HashSet<Connection>();

public Socket Socket { get; internal set; }

public override string ToString() => Socket.ToString();
Expand All @@ -42,18 +45,40 @@ public bool Close()

public virtual void OnConnectionChanged( Connection connection, ConnectionInfo info )
{
//
// Some notes:
// - Update state before the callbacks, in case an exception is thrown
// - ConnectionState.None happens when a connection is destroyed, even if it was already disconnected (ClosedByPeer / ProblemDetectedLocally)
//
switch ( info.State )
{
case ConnectionState.Connecting:
OnConnecting( connection, info );
if ( !Connecting.Contains( connection ) && !Connected.Contains( connection ) )
{
Connecting.Add( connection );

OnConnecting( connection, info );
}
break;
case ConnectionState.Connected:
OnConnected( connection, info );
if ( Connecting.Contains( connection ) && !Connected.Contains( connection ) )
{
Connecting.Remove( connection );
Connected.Add( connection );

OnConnected( connection, info );
}
break;
case ConnectionState.ClosedByPeer:
case ConnectionState.ProblemDetectedLocally:
case ConnectionState.None:
OnDisconnected( connection, info );
if ( Connecting.Contains( connection ) || Connected.Contains( connection ) )
{
Connecting.Remove( connection );
Connected.Remove( connection );

OnDisconnected( connection, info );
}
break;
}
}
Expand Down

0 comments on commit f84872b

Please sign in to comment.