Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions Assets/Tests/InputSystem/Plugins/OnScreenTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,33 @@ public void Devices_DisablingLastOnScreenControlRemovesCreatedDevice()
Assert.That(InputSystem.devices, Has.None.InstanceOf<Keyboard>());
}

[Test]
[Category("Devices")]
public void Devices_DisablingLastOnScreenControlDoesReportActiveControl()
{
var gameObject = new GameObject();

Assert.That(OnScreenControl.HasAnyActive, Is.False);

var buttonA = gameObject.AddComponent<OnScreenButton>();

Assert.That(OnScreenControl.HasAnyActive, Is.True);

var buttonB = gameObject.AddComponent<OnScreenButton>();
buttonA.controlPath = "/<Keyboard>/a";
buttonB.controlPath = "/<Keyboard>/b";

Assert.That(OnScreenControl.HasAnyActive, Is.True);

buttonA.enabled = false;

Assert.That(OnScreenControl.HasAnyActive, Is.True);

buttonB.enabled = false;

Assert.That(OnScreenControl.HasAnyActive, Is.False);
}

// https://fogbugz.unity3d.com/f/cases/1271942
[UnityTest]
[Category("Devices")]
Expand Down
53 changes: 53 additions & 0 deletions Assets/Tests/InputSystem/Plugins/PlayerInputTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
using Object = UnityEngine.Object;
using Gyroscope = UnityEngine.InputSystem.Gyroscope;
using Is = UnityEngine.TestTools.Constraints.Is;
using UnityEngine.InputSystem.OnScreen;

/// <summary>
/// Tests for <see cref="PlayerInput"/> and <see cref="PlayerInputManager"/>.
Expand Down Expand Up @@ -663,6 +664,58 @@ public void PlayerInput_CanAutoSwitchControlSchemesInSinglePlayer()
}));
}

[Test]
[Category("PlayerInput")]
public void PlayerInput_AutoSwitchControlSchemesInSinglePlayerWithOnScreenControl_AutoSwitchToTargetDeviceAndIgnoreMouse()
{
var keyboard = InputSystem.AddDevice<Keyboard>();
var mouse = InputSystem.AddDevice<Mouse>();

var go = new GameObject();

var onScreenButton = go.AddComponent<OnScreenButton>();
onScreenButton.enabled = false;
onScreenButton.controlPath = "<Gamepad>/buttonSouth";

var playerInput = go.AddComponent<PlayerInput>();
playerInput.defaultControlScheme = "Keyboard&Mouse";
playerInput.defaultActionMap = "gameplay";
playerInput.actions = InputActionAsset.FromJson(kActions);

Assert.That(playerInput.devices, Is.EquivalentTo(new InputDevice[] { keyboard, mouse }));

// enable the OnScreenButton, it should switch to Gamepad
onScreenButton.enabled = true;
var gamepad = onScreenButton.control.device;
Assert.That(gamepad, Is.TypeOf<Gamepad>());
Assert.That(playerInput.devices, Is.EquivalentTo(new InputDevice[] { gamepad }));
Assert.That(playerInput.user.controlScheme, Is.Not.Null);
Assert.That(playerInput.user.controlScheme.Value.name, Is.EqualTo("Gamepad"));

// Perform mouse move and click. to try to switch to Keyboard&Mouse scheme
Move(mouse.position, new Vector2(0.123f, 0.234f));
Click(mouse.leftButton);
Move(mouse.position, new Vector2(100f, 100f));
InputSystem.Update();

// The controlScheme shouldn't have changed
Copy link
Collaborator

Choose a reason for hiding this comment

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

I remain confused regarding why this would be expected... I guess the current problem is that OnScreenStick and OnScreenButton is using pointer instead of touch so this leads to ping pong in editor setting. Still given how things currently work don't we define auto switching as being triggered by input on related control schemes? If this scenario is about on screen sticks it feels like what would make sense would be for the on screen controls to turn of auto switching since it makes little sense to have onscreen controls and auto-switching combined?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't know it's a valid use case to keep auto-switching with onscreen control.
If we are afraid of side effect we could add an opt out parameter to allow autoswitch when there is an on screen control.

Assert.That(playerInput.devices, Is.EquivalentTo(new[] { gamepad }));
Assert.That(playerInput.user.controlScheme, Is.Not.Null);
Assert.That(playerInput.user.controlScheme.Value.name, Is.EqualTo("Gamepad"));

// disabling the OnScreenButton to ensure that it will now switch to Keyboard&Mouse as expected
onScreenButton.enabled = false;

// Perform mouse move and click. to try to switch to Keyboard&Mouse scheme
Move(mouse.position, new Vector2(0.123f, 0.234f));
Click(mouse.leftButton);
Move(mouse.position, new Vector2(100f, 100f));

Assert.That(playerInput.devices, Is.EquivalentTo(new InputDevice[] { keyboard, mouse }));
Assert.That(playerInput.user.controlScheme, Is.Not.Null);
Assert.That(playerInput.user.controlScheme.Value.name, Is.EqualTo("Keyboard&Mouse"));
}

