From e828e7685255a4c0f2091288a209a36258b78d2b Mon Sep 17 00:00:00 2001 From: "@permadeath.com" Date: Wed, 19 Aug 2026 23:48:23 -0400 Subject: [PATCH] refactor(unit-rules): read the damaged oracle in one place `helm-bridge` reads the other three dumps and now reads `damaged.jsonl` too, which the conformance tests were parsing by hand - twice, with the location and slot mapping written out each time. `helm bv-report --damaged` prints the comparison the test asserts on, which is how the rules that only damage reaches can be looked at without running a test. --- README.md | 1 + crates/helm-bridge/src/lib.rs | 78 +++++++++++++++++++++++++++ crates/helm-bv/tests/conformance.rs | 80 +++++++++++---------------- crates/helm-cli/src/main.rs | 83 +++++++++++++++++++++++++++++ 4 files changed, 194 insertions(+), 48 deletions(-) diff --git a/README.md b/README.md index a024335..d34da5b 100644 --- a/README.md +++ b/README.md @@ -62,6 +62,7 @@ for reporting: helm bv-report ... --clusters # group the failures by what they share helm bv-report ... --unit "Atlas" # one design's working beside MegaMek's helm bv-report ... --rules # how many designs each rule fires on + helm bv-report ... --damaged # the same, for designs that were shot at A `.mul` is a force in a state rather than a design, so scoring one is its own command - and `--why` says where a damaged machine's value went, term by term, diff --git a/crates/helm-bridge/src/lib.rs b/crates/helm-bridge/src/lib.rs index cbfeca7..be588f7 100644 --- a/crates/helm-bridge/src/lib.rs +++ b/crates/helm-bridge/src/lib.rs @@ -16,6 +16,7 @@ //! generic JSON object and ignores the rest, so the dumper can keep emitting //! every field MegaMek has while this side takes only what is modelled. +use std::collections::{BTreeMap, BTreeSet}; use std::path::Path; use helm_core::{AlphaStrike, BvBreakdown, Catalogue, ComputedStats, EquipmentEntry}; @@ -83,6 +84,83 @@ pub fn read_bv(path: &Path) -> Result, Error> { Ok(out) } +/// One row of `damaged.jsonl`: a design, a way of shooting it, and what +/// MegaMek made of the result. +/// +/// A `.mtf` describes a design as it leaves the factory and nothing else, so +/// what a Mek is worth once its plate is gone or its magazines are empty has +/// to be asked of MegaMek directly. The state is recorded the way helm names +/// it - armour and structure by location code, wrecked slots and empty bins by +/// location and index - so it drops straight into a `Condition`. +#[derive(Debug, Clone, PartialEq)] +pub struct Damaged { + pub name: String, + /// What was done to it: `stripped`, `hip-gone`, `dry`. + pub scenario: String, + pub battle_value: i64, + pub defensive: Option, + pub offensive: Option, + pub armor: BTreeMap, + pub structure: BTreeMap, + pub destroyed: BTreeMap>, + pub empty_ammo: BTreeMap>, +} + +/// Read `damaged.jsonl`, the oracle for designs that have been shot at. +pub fn read_damaged(path: &Path) -> Result, Error> { + let mut out = Vec::new(); + for (_, v) in objects(path)? { + let (Some(name), Some(scenario), Some(bv)) = ( + v.get("name").and_then(Value::as_str), + v.get("scenario").and_then(Value::as_str), + v.get("bv").and_then(Value::as_i64), + ) else { + continue; + }; + let points = |field: &str| -> BTreeMap { + v.get(field) + .and_then(Value::as_object) + .map(|o| { + o.iter() + .filter_map(|(k, n)| Some((k.clone(), n.as_i64()?))) + .collect() + }) + .unwrap_or_default() + }; + let slots = |field: &str| -> BTreeMap> { + let mut map: BTreeMap> = BTreeMap::new(); + for pair in v + .get(field) + .and_then(Value::as_array) + .unwrap_or(&Vec::new()) + { + let (Some(location), Some(at)) = ( + pair.get(0).and_then(Value::as_str), + pair.get(1).and_then(Value::as_u64), + ) else { + continue; + }; + map.entry(location.to_string()) + .or_default() + .insert(at as usize); + } + map + }; + out.push(Damaged { + name: name.to_string(), + scenario: scenario.to_string(), + battle_value: bv, + defensive: v.get("defensiveValue").and_then(Value::as_f64), + offensive: v.get("offensiveValue").and_then(Value::as_f64), + armor: points("armor"), + structure: points("structure"), + destroyed: slots("destroyed"), + empty_ammo: slots("emptyAmmo"), + }); + } + Ok(out) +} + /// Read `equipment.jsonl` into the equipment catalogue. pub fn read_catalogue(path: &Path) -> Result { parse_catalogue(&std::fs::read_to_string(path)?) diff --git a/crates/helm-bv/tests/conformance.rs b/crates/helm-bv/tests/conformance.rs index 83cab3f..f052687 100644 --- a/crates/helm-bv/tests/conformance.rs +++ b/crates/helm-bv/tests/conformance.rs @@ -287,31 +287,24 @@ fn damaged_designs_agree_with_megamek() { let Some(inputs) = inputs() else { panic!("set HELM_MEGAMEK to a MegaMek install and HELM_BRIDGE to a bridge dump"); }; - let path = PathBuf::from(std::env::var("HELM_BRIDGE").unwrap()).join("damaged.jsonl"); - if !path.is_file() { - println!("no damaged.jsonl in the bridge dump; run bridge/dump.sh to make one"); + let Some(rows) = damaged_rows() else { return; - } - let text = std::fs::read_to_string(&path).expect("damaged.jsonl"); + }; let mut checked = 0; let mut wrong: Vec = Vec::new(); - for line in text.lines().filter(|l| !l.trim().is_empty()) { - let row: serde_json::Value = serde_json::from_str(line).expect("a damaged row"); - let name = row["name"].as_str().unwrap(); - let scenario = row["scenario"].as_str().unwrap(); - let theirs = row["bv"].as_i64().unwrap(); - + for row in &rows { + let (name, scenario, theirs) = (&row.name, &row.scenario, row.battle_value); let Some(unit) = inputs .library .units .iter() - .find(|u| u.display_name() == name) + .find(|u| u.display_name() == *name) else { continue; }; - let condition = condition_of(&row); + let condition = condition_from(row); checked += 1; match helm_bv::breakdown_in(unit, &inputs.catalogue, &condition) { @@ -329,8 +322,8 @@ fn damaged_designs_agree_with_megamek() { wrong.push(format!( "{name} {scenario}: ours {:?}, megamek {theirs} (def {}, off {})", ours.battle_value, - half(ours.defensive, row["defensiveValue"].as_f64()), - half(ours.offensive, row["offensiveValue"].as_f64()), + half(ours.defensive, row.defensive), + half(ours.offensive, row.offensive), )); } Err(e) => wrong.push(format!("{name} {scenario}: {e}, megamek {theirs}")), @@ -365,30 +358,26 @@ fn damaged_designs_agree_with_megamek() { /// The condition one row of `damaged.jsonl` describes. /// /// MegaMek was asked what a machine in this state is worth, so the state has -/// to be read back the same way by everything that checks against it. -fn condition_of(row: &serde_json::Value) -> helm_bv::Condition { - let mut condition = helm_bv::Condition::undamaged(); - for (loc, points) in row["armor"].as_object().unwrap() { - condition - .armor - .insert(loc.clone(), points.as_i64().unwrap()); - } - for (loc, points) in row["structure"].as_object().unwrap() { - condition - .structure - .insert(loc.clone(), points.as_i64().unwrap()); +/// to be read back the same way by everything that checks against it - which +/// is why the row itself is read by `helm-bridge` beside the other dumps +/// rather than here. +fn condition_from(row: &helm_bridge::Damaged) -> helm_bv::Condition { + helm_bv::Condition { + armor: row.armor.clone(), + structure: row.structure.clone(), + destroyed: row.destroyed.clone(), + empty_ammo: row.empty_ammo.clone(), } - for (field, into) in [ - ("destroyed", &mut condition.destroyed), - ("emptyAmmo", &mut condition.empty_ammo), - ] { - for pair in row[field].as_array().unwrap() { - let loc = pair[0].as_str().unwrap().to_string(); - let slot = pair[1].as_u64().unwrap() as usize; - into.entry(loc).or_default().insert(slot); - } +} + +/// The rows the bridge dumped, or nothing when there is no dump to read. +fn damaged_rows() -> Option> { + let path = PathBuf::from(std::env::var("HELM_BRIDGE").ok()?).join("damaged.jsonl"); + if !path.is_file() { + println!("no damaged.jsonl in the bridge dump; run bridge/dump.sh to make one"); + return None; } - condition + Some(helm_bridge::read_damaged(&path).expect("damaged.jsonl")) } /// Every point of damage MegaMek was asked about, accounted for by name. @@ -406,29 +395,24 @@ fn attribution_accounts_for_every_point_of_damage() { let Some(inputs) = inputs() else { panic!("set HELM_MEGAMEK to a MegaMek install and HELM_BRIDGE to a bridge dump"); }; - let path = PathBuf::from(std::env::var("HELM_BRIDGE").unwrap()).join("damaged.jsonl"); - if !path.is_file() { - println!("no damaged.jsonl in the bridge dump; run bridge/dump.sh to make one"); + let Some(rows) = damaged_rows() else { return; - } - let text = std::fs::read_to_string(&path).expect("damaged.jsonl"); + }; let fresh = helm_bv::Condition::undamaged(); let mut checked = 0; let mut wrong: Vec = Vec::new(); - for line in text.lines().filter(|l| !l.trim().is_empty()) { - let row: serde_json::Value = serde_json::from_str(line).expect("a damaged row"); - let name = row["name"].as_str().unwrap(); - let scenario = row["scenario"].as_str().unwrap(); + for row in &rows { + let (name, scenario) = (&row.name, &row.scenario); let Some(unit) = inputs .library .units .iter() - .find(|u| u.display_name() == name) + .find(|u| u.display_name() == *name) else { continue; }; - let condition = condition_of(&row); + let condition = condition_from(row); let Ok(a) = helm_bv::attribute(unit, &inputs.catalogue, &fresh, &condition) else { continue; }; diff --git a/crates/helm-cli/src/main.rs b/crates/helm-cli/src/main.rs index 6a8af1e..fed5181 100644 --- a/crates/helm-cli/src/main.rs +++ b/crates/helm-cli/src/main.rs @@ -83,6 +83,9 @@ BV-REPORT does not ship, and both look like an unwritten rule. --bench What a recompute costs, split by what changed. Reseating a pilot and repairing armour are not the same work. + --damaged Compare against damaged.jsonl instead of the whole + library: what a handful of designs are worth once they + have been shot at, which a .mtf cannot say. CHECK What is wrong with a .mul before anything else touches it: designs that are @@ -127,6 +130,7 @@ struct Opts { label: Option, rules: bool, bench: bool, + damaged: bool, why: bool, to: Option, battle: bool, @@ -144,6 +148,7 @@ fn parse_opts(args: &[String]) -> Result { label: None, rules: false, bench: false, + damaged: false, why: false, to: None, battle: false, @@ -211,6 +216,10 @@ fn parse_opts(args: &[String]) -> Result { o.battle = true; i += 2; } + "--damaged" => { + o.damaged = true; + i += 1; + } "--why" => { o.why = true; i += 1; @@ -428,6 +437,14 @@ fn bv_report(args: &[String]) -> Result<(), String> { return one_unit(&library, &catalogue, &megamek_bv, needle); } + if opts.damaged { + return print_damaged(&library, &catalogue, &bridge); + } + + if opts.damaged { + return print_damaged(&library, &catalogue, &bridge); + } + if opts.bench { return print_bench(&library, &catalogue); } @@ -1034,6 +1051,72 @@ fn print_rules(library: &helm_unitfile::Library, catalogue: &Catalogue) -> Resul Ok(()) } +/// How helm scores designs that have been shot at, against MegaMek's own +/// answers for the same states. +/// +/// A `.mtf` describes a design as it leaves the factory, so this is the only +/// way to check the rules that only damage reaches - and it is the report the +/// conformance test asserts on, printed for a human. +fn print_damaged( + library: &helm_unitfile::Library, + catalogue: &Catalogue, + bridge: &std::path::Path, +) -> Result<(), String> { + let path = bridge.join("damaged.jsonl"); + let rows = helm_bridge::read_damaged(&path).map_err(|e| format!("{}: {e}", path.display()))?; + if rows.is_empty() { + return Err(format!( + "{} holds no damaged designs; run bridge/dump.sh to make some", + path.display() + )); + } + + let mut checked = 0; + let mut wrong = 0; + let mut last = String::new(); + for row in &rows { + let Some(unit) = library.units.iter().find(|u| u.display_name() == row.name) else { + continue; + }; + let condition = helm_bv::Condition { + armor: row.armor.clone(), + structure: row.structure.clone(), + destroyed: row.destroyed.clone(), + empty_ammo: row.empty_ammo.clone(), + }; + checked += 1; + if row.name != last { + println!("{}", row.name); + last = row.name.clone(); + } + match helm_bv::battle_value_in(unit, catalogue, &condition) { + Ok(ours) if ours == row.battle_value => { + println!(" {:<18} {:>6}", row.scenario, ours); + } + Ok(ours) => { + wrong += 1; + println!( + " {:<18} {:>6} megamek {} ({:+})", + row.scenario, + ours, + row.battle_value, + ours - row.battle_value + ); + } + Err(why) => { + wrong += 1; + println!(" {:<18} {why}", row.scenario); + } + } + } + + println!("\n{} of {checked} states agree", checked - wrong); + if wrong > 0 { + return Err(format!("{wrong} damaged states disagree")); + } + Ok(()) +} + /// Time the operations a force-building screen actually performs. /// /// The question this answers is whether a browser can recompute battle value -- 2.51.2