chore: enable unified hybrid tests (#4122)

* update - ci

Adding an additional unified hybrid spawning test pass.

* fix

Fixed:
- Issue with NetworkObject throwing an exception during hybrid integration test.
- Issue with setting the active world prior to spawning a hybrid prefab during integration tests.
- Issue with UnifiedNetcodeUpdateSystem not overriding the OnCreate method along with checks for a valid NetworkManager and/or transport within OnUpdate.
- Issues (minor) with the original UnifiedNetworkTransformTest.
- Adjustments to the NetcodeIntegrationTest that assures the correct active world is assigned when spawning.

* update

Adding the modified NetcodeConfig specific to NGO.
Adding the unified manifest file for running unified tests.

* update

Unified specific settings

* fix

Formatting of the unified test filter.

* update

One more time... just running the UnifiedNetworkTransformTest for this phase.
A more integrated one will be coming in the next PR for this effort.

* Apply suggestions from code review

Co-authored-by: Emma <emma.mcmillan@unity3d.com>

* Update .yamato/_triggers.yml

Co-authored-by: Emma <emma.mcmillan@unity3d.com>

* Update TODO comment for Netcode singleton issue

* Update unified-tests.yml to simplify commands

Removed command to delete packages-lock.json from unified tests.

---------

Co-authored-by: Emma <emma.mcmillan@unity3d.com>

Author: NoelStephensUnity

.yamato/_run-all.yml

@@ -368,6 +368,21 @@ run_all_project_tests_console_standalone_default:
 {% endfor -%}
 
 
+# Runs all unified (NGO + N4E) tests
+# These run on their own pinned editor (unified_editors) rather than the validation_editors, because
+# they need an editor that bundles a com.unity.netcode with the unified API. See unified-tests.yml.
+run_all_unified_tests:
+  name: Run All Unified Tests
+  dependencies:
+{% for project in projects.default -%}
+{% for platform in unified_test_platforms -%}
+{% for editor in unified_editors.default -%}
+    - .yamato/unified-tests.yml#unified_test_{{ project.name }}_{{ platform.name }}_{{ editor }}
+{% endfor -%}
+{% endfor -%}
+{% endfor -%}
+
+
 # Runs all CMB service tests
 run_all_project_tests_cmb_service:
   name: Run All CMB Service Tests

.yamato/_triggers.yml

@@ -90,6 +90,7 @@ 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 }}
+    - .yamato/_run-all.yml#run_all_unified_tests
     # Run code coverage test (PRs use the pinned "safe" trunk)
     - .yamato/code-coverage.yml#code_coverage_project_test_testproject_ubuntu_{{ pinnedTrunk }}
   triggers:
@@ -120,6 +121,19 @@ pr_code_changes_checks:
 
 
 
+# Unified (NGO + N4E) validation, on demand.
+# This job allows the Unified tests to be kicked off by commenting "/ci unified". 
+# This is useful for PRs where pr_code_changes_checks doesn't trigger.
+unified_pr_checks:
+  name: Unified (NGO + N4E) checks [on demand]
+  dependencies:
+    - .yamato/_run-all.yml#run_all_unified_tests
+  triggers:
+    expression: |-
+      pull_request.comment eq "unified"
+    cancel_old_ci: true
+
+
 # Run all tests on nightly basis.
 # Same subset as pull_request_trigger with addition of mobile/desktop/console tests and webgl builds
 # Those tests are all running on trunk and the default editor (since it's daily and running all of them would add a lot of overhead)
@@ -156,6 +170,8 @@ develop_nightly:
     # Run Runtime tests against cmb service on trunk and default editors
     - .yamato/_run-all.yml#run_all_project_tests_cmb_service_trunk
     - .yamato/_run-all.yml#run_all_project_tests_cmb_service_default
+    # Run the unified (NGO + N4E) tests on their own pinned editor (see .yamato/unified-tests.yml)
+    - .yamato/_run-all.yml#run_all_unified_tests
     # Build player for webgl platform on trunk and default editors
     - .yamato/project-updated-dependencies-test.yml#updated-dependencies_testproject_NGO_ubuntu_trunk
     - .yamato/project-updated-dependencies-test.yml#updated-dependencies_testproject_NGO_win_{{ validation_editors.default }}

