fix: size migration content SaveFlag enums like the live writer (v4.1.1) - #59
Merged
Merged
Conversation
The V{N}Content struct emitted one int-backed V{N}SaveFlag regardless of
flag count. The live generator that wrote vN uses ulong past 32 flags and
one extra ulong enum per 64 past 64, so migrating from a version with 33+
save-flagged fields failed with CS0266, and 65+ would misread the stream.
The content struct now partitions flags with the same rule, reads every
flag enum before any field, and tests each field against its own enum.
Output for 32 or fewer flags is unchanged.
Adds a round-trip test (20/40/70 flags) that serializes with the live
generator and migrates through MigrateFrom(V0Content), plus a
MigrationSaveFlagsMultiEnum snapshot.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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
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.
Problem
GenerateMigrationContentStructemitted a singleV{N}SaveFlagenum with no underlying type (soint), whatever the flag count. The live generator that wrote vN sizes itsSaveFlagenums from the count:intup to 32 flags,ulongfor 33-64, then one extraulongenum (SaveFlag2, ...) per 64 flags.0x80000000into anintenum, which fails withCS0266: Cannot implicitly convert type 'uint' to 'int'. This blocks ModernUO #2670, which needs to migrate fromBaseCreaturev23 (54 save-flagged fields).Fix
The content struct now uses the same partitioning as the live side (
Class.cs):V{N}SaveFlag,V{N}SaveFlag2, ... are: ulongonce there are more than 32 flags, with values fromGenerateEnumValueLongOutput for 32 or fewer flags is byte-identical, and every existing snapshot still matches.
Readonly fields can't shift the bit numbering:
Class.csalready leaves them out of the migration schema, the same way the live side skips them for flag bits.Tests
MigrationSaveFlagTests: a real round trip at 20, 40 and 70 flags. It compiles a v0 entity, serializes it with the live generator through a binary writer that sizes enums by their underlying type (as ModernUO does), then deserializes the bytes into v1 throughMigrateFrom(V0Content). It checks every value (present values around bits 31/32 and 63/64, unset fields asnull, a flagged bool) and that the whole stream was consumed. Before the fix, the 40 and 70 cases failed with the CS0266 above.Snapshots/MigrationSaveFlagsMultiEnum(66 flags) pins the emittedV0SaveFlag : ulong/V0SaveFlag2.SourceGeneratorTestHelper.CompileAndLoadso tests can execute generated code. It shares its compile code withRunGeneratorAllOutputs.dotnet test: 119 passed, 0 failed.Release
Bumps Annotations, Generator and SchemaGenerator to 4.1.1.