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

FIX: Fixed documentation of bindings with modifiers and UI Toolkit tooltips in the asset editor (ISBX-806) #2048

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open
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
53 changes: 49 additions & 4 deletions Assets/Tests/InputSystem/CoreTests_Actions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -319,22 +319,67 @@ public void Actions_WhenShortcutsEnabled_PressingShortcutSequenceInWrongOrder_Do
[TestCase("leftShift", "leftAlt", "space", true)]
[TestCase("leftShift", null, "space", false)]
[TestCase("leftShift", "leftAlt", "space", false)]
public void Actions_PressingShortcutSequenceInWrongOrder_DoesNotTriggerShortcut_ExceptIfOverridden(string modifier1, string modifier2, string binding,
bool legacyComposites)
public void Actions_WhenShortcutsDisabled_PressingShortcutSequenceInWrongOrder_DoesNotTriggerShortcutIfOverridden(string modifier1, string modifier2, string binding, bool legacyComposites)
{
var keyboard = InputSystem.AddDevice<Keyboard>();

var action = new InputAction();
if (!string.IsNullOrEmpty(modifier2))
{
action.AddCompositeBinding((legacyComposites ? "ButtonWithTwoModifiers" : "TwoModifiers") + "(overrideModifiersNeedToBePressedFirst)")
action.AddCompositeBinding((legacyComposites ? "ButtonWithTwoModifiers" : "TwoModifiers") + "(modifiersOrder=1)")
.With("Modifier1", "<Keyboard>/" + modifier1)
.With("Modifier2", "<Keyboard>/" + modifier2)
.With(legacyComposites ? "Button" : "Binding", "<Keyboard>/" + binding);
}
else
{
action.AddCompositeBinding((legacyComposites ? "ButtonWithOneModifier" : "OneModifier") + "(overrideModifiersNeedToBePressedFirst)")
action.AddCompositeBinding((legacyComposites ? "ButtonWithOneModifier" : "OneModifier") + "(modifiersOrder=1)")
.With("Modifier", "<Keyboard>/" + modifier1)
.With(legacyComposites ? "Button" : "Binding", "<Keyboard>/" + binding);
}

action.Enable();

var wasPerformed = false;
action.performed += _ => wasPerformed = true;

// Press binding first, then modifiers.
Press((ButtonControl)keyboard[binding]);
Press((ButtonControl)keyboard[modifier1]);
if (!string.IsNullOrEmpty(modifier2))
Press((ButtonControl)keyboard[modifier2]);

Assert.That(wasPerformed, Is.False);
}

