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

Added ability to only remove composite part bindings and not binding itself #1641

Open
wants to merge 1 commit 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
26 changes: 26 additions & 0 deletions Assets/Tests/InputSystem/CoreTests_Actions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3644,6 +3644,32 @@ public void Actions_CanAddBindingsToActions_ToExistingComposite()
.Property("path").EqualTo("<Keyboard>/rightArrow"));
}

[Test]
[Category("Actions")]
public void Actions_CanRemovePartBindings_ButNotExistingComposite()
{
var keyboard = InputSystem.AddDevice<Keyboard>();

var action = new InputAction();
action.AddCompositeBinding("Axis")
.With("Negative", "<Keyboard>/a")
.With("Positive", "<Keyboard>/d");
action.ChangeCompositeBinding("Axis").Erase(onlyRemoveCompositeParts: true);

Assert.That(action.bindings, Has.Count.EqualTo(1));
Assert.That(action.controls, Has.Count.EqualTo(0));

try
{
action.AddBinding("<Keyboard>/a");
action.ChangeBinding(1).Erase(onlyRemoveCompositeParts: true);
}
catch (InvalidOperationException ex)
{
Assert.That(ex.Message, Is.EqualTo("Binding should be a composite binding"));
}
}

[Serializable]
public enum Modification
{
Expand Down
4 changes: 4 additions & 0 deletions Packages/com.unity.inputsystem/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ however, it has to be formatted properly to pass verification tests.

- Fixed unclosed profiler marker in `InvokeCallbacksSafe_AnyCallbackReturnsTrue` which would lead to eventually broken profiler traces in some cases like using `PlayerInput` (case ISXB-393).

### Added

- Added `Erase(onlyRemoveCompositeParts: true)` to be able to remove only binding parts of the composite binding but not the composite binding itself. Based on the user contribution from [steinbitglis](https://github.com/steinbitglis) in [#1360](https://github.com/Unity-Technologies/InputSystem/pull/1360).

## [1.5.0] - 2023-01-24

### Added
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1564,19 +1564,26 @@ private BindingSyntax IteratePartBinding(bool next, string partName)
/// that got erased was the last one in the array).
/// </remarks>
/// <exception cref="InvalidOperationException">The instance is not <see cref="valid"/>.</exception>
public void Erase()
public void Erase(bool onlyRemoveCompositeParts = false)
{
if (!valid)
throw new InvalidOperationException("Instance not valid");

var isComposite = m_ActionMap.m_Bindings[m_BindingIndexInMap].isComposite;
ArrayHelpers.EraseAt(ref m_ActionMap.m_Bindings, m_BindingIndexInMap);
if (onlyRemoveCompositeParts && !isComposite)
throw new InvalidOperationException("Binding should be a composite binding");

var index = m_BindingIndexInMap;
if (!onlyRemoveCompositeParts)
ArrayHelpers.EraseAt(ref m_ActionMap.m_Bindings, index);
else
index++;
Comment on lines +1578 to +1580
Copy link
Collaborator

Choose a reason for hiding this comment

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

Optional: I would maybe a comment here with something like in "Skips composite bindings parent index" (line 1580)


// If it's a composite, also erase part bindings.
if (isComposite)
{
while (m_BindingIndexInMap < m_ActionMap.m_Bindings.LengthSafe() && m_ActionMap.m_Bindings[m_BindingIndexInMap].isPartOfComposite)
ArrayHelpers.EraseAt(ref m_ActionMap.m_Bindings, m_BindingIndexInMap);
while (index < m_ActionMap.m_Bindings.LengthSafe() && m_ActionMap.m_Bindings[index].isPartOfComposite)
ArrayHelpers.EraseAt(ref m_ActionMap.m_Bindings, index);
}

m_ActionMap.OnBindingModified();
Expand Down