ModTools configuration redone

Defining config for ModTools now is similar to that of other features.

Additionally, merged the Ban and Kick commands into a single file,
and it no longer scans itself for attribute data on load.

Feature/ModTools/CommandBase.cs

@@ -1,8 +1,11 @@
 using Discord.WebSocket;
 using Newtonsoft.Json.Linq;
 using Noikoio.RegexBot.ConfigItem;
+using System;
 using System.Collections.Generic;
+using System.Collections.ObjectModel;
 using System.Diagnostics;
+using System.Reflection;
 using System.Text.RegularExpressions;
 using System.Threading.Tasks;
 
@@ -11,24 +14,17 @@ namespace Noikoio.RegexBot.Feature.ModTools
     [DebuggerDisplay("{Label}-type command")]
     abstract class CommandBase
     {
-        private readonly CommandListener _modtools;
+        private readonly ModTools _modtools;
         private readonly string _label;
         private readonly string _command;
-
+        
-        public static readonly Regex UserMention = new Regex(@"<@!?(?<snowflake>\d+)>", RegexOptions.Compiled);
-        public static readonly Regex RoleMention = new Regex(@"<@&(?<snowflake>\d+)>", RegexOptions.Compiled);
-        public static readonly Regex ChannelMention = new Regex(@"<#(?<snowflake>\d+)>", RegexOptions.Compiled);
-        public static readonly Regex EmojiMatch = new Regex(@"<:(?<name>[A-Za-z0-9_]{2,}):(?<ID>\d+)>", RegexOptions.Compiled);
-
         public string Label => _label;
         public string Command => _command;
 
-        protected CommandBase(CommandListener l, JObject conf)
+        protected CommandBase(ModTools l, string label, JObject conf)
         {
             _modtools = l;
-            _label = conf["label"].Value<string>();
+            _label = label;
-            if (string.IsNullOrWhiteSpace(_label))
-                throw new RuleImportException("Command label is missing.");
             _command = conf["command"].Value<string>();
         }
 
@@ -38,5 +34,54 @@ namespace Noikoio.RegexBot.Feature.ModTools
         {
             return _modtools.Log($"{Label}: {text}");
         }
+
+        #region Config loading
+        private static readonly ReadOnlyDictionary<string, Type> _commands =
+            new ReadOnlyDictionary<string, Type>(
+            new Dictionary<string, Type>(StringComparer.OrdinalIgnoreCase)
+            {
+                // Define all command types and their corresponding Types here
+                { "ban",        typeof(Commands.Ban) },
+                { "kick",       typeof(Commands.Kick) }
+            });
+
+        public static CommandBase CreateInstance(ModTools root, JProperty def)
+        {
+            string label = def.Name;
+            if (string.IsNullOrWhiteSpace(label)) throw new RuleImportException("Label cannot be blank.");
+
+            var definition = (JObject)def.Value;
+            string cmdinvoke = definition["command"].Value<string>();
+            if (string.IsNullOrWhiteSpace(cmdinvoke))
+                throw new RuleImportException($"{label}: 'command' value was not specified.");
+            if (cmdinvoke.Contains(" "))
+                throw new RuleImportException($"{label}: 'command' must not contain spaces.");
+
+            string ctypestr = definition["type"]?.Value<string>();
+            if (string.IsNullOrWhiteSpace(ctypestr))
+                throw new RuleImportException($"Value 'type' must be specified in definition for '{label}'.");
+            Type ctype;
+            if (!_commands.TryGetValue(ctypestr, out ctype))
+                throw new RuleImportException($"The given 'type' value is invalid in definition for '{label}'.");
+            
+            try
+            {
+                return (CommandBase)Activator.CreateInstance(ctype, root, label, definition);
+            }
+            catch (TargetInvocationException ex)
+            {
+                if (ex.InnerException is RuleImportException)
+                    throw new RuleImportException($"Error in configuration for command '{label}': {ex.InnerException.Message}");
+                else throw;
+            }
+        }
+        #endregion
+
+        #region Helper methods and values
+        protected static readonly Regex UserMention = new Regex(@"<@!?(?<snowflake>\d+)>", RegexOptions.Compiled);
+        protected static readonly Regex RoleMention = new Regex(@"<@&(?<snowflake>\d+)>", RegexOptions.Compiled);
+        protected static readonly Regex ChannelMention = new Regex(@"<#(?<snowflake>\d+)>", RegexOptions.Compiled);
+        protected static readonly Regex EmojiMatch = new Regex(@"<:(?<name>[A-Za-z0-9_]{2,}):(?<ID>\d+)>", RegexOptions.Compiled);
+        #endregion
     }
 }

