Skip to content

Commit

Permalink
Comments for things to explore
Browse files Browse the repository at this point in the history
  • Loading branch information
Nick Schulz committed Jun 12, 2024
1 parent eb33588 commit 8881870
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 0 deletions.
3 changes: 3 additions & 0 deletions Hazel/Udp/UdpConnection.KeepAlive.cs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ private void HandleKeepAlive(object state)
{
if (this.State != ConnectionState.Connected) return;

// NOTE/TODO: If we decrease the ping interval we may need to incrase the `MissingPingsUntilDisconnect`
if (this.pingsSinceAck >= this.MissingPingsUntilDisconnect)
{
this.DisposeKeepAliveTimer();
Expand All @@ -94,6 +95,8 @@ private void HandleKeepAlive(object state)

try
{
// TODO: This is a race, we should make a new branch off main and change this to Interlocked.increment and Interlocked.exchange for setting to 0
// because volatile doesn't protect against concurrency, just against thread collisions...
this.pingsSinceAck++;
SendPing();
}
Expand Down
9 changes: 9 additions & 0 deletions Hazel/Udp/UdpConnection.Reliable.cs
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,11 @@ public int Resend(bool force = false)
return 0;
}

// TODO: Whenever we resend we aren't resetting the packet lifetime, this means that when this packet does get
// acked it is calculating the RT from the moment this packet was created instead of from the moment it
// was resent. We should do a nother expirment where we fix the influence these resent packets can have on ping
// by properly updating packet with a LastSentTime or something....

var connection = this.Connection;
int lifetimeMs = (int)this.Stopwatch.ElapsedMilliseconds;
if (lifetimeMs >= connection.DisconnectTimeoutMs)
Expand Down Expand Up @@ -272,6 +277,8 @@ protected void AttachReliableID(SmartBuffer buffer, int offset, int length, Acti

public int CalculateNextResendDelayMs(int lastDelayMs)
{
// TODO: This should maybe just be lastDelayMs * resendPingMultipler and not also adding that to the previous lastDelayMs
// This can be experiment 2, where we just remove the + here...should also make a new branch from main
return lastDelayMs + (int)Math.Min(lastDelayMs * this.ResendPingMultiplier, this.MaxAdditionalResendDelayMs);
}

Expand Down Expand Up @@ -470,6 +477,8 @@ private void AcknowledgeMessageId(ushort id)
this.logger.WriteVerbose($"Packet {id} RTT: {rt}ms Ping:{this._pingMs} Active: {reliableDataPacketsSent.Count}/{activePingPackets.Count}");
#endif
}
// TODO: So...if we drop a ping packet, we may never actually call remove on it because we don't
// ever resend pings...
else if (this.activePingPackets.TryRemove(id, out PingPacket pingPkt))
{
this.Statistics.LogReliablePacketAcknowledged();
Expand Down
1 change: 1 addition & 0 deletions Hazel/Udp/UdpConnection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ protected void SendHello(byte[] bytes, Action acknowledgeCallback)
/// <inheritdoc/>
protected override void Dispose(bool disposing)
{
// TODO: If disposing == false, these don't get cleaned up and there is a case with UDPClientConnection destructor
if (disposing)
{
DisposeKeepAliveTimer();
Expand Down

0 comments on commit 8881870

Please sign in to comment.