.yamato/project.metafile

@@ -190,6 +190,55 @@ validation_editors:
 pinnedTrunk:  5fe7931aab8c4fff9274e15ef0800125c68b8d6a
 
 
+# UNIFIED (NGO + N4E) CONFIGURATION---------------------------------------------------------------------
+# The unified test job validates NGO running against Netcode for Entities (N4E) through the unified API.
+# It is deliberately kept separate from every other job in this repo because it needs an editor that
+# bundles com.unity.netcode with the unified API, and that editor is NOT one of the validation_editors.
+#
+# WHY THIS IS PINNED TO 6000.7.0a5 (and not 6000.7.0a2):
+#   UnifiedNetcodeTransport is compiled behind "#if UNIFIED_NETCODE && OUT_OF_BAND_RPC" and needs the
+#   N4E out-of-band RPC API (IOutOfBandRpcCommand / OutgoingOutOfBandRpcDataStreamBuffer).
+#   Both 6000.7.0a2 and 6000.7.0a5 bundle com.unity.netcode 6.7.0, but that API only landed in the a5
+#   snapshot. On a2 the unified transport does not compile, so the unified tests cannot run there.
+#   Bump this pin (and unified_netcode_version below) together whenever the required N4E API moves.
+#
+# An explicit alpha version is used instead of a trunk revision hash because published alphas are
+# already immutable, so there is nothing to pin against.
+unified_editors:
+    default:
+        - 6000.7.0a5
+
+# Version of com.unity.netcode (N4E) bundled with unified_editors.default.
+# It resolves as a "builtin" package out of the editor install, so it must match the editor exactly.
+unified_netcode_version: 6.7.0
+
+# The unified job runs in Editor context only, so a single fast platform is enough.
+unified_test_platforms:
+    - name: ubuntu
+      type: Unity::VM
+      image: package-ci/ubuntu-22.04:v4.87.0
+      flavor: b1.large
+
+# Restricts the unified job to unified-only tests so nothing else runs on the alpha editor.
+#
+# This is a REGULAR EXPRESSION, not a glob. UTR passes it to the editor as -testFilter, which ends up
+# in UnityEngine.TestRunner's FullNameFilter -> NUnit ValueMatchFilter -> new Regex(pattern).IsMatch().
+# A glob-style "*Unified*" throws "Quantifier {x,y} following nothing" and fails the whole run.
+# The other jobs' "Unity.Netcode.RuntimeTests.*" works because it is also a valid regex - it just
+# happens to read like a glob.
+#
+# The match is against the NUnit *full* test name, which includes both fixture and method arguments.
+#
+# DO NOT widen this to ".*Unified.*". "Unified" appearing in a test name does NOT mean that test was
+# deliberately converted to run against hybrid prefabs. HostOrServer gained UnifiedServer/UnifiedHost
+# members under UNIFIED_NETCODE, and NUnit expands a bare [Values] on an enum parameter (and
+# enum-typed fixture constructors) to every member. So with N4E installed, much of the existing suite
+# silently grows unified cases - e.g. NetworkVariableTests(Default).AllNetworkVariableTypes(UnifiedHost),
+# which nobody wrote. ".*Unified.*" selected 138 tests, 84 of which failed, against the 1 test this
+# job exists to validate. Deciding which of those should pass, and how they opt in, is Goal-2.
+unified_test_filter: ".*UnifiedNetworkTransformTest.*"
+
+
 # Scripting backends used by Standalone RunTimeTests---------------------------------------------------
 
 scripting_backends:

.yamato/unified-tests.yml

