I have spent more time today cleaning up garbage than coding new shit, and I am mad (#31246)

* Fix logging of GetWebhook errors

Yeah let's just not log the error only stack trace.

* I have spent more time today cleaning up garbage than coding new shit, and I am mad

Cleans up the custom vote Discord webhook code because I *happened* to lay my eyes on how completely terrible it was and immediately found an obvious bug with it.

Also did basic QA because jesus christ: it more clearly reports pending votes, properly indicates cancelled votes, improves footer formatting, better error logging, all the usual shit.

Requires https://github.com/space-wizards/RobustToolbox/pull/5375 to avoid test failures

Content.Server/Discord/DiscordWebhook.cs

@@ -33,9 +33,9 @@ public sealed class DiscordWebhook : IPostInjectInit
         {
             return await _http.GetFromJsonAsync<WebhookData>(url);
         }
-        catch
+        catch (Exception e)
         {
-            _sawmill.Error($"Error getting discord webhook data. Stack trace:\n{Environment.StackTrace}");
+            _sawmill.Error($"Error getting discord webhook data.\n{e}");
             return null;
         }
     }

Content.Server/Voting/VoteCommands.cs

@@ -1,5 +1,4 @@
 using System.Linq;
-using System.Net.Http;
 using System.Text.Json;
 using System.Text.Json.Nodes;
 using Content.Server.Administration;
@@ -12,10 +11,10 @@ using Content.Shared.Administration;
 using Content.Shared.CCVar;
 using Content.Shared.Database;
 using Content.Shared.Voting;
-using Robust.Server.Player;
+using Robust.Server;
-using Robust.Shared;
 using Robust.Shared.Configuration;
 using Robust.Shared.Console;
+using Robust.Shared.Utility;
 
 namespace Content.Server.Voting
 {
@@ -67,27 +66,23 @@ namespace Content.Server.Voting
     }
 
     [AdminCommand(AdminFlags.Moderator)]
