Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
110e5c6
ISXB-543 Implemented custom drawer base for enums as well as on spe…
adrian-koretski-unity3d Aug 7, 2024
50f2a47
Removed custom enum drawer.
adrian-koretski-unity3d Aug 13, 2024
4035fe5
Added changes to changelog.
adrian-koretski-unity3d Aug 13, 2024
60640fb
Merge branch 'develop' into isxb-543
adrian-koretski-unity3d Aug 13, 2024
3044d28
Re-added aliased enums into GamepadButton for compatibility with exis…
adrian-koretski-unity3d Aug 14, 2024
6e50417
Merge branch 'develop' of https://github.com/Unity-Technologies/Input…
adrian-koretski-unity3d Sep 9, 2024
e09b575
Ran the formatter on commit changes.
adrian-koretski-unity3d Sep 9, 2024
a964957
Merge branch 'develop' into isxb-543
adrian-koretski-unity3d Sep 10, 2024
806f943
Merge branch 'develop' into isxb-543
adrian-koretski-unity3d Sep 12, 2024
1318c70
Revert "ISXB-543 Implemented custom drawer base for enums as well a…
adrian-koretski-unity3d Sep 13, 2024
5f1b160
Merge branch 'isxb-543' of https://github.com/Unity-Technologies/Inpu…
adrian-koretski-unity3d Sep 13, 2024
e7e69d3
Reworked GpadButtonPropertyDrawer according to feedback.
adrian-koretski-unity3d Sep 16, 2024
d85c1ed
Merge branch 'develop' into isxb-543
adrian-koretski-unity3d Sep 18, 2024
dd2de37
Fixed GamepadButtonPropertyDrawer issues based on PR feedback.
adrian-koretski-unity3d Sep 18, 2024
d3fa10e
Moved changelog entry for ISXB-543 into correct version and added lin…
adrian-koretski-unity3d Sep 19, 2024
fc85832
Changed jira link in changelog for ISXB-543 to public issue tracker.
adrian-koretski-unity3d Sep 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Assets/Tests/InputSystem/CoreTests_Actions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4661,7 +4661,7 @@ public void Actions_PressingAndReleasingButtonInSameUpdate_StillTriggersAction()
ctx => { ++receivedCalls; };
action.Enable();

var firstState = new GamepadState {buttons = 1 << (int)GamepadButton.B};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix shouldn't have any impact on existing tests?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hence I wouldn't expect any changes to any tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is due to my first pass where I removed the aliased values. Will revert

var firstState = new GamepadState {buttons = 1 << (int)GamepadButton.East};
var secondState = new GamepadState {buttons = 0};

InputSystem.QueueStateEvent(gamepad, firstState);
Expand Down Expand Up @@ -6925,7 +6925,7 @@ public void Actions_CanDistinguishTapAndSlowTapOnSameAction()
trace.SubscribeTo(action);

// Perform tap.
InputSystem.QueueStateEvent(gamepad, new GamepadState().WithButton(GamepadButton.A), 0.0);
InputSystem.QueueStateEvent(gamepad, new GamepadState().WithButton(GamepadButton.South), 0.0);
InputSystem.QueueStateEvent(gamepad, new GamepadState(), 0.05);
InputSystem.Update();

Expand All @@ -6942,7 +6942,7 @@ public void Actions_CanDistinguishTapAndSlowTapOnSameAction()
trace.Clear();

// Perform slow tap.
InputSystem.QueueStateEvent(gamepad, new GamepadState().WithButton(GamepadButton.A), 2.0);
InputSystem.QueueStateEvent(gamepad, new GamepadState().WithButton(GamepadButton.South), 2.0);
InputSystem.QueueStateEvent(gamepad, new GamepadState(),
2.0 + InputSystem.settings.defaultSlowTapTime + 0.0001);
InputSystem.Update();
Expand Down
8 changes: 4 additions & 4 deletions Assets/Tests/InputSystem/CoreTests_Devices.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4180,7 +4180,7 @@ public void Devices_AreMadeCurrentWhenReceivingStateEvent()

Assert.That(Gamepad.current, Is.Not.SameAs(gamepad1));

InputSystem.QueueStateEvent(gamepad1, new GamepadState().WithButton(GamepadButton.A));
InputSystem.QueueStateEvent(gamepad1, new GamepadState().WithButton(GamepadButton.South));
InputSystem.Update();

Assert.That(Gamepad.current, Is.SameAs(gamepad1));
Expand All @@ -4199,15 +4199,15 @@ public void Devices_AreNotMadeCurrentWhenReceivingStateEventWithNoControlsChange
var gamepad1 = InputSystem.AddDevice<Gamepad>();
var gamepad2 = InputSystem.AddDevice<Gamepad>();

