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

NEW: Add toggle to disable automatic deselection in UI module. #1040

Merged
merged 1 commit into from
Feb 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
149 changes: 141 additions & 8 deletions Assets/Tests/InputSystem/Plugins/UITests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -543,9 +543,9 @@ public IEnumerator UI_CanOperateMultiplayerUILocallyUsingGamepads()
// Create actions.
var map = new InputActionMap("map");
asset.AddActionMap(map);
var moveAction = map.AddAction("move");
var submitAction = map.AddAction("submit");
var cancelAction = map.AddAction("cancel");
var moveAction = map.AddAction("move", type: InputActionType.PassThrough);
var submitAction = map.AddAction("submit", type: InputActionType.PassThrough);
var cancelAction = map.AddAction("cancel", type: InputActionType.PassThrough);

// Create bindings.
moveAction.AddBinding(gamepads[i].leftStick);
Expand Down Expand Up @@ -661,9 +661,9 @@ public IEnumerator UI_CanDriveUIFromGamepad()
// Create actions.
var map = new InputActionMap("map");
asset.AddActionMap(map);
var moveAction = map.AddAction("move");
var submitAction = map.AddAction("submit");
var cancelAction = map.AddAction("cancel");
var moveAction = map.AddAction("move", type: InputActionType.PassThrough);
var submitAction = map.AddAction("submit", type: InputActionType.PassThrough);
var cancelAction = map.AddAction("cancel", type: InputActionType.PassThrough);

// Create bindings.
moveAction.AddBinding(gamepad.leftStick);
Expand Down Expand Up @@ -764,8 +764,8 @@ public void UI_CanReassignUIActions()
{
""name"" : ""UI"",
""actions"" : [
{ ""name"" : ""Navigate"" },
{ ""name"" : ""Point"" }
{ ""name"" : ""Navigate"", ""type"" : ""PassThrough"" },
{ ""name"" : ""Point"", ""type"" : ""PassThrough"" }
]
}
]
Expand Down Expand Up @@ -870,6 +870,131 @@ public void UI_MovingAndClickingMouseDoesNotAllocateGCMemory()
}, Is.Not.AllocatingGCMemory());
}

// https://forum.unity.com/threads/feature-request-option-to-disable-deselect-in-ui-input-module.761531
[UnityTest]
[Category("UI")]
public IEnumerator UI_CanPreventAutomaticDeselectionOfGameObjects()
{
var mouse = InputSystem.AddDevice<Mouse>();

var actions = ScriptableObject.CreateInstance<InputActionAsset>();
var uiActions = actions.AddActionMap("UI");
var pointAction = uiActions.AddAction("Point", type: InputActionType.PassThrough, binding: "<Mouse>/position");
var clickAction = uiActions.AddAction("Click", type: InputActionType.PassThrough, binding: "<Mouse>/leftButton");

actions.Enable();

var testObjects = CreateScene(0, 200);

testObjects.uiModule.actionsAsset = actions;
testObjects.uiModule.point = InputActionReference.Create(pointAction);
testObjects.uiModule.leftClick = InputActionReference.Create(clickAction);

// Deselect behavior should be on by default as this corresponds to the behavior before
// we introduced the switch that allows toggling the behavior off.
Assert.That(testObjects.uiModule.deselectOnBackgroundClick, Is.True);

// Give canvas a chance to set itself up.
yield return null;

// Click on left GO and make sure it gets selected.
Set(mouse.position, new Vector2(10, 10));
Press(mouse.leftButton);
yield return null;
Release(mouse.leftButton);

Assert.That(testObjects.eventSystem.currentSelectedGameObject, Is.SameAs(testObjects.leftGameObject));

// Click outside of GOs and make sure the selection gets cleared.
Set(mouse.position, new Vector2(50, 250));
Press(mouse.leftButton);
yield return null;
Release(mouse.leftButton);

Assert.That(testObjects.eventSystem.currentSelectedGameObject, Is.Null);

testObjects.uiModule.deselectOnBackgroundClick = false;

// Click on left GO and make sure it gets selected.
Set(mouse.position, new Vector2(10, 10));
Press(mouse.leftButton);
yield return null;
Release(mouse.leftButton);

Assert.That(testObjects.eventSystem.currentSelectedGameObject, Is.SameAs(testObjects.leftGameObject));

// Click outside of GOs and make sure our selection does NOT get cleared.
Set(mouse.position, new Vector2(50, 250));
Press(mouse.leftButton);
yield return null;
Release(mouse.leftButton);

Assert.That(testObjects.eventSystem.currentSelectedGameObject, Is.SameAs(testObjects.leftGameObject));
}

