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

map/static/map/data.js

@@ -17,6 +17,28 @@
     loadStatus.textContent = text;
   }
 
+  // "Loaded: <session> (<date>)", plus a note when the parser had to skip
+  // objects. Skipping is normal on modded saves -- the parser doesn't know
+  // every mod's property layout -- and buildables still draw, from their
+  // headers, so the note explains a real but narrow gap (their contents and
+  // details) rather than announcing a failure.
+  function loadedStatus(payload) {
+    var text = "Loaded: " + payload.sessionName + " (" + payload.saveDatetime + ")";
+    var skipped = payload.unreadableObjects;
+    if (!skipped || !skipped.count) {
+      return text;
+    }
+    text += " — " + skipped.count + " object" + (skipped.count === 1 ? "" : "s") +
+      " could not be read (usually modded); shown on the map without contents";
+    // The type paths and byte offsets are the useful half of a bug report,
+    // but far too much for the status line -- console only.
+    if (skipped.samples && skipped.samples.length) {
+      console.warn("Skipped " + skipped.count + " unreadable object(s):");
+      skipped.samples.forEach(function(sample) { console.warn("  " + sample); });
+    }
+    return text;
+  }
+
   function showProgress(phase, percent) {
     progressBar.style.display = "block";
     progressFill.style.width = Math.max(0, Math.min(100, percent)) + "%";
@@ -275,7 +297,7 @@
           restorePinnedSelection(pinnedSelection);
         }
         Analytics.saveLoaded("file", file.size, performance.now() - startedAt);
