From 6b1af0f2f9077efaeb4b9827083edb189b41121f Mon Sep 17 00:00:00 2001 From: "@permadeath.com" Date: Wed, 19 Aug 2026 18:23:38 -0400 Subject: [PATCH] test(unit-rules): no complaint about a file MegaMek wrote The way a check like this fails is by refusing good files, so it is run over every .mul the install ships: 99 machines and no complaint. Four name a design MegaMek does not ship, which is a fault in its own example.mul and is written up in UPSTREAM.md. --- TODO.md | 36 +++++++++++++-- crates/helm-bv/UPSTREAM.md | 18 ++++++++ crates/helm-bv/tests/conformance.rs | 69 +++++++++++++++++++++++++++++ 3 files changed, 119 insertions(+), 4 deletions(-) diff --git a/TODO.md b/TODO.md index 21c44ab..2fac33f 100644 --- a/TODO.md +++ b/TODO.md @@ -383,9 +383,35 @@ not obvious from any one of them. nought this crate invented, and a slot still says which equipment is in it. -- [ ] **Take a `.mul` a player uploaded**, and say what is wrong with it +- [x] **Take a `.mul` a player uploaded**, and say what is wrong with it before anything else touches it - designs not in the library, a crew - outside 0-8, a location a shape does not have. + outside 0-8, a location a shape does not have. `helm_bv::check` reports + all of it at once and refuses nothing; `helm check ` prints it + and exits non-zero. + + Most of the checks are about what the *reader* forgave. A location index + it cannot place is skipped, an unparseable figure is left out, more + structure than a design has is clamped when it is scored - all right for + a reader, since a force that is nine tenths readable is worth reading, + and all wrong for an upload, where the nine tenths get stored and nobody + is told about the last tenth. Two of the checks can only be answered + from the document, because by the time it is a model the evidence is + gone. + + A complaint says whether it is about the file or about us. A force of + tanks is a good file helm cannot score yet, and refusing it would be + charging a player for our own gap - so only the file's own faults fail + the command. + + The guard against the obvious failure - a check that refuses good files - + is every `.mul` the install ships: 99 machines, no complaint about any of + them. Four name a design MegaMek does not ship, which is a fault in + MegaMek's own `example.mul` and is written up in `UPSTREAM.md`. + + Not reachable from a page yet: the check needs the design to compare + against, and how a design gets to the page is the decision still open + below. Everything here is `helm-core` and `helm-unitfile` only, so it + crosses when that is settled. - [ ] **Store one on an ATProto record and read it back.** The record holds the `.mul` itself, with metadata beside it. The file now keeps the version that wrote it - MekBay stamps its own build where MegaMek writes @@ -443,8 +469,10 @@ not obvious from any one of them. factor at the wrong moment passed it - the sweep over the library is what closes that, and both wrong splits were tried on purpose to see the guard fire. -- [ ] **Verify a post-match file and show it back.** The upload check above, - run on the way out rather than the way in. +- [ ] **Verify a post-match file and show it back.** The upload check is + written and is the same check either way; what is left is showing it, + which wants the attribution beside it - what the file says went wrong, + and what it cost. - [x] **Checked against the published rules, not only against MegaMek.** The TechManual's own three worked examples, run through helm term by term: diff --git a/crates/helm-bv/UPSTREAM.md b/crates/helm-bv/UPSTREAM.md index 407e967..ee74bda 100644 --- a/crates/helm-bv/UPSTREAM.md +++ b/crates/helm-bv/UPSTREAM.md @@ -158,3 +158,21 @@ helm reproduces the behaviour rather than the intent: scoring the Watchdog at 7 as written costs twenty-one designs. Whether the intent or the behaviour is right is a question for MegaMek; the answers are what conformance is measured against, so the answers are what helm follows. + +## `example.mul` names a variant MegaMek does not ship + +`data/scenariotemplates/fixedmuls/example.mul` puts four entities in as +`chassis="Centipede Scout Car" model="(Standard)"`. The install ships +`Centipede Scout Car.blk`, whose `` is empty, alongside the `(SRM)` and +`(TAG)` variants - so the cache holds `Centipede Scout Car` and no +`Centipede Scout Car (Standard)`. + +`MULParser.getEntity` looks the pair up as `chassis + " " + model`, then tries +the two the other way round, and warns `Could not find Entity with chassis: +Centipede Scout Car, and model: (Standard)` when neither answers. So MegaMek +does not load these four either; the file is wrong rather than the lookup. + +Found by running helm's upload check over every `.mul` the install ships: 99 +machines, and these are the only four that name a design the library does not +have. The fix is `model=""`, which is what the other entities in the same file +do for a base variant. diff --git a/crates/helm-bv/tests/conformance.rs b/crates/helm-bv/tests/conformance.rs index 95cfbc1..f949b44 100644 --- a/crates/helm-bv/tests/conformance.rs +++ b/crates/helm-bv/tests/conformance.rs @@ -520,6 +520,75 @@ fn attribution_accounts_for_every_point_of_damage() { ); } +/// Nothing MegaMek itself wrote is called wrong by the upload check. +/// +/// The check exists to refuse a file that says something no machine could be, +/// and the way it fails is by refusing a good one. These files are the +/// strongest evidence available that it does not: MegaMek wrote every one of +/// them, so a complaint about a shape, a location, a plate figure or a slot +/// index is this crate misreading the format rather than a fault in the file. +/// +/// Two complaints are allowed through, because neither is about the file. A +/// design the library does not have is a file naming something this MegaMek +/// does not ship - `example.mul` names a `Centipede Scout Car (Standard)`, +/// which is not in the cache, and MegaMek's own parser warns about it in the +/// same words. A unit type with no calculator is our gap. +#[test] +#[ignore = "needs a MegaMek install and a bridge dump; set HELM_MEGAMEK and HELM_BRIDGE"] +fn nothing_megamek_wrote_is_called_wrong() { + let Some(inputs) = inputs() else { + panic!("set HELM_MEGAMEK to a MegaMek install and HELM_BRIDGE to a bridge dump"); + }; + let root = PathBuf::from(std::env::var("HELM_MEGAMEK").unwrap()); + let mut files = Vec::new(); + collect_muls(&root, &mut files); + assert!( + !files.is_empty(), + "the install ships no .mul files to check" + ); + + let mut machines = 0; + let mut unknown = 0; + let mut wrong: Vec = Vec::new(); + for path in &files { + let Ok(text) = std::fs::read_to_string(path) else { + continue; + }; + let Ok(mul) = helm_unitfile::parse_mul(&text) else { + continue; + }; + for entity in mul.machines() { + machines += 1; + let design = inputs + .library + .units + .iter() + .find(|u| u.answers_to(&entity.chassis, &entity.model)); + for trouble in helm_bv::check(design, &inputs.catalogue, entity) { + match trouble { + helm_bv::Trouble::UnknownDesign => unknown += 1, + t if t.severity() == helm_bv::Severity::Unsupported => {} + t => wrong.push(format!( + "{}: {}: {t}", + path.file_name().unwrap_or_default().to_string_lossy(), + entity.display_name() + )), + } + } + } + } + + println!("{machines} machines MegaMek wrote, {unknown} naming a design it does not ship"); + for line in &wrong { + println!(" {line}"); + } + assert!( + wrong.is_empty(), + "{} complaints about files MegaMek wrote", + wrong.len() + ); +} + /// Every `.mul` the MegaMek install ships, read, written back, and read again. /// /// The structural round trip is checked in helm-unitfile against files written -- 2.51.2