Proposal: extend SuggestorOptions

[Nikita Pushnov]

It would be nice to have in-asset support of filtering displayed suggestions-commands.

Example action:

[Command("camera.fov")]
private float CameraFov (float fov = -1)
{
    if (fov is not -1)
    {
        Camera.main.fieldOfView = fov;
    }

    return Camera.main.fieldOfView;
}

‌

With camera.fov query we’ll get 2 variants in the suggestion list: with and without fov param.

I have a lot of these functions and they all show up twice. I had to add a few lines of code to filter the results with the same PrimarySignature (this is not a code suggestion but a simple hack to get things done, attached image).

I checked Custom Suggestion Filters, but I'm not sure if this is enough to achieve the result.
Some extra SuggestorOptions will be nice to see in future releases.

‌

[Yusuf Ismail]

Thanks for getting in touch about this!

You’re correct that unfortunately you wouldn’t be able to use a suggestion filter to do this, as you would need context over all suggestions when making the decision to filter out each individual suggestion (maybe we should change how filters work so that it is possible with a filter, not sure)

I could add an option to SuggestorOptions for only including a single overload per command, and it would only display the overload with the most arguments. My only concern is it would get a bit hairy when you have overloads like this

command foo bar
command arg0 arg1 arg2

The command foo bar variant just wouldn’t be shown at all, so you wouldn’t know that it can take a completely different set of arguments. Do you think this would be fine or did you have something else in mind for these scenarios?

[Nikita Pushnov]

Yes, I do realize that proposed solution is just a workaround for my particular issue in my particular situation.

But talking about more general and complex solution, I see the following options:

• Simple checkbox in settings: "show/hide all function signature types with the same optional parameters", so that user could indicate whether he wants to display similar signatures with optional parameters as a separate suggestions;
• Grouping functions with optional parameters by default (cannot be disabled), and highlighting all optional parameters in the UI console with a color or a question mark / brackets (function arg1 arg2? [arg3]);
• Custom filter for the entire set of suggestions (as you suggested), similar to "Custom Suggestion Filters". Below you'll find an example of pseudo-code:

public interface ISuggestionsSetFilter
{
    IEnumerable<IQcSuggestion> Filter (
        // source set of suggestions
        IEnumerable<IQcSuggestion> suggestions,  
        // extra context or options aka 'IsSuggestionPermitted'
        SuggestionContext context, SuggestorOptions options
    );
}

public class SuggestionsSetFilter : ISuggestionsSetFilter
{
    public IEnumerable<IQcSuggestion> Filter (
        IEnumerable<IQcSuggestion> suggestions, 
        SuggestionContext context, 
        SuggestorOptions options)
    {
        return suggestions.Where(s => s.PrimarySignature.Contains("test"));
    }
}

‌

[Yusuf Ismail]

Thanks @{557058:811a33f7-4dbe-4f63-8d9f-63b551bc5e6c} for the feedback

As for solution 1,2 - I would be happy to try to do something like this, but the problem is “optional parameters“ aren’t actually a thing in the command system, just overloads. In the scenario I suggested in my previous reply where all the argument names were different in each overload, would you want it to still display the separate overloads in that case? I.e are we only hiding overloads if theyre essentially a subset of another overload? 2 would help with this a bit in terms of making it clear when an argument is optional but what about a scenario like this

command arg0
command arg0 arg1 arg2

In this case we can’t really mark arg1 and arg2 as optional, as if you specify arg1 you now must specify arg2 which wouldn’t be clear from the suggestions. Maybe we would restrict this to only scenarios where this wouldn’t be an issue?

As for 3, I think it probably makes sense to include a ISuggestionSetFilter or something of the sorts. It feels strange to have both filters that act on a single suggestion and filters that work on the entire set but there’s valid use cases for both

‌

[Nikita Pushnov]

1 & 2 solutions are pretty similar.
1 solution affects only same signatures with optional params:

// Command with 1 optional param
[Command("command")]
private void Command1 (int arg0 = 1) { }

// SHOW => 2 suggestions:
// command
// command arg0

// HIDE => 1 suggestion (united):
// command arg0   OR   command [arg0]

2 solution might works somehow like this:

// command arg0
[Command("command")]
private void Command1 (int arg0) { }