@@ -0,0 +1,75 @@
+{% metadata_file .yamato/project.metafile %} # All configuration that is used to create different configurations (used in for loops) is taken from this file.
+---
+
+# DESCRIPTION--------------------------------------------------------------------------
+  # This job validates NGO running against Netcode for Entities (N4E) through the unified API.
+  # A "hybrid prefab" is an NGO prefab (NetworkObject at the root) that also carries a GhostObject.
+  # When one or more hybrid prefabs are in the NetworkManager's prefab list, NGO hands transform
+  # synchronization to N4E's snapshot system and tunnels its own batched messages over
+  # UnifiedNetcodeTransport (N4E's out-of-band RPC) instead of using a NetworkTransport such as UTP.
+  # UnifiedNetworkTransformTest is the validation test that this path works end to end.
+
+# WHY THIS JOB IS SEPARATE FROM EVERY OTHER JOB------------------------------------------
+  # 1. It needs an editor that bundles a com.unity.netcode with the unified API. That editor
+  #    (unified_editors.default in project.metafile) is not one of the validation_editors, and NGO
+  #    still has to keep building and testing against editors that have no unified API at all.
+  # 2. It needs com.unity.netcode in the testproject so that the UNIFIED_NETCODE define is set
+  #    (see the versionDefines in Unity.Netcode.Runtime.asmdef). The committed
+  #    testproject/Packages/manifest.json deliberately does NOT reference it, so this job swaps in
+  #    testproject/Packages/manifest-unified.json instead.
+  # 3. It is wired into pr_code_changes_checks and develop_nightly the same way the CMB service tests
+  #    are, so it runs automatically on PRs targeting develop / develop-3.x.x / release. Be aware that
+  #    this makes an unsupported alpha editor part of the PR gate: when N4E lands breaking changes in
+  #    trunk this job goes red and the pin has to be bumped to unblock PRs.
+  #    _triggers.yml also has unified_pr_checks, so it can be kicked off with "/ci unified" on PRs
+  #    that pr_code_changes_checks does not cover.
+
+# CONFIGURATION STRUCTURE--------------------------------------------------------------
+  # Jobs are generated using nested loops:
+    # 1. For all unified test platforms (currently Ubuntu only, see project.metafile)
+    # 2. For all unified editors (currently a single pinned alpha, see project.metafile)
+
+# TECHNICAL CONSIDERATIONS---------------------------------------------------------------
+  # This job runs in Editor context only (no player builds required), like project-tests.yml.
+  # Only playmode is run: every unified test is an integration test and there are no unified EditMode tests.
+  # The run is restricted to unified tests via unified_test_filter so that nothing else is exercised
+  # on the alpha editor. Non-unified tests are covered by the regular jobs on the supported editors.
+  # packages-lock.json is removed because the committed lock was resolved against a much older editor
+  # and its builtin package versions do not exist in the unified editor.
+
+# QUALITY CONSIDERATIONS--------------------------------------------------------------------
+  # TODO: the manifest swap means testproject/Packages/manifest-unified.json has to be kept in sync
+  #       with manifest.json by hand. Revisit once N4E is a hard dependency of NGO and one manifest
+  #       can cover both cases.
+  # TODO: unified_test_filter is a test-name pattern. Replacing it with an NUnit category (for
+  #       example [Category("Unified")]) would be less fragile once more fixtures gain unified variants.
+
+#------------------------------------------------------------------------------------
+
+{% for project in projects.default -%}
+{% for platform in unified_test_platforms -%}
+{% for editor in unified_editors.default -%}
+unified_test_{{ project.name }}_{{ platform.name }}_{{ editor }}:
+  name : Unified Test - NGO {{ project.name }} [{{ platform.name }}, {{ editor }}]
+  agent:
+    type: {{ platform.type }}
+    image: {{ platform.image }}
+    flavor: {{ platform.flavor }}
+{% if platform.model %}
+    model: {{ platform.model }} # This is set only in platforms where we want non-default model to use (more information in project.metafile)
+{% endif %}
+  commands:
+    # Swap in the manifest that pulls in com.unity.netcode (N4E). This is what causes UNIFIED_NETCODE
+    # to be defined and therefore what makes the unified tests compile at all.
+    - cp {{ project.path }}/Packages/manifest-unified.json {{ project.path }}/Packages/manifest.json
+    - unity-downloader-cli --fast --wait -u {{ editor }} -c Editor # Installing basic editor for tests execution
+    - UnifiedTestRunner --testproject={{ project.path }} --suite=playmode --artifacts-path=test-results --editor-location=.Editor --testfilter="{{ unified_test_filter }}" --rerun-strategy=Test --retry={{ num_test_retries }} --clean-library-on-rerun --timeout={{ test_timeout }}
+  artifacts:
+    logs:
+      paths:
+        - "test-results/**/*"
+  dependencies:
+    - .yamato/_run-all.yml#run_quick_checks # initial checks to perform fast validation of common errors
+{% endfor -%}
+{% endfor -%}
+{% endfor -%}

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

