fix: fix multiple scene loading order issues #418
Merged
Conversation
This file contains 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
paulpach
reviewed
Oct 20, 2020
There was a problem hiding this comment.
After: On connect the server will ALWAYS tell you the NetworkSceneName (returns SceneManager.GetActiveScene().name) every time.
Use .path instead of .name. We usually have lots of scenes with the same name in different paths.
After: PlayerSpawner uses ClientSceneChanged (Normal scene change only) to AddNewPlayerMessage.
What happens if I only have one scene for everything (like in basic example)?
uweeby
force-pushed
the
fixSceneLoadingOrder
branch
from
7d3563f
to
5bc88b3
Compare
|
SonarCloud Quality Gate failed.
|
uweeby
changed the title
github-actions bot
pushed a commit
that referenced
this pull request
Oct 21, 2020
## [51.1.2](v51.1.1...v51.1.2) (2020-10-21) ### Bug Fixes * fix multiple scene loading order issues ([#418](#418)) ([6d8265d](6d8265d))
|
🎉 This PR is included in version 51.1.2 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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.
This PR fixes a handful of related timing issues with scene loading. I tried to break this up in other smaller PRs but this chunk is too tangled. The intent is not to change functionality or flow but instead fix bugs or oversights in the existing code.
Before: FinishLoadScene would fire before scene change actually completed
After: hook AsyncOperation.Complete to wait for completion.
Reason for change: this was the original inspiration for the change Unity Developer Forum Post. After some testing I confirmed that the UnityEvents for Post scene change were firing before the scene change completed in some scenarios. This was hidden by the IsReady code.
Before: On connect the server will only tell you the NetworkSceneName if the server had changed scene since it started.
After: On connect the server will ALWAYS tell you the NetworkSceneName (returns SceneManager.GetActiveScene().name) every time.
Reason for change: this prevent ambiguous situations where the client and server could start in different scenes.
Before: HostClient would not register any Scene related msg handlers.
After: HostClient registers Scene change msg handler.
Reason for change: this sends all scene changes through the same code paths. Eventually results in less redundant calls and host/client checks.
Before: PlayerSpawner used Authenticated to AddNewPlayerMessage.
After: PlayerSpawner uses ClientSceneChanged (Normal scene change only) to AddNewPlayerMessage.
Reason for change: The player will need to be spawned after a normal scene change happens (as its detroyed in that process since its not DDOL). There is no reason to couple that action to Authenticated as that may have unintended side effects. Also helps remove code from NetworkSceneManager as the existing event can handle this situation instead.