- Started cleanup of command handler

- Removed IUnloadableService
- Started removing INService (removed it from services which implement behavior interfaces) - wip
- Added scrutor for better service registration - wip

src/NadekoBot/Services/CommandHandler.cs

@@ -2,7 +2,6 @@
 using Discord.Commands;
 using Discord.Net;
 using Discord.WebSocket;
-using Microsoft.Extensions.DependencyInjection;
 using NadekoBot.Common.Collections;
 using NadekoBot.Common.ModuleBehaviors;
 using NadekoBot.Extensions;
@@ -10,26 +9,16 @@ using System;
 using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Collections.Immutable;
-using System.Diagnostics;
-using System.IO;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
+using Microsoft.Extensions.DependencyInjection;
 using NadekoBot.Common.Configs;
-using NadekoBot.Services;
 using NadekoBot.Db;
-using NadekoBot.Modules.Administration;
 using Serilog;
 
 namespace NadekoBot.Services
 {
-    public class GuildUserComparer : IEqualityComparer<IGuildUser>
-    {
-        public bool Equals(IGuildUser x, IGuildUser y) => x.Id == y.Id;
-
-        public int GetHashCode(IGuildUser obj) => obj.Id.GetHashCode();
-    }
-
     public class CommandHandler : INService
     {
         public const int GlobalCommandsCooldown = 750;
@@ -38,11 +27,8 @@ namespace NadekoBot.Services
         private readonly CommandService _commandService;
         private readonly BotConfigService _bss;
         private readonly Bot _bot;
+        private readonly IBehaviourExecutor _behaviourExecutor;
         private IServiceProvider _services;
-        private IEnumerable<IEarlyBehavior> _earlyBehaviors;
-        private IEnumerable<IInputTransformer> _inputTransformers;
-        private IEnumerable<ILateBlocker> _lateBlockers;
-        private IEnumerable<ILateExecutor> _lateExecutors;
         
         private ConcurrentDictionary<ulong, string> _prefixes { get; } = new ConcurrentDictionary<ulong, string>();
 
@@ -56,17 +42,24 @@ namespace NadekoBot.Services
         public ConcurrentHashSet<ulong> UsersOnShortCooldown { get; } = new ConcurrentHashSet<ulong>();
         private readonly Timer _clearUsersOnShortCooldown;
 
-        public CommandHandler(DiscordSocketClient client, DbService db, CommandService commandService,
-            BotConfigService bss, Bot bot, IServiceProvider services)
+        // todo move behaviours to a separate service
+        public CommandHandler(
+            DiscordSocketClient client,
+            DbService db,
+            CommandService commandService,
+            BotConfigService bss,
+            Bot bot,
+            IBehaviourExecutor behaviourExecutor,
+            IServiceProvider services)
         {
             _client = client;
             _commandService = commandService;
             _bss = bss;
             _bot = bot;
+            _behaviourExecutor = behaviourExecutor;
             _db = db;
             _services = services;
 
-            
             _clearUsersOnShortCooldown = new Timer(_ =>
             {
                 UsersOnShortCooldown.Clear();
@@ -118,27 +111,6 @@ namespace NadekoBot.Services
             return prefix;
         }
 
-
-        public void AddServices(IServiceCollection services)
-        {
-            _lateBlockers = services.Where(x => x.ImplementationType?.GetInterfaces().Contains(typeof(ILateBlocker)) ?? false)
-                .Select(x => _services.GetService(x.ImplementationType) as ILateBlocker)
-                .OrderByDescending(x => x.Priority)
-                .ToArray();
-
-            _lateExecutors = services.Where(x => x.ImplementationType?.GetInterfaces().Contains(typeof(ILateExecutor)) ?? false)
-                .Select(x => _services.GetService(x.ImplementationType) as ILateExecutor)
-                .ToArray();
-
-            _inputTransformers = services.Where(x => x.ImplementationType?.GetInterfaces().Contains(typeof(IInputTransformer)) ?? false)
-                .Select(x => _services.GetService(x.ImplementationType) as IInputTransformer)
-                .ToArray();
-
-            _earlyBehaviors = services.Where(x => x.ImplementationType?.GetInterfaces().Contains(typeof(IEarlyBehavior)) ?? false)
-                .Select(x => _services.GetService(x.ImplementationType) as IEarlyBehavior)
-                .ToArray();
-        }
-
         public async Task ExecuteExternal(ulong? guildId, ulong channelId, string commandText)
         {
             if (guildId != null)
@@ -172,8 +144,7 @@ namespace NadekoBot.Services
 
         private Task LogSuccessfulExecution(IUserMessage usrMsg, ITextChannel channel, params int[] execPoints)
         {
-            var bss = _services.GetService<BotConfigService>();
-            if (bss.Data.ConsoleOutputType == ConsoleOutputType.Normal)
+            if (_bss.GetRawData().ConsoleOutputType == ConsoleOutputType.Normal)
             {
                 Log.Information($"Command Executed after " + string.Join("/", execPoints.Select(x => (x * _oneThousandth).ToString("F3"))) + "s\n\t" +
                         "User: {0}\n\t" +
@@ -199,8 +170,7 @@ namespace NadekoBot.Services
 
         private void LogErroredExecution(string errorMessage, IUserMessage usrMsg, ITextChannel channel, params int[] execPoints)
         {
-            var bss = _services.GetService<BotConfigService>();
-            if (bss.Data.ConsoleOutputType == ConsoleOutputType.Normal)
+            if (_bss.GetRawData().ConsoleOutputType == ConsoleOutputType.Normal)
             {
                 Log.Warning($"Command Errored after " + string.Join("/", execPoints.Select(x => (x * _oneThousandth).ToString("F3"))) + "s\n\t" +
                             "User: {0}\n\t" +
@@ -231,7 +201,7 @@ namespace NadekoBot.Services
         {
             try
             {
-                if (msg.Author.IsBot || !_bot.Ready.Task.IsCompleted) //no bots, wait until bot connected and initialized
+                if (msg.Author.IsBot || !_bot.IsReady) //no bots, wait until bot connected and initialized
                     return;
 
                 if (!(msg is SocketUserMessage usrMsg))
@@ -258,62 +228,33 @@ namespace NadekoBot.Services
 
         public async Task TryRunCommand(SocketGuild guild, ISocketMessageChannel channel, IUserMessage usrMsg)
         {
-            var execTime = Environment.TickCount;
+            var startTime = Environment.TickCount;
 
-            //its nice to have early blockers and early blocking executors separate, but
-            //i could also have one interface with priorities, and just put early blockers on
-            //highest priority. :thinking:
-            foreach (var beh in _earlyBehaviors)
-            {
-                if (await beh.RunBehavior(_client, guild, usrMsg).ConfigureAwait(false))
-                {
-                    if (beh.BehaviorType == ModuleBehaviorType.Blocker)
-                    {
-                        Log.Information("Blocked User: [{0}] Message: [{1}] Service: [{2}]", usrMsg.Author,
-                            usrMsg.Content, beh.GetType().Name);
-                    }
-                    else if (beh.BehaviorType == ModuleBehaviorType.Executor)
-                    {
-                        Log.Information("User [{0}] executed [{1}] in [{2}]", usrMsg.Author, usrMsg.Content,
-                            beh.GetType().Name);
+            var blocked = await _behaviourExecutor.RunEarlyBehavioursAsync(guild, usrMsg);
+            if (blocked)
+                return;
 
-                    }
-
-                    return;
-                }
-            }
-
-            var exec2 = Environment.TickCount - execTime;
+            var blockTime = Environment.TickCount - startTime;
 
+            var messageContent = await _behaviourExecutor.RunInputTransformersAsync(guild, usrMsg);
             
-            string messageContent = usrMsg.Content;
-            foreach (var exec in _inputTransformers)
-            {
-                string newContent;
-                if ((newContent = await exec.TransformInput(guild, usrMsg.Channel, usrMsg.Author, messageContent)
-                    .ConfigureAwait(false)) != messageContent.ToLowerInvariant())
-                {
-                    messageContent = newContent;
-                    break;
-                }
-            }
             var prefix = GetPrefix(guild?.Id);
             var isPrefixCommand = messageContent.StartsWith(".prefix", StringComparison.InvariantCultureIgnoreCase);
             // execute the command and measure the time it took
             if (messageContent.StartsWith(prefix, StringComparison.InvariantCulture) || isPrefixCommand)
             {
                 var (Success, Error, Info) = await ExecuteCommandAsync(new CommandContext(_client, usrMsg), messageContent, isPrefixCommand ? 1 : prefix.Length, _services, MultiMatchHandling.Best).ConfigureAwait(false);
-                execTime = Environment.TickCount - execTime;
+                startTime = Environment.TickCount - startTime;
 
                 if (Success)
                 {
-                    await LogSuccessfulExecution(usrMsg, channel as ITextChannel, exec2, execTime).ConfigureAwait(false);
+                    await LogSuccessfulExecution(usrMsg, channel as ITextChannel, blockTime, startTime).ConfigureAwait(false);
                     await CommandExecuted(usrMsg, Info).ConfigureAwait(false);
                     return;
                 }
                 else if (Error != null)
                 {
-                    LogErroredExecution(Error, usrMsg, channel as ITextChannel, exec2, execTime);
+                    LogErroredExecution(Error, usrMsg, channel as ITextChannel, blockTime, startTime);
                     if (guild != null)
                         await CommandErrored(Info, channel as ITextChannel, Error).ConfigureAwait(false);
                 }
@@ -323,11 +264,7 @@ namespace NadekoBot.Services
                 await OnMessageNoTrigger(usrMsg).ConfigureAwait(false);
             }
 
-            foreach (var exec in _lateExecutors)
-            {
-                await exec.LateExecute(_client, guild, usrMsg).ConfigureAwait(false);
-            }
-
+            await _behaviourExecutor.RunLateExecutorsAsync(guild, usrMsg);
         }
 
         public Task<(bool Success, string Error, CommandInfo Info)> ExecuteCommandAsync(CommandContext context, string input, int argPos, IServiceProvider serviceProvider, MultiMatchHandling multiMatchHandling = MultiMatchHandling.Exception)
@@ -423,17 +360,9 @@ namespace NadekoBot.Services
                 return (false, null, cmd);
             //return SearchResult.FromError(CommandError.Exception, "You are on a global cooldown.");
 
-            var commandName = cmd.Aliases.First();
-            foreach (var exec in _lateBlockers)
-            {
-                if (await exec.TryBlockLate(_client, context, cmd.Module.GetTopLevelModule().Name, cmd)
-                    .ConfigureAwait(false))
-                {
-                    Log.Information("Late blocking User [{0}] Command: [{1}] in [{2}]", context.User, commandName,
-                        exec.GetType().Name);
-                    return (false, null, cmd);
-                }
-            }
+            var blocked = await _behaviourExecutor.RunLateBlockersAsync(context, cmd);
+            if (blocked)
+                return (false, null, cmd);
 
             //If we get this far, at least one parse was successful. Execute the most likely overload.
             var chosenOverload = successfulParses[0];

src/NadekoBot/Services/IBehaviourExecutor.cs

@@ -0,0 +1,15 @@
+using System.Threading.Tasks;
+using Discord;
+using Discord.Commands;
+using Discord.WebSocket;
+
+namespace NadekoBot.Services
+{
+    public interface IBehaviourExecutor
+    {
+        public Task<bool> RunEarlyBehavioursAsync(SocketGuild guild, IUserMessage usrMsg);
+        public Task<string> RunInputTransformersAsync(SocketGuild guild, IUserMessage usrMsg);
+        Task<bool> RunLateBlockersAsync(ICommandContext context, CommandInfo cmd);
+        Task RunLateExecutorsAsync(SocketGuild guild, IUserMessage usrMsg);
+    }
+}

src/NadekoBot/Services/INService.cs

@@ -1,6 +1,4 @@
-using System.Threading.Tasks;
-
-namespace NadekoBot.Services
+namespace NadekoBot.Services
 {
     /// <summary>
     /// All services must implement this interface in order to be auto-discovered by the DI system
@@ -9,12 +7,4 @@ namespace NadekoBot.Services
     {
         
     }
-
-    /// <summary>
-    /// All services which require cleanup after they are unloaded must implement this interface
-    /// </summary>
-    public interface IUnloadableService
-    {
-        Task Unload();
-    }
 }

src/NadekoBot/Services/Impl/BehaviorExecutor.cs

@@ -0,0 +1,100 @@
+using System;
+using System.Collections.Generic;
+using System.Threading.Tasks;
+using Discord;
+using Discord.Commands;
+using Discord.WebSocket;
+using NadekoBot.Common.ModuleBehaviors;
+using NadekoBot.Extensions;
+using Serilog;
+
+namespace NadekoBot.Services
+{
+    public sealed class BehaviorExecutor : IBehaviourExecutor
+    {
+        private readonly DiscordSocketClient _client;
+        private readonly IEnumerable<ILateExecutor> _lateExecutors;
+        private readonly IEnumerable<ILateBlocker> _lateBlockers;
+        private readonly IEnumerable<IEarlyBehavior> _earlyBehaviors;
+        private readonly IEnumerable<IInputTransformer> _transformers;
+
+        public BehaviorExecutor(
+            DiscordSocketClient client,
+            IEnumerable<ILateExecutor> lateExecutors,
+            IEnumerable<ILateBlocker> lateBlockers,
+            IEnumerable<IEarlyBehavior> earlyBehaviors,
+            IEnumerable<IInputTransformer> transformers)
+        {
+            _client = client;
+            _lateExecutors = lateExecutors;
+            _lateBlockers = lateBlockers;
+            _earlyBehaviors = earlyBehaviors;
+            _transformers = transformers;
+        }
+
+        // todo early behaviors should print for themselves
+        public async Task<bool> RunEarlyBehavioursAsync(SocketGuild guild, IUserMessage usrMsg)
+        {
+            foreach (var beh in _earlyBehaviors)
+            {
+                if (await beh.RunBehavior(guild, usrMsg))
+                {
+                    return true;
+                }
+            }
+
+            return false;
+        }
+
+        public async Task<string> RunInputTransformersAsync(SocketGuild guild, IUserMessage usrMsg)
+        {
+            var messageContent = usrMsg.Content;
+            foreach (var exec in _transformers)
+            {
+                string newContent;
+                if ((newContent = await exec.TransformInput(guild, usrMsg.Channel, usrMsg.Author, messageContent)) 
+                    != messageContent.ToLowerInvariant())
+                {
+                    messageContent = newContent;
+                    break;
+                }
+            }
+
+            return messageContent;
+        }
+
+        public async Task<bool> RunLateBlockersAsync(ICommandContext ctx, CommandInfo cmd)
+        {
+            foreach (var exec in _lateBlockers)
+            {
+                if (await exec.TryBlockLate(ctx, cmd.Module.GetTopLevelModule().Name, cmd))
+                {
+                    Log.Information("Late blocking User [{0}] Command: [{1}] in [{2}]", 
+                        ctx.User,
+                        cmd.Aliases[0],
+                        exec.GetType().Name);
+                    return true;
+                }
+            }
+
+            return false;
+        }
+
+        public async Task RunLateExecutorsAsync(SocketGuild guild, IUserMessage usrMsg)
+        {
+            foreach (var exec in _lateExecutors)
+            {
+                try
+                {
+                    await exec.LateExecute(guild, usrMsg).ConfigureAwait(false);
+                }
+                catch (Exception ex)
+                {
+                    Log.Error(ex, "Error in {TypeName} late executor: {ErrorMessage}",
+                        exec.GetType().Name,
+                        ex.Message);
+                }
+            }
+        }
+    }
+}

src/NadekoBot/Services/Impl/ImagesService.cs

@@ -24,7 +24,6 @@ namespace NadekoBot.Services
         private IDatabase _db => _con.GetDatabase();
 
         private const string _basePath = "data/";
-        private const string _oldBasePath = "data/images/";
         private const string _cardsPath = "data/images/cards";
 
         public ImageUrls ImageUrls { get; private set; }
@@ -79,112 +78,11 @@ namespace NadekoBot.Services
             _con = con;
             _creds = creds;
             _http = new HttpClient();
-
-            Migrate();
+            
             ImageUrls = JsonConvert.DeserializeObject<ImageUrls>(
                         File.ReadAllText(Path.Combine(_basePath, "images.json")));
         }
 
-        private void Migrate()
-        {
-            try
-            {
-                Migrate1();
-                Migrate2();
-                Migrate3();
-            }
-            catch (Exception ex)
-            {
-                Log.Warning(ex.Message);
-                Log.Error("Something has been incorrectly formatted in your 'images.json' file.\n" +
-                               "Use the 'images_example.json' file as reference to fix it and restart the bot.");
-            }
-        }
-
-        private void Migrate1()
-        {
-            if (!File.Exists(Path.Combine(_oldBasePath, "images.json")))
-                return;
-            Log.Information("Migrating images v0 to images v1.");
-            // load old images
-            var oldUrls = JsonConvert.DeserializeObject<ImageUrls>(
-                    File.ReadAllText(Path.Combine(_oldBasePath, "images.json")));
-            // load new images
-            var newUrls = JsonConvert.DeserializeObject<ImageUrls>(
-                    File.ReadAllText(Path.Combine(_basePath, "images.json")));
-
-            //swap new links with old ones if set. Also update old links.
-            newUrls.Coins = oldUrls.Coins;
-
-            newUrls.Currency = oldUrls.Currency;
-            newUrls.Dice = oldUrls.Dice;
-            newUrls.Rategirl = oldUrls.Rategirl;
-            newUrls.Xp = oldUrls.Xp;
-            newUrls.Version = 1;
-
-            File.WriteAllText(Path.Combine(_basePath, "images.json"),
-                JsonConvert.SerializeObject(newUrls, Formatting.Indented));
-            File.Delete(Path.Combine(_oldBasePath, "images.json"));
-        }
-
-        private void Migrate2()
-        {
-            // load new images
-            var urls = JsonConvert.DeserializeObject<ImageUrls>(File.ReadAllText(Path.Combine(_basePath, "images.json")));
-
-            if (urls.Version >= 2)
-                return;
-            Log.Information("Migrating images v1 to images v2.");
-            urls.Version = 2;
-
-            var prefix = $"{_creds.RedisKey()}_localimg_";
-            _db.KeyDelete(new[] {
-                prefix + "heads",
-                prefix + "tails",
-                prefix + "dice",
-                prefix + "slot_background",
-                prefix + "slotnumbers",
-                prefix + "slotemojis",
-                prefix + "wife_matrix",
-                prefix + "rategirl_dot",
-                prefix + "xp_card",
-                prefix + "rip",
-                prefix + "rip_overlay" }
-            .Select(x => (RedisKey)x).ToArray());
-
-            File.WriteAllText(Path.Combine(_basePath, "images.json"), JsonConvert.SerializeObject(urls, Formatting.Indented));
-        }
-
-        private void Migrate3()
-        {
-            var urls = JsonConvert.DeserializeObject<ImageUrls>(
-                    File.ReadAllText(Path.Combine(_basePath, "images.json")));
-
-            if (urls.Version >= 3)
-                return;
-            urls.Version = 3;
-            Log.Information("Migrating images v2 to images v3.");
-
-            var baseStr = "https://nadeko-pictures.nyc3.digitaloceanspaces.com/other/currency/";
-
-            var replacementTable = new Dictionary<Uri, Uri>()
-            {
-                {new Uri(baseStr + "0.jpg"), new Uri(baseStr + "0.png") },
-                {new Uri(baseStr + "1.jpg"), new Uri(baseStr + "1.png") },
-                {new Uri(baseStr + "2.jpg"), new Uri(baseStr + "2.png") }
-            };
-
-            if (replacementTable.Keys.Any(x => urls.Currency.Contains(x)))
-            {
-                urls.Currency = urls.Currency.Select(x => replacementTable.TryGetValue(x, out var newUri)
-                    ? newUri
-                    : x).Append(new Uri(baseStr + "3.png"))
-                    .ToArray();
-            }
-
-            File.WriteAllText(Path.Combine(_basePath, "images.json"), JsonConvert.SerializeObject(urls, Formatting.Indented));
-        }
-
         public async Task<bool> AllKeysExist()
         {
             try