Savegames: load a real .ess and play the world it describes - #35
Merged
Conversation
…es to their new cells
…at the record gives it
Memory safety on untrusted bytes. The Papyrus flattener fed push_back an element of the vector it was growing, so an inherited member name came back as a fragment of a freed heap pointer; the parent chain now has a depth cap as well, since a save alone decides how long it is. Change-form inflation gains a running byte budget, because a per-record ratio bound still lets a small file ask for hundreds of gigabytes. Cell coordinates out of a save are checked for finiteness rather than cast straight to i16. The actor perk and spell scans get a work budget so a crafted block cannot hang the load. VM lifetimes. SetDeclaredMember assigned through operator[] using a key that references the map's own slot, which a rehash destroys first; it now assigns through the iterator. AddScript no longer replaces a loaded script, which would free bytecode a parked fiber is still executing. World state. A reference the save deleted stays deleted instead of coming back enabled (Skyrim writes the deleted flag, not the disabled one, for a looted item). An emptied container keeps a live, empty inventory, so it is no longer refilled from its record on reopen. A quest whose change form recorded no stages is no longer marked as having reached stage 0. A relocated reference whose parent is neither cell nor worldspace is refused rather than filed under a cell that never streams, which made it vanish. A bailed-out player location no longer teleports the player into an unstreamed part of another worldspace. Riders can get off a ride. Map labels fall back to inline FULL text before editor ids. The papyrus restore test expected 10 restored members; that count was the use-after-free, not a rule, and it is 11.
__builtin_popcount does not exist on MSVC, so the windows build failed on it. std::popcount is the C++20 spelling and compiles everywhere.
Force67
force-pushed
the
feat/helgen-from-records
branch
from
August 28, 2026 13:40
8d80dc2 to
fcad21d
Compare
A .ess is the only file this engine parses that the player chose, and every count, offset and length in it is a number the file picks. Three memory-safety bugs shipped in this reader and all three were found by reading it: a self-referential push_back that read freed heap, a parent chain that recursed as deep as the file said, and a per-record inflate bound with no running total. Reading is not a repeatable way to find the fourth. savegame_fuzztest drives all three layers - the container, the Papyrus heap and the change form decoders, then ApplySave over the lot - with truncations at every length, single-byte corruption at every offset, multi-byte mutation and pure garbage. About 26000 hostile files a run, from a fixed seed, so a failure reproduces. It asserts only that the reader returns rather than crashes, which is the property that matters and the one no unit test states. The synthetic saves move out of savegametest into savegame_fixture so the fuzzer seeds from the same bytes the assertions check, and the Papyrus heap builder moves out of papyrus_restoretest for the same reason: mutating a valid table reaches the flattener, while assembling bytes blind is refused at the header and proves nothing. Verified the harness has teeth: reintroducing the push_back bug and running under RECREATION_SANITIZE reports heap-use-after-free at savegame_papyrus.cc:355 within one run. Clean on the fixed tree. Two more findings from the same reading. A reference's scale is four file-chosen bytes headed for a transform and a Jolt shape, so a NaN or an absurd magnitude now leaves the record's own scale in place. ReadWholeFile gains a size ceiling, because --load-save takes a path and a typo naming a disk image should not be read into memory before the reader gets to refuse it.
Chargen rewrites the player's race and sex, and the save carries that rather than the generic Player NPC_ record. The head already asked the save, through FaceBuilder's overrides; the worn armour still read the record, so a resumed character wore the armature for whoever the plugin shipped - the wrong race's model, or none at all when the ARMA the record picks has no model for the body that is actually standing there. Both questions now have one answer. Measured on the reference save: 6 actors come back in a race other than their record's. The container tally also says how many chests stay empty rather than only how many were seeded, which is the visible half of the emptied-container fix: 3433 of them on that save.
mem_size is long long unsigned, which %zu is not.
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.
Load a real Skyrim savegame and play the game it describes.
The branch started on the Helgen cart ride and ended somewhere larger: an
11 MB 100%-complete
.essnow boots into a world that is where the save leftit, standing on the ground, carrying what the save says the player carries.
Reading the file
components/bethesda/savegame.hsplits the job three ways, because the threeparts fail differently and want testing apart: the container parse, the
ChangeForm payload decode, and applying the result through an abstract sink that
runtime/app/savegame_load.ccimplements. Nothing above the container layerbranches on the game, so Skyrim LE/SE and Fallout 4 ride one path.
Every field here was measured, not read off a wiki. UESP, FallrimTools and sol
were each wrong about something we had to decode: a REFR field that does not
exist, a vsval tag width, the FACT subrecord order, a CELL flag map, the Fallout
4 change-form type numbers, and FallrimTools' u16 string index in the Papyrus
table, which is really u32. The bar for a decode was a walk that consumes its
payload to the last byte plus an independent cross-check, resolving the id and
reading the record signature back out of the masters.
What comes back
The 100% save reports, at load: 933 globals; 1547 quests with 4450 stages, 1431
objectives and 1785 alias fills; 926 actors, their levels, temperaments, faction
ranks and infamy; 9822 dialogue lines already heard; 11153 explored map grids
and 424 discovered markers, 420 of them travelable; 374 locations, 233 encounter
zones, 766 books read, 108 ingredients with known effects; 69194 item stacks
into 20035 containers; 297 perks, 164 spells, 28 shouts, 84 words of power; the
player's face, race and equipped gear; the sky it was left under; and 122671
addressable Papyrus script instances with 827439 member variables put back into
the VM, including the states they were suspended in.
The empty grey world
A resumed save opened on a flat grey plane. The fly camera at the same
coordinates rendered Skyrim fine, which is what pointed at the player rather
than the world.
A per-frame probe of (position, terrain height, in-interior) found only two
frames between placing the player and them being 310 m under the map. The
frame that finishes streaming a cell ring is seconds long, and the character
controller integrated it as a single gravity step, through a heightfield
collider that had not been built yet. Nothing was wrong with the save, the
placement, or the renderer.
Three fixes:
OnSimulate/OnUpdateclamp the step to 1/15 s, because a longframe is a stall to absorb and not time the world has to live through;
PlayerController::KeepAboveTerrain()puts the player back when they are morethan 3 m under
GroundHeight, which is decoded from LAND long before thecollider exists; and every placement onto terrain now leaves
CellStreamer::kGroundClearance, since a capsule set down exactly on theheightfield starts inside it and the character solver resolves that downward.
Where a moved thing goes
A record authors a reference in one cell and the streamer places it when that
cell comes in. 1742 of the save's 26392 moved references had left that cell, so
they used to appear where the record puts them and vanish with a cell they were
no longer standing in. They are binned into the cell they now occupy
(
SavedSpawnIndex,relocated), skipped by the cell that authors them, andplaced from their own record with the save's placement substituted, so a
re-homed door keeps its destination, a container its contents and a scripted ref
its script.
Carrying it
The save's inventory landed in the script bindings, which is where Papyrus asks
about it, and nowhere the world could use.
ItemBridge::SeedFromSavegamenowmirrors the player's share into the rx
Inventory(275 stacks, 1.7 M items);world models are built lazily, since a save restores hundreds of item kinds and
almost none are ever dropped. A save-booted run neither reads nor writes the
sandbox profile: the save is the whole account of what that player owns.
Opening a chest read the container record's authored CNTO list, so a chest the
player emptied hours ago came back full. It now reads the live inventory
whenever one exists, which is exactly what the save seeds and what a script
writes to.
The 394 forms the save invented -- potions and poisons the player brewed, the
enchantments they made -- have no record anywhere, so they come back under a
handle in
bethesda::kCreatedFormPlugin, described byworld::CreatedFormsandnamed after their strongest effect the way the game names them. The 64 item
stacks that name one now land instead of being dropped.
Along the way
The branch also carries the Helgen opening driven off MQ101's own stage and
package chain rather than a scripted prototype (the journal is a high-water
mark; a travel package's target is the head of an XLKR chain, not the
destination; a
LoadedScriptheld by value rehashed out from under a suspendedfragment's
PexFile&), the flat monochrome HUD and menu rebuild with the pagedlauncher front screen,
game_ui.ccsplit into editor/menu/document units, theSteam Deck platform profile with finger-sized menus, and a managed cart-racing
game mode.
Verification
101/101 ctest. A five-minute soak from the 100% save with 1547 quests and 125 K
script instances running, no crash and no new errors. Screenshot-verified: the
resume standing in a snowstorm on solid ground, and a fast travel landing in a
fully rendered Riverwood with an NPC activate prompt.
Known gaps
Seven cells whose owner changed and 201 actor bases with no placed reference in
this load order are still dropped, and the load report says so rather than
staying quiet about it. There is no player inventory screen yet, so the seeded
pack is reachable through equip and drop only -- related, 1.69 M gold splits
into ~1694 stacks against
ItemDef::max_stack, which is harmless today andwants fixing before that screen exists. The Fallout 4 path is written and unit
tested but unverified end to end: there is no
.foson the machine this wasbuilt on.