Skip to content

fix: networkshow client synchronization duplicate players #3488

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

NoelStephensUnity
Copy link
Collaborator

@NoelStephensUnity NoelStephensUnity commented Jun 6, 2025

The PR resolves the issue discovered when investigating the Forum-1651980 issue. 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

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.

Testing and Documentation

  • Includes the PlayerSpawnObjectVisibilityTests.
  • No documentation changes or additions were necessary.

Backport

This requires a backport to v1.0 (#3493)

This fixes the issue where spawning a prefab that is configured to be spawned with no observers just prior to synchronizing a client and showing the spawned object to the client about to be synchronized within a NetworkBehaviour attached to the newly spawn prefab instance would result in a duplicate creation of the same prefab instance due to NetworkShow being deferred until the end of the frame.

This fix excludes any spawned NetworkObjects that have pending visibility for a client that is being synchronized.
adding white space
PlayerSpawnObjectVisibilityTests integration test to validate this fix.
@NoelStephensUnity NoelStephensUnity added the port:1.x-needed This issue needs to be ported to 1.X branch label Jun 6, 2025
@NoelStephensUnity NoelStephensUnity marked this pull request as ready for review June 9, 2025 20:42
@NoelStephensUnity NoelStephensUnity requested a review from a team as a code owner June 9, 2025 20:42
@NoelStephensUnity NoelStephensUnity enabled auto-merge (squash) June 9, 2025 21:00
@NoelStephensUnity NoelStephensUnity merged commit d4a9810 into develop-2.0.0 Jun 10, 2025
35 checks passed
@NoelStephensUnity NoelStephensUnity deleted the fix/networkshow-client-synchronization-duplicate-players branch June 10, 2025 18:00
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
Labels
port:1.x-completed This issue was ported to 1.X branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants