Fix ICommonSession.Ping

space-wizards · Sep 20, 2024 · ab8594b · ab8594b
1 parent c86cb0b
commit ab8594b
Show file tree

Hide file tree

Showing 8 changed files with 19 additions and 11 deletions.
diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md
@@ -43,7 +43,7 @@ END TEMPLATE-->
 
 ### Bugfixes
 
-*None yet*
+* Fixed `ICommonSession.Ping` always returning zero instead of the ping. Note that this will still return zero for client-side code when trying to get the ping of other players.
 
 ### Other
 

diff --git a/Robust.Client/Player/PlayerManager.cs b/Robust.Client/Player/PlayerManager.cs
@@ -261,7 +261,6 @@ private bool UpdatePlayerList(IEnumerable<SessionState> remotePlayers, bool full
                     // This is a new userid, so we create a new session.
                     DebugTools.Assert(state.UserId != LocalPlayer?.UserId);
                     var newSession = (ICommonSessionInternal)CreateAndAddSession(state.UserId, state.Name);
-                    newSession.SetPing(state.Ping);
                     SetStatus(newSession, state.Status);
                     SetAttachedEntity(newSession, controlled, out _, true);
                     dirty = true;
@@ -271,7 +270,6 @@ private bool UpdatePlayerList(IEnumerable<SessionState> remotePlayers, bool full
                 // Check if the data is actually different
                 if (session.Name == state.Name
                     && session.Status == state.Status
-                    && session.Ping == state.Ping
                     && session.AttachedEntity == controlled)
                 {
                     continue;
@@ -280,7 +278,6 @@ private bool UpdatePlayerList(IEnumerable<SessionState> remotePlayers, bool full
                 dirty = true;
                 var local = (ICommonSessionInternal)session;
                 local.SetName(state.Name);
-                local.SetPing(state.Ping);
                 SetStatus(local, state.Status);
                 SetAttachedEntity(local, controlled, out _, true);
             }

diff --git a/Robust.Server/Player/PlayerManager.cs b/Robust.Server/Player/PlayerManager.cs
@@ -137,8 +137,7 @@ private void HandlePlayerListReq(MsgPlayerListReq message)
                 {
                     UserId = client.UserId,
                     Name = client.Name,
-                    Status = client.Status,
-                    Ping = client.Channel!.Ping
+                    Status = client.Status
                 };
                 list.Add(info);
             }

diff --git a/Robust.Shared/GameStates/SessionState.cs b/Robust.Shared/GameStates/SessionState.cs
@@ -21,6 +21,10 @@ public sealed class SessionState
         [ViewVariables]
         public SessionStatus Status { get; set; }
 
+        // TODO PlayerManager
+        // Network ping information, though probably do it outside of SessionState to avoid re-sending the name and such
+        // for all players every few seconds.
+        [Obsolete("Ping data is not currently networked")]
         [ViewVariables]
         public short Ping { get; set; }
 
@@ -34,7 +38,6 @@ public SessionState Clone()
                 UserId = UserId,
                 Name = Name,
                 Status = Status,
-                Ping = Ping,
                 ControlledEntity = ControlledEntity
             };
         }

diff --git a/Robust.Shared/Network/Messages/MsgPlayerList.cs b/Robust.Shared/Network/Messages/MsgPlayerList.cs
@@ -25,7 +25,6 @@ public override void ReadFromBuffer(NetIncomingMessage buffer, IRobustSerializer
                     UserId = new NetUserId(buffer.ReadGuid()),
                     Name = buffer.ReadString(),
                     Status = (SessionStatus)buffer.ReadByte(),
-                    Ping = buffer.ReadInt16()
                 };
                 Plyrs.Add(plyNfo);
             }
@@ -40,7 +39,6 @@ public override void WriteToBuffer(NetOutgoingMessage buffer, IRobustSerializer
                 buffer.Write(ply.UserId.UserId);
                 buffer.Write(ply.Name);
                 buffer.Write((byte) ply.Status);
-                buffer.Write(ply.Ping);
             }
         }
     }

diff --git a/Robust.Shared/Player/CommonSession.cs b/Robust.Shared/Player/CommonSession.cs
@@ -20,7 +20,12 @@ internal sealed class CommonSession : ICommonSessionInternal
     public string Name { get; set; } = "<Unknown>";
 
     [ViewVariables]
-    public short Ping { get; set; }
+    public short Ping
+    {
+        get => Channel?.Ping ?? _ping;
+        set => _ping = value;
+    }
+    private short _ping;
 
     [ViewVariables]
     public DateTime ConnectedTime { get; set; }

diff --git a/Robust.Shared/Player/ICommonSession.cs b/Robust.Shared/Player/ICommonSession.cs
@@ -33,9 +33,12 @@ public interface ICommonSession
     string Name { get; }
 
     /// <summary>
-    /// Current connection latency of this session from the server to their client.
+    /// Current connection latency of this session. If <see cref="Channel"/> is not null this simply returns
+    /// <see cref="INetChannel.Ping"/>. This is not currently usable by client-side code that wants to try access ping
+    /// information of other players.
     /// </summary>
     short Ping { get; }
+    // TODO PlayerManager ping networking.
 
     /// <summary>
     /// The current network channel for this session.

diff --git a/Robust.Shared/Player/SharedPlayerManager.State.cs b/Robust.Shared/Player/SharedPlayerManager.State.cs
@@ -18,6 +18,9 @@ public void GetPlayerStates(GameTick fromTick, List<SessionState> states)
         if (LastStateUpdate < fromTick)
             return;
 
+        // TODO PlayerManager delta states
+        // Track last update tick/time per session, and only send sessions that actually changed.
+
         states.EnsureCapacity(InternalSessions.Count);
         foreach (var player in InternalSessions.Values)
         {