Rejig LogStringHandler (#30706)

* Rejig LogStringHandler * Fix session logs * Fix properly * comments * IAsType support * Fix mind logs * Fix mind logging AGAIN --------- Co-authored-by: PJB3005 <pieterjan.briers+git@gmail.com>
2025-09-06 00:22:49 +12:00
parent f45bf4590f
commit 828b1f2044
17 changed files with 413 additions and 198 deletions
--- a/Content.Server/Administration/Logs/AdminLogManager.Json.cs
+++ b/Content.Server/Administration/Logs/AdminLogManager.Json.cs
@@ -2,9 +2,7 @@
 using System.Text.Json;
 using System.Text.Json.Serialization;
 using Content.Server.Administration.Logs.Converters;
-using Robust.Server.GameObjects;
-using Robust.Shared.Map;
-using Robust.Shared.Player;
+using Robust.Shared.Collections;

 namespace Content.Server.Administration.Logs;

@@ -22,55 +20,25 @@ public sealed partial class AdminLogManager
            PropertyNamingPolicy = NamingPolicy
        };

+        var interfaces = new ValueList<IAdminLogConverter>();
+
        foreach (var converter in _reflection.FindTypesWithAttribute<AdminLogConverterAttribute>())
        {
            var instance = _typeFactory.CreateInstance<JsonConverter>(converter);
-            (instance as IAdminLogConverter)?.Init(_dependencies);
+            if (instance is IAdminLogConverter converterInterface)
+            {
+                interfaces.Add(converterInterface);
+                converterInterface.Init(_dependencies);
+            }
            _jsonOptions.Converters.Add(instance);
        }

+        foreach (var @interface in interfaces)
+        {
+            @interface.Init2(_jsonOptions);
+        }
+
        var converterNames = _jsonOptions.Converters.Select(converter => converter.GetType().Name);
        _sawmill.Debug($"Admin log converters found: {string.Join(" ", converterNames)}");
    }
-
-    private (JsonDocument Json, HashSet<Guid> Players) ToJson(
-        Dictionary<string, object?> properties)
-    {
-        var players = new HashSet<Guid>();
-        var parsed = new Dictionary<string, object?>();
-
-        foreach (var key in properties.Keys)
-        {
-            var value = properties[key];
-            value = value switch
-            {
-                ICommonSession player => new SerializablePlayer(player),
-                EntityCoordinates entityCoordinates => new SerializableEntityCoordinates(_entityManager, entityCoordinates),
-                _ => value
-            };
-
-            var parsedKey = NamingPolicy.ConvertName(key);
-            parsed.Add(parsedKey, value);
-
-            var entityId = properties[key] switch
-            {
-                EntityUid id => id,
-                EntityStringRepresentation rep => rep.Uid,
-                ICommonSession {AttachedEntity: {Valid: true}} session => session.AttachedEntity,
-                IComponent component => component.Owner,
-                _ => null
-            };
-
-            if (_entityManager.TryGetComponent(entityId, out ActorComponent? actor))
-            {
-                players.Add(actor.PlayerSession.UserId.UserId);
-            }
-            else if (value is SerializablePlayer player)
-            {
-                players.Add(player.Player.UserId.UserId);
-            }
-        }
-
-        return (JsonSerializer.SerializeToDocument(parsed, _jsonOptions), players);
-    }
 }
