From 4e138de3ff0b64dd2b54993f722cece27938c5d0 Mon Sep 17 00:00:00 2001 From: @permadeath.com Date: Wed, 19 Aug 2026 18:54:03 +0000 Subject: [PATCH] fix(unit-rules): a .mul keeps the version that wrote it MekBay stamps its own build where MegaMek writes 0.51.00, and rewriting one as the other destroyed the only record of where a force came from. A file this crate authors writes MegaMek's, since that is what has to read it. CONFORMED_TO_MEGAMEK says which MegaMek the rules agree with, for storing beside a figure that outlives the session that computed it - a battle value kept without it cannot be told from one computed against another MegaMek. A test refuses to let it drift from what conformance.txt was measured against. Change-Id: I342a909f67b5dc9c0ae1d0ff2c5973ff106863c8 --- TODO.md | 14 +++++++++++--- crates/helm-bv/src/lib.rs | 12 ++++++++++++ crates/helm-bv/tests/conformance.rs | 35 +++++++++++++++++++++++++++++++++++ crates/helm-bv/tests/performance.rs | 1 + crates/helm-unitfile/src/lib.rs | 2 +- crates/helm-unitfile/src/mul.rs | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--- 6 file(s) changed, 115 insertion(s)(+), 7 deletion(s)(-) diff --git a/TODO.md b/TODO.md --- a/TODO.md +++ b/TODO.md @@ -340,9 +340,17 @@ - [ ] **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. -- [ ] **Store one on an ATProto record and read it back.** Wants the lossless - round trip above, and a decision about whether the record holds the file - or a parse of it. The file is the safer thing to hold. +- [ ] **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 + `0.51.00`, and rewriting one as the other destroyed the only record of + where a force came from. + + What the metadata has to carry, so a stored figure can be told from one + computed against something else: `helm_bv::CONFORMED_TO_MEGAMEK`, the + MegaMek these rules agree with, and `helm_bv::RULES_VERSION`, the build + that produced the figure. A conformance test now refuses to let either + drift from what `conformance.txt` was actually measured against. - [ ] **Edit a force in the page, with the value following.** Held designs already make this cheap: a re-crew is 1us and a re-score 200us, so a lance follows a slider inside a frame. diff --git a/crates/helm-bv/src/lib.rs b/crates/helm-bv/src/lib.rs --- a/crates/helm-bv/src/lib.rs +++ b/crates/helm-bv/src/lib.rs @@ -107,6 +107,18 @@ /// that is exactly the case this guards. pub const RULES_VERSION: &str = "0.1.0"; +/// Which MegaMek these rules were written against and checked with. +/// +/// Battle value is defined by what MegaMek computes rather than by a document, +/// so an answer is only meaningful beside the version it agrees with. +/// `conformance.txt` records the same figure, and the two are checked against +/// each other by the conformance test. +/// +/// Worth storing beside any figure kept for later: a battle value recorded +/// without it cannot be told from one computed against a different MegaMek, +/// and the difference is silent. +pub const CONFORMED_TO_MEGAMEK: &str = "0.51.0"; + /// What has happened to a machine since it left the factory. /// /// Battle value is worked out in four scopes and this is the second of them. diff --git a/crates/helm-bv/tests/conformance.rs b/crates/helm-bv/tests/conformance.rs --- a/crates/helm-bv/tests/conformance.rs +++ b/crates/helm-bv/tests/conformance.rs @@ -503,3 +503,38 @@ } } } + +/// The version the rules claim to agree with must be the one they were +/// measured against. +/// +/// `CONFORMED_TO_MEGAMEK` is what a caller stores beside a battle value to say +/// what it means, and `conformance.txt` is what the measurement actually used. +/// If the two drift, every figure kept from here carries the wrong provenance +/// and nothing says so. +#[test] +fn the_version_the_rules_claim_is_the_one_they_were_measured_against() { + let report = std::fs::read_to_string(report_path()).expect("conformance.txt"); + let measured = report + .lines() + .find_map(|l| l.strip_prefix("megamek")) + .map(str::trim) + .expect("conformance.txt records a megamek version"); + let rules = report + .lines() + .find_map(|l| l.strip_prefix("rules")) + .map(str::trim) + .expect("conformance.txt records a rules version"); + assert_eq!( + rules, + helm_bv::RULES_VERSION, + "conformance.txt was blessed by rules {rules} and this is {}", + helm_bv::RULES_VERSION + ); + assert_eq!( + measured, + helm_bv::CONFORMED_TO_MEGAMEK, + "conformance.txt was measured against MegaMek {measured}, and the rules \ + claim {}. Re-bless the report, or correct the constant.", + helm_bv::CONFORMED_TO_MEGAMEK + ); +} diff --git a/crates/helm-bv/tests/performance.rs b/crates/helm-bv/tests/performance.rs --- a/crates/helm-bv/tests/performance.rs +++ b/crates/helm-bv/tests/performance.rs @@ -123,6 +123,7 @@ armor: [("CT".to_string(), 3)].into_iter().collect(), ..Default::default() }], + ..Default::default() }); let damaged = helm_bv::Condition { armor: [("CT".to_string(), 3)].into_iter().collect(), diff --git a/crates/helm-unitfile/src/lib.rs b/crates/helm-unitfile/src/lib.rs --- a/crates/helm-unitfile/src/lib.rs +++ b/crates/helm-unitfile/src/lib.rs @@ -27,7 +27,7 @@ pub use blk::parse_blk; pub use mtf::{armor_location_order, parse_mtf, split_pipe_list, split_system_field}; -pub use mul::{Mul, MulUnit, parse_mul, write_mul}; +pub use mul::{MUL_VERSION, Mul, MulUnit, parse_mul, write_mul}; #[cfg(feature = "library")] mod library; diff --git a/crates/helm-unitfile/src/mul.rs b/crates/helm-unitfile/src/mul.rs --- a/crates/helm-unitfile/src/mul.rs +++ b/crates/helm-unitfile/src/mul.rs @@ -99,11 +99,23 @@ impl std::error::Error for Error {} +/// The MUL version this writes when it has none to keep. +/// +/// MegaMek's own, because that is what a file this authors has to be read by. +/// A file that arrived with a version keeps it: MekBay stamps its own build in +/// there - `mekbay-0.4.3905 (main/4ff2429e)` - and rewriting that as MegaMek's +/// would destroy the only record of where the force came from. +pub const MUL_VERSION: &str = "0.51.00"; + /// A force, as a `.mul` records it. #[derive(Debug, Clone, Default, PartialEq, Eq)] pub struct Mul { /// Every entity in the file, crews that got out included. pub units: Vec, + /// What the file said it was written by, if it said. Kept so it can be + /// written back, and so a reader can tell MegaMek's files from another + /// tool's. + pub version: Option, } impl Mul { @@ -231,7 +243,13 @@ loop { match reader.read_event().map_err(|e| Error(e.to_string()))? { Event::Eof => break, - Event::Start(tag) => open.push(element(&tag)?), + Event::Start(tag) => { + let kept = element(&tag)?; + if kept.name == "unit" && mul.version.is_none() { + mul.version = kept.get("version").map(str::to_string); + } + open.push(kept); + } Event::Empty(tag) => { let done = element(&tag)?; close(&mut open, done, &mut mul); @@ -403,8 +421,10 @@ /// numbered from one, and a location with nothing to say is not written at all /// rather than written as zero. pub fn write_mul(mul: &Mul) -> String { - let mut out = - String::from("\n\n\n"); + let mut out = String::from("\n\n\n"); for unit in &mul.units { write_element(&mut out, &entity_of(unit), 1); } @@ -839,6 +859,7 @@ .insert("Right Arm".into(), [0, 5, 11].into_iter().collect()); let written = write_mul(&Mul { units: vec![unit.clone()], + ..Default::default() }); // Written one higher than they are held. assert!(written.contains(r#"index="1" isHit="true""#)); @@ -862,6 +883,7 @@ }; let back = parse_mul(&write_mul(&Mul { units: vec![unit.clone()], + ..Default::default() })) .unwrap(); assert_eq!(back.units[0].chassis, unit.chassis); @@ -883,6 +905,7 @@ }; let written = write_mul(&Mul { units: vec![unit.clone()], + ..Default::default() }); assert!(!written.contains("Left Arm"), "{written}"); let back = parse_mul(&written).unwrap(); @@ -993,6 +1016,35 @@ assert_eq!(back.units[0].gunnery, 2); assert_eq!(back.units[0].armor, mul.units[0].armor); assert_eq!(back.units[0].destroyed, mul.units[0].destroyed); + } + + // A file says what wrote it, and rewriting that as MegaMek's version would + // destroy the only record of where the force came from. MekBay stamps its + // own build in there, and files from the two tools differ in ways worth + // being able to see. + #[test] + fn the_file_keeps_the_version_that_wrote_it() { + let mekbay = r#" + + "#; + let mul = parse_mul(mekbay).unwrap(); + assert_eq!( + mul.version.as_deref(), + Some("mekbay-0.4.3905 (main/4ff2429e)") + ); + assert!(write_mul(&mul).contains(r#""#)); + + // A force this crate made up has none to keep, so it writes the + // version MegaMek reads. + let ours = Mul { + units: vec![MulUnit { + chassis: "Atlas".into(), + model: "AS7-D".into(), + ..Default::default() + }], + ..Default::default() + }; + assert!(write_mul(&ours).contains(&format!(r#""#))); } // `type` must not match `crewType`, and `armor` must not match -- tangled.sh