-        setStatus("Loaded: " + payload.sessionName + " (" + payload.saveDatetime + ")");
+        setStatus(loadedStatus(payload));
       })
       .catch(function(error) {
         hideProgress();
@@ -324,7 +346,7 @@
         if (pinnedSelection) {
           restorePinnedSelection(pinnedSelection);
         }
-        setStatus("Loaded: " + payload.sessionName + " (" + payload.saveDatetime + ")");
+        setStatus(loadedStatus(payload));
       })
       .catch(function(error) {
         hideProgress();
@@ -418,7 +440,7 @@
           restorePinnedSelection(pinnedSelection);
         }
         Analytics.saveLoaded("url", bytes, performance.now() - startedAt);
-        setStatus("Loaded: " + payload.sessionName + " (" + payload.saveDatetime + ")");
+        setStatus(loadedStatus(payload));
       })
       .catch(function(error) {
         hideProgress();

rust_parser/core/src/editor/apply.rs

@@ -421,8 +421,25 @@ pub(crate) fn actor_slots_of_types(store: &SaveStore, candidates: &[&str]) -> Ve
 
 /// One object, re-parsed on demand from its span (identical to the eagerly
 /// parsed model; StrRefs point into the same `store.data`).
+///
+/// A body this parser cannot read is no longer fatal at load -- the map
+/// skips such an object and still draws it from its header -- so edits are
+/// where the user now meets one, and the raw "String decode failure at
+/// offset N" says nothing about what they did or what to do. Every editor
+/// op reaches an object through here, so one wrap covers all of them.
 pub(crate) fn fetch(store: &SaveStore, li: usize, oi: usize) -> PResult<Object> {
-    store.parse_object_at(li, oi)
+    store.parse_object_at(li, oi).map_err(|e| {
+        let name = match &store.levels[li].headers[oi] {
+            Header::Actor(a) => a.instance_name,
+            Header::Component(c) => c.instance_name,
+        };
+        perr!(
+            "cannot edit {}: this object is stored in a format this app cannot read \
+             (usually a mod's own data), so it can be viewed on the map but not edited [{}]",
+            String::from_utf8_lossy(name.bytes(&store.data)),
+            e.msg,
+        )
+    })
 }
 
 // ---------------------------------------------------------------------------

rust_parser/core/src/mapdata/index.rs

@@ -12,7 +12,7 @@ use super::geometry::{project_xy, world_z_to_meters};
 use super::jsonval::jnum;
 use super::names::readable_label;
 use super::props;
-use super::scan::{SaveScan, Slot};
+use super::scan::{ParseFailures, SaveScan, Slot};
 use crate::extract::find_prop;
 use crate::store::*;
 use indexmap::IndexMap;
@@ -120,9 +120,53 @@ pub struct MapIndex {
     pub item_location_index: IndexMap<Vec<u8>, Vec<(Vec<u8>, i64)>>,
     pub dimensional_depot_by_item: IndexMap<String, i64>,
     pub static_item_locations: IndexMap<String, Vec<StaticItemLocation>>,
+    /// Objects the build could not re-parse and therefore skipped (modded
+    /// property shapes, overwhelmingly). Carried on the index so it survives
+    /// the CBOR handoff to the lean worker, which never runs a build of its
+    /// own yet still has to diff this set when an edit rebuilds (see
+    /// SaveSession::finish_edit). `default` keeps older blobs loadable.
+    #[serde(default)]
+    pub parse_failures: ParseFailures,
 }
 
 impl MapIndex {
+    /// The edit-path gate, `self` being the index rebuilt after an edit.
+    ///
+    /// A build tolerates objects it cannot re-parse (modded property shapes
+    /// this parser has never seen), but an object that EXISTED and parsed
+    /// before an edit and stops parsing after it is this editor corrupting
+    /// the save -- a mis-computed splice damaging its neighbours looks
+    /// exactly like that -- so the rebuild stays that check and the edit is
+    /// refused.
+    ///
+    /// Deliberately scoped to objects present before the edit. Copy/paste
+    /// splices an object's bytes under a fresh instanceName, so pasting an
+    /// unreadable modded buildable legitimately produces a new unreadable
+    /// object; treating that as corruption would block a working operation.
+    /// Newly created objects are the editor's own output and are covered by
+    /// the editor round-trip tests instead.
+    ///
+    /// `before` is None for a session with nothing to diff against (one
+    /// recovering from a failed edit), which must not read as "all new".
+    pub fn reject_new_parse_failures(&self, before: Option<&MapIndex>) -> Result<(), String> {
+        let Some(before) = before else { return Ok(()) };
+        let broken: Vec<&[u8]> = self
+            .parse_failures
+            .newly_failing(&before.parse_failures)
+            .into_iter()
+            .filter(|name| before.by_instance_name.contains_key(*name))
+            .collect();
+        if broken.is_empty() {
+            return Ok(());
+        }
+        Err(format!(
+            "edit aborted: {} object(s) that parsed before this edit no longer do \
+             (e.g. {}) -- the save was left unchanged",
+            broken.len(),
+            String::from_utf8_lossy(broken[0]),
+        ))
+    }
+
     /// scan.headersByInstanceName.get(name) -- the Header, or None.
     pub fn header_by_name<'a>(&self, store: &'a SaveStore, name: &[u8]) -> Option<&'a Header> {
         self.by_instance_name.get(name).map(|&(li, oi)| &store.levels[li].headers[oi])
@@ -417,6 +461,10 @@ impl MapIndex {
             item_location_index,
             dimensional_depot_by_item,
             static_item_locations,
+            // Last field for a reason: every collector above has run, so this
+            // snapshot covers the whole build (the payload build shares the
+            // scan and runs first -- see build_all_json).
+            parse_failures: scan.parse_failures(),
         }
     }
 }

rust_parser/core/src/mapdata/mod.rs

@@ -305,12 +305,6 @@ fn build_payload_json_with_scan(
             cb(done, BUILD_STEP_COUNT);
         }
     }
-    // On-demand re-parses can't fail on bytes that already parsed, but once
-    // the pipeline is lean this build validates edited bodies -- fail loud
-    // rather than emit a payload with a silently-skipped object.
-    if let Some(e) = scan.parse_error() {
-        return Err(format!("object re-parse failed during payload build: {e}"));
-    }
     out.push(b'}');
     Ok(out)
 }
@@ -334,10 +328,41 @@ pub fn build_all_json(
         collectors::STEP_ORDER.to_vec(),
         Some(&mut tick),
     )?;