Feature/ModTools/CommandTypeAttribute.cs

@@ -1,54 +0,0 @@
-using System;
-using System.Collections.Generic;
-using System.Linq;
-using System.Reflection;
-
-namespace Noikoio.RegexBot.Feature.ModTools
-{
-    /// <summary>
-    /// Specifies this class's corresponding value when being defined in configuration
-    /// under a custom command's "type" value.
-    /// </summary>
-    [AttributeUsage(AttributeTargets.Class, AllowMultiple = false, Inherited = false)]
-    class CommandTypeAttribute : Attribute
-    {
-        readonly string _type;
-        public string TypeName => _type;
-        public CommandTypeAttribute(string typeName) => _type = typeName;
-
-        private static Dictionary<string, Type> _sTypes;
-        /// <summary>
-        /// Translates a command type defined from configuration into a usable
-        /// <see cref="System.Type"/> deriving from CommandBase.
-        /// </summary>
-        internal static Type GetCommandType(string input)
-        {
-            if (_sTypes == null)
-            {
-                var newtypelist = new Dictionary<string, Type>(StringComparer.OrdinalIgnoreCase);
-                var ctypes = from type in Assembly.GetEntryAssembly().GetTypes()
-                             where typeof(CommandBase).IsAssignableFrom(type)
-                             select type;
-                foreach (var type in ctypes)
-                {
-                    var attr = type.GetTypeInfo().GetCustomAttribute<CommandTypeAttribute>();
-                    if (attr == null)
-                    {
-#if DEBUG
-                        Console.WriteLine($"{type.FullName} does not define a {nameof(CommandTypeAttribute)}");
-#endif
-                        continue;
-                    }
-                    newtypelist.Add(attr.TypeName, type);
-                }
-                _sTypes = newtypelist;
-            }
-
-            if (_sTypes.TryGetValue(input, out var cmdtype))
-            {
-                return cmdtype;
-            }
-            return null;
-        }
-    }
-}

Feature/ModTools/Commands/BanKick.cs

@@ -6,26 +6,34 @@ using System;
 using System.Text.RegularExpressions;
 using System.Threading.Tasks;
 
