Files
Sylpheed/crates/sylpheed-export/src/check.rs
Fabian Hamm ed54f95d54 style: rustfmt sweep -- 774 hunks across 154 files -> 0
`cargo fmt --all -- --check` has failed on every run in this repository's
history, identically on `main` and on every branch. This is #12.

Mechanical: `cargo fmt --all`, nothing else. 154 files, all `.rs`, no other
extension touched. `cargo check --workspace` exits 0 afterwards, so nothing
changed semantically.

ON THE ORDERING, WHICH WAS THE REAL QUESTION.

HANDOFF-2026-09-06 section 7 warns this is the expensive fix: a whole-tree
reformat before #7 and #8 return "would put a conflict in every file of 861
commits and make the reviews those items exist to enable unreadable".

That is measurably too pessimistic, and it had been reasoned rather than
tested. Measured here by three-way merging a rustfmt'd `main` against both
unmerged branches, file by file:

  file/branch pairs tested   32
  merges CLEAN               28
  merges CONFLICTING          4   (8 conflict hunks total)

    sylpheed-cli/src/main.rs      1 hunk
    sylpheed-export/src/check.rs  1
    sylpheed-export/src/screen.rs 4
    sylpheed-export/src/video.rs  2

All four are against `auto/frame-blend-draw-path` only;
`auto/port-p6-audio` does not conflict anywhere. The earlier framing --
154 dirty files, 133 that cannot collide, 21 that can, the collision set
carrying 147 of 774 hunks (19%) -- reproduces exactly. What it did not say
is that most of the 21 still merge cleanly, because rustfmt's edits and the
branches' edits rarely land on the same lines.

So the cost of sweeping now is 4 files and 8 hunks for one branch, against
a check that is otherwise red forever. Deliberately NOT folded into the
WASM PR: 154 reformatted files would make that one unreviewable.

Closes #12
2026-09-08 20:07:01 +02:00

426 lines
17 KiB
Rust

