Add support for optional and params T[] toolshed command arguments.

RELEASE-NOTES.md

@@ -35,19 +35,26 @@ END TEMPLATE-->
 
 ### Breaking changes
 
-*None yet*
+* The signature of Toolshed type parsers have changed. Instead of taking in an optional command argument name string, they now take in a `CommandArgument` struct.
+* Toolshed commands can no longer contain a '|', as this symbol is now used for explicitly piping the output of one command to another. command pipes. The existing `|` and '|~' commands have been renamed to `bitor` and `bitnotor`.
+* Semicolon terminated command blocks in toolshed commands no longer return anything. I.e., `i { i 2 ; }` is no longer a valid command, as the block has no return value.
 
 ### New features
 
-*None yet*
+* Toolshed commands now support optional and `params T[]` arguments. optional / variable length commands can be terminated using ';' or '|'.
 
 ### Bugfixes
 
-*None yet*
+* The map-like Toolshed commands now work when a collection is piped in.
+* Fixed a bug in toolshed that could cause it to preferentially use the incorrect command implementation.
+  * E.g., passing a concrete enumerable type would previously use the command implementation that takes in an unconstrained generic parameter `T` instead of a dedicated `IEnumeerable<T>` implementation.
 
 ### Other
 
-*None yet*
+* The default auto-completion hint for Toolshed commands have been changed and somewhat standardized. Most type parsers should now have a hint of the form:
+  * `<name (Type)>` for mandatory arguments
+  * `[name (Type)]` for optional arguments
+  * `[name (Type)]...` for variable length arguments (i.e., for `params T[]`)
 
 ### Internal
 

Resources/Locale/en-US/toolshed-commands.ftl

@@ -42,8 +42,7 @@ command-description-as =
 command-description-count =
     Counts the amount of entries in it's input, returning an integer.
 command-description-map =
-    Maps the input over the given block, with the provided expected return type.
-    This command may be modified to not need an explicit return type in the future.
+    Maps the input over the given block.
 command-description-select =
     Selects N objects or N% of objects from the input.
     One can additionally invert this command with not to make it select everything except N objects instead.
@@ -149,7 +148,7 @@ command-description-max =
     Returns the maximum of two values.
 command-description-BitAndCommand =
     Performs bitwise AND.
-command-description-BitOrCommand =
+command-description-bitor =
     Performs bitwise OR.
 command-description-BitXorCommand =
     Performs bitwise XOR.
@@ -203,11 +202,11 @@ command-description-mappos =
 command-description-pos =
     Returns an entity's coordinates.
 command-description-tp-coords =
-    Teleports the target to the given coordinates.
+    Teleports the given entities to the target coordinates.
 command-description-tp-to =
-    Teleports the target to the given other entity.
+    Teleports the given entities to the target entity.
 command-description-tp-into =
-    Teleports the target "into" the given other entity, attaching it at (0 0) relative to it.
+    Teleports the given entities "into" the target entity, attaching it at (0 0) relative to it.
 command-description-comp-get =
     Gets the given component from the given entity.
 command-description-comp-add =
@@ -277,7 +276,7 @@ command-description-ModVecCommand =
     Performs the modulus operation over the input with the given constant right-hand value.
 command-description-BitAndNotCommand =
     Performs bitwise AND-NOT over the input.
-command-description-BitOrNotCommand =
+command-description-bitornot =
     Performs bitwise OR-NOT over the input.
 command-description-BitXnorCommand =
     Performs bitwise XNOR over the input.

Robust.Shared/Prototypes/EntProtoId.cs

@@ -17,7 +17,8 @@ namespace Robust.Shared.Prototypes;
 /// </remarks>
 /// <remarks><seealso cref="ProtoId{T}"/> for a wrapper of other prototype kinds.</remarks>
 [Serializable, NetSerializable]
-public readonly record struct EntProtoId(string Id) : IEquatable<string>, IComparable<EntProtoId>, IAsType<string>
+public readonly record struct EntProtoId(string Id) : IEquatable<string>, IComparable<EntProtoId>, IAsType<string>,
+    IAsType<ProtoId<EntityPrototype>>
 {
     public static implicit operator string(EntProtoId protoId)
     {
@@ -49,7 +50,9 @@ public int CompareTo(EntProtoId other)
         return string.Compare(Id, other.Id, StringComparison.Ordinal);
     }
 
-    public string AsType() => Id;
+    string IAsType<string>.AsType() => Id;
+
+    ProtoId<EntityPrototype> IAsType<ProtoId<EntityPrototype>>.AsType() => new(Id);
 
     public override string ToString() => Id ?? string.Empty;
 }