@@ -3968,6 +3968,17 @@ private void Start()
 
         private void InitGhost()
         {
+            // Note: If hybrid prefabs are created prior to any NetworkManager instances,
+            // then the next line throws and exception. This avoids that issue.
+            // We might come up with some global way to verify if we are running integration
+            // tests and add additional logic within to determine if we should log an error
+            // or not.
+            if (NetworkManager == null)
+            {
+                return;
+            }
+
+
             if (!NetworkManager.IsListening)
             {
                 if (NetworkManager.LogLevel == LogLevel.Developer)

com.unity.netcode.gameobjects/Runtime/Transports/Unified/UnifiedNetcodeTransport.cs

@@ -124,10 +124,11 @@ public void OnUpdate(ref SystemState state)
     [UpdateBefore(typeof(RpcSystem))]
     internal partial class UnifiedNetcodeUpdateSystem : SystemBase
     {
-        public void OnCreate(ref SystemState state)
+        protected override void OnCreate()
         {
-            state.RequireForUpdate<RpcCollection>();
-            state.RequireForUpdate<NetworkId>();
+            RequireForUpdate<RpcCollection>();
+            RequireForUpdate<NetworkId>();
+            base.OnCreate();
         }
 
         public UnifiedNetcodeTransport Transport;
@@ -155,6 +156,11 @@ public void SendRpc(TransportRpc rpc, Entity connectionEntity)
 
         protected override void OnUpdate()
         {
+            if (NetworkManager == null || Transport == null)
+            {
+                return;
+            }
+
             NetworkManager.MessageManager.ProcessSendQueues();
 
             using var commandBuffer = new EntityCommandBuffer(Allocator.Temp);

com.unity.netcode.gameobjects/Tests/Runtime/NetworkTransform/UnifiedNetworkTransformTest.cs

@@ -1,5 +1,6 @@
 #if UNIFIED_NETCODE
 using System.Collections;
+using System.Text;
 using NUnit.Framework;
 using Unity.Netcode.Components;
 using Unity.Netcode.TestHelpers.Runtime;
@@ -52,38 +53,45 @@ protected override IEnumerator OnSetup()
             return base.OnSetup();
         }
 
+        private bool ValidatePosition(StringBuilder errorLog)
+        {
+            var authorityPosition = m_AuthorityInstanceTransform.position;
+            foreach (var client in m_ClientNetworkManagers)
+            {
+                var instancePosition = client.SpawnManager.SpawnedObjects[m_Instance.NetworkObjectId].transform.position;
+                if (!Approximately(authorityPosition, instancePosition))
+                {
+                    errorLog.AppendLine($"[Client-{client.LocalClientId}] Clone instance position {GetVector3Values(instancePosition)} " +
+                        $"does not equal the authority's {GetVector3Values(authorityPosition)}");
+                }
+            }
+            return errorLog.Length == 0;
+        }
+
+        private Transform m_AuthorityInstanceTransform;
+
         [UnityTest]
         public IEnumerator BasicMovementTest()
         {
             var authority = GetAuthorityNetworkManager();
             m_Instance = SpawnObject(m_Prefab, m_ServerNetworkManager).GetComponent<NetworkObject>();
-
-            // Wait 5 seconds so we will dump any deferred messages if it failed on clients
-            // when checking to see if it spawned or not on the clients next.
-            // Enable this to debug deferred
-            //yield return new WaitForSeconds(5);
+            m_AuthorityInstanceTransform = m_Instance.transform;
 
             yield return WaitForSpawnedOnAllOrTimeOut(m_Instance);
             AssertOnTimeout($"Failed to spawn {m_Instance.name} on all clients!");
 
             VerboseDebug("All clients spawned instance!");
 
+            yield return WaitForConditionOrTimeOut(ValidatePosition);
+            AssertOnTimeout($"A client failed to synchronize the position on the initial spawn!");
+
             var originalPos = authority.LocalClient.PlayerObject.transform.position;
             var newPos = originalPos + new Vector3(1, 1, 1);
 
             m_Instance.transform.position = newPos;
 
-            foreach (var client in m_ClientNetworkManagers)
-            {
-                Assert.IsTrue(Approximately(originalPos, s_GlobalNetworkObjects[client.LocalClientId][m_Instance.NetworkObjectId].transform.position));
-            }
-
-            yield return new WaitForSeconds(1);
-
-            foreach (var client in m_ClientNetworkManagers)
-            {
-                Assert.IsTrue(Approximately(newPos, s_GlobalNetworkObjects[client.LocalClientId][m_Instance.NetworkObjectId].transform.position));
-            }
+            yield return WaitForConditionOrTimeOut(ValidatePosition);
+            AssertOnTimeout($"A client failed to synchronize the changed position: {GetVector3Values(newPos)}!");
             VerboseDebug("Test Passed!");
         }
     }

com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTest.cs

@@ -2436,13 +2436,9 @@ protected GameObject CreateHybridPrefab(string baseName, bool moveToDDOL = true)
             // - Then set it back to active.
             gameObject.SetActive(false);
             var adapter = gameObject.AddComponent<GhostObject>();
-            // Mark the reference as post processing to avoid registering this instance automatically.
-            GhostPrefabReference.s_IsPostProcessing = true;
-            adapter.prefabReference = ScriptableObject.CreateInstance<GhostPrefabReference>();
-            adapter.prefabReference.name = "GhostPrefabReference";
-            adapter.prefabReference.Prefab = gameObject;
 
-            GhostPrefabReference.s_IsPostProcessing = false;
+            // Initialize it as a prefab
+            adapter.InitializeAsPrefab();
 
             // TODO: This might be part of the CreateHybridPrefab parameters
             // For now, just use normal interpolation until we get integration
@@ -2610,6 +2606,20 @@ protected void SpawnObjectInstance(NetworkObject networkObjectToSpawn, NetworkMa
         private GameObject SpawnObject(NetworkObject prefabNetworkObject, NetworkManager owner, bool destroyWithScene = false, bool isPlayerObject = false)
         {
             Assert.IsTrue(prefabNetworkObject.GlobalObjectIdHash > 0, $"{nameof(GameObject)} {prefabNetworkObject.name} has a {nameof(NetworkObject.GlobalObjectIdHash)} value of 0! Make sure to make it a valid prefab before trying to spawn!");
+#if UNIFIED_NETCODE
+            // This has to happen *before* Instantiate, not after. The hybrid prefab is active, so the clone's
+            // GhostObject.Awake runs synchronously inside Object.Instantiate below. The clone is not a prefab
+            // (its prefabReference.Prefab points at the prefab, not at itself), so Awake acquires an entity
+            // reference, which resolves the world to spawn into from the Netcode.Instance.m_ActiveWorld singleton.
+            // N4E's rate managers reassign that singleton on every world update, so by the time a test body runs
+            // it points at whichever world updated last - typically a client world - and the spawn is rejected with
+            // "You can only spawn a ghost on a server or during prediction on a client."
+            // TODO-UNIFIED: NetCode.Netcode.Instance is a singleton and might cause issues assigning this.
+            if (prefabNetworkObject.HasGhost)
+            {
+                NetCode.Netcode.Instance.m_ActiveWorld = m_ServerNetworkManager.NetcodeWorld;
+            }
+#endif
             var newInstance = Object.Instantiate(prefabNetworkObject.gameObject);
             var networkObjectToSpawn = newInstance.GetComponent<NetworkObject>();
             SpawnObjectInstance(networkObjectToSpawn, owner, destroyWithScene, isPlayerObject);