+    // Objects whose bodies would not re-parse are skipped, not fatal: a
+    // modded save serializes property shapes this parser has never seen, and
+    // one such object must not sink a 40k-object save. Every collector
+    // already skips a None object, and buildings render off the header
+    // (position/rotation/typePath), so a skipped buildable still appears on
+    // the map -- only its body-derived detail is missing. The set rides out
+    // on the index; the edit paths diff it to keep catching real corruption.
     let map_index = index::MapIndex::build_with_scan(&scan);
-    if let Some(e) = scan.parse_error() {
-        return Err(format!("object re-parse failed during index build: {e}"));
-    }
+    // The payload is serialized before the index is built, but objects the
+    // INDEX build could not read belong in the same report -- so splice the
+    // key in here, where both halves are in hand, rather than teach every
+    // collector about it.
+    let mut payload = payload;
+    append_unreadable_report(&mut payload, &map_index.parse_failures);
     tick(BUILD_STEP_COUNT, BUILD_STEP_COUNT);
     Ok((payload, map_index))
 }
+
+/// Add `"unreadableObjects": {count, samples}` to a finished payload object,
+/// so the frontend can tell the user what the map is missing. Absent
+/// entirely on a save that parsed cleanly -- which is nearly all of them.
+fn append_unreadable_report(payload: &mut Vec<u8>, failures: &scan::ParseFailures) {
+    if failures.is_empty() {
+        return;
+    }
+    debug_assert_eq!(payload.last(), Some(&b'}'));
+    payload.pop();
+    if payload.last() != Some(&b'{') {
+        payload.push(b',');
+    }
+    let report = serde_json::json!({
+        "count": failures.len(),
+        "samples": failures.samples,
+    });
+    payload.extend_from_slice(b"\"unreadableObjects\":");
+    payload.extend_from_slice(report.to_string().as_bytes());
+    payload.push(b'}');
+}

rust_parser/core/src/mapdata/scan.rs

@@ -6,7 +6,7 @@
 use super::consts::{GAME_STATE_TYPE_PATH_SUBSTRING, LIGHTWEIGHT_BUILDABLE_SUBSYSTEM_TYPE_PATH};
 use crate::extract::InstanceSlots;
 use crate::store::*;
-use indexmap::IndexMap;
+use indexmap::{IndexMap, IndexSet};
 use std::cell::{OnceCell, RefCell};
 use std::collections::HashMap;
 
@@ -44,11 +44,49 @@ pub struct SaveScan<'a> {
     hard_drives: OnceCell<serde_json::Value>,
     depot_contents: OnceCell<serde_json::Value>,
     catalog_drops: OnceCell<Vec<(&'static str, i64, [f64; 3], &'static str)>>,
-    /// First on-demand re-parse failure seen during a build. Re-parsing bytes
-    /// that already parsed cannot fail, but once the pipeline is lean the
-    /// build IS the deep validation of an edited body -- so build_all_json
-    /// turns a latched error into Err instead of emitting a wrong payload.
-    parse_error: RefCell<Option<String>>,
+    /// Objects whose body failed to re-parse during a build. Modded saves
+    /// serialize property shapes this parser does not know, so a failure here
+    /// is normal content rather than corruption: the collectors already skip
+    /// a None object, and the build reports the set instead of failing. The
+    /// edit paths turn it back into a hard error for objects that parsed
+    /// before an edit and stopped after it (see finish_edit) -- that IS
+    /// corruption, and the build is still its deep validation.
+    parse_failures: RefCell<ParseFailures>,
+}
+
+/// Objects a build could not re-parse, identified by instanceName so the set
+/// survives an edit that shifts slot indices.
+#[derive(Default, Clone, serde::Serialize, serde::Deserialize)]
+pub struct ParseFailures {
+    /// instanceName of each unparsable object, deduplicated, in first-seen
+    /// order (one object is re-parsed by several collectors).
+    pub names: IndexSet<Vec<u8>>,
+    /// The first few failures verbatim, for the user-facing report. Capped:
+    /// a save where everything fails must not build a 600k-string list.
+    pub samples: Vec<String>,
+}
+
+/// How many failure messages `ParseFailures::samples` keeps.
+const MAX_FAILURE_SAMPLES: usize = 5;
+
+impl ParseFailures {
+    pub fn is_empty(&self) -> bool {
+        self.names.is_empty()
+    }
+
+    pub fn len(&self) -> usize {
+        self.names.len()
+    }
+
+    /// Failures present in `self` but not in `baseline` -- an edit's new
+    /// breakage, as opposed to mod content that never parsed to begin with.
+    pub fn newly_failing<'s>(&'s self, baseline: &ParseFailures) -> Vec<&'s [u8]> {
+        self.names
+            .iter()
+            .filter(|n| !baseline.names.contains(*n))
+            .map(|n| n.as_slice())
+            .collect()
+    }
 }
 
 impl<'a> SaveScan<'a> {
@@ -86,7 +124,7 @@ impl<'a> SaveScan<'a> {
             hard_drives: OnceCell::new(),
             depot_contents: OnceCell::new(),
             catalog_drops: OnceCell::new(),
-            parse_error: RefCell::new(None),
+            parse_failures: RefCell::new(ParseFailures::default()),
         }
     }
 
