Merge branch 'develop-2.0.0' into fix/spawn-disabled-in-scene-placed-and-network-prefab-registration

Author: NoelStephensUnity

.yamato/_triggers.yml

@@ -89,8 +89,8 @@ pr_code_changes_checks:
     # Note that our daily tests will anyway run both test configurations in "minimal supported" and "trunk" configurations
     - .yamato/desktop-standalone-tests.yml#desktop_standalone_test_testproject_ubuntu_il2cpp_{{ pinnedTrunk }}
     - .yamato/cmb-service-standalone-tests.yml#cmb_service_standalone_test_testproject_ubuntu_il2cpp_{{ pinnedTrunk }}
-    # Run code coverage test
-    - .yamato/code-coverage.yml#code_coverage_ubuntu_{{ validation_editors.default }}
+    # Run code coverage test (PRs use the pinned "safe" trunk)
+    - .yamato/code-coverage.yml#code_coverage_project_test_testproject_ubuntu_{{ pinnedTrunk }}
   triggers:
     expression: |-
       (pull_request.comment eq "ngo" OR
@@ -159,8 +159,8 @@ develop_nightly:
     - .yamato/project-updated-dependencies-test.yml#updated-dependencies_testproject_NGO_win_6000.0
     # Run API validation to early-detect all new APIs that would force us to release new minor version of the package. Note that for this to work the package version in package.json must correspond to "actual package state" which means that it should be higher than last released version
     - .yamato/vetting-test.yml#vetting_test
-  # Run code coverage test
-    - .yamato/code-coverage.yml#code_coverage_ubuntu_{{ validation_editors.default }}
+  # Run code coverage test (nightly uses actual trunk)
+    - .yamato/code-coverage.yml#code_coverage_project_test_testproject_ubuntu_trunk
 
 
 # Run all tests on weekly bases

.yamato/code-coverage.yml

@@ -2,33 +2,37 @@
 ---
 
 # DESCRIPTION--------------------------------------------------------------------------
-  # This job is responsible for executing package tests with code coverage analysis enabled.
+  # This job runs the in-repo testproject tests with code coverage analysis enabled.
+  # The project tests also exercise the NGO package tests, so a separate package-test coverage job is not needed.
   # Coverage analysis provides insights into:
     # Test coverage metrics for NGO assemblies
     # Line and branch coverage statistics
     # Generated HTML reports for coverage visualization
     # Additional metrics for coverage analysis
 
 # CONFIGURATION STRUCTURE--------------------------------------------------------------
-  # Jobs are generated using nested loops through:
-    # 1. For default platform only (Ubuntu) since coverage would not vary between platforms (no need for checks on more platforms)
-    # 2. For default editor version (trunk) since coverage would not vary between editors (no need for checks on more editors)
+  # Jobs are generated for:
+    # 1. Default platform only (Ubuntu) since coverage would not vary between platforms (no need for checks on more platforms)
+    # 2. Two editors: actual "trunk" (used by nightly) and the "pinnedTrunk" safe version (used by PRs).
+    #    Occasionally trunk breaks our tests, so PRs run against a pinned "safe" trunk while nightly runs actual trunk.
 
 #TECHNICAL CONSIDERATIONS---------------------------------------------------------------
-  # In theory this job also runs package tests, but we don't want to use it as default since is heavier (because of added coverage analysis) and coverage is not changing that often
   # Requires Unity Editor installation
   # Burst compilation is disabled to ensure accurate coverage measurement
   # In order to properly use -coverage-results-path parameter we need to start it with $PWD (which means the absolute path). Otherwise coverage results will not be visible
 
 # QUALITY CONSIDERATIONS--------------------------------------------------------------------
-  # To see where this job is included (in trigger job definitions) look into _triggers.yml file
-
+  # To see where this job is included (in trigger job definitions) look into _triggers.yml file. Currently:
+    # [Code Coverage] Project Test runs on PR changes (pr_code_changes_checks, pinnedTrunk) and nightly (develop_nightly, trunk)
 
+{% assign coverage_editors = "trunk," | append: pinnedTrunk | split: "," -%}
 
 {% for platform in test_platforms.default -%}
-{% for editor in validation_editors.default -%}
-code_coverage_{{ platform.name }}_{{ editor }}:
-  name: Code Coverage - NGO [{{ platform.name }}, {{ editor }}]
+{% for project in projects.default -%}
+{% for editor in coverage_editors -%}
+{% if editor == "trunk" -%}{% assign editor_label = "trunk" -%}{% else -%}{% assign editor_label = "pinnedTrunk" -%}{% endif -%}
+code_coverage_project_test_{{ project.name }}_{{ platform.name }}_{{ editor }}:
+  name: '[Code Coverage] Project Test - NGO {{ project.name }} [{{ platform.name }}, {{ editor }}]'
   agent:
     type: {{ platform.type }}
     image: {{ platform.image }}
@@ -38,13 +42,14 @@ code_coverage_{{ platform.name }}_{{ editor }}:
 {% endif %}
   commands:
     - unity-downloader-cli --fast --wait -u {{ editor }} -c Editor {% if platform.name == "mac" %} --arch arm64 {% endif %} # For macOS we use ARM64 models
-    - upm-pvp create-test-project test-project --packages "upm-ci~/packages/*.tgz" --unity .Editor
-    - UnifiedTestRunner --suite=editor --suite=playmode --editor-location=.Editor --testproject=test-project --enable-code-coverage --coverage-upload-options="reportsDir:$PWD/test-results/CoverageResults;name:NGOv2_{{ platform.name }}_{{ editor }};flags:NGOv2_{{ platform.name }}_{{ editor }};verbose" --coverage-results-path=$PWD/test-results/CoverageResults --coverage-options="generateHtmlReport;generateAdditionalMetrics;assemblyFilters:+Unity.Netcode.Editor,+Unity.Netcode.Runtime" --extra-editor-arg=--burst-disable-compilation --timeout={{ test_timeout }} --rerun-strategy=Test --retry={{ num_test_retries }} --clean-library-on-rerun --artifacts-path=test-results
+    - UnifiedTestRunner --testproject={{ project.path }} --suite=editor --suite=playmode --editor-location=.Editor --enable-code-coverage --coverage-upload-options="reportsDir:$PWD/test-results/CoverageResults;name:NGOv2_project_{{ project.name }}_{{ platform.name }}_{{ editor_label }};flags:NGOv2_project_{{ project.name }}_{{ platform.name }}_{{ editor_label }};verbose" --coverage-results-path=$PWD/test-results/CoverageResults --coverage-options="generateHtmlReport;generateAdditionalMetrics;assemblyFilters:+Unity.Netcode.Editor,+Unity.Netcode.Runtime" --extra-editor-arg=--burst-disable-compilation --timeout={{ test_timeout }} --rerun-strategy=Test --retry={{ num_test_retries }} --clean-library-on-rerun --artifacts-path=test-results
   artifacts:
     logs:
       paths:
         - "test-results/**/*"
   dependencies:
+    - .yamato/_run-all.yml#run_quick_checks # initial checks to perform fast validation of common errors
     - .yamato/package-pack.yml#package_pack_-_ngo_{{ platform.name }}
 {% endfor -%}
 {% endfor -%}
+{% endfor -%}

Tools/CI/service.cmb/README.md

@@ -1,6 +1,6 @@
 # Testing against the CMB Service
 
-The CMB Service is a tool that is external to our repository. The tool is inside the `runtime` folder in the [mps-common-multiplayer-backend](https://github.com/Unity-Technologies/mps-common-multiplayer-backend) repository.
+The CMB Service is a tool that is external to our repository. The tool is inside the `runtime` folder of the [CMB service](https://github.com/Unity-Technologies/unity-player-services/tree/main/services/common-multiplayer-backend) in the Unity Player Services monorepo.
 
 Due to this, there is some more setup needed when running tests against the CMB Service.
 
@@ -56,3 +56,7 @@ The following environment variables allow for further configuration of the setup
 `CMB_SERVICE_PORT` defines the port where the tests will try to connect to the service (defaults to `7789`).
 
 `NGO_HOST` defines the http address where the tests will try to connect to the service (defaults to `127.0.0.1`).
+
+## Running on CI (Yamato)
+
+The CMB tests can also be run from Yamato. The jobs are defined in [`.yamato/cmb-service-standalone-tests.yml`](../../../.yamato/cmb-service-standalone-tests.yml) and appear in Yamato as `CMB Service Test - NGO <project> - [<platform>, <editor>, <backend>]`. The job can be triggered manually from any branch meaning it can be easier to run the CMB tests from Yamato rather than set them up locally. The test uses [`run_cmb_service.sh`](./run_cmb_service.sh) to setup and run the CMB service.

Tools/CI/service.cmb/run_cmb_service.sh

@@ -17,7 +17,7 @@
   # Example usage:
     # ./<path-to-script>/run_cmb_service.sh -e 7788 -s 7799
 
-  # This script is currently used in the desktop-standalone-tests yamato job.
+  # This script is currently used in the cmb-service-standalone-tests yamato job (found at ../../../.yamato/cmb-service-standalone-tests.yml).
 
 # TECHNICAL CONSIDERATIONS---------------------------------------------------------------
   # This is a bash script and so needs to be run on a Unix based system.
@@ -136,11 +136,14 @@ else
 logMessage "Protocol Buffer Compiler Installed & ENV variables verified!\n PROTOC path is: $PROTOC"
 fi
 
-# clone the cmb service repo
-git clone https://github.com/Unity-Technologies/mps-common-multiplayer-backend.git
+# Sparse-checkout only the CMB service directory from the unity-player-services monorepo.
+# --filter=blob:none + --depth 1 avoids downloading file contents and history for the rest of the monorepo.
+git clone --depth 1 --filter=blob:none --sparse https://github.com/Unity-Technologies/unity-player-services.git
+cd ./unity-player-services
+git sparse-checkout set services/common-multiplayer-backend/runtime
 
 # navigate to the cmb service directory
-cd ./mps-common-multiplayer-backend/runtime
+cd ./services/common-multiplayer-backend/runtime
 
 # Install rust
 curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y

com.unity.netcode.gameobjects/Runtime/Core/FindObjects.cs

@@ -69,8 +69,8 @@ private struct ObjectsInSceneEnumerator<T> : IEnumerable<T>, IEnumerator<T> wher
             internal ObjectsInSceneEnumerator(Scene scene, bool includeInactive)
             {
                 m_IncludeInactive = includeInactive;
-
-                m_RootObjects = scene.GetRootGameObjects();
+                // If the scene is invalid, use an empty array.
+                m_RootObjects = scene.IsValid() ? scene.GetRootGameObjects() : new UnityEngine.GameObject[0];
                 m_RootIndex = 0;
                 m_CurrentChildObjects = null;
                 m_CurrentChildIndex = 0;

com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs

@@ -3008,17 +3008,31 @@ internal void SynchronizeOwnerNetworkVariables(ulong originalOwnerId, ulong orig
                 childBehaviour.MarkOwnerReadDirtyAndCheckOwnerWriteIsDirty();
             }
 
+            // If the spawn authority of a distributed authority network topology has invoked a change in ownership,
+            // then we want to invoke the NetworkBehaviourUpdate prior to changing the owner back.
+            if (NetworkManager.DistributedAuthorityMode)
+            {
+                // Force send a state update for all owner read NetworkVariables  and any currently dirty
+                // owner write NetworkVariables.
+                NetworkManagerOwner.BehaviourUpdater.NetworkBehaviourUpdate(true);
+            }
+
             // Now set the new owner and previous owner identifiers back to their original new values
-            // before we run the NetworkBehaviourUpdate. For owner read only permissions this order of
-            // operations is **particularly important** as we need to first (above) mark things as dirty
-            // from the context of the original owner and then second (below) we need to send the messages
-            // which requires the new owner to be set for owner read permission NetworkVariables.
+            // after we run the NetworkBehaviourUpdate.
             OwnerClientId = currentOwnerId;
             PreviousOwnerId = originalOwnerId;
 
-            // Force send a state update for all owner read NetworkVariables  and any currently dirty
-            // owner write NetworkVariables.
-            NetworkManagerOwner.BehaviourUpdater.NetworkBehaviourUpdate(true);
+            // For owner read only permissions this order of operations is **particularly important** as
+            // we need to first (above) mark things as dirty from the context of the original owner and
+            // then second (below) we need to send the messages which requires the new owner to be set.
+            // Note: Owner read only NetworkVariables do not make sense in a distributed authority topology
+            // since the only instance that can write is the owner.
+            if (!NetworkManager.DistributedAuthorityMode)
+            {
+                // Force send a state update for all owner read NetworkVariables and any currently dirty
+                // owner write NetworkVariables.
+                NetworkManagerOwner.BehaviourUpdater.NetworkBehaviourUpdate(true);
+            }
         }
 
         // NGO currently guarantees that the client will receive spawn data for all objects in one network tick.