* Rewrote cmdcd service, prettified and paginated .cmdcds

* Cleaned up/improved some command handler code
* Fixed .yun when channel id has an underscore

src/NadekoBot/Modules/Permissions/CommandCooldown/CmdCdService.cs

@@ -1,68 +1,119 @@
 #nullable disable
 using NadekoBot.Common.ModuleBehaviors;
-using NadekoBot.Services.Database.Models;
 
 namespace NadekoBot.Modules.Permissions.Services;
 
-public class CmdCdService : IExecPreCommand, INService
+public sealed class CmdCdService : IExecPreCommand, IReadyExecutor, INService
 {
-    public ConcurrentDictionary<ulong, ConcurrentHashSet<CommandCooldown>> CommandCooldowns { get; }
-    public ConcurrentDictionary<ulong, ConcurrentHashSet<ActiveCooldown>> ActiveCooldowns { get; } = new();
+    private readonly ConcurrentDictionary<ulong, ConcurrentDictionary<string, int>> _settings = new();
 
-    public int Priority { get; } = 0;
+    private readonly ConcurrentDictionary<(ulong, string), ConcurrentDictionary<ulong, DateTime>> _activeCooldowns =
+        new();
+
+    public int Priority => 0;
 
     public CmdCdService(Bot bot)
-        => CommandCooldowns = new(bot.AllGuildConfigs.ToDictionary(k => k.GuildId,
-            v => new ConcurrentHashSet<CommandCooldown>(v.CommandCooldowns)));
-
-    public Task<bool> TryBlock(IGuild guild, IUser user, string commandName)
     {
-        if (guild is null)
-            return Task.FromResult(false);
+        _settings = bot
+            .AllGuildConfigs
+            .ToDictionary(x => x.GuildId, x => x.CommandCooldowns
+                .ToDictionary(c => c.CommandName, c => c.Seconds)
+                .ToConcurrent())
+            .ToConcurrent();
+    }
 
-        var cmdcds = CommandCooldowns.GetOrAdd(guild.Id, new ConcurrentHashSet<CommandCooldown>());
-        CommandCooldown cdRule;
-        if ((cdRule = cmdcds.FirstOrDefault(cc => cc.CommandName == commandName)) is not null)
+    public Task<bool> ExecPreCommandAsync(ICommandContext context, string moduleName, CommandInfo command)
+        => TryBlock(context.Guild, context.User, command.Name.ToLowerInvariant());
+
+    public async Task<bool> TryBlock(IGuild guild, IUser user, string commandName)
+    {
+        if (!_settings.TryGetValue(guild.Id, out var cooldownSettings))
+            return false;
+
+        if (!cooldownSettings.TryGetValue(commandName, out var cdSeconds))
+            return false;
+
+        var cooldowns = _activeCooldowns.GetOrAdd(
+            (guild.Id, commandName),
+            static _ => new());
+
+        // if user is not already on cooldown, add 
+        if (cooldowns.TryAdd(user.Id, DateTime.UtcNow))
         {
-            var activeCdsForGuild = ActiveCooldowns.GetOrAdd(guild.Id, new ConcurrentHashSet<ActiveCooldown>());
-            if (activeCdsForGuild.FirstOrDefault(ac => ac.UserId == user.Id && ac.Command == commandName) is not null)
-                return Task.FromResult(true);
-
-            activeCdsForGuild.Add(new()
-            {
-                UserId = user.Id,
-                Command = commandName
-            });
-
-            _ = Task.Run(async () =>
-            {
-                try
-                {
-                    await Task.Delay(cdRule.Seconds * 1000);
-                    activeCdsForGuild.RemoveWhere(ac => ac.Command == commandName && ac.UserId == user.Id);
-                }
-                catch
-                {
-                    // ignored
-                }
-            });
+            return false;
         }
 
-        return Task.FromResult(false);
+        // if there is an entry, maybe it expired. Try to check if it expired and don't fail if it did
+        // - just update
+        if (cooldowns.TryGetValue(user.Id, out var oldValue))
+        {
+            var diff = DateTime.UtcNow - oldValue;
+            if (diff.Seconds > cdSeconds)
+            {
+                if (cooldowns.TryUpdate(user.Id, DateTime.UtcNow, oldValue))
+                    return false;
+            }
+        }
+
+        return true;
     }
 
-    public Task<bool> ExecPreCommandAsync(ICommandContext ctx, string moduleName, CommandInfo command)
+    public async Task OnReadyAsync()
     {
-        var guild = ctx.Guild;
-        var user = ctx.User;
-        var commandName = command.Name.ToLowerInvariant();
+        using var timer = new PeriodicTimer(TimeSpan.FromHours(1));
 
-        return TryBlock(guild, user, commandName);
+        while (await timer.WaitForNextTickAsync())
+        {
+            var now = DateTime.UtcNow;
+            // once per hour delete expired entries
+            foreach (var ((guildId, commandName), dict) in _activeCooldowns)
+            {
+                // if this pair no longer has associated config, that means it has been removed.
+                // remove all cooldowns
+                if (!_settings.TryGetValue(guildId, out var inner)
+                    || !inner.TryGetValue(commandName, out var cdSeconds))
+                {
+                    _activeCooldowns.Remove((guildId, commandName), out _);
+                    continue;
+                }
+                
+                Cleanup(dict, cdSeconds);
+            }
+        }
     }
-}
 