-    public sealed class CreateCustomCommand : IConsoleCommand
+    public sealed class CreateCustomCommand : LocalizedEntityCommands
     {
+        [Dependency] private readonly IVoteManager _voteManager = default!;
         [Dependency] private readonly IAdminLogManager _adminLogger = default!;
-        [Dependency] private readonly IEntitySystemManager _entitySystem = default!;
         [Dependency] private readonly IConfigurationManager _cfg = default!;
         [Dependency] private readonly DiscordWebhook _discord = default!;
+        [Dependency] private readonly GameTicker _gameTicker = default!;
+        [Dependency] private readonly IBaseServer _baseServer = default!;
+        [Dependency] private readonly IChatManager _chatManager = default!;
 
         private ISawmill _sawmill = default!;
 
         private const int MaxArgCount = 10;
 
-        public string Command => "customvote";
+        public override string Command => "customvote";
-        public string Description => Loc.GetString("cmd-customvote-desc");
-        public string Help => Loc.GetString("cmd-customvote-help");
 
-        // Webhook stuff
+        public override void Execute(IConsoleShell shell, string argStr, string[] args)
-        private string _webhookUrl = string.Empty;
-        private ulong _webhookId;
-        private WebhookIdentifier? _webhookIdentifier;
-
-        public void Execute(IConsoleShell shell, string argStr, string[] args)
         {
             _sawmill = Logger.GetSawmill("vote");
 
@@ -99,8 +94,6 @@ namespace Content.Server.Voting
 
             var title = args[0];
 
-            var mgr = IoCManager.Resolve<IVoteManager>();
-
             var options = new VoteOptions
             {
                 Title = title,
@@ -112,41 +105,6 @@ namespace Content.Server.Voting
                 options.Options.Add((args[i], i));
             }
 
-            // Set up the webhook payload
-            string _serverName = _cfg.GetCVar(CVars.GameHostName);
-            _webhookUrl = _cfg.GetCVar(CCVars.DiscordVoteWebhook);
-
-
-            var _gameTicker = _entitySystem.GetEntitySystem<GameTicker>();
-
-            var payload = new WebhookPayload()
-            {
-                Username = Loc.GetString("custom-vote-webhook-name"),
-                Embeds = new List<WebhookEmbed>
-                {
-                    new()
-                    {
-                        Title = $"{shell.Player}",
-                        Color = 13438992,
-                        Description = options.Title,
-                        Footer = new WebhookEmbedFooter
-                        {
-                            Text = $"{_serverName} {_gameTicker.RoundId} {_gameTicker.RunLevel}",
-                        },
-
-                        Fields = new List<WebhookEmbedField> {},
-                    },
-                },
-            };
-
-            foreach (var voteOption in options.Options)
-            {
-                var NewVote = new WebhookEmbedField() { Name = voteOption.text,  Value = "0"};
-                payload.Embeds[0].Fields.Add(NewVote);
-            }
-
-            WebhookMessage(payload);
-
             options.SetInitiatorOrServer(shell.Player);
 
             if (shell.Player != null)
@@ -154,38 +112,34 @@ namespace Content.Server.Voting
             else
                 _adminLogger.Add(LogType.Vote, LogImpact.Medium, $"Initiated a custom vote: {options.Title} - {string.Join("; ", options.Options.Select(x => x.text))}");
 
-            var vote = mgr.CreateVote(options);
+            var vote = _voteManager.CreateVote(options);
+
+            var webhookState = CreateWebhookIfConfigured(options);
 
             vote.OnFinished += (_, eventArgs) =>
             {
-                var chatMgr = IoCManager.Resolve<IChatManager>();
                 if (eventArgs.Winner == null)
                 {
                     var ties = string.Join(", ", eventArgs.Winners.Select(c => args[(int) c]));
                     _adminLogger.Add(LogType.Vote, LogImpact.Medium, $"Custom vote {options.Title} finished as tie: {ties}");
-                    chatMgr.DispatchServerAnnouncement(Loc.GetString("cmd-customvote-on-finished-tie",("ties", ties)));
+                    _chatManager.DispatchServerAnnouncement(Loc.GetString("cmd-customvote-on-finished-tie", ("ties", ties)));
                 }
                 else
                 {
                     _adminLogger.Add(LogType.Vote, LogImpact.Medium, $"Custom vote {options.Title} finished: {args[(int) eventArgs.Winner]}");
-                    chatMgr.DispatchServerAnnouncement(Loc.GetString("cmd-customvote-on-finished-win",("winner", args[(int) eventArgs.Winner])));
+                    _chatManager.DispatchServerAnnouncement(Loc.GetString("cmd-customvote-on-finished-win", ("winner", args[(int) eventArgs.Winner])));
                 }
 
-                for (int i = 0; i < eventArgs.Votes.Count; i++)
+                UpdateWebhookIfConfigured(webhookState, eventArgs);
-                {
+            };
-                    var oldName = payload.Embeds[0].Fields[i].Name;
-                    var newValue = eventArgs.Votes[i].ToString();
-                    var newEmbed = payload.Embeds[0];
-                    newEmbed.Color = 2353993;
-                    payload.Embeds[0] = newEmbed;
-                    payload.Embeds[0].Fields[i] = new WebhookEmbedField() { Name = oldName, Value = newValue, Inline =  true};
-                }
 
-                WebhookMessage(payload, _webhookId);
+            vote.OnCancelled += _ =>
+            {
+                UpdateCancelledWebhookIfConfigured(webhookState);
             };
         }
 
-        public CompletionResult GetCompletion(IConsoleShell shell, string[] args)
+        public override CompletionResult GetCompletion(IConsoleShell shell, string[] args)
         {
             if (args.Length == 1)
                 return CompletionResult.FromHint(Loc.GetString("cmd-customvote-arg-title"));
@@ -197,40 +151,160 @@ namespace Content.Server.Voting
             return CompletionResult.FromHint(Loc.GetString("cmd-customvote-arg-option-n", ("n", n)));
         }
 
-        // Sends the payload's message.
+        private WebhookState? CreateWebhookIfConfigured(VoteOptions voteOptions)
-        private async void WebhookMessage(WebhookPayload payload)
         {
-            if (string.IsNullOrEmpty(_webhookUrl))
+            // All this webhook code is complete garbage.
+            // I tried to clean it up somewhat, at least to fix the glaring bugs in it.
+            // Jesus christ man what is with our code review process.
+
+            var webhookUrl = _cfg.GetCVar(CCVars.DiscordVoteWebhook);
+            if (string.IsNullOrEmpty(webhookUrl))
+                return null;
+
+            // Set up the webhook payload
+            var serverName = _baseServer.ServerName;
+
+            var fields = new List<WebhookEmbedField>();
+
+            foreach (var voteOption in voteOptions.Options)
+            {
+                var newVote = new WebhookEmbedField
+                {
+                    Name = voteOption.text,
+                    Value = Loc.GetString("custom-vote-webhook-option-pending")
+                };
+                fields.Add(newVote);
+            }
+
+            var runLevel = Loc.GetString($"game-run-level-{_gameTicker.RunLevel}");
+
+            var payload = new WebhookPayload()
+            {
+                Username = Loc.GetString("custom-vote-webhook-name"),
+                Embeds = new List<WebhookEmbed>
+                {
+                    new()
+                    {
+                        Title = voteOptions.InitiatorText,
+                        Color = 13438992, // #CD1010
+                        Description = voteOptions.Title,
+                        Footer = new WebhookEmbedFooter
+                        {
+                            Text = Loc.GetString(
+                                "custom-vote-webhook-footer",
+                                ("serverName", serverName),
+                                ("roundId", _gameTicker.RoundId),
+                                ("runLevel", runLevel)),
+                        },
+
+                        Fields = fields,
+                    },
+                },
+            };
+
+            var state = new WebhookState
+            {
+                WebhookUrl = webhookUrl,
+                Payload = payload,
+            };
+
+            CreateWebhookMessage(state, payload);
+
+            return state;
+        }
+
+        private void UpdateWebhookIfConfigured(WebhookState? state, VoteFinishedEventArgs finished)
+        {
+            if (state == null)
                 return;
 
-            if (await _discord.GetWebhook(_webhookUrl) is not { } identifier)
+            var embed = state.Payload.Embeds![0];
+            embed.Color = 2353993; // #23EB49
+
+            for (var i = 0; i < finished.Votes.Count; i++)
+            {
+                var oldName = embed.Fields[i].Name;
+                var newValue = finished.Votes[i].ToString();
+                embed.Fields[i] = new WebhookEmbedField { Name = oldName, Value = newValue, Inline =  true};
+            }
+
+            state.Payload.Embeds[0] = embed;
+
+            UpdateWebhookMessage(state, state.Payload, state.MessageId);
+        }
+
+        private void UpdateCancelledWebhookIfConfigured(WebhookState? state)
+        {
+            if (state == null)
                 return;
 
-            _webhookIdentifier = identifier.ToIdentifier();
+            var embed = state.Payload.Embeds![0];
+            embed.Color = 13356304; // #CBCD10
+            embed.Description += "\n\n" + Loc.GetString("custom-vote-webhook-cancelled");
 
-            _sawmill.Debug(JsonSerializer.Serialize(payload));
+            for (var i = 0; i < embed.Fields.Count; i++)
+            {
+                var oldName = embed.Fields[i].Name;
+                embed.Fields[i] = new WebhookEmbedField { Name = oldName, Value = Loc.GetString("custom-vote-webhook-option-cancelled"), Inline =  true};
+            }
 
-            var request = await _discord.CreateMessage(_webhookIdentifier.Value, payload);
+            state.Payload.Embeds[0] = embed;
-            var content = await request.Content.ReadAsStringAsync();
+
-            _webhookId = ulong.Parse(JsonNode.Parse(content)?["id"]!.GetValue<string>()!);
+            UpdateWebhookMessage(state, state.Payload, state.MessageId);
+        }
+
+        // Sends the payload's message.
+        private async void CreateWebhookMessage(WebhookState state, WebhookPayload payload)
+        {
+            try
+            {
+                if (await _discord.GetWebhook(state.WebhookUrl) is not { } identifier)
+                    return;
+
+                state.Identifier = identifier.ToIdentifier();
+
+                _sawmill.Debug(JsonSerializer.Serialize(payload));
+
+                var request = await _discord.CreateMessage(identifier.ToIdentifier(), payload);
+                var content = await request.Content.ReadAsStringAsync();
+                state.MessageId = ulong.Parse(JsonNode.Parse(content)?["id"]!.GetValue<string>()!);
+            }
+            catch (Exception e)
+            {
+                _sawmill.Error($"Error while sending vote webhook to Discord: {e}");
+            }
         }
 
         // Edits a pre-existing payload message, given an ID
-        private async void WebhookMessage(WebhookPayload payload, ulong id)
+        private async void UpdateWebhookMessage(WebhookState state, WebhookPayload payload, ulong id)
         {
-            if (string.IsNullOrEmpty(_webhookUrl))
+            if (state.MessageId == 0)
+            {
+                _sawmill.Warning("Failed to deliver update to custom vote webhook: message ID was zero. This likely indicates a previous connection error sending the original message.");
                 return;
+            }
 
-            if (await _discord.GetWebhook(_webhookUrl) is not { } identifier)
+            DebugTools.Assert(state.Identifier != default);
-                return;
 
-            _webhookIdentifier = identifier.ToIdentifier();
+            try
+            {
+                await _discord.EditMessage(state.Identifier, id, payload);
+            }
+            catch (Exception e)
+            {
+                _sawmill.Error($"Error while updating vote webhook on Discord: {e}");
+            }
+        }
 
-            var request = await _discord.EditMessage(_webhookIdentifier.Value, id, payload);
+        private sealed class WebhookState
+        {
+            public required string WebhookUrl;
+            public required WebhookPayload Payload;
+            public WebhookIdentifier Identifier;
+            public ulong MessageId;
         }
     }
 