[Test]
[Category("PlayerInput")]
public void PlayerInput_CanAutoSwitchControlSchemesInSinglePlayer_WithSomeDevicesSharedBetweenSchemes()
Expand Down
5 changes: 5 additions & 0 deletions Assets/Tests/InputSystem/Plugins/UITests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2630,6 +2630,11 @@ public void UI_ClickDraggingMouseDoesNotAllocateGCMemory()
Release(mouse.leftButton);
scene.eventSystem.InvokeUpdate();

#if UNITY_2023_2_OR_NEWER // UnityEngine.InputForUI Module unavailable in earlier releases
// Process all queued UI events to ensure that next events will not make the events list capacity growing
UnityEngine.InputForUI.EventProvider.NotifyUpdate();
#endif

var kProfilerRegion = "UI_ClickDraggingDoesNotAllocateGCMemory";

// Now for real.
Expand Down
6 changes: 3 additions & 3 deletions Packages/com.unity.inputsystem/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@ however, it has to be formatted properly to pass verification tests.
- Fixed pointerId staying the same when simultaneously releasing and then pressing in the same frame on mobile using touch. [ISXB-1006](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-845)
- Fixed ISubmitHandler.OnSubmit event processing when operating in Manual Update mode (ISXB-1141)
- Fixed Rename mode is not entered and name is autocompleted to default when creating a new Action Map on 2022.3. [ISXB-1151](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1151)
- Fixed unexpected control scheme switch when using `OnScreenControl` and pointer based schemes which registed "Cancel" event on every frame.[ISXB-656](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-656)

### Changed
- Added back the InputManager to InputSystem project-wide asset migration code with performance improvement (ISX-2086)
- Changed `OnScreenControl` to automaticaly switch, in Single Player with autoswitch enabled, to the target device control scheme when the first component is enabled to prevent bad interactions when it start.

## [1.11.2] - 2024-10-16

Expand All @@ -30,14 +32,12 @@ however, it has to be formatted properly to pass verification tests.
- Fixed "AnalyticsResult" errors on consoles [ISXB-1107]
- Fixed wrong `Display Index` value for touchscreen events.[ISXB-1101](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1101)
- Fixed event handling when using Fixed Update processing where WasPressedThisFrame could appear to true for consecutive frames [ISXB-1006](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1006)
- Removed a redundant warning when using fallback code to parse a HID descriptor. (UUM-71260)

### Added
- Added the display of the device flag `CanRunInBackground` in device debug view.
- Added analytics for programmatic `InputAction` setup via `InputActionSetupExtensions` when exiting play-mode.

### Fixed
- Removed a redundant warning when using fallback code to parse a HID descriptor. (UUM-71260)

### Changed
- Removed the InputManager to InputSystem project-wide asset migration code for performance improvement (ISX-2086)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using Unity.Collections;
using UnityEngine.InputSystem.Layouts;
using UnityEngine.InputSystem.LowLevel;
using UnityEngine.InputSystem.Users;
using UnityEngine.InputSystem.Utilities;

////REVIEW: should we make this ExecuteInEditMode?
Expand Down Expand Up @@ -34,6 +35,14 @@ namespace UnityEngine.InputSystem.OnScreen
/// types of device layouts (e.g. one control references 'buttonWest' on
/// a gamepad and another references 'leftButton' on a mouse), then a device
/// is created for each type referenced by the setup.
///
/// The <see cref="OnScreenControl"/> works by simulating events from the device specified in the <see cref="OnScreenControl.controlPath"/>
/// property. Some parts of the Input System, such as the <see cref="PlayerInput"/> component, can be set up to
/// auto-switch <see cref="PlayerInput.neverAutoSwitchControlSchemes"/> to a new device when input from them is detected.
/// When a device is switched, any currently running inputs from the previously active device are cancelled.
///
/// To avoid this situation, you need to ensure, depending on your case, that the Mouse, Pen, Touchsceen and/or XRController devices are not used in a concurent
/// control schemes of the simulated device.
/// </remarks>
public abstract class OnScreenControl : MonoBehaviour
{
Expand Down Expand Up @@ -208,13 +217,45 @@ protected void SentDefaultValueToControl()
InputSystem.QueueEvent(m_InputEventPtr);
}

