Adding generalized support for controller usages with the binding path dropdown. Added optional info blurb on the binding paths covered

[RogPodge]

Migrated from #1672

Description

This PR augments the binding path dropdown, allowing users to add control usages for specific devices.

This PR also adds a "Show Matching Paths" checkbox, which shows the users exactly which specific registered layout paths their current binding path will match with.

The goal of this PR is to address some grievances found in this thread: https://forum.unity.com/threads/openxr-primary2daxisclick-not-working.1120864/

General.Binding.Editing.mp4

Failure.Case.mp4

Changes made

Updated the InputControlPickerDropdown to allow for control usages to be selected and added to the paths in after specifying a desired device.

Augmented the UsageDropdown to be usable in more contexts.

Added unit tests for the new InputControlPath function.

Added new option to the InputBindingsPropertiesView.

Notes

Please write down any additional notes, remove the section if not applicable.

Checklist

Before review:

• Changelog entry added.
  • Explains the change in Changed, Fixed, Added sections.
  • For API change contains an example snippet and/or migration example.
  • FogBugz ticket attached, example ([case %number%](https://issuetracker.unity3d.com/issues/...)).
  • FogBugz is marked as "Resolved" with next release version correctly set.
• Tests added/changed, if applicable.
  • Functional tests Area_CanDoX, Area_CanDoX_EvenIfYIsTheCase, Area_WhenIDoX_AndYHappens_ThisIsTheResult.
  • Performance tests.
  • Integration tests.
• Docs for new/changed API's.
  • Xmldoc cross references are set correctly.
  • Added explanation how the API works.
  • Usage code examples added.
  • The manual is updated, if needed.

During merge:

• Commit message for squash-merge is prefixed with one of the list:
  • NEW: ___.
  • FIX: ___.
  • DOCS: ___.
  • CHANGE: ___.
  • RELEASE: 1.1.0-preview.3.

[Rene-Damm]

Using a foldout instead of a checkbox probably makes for a sightly nicer UI. Also, "Matching Controls" maybe slightly clearly naming than "Matching Paths".

[RogPodge]

Tweaked the UI to the PR suggestions.

@reviewers this PR makes some changes to the editor display code, and potentially incurs some new performance overhead. Are there any tests or things we can run to see if the performance impacts are acceptable?

[Billreyn]

Hi there, finally got time to look at this.

So, as I understand it, the idea is to make it clear what the general purpose abstractions in the input system actually bind to, right?

It's probably a good idea to add this - I thought about it before too.

My idea for how to solve it was to do it an a more visual way though.

Question: Is the flyout open or closed by default? Probably I think it should be closed, otherwise it's quite overwhelming.

And, is it displayed when you bind an action directly to an input control? Does it only show if you use an abstraction like GamePad? If not, it probably should do that I think - I mean only be visible when the user is binding to a device abstraction.

[Billreyn]

So, I looked at this more.

I suggest solving this problem in a slightly different way. These bindings are like 'derived bindings'. So I think it fits better in the binding list, closed by default and only visible when users pick an abstraction (usage or gamepad etc)

[Billreyn]

Also, as you can see in the screenshot above, I am wondering if we really need to show the derived bindings different between Xbox Controller, Xbox Controller iOS, Xbox Controller Android? They always seem to be the same anyway, so it seems like redundant information?

[lyndon-unity]

Note also that we are working on a UI Toolkit implementation of this UX so we need to add support into that implementation too. The current work is in develop and behind a flag. See https://docs.google.com/document/d/1ot1-Cr85TxpZGudm-VXvE4-C2YAATqiQcYkc_ww76eI/edit#heading=h.aasuoa63b4nd

Specifically this feature flag needs to be set to true to see the (incomplete) UI TK version InputFeatureNames.kUseUIToolkitEditor

[vrdave-unity]

Just a few questions and nitpicks.

[RogPodge]

Thanks for the suggestions @vrdave-unity. I incorporated all of them, though I have a question. On a given layout, 2 physical buttons should never have the same usage correct?

@Billreyn I really like the layout mockup idea you have there. It's much more elegant than an optional dropdown that you have to expand in the binding properties pane, and the nested structure really helps contain the bloat. Do you have a reference implementation, or can you point me to the code paths I need to touch to make this a reality?

Also, as you can see in the screenshot above, I am wondering if we really need to show the derived bindings different between Xbox Controller, Xbox Controller iOS, Xbox Controller Android? They always seem to be the same anyway, so it seems like redundant information?

On this topic, I think it's still important to make this information accessible, since it helps expose all of the controller layouts that are currently registered (and in some cases, exposes the fact that you may have missed registering certain layouts!) This is especially useful in the XR space, since there's a bit of a manual process to configure your project to support the wide range of XR controllers available.

@lyndon-unity I'm not certain what additional work needs to be done here to account for the concurrent UI toolkit work. Can you give some more details?

[ritamerkl]

Like Lyndon already mentioned we will need this feature to support the newer UIToolkit editor window as well.
Currently the UIToolkit InputActionsEditor is available on the develop branch but has to be enabled by using Unity 2022.1 or newer and modifying a line in InputActionsEditorWindow.cs (see this document)
To support the UIToolkit for this feature additional files have to change. A starting point: You can find the binding properties view here: BindingPropertiesView.cs and the dropdown here: InputControlPathEditor.cs.

[RogPodge]

Hey there guys,

I haven't gotten a chance to update the UIToolkit editor window, but I have updated the Matching Controls section to be a foldout.

I did try to incorporate the feedback to move this information directly into the bindings list, but encountered some issues with creating the UI elements there, since it's implemented as a TreeView. I also feel like it could make for a more confusing UI experience, especially when multiple bindings are mapped to the same action like here:

I left some comments in the code regarding some edge cases I encountered as I was debugging and implementing the foldouts, if anyone has any feedback, it'd be greatly appreciated!

https://github.com/Unity-Technologies/InputSystem/pull/1676/files#diff-6090a6d9adc85bc6f186a6206f2f0659bb1627a735d1474bf71e8153506ff84fR184-R189

[Billreyn]

Hi Roger. Derived bindings is fine with us :)

[lyndon-unity]

*/{PrimaryAction}
Doesn't seem to show any matches for me - this looks like a bug

[lyndon-unity]

Also as you mention above

This doesn't show a match because its a single direct match
/buttonEast

This does feel quite confusing to me. I thought it might be a invalid path, its not possible to tell the difference between invalid path and valid path (unless you've got one that has some derived types that it lists).
Ideally we'd add something like "Valid binding" for those that match something and "No matches" for those that have none.

I'm still not quite satisfied with it, since we don't have additional UI to indicate that the user provided binding is itself a valid binding (doing so would probably best be done in another part of the codebase), but I think the dev time to make something completely comprehensive might be out of scope at the moment 😓

[RogPodge]

*/{PrimaryAction} Doesn't seem to show any matches for me - this looks like a bug

You actually caught a bug that was introduced where we didn't process the 'match any device' case properly.

It should be fixed now.

[Pauliusd01]

Found a bug, when adding 2 or more * at the front of the binding path the whole binding properties sidebar layout breaks. Steps to repro: Type in this path */{PrimaryAction} -> add an extra * at the front
https://github.com/Unity-Technologies/InputSystem/assets/54306142/206099ac-3464-46fe-b76a-8bbbf96f72fe

Alternatively you can also break the UI by using multiple categories in the same path such as: <Gamepad>/<Gamepad>

[RogPodge]

@Pauliusd01 bugs should be fixed!

[lyndon-unity]

Formatting tests are failing - looks like it needs the formatting run on it

[19:05:56.456 Information] Starting process on subprocess runner
[19:06:04.263 Information] warning: Needs formatting: Packages/com.unity.inputsystem/InputSystem/Controls/InputControlPath.cs
[19:06:06.452 Information] warning: Needs formatting: Packages/com.unity.inputsystem/InputSystem/Editor/AssetEditor/InputBindingPropertiesView.cs

[lyndon-unity]

There is also a code analyzer failure.

Seem to be this error :
System.ExecutionEngineException: System.InvalidOperationException: Unexpected value '4' of type 'Microsoft.CodeAnalysis.RefKind' ---> System.InvalidOperationException: Unexpected value '4' of type 'Microsoft.CodeAnalysis.RefKind'

I can't quite see what's triggering it. Here's a link about that kind of error: dotnet/roslyn#29371

I see a few occurrences of this failure on your branch
https://unity-ci.cds.internal.unity3d.com/project/130/branch/inputbinding-controlusages-UI/jobDefinition/.yamato%2Fanalyze.yml%23code_analyser

I'm not seeing the failure on develop
https://unity-ci.cds.internal.unity3d.com/project/130/branch/develop/jobDefinition/.yamato%2Fanalyze.yml%23code_analyser?nav=jobDefinitions

[RogPodge]

small ping after fixing the formatter errors @lyndon-unity @Pauliusd01

[Pauliusd01]

LGTM. Things checked:
Case sensitive scenarios
Normal usage such as (*/{PrimaryAction} / /{Back})
Aliases such as (/a/b/x/y/triangle/etc)
Direct control names such as (/buttonSouth)
Checked what results are shown when using the binding menu itself

Note: One bug that I did notice was a fairly rare Editor freeze under the Application.Message.MouseMove call when selecting the binding path's text (only 4 times in the whole testing period) but it was ruled out as potentially caused by this PR by Roger so I'll try to nail it down and report separately

[lyndon-unity]

Happy to approve now that Paulius has QA'd it.

[RogPodge]

Waiting for someone here to ultimately merge this PR

[jamesmcgill]

Thanks for getting this this over the line @RogPodge