-namespace Noikoio.RegexBot.Feature.ModTools
+namespace Noikoio.RegexBot.Feature.ModTools.Commands
 {
-    [CommandType("ban")]
+    
-    class BanCommand : CommandBase
+    class BanKick : CommandBase
     {
+        // Ban and kick commands are highly similar in implementation, and thus are handled in a single class.
+        protected enum CommandMode { Ban, Kick }
+        private readonly CommandMode _mode;
+
         private readonly bool _forceReason;
         private readonly int _purgeDays;
+        private readonly string _successMsg;
 
         // Configuration:
         // "forcereason" - boolean; Force a reason to be given. Defaults to false.
-        // "purgedays" - integer; Number of days of target's post history to delete. Must be between 0-7 inclusive.
+        // "purgedays" - integer; Number of days of target's post history to delete, if banning.
-        //               Defaults to 0.
+        //               Must be between 0-7 inclusive. Defaults to 0.
-        public BanCommand(CommandListener l, JObject conf) : base(l, conf)
+        // "successmsg" - Message to display on command success. Overrides default.
+        protected BanKick(ModTools l, string label, JObject conf, CommandMode mode) : base(l, label, conf)
         {
+            _mode = mode;
             _forceReason = conf["forcereason"]?.Value<bool>() ?? false;
             _purgeDays = conf["purgedays"]?.Value<int>() ?? 0;
-            if (_purgeDays > 7 || _purgeDays < 0)
+            if (_mode == CommandMode.Ban && (_purgeDays > 7 || _purgeDays < 0))
             {
                 throw new RuleImportException("The value of 'purgedays' must be between 0 and 7.");
             }
+            _successMsg = conf["successmsg"]?.Value<string>();
         }
 
         // Usage: (command) (mention) (reason)
@@ -51,7 +59,7 @@ namespace Noikoio.RegexBot.Feature.ModTools
                 // No reason given
                 if (_forceReason)
                 {
-                    await SendUsageMessage(msg, ":x: **You must specify a ban reason.**");
+                    await SendUsageMessage(msg, ":x: **You must specify a reason.**");
                     return;
                 }
             }
@@ -66,7 +74,7 @@ namespace Noikoio.RegexBot.Feature.ModTools
             if (ulong.TryParse(targetstr, out targetuid))
             {
                 targetobj = g.GetUser(targetuid);
-                targetdisp = (targetobj == null ? $"with ID {targetuid}" : targetobj.ToString());
+                targetdisp = (targetobj == null ? $"ID {targetuid}" : targetobj.ToString());
             }
             else
             {
@@ -74,26 +82,35 @@ namespace Noikoio.RegexBot.Feature.ModTools
                 return;
             }
 
+            if (_mode == CommandMode.Kick && targetobj == null)
+            {
+                // Can't kick without obtaining the user object
+                await SendUsageMessage(msg, ":x: **Unable to find the target user.**");
+                return;
+            }
+
             try
             {
                 if (reason != null) reason = Uri.EscapeDataString(reason); // TODO remove when fixed in library
-                await g.AddBanAsync(targetuid, _purgeDays, reason);
+                if (_mode == CommandMode.Ban) await g.AddBanAsync(targetuid, _purgeDays, reason);
-                await msg.Channel.SendMessageAsync($":white_check_mark: Banned user **{targetdisp}**.");
+                else await targetobj.KickAsync(reason);
+                string resultmsg = BuildSuccessMessage(targetdisp);
+                await msg.Channel.SendMessageAsync(resultmsg);
             }
             catch (Discord.Net.HttpException ex)
             {
-                const string err = ":x: **Failed to ban user.** ";
+                string err = ":x: **Failed to " + (_mode == CommandMode.Ban ? "ban" : "kick") + " user:** ";
                 if (ex.HttpCode == System.Net.HttpStatusCode.Forbidden)
                 {
-                    await msg.Channel.SendMessageAsync(err + "I do not have permission to do that action.");
+                    await msg.Channel.SendMessageAsync(err + "I do not have sufficient permissions to do that action.");
                 }
                 else if (ex.HttpCode == System.Net.HttpStatusCode.NotFound)
                 {
-                    await msg.Channel.SendMessageAsync(err + "The target user appears to have left the server.");
+                    await msg.Channel.SendMessageAsync(err + "The target user appears to no longer exist.");
                 }
                 else
                 {
-                    await msg.Channel.SendMessageAsync(err + "An unknown error prevented me from doing that action.");
+                    await msg.Channel.SendMessageAsync(err + "An unknown error occurred. Details have been logged.");
                     await Log(ex.ToString());
                 }
             }
@@ -114,5 +131,27 @@ namespace Noikoio.RegexBot.Feature.ModTools
             };
             await m.Channel.SendMessageAsync(message ?? "", embed: usageEmbed);
         }
+
+        private string BuildSuccessMessage(string targetstr)
+        {
+            const string defaultmsgBan = ":white_check_mark: Banned user **$target**.";
+            const string defaultmsgKick = ":white_check_mark: Kicked user **$target**.";
+
+            string msg = _successMsg ?? (_mode == CommandMode.Ban ? defaultmsgBan : defaultmsgKick);
+
+            return msg.Replace("$target", targetstr);
+        }
+    }
+
+    class Ban : BanKick
+    {
+        public Ban(ModTools l, string label, JObject conf)
+            : base(l, label, conf, CommandMode.Ban) { }
+    }
+
+    class Kick : BanKick
+    {
+        public Kick(ModTools l, string label, JObject conf)
+            : base(l, label, conf, CommandMode.Kick) { }
     }
 }

Feature/ModTools/KickCommand.cs

