From 2afdcdd397c77a494feada2c2b3d5aea3039d8e8 Mon Sep 17 00:00:00 2001 From: Anthony Yakovlev Date: Wed, 11 Mar 2026 20:50:34 +0300 Subject: [PATCH] Fix ISXB-1790: ArrayHelpers.Merge(IEqualityComparer) used wrong Equals arguments - comparer.Equals(secondValue) was comparing against comparer instance. - Use comparer.Equals(x, secondValue) to check if any merged element x equals secondValue. - Add Utilities_MergeWithComparer_UsesComparerToDeduplicate test. Made-with: Cursor --- .../InputSystem/Utilities/ArrayHelperTests.cs | 30 +++++++++++++++++++ .../InputSystem/Utilities/ArrayHelpers.cs | 4 +-- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/Assets/Tests/InputSystem/Utilities/ArrayHelperTests.cs b/Assets/Tests/InputSystem/Utilities/ArrayHelperTests.cs index e2096638f7..04be1e6115 100644 --- a/Assets/Tests/InputSystem/Utilities/ArrayHelperTests.cs +++ b/Assets/Tests/InputSystem/Utilities/ArrayHelperTests.cs @@ -1,3 +1,4 @@ +using System; using System.Globalization; using NUnit.Framework; using Unity.Collections; @@ -118,6 +119,35 @@ public void Utilities_IndexOfReference__IsUsingReferenceEqualsAndConstrainedBySt Assert.AreEqual(2, arr.IndexOfReference(arr[2], 1, 3)); } + [Test] + [Category("Utilities")] + public void Utilities_HaveDuplicateReferences_DetectsDuplicatesInFullRange() + { + var withDup = new object[] { new object(), new object(), new object() }; + withDup[2] = withDup[0]; // duplicate at 0 and 2 + Assert.That(withDup.HaveDuplicateReferences(0, 3), Is.True); + + var noDup = new object[] { new object(), new object(), new object() }; + Assert.That(noDup.HaveDuplicateReferences(0, 3), Is.False); + + // Regression test for ISXB-1792: inner loop was "n < count - i" so later pairs were never checked + var dupAtEnd = new object[] { new object(), new object(), new object(), new object() }; + dupAtEnd[3] = dupAtEnd[2]; // duplicate at 2 and 3 + Assert.That(dupAtEnd.HaveDuplicateReferences(0, 4), Is.True); + } + + [Test] + [Category("Utilities")] + public void Utilities_MergeWithComparer_UsesComparerToDeduplicate() + { + // Regression test for ISXB-1790: Merge(IEqualityComparer) was calling comparer.Equals(secondValue) + // instead of comparer.Equals(x, secondValue), so it compared against the comparer instance. + var first = new[] { "a", "b" }; + var second = new[] { "A", "c" }; // "A" equals "a" with case-insensitive comparer + var merged = ArrayHelpers.Merge(first, second, StringComparer.OrdinalIgnoreCase); + Assert.That(merged, Is.EqualTo(new[] { "a", "b", "c" })); + } + [Test] [Category("Utilities")] public void Utilities_IndexOfPredicate__IsUsingPredicateForEqualityAndConstraintedByStartIndexAndCount() diff --git a/Packages/com.unity.inputsystem/InputSystem/Utilities/ArrayHelpers.cs b/Packages/com.unity.inputsystem/InputSystem/Utilities/ArrayHelpers.cs index 54a4972495..193c6b8626 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Utilities/ArrayHelpers.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Utilities/ArrayHelpers.cs @@ -120,7 +120,7 @@ public static bool HaveDuplicateReferences(this TFirst[] first, int inde for (var i = 0; i < count; ++i) { var element = first[i]; - for (var n = i + 1; n < count - i; ++n) + for (var n = i + 1; n < count; ++n) { if (ReferenceEquals(element, first[n])) return true; @@ -552,7 +552,7 @@ public static TValue[] Merge(TValue[] first, TValue[] second, IEqualityC for (var i = 0; i < second.Length; ++i) { var secondValue = second[i]; - if (!merged.Exists(x => comparer.Equals(secondValue))) + if (!merged.Exists(x => comparer.Equals(x, secondValue))) { merged.Add(secondValue); }