@@ -144,22 +182,42 @@ impl<'a> SaveScan<'a> {
     }
 
     /// Re-parse the object at `slot` from its byte span (owned; identical to
-    /// the eager parse). Returns None on failure, latching the first error so
-    /// the build can fail cleanly. Replaces the old `object()` borrow into the
-    /// resident model, so the builder never needs `parsed_objects()`.
+    /// the eager parse). Returns None on failure, recording the object so the
+    /// build can report what it skipped. Replaces the old `object()` borrow
+    /// into the resident model, so the builder never needs `parsed_objects()`.
     pub fn parse_object(&self, slot: Slot) -> Option<Object> {
         match self.store.parse_object_at(slot.0, slot.1) {
             Ok(object) => Some(object),
             Err(e) => {
-                let mut latched = self.parse_error.borrow_mut();
-                if latched.is_none() {
-                    *latched = Some(format!("object at {slot:?}: {e}"));
-                }
+                self.record_parse_failure(slot, &e.msg);
                 None
             }
         }
     }
 
+    /// Identity is the instanceName, not the slot: an edit that splices bytes
+    /// renumbers slots, and the edit paths diff these sets across that.
+    fn record_parse_failure(&self, slot: Slot, msg: &str) {
+        let name = match self.header(slot) {
+            Header::Actor(a) => a.instance_name,
+            Header::Component(c) => c.instance_name,
+        };
+        let mut failures = self.parse_failures.borrow_mut();
+        if !failures.names.insert(name.bytes(self.data()).to_vec()) {
+            return; // already recorded -- several collectors parse one object
+        }
+        if failures.samples.len() < MAX_FAILURE_SAMPLES {
+            let type_path = match self.header(slot) {
+                Header::Actor(a) => a.type_path,
+                Header::Component(c) => c.class_name,
+            };
+            failures.samples.push(format!(
+                "{} at {slot:?}: {msg}",
+                String::from_utf8_lossy(type_path.bytes(self.data()))
+            ));
+        }
+    }
+
     /// scan.objectsByInstanceName.get(name), re-parsed on demand.
     pub fn parse_object_by_name(&self, name: &[u8]) -> Option<Object> {
         let slot = *self.by_instance_name.get(name)?;
@@ -181,9 +239,9 @@ impl<'a> SaveScan<'a> {
             .as_ref()
     }
 
-    /// The first on-demand re-parse error latched during the build, if any.
-    pub fn parse_error(&self) -> Option<String> {
-        self.parse_error.borrow().clone()
+    /// Objects this build could not re-parse (empty on a clean save).
+    pub fn parse_failures(&self) -> ParseFailures {
+        self.parse_failures.borrow().clone()
     }
 
     pub fn header_by_name(&self, name: &[u8]) -> Option<&'a Header> {