////REVIEW: While `deselectOnBackgroundClick` does solve the problem of breaking keyboard and gamepad navigation, the question
//// IMO is whether navigation should even be affected that way by not having a current selection. Seems to me that the
//// the system should remember the last selected object and start up navigation from there when nothing is selected.
//// However, given EventSystem.lastSelectedGameObject is no longer supported (why???), it seems like this would require
//// some larger changes.
[UnityTest]
[Category("UI")]
[Ignore("TODO")]
public IEnumerator TODO_UI_CanStartNavigationWhenNothingIsSelected()
{
var mouse = InputSystem.AddDevice<Mouse>();
var gamepad = InputSystem.AddDevice<Gamepad>();

var actions = ScriptableObject.CreateInstance<InputActionAsset>();
var uiActions = actions.AddActionMap("UI");
var pointAction = uiActions.AddAction("Point", type: InputActionType.PassThrough, binding: "<Mouse>/position");
var clickAction = uiActions.AddAction("Click", type: InputActionType.PassThrough, binding: "<Mouse>/leftButton");
var navigateAction = uiActions.AddAction("Navigate", type: InputActionType.PassThrough, binding: "<Gamepad>/dpad");

actions.Enable();

var testObjects = CreateScene(0, 200);

testObjects.uiModule.actionsAsset = actions;
testObjects.uiModule.point = InputActionReference.Create(pointAction);
testObjects.uiModule.leftClick = InputActionReference.Create(clickAction);
testObjects.uiModule.move = InputActionReference.Create(navigateAction);

// Give canvas a chance to set itself up.
yield return null;

// Select left GO.
Set(mouse.position, new Vector2(10, 10));
Press(mouse.leftButton);
yield return null;
Release(mouse.leftButton);

Assert.That(testObjects.eventSystem.currentSelectedGameObject, Is.SameAs(testObjects.leftGameObject));

// Click on background and make sure we deselect.
Set(mouse.position, new Vector2(50, 250));
Press(mouse.leftButton);
yield return null;
Release(mouse.leftButton);

Assert.That(testObjects.eventSystem.currentSelectedGameObject, Is.Null);

// Now perform a navigate-right action. Given we have no current selection, this should
// cause the right GO to be selected based on the fact that the left GO was selected last.
Press(gamepad.dpad.right);
yield return null;

Assert.That(testObjects.eventSystem.currentSelectedGameObject, Is.SameAs(testObjects.rightGameObject));

// Just to make extra sure, navigate left and make sure that results in the expected selection
// change over to the left GO.
Release(gamepad.dpad.right);
Press(gamepad.dpad.left);
yield return null;

Assert.That(testObjects.eventSystem.currentSelectedGameObject, Is.SameAs(testObjects.leftGameObject));
}

// The tracked device tests fail with NullReferenceException in the windows editor on yamato. I cannot reproduce this locally, so will disable them on windows for now.
#if !UNITY_EDITOR_WIN

Expand Down Expand Up @@ -1370,5 +1495,13 @@ public void InvokeUpdate()
{
Update();
}

protected override void OnApplicationFocus(bool hasFocus)
{
// Sync our focus state to that of the test runtime rather than to the Unity test runner (where
// debugging may still focus and thus alter the test run).
hasFocus = ((InputTestRuntime)InputRuntime.s_Instance).hasFocus;
base.OnApplicationFocus(hasFocus);
}
}
}
1 change: 1 addition & 0 deletions Packages/com.unity.inputsystem/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ however, it has to be formatted properly to pass verification tests.
- We've added a new `Simple Multiplayer` sample which demonstrates a simple, bare-bones local multiplayer setup.
- We've also added a `Gamepad Mouse Cursor` sample that shows how to drive a UI mouse cursor using the gamepad.
- The sample contains a reusable `VirtualMouseInput` component that does most of the work.
- Added a `Deselect On Background Click` option to `InputSystemUIInputModule`. This allows toggling the behavior off where clicking the mouse and not hitting a `GameObject` will automatically clear the current selection -- which will break keyboard and gamepad navigation.

