From 973e8020e482cca0aaacaa50451380d79ca5e736 Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Fri, 11 Sep 2026 20:47:49 -0700 Subject: [PATCH 1/8] fix(triggers): registration follows TriggerActivated and gump edits; deactivate unconditionally on stop and delete Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr --- .../ModernSpawnerTriggerRegistrationTests.cs | 94 +++++++++++++++++++ Projects/ModernSpawner/Core/ModernSpawner.cs | 40 +++++--- .../ModernSpawner/Gumps/TriggerConfigGump.cs | 8 +- .../Serialization/SpawnerJsonImporter.cs | 8 +- .../ModernSpawner/Triggers/TriggerSystem.cs | 6 ++ 5 files changed, 141 insertions(+), 15 deletions(-) create mode 100644 Projects/ModernSpawner.Tests/Core/ModernSpawnerTriggerRegistrationTests.cs diff --git a/Projects/ModernSpawner.Tests/Core/ModernSpawnerTriggerRegistrationTests.cs b/Projects/ModernSpawner.Tests/Core/ModernSpawnerTriggerRegistrationTests.cs new file mode 100644 index 0000000..fa34348 --- /dev/null +++ b/Projects/ModernSpawner.Tests/Core/ModernSpawnerTriggerRegistrationTests.cs @@ -0,0 +1,94 @@ +using System; +using Server.Engines.ModernSpawner.Triggers; +using Xunit; + +namespace Server.Engines.ModernSpawner.Tests; + +/// +/// Trigger registration follows every flag and list change: the +/// setter registers and unregisters immediately, and stopping or deleting a spawner tears the registration +/// down regardless of what the flag says at that moment. +/// +[Collection("Sequential ModernSpawner Tests")] +public class ModernSpawnerTriggerRegistrationTests +{ + private const string Proximity = "proximity:8:true"; + + private static ModernSpawner Place() + { + var spawner = new ModernSpawner(1, TimeSpan.FromMinutes(5), TimeSpan.FromMinutes(10), 0, default, "Rabbit"); + spawner.MoveToWorld(new Point3D(1500, 1500, 0), Map.Felucca); + return spawner; + } + + [Fact] + public void TogglingTriggerActivated_RegistersAndUnregisters() + { + var spawner = Place(); + spawner.AddToTriggerDefinitions(Proximity); + Assert.False(spawner.HandlesOnMovement); + + spawner.TriggerActivated = true; + Assert.True(spawner.HandlesOnMovement); + + spawner.TriggerActivated = false; + Assert.False(spawner.HandlesOnMovement); + spawner.Delete(); + } + + [Fact] + public void AddingDefinitionToActivatedRunningSpawner_RegistersImmediately() + { + var spawner = Place(); + spawner.TriggerActivated = true; + Assert.False(spawner.HandlesOnMovement); + + spawner.AddToTriggerDefinitions(Proximity); + spawner.EnsureTriggersActive(); + Assert.True(spawner.HandlesOnMovement); + + spawner.RemoveFromTriggerDefinitions(Proximity); + spawner.EnsureTriggersActive(); + Assert.False(spawner.HandlesOnMovement); + spawner.Delete(); + } + + [Fact] + public void ClearingTriggerActivated_ThenDeleting_LeavesNothingRegistered() + { + var spawner = Place(); + spawner.AddToTriggerDefinitions(Proximity); + spawner.TriggerActivated = true; + Assert.True(spawner.HandlesOnMovement); + + // Flag cleared through the raw field path the gump used to take: registration must still be torn down. + spawner.TriggerActivated = false; + spawner.TriggerActivated = true; + spawner.Delete(); + + Assert.False(TriggerSystem.Instance.IsRegistered(spawner)); + } + + [Fact] + public void Stop_UnregistersEvenWhenFlagWasClearedAfterRegistration() + { + var spawner = Place(); + spawner.AddToTriggerDefinitions(Proximity); + spawner.TriggerActivated = true; + spawner.Stop(); + Assert.False(TriggerSystem.Instance.IsRegistered(spawner)); + + // A registration that outlived its flag - the state a raw field write or a pre-fix gump edit + // could leave behind. Teardown does not consult the flag, so it still has to be cleaned up. + spawner.TriggerActivated = false; + spawner.Start(); + Assert.False(TriggerSystem.Instance.IsRegistered(spawner)); + + TriggerSystem.Instance.ActivateTriggers(spawner); + Assert.True(TriggerSystem.Instance.IsRegistered(spawner)); + + spawner.Stop(); + Assert.False(TriggerSystem.Instance.IsRegistered(spawner)); + spawner.Delete(); + } +} diff --git a/Projects/ModernSpawner/Core/ModernSpawner.cs b/Projects/ModernSpawner/Core/ModernSpawner.cs index ff7cdc2..ccdc1a5 100644 --- a/Projects/ModernSpawner/Core/ModernSpawner.cs +++ b/Projects/ModernSpawner/Core/ModernSpawner.cs @@ -81,11 +81,28 @@ public partial class ModernSpawner : Spawner private List _triggerDefinitions = []; /// - /// Whether this spawner is trigger-activated (vs. timer-based). + /// Whether this spawner is trigger-activated (vs. timer-based). Master switch for this spawner's + /// trigger definitions: setting it registers or unregisters the triggers immediately through + /// , so there is no window where the flag and the trigger + /// registry disagree. The backing field is generated; serialization order 9 is unchanged. /// - [SerializableField(9)] - [SerializedCommandProperty(AccessLevel.Developer)] - private bool _triggerActivated; + [SerializableProperty(9)] + [CommandProperty(AccessLevel.Developer)] + public bool TriggerActivated + { + get => _triggerActivated; + set + { + if (_triggerActivated == value) + { + return; + } + + _triggerActivated = value; + this.MarkDirty(); + EnsureTriggersActive(); + } + } /// /// External trigger state - set by trigger system. @@ -622,10 +639,9 @@ protected override void OnStopped() ScriptEngine.Instance.Execute(deactivateScript, new ScriptContext(null, this)); } - if (_triggerActivated) - { - TriggerSystem.Instance.DeactivateTriggers(this); - } + // DeactivateTriggers is a no-op when nothing is registered, so no flag check: the flag can be + // cleared after registration and must not leave a stale entry behind. + TriggerSystem.Instance.DeactivateTriggers(this); } /// @@ -1036,11 +1052,9 @@ public override void OnDelete() // Unsubscribe from extended area movement before deletion UnsubscribeFromExtendedAreaMovement(); - // Deactivate triggers before deletion - if (_triggerActivated) - { - TriggerSystem.Instance.DeactivateTriggers(this); - } + // Deactivate triggers before deletion. DeactivateTriggers is a no-op when nothing is registered, + // so no flag check: the flag can be cleared after registration and must not leave a stale entry behind. + TriggerSystem.Instance.DeactivateTriggers(this); base.OnDelete(); } diff --git a/Projects/ModernSpawner/Gumps/TriggerConfigGump.cs b/Projects/ModernSpawner/Gumps/TriggerConfigGump.cs index 29ed3c5..a3f43b2 100644 --- a/Projects/ModernSpawner/Gumps/TriggerConfigGump.cs +++ b/Projects/ModernSpawner/Gumps/TriggerConfigGump.cs @@ -280,6 +280,7 @@ public override void OnResponse(NetState state, in RelayInfo info) range = Math.Max(1, parsedRange); } _spawner.AddToTriggerDefinitions($"proximity:{range}:true"); + _spawner.EnsureTriggersActive(); from.SendMessage($"Added proximity trigger with {range} tile range."); break; } @@ -299,12 +300,14 @@ public override void OnResponse(NetState state, in RelayInfo info) endHour = Math.Clamp(parsedEnd, 0, 23); } _spawner.AddToTriggerDefinitions($"walltime:{startHour}:{endHour}"); + _spawner.EnsureTriggersActive(); from.SendMessage($"Added time window trigger: {startHour}:00 - {endHour}:00."); break; } case ButtonId_AddGameTime: _spawner.AddToTriggerDefinitions("gametime:night"); + _spawner.EnsureTriggersActive(); from.SendMessage("Added game time trigger for night hours."); break; @@ -315,7 +318,10 @@ public override void OnResponse(NetState state, in RelayInfo info) var deleteIndex = info.ButtonID - ButtonId_DeleteBase; if (deleteIndex >= 0 && deleteIndex < triggers.Count) { - triggers.RemoveAt(deleteIndex); + // triggers is the live list: remove through the generated index helper so the + // spawner is marked dirty and duplicate definitions still delete by position. + _spawner.RemoveFromTriggerDefinitionsAt(deleteIndex); + _spawner.EnsureTriggersActive(); from.SendMessage("Trigger removed."); } } diff --git a/Projects/ModernSpawner/Serialization/SpawnerJsonImporter.cs b/Projects/ModernSpawner/Serialization/SpawnerJsonImporter.cs index 44452c7..22ea7f5 100644 --- a/Projects/ModernSpawner/Serialization/SpawnerJsonImporter.cs +++ b/Projects/ModernSpawner/Serialization/SpawnerJsonImporter.cs @@ -202,7 +202,13 @@ public static void ConfigureSpawner(ModernSpawner spawner, SpawnerExportData dat // Import triggers (clears existing) if (data.Triggers != null) { - spawner.TriggerDefinitions?.Clear(); + // Through the generated helper so the spawner is marked dirty; it dereferences the list, + // so the null check stays. + if (spawner.TriggerDefinitions != null) + { + spawner.ClearTriggerDefinitions(); + } + ImportTriggers(spawner, data.Triggers); } diff --git a/Projects/ModernSpawner/Triggers/TriggerSystem.cs b/Projects/ModernSpawner/Triggers/TriggerSystem.cs index 5527095..9914d76 100644 --- a/Projects/ModernSpawner/Triggers/TriggerSystem.cs +++ b/Projects/ModernSpawner/Triggers/TriggerSystem.cs @@ -101,6 +101,12 @@ public void ActivateTriggers(ModernSpawner spawner) } } + /// + /// Whether currently has triggers registered with this system, i.e. whether + /// has run for it without a matching . + /// + internal bool IsRegistered(ModernSpawner spawner) => spawner != null && _allTriggers.ContainsKey(spawner); + public void DeactivateTriggers(ModernSpawner spawner) { if (spawner == null) From b727743c1d411ae5d5b58822ba9925c29179893d Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Fri, 11 Sep 2026 20:57:20 -0700 Subject: [PATCH 2/8] test(triggers): deleting a stopped spawner with a stale registration unregisters it Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr --- .../ModernSpawnerTriggerRegistrationTests.cs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/Projects/ModernSpawner.Tests/Core/ModernSpawnerTriggerRegistrationTests.cs b/Projects/ModernSpawner.Tests/Core/ModernSpawnerTriggerRegistrationTests.cs index fa34348..4b12a83 100644 --- a/Projects/ModernSpawner.Tests/Core/ModernSpawnerTriggerRegistrationTests.cs +++ b/Projects/ModernSpawner.Tests/Core/ModernSpawnerTriggerRegistrationTests.cs @@ -54,14 +54,13 @@ public void AddingDefinitionToActivatedRunningSpawner_RegistersImmediately() } [Fact] - public void ClearingTriggerActivated_ThenDeleting_LeavesNothingRegistered() + public void DeletingAnActivatedSpawner_LeavesNothingRegistered() { var spawner = Place(); spawner.AddToTriggerDefinitions(Proximity); spawner.TriggerActivated = true; Assert.True(spawner.HandlesOnMovement); - // Flag cleared through the raw field path the gump used to take: registration must still be torn down. spawner.TriggerActivated = false; spawner.TriggerActivated = true; spawner.Delete(); @@ -91,4 +90,17 @@ public void Stop_UnregistersEvenWhenFlagWasClearedAfterRegistration() Assert.False(TriggerSystem.Instance.IsRegistered(spawner)); spawner.Delete(); } + + [Fact] + public void DeletingAStoppedSpawner_WithStaleRegistration_Unregisters() + { + var spawner = Place(); + spawner.AddToTriggerDefinitions(Proximity); + spawner.Stop(); // Running false: OnStopped is out of the picture + TriggerSystem.Instance.ActivateTriggers(spawner); // stale registration behind a false flag + Assert.True(TriggerSystem.Instance.IsRegistered(spawner)); + + spawner.Delete(); // only OnDelete can clean this up + Assert.False(TriggerSystem.Instance.IsRegistered(spawner)); + } } From 64b176b160ee58d38cbf1e9147b6abd680d09e64 Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Fri, 11 Sep 2026 21:07:50 -0700 Subject: [PATCH 3/8] fix(migration): honour Running=false when migrating XmlSpawner nodes Both migrator construction paths were constructed running and only ever called Start(); a "Running=false" node had nothing to stop it, so the spawner stayed running (and its triggers stayed registered) regardless of what the file said. Both paths now call Stop() when the file's Running flag is false, matching the ParseSpawnPointNode form's default of true when the attribute is absent, then re-derive trigger registration through EnsureTriggersActive(). Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr --- .../ModernSpawnerTriggerRegistrationTests.cs | 56 +++++++++++++++++++ .../Migration/XmlSpawnerMigrator.cs | 28 +++++++--- 2 files changed, 76 insertions(+), 8 deletions(-) diff --git a/Projects/ModernSpawner.Tests/Core/ModernSpawnerTriggerRegistrationTests.cs b/Projects/ModernSpawner.Tests/Core/ModernSpawnerTriggerRegistrationTests.cs index 4b12a83..cf1ad62 100644 --- a/Projects/ModernSpawner.Tests/Core/ModernSpawnerTriggerRegistrationTests.cs +++ b/Projects/ModernSpawner.Tests/Core/ModernSpawnerTriggerRegistrationTests.cs @@ -1,4 +1,6 @@ using System; +using System.Xml; +using Server.Engines.ModernSpawner.Migration; using Server.Engines.ModernSpawner.Triggers; using Xunit; @@ -103,4 +105,58 @@ public void DeletingAStoppedSpawner_WithStaleRegistration_Unregisters() spawner.Delete(); // only OnDelete can clean this up Assert.False(TriggerSystem.Instance.IsRegistered(spawner)); } + + private static XmlNode ParseNode(string xml) + { + var doc = new XmlDocument(); + doc.LoadXml(xml); + return doc.DocumentElement; + } + + // ProximityRange is the attribute ParseXmlSpawnerNode maps to a proximity trigger definition plus + // TriggerActivated = true, so this form exercises trigger (de)registration alongside Running. + private static string XmlSpawnerNode(string running) + { + var runningAttribute = running != null ? $" Running=\"{running}\"" : string.Empty; + return ""; + } + + // The SpawnPoint form has no trigger-mapped attribute, so these tests assert Running only. + private static string SpawnPointNode(string running) + { + var runningAttribute = running != null ? $" Running=\"{running}\"" : string.Empty; + return ""; + } + + [Fact] + public void Migrator_RunningFalse_ProducesStoppedSpawnerWithNoRegisteredTriggers() + { + var stopped = XmlSpawnerMigrator.ParseXmlSpawnerNode(ParseNode(XmlSpawnerNode("false"))); + Assert.False(stopped.Running); + Assert.False(TriggerSystem.Instance.IsRegistered(stopped)); + stopped.Delete(); + + // Running="true" (the same construction path) must still register and run, so the fix for the + // false case did not just make everything stop. + var running = XmlSpawnerMigrator.ParseXmlSpawnerNode(ParseNode(XmlSpawnerNode("true"))); + Assert.True(running.Running); + Assert.True(TriggerSystem.Instance.IsRegistered(running)); + running.Delete(); + } + + [Fact] + public void SpawnPointMigrator_RunningFalse_ProducesStoppedSpawner() + { + var stopped = XmlSpawnerMigrator.ParseSpawnPointNode(ParseNode(SpawnPointNode("false"))); + Assert.False(stopped.Running); + stopped.Delete(); + + // Running absent defaults to true - the historical "always start" behavior for a form that had + // no Running attribute before this fix. + var running = XmlSpawnerMigrator.ParseSpawnPointNode(ParseNode(SpawnPointNode(null))); + Assert.True(running.Running); + running.Delete(); + } } diff --git a/Projects/ModernSpawner/Migration/XmlSpawnerMigrator.cs b/Projects/ModernSpawner/Migration/XmlSpawnerMigrator.cs index 5cf0834..139fab3 100644 --- a/Projects/ModernSpawner/Migration/XmlSpawnerMigrator.cs +++ b/Projects/ModernSpawner/Migration/XmlSpawnerMigrator.cs @@ -128,7 +128,7 @@ public static (int success, int failed) ImportFromFile(string path) /// /// Parses an XmlSpawner node from the save format. /// - private static ModernSpawner ParseXmlSpawnerNode(XmlNode node) + internal static ModernSpawner ParseXmlSpawnerNode(XmlNode node) { // Parse location var x = GetIntAttribute(node, "X", 0); @@ -247,15 +247,20 @@ private static ModernSpawner ParseXmlSpawnerNode(XmlNode node) // Place the spawner spawner.MoveToWorld(new Point3D(x, y, z), map); - // Start if it was running + // Start if it was running, stop otherwise - the spawner is constructed already running, so + // "Running=false" (or no entries to run with) has to be applied explicitly. var running = GetBoolAttribute(node, "Running", true); if (running && spawner.Entries.Count > 0) { spawner.Start(); } + else + { + spawner.Stop(); + } - // 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. + // Start()/Stop() only reach OnStarted/OnStopped when Running flips; the spawner was constructed + // running, so register (or unregister) explicitly for the state the file asked for. spawner.EnsureTriggersActive(); return spawner; @@ -264,7 +269,7 @@ private static ModernSpawner ParseXmlSpawnerNode(XmlNode node) /// /// Parses a SpawnPoint node (alternative XmlSpawner export format). /// - private static ModernSpawner ParseSpawnPointNode(XmlNode node) + internal static ModernSpawner ParseSpawnPointNode(XmlNode node) { var x = GetIntAttribute(node, "X", 0); var y = GetIntAttribute(node, "Y", 0); @@ -298,13 +303,20 @@ private static ModernSpawner ParseSpawnPointNode(XmlNode node) spawner.MoveToWorld(new Point3D(x, y, z), map); - if (spawner.Entries.Count > 0) + // This node form has no dedicated attribute for stopped spawners in the wild, but honour one if + // present; default true preserves the historical "always start" behavior when it is absent. + var running = GetBoolAttribute(node, "Running", true); + if (running && spawner.Entries.Count > 0) { spawner.Start(); } + else + { + spawner.Stop(); + } - // Start() is a no-op on a spawner that was constructed running, so OnStarted never registers - // any triggers this path set. Register them here. + // Start()/Stop() only reach OnStarted/OnStopped when Running flips; the spawner was constructed + // running, so register (or unregister) explicitly for the state the file asked for. spawner.EnsureTriggersActive(); return spawner; From 857281b970846caefbc34aea2d38bd0a8bac5cc5 Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Fri, 11 Sep 2026 21:15:38 -0700 Subject: [PATCH 4/8] refactor: no nullable value types in the importer, skill trigger, or script context Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr --- .../Core/XmlSpawnerImporterEntryDelayTests.cs | 108 ++++++++++++++++++ .../ModernSpawner/Scripting/ScriptContext.cs | 18 ++- .../Serialization/XmlSpawnerImporter.cs | 8 +- .../ModernSpawner/Triggers/SkillTrigger.cs | 9 +- 4 files changed, 134 insertions(+), 9 deletions(-) create mode 100644 Projects/ModernSpawner.Tests/Core/XmlSpawnerImporterEntryDelayTests.cs diff --git a/Projects/ModernSpawner.Tests/Core/XmlSpawnerImporterEntryDelayTests.cs b/Projects/ModernSpawner.Tests/Core/XmlSpawnerImporterEntryDelayTests.cs new file mode 100644 index 0000000..b21a038 --- /dev/null +++ b/Projects/ModernSpawner.Tests/Core/XmlSpawnerImporterEntryDelayTests.cs @@ -0,0 +1,108 @@ +using System; +using System.IO; +using Server.Engines.ModernSpawner.Serialization; +using Xunit; + +namespace Server.Engines.ModernSpawner.Tests; + +/// +/// Covers the sentinel-based min/max delay handling in +/// 's entry parsing (Task 3: no nullable value types). This needs a +/// resolvable , so it runs against the real world in the sequential collection +/// rather than as a pure unit test. +/// +[Collection("Sequential ModernSpawner Tests")] +public class XmlSpawnerImporterEntryDelayTests +{ + private static string BuildXml(string name, string objects2) => $""" + + + {name} + Felucca + 1500 + 1500 + 0 + 1500 + 1500 + 0 + 0 + 4 + 5 + 5 + 10 + False + -1 + 0 + False + False + False + {objects2} + + + """; + + private static ModernSpawner FindByName(string name) + { + foreach (var item in World.Items.Values) + { + if (item is ModernSpawner spawner && spawner.Name == name) + { + return spawner; + } + } + + Assert.Fail($"No imported ModernSpawner named '{name}' was found in the world."); + return null; + } + + [Fact] + public void ParseEntry_NoDelayTokens_LeavesEntryAtSpawnerDefault() + { + var name = "ImporterDelayTest-" + Guid.NewGuid(); + var tempFile = Path.GetTempFileName(); + try + { + File.WriteAllText(tempFile, BuildXml(name, "Rabbit:MX=3")); + + var result = XmlSpawnerImporter.ImportFromFile(tempFile, respawn: false); + Assert.Equal(1, result.Imported); + + var spawner = FindByName(name); + var entry = Assert.Single(spawner.ModernEntries); + + Assert.Equal(TimeSpan.Zero, entry.MinDelay); + Assert.Equal(spawner.MinDelay, entry.EffectiveMinDelay); + + spawner.Delete(); + } + finally + { + File.Delete(tempFile); + } + } + + [Fact] + public void ParseEntry_DnToken_SetsMinDelayInMinutes() + { + var name = "ImporterDelayTest-" + Guid.NewGuid(); + var tempFile = Path.GetTempFileName(); + try + { + File.WriteAllText(tempFile, BuildXml(name, "Rabbit:MX=3:DN=2")); + + var result = XmlSpawnerImporter.ImportFromFile(tempFile, respawn: false); + Assert.Equal(1, result.Imported); + + var spawner = FindByName(name); + var entry = Assert.Single(spawner.ModernEntries); + + Assert.Equal(TimeSpan.FromMinutes(2), entry.MinDelay); + + spawner.Delete(); + } + finally + { + File.Delete(tempFile); + } + } +} diff --git a/Projects/ModernSpawner/Scripting/ScriptContext.cs b/Projects/ModernSpawner/Scripting/ScriptContext.cs index b8170ae..2714adb 100644 --- a/Projects/ModernSpawner/Scripting/ScriptContext.cs +++ b/Projects/ModernSpawner/Scripting/ScriptContext.cs @@ -53,11 +53,23 @@ public class ScriptContext /// public bool CancelSpawn { get; set; } + /// True when a script set . + public bool HasLocationOverride { get; private set; } + + private Point3D _locationOverride; + /// - /// Gets or sets custom spawn location override. - /// If set, the entity will be placed at this location instead of the calculated position. + /// Custom spawn location a script may set; consult before reading. /// - public Point3D? LocationOverride { get; set; } + public Point3D LocationOverride + { + get => _locationOverride; + set + { + _locationOverride = value; + HasLocationOverride = true; + } + } public ScriptContext(ModernSpawner spawner, ModernSpawnerEntry entry = null, IEntity target = null) { diff --git a/Projects/ModernSpawner/Serialization/XmlSpawnerImporter.cs b/Projects/ModernSpawner/Serialization/XmlSpawnerImporter.cs index 0b14501..1d4bd8c 100644 --- a/Projects/ModernSpawner/Serialization/XmlSpawnerImporter.cs +++ b/Projects/ModernSpawner/Serialization/XmlSpawnerImporter.cs @@ -291,8 +291,8 @@ private static void ParseEntry(ModernSpawner spawner, string entryStr, int defau var maxCount = defaultMaxCount; var probability = 100; var subgroup = 0; - TimeSpan? minDelay = null; - TimeSpan? maxDelay = null; + var minDelay = TimeSpan.Zero; + var maxDelay = TimeSpan.Zero; string properties = null; for (var i = 1; i < parts.Length; i++) @@ -371,8 +371,8 @@ private static void ParseEntry(ModernSpawner spawner, string entryStr, int defau maxCount: maxCount, properties: properties, parameters: parameters, - minDelay: minDelay ?? TimeSpan.Zero, - maxDelay: maxDelay ?? TimeSpan.Zero, + minDelay: minDelay, + maxDelay: maxDelay, dotimer: false ); entry.Subgroup = subgroup; diff --git a/Projects/ModernSpawner/Triggers/SkillTrigger.cs b/Projects/ModernSpawner/Triggers/SkillTrigger.cs index af84396..37e0733 100644 --- a/Projects/ModernSpawner/Triggers/SkillTrigger.cs +++ b/Projects/ModernSpawner/Triggers/SkillTrigger.cs @@ -41,13 +41,18 @@ public class SkillTrigger : ITrigger private ModernSpawner _spawner; private DateTime _lastTriggered; - public SkillTrigger(SkillName skill, int range = 10, double minSkillValue = 0, bool requireLOS = false, TimeSpan? cooldown = null) + public SkillTrigger(SkillName skill, int range = 10, double minSkillValue = 0, bool requireLOS = false) + : this(skill, range, minSkillValue, requireLOS, TimeSpan.FromSeconds(5)) + { + } + + public SkillTrigger(SkillName skill, int range, double minSkillValue, bool requireLOS, TimeSpan cooldown) { TargetSkill = skill; Range = Math.Max(1, range); MinSkillValue = minSkillValue; RequireLOS = requireLOS; - Cooldown = cooldown ?? TimeSpan.FromSeconds(5); + Cooldown = cooldown; } public void Activate(ModernSpawner spawner) From dd4b5cd12a00c9025fae932426fcdd183ca52617 Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Fri, 11 Sep 2026 21:19:44 -0700 Subject: [PATCH 5/8] docs: trigger registration follows every list and flag change; ModernUO #2636 recorded Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr --- CLAUDE.md | 4 ++++ dev-docs/architecture.md | 9 +++++++-- dev-docs/modernuo-prerequisites.md | 6 +----- dev-docs/product-spec.md | 4 ++-- 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 9415134..4a53363 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -62,6 +62,10 @@ ModernSpawner-specific: 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. +- Trigger list and flag changes go through the generated helpers (`AddToTriggerDefinitions`, + `RemoveFromTriggerDefinitionsAt`, `ClearTriggerDefinitions`) or the `TriggerActivated` setter, then call + `EnsureTriggersActive()`; never call `TriggerSystem.ActivateTriggers` directly — it is not idempotent, + and `EnsureTriggersActive` is its only caller. ## ModernUO changes diff --git a/dev-docs/architecture.md b/dev-docs/architecture.md index cc5f53a..1aa046c 100644 --- a/dev-docs/architecture.md +++ b/dev-docs/architecture.md @@ -75,8 +75,13 @@ when the spawner is running, is `TriggerActivated` and actually has definitions, 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: +flips. The same helper is the mandatory follow-up for every other list or flag change: the +`TriggerActivated` setter calls it, and so do `TriggerConfigGump`'s add/remove handlers and the JSON +importer's clear path, all of which mutate `_triggerDefinitions` only through the generated +`AddToTriggerDefinitions`/`RemoveFromTriggerDefinitionsAt`/`ClearTriggerDefinitions` helpers so the change +is tracked for serialization before triggers are re-registered. Deactivation is unconditional in +`OnStopped` (reached by `Stop()` and, through `BaseSpawner.OnDelete`, by deletion) and in `OnDelete`, and +`XmlSpawnerMigrator` honours an explicit `Running="false"` on both node forms it reads. Wiring: | Trigger | Source event | Wired | |---|---|---| diff --git a/dev-docs/modernuo-prerequisites.md b/dev-docs/modernuo-prerequisites.md index be8d78d..d929fb2 100644 --- a/dev-docs/modernuo-prerequisites.md +++ b/dev-docs/modernuo-prerequisites.md @@ -10,7 +10,7 @@ per-movement paths without a measurement, because shards run 12k+ spawners. | PR | Change | Why ModernSpawner needs it | Submodule pin | |---|---|---|---| -(none) +| [#2636](https://github.com/modernuo/ModernUO/pull/2636) | `SkillEvents.SkillChecked` (`Action`, `Server.Misc`) raised from `SkillCheck.CheckSkill`; `InternalsVisibleTo("ModernSpawner.Tests")` on `Server.csproj` | D3 skill triggers subscribe cross-assembly (generated events are static dispatch inside UOContent); the test fixture can seed `Core._now` | not yet pinned; the skill-wiring PR pins to `4af216edb` (or main after merge) | ## Merged @@ -21,10 +21,6 @@ per-movement paths without a measurement, because shards run 12k+ spawners. ## 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), 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 db2fef4..4b201ed 100644 --- a/dev-docs/product-spec.md +++ b/dev-docs/product-spec.md @@ -88,7 +88,7 @@ Status columns reflect the audit at `8935ca4`. "Target" is the v1 commitment. | Proximity beyond 24 tiles | Stubbed | Range clamped with a warning; wider ranges need a ModernUO area-subscription API (tracked in `modernuo-prerequisites.md`) | | Speech | Implemented | Kept; regex timeout; whether it may wake a stopped spawner is per-trigger (`wake:`) under **D2** | | Kill | Stubbed | Wired via a new upstream `BaseSpawner.OnSpawnedDeath` hook (the creature-death event fires after the spawner link is cleared) | -| Skill | Stubbed | Wired via an upstream ModernUO hook; until then `skill:` definitions are rejected at parse time with a visible error (**D3**) | +| Skill | Stubbed | `skill:` definitions parse and register, but nothing calls `ModernSpawnerEvents.OnSkillUsed` yet, so they never fire; wired once the upstream ModernUO hook lands (**D3**) | | Game-time window | Partial | Constant derived from `Clock.SecondsPerUOMinute`; recomputed on map change | | Wall-clock window | Partial | Day/month filters apply to the open edge only; weekly/monthly recurrence exposed | | Legacy `timeofday` | Implemented | Retired in favour of `game_time_window` (importer maps to it) | @@ -207,7 +207,7 @@ stated conditions. | **D0** | Distribution: source submodule vs DLL | Source submodule for v1; DLL later | Open (default assumed) | | **D1** | Entry ownership | Change ModernUO: abstract entry ownership (`architecture.md` §4), including any streamlining of `BaseSpawner` that makes it more agnostic. **Condition:** no performance regression; trade-offs reported before merge | **Ruled** | | **D2** | Trigger semantics | State machine: gate set (windows) + bounded pending-cycle queue (events); `architecture.md` §5. **Condition:** no per-tick/per-movement cost growth at 12k+ spawners; implementation reviewed | **Ruled** | -| **D3** | Skill trigger source | Upstream `SkillCheck` hook via ModernUO PR | Open (default assumed) | +| **D3** | Skill trigger source | Upstream `SkillCheck` hook via ModernUO PR | ModernUO #2636 open; wiring PR follows | | **D4** | Script language | Retire ModernSpawner's current `SET/Hits/100` command syntax (a copy of XmlSpawner's style, not XmlSpawner itself); add statements and actions on top of the existing, tested expression engine rather than writing a new engine | **Ruled** (retire); statement design pending review | | **D5** | Canonical export format | ModernUO `SpawnerDto`; own JSON and YAML removed; generalise upstream where needed | **Ruled** | | **D6** | Entry `Properties` syntax | ModernUO's `Name Value` pairs; ranges/expressions live in entry scripts | **Ruled** | From 170409350954be2277af5c6b96f25b333deb7d98 Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Fri, 11 Sep 2026 21:35:23 -0700 Subject: [PATCH 6/8] fix(triggers): perf seed registers its trigger; gump time triggers use registered type names; page clamp Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr --- .../ModernSpawnerTriggerRegistrationTests.cs | 23 ++++++++ .../Fixtures/ModernSpawnerTestServer.cs | 3 + Projects/ModernSpawner/Core/ModernSpawner.cs | 3 + .../ModernSpawner/Gumps/TriggerConfigGump.cs | 56 +++++++++++++++---- .../ModernSpawner/Perf/SpawnerPerfCommands.cs | 4 +- 5 files changed, 77 insertions(+), 12 deletions(-) diff --git a/Projects/ModernSpawner.Tests/Core/ModernSpawnerTriggerRegistrationTests.cs b/Projects/ModernSpawner.Tests/Core/ModernSpawnerTriggerRegistrationTests.cs index cf1ad62..12a2198 100644 --- a/Projects/ModernSpawner.Tests/Core/ModernSpawnerTriggerRegistrationTests.cs +++ b/Projects/ModernSpawner.Tests/Core/ModernSpawnerTriggerRegistrationTests.cs @@ -93,6 +93,29 @@ public void Stop_UnregistersEvenWhenFlagWasClearedAfterRegistration() spawner.Delete(); } + [Fact] + public void GumpTimeTriggerDefinitions_ParseAndRegister() + { + // Built exactly as TriggerConfigGump builds them from its hour fields: the registered type + // names, in the argument format each Parse accepts. + const int startHour = 18; + const int endHour = 6; + var wallTime = $"wall_time_window:{startHour}:0:{endHour}:0"; + const string gameTime = "game_time_window:21:5:true"; + + // ParseTrigger returns null for an unrecognised type (it only logs), so this pins the branch. + Assert.NotNull(TriggerSystem.Instance.ParseTrigger(wallTime)); + Assert.NotNull(TriggerSystem.Instance.ParseTrigger(gameTime)); + + var spawner = Place(); + spawner.AddToTriggerDefinitions(wallTime); + spawner.AddToTriggerDefinitions(gameTime); + spawner.TriggerActivated = true; + + Assert.True(TriggerSystem.Instance.IsRegistered(spawner)); + spawner.Delete(); + } + [Fact] public void DeletingAStoppedSpawner_WithStaleRegistration_Unregisters() { diff --git a/Projects/ModernSpawner.Tests/Fixtures/ModernSpawnerTestServer.cs b/Projects/ModernSpawner.Tests/Fixtures/ModernSpawnerTestServer.cs index 6f0b7b3..44f93dd 100644 --- a/Projects/ModernSpawner.Tests/Fixtures/ModernSpawnerTestServer.cs +++ b/Projects/ModernSpawner.Tests/Fixtures/ModernSpawnerTestServer.cs @@ -81,6 +81,9 @@ public static void Initialize() World.ExitSerializationThreads(); DecayScheduler.Configure(); + // WallTimeWindowTrigger.Activate schedules through EventScheduler.Shared, which is null + // until this runs (production reaches it through UOContent's Configure pass). + Server.Engines.Events.EventScheduler.Configure(); // Without npc-speeds.json every BaseCreature constructor throws. Server.Mobiles.NPCSpeeds.Configure(); Server.Engines.Spawners.SpawnerJsonSerializer.Configure(); diff --git a/Projects/ModernSpawner/Core/ModernSpawner.cs b/Projects/ModernSpawner/Core/ModernSpawner.cs index ccdc1a5..b3eee6e 100644 --- a/Projects/ModernSpawner/Core/ModernSpawner.cs +++ b/Projects/ModernSpawner/Core/ModernSpawner.cs @@ -86,6 +86,9 @@ public partial class ModernSpawner : Spawner /// , so there is no window where the flag and the trigger /// registry disagree. The backing field is generated; serialization order 9 is unchanged. /// + // Hand-written [SerializableProperty] rather than [SerializableField(9, fieldChanged:)] so the + // registration call sits at the mutation point with this doc comment; the generated pipeline + // (equality check -> assign -> MarkDirty -> callback) is equivalent. [SerializableProperty(9)] [CommandProperty(AccessLevel.Developer)] public bool TriggerActivated diff --git a/Projects/ModernSpawner/Gumps/TriggerConfigGump.cs b/Projects/ModernSpawner/Gumps/TriggerConfigGump.cs index a3f43b2..11adbe2 100644 --- a/Projects/ModernSpawner/Gumps/TriggerConfigGump.cs +++ b/Projects/ModernSpawner/Gumps/TriggerConfigGump.cs @@ -203,24 +203,50 @@ private static void FormatTriggerDisplay(string definition, scoped ref ValueStri return; } - if (triggerType.InsensitiveEquals("walltime")) + if (triggerType.InsensitiveEquals("wall_time_window")) { - // Format: walltime:startHour:endHour - Span parts = stackalloc Range[4]; + // Format: wall_time_window:startHour:startMin:endHour:endMin:allowedDays:allowedMonths:timezone + Span parts = stackalloc Range[8]; var count = span.Split(parts, ':'); var startHour = count > 1 ? span[parts[1]] : "0"; - var endHour = count > 2 ? span[parts[2]] : "24"; + var startMinute = count > 2 ? span[parts[2]] : "00"; + var endHour = count > 3 ? span[parts[3]] : "23"; + var endMinute = count > 4 ? span[parts[4]] : "59"; sb.Append("Real Time: "); sb.Append(startHour); - sb.Append(":00 - "); + sb.Append(':'); + sb.Append(startMinute); + sb.Append(" - "); sb.Append(endHour); - sb.Append(":00"); + sb.Append(':'); + sb.Append(endMinute); return; } - if (triggerType.InsensitiveEquals("gametime")) + if (triggerType.InsensitiveEquals("game_time_window")) { - sb.Append("Game Time: Night hours"); + // Format: game_time_window:startHour:endHour:nightOnly:dayOnly + Span parts = stackalloc Range[5]; + var count = span.Split(parts, ':'); + if (count > 3 && span[parts[3]].InsensitiveEquals("true")) + { + sb.Append("Game Time: Night hours"); + return; + } + + if (count > 4 && span[parts[4]].InsensitiveEquals("true")) + { + sb.Append("Game Time: Day hours"); + return; + } + + var startHour = count > 1 ? span[parts[1]] : "0"; + var endHour = count > 2 ? span[parts[2]] : "23"; + sb.Append("Game Time: "); + sb.Append(startHour); + sb.Append(":00 - "); + sb.Append(endHour); + sb.Append(":00"); return; } @@ -299,14 +325,18 @@ public override void OnResponse(NetState state, in RelayInfo info) { endHour = Math.Clamp(parsedEnd, 0, 23); } - _spawner.AddToTriggerDefinitions($"walltime:{startHour}:{endHour}"); + // WallTimeWindowTrigger.Parse reads wall_time_window:startHour:startMin:endHour:endMin; + // the gump only offers whole hours, so the minute fields are zero. + _spawner.AddToTriggerDefinitions($"wall_time_window:{startHour}:0:{endHour}:0"); _spawner.EnsureTriggersActive(); from.SendMessage($"Added time window trigger: {startHour}:00 - {endHour}:00."); break; } case ButtonId_AddGameTime: - _spawner.AddToTriggerDefinitions("gametime:night"); + // GameTimeWindowTrigger.Parse reads game_time_window:startHour:endHour:nightOnly; + // NightOnly is the parser's night preset and overrides the hours it is given. + _spawner.AddToTriggerDefinitions("game_time_window:21:5:true"); _spawner.EnsureTriggersActive(); from.SendMessage("Added game time trigger for night hours."); break; @@ -328,6 +358,10 @@ public override void OnResponse(NetState state, in RelayInfo info) break; } - from.SendGump(new TriggerConfigGump(_spawner, _page)); + // A delete can empty the page that was being viewed, so clamp before re-sending: the list is + // re-read because the switch above may have added to or removed from it. + var remaining = GetTriggerList(); + var lastPage = Math.Max(0, (remaining.Count - 1) / TriggersPerPage); + from.SendGump(new TriggerConfigGump(_spawner, Math.Min(_page, lastPage))); } } diff --git a/Projects/ModernSpawner/Perf/SpawnerPerfCommands.cs b/Projects/ModernSpawner/Perf/SpawnerPerfCommands.cs index 8204f0b..236bc66 100644 --- a/Projects/ModernSpawner/Perf/SpawnerPerfCommands.cs +++ b/Projects/ModernSpawner/Perf/SpawnerPerfCommands.cs @@ -118,8 +118,10 @@ private static void PerfSeed_OnCommand(CommandEventArgs e) maxCount: 1, dotimer: false); - // Add a proximity trigger so player sweeps exercise the dispatch path. + // Add a proximity trigger so player sweeps exercise the dispatch path. The flag is set + // after the definition exists: its setter registers whatever is in the list at that moment. spawner.AddToTriggerDefinitions("proximity:8:true"); + spawner.TriggerActivated = true; _seeded.Add(spawner); created++; From 1f560ac27cbda68184242da62f796db904830fd1 Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Fri, 11 Sep 2026 21:36:51 -0700 Subject: [PATCH 7/8] docs: skill trigger keeps its fail-loud v1 target; trigger rule scoped to the engine project Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr --- CLAUDE.md | 9 +++++---- dev-docs/architecture.md | 21 ++++++++++++--------- dev-docs/modernuo-prerequisites.md | 2 +- dev-docs/product-spec.md | 4 ++-- 4 files changed, 20 insertions(+), 16 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 4a53363..287ea2e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -62,10 +62,11 @@ ModernSpawner-specific: 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. -- Trigger list and flag changes go through the generated helpers (`AddToTriggerDefinitions`, - `RemoveFromTriggerDefinitionsAt`, `ClearTriggerDefinitions`) or the `TriggerActivated` setter, then call - `EnsureTriggersActive()`; never call `TriggerSystem.ActivateTriggers` directly — it is not idempotent, - and `EnsureTriggersActive` is its only caller. +- Trigger list changes go through the generated helpers (`AddToTriggerDefinitions`, + `RemoveFromTriggerDefinitionsAt`, `ClearTriggerDefinitions`), then call `EnsureTriggersActive()`; the + `TriggerActivated` setter does this for you. Never call `TriggerSystem.ActivateTriggers` directly — it is + not idempotent, and within `Projects/ModernSpawner` `EnsureTriggersActive` is its only caller (tests call + it deliberately, to build the stale registrations teardown has to survive). ## ModernUO changes diff --git a/dev-docs/architecture.md b/dev-docs/architecture.md index 1aa046c..843b789 100644 --- a/dev-docs/architecture.md +++ b/dev-docs/architecture.md @@ -69,16 +69,19 @@ triggers and runs the entry's `OnDespawnScript`. `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`). 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. The same helper is the mandatory follow-up for every other list or flag change: the +`TriggerSystem.ActivateTriggers` outside the trigger system, within the engine project: it deactivates +first and re-registers only when the spawner is running, is `TriggerActivated` and actually has +definitions, which makes it idempotent. `ActivateTriggers` on its own is not: it *replaces* +`_allTriggers[spawner]` with the batch it just parsed while the per-type lists it feeds +(`_proximityTriggers`, `_speechTriggers`, …) *append*, so calling it twice duplicates dispatch and orphans +the first batch — those triggers are no longer reachable for `Deactivate()`. `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. The same helper is the mandatory follow-up for every other list or flag change: the `TriggerActivated` setter calls it, and so do `TriggerConfigGump`'s add/remove handlers and the JSON -importer's clear path, all of which mutate `_triggerDefinitions` only through the generated -`AddToTriggerDefinitions`/`RemoveFromTriggerDefinitionsAt`/`ClearTriggerDefinitions` helpers so the change +importer's clear path. Those list mutations go only through the generated +`AddToTriggerDefinitions`/`RemoveFromTriggerDefinitionsAt`/`ClearTriggerDefinitions` helpers, so the change is tracked for serialization before triggers are re-registered. Deactivation is unconditional in `OnStopped` (reached by `Stop()` and, through `BaseSpawner.OnDelete`, by deletion) and in `OnDelete`, and `XmlSpawnerMigrator` honours an explicit `Running="false"` on both node forms it reads. Wiring: diff --git a/dev-docs/modernuo-prerequisites.md b/dev-docs/modernuo-prerequisites.md index d929fb2..09b0d13 100644 --- a/dev-docs/modernuo-prerequisites.md +++ b/dev-docs/modernuo-prerequisites.md @@ -10,7 +10,7 @@ per-movement paths without a measurement, because shards run 12k+ spawners. | PR | Change | Why ModernSpawner needs it | Submodule pin | |---|---|---|---| -| [#2636](https://github.com/modernuo/ModernUO/pull/2636) | `SkillEvents.SkillChecked` (`Action`, `Server.Misc`) raised from `SkillCheck.CheckSkill`; `InternalsVisibleTo("ModernSpawner.Tests")` on `Server.csproj` | D3 skill triggers subscribe cross-assembly (generated events are static dispatch inside UOContent); the test fixture can seed `Core._now` | not yet pinned; the skill-wiring PR pins to `4af216edb` (or main after merge) | +| [#2636](https://github.com/modernuo/ModernUO/pull/2636) | `SkillEvents.SkillChecked` (`Action`, `Server.Misc`) raised from `SkillCheck.CheckSkill`; `InternalsVisibleTo("ModernSpawner.Tests")` on `Server.csproj` | D3 skill triggers subscribe cross-assembly (generated events are static dispatch inside UOContent); the test fixture can seed `Core._now` | no pin needed until the ModernSpawner wiring PR consumes it; that PR pins the submodule to the PR head, or to `main` if it has merged by then | ## Merged diff --git a/dev-docs/product-spec.md b/dev-docs/product-spec.md index 4b201ed..4506970 100644 --- a/dev-docs/product-spec.md +++ b/dev-docs/product-spec.md @@ -88,7 +88,7 @@ Status columns reflect the audit at `8935ca4`. "Target" is the v1 commitment. | Proximity beyond 24 tiles | Stubbed | Range clamped with a warning; wider ranges need a ModernUO area-subscription API (tracked in `modernuo-prerequisites.md`) | | Speech | Implemented | Kept; regex timeout; whether it may wake a stopped spawner is per-trigger (`wake:`) under **D2** | | Kill | Stubbed | Wired via a new upstream `BaseSpawner.OnSpawnedDeath` hook (the creature-death event fires after the spawner link is cleared) | -| Skill | Stubbed | `skill:` definitions parse and register, but nothing calls `ModernSpawnerEvents.OnSkillUsed` yet, so they never fire; wired once the upstream ModernUO hook lands (**D3**) | +| Skill | Stubbed - today `skill:` definitions parse and register, but nothing calls `ModernSpawnerEvents.OnSkillUsed`, so they never fire | `skill:` definitions are rejected at parse time with a visible error until the wiring PR subscribes to ModernUO #2636's `SkillEvents.SkillChecked` (**D3**); a trigger that cannot fire must not look configured | | Game-time window | Partial | Constant derived from `Clock.SecondsPerUOMinute`; recomputed on map change | | Wall-clock window | Partial | Day/month filters apply to the open edge only; weekly/monthly recurrence exposed | | Legacy `timeofday` | Implemented | Retired in favour of `game_time_window` (importer maps to it) | @@ -207,7 +207,7 @@ stated conditions. | **D0** | Distribution: source submodule vs DLL | Source submodule for v1; DLL later | Open (default assumed) | | **D1** | Entry ownership | Change ModernUO: abstract entry ownership (`architecture.md` §4), including any streamlining of `BaseSpawner` that makes it more agnostic. **Condition:** no performance regression; trade-offs reported before merge | **Ruled** | | **D2** | Trigger semantics | State machine: gate set (windows) + bounded pending-cycle queue (events); `architecture.md` §5. **Condition:** no per-tick/per-movement cost growth at 12k+ spawners; implementation reviewed | **Ruled** | -| **D3** | Skill trigger source | Upstream `SkillCheck` hook via ModernUO PR | ModernUO #2636 open; wiring PR follows | +| **D3** | Skill trigger source | Upstream `SkillCheck` hook via ModernUO PR | Open (default assumed); ModernUO #2636 open | | **D4** | Script language | Retire ModernSpawner's current `SET/Hits/100` command syntax (a copy of XmlSpawner's style, not XmlSpawner itself); add statements and actions on top of the existing, tested expression engine rather than writing a new engine | **Ruled** (retire); statement design pending review | | **D5** | Canonical export format | ModernUO `SpawnerDto`; own JSON and YAML removed; generalise upstream where needed | **Ruled** | | **D6** | Entry `Properties` syntax | ModernUO's `Name Value` pairs; ranges/expressions live in entry scripts | **Ruled** | From 8aa17251c0ea4d40d5f5a6bac2bcae61eee0ea04 Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Fri, 11 Sep 2026 21:43:12 -0700 Subject: [PATCH 8/8] fix(gumps): zero-pad minutes in the time-window row; EnsureTriggersActive doc matches the registry semantics Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr --- Projects/ModernSpawner/Core/ModernSpawner.cs | 3 ++- Projects/ModernSpawner/Gumps/TriggerConfigGump.cs | 15 +++++++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/Projects/ModernSpawner/Core/ModernSpawner.cs b/Projects/ModernSpawner/Core/ModernSpawner.cs index b3eee6e..1e4bde2 100644 --- a/Projects/ModernSpawner/Core/ModernSpawner.cs +++ b/Projects/ModernSpawner/Core/ModernSpawner.cs @@ -598,7 +598,8 @@ private void MaybeAutoResetSequence() /// /// 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 + /// replaces the spawner's batch in the registry but + /// appends to the per-type dispatch lists, so a second call would duplicate dispatch; 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 diff --git a/Projects/ModernSpawner/Gumps/TriggerConfigGump.cs b/Projects/ModernSpawner/Gumps/TriggerConfigGump.cs index 11adbe2..a093f79 100644 --- a/Projects/ModernSpawner/Gumps/TriggerConfigGump.cs +++ b/Projects/ModernSpawner/Gumps/TriggerConfigGump.cs @@ -175,6 +175,17 @@ protected override void BuildLayout(ref DynamicGumpBuilder builder) private List GetTriggerList() => _spawner.TriggerDefinitions ?? []; + /// Appends a minute field, zero-padding single digits so 18:0 renders as 18:00. + private static void AppendMinutes(scoped ref ValueStringBuilder sb, ReadOnlySpan minutes) + { + if (minutes.Length == 1) + { + sb.Append('0'); + } + + sb.Append(minutes); + } + private static void FormatTriggerDisplay(string definition, scoped ref ValueStringBuilder sb) { if (string.IsNullOrEmpty(definition)) @@ -215,11 +226,11 @@ private static void FormatTriggerDisplay(string definition, scoped ref ValueStri sb.Append("Real Time: "); sb.Append(startHour); sb.Append(':'); - sb.Append(startMinute); + AppendMinutes(ref sb, startMinute); sb.Append(" - "); sb.Append(endHour); sb.Append(':'); - sb.Append(endMinute); + AppendMinutes(ref sb, endMinute); return; }