| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
Adds an integration test that measures how far behind the server clock the state a non-authority NetworkTransform is interpolating towards was sent. Only states sent at or before the render time are eligible to be interpolated towards, and the render time is the server clock minus the tick latency, so that measurement can never be less than the tick latency. It currently is, and goes negative, meaning the interpolator is chasing a state that the server clock says has not happened yet. An in-process integration test has effectively no round trip time, so the test first widens the client's local time buffer to separate LocalTime and ServerTime by a known amount and waits for that separation to take hold. Without it the two clocks sit close enough together that the test would pass regardless of which one the render time is derived from. This commit contains the test only, so it can be run against an unfixed tree.
A NetworkTransform state's SentTime comes from its NetworkTick, which is a server tick, but the render time the interpolators were given was derived from LocalTime. That mixes two clocks. LocalTime leads ServerTime, so subtracting the tick latency from it lands the render time back at approximately ServerTime rather than a whole tick latency behind it, and a state's SentTime is floored to a tick boundary on top of that. The render time therefore sat at or ahead of the newest state that could exist and the interpolator had nothing to interpolate towards. Measuring from ServerTime makes the offset the whole tick latency instead of whatever is left of it, and is self correcting: as the round trip time grows the tick latency grows and the render time moves further back with it. This also matches the rest of the component, which already resets the interpolators using ServerTime. This is a no-op on a host or server, where the two clocks are the same, so it only affects clients. GetTickLatencyInSeconds returns an absolute time rather than a duration and had the same defect, so it now derives from ServerTime as well. GetTickLatency is left alone because it returns a tick count rather than a point in time.
Comment and changelog wording only, no behavioral or test logic changes. Trims the explanation in UpdateInterpolation from twenty one lines to six and drops the measurement anecdote and the unfilled Jira placeholder, keeping the reason the server clock is the correct one to measure from. Shortens the test's remarks and constant comments to match the density of the surrounding tests. The removed detail, the measurements behind the fix, and the metrics that were tried and rejected while building the test are recorded outside the repository.
Codecov ReportAll modified and coverable lines are covered by tests ✅ @@ Coverage Diff @@
## develop-2.0.0 #4133 +/- ##
=================================================
- Coverage 73.94% 73.90% -0.05%
=================================================
Files 172 172
Lines 28099 28100 +1
=================================================
- Hits 20779 20768 -11
- Misses 7320 7332 +12
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes
|
Sorry, something went wrong.
There was a problem hiding this comment.
The interpolation clock change aligns render-time calculations with the timestamps assigned to received states, but the altered public helper remains inconsistent with its documented duration contract.
Reviewed commit 347eea6
🤖 Helpful? 👍/👎
Sorry, something went wrong.
GetTickLatencyInSeconds returned TimeTicksAgo(...).Time, which is an absolute network timestamp rather than a duration, so the value grew for as long as the session ran. It is documented as returning the tick latency in seconds, and NetworkTimeSystem.TickLatency points at it as a way to inspect that latency, so the contract was misleading regardless of which clock it was measured from. It now returns the tick count multiplied by the tick interval. This also takes the clock question out of this method entirely, since a duration does not reference LocalTime or ServerTime. The change to derive interpolation render time from ServerTime now applies only to UpdateInterpolation. Adds integration tests covering the documented contract: the value tracks the tick latency rather than elapsed time, and lengthens by exactly the tick interval for each tick of additional buffering. Both fail against the previous implementation, the second regardless of how long the session has run, since buffering more ticks used to make the reported latency smaller.
NetworkTimeSystem.TickLatency is recomputed from the averaged round trip time and can legitimately change mid-run. Both tests assumed it would not, and one failed on macOS when it moved from two ticks to three, reporting the value as having gone from 0.0666s to 0.1s. The duration is now only held to being unchanged across samples where the tick latency itself did not change, and the buffer offset test accounts for any tick latency movement between its two samples so that only the buffering is held to an exact figure. Both still fail against the previous absolute timestamp implementation.
| Back | FazBrowse Home | New Git URL |
Purpose of this PR
A NetworkTransform state's SentTime comes from its NetworkTick — a server tick — but the render time the interpolators were given was derived from LocalTime. Since LocalTime already leads ServerTime by roughly the tick latency, subtracting the tick latency from it put the render time back at approximately ServerTime instead of a buffer behind it. Clients were therefore asked to render a point in time at or ahead of the newest state that could exist, leaving the interpolator with nothing to interpolate towards.
Now derived from ServerTime, which also matches the rest of the component — NetworkTransform already resets its interpolators using ServerTime.
The tradeoff reviewers should weigh: non-authority instances now sit a full tick latency behind rather than approximately at the present. That is roughly 75ms of additional visual latency, in exchange for actual interpolation instead of snapping between state updates. No-op on host/server, where both clocks are the same.
GetTickLatencyInSeconds now returns the actual tick latency in seconds.
Jira ticket
TODO: add ticket
Changelog
Documentation
Testing & QA (How your changes can be verified during release Playtest)
New integration test measures how far behind ServerTime the state being interpolated towards was sent. That value can never be less than the tick latency, since the render time is ServerTime minus the tick latency and only states sent at or before it are eligible.
Validated in both directions on develop-2.0.0: without the fix both fixtures fail, reporting the target as −0.899 and −1.059 ticks — the interpolator chasing a state the server clock says has not happened yet. With the fix both pass. The full NetworkTransform playmode suite is green (3942/3942).
The test widens the client's local time buffer before measuring, because an in-process test has no round trip time to separate the two clocks and would otherwise pass regardless of which one is used. It waits for that separation to take hold and fails if it never does, so it cannot silently become a no-op.
For release playtest, the thing to look at is client-side smoothness of moving networked objects, and whether the added latency is acceptable.
Functional Testing
Manual testing :
Automated tests:
Does the change require QA team to:
Up-port
Up-port: #4135
Required.
Backports
Not needed.