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/ModernUO b/ModernUO index 4bad0cc..a52ce6e 160000 --- a/ModernUO +++ b/ModernUO @@ -1 +1 @@ -Subproject commit 4bad0cc9e6c2ac0d2ddf492adc667903f3ba9f71 +Subproject commit a52ce6ef705184117dbd008c42dcad577c99add4 diff --git a/Projects/ModernSpawner.Tests/Core/ModernSpawnerLifecycleTests.cs b/Projects/ModernSpawner.Tests/Core/ModernSpawnerLifecycleTests.cs new file mode 100644 index 0000000..9d7fad2 --- /dev/null +++ b/Projects/ModernSpawner.Tests/Core/ModernSpawnerLifecycleTests.cs @@ -0,0 +1,279 @@ +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(); + } + } + + 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() + { + 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"); + + // 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; + + // [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); + 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(); + } + + [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 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() + { + 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"); + + // 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(); + + // 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); + 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/Core/ModernSpawner.Dto.cs b/Projects/ModernSpawner/Core/ModernSpawner.Dto.cs index 2e13bd8..e6db313 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() @@ -140,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 38a7ab4..ff7cdc2 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. @@ -41,7 +43,10 @@ public partial class ModernSpawner : BaseSpawner 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; @@ -71,6 +76,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 +94,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 +112,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 +122,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 +137,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 +152,106 @@ 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) + { + // Adopting our own list would clear the entries we are about to copy out of it. + if (ReferenceEquals(entries, _spawnEntries)) + { + return; + } - /// - /// Returns the bounds to use for a single spawn attempt. - /// - protected override Rectangle3D GetBoundsForSpawnAttempt() => SpawnBounds; + 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 all possible spawn bounds for cache operations. - /// - protected override ReadOnlySpan GetAllSpawnBounds() => new(ref _spawnArea); + entry.SetParent(this); + AddEntryCore(entry); + } + } + + /// + 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. @@ -222,7 +269,9 @@ public override Rectangle3D SpawnBounds 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); @@ -305,36 +354,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 +370,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; + } + + MaybeAutoResetSequence(); - using (SpawnerMetrics.MeasureEntrySelection()) + 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 +434,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 +472,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 +490,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. @@ -629,108 +576,146 @@ private void MaybeAutoResetSequence() } /// - /// Spawns from a specific modern entry and executes entry-level scripts. + /// 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. /// - public bool SpawnFromEntry(ModernSpawnerEntry entry, out EntryFlags flags) + internal void EnsureTriggersActive() { - using var _ = SpawnerMetrics.MeasureSpawnFromEntry(); + TriggerSystem.Instance.DeactivateTriggers(this); - flags = EntryFlags.None; - - if (entry == null) + if (Running && _triggerActivated && _triggerDefinitions is { 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); + /// + protected override void OnStarted() + { + EnsureTriggersActive(); - 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); - } - } + /// + /// 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: + /// sets only after + /// has returned, so there is no state here that distinguishes a stop from a deletion. + /// + 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 +736,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 +746,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,30 +990,11 @@ 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) - { - TriggerSystem.Instance.ActivateTriggers(this); - } + EnsureTriggersActive(); } /// @@ -1041,61 +1009,23 @@ private void AfterWorldLoad() } /// - /// Called when the spawner starts running. Activates triggers. - /// - public new void Start() - { - base.Start(); - OnSpawnerStarted(); - } - - /// - /// Called when the spawner stops running. Deactivates triggers. + /// 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. /// - public new void Stop() + /// The freshly duped item. + public override void OnAfterDuped(Item newItem) { - OnSpawnerStopping(); - base.Stop(); - } + base.OnAfterDuped(newItem); - /// - /// Hook called after the spawner has started. - /// - private void OnSpawnerStarted() - { - // Activate triggers when spawner starts - if (_triggerActivated && _triggerDefinitions.Count > 0) + if (newItem is not ModernSpawner copy) { - 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); + return; } - // Deactivate triggers when spawner stops - if (_triggerActivated) - { - TriggerSystem.Instance.DeactivateTriggers(this); - } + // Through the generated setter so the copy is marked dirty. + copy.TriggerDefinitions = new List(_triggerDefinitions); + copy.EnsureTriggersActive(); } /// @@ -1120,7 +1050,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 +1063,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..5cf0834 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) ); @@ -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/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..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.ModernSpawned) + 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++; 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/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 { 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..44452c7 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); } @@ -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; } @@ -179,14 +183,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) @@ -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 a50589d..0b14501 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 { @@ -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; } diff --git a/dev-docs/architecture.md b/dev-docs/architecture.md index df7c832..cc5f53a 100644 --- a/dev-docs/architecture.md +++ b/dev-docs/architecture.md @@ -37,39 +37,52 @@ 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/`) `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) | @@ -95,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/`) @@ -110,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 | @@ -125,15 +140,19 @@ 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 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 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. ## 4. Target: entry ownership (D1) @@ -141,92 +160,109 @@ 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**, 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 +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) +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) +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 `[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) @@ -248,11 +284,11 @@ construction and property application, before `GetSpawnPosition`. `[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. -- **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 @@ -330,14 +366,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 +393,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..be8d78d 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. 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) - `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. 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