Repository navigation
fix: hybrid spawn world selection, hybrid parenting compatibility, and parent revert pose - #4202
Open
NoelStephensUnity wants to merge 14 commits into
Open
NoelStephensUnity wants to merge 14 commits into
NoelStephensUnity wants to merge 14 commits into
Conversation
N4E spawns a ghost into its active world, which is whichever world updated last when several worlds share a process. Hybrid player prefabs, InstantiateAndSpawn and Instantiate followed by Spawn could therefore spawn the ghost into a client world and fail with "Invalid world ... for spawn". Hybrid prefabs now delay their ghost spawn (GhostObject.DelaySpawning, available in N4E 7.1.0), and the spawn authority spawns the ghost in its own NetworkManager's world before OnNetworkSpawn. The integration test harness no longer sets N4E's active world itself. Adds HybridSpawnWorldSelectionTests, which make a client world active before each spawn.
…enable hybrid NetworkObjectSpawning N4E makes the server world active again after each client world update, so making a client world active from the test body no longer reached the player spawn, and LateJoinPlayerSpawnsGhostInServerWorld passed without the fix. A server-side prefab handler now makes the client world active right before the player is instantiated. Re-adds the UnifiedHost fixture to the testproject NetworkObjectSpawning tests, now that InstantiateAndSpawn selects the NetworkManager's world for hybrid prefabs. SpawnOnSynchronizedNotification is skipped under hybrid (in-scene placed NetworkObjects).
When a hybrid prefab's GhostObject has ParentReplication enabled, Netcode for Entities owns parenting and NGO stands down: - TrySetParent and TryRemoveParent log a warning and return false. - OnTransformParentChanged and ApplyNetworkParenting exit silently. - The spawn message no longer carries the parent; the ghost's spawn does. - Despawning a parent finds first generation children by hierarchy. ParentReplication only exists in com.unity.netcode 7.0.1+, so this is gated on the new UNIFIED_NETCODE_PARENT_REPLICATION version define. When NGO handles parenting of a hybrid instance, the authority logs at Developer level when the result can show visual artifacts: re-parenting with worldPositionStays enabled, or parenting where only one of child and parent is a hybrid instance.
…he child in place Ignored until N4E syncs a reparent made without parent replication to the entity.
Unity.Netcode.Runtime.Tests declared a versionDefine for com.unity.modules.physics but not for
com.unity.modules.physics2d, while Unity.Netcode.Runtime declares both. Tests/Runtime/Physics/
NetworkRigidbodyTest.cs opens with
#if COM_UNITY_MODULES_PHYSICS && COM_UNITY_MODULES_PHYSICS2D
so COM_UNITY_MODULES_PHYSICS2D was never defined for the assembly the file belongs to and the
whole fixture compiled to nothing - three TestFixture variants over five rigidbody
interpolation configurations, the only NetworkRigidbody integration coverage there is. It
reported no results rather than failing, so the suite has been green on tests that do not
exist. Confirmed absent from the built Unity.Netcode.Runtime.Tests.dll, and present on
develop-2.0.0 as well.
The two sibling files guarded on the same pair use || rather than &&, so they were unaffected.
PHYSICS2D_LINEAR is deliberately not added - no test references it.
With the define in place the fixture compiles and passes 3/3 on 6000.7.0a5.
UNIFIED_NETCODE already requires N4E 7.2.0, and GhostObject.ParentReplication ships from 7.1.0, so the separate N4E >= 7.0.1 define is no longer needed.
A hybrid instance's ghost replicates its local transform. With worldPositionStays the authority rebases that local transform at the reparent, but clients apply ParentSyncMessage ahead of the interpolated snapshot carrying the rebased values, so the object pops by the full reparent offset. With host smoothing it also stayed displaced. When GhostObject.ParentReplication is off, TrySetParent and TryRemoveParent with worldPositionStays enabled now log an error and return false. A direct Transform parent change, which carries no worldPositionStays value, is accepted when it kept the local transform and otherwise reverted to the previous parent and local transform, captured in OnBeforeTransformParentChanged. Hybrid fixtures that parented with worldPositionStays enabled now disable it.
When NGO rejected a parent change (no parenting authority, an unspawned NetworkObject, a parent without a NetworkObject), it reverted by assigning transform.parent, which keeps the world position. A change made with worldPositionStays disabled had already moved the object, so the parent came back but the object stayed at the moved position. OnBeforeTransformParentChanged now captures the local position, rotation and scale before every parent change, and a revert restores the cached parent with those values. The hybrid direct parent change check uses the same capture instead of its own. Adds NetworkObjectParentRevertTests (Host, Server, DAHost).
Short sentences, N4E instead of the full name, lists for separate cases, and full XML docs on the ShouldCheckForSpawnedPlayers override. MakeClientWorldActive becomes an inlined MakeWorldActive(NetworkManager) helper without a comment that restated its name.
NetworkTransformOwnershipTests and RigidbodyContactEventManagerTests use Rigidbody, NetworkRigidbody and RigidbodyContactEventManager, which only exist with com.unity.modules.physics. Now that the test assembly defines COM_UNITY_MODULES_PHYSICS2D, their PHYSICS || PHYSICS2D guard would fail to compile in a project with Physics2D but without 3D physics.
2 of 6 tasks
…pawn-world-selection
…efabs Only prefab registration rejected hybrid prefabs. An unregistered hybrid prefab instance, for example one spawned through a prefab handler, spawned into a session that never entered hybrid mode and ran its ghost spawn, ownership and bridge code without a unified world. The spawn authority now logs an error and refuses the spawn.
2 tasks done
This branch has not been deployed
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
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.
Purpose of this PR
Hybrid prefab instances now always spawn their GhostObject in their own NetworkManager's world, NGO parenting of hybrid instances no longer produces visual artifacts, and a rejected NGO parent change now restores the object's original position. This PR bundles four related hybrid changes, one NGO parenting fix and one test-compile fix.
Spawning a hybrid instance in its NetworkManager's world: N4E spawns a ghost into its active world. A hybrid player prefab,
InstantiateAndSpawn, orInstantiatefollowed bySpawncould therefore spawn the ghost into a client world and fail with "Invalid world ... for spawn". Hybrid prefabs now delay their ghost spawn (GhostObject.DelaySpawning), and the spawn authority spawns the ghost in its own NetworkManager's world beforeOnNetworkSpawn. The integration test harness no longer sets N4E's active world itself.Parent replication compatibility: when
GhostObject.ParentReplicationis enabled, N4E replicates the parent of a hybrid instance and the NGO parenting API and processing are disabled for those instances:TrySetParent/TryRemoveParentlog a warning and return false,OnTransformParentChangedearly-exits, and the spawn message no longer carries a parent (one less byte per parented hybrid spawn). When hybrid parent replication is enabled, the normaltransform.SetParentand direct assignment oftransform.parentare how user scripts activate parenting.NGO parenting of hybrid instances requires worldPositionStays disabled: when NGO handles the parenting (
ParentReplicationdisabled), a reparent with worldPositionStays enabled made clients apply the new parent before the ghost's rebased local transform arrived, so the object popped by the full reparent offset, and with full-tick smoothing it stayed displaced. Users attempting this will now see an error and the parent change does not happen:TrySetParent(parent, true)/TryRemoveParent(true)return false. Note: both default totrue, soTrySetParent(parent)must becomeTrySetParent(parent, false)for hybrid instances.transform.parent = parent(orSetParent(parent, true)) is reverted to the previous parent and local transform. A directSetParent(parent, false)is accepted and synchronized as worldPositionStays disabled.A rejected parent change restores the original position (NGO): when NGO rejects a parent change (for example a client without parenting authority, an unspawned
NetworkObject, or a parent without aNetworkObject), it reverted the parent by assigningtransform.parent, which keeps the world position. A change made with worldPositionStays disabled had already moved the object, so the parent was restored but the object kept its moved position. The position, rotation and scale from before the change are now captured inOnBeforeTransformParentChangedand restored along with the parent.Hybrid instances need a hybrid session: NGO only checked hybrid prefabs when they were registered, not when an instance was spawned. An unregistered hybrid prefab instance, for example one spawned through a prefab handler, spawned into a session that never entered hybrid mode. Spawning one now logs an error and the spawn does not happen.
NetworkRigidbody tests never compiled: the runtime test assembly was missing the
com.unity.modules.physics2dversion define, soNetworkRigidbodyTestcompiled to nothing on every branch. It now compiles and runs.PR Scope:
NetworkSpawnManager: ghost spawn in the NetworkManager's world for hybrid instances, and hybrid instances are rejected when no hybrid prefab was registered before the session started.NetworkObjectBridge: hybrid prefabs setDelaySpawning.NetworkObject: parent replication disabling NGO parenting, worldPositionStays rejection, the direct parent change check, and restoring the original pose when a parent change is reverted (OnBeforeTransformParentChanged).HybridSpawnWorldSelectionTests,HybridParentingTests,NetworkObjectParentRevertTests, testprojectNetworkObjectSpawninghybrid fixture re-enabled, the runtime test asmdef physics2d define (the two 3D-only physics fixtures now guard onCOM_UNITY_MODULES_PHYSICSalone), andUnifiedHybridPrefabValidationTests.HybridSpawnWithoutHybridPrefabsIsRejected.Issues found, but were not fixed for this pass and require one or more follow-up PRs
ParentReplicationenabled is intermittent until N4E PR 129044 ships.The affected test (
ParentReplication_ParentDespawned_ChildSurvivesAtRoot, parent replication enabled) is ignored until trunk PR-129044 lands.Jira ticket
MTT-15275 ---> NGO side.
Changelog
NetworkObject's parent but left it at the moved position when the change was made withworldPositionStaysdisabled. The original position, rotation and scale are now restored.Documentation
Testing & QA (How your changes can be verified during release Playtest)
Functional Testing
Manual testing :
Manual testing doneAutomated tests:
Covered by existing automated testsCovered by new automated testsDoes the change require QA team to:
Review automated tests?Execute manual tests?Provide feedback about the PR?Up-port
Not applicable.
Backports
The parent revert fix will be ported to develop-2.0.0 in a separate PR. The hybrid changes exist on develop-3.x.x only.