Dirty-tracking diagnostics, [VolatileSerializedState], and StopXxx timer helpers - #55
Merged
Merged
Conversation
…et; SG3020 rejects [ManualDirtyChecking] on generated classes A [SerializationGenerator] reference type that is neither ISerializable nor declares a [DirtyTrackingEntity] member resolves markDirtyMethod to null, and every generated setter and collection mutator silently marks nothing. SG3019 (warning) names the class; the collection mutators no longer emit a bare ';' in that case. SG3020 (error) rejects [ManualDirtyChecking] on a generated class, since the generator already tracks and the attribute is a false claim. Existing snapshots are byte-identical; NoDirtyTarget pins the no-target emission. A new test asserts every DiagnosticDescriptor is registered with DiagnosticInfo. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…elpers A serialized Timer persists its next tick, which moves whenever the timer fires, restarts, or is stopped on the timer object; a [DeltaDateTime] field is written relative to the save. Neither passes through the entity, so dirty-flag delta saves cannot trust such a type. The generator now stamps [VolatileSerializedState(reason)] on those classes (a hand-declared attribute wins and suppresses emission) so the runtime can keep them on the always-serialize path, and emits StopXxx() for every serialized timer field so stopping goes through the entity and marks it dirty. Snapshots: Timers, TimerAndDirtyTracking, NonServerNamespace and BasicFields change by the attribute line and the helpers only; everything else is byte-identical. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
kamronbatman
changed the title
… of declaration order TryGetEmptyOrParentCtor took the first matching constructor, so a sub-object declaring its parameterless constructor before the parent one was deserialized with no owner and could never mark it dirty. The parent-accepting constructor now wins whenever one exists; the parameterless one is the fallback. New fixture ParentCtorPreferred pins list, dictionary and single-field cases; no existing snapshot changes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
kamronbatman
marked this pull request as ready for review
September 6, 2026 04:04
kamronbatman
added a commit
to modernuo/ModernUO
that referenced
this pull request
Sep 6, 2026
…ect owners, BaseVendor (#2609) ## Why Delta world saves re-serialize an entity only when it has been marked dirty. An audit of the generated classes found three ways serialized state changes without a mark; this PR closes the ones that do not need a new generator package. ## What - **Custom `[SerializableProperty]` setters mark dirty.** Twelve hand-written setters assigned their backing field without `this.MarkDirty()`. `PlagueBeastLord.OpenedBy` was an auto-property carrying the attribute with no backing field at all; it is now a generated field with the same name, order and command-property exposure. - **Generated sub-objects are linked to their owner.** Without a `[DirtyTrackingEntity]` member the generator emits setters that mark nothing. Eight entity-owned sub-objects now carry the link and receive the owner through the constructor the generator calls: `BOBFilter` (owner is `IEntity`: a `PlayerMobile` or a `BulkOrderBook`), `BOBLargeSubEntry`, `PuzzleChestSolution` and `PuzzleChestSolutionAndTime`, `TalismanAttribute` (the random factories now take the talisman), `VendorItem`, `PlayerBBMessage`, `RaffleEntry`, `ShardPollOption`. Migration schemas were regenerated with the pinned tool; the only change is the value rule argument becoming `DeserializationRequiresParent`. - **BaseVendor uses the generator; restock amounts are no longer persisted.** The only state it wrote was which buy entries had grown restock amounts, packed by index into the live `SBInfos` tables. Restock is transient now and rebuilds on load; version 1 records are read and discarded through the legacy path. Every vendor subclass now serializes through a generated chain. ## Generator 4.1.0 This PR adopts SerializationGenerator 4.1.0 (modernuo/SerializationGenerator#55): SG3019/SG3020 diagnostics, `[VolatileSerializedState]`, `StopXxx()` timer helpers, and owner-constructor preference for sub-objects (so dictionary values are constructed with their owner; the 4.0.0 relink fallback is gone). `PlayerVendor.v3.json` gains `DeserializationRequiresParent` for its `VendorItem` values so the migration content struct constructs them with their vendor. SG3019 is an error under `TreatWarningsAsErrors` and generator diagnostics ignore pragmas, so the five contexts that live in whole-file player-keyed persistences (`ChampionTitle`, `ChampionTitleContext`, `MurderContext`, `VirtueContext`, `JailRecord`) now carry a `[DirtyTrackingEntity]` link to their `PlayerMobile`, which is where they will live once those blobs move onto the player record. `JailSystem.EmptyRecord` keeps a null player (`[CanBeNull]`). `ChampionTitle` needs both a context and a player constructor because the generator resolves the rule against the containing type but emits the call with the parent field; a comment in the file records this. ## Wire format Unchanged. No version bumps except `BaseVendor` 1 to 2 (which now writes nothing of its own). Schema diffs are rule-argument only (`DeserializationRequiresParent`). ## Testing Server.Tests 848 passed, UOContent.Tests 759 passed.
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.
Why
ModernUO is moving toward delta world saves, where an entity is re-serialized only when it has been marked dirty. Two generator gaps make that unsafe today:
[SerializationGenerator]reference type that is neitherISerializablenor declares a[DirtyTrackingEntity]member getsmarkDirtyMethod == null, and every generated setter and collection mutator silently marks nothing. ModernUO has 14 such classes (BOB entries, champion titles, murder and virtue contexts, vendor items, jail records, and others), none of which has ever warned.Timerpersists its next tick (which changes when it fires, restarts, or is stopped on the timer object), and[DeltaDateTime]fields are written relative to the save. The runtime has no way to know which types are like that.What
readonlyare exempt. Generation still proceeds. Collection mutators no longer emit a bare;when there is no target.[ManualDirtyChecking]combined with[SerializationGenerator]. The generator already tracks; the attribute is a false claim.[VolatileSerializedState(VolatileReason)](new, Annotations): emitted on classes with a serialized timer or a[DeltaDateTime]field. A hand-declared attribute wins and suppresses emission, so a class can also declareVolatileReason.Declaredfor state the generator cannot see. The runtime uses it to keep such types on the always-serialize path.StopXxx()for every serialized timer field: stops the timer, nulls the field, marks dirty. TheDeserializeTimerAttributedocs now recommend persisting an[AnchoredDateTime]deadline through its property (and keeping the timer unserialized) for types that should stay eligible for dirty-flag skipping.DiagnosticInforegisters the new ids; a reflection test asserts everyDiagnosticDescriptoris registered.Snapshots
New fixture
NoDirtyTarget.Timers,TimerAndDirtyTracking,NonServerNamespaceandBasicFieldschange by the attribute line and theStopXxx()helpers only; all otherExpected/files are byte-identical.Versioning
Additive: minor bump (4.1.0). SG3019 ships as a warning. Note for ModernUO: it builds with warnings as errors, so the package bump must land together with fixes for the 14 affected classes (or a temporary
WarningsNotAsErrorsfor SG3019).Testing
113 tests green (11 new: SG3019 positive and four negative cases, SG3020, descriptor registration, and four volatile-state/timer-helper cases).