breaking: NetworkClient as component #1372
Conversation
|
|
|
I was planning to remove it in another PR, but can do it in this one if you prefer |
|
please try to do it in this pr |
Lymdun
force-pushed
the
NetworkClient_Components
branch
3 times, most recently
from
d81b78d
to
26e589e
Compare
Lymdun
force-pushed
the
NetworkClient_Components
branch
from
219aba7
to
48486fc
Compare
| @@ -163,7 +163,7 @@ void SceneLoadedForPlayer(NetworkConnection conn, GameObject roomPlayer) | |||
| return; | |||
|
|
|||
| // replace room player with game player | |||
| NetworkServer.ReplacePlayerForConnection(conn, gamePlayer, true); | |||
| NetworkServer.ReplacePlayerForConnection(conn, client, gamePlayer, true); | |||
There was a problem hiding this comment.
Do we need the connection anymore?
| @@ -46,7 +46,7 @@ private void OnCreatePlayer(NetworkConnection connection, CreatePlayerMessage cr | |||
| playergo.GetComponent<Player>().playerName = createPlayerMessage.name; | |||
|
|
|||
| // set it as the player | |||
| NetworkServer.AddPlayerForConnection(connection, playergo); | |||
| NetworkServer.AddPlayerForConnection(connection, client, playergo); | |||
There was a problem hiding this comment.
Do we need the connection?
There was a problem hiding this comment.
Perhaps I'm wrong, but what I understood from my tests is that we need it for OnServerAddPlayer. If we get the NetworkConnection from client.connection, we end up with a NetworkConnectionToServer instead of a NetworkConnectionToClient.
There was a problem hiding this comment.
ohhhh, I see, and why do we need the client here? this method is supposed to be server side no?
There was a problem hiding this comment.
For some reason, it doesn't need it but only in host-mode case. Perhaps a Mirror bug?
Lymdun
force-pushed
the
NetworkClient_Components
branch
from
17a9467
to
388bbc1
Compare
| @@ -35,7 +35,7 @@ public override void OnStartServer() | |||
| public override void OnStartClient() | |||
| { | |||
| // register a handler for the authentication response we expect from server | |||
| NetworkClient.RegisterHandler<AuthResponseMessage>(OnAuthResponseMessage, false); | |||
| NetworkManager.singleton.client.RegisterHandler<AuthResponseMessage>(OnAuthResponseMessage, false); | |||
There was a problem hiding this comment.
what about the NetworkManager.singleton calls everywhere?
right now, NetworkClient is static. which imho is way better than the singleton usage here.
miwarnec
force-pushed
the
master
branch
2 times, most recently
from
2007d1e
to
a71ecdb
Compare
Lymdun
force-pushed
the
NetworkClient_Components
branch
from
388bbc1
to
8f6b6fa
Compare
|
closing as discussed in discord |
Alternative to #1341 but without singleton. Weaver updated too.
Note that this PR will allows for unit-testing.