SoftwareGuy / Ignorance

Ignorance utilizes the power of ENet to provide a reliable UDP networking transport for Mirror Networking.
Other
248 stars 31 forks source link

Calling networkConnection.Disconnect() has no effect #60

Closed David548219 closed 3 years ago

David548219 commented 4 years ago

Describe the bug networkConnection.Disconnect() calls have no effect. Using the debugger revealed that connection is still present in NetworkServer.connections after Disconnect is called. I have used basic authentication component to demonstrate it, since this is how I discovered it and it is among the faster ways to set up, but this error is not limited to it. This happens on both client & server. Switching to Telepathy makes everything run smooth. NetworkManager OnServerDisconnect event is also not fired, since no actual disconnect carries out. With enabled debug both on server & client a log message is printed

Ignorance: Client disconnection acknowledged

Repro project

To Reproduce Steps to reproduce the behavior:

  1. Build Player for Windows x86_64 with Mono backend
  2. Open 2 instances of the player (One in editor to change password from inspector, one in standalone player)
  3. Launch Host/Server on one instance
  4. Connect to first instance from the second with incorrect credentials.

Expected behavior Client disconnected from server.

Actual behavior Client is stuck in the middle of the connection procedure.

Desktop

SoftwareGuy commented 4 years ago

Another bloody good bug report - nice work!

I've still got the project from #59 in my workbench folder. I'll take a look at this for you using the same repro scene.

SoftwareGuy commented 4 years ago

Looking into this now

SoftwareGuy commented 4 years ago

Replicated without Network Auth component. Going in hot.

SoftwareGuy commented 4 years ago

Okay, I messed up. This will be fixed in Ignorance 1.3.7, which I'm working on getting out the door now. Please test with 1.3.7 and let me know if you still have this problem.

David548219 commented 4 years ago

I have tested this in 2019.2.14f1. Looks like it works for NetworkConnection only server side, client still has a problem.

Here's my result after Disconnect() is called on the client: изображение NetworkClient.active returns true too.

Server side calls are behaving as expected, connections are disposed and client is disconnected gracefully.

SoftwareGuy commented 4 years ago

Hmm, is this still happening in the latest Mirror version? I'll take a look if it is.

David548219 commented 4 years ago

Still present as of Mirror v16.2.2 and Ignorance 1.3.7 on the client side. Sorry for late response. Looks like this might warrant reopening the issue? @SoftwareGuy

SoftwareGuy commented 4 years ago

Are you calling Disconnect on the HostClient connection (connection ID 0)? If you’re in HostClient mode, connection ID 0 is a special connection that is not really connected, but it is there to emulate that it is connected. If you call Disconnect on that, the client will become not ready.

Can you tell me how to reproduce this, like using the Mirror Basic Example?

David548219 commented 4 years ago

This is related to separate client instances, not host client. Sorry for not including repro steps initially, my bad, here they are:

To Reproduce Steps to reproduce the behavior:

  1. Load empty project and import Mirror v16.2.2 and Ignorance 1.3.7.
  2. Navigate to Mirror/Examples/Basic/Scenes and open scene Example
  3. Change transport from Telepathy to Ignorance Threaded
  4. Add BasicAuthenticator component (And drag it to Authenticator property of the manager) and set user and password to some values.
  5. Build player (Windows Mono x64).
  6. Change username & password values in the inspector (so that they are different from the ones in player).
  7. Start a server or host from editor.
  8. Try to connect from standalone player to editor.

Expected behavior Client gets disconnected from server because the credentials are wrong.

Actual behavior Client is stuck in the middle of the connection procedure, while on server it gets disconnected.

I use BasicAuthenticator in the example since it uses Disconnect() both server and client side and is the fastest to apply. The issue on client happens because BasicAuthenticator receives the invalid credentials message from server and tries to use Disconnect() to terminate the connection. If Disconnect() is called on server it is fine after the fix you pushed, but calling it on client still has the issue. Switching transport to Telepathy fixes the problem.

SoftwareGuy commented 4 years ago

I'm gonna have to refactor the client side of things, which means I might have to fast track Ignorance 1.4 sooner than I thought I would be doing so. At least the client portion of the code. The server side will be a fun challenge.

I tried a few work arounds and somehow got Ignorance to correctly disconnect. However, sometimes it's instantly, sometimes it's like a second after the request, sometimes it doesn't obey and it tries to send data on the connection (which causes ENet to throw a "Cannot send data" message) in a unknown state.

I'll try refactoring and going from there.

CorneliusCornbread commented 4 years ago

I think I'm having a similar issue. When calling NetworkManager.singleton.StartHost() and then called NetworkManager.singleton.StopHost() then calling NetworkManager.singleton.StartHost() again, I get an error:

Ignorance encountered a fatal exception. I'm sorry, but I gotta bail - if you believe you found a bug, please report it on the GitHub. The exception returned was: System.InvalidOperationException: Host creation call failed

I just updated to the latest version, this didn't solve the issue, I get the exact same error.

Edit: just tried it with the Telepathy transport, it works fine with telepathy. An ETA on this fix would be awesome.

SoftwareGuy commented 4 years ago

How fast are you spamming StartHost and StopHost? Rapidly doing this will be prone to failure.

I can't really give an ETA as per say but I might be able to get a development version of Ignorance 1.4 committed later this week.

CorneliusCornbread commented 4 years ago

How fast are you spamming StartHost and StopHost? Rapidly doing this will be prone to failure.

I can't really give an ETA as per say but I might be able to get a development version of Ignorance 1.4 committed later this week.

It's done through UI, so probably a second at minimum between start and stop host. I'm using Telepathy in the meantime so it's fine.

SoftwareGuy commented 3 years ago

Finally fixed in commit 041bb80 for Ignorance 1.4.0b1. Going to close this. If it's still not working for you, let me know.