From 0947b227a74d24b3aaf7a353a8dabf648222dffe Mon Sep 17 00:00:00 2001 From: metalgearsloth Date: Wed, 24 Jun 2026 23:41:47 +1000 Subject: [PATCH 1/2] Fix ValueList Peek --- .../Collections/ValueListTest.cs | 162 ++++++++++++++++++ Robust.Shared/Collections/ValueList.cs | 2 +- 2 files changed, 163 insertions(+), 1 deletion(-) create mode 100644 Robust.Shared.Tests/Collections/ValueListTest.cs diff --git a/Robust.Shared.Tests/Collections/ValueListTest.cs b/Robust.Shared.Tests/Collections/ValueListTest.cs new file mode 100644 index 000000000..e8f7024c0 --- /dev/null +++ b/Robust.Shared.Tests/Collections/ValueListTest.cs @@ -0,0 +1,162 @@ +using System.Collections.Generic; +using System.Reflection; +using NUnit.Framework; +using Robust.Shared.Collections; + +namespace Robust.Shared.Tests.Collections; + +[Parallelizable(ParallelScope.All | ParallelScope.Fixtures)] +[TestFixture, TestOf(typeof(ValueList<>))] +internal sealed class ValueListTest +{ + [Test] + public void PeekCorrectReference() + { + var list = new ValueList(2) + { + false, + true, + }; + + Assert.That(list.TryPeek(out var popped), Is.True); + Assert.That(popped, Is.True); + + list.TryPop(out _); + + Assert.That(list.TryPeek(out popped), Is.True); + Assert.That(popped, Is.False); + + list.TryPop(out _); + Assert.That(list.TryPeek(out popped), Is.False); + } + + [Test] + public void TryPopClearsRemovedReference() + { + var list = new ValueList(1); + var item = new object(); + list.Add(item); + + Assert.That(list.TryPop(out var popped), Is.True); + Assert.That(popped, Is.SameAs(item)); + + var itemsField = typeof(ValueList).GetField("_items", BindingFlags.NonPublic | BindingFlags.Instance); + var items = (object?[]?) itemsField!.GetValue(list); + + Assert.That(items, Is.Not.Null); + Assert.That(items![0], Is.Null); + } + + [Test] + public void IListMethodsMatchList() + { + IList expected = new List(); + IList actual = new ValueList(); + + AssertListEqual(expected, actual); + Assert.That(actual.IsReadOnly, Is.EqualTo(expected.IsReadOnly)); + + expected.Add(1); + actual.Add(1); + AssertListEqual(expected, actual); + + expected.Add(3); + actual.Add(3); + AssertListEqual(expected, actual); + + expected.Insert(1, 2); + actual.Insert(1, 2); + AssertListEqual(expected, actual); + + expected[2] = 4; + actual[2] = 4; + AssertListEqual(expected, actual); + + Assert.That(actual.IndexOf(2), Is.EqualTo(expected.IndexOf(2))); + Assert.That(actual.IndexOf(99), Is.EqualTo(expected.IndexOf(99))); + Assert.That(actual.Contains(4), Is.EqualTo(expected.Contains(4))); + Assert.That(actual.Contains(99), Is.EqualTo(expected.Contains(99))); + + var expectedCopy = new int[5]; + var actualCopy = new int[5]; + expected.CopyTo(expectedCopy, 1); + actual.CopyTo(actualCopy, 1); + Assert.That(actualCopy, Is.EqualTo(expectedCopy)); + + Assert.That(actual.Remove(2), Is.EqualTo(expected.Remove(2))); + AssertListEqual(expected, actual); + + expected.RemoveAt(1); + actual.RemoveAt(1); + AssertListEqual(expected, actual); + + expected.Clear(); + actual.Clear(); + AssertListEqual(expected, actual); + } + + [Test] + public void IListInsertGrowsDefaultList() + { + IList expected = new List(); + IList actual = new ValueList(); + + expected.Insert(0, 10); + actual.Insert(0, 10); + + AssertListEqual(expected, actual); + } + + [Test] + public void IListCopyToExceptionsMatchList() + { + IList expected = new List { 1, 2, 3 }; + IList actual = new ValueList(expected); + + AssertSameException(() => expected.CopyTo(null!, 0), () => actual.CopyTo(null!, 0)); + AssertSameException(() => expected.CopyTo(new int[3], -1), () => actual.CopyTo(new int[3], -1)); + AssertSameException(() => expected.CopyTo(new int[3], 1), () => actual.CopyTo(new int[3], 1)); + AssertSameException(() => expected.CopyTo(new int[2], 0), () => actual.CopyTo(new int[2], 0)); + } + + [Test] + public void IListIndexExceptionsMatchList() + { + IList expected = new List { 1, 2, 3 }; + IList actual = new ValueList(expected); + + AssertSameException(() => _ = expected[-1], () => _ = actual[-1]); + AssertSameException(() => _ = expected[3], () => _ = actual[3]); + AssertSameException(() => expected[-1] = 0, () => actual[-1] = 0); + AssertSameException(() => expected[3] = 0, () => actual[3] = 0); + } + + [Test] + public void IListMutationExceptionsMatchList() + { + IList expected = new List { 1, 2, 3 }; + IList actual = new ValueList(expected); + + AssertSameException(() => expected.Insert(-1, 0), () => actual.Insert(-1, 0)); + AssertSameException(() => expected.Insert(4, 0), () => actual.Insert(4, 0)); + AssertSameException(() => expected.RemoveAt(-1), () => actual.RemoveAt(-1)); + AssertSameException(() => expected.RemoveAt(3), () => actual.RemoveAt(3)); + } + + private static void AssertListEqual(IList expected, IList actual) + { + Assert.Multiple(() => + { + Assert.That(actual.Count, Is.EqualTo(expected.Count)); + Assert.That(actual, Is.EqualTo(expected)); + }); + } + + private static void AssertSameException(TestDelegate expected, TestDelegate actual) + { + var expectedException = Assert.Throws(Is.InstanceOf(), expected); + var actualException = Assert.Throws(Is.InstanceOf(), actual); + + Assert.That(actualException, Is.TypeOf(expectedException!.GetType())); + } +} diff --git a/Robust.Shared/Collections/ValueList.cs b/Robust.Shared/Collections/ValueList.cs index 4d45c1b4b..69ae7480d 100644 --- a/Robust.Shared/Collections/ValueList.cs +++ b/Robust.Shared/Collections/ValueList.cs @@ -690,7 +690,7 @@ public bool TryPeek([MaybeNullWhen(false)] out T value) return false; } - value = _items![Count]; + value = _items![Count - 1]; return true; } } From 07877ea32a16819a7cbd2dd083de64db0fc28b4e Mon Sep 17 00:00:00 2001 From: metalgearsloth Date: Thu, 25 Jun 2026 18:33:30 +1000 Subject: [PATCH 2/2] RN --- RELEASE-NOTES.md | 1 + 1 file changed, 1 insertion(+) diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md index cb28b9ac8..103ea0fe4 100644 --- a/RELEASE-NOTES.md +++ b/RELEASE-NOTES.md @@ -45,6 +45,7 @@ END TEMPLATE--> ### Bugfixes * Fix ValueList TryPop not clearing element references. +* Fix ValueList Peek always throwing by referencing the wrong index. ### Other