InputSystem.QueueStateEvent(gamepad1, new GamepadState().WithButton(GamepadButton.A));
InputSystem.QueueStateEvent(gamepad1, new GamepadState().WithButton(GamepadButton.South));
InputSystem.Update();
Assert.That(Gamepad.current, Is.SameAs(gamepad1));

InputSystem.QueueStateEvent(gamepad2, new GamepadState().WithButton(GamepadButton.B));
InputSystem.QueueStateEvent(gamepad2, new GamepadState().WithButton(GamepadButton.East));
InputSystem.Update();
Assert.That(Gamepad.current, Is.SameAs(gamepad2));

InputSystem.QueueStateEvent(gamepad1, new GamepadState().WithButton(GamepadButton.A));
InputSystem.QueueStateEvent(gamepad1, new GamepadState().WithButton(GamepadButton.South));
InputSystem.Update();

// If none of the controls changed, a state event shouldn't switch current gamepad.
Expand Down
18 changes: 9 additions & 9 deletions Assets/Tests/InputSystem/CoreTests_Events.cs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ public void Events_CanListenForEvents()
allButtonPresses.Clear();

Release(gamepad.buttonSouth);
InputSystem.QueueStateEvent(gamepad, new GamepadState(GamepadButton.A, GamepadButton.B));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not expecting any changes needed to this file

InputSystem.QueueStateEvent(gamepad, new GamepadState(GamepadButton.South, GamepadButton.East));
InputSystem.Update();

Assert.That(callOnceOnButtonPressCount, Is.EqualTo(1));
Expand Down Expand Up @@ -135,7 +135,7 @@ public void Events_CanListenForButtonPresses()

Assert.That(callCount, Is.Zero);

InputSystem.QueueStateEvent(gamepad, new GamepadState(GamepadButton.A));
InputSystem.QueueStateEvent(gamepad, new GamepadState(GamepadButton.South));
InputSystem.Update();

Assert.That(callCount, Is.EqualTo(1));
Expand All @@ -145,7 +145,7 @@ public void Events_CanListenForButtonPresses()

Assert.That(callCount, Is.EqualTo(1));

InputSystem.QueueStateEvent(gamepad, new GamepadState(GamepadButton.A));
InputSystem.QueueStateEvent(gamepad, new GamepadState(GamepadButton.South));
InputSystem.Update();

Assert.That(callCount, Is.EqualTo(1));
Expand Down Expand Up @@ -222,7 +222,7 @@ public void Events_CanGetAllButtonPressesInEvent()
controls = eventPtr.GetAllButtonPresses().ToList();
};

InputSystem.QueueStateEvent(gamepad, new GamepadState(GamepadButton.A, GamepadButton.B));
InputSystem.QueueStateEvent(gamepad, new GamepadState(GamepadButton.South, GamepadButton.East));
InputSystem.Update();

