[EmandM]

Purpose of this PR

Fixes: #4179
Continues: #4200

If a client is synchronized while a NetworkList<T> still has unsent changes, the synchronization snapshot already contains those changes. On the next tick the same events are sent to the new observer again. Add, Insert, Remove, RemoveAt and Set are not idempotent, so the late-joining client ends up with duplicate entries, and later index-based events land on the wrong elements. #3081 fixed this for NetworkVariable<T> through WriteFieldSynchronization, but NetworkList<T> never overrode it.

NetworkList<T> now overrides WriteFieldSynchronization, mirroring NetworkVariable<T>. While local changes are pending, it writes the list as it was before the first of them. The pending events then bring the client up to date exactly once with the next delta.

Jira ticket

n/a

Changelog

• Fixed: Issue where a client synchronized while a NetworkList<T> still had unsent changes applied those changes twice, resulting in duplicate entries and index-based events landing on the wrong elements on that client.

Documentation

• No documentation changes or additions were necessary.

Testing & QA (How your changes can be verified during release Playtest)

Functional Testing

Manual testing :

• Manual testing done

Automated tests:

• Covered by existing automated tests
• Covered by new automated tests

Does the change require QA team to:

• Review automated tests?
• Execute manual tests?
• Provide feedback about the PR?

Up-port

Up-ported in #4216

Backports

Not needed.

[u-pr]

💡 Harness Review

The snapshot approach addresses duplicate delta application, but it relies on a delivery guarantee that the update scheduler does not always preserve.

_{Reviewed commit 551b825}

_{🤖 Helpful? 👍/👎}

[u-pr]

Showing an object while list updates are throttled can leave the new observer permanently missing pending edits. For example, add an item and call NetworkShow before MinSecondsBetweenUpdates expires. ForceSendIfDirtyOnNetworkShow removes the object from the updater even when CanSend() prevents sending or clearing its dirty events. WriteFieldSynchronization then sends m_LastSentList, but subsequent ticks cannot deliver those events unless another mutation requeues the object. Previously, synchronization sent the current list. Keep the object queued when pending events cannot be sent. Add a regression test for throttled updates during NetworkShow.

_{🤖 Helpful? 👍/👎}

[codecov-github-com]

Codecov Report

All modified and coverable lines are covered by tests ✅

@@                Coverage Diff                @@
##           develop-2.0.0    #4215      +/-   ##
=================================================
+ Coverage          74.20%   74.23%   +0.02%     
=================================================
  Files                172      172              
  Lines              28200    28258      +58     
=================================================
+ Hits               20926    20976      +50     
- Misses              7274     7282       +8     

Flag	Coverage Δ
NGOv2_project_testproject_ubuntu	73.83% <100.00%> (-0.04%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...Runtime/NetworkVariable/Collections/NetworkList.cs	81.49% <100.00%> (+1.73%)	⬆️

... and 2 files with indirect coverage changes

Components	Coverage Δ
com.unity.netcode.gameobjects	74.23% <79.03%> (+0.02%)	⬆️

ℹ️ Need help interpreting these results?

[quantum-shubham]

Thanks for taking this over, @EmandM!

I looked into the P1 from the harness review and it's valid. ForceSendIfDirtyOnNetworkShow drops the NetworkObject from the updater even when NetworkVariableUpdateTraits hold back its dirty variables, so those changes are not sent until the variable is modified again. This was already the case for existing observers. With the NetworkList change, the newly shown client is affected too.

Follow-up commit on my fork: quantum-shubham@3df3e551

• Re-queues the object when anything is still dirty after the forced flush, so a later tick sends it.
• Adds NetworkListTests.NetworkShowWithThrottledPendingChanges, the regression test the review asked for: hide, throttle, modify, show, release, verify all clients.
• Adds a changelog line.

Tested locally on 6000.0.72f1 (PlayMode, testproject): NetworkListTests, NetworkShowHideTests and NetworkVariableTraitsTests, 47/47 passing. Without the updater change, the new test fails in Host, DAHost and Server (Expected: 5, Have: 0).

Could you cherry-pick it onto this branch?