Skip to content
Draft
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
32 changes: 32 additions & 0 deletions Assets/Tests/InputSystem/CoreTests_Actions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9238,6 +9238,38 @@ 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", "<Keyboard>/a", groups: "Keyboard")
.With("Positive", "<Keyboard>/d", groups: "Keyboard");

Assert.That(action.GetBindingDisplayString(InputBinding.MaskByGroup("Keyboard")),
Is.EqualTo("A/D"));
Comment thread
Pauliusd01 marked this conversation as resolved.
}

// 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", "<Keyboard>/a", groups: "Keyboard")
.With("Positive", "<Mouse>/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"));
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

To address the missing coverage reported by Codecov and verify that composites are correctly excluded when no parts match the mask, consider adding a test case like this:

[Test]
[Category("Actions")]
public void Actions_WhenGettingDisplayTextForCompositeWithNoMatchingGroups_IsExcluded()
{
    var action = new InputAction();
    action.AddCompositeBinding("1DAxis")
        .With("Negative", "<Keyboard>/a", groups: "Keyboard")
        .With("Positive", "<Keyboard>/d", groups: "Keyboard");

    // Should return empty because the 'Gamepad' mask doesn't match any part of the composite
    Assert.That(action.GetBindingDisplayString(InputBinding.MaskByGroup("Gamepad")),
        Is.Empty);
}

🤖 Helpful? 👍/👎

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This makes sense to add.


// https://fogbugz.unity3d.com/f/cases/1321175/
[Test]
[Category("Actions")]
Expand Down
1 change: 1 addition & 0 deletions Packages/com.unity.inputsystem/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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].
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,25 @@ 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 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 anyPartMatches = false;
for (var partIndex = i + 1; partIndex < bindings.Count && bindings[partIndex].isPartOfComposite; ++partIndex)
{
if (bindingMask.Matches(bindings[partIndex]))
{
anyPartMatches = true;
break;
}
}
Comment thread
Pauliusd01 marked this conversation as resolved.
if (!anyPartMatches)
continue;
}

////REVIEW: should this filter out bindings that are not resolving to any controls?

Expand Down
Loading