diff --git a/map/static/map/data.js b/map/static/map/data.js index 11d5cbb..0f0d780 100644 --- a/map/static/map/data.js +++ b/map/static/map/data.js @@ -17,6 +17,28 @@ loadStatus.textContent = text; } + // "Loaded: ()", 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(); diff --git a/rust_parser/core/src/editor/apply.rs b/rust_parser/core/src/editor/apply.rs index acf2857..0a7d963 100644 --- a/rust_parser/core/src/editor/apply.rs +++ b/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 { - 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, + ) + }) } // --------------------------------------------------------------------------- diff --git a/rust_parser/core/src/mapdata/index.rs b/rust_parser/core/src/mapdata/index.rs index 3bc4c92..a35dd47 100644 --- a/rust_parser/core/src/mapdata/index.rs +++ b/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<(Vec, i64)>>, pub dimensional_depot_by_item: IndexMap, pub static_item_locations: IndexMap>, + /// 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(), } } } diff --git a/rust_parser/core/src/mapdata/mod.rs b/rust_parser/core/src/mapdata/mod.rs index f2bb275..7fc603c 100644 --- a/rust_parser/core/src/mapdata/mod.rs +++ b/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, 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'}'); +} diff --git a/rust_parser/core/src/mapdata/scan.rs b/rust_parser/core/src/mapdata/scan.rs index 920014e..b7896e8 100644 --- a/rust_parser/core/src/mapdata/scan.rs +++ 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, depot_contents: OnceCell, catalog_drops: OnceCell>, - /// 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>, + /// 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, +} + +/// 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>, + /// 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, +} + +/// 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 { 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 { 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 { - 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> { diff --git a/rust_parser/core/tests/unparsable_objects.rs b/rust_parser/core/tests/unparsable_objects.rs new file mode 100644 index 0000000..98ed7c6 --- /dev/null +++ b/rust_parser/core/tests/unparsable_objects.rs @@ -0,0 +1,158 @@ +//! A body this parser cannot read must not sink the whole save. +//! +//! Modded saves serialise property shapes sav_core has never seen (the case +//! that prompted this: Modular Load Balancers stores a native 12-byte struct +//! as a MapProperty value, where the parser expects a tagged property list). +//! The build skips such an object and reports it instead of failing, and the +//! object still draws on the map because buildings render off the header. + +use sav_core::level::parse_full_save_lean; +use sav_core::mapdata; +use sav_core::mapdata::index::MapIndex; +use sav_core::mapdata::scan::ParseFailures; +use sav_core::object::ClassTables; +use sav_core::store::{Header, SaveStore}; +use std::path::PathBuf; + +fn load_lean(name: &str) -> SaveStore { + let path = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("../../map/uploads").join(name); + let bytes = std::fs::read(path).expect("test save present"); + parse_full_save_lean(&bytes, &ClassTables::embedded(), None).unwrap() +} + +const SAVE: &str = "All_080726-163150.sav"; + +/// Find a buildable whose body parses today, and return its slot, type path +/// and instance name. Buildable so the "still renders" half is testable. +fn a_parsable_buildable(store: &SaveStore) -> ((usize, usize), String, String) { + for (li, level) in store.levels.iter().enumerate() { + for (oi, header) in level.headers.iter().enumerate() { + let Header::Actor(actor) = header else { continue }; + let type_path = actor.type_path.to_string(&store.data); + if !type_path.contains("/Build_") || store.parse_object_at(li, oi).is_err() { + continue; + } + return ((li, oi), type_path, actor.instance_name.to_string(&store.data)); + } + } + panic!("no parsable buildable in {SAVE}"); +} + +/// 0xFF over the whole body span: every length prefix in it becomes absurd, +/// so the property walk fails wherever it first looks. The headers (and the +/// recorded spans) are untouched -- exactly the shape of the real bug, where +/// the header is fine and only the body is unreadable. +fn clobber_body(store: &mut SaveStore, (li, oi): (usize, usize)) { + let (off, len) = store.levels[li].object_spans[oi]; + store.data[off..off + len as usize].fill(0xFF); + assert!( + store.parse_object_at(li, oi).is_err(), + "clobbered body still parsed -- the test corrupts nothing", + ); +} + +#[test] +fn clean_save_reports_no_unreadable_objects() { + let store = load_lean(SAVE); + let (payload, index) = mapdata::build_all_json(&store, None).expect("build"); + assert!(index.parse_failures.is_empty()); + // Absent entirely rather than a zero count: nearly every save is clean, + // and the frontend keys the notice off the key's presence. + let payload: serde_json::Value = serde_json::from_slice(&payload).unwrap(); + assert!(payload.get("unreadableObjects").is_none()); +} + +#[test] +fn unreadable_body_is_skipped_and_reported_not_fatal() { + let mut store = load_lean(SAVE); + let (slot, type_path, instance_name) = a_parsable_buildable(&store); + clobber_body(&mut store, slot); + + // The whole point: this used to be Err. + let (payload, index) = mapdata::build_all_json(&store, None).expect("build must not fail"); + + assert_eq!(index.parse_failures.len(), 1); + assert!(index.parse_failures.names.contains(instance_name.as_bytes())); + + let payload: serde_json::Value = serde_json::from_slice(&payload).unwrap(); + let report = &payload["unreadableObjects"]; + assert_eq!(report["count"], 1); + assert!( + report["samples"][0].as_str().unwrap().contains(&type_path), + "sample should name the class: {report}", + ); +} + +/// The map keeps the building: position/rotation/typePath live in the header, +/// which parses fine -- only the body-derived detail is lost. +#[test] +fn unreadable_buildable_still_renders() { + let store = load_lean(SAVE); + let (slot, type_path, instance_name) = a_parsable_buildable(&store); + let short_name = instance_name.rsplit('.').next().unwrap().to_string(); + + let mut broken = load_lean(SAVE); + clobber_body(&mut broken, slot); + let (payload, _) = mapdata::build_all_json(&broken, None).expect("build"); + let payload: serde_json::Value = serde_json::from_slice(&payload).unwrap(); + + let rendered = payload["buildingCategories"] + .as_array() + .unwrap() + .iter() + .flat_map(|category| category["types"].as_array().unwrap()) + .filter(|bucket| bucket["typePath"] == type_path.as_str()) + .any(|bucket| { + bucket["ids"] + .as_array() + .unwrap() + .iter() + .any(|id| id.as_str() == Some(short_name.as_str())) + }); + assert!(rendered, "{instance_name} vanished from the map after its body failed to parse"); +} + +/// Edits stay strictly checked: an object that existed and parsed before an +/// edit must still parse after it. Pre-existing failures and objects the +/// edit itself created are not that. +#[test] +fn edit_gate_rejects_only_newly_broken_existing_objects() { + // A save holding two objects, one of them unreadable modded content. + let mut before = MapIndex::build(&load_lean(SAVE)); + before.by_instance_name.clear(); + for name in [b"modded_thing".as_slice(), b"was_fine".as_slice()] { + before.by_instance_name.insert(name.to_vec(), (0, 0)); + } + before.parse_failures = ParseFailures::default(); + before.parse_failures.names.insert(b"modded_thing".to_vec()); + + let after = |failures: &[&[u8]]| { + let mut index = MapIndex::build(&load_lean(SAVE)); + index.parse_failures = ParseFailures::default(); + for name in failures { + index.parse_failures.names.insert(name.to_vec()); + } + index + }; + + // Unchanged, and "the edit removed the broken object", are both fine. + assert!(after(&[b"modded_thing"]).reject_new_parse_failures(Some(&before)).is_ok()); + assert!(after(&[]).reject_new_parse_failures(Some(&before)).is_ok()); + + // An object that parsed before this edit and no longer does: corruption. + let err = after(&[b"modded_thing", b"was_fine"]) + .reject_new_parse_failures(Some(&before)) + .unwrap_err(); + assert!(err.contains("was_fine"), "{err}"); + assert!(!err.contains("modded_thing"), "pre-existing failure reported as new: {err}"); + + // Pasting an unreadable buildable splices its bytes under a fresh name -- + // a new unreadable object, not damage to anything that existed. + assert!(after(&[b"modded_thing", b"pasted_copy"]) + .reject_new_parse_failures(Some(&before)) + .is_ok()); + + // No baseline (a session recovering from a failed edit) must not read as + // "everything is new". + assert!(after(&[b"was_fine"]).reject_new_parse_failures(None).is_ok()); +} diff --git a/rust_parser/tauri/src/session.rs b/rust_parser/tauri/src/session.rs index 90af65f..455e7ac 100644 --- a/rust_parser/tauri/src/session.rs +++ b/rust_parser/tauri/src/session.rs @@ -96,6 +96,7 @@ impl AppSession { let mut build_progress = |current: u64, total: u64| progress(2, current, total); let (payload_json, index) = mapdata::build_all_json(&new_store, Some(&mut build_progress))?; + index.reject_new_parse_failures(self.index.as_ref())?; self.store = Some(new_store); self.index = Some(index); Ok(payload_json) diff --git a/rust_parser/wasm/src/lib.rs b/rust_parser/wasm/src/lib.rs index 0257a53..d3116e1 100644 --- a/rust_parser/wasm/src/lib.rs +++ b/rust_parser/wasm/src/lib.rs @@ -166,6 +166,7 @@ impl SaveSession { let (payload_json, index) = mapdata::build_all_json(&new_store, Some(&mut build_progress)) .map_err(|e| JsError::new(&e))?; + index.reject_new_parse_failures(self.index.as_ref()).map_err(|e| JsError::new(&e))?; self.store = Some(Arc::new(new_store)); self.index = Some(index); Ok(payload_json)