-
-
Notifications
You must be signed in to change notification settings - Fork 73
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add delay for ack packets #156
base: master
Are you sure you want to change the base?
Conversation
Why put the ack delay in the ack packet, and not as a config? This makes ack packets bigger and adds risk of malicious effects. It also breaks from the netcode standard. |
Just a late reply, this changes the custom renet protocol, shouldn't interfere with the netcode protocol. The delay is added to the ack packet so that we can use it to calculate the RTT, if the ack delay is 0, we know that the ack was sent instantly. Otherwise, if the packet had some delay, we sent how long it waited, so when it is received the delay can be used to calculate the correctly the RTT. So why do this at all. If we don't send any reliable messages, we would never send acks instantly and without the delay specified in the ack packet, we could not calculate the RTT correctly. |
What about adding the config to |
In my mind, this is not something a user should need to configure or know/worry about. Most of the times this value won't impact much because if any reliable packet is received it will always send an ack immediatly. A value between 250 - 500ms should be good enough. Also, this would not be an intuitive config for some users. |
How about instead of |
Currently, renet sends acks every update frame, this increases the bandwidth for connections.
This PR adds a delay for sending ACK packets, when receiving a reliable message this delay is ignored and the ACK is sent immediatly (so we don't delay acks for reliable messages).
This reduces the bandwidth of connections. For idle connections this seems to reduce from 2.15 kbps to 0.5 kbps (at 60 hz and 200ms max delay). If the connection is receiving reliable messages every frame this PR will not change the bandwidth since it will still send ACKs every frame.
This change will affect a bit the stats for the connection , since the values can be delaye, but at small MAX_ACK_DELAY ( 200-300ms), this shouldn't impact much.
TODO: make sure the RTT calculations is working correctly and check that the new delay field in the ACK packet cannot be wrongly used with malicious packets.