NEW: Add new public setting to enable shortcut key consumption (ISXB-254) (#1602)

* CHANGE: shortcut support disabled by default

* add test and fix shortcut specific tests

* review changes. fix changelog entry

* review comments: add shortcut feature toggle to the Project Settings

* review comments: modify changelog

* formatting

* review comments: improve descriptions

* remove deprecation warning

* more specific property name

* NEW: Add new public setting to enable shortcut key consumption

* review comments: improve comments

* add changelog entry

Author: jamesmcgill

Assets/Tests/InputSystem/CoreTests_Actions.cs

@@ -72,7 +72,7 @@ public void Actions_WhenShortcutsDisabled_AllConflictingActionsTrigger()
     [TestCase(false)]
     public void Actions_WhenShortcutsEnabled_CanConsumeInput(bool legacyComposites)
     {
-        InputSystem.settings.SetInternalFeatureFlag(InputFeatureNames.kDisableShortcutSupport, false);
+        InputSystem.settings.shortcutKeysConsumeInput = true;
 
         var keyboard = InputSystem.AddDevice<Keyboard>();
 
@@ -164,7 +164,7 @@ public void Actions_WhenShortcutsEnabled_CanConsumeInput(bool legacyComposites)
     [Category("Actions")]
     public void Actions_ShortcutSupportDisabledByDefault()
     {
-        Assert.That(InputSystem.settings.IsFeatureEnabled(InputFeatureNames.kDisableShortcutSupport), Is.True);
+        Assert.That(InputSystem.settings.shortcutKeysConsumeInput, Is.False);
 
         var keyboard = InputSystem.AddDevice<Keyboard>();
 
@@ -252,7 +252,7 @@ public void Actions_CanBindMultipleShortcutSequencesBasedOnSameModifiers()
     [TestCase("leftShift", "leftAlt", "space", false)]
     public void Actions_WhenShortcutsEnabled_PressingShortcutSequenceInWrongOrder_DoesNotTriggerShortcut(string modifier1, string modifier2, string binding, bool legacyComposites)
     {
-        InputSystem.settings.SetInternalFeatureFlag(InputFeatureNames.kDisableShortcutSupport, false);
+        InputSystem.settings.shortcutKeysConsumeInput = true;
 
         var keyboard = InputSystem.AddDevice<Keyboard>();
 
@@ -330,7 +330,7 @@ public void Actions_PressingShortcutSequenceInWrongOrder_DoesNotTriggerShortcut_
     [Category("Actions")]
     public void Actions_WhenShortcutsAreEnabled_CanHaveShortcutsWithButtonsUsingInitialStateChecks()
     {
-        InputSystem.settings.SetInternalFeatureFlag(InputFeatureNames.kDisableShortcutSupport, false);
+        InputSystem.settings.shortcutKeysConsumeInput = true;
 
         var keyboard = InputSystem.AddDevice<Keyboard>();
 
@@ -8229,7 +8229,7 @@ public void Actions_CompositesReportControlThatTriggeredTheCompositeInCallback()
     public void Actions_CompositesInDifferentMapsTiedToSameControlsWork()
     {
         // This test relies on the same single input getting picked up by two different composites.
-        InputSystem.settings.SetInternalFeatureFlag(InputFeatureNames.kDisableShortcutSupport, true);
+        InputSystem.settings.shortcutKeysConsumeInput = false;
 
         var keyboard = InputSystem.AddDevice<Keyboard>();
         var gamepad = InputSystem.AddDevice<Gamepad>();
@@ -10204,7 +10204,7 @@ public void Actions_WithMultipleCompositeBindings_WithoutEvaluateMagnitude_Works
     [TestCase(false)]
     public void Actions_ImprovedShortcutSupport_ConsumesWASD(bool shortcutsEnabled)
     {
-        InputSystem.settings.SetInternalFeatureFlag(InputFeatureNames.kDisableShortcutSupport, !shortcutsEnabled);
+        InputSystem.settings.shortcutKeysConsumeInput = shortcutsEnabled;
 
         var keyboard = InputSystem.AddDevice<Keyboard>();
 

Packages/com.unity.inputsystem/CHANGELOG.md

@@ -12,6 +12,7 @@ however, it has to be formatted properly to pass verification tests.
 
 ### Added
 - Added support for reading Tracking State in [TrackedPoseDriver](xref:UnityEngine.InputSystem.XR.TrackedPoseDriver) to constrain whether the input pose is applied to the Transform. This should be used when the device supports valid flags for the position and rotation values, which is the case for XR poses.
+- Added `InputSettings.shortcutKeysConsumeInput`. This allows programmatic access to opt-in to the enhanced shortcut key behaviour ([case ISXB-254](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-254))).
 
 ### Fixed
 - Fixed composite bindings incorrectly getting a control scheme assigned when pasting into input asset editor with a control scheme selected.

Packages/com.unity.inputsystem/InputSystem/Actions/Composites/ButtonWithOneModifier.cs

@@ -131,8 +131,7 @@ public override float EvaluateMagnitude(ref InputBindingCompositeContext context
         protected override void FinishSetup(ref InputBindingCompositeContext context)
         {
             if (!overrideModifiersNeedToBePressedFirst)
-                overrideModifiersNeedToBePressedFirst =
-                    InputSystem.settings.IsFeatureEnabled(InputFeatureNames.kDisableShortcutSupport);
+                overrideModifiersNeedToBePressedFirst = !InputSystem.settings.shortcutKeysConsumeInput;
         }
     }
 }

Packages/com.unity.inputsystem/InputSystem/Actions/Composites/ButtonWithTwoModifiers.cs

@@ -147,8 +147,7 @@ public override float EvaluateMagnitude(ref InputBindingCompositeContext context
         protected override void FinishSetup(ref InputBindingCompositeContext context)
         {
             if (!overrideModifiersNeedToBePressedFirst)
-                overrideModifiersNeedToBePressedFirst =
-                    InputSystem.settings.IsFeatureEnabled(InputFeatureNames.kDisableShortcutSupport);
+                overrideModifiersNeedToBePressedFirst = !InputSystem.settings.shortcutKeysConsumeInput;
         }
     }
 }

Packages/com.unity.inputsystem/InputSystem/Actions/Composites/OneModifierComposite.cs

@@ -145,8 +145,7 @@ protected override void FinishSetup(ref InputBindingCompositeContext context)
             DetermineValueTypeAndSize(ref context, binding, out m_ValueType, out m_ValueSizeInBytes, out m_BindingIsButton);
 
             if (!overrideModifiersNeedToBePressedFirst)
-                overrideModifiersNeedToBePressedFirst =
-                    InputSystem.settings.IsFeatureEnabled(InputFeatureNames.kDisableShortcutSupport);
+                overrideModifiersNeedToBePressedFirst = !InputSystem.settings.shortcutKeysConsumeInput;
         }
 
         public override object ReadValueAsObject(ref InputBindingCompositeContext context)

Packages/com.unity.inputsystem/InputSystem/Actions/Composites/TwoModifiersComposite.cs

@@ -158,8 +158,7 @@ protected override void FinishSetup(ref InputBindingCompositeContext context)
             OneModifierComposite.DetermineValueTypeAndSize(ref context, binding, out m_ValueType, out m_ValueSizeInBytes, out m_BindingIsButton);
 
             if (!overrideModifiersNeedToBePressedFirst)
-                overrideModifiersNeedToBePressedFirst =
-                    InputSystem.settings.IsFeatureEnabled(InputFeatureNames.kDisableShortcutSupport);
+                overrideModifiersNeedToBePressedFirst = !InputSystem.settings.shortcutKeysConsumeInput;
         }
 
         public override object ReadValueAsObject(ref InputBindingCompositeContext context)

Packages/com.unity.inputsystem/InputSystem/Actions/InputActionState.cs

@@ -140,7 +140,7 @@ private void ComputeControlGroupingIfNecessary()
 
             // If shortcut support is disabled, we simply put put all bindings at complexity=1 and
             // in their own group.
-            var disableControlGrouping = InputSystem.settings.IsFeatureEnabled(InputFeatureNames.kDisableShortcutSupport);
+            var disableControlGrouping = !InputSystem.settings.shortcutKeysConsumeInput;
 
             var currentGroup = 1u;
             for (var i = 0; i < totalControlCount; ++i)

Packages/com.unity.inputsystem/InputSystem/Editor/Settings/InputSettingsProvider.cs

@@ -152,9 +152,10 @@ public override void OnGUI(string searchContext)
                 if (m_ShortcutKeysConsumeInputs.boolValue)
                     EditorGUILayout.HelpBox("Please note that enabling Improved Shortcut Support will cause actions with composite bindings to consume input and block any other actions which are enabled and sharing the same controls. "
                         + "Input consumption is performed in priority order, with the action containing the greatest number of bindings checked first. "
-                        + "Therefore actions requiring less keypresses will not be triggered if an action using more keypresses is triggered and has overlapping controls. "
+                        + "Therefore actions requiring fewer keypresses will not be triggered if an action using more keypresses is triggered and has overlapping controls. "
                         + "This works for shortcut keys, however in other cases this might not give the desired result, especially where there are actions with the exact same number of composite controls, in which case it is non-deterministic which action will be triggered. "
                         + "These conflicts may occur even between actions which belong to different Action Maps e.g. if using an UIInputModule with the Arrow Keys bound to the Navigate Action in the UI Action Map, this would interfere with other Action Maps using those keys. "
+                        + "However conflicts would not occur between actions which belong to different Action Assets. "
                         + "Since event consumption only occurs for enabled actions, you can resolve unexpected issues by ensuring that only those Actions or Action Maps that are relevant to your game's current context are enabled. Enabling or disabling actions as your game or application moves between different contexts. "
                         , MessageType.None);
 

Packages/com.unity.inputsystem/InputSystem/InputFeatureNames.cs

@@ -4,7 +4,6 @@ internal static class InputFeatureNames
     {
         public const string kRunPlayerUpdatesInEditMode = "RUN_PLAYER_UPDATES_IN_EDIT_MODE";
         public const string kDisableUnityRemoteSupport = "DISABLE_UNITY_REMOTE_SUPPORT";
-        public const string kDisableShortcutSupport = "DISABLE_SHORTCUT_SUPPORT"; ////TODO: In v2, kill this.
         public const string kUseWindowsGamingInputBackend = "USE_WINDOWS_GAMING_INPUT_BACKEND";
     }
 }

Packages/com.unity.inputsystem/InputSystem/InputSettings.cs

@@ -660,6 +660,32 @@ public bool disableRedundantEventsMerging
             }
         }
 
