Skip to content

Commit aa0960d

Browse files
authored
FIX: ArrayHelpers.Merge used the comparer incorrectly [ISXB-1790] (#2377)
1 parent 2d1a606 commit aa0960d

File tree

3 files changed

+45
-5
lines changed

3 files changed

+45
-5
lines changed

Assets/Tests/InputSystem/Utilities/ArrayHelperTests.cs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System;
12
using System.Globalization;
23
using NUnit.Framework;
34
using Unity.Collections;
@@ -118,6 +119,35 @@ public void Utilities_IndexOfReference__IsUsingReferenceEqualsAndConstrainedBySt
118119
Assert.AreEqual(2, arr.IndexOfReference(arr[2], 1, 3));
119120
}
120121

122+
[Test]
123+
[Category("Utilities")]
124+
public void Utilities_HaveDuplicateReferences_DetectsDuplicatesInFullRange()
125+
{
126+
var withDup = new object[] { new object(), new object(), new object() };
127+
withDup[2] = withDup[0]; // duplicate at 0 and 2
128+
Assert.That(withDup.HaveDuplicateReferences(0, 3), Is.True);
129+
130+
var noDup = new object[] { new object(), new object(), new object() };
131+
Assert.That(noDup.HaveDuplicateReferences(0, 3), Is.False);
132+
133+
// Regression test for ISXB-1792: inner loop was "n < count - i" so later pairs were never checked
134+
var dupAtEnd = new object[] { new object(), new object(), new object(), new object() };
135+
dupAtEnd[3] = dupAtEnd[2]; // duplicate at 2 and 3
136+
Assert.That(dupAtEnd.HaveDuplicateReferences(0, 4), Is.True);
137+
}
138+
139+
[Test]
140+
[Category("Utilities")]
141+
public void Utilities_MergeWithComparer_UsesComparerToDeduplicate()
142+
{
143+
// Regression test for ISXB-1790: Merge(IEqualityComparer) was calling comparer.Equals(secondValue)
144+
// instead of comparer.Equals(x, secondValue), so it compared against the comparer instance.
145+
var first = new[] { "a", "b" };
146+
var second = new[] { "A", "c" }; // "A" equals "a" with case-insensitive comparer
147+
var merged = ArrayHelpers.Merge(first, second, StringComparer.OrdinalIgnoreCase);
148+
Assert.That(merged, Is.EqualTo(new[] { "a", "b", "c" }));
149+
}
150+
121151
[Test]
122152
[Category("Utilities")]
123153
public void Utilities_IndexOfPredicate__IsUsingPredicateForEqualityAndConstraintedByStartIndexAndCount()

Packages/com.unity.inputsystem/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,13 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
1616
- Improved New Input System warning dialog, Native Device Inputs Not Enabled [UUM-132151].
1717
- Fixed caching for InputControlPath display name [ISX-2501](https://jira.unity3d.com/browse/ISX-2501)
1818
- Fixed editor closing and not saving the input asset when clicking cancel in the dialog prompt [UUM-134748](https://jira.unity3d.com/browse/UUM-134748)
19+
- Fixed an incorrect ArraysHelper.Merge implementation that used the comparer incorrectly [ISXB-1790] (https://jira.unity3d.com/browse/ISXB-1790)
1920
- Fixed the input actions editor window entering a corrupt state when restarting Unity after the asset was edited. [UUM-134737](https://jira.unity3d.com/browse/UUM-134737)
2021
- Fixed the Input Actions Editor Window toolbar buttons overlapping each other when the window size is too small [ISXB-1783] (https://jira.unity3d.com/browse/ISXB-1783)
2122
- Fixed `buttonSouth` returning the state of the east button (and so on for all the compass named buttons) when using a Nintendo Switch Pro Controller on iOS [ISXB-1632](issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1632)
2223
- Fixed `aButton` returning the state of the east button (and so on for all the letter named buttons) when using a Nintendo Switch Pro Controller on Standalone & iOS [ISXB-1632](issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1632)
2324
- Fixed an issue where `UIToolkit` `ClickEvent` could be fired on Android after device rotation due to inactive touch state being replayed during action initial state checks [UUM-100125](https://jira.unity3d.com/browse/UUM-100125).
25+
- Fixed an incorrect ArraysHelper.HaveDuplicateReferences implementation that didn't use its arguments right [ISXB-1792] (https://github.com/Unity-Technologies/InputSystem/pull/2376)
2426

2527
### Changed
2628

Packages/com.unity.inputsystem/InputSystem/Utilities/ArrayHelpers.cs

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -114,13 +114,12 @@ public static bool ContainsReference<TFirst, TSecond>(this TFirst[] array, int s
114114
return IndexOfReference(array, value, startIndex, count) != -1;
115115
}
116116

117-
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Usage", "CA1801:ReviewUnusedParameters", MessageId = "index", Justification = "Keep this for future implementation")]
118-
public static bool HaveDuplicateReferences<TFirst>(this TFirst[] first, int index, int count)
117+
public static bool HaveDuplicateReferences<TFirst>(this TFirst[] first, int index, int count) where TFirst : class
119118
{
120-
for (var i = 0; i < count; ++i)
119+
for (var i = index; i < index + count; ++i)
121120
{
122121
var element = first[i];
123-
for (var n = i + 1; n < count - i; ++n)
122+
for (var n = i + 1; n < index + count; ++n)
124123
{
125124
if (ReferenceEquals(element, first[n]))
126125
return true;
@@ -552,7 +551,16 @@ public static TValue[] Merge<TValue>(TValue[] first, TValue[] second, IEqualityC
552551
for (var i = 0; i < second.Length; ++i)
553552
{
554553
var secondValue = second[i];
555-
if (!merged.Exists(x => comparer.Equals(secondValue)))
554+
bool found = false;
555+
foreach (var x in merged)
556+
{
557+
if (comparer.Equals(x, secondValue))
558+
{
559+
found = true;
560+
break;
561+
}
562+
}
563+
if (!found)
556564
{
557565
merged.Add(secondValue);
558566
}

0 commit comments

Comments
 (0)