--- a/Content.Server/Administration/Logs/AdminLogManager.cs
+++ b/Content.Server/Administration/Logs/AdminLogManager.cs
@@ -25,7 +25,6 @@ namespace Content.Server.Administration.Logs;
 public sealed partial class AdminLogManager : SharedAdminLogManager, IAdminLogManager
 {
    [Dependency] private readonly IConfigurationManager _configuration = default!;
-    [Dependency] private readonly IEntityManager _entityManager = default!;
    [Dependency] private readonly ILogManager _logManager = default!;
    [Dependency] private readonly IServerDbManager _db = default!;
    [Dependency] private readonly IGameTiming _timing = default!;
@@ -72,7 +71,6 @@ public sealed partial class AdminLogManager : SharedAdminLogManager, IAdminLogMa

    // CVars
    private bool _metricsEnabled;
-    private bool _enabled;
    private TimeSpan _queueSendDelay;
    private int _queueMax;
    private int _preRoundQueueMax;
@@ -103,7 +101,7 @@ public sealed partial class AdminLogManager : SharedAdminLogManager, IAdminLogMa
        _configuration.OnValueChanged(CVars.MetricsEnabled,
            value => _metricsEnabled = value, true);
        _configuration.OnValueChanged(CCVars.AdminLogsEnabled,
-            value => _enabled = value, true);
+            value => Enabled = value, true);
        _configuration.OnValueChanged(CCVars.AdminLogsQueueSendDelay,
            value => _queueSendDelay = TimeSpan.FromSeconds(value), true);
        _configuration.OnValueChanged(CCVars.AdminLogsQueueMax,
@@ -123,6 +121,12 @@ public sealed partial class AdminLogManager : SharedAdminLogManager, IAdminLogMa
        }
    }

+    public override string ConvertName(string name)
+    {
+        // JsonNamingPolicy is not whitelisted by the sandbox.
+        return NamingPolicy.ConvertName(name);
+    }
+
    public async Task Shutdown()
    {
        if (!_logQueue.IsEmpty)
@@ -292,8 +296,17 @@ public sealed partial class AdminLogManager : SharedAdminLogManager, IAdminLogMa
        }
    }

-    private void Add(LogType type, LogImpact impact, string message, JsonDocument json, HashSet<Guid> players)
+    public override void Add(LogType type, [System.Runtime.CompilerServices.InterpolatedStringHandlerArgument("")] ref LogStringHandler handler)
    {
+        Add(type, LogImpact.Medium, ref handler);
+    }
+
+    public override void Add(LogType type, LogImpact impact, [System.Runtime.CompilerServices.InterpolatedStringHandlerArgument("")] ref LogStringHandler handler)
+    {
+        var message = handler.ToStringAndClear();
+        if (!Enabled)
+            return;
+
        var preRound = _runLevel == GameRunLevel.PreRoundLobby;
        var count = preRound ? _preRoundLogQueue.Count : _logQueue.Count;
        if (count >= _dropThreshold)
@@ -302,6 +315,10 @@ public sealed partial class AdminLogManager : SharedAdminLogManager, IAdminLogMa
            return;
        }

