## 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.
271 lines
7.9 KiB
C#
271 lines
7.9 KiB
C#
using System;
|
|
using System.Collections;
|
|
using System.Collections.Generic;
|
|
using System.IO;
|
|
using System.Runtime.InteropServices;
|
|
using System.Text.Json.Serialization;
|
|
using ModernUO.CodeGeneratedEvents;
|
|
using Server.Collections;
|
|
using Server.Json;
|
|
using Server.Mobiles;
|
|
|
|
namespace Server.Misc;
|
|
|
|
public static class AntiMacroSystem
|
|
{
|
|
// *** NOTE ***: Modifying these values will not change an already created antimacro.json file!
|
|
private static readonly bool[] _antiMacroSkillDefaults =
|
|
{
|
|
false, // Alchemy = 0,
|
|
true, // Anatomy = 1,
|
|
true, // AnimalLore = 2,
|
|
true, // ItemID = 3,
|
|
true, // ArmsLore = 4,
|
|
false, // Parry = 5,
|
|
true, // Begging = 6,
|
|
false, // Blacksmith = 7,
|
|
false, // Fletching = 8,
|
|
true, // Peacemaking = 9,
|
|
true, // Camping = 10,
|
|
false, // Carpentry = 11,
|
|
false, // Cartography = 12,
|
|
false, // Cooking = 13,
|
|
true, // DetectHidden = 14,
|
|
true, // Discordance = 15,
|
|
true, // EvalInt = 16,
|
|
true, // Healing = 17,
|
|
true, // Fishing = 18,
|
|
true, // Forensics = 19,
|
|
true, // Herding = 20,
|
|
true, // Hiding = 21,
|
|
true, // Provocation = 22,
|
|
false, // Inscribe = 23,
|
|
true, // Lockpicking = 24,
|
|
true, // Magery = 25,
|
|
true, // MagicResist = 26,
|
|
false, // Tactics = 27,
|
|
true, // Snooping = 28,
|
|
true, // Musicianship = 29,
|
|
true, // Poisoning = 30,
|
|
false, // Archery = 31,
|
|
true, // SpiritSpeak = 32,
|
|
true, // Stealing = 33,
|
|
false, // Tailoring = 34,
|
|
true, // AnimalTaming = 35,
|
|
true, // TasteID = 36,
|
|
false, // Tinkering = 37,
|
|
true, // Tracking = 38,
|
|
true, // Veterinary = 39,
|
|
false, // Swords = 40,
|
|
false, // Macing = 41,
|
|
false, // Fencing = 42,
|
|
false, // Wrestling = 43,
|
|
true, // Lumberjacking = 44,
|
|
true, // Mining = 45,
|
|
true, // Meditation = 46,
|
|
true, // Stealth = 47,
|
|
true, // RemoveTrap = 48,
|
|
true, // Necromancy = 49,
|
|
false, // Focus = 50,
|
|
true, // Chivalry = 51
|
|
true, // Bushido = 52
|
|
true, // Ninjitsu = 53
|
|
true, // Spellweaving
|
|
true, // Mysticism = 55
|
|
true, // Imbuing = 56
|
|
false, // Throwing = 57
|
|
};
|
|
|
|
private static Dictionary<Mobile, PlayerAntiMacro> _antiMacroTable;
|
|
private static Dictionary<Mobile, Timer> _logoutCleanup;
|
|
|
|
private const string _antiMacroPath = "Configuration/antimacro.json";
|
|
public static AntiMacroSettings Settings { get; private set; }
|
|
|
|
public static void Configure()
|
|
{
|
|
var path = Path.Combine(Core.BaseDirectory, _antiMacroPath);
|
|
|
|
if (File.Exists(path))
|
|
{
|
|
Settings = JsonConfig.Deserialize<AntiMacroSettings>(path);
|
|
}
|
|
else
|
|
{
|
|
Settings = new AntiMacroSettings
|
|
{
|
|
Enabled = false,
|
|
Allowance = 3,
|
|
LocationSize = 5,
|
|
Expire = TimeSpan.FromMinutes(5.0),
|
|
SkillsThatUseAntiMacro = new BitArray(_antiMacroSkillDefaults)
|
|
};
|
|
|
|
JsonConfig.Serialize(Path.Join(Core.BaseDirectory, _antiMacroPath), Settings);
|
|
}
|
|
}
|
|
|
|
public static void Initialize()
|
|
{
|
|
EventSink.WorldSave += OnWorldSave;
|
|
EventSink.Logout += OnLogout;
|
|
}
|
|
|
|
private static void OnWorldSave()
|
|
{
|
|
if (_antiMacroTable == null)
|
|
{
|
|
return;
|
|
}
|
|
|
|
var now = Core.Now;
|
|
|
|
foreach (var (m, antiMacro) in _antiMacroTable)
|
|
{
|
|
if (antiMacro._lastExpiration <= now)
|
|
{
|
|
_antiMacroTable.Remove(m);
|
|
}
|
|
else
|
|
{
|
|
antiMacro.CleanExpired();
|
|
}
|
|
}
|
|
}
|
|
|
|
[OnEvent(nameof(PlayerMobile.PlayerLoginEvent))]
|
|
public static void OnLogin(PlayerMobile pm)
|
|
{
|
|
// Stop the clear out timer
|
|
if (_logoutCleanup?.Remove(pm, out var timer) == true)
|
|
{
|
|
timer.Stop();
|
|
}
|
|
}
|
|
|
|
private static void OnLogout(Mobile m)
|
|
{
|
|
if (_antiMacroTable?.TryGetValue(m, out var antiMacro) != true)
|
|
{
|
|
return;
|
|
}
|
|
|
|
if (antiMacro._lastExpiration < Core.Now)
|
|
{
|
|
_antiMacroTable.Remove(m);
|
|
return;
|
|
}
|
|
|
|
_logoutCleanup ??= new Dictionary<Mobile, Timer>();
|
|
if (_logoutCleanup.TryGetValue(m, out var timer))
|
|
{
|
|
timer.Stop();
|
|
}
|
|
else
|
|
{
|
|
_logoutCleanup[m] = timer = Timer.DelayCall(Settings.Expire, CleanupPlayer, m);
|
|
}
|
|
|
|
timer.Start();
|
|
}
|
|
|
|
public static void CleanupPlayer(Mobile pm)
|
|
{
|
|
if (_antiMacroTable?.Remove(pm, out var antiMacro) == true)
|
|
{
|
|
// Hint to GC that we don't want this
|
|
antiMacro._antiMacroTracking.Clear();
|
|
antiMacro._antiMacroTracking = null;
|
|
}
|
|
|
|
if (_logoutCleanup?.Remove(pm, out var timer) == true)
|
|
{
|
|
timer.Stop();
|
|
}
|
|
}
|
|
|
|
public static bool UseAntiMacro(int skillId) =>
|
|
skillId >= 0 && skillId < Settings.SkillsThatUseAntiMacro.Length && Settings.SkillsThatUseAntiMacro[skillId];
|
|
|
|
public static bool AntiMacroCheck(PlayerMobile pm, Skill skill, object obj)
|
|
{
|
|
if (!Settings.Enabled || obj == null || pm.AccessLevel != AccessLevel.Player || !UseAntiMacro(skill.Info.SkillID))
|
|
{
|
|
return true;
|
|
}
|
|
|
|
_antiMacroTable ??= new Dictionary<Mobile, PlayerAntiMacro>();
|
|
|
|
// Hot path so use optimized code
|
|
ref var antiMacro = ref CollectionsMarshal.GetValueRefOrAddDefault(_antiMacroTable, pm, out var exists);
|
|
if (!exists)
|
|
{
|
|
antiMacro = new PlayerAntiMacro();
|
|
}
|
|
|
|
return antiMacro.AntiMacroCheck(skill, obj);
|
|
}
|
|
|
|
public record AntiMacroSettings
|
|
{
|
|
// How many times may we use the same location/target for gain
|
|
public int Allowance { get; init; }
|
|
|
|
// The size of each location, make this smaller so players dont have to move as far
|
|
public int LocationSize { get; init; }
|
|
|
|
public bool Enabled { get; init; }
|
|
|
|
// How long do we remember targets/locations?
|
|
public TimeSpan Expire { get; init; }
|
|
|
|
[JsonConverter(typeof(BitArrayEnumIndexConverter<SkillName>))]
|
|
public BitArray SkillsThatUseAntiMacro { get; init; }
|
|
}
|
|
|
|
private class PlayerAntiMacro
|
|
{
|
|
// This can get quite large. If a player is logged in for a while, this can be promoted to Gen 2 and
|
|
// become a memory leak.
|
|
public Dictionary<(Skill, object), CountAndTimeStamp> _antiMacroTracking = new();
|
|
public DateTime _lastExpiration;
|
|
|
|
public bool AntiMacroCheck(Skill skill, object obj)
|
|
{
|
|
var now = Core.Now;
|
|
|
|
// Potential hot path, so use optimized code
|
|
ref var _countTimeStamp =
|
|
ref CollectionsMarshal.GetValueRefOrAddDefault(_antiMacroTracking, (skill, obj), out var exists);
|
|
|
|
_countTimeStamp._count++;
|
|
|
|
if (!exists || _countTimeStamp._expiration <= now || _countTimeStamp._count < Settings.Allowance)
|
|
{
|
|
_countTimeStamp._expiration = _lastExpiration = now + Settings.Expire;
|
|
return true;
|
|
}
|
|
|
|
return false;
|
|
}
|
|
|
|
public void CleanExpired()
|
|
{
|
|
var now = Core.Now;
|
|
|
|
foreach (var (key, countAndTimeStamp) in _antiMacroTracking)
|
|
{
|
|
if (countAndTimeStamp._count <= 0 || countAndTimeStamp._expiration <= now)
|
|
{
|
|
_antiMacroTracking.Remove(key);
|
|
}
|
|
}
|
|
}
|
|
}
|
|
|
|
private struct CountAndTimeStamp
|
|
{
|
|
public int _count;
|
|
public DateTime _expiration;
|
|
}
|
|
}
|