-
Notifications
You must be signed in to change notification settings - Fork 449
fix: client reconnect using NetworkSceneTable state [MTT-3363] #1886
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
NoelStephensUnity
wants to merge
51
commits into
develop
from
fix/client-reconnect-using-networktablestate
Closed
fix: client reconnect using NetworkSceneTable state [MTT-3363] #1886
NoelStephensUnity
wants to merge
51
commits into
develop
from
fix/client-reconnect-using-networktablestate
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Still looking into integration tests, but this includes an update to SceneTransitioningAdditive that allows the host/server to disconnect all connected clients and the clients will automatically attempt to reconnect without having to exit SceneTransitioningAdditive and enter back into it again using the fix applied in this PR. This also provides a way to manually test clients disconnecting and unloading some of the scenes loaded prior to the client being disconnected to assure the client will always have the appropriate scenes loaded and properly synchronizes dynamically spawned and in-scene placed NetworkObjects.
…chronization-process
Updating SceneTransitioningAdditive test to use UnityTransport and making sure the scene to switch to has a disconnect client button. Updating some edge case scenarios to take into consideration when disconnecting and reconnecting a client and the object pools. Making the ClientDisconnectHandler only implement the auto flush/unload scenes no longer loaded when reconnecting.
Fixes additional exposed issues: - A very edge case, but possible, scenario where the scene to be flushed was unloaded right as the client reconnects, so just updated NetworkSceneManager to make sure the scene is valid and loaded before attempting to unload it. - When using UnityTransport and a server disconnects a client, NetworkManager was calling Shutdown() without specifying it should not try to send any outbound messages in the queue since the server-side transport no longer has a connection to the client-side transport.
Adding the ability to start just one client after it has been stopped within NetcodeIntegrationTestHelpers. Adding OnPostTearDown virtual method to NetcodeIntegrationTest in order to provide developers with a place to, on a per test basis, have something invoked just after all NetworkManagers were shutdown.
SceneEventTest: This fixes an edge case issue with SceneEventTest when listening for the received load messages where in the editor it would almost always have just received the resynchronize message when it started waiting for the load message and the found metric would be incorrect. However, when running as a stand alone testrunner build it would always have already processed the resynchronize message (on Windows) and so the resolution was to see if the message was a resynchronize message and if so create a new wait metric and listen for the expected Load message. PacketLossMetricsTests: I disabled scene management as the simulated packet dropping seems to be impacted possibly by the additional resynchronization message. Also added wait for condition checks in place of the WaitForMetricsReceived as it appears 60 frames can fail when running on Windows as a stand alone testrunner build. This is a suggested fix, but will get MTT-Tools to determine how they want to handle this (or if this is fine).
Manual Test Related Fixes This fixes some additional edge case scenarios that were exposed while testing the client connect-disconnect using NetworkSceneTable state fix for reconnecting a disconnected client. All of these changes are just to be able to manually test client disconnect-reconnect scenarios (i.e. disconnect a client and switch the scene or unload one scene and load a different scene or any other number of potential combinations using SceneTransitioningAdditive)
Fixed TrackTotalNumberOfBytesSent tracking the number of bytes received and not sent. Fixed an edge case scenario where the server sends a resynchronization message when in the middle of tracking the number of bytes sent which cause the test to fail. Disabled scene management to avoid this scenario for this test since it is really only focusing on testing that sent/received byte metrics are working.
Some additional fixes for edge case scenarios (manual test assets and code only)
This fixes the issue where a client could be disconnected. the server unload and reloads the same scene while the client is disconnected, and then the client reconnects using the NetworkTableState prior to the same scene being unloaded and reloaded. The issue was that in the GetAndAddNewlyLoadedSceneByName we needed to exclude any scenes that are in the NetworkTableState as they are the previously loaded scenes and GetAndAddNewlyLoadedSceneByName is only invoked if a scene has to be loaded and a scene only has to be reloaded if the NetworkSceneHandle doesn't already exist in the NetworkTableState.
Just added some additional debug information that I think will be useful
Added additional debug info when a scene load fails. Added the case for the same scene to be unloaded and reloaded while a client is disconnected and then the client connects. This added test verifies that the scenes in the NetworkSceneTable state are not taken into consideration when trying to find a newly loaded scene.
4 tasks
4 tasks
Closed and re-opened to fix the JIRA association. Please make sure to add the JIRA ticket to the title upon PR creation in the future. |
closing this PR but preserving the branch |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
(WIP)Refactoring: removing public facing API changes
When a client is disconnected from a network session for external reasons there is no easy way for a user to reconnect the client without first unloading all scenes loaded by the server and have the client reload all scenes.
This PR introduces some additional methods that allows a user to save off the current NetworkSceneTable state that can be applied when reconnecting in order to avoid having to reload already loaded scenes.
MTT-3363
Changelog
NetworkSceneManager.GetNetworkSceneTableState
andNetworkSceneManager.SetNetworkSceneTableState
to provide users with a way to reconnect to a network session without having to unload and reload the scenes.Testing and Documentation