Skip to content

Re-arm the replicate start delay when inputs resume - #1094

Open
jimdroberts wants to merge 1 commit into
FirstGearGames:mainfrom
jimdroberts:fix/replicate-start-delay
Open

jimdroberts wants to merge 1 commit into
FirstGearGames:mainfrom
jimdroberts:fix/replicate-start-delay

Conversation

@jimdroberts

Copy link
Copy Markdown
Contributor

Problem

PredictionManager.StateInterpolation is documented as "How many states to try and hold in a buffer before running them". For the server running a client's inputs (and for clients using Appended state order), that buffer is built by a start delay. When inputs start arriving into an empty queue, they are held for StateInterpolation ticks before the first one runs. Since 4.6.19R that delay never starts. The server runs each input on the tick it arrives, so the queue holds no buffer when an owner starts sending after being idle. A packet that arrives a tick late then finds the queue empty, and the server runs default data for that tick.

Cause

NetworkBehaviour.Prediction.cs on main:

  • L207: _replicateCurrentStartTick is initialised to TimeManager.UNSET_TICK (0).
  • L641-644 (Replicate_NonAuthoritative): it is set back to UNSET_TICK whenever localTick >= _replicateCurrentStartTick.
  • L1177-1178 (Replicate_EnqueueReceivedReplicate) is the only place it is armed:
    if (_replicateCurrentStartTick != TimeManager.UNSET_TICK && (isServer || isAppendedOrder) && startQueueCount == 0 && replicatesQueue.Count > 0)
        _replicateCurrentStartTick = _networkObjectCache.TimeManager.LocalTick + pm.StateInterpolation;
    The field is only non-UNSET after this assignment has already run, so the assignment can never run.

As a result, localTick >= _replicateCurrentStartTick at L641 is always localTick >= 0. The start-delay adjustment in Replicate_SendNonAuthoritative (L1074-1075) is also never applied.

The guard arrived in 4.6.19R (e005324), which renamed _replicateStartTick to _replicateCurrentStartTick. Up to and including 4.6.18 the line was if ((isServer || isAppendedOrder) && startQueueCount == 0 && replicatesQueue.Count > 0). The 4.6.19R changelog does not mention removing the delay. The field, its three uses, the comment above L1177 ("start the queued inputs delay") and the StateInterpolation tooltip all still describe the delay, so the guard looks unintentional.

Fix

Remove the _replicateCurrentStartTick != TimeManager.UNSET_TICK && condition. This restores the ≤4.6.18 behaviour: whenever the queue goes from empty to non-empty, the start tick is set to LocalTick + StateInterpolation.

I chose removal over flipping the guard to == UNSET_TICK because the field is not reset in ResetState_Prediction/ClearReplicateCache. If a replicate cache is cleared while a delay is pending, a stale start tick survives. The unconditional re-arm overwrites it on the next input. With == the stale value would stay, and L1075 could then compute start - LocalTick on a start tick that is already in the past, which wraps the uint.

If disabling the delay was intentional, I'm happy to change this PR so it removes the field, its uses and the stale comments instead.

Evidence

A throwaway model of the two methods (Replicate_EnqueueReceivedReplicate arming, plus Replicate_NonAuthoritative consuming one entry per tick with DropExcessiveReplicates on) was also run. The owner sends one input per tick in bursts of 30-120 ticks with 10-60 idle ticks between them, latency of 5 ticks + 0-3 ticks jitter, 2% loss, redundancy = StateInterpolation + 1 = 3, and StateInterpolation = 2. Totals over 5 seeds × 200k ticks:

main (guard: start != UNSET)         armed=     0 inputs_run= 682914 empty-queue ticks mid-burst= 16761 start-delay ticks=     0
guard removed (4.6.12 behaviour)     armed= 15393 inputs_run= 682914 empty-queue ticks mid-burst=  1651 start-delay ticks= 30786

On main the delay is never armed, and the server hits an empty queue mid-stream about 10× as often. The fix trades that for the documented delay: StateInterpolation ticks each time a stream starts. This is a model of the queue logic, not a FishNet runtime measurement.

Behavior change

The documented one. When an owner's inputs resume into an empty queue, the server (and Appended-order clients for spectated objects) waits StateInterpolation ticks before running them, as in 4.6.18 and earlier. The "run tick of last entry" sent to spectators includes that delay again.

The only assignment of _replicateCurrentStartTick was guarded by
_replicateCurrentStartTick != UNSET_TICK, but the field starts unset and is
reset to unset once reached, so the StateInterpolation start delay never armed
after 4.6.19. Remove the guard, restoring the 4.6.18 behaviour of delaying
inputs that resume into an empty queue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant