Add best-choice command selection to CommandService

src/Discord.Net.Commands/CommandService.cs

@@ -33,7 +33,7 @@ namespace Discord.Commands
 
         public IEnumerable<ModuleInfo> Modules => _moduleDefs.Select(x => x);
         public IEnumerable<CommandInfo> Commands => _moduleDefs.SelectMany(x => x.Commands);
-        public ILookup<Type, TypeReader> TypeReaders => _typeReaders.SelectMany(x => x.Value.Select(y => new {y.Key, y.Value})).ToLookup(x => x.Key, x => x.Value);
+        public ILookup<Type, TypeReader> TypeReaders => _typeReaders.SelectMany(x => x.Value.Select(y => new { y.Key, y.Value })).ToLookup(x => x.Key, x => x.Value);
 
         public CommandService() : this(new CommandServiceConfig()) { }
         public CommandService(CommandServiceConfig config)
@@ -59,6 +59,9 @@ namespace Discord.Commands
             foreach (var type in PrimitiveParsers.SupportedTypes)
                 _defaultTypeReaders[type] = PrimitiveTypeReader.Create(type);
 
+            _defaultTypeReaders[typeof(string)] =
+                new PrimitiveTypeReader<string>((string x, out string y) => { y = x; return true; }, 0);
+
             var entityTypeReaders = ImmutableList.CreateBuilder<Tuple<Type, Type>>();
             entityTypeReaders.Add(new Tuple<Type, Type>(typeof(IMessage), typeof(MessageTypeReader<>)));
             entityTypeReaders.Add(new Tuple<Type, Type>(typeof(IChannel), typeof(ChannelTypeReader<>)));
@@ -195,7 +198,7 @@ namespace Discord.Commands
         }
         public void AddTypeReader(Type type, TypeReader reader)
         {
-            var readers = _typeReaders.GetOrAdd(type, x=> new ConcurrentDictionary<Type, TypeReader>());
+            var readers = _typeReaders.GetOrAdd(type, x => new ConcurrentDictionary<Type, TypeReader>());
             readers[reader.GetType()] = reader;
         }
         internal IDictionary<Type, TypeReader> GetTypeReaders(Type type)
@@ -232,13 +235,13 @@ namespace Discord.Commands
         }
 
         //Execution
