test: additive scene loading #1030

NoelStephensUnity · 2021-08-06T22:50:38Z

This is the test for PR-955 --> Please Review that first. :)
(this will be much easier to review once PR-955 is merged)

This includes the unit testing for additive scene loading using multi-instances.
It includes a minor adjustment to NetworkSceneManager to handle unit testing with multi-instances.
This removes the legacy Runtime NetworkSceneManagerTest and migrates the check for assert when EnableSceneManagement is set to false into the new NetworkSceneManagerTests.

Current Tests:

1 host and 9 clients loading a scene additively and then unloading it in unit test mode. This tests the scene event messaging and local notification pipelines for loading and unloading additive scenes. (see NetworkSceneManagerTests)
Tests the various exception cases to assure if EnableSceneManagement is not set an exception is thrown or if a client attempts to start a load or unload scene event an exception is thrown
Verifies that a scene event cannot be started during an already in progress scene event
Verifies that trying to unload a scene not already loaded returns the appropriate failed unload status
Verifies that trying to load a scene name for a scene that does not exist (is not registered) returns the appropriate failed to start scene event status.

This is a quick and dirty additive scene loading implementation example with minimal changes to the develop branch.

Updating basic scene registration to accept SceneAssets as opposed to string based names.

Removing the automatic addition of a scene asset to NetworkConfig. This has been a problematic approach to assuring the scene containing the NetworkManager is included.

Adjusting scene related commands. Keeping the original commands in place, but providing two new commands: SCENE_EVENT: This will contain sub-commands and additional information specific to the command. SCENE_NOTIFICATION: This is more for providing a mechanism to send various notifications (i.e. scene switch complete, scene loaded, scene unloaded, errors, etc) with the ability to include additional information about the event.

Added new SCENE_EVENT that will handle "all things scene relative" with the only exception of the approval process. That will need to be handled in a different PR as that requires a bunch of additional changes. Focus for this branch is to get additive scene loading and unloading working.

Created the LoadScene method which will only load scenes additively. Broke up some of the commonly shared initializations and checks. This helps to show what the real differences between single and additive scene loading are. (like two method calls)

Just cleaning up the code, putting place holder notes, and adding some comments

Just comment and style adjustments

Added additive scene unloading into the mix. Currently no events are wired for additive loading complete or additive unloading complete, but console logging on the server side verifies that the client sends the appropriate scene event notification once finished loading or unloading. Cleaned up some of the test project code to make sure the client isn't creating a pool to make sure the client is only replicating what the server is telling it to as well as various other minor test project tweaks for additive scene loading demonstration purposes.

Swapped out the switch scene cycle hack for actual toggle buttons that allow one to load each sample additive scene or unload it based on the state of the toggle button.

This includes new assets to demonstrate additive scene loading and scene switching. Known issues are when additive scenes have been loaded that they are not synchronized with clients when they join.

A wip commit that is progress towards removing scene and object synchronization from the approval process.

naming issue and making sure we scope the pooled network reader.

…n-additivescenes

Closer to decoupling approval from the approval process. There are still some issues with regards to the active scene and this could potentially be fixed by doing client synchronization first and then finalizing the approval. Fixed an issue with NetworkManager replicating upon exiting.

Joining with additive scenes already loaded as well as late joining players are synchronizing properly. The handle approval process is no longer directly tied to scene loading. Minor updates to stats display to show the currently active scene

Removed static properties in NetworkSceneManager that were no longer being used. Fixed issue with host not having the proper network prefab override (i.e. running as host vs running as server). Moved SceneEventData into its own .cs file. Excluding the local notification events, this is very close to being finalized, with the exception of custom prefab overrides and additive scene loading. This still has issues with late joining (most likely this has to do with the NetworkPrefabPool code base, still investigating this issue).

