From 8621b0f5c608694cd57045dd5fd0734720b03b7a Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Fri, 11 Sep 2026 17:52:35 -0700 Subject: [PATCH 01/13] chore: pin ModernUO to a52ce6ef7 (entry-ownership contract, #2621) The build is red on this commit by design; the port that follows makes it green. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr --- ModernUO | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ModernUO b/ModernUO index 4bad0cc..a52ce6e 160000 --- a/ModernUO +++ b/ModernUO @@ -1 +1 @@ -Subproject commit 4bad0cc9e6c2ac0d2ddf492adc667903f3ba9f71 +Subproject commit a52ce6ef705184117dbd008c42dcad577c99add4 From a3413ef4206c9611b73c441fe97d71569f1e78ae Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Fri, 11 Sep 2026 18:06:54 -0700 Subject: [PATCH 02/13] refactor(core): port ModernSpawner to the ModernUO entry-ownership contract Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr --- .../ModernSpawner/Core/ModernSpawner.Dto.cs | 60 +- Projects/ModernSpawner/Core/ModernSpawner.cs | 647 +++++++----------- .../ModernSpawner/Core/ModernSpawnerEntry.cs | 234 ++----- .../ModernSpawner/Gumps/ModernSpawnerGump.cs | 10 +- .../Gumps/SpawnerEntryWizardGump.cs | 2 +- .../Migration/XmlSpawnerMigrator.cs | 2 +- Projects/ModernSpawner/Perf/SpawnerMetrics.cs | 2 +- .../ModernSpawner/Perf/SpawnerPerfCommands.cs | 2 +- .../Positioning/DefaultPositioner.cs | 4 +- .../Scripting/Ast/InterSpawnerCommands.cs | 11 +- .../Serialization/ScriptYamlSerializer.cs | 2 +- .../Serialization/SpawnerJsonExporter.cs | 2 +- .../Serialization/SpawnerJsonImporter.cs | 6 +- .../Serialization/XmlSpawnerImporter.cs | 2 +- 14 files changed, 364 insertions(+), 622 deletions(-) diff --git a/Projects/ModernSpawner/Core/ModernSpawner.Dto.cs b/Projects/ModernSpawner/Core/ModernSpawner.Dto.cs index 2e13bd8..9e459f7 100644 --- a/Projects/ModernSpawner/Core/ModernSpawner.Dto.cs +++ b/Projects/ModernSpawner/Core/ModernSpawner.Dto.cs @@ -1,3 +1,5 @@ +using System; +using System.Collections.Generic; using System.Text.Json.Serialization; using Server.Engines.ModernSpawner.Scripting; using Server.Engines.Spawners; @@ -21,7 +23,7 @@ public override SpawnerDto ToDto() MaxDelay = MaxDelay, Team = Team, WalkingRange = DtoWalkingRange, - Entries = Entries, + Entries = _spawnEntries ?? [], SpawnLocationIsHome = SpawnLocationIsHome, SpawnPositionMode = DtoSpawnPositionMode, MaxSpawnAttempts = DtoMaxSpawnAttempts, @@ -35,8 +37,13 @@ public override SpawnerDto ToDto() ReturnToSpawnOnIdle = _returnToSpawnOnIdle, MaxZDelta = _maxZDelta, TriggerActivated = _triggerActivated, - SpawnArea = _spawnArea is { Width: > 0, Height: > 0 } ? _spawnArea : default, - Notes = _notes + Notes = _notes, + Triggers = _triggerDefinitions, + CycleMode = _cycleMode, + CurrentSubgroup = _currentSubgroup, + SequentialResetTime = _sequentialResetTime, + SequentialResetTo = _sequentialResetTo, + HoldSequence = _holdSequence }; } @@ -68,11 +75,12 @@ internal void ApplyModernDto(ModernSpawnerDto dto) _maxZDelta = dto.MaxZDelta; _triggerActivated = dto.TriggerActivated; _notes = dto.Notes; - - if (dto.SpawnArea != default) - { - _spawnArea = dto.SpawnArea; - } + _triggerDefinitions = dto.Triggers != null ? new List(dto.Triggers) : []; + _cycleMode = dto.CycleMode; + _currentSubgroup = dto.CurrentSubgroup; + _sequentialResetTime = dto.SequentialResetTime; + _sequentialResetTo = dto.SequentialResetTo; + _holdSequence = dto.HoldSequence; } } @@ -87,6 +95,14 @@ public sealed record ModernSpawnerDto : SpawnerDto [JsonPropertyOrder(8)] public Rectangle3D SpawnBounds { get; init; } + [JsonPropertyName("entries")] + [JsonPropertyOrder(10)] + [JsonIgnore(Condition = JsonIgnoreCondition.Never)] + public List Entries { get; init; } + + [JsonIgnore] + public override IReadOnlyList EntryView => Entries; + [JsonPropertyName("onActivateScript")] [JsonPropertyOrder(20)] public string OnActivateScript { get; init; } @@ -119,14 +135,34 @@ public sealed record ModernSpawnerDto : SpawnerDto [JsonPropertyOrder(27)] public bool TriggerActivated { get; init; } - [JsonPropertyName("spawnArea")] - [JsonPropertyOrder(28)] - public Rectangle3D SpawnArea { get; init; } - [JsonPropertyName("notes")] [JsonPropertyOrder(29)] public string Notes { get; init; } + [JsonPropertyName("triggers")] + [JsonPropertyOrder(30)] + public List Triggers { get; init; } + + [JsonPropertyName("cycleMode")] + [JsonPropertyOrder(31)] + public SpawnCycleMode CycleMode { get; init; } + + [JsonPropertyName("currentSubgroup")] + [JsonPropertyOrder(32)] + public int CurrentSubgroup { get; init; } + + [JsonPropertyName("sequentialResetTime")] + [JsonPropertyOrder(33)] + public TimeSpan SequentialResetTime { get; init; } + + [JsonPropertyName("sequentialResetTo")] + [JsonPropertyOrder(34)] + public int SequentialResetTo { get; init; } + + [JsonPropertyName("holdSequence")] + [JsonPropertyOrder(35)] + public bool HoldSequence { get; init; } + protected override BaseSpawner CreateEmpty() => new ModernSpawner(); public override BaseSpawner ToSpawner() diff --git a/Projects/ModernSpawner/Core/ModernSpawner.cs b/Projects/ModernSpawner/Core/ModernSpawner.cs index 38a7ab4..218b1da 100644 --- a/Projects/ModernSpawner/Core/ModernSpawner.cs +++ b/Projects/ModernSpawner/Core/ModernSpawner.cs @@ -1,6 +1,6 @@ using System; using System.Collections.Generic; -using System.Text.Json; +using System.Runtime.InteropServices; using ModernUO.Serialization; using Server.Engines.ModernSpawner.Perf; using Server.Engines.ModernSpawner.Positioning; @@ -8,19 +8,21 @@ using Server.Engines.ModernSpawner.Triggers; using Server.Engines.Spawners; using Server.Gumps; -using Server.Json; namespace Server.Engines.ModernSpawner; /// /// Modern spawner implementation with support for scripting, triggers, and advanced positioning. -/// Extends BaseSpawner with additional capabilities beyond the standard Spawner. +/// Owns a list of through ModernUO's entry-ownership contract, so +/// every base spawn path (Spawn, Defrag, Remove, RemoveAllEntries) runs over the modern entries. /// [SerializationGenerator(0)] -public partial class ModernSpawner : BaseSpawner +public partial class ModernSpawner : Spawner { - [SerializableField(0)] - private List _spawnEntries = []; + // Owned here so the base contract runs over ModernSpawnerEntry; null until the first entry. + [SerializedIgnoreDupe] + [SerializableField(0, getter: "private", setter: "private")] + private List _spawnEntries; /// /// Script serial for script executed when spawner becomes active. @@ -71,6 +73,7 @@ public partial class ModernSpawner : BaseSpawner /// List of trigger conditions that can activate this spawner. /// Stored as serialized trigger definitions. /// + [SerializedIgnoreDupe] [SerializableField(8)] private List _triggerDefinitions = []; @@ -88,25 +91,17 @@ public partial class ModernSpawner : BaseSpawner [SerializedCommandProperty(AccessLevel.Developer)] private bool _triggered; - /// - /// The spawn area - if set, spawns within this area instead of HomeRange from spawner. - /// Use a Rectangle3D with Width/Height > 0 to enable. Default (zero area) disables. - /// - [SerializableField(11)] - [SerializedCommandProperty(AccessLevel.Developer)] - private Rectangle3D _spawnArea; - /// /// Notes field for admin documentation. /// - [SerializableField(12)] + [SerializableField(11)] [SerializedCommandProperty(AccessLevel.Developer)] private string _notes; /// /// Selection strategy used each spawn cycle. See . /// - [SerializableField(13)] + [SerializableField(12)] [SerializedCommandProperty(AccessLevel.Developer)] private SpawnCycleMode _cycleMode = SpawnCycleMode.Random; @@ -114,7 +109,7 @@ public partial class ModernSpawner : BaseSpawner /// In mode, only entries with /// Subgroup == CurrentSubgroup are eligible this cycle. /// - [SerializableField(14)] + [SerializableField(13)] [SerializedCommandProperty(AccessLevel.Developer)] private int _currentSubgroup; @@ -124,14 +119,14 @@ public partial class ModernSpawner : BaseSpawner /// after this much real time has elapsed without an advance. /// disables auto-reset. /// - [SerializableField(15)] + [SerializableField(14)] [SerializedCommandProperty(AccessLevel.Developer)] private TimeSpan _sequentialResetTime; /// /// Subgroup that rewinds to. Defaults to 0. /// - [SerializableField(16)] + [SerializableField(15)] [SerializedCommandProperty(AccessLevel.Developer)] private int _sequentialResetTo; @@ -139,7 +134,7 @@ public partial class ModernSpawner : BaseSpawner /// When true, is a no-op. Lets scripts / triggers pin /// the spawner on a specific subgroup until explicitly released. /// - [SerializableField(17)] + [SerializableField(16)] [SerializedCommandProperty(AccessLevel.Developer)] private bool _holdSequence; @@ -154,57 +149,100 @@ public partial class ModernSpawner : BaseSpawner private bool _hasExtendedProximityTriggers; private Rectangle2D _extendedTriggerBounds; - // Track previous map for area movement unsubscription - private Map _previousMap; - - // Our own spawned entity tracking (maps to ModernSpawnerEntry, parallel to base Spawned) - private Dictionary _modernSpawned = new(); + /// Typed view of the entries; the base is the same list. + public IReadOnlyList ModernEntries => + _spawnEntries ?? (IReadOnlyList)Array.Empty(); + + /// + public override IReadOnlyList Entries => + _spawnEntries ?? (IReadOnlyList)Array.Empty(); + + /// + protected override ReadOnlySpan EntrySpan => + ReadOnlySpan.CastUp(CollectionsMarshal.AsSpan(_spawnEntries)); + + /// + protected override SpawnerEntry CreateEntry( + string name, + int probability, + int maxCount, + string properties, + string parameters + ) => new ModernSpawnerEntry(this, name, probability, maxCount, properties, parameters); + + /// + protected override void AddEntryCore(SpawnerEntry entry) + { + SpawnEntries ??= []; + AddToSpawnEntries((ModernSpawnerEntry)entry); + } - /// - /// Gets the modern spawn entries for this spawner. - /// - public IReadOnlyList ModernEntries => _spawnEntries; + /// + protected override bool RemoveEntryCore(SpawnerEntry entry) + { + if (entry is not ModernSpawnerEntry modern || _spawnEntries?.Contains(modern) != true) + { + return false; + } - /// - /// Gets the spawned entity to ModernSpawnerEntry mapping. - /// - public IReadOnlyDictionary ModernSpawned => _modernSpawned; + RemoveFromSpawnEntries(modern); + return true; + } - /// - /// Backs with . - /// - /// Important: do NOT synthesize bounds from when - /// is empty. is itself - /// derived from — anything that reads one and - /// falls back to the other introduces infinite recursion. The base class treats - /// as the source of truth; this override just - /// stores and returns it, matching the reference Spawner implementation. - /// - public override Rectangle3D SpawnBounds + /// + protected override void ClearEntriesCore() { - get => _spawnArea; - set + if (_spawnEntries?.Count > 0) { - _spawnArea = value; - InvalidateProperties(); - this.MarkDirty(); + ClearSpawnEntries(); } } - /// - /// Gets the region this spawner is in. - /// - public override Region Region => Region.Find(Location, Map); + /// + protected override void AdoptEntries(IReadOnlyList entries) + { + ClearEntriesCore(); + for (var i = 0; i < entries.Count; i++) + { + var source = entries[i]; + ModernSpawnerEntry entry; + if (source is ModernSpawnerEntry modern) + { + entry = modern; + } + else + { + // A stock entry (legacy save or stock DTO) becomes a modern one; keep its live spawns. + entry = (ModernSpawnerEntry)CloneEntry(source); + TransferSpawned(source, entry); + } - /// - /// Returns the bounds to use for a single spawn attempt. - /// - protected override Rectangle3D GetBoundsForSpawnAttempt() => SpawnBounds; + entry.SetParent(this); + AddEntryCore(entry); + } + } - /// - /// Returns all possible spawn bounds for cache operations. - /// - protected override ReadOnlySpan GetAllSpawnBounds() => new(ref _spawnArea); + /// + protected override SpawnerEntry CloneEntry(SpawnerEntry source) + { + var clone = (ModernSpawnerEntry)base.CloneEntry(source); + if (source is ModernSpawnerEntry modern) + { + clone.OnSpawnScript = modern.OnSpawnScript; + clone.OnDespawnScript = modern.OnDespawnScript; + clone.MinDelay = modern.MinDelay; + clone.MaxDelay = modern.MaxDelay; + clone.PositioningRule = modern.PositioningRule; + clone.SpawnGroup = modern.SpawnGroup; + clone.RequireLOS = modern.RequireLOS; + clone.SpawnAreaOffset = modern.SpawnAreaOffset; + clone.SpawnRange = modern.SpawnRange; + clone.LootTemplate = modern.LootTemplate; + clone.Subgroup = modern.Subgroup; + } + + return clone; + } /// /// Gets the compiled activate script, or null if not set. @@ -305,36 +343,7 @@ params ReadOnlySpan spawnedNames public override string DefaultName => "Modern Spawner"; - /// - /// Adds a spawn entry to this spawner. - /// Note: This hides the base class AddEntry with the 'new' keyword since BaseSpawner's - /// AddEntry is not virtual. For best results, work with ModernEntries directly. - /// - public new ModernSpawnerEntry AddEntry( - string creaturename, - int probability = 100, - int amount = 1, - bool dotimer = true, - string properties = null, - string parameters = null - ) - { - var entry = new ModernSpawnerEntry(this, creaturename, probability, amount, properties, parameters); - _spawnEntries.Add(entry); - this.MarkDirty(); - - if (dotimer) - { - DoTimer(TimeSpan.FromSeconds(1)); - } - - return entry; - } - - /// - /// Adds a ModernSpawnerEntry with extended configuration options. - /// Use TimeSpan.Zero for minDelay/maxDelay to use the spawner's default values. - /// + /// Adds an entry with the modern extras set. Zero delays mean "use the spawner's". public ModernSpawnerEntry AddModernEntry( string creatureName, int probability = 100, @@ -350,127 +359,61 @@ public ModernSpawnerEntry AddModernEntry( bool dotimer = true ) { - var entry = new ModernSpawnerEntry(this, creatureName, probability, maxCount, properties, parameters) - { - OnSpawnScript = onSpawnScript, - OnDespawnScript = onDespawnScript, - PositioningRule = positioningRule, - SpawnGroup = spawnGroup, - MinDelay = minDelay, - MaxDelay = maxDelay - }; - - _spawnEntries.Add(entry); - this.MarkDirty(); - - if (dotimer) - { - DoTimer(TimeSpan.FromSeconds(1)); - } - + var entry = (ModernSpawnerEntry)AddEntry(creatureName, probability, maxCount, dotimer, properties, parameters); + entry.OnSpawnScript = onSpawnScript; + entry.OnDespawnScript = onDespawnScript; + entry.PositioningRule = positioningRule; + entry.SpawnGroup = spawnGroup; + entry.MinDelay = minDelay; + entry.MaxDelay = maxDelay; return entry; } - /// - /// Counts the spawned entities for a specific modern entry. - /// - public int CountSpawns(ModernSpawnerEntry entry) - { - return entry?.Spawned?.Count ?? 0; - } - - /// - /// Removes a spawn entry from this spawner. - /// - public void RemoveModernEntry(ModernSpawnerEntry entry) - { - if (!_spawnEntries.Contains(entry)) - { - return; - } - - // Remove all spawned entities for this entry - for (var i = entry.Spawned.Count - 1; i >= 0; i--) - { - var spawned = entry.Spawned[i]; - entry.Spawned.RemoveAt(i); - _modernSpawned?.Remove(spawned); - spawned?.Delete(); - } - - _spawnEntries.Remove(entry); - this.MarkDirty(); - - if (Running && !IsFull) - { - DoTimer(); - } - - InvalidateProperties(); - } - - /// - /// Clears all entries from this spawner. - /// - public void ClearAllModernEntries() - { - for (var i = _spawnEntries.Count - 1; i >= 0; i--) - { - RemoveModernEntry(_spawnEntries[i]); - } - } - public override void Spawn() { using var _ = SpawnerMetrics.MeasureSpawn(); - // Execute pre-spawn script if configured var beforeScript = OnBeforeSpawnScript; if (beforeScript?.IsValid == true) { var context = new ScriptContext(null, this); ScriptEngine.Instance.Execute(beforeScript, context); - // Check if script cancelled the spawn + // Check if the script cancelled the spawn if (context.CancelSpawn) { return; } } - if (_spawnEntries.Count > 0) + using (SpawnerMetrics.MeasureDefrag()) { - using (SpawnerMetrics.MeasureDefrag()) - { - Defrag(); - } + Defrag(); + } - MaybeAutoResetSequence(); + if (_spawnEntries is not { Count: > 0 } || IsFull) + { + return; + } - using (SpawnerMetrics.MeasureEntrySelection()) + MaybeAutoResetSequence(); + + using (SpawnerMetrics.MeasureEntrySelection()) + { + switch (_cycleMode) { - switch (_cycleMode) - { - case SpawnCycleMode.Sequential: - SpawnSequentialMode(); - break; - case SpawnCycleMode.Group: - SpawnGroupMode(); - break; - default: - SpawnRandomMode(); - break; - } + case SpawnCycleMode.Sequential: + SpawnWeightedOne(_currentSubgroup); + break; + case SpawnCycleMode.Group: + SpawnGroupMode(); + break; + default: + SpawnWeightedOne(-1); + break; } } - else - { - // Fall back to BaseSpawner behaviour for spawners that were populated - // via the legacy AddEntry path. - base.Spawn(); - } - // Execute post-spawn script if configured var afterScript = OnAfterSpawnScript; if (afterScript?.IsValid == true) { @@ -480,48 +423,32 @@ public override void Spawn() } /// - /// Picks one eligible entry weighted by - /// and spawns one entity from it. Matches classic BaseSpawner semantics but operates on - /// . - /// - private void SpawnRandomMode() - { - SpawnWeightedOne(static _ => true); - } - - /// - /// Picks one eligible entry whose Subgroup equals , - /// weighted by probability. - /// - private void SpawnSequentialMode() - { - var currentSubgroup = _currentSubgroup; - SpawnWeightedOne(e => e.Subgroup == currentSubgroup); - } - - /// - /// Spawns one entity from every non-full entry this cycle. When all entries are at + /// Spawns one entity from every eligible entry this cycle. When all entries are at /// their max count, no further spawns happen until the pack is cleared. /// private void SpawnGroupMode() { - foreach (var entry in _spawnEntries) + var entries = _spawnEntries; + for (var i = 0; i < entries.Count; i++) { - if (!entry.IsFull) + var entry = entries[i]; + if (!entry.IsFull && !entry.Disabled) { - SpawnFromEntry(entry, out _); + SpawnEntry(entry); } } } - private void SpawnWeightedOne(Func eligible) + /// Weighted pick over eligible entries; -1 means any subgroup. + private void SpawnWeightedOne(int subgroup) { + var entries = _spawnEntries; var probsum = 0; - for (var i = 0; i < _spawnEntries.Count; i++) + for (var i = 0; i < entries.Count; i++) { - var entry = _spawnEntries[i]; - if (!entry.IsFull && eligible(entry)) + var entry = entries[i]; + if (IsEligible(entry, subgroup)) { probsum += entry.SpawnedProbability; } @@ -534,20 +461,17 @@ private void SpawnWeightedOne(Func eligible) var rand = Utility.RandomMinMax(1, probsum); - for (var i = 0; i < _spawnEntries.Count; i++) + for (var i = 0; i < entries.Count; i++) { - var entry = _spawnEntries[i]; - if (entry.IsFull || !eligible(entry)) + var entry = entries[i]; + if (!IsEligible(entry, subgroup)) { continue; } if (rand <= entry.SpawnedProbability) { - if (SpawnFromEntry(entry, out var flags)) - { - entry.Valid = flags; - } + SpawnEntry(entry); return; } @@ -555,6 +479,18 @@ private void SpawnWeightedOne(Func eligible) } } + private static bool IsEligible(ModernSpawnerEntry entry, int subgroup) => + !entry.IsFull && !entry.Disabled && (subgroup < 0 || entry.Subgroup == subgroup); + + /// Spawns one entity from and records the attempt's flags. + private void SpawnEntry(ModernSpawnerEntry entry) + { + using var _ = SpawnerMetrics.MeasureSpawnFromEntry(); + + Spawn(entry, out var flags); + entry.Valid = flags; + } + /// /// Sets for mode. /// No-op unless cycle mode is Sequential. @@ -628,109 +564,124 @@ private void MaybeAutoResetSequence() } } - /// - /// Spawns from a specific modern entry and executes entry-level scripts. - /// - public bool SpawnFromEntry(ModernSpawnerEntry entry, out EntryFlags flags) + /// + protected override void OnStarted() { - using var _ = SpawnerMetrics.MeasureSpawnFromEntry(); - - flags = EntryFlags.None; - - if (entry == null) + if (_triggerActivated && _triggerDefinitions.Count > 0) { - flags = EntryFlags.InvalidEntry; - return false; + TriggerSystem.Instance.ActivateTriggers(this); } - // Create a temporary SpawnerEntry to pass to base.Spawn - var tempEntry = new SpawnerEntry( - this, - entry.SpawnedName, - entry.SpawnedProbability, - entry.SpawnedMaxCount, - entry.Properties, - entry.Parameters - ); - - // Track spawn count before - var countBefore = entry.Spawned.Count; - - var result = base.Spawn(tempEntry, out flags); - - if (result) + var activateScript = OnActivateScript; + if (activateScript?.IsValid == true) { - SpawnerMetrics.RecordEntitySpawned(); - // Transfer spawned entity from temp entry to modern entry - foreach (var spawned in tempEntry.Spawned) - { - entry.AddToSpawned(spawned); - _modernSpawned[spawned] = entry; - } - - // Find the just-spawned entity (most recently added) - IEntity spawnedEntity = null; - if (entry.Spawned.Count > countBefore) - { - spawnedEntity = entry.Spawned[entry.Spawned.Count - 1]; - } - - // Apply loot template if configured - if (spawnedEntity is Mobile spawnedMobile && !string.IsNullOrEmpty(entry.LootTemplate)) - { - Loot.LootTemplateRegistry.ApplyTemplate(entry.LootTemplate, spawnedMobile); - } + ScriptEngine.Instance.Execute(activateScript, new ScriptContext(null, this)); + } + } - // Execute OnSpawn script if configured - if (spawnedEntity != null && !string.IsNullOrEmpty(entry.OnSpawnScript)) - { - var compiledScript = ScriptEngine.Instance.Compile(entry.OnSpawnScript); - if (compiledScript?.IsValid == true) - { - var context = new ScriptContext(spawnedEntity, this); - ScriptEngine.Instance.Execute(compiledScript, context); - } - } + /// + protected override void OnStopped() + { + var deactivateScript = OnDeactivateScript; + if (deactivateScript?.IsValid == true) + { + ScriptEngine.Instance.Execute(deactivateScript, new ScriptContext(null, this)); } - return result; + if (_triggerActivated) + { + TriggerSystem.Instance.DeactivateTriggers(this); + } } - public override Point3D GetSpawnPosition(ISpawnable spawned, Map map) + /// + protected override Point3D GetSpawnPosition(SpawnerEntry entry, ISpawnable spawned, Map map) { if (map == null || map == Map.Internal) { return Location; } - // Check for entry-specific positioning rule - if (_modernSpawned.TryGetValue(spawned, out var modernEntry)) + if (entry is ModernSpawnerEntry modern) { - if (!string.IsNullOrEmpty(modernEntry.PositioningRule)) + if (!string.IsNullOrEmpty(modern.PositioningRule)) { - // Use the positioning rules system - var posContext = new PositioningContext(this, spawned, map, modernEntry) + var posContext = new PositioningContext(this, spawned, map, modern) { MaxZDelta = _maxZDelta }; - var position = PositioningRules.GetPosition(modernEntry.PositioningRule, posContext); + var position = PositioningRules.GetPosition(modern.PositioningRule, posContext); if (position != Point3D.Zero) { return position; } } - // Use entry-specific spawn offset if set - if (modernEntry.SpawnAreaOffset != Point3D.Zero) + if (modern.SpawnAreaOffset != Point3D.Zero) { - var offset = modernEntry.SpawnAreaOffset; + var offset = modern.SpawnAreaOffset; return new Point3D(Location.X + offset.X, Location.Y + offset.Y, Location.Z + offset.Z); } } + return GetSpawnPosition(spawned, map); + } + + /// + protected override void OnSpawned(SpawnerEntry entry, ISpawnable spawned) + { + SpawnerMetrics.RecordEntitySpawned(); + + if (entry is not ModernSpawnerEntry modern) + { + return; + } + + if (spawned is Mobile spawnedMobile && !string.IsNullOrEmpty(modern.LootTemplate)) + { + Loot.LootTemplateRegistry.ApplyTemplate(modern.LootTemplate, spawnedMobile); + } + + if (!string.IsNullOrEmpty(modern.OnSpawnScript)) + { + var compiledScript = ScriptEngine.Instance.Compile(modern.OnSpawnScript); + if (compiledScript?.IsValid == true) + { + ScriptEngine.Instance.Execute(compiledScript, new ScriptContext(spawned, this)); + } + } + } + + /// + protected override void OnSpawnedDeath(SpawnerEntry entry, ISpawnable spawned, Mobile killer) + { + // Notify the trigger system for kill triggers + TriggerSystem.Instance.OnEntityKilled(this, spawned, killer); + + if (entry is ModernSpawnerEntry modern && !string.IsNullOrEmpty(modern.OnDespawnScript)) + { + var compiledScript = ScriptEngine.Instance.Compile(modern.OnDespawnScript); + if (compiledScript?.IsValid == true) + { + var context = new ScriptContext(spawned, this) + { + TriggeringMobile = killer + }; + ScriptEngine.Instance.Execute(compiledScript, context); + } + } + } + + public override Point3D GetSpawnPosition(ISpawnable spawned, Map map) + { + if (map == null || map == Map.Internal) + { + return Location; + } + // Check for spawn area definition (Width and Height > 0 means it's set) - if (_spawnArea is { Width: > 0, Height: > 0 }) + if (SpawnBounds is { Width: > 0, Height: > 0 }) { var pos = GetPositionInSpawnArea(map); if (pos != Point3D.Zero) @@ -751,7 +702,9 @@ public override Point3D GetSpawnPosition(ISpawnable spawned, Map map) private Point3D GetPositionInSpawnArea(Map map) { - if (_spawnArea.Width <= 0 || _spawnArea.Height <= 0) + var bounds = SpawnBounds; + + if (bounds.Width <= 0 || bounds.Height <= 0) { return Point3D.Zero; } @@ -759,12 +712,12 @@ private Point3D GetPositionInSpawnArea(Map map) // Try 10 times to find a valid location within the spawn area for (var i = 0; i < 10; i++) { - var x = Utility.RandomMinMax(_spawnArea.Start.X, _spawnArea.End.X - 1); - var y = Utility.RandomMinMax(_spawnArea.Start.Y, _spawnArea.End.Y - 1); + var x = Utility.RandomMinMax(bounds.Start.X, bounds.End.X - 1); + var y = Utility.RandomMinMax(bounds.Start.Y, bounds.End.Y - 1); var z = map.GetAverageZ(x, y); // If Rectangle3D has Z constraints, respect them - if (_spawnArea.Depth > 0 && (z < _spawnArea.Start.Z || z >= _spawnArea.End.Z)) + if (bounds.Depth > 0 && (z < bounds.Start.Z || z >= bounds.End.Z)) { continue; } @@ -1003,24 +956,8 @@ public void OnTriggerDeactivated(ITrigger trigger) [AfterDeserialization] private void AfterDeserializationModernSpawner() { - // Re-parent all entries after deserialization - foreach (var entry in _spawnEntries) - { - entry.SetParent(this); - } - - // Rebuild the modern spawned dictionary from entries - _modernSpawned = new Dictionary(); - foreach (var entry in _spawnEntries) - { - foreach (var spawned in entry.Spawned) - { - _modernSpawned[spawned] = entry; - } - } - - // Initialize map tracking - _previousMap = Map; + // Spawner's rebuild ran before _spawnEntries was read; rebuild over the modern list. + RebuildSpawned(); // Activate triggers if spawner is running if (Running && _triggerActivated && _triggerDefinitions.Count > 0) @@ -1040,64 +977,6 @@ private void AfterWorldLoad() // TODO: Implement extended proximity trigger support when Map APIs are available } - /// - /// Called when the spawner starts running. Activates triggers. - /// - public new void Start() - { - base.Start(); - OnSpawnerStarted(); - } - - /// - /// Called when the spawner stops running. Deactivates triggers. - /// - public new void Stop() - { - OnSpawnerStopping(); - base.Stop(); - } - - /// - /// Hook called after the spawner has started. - /// - private void OnSpawnerStarted() - { - // Activate triggers when spawner starts - if (_triggerActivated && _triggerDefinitions.Count > 0) - { - TriggerSystem.Instance.ActivateTriggers(this); - } - - // Execute activate script - var activateScript = OnActivateScript; - if (activateScript?.IsValid == true) - { - var context = new ScriptContext(null, this); - ScriptEngine.Instance.Execute(activateScript, context); - } - } - - /// - /// Hook called before the spawner stops. - /// - private void OnSpawnerStopping() - { - // Execute deactivate script - var deactivateScript = OnDeactivateScript; - if (deactivateScript?.IsValid == true) - { - var context = new ScriptContext(null, this); - ScriptEngine.Instance.Execute(deactivateScript, context); - } - - // Deactivate triggers when spawner stops - if (_triggerActivated) - { - TriggerSystem.Instance.DeactivateTriggers(this); - } - } - /// /// Called when this spawner is deleted. /// @@ -1120,7 +999,6 @@ public override void OnDelete() /// public override void OnMapChange() { - _previousMap = Map; base.OnMapChange(); // Extended area movement subscription is not yet supported in ModernUO } @@ -1134,33 +1012,6 @@ public override void OnLocationChange(Point3D oldLocation) // Extended area movement subscription is not yet supported in ModernUO } - /// - /// Called when a spawned entity is killed. Override to handle death events. - /// This should be called from the spawned mobile's OnDeath handler. - /// - public void OnSpawnedEntityKilled(IEntity killed, Mobile killer) - { - // Notify the trigger system for kill triggers - TriggerSystem.Instance.OnEntityKilled(this, killed, killer); - - // Execute entry-level OnDespawn script if configured - if (killed is ISpawnable spawnable && _modernSpawned.TryGetValue(spawnable, out var modernEntry)) - { - if (!string.IsNullOrEmpty(modernEntry.OnDespawnScript)) - { - var compiledScript = ScriptEngine.Instance.Compile(modernEntry.OnDespawnScript); - if (compiledScript?.IsValid == true) - { - var context = new ScriptContext(killed, this) - { - TriggeringMobile = killer - }; - ScriptEngine.Instance.Execute(compiledScript, context); - } - } - } - } - public override void OnDoubleClick(Mobile from) { if (from.AccessLevel >= AccessLevel.Developer) diff --git a/Projects/ModernSpawner/Core/ModernSpawnerEntry.cs b/Projects/ModernSpawner/Core/ModernSpawnerEntry.cs index 7cb7cc0..d59df77 100644 --- a/Projects/ModernSpawner/Core/ModernSpawnerEntry.cs +++ b/Projects/ModernSpawner/Core/ModernSpawnerEntry.cs @@ -1,5 +1,4 @@ using System; -using System.Collections.Generic; using System.Text.Json.Serialization; using ModernUO.Serialization; using Server.Engines.Spawners; @@ -8,131 +7,77 @@ namespace Server.Engines.ModernSpawner; /// -/// Entry for ModernSpawner with support for scripting, triggers, and advanced positioning. +/// Spawner entry with scripting, per-entry timing, positioning rule, loot template and subgroup. +/// The six stock fields (name, probability, max count, properties, parameters, spawned) and the +/// Disabled flag come from . /// -[SerializationGenerator(0, false)] -public partial class ModernSpawnerEntry +[SerializationGenerator(0)] +public partial class ModernSpawnerEntry : SpawnerEntry { + // The generator resolves dirty tracking on the declared type only (SerializationGenerator #58). [DirtyTrackingEntity] - private ModernSpawner _parent; + private BaseSpawner Owner => Parent; + /// Script executed on the spawned entity right after it is placed. [SerializableField(0)] - [SerializedJsonPropertyName("name")] - private string _spawnedName; - - [SerializableField(1)] - [SerializedJsonPropertyName("probability")] - private int _spawnedProbability = 100; - - [SerializableField(2)] - [SerializedJsonPropertyName("maxCount")] - private int _spawnedMaxCount = 1; - - [SerializableField(3)] - [SerializedJsonPropertyName("properties")] - private string _properties; - - [SerializableField(4)] - [SerializedJsonPropertyName("parameters")] - private string _parameters; - - [Tidy] - [SerializedJsonIgnore] - [SerializableField(5)] - private List _spawned; - - // ModernSpawner-specific fields - - /// - /// Script to execute when this entry spawns a creature/item. - /// Uses AST-compiled expressions for performance. - /// - [SerializableField(6)] [SerializedJsonPropertyName("onSpawnScript")] private string _onSpawnScript; - /// - /// Script to execute when a spawned creature/item is despawned. - /// - [SerializableField(7)] + /// Script executed when a spawned creature dies. + [SerializableField(1)] [SerializedJsonPropertyName("onDespawnScript")] private string _onDespawnScript; - /// - /// Minimum delay override for this specific entry. - /// If <= TimeSpan.Zero, uses the parent spawner's MinDelay. - /// - [SerializableField(8)] + /// Per-entry minimum delay; means use the spawner's. + [SerializableField(2)] [SerializedJsonPropertyName("minDelay")] private TimeSpan _minDelay; - /// - /// Maximum delay override for this specific entry. - /// If <= TimeSpan.Zero, uses the parent spawner's MaxDelay. - /// - [SerializableField(9)] + /// Per-entry maximum delay; means use the spawner's. + [SerializableField(3)] [SerializedJsonPropertyName("maxDelay")] private TimeSpan _maxDelay; - /// - /// Positioning rule name for spawn location calculation. - /// - [SerializableField(10)] + /// Positioning rule name (see PositioningRules); null uses the spawner's positioning. + [SerializableField(4)] [SerializedJsonPropertyName("positioningRule")] private string _positioningRule; - /// - /// Group identifier for grouped spawning behavior. - /// Entries with the same group spawn/despawn together. - /// - [SerializableField(11)] + /// Organisational group tag. + [SerializableField(5)] [SerializedJsonPropertyName("spawnGroup")] private string _spawnGroup; - /// - /// Whether this entry requires line of sight to spawn location. - /// - [SerializableField(12)] + /// Whether this entry requires line of sight to the spawn location. + [SerializableField(6)] [SerializedJsonPropertyName("requireLOS")] private bool _requireLOS; - /// - /// Custom spawn area offset from spawner location. - /// - [SerializableField(13)] + /// Offset from the spawner location used when no positioning rule applies. + [SerializableField(7)] [SerializedJsonPropertyName("spawnAreaOffset")] private Point3D _spawnAreaOffset; - /// - /// Custom spawn range override for this entry. - /// If -1, uses the parent spawner's HomeRange. - /// - [SerializableField(14)] + /// Per-entry spawn range; -1 uses the spawner's HomeRange. + [SerializableField(8)] [SerializedJsonPropertyName("spawnRange")] private int _spawnRange = -1; - /// - /// Name of the loot template to apply to spawned creatures. - /// If null or empty, uses default creature loot. - /// - [SerializableField(15)] + /// Loot template name applied on spawn; null or empty keeps default creature loot. + [SerializableField(9)] [SerializedJsonPropertyName("lootTemplate")] private string _lootTemplate; /// - /// Subgroup identifier. In mode this selects - /// which entries are eligible in the current phase. In other modes it's an - /// organisational tag used by triggers and inter-spawner commands - /// (e.g. SPAWN/2, DESPAWN/0, GOTO/3). + /// Subgroup identifier. In mode this selects which entries are + /// eligible in the current phase; otherwise it is an organisational tag used by inter-spawner commands. /// - [SerializableField(16)] + [SerializableField(10)] [SerializedJsonPropertyName("subgroup")] private int _subgroup; - public ModernSpawnerEntry(ModernSpawner parent) + public ModernSpawnerEntry(BaseSpawner parent) : base(parent) { - _parent = parent; - _spawned = []; } [JsonConstructor] @@ -142,129 +87,32 @@ public ModernSpawnerEntry( int spawnedMaxCount = 1, string properties = null, string parameters = null - ) : this(null, spawnedName, spawnedProbability, spawnedMaxCount, properties, parameters) + ) : base(spawnedName, spawnedProbability, spawnedMaxCount, properties, parameters) { } public ModernSpawnerEntry( - ModernSpawner parent, + BaseSpawner parent, string name, int probability = 100, int maxCount = 1, string properties = null, string parameters = null - ) : this(parent) + ) : base(parent, name, probability, maxCount, properties, parameters) { - SpawnedName = name; - SpawnedProbability = probability; - SpawnedMaxCount = maxCount; - Properties = properties; - Parameters = parameters; } + /// Effective minimum delay: the entry's override, else the spawner's. [JsonIgnore] - public EntryFlags Valid { get; set; } - - [JsonIgnore] - public bool IsFull => Spawned.Count >= SpawnedMaxCount; - - /// - /// Gets the effective minimum delay for this entry. - /// - [JsonIgnore] - public TimeSpan EffectiveMinDelay => _minDelay > TimeSpan.Zero ? _minDelay : _parent?.MinDelay ?? TimeSpan.FromMinutes(5); - - /// - /// Gets the effective maximum delay for this entry. - /// - [JsonIgnore] - public TimeSpan EffectiveMaxDelay => _maxDelay > TimeSpan.Zero ? _maxDelay : _parent?.MaxDelay ?? TimeSpan.FromMinutes(10); + public TimeSpan EffectiveMinDelay => + _minDelay > TimeSpan.Zero ? _minDelay : Parent != null ? Parent.MinDelay : TimeSpan.FromMinutes(5); - /// - /// Gets the effective spawn range for this entry. - /// + /// Effective maximum delay: the entry's override, else the spawner's. [JsonIgnore] - public int EffectiveSpawnRange => _spawnRange >= 0 ? _spawnRange : _parent?.HomeRange ?? 4; + public TimeSpan EffectiveMaxDelay => + _maxDelay > TimeSpan.Zero ? _maxDelay : Parent != null ? Parent.MaxDelay : TimeSpan.FromMinutes(10); - /// - /// Gets the list of spawned entities for this entry as a read-only list. - /// + /// Effective spawn range: the entry's override, else the spawner's home range. [JsonIgnore] - public IReadOnlyList SpawnedList => _spawned; - - // Note: AddToSpawned and RemoveFromSpawned are generated by SerializationGenerator - // for the _spawned List field and already handle dirty tracking - - public void Defrag(BaseSpawner parent) - { - for (var i = 0; i < Spawned.Count; ++i) - { - var spawned = Spawned[i]; - - if (parent.OnDefragSpawn(spawned, false)) - { - Spawned.RemoveAt(i--); - _parent?.MarkDirty(); - } - } - } - - /// - /// Sets the parent spawner reference. Called during deserialization. - /// - internal void SetParent(ModernSpawner parent) - { - _parent = parent; - - // Re-parent any spawned entities - foreach (var spawned in _spawned) - { - spawned?.Spawner = parent; - } - } - - /// - /// Adds an already-spawned entity to this entry. - /// Used during migration from other spawner types. - /// - public void AddSpawnedEntity(ISpawnable entity) - { - if (entity == null || _spawned.Contains(entity)) - { - return; - } - - AddToSpawned(entity); - entity.Spawner = _parent; - } - - /// - /// Removes a spawned entity from this entry. - /// - public void RemoveSpawnedEntity(ISpawnable entity) - { - if (entity != null && _spawned.Contains(entity)) - { - RemoveFromSpawned(entity); - } - } - - [AfterDeserialization] - private void AfterDeserialization() - { - for (var i = Spawned.Count - 1; i >= 0; i--) - { - var e = Spawned[i]; - if (e == null) - { - Spawned.RemoveAt(i); - } - else - { - e.Spawner = _parent; - } - } - - Spawned.TrimExcess(); - } + public int EffectiveSpawnRange => _spawnRange >= 0 ? _spawnRange : Parent != null ? Parent.HomeRange : 4; } diff --git a/Projects/ModernSpawner/Gumps/ModernSpawnerGump.cs b/Projects/ModernSpawner/Gumps/ModernSpawnerGump.cs index 022f894..4c2d559 100644 --- a/Projects/ModernSpawner/Gumps/ModernSpawnerGump.cs +++ b/Projects/ModernSpawner/Gumps/ModernSpawnerGump.cs @@ -310,7 +310,7 @@ public void CreateArray(RelayInfo info, Mobile from) prob = Utility.ToInt32(probEntry.Trim()); } - entry = _spawner.AddEntry(str, prob, maxCount); + entry = (ModernSpawnerEntry)_spawner.AddEntry(str, prob, maxCount); } if (paramsEntry != null) @@ -346,7 +346,7 @@ public void CreateArray(RelayInfo info, Mobile from) while (queue.Count > 0) { - _spawner.RemoveModernEntry(queue.Dequeue()); + _spawner.RemoveEntry(queue.Dequeue()); } if (ocount == 0 && _spawner.ModernEntries.Count > 0) @@ -385,7 +385,7 @@ public override void OnResponse(NetState state, in RelayInfo info) { case 0: // Previous page { - if (_spawner.ModernEntries != null && _page > 0) + if (_page > 0) { _page--; _entry = null; @@ -395,7 +395,7 @@ public override void OnResponse(NetState state, in RelayInfo info) case 1: // Next page { - if ((_page + 1) * EntriesPerPage <= _spawner.ModernEntries?.Count) + if ((_page + 1) * EntriesPerPage <= _spawner.ModernEntries.Count) { _page++; _entry = null; @@ -500,7 +500,7 @@ public override void OnResponse(NetState state, in RelayInfo info) } } - if (_entry != null && _spawner.ModernEntries?.Contains(_entry) == true) + if (_entry != null && _spawner.ModernEntries.Contains(_entry)) { state.Mobile.SendGump(new ModernSpawnerGump(_spawner, _entry, _page)); } diff --git a/Projects/ModernSpawner/Gumps/SpawnerEntryWizardGump.cs b/Projects/ModernSpawner/Gumps/SpawnerEntryWizardGump.cs index 741da72..9146e90 100644 --- a/Projects/ModernSpawner/Gumps/SpawnerEntryWizardGump.cs +++ b/Projects/ModernSpawner/Gumps/SpawnerEntryWizardGump.cs @@ -214,7 +214,7 @@ public override void OnResponse(NetState state, in RelayInfo info) return; case ButtonId_Delete: - _spawner.RemoveModernEntry(_entry); + _spawner.RemoveEntry(_entry); from.SendMessage("Entry deleted."); from.SendGump(new ModernSpawnerGump(_spawner)); return; diff --git a/Projects/ModernSpawner/Migration/XmlSpawnerMigrator.cs b/Projects/ModernSpawner/Migration/XmlSpawnerMigrator.cs index eaafd64..3556d7e 100644 --- a/Projects/ModernSpawner/Migration/XmlSpawnerMigrator.cs +++ b/Projects/ModernSpawner/Migration/XmlSpawnerMigrator.cs @@ -171,7 +171,7 @@ private static ModernSpawner ParseXmlSpawnerNode(XmlNode node) var spawnRange = GetIntAttribute(node, "SpawnRange", -1); if (spawnRange > 0) { - spawner.SpawnArea = new Rectangle3D( + spawner.SpawnBounds = new Rectangle3D( new Point3D(x - spawnRange, y - spawnRange, z - 20), new Point3D(x + spawnRange, y + spawnRange, z + 20) ); diff --git a/Projects/ModernSpawner/Perf/SpawnerMetrics.cs b/Projects/ModernSpawner/Perf/SpawnerMetrics.cs index a45e048..2ecb66b 100644 --- a/Projects/ModernSpawner/Perf/SpawnerMetrics.cs +++ b/Projects/ModernSpawner/Perf/SpawnerMetrics.cs @@ -60,7 +60,7 @@ public static SpawnerMetricsScope MeasureSpawn() => new(Enabled, ref _spawnTicks, ref _spawnCalls); /// - /// Opens a measurement scope for . + /// Opens a measurement scope for a single entry spawn attempt. /// public static SpawnerMetricsScope MeasureSpawnFromEntry() => new(Enabled, ref _spawnFromEntryTicks, ref _spawnFromEntryCalls); diff --git a/Projects/ModernSpawner/Perf/SpawnerPerfCommands.cs b/Projects/ModernSpawner/Perf/SpawnerPerfCommands.cs index 23c7624..d6d5322 100644 --- a/Projects/ModernSpawner/Perf/SpawnerPerfCommands.cs +++ b/Projects/ModernSpawner/Perf/SpawnerPerfCommands.cs @@ -217,7 +217,7 @@ private static void ChurnTick() // Iterate the modern-entry mapping we already maintain — avoids touching // BaseSpawner internals and guarantees we only kill entities this seed owns. - foreach (var (spawned, _) in spawner.ModernSpawned) + foreach (var (spawned, _) in spawner.Spawned) { if (_churnRng.NextDouble() >= scale) { diff --git a/Projects/ModernSpawner/Positioning/DefaultPositioner.cs b/Projects/ModernSpawner/Positioning/DefaultPositioner.cs index 3a1fc7b..f742636 100644 --- a/Projects/ModernSpawner/Positioning/DefaultPositioner.cs +++ b/Projects/ModernSpawner/Positioning/DefaultPositioner.cs @@ -28,9 +28,9 @@ public Point3D GetPosition(PositioningContext context) } // Check for spawn area first - if (spawner.SpawnArea is { Width: > 0, Height: > 0 }) + if (spawner.SpawnBounds is { Width: > 0, Height: > 0 }) { - var areaPos = GetPositionInArea(context, spawner.SpawnArea); + var areaPos = GetPositionInArea(context, spawner.SpawnBounds); if (areaPos != Point3D.Zero) { return areaPos; diff --git a/Projects/ModernSpawner/Scripting/Ast/InterSpawnerCommands.cs b/Projects/ModernSpawner/Scripting/Ast/InterSpawnerCommands.cs index ea3276e..2c54505 100644 --- a/Projects/ModernSpawner/Scripting/Ast/InterSpawnerCommands.cs +++ b/Projects/ModernSpawner/Scripting/Ast/InterSpawnerCommands.cs @@ -151,8 +151,15 @@ private static void DespawnFromEntry(ModernSpawner spawner, ModernSpawnerEntry e } var spawned = entry.Spawned[^1]; - entry.RemoveSpawnedEntity(spawned); - spawned?.Delete(); + if (spawned == null) + { + entry.RemoveFromSpawned(spawned); + } + else + { + // Deleting routes through BaseSpawner.Remove, the single registry path. + spawned.Delete(); + } } } diff --git a/Projects/ModernSpawner/Serialization/ScriptYamlSerializer.cs b/Projects/ModernSpawner/Serialization/ScriptYamlSerializer.cs index 3d76531..cd44784 100644 --- a/Projects/ModernSpawner/Serialization/ScriptYamlSerializer.cs +++ b/Projects/ModernSpawner/Serialization/ScriptYamlSerializer.cs @@ -130,7 +130,7 @@ public static void ApplyToSpawner(ScriptExportData script, ModernSpawner spawner } // Clear and re-add entries - spawner.ClearAllModernEntries(); + spawner.RemoveAllEntries(); if (script.Entries != null) { foreach (var entryData in script.Entries) diff --git a/Projects/ModernSpawner/Serialization/SpawnerJsonExporter.cs b/Projects/ModernSpawner/Serialization/SpawnerJsonExporter.cs index 756b00d..214d370 100644 --- a/Projects/ModernSpawner/Serialization/SpawnerJsonExporter.cs +++ b/Projects/ModernSpawner/Serialization/SpawnerJsonExporter.cs @@ -45,7 +45,7 @@ public static SpawnerExportData ToExportData(ModernSpawner spawner) }; // Export spawn area if defined - var spawnArea = spawner.SpawnArea; + var spawnArea = spawner.SpawnBounds; if (spawnArea is { Width: > 0, Height: > 0 }) { export.Area.SpawnArea = new SpawnAreaData diff --git a/Projects/ModernSpawner/Serialization/SpawnerJsonImporter.cs b/Projects/ModernSpawner/Serialization/SpawnerJsonImporter.cs index 3a2337b..bd38f81 100644 --- a/Projects/ModernSpawner/Serialization/SpawnerJsonImporter.cs +++ b/Projects/ModernSpawner/Serialization/SpawnerJsonImporter.cs @@ -112,7 +112,7 @@ public static ModernSpawner CreateSpawner(SpawnerExportData data, Map map = null if (data.Area.SpawnArea != null) { var area = data.Area.SpawnArea; - spawner.SpawnArea = new Rectangle3D( + spawner.SpawnBounds = new Rectangle3D( area.X, area.Y, sbyte.MinValue, area.Width, area.Height, sbyte.MaxValue - sbyte.MinValue); } @@ -179,14 +179,14 @@ public static void ConfigureSpawner(ModernSpawner spawner, SpawnerExportData dat if (data.Area.SpawnArea != null) { var area = data.Area.SpawnArea; - spawner.SpawnArea = new Rectangle3D( + spawner.SpawnBounds = new Rectangle3D( area.X, area.Y, sbyte.MinValue, area.Width, area.Height, sbyte.MaxValue - sbyte.MinValue); } } // Clear existing entries and import new ones - spawner.ClearAllModernEntries(); + spawner.RemoveAllEntries(); if (data.Entries != null) { foreach (var entryData in data.Entries) diff --git a/Projects/ModernSpawner/Serialization/XmlSpawnerImporter.cs b/Projects/ModernSpawner/Serialization/XmlSpawnerImporter.cs index a50589d..ab60b22 100644 --- a/Projects/ModernSpawner/Serialization/XmlSpawnerImporter.cs +++ b/Projects/ModernSpawner/Serialization/XmlSpawnerImporter.cs @@ -199,7 +199,7 @@ private static ModernSpawner ImportXmlSpawnerPoint(XmlElement point) // Set spawn bounds if (width > 0 && height > 0) { - spawner.SpawnArea = new Rectangle3D(x, y, -128, width, height, 256); + spawner.SpawnBounds = new Rectangle3D(x, y, -128, width, height, 256); } else { From 4daf8e8b4897976781adad862841b234387e9194 Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Fri, 11 Sep 2026 18:17:39 -0700 Subject: [PATCH 03/13] fix(perf): snapshot spawned entities before churn kill ChurnTick enumerated BaseSpawner.Spawned while killing from it. Deleting a spawned entity routes through BaseSpawner.Remove, which removes it from that same dictionary, so the first kill threw "Collection was modified". Snapshot the candidates into a PooledRefList first, then kill from the snapshot. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr --- .../ModernSpawner/Perf/SpawnerPerfCommands.cs | 29 +++++++++++++++---- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/Projects/ModernSpawner/Perf/SpawnerPerfCommands.cs b/Projects/ModernSpawner/Perf/SpawnerPerfCommands.cs index d6d5322..8204f0b 100644 --- a/Projects/ModernSpawner/Perf/SpawnerPerfCommands.cs +++ b/Projects/ModernSpawner/Perf/SpawnerPerfCommands.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using Server.Collections; using Server.Commands; using Server.Logging; @@ -208,6 +209,10 @@ private static void ChurnTick() var killed = 0; var scale = _churnPercent / 100.0; + // Killing an entity routes through BaseSpawner.Remove, which mutates both the spawner + // registry and the owning entry's Spawned list, so the candidates are snapshotted first. + using var candidates = PooledRefList.Create(); + foreach (var spawner in _seeded) { if (spawner?.Deleted != false) @@ -215,21 +220,35 @@ private static void ChurnTick() continue; } - // Iterate the modern-entry mapping we already maintain — avoids touching - // BaseSpawner internals and guarantees we only kill entities this seed owns. - foreach (var (spawned, _) in spawner.Spawned) + candidates.Clear(); + + var entries = spawner.ModernEntries; + for (var i = 0; i < entries.Count; i++) + { + var spawned = entries[i].Spawned; + for (var j = 0; j < spawned.Count; j++) + { + var entity = spawned[j]; + if (entity != null) + { + candidates.Add(entity); + } + } + } + + for (var i = 0; i < candidates.Count; i++) { if (_churnRng.NextDouble() >= scale) { continue; } - if (spawned is Mobile mobile && !mobile.Deleted && mobile.Alive) + if (candidates[i] is Mobile mobile && !mobile.Deleted && mobile.Alive) { mobile.Kill(); killed++; } - else if (spawned is Item item && !item.Deleted) + else if (candidates[i] is Item item && !item.Deleted) { item.Delete(); killed++; From 3927fd86c231fbef0759228a02ca85cda8526402 Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Fri, 11 Sep 2026 18:30:55 -0700 Subject: [PATCH 04/13] test(core): ModernUO test-server fixture and end-to-end lifecycle tests for the ported spawner Boots a real ModernUO world in the test host (ModernSpawnerTestServer, guarded to run once, behind a non-parallel collection fixture) and exercises a live ModernSpawner end to end over the entry-ownership contract: entry construction, spawn/kill/respawn against the single Spawned registry, stop/start, Respawn idempotence, Dupe, the DTO round trip and the binary round trip that proves [AfterDeserialization] rebuilds Spawned over the modern entries. The fixture reuses ModernUO's own Server.Tests.Maps.TestMapDefinitions rather than copying the map table, so ModernSpawner.Tests now references Server.Tests.csproj and copies Distribution/Data to its output. Booting the fixture surfaced a startup defect in PropertyAccessorCache: PrewarmCache compiles a getter for every property of Mobile, and ref-returning properties such as Mobile.DamageEntries (ref ValueLinkList) cannot be expressed as Func, so Expression.Lambda threw and ModernSpawnerConfiguration.Configure crashed. FindProperty now reports ref, pointer and ref-struct properties as "not found", the way indexers are already skipped. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr --- .../Core/ModernSpawnerLifecycleTests.cs | 184 ++++++++++++++++++ .../Fixtures/ModernSpawnerFixture.cs | 15 ++ .../Fixtures/ModernSpawnerTestServer.cs | 91 +++++++++ .../ModernSpawner.Tests.csproj | 12 ++ .../Scripting/PropertyAccessorCache.cs | 20 +- 5 files changed, 321 insertions(+), 1 deletion(-) create mode 100644 Projects/ModernSpawner.Tests/Core/ModernSpawnerLifecycleTests.cs create mode 100644 Projects/ModernSpawner.Tests/Fixtures/ModernSpawnerFixture.cs create mode 100644 Projects/ModernSpawner.Tests/Fixtures/ModernSpawnerTestServer.cs diff --git a/Projects/ModernSpawner.Tests/Core/ModernSpawnerLifecycleTests.cs b/Projects/ModernSpawner.Tests/Core/ModernSpawnerLifecycleTests.cs new file mode 100644 index 0000000..0f65dfa --- /dev/null +++ b/Projects/ModernSpawner.Tests/Core/ModernSpawnerLifecycleTests.cs @@ -0,0 +1,184 @@ +using System; +using System.Collections.Generic; +using System.Text.Json; +using Server.Engines.Spawners; +using Server.Mobiles; +using Xunit; + +namespace Server.Engines.ModernSpawner.Tests; + +/// +/// End-to-end tests over a live ModernUO world: the spawner is placed on a real map and the base +/// entry-ownership contract (Spawned registry, Defrag, Respawn, Dupe, DTO and binary round trips) +/// is exercised against . +/// +[Collection("Sequential ModernSpawner Tests")] +public class ModernSpawnerLifecycleTests +{ + private static ModernSpawner Place(params ReadOnlySpan names) + { + var spawner = new ModernSpawner(1, TimeSpan.FromMinutes(5), TimeSpan.FromMinutes(10), 0, default, names); + spawner.MoveToWorld(new Point3D(1500, 1500, 0), Map.Felucca); + return spawner; + } + + private static void DeleteSpawned(ModernSpawner spawner) + { + foreach (var spawned in new List(spawner.Spawned.Keys)) + { + spawned.Delete(); + } + } + + [Fact] + public void Constructor_NamesLandInModernEntries() + { + var spawner = Place("Rabbit", "Bird"); + + Assert.Equal(2, spawner.ModernEntries.Count); + Assert.Same(spawner.ModernEntries[0], spawner.Entries[0]); + Assert.Same(spawner.ModernEntries[1], spawner.Entries[1]); + + spawner.Delete(); + } + + [Fact] + public void Spawn_Kill_Respawn_UsesOneRegistry() + { + var spawner = Place("Rabbit"); + spawner.Spawn(); + + var rabbit = Assert.Single(spawner.Spawned).Key as Mobile; + Assert.NotNull(rabbit); + Assert.Single(spawner.ModernEntries[0].Spawned); + + rabbit.Delete(); // Mobile.OnDelete -> BaseSpawner.Remove + Assert.Empty(spawner.Spawned); + Assert.Empty(spawner.ModernEntries[0].Spawned); + + spawner.Spawn(); + Assert.Single(spawner.Spawned); + Assert.Single(spawner.ModernEntries[0].Spawned); + + DeleteSpawned(spawner); + spawner.Delete(); + } + + [Fact] + public void Stop_Then_Start_Works_AndFiresActivateScript() + { + var spawner = Place("Rabbit"); + spawner.SetOnActivateScript("SETVAR/activated/1"); + Assert.True(spawner.OnActivateScript.IsValid); + + spawner.Stop(); + Assert.False(spawner.Running); + + spawner.Start(); + Assert.True(spawner.Running); + + DeleteSpawned(spawner); + spawner.Delete(); + } + + [Fact] + public void Respawn_DoesNotDuplicate() + { + var spawner = Place("Rabbit"); + spawner.Spawn(); + Assert.Single(spawner.Spawned); + + spawner.Respawn(); + + Assert.Single(spawner.Spawned); + Assert.Single(spawner.ModernEntries[0].Spawned); + + DeleteSpawned(spawner); + spawner.Delete(); + } + + [Fact] + public void Dupe_ClonesModernFields() + { + var spawner = Place("Rabbit"); + spawner.ModernEntries[0].OnSpawnScript = "SETVAR/x/1"; + spawner.ModernEntries[0].Subgroup = 3; + + var copy = new ModernSpawner(); + spawner.Dupe(copy); + + Assert.Single(copy.ModernEntries); + Assert.Equal("SETVAR/x/1", copy.ModernEntries[0].OnSpawnScript); + Assert.Equal(3, copy.ModernEntries[0].Subgroup); + Assert.NotSame(spawner.ModernEntries[0], copy.ModernEntries[0]); + Assert.Same(copy.ModernEntries[0], copy.Entries[0]); + + spawner.Delete(); + copy.Delete(); + } + + [Fact] + public void Dto_RoundTrip_CarriesEntriesTriggersAndCycleState() + { + var spawner = Place("Rabbit"); + spawner.ModernEntries[0].LootTemplate = "goblin"; + spawner.CycleMode = SpawnCycleMode.Sequential; + spawner.AddToTriggerDefinitions("proximity:8:true"); + + var json = SpawnerJsonSerializer.SerializeCompact>([spawner.ToDto()]); + var dtos = JsonSerializer.Deserialize>(json, SpawnerJsonSerializer.Options); + var loaded = (ModernSpawner)dtos[0].ToSpawner(); + + Assert.Equal("goblin", loaded.ModernEntries[0].LootTemplate); + Assert.Equal(SpawnCycleMode.Sequential, loaded.CycleMode); + Assert.Equal("proximity:8:true", Assert.Single(loaded.TriggerDefinitions)); + + DeleteSpawned(loaded); + loaded.Delete(); + spawner.Delete(); + } + + [Fact] + public void Binary_RoundTrip_RebuildsSpawnedOverModernEntries() + { + var spawner = Place("Rabbit"); + spawner.ModernEntries[0].Subgroup = 2; + spawner.Spawn(); + Assert.Single(spawner.Spawned); + + var writer = new BufferWriter(true); + spawner.Serialize(writer); + var bytes = writer.Buffer.AsSpan(0, (int)writer.Position).ToArray(); + + var loaded = new ModernSpawner((Serial)0x40004242u); + loaded.Deserialize(new BufferReader(bytes)); + + Assert.Equal(2, loaded.ModernEntries[0].Subgroup); + Assert.Single(loaded.ModernEntries[0].Spawned); + Assert.Single(loaded.Spawned); + + loaded.Delete(); + DeleteSpawned(spawner); + spawner.Delete(); + } + + [Fact] + public void Kill_DispatchesOnDespawnScriptAndKillTrigger() + { + var spawner = Place("Rabbit"); + spawner.ModernEntries[0].OnDespawnScript = "SETVAR/died/1"; + spawner.Spawn(); + + var rabbit = (BaseCreature)Assert.Single(spawner.Spawned).Key; + rabbit.Kill(); + + // The observable contract until a script-side assertion exists: the kill path runs without + // throwing and the entry no longer tracks the dead creature. + Assert.Empty(spawner.ModernEntries[0].Spawned); + Assert.Empty(spawner.Spawned); + + rabbit.Corpse?.Delete(); + DeleteSpawned(spawner); + spawner.Delete(); + } +} diff --git a/Projects/ModernSpawner.Tests/Fixtures/ModernSpawnerFixture.cs b/Projects/ModernSpawner.Tests/Fixtures/ModernSpawnerFixture.cs new file mode 100644 index 0000000..241c673 --- /dev/null +++ b/Projects/ModernSpawner.Tests/Fixtures/ModernSpawnerFixture.cs @@ -0,0 +1,15 @@ +using Xunit; + +namespace Server.Engines.ModernSpawner.Tests.Fixtures; + +/// +/// Collection fixture for every test that needs a live ModernUO world. All process-global +/// initialization lives in and runs exactly once. +/// Tearing down global state is intentionally omitted: the world and the serialization +/// workers are initialized once and reused for the whole test host. +/// +[CollectionDefinition("Sequential ModernSpawner Tests", DisableParallelization = true)] +public class ModernSpawnerFixture : ICollectionFixture +{ + public ModernSpawnerFixture() => ModernSpawnerTestServer.Initialize(); +} diff --git a/Projects/ModernSpawner.Tests/Fixtures/ModernSpawnerTestServer.cs b/Projects/ModernSpawner.Tests/Fixtures/ModernSpawnerTestServer.cs new file mode 100644 index 0000000..6f0b7b3 --- /dev/null +++ b/Projects/ModernSpawner.Tests/Fixtures/ModernSpawnerTestServer.cs @@ -0,0 +1,91 @@ +using System.Reflection; +using System.Threading; +using Server.Engines.ModernSpawner.Scripting; +using Server.Items; +using Server.Misc; +using Server.Movement; +using Server.Tests.Maps; + +namespace Server.Engines.ModernSpawner.Tests.Fixtures; + +/// +/// Single, process-wide ModernUO bootstrap for the ModernSpawner test host. Modelled on +/// ModernUO's UOContent.Tests fixture of the same shape, which is internal to that +/// assembly and so cannot be reused directly; the map registrations are shared rather than copied. +/// +/// ModernUO bootstraps its global singletons (Core, ServerConfiguration, AssemblyHandler, +/// NetState, World, Timer and the serialization workers) exactly once per process, and +/// is guarded to run once. Each xUnit collection fixture instance +/// calls , so the guard here keeps the bootstrap to a single run. +/// +/// Anything that needs the copyrighted UO client files (tile data, multi data, map tiles) is +/// deliberately skipped: it is absent on CI and no ModernSpawner test depends on it. +/// +public static class ModernSpawnerTestServer +{ + private static readonly Lock _lock = new(); + private static bool _initialized; + + public static void Initialize() + { + lock (_lock) + { + if (_initialized) + { + return; + } + + Core.ApplicationAssembly = Assembly.GetExecutingAssembly(); + Core.LoopContext = new EventLoopContext(); + Core.Expansion = Expansion.EJ; + + ServerConfiguration.Load(true); + ServerConfiguration.AssemblyDirectories.Add(Core.BaseDirectory); + + AssemblyHandler.LoadAssemblies(["Server.dll", "UOContent.dll", "ModernSpawner.dll"]); + + SkillsInfo.Configure(); + + // NOTE: production (Main.cs) and ModernUO's own fixtures seed the loop clock here with + // `Core._now = DateTime.UtcNow`. That field is `internal` to Server.dll and its + // InternalsVisibleTo list only names Server.Tests and UOContent.Tests, so this assembly + // cannot set it and there is no public equivalent. Core.Now therefore stays + // DateTime.MinValue for this host. Nothing on the spawner lifecycle paths depends on an + // absolute wall clock (deadlines are relative, and the timer wheel runs on tick counts), + // but a future test that moves or reads the clock will need ModernUO to expose a seam. + + // The timer wheel must exist before NetState.Configure(), which schedules a recurring + // sweep through Timer.DelayCall (production order in Main.cs: Timer.Init runs before + // AssemblyHandler.Invoke("Configure")). + Timer.Init(0); + Server.Network.NetState.Configure(); + + // Reuses ModernUO's own test map registrations (Server.Tests), so the map table here + // cannot drift from the engine's. + TestMapDefinitions.ConfigureTestMapDefinitions(); + + World.Configure(); + // Registers the Accounts entity persistence; without it no test can construct an Account. + Server.Accounting.Accounts.Configure(); + RaceDefinitions.Configure(); + Server.Movement.Movement.Configure(); + MovementImpl.Configure(); + PathFollower.Configure(); + + // ModernSpawner persistence and registries must exist before World.Load(): ScriptRegistry + // is a GenericPersistence and registers itself with World from its constructor. + ScriptRegistry.Configure(); + ModernSpawnerConfiguration.Configure(); + + World.Load(); + World.ExitSerializationThreads(); + + DecayScheduler.Configure(); + // Without npc-speeds.json every BaseCreature constructor throws. + Server.Mobiles.NPCSpeeds.Configure(); + Server.Engines.Spawners.SpawnerJsonSerializer.Configure(); + + _initialized = true; + } + } +} diff --git a/Projects/ModernSpawner.Tests/ModernSpawner.Tests.csproj b/Projects/ModernSpawner.Tests/ModernSpawner.Tests.csproj index acc1b70..06cfe45 100644 --- a/Projects/ModernSpawner.Tests/ModernSpawner.Tests.csproj +++ b/Projects/ModernSpawner.Tests/ModernSpawner.Tests.csproj @@ -15,5 +15,17 @@ + + + Configuration=Release + + + + + + + + + diff --git a/Projects/ModernSpawner/Scripting/PropertyAccessorCache.cs b/Projects/ModernSpawner/Scripting/PropertyAccessorCache.cs index 0b1c45a..f6c4e11 100644 --- a/Projects/ModernSpawner/Scripting/PropertyAccessorCache.cs +++ b/Projects/ModernSpawner/Scripting/PropertyAccessorCache.cs @@ -212,6 +212,24 @@ public static bool SetValue(object target, string propertyPath, object? value, c return accessor; } + /// + /// Resolves a named property on that the accessor cache can actually + /// compile, or null. + /// + private static PropertyInfo? FindProperty(Type type, string propertyName) + { + var prop = FindPropertyCore(type, propertyName); + + // A ref / ref readonly property (Mobile.DamageEntries returns ref ValueLinkList), + // a pointer property, or one returning a ref struct cannot be boxed into + // Func, so the expression tree would throw at compile time. Scripts have no + // way to address them either, so report them as "not found" the way indexers are skipped. + return prop != null && IsAccessorFriendly(prop.PropertyType) ? prop : null; + } + + private static bool IsAccessorFriendly(Type propertyType) => + !propertyType.IsByRef && !propertyType.IsPointer && !propertyType.IsByRefLike; + /// /// Resolves a named property on while avoiding /// : @@ -222,7 +240,7 @@ public static bool SetValue(object target, string propertyPath, object? value, c /// walks up the chain with and returns the /// most-derived match. Returns null if nothing resolves. /// - private static PropertyInfo? FindProperty(Type type, string propertyName) + private static PropertyInfo? FindPropertyCore(Type type, string propertyName) { try { From c447d8a4061d9586c9abc635dace6e82b1d6c918 Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Fri, 11 Sep 2026 18:45:07 -0700 Subject: [PATCH 05/13] test(core): kill and dupe lifecycle tests observe the hooks they name Both tests passed with the behaviour they name deleted, so they guarded nothing. Kill_DispatchesOnDespawnScriptAndKillTrigger now gives each half a real observable. SETVAR wrote to a per-ScriptContext dictionary discarded at the end of execution, so the despawn script left no trace; it now uses SET, which writes through PropertyAccessorCache to the ScriptContext target that OnSpawnedDeath binds to the dying creature, and the test asserts the creature's Name changed. The kill trigger half registers a KillTrigger definition, sets TriggerActivated and cycles Stop/Start so OnStarted runs ActivateTriggers, then asserts the spawner's Triggered flag was false before the kill and true after -- a flag only TriggerSystem's Trigger() call can set. Dupe_ClonesModernFields asserted 2 of the 12 fields the clone path copies. It now sets a distinct non-default value for all eleven ModernSpawner.CloneEntry copies plus the base clone's Disabled, and asserts each on the copy. Verified by mutation: gutting OnSpawnedDeath, removing only its TriggerSystem.OnEntityKilled call, and dropping three CloneEntry lines each fail the covering tests; the engine was restored afterwards. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr --- .../Core/ModernSpawnerLifecycleTests.cs | 70 +++++++++++++++---- 1 file changed, 57 insertions(+), 13 deletions(-) diff --git a/Projects/ModernSpawner.Tests/Core/ModernSpawnerLifecycleTests.cs b/Projects/ModernSpawner.Tests/Core/ModernSpawnerLifecycleTests.cs index 0f65dfa..b305d1c 100644 --- a/Projects/ModernSpawner.Tests/Core/ModernSpawnerLifecycleTests.cs +++ b/Projects/ModernSpawner.Tests/Core/ModernSpawnerLifecycleTests.cs @@ -101,17 +101,44 @@ public void Respawn_DoesNotDuplicate() public void Dupe_ClonesModernFields() { var spawner = Place("Rabbit"); - spawner.ModernEntries[0].OnSpawnScript = "SETVAR/x/1"; - spawner.ModernEntries[0].Subgroup = 3; + + // Every field ModernSpawner.CloneEntry copies, each given a value distinct from its default, + // so dropping any single line from CloneEntry fails this test. + var source = spawner.ModernEntries[0]; + source.OnSpawnScript = "SET/Name/on spawn"; + source.OnDespawnScript = "SET/Name/on despawn"; + source.MinDelay = TimeSpan.FromSeconds(11); + source.MaxDelay = TimeSpan.FromSeconds(22); + source.PositioningRule = "circle"; + source.SpawnGroup = "wave one"; + source.RequireLOS = true; + source.SpawnAreaOffset = new Point3D(3, -4, 5); + source.SpawnRange = 7; + source.LootTemplate = "goblin"; + source.Subgroup = 3; + // Carried by the base SpawnerEntry clone rather than the modern override. + source.Disabled = true; var copy = new ModernSpawner(); spawner.Dupe(copy); - Assert.Single(copy.ModernEntries); - Assert.Equal("SETVAR/x/1", copy.ModernEntries[0].OnSpawnScript); - Assert.Equal(3, copy.ModernEntries[0].Subgroup); - Assert.NotSame(spawner.ModernEntries[0], copy.ModernEntries[0]); - Assert.Same(copy.ModernEntries[0], copy.Entries[0]); + var clone = Assert.Single(copy.ModernEntries); + Assert.Equal("SET/Name/on spawn", clone.OnSpawnScript); + Assert.Equal("SET/Name/on despawn", clone.OnDespawnScript); + Assert.Equal(TimeSpan.FromSeconds(11), clone.MinDelay); + Assert.Equal(TimeSpan.FromSeconds(22), clone.MaxDelay); + Assert.Equal("circle", clone.PositioningRule); + Assert.Equal("wave one", clone.SpawnGroup); + Assert.True(clone.RequireLOS); + Assert.Equal(new Point3D(3, -4, 5), clone.SpawnAreaOffset); + Assert.Equal(7, clone.SpawnRange); + Assert.Equal("goblin", clone.LootTemplate); + Assert.Equal(3, clone.Subgroup); + Assert.True(clone.Disabled); + + // A deep copy parented to the new spawner, not the source entry shared between the two. + Assert.NotSame(source, clone); + Assert.Same(clone, copy.Entries[0]); spawner.Delete(); copy.Delete(); @@ -166,16 +193,33 @@ public void Binary_RoundTrip_RebuildsSpawnedOverModernEntries() public void Kill_DispatchesOnDespawnScriptAndKillTrigger() { var spawner = Place("Rabbit"); - spawner.ModernEntries[0].OnDespawnScript = "SETVAR/died/1"; - spawner.Spawn(); + // SET writes to the ScriptContext's target, which OnSpawnedDeath binds to the dying entity, + // so the script leaves a mark on the creature itself. SETVAR would only touch a per-context + // variable dictionary that is discarded when execution ends. + spawner.ModernEntries[0].OnDespawnScript = "SET/Name/despawn script ran"; + + // kill:requiredKills:requireAllDead:resetOnTrigger:filterType:requirePlayerKiller:cooldownSeconds + spawner.TriggerActivated = true; + spawner.AddToTriggerDefinitions("kill:1:false:true:any:false:0"); + + // Triggers are registered from OnStarted; the constructor leaves the spawner running without + // ever passing through it, so cycle it to get ActivateTriggers. + spawner.Stop(); + spawner.Start(); + Assert.True(spawner.Running); + Assert.False(spawner.Triggered); + + spawner.Spawn(); var rabbit = (BaseCreature)Assert.Single(spawner.Spawned).Key; + Assert.NotEqual("despawn script ran", rabbit.Name); + rabbit.Kill(); - // The observable contract until a script-side assertion exists: the kill path runs without - // throwing and the entry no longer tracks the dead creature. - Assert.Empty(spawner.ModernEntries[0].Spawned); - Assert.Empty(spawner.Spawned); + // OnSpawnedDeath compiled and ran the entry's OnDespawnScript against the dying creature... + Assert.Equal("despawn script ran", rabbit.Name); + // ...and handed the kill to TriggerSystem, whose KillTrigger fired Trigger() on the spawner. + Assert.True(spawner.Triggered); rabbit.Corpse?.Delete(); DeleteSpawned(spawner); From 7c3473c03380b940a5ecfda223dd46ed48e7a60a Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Fri, 11 Sep 2026 18:55:01 -0700 Subject: [PATCH 06/13] docs: describe the ported entry ownership, hooks, and test server Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr --- CLAUDE.md | 16 +++-- dev-docs/architecture.md | 101 ++++++++++++++++++----------- dev-docs/modernuo-prerequisites.md | 6 +- 3 files changed, 79 insertions(+), 44 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index abb2f63..9415134 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -14,8 +14,8 @@ pull requests; until a PR merges, the submodule may be pinned to that PR's head - `Projects/ModernSpawner/` — the engine (namespace `Server.Engines.ModernSpawner`). Primary editing target. - `Projects/ModernSpawner.Tests/` — xunit tests. Run after every change. - `Projects/ModernSpawner.Benchmarks/` — BenchmarkDotNet; standalone, no ModernUO reference. -- `ModernUO/` — submodule. Do NOT edit files inside it from this repo. Engine changes go on the support - branch in the ModernUO repository (see "ModernUO changes" below). +- `ModernUO/` — submodule. Do NOT edit files inside it from this repo. Engine changes go upstream as + ModernUO pull requests from branches off `main` (see "ModernUO changes" below). - `dev-docs/` — committed, **living** specs and design docs for this project. Only current documents live here; nothing historical. - `docs/` — gitignored. Working notes (implementation guide, audits, reviews) and anything historical: @@ -26,13 +26,17 @@ pull requests; until a PR merges, the submodule may be pinned to that PR's head ```sh dotnet build ModernSpawner.slnx # builds ModernUO Server/UOContent from the submodule too -dotnet test Projects/ModernSpawner.Tests # 400+ tests, sub-second +dotnet test Projects/ModernSpawner.Tests # 427 tests; the lifecycle collection boots a ModernUO test server dotnet build -c Analyze # analyzers + Rules.ruleset ``` `Directory.Build.props` here applies only to `Projects/**`; the submodule keeps its own. `TreatWarningsAsErrors` is on. The ModernUO serialization generator is referenced directly by `ModernSpawner.csproj` (it is a private asset in ModernUO and does not flow through project references). +World-backed tests (`Projects/ModernSpawner.Tests/Core/ModernSpawnerLifecycleTests.cs`) share a +process-wide ModernUO bootstrap in `Projects/ModernSpawner.Tests/Fixtures/ModernSpawnerTestServer.cs` +and run in a `DisableParallelization` xunit collection; use that fixture for any new test that needs a +live spawner rather than standing up World/Core state by hand. ## Rules @@ -52,8 +56,10 @@ All ModernUO rules apply verbatim. Read and follow the **Code Audit Rules** in ` ModernSpawner-specific: - Scripts and expressions parse once and execute many times. Never re-parse a script per spawn tick. -- `ModernSpawnerEntry` is separate from `BaseSpawner.SpawnerEntry`. The spawner keeps its own `_spawnEntries`; - the base `Entries` list should stay empty. Treat this as a known design tension (see the architecture spec). +- `ModernSpawner : Spawner` owns `List` where `ModernSpawnerEntry : SpawnerEntry`; the + base contract (`Entries`, `EntrySpan`, `CreateEntry`, `AddEntryCore`, …) runs over it and the lifecycle + hooks (`OnStarted`, `OnSpawned`, `OnSpawnedDeath`, entry-aware `GetSpawnPosition`) carry the modern + behaviour. Never add a parallel entry list or hide base members with `new`. - Triggers register through `TriggerSystem`; proximity uses `Item.HandlesOnMovement`/`OnMovement`, speech uses `HandlesOnSpeech`. Extended (beyond 24-tile) proximity is stubbed pending a ModernUO area-movement API. diff --git a/dev-docs/architecture.md b/dev-docs/architecture.md index df7c832..fd7ccf6 100644 --- a/dev-docs/architecture.md +++ b/dev-docs/architecture.md @@ -37,26 +37,32 @@ Everything runs on the game loop. There are no threads, locks or `Task.Run` anyw ### 2.1 Core (`Core/`) -`ModernSpawner` (1,260 lines) derives from `BaseSpawner` and adds 18 serialized fields: a -`List`, four script serials, positioning flags, trigger definitions and flags, spawn -area, notes, and cycle-mode state. `ModernSpawnerEntry` (270 lines) is a standalone generator class with 17 -fields: the six `SpawnerEntry` equivalents plus scripts, delays, positioning rule, group, LOS, area offset, -range, loot template and subgroup. - -**The dual-list problem.** `BaseSpawner` owns `List _entries` and `Dictionary Spawned`, and its `AddEntry`, `Start`, `Defrag`, `Remove`, `CountSpawns`, `RemoveEntry`, -`RemoveSpawn(s)`, `Respawn`, `Reset`, `NextSpawn`, `GetProperties`, `OnAfterDuped`, `AfterDeserialization`, -`ToDto`/`ApplyDto`, the stock gumps and `[EditSpawner` all operate on that list. `ModernSpawner` overrides -only `Spawn()`, `GetSpawnPosition`, `GetSpawnerProperties`, `OnDelete`, `OnDoubleClick`, hides `AddEntry`/ -`Start`/`Stop` with `new`, and keeps its own `_spawnEntries` and `_modernSpawned`. The base list is empty -for any spawner built through the modern API, so every base member above is a no-op or acts on stale -state. Consequences are itemised in `docs/audit/core.md` §3 and summarised in `docs/feature-audit.md` §3 -(#1–#3, #11, #24). - -Spawn flow today: `OnTick` → `Spawn()` (override) → select entry by cycle mode → build a throw-away -`SpawnerEntry` → `base.Spawn(tempEntry)` (creates, positions, places) → copy entity into the modern entry -and `_modernSpawned` → apply loot and entry script. Positioning runs inside `base.Spawn`, before the modern -entry is known. +`ModernSpawner` derives from `Spawner` and adds 17 serialized fields: `_spawnEntries` +(`List`, field 0), four script serials (1–4), three positioning flags (5–7), trigger +definitions and flags (8–10), notes (11), and five fields of cycle-mode state (12–16). `ModernSpawnerEntry` +derives from `SpawnerEntry` and adds only its 11 extra fields (0–10): scripts, delays, positioning rule, +group, LOS, area offset, range, loot template and subgroup — the six `SpawnerEntry` fields and the +`Disabled` flag come from the base class. + +**Entry ownership.** `ModernSpawner` owns `_spawnEntries` and implements the base contract over it: +`Entries` and `EntrySpan` (via `ReadOnlySpan.CastUp`) expose it to `Spawner`/`BaseSpawner`, +and `CreateEntry`, `AddEntryCore`, `RemoveEntryCore`, `ClearEntriesCore`, `AdoptEntries` (converting a +foreign entry with `CloneEntry` and carrying its live spawns over with `TransferSpawned`) and `CloneEntry` +(copying the 11 modern fields) let every base spawn path — `Spawn`, `Defrag`, `Remove`, +`RemoveAllEntries`, dupe, DTO and binary round trips — run over `ModernSpawnerEntry` with no parallel +list. Typed conveniences (`ModernEntries`, `AddModernEntry`) remain for callers that want +`ModernSpawnerEntry` directly instead of the base `SpawnerEntry` view. This replaced an earlier +"dual-list" design where the spawner kept its own `_spawnEntries` alongside an always-empty base +`_entries`; that design, and the bugs it caused, is history — see `docs/audit/core.md` §3. + +Spawn flow as ported: the timer calls `OnTick` → `Spawn()`, which runs the before-spawn script veto, +defrags, then selects an entry by cycle mode (`SpawnWeightedOne` for Random/Sequential, `SpawnGroupMode` +for Group) over `_spawnEntries` with plain `for` loops and calls the base `Spawn(entry, out flags)` for the +chosen entry. That base call positions the entity through the entry-aware `GetSpawnPosition(entry, spawned, +map)` override (entry `PositioningRule`, else the entry's `SpawnAreaOffset`, else the spawner's own +positioning) and, once placed, calls `OnSpawned(entry, spawned)` to apply the entry's loot template and +`OnSpawnScript`. On death, `OnSpawnedDeath(entry, spawned, killer)` notifies `TriggerSystem` for kill +triggers and runs the entry's `OnDespawnScript`. ### 2.2 Triggers (`Triggers/`) @@ -214,19 +220,27 @@ construction and property application, before `GetSpawnPosition`. ### 4.3 Shape of A (ModernSpawner side) +This shape is implemented on branch `port/entry-contract` (commits `a3413ef`–`c447d8a`) exactly as listed +below, with two differences from the original plan noted inline. + - `ModernSpawnerEntry : SpawnerEntry` (class inheritance; only the extra fields are declared here). Because it lives in another assembly, it must declare `[DirtyTrackingEntity] private BaseSpawner Owner => Parent;` so its generated setters mark the spawner dirty (see `modernuo-prerequisites.md`, generator follow-up). -- `ModernSpawner.Entries => _spawnEntries`; `CreateEntry` returns a `ModernSpawnerEntry`; delete - `_modernSpawned`, `AddModernEntry`, `RemoveModernEntry`, `ModernEntries`, `ModernSpawned`, the temp-entry - path, and the `new` `AddEntry/Start/Stop`. +- `ModernSpawner.Entries => _spawnEntries`; `CreateEntry` returns a `ModernSpawnerEntry`; the temp-entry + path, `_modernSpawned`, `RemoveModernEntry`, `ModernSpawned`, and the `new` `AddEntry`/`Start`/`Stop` + hides were deleted. **Difference:** `ModernEntries` and `AddModernEntry` were kept as typed + conveniences over the base `SpawnerEntry`-typed contract, not deleted — callers that want + `ModernSpawnerEntry` directly (tests, gumps) still use them. - `Spawn()` override keeps cycle-mode selection and calls `base.Spawn(entry, out flags)`. -- `OnBeforeSpawn(entry)` runs the entry condition and before-spawn script (veto); `OnSpawned(entry, spawned)` - applies loot and the entry spawn script. -- `GetSpawnPosition(entry, spawned, map)` applies the entry rule, else `base`. -- Start/Stop hooks: `BaseSpawner.Start/Stop` become `protected virtual OnStarted/OnStopped` callbacks (tiny - upstream change) so `Running = …` reaches trigger activation and the activate/deactivate scripts. +- **Difference:** `OnBeforeSpawn(entry)` is not overridden. The before-spawn script veto (`cancel()`) runs + inline at the top of `Spawn()`, ahead of `Defrag()` and entry selection, rather than through the base + hook. `OnSpawned(entry, spawned)` applies loot and the entry spawn script as planned. +- `GetSpawnPosition(entry, spawned, map)` applies the entry rule, else the entry's `SpawnAreaOffset`, else + `base`. +- Start/Stop hooks: `BaseSpawner.Start/Stop` gained `protected virtual OnStarted/OnStopped` callbacks + (upstream change), overridden here so `Running = …` reaches trigger activation and the + activate/deactivate scripts. ## 5. Target: triggers (D2, D3) @@ -330,14 +344,26 @@ carried across `Timer.DelayCall`; mutation-safe iteration and registration befor ## 10. Testing architecture -- A `SpawnerTestFixture` boots a ModernUO test server and places a `ModernSpawner` on a **non-Internal** - test map (`BaseSpawner.Spawn` refuses `Map.Internal`, `BaseSpawner.cs:997`). ModernUO's - `TestServerInitializer` is `internal` and loads only `Server`/`UOContent`, so the fixture either gets an - `InternalsVisibleTo` + assembly-list parameter upstream or a copy of the initializer here that - also registers the ModernSpawner assembly and runs its `Configure`. It exposes `Tick()` to advance timers. -- Every subsystem gets an end-to-end test that goes through the fixture: spawn/kill/respawn, stop/start, - trigger fire and gate, entry rule placement, loot application, script hooks, DTO round trip, binary save - round trip (`Serialize` to a buffer and `Deserialize` back). +- `Projects/ModernSpawner.Tests/Fixtures/ModernSpawnerTestServer.cs` boots a ModernUO test server and + places a `ModernSpawner` on a **non-Internal** test map (`BaseSpawner.Spawn` refuses `Map.Internal`). + ModernUO's own `TestServerInitializer` (in `UOContent.Tests`) is `internal` to that assembly, so this is + a copy of the initializer modelled on it — not an upstream `InternalsVisibleTo` grant — that also loads + `ModernSpawner.dll` and runs `ModernSpawnerConfiguration.Configure()`; it reuses ModernUO's own map table + through `Server.Tests.Maps.TestMapDefinitions` (a project reference) so the two cannot drift. + `Projects/ModernSpawner.Tests/Fixtures/ModernSpawnerFixture.cs` is the xunit `ICollectionFixture` that + calls `Initialize()` once per process; every world-backed test carries + `[Collection("Sequential ModernSpawner Tests")]` (`DisableParallelization = true`) since the bootstrap + and `World` are process-global singletons. +- Known limitation: `Core._now` is `internal` to `Server.dll` with `InternalsVisibleTo` naming only + `Server.Tests` and `UOContent.Tests`, so `ModernSpawnerTestServer` cannot seed the loop clock and + `Core.Now` stays `DateTime.MinValue` for this host. Nothing on the spawner lifecycle paths currently + depends on an absolute wall clock, but a future test that reads or advances the clock needs the + prerequisite in §11. +- `Projects/ModernSpawner.Tests/Core/ModernSpawnerLifecycleTests.cs` is the current end-to-end suite over + the fixture: spawn/kill/respawn, stop/start (with activate-script dispatch), dupe (asserts every field + `CloneEntry` copies), DTO round trip, binary save round trip, and kill-hook dispatch (`OnSpawnedDeath` + running the despawn script and handing the kill to `TriggerSystem`). Extend this suite as trigger gate, + entry-rule placement and loot-application coverage is added. - Parser tests remain; add "producer→parser" tests for every gump/importer-generated string. - Benchmarks stay in `ModernSpawner.Benchmarks`; add a tick-loop allocation benchmark. @@ -345,7 +371,8 @@ carried across `Timer.DelayCall`; mutation-safe iteration and registration befor Tracked in `modernuo-prerequisites.md`: DTO helper visibility (done), abstract entry ownership (§4.2), `OnStarted/OnStopped` and `OnConfigureSpawned` virtuals, `OnSpawnedDeath` hook, `SkillUsedEvent`, test -initializer access, GUID-based replacement in `[ImportSpawners` (today it deletes co-located same-type +initializer access, `InternalsVisibleTo("ModernSpawner.Tests")` on `Server.csproj` (so the test fixture +can seed `Core._now`), GUID-based replacement in `[ImportSpawners` (today it deletes co-located same-type spawners and calls `Respawn()` unconditionally, `ImportSpawnersCommand.cs:259`), sector-range movement subscription (deferred). diff --git a/dev-docs/modernuo-prerequisites.md b/dev-docs/modernuo-prerequisites.md index 6a34e21..b20ea2a 100644 --- a/dev-docs/modernuo-prerequisites.md +++ b/dev-docs/modernuo-prerequisites.md @@ -17,12 +17,14 @@ per-movement paths without a measurement, because shards run 12k+ spawners. | PR | Change | Why ModernSpawner needs it | |---|---|---| | [#2619](https://github.com/modernuo/ModernUO/pull/2619) | `BaseSpawner.Dto.cs`: `private protected` DTO helpers → `protected` | `ModernSpawner.ToDto()` lives in another assembly and needs the `Dto*` helpers and `BoundsFromHomeRange` | -| [#2621](https://github.com/modernuo/ModernUO/pull/2621) | Subclass-owned entries (`BaseSpawner` v13 / `Spawner` v2 owner contract), lifecycle hooks + `NotifySpawnedDeath`, `SpawnerEntry` v2 `Disabled`, DTO records own `entries`, save migration | D1/D11/D12: `ModernSpawner : Spawner` owns `List` with `ModernSpawnerEntry : SpawnerEntry`; kill trigger via the death hook. **ModernSpawner is not yet ported**: the submodule stays at `4bad0cc9e` (pre-#2621 main) until the port PR bumps it to `a52ce6ef7` | +| [#2621](https://github.com/modernuo/ModernUO/pull/2621) | Subclass-owned entries (`BaseSpawner` v13 / `Spawner` v2 owner contract), lifecycle hooks + `NotifySpawnedDeath`, `SpawnerEntry` v2 `Disabled`, DTO records own `entries`, save migration | D1/D11/D12: `ModernSpawner : Spawner` owns `List` with `ModernSpawnerEntry : SpawnerEntry`; kill trigger via the death hook. ModernSpawner ported on `port/entry-contract`; submodule at `a52ce6ef7` | ## Planned (see `architecture.md` §4–§5, §11; decisions D1, D2, D3, D11, D12) - `SkillEvents.SkillUsedEvent` raised from `SkillCheck` (D3). -- `TestServerInitializer` usable from an external test assembly (or a public variant that takes an assembly list). +- `TestServerInitializer` usable from an external test assembly (or a public variant that takes an assembly + list), and `InternalsVisibleTo("ModernSpawner.Tests")` on `Server.csproj` so the fixture can seed + `Core._now` (today the test clock is `DateTime.MinValue`). - `[ImportSpawners`: GUID-based replacement, preserve `running`, no unconditional `Respawn()`. - Deferred: sector-range movement subscription for proximity triggers wider than 24 tiles. From 3f81bec7f030bcc87073f3e451b4386c8bdd1631 Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Fri, 11 Sep 2026 19:03:21 -0700 Subject: [PATCH 07/13] =?UTF-8?q?docs:=20architecture=20=C2=A74.2=20descri?= =?UTF-8?q?bes=20the=20merged=20entry=20contract?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr --- dev-docs/architecture.md | 129 +++++++++++++++++++++------------------ 1 file changed, 69 insertions(+), 60 deletions(-) diff --git a/dev-docs/architecture.md b/dev-docs/architecture.md index fd7ccf6..8e13f9e 100644 --- a/dev-docs/architecture.md +++ b/dev-docs/architecture.md @@ -131,8 +131,8 @@ an opt-in, zero-alloc counter set with seven `[ModernSpawnerPerf*` commands. ## 3. Cross-cutting facts - **Assembly boundary.** ModernSpawner subclasses `BaseSpawner` from another assembly. Anything the base - keeps `private`/`private protected`/non-virtual is unreachable. The first support-branch change - (`79e3a8e34`) widened the DTO helpers for exactly this reason. + keeps `private`/`private protected`/non-virtual is unreachable. ModernUO PR #2619 widened the DTO + helpers for exactly this reason. - **Serialization generator** must be referenced directly (`PrivateAssets="all"` upstream). Version bumps need `Migrations/*.vN.json` produced by `ModernUOSchemaGenerator`; the repo has none yet. - **Bootstrap order.** `EventScheduler.Configure` runs before world load, so scheduling during @@ -151,72 +151,81 @@ an opt-in, zero-alloc counter set with seven `[ModernSpawnerPerf*` commands. | B. Subclass entry | `AddEntry` becomes virtual/`CreateEntry`; base `_entries` deserialization must construct the subclass (generator does not support polymorphic lists) → still needs a base change to let the subclass own serialization of the list | `ModernSpawnerEntry : SpawnerEntry` | Same benefits as A once the list-serialization problem is solved, which is most of A anyway | | C. Virtualise everything | ~12 members virtual (`Start/Stop/Defrag/Remove/RemoveSpawns/CountSpawns/RemoveEntry/RemoveSpawn/IsFull/NextSpawn/GetProperties`) | Override all of them, keep two lists | Base gumps/DTO/`[EditSpawner` still blind; every new base feature needs another override | -**Decision (D1, D11): A**, with `ModernSpawner` deriving from `Spawner` once `Spawner` exposes the needed -virtuals. B collapses into A; C is a treadmill. Performance constraint: entry access through -`IReadOnlyList` adds one interface dispatch per entry per selection, which runs once per -spawn cycle (minutes apart), not per tick or per movement — measured before merge regardless. The -per-entry `Enabled` flag (D12) is part of the same upstream change. +**Decision (D1, D11): A**, implemented as subclass-owned entries over the concrete `SpawnerEntry` base +class — Option B's shape, once B's list-serialization problem was solved, rather than a separate +`ISpawnerEntry` interface; see §4.2 for what actually merged. `ModernSpawner` derives from `Spawner`, +the first concrete owner of the abstract contract; C was rejected as a treadmill. Performance constraint: +entry access goes through a concrete-typed `ReadOnlySpan` (`EntrySpan`), not `List` or +per-entry interface dispatch, so the once-per-spawn-cycle (minutes apart, not per-tick or per-movement) +entry selection loop gained no measurable cost — measured before merge. The per-entry `Enabled`/`Disabled` +flag (D12) is part of the same upstream change (`SpawnerEntry` v2). -### 4.2 Shape of A (ModernUO side) +### 4.2 Shape as merged (ModernUO side, PR #2621) -```csharp -public interface ISpawnerEntry -{ - string SpawnedName { get; set; } - int SpawnedProbability { get; set; } - int SpawnedMaxCount { get; set; } - string Properties { get; set; } - string Parameters { get; set; } - List Spawned { get; } - EntryFlags Valid { get; set; } - bool IsFull => Spawned.Count >= SpawnedMaxCount; - void Defrag(BaseSpawner parent); - void AddToSpawned(ISpawnable s); void RemoveFromSpawned(ISpawnable s); -} +`BaseSpawner.Entries.cs` declares the abstract owner contract, typed on the concrete `SpawnerEntry` base +class throughout — no interface anywhere: +```csharp public abstract partial class BaseSpawner { - public abstract IReadOnlyList Entries { get; } // subclass-owned, serialized there - protected abstract ISpawnerEntry CreateEntry(string name, int prob, int max, string props, string args); - protected abstract void AddToEntries(ISpawnerEntry e); - protected abstract bool RemoveFromEntries(ISpawnerEntry e); - public Dictionary Spawned { get; } // unchanged shape, interface-typed - // AddEntry/RemoveEntry/Defrag/Remove/RemoveSpawns/Start/CountSpawns unchanged logic, interface-typed - protected virtual void OnSpawned(ISpawnerEntry entry, ISpawnable spawned) { } // after placement - protected virtual bool OnBeforeSpawn(ISpawnerEntry entry) => true; // veto - public virtual Point3D GetSpawnPosition(ISpawnerEntry entry, ISpawnable spawned, Map map) - => GetSpawnPosition(spawned, map); // entry-aware overload + public abstract IReadOnlyList Entries { get; } // cold, read-only view + protected abstract ReadOnlySpan EntrySpan { get; } // hot loops, zero-alloc + protected abstract SpawnerEntry CreateEntry( + string name, int probability, int maxCount, string properties, string parameters); + protected abstract void AddEntryCore(SpawnerEntry entry); + protected abstract bool RemoveEntryCore(SpawnerEntry entry); + protected abstract void ClearEntriesCore(); + protected abstract void AdoptEntries(IReadOnlyList entries); // legacy save / DTO import + protected virtual SpawnerEntry CloneEntry(SpawnerEntry source); // deep copy; no spawns + protected static void TransferSpawned(SpawnerEntry source, SpawnerEntry target); + protected void RebuildSpawned(); // rebuild Spawned, re-arm timer } ``` -Constraints the generator imposes (verified in the Codex review against `ModernUO.Serialization.Generator` -4.1.0): a serialized list is constructed from its *declared* element type with no discriminator, so the -serialized field must be a **concrete** list per ownership branch and the abstract `Entries` is an -unannotated interface view over it. `Spawner` owns `[SerializableField] List _entries` and -`ProximitySpawner`/`RegionSpawner` inherit it (they already derive from `Spawner`); `ModernSpawner` owns -`List`. - -Save migration is more than "v13 hands the list down": generated deserialization runs `base.Deserialize` -before the derived version is read, older `MigrateFrom(V10/V11)` handlers and the pre-generator reader assign -`_entries` directly, and `BaseSpawner.AfterDeserialization` rebuilds `Spawned` and arms the timer before -derived data exists. The sequence must be: base keeps a transient legacy-entry carrier populated by every -old reader; the concrete owner adopts it exactly once in its own deserialization; registry reconstruction and -timer start move to a deferred hook that runs after the concrete list is loaded. Old readers and their -encoding stay untouched; every stock subclass is tested against saves from v10, v11, v12 and the new format. - -Mutation and copy operations become explicit on the base (`ClearEntries`, `ReplaceEntries`, `CopyEntriesTo` -with dirty tracking); `OnAfterDuped` and `SpawnerControllerGump.CopyEntry` use them so modern entry fields -survive duplication and controller copies. `SpawnerEntry : ISpawnerEntry`. - -DTO: keep the stock JSON shape (root `$type`, undiscriminated `entries`) byte-for-byte. Each root DTO subtype -owns a concrete entry-DTO collection (`SpawnerDataDto.Entries : List`, -`ModernSpawnerDto.Entries : List`) and converts through `CreateEntry`; `ApplyDto` no -longer touches entries itself. `SpawnerGump`, `SpawnerControllerGump`, `EditSpawnerCommand` read -`ISpawnerEntry`. Estimated size: ~30 files in UOContent, one save migration with a transient carrier. - -A pre-placement hook is also needed so computed properties (D6) apply before positioning: -`protected virtual void OnConfigureSpawned(ISpawnerEntry entry, ISpawnable spawned)` called after -construction and property application, before `GetSpawnPosition`. +`CreateEntry` is the one factory per owner that keeps each owner's serialized list concrete for the +generator — the rationale that survives from the interface sketch this section used to carry. `AdoptEntries` +takes ownership of entries built elsewhere (a legacy save or a DTO import); an owner that converts a +foreign entry into its own type must carry its live spawns across with the static `TransferSpawned` +helper, since `CloneEntry` deliberately does not copy them. `RebuildSpawned` rebuilds the `Spawned` +registry from `EntrySpan` and re-arms the timer; `Spawner` calls it from the base `[AfterDeserialization]` +for its own list, and any subclass owning a different list — `ModernSpawner` included (§4.3) — must call +it again from its own `[AfterDeserialization]` once that list is loaded. + +Lifecycle hooks live in `BaseSpawner.Hooks.cs`, all `protected virtual`, all typed on `SpawnerEntry`: +`OnStarted()`/`OnStopped()` (after `Start()`/`Stop()`), `OnBeforeSpawn(entry) => true` (veto point before +construction), `OnConfigureSpawned(entry, spawned)` (after property application, before positioning, so +computed properties (D6) apply first), `GetSpawnPosition(entry, spawned, map)` (entry-aware, defaults to +the entry-agnostic overload), `OnSpawned(entry, spawned)` (after placement) and +`OnSpawnedDeath(entry, spawned, killer)`, reached through the public `NotifySpawnedDeath(spawned, killer)` +that `BaseCreature.OnDeath` calls while the spawner link is still intact. + +`Spawner` (`[SerializationGenerator(2)]`) is the first concrete owner: it declares +`[SerializableField(2)] List _entryList` and implements the abstract members directly over +it; `ProximitySpawner`/`RegionSpawner` inherit that ownership unchanged since they already derive from +`Spawner`. `ModernSpawner : Spawner` (§4.3) instead declares its own `List` and +re-implements the same members over that list — two sibling concrete owners of one abstract contract, not +an interface layer over both. + +`SpawnerEntry` (`[SerializationGenerator(2, false)]`) gained a `Disabled` flag (`Enabled` is its inverted +public toggle, so the common enabled case writes nothing) that weighted selection now skips, and +`protected BaseSpawner Parent => _parent` with a public `SetParent(BaseSpawner)` so an out-of-assembly +owner can re-parent an entry it adopts in `AdoptEntries`. + +Save migration: `BaseSpawner` bumped to v13. The existing `MigrateFrom(V10Content/V11Content/V12Content)` +legacy readers keep populating their own `Entries` field as before; each now finishes by calling +`AdoptEntries(content.Entries ?? [])` so the concrete owner takes the list over exactly once, instead of +`BaseSpawner` keeping its own `_entries` field for `AfterDeserialization` to rebuild from. Old readers and +their encoding are untouched. + +DTO: `SpawnerDto` is an `abstract record` with `abstract IReadOnlyList EntryView { get; }`; +each concrete DTO owns its own typed collection and overrides the view (`SpawnerDataDto.Entries : +List`, `ModernSpawnerDto.Entries : List`), and `BaseSpawner.ApplyDto` +calls `AdoptEntries(dto.EntryView)` — it never touches entries itself. + +Copy operations are explicit on the base: `RemoveAllEntries()` (deletes every live spawn, then +`ClearEntriesCore()`) and `CopyEntriesTo(BaseSpawner target)` (clones this spawner's entries onto `target` +via `CreateEntry`/`CloneEntry`); `BaseSpawner.OnAfterDuped` and `SpawnerControllerGump.CopyEntry` both use +`CopyEntriesTo` so modern entry fields survive duplication and controller copies. ### 4.3 Shape of A (ModernSpawner side) From 97ea8cbe9b733171c8c6abaa1a790b08a7d53c70 Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Fri, 11 Sep 2026 19:05:34 -0700 Subject: [PATCH 08/13] docs: no remaining ISpawnerEntry references Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr --- dev-docs/architecture.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dev-docs/architecture.md b/dev-docs/architecture.md index 8e13f9e..7910cc4 100644 --- a/dev-docs/architecture.md +++ b/dev-docs/architecture.md @@ -147,13 +147,13 @@ an opt-in, zero-alloc counter set with seven `[ModernSpawnerPerf*` commands. | Option | ModernUO change | ModernSpawner change | Result | |---|---|---|---| -| **A. Abstract entry ownership** (archived plan Phase 1) | `BaseSpawner` works over `IReadOnlyList` provided by the subclass; `CreateEntry` factory; `Spawner`/`Proximity`/`Region` own `List` (v13 migration moves `_entries` down); gumps/DTO/commands use `ISpawnerEntry` | Own `List` becomes *the* list; `ModernSpawnerEntry : ISpawnerEntry`; delete the parallel `_modernSpawned`, temp-entry trick, `new` hides | Every base member works; stock gumps, `[SpawnAdmin`, DTO see modern entries; positioning knows the entry | +| **A. Abstract entry ownership** (archived plan Phase 1) | `BaseSpawner` works over an `IReadOnlyList` of a shared abstract entry type provided by the subclass; `CreateEntry` factory; `Spawner`/`Proximity`/`Region` own `List` (v13 migration moves `_entries` down); gumps/DTO/commands use that abstract entry type | Own `List` becomes *the* list; `ModernSpawnerEntry` implements the shared abstract entry type; delete the parallel `_modernSpawned`, temp-entry trick, `new` hides | Every base member works; stock gumps, `[SpawnAdmin`, DTO see modern entries; positioning knows the entry | | B. Subclass entry | `AddEntry` becomes virtual/`CreateEntry`; base `_entries` deserialization must construct the subclass (generator does not support polymorphic lists) → still needs a base change to let the subclass own serialization of the list | `ModernSpawnerEntry : SpawnerEntry` | Same benefits as A once the list-serialization problem is solved, which is most of A anyway | | C. Virtualise everything | ~12 members virtual (`Start/Stop/Defrag/Remove/RemoveSpawns/CountSpawns/RemoveEntry/RemoveSpawn/IsFull/NextSpawn/GetProperties`) | Override all of them, keep two lists | Base gumps/DTO/`[EditSpawner` still blind; every new base feature needs another override | **Decision (D1, D11): A**, implemented as subclass-owned entries over the concrete `SpawnerEntry` base class — Option B's shape, once B's list-serialization problem was solved, rather than a separate -`ISpawnerEntry` interface; see §4.2 for what actually merged. `ModernSpawner` derives from `Spawner`, +abstract entry interface; see §4.2 for what actually merged. `ModernSpawner` derives from `Spawner`, the first concrete owner of the abstract contract; C was rejected as a treadmill. Performance constraint: entry access goes through a concrete-typed `ReadOnlySpan` (`EntrySpan`), not `List` or per-entry interface dispatch, so the once-per-spawn-cycle (minutes apart, not per-tick or per-movement) @@ -271,7 +271,7 @@ below, with two differences from the original plan noted inline. `[AfterDeserialization(false)]` hook. - **Kill.** `CreatureEvents.CreatureDeathEvent` fires *after* `Mobile.OnDeath`, which deletes non-player mobiles and clears `Spawner` on the way (`Mobile.cs:4647,4899`), so `bc.Spawner` is null by then. An - upstream PR adds `protected virtual void OnSpawnedDeath(ISpawnerEntry entry, ISpawnable spawned, Mobile killer)` + upstream PR adds `protected virtual void OnSpawnedDeath(SpawnerEntry entry, ISpawnable spawned, Mobile killer)` on `BaseSpawner`, invoked from `BaseCreature.OnDeath` before base death while the link is intact. Death is distinct from removal (taming, pickup, delete). `RequireAllDead` is evaluated after removal against the entry's remaining live count. From a8e59817e5988d00b52f92c7884ca8ed94547f77 Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Fri, 11 Sep 2026 19:26:18 -0700 Subject: [PATCH 09/13] fix(core): triggers register on every construction path; dupe copies trigger definitions Trigger registration now goes through one guarded helper, ModernSpawner. EnsureTriggersActive(), the only caller of TriggerSystem.ActivateTriggers outside the trigger system. It deactivates first and re-registers only when the spawner is running, is TriggerActivated and has definitions, so it is idempotent even though ActivateTriggers appends rather than replaces. BaseSpawner.Start() only reaches OnStarted when Running actually flips, and every construction path builds an already-running spawner, so migrated and imported spawners never registered the triggers they had just been given. ToSpawner(), both XmlSpawnerMigrator paths, XmlSpawnerImporter and both SpawnerJsonImporter entry points now end in EnsureTriggersActive(); OnStarted and [AfterDeserialization] use it too. [dupe also dropped triggers: _triggerDefinitions is [SerializedIgnoreDupe] with no OnAfterDuped override, so a copy came back TriggerActivated with an empty list. The new override copies the list through the generated setter and registers it. Also: OnStopped skips the deactivate script once the item is flagged deleted (the trigger deactivation still runs) and documents that deletion reaches it through BaseSpawner.OnDelete -> Stop(); AdoptEntries returns early when handed its own list; the after-spawn script doc records Spawn()'s early returns. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr --- .../Core/ModernSpawnerLifecycleTests.cs | 51 ++++++++++++ .../ModernSpawner/Core/ModernSpawner.Dto.cs | 4 + Projects/ModernSpawner/Core/ModernSpawner.cs | 79 ++++++++++++++++--- .../Migration/XmlSpawnerMigrator.cs | 8 ++ .../Serialization/SpawnerJsonImporter.cs | 7 ++ .../Serialization/XmlSpawnerImporter.cs | 3 + 6 files changed, 139 insertions(+), 13 deletions(-) diff --git a/Projects/ModernSpawner.Tests/Core/ModernSpawnerLifecycleTests.cs b/Projects/ModernSpawner.Tests/Core/ModernSpawnerLifecycleTests.cs index b305d1c..9d7fad2 100644 --- a/Projects/ModernSpawner.Tests/Core/ModernSpawnerLifecycleTests.cs +++ b/Projects/ModernSpawner.Tests/Core/ModernSpawnerLifecycleTests.cs @@ -30,6 +30,21 @@ private static void DeleteSpawned(ModernSpawner spawner) } } + private static ModernSpawnerDto MakeDto(bool triggerActivated, params string[] triggers) => + new() + { + Guid = Guid.NewGuid(), + Location = new Point3D(1500, 1500, 0), + Map = Map.Felucca, + Count = 1, + MinDelay = TimeSpan.FromMinutes(5), + MaxDelay = TimeSpan.FromMinutes(10), + HomeRange = 5, + Entries = [new ModernSpawnerEntry("Rabbit")], + TriggerActivated = triggerActivated, + Triggers = new List(triggers) + }; + [Fact] public void Constructor_NamesLandInModernEntries() { @@ -119,9 +134,21 @@ public void Dupe_ClonesModernFields() // Carried by the base SpawnerEntry clone rather than the modern override. source.Disabled = true; + // [SerializedIgnoreDupe] keeps the reflection dupe off the trigger list, so OnAfterDuped has + // to copy it by hand - otherwise the copy is TriggerActivated with nothing to activate. + spawner.TriggerActivated = true; + spawner.AddToTriggerDefinitions("proximity:8:true:false:5:0"); + var copy = new ModernSpawner(); spawner.Dupe(copy); + Assert.True(copy.TriggerActivated); + Assert.Equal("proximity:8:true:false:5:0", Assert.Single(copy.TriggerDefinitions)); + // Its own list, not the source's - editing one spawner's triggers must not touch the other. + Assert.NotSame(spawner.TriggerDefinitions, copy.TriggerDefinitions); + // And registered, so the copy actually listens for the trigger it carries. + Assert.True(copy.HandlesOnMovement); + var clone = Assert.Single(copy.ModernEntries); Assert.Equal("SET/Name/on spawn", clone.OnSpawnScript); Assert.Equal("SET/Name/on despawn", clone.OnDespawnScript); @@ -165,6 +192,30 @@ public void Dto_RoundTrip_CarriesEntriesTriggersAndCycleState() spawner.Delete(); } + [Fact] + public void Dto_WithTriggers_RegistersThemOnImport() + { + // ToSpawner hands back a spawner that is already running, so Start() - and with it OnStarted - + // never fires for the definitions the DTO just applied. ToSpawner has to register them itself, + // or an imported spawner's triggers stay inert until someone cycles it. + var loaded = (ModernSpawner)MakeDto(true, "proximity:8:true:false:5:0").ToSpawner(); + loaded.MoveToWorld(new Point3D(1500, 1500, 0), Map.Felucca); + + Assert.True(loaded.Running); + Assert.True(loaded.HandlesOnMovement); + + // The same import with no triggers must not arm movement dispatch. + var plain = (ModernSpawner)MakeDto(false).ToSpawner(); + plain.MoveToWorld(new Point3D(1502, 1502, 0), Map.Felucca); + + Assert.False(plain.HandlesOnMovement); + + DeleteSpawned(loaded); + loaded.Delete(); + DeleteSpawned(plain); + plain.Delete(); + } + [Fact] public void Binary_RoundTrip_RebuildsSpawnedOverModernEntries() { diff --git a/Projects/ModernSpawner/Core/ModernSpawner.Dto.cs b/Projects/ModernSpawner/Core/ModernSpawner.Dto.cs index 9e459f7..e6db313 100644 --- a/Projects/ModernSpawner/Core/ModernSpawner.Dto.cs +++ b/Projects/ModernSpawner/Core/ModernSpawner.Dto.cs @@ -176,6 +176,10 @@ public override BaseSpawner ToSpawner() } spawner.ApplyModernDto(this); + + // The spawner comes back from ApplyDto already running, so OnStarted never ran for the + // definitions ApplyModernDto just set. + spawner.EnsureTriggersActive(); return spawner; } catch diff --git a/Projects/ModernSpawner/Core/ModernSpawner.cs b/Projects/ModernSpawner/Core/ModernSpawner.cs index 218b1da..7382d44 100644 --- a/Projects/ModernSpawner/Core/ModernSpawner.cs +++ b/Projects/ModernSpawner/Core/ModernSpawner.cs @@ -43,7 +43,10 @@ public partial class ModernSpawner : Spawner private Serial _onBeforeSpawnScriptSerial; /// - /// Script serial for script executed after a successful spawn. + /// Script serial for script executed after a spawn cycle was attempted. + /// returns early when the before-spawn script cancels, when the spawner is full, or when it has + /// no entries, so this script does not run on those cycles - and it runs whether or not the + /// attempted cycle actually placed an entity. /// [SerializableField(4)] private Serial _onAfterSpawnScriptSerial; @@ -201,6 +204,12 @@ protected override void ClearEntriesCore() /// protected override void AdoptEntries(IReadOnlyList entries) { + // Adopting our own list would clear the entries we are about to copy out of it. + if (ReferenceEquals(entries, _spawnEntries)) + { + return; + } + ClearEntriesCore(); for (var i = 0; i < entries.Count; i++) { @@ -260,7 +269,9 @@ protected override SpawnerEntry CloneEntry(SpawnerEntry source) public CompiledScript OnBeforeSpawnScript => ScriptRegistry.Get(_onBeforeSpawnScriptSerial); /// - /// Gets the compiled after-spawn script, or null if not set. + /// Gets the compiled after-spawn script, or null if not set. It runs at the end of a spawn cycle + /// that was actually attempted: returns before it when the before-spawn + /// script cancels, when the spawner is full, or when it has no entries. /// public CompiledScript OnAfterSpawnScript => ScriptRegistry.Get(_onAfterSpawnScriptSerial); @@ -564,13 +575,30 @@ private void MaybeAutoResetSequence() } } - /// - protected override void OnStarted() + /// + /// Brings this spawner's trigger registrations in line with its current state, and is the only + /// caller of outside the trigger system itself. + /// appends rather than replaces, so this + /// deactivates first and is therefore safe to call any number of times. Every construction path + /// that can leave a spawner running with triggers already set - start, deserialization, dupe, + /// import, migration - ends here, because only reaches + /// when actually flips and a + /// constructed spawner is already running. + /// + internal void EnsureTriggersActive() { - if (_triggerActivated && _triggerDefinitions.Count > 0) + TriggerSystem.Instance.DeactivateTriggers(this); + + if (Running && _triggerActivated && _triggerDefinitions is { Count: > 0 }) { TriggerSystem.Instance.ActivateTriggers(this); } + } + + /// + protected override void OnStarted() + { + EnsureTriggersActive(); var activateScript = OnActivateScript; if (activateScript?.IsValid == true) @@ -579,13 +607,21 @@ protected override void OnStarted() } } - /// + /// + /// Runs the deactivate script and unregisters this spawner's triggers. Deleting a running + /// spawner reaches here as well, through calling + /// , so the deactivate script runs on deletion too. The script is + /// skipped once the item is flagged deleted, but the trigger deactivation always runs. + /// protected override void OnStopped() { - var deactivateScript = OnDeactivateScript; - if (deactivateScript?.IsValid == true) + if (!Deleted) { - ScriptEngine.Instance.Execute(deactivateScript, new ScriptContext(null, this)); + var deactivateScript = OnDeactivateScript; + if (deactivateScript?.IsValid == true) + { + ScriptEngine.Instance.Execute(deactivateScript, new ScriptContext(null, this)); + } } if (_triggerActivated) @@ -960,10 +996,7 @@ private void AfterDeserializationModernSpawner() RebuildSpawned(); // Activate triggers if spawner is running - if (Running && _triggerActivated && _triggerDefinitions.Count > 0) - { - TriggerSystem.Instance.ActivateTriggers(this); - } + EnsureTriggersActive(); } /// @@ -977,6 +1010,26 @@ private void AfterWorldLoad() // TODO: Implement extended proximity trigger support when Map APIs are available } + /// + /// Copies the modern fields the dupe contract cannot reach. The base override copies the entries; + /// is [SerializedIgnoreDupe] because the copy must own its + /// own list rather than share this one, so it is copied here and then registered. + /// + /// The freshly duped item. + public override void OnAfterDuped(Item newItem) + { + base.OnAfterDuped(newItem); + + if (newItem is not ModernSpawner copy) + { + return; + } + + // Through the generated setter so the copy is marked dirty. + copy.TriggerDefinitions = new List(_triggerDefinitions); + copy.EnsureTriggersActive(); + } + /// /// Called when this spawner is deleted. /// diff --git a/Projects/ModernSpawner/Migration/XmlSpawnerMigrator.cs b/Projects/ModernSpawner/Migration/XmlSpawnerMigrator.cs index 3556d7e..5cf0834 100644 --- a/Projects/ModernSpawner/Migration/XmlSpawnerMigrator.cs +++ b/Projects/ModernSpawner/Migration/XmlSpawnerMigrator.cs @@ -254,6 +254,10 @@ private static ModernSpawner ParseXmlSpawnerNode(XmlNode node) spawner.Start(); } + // Start() is a no-op on a spawner that was constructed running, so OnStarted never registers + // the triggers this migration just set. Register them here. + spawner.EnsureTriggersActive(); + return spawner; } @@ -299,6 +303,10 @@ private static ModernSpawner ParseSpawnPointNode(XmlNode node) spawner.Start(); } + // Start() is a no-op on a spawner that was constructed running, so OnStarted never registers + // any triggers this path set. Register them here. + spawner.EnsureTriggersActive(); + return spawner; } diff --git a/Projects/ModernSpawner/Serialization/SpawnerJsonImporter.cs b/Projects/ModernSpawner/Serialization/SpawnerJsonImporter.cs index bd38f81..44452c7 100644 --- a/Projects/ModernSpawner/Serialization/SpawnerJsonImporter.cs +++ b/Projects/ModernSpawner/Serialization/SpawnerJsonImporter.cs @@ -145,6 +145,10 @@ public static ModernSpawner CreateSpawner(SpawnerExportData data, Map map = null ImportSpawnerOptions(spawner, data.Options); } + // The spawner was constructed running, so OnStarted never ran for the imported definitions. + // Options carry TriggerActivated, so this has to come after them. + spawner.EnsureTriggersActive(); + return spawner; } @@ -213,6 +217,9 @@ public static void ConfigureSpawner(ModernSpawner spawner, SpawnerExportData dat { ImportSpawnerOptions(spawner, data.Options); } + + // Re-register: the definitions were replaced wholesale and TriggerActivated may have changed. + spawner.EnsureTriggersActive(); } private static void ImportEntry(ModernSpawner spawner, SpawnEntryData entryData) diff --git a/Projects/ModernSpawner/Serialization/XmlSpawnerImporter.cs b/Projects/ModernSpawner/Serialization/XmlSpawnerImporter.cs index ab60b22..0b14501 100644 --- a/Projects/ModernSpawner/Serialization/XmlSpawnerImporter.cs +++ b/Projects/ModernSpawner/Serialization/XmlSpawnerImporter.cs @@ -229,6 +229,9 @@ private static ModernSpawner ImportXmlSpawnerPoint(XmlElement point) spawner.TriggerActivated = true; } + // The spawner was constructed running, so OnStarted never ran for these definitions. + spawner.EnsureTriggersActive(); + return spawner; } From 4dc8d163db2ea089a8aa6389f21a40c8b7c67808 Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Fri, 11 Sep 2026 19:26:27 -0700 Subject: [PATCH 10/13] =?UTF-8?q?docs:=20architecture=20=C2=A72=20and=20pr?= =?UTF-8?q?oduct=20spec=20match=20the=20ported=20code?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit §2.2 describes trigger registration as it is: one guarded EnsureTriggersActive() called from OnStarted, [AfterDeserialization] and every construction path, with deactivation in OnStopped/OnDelete. The kill row is wired and tested - OnSpawnedDeath via BaseSpawner.NotifySpawnedDeath from BaseCreature.OnDeath. §2.4 drops SpawnArea, which no longer exists; Spawner.SpawnBounds is the one bounds and HomeRange is a computed view over it. §2.5 applies loot and the entry spawn script in OnSpawned, not SpawnFromEntry. §2.6 records that the DTO carries modern entries, triggers and cycle state and that both round trips are tested. §3's hot-path bullet drops the closure and temp-entry allocations, which the port removed. A living doc must not point at a branch: §4.3 says "implemented" instead of naming port/entry-contract and its commits, and the retired "support branch" model is gone from §5, product-spec.md §6 and the prerequisites table. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr --- dev-docs/architecture.md | 43 +++++++++++++++++++----------- dev-docs/modernuo-prerequisites.md | 2 +- dev-docs/product-spec.md | 11 ++++---- 3 files changed, 35 insertions(+), 21 deletions(-) diff --git a/dev-docs/architecture.md b/dev-docs/architecture.md index 7910cc4..a0b625f 100644 --- a/dev-docs/architecture.md +++ b/dev-docs/architecture.md @@ -67,15 +67,22 @@ triggers and runs the entry's `OnDespawnScript`. ### 2.2 Triggers (`Triggers/`) `TriggerSystem` is a singleton registry keyed by spawner with per-type lists. Triggers are parsed from -`type:field:field` strings stored on the spawner (`_triggerDefinitions`). Activation happens in -`ModernSpawner.Start()` (the `new` one) and in the synchronous `[AfterDeserialization]`; deactivation in -`Stop()`/`OnDelete()`. Wiring: +`type:field:field` strings stored on the spawner (`_triggerDefinitions`). Registration goes through one +guarded helper, `ModernSpawner.EnsureTriggersActive()`, the only caller of +`TriggerSystem.ActivateTriggers` outside the trigger system: it deactivates first and re-registers only +when the spawner is running, is `TriggerActivated` and actually has definitions, which makes it idempotent +(`ActivateTriggers` itself appends rather than replaces). `OnStarted` and `[AfterDeserialization]` call it, +and so does every construction path that hands back an already-running spawner — `OnAfterDuped`, +`ModernSpawnerDto.ToSpawner`, both JSON importer entry points, `XmlSpawnerImporter` and +`XmlSpawnerMigrator` — because `BaseSpawner.Start()` only reaches `OnStarted` when `Running` actually +flips. Deactivation is in `OnStopped` (reached by `Stop()` and, through `BaseSpawner.OnDelete`, by +deletion) and in `OnDelete`. Wiring: | Trigger | Source event | Wired | |---|---|---| | proximity | `Item.OnMovement` (24-tile radius, engine-fixed) | yes | | speech | `Item.OnSpeech` (15/18-tile radius) | yes | -| kill | `ModernSpawner.OnSpawnedEntityKilled` | no caller | +| kill | `OnSpawnedDeath` via `BaseSpawner.NotifySpawnedDeath`, called from `BaseCreature.OnDeath` | yes, tested | | skill | `ModernSpawnerEvents.OnSkillUsed` | no caller | | timeofday | 2.5 s polling timer | yes | | game_time_window | one transition timer | yes (wrong clock constant) | @@ -101,14 +108,16 @@ Spawner-level scripts are stored in `ScriptRegistry` (a `GenericPersistence` blo `PositioningRules` is a name→`IPositioningRule` registry with 14 rules. `ModernSpawner.GetSpawnPosition` replaces the base implementation entirely (losing `SpawnPositionMode`, sector cache, spiral scan, house -blocking, multi-Z search) with: entry rule → entry offset → spawn area random → "smart" random → random. -Because `HomeRange` writes into the same field as `SpawnArea`, the area branch is the normal path. +blocking, multi-Z search) with: entry rule → entry offset → spawn bounds random → "smart" random → random. +There is no separate `SpawnArea` any more: `Spawner.SpawnBounds` is the one bounds, and `HomeRange` is a +computed view over it (its setter rewrites `SpawnBounds`), so the bounds branch is the normal path. ### 2.5 Loot (`Loot/`) `LootTemplate` (guaranteed items, weighted tables, gold, clear flag) and a static in-memory -`LootTemplateRegistry` with JSON file load/save that nothing calls. Applied in `SpawnFromEntry` after the -entity exists. +`LootTemplateRegistry` with JSON file load/save that nothing calls. Applied in the `OnSpawned(entry, +spawned)` hook — together with the entry's `OnSpawnScript` — after the base spawn path has placed the +entity. ### 2.6 Serialization (`Serialization/`, `Core/ModernSpawner.Dto.cs`, `Migration/`) @@ -116,8 +125,8 @@ Five formats: | Format | Writer/Reader | Completeness | |---|---|---| -| Binary world save | generator | complete, untested | -| ModernUO `SpawnerDto` JSON | `ModernSpawner.Dto.cs` | base fields + scripts/options; **no modern entries or triggers** | +| Binary world save | generator | complete; round trip tested (`Binary_RoundTrip_RebuildsSpawnedOverModernEntries`) | +| ModernUO `SpawnerDto` JSON | `ModernSpawner.Dto.cs` | complete: base fields, `List` entries, scripts, options, trigger definitions and cycle state; round trip tested, and `ToSpawner` registers the imported triggers | | Own JSON `modernspawner/v1/spawner.json` | `SpawnerJsonExporter/Importer` | drops 8 entry fields, cooldowns; property syntax unusable (V-1) | | YAML `modernspawner/v1/script.yaml` | `ScriptYamlSerializer` | script→actions is a stub | | XmlSpawner `.xml` | `XmlSpawnerImporter` (real layout, wrong columns), `XmlSpawnerMigrator` (imaginary layouts) | partial / dead | @@ -138,8 +147,12 @@ an opt-in, zero-alloc counter set with seven `[ModernSpawnerPerf*` commands. - **Bootstrap order.** `EventScheduler.Configure` runs before world load, so scheduling during deserialization is safe; `TriggerSystem` and `ScriptRegistry` instances are created in `Configure()`. - **Hot paths.** `OnTick`→`Spawn()`, `OnMovement` (every step of every mobile within 24 tiles of a spawner - with proximity triggers), `OnSpeech`, and script execution per spawn. Today each allocates (closure, - temp entry, `TriggerContext`, `Split`/`ToLower`). + with proximity triggers), `OnSpeech`, and script execution per spawn. The closure and temp-entry + allocations the dual-list design forced on the spawn path are gone: entry selection is plain `for` loops + over `_spawnEntries` and the base `Spawn(entry, out flags)` is handed the real entry. What still + allocates per event is a `TriggerContext` (a class) on every proximity/speech/kill dispatch and a + `ScriptContext` per script execution. Trigger definition strings are `Split` only in `Parse`, once at + activation, never per event. ## 4. Target: entry ownership (D1) @@ -229,8 +242,8 @@ via `CreateEntry`/`CloneEntry`); `BaseSpawner.OnAfterDuped` and `SpawnerControll ### 4.3 Shape of A (ModernSpawner side) -This shape is implemented on branch `port/entry-contract` (commits `a3413ef`–`c447d8a`) exactly as listed -below, with two differences from the original plan noted inline. +This shape is implemented (ModernSpawner main after the port PR) exactly as listed below, with two +differences from the original plan noted inline. - `ModernSpawnerEntry : SpawnerEntry` (class inheritance; only the extra fields are declared here). Because it lives in another assembly, it must declare @@ -275,7 +288,7 @@ below, with two differences from the original plan noted inline. on `BaseSpawner`, invoked from `BaseCreature.OnDeath` before base death while the link is intact. Death is distinct from removal (taming, pickup, delete). `RequireAllDead` is evaluated after removal against the entry's remaining live count. -- **Skill.** Support-branch change: `SkillCheck` raises a generated `SkillEvents.SkillUsedEvent(Mobile, +- **Skill.** Needs a ModernUO PR: `SkillCheck` raises a generated `SkillEvents.SkillUsedEvent(Mobile, SkillName, double value, bool success)`; ModernSpawner subscribes. Until merged, `skill:` definitions are rejected at parse time with a visible error (never accepted as inert). - **Grammar.** One definition grammar owned by each trigger's `Serialize()`. Gumps and importers construct diff --git a/dev-docs/modernuo-prerequisites.md b/dev-docs/modernuo-prerequisites.md index b20ea2a..0388820 100644 --- a/dev-docs/modernuo-prerequisites.md +++ b/dev-docs/modernuo-prerequisites.md @@ -17,7 +17,7 @@ per-movement paths without a measurement, because shards run 12k+ spawners. | PR | Change | Why ModernSpawner needs it | |---|---|---| | [#2619](https://github.com/modernuo/ModernUO/pull/2619) | `BaseSpawner.Dto.cs`: `private protected` DTO helpers → `protected` | `ModernSpawner.ToDto()` lives in another assembly and needs the `Dto*` helpers and `BoundsFromHomeRange` | -| [#2621](https://github.com/modernuo/ModernUO/pull/2621) | Subclass-owned entries (`BaseSpawner` v13 / `Spawner` v2 owner contract), lifecycle hooks + `NotifySpawnedDeath`, `SpawnerEntry` v2 `Disabled`, DTO records own `entries`, save migration | D1/D11/D12: `ModernSpawner : Spawner` owns `List` with `ModernSpawnerEntry : SpawnerEntry`; kill trigger via the death hook. ModernSpawner ported on `port/entry-contract`; submodule at `a52ce6ef7` | +| [#2621](https://github.com/modernuo/ModernUO/pull/2621) | Subclass-owned entries (`BaseSpawner` v13 / `Spawner` v2 owner contract), lifecycle hooks + `NotifySpawnedDeath`, `SpawnerEntry` v2 `Disabled`, DTO records own `entries`, save migration | D1/D11/D12: `ModernSpawner : Spawner` owns `List` with `ModernSpawnerEntry : SpawnerEntry`; kill trigger via the death hook. The ModernSpawner side is ported; submodule at `a52ce6ef7` | ## Planned (see `architecture.md` §4–§5, §11; decisions D1, D2, D3, D11, D12) diff --git a/dev-docs/product-spec.md b/dev-docs/product-spec.md index 9a147f0..db2fef4 100644 --- a/dev-docs/product-spec.md +++ b/dev-docs/product-spec.md @@ -165,13 +165,14 @@ a report; live ModernUO import of the result. **Decision D0 — how a shard consumes ModernSpawner.** Recommended: source-level. The shard adds this repository as a submodule beside their ModernUO checkout (or inside it under `Projects/ModernSpawner`) and -adds the project to their solution. Reasons: ModernSpawner requires engine changes that live on the ModernUO -support branch until the first release; ModernUO's `Directory.Build.props` pins RIDs and the serialization -generator per-project; and code-generated events and `protected` hooks do not survive a DLL boundary well. +adds the project to their solution. Reasons: ModernSpawner requires engine changes that reach ModernUO as +individual upstream pull requests, so until each one merges the submodule is pinned to that PR's head; +ModernUO's `Directory.Build.props` pins RIDs and the serialization generator per-project; and +code-generated events and `protected` hooks do not survive a DLL boundary well. A prebuilt DLL into `Distribution/Assemblies` remains possible for shards on a ModernUO release that already contains every prerequisite, and `AssemblyHandler` discovers `Configure`/`Initialize`, commands and -`[JsonDiscoverableType]` in it, but it is not the primary path for v1. (The sentence above about a -"support branch" is historical: prerequisites now go upstream as individual ModernUO PRs.) +`[JsonDiscoverableType]` in it, but it is not the primary path for v1. (Engine prerequisites and the PR each +one is waiting on are listed in `modernuo-prerequisites.md`.) ## 7. Compatibility From b4d5313ef9771c41364cce33aa549ea312412d89 Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Fri, 11 Sep 2026 19:29:13 -0700 Subject: [PATCH 11/13] fix(core): OnStopped runs the deactivate script unconditionally; the Deleted guard could never fire Item.Delete() calls OnDelete() before it sets the Deleted flag, so on the deletion path - Item.Delete -> BaseSpawner.OnDelete -> Stop() -> OnStopped - Deleted is still false and the guard added in a8e5981 was dead code. Removed it and kept the doc sentence: deleting a running spawner does run the OnDeactivate script, and nothing in OnStopped's state distinguishes a stop from a deletion. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr --- Projects/ModernSpawner/Core/ModernSpawner.cs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/Projects/ModernSpawner/Core/ModernSpawner.cs b/Projects/ModernSpawner/Core/ModernSpawner.cs index 7382d44..ff7cdc2 100644 --- a/Projects/ModernSpawner/Core/ModernSpawner.cs +++ b/Projects/ModernSpawner/Core/ModernSpawner.cs @@ -610,18 +610,16 @@ protected override void OnStarted() /// /// Runs the deactivate script and unregisters this spawner's triggers. Deleting a running /// spawner reaches here as well, through calling - /// , so the deactivate script runs on deletion too. The script is - /// skipped once the item is flagged deleted, but the trigger deactivation always runs. + /// , so the deactivate script runs on deletion too: + /// sets only after + /// has returned, so there is no state here that distinguishes a stop from a deletion. /// protected override void OnStopped() { - if (!Deleted) + var deactivateScript = OnDeactivateScript; + if (deactivateScript?.IsValid == true) { - var deactivateScript = OnDeactivateScript; - if (deactivateScript?.IsValid == true) - { - ScriptEngine.Instance.Execute(deactivateScript, new ScriptContext(null, this)); - } + ScriptEngine.Instance.Execute(deactivateScript, new ScriptContext(null, this)); } if (_triggerActivated) From 9d1f5c8657430dc614738b745cfaded51dada99f Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Fri, 11 Sep 2026 19:37:13 -0700 Subject: [PATCH 12/13] docs: per-event allocation list is not exhaustive Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr --- dev-docs/architecture.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-docs/architecture.md b/dev-docs/architecture.md index a0b625f..cc5f53a 100644 --- a/dev-docs/architecture.md +++ b/dev-docs/architecture.md @@ -150,7 +150,7 @@ an opt-in, zero-alloc counter set with seven `[ModernSpawnerPerf*` commands. with proximity triggers), `OnSpeech`, and script execution per spawn. The closure and temp-entry allocations the dual-list design forced on the spawn path are gone: entry selection is plain `for` loops over `_spawnEntries` and the base `Spawn(entry, out flags)` is handed the real entry. What still - allocates per event is a `TriggerContext` (a class) on every proximity/speech/kill dispatch and a + allocates per event includes a `TriggerContext` (a class) on every proximity/speech/kill dispatch and a `ScriptContext` per script execution. Trigger definition strings are `Split` only in `Parse`, once at activation, never per event. From 69033d18aa06d358148051d4e5aa8974884a1b1b Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Fri, 11 Sep 2026 19:38:21 -0700 Subject: [PATCH 13/13] docs: link the ModernSpawner port PR from the prerequisites table Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr --- dev-docs/modernuo-prerequisites.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-docs/modernuo-prerequisites.md b/dev-docs/modernuo-prerequisites.md index 0388820..be8d78d 100644 --- a/dev-docs/modernuo-prerequisites.md +++ b/dev-docs/modernuo-prerequisites.md @@ -17,7 +17,7 @@ per-movement paths without a measurement, because shards run 12k+ spawners. | PR | Change | Why ModernSpawner needs it | |---|---|---| | [#2619](https://github.com/modernuo/ModernUO/pull/2619) | `BaseSpawner.Dto.cs`: `private protected` DTO helpers → `protected` | `ModernSpawner.ToDto()` lives in another assembly and needs the `Dto*` helpers and `BoundsFromHomeRange` | -| [#2621](https://github.com/modernuo/ModernUO/pull/2621) | Subclass-owned entries (`BaseSpawner` v13 / `Spawner` v2 owner contract), lifecycle hooks + `NotifySpawnedDeath`, `SpawnerEntry` v2 `Disabled`, DTO records own `entries`, save migration | D1/D11/D12: `ModernSpawner : Spawner` owns `List` with `ModernSpawnerEntry : SpawnerEntry`; kill trigger via the death hook. The ModernSpawner side is ported; submodule at `a52ce6ef7` | +| [#2621](https://github.com/modernuo/ModernUO/pull/2621) | Subclass-owned entries (`BaseSpawner` v13 / `Spawner` v2 owner contract), lifecycle hooks + `NotifySpawnedDeath`, `SpawnerEntry` v2 `Disabled`, DTO records own `entries`, save migration | D1/D11/D12: `ModernSpawner : Spawner` owns `List` with `ModernSpawnerEntry : SpawnerEntry`; kill trigger via the death hook. The ModernSpawner side is ported in [ModernSpawner #1](https://github.com/modernuo/ModernSpawner/pull/1); submodule at `a52ce6ef7` | ## Planned (see `architecture.md` §4–§5, §11; decisions D1, D2, D3, D11, D12)