Assert.That(controls, Is.EquivalentTo(new[] { gamepad.aButton, gamepad.bButton }));
Expand Down Expand Up @@ -1319,7 +1319,7 @@ public unsafe void Events_CanTraceEventsOfDevice_AndFilterEventsThroughCallback(

trace.Enable();

InputSystem.QueueStateEvent(gamepad, new GamepadState(GamepadButton.A));
InputSystem.QueueStateEvent(gamepad, new GamepadState(GamepadButton.South));
InputSystem.QueueStateEvent(gamepad, default(GamepadState));
InputSystem.Update();

Expand All @@ -1338,8 +1338,8 @@ public void Events_CanTraceEventsOfDevice_AndGrowBufferAsNeeded()
{
trace.Enable();

InputSystem.QueueStateEvent(device, new GamepadState().WithButton(GamepadButton.A));
InputSystem.QueueStateEvent(device, new GamepadState().WithButton(GamepadButton.B));
InputSystem.QueueStateEvent(device, new GamepadState().WithButton(GamepadButton.South));
InputSystem.QueueStateEvent(device, new GamepadState().WithButton(GamepadButton.East));

InputSystem.Update();

Expand All @@ -1358,7 +1358,7 @@ public void Events_CanTraceEventsOfDevice_AndRecordFrameBoundaries()
{
trace.Enable();

InputSystem.QueueStateEvent(gamepad, new GamepadState(GamepadButton.A));
InputSystem.QueueStateEvent(gamepad, new GamepadState(GamepadButton.South));
InputSystem.Update();

Assert.That(trace.eventCount, Is.EqualTo(2));
Expand Down Expand Up @@ -1539,7 +1539,7 @@ public void Events_CanPersistEventTracesInStream()
originalTrace.Enable();

InputSystem.QueueStateEvent(pen, new PenState { position = new Vector2(123, 234) });
InputSystem.QueueStateEvent(gamepad, new GamepadState(GamepadButton.A));
InputSystem.QueueStateEvent(gamepad, new GamepadState(GamepadButton.South));
InputSystem.QueueStateEvent(pen, new PenState { position = new Vector2(234, 345) });

InputSystem.Update();
Expand Down
6 changes: 3 additions & 3 deletions Assets/Tests/InputSystem/CoreTests_State.cs
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@ public void State_CanUpdateButtonStateUsingEvent()

Assert.That(gamepad.buttonEast.isPressed, Is.False);

var newState = new GamepadState {buttons = 1 << (int)GamepadButton.B};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not expecting any changes needed to this file

var newState = new GamepadState {buttons = 1 << (int)GamepadButton.East};
InputSystem.QueueStateEvent(gamepad, newState);
InputSystem.Update();

Expand All @@ -440,7 +440,7 @@ public void State_CanDetectWhetherButtonStateHasChangedThisFrame()
Assert.That(gamepad.buttonEast.wasPressedThisFrame, Is.False);
Assert.That(gamepad.buttonEast.wasReleasedThisFrame, Is.False);

var firstState = new GamepadState {buttons = 1 << (int)GamepadButton.B};
var firstState = new GamepadState {buttons = 1 << (int)GamepadButton.East};
InputSystem.QueueStateEvent(gamepad, firstState);
InputSystem.Update();

Expand Down Expand Up @@ -472,7 +472,7 @@ public void State_PressingAndReleasingButtonInSameFrame_ShowsStateChange(bool us

var gamepad = InputSystem.AddDevice<Gamepad>();

var firstState = new GamepadState {buttons = 1 << (int)GamepadButton.B};
var firstState = new GamepadState {buttons = 1 << (int)GamepadButton.East};
var secondState = new GamepadState {buttons = 0};

InputSystem.QueueStateEvent(gamepad, firstState);
Expand Down
2 changes: 1 addition & 1 deletion Assets/Tests/InputSystem/Plugins/UserTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -942,7 +942,7 @@ public void Users_CanDetectUseOfUnpairedDevice()

++InputUser.listenForUnpairedDeviceActivity;

InputSystem.QueueStateEvent(gamepad, new GamepadState { leftStick = new Vector2(1, 0)}.WithButton(GamepadButton.A));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't need to change either.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is due to my first pass where I removed the aliased values. Will revert

InputSystem.QueueStateEvent(gamepad, new GamepadState { leftStick = new Vector2(1, 0)}.WithButton(GamepadButton.South));
InputSystem.Update();

Assert.That(receivedControls, Has.Count.EqualTo(2));
Expand Down
1 change: 1 addition & 0 deletions Packages/com.unity.inputsystem/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ however, it has to be formatted properly to pass verification tests.

### Changed
- Use `ProfilerMarker` instead of `Profiler.BeginSample` and `Profiler.EndSample` when appropriate to enable recording of profiling data.
- Removed aliased values in GamepadButton enum.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be reverted as well, suggest to change to something like e.g. "Changed representation of GamepadButton enum values in Inspector to display aliased enum values as a single item to avoid confusion around selection and aliased value display when multiple enum items map to the same numerical value."


### Added
- Added Hinge Angle sensor support for foldable devices.
Expand Down
8 changes: 4 additions & 4 deletions Packages/com.unity.inputsystem/InputSystem/Devices/Gamepad.cs
Original file line number Diff line number Diff line change
Expand Up @@ -255,31 +255,31 @@ public enum GamepadButton
/// The upper action button on a gamepad.
/// </summary>
/// <remarks>
/// Identical to <see cref="Y"/> and <see cref="Triangle"/> which are the Xbox and PlayStation controller names for this button.
/// Identical to <see cref="North"/> and <see cref="North"/> which are the Xbox and PlayStation controller names for this button.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems incorrect? Referencing to the same thing

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is due to my first pass where I removed the aliased values. Will revert

/// </remarks>
North = 4,

/// <summary>
/// The right action button on a gamepad.
/// </summary>
/// <remarks>
/// Identical to <see cref="B"/> and <see cref="Circle"/> which are the Xbox and PlayStation controller names for this button.
/// Identical to <see cref="East"/> and <see cref="East"/> which are the Xbox and PlayStation controller names for this button.
/// </remarks>
East = 5,

/// <summary>
/// The lower action button on a gamepad.
/// </summary>
/// <remarks>
/// Identical to <see cref="A"/> and <see cref="Cross"/> which are the Xbox and PlayStation controller names for this button.
/// Identical to <see cref="South"/> and <see cref="South"/> which are the Xbox and PlayStation controller names for this button.
/// </remarks>
South = 6,

/// <summary>
/// The left action button on a gamepad.
/// </summary>
/// <remarks>
/// Identical to <see cref="X"/> and <see cref="Square"/> which are the Xbox and PlayStation controller names for this button.
/// Identical to <see cref="West"/> and <see cref="West"/> which are the Xbox and PlayStation controller names for this button.
/// </remarks>
West = 7,

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
#if UNITY_EDITOR
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2024-09-13 at 11 09 14
Screenshot 2024-09-13 at 11 11 23
I am afraid this is not doing what its supposed to.....

The following things need to be addressed:

  • Aliased values should be part of the same entry when the inspector is displayed, e.g. "South / X / A"
  • Drop down need to allow all values to be picked, e.g. either "South / X / A" or each one individually would be fine from my perspective.
  • At the moment when selecting e.g. South, the value displayed is "North", so something with mapping is incorrect

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll go over it and make sure it behaves and displays correctly


using System;
using System.Collections.Generic;
using System.Linq;
using System.Xml.Linq;
using UnityEditor;
using UnityEngine.UIElements;

namespace UnityEngine.InputSystem.Editor
{
/// <summary>
/// Abstract base class for a generic property drawer for aliased enums.
/// </summary>
internal abstract class AliasedEnumPropertyDrawer<T> : PropertyDrawer where T : Enum
{
private string[] m_EnumDisplayNames;

public override VisualElement CreatePropertyGUI(SerializedProperty property)
{
ProcessDisplayNamesForAliasedEnums();
return base.CreatePropertyGUI(property);
}

protected abstract bool TryGetNonAliasedNames(string enumName, string displayName, out string outputName);

private void ProcessDisplayNamesForAliasedEnums()
{
var enumNamesAndValues = new Dictionary<string, int>();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the enum is small and we want consistency, maybe skip this base class and just move into specific GamepadButtonPropertyDrawer class and just use a big switch statement to get it right? That would be quite straightforward. Otherwise there needs to be some smartness to get the order of each aliased combination separated by slashes consistent.

var enumDisplayNames = Enum.GetNames(typeof(T));
var enumValues = Enum.GetValues(typeof(T)).Cast<T>().ToArray();
var enumStringValues = enumValues.Select(v => v.ToString()).ToArray();
for (var i = 0; i < enumDisplayNames.Length; ++i)
{
var enumName = enumDisplayNames[i];

if (TryGetNonAliasedNames(enumStringValues[i], enumDisplayNames[i], out string aliasedName))
{
if (!string.IsNullOrEmpty(aliasedName))
{
enumName = $"{enumName} ({aliasedName})";
}
else
{
continue;
}
}

enumNamesAndValues.Add(enumName, (int)enumValues.GetValue(i));
}

var sortedEntries = enumNamesAndValues
.OrderBy(x => x.Value)
.ThenBy(x => x.Key.Contains("("));

m_EnumDisplayNames = sortedEntries.Select(x => x.Key).ToArray();
}

public override void OnGUI(Rect position, SerializedProperty property, GUIContent label)
{
EditorGUI.BeginProperty(position, label, property);

if (property.propertyType == SerializedPropertyType.Enum)
{
property.enumValueIndex = EditorGUI.Popup(position, label.text, property.enumValueIndex, m_EnumDisplayNames);
}

EditorGUI.EndProperty();
}
}
}
#endif // UNITY_EDITOR

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
#if UNITY_EDITOR

using UnityEditor;
using UnityEngine.InputSystem.LowLevel;

namespace UnityEngine.InputSystem.Editor
{
/// <summary>
/// Property drawer for <see cref = "GamepadButton" />
/// </summary >
[CustomPropertyDrawer(typeof(GamepadButton))]
internal class GpadButtonPropertyDrawer : AliasedEnumPropertyDrawer<GamepadButton>
{
protected override bool TryGetNonAliasedNames(string enumName, string displayName, out string outputName)
{
outputName = "";
switch (displayName)
{
case nameof(GamepadButton.Y):
case nameof(GamepadButton.Triangle):
case nameof(GamepadButton.A):
case nameof(GamepadButton.Cross):
case nameof(GamepadButton.B):
case nameof(GamepadButton.Circle):
case nameof(GamepadButton.X):
case nameof(GamepadButton.Square):
return true;
default:
return false;
}
}
}
}
#endif // UNITY_EDITOR

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.