Reduce movement sync bandwidth by throttling player movement packets#2804
Reduce movement sync bandwidth by throttling player movement packets#2804SmokerDoker wants to merge 4 commits into
Conversation
PlayerMovementBroadcaster used to send a PlayerMovement packet every single Update() frame with no throttling at all. It now mirrors the threshold/safety-window pattern already used for vehicles (WatchedEntry): packets are capped at MovementBroadcaster's 30Hz rate, suppressed entirely below a small position/rotation delta, and stopped completely once the player comes to rest (after sending exactly one final packet so remote clients settle instead of coasting on stale velocity). If the player has residual velocity without net displacement (e.g. oscillating in place), a periodic resync still corrects drift. PlayerMovement's delivery method changes from UNRELIABLE_SEQUENCED to RELIABLE_ORDERED_LAST so that final "stopped" packet is guaranteed to arrive instead of possibly being dropped, which would otherwise leave remote players sliding indefinitely. RemotePlayer.UpdatePosition now derives the velocity-correction time window from the actual elapsed time since the previous packet instead of assuming a fixed Time.fixedDeltaTime cadence, since packets can now arrive at irregular intervals (or not at all while idle). Addresses SubnauticaNitrox#2705
GetCorrectedVelocity/GetCorrectedAngularVelocity compute a "keep nudging towards the target" velocity meant to be re-evaluated on the next packet. That assumption held while every frame sent a packet, but now that a stopped player sends exactly one final packet and then goes silent, any left-over non-zero result from that last correction (e.g. residual jitter between the reported and actual transform) got baked into the Rigidbody's velocity/angularVelocity and kept being integrated by physics forever, since nothing was left to correct it back to zero. RemotePlayer.UpdatePosition now recognizes a (near) zero incoming velocity as the sender explicitly reporting a full stop, and in that case snaps directly to the reported position/rotation and hard-zeroes both velocities instead of running the generic corrective-velocity math. Found via manual playtesting of the previous movement-sync commit.
There was a problem hiding this comment.
This feature will be really helpful for people. Thanks for taking the time to fix it!
There's some delay when moving the player for the first time. Would it be possible to not have thresholds and detect mouse movement / WASD input instead? As the trigger to send data?
… still correctionTime was derived from the raw elapsed time since the last received packet. After a player has been standing still for a while (no packets sent by design), that elapsed time equals the entire idle duration. The first "moving again" packet reports only a tiny position delta (just past the movement threshold), so dividing it by the long idle duration produced a near-zero corrective velocity instead of the real reported speed - the remote player would visibly creep for one frame before self-correcting on the next packet ~30ms later. Since a stopped player's remote transform is already snapped exactly to rest, the idle duration never represented real drift to catch up on in the first place. RemotePlayer now remembers whether the previous update left the player stopped, and if so uses a normal short correction window for the next (resuming) packet instead of the stale idle time. Reported by a reviewer on PR SubnauticaNitrox#2804.
|
Thanks for the feedback! I looked into it, and I don't think switching the trigger to raw WASD/mouse input is the right fix, for a couple of reasons: Not all movement is input-driven — water currents, falling, knockback from creatures, and moving while inside a moving vehicle/sub wouldn't trigger a send at all if we only listened for key/mouse input, so remote players would appear frozen during all of that instead of just idle. Fixed in the latest commit: the remote player now remembers when it was left standing still, and uses a normal correction window for the packet that resumes movement instead of the stale idle time. Should be gone now — let me know if you still see it after this. |
dartasen
left a comment
There was a problem hiding this comment.
Will need an IG test with the team
… still correctionTime was derived from the raw elapsed time since the last received packet. After a player has been standing still for a while (no packets sent by design), that elapsed time equals the entire idle duration. The first "moving again" packet reports only a tiny position delta (just past the movement threshold), so dividing it by the long idle duration produced a near-zero corrective velocity instead of the real reported speed - the remote player would visibly creep for one frame before self-correcting on the next packet ~30ms later. Since a stopped player's remote transform is already snapped exactly to rest, the idle duration never represented real drift to catch up on in the first place. RemotePlayer now remembers whether the previous update left the player stopped, and if so uses a normal short correction window for the next (resuming) packet instead of the stale idle time. Reported by a reviewer on PR SubnauticaNitrox#2804.
dartasen
left a comment
There was a problem hiding this comment.
Tested the PR with 6 people today
Cause some regressions inside Subroots :
https://streamable.com/zh3d9v
But locally with two players it's pretty easy to reproduce the regression :
Enter a base with one player => the second player will see the player far away from it's real location
|
@dartasen Okay thanks for the test, im gonna take a look to see what causes this. |
PlayerMovementBroadcaster converted the local player's position to base-local space and then immediately called subRootTransform.TransformPoint() on it, which transforms a local point back to world space - mathematically undoing the preceding conversion. So the broadcaster was actually sending world position while RemotePlayer.UpdatePosition (correctly) expects base-local position for SubRoot.isBase, converting it back to world a second time and producing a wildly wrong result whenever a player was inside a base. This coordinate mismatch has existed since the position-subtraction lines were reintroduced in bec947f without removing the leftover TransformPoint call, but went unnoticed because movement packets used to be sent every frame - any bad correction was immediately overwritten by the next packet. The recent threshold-based throttling sends corrective packets far less often, so a single bad computation now produces a visible, lasting jump instead of a one-frame flicker. Restricted the conversion to SubRoot.isBase (mirroring the receiver's own condition) and dropped the erroneous TransformPoint call, so position/rotation sent for a base are genuinely base-local, and non-base subroots (e.g. Cyclops) keep going through unconverted as before.
|
@dartasen Thanks for the detailed report — found the cause, and it explains exactly why it only happened inside bases.
This coordinate mismatch has actually been sitting in the code since August 2024 (an unrelated commit re-added the position-subtraction lines without removing the leftover Fixed in Could you have your 6-person group re-test the same base scenario when you get a chance? Would be great to confirm this actually resolves it in practice. |
Test Results258 tests +2 253 ✅ ±0 9s ⏱️ -1s Results for commit 3c0c0bb. ± Comparison against base commit b8c4c42. This pull request removes 2 and adds 4 tests. Note that renamed tests count towards both. |
Linked Issues / Context
Addresses #2705
Description of Change
Movement data was being synced far more often than necessary:
PlayerMovementBroadcastersent aPlayerMovementpacket on every singleUpdate()frame, uncapped, regardless of whether the player was actually moving. Velocity was already part of the packet, so the main remaining bandwidth win was to stop sending when nothing meaningful changed.PlayerMovementBroadcasternow mirrors the threshold/safety-window pattern already used for vehicles (WatchedEntry):MovementBroadcaster.BROADCAST_PERIOD) instead of sending every rendered frame.WatchedEntry) still corrects any drift accumulating on remote machines — but only while not standing still, per the issue's request.PlayerMovement's delivery method changes fromUNRELIABLE_SEQUENCEDtoRELIABLE_ORDERED_LAST("last packet only", but guaranteed delivery — same as already used forPlayerStats), so that final "stopped" packet can't silently get dropped, which would otherwise leave remote players sliding indefinitely.RemotePlayer.UpdatePositionderives its velocity-correction time window from the actual elapsed time since the previous packet instead of assuming a fixedTime.fixedDeltaTimecadence, since packets can now arrive at irregular intervals (or not at all while idle).Additionally, while play-testing this change, we found that
MovementHelper.GetCorrectedVelocity/GetCorrectedAngularVelocitycompute a "keep nudging towards target" velocity that's only ever exactly zero by coincidence — under the old every-frame-packet model this self-corrected within one frame, but once a stopped player stops sending packets entirely, any left-over non-zero result from the last correction got applied by physics forever, causing remote players to drift or spin in place indefinitely after coming to rest.RemotePlayer.UpdatePositionnow recognizes a (near) zero incoming velocity as an explicit "I've stopped" signal and snaps directly to the reported position/rotation with both velocities hard-zeroed in that case, instead of running the generic corrective-velocity math.Validation
Manually tested with 2 clients (host + client) over an extended session:
RemotePlayer.UpdatePositionfix above after an initial test revealed the endless-drift/spin bug described there).Given this touches core movement netcode, further testing (in-game, different network conditions, vehicles/base entry edge cases) by maintainers/other players before merging is still recommended.
PR Checklist
masterat the time of opening🤖 This code was created with the help of AI.