From a7bb8a8222a525efab86e55e143a1ff881e3d443 Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Sun, 21 Jun 2026 23:22:55 -0700 Subject: [PATCH] refactor(archery): centralize SE ammo auto-recovery off PlayerMobile (#2496) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Streamlines the SE-era archery ammo auto-recovery (recovering spent arrows/bolts after a miss). The mechanic was previously spread across four unrelated trigger points and a weapon-scoped timer that was divorced from the recovery state living on `PlayerMobile`. It also contained dead code. ### Problems fixed - **Dead `!Warmode` gate** — `OnMiss` only runs from `OnSwing`, which requires warmode to fire, so the `if (!pm.Warmode)` branch that started the recovery timer could never trigger. - **Scattered, divorced state** — banked ammo lived on `PlayerMobile.RecoverableAmmo` while the timer lived on the weapon (`_recoveryTimerToken`), and recovery was kicked off from four different places (`OnWarmodeChanged`, `PlayerMobile.OnDamage` kill, `BaseCreature.OnDamage` kill, `OnBeforeDeath`). - **Per-player footprint** — every `PlayerMobile` carried a `RecoverableAmmo` field even though ~99% never miss with a bow (and most are offline). ### New design — `AmmoRecovery` side table - All state (banked ammo + one repeating timer) is keyed by player in a static dictionary, so only players who actually miss carry any state. Transient by design — this was never serialized. - **One feed point:** `OnMiss` banks the spent ammo type and starts the player's timer. - **One drain point:** the timer self-gates and gathers ammo into the backpack only once the archer has disengaged — **alive, out of warmode, and not running** — otherwise it retries next tick, so banked ammo is never lost while online. - **"Not running" allows standing still _or_ walking.** The `Direction.Running` bit is stale after a player stops, so it's paired with movement recency (`LastMoveTime`); only an *actively* running archer is blocked. - Removed the redundant scattered triggers, the dead `!Warmode` branch, `RecoverableAmmo`, `RecoverAmmo()`, and the now-empty `OnWarmodeChanged` override. `PlayerMobile.OnDelete` calls `AmmoRecovery.Forget`. ### Behavior notes - On death, banked ammo is **no longer flushed to the corpse** — it stays banked and is recovered after resurrection once the archer settles (player keeps it rather than dropping it to looters). - `OnHit` immediate recovery (the ~40% arrow-to-defender behavior) is **unchanged**. ## Test plan - [x] `dotnet build Projects/UOContent/UOContent.csproj -c Release` — succeeds, 0 warnings, 0 errors. - [ ] In-game (SE era): miss bow shots, then disengage (drop warmode + stop) and confirm the "You recover N arrows/bolts" message and backpack contents; confirm recovery does **not** fire while running and **does** while walking/standing. --- .../Items/Weapons/Ranged/AmmoRecovery.cs | 152 ++++++++++++++++++ .../Items/Weapons/Ranged/BaseRanged.cs | 28 +--- Projects/UOContent/Mobiles/BaseCreature.cs | 11 +- Projects/UOContent/Mobiles/PlayerMobile.cs | 63 +------- 4 files changed, 158 insertions(+), 96 deletions(-) create mode 100644 Projects/UOContent/Items/Weapons/Ranged/AmmoRecovery.cs diff --git a/Projects/UOContent/Items/Weapons/Ranged/AmmoRecovery.cs b/Projects/UOContent/Items/Weapons/Ranged/AmmoRecovery.cs new file mode 100644 index 000000000..680ef9bdf --- /dev/null +++ b/Projects/UOContent/Items/Weapons/Ranged/AmmoRecovery.cs @@ -0,0 +1,152 @@ +using System; +using System.Collections.Generic; +using Server.Mobiles; + +namespace Server.Items +{ + /// + /// SE-era archery ammo recovery. When a player misses a shot there is a chance the spent ammo + /// can be gathered back up afterwards. The banked ammo is held off of + /// (in this side table) so the vast majority of players who never miss with a bow carry no extra + /// state. A single repeating timer per tracked player gathers the ammo, but only once the archer + /// has disengaged: out of warmode and not running (standing still or walking is fine). + /// + public static class AmmoRecovery + { + // How often we re-check whether the archer has settled enough to gather their ammo. + private static readonly TimeSpan RecoveryInterval = TimeSpan.FromSeconds(10); + + // A running step is only "current" if one landed this recently. Comfortably above the running + // step cadence (run-foot ~200ms, run-mount ~100ms) so an actively running archer always reads as + // running, while a stopped runner ages out within ~half a second. + private const long RunningWindowMillis = 500; + + // Only players who have banked recoverable ammo appear here, so this stays tiny. + private static readonly Dictionary _states = new(); + + private sealed class RecoveryState + { + public readonly Dictionary Ammo = new(); + public RecoveryTimer Timer; + } + + private sealed class RecoveryTimer : Timer + { + private readonly PlayerMobile _player; + + // delay == interval, count 0 => first fire after the interval, then repeats until Stop(). + public RecoveryTimer(PlayerMobile player) : base(RecoveryInterval, 0) => _player = player; + + protected override void OnTick() => OnRecoveryTick(_player); + } + + // Called from BaseRanged.OnMiss when a player misses and the shot is recoverable. + public static void Bank(PlayerMobile player, Type ammoType) + { + if (!Core.SE || ammoType == null) + { + return; + } + + if (!_states.TryGetValue(player, out var state)) + { + state = new RecoveryState(); + state.Timer = new RecoveryTimer(player); + state.Timer.Start(); + _states[player] = state; + } + + state.Ammo.TryGetValue(ammoType, out var count); + state.Ammo[ammoType] = count + 1; + } + + // Stops tracking a player and cancels their pending recovery (e.g. on delete). + public static void Forget(PlayerMobile player) + { + if (_states.Remove(player, out var state)) + { + state.Timer?.Stop(); + } + } + + private static void OnRecoveryTick(PlayerMobile player) + { + if (!_states.TryGetValue(player, out var state)) + { + return; + } + + if (player.Deleted || state.Ammo.Count == 0) + { + Forget(player); + return; + } + + // Gather the scattered ammo only once the archer has disengaged: out of warmode and not running. + if (!player.Alive || player.Warmode || IsRunning(player)) + { + return; // Not settled yet; try again on the next tick. + } + + Recover(player, state); + Forget(player); + } + + // The Direction.Running bit is left set after the player stops, so it alone can't tell us whether + // the archer is still running. We pair it with movement recency: a running step is only "current" + // if one landed within roughly two run-step intervals. A stopped runner ages out of that window, + // and a walker never sets the bit at all -- both are treated as "not running", which is what we want. + private static bool IsRunning(Mobile m) => + (m.Direction & Direction.Running) != 0 && + Core.TickCount - m.LastMoveTime < RunningWindowMillis; + + private static void Recover(PlayerMobile player, RecoveryState state) + { + foreach (var (type, amount) in state.Ammo) + { + if (amount <= 0) + { + continue; + } + + Item ammo = null; + + try + { + ammo = type.CreateInstance(); + } + catch + { + // ignored + } + + if (ammo == null) + { + continue; + } + + ammo.Amount = amount; + + var name = ammo.Name; + if (name == null) + { + var label = ammo.LabelNumber; + + // Arrow (1023903/1023904) and bolt (1027163/1027164) name clilocs keep their plural form + // two entries above the singular, so bump the label when recovering more than one. + if (ammo.Amount != 1 && label is 1023903 or 1023904 or 1027163 or 1027164) + { + label += 2; + } + + name = $"#{label}"; + } + + player.PlaceInBackpack(ammo); + player.SendLocalizedMessage(1073504, $"{ammo.Amount}\t{name}"); // You recover ~1_NUM~ ~2_AMMO~. + } + + state.Ammo.Clear(); + } + } +} diff --git a/Projects/UOContent/Items/Weapons/Ranged/BaseRanged.cs b/Projects/UOContent/Items/Weapons/Ranged/BaseRanged.cs index bd9264603..4fd786809 100644 --- a/Projects/UOContent/Items/Weapons/Ranged/BaseRanged.cs +++ b/Projects/UOContent/Items/Weapons/Ranged/BaseRanged.cs @@ -1,5 +1,4 @@ using System; -using System.Collections.Generic; using ModernUO.Serialization; using Server.Mobiles; using Server.Network; @@ -21,8 +20,6 @@ namespace Server.Items [SerializedCommandProperty(AccessLevel.GameMaster)] private int _velocity; - private TimerExecutionToken _recoveryTimerToken; - public BaseRanged(int itemID) : base(itemID) { } @@ -144,29 +141,8 @@ namespace Server.Items { if (attacker is PlayerMobile pm) { - var ammo = AmmoType; - - if (ammo != null) - { - pm.RecoverableAmmo ??= new Dictionary(); - pm.RecoverableAmmo.TryGetValue(ammo, out var result); - pm.RecoverableAmmo[ammo] = result + 1; - } - - if (!pm.Warmode) - { - if (!_recoveryTimerToken.Running) - { - Timer.StartTimer(TimeSpan.FromSeconds(10), - () => - { - _recoveryTimerToken.Cancel(); - pm.RecoverAmmo(); - }, - out _recoveryTimerToken - ); - } - } + // Bank the spent ammo; it is gathered back up once the archer disengages. + AmmoRecovery.Bank(pm, AmmoType); } } else diff --git a/Projects/UOContent/Mobiles/BaseCreature.cs b/Projects/UOContent/Mobiles/BaseCreature.cs index 86f8e4848..af85a195a 100644 --- a/Projects/UOContent/Mobiles/BaseCreature.cs +++ b/Projects/UOContent/Mobiles/BaseCreature.cs @@ -1623,16 +1623,9 @@ namespace Server.Mobiles ReceivedHonorContext?.OnTargetDamaged(from, amount); - if (!willKill) + if (!willKill && CanBeDistracted && ControlOrder == OrderType.Follow) { - if (CanBeDistracted && ControlOrder == OrderType.Follow) - { - CheckDistracted(from); - } - } - else if (from is PlayerMobile mobile) - { - Timer.StartTimer(TimeSpan.FromSeconds(10), mobile.RecoverAmmo); + CheckDistracted(from); } base.OnDamage(amount, from, willKill); diff --git a/Projects/UOContent/Mobiles/PlayerMobile.cs b/Projects/UOContent/Mobiles/PlayerMobile.cs index 1eb9f45c5..0a81bb846 100644 --- a/Projects/UOContent/Mobiles/PlayerMobile.cs +++ b/Projects/UOContent/Mobiles/PlayerMobile.cs @@ -536,8 +536,6 @@ namespace Server.Mobiles set => SetFlag(PlayerFlag.RefuseTrades, value); } - public Dictionary RecoverableAmmo { get; set; } - [CommandProperty(AccessLevel.GameMaster)] public DateTime AcceleratedStart { get; set; } @@ -2381,11 +2379,6 @@ namespace Server.Mobiles ReceivedHonorContext?.OnTargetDamaged(from, amount); SentHonorContext?.OnSourceDamaged(from, amount); - if (willKill && from is PlayerMobile mobile) - { - Timer.StartTimer(TimeSpan.FromSeconds(10), mobile.RecoverAmmo); - } - base.OnDamage(amount, from, willKill); } @@ -2406,14 +2399,6 @@ namespace Server.Mobiles } } - public override void OnWarmodeChanged() - { - if (!Warmode) - { - Timer.StartTimer(TimeSpan.FromSeconds(10), RecoverAmmo); - } - } - [MethodImpl(MethodImplOptions.AggressiveInlining)] private bool FindItems_Callback(Item item) => !item.Deleted && (item.LootType == LootType.Blessed || item.Insured) && Backpack != item.Parent; @@ -2465,8 +2450,6 @@ namespace Server.Mobiles ReceivedHonorContext?.OnTargetKilled(); SentHonorContext?.OnSourceKilled(); - RecoverAmmo(); - return base.OnBeforeDeath(); } @@ -3727,50 +3710,6 @@ namespace Server.Mobiles AutoStabled = null; } - public void RecoverAmmo() - { - if (!Core.SE || !Alive || RecoverableAmmo == null) - { - return; - } - - foreach (var kvp in RecoverableAmmo) - { - if (kvp.Value > 0) - { - Item ammo = null; - - try - { - ammo = kvp.Key.CreateInstance(); - } - catch - { - // ignored - } - - if (ammo == null) - { - continue; - } - - ammo.Amount = kvp.Value; - - var name = ammo.Name ?? ammo switch - { - Arrow _ => $"arrow{(ammo.Amount != 1 ? "s" : "")}", - Bolt _ => $"bolt{(ammo.Amount != 1 ? "s" : "")}", - _ => $"#{ammo.LabelNumber}" - }; - - PlaceInBackpack(ammo); - SendLocalizedMessage(1073504, $"{ammo.Amount}\t{name}"); // You recover ~1_NUM~ ~2_AMMO~. - } - } - - RecoverableAmmo.Clear(); - } - private static int GetInsuranceCost(Item item) => 600; private void ToggleItemInsurance() @@ -4172,6 +4111,8 @@ namespace Server.Mobiles ReceivedHonorContext?.Cancel(); SentHonorContext?.Cancel(); + AmmoRecovery.Forget(this); + if (Stabled != null) { foreach (var stabled in Stabled)