@@ -1,98 +0,0 @@
-using Discord;
-using Discord.WebSocket;
-using Newtonsoft.Json.Linq;
-using System;
-using System.Text.RegularExpressions;
-using System.Threading.Tasks;
-
-namespace Noikoio.RegexBot.Feature.ModTools
-{
-    [CommandType("kick")]
-    class KickCommand : CommandBase
-    {
-        private readonly bool _forceReason;
-
-        // Configuration:
-        // "forcereason" - boolean; Force a reason to be given. Defaults to false.
-        public KickCommand(CommandListener l, JObject conf) : base(l, conf)
-        {
-            _forceReason = conf["forcereason"]?.Value<bool>() ?? false;
-        }
-        
-        public override async Task Invoke(SocketGuild g, SocketMessage msg)
-        {
-            string[] line = msg.Content.Split(new char[] { ' ' }, 3, StringSplitOptions.RemoveEmptyEntries);
-            string targetstr;
-            string reason = null;
-            if (line.Length < 2)
-            {
-                await SendUsageMessage(msg, null);
-                return;
-            }
-            targetstr = line[1];
-
-            if (line.Length == 3) reason = line[2]; 
-            if (_forceReason && reason == null)
-            {
-                await SendUsageMessage(msg, ":x: **You must specify a kick reason.**");
-                return;
-            }
-
-            // Getting SocketGuildUser target
-            Match m = UserMention.Match(targetstr);
-            if (m.Success) targetstr = m.Groups["snowflake"].Value;
-
-            SocketGuildUser targetobj = null;
-            if (ulong.TryParse(targetstr, out var snowflake))
-            {
-                targetobj = g.GetUser(snowflake);
-                if (targetobj == null)
-                {
-                    await SendUsageMessage(msg, ":x: **Unable to determine the target user.**");
-                    return;
-                }
-            }
-            else
-            {
-                await SendUsageMessage(msg, ":x: **The given target is not valid.**");
-                return;
-            }
-            
-            try
-            {
-                if (reason != null) reason = Uri.EscapeDataString(reason); // TODO remove when fixed in library
-                await targetobj.KickAsync(reason);
-                await msg.Channel.SendMessageAsync($":white_check_mark: Kicked user **{targetobj.ToString()}**.");
-            }
-            catch (Discord.Net.HttpException ex)
-            {
-                const string err = ":x: **Failed to kick user.** ";
-                if (ex.HttpCode == System.Net.HttpStatusCode.Forbidden)
-                {
-                    await msg.Channel.SendMessageAsync(err + "I do not have permission to do that action.");
-                }
-                else if (ex.HttpCode == System.Net.HttpStatusCode.NotFound)
-                {
-                    await msg.Channel.SendMessageAsync(err + "The target user appears to have left the server.");
-                }
-                else
-                {
-                    await msg.Channel.SendMessageAsync(err + "An unknown error prevented me from doing that action.");
-                    await Log(ex.ToString());
-                }
-            }
-        }
-
-        private async Task SendUsageMessage(SocketMessage m, string message)
-        {
-            var usageEmbed = new EmbedBuilder()
-            {
-                Title = "Usage",
-                Description = $"{this.Command} [user or user ID] " + (_forceReason ? "[reason]" : "*[reason]*") + "\n" +
-                    "Kicks the given user from this server and " + (_forceReason ? "" : "optionally ") +
-                    "logs a reason for the kick."
-            };
-            await m.Channel.SendMessageAsync(message ?? "", embed: usageEmbed);
-        }
-    }
-}

Feature/ModTools/ModTools.cs

@@ -14,12 +14,12 @@ namespace Noikoio.RegexBot.Feature.ModTools
     /// Entry point for the ModTools feature.
     /// This feature implements moderation commands that are defined and enabled in configuration.
     /// </summary>