+        /// <summary>
+        /// Improves shortcut key support by making composite controls consume control input
+        /// </summary>
+        /// <remarks>
+        /// Actions are exclusively triggered and will consume/block other actions sharing the same input.
+        /// E.g. when pressing the 'Shift+B' keys, the associated action would trigger but any action bound to just the 'B' key would be prevented from triggering at the same time.
+        /// Please note that enabling this will cause actions with composite bindings to consume input and block any other actions which are enabled and sharing the same controls.
+        /// Input consumption is performed in priority order, with the action containing the greatest number of bindings checked first.
+        /// Therefore actions requiring fewer keypresses will not be triggered if an action using more keypresses is triggered and has overlapping controls.
+        /// This works for shortcut keys, however in other cases this might not give the desired result, especially where there are actions with the exact same number of composite controls, in which case it is non-deterministic which action will be triggered.
+        /// These conflicts may occur even between actions which belong to different Action Maps e.g. if using an UIInputModule with the Arrow Keys bound to the Navigate Action in the UI Action Map, this would interfere with other Action Maps using those keys.
+        /// However conflicts would not occur between actions which belong to different Action Assets.
+        /// </remarks>
+        public bool shortcutKeysConsumeInput
+        {
+            get => m_ShortcutKeysConsumeInputs;
+            set
+            {
+                if (m_ShortcutKeysConsumeInputs == value)
+                    return;
+
+                m_ShortcutKeysConsumeInputs = value;
+                OnChange();
+            }
+        }
+
         /// <summary>
         /// Enable or disable an internal feature by its name.
         /// </summary>
