Repository navigation
fix: [UpPort] NetworkList changes pending during client synchronization are applied twice - #4216
Conversation
…plied twice Override WriteFieldSynchronization in NetworkList<T> so a client synchronized while local changes are still pending receives the list as of the last sent delta, mirroring NetworkVariable<T>. The pending events are then applied exactly once with the next delta. Fixes #4179
EmandM
changed the title
![u-pr[bot]](https://avatars.githubusercontent.com/in/1201017?s=60&v=4){kind=small}
| { | ||
| if (m_DirtyEvents.Length == 0) | ||
| { | ||
| m_LastSentList.CopyFrom(m_List); |
There was a problem hiding this comment.
An initially empty list populated with Add before SpawnWithOwnership(clientId) now spawns empty on that owner when write permission is Owner. CannotWrite() permits pre-initialization edits, so CaptureLastSentList records the empty list. InternalOnSpawned leaves those events pending because the spawning server lacks owner write permission. WriteFieldSynchronization therefore sends the empty snapshot. NetworkVariableDeltaMessage.Serialize suppresses owner-write deltas to the owner, so the initial values never arrive. Treat pre-initialization edits as initial state rather than a pending-delta snapshot. Add a regression test for prepopulating a list before spawning with remote ownership.
🤖 Helpful? 👍/👎
| { | ||
| // The pending events are sent to the synchronizing client with the next delta, so it must | ||
| // receive the list they apply to. Otherwise it would apply them twice. | ||
| if (m_HasLastSentList && m_DirtyEvents.Length > 0) |
There was a problem hiding this comment.
A client shown a list through NetworkShow while updates are throttled can remain permanently on the pre-change contents. NetworkShow calls ForceSendIfDirtyOnNetworkShow before serializing the spawn. CanSend() blocks sending and resetting the pending events, but ForceSendIfDirtyOnNetworkShow still removes the object from both updater sets. WriteFieldSynchronization now sends m_LastSentList, although the assumed next delta is no longer scheduled. Without another mutation, expiration of the update interval does not correct the client. Keep unsent objects queued after ForceSendIfDirtyOnNetworkShow. Add a throttled NetworkShow regression test with no subsequent mutations.
🤖 Helpful? 👍/👎
There was a problem hiding this comment.
💡 Harness Review
The change adds a pending-state snapshot for list synchronization and a late-join regression test. I examined mutation capture, snapshot cleanup, serialization, and delta delivery paths and found no additional actionable issues beyond those already reported.
Reviewed commit 2d19f42
🤖 Helpful? 👍/👎
There was a problem hiding this comment.
💡 Harness Review
The change adds a pre-delta list snapshot for client synchronization and a late-join regression test. I examined mutation coverage, snapshot cleanup, delta forwarding, and ownership transitions; I found no additional issues beyond those already reported.
Reviewed commit 2d19f42
🤖 Helpful? 👍/👎
Codecov ReportAll modified and coverable lines are covered by tests ✅ @@ Coverage Diff @@
## develop-3.x.x #4216 +/- ##
=================================================
- Coverage 76.91% 76.88% -0.03%
=================================================
Files 159 159
Lines 27603 27665 +62
=================================================
+ Hits 21230 21271 +41
- Misses 6373 6394 +21
Flags with carried forward coverage won't be shown. Click here to find out more.
|
…d updates ForceSendIfDirtyOnNetworkShow removed the NetworkObject from the updater even when NetworkVariableUpdateTraits held back its dirty variables, so those changes were not sent until the variable was modified again. Re-queue the object when anything is still dirty after the forced flush, and add a regression test for showing an object while NetworkList updates are throttled.
|
|
Requires #4236 to land before this PR can pass the unified tests.
Purpose of this PR
Override WriteFieldSynchronization in NetworkList so a client synchronized while local changes are still pending receives the list as of the last sent delta, mirroring NetworkVariable. The pending events are then applied exactly once with the next delta.
Fixes #4179
Jira ticket
n/a
Changelog
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
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?If any boxes above are checked the QA team will be automatically added as a PR reviewer.
Up-port
n/a
Backports
2.x.x PR: #4215