// Used by PlayerInput auto switch for scheme to prevent using Pointer device.
internal static bool HasAnyActive => s_nbActiveInstances != 0;
private static int s_nbActiveInstances = 0;

protected virtual void OnEnable()
{
++s_nbActiveInstances;
SetupInputControl();
if (m_Control == null)
return;
// if we are in single player and if it the first active switch to the target device.
if (s_nbActiveInstances == 1 &&
PlayerInput.isSinglePlayer)
{
var firstPlayer = PlayerInput.GetPlayerByIndex(0);
if (firstPlayer?.neverAutoSwitchControlSchemes == false)
{
var devices = firstPlayer.devices;
bool deviceFound = false;
// skip is the device is already part of the current scheme
foreach (var device in devices)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm less confident with that part I choose to switch to target device in all case but maybe I just only try to move away from a pointer device. 🤔

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah I started to think the same, that maybe the design error here is, first of all auto-switch with on-screen controls, but also that on screen controls rely on pointer instead of touch. Of course pointer is convenient when working in the editor but generally it feels like touch should be the underlying input here... In editor its possible to simulate touch from mouse so problem can be solved anyway....

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

without isolation it's hard to reliably filter touch event with ugui event.

{
if (m_Control.device.deviceId == device.deviceId)
{
deviceFound = true;
break;
}
}
if (!deviceFound)
{
firstPlayer.SwitchCurrentControlScheme(m_Control.device);
}
}
}
}

protected virtual void OnDisable()
{
--s_nbActiveInstances;
if (m_Control == null)
return;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,15 @@ namespace UnityEngine.InputSystem.OnScreen
/// <remarks>
/// The <see cref="OnScreenStick"/> works by simulating events from the device specified in the <see cref="OnScreenControl.controlPath"/>
/// property. Some parts of the Input System, such as the <see cref="PlayerInput"/> component, can be set up to
/// auto-switch to a new device when input from them is detected. When a device is switched, any currently running
/// inputs from the previously active device are cancelled. In the case of <see cref="OnScreenStick"/>, this can mean that the
/// <see cref="IPointerUpHandler.OnPointerUp"/> method will be called and the stick will jump back to center, even though
/// the pointer input has not physically been released.
/// auto-switch <see cref="PlayerInput.neverAutoSwitchControlSchemes"/> to a new device when input from them is detected.
/// When a device is switched, any currently running inputs from the previously active device are cancelled.
/// In the case of <see cref="OnScreenStick"/>, this can mean that the <see cref="IPointerUpHandler.OnPointerUp"/> method will be called
/// and the stick will jump back to center, even though the pointer input has not physically been released.
///
/// To avoid this situation, set the <see cref="useIsolatedInputActions"/> property to true. This will create a set of local
/// Input Actions to drive the stick that are not cancelled when device switching occurs.
/// You might also need to ensure, depending on your case, that the Mouse, Pen, Touchsceen and/or XRController devices are not used in a concurent
/// control schemes of the simulated device.
/// </remarks>
[AddComponentMenu("Input/On-Screen Stick")]
[HelpURL(InputSystem.kDocUrl + "/manual/OnScreen.html#on-screen-sticks")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using UnityEngine.InputSystem.LowLevel;
using UnityEngine.InputSystem.Users;
using UnityEngine.InputSystem.Utilities;
using UnityEngine.InputSystem.OnScreen;

#if UNITY_EDITOR
using UnityEngine.InputSystem.Editor;
Expand Down Expand Up @@ -1867,7 +1868,8 @@ private static bool OnPreFilterUnpairedDeviceUsed(InputDevice device, InputEvent
{
// Early out if the device isn't usable with any of our control schemes.
var actions = all[0].actions;
return actions != null && actions.IsUsableWithDevice(device);
// Skip Pointer device if any OnScreenControl is active since they will use it to generate device event
return actions != null && (!OnScreenControl.HasAnyActive || !(device is Pointer)) && actions.IsUsableWithDevice(device);
}

private void OnUnpairedDeviceUsed(InputControl control, InputEventPtr eventPtr)
Expand Down