-
     [AnyCommand]
     public sealed class VoteCommand : IConsoleCommand
     {

Resources/Locale/en-US/administration/commands/custom-vote-command.ftl

@@ -1 +1,5 @@
-custom-vote-webhook-name = Custom Vote Held
+custom-vote-webhook-name = Custom Vote Held
+custom-vote-webhook-footer = server: { $serverName }, round: { $roundId } { $runLevel }
+custom-vote-webhook-cancelled = **Vote cancelled**
+custom-vote-webhook-option-pending = TBD
+custom-vote-webhook-option-cancelled = N/A

Resources/Locale/en-US/game-ticking/game-ticker.ftl

@@ -41,4 +41,8 @@ latejoin-arrivals-dumped-from-shuttle = A mysterious force prevents you from lea
 latejoin-arrivals-teleport-to-spawn = A mysterious force teleports you off the arrivals shuttle. Have a safe shift!
 
 preset-not-enough-ready-players = Can't start {$presetName}. Requires {$minimumPlayers} players but we have {$readyPlayersCount}.
-preset-no-one-ready = Can't start {$presetName}. No players are ready.
+preset-no-one-ready = Can't start {$presetName}. No players are ready.
+
+game-run-level-PreRoundLobby = Pre-round lobby
+game-run-level-InRound = In round
+game-run-level-PostRound = Post round