-        public SearchResult Search(ICommandContext context, int argPos) 
+        public SearchResult Search(ICommandContext context, int argPos)
             => Search(context, context.Message.Content.Substring(argPos));
         public SearchResult Search(ICommandContext context, string input)
         {
             string searchInput = _caseSensitive ? input : input.ToLowerInvariant();
             var matches = _map.GetCommands(searchInput).OrderByDescending(x => x.Command.Priority).ToImmutableArray();
-            
+
             if (matches.Length > 0)
                 return SearchResult.FromSuccess(input, matches);
             else
@@ -255,47 +258,85 @@ namespace Discord.Commands
             if (!searchResult.IsSuccess)
                 return searchResult;
 
+            //Group commands by their alias
             var commands = searchResult.Commands;
-            for (int i = 0; i < commands.Count; i++)
+            var preconditionResults = new Dictionary<CommandMatch, PreconditionResult>();
+
+            foreach (var match in commands)
             {
-                var preconditionResult = await commands[i].CheckPreconditionsAsync(context, services).ConfigureAwait(false);
-                if (!preconditionResult.IsSuccess)
-                {
-                    if (commands.Count == 1)
-                        return preconditionResult;
-                    else
-                        continue;
-                }
+                preconditionResults[match] = await match.Command.CheckPreconditionsAsync(context, services).ConfigureAwait(false);
+            }
 
-                var parseResult = await commands[i].ParseAsync(context, searchResult, preconditionResult, services).ConfigureAwait(false);
-                if (!parseResult.IsSuccess)
-                {
-                    if (parseResult.Error == CommandError.MultipleMatches)
-                    {
-                        IReadOnlyList<TypeReaderValue> argList, paramList;
-                        switch (multiMatchHandling)
-                        {
-                            case MultiMatchHandling.Best:
-                                argList = parseResult.ArgValues.Select(x => x.Values.OrderByDescending(y => y.Score).First()).ToImmutableArray();
-                                paramList = parseResult.ParamValues.Select(x => x.Values.OrderByDescending(y => y.Score).First()).ToImmutableArray();
-                                parseResult = ParseResult.FromSuccess(argList, paramList);
-                                break;
-                        }
-                    }
+            var successfulPreconditions = preconditionResults
+                .Where(x => x.Value.IsSuccess)
+                .ToArray();
+
+            if (successfulPreconditions.Length == 0)
+            {
+                //All preconditions failed, return the one from the highest priority command
+                var bestCandidate = preconditionResults
+                    .OrderByDescending(x => x.Key.Command.Priority)
+                    .FirstOrDefault(x => !x.Value.IsSuccess);
+                return bestCandidate.Value;
+            }
+
+            //If we get this far, at least one precondition was successful.
 
-                    if (!parseResult.IsSuccess)
+            var parseResults = new Dictionary<CommandMatch, ParseResult>();
+            foreach (var pair in successfulPreconditions)
+            {
+                var parseResult = await pair.Key.ParseAsync(context, searchResult, pair.Value, services).ConfigureAwait(false);
+
+                if (parseResult.Error == CommandError.MultipleMatches)
+                {
+                    IReadOnlyList<TypeReaderValue> argList, paramList;
+                    switch (multiMatchHandling)
                     {
-                        if (commands.Count == 1)
-                            return parseResult;
-                        else
-                            continue;
+                        case MultiMatchHandling.Best:
+                            argList = parseResult.ArgValues.Select(x => x.Values.OrderByDescending(y => y.Score).First()).ToImmutableArray();
+                            paramList = parseResult.ParamValues.Select(x => x.Values.OrderByDescending(y => y.Score).First()).ToImmutableArray();
+                            parseResult = ParseResult.FromSuccess(argList, paramList);
+                            break;
                     }
                 }
 
-                return await commands[i].ExecuteAsync(context, parseResult, services).ConfigureAwait(false);
+                parseResults[pair.Key] = parseResult;
+            }
+
+            // Calculates the 'score' of a command given a parse result
+            float CalculateScore(CommandMatch match, ParseResult parseResult)
+            {
+                //TODO: is this calculation correct?
+                var argValuesScore = parseResult.ArgValues.Sum(x => x.Values.OrderByDescending(y => y.Score).FirstOrDefault().Score);
+                var paramValuesScore = parseResult.ParamValues.Sum(x => x.Values.OrderByDescending(y => y.Score).FirstOrDefault().Score);
+
+                /* Since argValuesScore and paramValuesScore are in the range [0, numOfParams]
+                 * we multiply the priority by the number of parameters plus one, so that it is
+                 * always the most important value.
+                 */
+                var priorityScore = match.Command.Priority * (match.Command.Parameters.Count + 1);
+
+                return priorityScore + argValuesScore + paramValuesScore;
+            }
+
+            //Order the parse results by their score so that we choose the most likely result to execute
+            var successfulParses = parseResults
+                .Where(x => x.Value.IsSuccess)
+                .OrderByDescending(x => CalculateScore(x.Key, x.Value))
+                .ToArray();
+
+            if (successfulParses.Length == 0)
+            {
+                //All parses failed, return the one from the highest priority command, using score as a tie breaker
+
+                var bestMatch = parseResults
+                    .FirstOrDefault(x => !x.Value.IsSuccess);
+                return bestMatch.Value;
             }
 
-            return SearchResult.FromError(CommandError.UnknownCommand, "This input does not match any overload.");
+            //If we get this far, at least one parse was successful. Execute the most likely overload.
+            var chosenOverload = successfulParses[0];
+            return await chosenOverload.Key.ExecuteAsync(context, chosenOverload.Value, services).ConfigureAwait(false);
         }
     }
 }

src/Discord.Net.Commands/PrimitiveParsers.cs

@@ -31,11 +31,6 @@ namespace Discord.Commands
             parserBuilder[typeof(DateTimeOffset)] = (TryParseDelegate<DateTimeOffset>)DateTimeOffset.TryParse;
             parserBuilder[typeof(TimeSpan)] = (TryParseDelegate<TimeSpan>)TimeSpan.TryParse;
             parserBuilder[typeof(char)] = (TryParseDelegate<char>)char.TryParse;
-            parserBuilder[typeof(string)] = (TryParseDelegate<string>)delegate (string str, out string value)
-            {
-                value = str;
-                return true;
-            };
             return parserBuilder.ToImmutable();
         }
 

src/Discord.Net.Commands/Readers/PrimitiveTypeReader.cs

@@ -15,16 +15,25 @@ namespace Discord.Commands
     internal class PrimitiveTypeReader<T> : TypeReader
     {
         private readonly TryParseDelegate<T> _tryParse;
+        private readonly float _score;
 
         public PrimitiveTypeReader()
+            : this(PrimitiveParsers.Get<T>(), 1)
+        { }
+
+        public PrimitiveTypeReader(TryParseDelegate<T> tryParse, float score)
         {
-            _tryParse = PrimitiveParsers.Get<T>();
+            if (score < 0 || score > 1)
+                throw new ArgumentOutOfRangeException(nameof(score), score, "Scores must be within the range [0, 1]");
+
+            _tryParse = tryParse;
+            _score = score;
         }
 
         public override Task<TypeReaderResult> Read(ICommandContext context, string input, IServiceProvider services)
         {
             if (_tryParse(input, out T value))
-                return Task.FromResult(TypeReaderResult.FromSuccess(value));
+                return Task.FromResult(TypeReaderResult.FromSuccess(new TypeReaderValue(value, _score)));
             return Task.FromResult(TypeReaderResult.FromError(CommandError.ParseFailed, $"Failed to parse {typeof(T).Name}"));
         }
     }