From e6071c5b75fa0e4ef41ca3d9986b877b5d0c9c39 Mon Sep 17 00:00:00 2001 From: Nathan Leiby Date: Sun, 10 Nov 2024 16:35:57 -0800 Subject: [PATCH] chore: fix clippy errors manually --- .github/workflows/rust.yml | 4 +- lint.sh | 2 + src/audio.rs | 2 +- src/config.rs | 2 + src/egui_ui.rs | 52 ++++++++++--------------- src/fps.rs | 4 +- src/game.rs | 77 ++++++++++++++++++-------------------- src/main.rs | 5 +-- src/midi_input_handler.rs | 2 +- src/score.rs | 36 +++++++----------- src/ui.rs | 5 ++- 11 files changed, 84 insertions(+), 107 deletions(-) create mode 100755 lint.sh diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index d7683b4..be6b115 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -19,5 +19,7 @@ jobs: run: sudo apt install libasound2-dev - name: Build run: cargo build --verbose - - name: Run tests + - name: Unit tests run: cargo test --verbose + - name: Lint + run: cargo clippy --all --all-features --tests -- -D warnings diff --git a/lint.sh b/lint.sh new file mode 100755 index 0000000..2c283ce --- /dev/null +++ b/lint.sh @@ -0,0 +1,2 @@ +#!/bin/sh +git ls-files | entr cargo clippy --all --all-features --tests -- -D warnings diff --git a/src/audio.rs b/src/audio.rs index e271f46..584275a 100644 --- a/src/audio.rs +++ b/src/audio.rs @@ -276,7 +276,7 @@ impl Audio { /// schedules notes for a single sound to be played between last_scheduled_tick and tick_to_schedule fn schedule_audio( - notes: &Vec, + notes: &[f64], sound: &StaticSoundData, volume: f64, manager: &mut AudioManager, diff --git a/src/config.rs b/src/config.rs index 3967a33..49670a7 100644 --- a/src/config.rs +++ b/src/config.rs @@ -15,6 +15,8 @@ impl AppConfig { } pub fn save(&self) { + // TODO: We may remove confy. Ignore for now + #[allow(clippy::match_single_binding)] match confy::store("macroix", "AppConfig", self) { // ignore failures. these happen in web builds _ => (), diff --git a/src/egui_ui.rs b/src/egui_ui.rs index 4155bbb..038f3b6 100644 --- a/src/egui_ui.rs +++ b/src/egui_ui.rs @@ -1,5 +1,3 @@ -// mod app; - use egui::{ self, emath::{self, RectTransform}, @@ -34,7 +32,9 @@ pub struct UIState { is_playing: bool, bpm: f32, is_metronome_enabled: bool, + #[allow(dead_code)] volume_metronome: f32, + #[allow(dead_code)] volume_target_notes: f32, // audio @@ -102,6 +102,8 @@ impl Default for UIState { impl UIState { // TODO: rename related to choosing a loop + // TODO: investigate a fix for the clippy error. Changing to a slice threw errors elsewhere in code. + #[allow(clippy::ptr_arg)] pub fn selector_vec(mut self, selector_vec: &Vec) -> Self { self.selector_vec = selector_vec.clone(); self @@ -135,8 +137,8 @@ impl UIState { self.latency_offset_s = offset; } - pub fn set_user_hits(&mut self, hits: &Vec) { - self.user_hits = hits.clone(); + pub fn set_user_hits(&mut self, hits: &[UserHit]) { + self.user_hits = hits.to_vec().clone(); } pub fn set_desired_hits(&mut self, voices: &Voices) { @@ -434,7 +436,7 @@ fn is_beat_enabled( visible_row: usize, col: usize, enabled_beats: EnabledBeats, - visible_instruments: &Vec<&Instrument>, + visible_instruments: &[&Instrument], ) -> bool { // determine instrument let res = visible_instruments @@ -497,8 +499,9 @@ fn draw_beat_grid(ui_state: &UIState, ui: &mut egui::Ui, events: &mut Vec "Hi-hat", Instrument::Snare => "Snare", Instrument::Kick => "Kick", @@ -623,7 +626,7 @@ fn rect_for_col_row( let base_pos = pos2(col as f32 * width_scale, row as f32 * height_scale); // TODO: fix scaling to always draw a nicer looking square based grid - + to_screen.transform_rect(egui::Rect { min: base_pos, max: base_pos + egui::Vec2::new(width_scale * 0.95, height_scale * 0.95), @@ -656,7 +659,7 @@ fn draw_user_hits( to_screen: RectTransform, shapes: &mut Vec, height_scale: f32, - visible_instruments: &Vec<&Instrument>, + visible_instruments: &[&Instrument], ) { for (instrument_idx, instrument) in visible_instruments.iter().enumerate() { let user_notes = get_user_hit_timings_by_instrument(&ui_state.user_hits, **instrument); @@ -679,7 +682,7 @@ fn draw_user_hit( user_beat: f64, row: usize, audio_latency_beats: f64, - desired_hits: &Vec, + desired_hits: &[f64], to_screen: RectTransform, shapes: &mut Vec, height_scale: f32, @@ -721,8 +724,9 @@ fn draw_user_hit( shapes.push(shape); } +#[allow(clippy::too_many_arguments)] fn draw_note_successes( - user_hits: &Vec, + user_hits: &[UserHit], desired_hits: &Voices, audio_latency: f64, loop_current_beat: f64, @@ -730,7 +734,7 @@ fn draw_note_successes( shapes: &mut Vec, width_scale: f32, height_scale: f32, - visible_instruments: &Vec<&Instrument>, + visible_instruments: &[&Instrument], ) { for (instrument_idx, instrument) in visible_instruments.iter().enumerate() { let actual = get_user_hit_timings_by_instrument(user_hits, **instrument); @@ -797,11 +801,7 @@ fn gold_mode(ui: &mut egui::Ui, ui_state: &UIState) { &ui_state.user_hits, (ui_state.current_loop as i32 - i) as usize, // TODO: check for overflow ); - let summary_data = compute_last_loop_summary( - &nth_loop_hits, - &ui_state.desired_hits, - ui_state.get_audio_latency_in_beats() as f64, - ); + let summary_data = compute_last_loop_summary(&nth_loop_hits, &ui_state.desired_hits); // Simpler than chart.. TODO: support for colored emoji // 🔴 @@ -838,17 +838,3 @@ fn gold_mode(ui: &mut egui::Ui, ui_state: &UIState) { plot_ui.line(line); }); } - -fn powered_by_egui_and_eframe(ui: &mut egui::Ui) { - ui.horizontal(|ui| { - ui.spacing_mut().item_spacing.x = 0.0; - ui.label("Powered by "); - ui.hyperlink_to("egui", "https://github.com/emilk/egui"); - ui.label(" and "); - ui.hyperlink_to( - "eframe", - "https://github.com/emilk/egui/tree/master/crates/eframe", - ); - ui.label("."); - }); -} diff --git a/src/fps.rs b/src/fps.rs index 69db0ee..33cb74f 100644 --- a/src/fps.rs +++ b/src/fps.rs @@ -6,13 +6,13 @@ use std::collections::VecDeque; use macroquad::prelude::*; -pub struct FPS { +pub struct Fps { fps_tracker: VecDeque, last_fps: i32, last_updated_fps_timestamp: f64, } -impl FPS { +impl Fps { pub fn new() -> Self { Self { fps_tracker: VecDeque::::with_capacity(10), diff --git a/src/game.rs b/src/game.rs index c0e3b25..7b2d632 100644 --- a/src/game.rs +++ b/src/game.rs @@ -135,52 +135,46 @@ pub fn process_system_events( gold_mode: &mut GoldMode, ) { // read events - loop { - match rx.try_recv() { - Ok(msg) => { - info!("[system event] {:?}", msg); - match msg { - TxMsg::AudioNew => (), - TxMsg::StartingLoop(loop_num) => { - let last_loop_hits = get_hits_from_nth_loop( - &audio.user_hits, - (audio.current_loop() - 1) as usize, - ); - let audio_latency = audio.get_configured_audio_latency_seconds(); - let summary_data = - compute_last_loop_summary(&last_loop_hits, voices, audio_latency); - info!("last loop summary = {:?}", summary_data); - let totals = summary_data.total(); - if loop_num > 0 { - // Log user metric to a file, for eventual data analysis - let user_metric = UserMetric { - system_time_ms: current_time_millis(), - bpm: audio.get_bpm(), - score: totals.score(), - }; - let log_result = log_user_metric(&user_metric); - if let Err(e) = log_result { println!("error logging user_metric. error was: {e}") } - } + while let Ok(msg) = rx.try_recv() { + info!("[system event] {:?}", msg); + match msg { + TxMsg::AudioNew => (), + TxMsg::StartingLoop(loop_num) => { + let last_loop_hits = + get_hits_from_nth_loop(&audio.user_hits, (audio.current_loop() - 1) as usize); + let summary_data = compute_last_loop_summary(&last_loop_hits, voices); + info!("last loop summary = {:?}", summary_data); + let totals = summary_data.total(); - gold_mode.was_gold = false; - if totals.score() == 1. { - gold_mode.correct_takes += 1; - } else { - gold_mode.correct_takes = 0; - } - - if gold_mode.correct_takes == GOLD_MODE_CORRECT_TAKES { - audio.set_bpm(audio.get_bpm() + GOLD_MODE_BPM_STEP); - gold_mode.correct_takes = 0; - gold_mode.was_gold = true; - // TODO: schedule a 1-off "success!" SFX to play - // TOOD: Maybe -- clear existing noise from mistaken notes - } + if loop_num > 0 { + // Log user metric to a file, for eventual data analysis + let user_metric = UserMetric { + system_time_ms: current_time_millis(), + bpm: audio.get_bpm(), + score: totals.score(), + }; + let log_result = log_user_metric(&user_metric); + if let Err(e) = log_result { + println!("error logging user_metric. error was: {e}") } } + + gold_mode.was_gold = false; + if totals.score() == 1. { + gold_mode.correct_takes += 1; + } else { + gold_mode.correct_takes = 0; + } + + if gold_mode.correct_takes == GOLD_MODE_CORRECT_TAKES { + audio.set_bpm(audio.get_bpm() + GOLD_MODE_BPM_STEP); + gold_mode.correct_takes = 0; + gold_mode.was_gold = true; + // TODO: schedule a 1-off "success!" SFX to play + // TOOD: Maybe -- clear existing noise from mistaken notes + } } - Err(_) => break, } } } @@ -199,6 +193,7 @@ fn log_user_metric(user_metric: &UserMetric) -> Result<(), Box> { } /// update application state based on events (that came from user input) +#[allow(clippy::too_many_arguments)] pub fn process_user_events( voices: &mut Voices, audio: &mut Audio, diff --git a/src/main.rs b/src/main.rs index f4a7ee9..ed2223f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -21,7 +21,7 @@ use std::error::Error; use std::sync::mpsc::{self}; use crate::config::AppConfig; -use crate::fps::FPS; +use crate::fps::Fps; use crate::ui::*; use audio::Audio; @@ -118,7 +118,7 @@ async fn main() -> Result<(), Box> { audio.initialize().await?; // debug - let mut fps_tracker = FPS::new(); + let mut fps_tracker = Fps::new(); let mut ui = UI::new(); loop { @@ -162,7 +162,6 @@ async fn main() -> Result<(), Box> { fn process_cli_args() -> String { // read commnand line arg as directory name - std::env::args() .nth(1) diff --git a/src/midi_input_handler.rs b/src/midi_input_handler.rs index 2b1cf33..1c4cc0a 100644 --- a/src/midi_input_handler.rs +++ b/src/midi_input_handler.rs @@ -152,7 +152,7 @@ fn get_midi_as_user_hits(midi_input: &MidiInput) -> Vec { }; let ic_midi = match midi_input.get_device_name() { - s if s == "MPK Mini Mk II" => mpk_mini_mk_ii, + "MPK Mini Mk II" => mpk_mini_mk_ii, s if s.contains("TD-17") => td17, s if s.contains("TD-27") => td27, s if s.contains("Nitro") => alesis_nitro, diff --git a/src/score.rs b/src/score.rs index deee6f0..6855b46 100644 --- a/src/score.rs +++ b/src/score.rs @@ -32,7 +32,7 @@ pub const MISS_MARGIN: f64 = 0.3; /// returns a tuple of (accuracy rating, a bool of whether not this measurement is wrapping around to the _next_ loop) pub fn compute_accuracy_of_single_hit( user_beat_with_latency: f64, - desired_hits: &Vec, + desired_hits: &[f64], // correct_margin, // miss_margin, ) -> (Accuracy, bool) { @@ -153,11 +153,7 @@ impl LastLoopSummary { } } - pub fn set_score_tracker( - &mut self, - instrument: &Instrument, - score_tracker: ScoreTracker, - ) { + pub fn set_score_tracker(&mut self, instrument: &Instrument, score_tracker: ScoreTracker) { let to_update: &mut ScoreTracker = self.get_mut_score_tracker(instrument); *to_update = score_tracker; } @@ -188,7 +184,7 @@ impl LastLoopSummary { } pub fn get_user_hit_timings_by_instrument( - user_hits: &Vec, + user_hits: &[UserHit], instrument: Instrument, ) -> Vec { user_hits @@ -219,7 +215,7 @@ pub fn compute_loop_performance_for_voice( // find the first user hit that a non-miss let mut was_miss = true; for user_hit in user_hits { - let (acc, _) = compute_accuracy_of_single_hit(*user_hit, &vec![*desired_hit]); + let (acc, _) = compute_accuracy_of_single_hit(*user_hit, &[*desired_hit]); if acc != Accuracy::Miss { was_miss = false; out.push(acc); @@ -234,14 +230,10 @@ pub fn compute_loop_performance_for_voice( out } -pub fn compute_last_loop_summary( - user_hits: &Vec, - desired_hits: &Voices, - audio_latency: f64, -) -> LastLoopSummary { +pub fn compute_last_loop_summary(user_hits: &[UserHit], desired_hits: &Voices) -> LastLoopSummary { let mut out = LastLoopSummary::new(); - for (_, instrument) in ALL_INSTRUMENTS.iter().enumerate() { + for instrument in ALL_INSTRUMENTS.iter() { // // get accuracy of hihat let user_timings = get_user_hit_timings_by_instrument(user_hits, *instrument); let desired_timings = desired_hits.get_instrument_beats(instrument); @@ -267,8 +259,6 @@ pub fn compute_last_loop_summary( #[cfg(test)] mod tests { - use std::f64::EPSILON; - use crate::{ consts::{UserHit, BEATS_PER_LOOP}, score::{ @@ -318,7 +308,7 @@ mod tests { assert_eq!(result, Accuracy::Early); // beyond the miss margin - let miss = MISS_MARGIN + EPSILON; + let miss = MISS_MARGIN + f64::EPSILON; let result = compute_accuracy_legacy(miss, &vec![0.0]); assert_eq!(result, Accuracy::Miss); @@ -341,7 +331,7 @@ mod tests { assert_eq!(result, Accuracy::Correct); let result = compute_accuracy_legacy( - BEATS_PER_LOOP - CORRECT_MARGIN - EPSILON * 5., + BEATS_PER_LOOP - CORRECT_MARGIN - f64::EPSILON * 5., &vec![0.0, 1.0], ); assert_eq!(result, Accuracy::Early); @@ -352,12 +342,12 @@ mod tests { #[test] fn it_computes_accuracy_considering_is_next_loop() { - let result = compute_accuracy_of_single_hit(BEATS_PER_LOOP - CORRECT_MARGIN, &vec![0.0]); + let result = compute_accuracy_of_single_hit(BEATS_PER_LOOP - CORRECT_MARGIN, &[0.0]); assert_eq!(result, (Accuracy::Correct, true)); let result = compute_accuracy_of_single_hit( - BEATS_PER_LOOP - CORRECT_MARGIN - EPSILON * 5., - &vec![0.0], + BEATS_PER_LOOP - CORRECT_MARGIN - f64::EPSILON * 5., + &[0.0], ); assert_eq!(result, (Accuracy::Early, true)); } @@ -372,7 +362,7 @@ mod tests { let mut desired_hits = Voices::new(); desired_hits.toggle_beat(Instrument::Kick, 0.0); - let result = compute_last_loop_summary(&user_hits, &desired_hits, 0.0); + let result = compute_last_loop_summary(&user_hits, &desired_hits); assert_eq!( result.get_score_tracker(&Instrument::Kick).accuracies, vec![Accuracy::Correct], @@ -385,7 +375,7 @@ mod tests { let mut desired_hits = Voices::new(); desired_hits.toggle_beat(Instrument::Kick, 0.0); - let result = compute_last_loop_summary(&user_hits, &desired_hits, 0.0); + let result = compute_last_loop_summary(&user_hits, &desired_hits); assert_eq!( result.get_score_tracker(&Instrument::Kick).accuracies, vec![Accuracy::Miss], diff --git a/src/ui.rs b/src/ui.rs index 76d8cb5..91a9edc 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -31,14 +31,15 @@ impl UI { } } -pub fn get_hits_from_nth_loop(user_hits: &Vec, desired_loop_idx: usize) -> Vec { +pub fn get_hits_from_nth_loop(user_hits: &[UserHit], desired_loop_idx: usize) -> Vec { let last_loop_hits: Vec = user_hits .iter() .filter(|hit| { // include hits from just before start of loop (back to 0 - MISS), since those could be early or on-time hits let loop_num_for_hit = ((hit.clock_tick + MISS_MARGIN) / BEATS_PER_LOOP) as usize; loop_num_for_hit == desired_loop_idx - }).cloned() + }) + .cloned() .collect::>(); last_loop_hits }