Misc Toolshed tweaks (#4990)

* Toolshed tweaks

* oops

* Apply suggestions from code review

Co-authored-by: Moony <moony@hellomouse.net>

* Re-add NotImplementedException

* Move error message

---------

Co-authored-by: Moony <moony@hellomouse.net>
Co-authored-by: metalgearsloth <31366439+metalgearsloth@users.noreply.github.com>

RELEASE-NOTES.md

@@ -41,9 +41,12 @@ END TEMPLATE-->
 
 * Add a CompletionHelper for audio filepaths that handles server packaging.
 * Add Random.NextAngle(min, max) method and Pick for `ValueList<T>`.
+* Added an `ICommonSession` parser for toolshed commands.
 
 ### Bugfixes
 
+* Fixed some issues where toolshed commands were generating completions for the wrong arguments
+
 *None yet*
 
 ### Other

Resources/Locale/en-US/commands.ftl

@@ -11,6 +11,7 @@ cmd-parse-failure-uid = {$arg} is not a valid entity UID.
 cmd-parse-failure-mapid = {$arg} is not a valid MapId.
 cmd-parse-failure-grid = {$arg} is not a valid grid.
 cmd-parse-failure-entity-exist = UID {$arg} does not correspond to an existing entity.
+cmd-parse-failure-session = There is no session with username: {$username}
 
 cmd-error-file-not-found = Could not find file: {$file}.
 cmd-error-dir-not-found = Could not find directory: {$dir}.

Robust.Shared/Toolshed/Syntax/Expression.cs

@@ -46,6 +46,10 @@ public sealed class CommandRun
 
             if (parserContext.EatTerminator())
                 break;
+
+            // Prevent auto completions from dumping a list of all commands at the end of any complete command.
+            if (parserContext.Index > parserContext.MaxIndex)
+                break;
         }
 
         if (error is OutOfInputError && noCommand)

Robust.Shared/Toolshed/ToolshedCommand.Entities.cs

@@ -111,7 +111,7 @@ public abstract partial class ToolshedCommand
     ///     A shorthand for attempting to retrieve the given component for an entity.
     /// </summary>
     [PublicAPI, MethodImpl(MethodImplOptions.AggressiveInlining)]
-    protected bool TryComp<T>(EntityUid? entity, [NotNullWhen(true)] out T? component)
+    protected bool TryComp<T>([NotNullWhen(true)] EntityUid? entity, [NotNullWhen(true)] out T? component)
         where T: IComponent
         => EntityManager.TryGetComponent(entity, out component);
 

Robust.Shared/Toolshed/ToolshedCommand.cs

@@ -7,6 +7,7 @@ using System.Threading.Tasks;
 using Robust.Shared.Console;
 using Robust.Shared.GameObjects;
 using Robust.Shared.IoC;
+using Robust.Shared.Localization;
 using Robust.Shared.Reflection;
 using Robust.Shared.Toolshed.Errors;
 using Robust.Shared.Toolshed.Syntax;
@@ -47,6 +48,7 @@ namespace Robust.Shared.Toolshed;
 public abstract partial class ToolshedCommand
 {
     [Dependency] protected readonly ToolshedManager Toolshed = default!;
+    [Dependency] protected readonly ILocalizationManager Loc = default!;
 
     /// <summary>
     ///     The user-facing name of the command.
@@ -99,7 +101,7 @@ public abstract partial class ToolshedCommand
             };
 
         var impls = GetGenericImplementations();
-        Dictionary<string, SortedDictionary<string, Type>> parameters = new();
+        Dictionary<(string, Type?), SortedDictionary<string, Type>> parameters = new();
 
         foreach (var impl in impls)
         {
@@ -117,27 +119,26 @@ public abstract partial class ToolshedCommand
                     };
             }
 
