FIX: ISXB-543 GamepadButton property drawer fix

[ekcoh]

Description

FIX: ISXB-543 GamepadButton property drawer now has consistent behaviour for which aliased enum value is selected and drop-down shows alias mapping. https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-543

Original contribution by @ppandi-rythmos in #1860 routed via this PR.

Changes made

Introducing a custom aliased enum property drawer and a specialised version for GamepadButton enum type.

Notes

Easiest to test by creating a script with a GamepadButton public property and check in Inspector.

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.

[Pauliusd01]

I am still reproducing the issue:

Unity_2024-02-29_11-56-25.mp4

Is it because the circle is technically the east button so It's kind of giving me the right option? But why have them split to more platform specific options then

[ekcoh]

I am still reproducing the issue:

Unity_2024-02-29_11-56-25.mp4

Is it because the circle is technically the east button so It's kind of giving me the right option? But why have them split to more platform specific options then

This is a pure usability problem, it could be done differently. What would you suggest? That we combine the items on a single line? E.g. "North / Y / Triangle" instead? Its the same value.

Yes technically they are the same value.

[ekcoh]

Before this PR it was completely random which one of the aliased values was being converted to string making it even more confusing.

[Pauliusd01]

I am still reproducing the issue:
Unity_2024-02-29_11-56-25.mp4
Is it because the circle is technically the east button so It's kind of giving me the right option? But why have them split to more platform specific options then

This is a pure usability problem, it could be done differently. What would you suggest? That we combine the items on a single line? E.g. "North / Y / Triangle" instead? Its the same value.

Yes technically they are the same value.

Thanks for explaining, yes combining them makes sense to me but it is not necessary to do in this PR. I've found that It also breaks the inspector and throws these errors when actually used in PlayMode:

NullReferenceException: Object reference not set to an instance of an object
UnityEditor.EditorGUIUtility.TempContent (System.String[] texts) (at D:/Gitrepo/unity/Editor/Mono/EditorGUIUtility.cs:1064)
UnityEditor.EditorGUI.Popup (UnityEngine.Rect position, System.String label, System.Int32 selectedIndex, System.String[] displayedOptions, UnityEngine.GUIStyle style) (at D:/Gitrepo/unity/Editor/Mono/EditorGUI.cs:8664)
UnityEditor.EditorGUI.Popup (UnityEngine.Rect position, System.String label, System.Int32 selectedIndex, System.String[] displayedOptions) (at D:/Gitrepo/unity/Editor/Mono/EditorGUI.cs:8659)
UnityEngine.InputSystem.Editor.AliasedEnumPropertyDrawer`1[T].OnGUI (UnityEngine.Rect position, UnityEditor.SerializedProperty property, UnityEngine.GUIContent label) (at ./Packages/com.unity.inputsystem/InputSystem/Editor/PropertyDrawers/EnumPropertyDrawer.cs:56)
UnityEditor.PropertyDrawer.OnGUISafe (UnityEngine.Rect position, UnityEditor.SerializedProperty property, UnityEngine.GUIContent label) (at D:/Gitrepo/unity/Editor/Mono/Inspector/Core/ScriptAttributeGUI/PropertyDrawer.cs:28)
UnityEditor.PropertyHandler.OnGUI (UnityEngine.Rect position, UnityEditor.SerializedProperty property, UnityEngine.GUIContent label, System.Boolean includeChildren, UnityEngine.Rect visibleArea) (at D:/Gitrepo/unity/Editor/Mono/Inspector/Core/ScriptAttributeGUI/PropertyHandler.cs:195)
UnityEditor.PropertyHandler.OnGUI (UnityEngine.Rect position, UnityEditor.SerializedProperty property, UnityEngine.GUIContent label, System.Boolean includeChildren) (at D:/Gitrepo/unity/Editor/Mono/Inspector/Core/ScriptAttributeGUI/PropertyHandler.cs:155)
UnityEditor.PropertyHandler.OnGUILayout (UnityEditor.SerializedProperty property, UnityEngine.GUIContent label, System.Boolean includeChildren, UnityEngine.GUILayoutOption[] options) (at D:/Gitrepo/unity/Editor/Mono/Inspector/Core/ScriptAttributeGUI/PropertyHandler.cs:314)
UnityEditor.EditorGUILayout.PropertyField (UnityEditor.SerializedProperty property, UnityEngine.GUIContent label, System.Boolean includeChildren, UnityEngine.GUILayoutOption[] options) (at D:/Gitrepo/unity/Editor/Mono/EditorGUILayout.cs:2079)
UnityEditor.EditorGUILayout.PropertyField (UnityEditor.SerializedProperty property, System.Boolean includeChildren, UnityEngine.GUILayoutOption[] options) (at D:/Gitrepo/unity/Editor/Mono/EditorGUILayout.cs:2073)
UnityEditor.UIElements.PropertyField.<CreatePropertyIMGUIContainer>b__49_0 () (at D:/Gitrepo/unity/Editor/Mono/UIElements/Controls/PropertyField.cs:439)
UnityEngine.UIElements.IMGUIContainer.DoOnGUI (UnityEngine.Event evt, UnityEngine.Matrix4x4 parentTransform, UnityEngine.Rect clippingRect, System.Boolean isComputingLayout, UnityEngine.Rect layoutSize, System.Action onGUIHandler, System.Boolean canAffectFocus) (at D:/Gitrepo/unity/Modules/UIElements/Core/IMGUIContainer.cs:396)
UnityEngine.GUIUtility:ProcessEvent(Int32, IntPtr, Boolean&) (at D:/Gitrepo/unity/Modules/IMGUI/GUIUtility.cs:219)

This is all my script is doing:

using UnityEngine;
using UnityEngine.InputSystem.LowLevel;

public class NewMonoBehaviourScript : MonoBehaviour
{
    public GamepadButton myButton;
 
    void Update()
    {
        Debug.Log(myButton.ToString());
    }
}

And it does not reproduce in Develop

Unity_2024-02-29_13-37-32.mp4

[Pauliusd01]

Updating status - bug above needs fixing

[ekcoh]

@Pauliusd01 The bug should be fixed via c1f4c9e

[ekcoh]

I did not modify behavior so UX question remains:

a) Do we want current behavior where one aliased enumeration type is displayed e.g. "North" with aliased enums listed as e.g. "Triangle (North)" and "Y (North)". OR
b) Do we want behavior where we reduce number of items in list by collapsing them, e.g. "North / Triangle / Y".

Notice that this is due to they all mapping to same integer value so only different "labels / identifiers" for the same option (usages).

[Billreyn]

@ekcoh Thanks for looking into this.

In my view, the aliasing behaviour is not ideal, since it breaks the mental model of an enum of choices.

My suggestion is to go with option 2 - we just collapse the items to the real underlying enum and avoid the aliasing altogether.

That means the menu would have items such as:

1. North Triangle/X
2. South / Cross / B
3. East / Square / Y
4. West / Circle / A

This avoids the need for aliasing and means users have an expected behaviour with the menu.

[lyndon-unity]

Code looks reasonable but I haven't tested it in action.
Suggest adding QA or design for a manual verification

[lyndon-unity]

I think this PR is out of data and needs closing.
New PR is here:
#1983 (review)

[ekcoh]

This is no longe relevant, superseded by https://github.com/Unity-Technologies/InputSystem/pull/1983/files