-    // We are not using Discord.Net's Commands extension, as it doesn't allow for changes during runtime.
+    // We are not using Discord.Net's Commands extension, as it does not allow for changes during runtime.
-    class CommandListener : BotFeature
+    class ModTools : BotFeature
     {
         public override string Name => "ModTools";
         
-        public CommandListener(DiscordSocketClient client) : base(client)
+        public ModTools(DiscordSocketClient client) : base(client)
         {
             client.MessageReceived += Client_MessageReceived;
         }
@@ -67,44 +67,21 @@ namespace Noikoio.RegexBot.Feature.ModTools
         [ConfigSection("modtools")]
         public override async Task<object> ProcessConfiguration(JToken configSection)
         {
-            var newcmds = new Dictionary<string, CommandBase>(StringComparer.OrdinalIgnoreCase);
+            var commands = new Dictionary<string, CommandBase>(StringComparer.OrdinalIgnoreCase);
-            foreach (JObject definition in configSection)
-            {
-                string label = definition["label"].Value<string>();
-                if (string.IsNullOrWhiteSpace(label))
-                    throw new RuleImportException("A 'label' value was not specified in a command definition.");
 
-                string cmdinvoke = definition["command"].Value<string>();
+            foreach (var def in configSection.Children<JProperty>())
-                if (string.IsNullOrWhiteSpace(cmdinvoke))
+            {
-                    throw new RuleImportException($"{label}: 'command' value was not specified.");
+                string label = def.Name;
-                if (cmdinvoke.Contains(" "))
+                var cmd = CommandBase.CreateInstance(this, def);
-                    throw new RuleImportException($"{label}: 'command' must not contain spaces.");
+                if (commands.ContainsKey(cmd.Command))
-                if (newcmds.TryGetValue(cmdinvoke, out var cmdexisting))
                     throw new RuleImportException(
                         $"{label}: 'command' value must not be equal to that of another definition. " +
-                        $"Given value is being used for {cmdexisting.Label}.");
+                        $"Given value is being used for {commands[cmd.Command].Label}.");
-                        
 
-                string ctypestr = definition["type"].Value<string>();
+                commands.Add(cmd.Command, cmd);
-                if (string.IsNullOrWhiteSpace(ctypestr))
-                    throw new RuleImportException($"Value 'type' must be specified in definition for '{label}'.");
-                var ctype = CommandTypeAttribute.GetCommandType(ctypestr);
-
-                CommandBase cmd;
-                try
-                {
-                    cmd = (CommandBase)Activator.CreateInstance(ctype, this, definition);
-                }
-                catch (TargetInvocationException ex)
-                {
-                    if (ex.InnerException is RuleImportException)
-                        throw new RuleImportException($"Error in configuration for '{label}': {ex.InnerException.Message}");
-                    throw;
-                }
-                await Log($"'{label}' created; using command {cmdinvoke}");
-                newcmds.Add(cmdinvoke, cmd);
             }
-            return new ReadOnlyDictionary<string, CommandBase>(newcmds);
+            await Log($"Loaded {commands.Count} command definition(s).");
+            return new ReadOnlyDictionary<string, CommandBase>(commands);
         }
 
         public new Task Log(string text) => base.Log(text);

Feature/ModTools/TestCommand.cs

@@ -1,24 +0,0 @@
-using Discord.WebSocket;
-using Newtonsoft.Json.Linq;
-using Noikoio.RegexBot.ConfigItem;
-using System.Threading.Tasks;
-
-namespace Noikoio.RegexBot.Feature.ModTools
-{
-#if DEBUG
-    [CommandType("test")]
-    class TestCommand : CommandBase
-    {
-        public TestCommand(CommandListener l, JObject conf) : base(l, conf) {
-            bool? doCrash = conf["crash"]?.Value<bool>();
-            if (doCrash.HasValue && doCrash.Value)
-                throw new RuleImportException("Throwing exception in constructor upon request.");
-        }
-
-        public override async Task Invoke(SocketGuild g, SocketMessage msg)
-        {
-            await msg.Channel.SendMessageAsync("This is the test command. It is labeled: " + this.Label);
-        }
-    }
-#endif
-}

RegexBot.cs

@@ -45,7 +45,7 @@ namespace Noikoio.RegexBot
             _features = new BotFeature[]
             {
                 new Feature.AutoMod.AutoMod(_client),
-                new Feature.ModTools.CommandListener(_client),
+                new Feature.ModTools.ModTools(_client),
                 new Feature.AutoRespond.AutoRespond(_client)
             };
             var dlog = Logger.GetLogger("Discord.Net");

RegexBot.csproj

@@ -4,7 +4,7 @@
     <OutputType>Exe</OutputType>
     <TargetFramework>netcoreapp2.0</TargetFramework>
     <RootNamespace>Noikoio.RegexBot</RootNamespace>
-    <AssemblyVersion>2.0.0.0</AssemblyVersion>
+    <AssemblyVersion>2.1.0.0</AssemblyVersion>
     <Description>Highly configurable Discord moderation bot</Description>
     <Authors>Noikoio</Authors>
     <Company />