Improved the NetworkPrefabPool and NetworkPrefabPoolAdditive implementations to properly handle the various potential use case scenarios. These changes assure that both the client and the server leverage from the pool. This also takes into account NetworkManager defined NetworkPrefab overrides. Added temporary LoadSceneEvent callback that notifies if an additive scene is being loaded or unloaded. Updated the NetworkPrefabPoolAdditive sample to demonstrate how a user can assure their NetworkObjects are "spawned" in the same additive scene as their spawn generator. This pattern will always destroy all NetworkObjects if the additive scene is unloaded. Added sorting of NetworkObjects just prior to the server side serializing them in order to assure INetworkPrefabInstanceHandler implementations are registered first, this is required in order to assure that the handler is registered before any NetworkObjects associated with it are spawned.

Some temporary fixes for message ordering and buffering during late-join until this gets merged with the message ordering updates and we add the ability to buffer messages while a player is late-joining. Primarily in the GenericNetworkObjectBehaviour class. Added a proposed solution to handle order of operations if user decides to allow all NetworkObjects from a prefab pool to be spawned in the active scene. This requires the ability to separate NetworkObjects (server side) by associated scene so that they will be synchronized properly with their spawn generator's pool. Now tracking which additive scenes have been loaded to be assured they are unloaded when doing a full scene switch (i.e. singlemode loading), otherwise there can be issues that arise with currently loaded additive scenes.

Minor checks for an object still being around during clean up.

Fixes the basic scene transitioning test failure.

Fixing other unit test issues. Removing the temporary block for clients to receive messages (it was an artifact from testing). Still have a few tests that are failing.

Two issues were discovered while getting the parenting test working: 1.) t was doing a move objects to scene during the client side synchronization process (which doesn't need to happen) and as such was setting all parents to null which was throwing an exception. 2.) The server side wasn't filtering out any pre-loaded scenes during the synchronization process.

Adding missing scene assets for the multiprocess test. Making sure the SceneLoading test destroys the network manager instance.

PascalCased my enums and added event in front to avoid any NDA complications for the time being.

Minor test project fixes and additional tests for the AdditiveSceneToggleHandler to test back to back scene loading during a scene transition. Removed the distinguishing difference between switchscene and loadscene, left the switchscene as a wrapper around loadscene. Updated existing profiling code base to remove legacy scene manager profiler checks.

…iveSceneLoading

Removing Scene Loading Test: This test needs to be completely refactored and no longer is valid. WIP

Removes returning bool to determine if NetworkPrefabHandler should destroy an object with Fatih's awesome fixes from PR-1068 #1068

…eneLoading

This resolves the issue where disabling scene management would result in no NetworkObjectSynchronization of client relative observed NetworkObjects. This also further separates NetworkObject serialization even further from the NetworkSceneManager and is contained either in: NetworkSpawnManager, SceneEventData, and NetworkObject itself. This will help in future efforts to further unify this process with some form of "NetworkObjectSynchronization" system that ties together NetworkSceneManager, Snapshot, and NetworkSpawnManager related client/server NetworkObject synchronization/registration related tasks into a single management system that should work independently of other systems (at least something in that general direction).

Removing the need to set NetworkSceneManager.IsTesting to true. Now NetworkSceneManager automatically detects if a unit test is running.

------------------------------------------------------------- After further consideration, the best idea for the whole unit test issue is to: 1.) Not try to detect it ( silly idea of mine ) 2,) Make changes to the NetworkSceneManager's private m_ScenesLoaded property by renaming it to ScenesLoaded and making it internal. 3.) Update the one Unit test that was failing to populate ScenesLoaded and ServerSceneHandleToClientSceneHandle tables so the SetTheSceneBeingSynchronized method could properly be tested during the NetworkObjectSceneSerializationFailure test. It turns out my brain decided to take a "(dis)connect" day a few days too early. No need to check for unit tests at all for this PR. ------------------------------------------------------------- This also includes a minor adjustment for when the NetworkPrefabPool (test project script) unsubscribes from the NetworkSceneManager.OnSceneEvent to avoid a null access exception.

Removing the check for a null SpawnManager in NetworkObject.Despawn as this no longer seems to be an issue after the more recent merges in develop.

