Skip to content

Skip objects whose bodies won't parse instead of failing the load - #31

Merged
valentinps merged 2 commits into
mainfrom
fix/tolerate-unparsable-objects
Sep 13, 2026
Merged

valentinps merged 2 commits into
mainfrom
fix/tolerate-unparsable-objects

Conversation

@valentinps

Copy link
Copy Markdown
Owner

Closes #30.

The bug

The reported save is modded (Modular Load Balancers, FicsitWiremod and others). 106 of its 37,958 objects would not re-parse, and a single one of those sank the entire load:

Failed to load save: object re-parse failed during index build:
object at (3946, 5836): String decode failure at offset 24369724 of length 3670015

All 106 are Modular Load Balancer buildables (80 Filtered_Output, 24 Input, 2 Output). Each carries an LBBalancerData struct holding mIndexMapping, a MapProperty of ObjectPropertyStructProperty("LBBalancerIndexing"). That value is a native struct written as 12 raw bytes (three int32s), not the tagged property list the parser expects:

+1124  08                 zero_or_eight
+1125  00 00 00 00        no ModeType
+1129  01 00 00 00        1 entry
+1133  00 00 00 00        key level name = ""
+1137  57 00 00 00 ...    key path name  = ".../Desc_CompactedCoal_C"
+1228  ff ff ff ff ff ff ff ff 37 00 00 00     <- the value: 12 RAW bytes
+1240  0c 00 00 00 "mInputIndex\0"             <- next property, exactly at start+115

start + 115 lands precisely on the next property, so the map body is well formed — the value is simply opaque. parse_properties read those bytes as a property name/type pair: ff ff ff ff decoded as an empty UTF-16 string, then the next length came out of ff ff 37 00 = 0x0037FFFF = 3670015, which fails UTF-8 validation. (The two Output failures are the same bug with a different payload — 0a 00 00 00 ... reads as a plausible length 10, hence the different message.)

StructProperty and ArrayProperty already fall back to opaque bytes for unknown modded structs. The map-value path had no such fallback.

The fix

Rather than teach the parser one more shape, stop treating an unparsable body as fatal. That fixes this class of bug for every mod rather than this one struct.

The skip machinery already existed — all 44 scan.parse_object call sites do else { continue }. Only two return Err checks in build_all_json made it fatal. Failed objects are now collected by instanceName, carried on MapIndex (so the set survives the CBOR handoff to the lean worker), and reported on the payload as unreadableObjects: { count, samples }. The load status line says so; type paths and byte offsets go to the console.

Nothing is lost visually. collect_buildings is generic and header-only — it keeps any type path containing /Buildable/ or /Build_ and draws from position/rotation/instanceName, never parsing the body. All 159 balancers still render, under "Unknown", with correct labels and positions. Only their body-derived contents are missing.

Edits stay strictly checked. MapIndex::reject_new_parse_failures refuses an edit that makes a previously-parsable object stop parsing — a mis-computed splice damaging its neighbours still trips it. It is deliberately scoped to objects that existed before the edit: copy/paste splices bytes under a fresh instanceName, so duplicating an unreadable modded buildable legitimately produces a new unreadable object, and an earlier version of this check blocked that working operation.

Clearer edit errors. Editor ops parse their target eagerly, so editing an unreadable object reported the raw String decode failure at offset 24369724. That was unreachable before (the save did not load at all). All 14 op call sites reach an object through apply::fetch, so one wrap there now names the object and explains it can be viewed but not edited.

Verification

  • The reported save loads; all 159 balancers render; status reads "106 objects could not be read (usually modded); shown on the map without contents".
  • Four vanilla saves produce byte-identical payloads and indexes against main (up to 379 MB) — no regression, and no unreadableObjects key on a clean save.
  • Full suite green (78 tests), including a new tests/unparsable_objects.rs: a clobbered body must not fail the build, must be reported, must still render as a building, and the edit gate must fire on exactly the newly-broken pre-existing objects.

Not included

The opaque-map fallback that would let these buildables show their real contents. They render and can be selected, but their inventories are missing and they cannot be edited. Worth a follow-up if modded map-value structs turn up more often.

🤖 Generated with Claude Code

valentinps and others added 2 commits September 13, 2026 19:19
A modded save serialises property shapes this parser has never seen. One
such object sank the whole save: the payload/index build latched the first
re-parse error and returned Err, so 106 Modular Load Balancer buildables
out of 37,958 objects produced "Failed to load save: object re-parse failed
during index build: ... String decode failure".

The mod's mIndexMapping is a Map<ObjectProperty, StructProperty> whose
LBBalancerIndexing value is a native struct written as 12 raw bytes, not a
tagged property list. StructProperty and ArrayProperty already fall back to
opaque bytes for unknown modded structs; the map-value path had no such
fallback and parsed those bytes as a property name/type pair.

Rather than teach the parser one more shape, stop treating an unparsable
body as fatal. Every collector already skips a None object, and buildings
render from the header (typePath/position/rotation), so all 159 balancers
still draw -- only their body-derived contents are missing. The set of
skipped objects is reported on the payload as `unreadableObjects` and noted
in the load status line.

Edits stay strictly checked: the failure set rides on MapIndex (so it
survives the CBOR handoff to the lean worker) and finish_edit refuses an
edit that makes a previously-parsable object stop parsing -- that is this
editor corrupting the save, which the rebuild still exists to catch.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two follow-ups from testing the skip on the reporter's save.

The edit gate now lives on MapIndex and only rejects objects that EXISTED
and parsed before the edit. Copy/paste splices an object's bytes under a
fresh instanceName, so duplicating an unreadable modded buildable produces
a new unreadable object; the earlier name-set diff read that as corruption
and blocked a working operation. A mis-computed splice damaging a
neighbour -- the case the check exists for -- still trips it.

Editor ops parse their target eagerly, so editing an unreadable object now
reports "String decode failure at offset 24369724 of length 3670015". That
was unreachable before (the save did not load at all) and says nothing
about what the user did. All fourteen op call sites reach an object through
apply::fetch, so one wrap there names the object and explains it can be
viewed but not edited.

Adds tests/unparsable_objects.rs: a clobbered body must not fail the build,
must be reported on the payload, must still render as a building, and the
edit gate must fire on exactly the newly-broken pre-existing objects.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@valentinps
valentinps merged commit c58d592 into main Sep 13, 2026
2 checks passed
@valentinps
valentinps deleted the fix/tolerate-unparsable-objects branch September 13, 2026 18:08
This was referenced Sep 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

object re-parse failed during index build

1 participant