From 7fb1bac5b010c9bc5a881dd9fe254a4aec1b6639 Mon Sep 17 00:00:00 2001 From: dagit Date: Tue, 12 Nov 2024 19:53:22 -0800 Subject: [PATCH 1/2] Change error handling to avoid macOS crash --- Cargo.lock | 7 ++++ Cargo.toml | 1 + src/livesplit_renderer.rs | 69 +++++++++++++++++++++++---------------- src/utils.rs | 20 ++++++++++-- 4 files changed, 65 insertions(+), 32 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6b72509..41f56f6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -193,6 +193,7 @@ dependencies = [ name = "annelid" version = "0.1.0" dependencies = [ + "anyhow", "bytemuck", "clap", "directories", @@ -268,6 +269,12 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "anyhow" +version = "1.0.93" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4c95c10ba0b00a02636238b814946408b1322d5ac4760326e6fb8ec956d85775" + [[package]] name = "arboard" version = "3.4.1" diff --git a/Cargo.toml b/Cargo.toml index 6eaee26..6b36837 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,6 +34,7 @@ glow = "0.14" bytemuck = { version = "*", features = ["derive"] } memoffset = "*" thread-priority = "1" +anyhow = "1.0.93" # Remember to test with --release [profile.dev] diff --git a/src/livesplit_renderer.rs b/src/livesplit_renderer.rs index e311db9..8c63a1f 100644 --- a/src/livesplit_renderer.rs +++ b/src/livesplit_renderer.rs @@ -1,9 +1,9 @@ use crate::autosplitters::supermetroid::{SNESState, Settings}; +use anyhow::{anyhow, Result}; use eframe::egui; use livesplit_core::{Layout, SharedTimer, Timer}; use livesplit_hotkey::Hook; use parking_lot::RwLock; -use std::error::Error; use std::sync::Arc; use thread_priority::{set_current_thread_priority, ThreadBuilder, ThreadPriority}; @@ -32,6 +32,7 @@ pub struct LiveSplitCoreRenderer { app_config_processed: bool, glow_canvas: GlowCanvas, global_hotkey_hook: Option, + load_errors: Vec, } fn show_children( @@ -92,6 +93,7 @@ impl LiveSplitCoreRenderer { app_config_processed: false, glow_canvas: GlowCanvas::new(), global_hotkey_hook: None, + load_errors: vec![], } } @@ -208,33 +210,41 @@ impl LiveSplitCoreRenderer { } pub fn process_app_config(&mut self, ctx: &egui::Context) { - messagebox_on_error(|| { + use anyhow::Context; + let mut queue = vec![]; + std::mem::swap(&mut queue, &mut self.load_errors); + queue_on_error(&mut queue, || { // Now that we've converged on a config, try loading what we can let config = self.app_config.read().unwrap().clone(); if let Some(layout) = config.recent_layout { - let f = std::fs::File::open(layout)?; - self.load_layout(&f, ctx)?; + let f = std::fs::File::open(&layout) + .with_context(|| format!("Failed to open layout file \"{}\"", layout))?; + self.load_layout(&f, ctx) + .with_context(|| format!("Failed to load layout file \"{}\"", layout))?; } if let Some(splits) = config.recent_splits { - let f = std::fs::File::open(&splits)?; + let f = std::fs::File::open(&splits) + .with_context(|| format!("Failed to open splits file \"{}\"", splits))?; let path = std::path::Path::new(&splits) .parent() - .ok_or("failed to find parent directory")?; - self.load_splits(&f, path.to_path_buf())?; + .ok_or(anyhow!("failed to find parent directory"))?; + self.load_splits(&f, path.to_path_buf()) + .with_context(|| format!("Failed to load splits file \"{}\"", splits))?; } if let Some(autosplitter) = config.recent_autosplitter { - let f = std::fs::File::open(autosplitter)?; - self.load_autosplitter(&f)?; + let f = std::fs::File::open(&autosplitter).with_context(|| { + format!("Failed to open autosplitter config \"{}\"", autosplitter) + })?; + self.load_autosplitter(&f).with_context(|| { + format!("Failed to load autosplitter config \"{}\"", autosplitter) + })?; } Ok(()) }); + self.load_errors = queue; } - pub fn load_layout( - &mut self, - f: &std::fs::File, - ctx: &egui::Context, - ) -> Result<(), Box> { + pub fn load_layout(&mut self, f: &std::fs::File, ctx: &egui::Context) -> Result<()> { use std::io::Read; let mut reader = std::io::BufReader::new(f); let mut layout_file = String::new(); @@ -280,21 +290,17 @@ impl LiveSplitCoreRenderer { Ok(()) } - pub fn load_splits( - &mut self, - f: &std::fs::File, - path: std::path::PathBuf, - ) -> Result<(), Box> { + pub fn load_splits(&mut self, f: &std::fs::File, path: std::path::PathBuf) -> Result<()> { use livesplit_core::run::parser::composite; use std::io::Read; - let file_contents: Result, _> = f.bytes().collect(); + let file_contents: std::result::Result, _> = f.bytes().collect(); // TODO: fix this unwrap *self.timer.write().unwrap() = Timer::new(composite::parse(&file_contents?, path.parent())?.run)?; Ok(()) } - pub fn load_autosplitter(&mut self, f: &std::fs::File) -> Result<(), Box> { + pub fn load_autosplitter(&mut self, f: &std::fs::File) -> Result<()> { *self.settings.write() = serde_json::from_reader(std::io::BufReader::new(f))?; Ok(()) } @@ -394,7 +400,7 @@ impl LiveSplitCoreRenderer { default_dir: &str, default_fname: &str, file_type: (&str, &str), - save_action: impl FnOnce(&mut Self, std::fs::File) -> Result<(), Box>, + save_action: impl FnOnce(&mut Self, std::fs::File) -> Result<()>, ) { use rfd::FileDialog; messagebox_on_error(|| { @@ -495,11 +501,7 @@ impl LiveSplitCoreRenderer { &mut self, default_dir: &str, file_type: (&str, &str), - open_action: impl FnOnce( - &mut Self, - std::fs::File, - std::path::PathBuf, - ) -> Result<(), Box>, + open_action: impl FnOnce(&mut Self, std::fs::File, std::path::PathBuf) -> Result<()>, ) { use rfd::FileDialog; messagebox_on_error(|| { @@ -518,7 +520,7 @@ impl LiveSplitCoreRenderer { }); } - pub fn enable_global_hotkeys(&mut self) -> Result<(), Box> { + pub fn enable_global_hotkeys(&mut self) -> Result<()> { // It would be more elegant to use get_or_insert_with, however // the `with` branch cannot have a `Result` type if we do that. let hook: &Hook = match self.global_hotkey_hook.as_ref() { @@ -676,6 +678,15 @@ impl LiveSplitCoreRenderer { impl eframe::App for LiveSplitCoreRenderer { fn update(&mut self, ctx: &egui::Context, frame: &mut eframe::Frame) { //let update_timer = std::time::Instant::now(); + if self.app_config_processed { + if self.load_errors.len() > 0 { + let mut queue: Vec = vec![]; + std::mem::swap(&mut queue, &mut self.load_errors); + for e in queue.into_iter() { + messagebox_on_error(move || Err(e)) + } + } + } if !self.app_config_processed { self.process_app_config(ctx); self.app_config_processed = true; @@ -970,7 +981,7 @@ pub fn app_init( // polling of SNES state .spawn(move |_| loop { let latency = Arc::new(RwLock::new((0.0, 0.0))); - print_on_error(|| -> std::result::Result<(), Box> { + print_on_error(|| -> std::result::Result<(), Box> { let mut client = crate::usb2snes::SyncClient::connect()?; client.set_name("annelid")?; println!("Server version is {:?}", client.app_version()?); diff --git a/src/utils.rs b/src/utils.rs index ef488bb..fa3af22 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -1,12 +1,14 @@ +use anyhow::Result; + pub fn messagebox_on_error(f: F) where - F: FnOnce() -> std::result::Result<(), Box>, + F: FnOnce() -> Result<()>, { use rfd::{MessageDialog, MessageLevel}; match f() { Ok(()) => {} Err(e) => { - println!("{}", e); + println!("about to show messagebox due to: {}", e); MessageDialog::new() .set_level(MessageLevel::Error) .set_title("Error") @@ -18,7 +20,7 @@ where pub fn print_on_error(f: F) where - F: FnOnce() -> std::result::Result<(), Box>, + F: FnOnce() -> Result<(), Box>, { match f() { Ok(()) => {} @@ -28,6 +30,18 @@ where } } +pub fn queue_on_error(queue: &mut Vec, f: F) +where + F: FnOnce() -> Result<()>, +{ + match f() { + Ok(()) => {} + Err(e) => { + queue.push(e.into()); + } + } +} + pub fn from_de_error(e: toml::de::Error) -> std::io::Error { std::io::Error::new(std::io::ErrorKind::InvalidData, e.to_string()) } From e01834bd264bda018d21bc5d48875ab786fa23ae Mon Sep 17 00:00:00 2001 From: Jason Dagit Date: Tue, 12 Nov 2024 20:01:37 -0800 Subject: [PATCH 2/2] clippy clean up --- src/livesplit_renderer.rs | 12 +++++------- src/utils.rs | 2 +- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/livesplit_renderer.rs b/src/livesplit_renderer.rs index 8c63a1f..1ee53e6 100644 --- a/src/livesplit_renderer.rs +++ b/src/livesplit_renderer.rs @@ -678,13 +678,11 @@ impl LiveSplitCoreRenderer { impl eframe::App for LiveSplitCoreRenderer { fn update(&mut self, ctx: &egui::Context, frame: &mut eframe::Frame) { //let update_timer = std::time::Instant::now(); - if self.app_config_processed { - if self.load_errors.len() > 0 { - let mut queue: Vec = vec![]; - std::mem::swap(&mut queue, &mut self.load_errors); - for e in queue.into_iter() { - messagebox_on_error(move || Err(e)) - } + if self.app_config_processed && !self.load_errors.is_empty() { + let mut queue: Vec = vec![]; + std::mem::swap(&mut queue, &mut self.load_errors); + for e in queue.into_iter() { + messagebox_on_error(move || Err(e)) } } if !self.app_config_processed { diff --git a/src/utils.rs b/src/utils.rs index fa3af22..6cae8c9 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -37,7 +37,7 @@ where match f() { Ok(()) => {} Err(e) => { - queue.push(e.into()); + queue.push(e); } } }