-public class ActiveCooldown
-{
-    public string Command { get; set; }
-    public ulong UserId { get; set; }
+    private void Cleanup(ConcurrentDictionary<ulong, DateTime> dict, int cdSeconds)
+    {
+        var now = DateTime.UtcNow;
+        foreach (var (key, _) in dict.Where(x => (now - x.Value).Seconds > cdSeconds).ToArray())
+        {
+            dict.TryRemove(key, out _);
+        }
+    }
+
+    public void ClearCooldowns(ulong guildId, string cmdName)
+    {
+        if (_settings.TryGetValue(guildId, out var dict))
+            dict.TryRemove(cmdName, out _);
+
+        _activeCooldowns.TryRemove((guildId, cmdName), out _);
+    }
+
+    public void AddCooldown(ulong guildId, string name, int secs)
+    {
+        var sett = _settings.GetOrAdd(guildId, static _ => new());
+        sett[name] = secs;
+
+        // force cleanup 
+        if (_activeCooldowns.TryGetValue((guildId, name), out var dict))
+            Cleanup(dict, secs);
+    }
+
+    public IReadOnlyCollection<(string CommandName, int Seconds)> GetCommandCooldowns(ulong guildId)
+    {
+        if (!_settings.TryGetValue(guildId, out var dict))
+            return Array.Empty<(string, int)>();
+
+        return dict.Select(x => (x.Key, x.Value)).ToArray();
+    }
 }

src/NadekoBot/Modules/Permissions/CommandCooldown/CmdCdsCommands.cs

@@ -1,4 +1,5 @@
 #nullable disable
+using Humanizer.Localisation;
 using Microsoft.EntityFrameworkCore;
 using NadekoBot.Common.TypeReaders;
 using NadekoBot.Db;
@@ -12,12 +13,6 @@ public partial class Permissions
     [Group]
     public partial class CmdCdsCommands : NadekoModule
     {
-        private ConcurrentDictionary<ulong, ConcurrentHashSet<CommandCooldown>> CommandCooldowns
-            => _service.CommandCooldowns;
-
-        private ConcurrentDictionary<ulong, ConcurrentHashSet<ActiveCooldown>> ActiveCooldowns
-            => _service.ActiveCooldowns;
-
         private readonly DbService _db;
         private readonly CmdCdService _service;
 
@@ -40,12 +35,10 @@ public partial class Permissions
             await using (var uow = _db.GetDbContext())
             {
                 var config = uow.GuildConfigsForId(channel.Guild.Id, set => set.Include(gc => gc.CommandCooldowns));
-                var localSet = CommandCooldowns.GetOrAdd(channel.Guild.Id, new ConcurrentHashSet<CommandCooldown>());
 
                 var toDelete = config.CommandCooldowns.FirstOrDefault(cc => cc.CommandName == name);
                 if (toDelete is not null)
                     uow.Set<CommandCooldown>().Remove(toDelete);
-                localSet.RemoveWhere(cc => cc.CommandName == name);
                 if (secs != 0)
                 {
                     var cc = new CommandCooldown
@@ -54,7 +47,7 @@ public partial class Permissions
                         Seconds = secs
                     };
                     config.CommandCooldowns.Add(cc);
-                    localSet.Add(cc);
+                    _service.AddCooldown(channel.Guild.Id, name, secs);
                 }
 
                 await uow.SaveChangesAsync();
@@ -62,8 +55,7 @@ public partial class Permissions
 
             if (secs == 0)
             {
-                var activeCds = ActiveCooldowns.GetOrAdd(channel.Guild.Id, new ConcurrentHashSet<ActiveCooldown>());
-                activeCds.RemoveWhere(ac => ac.Command == name);
+                _service.ClearCooldowns(ctx.Guild.Id, cmdName);
                 await ReplyConfirmLocalizedAsync(strs.cmdcd_cleared(Format.Bold(name)));
             }
             else
@@ -84,19 +76,29 @@ public partial class Permissions
 
         [Cmd]
         [RequireContext(ContextType.Guild)]
-        public async Task AllCmdCooldowns()
+        public async Task AllCmdCooldowns(int page = 1)
         {
+            if (--page < 0)
+                return;
+            
             var channel = (ITextChannel)ctx.Channel;
-            var localSet = CommandCooldowns.GetOrAdd(channel.Guild.Id, new ConcurrentHashSet<CommandCooldown>());
+            var localSet = _service.GetCommandCooldowns(ctx.Guild.Id);
 
             if (!localSet.Any())
                 await ReplyConfirmLocalizedAsync(strs.cmdcd_none);
             else
             {
-                await channel.SendTableAsync("",
-                    localSet.Select(c => c.CommandName + ": " + c.Seconds + GetText(strs.sec)),
-                    s => $"{s,-30}",
-                    2);
+                await ctx.SendPaginatedConfirmAsync(page, curPage =>
+                {
+                    var items = localSet.Skip(curPage * 15)
+                        .Take(15)
+                        .Select(x => $"{Format.Code(x.CommandName)}: {x.Seconds.Seconds().Humanize(maxUnit: TimeUnit.Second, culture: Culture)}");
+
+                    return _eb.Create(ctx)
+                        .WithOkColor()
+                        .WithDescription(items.Join("\n"));
+
+                }, localSet.Count, 15);
             }
         }
     }