Repository navigation
refactor: Changes BaseCreature to the SerializationGenerator (delta save requirement) - #2611
Merged
Merged
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> (cherry picked from commit f4327e6)
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> (cherry picked from commit 963b9b3)
(cherry picked from commit 232c5ed)
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> (cherry picked from commit 93b9238)
No overrides exist. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit f46780d)
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> (cherry picked from commit f2f8313)
The bulk helpers (SetDamage, SetResistance, ClearMoveSpeed, ScaleMoveSpeed, SnapSpeedsToTable), the loot-generated flag, the feed-loyalty bump and the delete-timer field assign generated fields directly, bypassing the setters that mark the entity dirty. Mark explicitly at those sites, matching the standard #2609 set for hand-written [SerializableProperty] setters. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
An energy-vortex-style creature (Summoned, never Controlled) must keep its SummonMaster with no ControlMaster. BaseVendor (generated, v2) now sits on a generated BaseCreature; a vendor stub with an empty SBInfo table must write and read both sections in order with exact byte consumption. Also normalizes the indentation of the fossilized v22 replica writer. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The AI setter and SetResistance(type, val) assign generated fields directly without a mark. The three delete-timer clears (OnAfterDelete, SetControlMaster, StopDeleteTimer) now route through the generated StopPendingDeleteTimer(), which stops, nulls and marks in one place. The earlier sweep also dropped a mark into SetSkill by mistake - Skills are Mobile state, not a generated field here - so that one goes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The speed SaveFlags consult GetSpeeds on every save and every elided load, so a shard whose Data/npc-speeds.json is missing or no longer covers a type would finish loading and then fail its first save. Without a table entry the creature is now its own reference (its serialized speeds stand, and elision compares against them), and only the constructor refuses to proceed - the same place main failed, with a plain-string message instead of the misapplied property-list hole. A creature whose elided speeds cannot be restored (table changed or vanished since the save) would otherwise resume at 0 and spin its AI timer at wheel resolution; AfterDeserialization logs once and paces it at the Medium bucket's values instead. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Legacy v22 streams now cover a controlled pet, a controlled summon with SummonEnd on the wire, and the EnragedCreature shape (SummonMaster with neither flag) - both references must survive the load independently. A running DeleteTimer round-trips through [DeserializeTimer] with its remaining countdown, and Friends/CurrentWayPoint/HomeMap round-trip as populated references. Fixes the stale "v22 tail" comment in MoveSpeedTests. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
kamronbatman
force-pushed
the
kb/basecreature-codegen
branch
from
September 6, 2026 21:08
9453ef4 to
14c4532
Compare
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.
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 ofControlMaster/SummonMasterinto 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
BaseCreaturebecomes[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).private void Deserialize(IGenericReader reader, int version)for every pre-codegen version (0..22); post-codegen bumps useMigrateFromfrom here on.[AfterDeserialization]carries the post-load fixups main did after reading (stat timers, AI type, followers, reacquire seeding).ControlMasterandSummonMasterstay two independent fields with main's semantics, each elided when null.DamageMin/DamageMax/ActiveSpeed/PassiveSpeedare no longer virtual (nothing in the tree overrode them); comments swept to the constraints that matter.SetDamage,SetResistance, the move-speed helpers, the loot flag, feed loyalty, the delete timer) callthis.MarkDirty(), matching the fix: Fixes dirty-tracking gaps in generated content: setters, sub-object owners, BaseVendor #2609 standard, so the class is ready for delta saves onceMobileis audited.Server.Mobiles.BaseCreature.v23.jsonregenerated by the tool (a second run produces no diff).Deferred to follow-up PRs
SpeedClass: the serialized
_speedClassfield,DefaultSpeedClassreplacing 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
Masterwith aControlled/Summonedfan-out and theSetControlMasterlockstep.Tests
UOContent.Tests 776 / Server.Tests 855 green.
BaseCreatureSerializationTestscovers: 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'sSerialize) load through the legacy reader for a wild tamable, a controlled pet, a controlled summon with an anchoredSummonEnd, and a summon-master-only creature (the EnragedCreature shape); a running delete timer round-trips through[DeserializeTimer];Friends,CurrentWayPointandHomeMapround-trip; aBaseVendorstub round-trips the generated BaseVendor v2 → generated BaseCreature v23 chain.Behaviour notes for reviewers
ActiveMoveSpeed/PassiveMoveSpeedgetters return the raw override (0 = inherit);CurrentMoveSpeedis the resolved pace.npc-speeds.jsonon load, so table edits reach unmodified spawns on restart.GetSpeedsno 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, wasKeyNotFoundException). An elided load with no table entry would otherwise resume at speed 0, so[AfterDeserialization]logs once and paces it at Medium.virtualremoved fromActiveSpeed,PassiveSpeed,DamageMin,DamageMax(no overrides in the tree; forks may have some).ControlMaster,SummonMaster,ControlOrder,Tamable,IsParagonare[SerializableProperty]over hand-written setters because follower bookkeeping must run before the assignment, which afieldChangedhook 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.