`ConnectedPlayer#disconnected` called even when connection fails due to duplicate connection

[WouterGritter]

Expected Behavior

This method should'nt be called.

Actual Behavior

This method does get called, possibly resulting in undesired VelocityBossBarImplementation#viewerDisconnected calls.
For reference, the entire ConnectedPlayer#disconnected method:

public void disconnected() {
  for (final VelocityBossBarImplementation bar : this.bossBars) {
    bar.viewerDisconnected(this);
  }
}

Steps to Reproduce

Add a printline/log statement in ConnectedPlayer#disconnected, join the proxy with minecraft account A, try to re-join the proxy with the same account.
The second connection will fail, and ConnectedPlayer#disconnected still gets called.

Plugin List

n/a

Velocity Version

Compiled from latest commit e0db256 + the log statement in disconnected().

Additional Information

This currently isn't an issue, as bossbars won't be registered to the ConnectedPlayer. However if more logic ever gets added to this method this might be an issue.

My proposed fix is to gate the connection.disconnected() call in VelocityServer#unregisterConnection:

public void unregisterConnection(ConnectedPlayer connection) {
  connectionsByName.remove(connection.getUsername().toLowerCase(Locale.US), connection);
  connectionsByUuid.remove(connection.getUniqueId(), connection);
  connection.disconnected(); // Only call when actually removed from the map(s).
}

However, this may result in unwanted behavior due to VelocityBossBarImplementation storing a Set<ConnectedPlayer> viewers. If a player is added to this list while they haven't been properly registered in VelocityServer yet, this will cause a memory leak. It could be worth it to make sure we only add viewers to bossbars when they've been properly registered (I don't think that's the case here though, because that would mean bossbar packets are being sent before the ServerLoginSuccessPacket, but this is untested).