//! `sylpheed-export check` — validate an export tree against `docs/FORMAT.md`.
//!
//! This is the executable form of FORMAT.md, and the reason it exists is that
//! "the export is correct" is otherwise an assertion. It reads `export/` the way
//! the Godot project will — as a stranger, with no access to the disc, the
//! decoders or this exporter's internals — and fails on anything a consumer
//! could not act on:
//!
//! * a document whose `format` is not the version this build writes;
//! * a required field missing, or a colour that is not `0x` + 8 hex digits;
//! * a `paint_order` that is not a permutation of the element indices;
//! * a `buttons` entry naming an element that is not a button, or out of
//! resting-Y order;
//! * a sprite path that does not exist, or a PNG that does not decode;
//! * a name presented as recovered when it was authored;
//! * an audio file that is silent or clips -- the two audio failures that pass
//! every check that is not looking for them.
//!
//! It deliberately does **not** check that the export matches the disc. That is
//! what `sylpheed-cli screen render` is for.
use anyhow::{bail, Result};
use serde_json::Value;
use std::path::Path;
const SCREEN_FORMAT: &str = "sylpheed.screen/3";
const MANIFEST_FORMAT: &str = "sylpheed.manifest/1";
struct Ctx {
file: String,
errors: Vec<String>,
}
impl Ctx {
fn err(&mut self, msg: impl Into<String>) {
self.errors.push(format!("{}: {}", self.file, msg.into()));
}
fn require<'a>(&mut self, v: &'a Value, key: &str) -> Option<&'a Value> {
match v.get(key) {
Some(Value::Null) | None => {
self.err(format!("missing required field `{key}`"));
None
}
Some(x) => Some(x),
}
}
}
/// A colour is exported as `0x` + 8 hex digits, with its byte order in the key
/// name. Anything else means a consumer has to guess, which is the whole thing
/// the format exists to prevent.
fn is_hex32(v: Option<&Value>) -> bool {
v.and_then(Value::as_str).is_some_and(|s| {
s.len() == 10 && s.starts_with("0x") && s[2..].chars().all(|c| c.is_ascii_hexdigit())
})
}
fn check_pose(c: &mut Ctx, where_: &str, p: &Value) {
for (key, want_len) in [("pos", 2usize), ("scale", 2)] {
match p.get(key).and_then(Value::as_array) {
Some(a) if a.len() == want_len && a.iter().all(Value::is_i64) => {}
_ => c.err(format!("{where_}: `{key}` must be {want_len} integers")),
}
}
for key in ["tint_rgba", "fade_argb"] {
if !is_hex32(p.get(key)) {
c.err(format!("{where_}: `{key}` must be 0x + 8 hex digits"));
}
}
}
fn check_screen(root: &Path, rel: &str, errors: &mut Vec<String>) -> Result<()> {
let raw = std::fs::read_to_string(root.join(rel))?;
let v: Value = serde_json::from_str(&raw)?;
let mut c = Ctx {
file: rel.to_string(),
errors: Vec::new(),
};
if v.get("format").and_then(Value::as_str) != Some(SCREEN_FORMAT) {
c.err(format!(
"format is {:?}, expected {SCREEN_FORMAT:?}",
v.get("format")
));
}
for key in ["exporter", "formats_rev", "name", "name_source"] {
c.require(&v, key);
}
// Rule 2 of the format: a modder must be able to tell a recovered name from
// an invented one, so the provenance is mandatory and closed.
match v.get("name_source").and_then(Value::as_str) {
Some("authored") => {
if v.get("name_why")
.and_then(Value::as_str)
.is_none_or(str::is_empty)
{
c.err("name_source is `authored` but there is no `name_why`");
}
}
Some("index") => {}
other => c.err(format!(
"name_source must be `authored` or `index`, got {other:?}"
)),
}
if let Some(s) = v.get("source") {
for key in ["archive", "entry", "build"] {
if s.get(key).is_none() {
c.err(format!("source is missing `{key}`"));
}
}
} else {
c.err("missing required field `source`");
}
match v.get("design").and_then(Value::as_array) {
Some(d) if d.len() == 2 && d.iter().all(Value::is_u64) => {}
_ => c.err("`design` must be two positive integers"),
}
let Some(elements) = v.get("elements").and_then(Value::as_array) else {
c.err("missing required field `elements`");
errors.append(&mut c.errors);
return Ok(());
};
let mut indices = Vec::new();
let mut buttons_by_y: Vec<(i64, String)> = Vec::new();
for (i, el) in elements.iter().enumerate() {
let id = el
.get("id")
.and_then(Value::as_str)
.unwrap_or("<no id>")
.to_string();
let at = format!("element {i} ({id})");
for key in [
"index",
"id",
"declared",
"role",
"kind_raw",
"pivot",
"layer_source",
"keyframes",
] {
if el.get(key).is_none() {
c.err(format!("{at}: missing `{key}`"));
}
}
let Some(idx) = el.get("index").and_then(Value::as_u64) else {
c.err(format!("{at}: `index` is not an integer"));
continue;
};
if idx as usize != i {
c.err(format!(
"{at}: `index` {idx} does not match its position {i}"
));
}
indices.push(idx as usize);
let role = el.get("role").and_then(Value::as_str).unwrap_or("");
if !matches!(role, "button" | "decoration" | "primitive" | "unknown") {
c.err(format!("{at}: role {role:?} is not one FORMAT.md defines"));
}
// A role of `unknown` must still carry the raw kind, or the information
// is simply lost.
if role == "unknown" && el.get("kind_raw").is_none() {
c.err(format!("{at}: role `unknown` without `kind_raw`"));
}
// A primitive has no texture, so its quad size has to come from the file.
if role == "primitive" && el.get("size").is_none() {
c.err(format!("{at}: primitive without a `size`"));
}
match el.get("layer_source").and_then(Value::as_str) {
Some("sprite") | Some("implied") => {
if !is_hex32(el.get("layer")) {
c.err(format!(
"{at}: layer_source claims a key but `layer` is not one"
));
}
}
Some("none") => {
if el.get("layer").is_some() {
c.err(format!(
"{at}: layer_source `none` but a `layer` is present"
));
}
}
other => c.err(format!(
"{at}: layer_source must be sprite/implied/none, got {other:?}"
)),
}
for key in ["sprite", "focus_sprite"] {
if let Some(p) = el.get(key).and_then(Value::as_str) {
let path = root.join(p);
if !path.exists() {
c.err(format!("{at}: `{key}` points at {p}, which does not exist"));
} else if let Err(e) = image::open(&path) {
c.err(format!(
"{at}: `{key}` {p} does not decode as an image: {e}"
));
}
}
}
if let Some(r) = el.get("rest") {
check_pose(&mut c, &at, r);
if role == "button" {
if let Some(y) = r
.get("pos")
.and_then(Value::as_array)
.and_then(|a| a[1].as_i64())
{
buttons_by_y.push((y, id.clone()));
}
}
}
if let Some(kfs) = el.get("keyframes").and_then(Value::as_array) {
for (k, kf) in kfs.iter().enumerate() {
check_pose(&mut c, &format!("{at} keyframe {k}"), kf);
}
// 🔴 INVERTED 2026-08-29, and the old rule is the more interesting
// half. It read: "the last keyframe of a group carries no time slot
// on the disc, and an invented one is exactly the kind of value this
// format refuses." That was true of the OLD keyframe association,
// where a group's data stopped four bytes short of its final block's
// time slot.
//
// Under the corrected layout (`formats-pin-2026-08-29c` onward) a
// group is an 8-byte header then `frames` x {u32 time; 36-byte
// pose}, so **pose 0's time is the group's lead-in word and EVERY
// POSE IS TIMED, including the last.** The rule now says the
// opposite, and an untimed keyframe is the thing to refuse.
//
// ⚠️ This fired 150 times on a re-export and I had not run `check`
// between pinning the tag and measuring against the oracle -- the
// pixel harness was green while the format validator was failing on
// every screen with a multi-keyframe group. A correctness harness
// does not replace a format one; they fail at different layers.
if kfs.len() > 1 && kfs.iter().any(|k| k.get("t").is_none()) {
c.err(format!("{at}: a keyframe has no `t`; every pose is timed under the corrected record layout"));
}
}
}
// Paint order must be a permutation of the element indices, or the runtime
// either drops an element or draws one twice.
match v.get("paint_order").and_then(Value::as_array) {
Some(po) => {
let mut got: Vec<usize> = po
.iter()
.filter_map(|x| x.as_u64().map(|v| v as usize))
.collect();
if got.len() != po.len() {
c.err("`paint_order` holds a non-integer");
}
let mut want = indices.clone();
got.sort_unstable();
want.sort_unstable();
if got != want {
c.err("`paint_order` is not a permutation of the element indices");
}
}
None => c.err("missing required field `paint_order`"),
}
// `buttons` is navigation order and is defined as resting Y, ascending. If
// it is not sorted, it is not the thing FORMAT.md says it is.
match v.get("buttons").and_then(Value::as_array) {
Some(b) => {
let listed: Vec<&str> = b.iter().filter_map(Value::as_str).collect();
buttons_by_y.sort_by(|a, b| a.0.cmp(&b.0).then_with(|| a.1.cmp(&b.1)));
let want: Vec<&str> = buttons_by_y.iter().map(|(_, n)| n.as_str()).collect();
if listed != want {
c.err(format!(
"`buttons` is {listed:?} but resting-Y order is {want:?}"
));
}
}
None => c.err("missing required field `buttons`"),
}
if v.get("unresolved").and_then(Value::as_array).is_none() {
c.err("missing required field `unresolved` (an empty list is a claim; absence is a gap)");
}
errors.append(&mut c.errors);
Ok(())
}
/// Validate a whole export tree. Returns the number of screens checked.
pub fn run(root: &Path) -> Result<usize> {
let manifest_path = root.join("manifest.json");
if !manifest_path.exists() {
bail!(
"{} has no manifest.json — is that an export tree?",
root.display()
);
}
let m: Value = serde_json::from_str(&std::fs::read_to_string(&manifest_path)?)?;
let mut errors = Vec::new();
if m.get("format").and_then(Value::as_str) != Some(MANIFEST_FORMAT) {
errors.push(format!("manifest.json: format is not {MANIFEST_FORMAT:?}"));
}
for key in ["exporter", "formats_rev", "screens", "warnings"] {
if m.get(key).is_none() {
errors.push(format!("manifest.json: missing `{key}`"));
}
}
let screens = m
.get("screens")
.and_then(Value::as_array)
.map(|s| s.to_vec())
.unwrap_or_default();
for s in &screens {
let Some(file) = s.get("file").and_then(Value::as_str) else {
errors.push("manifest.json: a screen entry has no `file`".into());
continue;
};
if !root.join(file).exists() {
errors.push(format!("manifest.json: lists {file}, which does not exist"));
continue;
}
check_screen(root, file, &mut errors)?;
}
check_audio(root, &m, &mut errors);
if !errors.is_empty() {
for e in &errors {
eprintln!("{e}");
}
bail!("{} problem(s) in {}", errors.len(), root.display());
}
Ok(screens.len())
}
/// The `audio` array, checked the way a consumer would have to.
///
/// Two of these are content checks rather than schema checks, and they are here
/// on purpose. `docs/port/AUDIO-VERIFICATION.md` names silence as "the failure
/// that looks like success": a file of exactly the right duration, the right
/// channel count and the right size, full of zeroes, because something opened
/// the wrong thing. Every structural check passes it. So does clipping, which
/// the BGM can produce because it is a **sum of two stems** at unity gain.
///
/// The exporter measures both at export time and writes them here; this refuses
/// the tree if what it wrote is a file nobody would want to play. Neither is a
/// judgement about whether the audio is the RIGHT audio — nothing in this
/// binary can know that, and `docs/port/BLOCKED.md` says which parts are still
/// authored guesses.
fn check_audio(root: &Path, m: &Value, errors: &mut Vec<String>) {
let Some(audio) = m.get("audio").and_then(Value::as_array) else {
// Absent is correct for every export taken before P6.
return;
};
for a in audio {
let name = a.get("name").and_then(Value::as_str).unwrap_or("?");
let kind = a.get("kind").and_then(Value::as_str).unwrap_or("");
if !matches!(kind, "se" | "bgm" | "voice") {
errors.push(format!(
"manifest.json: audio `{name}` has kind {kind:?}, which a consumer cannot dispatch on"
));
}
for key in ["file", "command", "why"] {
if a.get(key).and_then(Value::as_str).is_none_or(str::is_empty) {
errors.push(format!("manifest.json: audio `{name}` has no `{key}`"));
}
}
let Some(file) = a.get("file").and_then(Value::as_str) else {
continue;
};
if !root.join(file).exists() {
errors.push(format!(
"manifest.json: lists audio {file}, which does not exist"
));
continue;
}
match a.get("peak_dbfs").and_then(Value::as_f64) {
None => errors.push(format!(
"manifest.json: audio `{name}` carries no `peak_dbfs` -- it was not measured, \
and silence is the audio failure that passes every check that is not looking \
for it"
)),
Some(p) if p <= -90.0 => errors.push(format!(
"{file}: peak is {p:.1} dBFS -- this file is silent"
)),
// The bound differs by kind, and the difference is the point. A
// `bgm` is something WE combined -- a sum of stems -- so a peak at
// or above full scale is our arithmetic and is refused outright. An
// `se` is a single wave off the disc: it is mastered near full
// scale, and a lossy decode of a near-full-scale signal overshoots
// by a fraction of a dB (`confirm` lands at +0.18). Refusing that
// would be refusing the disc's own mastering, and "fixing" it would
// mean attenuating a game asset to make a number smaller.
//
// 🟡 +1.0 dB is a JUDGEMENT, not a measurement: a few tenths is
// reconstruction overshoot, a whole dB is not. Nobody has measured
// the overshoot distribution across a corpus of cues, and if a cue
// ever trips this the right response is that measurement, not a
// looser bound.
// `voice` was on the strict side of this bound while it was a SUM of a
// region's chunks. It no longer is: a region carries three
// presentations of one take, so the exporter keeps ONE stream and
// performs no arithmetic on it. That puts `voice` with `se` -- a
// single wave off the disc, mastered near full scale, whose lossy
// decode overshoots by a fraction of a dB. `ADV`'s louder
// presentation measures +0.0003 dBFS at source; refusing that would
// be refusing the disc's own mastering.
Some(p) if kind == "bgm" && p >= 0.0 => errors.push(format!(
"{file}: peak is {p:.1} dBFS -- a SUM we produced clips"
)),
Some(p) if kind != "bgm" && p > 1.0 => errors.push(format!(
"{file}: peak is {p:.1} dBFS -- too far over full scale to be decode overshoot"
)),
Some(_) => {}
}
match a.get("duration_s").and_then(Value::as_f64) {
Some(d) if d > 0.0 => {}
_ => errors.push(format!(
"{file}: no positive `duration_s` -- a zero-length asset plays as silence"
)),
}
}
}