fix: networktransform interpolation render time up port - #4135
Open
NoelStephensUnity wants to merge 5 commits into
Open
fix: networktransform interpolation render time up port#4135NoelStephensUnity wants to merge 5 commits into
NoelStephensUnity wants to merge 5 commits into
Conversation
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.
6 tasks
NoelStephensUnity
marked this pull request as ready for review
August 19, 2026 16:47
Codecov ReportAttention: Patch coverage is
@@ Coverage Diff @@
## develop-3.x.x #4135 +/- ##
=================================================
+ Coverage 78.01% 78.03% +0.01%
=================================================
Files 153 153
Lines 26254 26254
=================================================
+ Hits 20483 20486 +3
+ Misses 5771 5768 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose of this PR
This is an up-port of #4133
Jira ticket
TODO: add ticket
Changelog
NetworkTransforminterpolated towards a point in time taken from the local clock rather than the server clock that state updates are stamped on, which starved the interpolator on clients and reduced interpolation to snapping between state updates.NetworkTransform.GetTickLatencyInSecondsreturned a time based on the local clock instead of the server clock used for interpolation.Documentation
Testing & QA (How your changes can be verified during release Playtest)
New integration test measures how far behind
ServerTimethe state being interpolated towards was sent. That value can never be less than the tick latency, since the render time isServerTimeminus 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 fullNetworkTransformplaymode 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 :
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?Up-port
Not needed. This is an up-port.
Backports
Not needed.