ModernUO/Projects/UOContent/Engines/ML Quests/Gumps/RaceChangeGump.cs
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

347 lines
10 KiB
C#

using System;
using System.Buffers;
using System.Collections.Generic;
using ModernUO.Serialization;
using Server.Gumps;
using Server.Items;
using Server.Mobiles;
using Server.Network;
using Server.Spells.Fifth;
using Server.Spells.Ninjitsu;
using Server.Spells.Seventh;
namespace Server.Engines.MLQuests.Gumps
{
public interface IRaceChanger
{
bool CheckComplete(PlayerMobile from);
void ConsumeNeeded(PlayerMobile from);
void OnCancel(PlayerMobile from);
}
public class RaceChangeConfirmGump : DynamicGump
{
private static Dictionary<NetState, RaceChangeState> m_Pending;
private readonly IRaceChanger _owner;
private readonly Race _race;
public override bool Singleton => true;
private RaceChangeConfirmGump(IRaceChanger owner, Race targetRace) : base(50, 50)
{
_owner = owner;
_race = targetRace;
}
public static void DisplayTo(PlayerMobile from, IRaceChanger owner, Race targetRace)
{
if (from?.NetState == null || targetRace == null)
{
return;
}
from.SendGump(new RaceChangeConfirmGump(owner, targetRace));
}
protected override void BuildLayout(ref DynamicGumpBuilder builder)
{
builder.AddPage();
builder.AddBackground(0, 0, 240, 135, 0x2422);
if (_race == Race.Human)
{
builder.AddHtmlLocalized(15, 15, 210, 75, 1073643, 0); // Are you sure you wish to embrace your humanity?
}
else if (_race == Race.Elf)
{
builder.AddHtmlLocalized(15, 15, 210, 75, 1073642, 0); // Are you sure you want to follow the elven ways?
}
else
{
builder.AddHtml(15, 15, 210, 75, $"Are you sure you want to change your race to {_race.Name}?");
}
builder.AddButton(160, 95, 0xF7, 0xF8, 1);
builder.AddButton(90, 95, 0xF2, 0xF1, 0);
}
public override void OnResponse(NetState sender, in RelayInfo info)
{
var from = sender.Mobile;
switch (info.ButtonID)
{
case 0: // Cancel
{
_owner?.OnCancel((PlayerMobile)from);
break;
}
case 1: // Okay
{
if (_owner?.CheckComplete((PlayerMobile)from) != false)
{
Offer(_owner, (PlayerMobile)from, _race);
}
break;
}
}
}
public static unsafe void Initialize()
{
m_Pending = new Dictionary<NetState, RaceChangeState>();
IncomingExtendedCommandPackets.RegisterExtended(0x2A, true, &RaceChangeReply);
}
public static bool IsPending(NetState state) => state != null && m_Pending.ContainsKey(state);
private static void Offer(IRaceChanger owner, PlayerMobile from, Race targetRace)
{
var ns = from.NetState;
if (ns == null || !CanChange(from, targetRace))
{
return;
}
CloseCurrent(ns);
m_Pending[ns] = new RaceChangeState(owner, ns, targetRace);
ns.SendRaceChanger(from.Female, targetRace);
}
private static void CloseCurrent(NetState ns)
{
if (m_Pending.Remove(ns, out var state))
{
state._timeoutToken.Cancel();
}
ns.SendCloseRaceChanger();
}
private static void Timeout(NetState ns)
{
if (IsPending(ns))
{
m_Pending.Remove(ns);
ns.SendCloseRaceChanger();
}
}
public static bool IsWearingEquipment(Mobile from)
{
foreach (var item in from.Items)
{
switch (item.Layer)
{
case Layer.Hair:
case Layer.FacialHair:
case Layer.Backpack:
case Layer.Mount:
case Layer.Bank:
{
continue; // ignore
}
default:
{
return true;
}
}
}
return false;
}
private static bool CanChange(PlayerMobile from, Race targetRace)
{
if (from.Deleted)
{
return false;
}
if (from.Race == targetRace)
{
from.SendLocalizedMessage(1111918); // You are already that race.
}
else if (!MondainsLegacy.CheckML(from, false))
{
from.SendLocalizedMessage(1073651); // You must have Mondain's Legacy before proceeding...
}
else if (!from.Alive)
{
from.SendLocalizedMessage(1073646); // Only the living may proceed...
}
else if (from.Mounted)
{
from.SendLocalizedMessage(1073647); // You may not continue while mounted...
}
// TODO: Does this cover everything?
else if (!from.CanBeginAction<PolymorphSpell>() || DisguisePersistence.IsDisguised(from) ||
AnimalForm.UnderTransformation(from) || !from.CanBeginAction<IncognitoSpell>() ||
from.IsBodyMod)
{
from.SendLocalizedMessage(1073648); // You may only proceed while in your original state...
}
else if (from.Spell?.IsCasting == true)
{
from.SendLocalizedMessage(1073649); // One may not proceed while embracing magic...
}
else if (from.Poisoned)
{
from.SendLocalizedMessage(1073652); // You must be healthy to proceed...
}
else if (IsWearingEquipment(from))
{
from.SendLocalizedMessage(1073650); // To proceed you must be unburdened by equipment...
}
else
{
return true;
}
return false;
}
private static void RaceChangeReply(NetState state, SpanReader reader)
{
if (!m_Pending.TryGetValue(state, out var raceChangeState))
{
return;
}
CloseCurrent(state);
if (state.Mobile is not PlayerMobile pm)
{
return;
}
var owner = raceChangeState.m_Owner;
var targetRace = raceChangeState.m_TargetRace;
if (reader.Length == 5)
{
owner?.OnCancel(pm);
return;
}
if (!CanChange(pm, targetRace) || owner?.CheckComplete(pm) == false)
{
return;
}
int hue = reader.ReadUInt16();
int hairItemId = reader.ReadUInt16();
int hairHue = reader.ReadUInt16();
int facialHairItemId = reader.ReadUInt16();
int facialHairHue = reader.ReadUInt16();
pm.Race = targetRace;
pm.Hue = targetRace.ClipSkinHue(hue) | 0x8000;
if (targetRace.ValidateHair(pm, hairItemId))
{
pm.HairItemID = hairItemId;
pm.HairHue = targetRace.ClipHairHue(hairHue);
}
else
{
pm.HairItemID = 0;
}
if (targetRace.ValidateFacialHair(pm, facialHairItemId))
{
pm.FacialHairItemID = facialHairItemId;
pm.FacialHairHue = targetRace.ClipHairHue(facialHairHue);
}
else
{
pm.FacialHairItemID = 0;
}
if (targetRace == Race.Human)
{
pm.SendLocalizedMessage(1073654); // You are now fully human.
}
else if (targetRace == Race.Elf)
{
pm.SendLocalizedMessage(1073653); // You are now fully initiated into the Elven culture.
}
else
{
pm.SendMessage($"You have fully changed your race to {targetRace.Name}.");
}
owner?.ConsumeNeeded(pm);
}
private class RaceChangeState
{
private static readonly TimeSpan m_TimeoutDelay = TimeSpan.FromMinutes(1);
public readonly IRaceChanger m_Owner;
public readonly Race m_TargetRace;
public TimerExecutionToken _timeoutToken;
public RaceChangeState(IRaceChanger owner, NetState ns, Race targetRace)
{
m_Owner = owner;
m_TargetRace = targetRace;
Timer.StartTimer(m_TimeoutDelay, () => Timeout(ns), out _timeoutToken);
}
}
}
[SerializationGenerator(0, false)]
public partial class RaceChangeDeed : Item, IRaceChanger
{
[Constructible]
public RaceChangeDeed() : base(0x14F0) => LootType = LootType.Blessed;
public override string DefaultName => "a race change deed";
public bool CheckComplete(PlayerMobile pm)
{
if (Deleted)
{
return false;
}
if (!IsChildOf(pm.Backpack))
{
pm.SendLocalizedMessage(1042001); // That must be in your pack for you to use it.
return false;
}
return true;
}
public void ConsumeNeeded(PlayerMobile pm)
{
Consume();
}
public void OnCancel(PlayerMobile pm)
{
}
public override void OnDoubleClick(Mobile from)
{
if (from is not PlayerMobile pm)
{
return;
}
if (CheckComplete(pm))
{
RaceChangeConfirmGump.DisplayTo(pm, this, pm.Race == Race.Human ? Race.Elf : Race.Human);
}
}
}
}