search

lucaspoffo / renet

Server/Client network library for multiplayer games with authentication and connection management made with Rust
Apache License 2.0
685 stars 69 forks source link

Add delay for ack packets #156

Open lucaspoffo opened 8 months ago

lucaspoffo commented 8 months ago

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.

UkoeHB commented 8 months ago

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.

lucaspoffo commented 1 month ago

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.

UkoeHB commented 1 month ago

What about adding the config to ConnectionConfig? Users are already expected to use the same channels, so including ack delay wouldn't be too unusual. You can make it Option<Duration> and then default to 60Hz if not set (or use Some(60Hz) as the default when constructing config, and allow acks to be sent every tick when None).

lucaspoffo commented 1 month ago

What about adding the config to ConnectionConfig? Users are already expected to use the same channels, so including ack delay wouldn't be too unusual. You can make it Option<Duration> and then default to 60Hz if not set (or use Some(60Hz) as the default when constructing config, and allow acks to be sent every tick when None).

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.