Add add_record_override() for REFR overrides with proper group hierarchy - #4
Open
jeditobe1 wants to merge 3 commits into
Open
Add add_record_override() for REFR overrides with proper group hierarchy#4jeditobe1 wants to merge 3 commits into
jeditobe1 wants to merge 3 commits into
Conversation
REFR and other placed records must live inside the correct cell/ worldspace group structure (WRLD > worldspace > cell block > persistent children) — and alongside parent WRLD/CELL records the engine expects in the hierarchy. The existing add_record() places everything in flat type-0 groups, which causes save-time crashes for REFR overrides. add_record_override(record, source_plugin) finds the group path to the record in the source plugin (collecting parent WRLD before type-1 groups and parent CELL before type-6/8/9 groups), then creates/finds matching groups in the destination and clones the parent records into place. Falls back to flat grouping if the path isn't found. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…cord_override
When add_record_override clones a parent CELL/WRLD record to host
the override REFR's group hierarchy, copy.copy() brings every
subrecord along verbatim — including FULL string IDs and FormID
subrecords (LTMP, XLCN, XEZN, XOWN, etc.) that point into the
source plugin's tables and master list. Skyrim does NOT field-merge
CELL overrides, so the cloned CELL fully replaces vanilla and the
engine reads stale bytes when resolving names and references.
Symptoms before this fix:
- In a non-localized destination the FULL bytes rendered as garbage
windows-1252 text ("Xa") for any cascade walking into the cloned
cell (REFR -> XTEL -> dest REFR -> parent CELL -> FULL).
- In a localized destination the FULL ID resolved against our own
empty/wrong table.
- LTMP/XLCN/etc. with master indices pointing at the source plugin's
self-index aliased to a different plugin after master-list
reordering, half-loading the cell at runtime (skybox only) on any
transition into the clone. Surfaced on Skyrim Randomizer Phase 2.7
door gating: LTMP on DLC1DimHollowCrypt01 was 0x02006AE9 (master 2
= Dawnguard self in Dawnguard.esm), should remap to master 1 in a
destination pack with master list [Skyrim, Dawnguard,
SkyrimRandomizer].
Fixes for the parent-clone path in add_record_override:
1) String localization: branch on destination localization.
- Non-localized destination: call _delocalize_strings to resolve
the source's string ID via the source plugin's strings table
and write the resolved text as inline cp1252 bytes. Mirrors
the existing copy_record delocalize step.
- Localized destination: call new _localize_strings to resolve
the source's string ID, allocate a fresh ID via
StringTable.allocate_id (max+1, starting at 1), and write the
new 4-byte ID + register the text in our table. The
destination string_tables is lazy-initialized as a fresh
StringTableManager if not already present.
2) FormID remapping: call _remap_subrecord_formids on the parent
clone before localization fixups, and remap the cloned parent's
own FormID through remap_formid (the override record itself was
already getting this treatment further down; parent clones were
missing it).
3) Extend _FORMID_SUBRECORD_SIGS_FALLBACK with CELL FormID subrecord
signatures (LTMP, XLCN, XEZN, XCMO, XCAS, XCCM, XCIM, XCWT, XOWN).
Without them, CELL records hitting the fallback path (no schema
bound) still miss their FormID subrecords.
Adds:
- Plugin._LOCALIZED_STRING_TABLE_TYPE: signature -> table-type map.
Verified against vanilla Skyrim.esm: FULL/SHRT/ITXT live in
.STRINGS, DESC/NNAM in .DLSTRINGS. Note that MESG ITXT lives in
.STRINGS not .ILSTRINGS despite the extension naming.
- Plugin._localize_strings: inverse of _delocalize_strings.
- StringTable.allocate_id: simple max(keys)+1 allocator, starts at 1.
Verified end-to-end via test_door_lock_esplib.py fixture in the
randomizer repo, both flavors:
- Flavor A (non-localized destination): cloned BFB01 CELL FULL bytes
become b'Bleak Falls Temple\x00'.
- Flavor B (localized destination): cloned BFB01 CELL FULL bytes
become a 4-byte string ID, sidecar contains the text at that ID.
In-game (Skyrim SE): both flavors display "Bleak Falls Temple"
correctly on the BFB main entrance prompt and on the deep interior
REFR 0x37603 (BFB01 -> exterior reverse door) which was previously
unfixable under any workaround.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds Plugin.clone_for_override(source_record, source_plugin) as a documented public alternative to caller-side hand-rolled REFR clones. It deep-copies the source record and remaps master indices in subrecord FormIDs (NAME, XTEL, KWDA, VMAD, etc.) from the source plugin's master ordering to ours, but leaves the record's own FormID in source-plugin space so add_record_override can use it to look up the source group hierarchy. Also updates add_record_override to remap the record's own FormID from source-plugin space to destination-plugin space after the group path lookup completes. This was previously a silent latent bug — when overriding a record from a non-Skyrim master into a plugin where that master's index differs (e.g. Dawnguard.esm at index 0 in Dawnguard.esm itself, but at index 1 in a multi-master content pack), the override's own FormID kept the source-side master index and ended up pointing into the wrong slot of the destination's master list. Skyrim.esm-only callers were never affected because index 0 maps to index 0 in any master list that includes it. Both changes are backward compatible: clone_for_override is a new method, and the FormID remap inside add_record_override is a no-op when source and destination master orderings already align (which is the only state existing callers exercised). Co-Authored-By: Claude Opus 4.6 (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
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.
add_record() places everything in flat type-0 groups, which causes
save-time crashes for REFR overrides — placed records must live
inside the correct cell/worldspace structure (WRLD > worldspace >
cell block > persistent children) alongside the parent WRLD/CELL
records the engine expects.
This PR adds
add_record_override(record, source_plugin)plus thesupporting machinery to clone a placed record from a source plugin
into a destination patch with the full hierarchy intact.
Three commits, each with a self-contained rationale in the message:
Add add_record_override() with proper REFR group hierarchy.
Walks the source plugin's group path, collecting parent WRLD
before type-1 groups and parent CELL before type-6/8/9 groups,
then creates/finds matching groups in the destination and clones
the parent records into place.
Translate strings and remap FormIDs on parent-record clones.
copy.copy() of the parent CELL/WRLD brings every subrecord along
verbatim — including FULL string IDs and FormID subrecords (LTMP,
XLCN, XEZN, XOWN, etc.) that point into the source plugin's
tables and master list. Skyrim does not field-merge CELL
overrides, so the clone fully replaces vanilla. Without this fix
cell names render as garbage (non-localized destination) or
resolve against the wrong table (localized destination), and
FormID subrecords with self-index master refs alias to the wrong
plugin after master-list reordering, half-loading the cell at
runtime.
Adds
_localize_strings(inverse of_delocalize_strings),StringTable.allocate_id, and a_LOCALIZED_STRING_TABLE_TYPEsignature → table-type map (FULL/SHRT/ITXT → .STRINGS,
DESC/NNAM → .DLSTRINGS; note MESG ITXT lives in .STRINGS not
.ILSTRINGS despite the extension naming). Extends
_FORMID_SUBRECORD_SIGS_FALLBACKwith CELL FormID signatures(LTMP, XLCN, XEZN, XCMO, XCAS, XCCM, XCIM, XCWT, XOWN) for
records that hit the fallback path with no schema bound.
Add clone_for_override + remap record FormID in
add_record_override. Documents the deep-copy + master-remap
step as a public method, and fixes a latent bug where the
override record's own FormID kept the source-side master index
when source/destination master orderings differed (no-op for
Skyrim.esm-only callers since index 0 maps to 0 in any list
that includes it).