Robust.Shared/Toolshed/Commands/Generic/CountCommand.cs

@@ -7,8 +7,8 @@ namespace Robust.Shared.Toolshed.Commands.Generic;
 public sealed class CountCommand : ToolshedCommand
 {
     [CommandImplementation, TakesPipedTypeAsGeneric]
-    public int Count<T>([PipedArgument] IEnumerable<T> enumerable)
+    public int Count<T>([PipedArgument] IEnumerable<T> input)
     {
-        return enumerable.Count();
+        return input.Count();
     }
 }

Robust.Shared/Toolshed/Commands/Generic/EmplaceCommand.cs

@@ -174,8 +174,14 @@ private sealed class EmplaceBlockParser : CustomTypeParser<Block>
     {
         public static bool TryParse(ParserContext ctx, [NotNullWhen(true)] out CommandRun? result)
         {
+            if (ctx.Bundle.PipedType == null)
+            {
+                result = null;
+                return false;
+            }
+
             // If the piped type is IEnumerable<T> we want to extract the type T.
-            var pipeInferredType = ctx.Bundle.PipedType!;
+            var pipeInferredType = ctx.Bundle.PipedType;
             if (pipeInferredType.IsGenericType(typeof(IEnumerable<>)))
                 pipeInferredType = pipeInferredType.GetGenericArguments()[0];
 
@@ -219,7 +225,7 @@ public override bool TryParse(ParserContext ctx, [NotNullWhen(true)] out Block?
             return true;
         }
 
-        public override CompletionResult? TryAutocomplete(ParserContext ctx, string? argName)
+        public override CompletionResult? TryAutocomplete(ParserContext ctx, CommandArgument? arg)
         {
             TryParse(ctx, out _);
             return ctx.Completions;
@@ -231,6 +237,7 @@ public override bool TryParse(ParserContext ctx, [NotNullWhen(true)] out Block?
     /// </summary>
     private sealed class EmplaceBlockOutputParser : CustomTypeParser<Type>
     {
+        public override bool ShowTypeArgSignature => false;
         public override bool TryParse(ParserContext ctx, [NotNullWhen(true)] out Type? result)
         {
             result = null;
@@ -246,7 +253,7 @@ public override bool TryParse(ParserContext ctx, [NotNullWhen(true)] out Type? r
             return true;
         }
 
-        public override CompletionResult? TryAutocomplete(ParserContext ctx, string? argName)
+        public override CompletionResult? TryAutocomplete(ParserContext ctx, CommandArgument? arg)
         {
             EmplaceBlockParser.TryParse(ctx, out _);
             return ctx.Completions;

Robust.Shared/Toolshed/Commands/Generic/ReduceCommand.cs

@@ -66,7 +66,7 @@ public override bool TryParse(ParserContext ctx, [NotNullWhen(true)] out Block?
             return true;
         }
 
-        public override CompletionResult? TryAutocomplete(ParserContext ctx, string? argName)
+        public override CompletionResult? TryAutocomplete(ParserContext ctx, CommandArgument? arg)
         {
             if (ctx.Bundle.PipedType is not {IsGenericType: true})
                 return null;

Robust.Shared/Toolshed/Commands/Generic/Variables/VarsCommand.cs

@@ -8,6 +8,6 @@ public sealed class VarsCommand : ToolshedCommand
     [CommandImplementation]
     public void Vars(IInvocationContext ctx)
     {
-        ctx.WriteLine(Toolshed.PrettyPrintType(ctx.GetVars().Select(x => $"{x} = {ctx.ReadVar(x)}"), out var more));
+        ctx.WriteLine(Toolshed.PrettyPrintType(ctx.GetVars().Select(x => $"{x} = {Toolshed.PrettyPrintType(ctx.ReadVar(x), out _)}"), out _));
     }
 }

Robust.Shared/Toolshed/Commands/Math/ArithmeticCommands.cs

@@ -295,7 +295,7 @@ public IEnumerable<T> Operation<T>([PipedArgument] IEnumerable<T> x, IEnumerable
         });
 }
 
-[ToolshedCommand(Name = "|")]
+[ToolshedCommand]
 public sealed class BitOrCommand : ToolshedCommand
 {
     [CommandImplementation, TakesPipedTypeAsGeneric]
@@ -314,7 +314,7 @@ public IEnumerable<T> Operation<T>([PipedArgument] IEnumerable<T> x, IEnumerable
         });
 }
 
-[ToolshedCommand(Name = "|~")]
+[ToolshedCommand]
 public sealed class BitOrNotCommand : ToolshedCommand
 {
     [CommandImplementation, TakesPipedTypeAsGeneric]

Robust.Shared/Toolshed/Commands/Misc/ExplainCommand.cs

@@ -16,32 +16,40 @@ CommandRun expr
         var builder = new StringBuilder();
         foreach (var (cmd, _) in expr.Commands)
         {
+            builder.AppendLine();
             var name = cmd.Implementor.FullName;
             builder.AppendLine($"{name} - {cmd.Implementor.Description()}");
 
+            var piped = cmd.PipedType?.PrettyName() ?? "[none]";
+            builder.AppendLine($"Pipe input: {piped}");
+            builder.AppendLine($"Pipe output: {cmd.ReturnType.PrettyName()}");
+
+            builder.Append($"Signature:\n  ");
+
             if (cmd.PipedType != null)
             {
                 var pipeArg = cmd.Method.Base.PipeArg;
                 DebugTools.AssertNotNull(pipeArg);
-                builder.Append($"<{pipeArg?.Name} ({ToolshedCommandImplementor.GetFriendlyName(cmd.PipedType)})> -> ");
+
+                var locKey = $"command-arg-sig-{cmd.Implementor.LocName}-{pipeArg?.Name}";
+                if (Loc.TryGetString(locKey, out var msg))
+                {
+                    builder.Append(msg);
+                    builder.Append(" → ");
+                }
+                else
+                {
+                    builder.Append($"<{pipeArg?.Name}> → "); // No type information, as that is already given above.
+                }
             }
 
             if (cmd.Bundle.Inverted)
                 builder.Append("not ");
 
-            builder.Append($"{name}");
-            foreach (var (argName, argType, _) in cmd.Method.Args)
-            {
-                builder.Append($" <{argName} ({ToolshedCommandImplementor.GetFriendlyName(argType)})>");
-            }
-
-            builder.AppendLine();
-            var piped = cmd.PipedType?.PrettyName() ?? "[none]";
-            var returned = cmd.ReturnType?.PrettyName() ?? "[none]";
-            builder.AppendLine($"{piped} -> {returned}");
+            cmd.Implementor.AddMethodSignature(builder, cmd.Method.Args, cmd.Bundle.TypeArguments);
             builder.AppendLine();
         }
 
-        ctx.WriteLine(builder.ToString());
+        ctx.WriteLine(builder.ToString().TrimEnd());
     }
 }

Robust.Shared/Toolshed/ReflectionExtensions.cs

@@ -191,8 +191,20 @@ public static Expression CreateEmptyExpr(this Type t)
     }
 
     // IEnumerable<EntityUid> ^ IEnumerable<T> -> EntityUid
+    // List<T> ^ IEnumerable<T> -> EntityUid
+    // T[] ^ IEnumerable<T> -> EntityUid
     public static Type Intersect(this Type left, Type right)
     {
+        // TODO TOOLSHED implement this properly.
+        // AAAAHhhhhh
+        // this is all sphagetti and needs fixing.
+        // I'm just bodging a fix for now that makes it treat arrays as equivalent to a list.
+        if (left.IsArray)
+            return Intersect(typeof(List<>).MakeGenericType(left.GetElementType()!), right);
+
+        if (right.IsArray)
+            return Intersect(left, typeof(List<>).MakeGenericType(right.GetElementType()!));
+
         if (!left.IsGenericType)
             return left;
 

Robust.Shared/Toolshed/Syntax/Expression.cs

@@ -4,6 +4,7 @@
 using System.Diagnostics.CodeAnalysis;
 using Robust.Shared.Maths;
 using Robust.Shared.Toolshed.Errors;
+using Robust.Shared.Toolshed.TypeParsers.Math;
 using Robust.Shared.Utility;
 
 namespace Robust.Shared.Toolshed.Syntax;
@@ -79,13 +80,13 @@ public static bool TryParse(
     {
         expr = null;
         var cmds = new List<(ParsedCommand, Vector2i)>();
-        var start = ctx.Index;
         ctx.ConsumeWhitespace();
         DebugTools.AssertNull(ctx.Error);
         DebugTools.AssertNull(ctx.Completions);
         if (pipedType == typeof(void))
             throw new ArgumentException($"Piped type cannot be void");
 
+        var start = ctx.Index;
         if (ctx.PeekBlockTerminator())
         {
             // Trying to parse an empty block as a command run? I.e. " { } "
@@ -94,6 +95,7 @@ public static bool TryParse(
             return false;
         }
 
+        bool commandExpected;
         while (true)
         {
             if (!ParsedCommand.TryParse(ctx, pipedType, out var cmd))
@@ -107,27 +109,55 @@ public static bool TryParse(
             cmds.Add((cmd, (start, ctx.Index)));
             ctx.ConsumeWhitespace();
 
-            if (ctx.EatCommandTerminators())
-            {
-                ctx.ConsumeWhitespace();
-                pipedType = null;
-            }
+            ctx.EatCommandTerminators(ref pipedType, out commandExpected);
 
             // If the command run encounters a block terminator we exit out.
             // The parser that pushed the block terminator is what should actually eat & pop it, so that it can
             // return appropriate errors if the block was not terminated.
             if (ctx.PeekBlockTerminator())
-                break;
+            {
+                if (!commandExpected)
+                    break;
+
+                // Lets enforce that poeple don't end command blocks with a dangling explicit pipe.
+                // I.e., force people to use `{ i 2 }` instead of `{ i 2 | }`.
+                if (!ctx.GenerateCompletions)
+                {
+                    ctx.Error = new UnexpectedCloseBrace();
+                    ctx.Error.Contextualize(ctx.Input, (ctx.Index, ctx.Index + 1));
+                }
+                return false;
+            }
 
             if (ctx.OutOfInput)
-                break;
+            {
+                // If the last command was terminated by an explicit pipe symbol we require that there be a follow-up
+                // command
+                if (!commandExpected)
+                    break;
+
+                if (ctx.GenerateCompletions)
+                {
+                    // TODO TOOLSHED COMPLETIONS improve this
+                    // Currently completions are only shown if a command ends in a space. I.e. "| " instead of "|".
+                    // Ideally the completions should still be shown, and it should know to auto-insert a leading space.
+                    // AFAIK this requires updating the client-side code like FilterCompletions(), as well as somehow
+                    // communicating this to the client, maybe by adding a new bit to the CompletionOptionFlags, or
+                    // adding some similar flag field to the whole whole completion collection?
+                    ParsedCommand.TryParse(ctx, pipedType, out _);
+                }
+                else
+                    ctx.Error = new OutOfInputError();
+
+                return false;
+            }
 
             start = ctx.Index;
 
             if (pipedType != typeof(void))
                 continue;
 
-            // The previously parsed command does not generate any output that can be piped/chained into another
+            // The  previously parsed command does not generate any output that can be piped/chained into another
             // command. This can happen if someone tries to provide more arguments than a command accepts.
             // e.g., " i 5 5". In this case, the parsing fails and should make it clear that no more input was expected.
             // Multiple unrelated commands on a single line are still supported via the ';' terminator.
@@ -147,8 +177,7 @@ public static bool TryParse(
             return false;
         }
 
-        // Return the last type, even if the command ended with a ';'
-        var returnType = cmds[^1].Item1.ReturnType;
+        var returnType = pipedType != null ? cmds[^1].Item1.ReturnType : typeof(void);
         if (targetOutput != null && !returnType.IsAssignableTo(targetOutput))
         {
             ctx.Error = new WrongCommandReturn(targetOutput, returnType);
@@ -333,6 +362,6 @@ public sealed class EndOfCommandError : ConError
 {
     public override FormattedMessage DescribeInner()
     {
-        return FormattedMessage.FromUnformatted("Expected an end of command (;)");
+        return FormattedMessage.FromUnformatted("Expected a command or block terminator (';' or '}')");
     }
 }