diff --git a/Cargo.lock b/Cargo.lock index 381c93b..9fed93e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -63,7 +63,7 @@ dependencies = [ [[package]] name = "bridge-solver" -version = "0.4.0" +version = "0.5.0" dependencies = [ "bridge-types", "clap", diff --git a/Cargo.toml b/Cargo.toml index f07c902..b98feaf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "bridge-solver" -version = "0.4.0" +version = "0.5.0" edition = "2021" license = "GPL-2.0-only" description = "Bridge Double-Dummy Solver - Port of macroxue/bridge-solver" @@ -43,6 +43,11 @@ name = "bench_fixtures" path = "examples/bench_fixtures.rs" required-features = ["play-analysis"] +[[example]] +name = "time_optimal_line" +path = "examples/time_optimal_line.rs" +required-features = ["play-analysis"] + [dependencies] bridge-types = { git = "https://github.com/bridge-craftwork/bridge-types" } clap = { version = "4", features = ["derive"], optional = true } diff --git a/README.md b/README.md index b39d2c1..360712d 100644 --- a/README.md +++ b/README.md @@ -26,3 +26,37 @@ The following are original work with no upstream counterpart: - `wasm/` — WebAssembly bindings and LIN parsing - `web/` — the browser application - PBN input/output, and the `bin/bridge-solver` PBN-processing binary + +## Annotating PBN files + +The `bridge-solver` binary (build with `--features cli`) fills in double-dummy +and par analysis on PBN files, writing Bridge Composer compatible tags: +`DoubleDummyTricks`, `OptimumResultTable`, and — when the board states its +vulnerability — `OptimumScore` and `ParContract`. + +```sh +bridge-solver -i deals.pbn # analyze one file to stdout +bridge-solver -i deals.pbn -o out.pbn # ...or to another file +bridge-solver -w -i Curated/ # annotate a tree in place +``` + +Directories are searched recursively for `*.pbn`, so a build can annotate a +whole collection in one command with no scripting around it. The pass is +designed to be safe to run over source material: + +- **Only what is missing.** A board that already has a `DoubleDummyTricks` tag + is passed through byte-for-byte. Use `--recalculate` to redo those too. +- **Nothing else is touched.** Files are edited line by line rather than + reparsed and rewritten, so `%` directives (Bridge Composer's fonts, page + setup and colours), `;` comments, and hand-authored `{...}` commentary all + survive exactly as written. Annotating a collection only ever adds lines. +- **Incomplete deals are skipped.** Auction-only teaching boards, written as + `[Deal "N:... ... ... ..."]`, parse into empty hands; they are left alone + rather than stamped with a fabricated all-zero table. +- **`--mark-verified`** sets bit `0x00080000` ("double-dummy data has been + verified") in each annotated board's `[BCFlags]`, adding the tag if absent and + preserving every bit already there. This records provenance only — no + documented BCFlags bit controls whether the DD table is *displayed*, and + Bridge Composer does not set this bit itself when it computes a table. +- **Re-runs are no-ops.** Unchanged files are not rewritten, so mtimes do not + churn. In-place writes go through a temporary file and a rename. diff --git a/src/bin/bridge-solver/main.rs b/src/bin/bridge-solver/main.rs index 006a9cb..919bdc6 100644 --- a/src/bin/bridge-solver/main.rs +++ b/src/bin/bridge-solver/main.rs @@ -7,7 +7,12 @@ //! - ParContract (if vulnerability is known) //! - OptimumResultTable (full table) //! -//! Usage: bridge-solver --input --output +//! Boards whose deal is incomplete are passed through untouched. +//! +//! Usage: +//! bridge-solver -i -o # one file to another +//! bridge-solver -i # one file to stdout +//! bridge-solver -w -i ... # annotate in place, recursively use bridge_solver::{ par, CutoffCache, DdTricks, Hands, PatternCache, Solver, CLUB, DIAMOND, EAST, HEART, NORTH, @@ -16,20 +21,39 @@ use bridge_solver::{ use clap::Parser; use std::fs; use std::io::{self, Write}; +use std::path::{Path, PathBuf}; #[derive(Parser)] #[command(name = "bridge-solver")] #[command(about = "Double-dummy solver for PBN files")] #[command(version)] struct Args { - /// Input PBN file - #[arg(short = 'i', long = "input", required = true)] - input: String, + /// Input PBN file(s) or director(ies); directories are searched recursively + /// for *.pbn. Accepts several, which requires --in-place. + #[arg(short = 'i', long = "input", required = true, num_args = 1..)] + input: Vec, /// Output PBN file (if not specified, writes to stdout) - #[arg(short = 'o', long = "output")] + #[arg(short = 'o', long = "output", conflicts_with = "in_place")] output: Option, + /// Rewrite each input file in place. Files whose content is unchanged are + /// left alone, so a re-run touches nothing and build systems see no churn. + #[arg(short = 'w', long = "in-place")] + in_place: bool, + + /// Set the "double-dummy data has been verified" bit (0x00080000) in each + /// annotated board's [BCFlags], adding the tag if absent. Note this marks + /// provenance; it does not make Bridge Composer display the DD table. + #[arg(long = "mark-verified")] + mark_verified: bool, + + /// Recompute analysis for boards that already carry it. By default a board + /// with a [DoubleDummyTricks] tag is left exactly as found, so annotating a + /// collection only fills in what is missing. + #[arg(long = "recalculate")] + recalculate: bool, + /// Verbose output - show progress #[arg(short = 'v', long = "verbose")] verbose: bool, @@ -79,37 +103,141 @@ impl DdResults { fn main() { let args = Args::parse(); - // Read input file - let content = match fs::read_to_string(&args.input) { - Ok(c) => c, + let files = match collect_inputs(&args.input) { + Ok(f) => f, Err(e) => { - eprintln!("Error reading input file '{}': {}", args.input, e); + eprintln!("Error: {e}"); std::process::exit(1); } }; + if files.is_empty() { + eprintln!("Error: no .pbn files found in the given input(s)"); + std::process::exit(1); + } + if files.len() > 1 && !args.in_place { + eprintln!( + "Error: {} input files matched; use --in-place to annotate them, \ + or name a single file with --output", + files.len() + ); + std::process::exit(1); + } - // Process the PBN content - let result = process_pbn(&content, args.verbose); + let mut changed = 0usize; + for path in &files { + let content = match fs::read_to_string(path) { + Ok(c) => c, + Err(e) => { + eprintln!("Error reading input file '{}': {}", path.display(), e); + std::process::exit(1); + } + }; + if args.verbose { + eprintln!("Processing {}...", path.display()); + } + let result = process_pbn(&content, args.verbose, args.recalculate, args.mark_verified); - // Write output - match args.output { - Some(path) => { - if let Err(e) = fs::write(&path, &result) { - eprintln!("Error writing output file '{}': {}", path, e); + if args.in_place { + // Unchanged files are left untouched so a re-run is a true no-op + // and does not churn mtimes in a build. + if result == content { + continue; + } + if let Err(e) = write_atomically(path, &result) { + eprintln!("Error writing '{}': {}", path.display(), e); std::process::exit(1); } - if args.verbose { - eprintln!("Output written to {}", path); + changed += 1; + } else { + match args.output { + Some(ref out) => { + if let Err(e) = fs::write(out, &result) { + eprintln!("Error writing output file '{out}': {e}"); + std::process::exit(1); + } + if args.verbose { + eprintln!("Output written to {out}"); + } + } + None => { + io::stdout().write_all(result.as_bytes()).unwrap(); + } } } - None => { - io::stdout().write_all(result.as_bytes()).unwrap(); + } + + if args.in_place && args.verbose { + eprintln!("{changed} of {} file(s) updated", files.len()); + } +} + +/// Expand the input arguments into a sorted, de-duplicated list of PBN files. +/// A directory contributes every `*.pbn` beneath it; a file is taken as given, +/// whatever its extension, so an oddly-named file can still be named directly. +fn collect_inputs(inputs: &[String]) -> io::Result> { + let mut files = Vec::new(); + for raw in inputs { + let path = PathBuf::from(raw); + if path.is_dir() { + collect_pbn_files(&path, &mut files)?; + } else if path.exists() { + files.push(path); + } else { + return Err(io::Error::new( + io::ErrorKind::NotFound, + format!("no such file or directory: {raw}"), + )); } } + files.sort(); + files.dedup(); + Ok(files) +} + +/// Recursively gather `*.pbn` under `dir`. +fn collect_pbn_files(dir: &Path, out: &mut Vec) -> io::Result<()> { + for entry in fs::read_dir(dir)? { + let path = entry?.path(); + if path.is_dir() { + collect_pbn_files(&path, out)?; + } else if path + .extension() + .is_some_and(|e| e.eq_ignore_ascii_case("pbn")) + { + out.push(path); + } + } + Ok(()) +} + +/// Write via a sibling temporary file and rename, so an interrupted run cannot +/// leave a half-written lesson file behind. +fn write_atomically(path: &Path, contents: &str) -> io::Result<()> { + let tmp = path.with_extension("pbn.tmp"); + fs::write(&tmp, contents)?; + fs::rename(&tmp, path) +} + +/// Bridge Composer's [BCFlags] bit meaning "double-dummy data has been +/// verified". Note this records provenance only: no documented BCFlags bit +/// controls whether the DD table is *displayed*, and Bridge Composer does not +/// set this one itself when it computes a table. +const BC_FLAG_DD_VERIFIED: u64 = 0x0008_0000; + +/// Return a `[BCFlags]` line with the verified bit set, preserving every other +/// bit. An unparsable value is replaced rather than propagated, since the tag +/// is meaningless if it is not hex. +fn with_verified_bit(line: &str) -> String { + let current = line + .split('"') + .nth(1) + .and_then(|v| u64::from_str_radix(v.trim(), 16).ok()) + .unwrap_or(0); + format!("[BCFlags \"{:x}\"]", current | BC_FLAG_DD_VERIFIED) } /// Process a PBN file: find deals, solve them, insert/replace DD tags -fn process_pbn(content: &str, verbose: bool) -> String { +fn process_pbn(content: &str, verbose: bool, recalculate: bool, mark_verified: bool) -> String { // Split into deal blocks (separated by blank lines outside of brace comments) let mut result = String::new(); let mut deal_count = 0; @@ -159,7 +287,13 @@ fn process_pbn(content: &str, verbose: bool) -> String { // Process this block let block_lines = &lines[block_start..block_end]; - let processed = process_deal_block(block_lines, &mut deal_count, verbose); + let processed = process_deal_block( + block_lines, + &mut deal_count, + verbose, + recalculate, + mark_verified, + ); result.push_str(&processed); } @@ -171,7 +305,13 @@ fn process_pbn(content: &str, verbose: bool) -> String { } /// Process a single deal block -fn process_deal_block(lines: &[&str], deal_count: &mut usize, verbose: bool) -> String { +fn process_deal_block( + lines: &[&str], + deal_count: &mut usize, + verbose: bool, + recalculate: bool, + mark_verified: bool, +) -> String { // Find the Deal tag to extract hands let mut deal_str: Option<&str> = None; let mut vulnerability: Option = None; @@ -199,16 +339,42 @@ fn process_deal_block(lines: &[&str], deal_count: &mut usize, verbose: bool) -> return out; }; - // Parse the deal - let Some(hands) = Hands::from_pbn(deal_str) else { - // Can't parse, pass through unchanged + // Parse the deal. A board with no cards — BridgeComposer writes + // [Deal "N:... ... ... ..."] for auction-only teaching boards — parses fine + // into empty hands, so completeness is checked too. Annotating one of those + // would stamp a fabricated all-zero table and a "Pass" par onto a board that + // has no deal to analyze. + let unchanged = || { let mut out = String::new(); for line in lines { out.push_str(line); out.push('\n'); } - return out; + out + }; + // Already analyzed? Leave it alone unless asked to redo the work. The + // [DoubleDummyTricks] tag is the marker: a board carrying a stray par tag + // but no DD table has not been analyzed, and still gets filled in. + if !recalculate + && lines + .iter() + .any(|l| extract_tag_name(l) == Some("DoubleDummyTricks")) + { + if verbose { + eprintln!("Skipping board that already has analysis"); + } + return unchanged(); + } + + let Some(hands) = Hands::from_pbn(deal_str) else { + return unchanged(); }; + if !hands.is_complete() { + if verbose { + eprintln!("Skipping board with an incomplete deal"); + } + return unchanged(); + } *deal_count += 1; if verbose { @@ -226,6 +392,7 @@ fn process_deal_block(lines: &[&str], deal_count: &mut usize, verbose: bool) -> // 2. Insert our new DD tags in the right place let mut output_lines: Vec = Vec::new(); + let mut saw_bcflags = false; let mut found_dd_tag = false; let mut skipping_optimum_data = false; let mut insertion_point: Option = None; @@ -241,6 +408,14 @@ fn process_deal_block(lines: &[&str], deal_count: &mut usize, verbose: bool) -> for line in lines { let trimmed = line.trim(); + // Fold the verified bit into an existing [BCFlags], keeping every other + // bit the board already carried. + if mark_verified && extract_tag_name(trimmed) == Some("BCFlags") { + saw_bcflags = true; + output_lines.push(with_verified_bit(trimmed)); + continue; + } + // Check if this is one of our DD tags if let Some(tag_name) = extract_tag_name(trimmed) { if dd_tag_names.contains(&tag_name) { @@ -290,6 +465,13 @@ fn process_deal_block(lines: &[&str], deal_count: &mut usize, verbose: bool) -> // Build the output let mut result = String::new(); + // A board with no [BCFlags] of its own still needs one to carry the bit. + let dd_tags = if mark_verified && !saw_bcflags { + format!("[BCFlags \"{:x}\"]\n{dd_tags}", BC_FLAG_DD_VERIFIED) + } else { + dd_tags + }; + let insert_at = insertion_point.unwrap_or(output_lines.len()); for (idx, line) in output_lines.iter().enumerate() { @@ -573,7 +755,7 @@ mod tests { [Deal "N:AKQT3.J6.KJ42.95 652.AK42.AQ87.T4 J74.QT95.T.AK863 98.873.9653.QJ72"] [Dealer "N"] "#; - let result = process_pbn(pbn, false); + let result = process_pbn(pbn, false, false, false); assert!(result.contains("[DoubleDummyTricks")); assert!(result.contains("[OptimumResultTable")); assert!(result.contains("N NT")); @@ -610,7 +792,7 @@ W D 0 W C 0 [Dealer "N"] "#; - let result = process_pbn(pbn, false); + let result = process_pbn(pbn, false, true, false); // Each tag we generate must appear exactly once: the stale copy is // stripped and replaced, not duplicated. `Vulnerable` is present, so // the par tags are generated too. @@ -625,6 +807,115 @@ W C 0 assert!(result.contains(r#"[ParContract "NS 4S="]"#)); } + /// The default is to fill in only what is missing: a board that already + /// carries a DD table is passed through byte-for-byte, however stale its + /// values, so a builder can point the tool at a whole collection safely. + #[test] + fn test_existing_analysis_is_kept_unless_recalculating() { + let pbn = r#"[Event "Test"] +[Vulnerable "None"] +[Deal "N:AKQT3.J6.KJ42.95 652.AK42.AQ87.T4 J74.QT95.T.AK863 98.873.9653.QJ72"] +[DoubleDummyTricks "00000000000000000000"] +[Dealer "N"] +"#; + let kept = process_pbn(pbn, false, false, false); + assert_eq!(kept, pbn, "default must not touch an analyzed board"); + + let redone = process_pbn(pbn, false, true, false); + assert!(redone.contains(r#"[DoubleDummyTricks "9a8789a8784346543465"]"#)); + } + + /// A board holding a par tag but no DD table has not been analyzed, so the + /// default still fills it in — and replaces the orphaned par value. + #[test] + fn test_par_tag_alone_does_not_count_as_analyzed() { + let pbn = r#"[Event "Test"] +[Vulnerable "None"] +[Deal "N:AKQT3.J6.KJ42.95 652.AK42.AQ87.T4 J74.QT95.T.AK863 98.873.9653.QJ72"] +[OptimumScore "NS 9999"] +[Dealer "N"] +"#; + let result = process_pbn(pbn, false, false, false); + assert_eq!(result.matches("[DoubleDummyTricks").count(), 1); + assert!(!result.contains("NS 9999")); + } + + /// --mark-verified folds bit 0x00080000 into the board's existing BCFlags + /// without disturbing the bits it already carried. + #[test] + fn test_mark_verified_preserves_other_bcflags_bits() { + let pbn = r#"[Event "Test"] +[Vulnerable "None"] +[Deal "N:AKQT3.J6.KJ42.95 652.AK42.AQ87.T4 J74.QT95.T.AK863 98.873.9653.QJ72"] +[BCFlags "40001f"] +"#; + let result = process_pbn(pbn, false, false, true); + // 0x40001f | 0x80000 == 0x48001f — every original bit survives. + assert!( + result.contains(r#"[BCFlags "48001f"]"#), + "got: +{result}" + ); + assert_eq!(result.matches("[BCFlags").count(), 1); + + // Without the flag the tag is left exactly as written. + let plain = process_pbn(pbn, false, false, false); + assert!(plain.contains(r#"[BCFlags "40001f"]"#)); + } + + /// A board with no BCFlags of its own gets one carrying just that bit. + #[test] + fn test_mark_verified_adds_bcflags_when_absent() { + let pbn = r#"[Event "Test"] +[Vulnerable "None"] +[Deal "N:AKQT3.J6.KJ42.95 652.AK42.AQ87.T4 J74.QT95.T.AK863 98.873.9653.QJ72"] +"#; + let result = process_pbn(pbn, false, false, true); + assert!( + result.contains(r#"[BCFlags "80000"]"#), + "got: +{result}" + ); + assert_eq!(result.matches("[BCFlags").count(), 1); + } + + /// A board with no cards must be left exactly as found. BridgeComposer + /// writes `[Deal "N:... ... ... ..."]` for auction-only teaching boards; + /// those parse into empty hands, and annotating one would stamp a + /// fabricated all-zero table and a "Pass" par onto a board with no deal. + #[test] + fn test_placeholder_deals_pass_through_untouched() { + let pbn = r#"[Event "Test"] +[Board "1"] +[Vulnerable "None"] +[Deal "N:... ... ... ..."] +[Auction "N"] +1S Pass 2S AP +"#; + let result = process_pbn(pbn, false, false, false); + assert_eq!(result, pbn, "placeholder board must be byte-identical"); + assert!(!result.contains("DoubleDummyTricks")); + assert!(!result.contains("OptimumScore")); + } + + /// A file mixing real and placeholder boards annotates only the real ones. + #[test] + fn test_annotates_only_complete_deals_in_mixed_file() { + let pbn = r#"[Event "Test"] +[Board "1"] +[Vulnerable "None"] +[Deal "N:... ... ... ..."] + +[Event "Test"] +[Board "2"] +[Vulnerable "None"] +[Deal "N:AKQT3.J6.KJ42.95 652.AK42.AQ87.T4 J74.QT95.T.AK863 98.873.9653.QJ72"] +"#; + let result = process_pbn(pbn, false, false, false); + assert_eq!(result.matches("[DoubleDummyTricks").count(), 1); + assert!(!result.contains("\"00000000000000000000\"")); + } + /// Without a `Vulnerable` tag par cannot be scored, so the par tags are /// omitted — and a stale copy in the input is still stripped. #[test] @@ -635,7 +926,7 @@ W C 0 [ParContract "NS Pass"] [Dealer "N"] "#; - let result = process_pbn(pbn, false); + let result = process_pbn(pbn, false, false, false); assert_eq!(result.matches("[DoubleDummyTricks").count(), 1); assert_eq!(result.matches("[OptimumScore").count(), 0); assert_eq!(result.matches("[ParContract").count(), 0); diff --git a/src/hands.rs b/src/hands.rs index 82a4356..f621b08 100644 --- a/src/hands.rs +++ b/src/hands.rs @@ -59,6 +59,17 @@ impl Hands { self.hands[WEST].size() } + /// True only when all four seats hold a full 13-card hand. + /// + /// PBN files legitimately carry placeholder deals — BridgeComposer writes + /// `[Deal "N:... ... ... ..."]` for auction-only teaching boards — and those + /// parse successfully into empty hands. Solving one yields an all-zero + /// table, so callers that annotate files must skip incomplete deals rather + /// than record a fabricated result. + pub fn is_complete(&self) -> bool { + self.hands.iter().all(|h| h.size() == 13) + } + /// Parse from PBN-style deal string /// Format: "N:AKQT3.J6.KJ42.95 652.AK42.AQ87.T4 J74.QT95.T.AK863 98.873.9653.QJ72" /// Order after first seat: rotates clockwise (N E S W or W N E S, etc.) diff --git a/src/pattern.rs b/src/pattern.rs index 6b41071..6b63f11 100644 --- a/src/pattern.rs +++ b/src/pattern.rs @@ -14,6 +14,12 @@ use super::types::*; /// Pack bits: extract bits from source where mask has 1s, compress them to low bits /// Example: PackBits(0b10100, 0b11100) = 0b101 (extracts bits 2,3,4 and packs to 0,1,2) #[inline] +// `m & m.wrapping_neg()` is the classic isolate-lowest-set-bit idiom. Clippy +// (since the toolchain that stabilized it) wants `m.isolate_lowest_one()`, but +// that method is still unstable on older compilers, so adopting it would raise +// this crate's minimum Rust version for no benefit in a hot path. `unknown_lints` +// keeps compilers that predate the lint quiet about the allow itself. +#[allow(unknown_lints, clippy::manual_isolate_lowest_one)] pub fn pack_bits(source: u64, mask: u64) -> u64 { #[cfg(target_feature = "bmi2")] { @@ -43,6 +49,12 @@ pub fn pack_bits(source: u64, mask: u64) -> u64 { /// Unpack bits: scatter source bits to positions where mask has 1s /// Example: UnpackBits(0b101, 0b11100) = 0b10100 (scatters bits 0,1,2 to positions 2,3,4) #[inline] +// `m & m.wrapping_neg()` is the classic isolate-lowest-set-bit idiom. Clippy +// (since the toolchain that stabilized it) wants `m.isolate_lowest_one()`, but +// that method is still unstable on older compilers, so adopting it would raise +// this crate's minimum Rust version for no benefit in a hot path. `unknown_lints` +// keeps compilers that predate the lint quiet about the allow itself. +#[allow(unknown_lints, clippy::manual_isolate_lowest_one)] pub fn unpack_bits(source: u64, mask: u64) -> u64 { #[cfg(target_feature = "bmi2")] {