// command arg0 arg1 arg2
[Command("command")]
private void Command2 (int arg0, int arg1, int arg2) { }

// new: command arg0 arg1 [arg2_optional]    -- 2nd solution suggestion
// old: command arg0 arg1                    -- current behavior
//      command arg0 arg1 arg2_optional
[Command("command")]
private void Command3 (int arg0, int arg1, int arg2_optional = 1) { } 

In your example you have 2 different commands (2 different signatures of commands), no need to merge them, lets just keep them as separate commands (they are really not equal, different signatures / args, so they should have separate suggestions).

3 can be a bit confusing and overhead, but if there is some more or less shorthand way to include both types of filters, that would be nice. In any case, I'm not sure that filters for the entire SET will become a really popular feature if the asset provides 1/2 option, it’s just a nice additional feature, not a “S-tier”, “must have”, “high priority”.

[Yusuf Ismail]

In your example you have 2 different commands (2 different signatures of commands), no need to merge them, lets just keep them as separate commands (they are really not equal, different signatures / args, so they should have separate suggestions).

Technically speaking all commands are different commands in the QC command system. Optional arguments aren’t actually part of the command system, when a function is used as a command and it has optional arguments, the command generator will generate multiple unique commands to match the behaviour. That’s what makes this a bit trickier but I’m sure I could find a reasonable way to determine whether an overload is effectively just optional arguments

I agree with your point about 3, if we add proper support for 1,2 then we probably don’t need 3 at this point in time. I’ll have a think about it :)

[Yusuf Ismail]

Well some good news @{557058:811a33f7-4dbe-4f63-8d9f-63b551bc5e6c} , I worked out an efficient enough way to do 2 as we wanted, and its configurable so you can disable it and revert to the old behaviour if desired. It even works for commands that were originally overloads such as the following

[Command("command")]
private void Command1 (int arg0) { }

[Command("command")]
private void Command2 (int arg0, int arg1) { }

// command arg0 [arg1]

Would you like to try it before this update of QC goes live? If so just get in touch by Discord/email

[Yusuf Ismail]

Resolved in V2.6.2

[Yusuf Ismail]

‌

[Nikita Pushnov]

Oh, you've found some additional situations for a reasonable grouping of different functions/signatures that can be combined into one with optional parameters, great job!

So, as far as I understand, from now on, different functions with some common part of the arguments are combined into one fat function. Does it affect only different methods or method with optional args are suitable for merge as well? Just an example to be clear:

// FIRST situation for merge (different methods):

[Command("command")]
private void Command1 (int arg0) { }

[Command("command")]
private void Command2 (int arg0, int arg1) { }

// Command1 & Command2 has common 'int arg0', 
// so we can merge them into one suggestion 'command arg0 [arg1]'

// SECOND situation for merge (one method with optional args):

[Command("command")]
private void Command0 (int arg0, int arg1_opt = 1) { }

// Command0 contains optional 'arg1_opt', so we can 
// display only one suggestion for all use-cases 'command arg0 [arg1_opt]'

// THIRD situation (not the subject of the current discussion, but on topic):

[Command("command")]
private void Command (int arg0, int? arg1, int? arg2 = default) { }

// Are there any plans to support nullable args? 
// Right now, suggestor (or even the whole system) 
// doesn't support functions with nullable types. 
// Or we can go to a separate issue / discord to discuss this.

‌

[Yusuf Ismail]

The criteria for merging a command a into another command b is as follows

• The signature of a is a subset of the signature of b
• b has 1 more parameter than a

This stops the following scenarios from being incorrectly merged

cmd arg0
cmd arg0 arg1 arg2

cmd arg0
cmd argA argB

As for your examples, both the first and second scenarios should work just as you described!

As for the third example, there is currently no parser for nullable arguments. If there was, then they should work fine and the suggestor system shouldn’t have any issue with it

There’s an open issue for nullable parser support - I don’t remember why but I remember having issues trying to implement it and it was less trivial than you would expect

https://bitbucket.org/QFSW/quantum-console/issues/161/serialize-parse-nullable

[Yusuf Ismail]

Scratch that, I’m really not sure why that ended up looking like it would be problematic - I’ll give it a go tomorrow and I should be able to get good nullable support into the parser system