TrackingDictionary.Remove(KeyValuePair) removes entries without notifying subscribers

Release Type: GitHub

Version: master, commit eb9c619e16e589c588f0dccc9ba062223ecf52cd

Platform(s): Reproduced with source-linked collection tests on macOS arm64 / .NET SDK 9.0.112. This is managed collection code; other platforms and the full engine build have not been tested locally.

Describe the bug

TrackingDictionary<TKey, TValue>.Remove(KeyValuePair<TKey, TValue>) successfully removes a matching entry without raising CollectionChanged. Removing the same entry through Remove(TKey) raises a removal notification before the entry is removed.

This means subscribers can miss removals performed through ICollection<KeyValuePair<TKey, TValue>>.

To Reproduce

var dictionary = new TrackingDictionary<string, int> { { "key", 42 } };
var notifications = 0;
dictionary.CollectionChanged += (_, _) => notifications++;

ICollection<KeyValuePair<string, int>> collection = dictionary;
var removed = collection.Remove(new KeyValuePair<string, int>("key", 42));

// Actual: removed == true, dictionary.Count == 0, notifications == 0.
// Expected: notifications == 1, with the same pre-removal timing as Remove(TKey).

Expected behavior

A matching pair raises one removal notification before mutation, preserving the existing keyed-removal contract. Missing keys and mismatched values return false, leave the dictionary intact, and raise no event.

Screenshots

Not applicable to this managed collection behavior.

Log and callstacks

With the new regression tests and the original implementation, the source-linked test run has 20 passing cases and one failure:

Remove_KeyValuePair_NotifiesBeforeRemovingMatchingEntry
Assert.Single() Failure: The collection was empty

The focused fix changes the matching-pair branch from innerDictionary.Remove(item.Key) to Remove(item.Key). All 21 dictionary test cases then pass, including six added regression cases. Serialization metadata types are stubbed in the standalone harness; collection/event code and the test file are linked directly from the checkout. This does not represent a full Stride build or editor test. Stride's current project targets .NET 10, while the installed local SDK used for this isolated reproduction is .NET 9.

Additional context

Related: #3171 and the closed #3209 by @marko1olo. The missing pair-removal notification was already identified in that earlier PR. This proposal isolates it while preserving the pre-mutation timing discussed by @Kryptos-FR. It does not attempt to resolve #3171's broader event-timing/API question.

@stride3d/stride-contributors Would you approve a US$20 bounty for this focused fix and regression coverage under the published contribution bounty program? A patch is prepared locally. If approved and reserved, I can submit it as a pull request and address review feedback. Please reply here; payment/contact details can be provided privately through the official payout process.

This patch and its tests were prepared with AI assistance.

Additional validation — actual Stride test project

The 21 TrackingDictionaryTests cases also pass in the actual Stride.Core.Tests.csproj on macOS arm64 with .NET SDK 10.0.400. The real Stride collections and serialization dependencies were compiled, and Stride's assembly processor ran for the test assembly. No serialization stubs are used in this run. The full engine test suite and editor have not been run.

The focused run used dotnet test sources/core/Stride.Core.Tests/Stride.Core.Tests.csproj --filter FullyQualifiedName~TrackingDictionaryTests with StridePlatforms=macOS, StrideTestRuntimeIdentifier=osx-arm64, and scratch-directory build/cache settings.

Start with the TrackingDictionaryTests cases in sources/core/Stride.Core.Tests/Stride.Core.Tests.csproj and run the focused dotnet test command described in the issue. Compare ICollection<KeyValuePair<TKey, TValue>> removal with Remove(TKey); done means matching pairs notify before mutation, while missing or mismatched pairs remain unchanged and silent, with all 21 tests passing.