Repository navigation
refactor: BaseCreature to the SerializationGenerator with SaveFlag elision - #2592
kamronbatman wants to merge 11 commits into
Conversation
…ision Converts BaseCreature's hand-written v22 serialization to codegen v23. Nearly every field sits behind a [SaveFlag], so a creature matching its defaults writes only [version int][ulong flags][default AI] - 13 bytes - instead of ~236, and the writer's work is mostly branch-not-taken. With 500k-1M creatures in a world, this is the dominant slice of mobile save freeze time and disk. - Speeds serialize only when they differ from the creature's npc-speeds values (GetSpeeds/GetMoveSpeeds on both the flag check and the load default), so table edits now reach existing unmodified spawns on restart, and former paragons (snapped back to table values) elide fully. CurrentSpeed writes only when it differs from PassiveSpeed. - The delete countdown is a [DeserializeTimer] field (anchored); stabled/controlled pets never persist one, and the abandoned-pet 3-day fallback lives in AfterDeserialization for both load paths. - SummonEnd stays anchored, written only while summoned. - Old saves (v0-22) load through the retained legacy Deserialize(reader, version), now assigning raw fields; the shared post-load fixups (stat timers, AI creation, followers, animate-dead registration, unsummon timer) moved to [AfterDeserialization]. - Side-effect setters became generated-field hooks (Team, Controlled, Summoned, Loyalty clamp, resistance seeds, CurrentSpeed); properties whose semantics the hooks cannot express stay hand-written as [SerializableProperty] (ControlMaster/SummonMaster bracket the assignment with follower bookkeeping, ControlOrder must run on equal re-assignment, Tamable/IsParagon/move speeds have custom getters). - CreatureDeathEvent/CreatureDeletedEvent moved to a CreatureEvents host class: the events generator and the serialization generator each emit a [GeneratedCode] partial for the declaring type, and the attribute forbids duplicates (CS0579). - m_ fields renamed to _camelCase. Tests: new-format round trips (default and fully populated) with exact byte consumption, back-to-back saves byte-identical, and a byte-authentic fossilized v22 stream loading through the legacy path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ActiveMoveSpeed/PassiveMoveSpeed become plain [SerializableField]s (not virtual): the properties now read the raw override (0 = inheriting) and CurrentMoveSpeed carries the inherit resolution - it was the only production reader of the resolving getters. The <=0 coercion moves to an allowFieldChange hook. Wire format unchanged (schema diff is empty). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Removes narrative and developer commentary (historical essays, changelog notes, say-what-the-code-does lines, flavor) and condenses the keepers: constraint statements, wire-format markers, era-behavior notes, and the weighted-random distribution table. TODOs use the terse house style. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
No overrides exist. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Serialization consults the speed table four times per mob per save (the Should* flag checks) and again through the Default* methods on elided loads - each a SpeedClass/type dictionary walk. The resolved SpeedClassEntry is now cached on the creature (one reference; the table is immutable after Configure), so those become a null-check and field reads. GetSpeeds/GetMoveSpeeds stay the virtual override point, so stubs and forks that override them still steer elision; only their default implementations read the cache. NPCSpeeds' per-call lookups collapse into FindEntry. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A virtual SpeedClass could be overridden dynamically (boss state change) and silently diverge from the cached speed entry. SpeedClass is now non-virtual instance state: assigning it invalidates the cached entry, applies the new bucket's think and move speeds (preserving the active/passive mode), and serializes only when it differs from the type's DefaultSpeedClass - so a runtime bucket change survives a save while its (bucket-matching) speeds still elide. Works from [props too. Overrides become: DefaultSpeedClass for a type's constant bucket, SpeedClass assignment for state changes, GetSpeeds/GetMoveSpeeds to bypass the table entirely - none of which can leave the cache stale. SpeedClass deserializes before the speed fields (index 7; later indexes shift by one - v23 was never released, schema regenerated). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Either all four values (active/passive think + move) conform to the creature's speed entry - elided as a set - or the creature is fully custom and all four serialize. Partial conformance cannot exist on the wire, so no value is ever left silently tracking the table beside a hand-tuned sibling. "Fully custom" is a real state: SpeedLevel.Custom (None already means "resolve by type list" for the type-listed species, so it cannot double as the custom marker). Tuning any speed flips the bucket to Custom (the label never lies), Custom resolves no entry and short-circuits the conformance check, a custom creature is its own GetSpeeds reference (paragon snap becomes a natural no-op), and assigning a real bucket un-customs it via ApplySpeedClass. A re-entrancy guard keeps the flip from misreading ApplySpeedClass's half-assigned block, and constructors seed raw fields so DefaultSpeedClass types do not flip at birth. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The None -> type list -> Medium chain was construction-time defaulting (so creatures without a bucket or a SetSpeed call never spawn at 0/0), not a live semantic. The constructor now resolves it once into _speedClass itself, so at runtime a concrete bucket means table-backed and None means the creature's own speeds are authoritative - which is what SpeedLevel.Custom was; it is removed. Runtime entry resolution collapses to a single level lookup, and the SpeedClass byte still elides by comparing against the (cached) resolved type default. Legacy loads guess the type default and demote to None when the loaded speeds do not conform, so pre-codegen customized creatures (SetSpeed vendors) come out honestly labeled. A constructor guard keeps a missing speed table loud instead of spawning 0-delay creatures that spin their AI timers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
It served its purpose validating the migration during development; the legacy path is one-time upgrade code and the replica writer was most of the file. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ControlMaster and SummonMaster, when both set, are always the same mobile: BaseCreature.Summon assigns both to the caster, and every pet management flow (transfer, stable, claim, ball of summoning, GM obey, login overflow) followed SetControlMaster with an identical SummonMaster assignment. They differ only in presence - uncontrolled summons carry only a summon master, pets only a control master. So one _master reference serializes (refreshed at save, fanned back out through the Controlled/Summoned flags in AfterDeserialization; legacy loads feed the same path), and SetControlMaster now keeps SummonMaster in lockstep itself, deleting the six hand-rolled copies of that boilerplate. Also: a creature constructed without speeds (missing npc-speeds.json) now logs debug and defaults to Medium (0.25/0.5) instead of throwing - this is the place a sane default belongs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Audit: ControlMaster vs SummonMaster, for the master collapseThe pure generator conversion moved to #2611 (Delta Saves #7, phase 3). This PR now carries two behaviour changes on top of it: SpeedClass and the single- Writes. Every writer sets them together or sets exactly one, gated by the flags:
Reads. The only state with two different non-null masters is reachable but accidental: a talisman summon transferred to another player ( A released summon keeps its caster on purpose. After "release" or loyalty 0 on a controlled summon, Recommendation for this PR: one serialized
Sites that then become no-ops and can be deleted: the Behaviour deltas, both arguably fixes: a transferred talisman summon no longer exempts its original summoner from area harm; a jail-stabled talisman summon no longer holds the summoner's follower slot. |
Correction to the audit above: a summon can never be releasedThe "released summon keeps its caster" paragraph is wrong, and with it the
So
Separate bug found (pre-existing, tame pets, not summons): the loyalty drain at |
…ave requirement) (#2611) The pure SerializationGenerator conversion of `BaseCreature`, split out of #2592 so it can serve as the reference for converting every other large hand-written class in the Delta Saves project (#7, phase 3). Two behaviour changes from #2592 are deliberately not here and follow in their own PRs on top of this one: `SpeedClass`, and the collapse of `ControlMaster`/`SummonMaster` into one reference (a creature can lose or keep either independently: Blade Spirits and Energy Vortexes are summoned but never controlled, EnragedCreature and talisman summons keep a summon master with neither flag set). ## What this is - `BaseCreature` becomes `[SerializationGenerator(23, false)]` with a `[SerializableField]` per serialized slot and `[SaveFlag]` elision on nearly every field, so a stock creature serializes to its version plus flags. Field orders run 0..53 with no gaps (54 fields). - The hand-written reader stays as `private void Deserialize(IGenericReader reader, int version)` for every pre-codegen version (0..22); post-codegen bumps use `MigrateFrom` from here on. `[AfterDeserialization]` carries the post-load fixups main did after reading (stat timers, AI type, followers, reacquire seeding). - `ControlMaster` and `SummonMaster` stay two independent fields with main's semantics, each elided when null. - `DamageMin`/`DamageMax`/`ActiveSpeed`/`PassiveSpeed` are no longer virtual (nothing in the tree overrode them); comments swept to the constraints that matter. - Direct writes to serialized backing fields outside the generated setters (`SetDamage`, `SetResistance`, the move-speed helpers, the loot flag, feed loyalty, the delete timer) call `this.MarkDirty()`, matching the #2609 standard, so the class is ready for delta saves once `Mobile` is audited. - Schema `Server.Mobiles.BaseCreature.v23.json` regenerated by the tool (a second run produces no diff). ## Deferred to follow-up PRs SpeedClass: the serialized `_speedClass` field, `DefaultSpeedClass` replacing the type constant, `ApplySpeedClass`/`OnSpeedClassChange`, "None means custom", the four-speeds-as-one-block elision, `NPCSpeeds.FindEntry(SpeedLevel)`, the constructor fallback to Medium, and their tests. Master references: serializing one `Master` with a `Controlled`/`Summoned` fan-out and the `SetControlMaster` lockstep. ## Tests UOContent.Tests 776 / Server.Tests 855 green. `BaseCreatureSerializationTests` covers: a default creature elides to version + flags; a populated creature round-trips with exact byte consumption; back-to-back saves are byte-identical; an uncontrolled summon keeps its SummonMaster; byte-authentic v22 legacy streams (replicas of main's `Serialize`) load through the legacy reader for a wild tamable, a controlled pet, a controlled summon with an anchored `SummonEnd`, and a summon-master-only creature (the EnragedCreature shape); a running delete timer round-trips through `[DeserializeTimer]`; `Friends`, `CurrentWayPoint` and `HomeMap` round-trip; a `BaseVendor` stub round-trips the generated BaseVendor v2 → generated BaseCreature v23 chain. ## Behaviour notes for reviewers - `ActiveMoveSpeed`/`PassiveMoveSpeed` getters return the raw override (0 = inherit); `CurrentMoveSpeed` is the resolved pace. - Speeds elided as table defaults re-snap to the current `npc-speeds.json` on load, so table edits reach unmodified spawns on restart. - `GetSpeeds` no longer throws on the save/load path when the table has no entry for the type: saves elide against the creature's own values and loads keep the stream. Construction still throws (`InvalidOperationException`, was `KeyNotFoundException`). An elided load with no table entry would otherwise resume at speed 0, so `[AfterDeserialization]` logs once and paces it at Medium. - `virtual` removed from `ActiveSpeed`, `PassiveSpeed`, `DamageMin`, `DamageMax` (no overrides in the tree; forks may have some). - `ControlMaster`, `SummonMaster`, `ControlOrder`, `Tamable`, `IsParagon` are `[SerializableProperty]` over hand-written setters because follower bookkeeping must run before the assignment, which a `fieldChanged` hook cannot express; the wire format is identical. ## Prerequisites for cherry-picking #2609 (BaseVendor is already generated on top of BaseCreature) and SerializationGenerator 4.1.0.
… breeder and notoriety guards (#2613) Pet and summon bugs found by the master-reference audit for #2592 (comments there have the full inventory). Independent of delta saves. ## Releasing a pet never finished The Release order ran two half-releases that never met: - The order handler cleared the master. That set `Controlled` to false, so `Obey` never ran again and the think-side `DoOrderRelease` (re-home, three-day delete timer, backpack drop) was dead code: a released pet kept its pack and never despawned. - The loyalty drain called `DoOrderRelease` directly, so a pet whose loyalty hit zero got the countdown and dropped its pack but kept its master and its owner's follower slots, the opposite of the code comment's intent. `DoOrderRelease` is now the whole release (targets, bonding, `SetControlMaster(null)`, re-home, delete or countdown, pack drop) and runs once, synchronously, from the handler or the loyalty drain. Summons still die on release, as before. Two tests pin both entry points: master cleared, follower slots returned, countdown running, home anchored. This is the one place the `PetOrders` / `PetOrderHandlers` split bit; the wider audit of that duplication is a separate task. ## Summon master follows the pet Transfer, stable claim, GM "obey" and Ball of Summoning copied `SummonMaster` only when `Summoned`, so a talisman summon (`Summoned` is false, `SummonMaster` set) kept its original summoner after a transfer: two different masters on one creature, with the original summoner's area spells still exempting it. They now mirror the summon master whenever it is set. Jail stabling cleared only `ControlMaster`, so a jailed talisman summon kept charging the summoner's follower slots; it now clears both, like the stable master and auto-stable already do. ## Small ones - The faction horse breeder set `Controlled`/`ControlMaster` directly; it now goes through `SetControlMaster` and tells the buyer why it refused (1049607) instead of silently deleting the horse. - `Notoriety` dereferenced `SummonMaster` on a summoned creature without a null check. ## Tests UOContent.Tests 778 green (two new).
Converts BaseCreature's hand-written v22 serialization to the SerializationGenerator (v23), aimed at save freeze time and disk for worlds with 500k–1M creatures.
The win
~40 fields sit behind
[SaveFlag](a singleulongflags word). A creature matching its defaults — most of a live world — writes a 13-byte tail (4 version + 8 flags + 1 default-AI) instead of ~236 bytes, and the writer's work becomes ~40 cheap predicates plus one flags write instead of ~50 unconditional writes. Written ints are[EncodedInt].Table speeds are not persisted
The speed flags and their load defaults both resolve through
GetSpeeds/GetMoveSpeeds: a creature whose speeds match its npc-speeds bucket writes nothing and re-reads the table at load.CurrentSpeedwrites only when it differs fromPassiveSpeed. Consequences:Structure
Stateful
SpeedClass+ cached speed entry: the four per-save table checks (and elided-load defaults) read aSpeedClassEntrycached on the creature instead of walking the dictionaries each time. To keep the cache impossible to hold wrong,SpeedClassis no longer virtual: a type's constant bucket isvirtual DefaultSpeedClass, and runtime state changes (boss enrage,[props) assignSpeedClass— the setter invalidates the cache, applies the new bucket's think/move speeds (preserving active/passive mode), and serializes only when it differs from the type default, so a changed bucket survives a save while bucket-matching speeds still elide.GetSpeeds/GetMoveSpeedsremain the override point for bypassing the table, with no cache in that path.The four speeds are one block, and
Nonemeans custom: either all of active/passive think + move conform to the bucket (elided as a set) or the creature is custom and all four serialize — partial conformance cannot exist on the wire. The oldNone→ type list → Medium chain was construction-time defaulting, so the constructor now resolves it once intoSpeedClassitself: at runtime a concrete bucket means table-backed,Nonemeans the creature's own speeds are authoritative. Tuning any speed flips the bucket toNone(the label never lies), assigning a real bucket un-customs, runtime entry resolution is a single level lookup, and legacy loads demote nonconforming creatures (SetSpeedvendors) toNonehonestly.Old saves (v0–22) load through the retained legacy
Deserialize(reader, version), now assigning raw fields. Shared post-load fixups (stat timers,ChangeAIType, followers, animate-dead registration, unsummon timer, abandoned-pet fallback) moved to a single[AfterDeserialization]hook serving both paths.Delete countdown is a
[DeserializeTimer]field (anchored — downtime does not consume it, matching the old remaining-TimeSpansemantics). Stabled/controlled pets never persist one; the 3-dayLastOwnerfallback lives in the hook.SummonEndstays[AnchoredDateTime], written only while summoned.Side-effect setters became generated-field hooks:
Team,Controlled,Summoned, resistance seeds (UpdateResistances),CurrentSpeed(AI timer notify),Loyalty(clamp viaallowFieldChange).Five properties stay hand-written as
[SerializableProperty]withuseField:—ControlMaster/SummonMaster(follower bookkeeping brackets the assignment),ControlOrder(order logic must run on equal re-assignment for pet commands),Tamable/IsParagon(custom getter / conversion must not run at load).ActiveMoveSpeed/PassiveMoveSpeedcollapsed to plain[SerializableField]s (no longer virtual): they read the raw override (0= inheriting) andCurrentMoveSpeedcarries the inherit resolution.CreatureDeathEvent/CreatureDeletedEventmoved to aCreatureEventshost class: the events generator and the serialization generator each emit a[GeneratedCode]partial for the declaring type, and the attribute forbids duplicates (CS0579) — BaseCreature is the first class using both. Can fold back if this is ever fixed generator-side.One serialized master reference:
ControlMasterandSummonMaster, when both set, are always the same mobile (Summonassigns both; every management flow set them in lockstep) — they differ only in presence (uncontrolled summons vs pets). One_masterref serializes and fans back out through theControlled/Summonedflags at load;SetControlMasternow maintains the lockstep itself, deleting six hand-rolled copies of that boilerplate.Missing npc-speeds.json degrades gracefully: a creature constructed without speeds logs debug and defaults to Medium (0.25/0.5) instead of spawning a 0-delay AI timer.
m_fields renamed to_camelCase;MoveSpeedMod-era vestiges stay gone.Testing
BaseCreatureSerializationTests: default and fully-populated round trips with exact byte-consumption asserts; back-to-back saves byte-identical (anchored timer keeps idle saves stable); bucket assignment and partial-tuning-flips-to-custom round trips. (A byte-authentic fossilized v22 stream was validated through the legacy path during development, then dropped to avoid test bloat.)Server.Mobiles.BaseCreature.v23.jsongenerated and committed.🤖 Generated with Claude Code