+        var json = JsonSerializer.SerializeToDocument(handler.Values, _jsonOptions);
+        var id = NextLogId;
+        var players = GetPlayers(handler.Values, id);
+
        // PostgreSQL does not support storing null chars in text values.
        if (message.Contains('\0'))
        {
@@ -311,31 +328,85 @@ public sealed partial class AdminLogManager : SharedAdminLogManager, IAdminLogMa

        var log = new AdminLog
        {
-            Id = NextLogId,
+            Id = id,
            RoundId = _currentRoundId,
            Type = type,
            Impact = impact,
            Date = DateTime.UtcNow,
            Message = message,
            Json = json,
-            Players = new List<AdminLogPlayer>(players.Count)
+            Players = players,
        };

-        var adminLog = false;
-        var adminSys = _entityManager.SystemOrNull<AdminSystem>();
+        DoAdminAlerts(players, message, impact);
+
+        if (preRound)
+        {
+            _preRoundLogQueue.Enqueue(log);
+        }
+        else
+        {
+            _logQueue.Enqueue(log);
+            CacheLog(log);
+        }
+    }
+
+    private List<AdminLogPlayer> GetPlayers(Dictionary<string, object?> values, int logId)
+    {
+        List<AdminLogPlayer> players = new();
+        foreach (var value in values.Values)
+        {
+            switch (value)
+            {
+                case SerializablePlayer player:
+                    AddPlayer(players, player.UserId, logId);
+                    continue;
+
+                case EntityStringRepresentation rep:
+                    if (rep.Session is {} session)
+                        AddPlayer(players, session.UserId.UserId, logId);
+                    continue;
+
+                case IAdminLogsPlayerValue playerValue:
+                    foreach (var player in playerValue.Players)
+                    {
+                        AddPlayer(players, player, logId);
+                    }
+
+                    break;
+            }
+        }
+
+        return players;
+    }
+
+    private void AddPlayer(List<AdminLogPlayer> players, Guid user, int logId)
+    {
+        // The majority of logs have a single player, or maybe two. Instead of allocating a List<AdminLogPlayer> and
+        // HashSet<Guid>, we just iterate over the list to check for duplicates.
+        foreach (var player in players)
+        {
+            if (player.PlayerUserId == user)
+                return;
+        }
+
+        players.Add(new AdminLogPlayer
+        {
+            LogId = logId,
+            PlayerUserId = user
+        });
+    }
+
+    private void DoAdminAlerts(List<AdminLogPlayer> players, string message, LogImpact impact)
+    {
+        var adminLog = true;
        var logMessage = message;

-        foreach (var id in players)
+        foreach (var player in players)
        {
-            var player = new AdminLogPlayer
-            {
-                LogId = log.Id,
-                PlayerUserId = id
-            };
+            var id = player.PlayerUserId;

-            log.Players.Add(player);
-
-            if (adminSys != null)
+            if (EntityManager.TrySystem(out AdminSystem? adminSys))
            {
                var cachedInfo = adminSys.GetCachedPlayerInfo(new NetUserId(id));
                if (cachedInfo != null && cachedInfo.Antag)
@@ -372,35 +443,6 @@ public sealed partial class AdminLogManager : SharedAdminLogManager, IAdminLogMa

        if (adminLog)
            _chat.SendAdminAlert(logMessage);
-
-        if (preRound)
-        {
-            _preRoundLogQueue.Enqueue(log);
-        }
-        else
-        {
-            _logQueue.Enqueue(log);
-            CacheLog(log);
-        }
-    }
-
-    public override void Add(LogType type, LogImpact impact, ref LogStringHandler handler)
-    {
-        if (!_enabled)
-        {
-            handler.ToStringAndClear();
-            return;
-        }
-
-        var (json, players) = ToJson(handler.Values);
-        var message = handler.ToStringAndClear();
-
-        Add(type, impact, message, json, players);
-    }
-
-    public override void Add(LogType type, ref LogStringHandler handler)
-    {
-        Add(type, LogImpact.Medium, ref handler);
    }

    public async Task<List<SharedAdminLog>> All(LogFilter? filter = null, Func<List<SharedAdminLog>>? listProvider = null)
--- a/Content.Server/Administration/Logs/Converters/AdminLogConverter.cs
+++ b/Content.Server/Administration/Logs/Converters/AdminLogConverter.cs
@@ -6,6 +6,13 @@ namespace Content.Server.Administration.Logs.Converters;
 public interface IAdminLogConverter
 {
    void Init(IDependencyCollection dependencies);
+
+    /// <summary>
+    /// Called after all converters have been added to the <see cref="JsonSerializerOptions"/>.
+    /// </summary>
+    void Init2(JsonSerializerOptions options)
+    {
+    }
 }

 public abstract class AdminLogConverter<T> : JsonConverter<T>, IAdminLogConverter
@@ -14,6 +21,10 @@ public abstract class AdminLogConverter<T> : JsonConverter<T>, IAdminLogConverte
    {
    }

+    public virtual void Init2(JsonSerializerOptions options)
+    {
+    }
+
    public override T Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
    {
        throw new NotSupportedException();
--- a/Content.Server/Administration/Logs/Converters/EntityCoordinatesConverter.cs
+++ b/Content.Server/Administration/Logs/Converters/EntityCoordinatesConverter.cs
@@ -6,7 +6,7 @@ using Robust.Shared.Map.Components;
 namespace Content.Server.Administration.Logs.Converters;

 [AdminLogConverter]
-public sealed class EntityCoordinatesConverter : AdminLogConverter<SerializableEntityCoordinates>
+public sealed class EntityCoordinatesConverter : AdminLogConverter<EntityCoordinates>
 {
    // System.Text.Json actually keeps hold of your JsonSerializerOption instances in a cache on .NET 7.
    // Use a weak reference to avoid holding server instances live too long in integration tests.
@@ -17,15 +17,16 @@ public sealed class EntityCoordinatesConverter : AdminLogConverter<SerializableE
        _entityManager = new WeakReference<IEntityManager>(dependencies.Resolve<IEntityManager>());
    }

-    public void Write(Utf8JsonWriter writer, SerializableEntityCoordinates value, JsonSerializerOptions options, IEntityManager entities)
+    public void Write(Utf8JsonWriter writer, EntityCoordinates value, JsonSerializerOptions options, IEntityManager entities)
    {
        writer.WriteStartObject();
-        WriteEntityInfo(writer, value.EntityUid, entities, "parent");
+        WriteEntityInfo(writer, value.EntityId, entities, "parent");
        writer.WriteNumber("x", value.X);
        writer.WriteNumber("y", value.Y);
-        if (value.MapUid.HasValue)
+        var mapUid = value.GetMapUid(entities);
+        if (mapUid.HasValue)
        {
-            WriteEntityInfo(writer, value.MapUid.Value, entities, "map");
+            WriteEntityInfo(writer, mapUid.Value, entities, "map");
        }
        writer.WriteEndObject();
    }
@@ -33,7 +34,7 @@ public sealed class EntityCoordinatesConverter : AdminLogConverter<SerializableE
    private static void WriteEntityInfo(Utf8JsonWriter writer, EntityUid value, IEntityManager entities, string rootName)
    {
        writer.WriteStartObject(rootName);
-        writer.WriteNumber("uid", value.GetHashCode());
+        writer.WriteNumber("uid", value.Id);
        if (entities.TryGetComponent(value, out MetaDataComponent? metaData))
        {
            writer.WriteString("name", metaData.EntityName);
@@ -51,7 +52,7 @@ public sealed class EntityCoordinatesConverter : AdminLogConverter<SerializableE
        writer.WriteEndObject();
    }

-    public override void Write(Utf8JsonWriter writer, SerializableEntityCoordinates value, JsonSerializerOptions options)
+    public override void Write(Utf8JsonWriter writer, EntityCoordinates value, JsonSerializerOptions options)
    {
        if (!_entityManager.TryGetTarget(out var entityManager))
            throw new InvalidOperationException("EntityManager got garbage collected!");
@@ -59,19 +60,3 @@ public sealed class EntityCoordinatesConverter : AdminLogConverter<SerializableE
        Write(writer, value, options, entityManager);
    }
 }
-
-public readonly struct SerializableEntityCoordinates
-{
-    public readonly EntityUid EntityUid;
-    public readonly float X;
-    public readonly float Y;
-    public readonly EntityUid? MapUid;
-
-    public SerializableEntityCoordinates(IEntityManager entityManager, EntityCoordinates coordinates)
-    {
-        EntityUid = coordinates.EntityId;
-        X = coordinates.X;
-        Y = coordinates.Y;
-        MapUid = entityManager.System<SharedTransformSystem>().GetMap(coordinates);
-    }
-}
--- a/Content.Server/Administration/Logs/Converters/EntityStringRepresentationConverter.cs
+++ b/Content.Server/Administration/Logs/Converters/EntityStringRepresentationConverter.cs
@@ -1,6 +1,5 @@
 using System.Text.Json;
 using Content.Server.Administration.Managers;
-using Robust.Server.Player;

 namespace Content.Server.Administration.Logs.Converters;

@@ -24,7 +23,7 @@ public sealed class EntityStringRepresentationConverter : AdminLogConverter<Enti
        {
            writer.WriteString("player", value.Session.UserId.UserId);

-            if (_adminManager.IsAdmin(value.Uid))
+            if (_adminManager.IsAdmin(value.Session))
            {
                writer.WriteBoolean("admin", true);
            }
--- a/Content.Server/Administration/Logs/Converters/MindStringRepresentationConverter.cs
+++ b/Content.Server/Administration/Logs/Converters/MindStringRepresentationConverter.cs
@@ -0,0 +1,38 @@
+using System.Text.Json;
+using System.Text.Json.Serialization;
+using Content.Shared.Mind;
+
+namespace Content.Server.Administration.Logs.Converters;
+
+[AdminLogConverter]
+public sealed class MindStringRepresentationConverter : AdminLogConverter<MindStringRepresentation>
+{
+    private JsonConverter<EntityStringRepresentation> _converter = null!;
+
+    public override void Init2(JsonSerializerOptions options)
+    {
+        base.Init2(options);
+
+        _converter = (JsonConverter<EntityStringRepresentation>)
+            options.GetConverter(typeof(EntityStringRepresentation));
+    }
+
+    public override void Write(Utf8JsonWriter writer, MindStringRepresentation value, JsonSerializerOptions options)
+    {
+        writer.WriteStartObject();
+
+        if (value.OwnedEntity is { } owned)
+        {
+            writer.WritePropertyName("owned");
+            _converter.Write(writer, owned, options);
+        }
+
+        if (value.Player is { } player)
+        {
+            writer.WriteString("player", player);
+            writer.WriteBoolean("present", value.PlayerPresent);
+        }
+
+        writer.WriteEndObject();
+    }
+}
--- a/Content.Server/Administration/Logs/Converters/PlayerSessionConverter.cs
+++ b/Content.Server/Administration/Logs/Converters/PlayerSessionConverter.cs
@@ -1,45 +1,23 @@
 using System.Text.Json;
-using Robust.Shared.Player;
+using Content.Shared.Administration.Logs;

 namespace Content.Server.Administration.Logs.Converters;

 [AdminLogConverter]
 public sealed class PlayerSessionConverter : AdminLogConverter<SerializablePlayer>
 {
-    // System.Text.Json actually keeps hold of your JsonSerializerOption instances in a cache on .NET 7.
-    // Use a weak reference to avoid holding server instances live too long in integration tests.
-    private WeakReference<IEntityManager> _entityManager = default!;
-
-    public override void Init(IDependencyCollection dependencies)
-    {
-        _entityManager = new WeakReference<IEntityManager>(dependencies.Resolve<IEntityManager>());
-    }
-
    public override void Write(Utf8JsonWriter writer, SerializablePlayer value, JsonSerializerOptions options)
    {
        writer.WriteStartObject();

-        if (value.Player.AttachedEntity is {Valid: true} playerEntity)
+        if (value.Uid is {Valid: true} playerEntity)
        {
-            if (!_entityManager.TryGetTarget(out var entityManager))
-                throw new InvalidOperationException("EntityManager got garbage collected!");
-
-            writer.WriteNumber("id", (int) value.Player.AttachedEntity);
-            writer.WriteString("name", entityManager.GetComponent<MetaDataComponent>(playerEntity).EntityName);
+            writer.WriteNumber("id", playerEntity.Id);
+            writer.WriteString("name", value.Name);
        }

-        writer.WriteString("player", value.Player.UserId.UserId);
+        writer.WriteString("player", value.UserId);

        writer.WriteEndObject();
    }
 }
-
-public readonly struct SerializablePlayer
-{
-    public readonly ICommonSession Player;
-
-    public SerializablePlayer(ICommonSession player)
-    {
-        Player = player;
-    }
-}
--- a/Content.Server/Botany/Systems/PlantHolderSystem.cs
+++ b/Content.Server/Botany/Systems/PlantHolderSystem.cs
@@ -2,6 +2,7 @@ using Content.Server.Atmos.EntitySystems;
 using Content.Server.Botany.Components;
 using Content.Server.Hands.Systems;
 using Content.Server.Popups;
+using Content.Shared.Administration.Logs;
 using Content.Shared.Chemistry.EntitySystems;
 using Content.Shared.Atmos;
 using Content.Shared.Botany;
@@ -886,7 +887,7 @@ public sealed class PlantHolderSystem : EntitySystem
            foreach (var entry in _solutionContainerSystem.RemoveEachReagent(component.SoilSolution.Value, amt))
            {
                var reagentProto = _prototype.Index<ReagentPrototype>(entry.Reagent.Prototype);
-                reagentProto.ReactionPlant(uid, entry, solution);
+                reagentProto.ReactionPlant(uid, entry, solution, EntityManager, _random, _adminLogger);
            }
        }