ChangeForm parser coverage fixes for heavily-modded saves (7 byte-exact fixes)#1
Open
WingedGuardian wants to merge 7 commits into
Open
ChangeForm parser coverage fixes for heavily-modded saves (7 byte-exact fixes)#1WingedGuardian wants to merge 7 commits into
WingedGuardian wants to merge 7 commits into
Conversation
Type 16 is part of the ExtraExtraData family (4/8/12/16/20), each reading N = type/4 nested ChangeFormExtraDataData with no count prefix. It was left commented out, so any extra-data array containing it desynced. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Final member of the ExtraExtraData family; reads 5 nested ChangeFormExtraDataData. Previously hit the default case. Validated byte-exact across 3 saves. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…mbs) case 135 was an unfinished TEST stub reading 4 RefIDs; the real record is larger, so the stub under-read and desynced the extra-data array. The misaligned bytes were then misread as bogus unknown types and Excessive array count errors, failing ~450 follower changeforms per save. Layout per UESP. Validated byte-exact (buffer position == bodyLen) across saves; phantom-type failures cleared with no regressions. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Type 113's DATA2 is a SayToTopicInfoData (vsval count + SayToTopicInfoData2[count] + uint16 + refID[4]), not a single bare SayToTopicInfoData2. The unread count byte corrupted the following TEXT1 wstring length -> BufferUnderflowException. Validated byte-exact; recovered the affected REFR/ACHR forms with zero regression. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…ctives/run-data readVSElemArray/readElements always return Element[], so the casts to QuestStage[], QuestObjective[], QuestRunDataItem1[], QuestRunDataItem2[], RefID[] threw ClassCastException unconditionally whenever the matching quest change flag was set. Only quests with none of those flags parsed. Byte-neutral: relax the 5 casts and 5 field types to Element[]. +2146 quests recovered on a 708-plugin save. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…nparsed gate Two byte-level parser bugs on quests with live run-time state: - CHANGE_QUEST_INSTANCES (bit 27) was never read; per UESP a QuestInstances block sits between RUNDATA and ALREADY_RUN, so every instanced quest silently under-read. - QuestRunDataItem3Data threw on type 0, which is valid and refID-typed (like 1/2/4). Also adds the missing self-checking 'if (!inline && readUnparsed) throw' gate that REFR/ACHR have (its absence is why the under-reads were silent). Validated byte-exact (position == body.limit) on all 2611 QUST across 3 saves; 0 throw, 0 residual. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…rflow) The promoted-ref list is already read as the EXTRADATA PromotedRef element (ChangeFormExtraDataData case 140), which the CHANGE_REFR_PROMOTED flag also triggers. The separate CHANGE_REFR_PROMOTED read here is a duplicate that consumes past the body end and throws BufferUnderflowException on every promoted REFR. Removing it recovers thousands of REFRs per save; every recovered form reaches the readUnparsed() boundary. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
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.
ChangeForm parser coverage fixes for heavily-modded saves
Seven byte-level fixes to the
resaver.essChangeForm parsers, found while parsing a heavily-moddedSkyrim VR save (708 plugins). Each was derived by decoding the actual save bytes and validated
read-only and byte-exact before inclusion — no fix was guessed.
How these were validated (no-guess methodology)
REFR and ACHR end their parse with
if (super.readUnparsed(input)) throw new UnparsedException(), so acorrect body layout consumes the whole body and leaves zero residual. That boundary is the oracle:
a candidate layout is accepted only when every affected change-form reaches
position == body.limit()across three independent saves with no regression on the rest. For QUST (which lacked that
boundary) the body length is known, so the same
position == limitcheck was constructed as a gate —and the missing boundary is added here so the reader is self-checking like the others. Layouts were
cross-referenced against the UESP Save File Format pages where documented.
The fixes
ChangeFormExtraDataDataExtraExtraData4, 4 nested)ChangeFormExtraDataDataExtraExtraData5, 5 nested)default→ throwChangeFormExtraDataDataFollowerSwimBreadcrumbs)ChangeFormExtraDataDataSayToTopicInfo) under-readBufferUnderflowExceptionChangeFormQustClassCastException(castElement[]to concrete types)ChangeFormQustQuestInstances+ acceptQuestRunDatatype 0 + addreadUnparsedgateChangeFormRefrBufferUnderflowExceptionon every promoted REFR (thousands/save)Notes:
ChangeFormQustand are ordered so the cast fix lands first; #6 is split outbecause it is a larger, independent parsing change (and adds the boundary check).
PromotedRefEXTRADATA element (case 140),which the same
CHANGE_REFR_PROMOTEDflag triggers; the separate read was a duplicate.message.
Scope / safety
Read-path only — no change to
write()/updateRawData, so this cannot alter how a save is written; itonly lets ReSaver parse change-forms it previously failed on. Validated on three saves with zero
regression to the previously-parsing forms. Builds cleanly against the current tree.
🤖 Generated with Claude Code