Mirror icon indicating copy to clipboard operation
Mirror copied to clipboard

Send Interval / Sync Interval Inaccuracy

Open jaf6802 opened this issue 2 years ago • 14 comments

Describe the bug The way send interval is implemented for Client and Server in NetworkTransform is quite inaccurate in terms of matching the desired send rate, up to 33% in some of my tests. Which I was able to visually notice some smoothness differences when I testing higher network latency and 60Hz vs 30Hz on a NetworkTransform.

// Culprit for Server 
if (NetworkTime.localTime >= lastServerSendTime + sendInterval)
{
    lastServerSendTime = NetworkTime.localTime;

This type of time checking for when to start the next interval, accumulates a lot of error because games are run in frames, and almost every time you pass this conditional it will be late by zero to few milliseconds or even up to an entire frame.

Game FPS Specified Send Interval (Hz) Actual Send Interval (Hz) Interval Error %
~160 60 ~54 10%
60 60 ~40 33%
60 30 ~24 20%

I acquired this data by adding some debug code to keep track of all sends and then processing the data afterwords. If you look at the attached unchanged source data, you can also see that the send timings or pacing is inconsistent as well.

I noticed this bug in NetworkTransformBase, but quickly looking through the source code, I believe NetworkAnimator, SyncVars in NetworkBehaviors, and probably other code that have similar interval checks are affected.

[IMPORTANT] How can we reproduce the issue, step by step: Simplest reproduction would be to create a new networked scene with a GameObject that has a NetworkTransform. Create a local host game and collect send interval data from adding debug code to NetworkTransform.

Example Debug Code:

        private List<double> times = new List<double>(2048);

        void OnDestroy()
        {
            if (times.Count == 0)
                return;

            int count = times.Count;
            double start = times[0];
            double end = times[count - 1];
            Debug.Log($"Elapsed Time(sec): {end - start:000.0000}, Send Count: {count}, Actual Send Rate Hz: {(Double)count / (end - start):000.000}");

            string results = "Send Time(sec)   |  Difference from Previous\n";
            results += "--------------------------------------------\n";
            results += $"{start:000.0000}         |        N/A\n";

            double avg = sendInterval;
            double max = sendInterval;
            double min = sendInterval;

            for (int i = 1; i < times.Count; i++)
            {
                var time = times[i];
                var diff = time - times[i - 1];
                results += $"{time:000.0000}         |      {diff:0.0000}\n";
                avg += diff;

                if (diff > max)
                    max = diff;

                if (diff < min)
                    min = diff;
            }
            avg = avg / count;
            Debug.Log($"Interval Differences(sec): Average: {avg:0.0000}, Max: {max:0.0000}, Min: {min:0.0000}");
            Debug.Log(results);
        }

Add times.Add(NetworkTime.localTime); to appropriate location in UpdateServer() and UpdateClient().

Expected behavior Send interval / sync intervals should be closer to the actual specified intervals.

Desktop (please complete the following information):

  • OS: All
  • Build target: Standalone
  • Unity version: 2021.2
  • Mirror branch: Master

Possible Solution: I quickly came up with possible solution but there may a better option. A send interval error accumulator could be added to keep track of all late sends, and then that error accumulator can be used to allow certain sends to run slightly early. This will allow the late and early sends to average out, and allow for more consistent send interval pacing.

Solution Data:

Game FPS Specified Send Interval (Hz) Actual Send Interval (Hz) Interval Error %
60 60 ~59.1 1.5%
60 30 ~29.8 0.7%

If you look at the attached solution data, you can also see that the send timings or pacing are much more consistent with specified interval.

Example Solution: Note: sendErrorMax affects results and optimal value should be determined

public abstract class NetworkTransformBase : NetworkBehaviour
{
            ...


        /// Accumulate error from being late on sends due to frame timings.
        /// Use accumulated error to allow some sends to run slightly early to average out late sends.
        ///
        /// Probably should initialize this with some error so it doesn't have to wait for it to accumulate at start.
        private double sendErrorAccumulator;

        /// Clamp sendErrorAccumulator to this value to prevent accumulating big errors from low fps.
        private double sendErrorMax;

        void Awake()
        {
            // Need max that isn't too big or too small, relative to frame timings.
            //
            // NOTE: Chose send interval out of thin air. 50% seems better than 25%
            sendErrorMax = 0.5d * sendInterval;
            sendErrorAccumulator = sendErrorMax;
        }

        // Modify this
        void UpdateServer()
        {
            if (NetworkTime.localTime >= lastServerSendTime + sendInterval - sendErrorAccumulator &&
                (!clientAuthority || IsClientWithAuthority))
            {
                   ...

                // Clamp in case of low fps drops to prevent accumulating a huge error
                sendErrorAccumulator += NetworkTime.localTime - (lastServerSendTime + sendInterval);
                sendErrorAccumulator = Math.Clamp(sendErrorAccumulator, 0d, sendErrorMax);

                lastServerSendTime = NetworkTime.localTime;

              ...
        }  

        // Modify this
        void UpdateClient()
        {
            if (IsClientWithAuthority)
            {
                if (!NetworkClient.ready) return;

                if (NetworkTime.localTime >= lastClientSendTime + sendInterval - sendErrorAccumulatorl)
                {
                   ....

                // Clamp in case of low fps drops to prevent accumulating a huge error
                sendErrorAccumulator += NetworkTime.localTime - (lastClientSendTime + sendInterval);
                sendErrorAccumulator = Math.Clamp(sendErrorAccumulator, 0d, sendErrorMax);

                lastClientSendTime = NetworkTime.localTime;

              ...
     }

Data 60fpsCapped_60hzSend_unchanged_source_data.txt

60fpsCapped_60hzSend_error_accumulator_data_0.txt

60fpsCapped_30hzSend_unchanged_source_data.txt

60fpsCapped_30hzSend_error_accumulator_data_0.txt

jaf6802 avatar Mar 11 '22 22:03 jaf6802

thanks for detailed bug report. @jaf6802 just to be clear, is the problem:

  • NetworkTime.localTime being constant during the frame? for example, calling it twice in the same frame will always give the same time.
  • or is it the way we calculate the intervals with the >= last + interval ?

miwarnec avatar Mar 19 '22 12:03 miwarnec

The latter, "the way we calculate the intervals with the >= last + interval"

jaf6802 avatar Mar 19 '22 20:03 jaf6802

if we want to make sure we don't "loose" any extra time due to only updating every x ms, how about this?

if (NetworkTime.localTime >= lastSendTime + sendInterval) {
    /* ... do send logic ... */
    lastSendTime += sendInterval;
}

of course that might cause send spam if the server freezes for some reason, so you might want to add a threshold where it "resets" instead of adding

imerr avatar Mar 23 '22 10:03 imerr

This seems like a misunderstanding of the sync interval.

sync interval is a variable interval not a fixed interval (think update vs fixedupdate)

As sync interval is designed atm it is not meant to have a constant HZ, It means "dont sync more often than this interval"

James-Frowen avatar Mar 24 '22 19:03 James-Frowen

syncInterval tooltip:

"Time in seconds until next change is synchronized to the client. '0' means send immediately if changed. '0.5' means only send changes every 500ms.\n(This is for state synchronization like SyncVars, SyncLists, OnSerialize. Not for Cmds, Rpcs, etc.)"

Just because it is variable does not mean there should have such a large error in the actual sync interval. The main source of error should be coming from network latency and jitter, and minimal from the from sync code.

NetworkAnimator also uses syncInterval to determine when to send updates. This component is visual in nature, and updating at 40Hz instead of a specified 60Hz, is probably detectable in visual smoothness. SyncVars may also used for data that is displayed visually. I stumbled upon this bug because of some subtle visual smoothness issues in NetworkTransform in the first place.

jaf6802 avatar Mar 24 '22 22:03 jaf6802

if we want to make sure we don't "loose" any extra time due to only updating every x ms, how about this?

if (NetworkTime.localTime >= lastSendTime + sendInterval) {
    /* ... do send logic ... */
    lastSendTime += sendInterval;
}

of course that might cause send spam if the server freezes for some reason, so you might want to add a threshold where it "resets" instead of adding

I did some quick tests with the following:

if (NetworkTime.localTime >= lastServerSendTime + sendInterval) {
    /* ... do send logic ... */
    lastServerSendTime += sendInterval;
    
     // Reset send time to current time if they have drifted at least an interval apart to prevent spam sends
     if (NetworkTime.localTime - lastServerSendTime > sendInterval)
         lastServerSendTime = NetworkTime.localTime;
}

It seems comparable in results the error accumulator solution. It seemed to have slightly a better average but with slightly more deviations from average. Also would save some bytes by not needing an additional field.

Data: 160fpsUncapped_60hzSend_imerr_1.0threshold_data_0.txt 60fpsCapped_60hzSend_imerr_1.0threshold_data_0.txt 60fpsCapped_30hzSend_imerr_1.0threshold_data_0.txt

jaf6802 avatar Mar 24 '22 22:03 jaf6802

Just because it is variable does not mean there should have such a large error in the actual sync interval.

there is not a larger error, you are taking sync interval as a fixed interval, it is like Time.deltaTime vs Time.FixedDeltaTime.

Think of this extreme, what if you set sendInterval = 1/500f would you then expect it to sync 500 times a second? or just every update.

The tooltip is misleading, because even if sendInterval = 0 changes are not sent immediately, they will be sent at the end of the frame. sendInterval = 0 just means send it every frame (if changed)

James-Frowen avatar Mar 24 '22 23:03 James-Frowen

Given the examples and data above, is it not an error because it was designed to work that way? There are two enhancements in this thread that seem simple to implement, are there downsides of making these enhancements? I think its beneficial for a developer to be able to specify an ideal sync rate and match that reasonably when possible.

jaf6802 avatar Mar 25 '22 23:03 jaf6802

They might sound simple, but there are potential problems too.

Lets say sync rate is 20hz. What if the server lags for a few seconds and is only to send 10 updates, the lastServerSendTime would then be a lot larger than sendInterval so would cause updates every frame. This could lead to a wildly fluctuating update rate, where 1 second might send 10 updates and next would send 30 updates but in long term is 20hz.

This is ofc not ideal, if server to slow to 10 updates because it is struggling you would want it to return to 20hz afterward it recovers.

James-Frowen avatar Mar 26 '22 01:03 James-Frowen

The ideal setup is to have have a custom update loop that runs Game Update and then network update.

This is what systems like Client side prediction do, where you create a custom Update loop with similar rules to unity's fixedUpdate.

timer += Time.UnscaledDetlaTime;
while (timer > NetworkFixedDeltaTime)
{
    timer -= NetworkFixedDeltaTime;
    tick++;

    NetworkReceive();       // receive new data (eg player inputs)
    CustuomGameUpdate();    // Runs users code here
    PhysicsUpdate();        // Simulate physics
    NetworkUpdate();        // Send changed state
}

This way every game state update is followed by a network update. But this requires custom update loops and is beyond what the SyncVar system can do

James-Frowen avatar Mar 26 '22 01:03 James-Frowen

They might sound simple, but there are potential problems too.

Lets say sync rate is 20hz. What if the server lags for a few seconds and is only to send 10 updates, the lastServerSendTime would then be a lot larger than sendInterval so would cause updates every frame. This could lead to a wildly fluctuating update rate, where 1 second might send 10 updates and next would send 30 updates but in long term is 20hz.

This is ofc not ideal, if server to slow to 10 updates because it is struggling you would want it to return to 20hz afterward it recovers.

Could you explain some of the other problems? The particular problem described was handled by both of the specified enhancements.

// Clamp in case of low fps drops to prevent accumulating a huge error
sendErrorAccumulator += NetworkTime.localTime - (lastServerSendTime + sendInterval);
sendErrorAccumulator = Math.Clamp(sendErrorAccumulator, 0d, sendErrorMax);
// Reset send time to current time if they have drifted at least an interval apart to prevent spam sends
if (NetworkTime.localTime - lastServerSendTime > sendInterval)
    lastServerSendTime = NetworkTime.localTime;

jaf6802 avatar Mar 26 '22 19:03 jaf6802

It makes the system a lot more complicated, making it semi-fixed but not really fixed rate because there is a max set by framerate.

I dont see the end goal. It seems like you want a fixed update type thing, but are using a variable (frame rate dependent) value to do that.

If you want to sync a NetworkTransform inline with physics then you should use FixedUpdate (or similar custom system), Which is how it is done in most tick based games.

James-Frowen avatar Mar 26 '22 21:03 James-Frowen

It makes the system a lot more complicated, making it semi-fixed but not really fixed rate because there is a max set by framerate. The current system already works like this. If you set sync interval to 1/30 and the game has 60fps, it will send ~24 syncs per second. If the game has 20fps it will send ~20 sync per second. All this enchantment does is make the scenario in which fps is >= sync interval, actually send 30 syncs per second. The enhancement suggested by imerr is 3 lines of simple code, its not making the system a lot more complicated.

I dont see the end goal.

The goal is for networking code to send / sync at the rate specified by the developer when possible. For the rate to be approximately the same, when possible, regardless of the fps of the client/server running the game. Why is that not a desirable goal?

It seems like you want a fixed update type thing, but are using a variable (frame rate dependent) value to do that.

Fixed update is used for physics simulation, it does not always run at a fixed interval. When fps is lower than the physics time step, it will run multiple fixed timesteps per frame to catch up but still says the deltaTime is equal to fixedDeltaTime even though it isn't. This allows for reliability, smoothness, and predictability in the physics simulations. If network updates were tied to fixed update then in the scenario in which fps was low, it would send multiple duplicate network updates in the same frame. Then there are also non-physics changes that could occur in update. So the ideal time to send the updates is at the end frame which is what mirror does, but there is some inaccuracy in the logic for "should we send/sync now?".

I'm going to submit a PR for this issue. You guys can choose to accept it or not there. I really wasn't expecting this much push back on this.

jaf6802 avatar Mar 27 '22 05:03 jaf6802

Fixed update is used for physics simulation, it does not always run at a fixed interval.

Yes it does, Every fixed update will have Time.FixedDeltaTime the same, now that might not be the same realtime but no one cares about realtime, you care about simulation time, even if you run 2 fixed updates back to back.

James-Frowen avatar Mar 27 '22 17:03 James-Frowen

Closing this as NetworkTransform has been replaced entirely for the next release, and Server Send Rate is used.

MrGadget1024 avatar Nov 24 '22 11:11 MrGadget1024