From f84872b38c2a14a2abf974fc47314016fe120ba3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Straubmeier?= Date: Wed, 19 Aug 2020 18:27:09 +0200 Subject: [PATCH] Fix for duplicate OnDisconnected callbacks when ConnectionState.ClosedByPeer or ConnectionState.ProblemDetectedLocally were followed by ConnectionState.None --- ...tworkingSocketsTest.TestSocketInterface.cs | 7 ---- Facepunch.Steamworks/Networking/Connection.cs | 3 +- .../Networking/ConnectionManager.cs | 36 +++++++++++++------ .../Networking/SocketManager.cs | 31 ++++++++++++++-- 4 files changed, 55 insertions(+), 22 deletions(-) diff --git a/Facepunch.Steamworks.Test/NetworkingSocketsTest.TestSocketInterface.cs b/Facepunch.Steamworks.Test/NetworkingSocketsTest.TestSocketInterface.cs index 39b65da..aee056f 100644 --- a/Facepunch.Steamworks.Test/NetworkingSocketsTest.TestSocketInterface.cs +++ b/Facepunch.Steamworks.Test/NetworkingSocketsTest.TestSocketInterface.cs @@ -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; @@ -14,8 +13,6 @@ namespace Steamworks { public bool HasFinished = false; - public List Connected = new List(); - public override void OnConnectionChanged( Connection connection, ConnectionInfo data ) { Console.WriteLine( $"[Socket{Socket}][connection:{connection}][data.Identity:{data.Identity}] [data.State:{data.State}]" ); @@ -34,8 +31,6 @@ namespace Steamworks /// public override void OnConnected( Connection connection, ConnectionInfo data ) { - Connected.Add( connection ); - Console.WriteLine( $"" ); Console.WriteLine( $"Socket -> OnConnected:" ); Console.WriteLine( $" data.Address: {data.Address}" ); @@ -57,8 +52,6 @@ namespace Steamworks /// public override void OnDisconnected( Connection connection, ConnectionInfo data ) { - Connected.Remove( connection ); - Console.WriteLine( $" - OnDisconnected" ); base.OnDisconnected( connection, data ); diff --git a/Facepunch.Steamworks/Networking/Connection.cs b/Facepunch.Steamworks/Networking/Connection.cs index f5f75e5..439f6cd 100644 --- a/Facepunch.Steamworks/Networking/Connection.cs +++ b/Facepunch.Steamworks/Networking/Connection.cs @@ -11,10 +11,11 @@ namespace Steamworks.Data /// You can override all the virtual functions to turn it into what you /// want it to do. /// - public struct Connection + public struct Connection : IEquatable { 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; diff --git a/Facepunch.Steamworks/Networking/ConnectionManager.cs b/Facepunch.Steamworks/Networking/ConnectionManager.cs index 86a194a..f12fb2b 100644 --- a/Facepunch.Steamworks/Networking/ConnectionManager.cs +++ b/Facepunch.Steamworks/Networking/ConnectionManager.cs @@ -47,18 +47,40 @@ namespace Steamworks { 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; } } @@ -69,8 +91,6 @@ namespace Steamworks public virtual void OnConnecting( ConnectionInfo info ) { Interface?.OnConnecting( info ); - - Connecting = true; } /// @@ -79,9 +99,6 @@ namespace Steamworks public virtual void OnConnected( ConnectionInfo info ) { Interface?.OnConnected( info ); - - Connected = true; - Connecting = false; } /// @@ -90,9 +107,6 @@ namespace Steamworks public virtual void OnDisconnected( ConnectionInfo info ) { Interface?.OnDisconnected( info ); - - Connected = false; - Connecting = false; } public void Receive( int bufferSize = 32 ) diff --git a/Facepunch.Steamworks/Networking/SocketManager.cs b/Facepunch.Steamworks/Networking/SocketManager.cs index 0c4a3ad..b09e6be 100644 --- a/Facepunch.Steamworks/Networking/SocketManager.cs +++ b/Facepunch.Steamworks/Networking/SocketManager.cs @@ -16,6 +16,9 @@ namespace Steamworks { public ISocketManager Interface { get; set; } + public HashSet Connecting = new HashSet(); + public HashSet Connected = new HashSet(); + public Socket Socket { get; internal set; } public override string ToString() => Socket.ToString(); @@ -42,18 +45,40 @@ namespace Steamworks 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; } }