+            Type? pipedType = null;
             foreach (var param in impl.GetParameters())
             {
                 if (param.GetCustomAttribute<CommandArgumentAttribute>() is not null)
-                {
-                    if (parameters.ContainsKey(param.Name!))
-                        continue;
+                    myParams.TryAdd(param.Name!, param.ParameterType);
 
-                    myParams.Add(param.Name!, param.ParameterType);
+                if (param.GetCustomAttribute<PipedArgumentAttribute>() is not null)
+                {
+                    if (pipedType != null)
+                        throw new NotSupportedException($"Commands cannot have more than one piped argument");
+                    pipedType = param.ParameterType;
                 }
             }
 
-            if (parameters.TryGetValue(subCmd ?? "", out var existing))
-            {
-                if (!existing.SequenceEqual(existing))
-                {
-                    throw new NotImplementedException("All command implementations of a given subcommand must share the same parameters!");
-                }
-            }
-            else
-                parameters.Add(subCmd ?? "", myParams);
+            var key = (subCmd ?? "", pipedType);
+            if (parameters.TryAdd(key, myParams))
+                continue;
 
+            if (!parameters[key].SequenceEqual(myParams))
+                throw new NotImplementedException("All command implementations of a given subcommand with the same pipe type must share the same argument types");
         }
     }
 
@@ -184,14 +185,11 @@ internal sealed class CommandArgumentBundle
     public required Type[] TypeArguments;
 }
 
-internal readonly record struct CommandDiscriminator(Type? PipedType, Type[] TypeArguments) : IEquatable<CommandDiscriminator?>
+internal readonly record struct CommandDiscriminator(Type? PipedType, Type[] TypeArguments)
 {
-    public bool Equals(CommandDiscriminator? other)
+    public bool Equals(CommandDiscriminator other)
     {
-        if (other is not {} value)
-            return false;
-
-        return value.PipedType == PipedType && value.TypeArguments.SequenceEqual(TypeArguments);
+        return other.PipedType == PipedType && other.TypeArguments.SequenceEqual(TypeArguments);
     }
 
     public override int GetHashCode()

Robust.Shared/Toolshed/ToolshedCommandImplementor.cs

@@ -115,6 +115,7 @@ internal sealed class ToolshedCommandImplementor
             return false;
         }
 
+        autocomplete = null;
         args = new();
         foreach (var argument in impl.ConsoleGetArguments())
         {
@@ -124,8 +125,9 @@ internal sealed class ToolshedCommandImplementor
             {
                 error?.Contextualize(parserContext.Input, (start, parserContext.Index));
                 args = null;
-                autocomplete = null;
-                if (doAutocomplete)
+
+                // Only generate auto-completions if the parsing error happened for the last argument.
+                if (doAutocomplete && parserContext.Index > parserContext.MaxIndex)
                 {
                     parserContext.Restore(chkpoint);
                     autocomplete = _toolshedManager.TryAutocomplete(parserContext, argument.ParameterType, null);
@@ -133,10 +135,19 @@ internal sealed class ToolshedCommandImplementor
                 return false;
             }
             args[argument.Name!] = parsed;
+
+            if (!doAutocomplete || parserContext.Index <= parserContext.MaxIndex)
+                continue;
+
+            // This was the end of the input, so we want to get completions for the current argument, not the next argument.
+            doAutocomplete = false;
+            var chkpoint2 = parserContext.Save();
+            parserContext.Restore(chkpoint);
+            autocomplete = _toolshedManager.TryAutocomplete(parserContext, argument.ParameterType, null);
+            parserContext.Restore(chkpoint2);
         }
 
         error = null;
-        autocomplete = null;
         return true;
     }
 

Robust.Shared/Toolshed/TypeParsers/EntityTypeParser.cs

@@ -3,7 +3,6 @@ using System.Diagnostics.CodeAnalysis;
 using System.Threading.Tasks;
 using Robust.Shared.Console;
 using Robust.Shared.GameObjects;
