Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Toolshed Rejig #5455

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

ElectroJr
Copy link
Member

@ElectroJr ElectroJr commented Sep 21, 2024

This PR reworks large parts of toolshed command parsing. Initially I wanted to make a few smaller PRs, but eventually just kind of gave up and it turned into this (though I did split some changes off in #5315, #5340, #5438). I've tried to give an overview of all the changes, but it probably makes more sense if you're familiar with toolshed, so I'd appreciate it if @moonheart08 could give input.

I don't think most of the changes are controversial, and I've tried to limit the number of breaking changes. SS14 actually works fine without content changes, though content integration tests need to be updated due to some changes in the ToolshedManager & ParserContext method signatures. So other than maybe breaking some previously valid commands hopefully it doesn't cause issues for downstreams.

Some parts of command implementations & command parsing were generally just re-written, and several checks/asserts were added throughout. Previously it was easy to unknowingly write an invalid toolshed command by forgetting an attribute or messing up the ordering of generic arguments. The new checks should hopefully stop that from happening again. E.g., the new checks in ToolshedCommand.Init() are what lead to the discovery of some of the bugs fixed in #5340 and #5438, and should stop similar ones in the future.

Global ValueRef<T> support

One of the nice things about toolshed is being able to use stored variables or command blocks in place of a commands argument. I.e., instead of writing out an EntityUid you could use something like $ent or { self }. However this relies on whoever wrote the command to manually implement it by making their method take in a ValueRef<T> instead of just a T. The end result is that it was quite inconsistent whether or not this feature could be used for any given command.

AFAIK there is no clear reason to not just support this feature for all commands. This PR changes the parsing so that all commands that take in a T argument can now implicitly also accept variable or block. The only time one would want to continue to use a command that explicitly takes in a ValueRef<T> is if you wanted to use delayed or repeated evaluation of variables or command block. E.g., to repeatedly evaluate a block that might generate a random number.

This will break any content defined type-parser that accepted tokens starting with '{' or '$', but I doubt any like that exist? And if they really need it, they can use a custom type parser.

A related change is that ValueRef<T> has been split into multiple different class that inherit from ValueRef<T>:

  • VarRef<T> is a reference to a variable, and can be used if a command specifically wants a variable (e.g., the => command)
  • BlockRef<T> is a command block
  • ParsedValue<T> is just a wrapper around an actual T value.

fixes #4314

Variable name auto-completion

TLDR: you can now get variable auto completions:
Content Client_yXdVWcFhfE

Toolshed doesn't currently have any way of generating auto-completion suggestions for variable names. This is mainly because the variables are defined in the IInvocationContext, which the ParserContext does not have any access to. As an additional complication, the available variables may change as the command gets invoked.

This PR partially fixes this by allowing the ParserContext to be aware of the variables that are expected to be available via a field that points to an IVariableParser. If the parser context is properly set up, it will be aware of any variables that are already defined in the invocation context that is about to be used to invoke the parsed command. Additionally, the completions will try to guess the variable's type. I.e., f 2 + $ should only suggest variables containing a float.

Any command that temporarily defines variables that are local to some command block should then modify the ParserContexts's IVariableParser, so that it can properly generate auto-completion suggestions for that block. As an example, the emplace command defines several local variables based on the type piped into the command. E.g., for an EntityUid, it defines variables that point to the world positions ($wx, $wy). As seen in the screenshot, using emplace & knowing what variables it provides is quite a bit easier now that it actually generates suggestions.

Note that the variable auto-completions still aren't perfect, but as long as you don't chain together commands that define new variables, they are quite useful.

VarCommand and inferred variable types

There is now a new command that is meant to make using variables easier by automatically inferring the type of the variable. I.e., if $x contains an EntityUid[], you can now simply pipe it into another command via var $x instead of having to manually specify the type using something like val EntityUid[] $x.

This works in largely the same way as variable name auto-completion, by using the ParserContext's current IVariableParser. As mentioned in the previous section, its still not perfect and for some commands to parse successfully you will still have to manually specify the variable type using the val command.

New type parsers

  • Added parsers for NetEntity, Entity<T>, Entity<T1,T2>, Entity<T1,T2,T3>
    • As with the EntityUid parser, these parse an integer as a NetEntity, and then convert to the appropriate type.
    • All the entity parsers now also accept an 'e' prefix, which causes the integer to instead be parsed as an EntityUid before converting
  • Added parsers for EntProtoId and prototype instances. When combined with the EntProtoId<T> parser, these effectively make the Prototype<T> parser & struct obsolete.

Custom type parsers

Command arguments can now override the default type parser by passing some type that implements CustomTypeParser<T> to the CommandArgument attribute. Previously you could kind of achieve this by just creating a new class that implements IAsType<T> that exists solely so that you could define a type parser for that class. Properly supporting custom parses just helps to avoid that jank. Note that by default parsing a type will prioritises trying to use variables & blocks, but this can be disabled by overriding ITypeParser.EnableValueRef (I can't think of a good name for the field).

Implicit attributes

The CommandArgument and CommandInvocationContext attributes are now optional. Any IInvocationContext parameter implicitly gets the CommandInvocationContext attribute, and any other parameter is implicitly a command argument. This just generally makes command definitions a bit nicer to read by significantly cutting down on the number of attributes.

Blocks now require braces

All command blocks now require opening & closing braces. IMO this is clearer to read and avoids confusion. E.g. i 2 * i 2 + 2 now HAS to be written as i 2 * { i 2 + 2 } which prevents it being confused with i 2 * 2 + 2

The main downside is that even simple single-word blocks like in fpi + fpi, now have to be written as fpi + { fpi }, which is not quite as nice to read. This might be particularly annoying for commands that use the self command as an argument. But IMO enforcing the additional clarity is worth it. It makes it much clearer in convoluted commands what parts are a command argument, and what parts are subsequent command that you are piping the results into. This is also one of the changes that means that previously valid commands may no longer work. This might be the most controversial change in this PR, but I feel pretty strongly about it.

Automatic argument hint loc strings

When generating auto-completions for arguments toolshed will now try to look for an argument hint loc string, and if found will use that to override the hint of the generated options. E.g., you could now add a custom hint to the WithCommand's ty parameter by creating a command-arg-hint-with-ty loc string.

TypeParameterParsers and MapLikeCommand changes

Currently each command has a Type[] TypeParameterParsers field, where each type has to be either be typeof(Type) or some type that implements IAsType<Type>. These types were used to infer a parser that will be used to parse type parameters for commands with generic type arguments. This PR changes this around a bit, so that instead this array directly provides the parsers, not the types that should be parsed. I.e., now it is a Type[] where each element should be either TypeTypeParser or some type that implements CustomTypeParser<Type>.

The main reason for this change is that I found myself creating a bunch of dummy classes that just existed that they could implement IAsType<Type> and have some specific parser. Now that custom type parsers exist, it makes much more sense to use them directly.

The internal MapLikeCommand attribute has now been removed and those commands just use a custom parser that gets the type by parsing a block and returning its output type. It is still somewhat janky, but at least it is more generalized and no longer given special treatment within the toolshed parsing code. It also content to define their own "map like" commands, or other commands that need to infer type arguments from command arguments.

Added command terminator (;)

; is now considered a special character, and will interrupt command argument parsing and cause any piped data to be dropped. On its own, its not very useful at the moment, though it does allow for void-returning commands to be chained together on a single line, e.g., delete 1; delete 2; delete 3;

The main reason for adding this is for a future PR: I want add support for commands with optional and params arguments, which requires introducing terminators and an explicit pipe symbol. Given that support for optional arguments is widely requested, I think this is not a controversial addition. A future PR has to actually add support for optional args, which will also require adding an explicit pipe symbol/terminator (i.e., like | in bash). This would probably conflict with the existing | and || I'm leaving that for a future PR.

Misc ITypeParser, TypeParser<T>, and other parsing changes

  • TypeParser<T>.TryParse is now a generic method that has to return a T? instead of just an object?.
    • This was mainly motivated by the fact that I kept accidentally writing broken parsers that returned the wrong type.
  • Several parsing related arguments have been moved into the parser context.
    • E.g., the doAutocomplete bool was only ever being set when the context was created, and constantly being passed around alongside the context. Now its just a field in ParserContext.
    • Other arguments that were moved include the auto-completion results, errors, and the CommandArgumentBundle.
  • The auto-completion method no longer return a ValueTask<(CompletionResult? result, IConError? error)>
    • Not a single use of the method was actually returning a task, they were all using something like return ValueTask.FromResult<(CompletionResult? result, IConError? error)>(...) and I got sick of the boilerplate
    • If async completions should be supported in the future, I'd rather find a way to do it that isn't contagious and forces all parser implementations to return a ValueTask
  • ITypeParser.Parses now now just a Type, instead of Type[]
    • No parsers were actually using this. All existing parsers inherited from TypeParser<T> which always returned an array of length 1 anyways.

Misc breaking changes and obsoletions

  • ValueRef<T, TAuto> is now obsolete
    • I'm not really sure what this was for, there wasn't really any documentation.
    • I think it might've been for commands that wanted to use another type to generate auto-completion options? I.e., ValueRef<string, Prototype<TagPrototype>> was one of the few use cases. But I have NFI why it wasn't just using that type directly?
  • EmplaceCommand's IEmplaceBreakout was removed.
    • AFAICT it was never actually used, and the way it was implemented prevents emplace variable auto-completion from working
    • I understand what it was meant to be used for, but there is probably a better way of implementing that functionality, which I want to try do in a later PR.
    • Prototype<T> is now marked as obsolete. You can just use T or ProtoId<T>

@ElectroJr ElectroJr added Status: Requires Content PR This PR breaks content and requires both to be merged together. Toolshed labels Sep 21, 2024
@Mervill
Copy link
Member

Mervill commented Sep 21, 2024

ezgif-6-2db901e715

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Requires Content PR This PR breaks content and requires both to be merged together. Toolshed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Parsing a Toolshed ValueRef<T> fails if T has no type parser.
2 participants