@@ -675,16 +701,6 @@ public void SetInternalFeatureFlag(string featureName, bool enabled)
             if (string.IsNullOrEmpty(featureName))
                 throw new ArgumentNullException(nameof(featureName));
 
-            // REMOVE: this is a temporary crutch to disable shortcut support by default but while also preserving the
-            // existing flag name, as users are aware of that now.
-            if (featureName == InputFeatureNames.kDisableShortcutSupport)
-            {
-                if (m_ShortcutKeysConsumeInputs == !enabled) return;
-                m_ShortcutKeysConsumeInputs = !enabled;
-                OnChange();
-                return;
-            }
-
             if (m_FeatureFlags == null)
                 m_FeatureFlags = new HashSet<string>();
 
@@ -728,10 +744,6 @@ public void SetInternalFeatureFlag(string featureName, bool enabled)
 
         internal bool IsFeatureEnabled(string featureName)
         {
-            // REMOVE: this is a temporary crutch to disable shortcut support by default but while also preserving the
-            // existing flag name, as some users are aware of that now.
-            if (featureName == InputFeatureNames.kDisableShortcutSupport) return !m_ShortcutKeysConsumeInputs;
-
             return m_FeatureFlags != null && m_FeatureFlags.Contains(featureName.ToUpperInvariant());
         }