-using Robust.Shared.IoC;
 using Robust.Shared.Maths;
 using Robust.Shared.Toolshed.Errors;
 using Robust.Shared.Toolshed.Syntax;

Robust.Shared/Toolshed/TypeParsers/PrototypeTypeParser.cs

@@ -57,6 +57,8 @@ internal sealed class PrototypeTypeParser<T> : TypeParser<Prototype<T>>
 public readonly record struct Prototype<T>(T Value) : IAsType<string>
     where T : class, IPrototype
 {
+    public ProtoId<T> Id => Value.ID;
+
     public string AsType()
     {
         return Value.ID;

Robust.Shared/Toolshed/TypeParsers/SessionTypeParser.cs

@@ -0,0 +1,64 @@
+using System.Diagnostics;
+using System.Diagnostics.CodeAnalysis;
+using System.Threading.Tasks;
+using Robust.Shared.Console;
+using Robust.Shared.IoC;
+using Robust.Shared.Localization;
+using Robust.Shared.Maths;
+using Robust.Shared.Player;
+using Robust.Shared.Toolshed.Errors;
+using Robust.Shared.Toolshed.Syntax;
+using Robust.Shared.Utility;
+
+namespace Robust.Shared.Toolshed.TypeParsers;
+
+/// <summary>
+/// Parse a username to an <see cref="ICommonSession"/>
+/// </summary>
+internal sealed class SessionTypeParser : TypeParser<ICommonSession>
+{
+    [Dependency] private ISharedPlayerManager _player = default!;
+
+    public override bool TryParse(ParserContext parser, [NotNullWhen(true)] out object? result, out IConError? error)
+    {
+        var start = parser.Index;
+        var word = parser.GetWord();
+        error = null;
+        result = null;
+
+        if (word == null)
+        {
+            error = new OutOfInputError();
+            return false;
+        }
+
+        if (_player.TryGetSessionByUsername(word, out var session))
+        {
+            result = session;
+            return true;
+        }
+
+        error = new InvalidUsername(Loc, word);
+        error.Contextualize(parser.Input, (start, parser.Index));
+        return false;
+    }
+
+    public override async ValueTask<(CompletionResult? result, IConError? error)> TryAutocomplete(ParserContext parserContext,
+        string? argName)
+    {
+        var opts = CompletionHelper.SessionNames(true, _player);
+        return (CompletionResult.FromHintOptions(opts, "<player session>"), null);
+    }
+
+    public record InvalidUsername(ILocalizationManager Loc, string Username) : IConError
+    {
+        public FormattedMessage DescribeInner()
+        {
+            return FormattedMessage.FromMarkup(Loc.GetString("cmd-parse-failure-session", ("username", Username)));
+        }
+
+        public string? Expression { get; set; }
+        public Vector2i? IssueSpan { get; set; }
+        public StackTrace? Trace { get; set; }
+    }
+}

Robust.Shared/Toolshed/TypeParsers/TypeParser.cs

@@ -4,6 +4,7 @@ using System.Threading.Tasks;
 using JetBrains.Annotations;
 using Robust.Shared.Console;
 using Robust.Shared.IoC;
+using Robust.Shared.Localization;
 using Robust.Shared.Log;
 using Robust.Shared.Toolshed.Errors;
 using Robust.Shared.Toolshed.Syntax;
@@ -26,8 +27,9 @@ public abstract class TypeParser<T> : ITypeParser
     where T: notnull
 {
     [Dependency] private readonly ILogManager _log = default!;
+    [Dependency] protected readonly ILocalizationManager Loc = default!;
 
-    protected ISawmill _sawmill = default!;
+    protected ISawmill Log = default!;
 
     public virtual Type Parses => typeof(T);
 
@@ -37,6 +39,6 @@ public abstract class TypeParser<T> : ITypeParser
 
     public virtual void PostInject()
     {
-        _sawmill = _log.GetSawmill(GetType().PrettyName());
+        Log = _log.GetSawmill(GetType().PrettyName());
     }
 }