[Test]
[Category("Actions")]
[TestCase("leftShift", null, "space", true, true)]
[TestCase("leftShift", "leftAlt", "space", true, true)]
[TestCase("leftShift", null, "space", false, true)]
[TestCase("leftShift", "leftAlt", "space", false, true)]
[TestCase("leftShift", null, "space", true, false)]
[TestCase("leftShift", "leftAlt", "space", true, false)]
[TestCase("leftShift", null, "space", false, false)]
[TestCase("leftShift", "leftAlt", "space", false, false)]
public void Actions_WhenShortcutsAreEnabled_PressingShortcutSequenceInWrongOrder_DoesNotTriggerShortcut_ExceptIfOverridden(string modifier1, string modifier2, string binding,
bool legacyComposites, bool overrideModifiersNeedToBePressedFirst)
{
InputSystem.settings.shortcutKeysConsumeInput = true;

var keyboard = InputSystem.AddDevice<Keyboard>();

var action = new InputAction();
if (!string.IsNullOrEmpty(modifier2))
{
action.AddCompositeBinding((legacyComposites ? "ButtonWithTwoModifiers" : "TwoModifiers") + (overrideModifiersNeedToBePressedFirst ? "(overrideModifiersNeedToBePressedFirst)" : "(modifiersOrder=2)"))
.With("Modifier1", "<Keyboard>/" + modifier1)
.With("Modifier2", "<Keyboard>/" + modifier2)
.With(legacyComposites ? "Button" : "Binding", "<Keyboard>/" + binding);
}
else
{
action.AddCompositeBinding((legacyComposites ? "ButtonWithOneModifier" : "OneModifier") + (overrideModifiersNeedToBePressedFirst ? "(overrideModifiersNeedToBePressedFirst)" : "(modifiersOrder=2)"))
.With("Modifier", "<Keyboard>/" + modifier1)
.With(legacyComposites ? "Button" : "Binding", "<Keyboard>/" + binding);
}
Expand Down
3 changes: 3 additions & 0 deletions Packages/com.unity.inputsystem/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,13 @@ however, it has to be formatted properly to pass verification tests.
- Fixed missing documentation for source generated Input Action Assets. This is now generated as part of the source code generation step when "Generate C# Class" is checked in the importer inspector settings.
- Fixed pasting into an empty map list raising an exception. [ISXB-1150](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1150)
- Fixed pasting bindings into empty Input Action asset. [ISXB-1180](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1180)
- Fixed tooltip support in the UI Toolkit version of the Input Actions Asset editor.
- Fixed documentation to clarify bindings with modifiers `overrideModifiersNeedToBePressedFirst` configuration [ISXB-806](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-806).

### 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.
- Changed paremeter `overrideModifiersNeedToBePressedFirst` to obsolete for `ButtonWithOneModifier`, `ButtonWithTwoModifiers`, `OneModifierComposite` and `TwoModifiersComposite` in flavor the the new `modifiersOrder` parameter which is more explicit.

## [1.11.2] - 2024-10-16

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System;
using System.ComponentModel;
using UnityEngine.InputSystem.Layouts;
using UnityEngine.InputSystem.Utilities;
Expand Down Expand Up @@ -79,16 +80,71 @@ public class ButtonWithOneModifier : InputBindingComposite<float>
/// still trigger. Default is false.
/// </summary>
/// <remarks>
/// By default, <see cref="modifier"/> is required to be in pressed state before or at the same time that <see cref="button"/>
/// By default, if the setting <see cref="InputSettings.shortcutKeysConsumeInput"/> is enabled,
/// <see cref="modifier"/> is required to be in pressed state before or at the same time that <see cref="button"/>
/// goes into pressed state for the composite as a whole to trigger. This means that binding to, for example, <c>Shift+B</c>,
/// the <c>shift</c> key has to be pressed before pressing the <c>B</c> key. This is the behavior usually expected with
/// keyboard shortcuts.
///
/// This parameter can be used to bypass this behavior and allow any timing between <see cref="modifier"/> and <see cref="button"/>.
/// The only requirement is for them both to concurrently be in pressed state.
///
/// To don't depends on the setting please consider using <see cref="modifiersOrder"/> instead.
/// </remarks>
[Tooltip("Obsolete please use modifiers Order. If enabled, this will override the Input Consumption setting, allowing the modifier keys to be pressed after the button and the composite will still trigger.")]
[Obsolete("Use ModifiersOrder.Unordered with 'modifiersOrder' instead")]
public bool overrideModifiersNeedToBePressedFirst;

/// <summary>
/// Determines how a <c>modifiers</c> keys need to be pressed in order or not.
/// </summary>
public enum ModifiersOrder
{
/// <summary>
/// By default, if the setting <see cref="InputSettings.shortcutKeysConsumeInput"/> is enabled,
/// <see cref="modifier"/> is required to be in pressed state before or at the same time that <see cref="button"/>
/// goes into pressed state for the composite as a whole to trigger. This means that binding to, for example, <c>Shift+B</c>,
/// the <c>shift</c> key has to be pressed before pressing the <c>B</c> key. This is the behavior usually expected with
/// keyboard shortcuts.
///
/// If the setting <see cref="InputSettings.shortcutKeysConsumeInput"/> is disabled,
/// modifiers can be pressed after the button and the composite will still trigger.
/// </summary>
Default = 0,

/// <summary>
/// <see cref="modifier"/> is required to be in pressed state before or at the same
/// time that <see cref="button"/> goes into pressed state for the composite as a whole to trigger. This means that binding to,
/// for example, <c>Ctrl+B</c>, the <c>ctrl</c> key have to be pressed before pressing the <c>B</c> key.
/// This is the behavior usually expected with keyboard shortcuts.
/// </summary>
Ordered = 1,

/// <summary>
/// <see cref="modifier"/> can be pressed after <see cref="button"/>
/// and the composite will still trigger. The only requirement is for all of them to concurrently be in pressed state.
/// </summary>
Unordered = 2
}

/// <summary>
/// If set to <c>Ordered</c> or <c>Unordered</c>, the built-in logic to determine if modifiers need to be pressed first is overridden.
/// </summary>
/// By default, if the setting <see cref="InputSettings.shortcutKeysConsumeInput"/> is enabled,
/// <see cref="modifier"/> is required to be in pressed state before or at the same time that <see cref="button"/>
/// goes into pressed state for the composite as a whole to trigger. This means that binding to, for example, <c>Shift+B</c>,
/// the <c>shift</c> key has to be pressed before pressing the <c>B</c> key. This is the behavior usually expected with
/// keyboard shortcuts.
///
/// If the setting <see cref="InputSettings.shortcutKeysConsumeInput"/> is disabled,
/// modifiers can be pressed after the button and the composite will still trigger.
///
/// This parameter can be used to bypass this behavior and enforce the timing order or allow any timing between <see cref="modifier"/> and <see cref="button"/>.
/// The only requirement is for them both to concurrently be in pressed state.
/// </remarks>
[Tooltip("By default it follow the Input Consumption setting to determine if the modifers keys need to be pressed first.")]
public ModifiersOrder modifiersOrder = ModifiersOrder.Default;

/// <summary>
/// Return the value of the <see cref="button"/> part if <see cref="modifier"/> is pressed. Otherwise
/// return 0.
Expand All @@ -107,7 +163,7 @@ private bool ModifierIsPressed(ref InputBindingCompositeContext context)
{
var modifierDown = context.ReadValueAsButton(modifier);

if (modifierDown && !overrideModifiersNeedToBePressedFirst)
if (modifierDown && modifiersOrder == ModifiersOrder.Ordered)
{
var timestamp = context.GetPressTime(button);
var timestamp1 = context.GetPressTime(modifier);
Expand All @@ -130,8 +186,17 @@ public override float EvaluateMagnitude(ref InputBindingCompositeContext context

protected override void FinishSetup(ref InputBindingCompositeContext context)
{
if (!overrideModifiersNeedToBePressedFirst)
overrideModifiersNeedToBePressedFirst = !InputSystem.settings.shortcutKeysConsumeInput;
if (modifiersOrder == ModifiersOrder.Default)
{
// Legacy. We need to reference the obsolete member here so temporarily
// turn off the warning.
#pragma warning disable CS0618
if (overrideModifiersNeedToBePressedFirst)
#pragma warning restore CS0618
modifiersOrder = ModifiersOrder.Unordered;
else
modifiersOrder = InputSystem.settings.shortcutKeysConsumeInput ? ModifiersOrder.Ordered : ModifiersOrder.Unordered;
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System;
using System.ComponentModel;
using UnityEngine.InputSystem.Layouts;
using UnityEngine.InputSystem.Utilities;
Expand Down Expand Up @@ -94,16 +95,70 @@ public class ButtonWithTwoModifiers : InputBindingComposite<float>
/// and the composite will still trigger. Default is false.
/// </summary>
/// <remarks>
/// By default, <see cref="modifier1"/> and <see cref="modifier2"/> are required to be in pressed state before or at the same
/// By default, if the setting <see cref="InputSettings.shortcutKeysConsumeInput"/> is enabled,
/// <see cref="modifier1"/> and <see cref="modifier2"/> are required to be in pressed state before or at the same
/// time that <see cref="button"/> goes into pressed state for the composite as a whole to trigger. This means that binding to,
/// for example, <c>Ctrl+Shift+B</c>, the <c>ctrl</c> <c>shift</c> keys have to be pressed before pressing the <c>B</c> key.
/// This is the behavior usually expected with keyboard shortcuts.
///
/// This parameter can be used to bypass this behavior and allow any timing between <see cref="modifier1"/>, <see cref="modifier2"/>,
/// and <see cref="button"/>. The only requirement is for all of them to concurrently be in pressed state.
///
/// To don't depends on the setting please consider using <see cref="modifiersOrder"/> instead.
/// </remarks>
[Tooltip("Obsolete please use modifiers Order. If enabled, this will override the Input Consumption setting, allowing the modifier keys to be pressed after the button and the composite will still trigger.")]
[Obsolete("Use ModifiersOrder.Unordered with 'modifiersOrder' instead")]
public bool overrideModifiersNeedToBePressedFirst;

/// <summary>
/// Determines how a <c>modifiers</c> keys need to be pressed in order or not.
/// </summary>
public enum ModifiersOrder
{
/// <summary>
/// By default, if the setting <see cref="InputSettings.shortcutKeysConsumeInput"/> is enabled,
/// <see cref="modifier1"/> and <see cref="modifier2"/> are required to be in pressed state before or at the same
/// time that <see cref="button"/> goes into pressed state for the composite as a whole to trigger. This means that binding to,
/// for example, <c>Ctrl+Shift+B</c>, the <c>ctrl</c> <c>shift</c> keys have to be pressed before pressing the <c>B</c> key.
/// This is the behavior usually expected with keyboard shortcuts.
///
/// If the setting <see cref="InputSettings.shortcutKeysConsumeInput"/> is disabled,
/// modifiers can be pressed after the button and the composite will still trigger.
/// </summary>
Default = 0,

/// <summary>
/// <see cref="modifier1"/> and <see cref="modifier2"/> are required to be in pressed state before or at the same
/// time that <see cref="button"/> goes into pressed state for the composite as a whole to trigger. This means that binding to,
/// for example, <c>Ctrl+Shift+B</c>, the <c>ctrl</c> <c>shift</c> keys have to be pressed before pressing the <c>B</c> key.
/// This is the behavior usually expected with keyboard shortcuts.
/// </summary>
Ordered = 1,

/// <summary>
/// <see cref="modifier1"/> and/or <see cref="modifier2"/> can be pressed after <see cref="button"/>
/// and the composite will still trigger. The only requirement is for all of them to concurrently be in pressed state.
/// </summary>
Unordered = 2
}

/// <summary>
/// If set to <c>Ordered</c> or <c>Unordered</c>, the built-in logic to determine if modifiers need to be pressed first is overridden.
/// </summary>
/// By default, if the setting <see cref="InputSettings.shortcutKeysConsumeInput"/> is enabled,
/// <see cref="modifier1"/> and <see cref="modifier2"/> are required to be in pressed state before or at the same
/// time that <see cref="button"/> goes into pressed state for the composite as a whole to trigger. This means that binding to,
/// for example, <c>Ctrl+Shift+B</c>, the <c>ctrl</c> <c>shift</c> keys have to be pressed before pressing the <c>B</c> key.
/// This is the behavior usually expected with keyboard shortcuts.
///
/// If the setting <see cref="InputSettings.shortcutKeysConsumeInput"/> is disabled,
/// modifiers can be pressed after the button and the composite will still trigger.
///
/// This field allows you to explicitly override this default inference.
/// </remarks>
[Tooltip("By default it follow the Input Consumption setting to determine if the modifers keys need to be pressed first.")]
public ModifiersOrder modifiersOrder = ModifiersOrder.Default;

/// <summary>
/// Return the value of the <see cref="button"/> part while both <see cref="modifier1"/> and <see cref="modifier2"/>
/// are pressed. Otherwise return 0.
Expand All @@ -122,7 +177,7 @@ private bool ModifiersArePressed(ref InputBindingCompositeContext context)
{
var modifiersDown = context.ReadValueAsButton(modifier1) && context.ReadValueAsButton(modifier2);

if (modifiersDown && !overrideModifiersNeedToBePressedFirst)
if (modifiersDown && modifiersOrder == ModifiersOrder.Ordered)
{
var timestamp = context.GetPressTime(button);
var timestamp1 = context.GetPressTime(modifier1);
Expand All @@ -146,8 +201,17 @@ public override float EvaluateMagnitude(ref InputBindingCompositeContext context

protected override void FinishSetup(ref InputBindingCompositeContext context)
{
if (!overrideModifiersNeedToBePressedFirst)
overrideModifiersNeedToBePressedFirst = !InputSystem.settings.shortcutKeysConsumeInput;
if (modifiersOrder == ModifiersOrder.Default)
{
// Legacy. We need to reference the obsolete member here so temporarily
// turn off the warning.
#pragma warning disable CS0618
if (overrideModifiersNeedToBePressedFirst)
#pragma warning restore CS0618
modifiersOrder = ModifiersOrder.Unordered;
else
modifiersOrder = InputSystem.settings.shortcutKeysConsumeInput ? ModifiersOrder.Ordered : ModifiersOrder.Unordered;
}
}
}
}
Loading