Skip to content
Merged
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 favour 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,72 @@ 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>
/// <remarks>
/// 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 follows 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 +164,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 +187,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)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like we are preserving old behaviour which is great to see.

#pragma warning restore CS0618
modifiersOrder = ModifiersOrder.Unordered;
else
modifiersOrder = InputSystem.settings.shortcutKeysConsumeInput ? ModifiersOrder.Ordered : ModifiersOrder.Unordered;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this changing the stored value from default to a specific value?
I'm thinking this should be on reading the value rather than changing what's stored.

That said this code was making a change directly before so its not altering behaviour

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It doesn't changed the stored value. The class is not serialized within the classic unity serialization.
It's serialized in the inputaction asset in json.
like

                {
                    "name": "Two Modifiers",
                    "id": "9796175a-7495-4ded-9b01-e17eafba2eaf",
                    "path": "TwoModifiers(overrideModifiersNeedToBePressedFirst=true,modifiersOrder=1)",
                    "interactions": "",
                    "processors": "",
                    "groups": "",
                    "action": "New action",
                    "isComposite": true,
                    "isPartOfComposite": false
                },

The value is only changed a runtime when the binding is setup

}
}
}
}
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,71 @@ 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.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Great you are clarifying this, but still unclear whether ctrl and shift is ordered or not. I suggest either adding this information, e.g. if its a sequence or a set of pressed buttons.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ctrl and shift doesn't need to be in order. I will precise it.

/// 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>
/// <remarks>
/// 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 follows 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 +178,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 +202,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