Skip to content

Commit

Permalink
desktop: Improve record format handling.
Browse files Browse the repository at this point in the history
* Don't parse it twice, but carry it through the args struct.
* Remove superfluous error conversions.
  • Loading branch information
kpreid committed Aug 22, 2024
1 parent 4b6eef2 commit 7eb86fb
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 18 deletions.
18 changes: 8 additions & 10 deletions all-is-cubes-desktop/src/bin/all-is-cubes/command_options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,16 +112,16 @@ pub(crate) struct AicDesktopArgs {
required_if_eq("graphics", "record"),
value_name = "FILE",
value_parser = clap::builder::PathBufValueParser::new().try_map(|value| {
let _format = determine_record_format(&value)?;
Ok::<PathBuf, &str>(value)
let format = determine_record_format(&value)?;
Ok::<(PathBuf, RecordFormat), &str>((value, format))
}),
verbatim_doc_comment,
)]
pub(crate) output_file: Option<PathBuf>,
pub(crate) output_file_and_format: Option<(PathBuf, RecordFormat)>,

/// Whether to record/export everything, rather than just the displayed scene.
#[cfg(feature = "record")]
#[arg(long, requires = "output_file")]
#[arg(long, requires = "output_file_and_format")]
pub(crate) save_all: bool,

// TODO: Generalize this to "exit after this much time has passed".
Expand Down Expand Up @@ -160,19 +160,17 @@ impl AicDesktopArgs {
///
/// Returns an error if options were inconsistent with each other.
///
/// Returns `Ok(None)` if recording was not requested (`output_file` not set).
/// Returns `Ok(None)` if recording was not requested (`output_file_and_format` not set).
///
/// Panics if `output_path` is not validated (this should have been checked at parse time).
#[cfg(feature = "record")]
#[allow(clippy::unnecessary_wraps)] // *currently* no error can happen
pub fn record_options(&self) -> clap::error::Result<Option<RecordOptions>> {
let Some(output_path) = self.output_file.clone() else {
pub fn record_options(&self) -> Result<Option<RecordOptions>, anyhow::Error> {
let Some((output_path, output_format)) = self.output_file_and_format.clone() else {
return Ok(None);
};
let output_format = determine_record_format(&output_path)
.expect("output_file should have been validated to specify a format");
let options = RecordOptions {
output_path: self.output_file.clone().unwrap(),
output_path,
output_format,
image_size: self
.display_size
Expand Down
12 changes: 4 additions & 8 deletions all-is-cubes-desktop/src/bin/all-is-cubes/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ fn main() -> Result<(), anyhow::Error> {
mut precompute_light,
input_file,
#[cfg(feature = "record")]
output_file: _, // used in RecordOptions
output_file_and_format: _, // used in RecordOptions
#[cfg(feature = "record")]
save_all: _, // used in RecordOptions
duration,
Expand All @@ -71,21 +71,17 @@ fn main() -> Result<(), anyhow::Error> {

let input_source = parse_universe_source(input_file, template, template_size, seed);

// TODO: record_options validation should just be part of the regular arg parsing
// (will need a wrapper type)
#[cfg(feature = "record")]
let record_options: Option<record::RecordOptions> = options
.record_options()
.inspect(|optropt| {
let record_options: Option<record::RecordOptions> =
options.record_options().inspect(|optropt| {
if graphics_type == GraphicsType::Record
&& optropt
.as_ref()
.is_some_and(|options| options.output_format.includes_light())
{
precompute_light = true;
}
})
.map_err(|e| e.format(&mut AicDesktopArgs::command()))?;
})?;

let graphics_options = if no_config_files {
GraphicsOptions::default()
Expand Down

0 comments on commit 7eb86fb

Please sign in to comment.