## [1.0.0-preview.4] - 2020-01-24

Expand Down
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions Packages/com.unity.inputsystem/Documentation~/UISupport.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ You can use the following properties to configure [`InputSystemUIInputModule`](.
|[`Repeat Delay`](../api/UnityEngine.InputSystem.UI.InputSystemUIInputModule.html#UnityEngine_InputSystem_UI_InputSystemUIInputModule_repeatDelay)|The initial delay (in seconds) between generating an initial navigation event and generating repeated navigation events when the __Move__ Action stays actuated.|
|[`Repeat Rate`](../api/UnityEngine.InputSystem.UI.InputSystemUIInputModule.html#UnityEngine_InputSystem_UI_InputSystemUIInputModule_repeatDelay)|The interval (in seconds) between repeated navigation events being generated when the __Move__ Action stays actuated.|
|[`Actions Asset`](../api/UnityEngine.InputSystem.UI.InputSystemUIInputModule.html#UnityEngine_InputSystem_UI_InputSystemUIInputModule_actionsAsset)|An [`Input Action Asset`](ActionAssets.md) containing all the Actions to control the UI. You can chose which Actions in the Asset correspond to which UI inputs using the following properties.<br><br>By default, this references a built-in asset named *DefaultInputActions*, which contains common default Actions for driving UI. If you want to set up your own Actions, [create a custom Input Action Asset](ActionAssets.md#creating-input-action-assets) and assign it here. When you assign a new Asset reference to this field in the Inspector, the Editor attempts to automatically map Actions to UI inputs based on common naming conventions.|
|[`Deselect on Background Click`](../api/UnityEngine.InputSystem.UI.InputSystemUIInputModule.html#UnityEngine_InputSystem_UI_InputSystemUIInputModule_deselectOnBackgroundClick)|By default, when the pointer is clicked and does not hit any `GameObject`, the current selection is cleared. This, however, can get in the way of keyboard and gamepad navigation which will want to work off the currently selected object. To prevent automatic deselection, set this property to false.|

The following properties let you map Actions from the chosen [__Actions Asset__](../api/UnityEngine.InputSystem.UI.InputSystemUIInputModule.html#UnityEngine_InputSystem_UI_InputSystemUIInputModule_actionsAsset) to UI input Actions. In the Inspector, these appear as foldout lists that contain all the Actions in the Asset:

Expand Down
2 changes: 2 additions & 0 deletions Packages/com.unity.inputsystem/InputSystem/InputSystem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2949,6 +2949,8 @@ internal static void InitializeInEditor(IInputRuntime runtime = null)

internal static void OnPlayModeChange(PlayModeStateChange change)
{
////REVIEW: should we pause haptics when play mode is paused and stop haptics when play mode is exited?

switch (change)
{
case PlayModeStateChange.ExitingEditMode:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,8 @@ private void OnUnpairedDeviceUsed(InputControl control, InputEventPtr eventPtr)
if (!IsDeviceUsableWithPlayerActions(control.device))
return;

////REVIEW: should we log a warning or error when the actions for the player do not have control schemes?

JoinPlayer(pairWithDevice: control.device);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,20 @@ namespace UnityEngine.InputSystem.UI
/// </remarks>
public class InputSystemUIInputModule : BaseInputModule
{
/// <summary>
/// Whether to clear the current selection when a click happens that does not hit any <c>GameObject</c>.
/// </summary>
/// <value>If true (default), clicking outside of any GameObject will reset the current selection.</value>
/// <remarks>
/// By toggling this behavior off, background clicks will keep the current selection. I.e.
/// <c>EventSystem.currentSelectedGameObject</c> will not be changed.
/// </remarks>
public bool deselectOnBackgroundClick
{
get => m_DeselectOnBackgroundClick;
set => m_DeselectOnBackgroundClick = value;
}

public override void ActivateModule()
{
base.ActivateModule();
Expand Down Expand Up @@ -207,9 +221,9 @@ private void ProcessMouseButton(ButtonDeltaState mouseButtonChanges, PointerEven

var selectHandlerGO = ExecuteEvents.GetEventHandler<ISelectHandler>(currentOverGo);

// If we have clicked something new, deselect the old thing
// and leave 'selection handling' up to the press event.
if (selectHandlerGO != eventSystem.currentSelectedGameObject)
// If we have clicked something new, deselect the old thing and leave 'selection handling' up
// to the press event (except if there's none and we're told to not deselect in that case).
if (selectHandlerGO != eventSystem.currentSelectedGameObject && (selectHandlerGO != null || m_DeselectOnBackgroundClick))
eventSystem.SetSelectedGameObject(null, eventData);

// search for the control that will receive the press.
Expand Down Expand Up @@ -754,6 +768,8 @@ private void OnAction(InputAction.CallbackContext context)
state.scrollDelta = context.ReadValue<Vector2>() * (1.0f / kPixelPerLine);
m_MouseStates[index] = state;
}
////FIXME: these are missing clicks that happen within the same frame :(
//// (for these actions, perform polling rather than doing the thing here)
else if (action == m_LeftClickAction?.action)
{
var index = GetMouseDeviceIndexForCallbackContext(context);
Expand Down Expand Up @@ -981,6 +997,8 @@ public InputActionAsset actionsAsset

[SerializeField, HideInInspector] private InputActionAsset m_ActionsAsset;

[SerializeField] private bool m_DeselectOnBackgroundClick = true;

private int m_RollingPointerId;
private bool m_OwnsEnabledState;
private bool m_ActionsHooked;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,15 +127,15 @@ public override void OnInspectorGUI()
var numActions = s_ActionNames.Length;
for (var i = 0; i < numActions; i++)
{
if (m_AvailableActionsInAsset != null)
{
int index = Array.IndexOf(m_AvailableActionsInAsset, m_ReferenceProperties[i].objectReferenceValue) + 1;
EditorGUI.BeginChangeCheck();
index = EditorGUILayout.Popup(s_ActionNiceNames[i], index, m_AvailableActionsInAssetNames);
if (m_AvailableActionsInAsset == null)
continue;

if (EditorGUI.EndChangeCheck())
m_ReferenceProperties[i].objectReferenceValue = index > 0 ? m_AvailableActionsInAsset[index - 1] : null;
}
var index = Array.IndexOf(m_AvailableActionsInAsset, m_ReferenceProperties[i].objectReferenceValue) + 1;
EditorGUI.BeginChangeCheck();
index = EditorGUILayout.Popup(s_ActionNiceNames[i], index, m_AvailableActionsInAssetNames);

if (EditorGUI.EndChangeCheck())
m_ReferenceProperties[i].objectReferenceValue = index > 0 ? m_AvailableActionsInAsset[index - 1] : null;
}

if (GUI.changed)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ internal class InputTestRuntime : IInputRuntime, IDisposable
{
public unsafe delegate long DeviceCommandCallback(int deviceId, InputDeviceCommand* command);

public bool hasFocus => m_HasFocus;

~InputTestRuntime()
{
Dispose();
Expand Down Expand Up @@ -197,6 +199,7 @@ public unsafe long DeviceCommand(int deviceId, InputDeviceCommand* commandPtr)

public void InvokePlayerFocusChanged(bool newFocusState)
{
m_HasFocus = newFocusState;
onPlayerFocusChanged?.Invoke(newFocusState);
}

Expand Down Expand Up @@ -365,6 +368,7 @@ public double currentTimeOffsetToRealtimeSinceStartup

public int eventCount => m_EventCount;

private bool m_HasFocus = true;
private int m_NextDeviceId = 1;
private int m_NextEventId = 1;
internal int m_EventCount;
Expand Down