feat(triggers): D2 trigger state machine — gate set, bounded pending cycles, refractory, per-entry deadlines, XmlSpawner migration - #5
Open
kamronbatman wants to merge 15 commits into
Conversation
…serialization v1
Trigger definitions gain identity. `List<string>` becomes `List<TriggerDefinition>`
({ Guid Id (v7), string Text }), so per-trigger runtime state and queued cycles can
name the definition they belong to across saves, gump reorders and DTO round trips.
ModernSpawner moves to SerializationGenerator(1): the definition list is typed, the
one-bit `_triggered` flag is gone, and the slots after it shift down by one. New
fields carry the state the D2 machine needs: a bounded `PendingCycles` queue whose
slots carry the triggering mobile's serial, `MaxPendingCycles` (default 1; 0 is
XmlSpawner run-now-or-drop), a spawner-wide refractory (min, max, absolute until),
and `TriggerStates` keyed by definition id. ModernSpawnerEntry moves to version 1
with an absolute `NextEligible` deadline. Both get `MigrateFrom(V0Content)`; schemas
regenerated, and the schema generator is now pinned in .config/dotnet-tools.json.
The generated collection helpers for the definition list are private - they cannot
mint an id - so AddTriggerDefinition / RemoveTriggerDefinitionAt / ClearTriggerDefinitions
are the only way in, each marking the spawner dirty and re-registering through
EnsureTriggersActive, which now also re-binds state by id and drops the state and
pending slots of definitions that went away. Runtime state is world-save only: the
DTO exports definitions as { id, text } plus the limits and nothing else.
SpawnCycleMode.Group is renamed AllEntries, with Group kept one release as an
obsolete alias so exported JSON still parses.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr
…shape; drop the text-matching id stopgap
Three findings from the task 1 review.
An id arriving from outside - a hand-edited export, a trigger block copied between
spawners - could already be in use, and two definitions sharing one id alias onto a
single TriggerRuntimeState and onto each other's pending slots. AddTriggerDefinition
and ApplyModernDto now route every incoming id through UniqueDefinitionId, which
mints a fresh v7 id on collision.
Exports written before definitions had ids wrote `"triggers": ["proximity:8:true"]`,
which threw on TriggerDefinitionDto. A JsonConverter on the DTO reads either that
bare string (id minted on import) or the `{ id, text }` object, and always writes
the object, so an old file imports and re-exports into the new shape with stable ids.
TriggerIdOf matched a parsed trigger to its definition by comparing Serialize() to
the definition text. The gump and the importers write shorter forms than Serialize()
produces, so in production it never matched and every window-open slot fell back to
Guid.Empty anyway. Removed; OnTriggerActivated passes Guid.Empty directly until
task 2 binds definition ids onto trigger instances at registration.
Minors: strip the UTF-8 BOM this branch introduced on fourteen pre-existing files;
correct the OnTriggerDeactivated comment (window close is G3, which does not itself
clear pending - the clearing is interim and task 3 replaces it); mark _refractoryUntil
[SerializedIgnoreDupe] alongside the other runtime state; restore the blank lines
before three [Fact] attributes.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr
…uate, outer-dispatch drain, shared tokens; timeofday retired One TriggerSet per spawner in one dictionary replaces the six per-type dictionaries, so a movement, speech or kill dispatch does one lookup and then walks a typed list by index. TriggerContext becomes a readonly record struct passed `in`, and both GetSpawnersWith... yield iterators are gone; skill dispatch keeps a per-map candidate list maintained on registration and map change. Evaluate is now pure: the cooldown is a read of TriggerRuntimeState.CooldownUntil and kill counting reads State.KillCount, so an evaluation the spawner goes on to reject leaves no trace. Dispatch never calls Spawn(): a match calls spawner.RequestCycle(trigger, in context), and the trigger system keeps a dispatch-depth counter that drains a pooled list once the outermost dispatch returns. ITrigger gains Kind, Id, DefinitionIndex, State, Wake, Mode, When and Cooldown; Id, DefinitionIndex and State are bound at registration before anything can fire, and the gates report their open and close edges by definition index through ModernSpawner.OnGateOpened/OnGateClosed. TriggerTokens parses and writes the shared wake:, mode: and when: tokens in any order, leaving pre-token definitions byte-identical on round trip; when: compiles once at parse time. TimeOfDayTrigger is deleted and survives only as a factory alias onto game_time_window, whose exclusive end hour now reaches 24 so the legacy inclusive end maps cleanly. Extended proximity is clamped to Core.GlobalMaxUpdateRange with a warning and the _extendedTriggerBounds machinery is removed; SpeechTrigger's regex gains a 50ms match timeout. RequestCycle, DrainOne and the gate callbacks are interim shims that keep the old behaviour until the spawner state machine lands. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr
… kill-counter tests; token scan past positionals The legacy timeofday grammar read end < start as a wrap past midnight, so end == start - 1 (mod 24) - 0:23, 10:9, 23:22, 1:0 - named every hour of the day. Mapping the inclusive end to an exclusive one unconditionally turned those into [start, start), the empty window and the exact opposite, leaving a gate that never opens. One helper, MapLegacyTimeOfDayHours, now owns the legacy rules and both the timeofday factory alias and the JSON importer go through it. A speech trigger's regex is compiled at registration, and registration runs from world load, so an invalid hand-authored pattern threw ArgumentException out of EnsureTriggersActive and left a half-activated trigger set behind. The pattern is now built at most once behind EnsureRegex, a bad one is logged and leaves the trigger unable to match, and matching tolerates the missing regex. TriggerTokens.Strip now takes the caller's positional arity and only looks for tokens past it, so a kill filter type or a keyword literally spelled Wake, Mode or When stays an argument instead of being eaten and shifting every later argument. Tokens are therefore a suffix: positional arguments have to be written out before them, which is what Serialize already does. KillTrigger.CountsKill no longer needs a bound spawner - RequireAllDead is the only part that reads one, so that requirement moved into Evaluate - which lets the kill-counter split be tested purely above threshold 1: the threshold kill, ResetOnTrigger on and off, a filtered-out kill, RequireAllDead blocking a cycle without dropping the kill, and kill progress surviving a re-registration bound to a fresh trigger object by definition id. Also drops the unused TriggerContext.ForGate and rewrites EnsureTriggersActive's summary, which still described the per-type dispatch lists that are gone. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr
…ry deadlines; OnTick applies the D2 table The spawner side of the D2 trigger state machine replaces the interim bridge that spawned from inside event dispatch. - ModernSpawner.Triggers.cs (new): the gate set (a ulong bitmask by definition index with a List<int> overflow past 64), the acceptance order every event passes through (registration, Evaluate, cooldown, refractory, when:, queue room) with every side effect applied only on acceptance, the E0-E6 routing, D1 re-validation in DrainOne, RunCycle with its re-entrancy guard and a per-dispatch drain bound of 10, G1-G4 on the gate edges, M3/M4, X1 and the deferred L1/L2 restore. - OnTick is overridden with the T0-T6 precedence; a row that parks leaves the timer unarmed, and a row that runs re-arms at the earliest entry deadline. - Entry selection filters on ModernSpawnerEntry.NextEligible; a placement sets the entry's own delay, a failure backs it off by min(30s, MinDelay). Event cycles and manual spawns bypass the deadlines, timer cycles honour them. - Registration no longer depends on Running (A1/A2): OnStopped keeps it, only deactivation and deletion tear it down, and movement/speech dispatch while stopped so a wake: trigger can start the spawner. - Group respawn is one bulk operation with the before/after scripts run once around it rather than once per Spawn() inside it. - The kill counter moves to the spawner's acceptance path; the kill dispatch hands over every kill that passes the trigger's filters and the spawner both evaluates it and advances the counter. - The cycle's triggering mobile is resolved from the queued slot and threaded into PositioningContext and the script contexts, so player_relative still positions relative to the player after a deferred drain. Tests: TriggerStateMachineTests covers the design's §11 matrix row by row; SkillTriggerTests, SpeechTriggerRegexTests and the lifecycle and registration tests are updated to the D2 contract. 539 pass. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr
…ics on every cycle source; drain on cycle exit Fix round 1 on the D2 state machine. Defects - OnGateOpened called DrainOne directly, and the per-dispatch recursion budget is only reset when the drain list is exhausted, so every gate opening leaked one drain: after ten E2 -> G1 sequences the eleventh opening logged the recursion warning and stopped draining. G1 now goes through RequestDrain, which flushes at depth 0 and hands the budget back. - The Max == 0 run-now request could latch. DrainOne's "a cycle is already in flight" early return left the flag and the mobile set for an arbitrary later drain to spend; it now drops the request (it is one-shot by definition), as does the budget-exhausted return, and the mobile is held as a Serial rather than as a reference that could root a deleted mobile. - Event and gate cycles ignored base Group. T1 now applies to every cycle source: RunCycleCore does nothing while the pack is alive (the slot is kept) and runs the one bulk Respawn when it is dead, so a tick, an event drain and a window opening all behave the same on a group spawner. Rulings - A cycle's follow-ups are drained when the cycle exits, through the same bounded drain list, so a tick-initiated cycle no longer defers what its scripts bought to the next tick. - G1's window-open cycle honours per-entry deadlines; the bypass belongs to mode:now event drains only. - A kill below the threshold counts while the trigger is on cooldown: the cooldown gates the trigger firing, not the kills that build up to it, so KillTrigger.CountsKill no longer compares it. - OnStarted no longer re-registers: registration follows TriggerActivated and the definition list, never Running (A1). Minors: OnTick reads IsAuthorizedForTick instead of two inlined copies; RunCycleCore reports whether it ran so a caller keeps its slot; OnGateClosed gets OnGateOpened's Deleted guard; RequestCycle documents that non-kill triggers arrive pre-evaluated by their dispatcher. Tests: seven new facts - the gate-open budget, the one-shot run-now, group semantics on an event cycle (populated and dead pack), kill counting under cooldown, when: accept/reject, and the tick-initiated follow-up drain. 546 pass. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr
…riggers, group and TOD mapping onto the D2 model Both XmlSpawner ingestion paths (XmlSpawnerMigrator and XmlSpawnerImporter) now map: MinRefractory/MaxRefractory to RefractoryMin/Max (minutes); SpawnOnTrigger=False to MaxPendingCycles=1 plus mode:tick on every emitted event trigger, absent/True to MaxPendingCycles=0 (XmlSpawner run-now-or-drop parity); ProximityRange+SpeechTrigger(+ PlayerPropertyName) to one conjunctive speech trigger with a when: expression (new shared XmlSpawnerPropertyExpression translator) instead of two independent triggers; IsGroup to base Group only, no longer forcing SpawnCycleMode.AllEntries; TODStart/ TODEnd/TODMode to a wall_time_window or game_time_window gate plus a despawn-on-close report note; Duration to a report-only note (D10). Both paths report through a shared mechanism: MigrationReport for the migrator, an extended ImportResult.Notes for the importer. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr
TriggerTokens.Strip documents when: as consuming everything after it, so appending mode:tick after a when: suffix swallowed the token into the expression source and left it uncompilable - the migrated trigger never deferred and its when: never passed. Both ingestion paths now build every event trigger definition through one BuildEventDefinition helper that emits positional[:mode:tick][:when:<expr>], the only writer at all six call sites, so the ordering cannot drift again. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr
The D2 performance conditions (design section 9) need two measurements: BenchmarkDotNet micro-benchmarks over the dispatch shapes, and a 12k-spawner movement and tick measurement. The second one was specified as a lap walked on a live shard, which is neither reproducible nor runnable without a shard, so it lands as a world-backed harness in the test project instead. Micro-benchmarks (mock types, no ModernUO reference, as the project requires): one TriggerSet dictionary is 2.24x cheaper per dispatch than probing one dictionary per trigger class at N=12,000, and neither allocates. The bounded queue plus the reused drain list is 1.51x-2.05x faster than an unbounded queue with a fresh list per round at pending depths 1/8/64, allocates 1.46x less, and keeps Gen2 at zero for depths 1 and 8. The harness seeds 12,000 spawners on Felucca through the grid-seeding helper now shared with the PerfSeed command, walks a placed player along a fixed 2,000-step lap dispatching OnMovement to every spawner inside Core.GlobalMaxUpdateRange of each step, and ticks the whole population with the gate closed and again with it open. It is opt-in through MODERNSPAWNER_PERF=1, so the default suite is unaffected, and it writes perf-results.json beside the test binaries so two trees can be diffed. Release numbers on the branch: 130.1 ns and 0 bytes per movement dispatch in the steady state, 50.7-51.7 ns and 0 bytes per tick. SpawnerMetrics gains a MeasureTick bucket, opened at the top of OnTick above T0 so the parked rows are measured too - the condition is about what a tick costs when it does nothing. Like every other bucket it is a no-op branch while the counters are disabled, so OnTick is unchanged in production. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr
…s; generation-checked requests; deadline arming; dispatch continues past a rejected trigger The single fix wave from the whole-branch and transition-table conformance reviews. Correctness - GameTimeWindowTrigger took a game hour to be 12 real minutes. The engine's own clock says 5 real seconds per UO minute, so a game hour is 5 real minutes: every window edge was 2.4x late. The constant now comes from Clock.SecondsPerUOMinute instead of being restated. - SkillTrigger.Evaluate rejected a stopped spawner, so A2/E4/E5 held for proximity and speech but not for skill. Only deletion takes a trigger out. - A cycle request now carries the generation of the registration it was raised from, and is dropped when that is not the spawner's current one. External Trigger() carries the current generation, which is how a script or command is honoured on a spawner whose triggers are deactivated - so the request path no longer tests TriggerActivated at all. - ToDto lost the base Group flag, so an exported group spawner came back as a plain one. - Movement, speech and skill dispatch stopped at the first trigger whose Evaluate matched, even when the spawner refused the request. A refused trigger no longer ends the dispatch: the next one gets its turn, as the kill loop already worked. - RequireAllDead could never be satisfied by the kill that clears the pack, because the spawner is notified before the base death path removes the dying spawn. "All dead" is now read excluding it. - A counted kill advances its counter even when the cooldown, the refractory or the queue refuses the cycle it would have bought - exactly once per dispatch - and KillTrigger.CountsKill drops a lifted bool? that let a type with no FullName through the filter by accident. Timers - ArmAtEarliestDeadline arms at the earliest future entry deadline with no MaxDelay clamp (a per-entry delay is allowed to be longer than the spawner's) and a floor of one slice; an entry with no deadline of its own contributes the spawner's random delay rather than being ignored. T6 also spends a queued slot on a spawner with no event definitions, which is the only way an external Trigger() slot is ever popped. - A4 re-arms a running spawner that was parked behind its triggers. - G1 does not arm when the group cycle refused: a live pack parks until removal, as T1 does. - The drain cap arms for the next tick when it leaves a slot behind. Logging and cost - A when: that does not compile is reported once, at parse time, and the trigger runs unconditioned rather than silently never firing. - ExpressionEngine's Console.WriteLine becomes the file's logger, once per compiled expression: a when: is evaluated per movement event. - The per-map skill candidate list carries each spawner's TriggerSet, so an attempt costs one dictionary lookup for the map and none per candidate. - The XmlSpawner migrator notes an inverted MinRefractory/MaxRefractory range in the migration report instead of clamping it silently. Tests: skill on stopped spawners (wake and non-wake), a request through a retired registration, a real two-definition reorder, a map move that re-registers and re-hydrates a game-time gate, the after-script once on a bulk respawn, G1 parking without arming, the drain cap's next tick, dispatch continuing past a refused trigger, RequireAllDead on both sides, the DTO Group round trip, dupe negative asserts, the inverted refractory note, and NextSpawn assertions on the T5/T6 and A4 rows. 598 pass. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr
- §2.1: the spawner carries 22 serialized fields in v1, not 17 - _notes moved
to 10, cycle state to 11-15, and 16-21 are the D2 runtime (queue, bound,
refractory range and deadline, per-definition state); the entry carries 12
with NextEligible. The spawn flow paragraph describes the T0-T6 tick with
Spawn() as the manual M1 path and names SpawnAllEntries rather than the
renamed SpawnGroupMode, and records the per-entry deadline the cycle writes.
- §2.2: the game-time window derives its constant from Clock.SecondsPerUOMinute
(the table row and the §5 Clock bullet both said it was hardcoded), and the
dispatch paragraph records the generation stamp a request carries and that a
refused trigger no longer ends the dispatch.
- §2.6: the DTO carries trigger definitions as { id, text } plus
maxPendingCycles and the refractory range and the base Group flag, and never
carries runtime state; the own-JSON format drops all three D2 fields and
mints fresh definition ids on import.
- §3: TriggerContext is a readonly record struct, so no dispatch allocates; the
repo has seven schema files; the one remaining per-event allocation is the
ScriptContext a when: condition needs, bounded by its trigger's cooldown.
- §5: RequireAllDead is evaluated against the spawner's live count with the
dying spawn excluded, because the notification precedes removal; a map change
re-registers so a gate recomputes against the new map's clock; the
movement-dispatch figure reads 131-133 ns to match the measured numbers.
- §7: the target export shape lists the trigger { id, text } objects and the D2
fields alongside them.
- Line 4 no longer pins commit hashes.
- ModernSpawner.EnsureTriggersActive's XML doc no longer lists "start" among
its callers: registration has been independent of Running since A1/A2.
- xmlspawner-migration.md keeps PlayerTriggerName in the unsupported property
trigger row, with the reason.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr
…: parsing; L1 asserts a live timer Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase 2: the D2 trigger state machine, per the approved design (
docs/proposals/2026-09-12-d2-trigger-state-machine.md, gitignored; the transition table is reproduced indev-docs/architecture.md§5).c02909e2c(#2640: virtualOnTick, DTOgroup)._triggeredis gone. Serialization v0 → v1 onModernSpawnerandModernSpawnerEntrywithMigrateFromand regenerated schemas.OnTickapplies the table (T0–T6): park when the gate is closed, the spawner is full, or an event-mode spawner has nothing pending; group spawners wait for all dead then run one bulk respawn with scripts once; the timer re-arms at the earliest entry deadline.RequestCycle(acceptance order: registration, evaluate, cooldown, refractory,when:, queue room; side effects only on acceptance), and the outermost dispatch drains after enumeration ends, bounded by the script recursion limit.mode:nowruns after dispatch returns;mode:tickat the next tick;wake:truestarts a stopped spawner;MaxPendingCycles = 0is XmlSpawner's run-now-or-drop.Running. Stopped spawners keep dispatching;Start()no longer reparses.TriggerSetper spawner,TriggerContextas areadonly record struct, pureEvaluate, noyielditerators, extended proximity clamped with a warning, speech regex with a timeout,timeofdayretired (aliased togame_time_window, whole-day wraps preserved).{ id, text }in the DTO; the legacy string array still imports).SpawnOnTrigger, conjunctive proximity+speech+property triggers (onespeechtrigger with the range and awhen:expression),IsGroup→ baseGroup, TOD modes,Durationand TOD-close despawn flagged in a new migration report.wake:,mode:,when:are a suffix after each trigger's full positional list; every existing definition parses with unchanged meaning.Performance (D2's merge condition: no per-tick or per-movement cost growth at 12k spawners)
mainmainhas no gated tick, shown for reference)Micro-benchmarks (BenchmarkDotNet, mock shapes): one
TriggerSetdictionary 17.9 ns vs six per-class dictionaries 40.1 ns; bounded queue + reused drain list 1.5–2.1× faster than unbounded + fresh list with 1.46× less allocation.Notes on the numbers: the per-dispatch figure includes the harness's grid-box candidate lookup (an approximation of the engine's sector dispatch), so it is dispatch plus an amortised share of candidate math, identically on both sides. The steady-state number measures the rejected-event path (the bounded queue is full); an accepted event allocates one
PendingCycleslot, bounded byMaxPendingCyclesand the trigger cooldown, and awhen:expression allocates a script context per candidate event.Harness:
MODERNSPAWNER_PERF=1 dotnet test Projects/ModernSpawner.Tests --filter "FullyQualifiedName~TriggerPerfHarness". Micro-benchmarks:dotnet run -c Release --project Projects/ModernSpawner.Benchmarks -- --filter *TriggerDispatch*.Test plan
TriggerStateMachineTests), driven through the real skill/movement/speech/death handlers with a seeded clock; key rows mutation-checked.TreatWarningsAsErrors.Clock.SecondsPerUOMinute, skill triggers honour stopped spawners, generation-checked requests,Groupin the DTO, dispatch continues past a rejected trigger, deadline arming without theMaxDelayclamp,RequireAllDeadevaluated excluding the dying spawn.🤖 Generated with Claude Code
https://claude.ai/code/session_01KXJwRhwvHQXV2ABJHicwsr