[UTY-1613] Gracefully handle disconnect in Playground #686
Conversation
improbable-prow-robot
added
the
size/M
improbable-prow-robot
added
size/S
|
What's up with the branch target? |
| } | ||
|
|
||
| // Disable system after first run. | ||
| Enabled = false; |
There was a problem hiding this comment.
This isn't related to PR, but I think this was missing here. The code in the system isn't really intended to be ran multiple times
There was a problem hiding this comment.
this might disable the system before it is actually run depending on when the components are added to the entities
There was a problem hiding this comment.
Good point. This should be fixed if moved to the cycle above, right?
There was a problem hiding this comment.
Yeah if its in the for loop it should be good 👍
|
|
||
|
|
||
| private void OnDisconnected(string reason) | ||
| protected virtual void OnDisconnected(string reason) |
There was a problem hiding this comment.
instead of making this protected, just register to the event directly on the worker:
e.g. Worker.OnDisconnect +=ConnectionScreenController.OnDisconnected
|
@jamiebrynes7 this is temporary to exclude unrelated code in mobile-clients-fix branch until it lands. Will be switched to master then |
|
|
||
| protected override void OnDestroyManager() | ||
| { | ||
| UnityEngine.Object.Destroy(UIComponent.Main.gameObject); |
There was a problem hiding this comment.
use the UnityObjectDestroyer
There was a problem hiding this comment.
Interesting, didn't know about that one. Not related, but I am curious whey do we use DestoyImmediate in tests
gkassabli
force-pushed
the
feature/mobile-clients-fix
branch
from
2d253eb to
03c71a4
Compare
jamiebrynes7
added
T: enhancement
improbable-prow-robot
added
size/M
gkassabli
force-pushed
the
feature/playground_disconnect
branch
from
202a7c1 to
da603ef
Compare
improbable-prow-robot
added
size/S
|
Tested on an iOS device by connecting to a local device and then turning off the Wifi on the phone. Didn't seem to work |
gkassabli
force-pushed
the
feature/playground_disconnect
branch
2 times, most recently
from
a5b2923 to
0781e2f
Compare
|
Verified this works as expected with KCP on master. |
jamiebrynes7
left a comment
jamiebrynes7
left a comment
There was a problem hiding this comment.
LGTM. To check - if you try to reconnect, does everything work as normal?
jessicafalk
left a comment
jessicafalk
left a comment
There was a problem hiding this comment.
tested on my side as well
Yup, after enabling connection back reconnect works as expected |
gkassabli
force-pushed
the
feature/playground_disconnect
branch
from
32829b0 to
e2f16f0
Compare
gkassabli
force-pushed
the
feature/playground_disconnect
branch
from
e2f16f0 to
761e0ae
Compare
Description
We didn't have any specific handling for disconnect on mobile. Whenever the server was killed, mobile client worker is going to get to weird stale state where it doesn't technically hang, but nothing was happening and the client wasn't interactive.
This PR adds the code to restore Playground to "Ready to connect" mode
Tests
Run Playground locally and ensure that when server is disconnected we are ready to connect again ->
After the server is back, we should be able to reconnect
Documentation
Primary reviewers