Took Fatih's suggestion regarding ScenesLoaded being over-complicated and now the ScenesLoaded is just a dictionary of scene handle keys and each key entry value is the Scene it is associated with. This did reduce some of the complexity in the unloading methods and the GetAndAddNewlyLoadedSceneByName method. Updated the NetworkObjectSceneSerializationFailure unit test to reflect the above changes.

…g' into test/AdditiveSceneLoading

Noticed one minor portion of the client loading process that really shouldn't be outside of SceneEventData. Removed 1 method call from NetworkSceneManager and 1 method from SceneEventData.

…eneLoading

In order to validate scene loading for (n) clients and a server/host, I did add an automated way to check if a unit test is running (for now) in order to assure clients don't try to load a single mode scene. Also merged the recent changes to the feat/MTT-820-AdditiveSceneLoading branch, and made some adjustments to those changes in NetworkSceneManagerTests.

Removed namespace no longer being used. (Standards check failed)

…eneLoading

Added the suggested || DEVELOPMENT_BUILD to the unit test check and adjusted the 3 locations to mirror this addition. As well, added comments to explain why we are doing this here.

0xFA11 · 2021-08-23T18:53:56Z

closed in favor of PR #1076

* redo #1030 * comment a todo for `m_IsRunningUnitTest` * test project Adding check for unit testing at the end of OnClientUnloadScene to determine if we need to just passthrough to the callback (for multiInstance unit testing only) Fixing minor issue with NetworkObject not being around after unloading a scene for NetworkPrefabPool. Co-authored-by: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>

…hnologies#1076) * redo Unity-Technologies#1030 * comment a todo for `m_IsRunningUnitTest` * test project Adding check for unit testing at the end of OnClientUnloadScene to determine if we need to just passthrough to the callback (for multiInstance unit testing only) Fixing minor issue with NetworkObject not being around after unloading a scene for NetworkPrefabPool. Co-authored-by: NoelStephensUnity <73188597+NoelStephensUnity@users.noreply.github.com>

NoelStephensUnity added 30 commits July 1, 2021 17:47

feat

039e4ab

This is a quick and dirty additive scene loading implementation example with minimal changes to the develop branch.

refator

8f7227b

Updating basic scene registration to accept SceneAssets as opposed to string based names.

refactor

000319b

Removing the automatic addition of a scene asset to NetworkConfig. This has been a problematic approach to assuring the scene containing the NetworkManager is included.

refactor

728764a

Created the LoadScene method which will only load scenes additively. Broke up some of the commonly shared initializations and checks. This helps to show what the real differences between single and additive scene loading are. (like two method calls)

refactor

969123a

Just cleaning up the code, putting place holder notes, and adding some comments

style

c4138b0

Just comment and style adjustments

refactor

9f2c6f4

Swapped out the switch scene cycle hack for actual toggle buttons that allow one to load each sample additive scene or unload it based on the state of the toggle button.

feat

cdc6a50

This includes new assets to demonstrate additive scene loading and scene switching. Known issues are when additive scenes have been loaded that they are not synchronized with clients when they join.

wip

ec62ae5

A wip commit that is progress towards removing scene and object synchronization from the approval process.

refactor

69b43dc

naming issue and making sure we scope the pooled network reader.

Merge branch 'experimental/additivescenes' into experimental/late-joi…

cafdc9d

…n-additivescenes

feat

de0012e

Joining with additive scenes already loaded as well as late joining players are synchronizing properly. The handle approval process is no longer directly tied to scene loading. Minor updates to stats display to show the currently active scene

refactor

5d10e27

Minor checks for an object still being around during clean up.

Merge branch 'develop' into feat/MTT-820-AdditiveSceneLoading

ef77369

fix

729a2ce

Fixes the basic scene transitioning test failure.

fix

35a46b7

Fixing other unit test issues. Removing the temporary block for clients to receive messages (it was an artifact from testing). Still have a few tests that are failing.

