fix: NetworkShow followed by ChangeOwnership generates ownership message error on target client #3468
Conversation
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
The server no longer sends the OwnershipChangeMessage to itself. The TrackOwnershipChangeSentMetric test was processing that extra message to just get to the actual ChangeOwnershipMessage metric. I removed that portion of this test.
5 tasks
…tra-ownership-message
EmandM
approved these changes
Jun 2, 2025
| /// <param name="clientId">the client to check</param> | ||
| /// <param name="networkObject">the <see cref="NetworkObject"/> to check if it is pending show</param> | ||
| [System.Runtime.CompilerServices.MethodImpl(System.Runtime.CompilerServices.MethodImplOptions.AggressiveInlining)] | ||
| internal bool IsObjectVisibilityPending(ulong clientId, ref NetworkObject networkObject) |
| { | ||
| protected const int k_TickRate = 60; | ||
|
|
||
| protected const int k_DefaultTimeTravelFrames = 1000; |
There was a problem hiding this comment.
Just out of interest, what does this change do?
NoelStephensUnity
deleted the
fix/networkshow-ownership-change-extra-ownership-message
branch
NoelStephensUnity
added
the
port:1.x-completed
NoelStephensUnity
added a commit
that referenced
this pull request
Jun 10, 2025
…Show with ChangeOwnership fixes [Backport] (#3493) The PR resolves the issue discovered when investigating the [Forum-1651980 issue](https://discussions.unity.com/t/netcode-for-gameobjects-2-4-0-released/1651980/2). The initial client synchronization pre-serialization preparation was not excluding any spawned `NetworkObject` instances that had a pending visibility update for the client being synchronized. This fix adds this check to that process. [MTTB-1372](https://jira.unity3d.com/browse/MTTB-1372) This addresses the [Forum-Support-1646996](https://discussions.unity.com/t/does-networkshow-assign-ownership-internally-in-unity-netcode/1646996/10) issue where it was determined if you invoke `NetworkObject.NetworkShow` and then immediately invoke `NetworkObject.ChangeOwnership` (or somewhere within the same callstack for that frame) the target client will get an error regarding an unnecessary `ChangeOwnershipMessage`. [MTTB-1337](https://jira.unity3d.com/browse/MTTB-1337) ## Changelog - Fixed: Issue where the initial client synchronization pre-serialization process was not excluding spawned `NetworkObjects` that already had pending visibility for the client being synchronized. - Fixed: Issue where invoking `NetworkObject.NetworkShow` and `NetworkObject.ChangeOwnership` consecutively within the same call stack location could result in an unnecessary change in ownership error message generated on the target client side. ## Testing and Documentation - Includes integration tests. - No Documentation changes were required. <!-- Uncomment and mark items off with a * if this PR deprecates any API: ### Deprecated API - [ ] An `[Obsolete]` attribute was added along with a `(RemovedAfter yyyy-mm-dd)` entry. - [ ] An [api updater] was added. - [ ] Deprecation of the API is explained in the CHANGELOG. - [ ] The users can understand why this API was removed and what they should use instead. --> ## Backport This is a back port of #3488. This is a partial back port of #3468. <!-- If this is a backport: - Add the following to the PR title: "\[Backport\] ..." . - Link to the original PR. If this needs a backport - state this here If a backport is not needed please provide the reason why. If the "Backports" section is not present it will lead to a CI test failure. -->
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.
Summary
MTTB-1337
This addresses the Forum-Support-1646996 issue where it was determined if you invoke
NetworkObject.NetworkShowand then immediately invokeNetworkObject.ChangeOwnership(or somewhere within the same callstack for that frame) the target client will get an error regarding an unnecessaryChangeOwnershipMessage.Because
NetworkObject.NetworkShowinvocations are actually queued and handled at the end of the netcode frame, theChangeOwnershipMessagewould proceed theCreateObjectMesage(NetworkShow). Since theCreateObjectMesageis created at the end of the frame for each queuedNetworkObject.NetworkShowinvocation, when created it would use the new owner assigned earlier and ownership would be applied upon showing/spawning theNetworkObjecton the target client side. Upon theNetworkObjectbecoming "visible"/spawned, the deferredChangeOwnershipMessage(because it was processed before theNetworkObjctwas created) would be handled on the target client side and would detect the newly spawnedNetworkObjectwas already owned by the client and log an error message.CMB Service Update: (Jira ticket required)
This PR includes a modification to the
ChangeOwnershipMessagegenerated when a change in ownership occurs during a CMB Service hosted distributed authority session. These modifications should be compatible with current and previous versions of the CMB Service.The NGO-SDK Change:
ChangeOwnershipMessage.ClientIdsis populated with a list of all clients that should receive the message.ChangeOwnershipMessage.ClientIdCountprovides the number of clients (size of the array).CMB Service Update:
ChangeOwnershipMessagefor a full ownership change.Changelog
NetworkObject.NetworkShowandNetworkObject.ChangeOwnershipconsecutively within the same call stack location could result in an unnecessary change in ownership error message generated on the target client side.Testing and Documentation
NetworkObjectOwnershipTests.NetworkShowAndChangeOwnership.Backport
Partial backport is required (#3493)