From 476953846b02348db132651f14dfbb441544a9c6 Mon Sep 17 00:00:00 2001 From: Paulius Dervinis Date: Tue, 5 May 2026 15:15:09 +0300 Subject: [PATCH 1/2] FIX: GetBindingDisplayString returns empty for composites filtered by group-based binding mask [UUM-141423] Auto-generated by input-fix-pr-prep after input-fix-judge approved the hypothesis from input-fix-deep-dive (after triage Step 5b was rejected on pass 1). InputActionRebindingExtensions.GetBindingDisplayString(InputAction, InputBinding, InputBinding.DisplayStringOptions) returns empty when the binding mask filters by group, because the per-binding loop skips isPartOfComposite parts but then tests the mask against composite headers which carry no group metadata, so the whole composite drops out. Jira: https://issuetracker.unity3d.com/product/unity/issues/guid/UUM-141423 Judge rationale: file and method exist, the mechanism is in the method body (lines 321-323), the fall-through reuses the existing scan idiom at lines 444-448 of the same file with no signature/exception/serialization change, no nearby test asserts the buggy current behaviour, no native/threading/unsafe touches in the method or its direct callees at one level, and the previously-cited mixed-group composite edge case is now explicitly addressed via documented atomic-composite semantics grounded in the line-440-492 renderer's existing convention. Co-authored-by: Cursor --- Assets/Tests/InputSystem/CoreTests_Actions.cs | 15 ++++++++++++ Packages/com.unity.inputsystem/CHANGELOG.md | 1 + .../Actions/InputActionRebindingExtensions.cs | 23 ++++++++++++++++++- 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/Assets/Tests/InputSystem/CoreTests_Actions.cs b/Assets/Tests/InputSystem/CoreTests_Actions.cs index 34cfa9299d..2bfec7c4c7 100644 --- a/Assets/Tests/InputSystem/CoreTests_Actions.cs +++ b/Assets/Tests/InputSystem/CoreTests_Actions.cs @@ -9238,6 +9238,21 @@ public void Actions_WhenGettingDisplayTextForBindingsOnAction_CompositesAreForma Assert.That(action.GetBindingDisplayString(8), Is.EqualTo("Left Shift|Right Shift+A")); } + // https://issuetracker.unity3d.com/product/unity/issues/guid/UUM-141423 + [Test] + [Category("Actions")] + public void Actions_WhenGettingDisplayTextForBindingsOnAction_CompositeIsIncludedWhenAtLeastOnePartMatchesBindingMask() + { + var action = new InputAction(); + + action.AddCompositeBinding("1DAxis") + .With("Negative", "/a", groups: "Keyboard") + .With("Positive", "/d", groups: "Keyboard"); + + Assert.That(action.GetBindingDisplayString(InputBinding.MaskByGroup("Keyboard")), + Is.EqualTo("A/D")); + } + // https://fogbugz.unity3d.com/f/cases/1321175/ [Test] [Category("Actions")] diff --git a/Packages/com.unity.inputsystem/CHANGELOG.md b/Packages/com.unity.inputsystem/CHANGELOG.md index b242446471..1cf29e91de 100644 --- a/Packages/com.unity.inputsystem/CHANGELOG.md +++ b/Packages/com.unity.inputsystem/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Fixed +- Fixed `InputActionRebindingExtensions.GetBindingDisplayString(InputAction, InputBinding, ...)` returning an empty string for composite bindings when the binding mask filters by group [UUM-141423](https://issuetracker.unity3d.com/product/unity/issues/guid/UUM-141423) - Fixed a `NullReferenceException` thrown when removing all action maps [UUM-137116](https://jira.unity3d.com/browse/UUM-137116) - Simplified default setting messaging by consolidating repetitive messages into a single HelpBox. - Fixed a `NullPointerReferenceException` thrown in `InputManagerStateMonitors.FireStateChangeNotifications` logging by adding validation [UUM-136095]. diff --git a/Packages/com.unity.inputsystem/InputSystem/Runtime/Actions/InputActionRebindingExtensions.cs b/Packages/com.unity.inputsystem/InputSystem/Runtime/Actions/InputActionRebindingExtensions.cs index 26dc558ffe..07c94144db 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Runtime/Actions/InputActionRebindingExtensions.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Runtime/Actions/InputActionRebindingExtensions.cs @@ -321,7 +321,28 @@ public static string GetBindingDisplayString(this InputAction action, InputBindi if (bindings[i].isPartOfComposite) continue; if (!bindingMask.Matches(bindings[i])) - continue; + { + // Composites are filtered atomically: any matching part promotes the whole + // composite, consistent with how the integer-index renderer at lines 440-492 + // already treats composites as one display unit; per-part filtering would + // require a separate API. + if (!bindings[i].isComposite) + continue; + var lastPartIndex = i + 1; + while (lastPartIndex < bindings.Count && bindings[lastPartIndex].isPartOfComposite) + ++lastPartIndex; + var anyPartMatches = false; + for (var partIndex = i + 1; partIndex < lastPartIndex; ++partIndex) + { + if (bindingMask.Matches(bindings[partIndex])) + { + anyPartMatches = true; + break; + } + } + if (!anyPartMatches) + continue; + } ////REVIEW: should this filter out bindings that are not resolving to any controls? From e3a0cb77bbfd93ff860824c5c9bd4ef968c14517 Mon Sep 17 00:00:00 2001 From: Paulius Dervinis Date: Wed, 6 May 2026 13:30:30 +0300 Subject: [PATCH 2/2] Address u-pr review feedback for UUM-141423 - Collapse the composite-part scan into a single loop. The original two-pass scan computed lastPartIndex but did not use it for outer-loop advancement, so merging the bounds check into the for-loop condition eliminates the redundant first pass without changing behaviour. - Reword the inline comment to reference the index-based GetBindingDisplayString overload by name instead of by line number, which goes stale as the file is edited. - Add Actions_WhenGettingDisplayTextForBindingsOnAction_MixedGroupCompositeIsRenderedAtomicallyWhenAnyPartMatchesBindingMask covering a mixed-group 1DAxis composite (Keyboard + Mouse parts) under both MaskByGroup("Keyboard") and MaskByGroup("Mouse"). Both masks must render the whole composite ("A/Left Button"), exercising the atomic-promotion claim. Co-authored-by: Cursor --- Assets/Tests/InputSystem/CoreTests_Actions.cs | 17 +++++++++++++++++ .../Actions/InputActionRebindingExtensions.cs | 11 ++++------- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/Assets/Tests/InputSystem/CoreTests_Actions.cs b/Assets/Tests/InputSystem/CoreTests_Actions.cs index 2bfec7c4c7..8fe8dd6437 100644 --- a/Assets/Tests/InputSystem/CoreTests_Actions.cs +++ b/Assets/Tests/InputSystem/CoreTests_Actions.cs @@ -9253,6 +9253,23 @@ public void Actions_WhenGettingDisplayTextForBindingsOnAction_CompositeIsInclude Is.EqualTo("A/D")); } + // https://issuetracker.unity3d.com/product/unity/issues/guid/UUM-141423 + [Test] + [Category("Actions")] + public void Actions_WhenGettingDisplayTextForBindingsOnAction_MixedGroupCompositeIsRenderedAtomicallyWhenAnyPartMatchesBindingMask() + { + var action = new InputAction(); + + action.AddCompositeBinding("1DAxis") + .With("Negative", "/a", groups: "Keyboard") + .With("Positive", "/leftButton", groups: "Mouse"); + + Assert.That(action.GetBindingDisplayString(InputBinding.MaskByGroup("Keyboard")), + Is.EqualTo("A/Left Button")); + Assert.That(action.GetBindingDisplayString(InputBinding.MaskByGroup("Mouse")), + Is.EqualTo("A/Left Button")); + } + // https://fogbugz.unity3d.com/f/cases/1321175/ [Test] [Category("Actions")] diff --git a/Packages/com.unity.inputsystem/InputSystem/Runtime/Actions/InputActionRebindingExtensions.cs b/Packages/com.unity.inputsystem/InputSystem/Runtime/Actions/InputActionRebindingExtensions.cs index 07c94144db..c8ce4be9e3 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Runtime/Actions/InputActionRebindingExtensions.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Runtime/Actions/InputActionRebindingExtensions.cs @@ -323,16 +323,13 @@ public static string GetBindingDisplayString(this InputAction action, InputBindi if (!bindingMask.Matches(bindings[i])) { // Composites are filtered atomically: any matching part promotes the whole - // composite, consistent with how the integer-index renderer at lines 440-492 - // already treats composites as one display unit; per-part filtering would - // require a separate API. + // composite, consistent with how the index-based GetBindingDisplayString + // overload below treats composites as one display unit; per-part filtering + // would require a separate API. if (!bindings[i].isComposite) continue; - var lastPartIndex = i + 1; - while (lastPartIndex < bindings.Count && bindings[lastPartIndex].isPartOfComposite) - ++lastPartIndex; var anyPartMatches = false; - for (var partIndex = i + 1; partIndex < lastPartIndex; ++partIndex) + for (var partIndex = i + 1; partIndex < bindings.Count && bindings[partIndex].isPartOfComposite; ++partIndex) { if (bindingMask.Matches(bindings[partIndex])) {