Merge branch 'develop' into feat/MTT-820-AdditiveSceneLoading

d882819

fix

d7846f8

Adding missing scene assets for the multiprocess test. Making sure the SceneLoading test destroys the network manager instance.

style

3a7a0e9

PascalCased my enums and added event in front to avoid any NDA complications for the time being.

Merge remote-tracking branch 'origin/develop' into feat/MTT-820-Addit…

4bcd931

…iveSceneLoading

fix

c686eee

Removing Scene Loading Test: This test needs to be completely refactored and no longer is valid. WIP

NoelStephensUnity and others added 16 commits August 18, 2021 17:57

refactor

b5afae7

Removes returning bool to determine if NetworkPrefabHandler should destroy an object with Fatih's awesome fixes from PR-1068 #1068

Merge branch 'feat/MTT-820-AdditiveSceneLoading' into test/AdditiveSc…

bf5ddd4

…eneLoading

Merge branch 'develop' into feat/MTT-820-AdditiveSceneLoading

da9fff4

refactor

32022df

Removing the need to set NetworkSceneManager.IsTesting to true. Now NetworkSceneManager automatically detects if a unit test is running.

Merge branch 'develop' into feat/MTT-820-AdditiveSceneLoading

e31e6aa

refactor

093bb75

Removing the check for a null SpawnManager in NetworkObject.Despawn as this no longer seems to be an issue after the more recent merges in develop.

Merge remote-tracking branch 'origin/feat/MTT-820-AdditiveSceneLoadin…

562f712

…g' into test/AdditiveSceneLoading

refactor minor

3958f15

Noticed one minor portion of the client loading process that really shouldn't be outside of SceneEventData. Removed 1 method call from NetworkSceneManager and 1 method from SceneEventData.

Merge branch 'feat/MTT-820-AdditiveSceneLoading' into test/AdditiveSc…

7d43ca5

…eneLoading

style

4b558a8

Removed namespace no longer being used. (Standards check failed)

Merge branch 'feat/MTT-820-AdditiveSceneLoading' into test/AdditiveSc…

65c9ebc

…eneLoading

test and style

adefb18

Added the suggested || DEVELOPMENT_BUILD to the unit test check and adjusted the 3 locations to mirror this addition. As well, added comments to explain why we are doing this here.

andrews-unity approved these changes Aug 23, 2021

View reviewed changes

0xFA11 changed the base branch from develop to feat/MTT-820-AdditiveSceneLoading August 23, 2021 17:36

Base automatically changed from feat/MTT-820-AdditiveSceneLoading to develop August 23, 2021 17:37

0xFA11 changed the base branch from develop to feat/MTT-820-AdditiveSceneLoading August 23, 2021 17:54

0xFA11 changed the base branch from feat/MTT-820-AdditiveSceneLoading to develop August 23, 2021 17:54

0xFA11 changed the base branch from develop to feat/MTT-820-AdditiveSceneLoading August 23, 2021 17:59

0xFA11 added a commit that referenced this pull request Aug 23, 2021

redo #1030

b7de156

0xFA11 mentioned this pull request Aug 23, 2021

test: add networkscenemanager additive scene loading tests #1076

Merged

Base automatically changed from feat/MTT-820-AdditiveSceneLoading to develop August 23, 2021 18:43

0xFA11 closed this Aug 23, 2021

NoelStephensUnity deleted the test/AdditiveSceneLoading branch March 22, 2022 22:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

test: additive scene loading #1030

test: additive scene loading #1030

Uh oh!

NoelStephensUnity commented Aug 6, 2021 •

edited

Loading

Uh oh!

0xFA11 commented Aug 23, 2021

Uh oh!

Uh oh!

test: additive scene loading #1030

test: additive scene loading #1030

Uh oh!

Conversation

NoelStephensUnity commented Aug 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xFA11 commented Aug 23, 2021

Uh oh!

Uh oh!

NoelStephensUnity commented Aug 6, 2021 •

edited

Loading