ModernUO/Projects/UOContent/Skills
Kamron Batman cce035f1c3
fix: Removes unnecessary dictionary removal guards (#2565)
## What

`Dictionary<K,V>.Remove` and `HashSet<T>.Remove` do not bump the collection's version, so removing an entry during a `foreach` does not invalidate the enumerator. A number of loops were still paying for a `PooledRefQueue`/`PooledRefList` to collect keys and drain them in a second pass. This drops those guards.

## Why it's safe

Verified against .NET 10.0.10 rather than taken on trust, since the documented guarantee covers only `Dictionary<TKey,TValue>.Remove` while several of these call sites are `HashSet<T>` or enumerate `.Keys`/`.Values`:

| Case | Result |
|---|---|
| `Dictionary` foreach + `Remove` | safe, all entries visited |
| `Dictionary.Keys` / `.Values` foreach + `Remove` | safe, all entries visited |
| `HashSet` foreach + `Remove` | safe, all entries visited |
| `Dictionary` foreach + `Remove` **then `Add`** | throws `InvalidOperationException` |

Reflection on `_version` confirms the mechanism: neither `Dictionary.Remove` nor `HashSet.Remove` touches it. Because `Remove` never bumps the version, the `Keys` and `Values` enumerators are just as safe as the dictionary's own, even though only `Dictionary.Remove` documents the behaviour. No entries were skipped in any case.

The `HashSet` half is confirmed by [stephentoub on dotnet/dotnet-api-docs#8177](https://github.com/dotnet/dotnet-api-docs/issues/8177#issuecomment-1167251052): *"Both HashSet and Dictionary have been improved to support removal during enumeration. The docs may just benefit from updating."* The gap is in the documentation, not the runtime.

`Remove` followed by `Add` in the same enumeration still throws. That is the line this PR does not cross.

## Guards removed

`VisibilityList`, `ChampionTitleSystem`, `Channel`, `BombingRun`, `Ruleset`, `PuzzleChest`, `RaceChangeGump`, `StepCache`, `PlayerMurderSystem`, `VirtueSystem`, `ProjectedItem`, `StaminaSystem`, `AIGroupMovement`, `PromotedGuard`, `AutoDenylist`, `LoginAllowlist`, `AntiMacroSystem`, `DetectHidden`.

Both collection kinds are covered: `Dictionary` (including loops over `.Keys` and `.Values`) and `HashSet` (`ProjectedItem._active`, `PlayerMurderSystem._contextTerms`, `StaminaSystem._resetHash`). In `StaminaSystem.ResetTimer` the `Count == queue.Count → Clear()` branch goes away with the queue — it only existed to avoid paying for N individual removes.

Where the collection supports it, `Contains` + `Remove` and `TryGetValue` + `Remove` also collapse into a single lookup (`if (list.Remove(x))`, `if (m_Pending.Remove(ns, out var state))`).

`Utility.Tidy<K,V>` keeps its two branches: when `K` is serializable the value is not inspected, otherwise the value is. Only the serializable side may be cast, so `Dictionary<Mobile, int>` and `Dictionary<Mobile, string>` stay valid.

## Deliberately unchanged

**`BaseCreature.LoyaltyTimer.OnTick`** keeps its deferred-delete queue. Removing from `World.Mobiles` while enumerating it is safe, but `Mobile.Delete()` is not a `Remove` — it runs `OnDelete`/`OnAfterDelete`, the `OnParentDeleted` cascade over the creature's pack, `DropHolding()`, and region and guild callbacks. Anything in that surface that constructs a `Mobile` is an `Add` into the dictionary being enumerated, which does invalidate it. `BaseHire.PayTimer.OnTick` has the same shape and is likewise untouched.

**Spatial-query buffers** — `GuardedRegion.CallGuards`, `Thunderstorm`, `Exorcism`, `LeverPuzzleController`, `BaseCreature.TeleportPets` — are a different hazard. They buffer the result of a range query because the drain moves or harms mobiles, which mutates sectors mid-enumeration.

**Re-entrant drains.** The `_users` sets in `Firebomb` and the explosion, conflagration and confusion-blast potions look like this pattern but are not: the loop collects, `Clear()`s, and only then runs `Target.Cancel` on each, which can re-enter. `AnimalTrainer` enumerates `pm.Stabled` and drains through `RemoveStabled`, which nulls the `Stabled` field once it empties — safe for an in-flight enumerator, which holds the set reference rather than the field, but subtle enough not to be worth inlining on a cold path.

## Verification

`dotnet build` clean with 0 warnings; 810 Server and 684 UOContent tests pass.
2026-08-08 11:50:01 -07:00
..
Tracking fix: Bumps deps. Updates copyrights (#2353) 2026-03-05 19:36:54 -08:00
Anatomy.cs fix: Fixes crafter deserialize, cleans up bandages, and codegens misc items (#1350) 2023-02-25 00:54:12 -08:00
AnimalLore.cs perf: Migrate NPC and Skill UI gumps to DynamicGump/StaticGump (#2414) 2026-04-25 19:51:42 -07:00
AnimalTaming.cs fix: Creature is given follow orders upon taming. (#2076) 2025-01-18 16:58:13 -08:00
AntiMacroSystem.cs fix: Removes unnecessary dictionary removal guards (#2565) 2026-08-08 11:50:01 -07:00
ArmsLore.cs fix: Fixes crafter deserialize, cleans up bandages, and codegens misc items (#1350) 2023-02-25 00:54:12 -08:00
Begging.cs chore: Use var everywhere (#2294) 2025-12-27 16:47:28 -08:00
Cartography.cs feat: Pre-Publish 14 Crafting (supersedes #2181, #2381) (#2476) 2026-06-07 20:27:22 -07:00
DetectHidden.cs fix: Removes unnecessary dictionary removal guards (#2565) 2026-08-08 11:50:01 -07:00
Discordance.cs fix: Adds name support for all mods (#1128) 2022-10-15 10:46:31 -07:00
EvalInt.cs fix: Fixes crafter deserialize, cleans up bandages, and codegens misc items (#1350) 2023-02-25 00:54:12 -08:00
ForensicEval.cs feat: Add zero-alloc interpolation handler to ValueStringBuilder, replace all StringBuilder usage (#2387) 2026-03-22 14:23:44 -07:00
Hiding.cs fix: Changes Map.Sector.Mobiles to link list & Fixes various related crash bugs (#1553) 2023-10-29 22:42:46 -07:00
Inscribe.cs feat: Pre-Publish 14 Crafting (supersedes #2181, #2381) (#2476) 2026-06-07 20:27:22 -07:00
ItemIdentification.cs chore: Use var everywhere (#2294) 2025-12-27 16:47:28 -08:00
Meditation.cs fix: Fixes buffs don't start/stop properly. Streamlines constructor and Add/Remove buffs. (#2082) 2025-01-24 18:20:23 -08:00
Peacemaking.cs fix: Removes the absurb 6 hour skill edge case and fixes target cancellations (#2212) 2025-06-07 13:58:01 -07:00
Poisoning.cs chore: Cleans up pattern checks. (#892) 2021-12-24 15:53:59 -08:00
Provocation.cs feat: Adjusts fame and karma system with era gates for OSI accuracy (#2389) 2026-04-25 11:19:57 -07:00
RemoveTrap.cs fix: Fixes crafter deserialize, cleans up bandages, and codegens misc items (#1350) 2023-02-25 00:54:12 -08:00
SkillCheck.cs fix: Fixes skill gain issue (#1558) 2023-10-21 10:03:50 -07:00
SkillsInfo.cs fix: Fixes random skill maximum (#1860) 2024-07-06 12:37:33 -07:00
Snooping.cs fix: Adds GetClients to map iterators (#1574) 2023-10-30 18:49:00 -07:00
SpiritSpeak.cs fix: Changes Map.Sector.Items to link list. (#1547) 2023-10-16 20:51:02 -07:00
Stealing.cs feat: Adjusts fame and karma system with era gates for OSI accuracy (#2389) 2026-04-25 11:19:57 -07:00
Stealth.cs fix: Updates races to properly support gargoyle equipment. (#744) 2021-08-29 00:43:55 -07:00
TasteID.cs Removes literal variables (#257) 2020-09-18 18:41:26 -07:00