From 0ef415c1c238462b1893510024eebd17be05991b Mon Sep 17 00:00:00 2001 From: nenikitov Date: Wed, 9 Oct 2024 00:49:37 -0400 Subject: [PATCH] refactor: clippy warnings --- engine/src/asset/sound/dat/finetune.rs | 15 ++---- engine/src/asset/sound/dat/mixer.rs | 53 +++++++++----------- engine/src/asset/sound/dat/pattern_effect.rs | 20 +++++--- engine/src/asset/sound/dat/pattern_event.rs | 25 ++++++--- engine/src/asset/sound/dat/t_instrument.rs | 15 +++--- engine/src/asset/sound/dat/t_song.rs | 43 ++++++++-------- engine/src/asset/sound/sample.rs | 10 ++-- engine/src/lib.rs | 2 + 8 files changed, 91 insertions(+), 92 deletions(-) diff --git a/engine/src/asset/sound/dat/finetune.rs b/engine/src/asset/sound/dat/finetune.rs index e11c44c..1b6a53d 100644 --- a/engine/src/asset/sound/dat/finetune.rs +++ b/engine/src/asset/sound/dat/finetune.rs @@ -1,9 +1,6 @@ -use std::{ - cmp::Ordering, - ops::{Add, AddAssign, Neg, Sub}, -}; +use std::ops::{Add, AddAssign, Neg, Sub}; -#[derive(Debug, Clone, Copy, PartialEq, Eq, Ord)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] pub struct FineTune { cents: i32, } @@ -51,7 +48,7 @@ impl Add for FineTune { impl AddAssign for FineTune { fn add_assign(&mut self, rhs: Self) { - self.cents = self.cents.saturating_add(rhs.cents) + self.cents = self.cents.saturating_add(rhs.cents); } } @@ -71,12 +68,6 @@ impl Neg for FineTune { } } -impl PartialOrd for FineTune { - fn partial_cmp(&self, other: &Self) -> Option { - self.cents.partial_cmp(&other.cents) - } -} - #[cfg(test)] mod tests { use assert_approx_eq::assert_approx_eq; diff --git a/engine/src/asset/sound/dat/mixer.rs b/engine/src/asset/sound/dat/mixer.rs index c007e46..1fa194a 100644 --- a/engine/src/asset/sound/dat/mixer.rs +++ b/engine/src/asset/sound/dat/mixer.rs @@ -85,7 +85,7 @@ impl PlayerChannel { envelope .volume_beginning() .get(self.pos_volume_envelope) - .cloned() + .copied() .unwrap_or(envelope.volume_loop()) } else { envelope @@ -94,7 +94,7 @@ impl PlayerChannel { self.pos_volume_envelope .saturating_sub(envelope.volume_beginning().len()), ) - .cloned() + .copied() .unwrap_or(0.) } } @@ -112,21 +112,19 @@ impl PlayerChannel { 0. }; - let current_sample = if let Some((previous, position)) = &mut self.previous { + if let Some((previous, position)) = &mut self.previous { let factor = (*position / Self::SAMPLE_BLEND) as f32; let previous_sample = previous.generate_sample(step); *position += step; if *position >= Self::SAMPLE_BLEND { - self.previous = None + self.previous = None; } previous_sample + factor * (current_sample - previous_sample) } else { current_sample - }; - - current_sample + } } fn trigger_note(&mut self) { @@ -139,8 +137,8 @@ impl PlayerChannel { self.pos_reset(); } - fn change_instrument(&mut self, instrument: Option>) { - if let Some(instrument) = instrument { + fn change_instrument(&mut self, instrument: PatternEventInstrument) { + if let PatternEventInstrument::Instrument(instrument) = instrument { self.instrument = Some(instrument); } @@ -164,7 +162,8 @@ impl PlayerChannel { self.note.finetune = Some(note); self.note.finetune_initial = Some(note); self.note.on = true; - self.sample = instrument.samples[note.note() as usize].clone(); + self.sample + .clone_from(&instrument.samples[note.note() as usize]); } } } else { @@ -204,15 +203,12 @@ impl PlayerChannel { fn advance_envelopes(&mut self) { if let Some(instrument) = &self.instrument && let TInstrumentVolume::Envelope(envelope) = &instrument.volume + && (!self.note.on + || envelope + .sustain + .map_or(true, |s| self.pos_volume_envelope < s)) { - if !self.note.on - || (self.note.on - && envelope - .sustain - .map_or(true, |s| self.pos_volume_envelope < s)) - { - self.pos_volume_envelope += 1; - } + self.pos_volume_envelope += 1; } } @@ -322,11 +318,12 @@ impl<'a> Player<'a> { self.row(); } - for channel in self.channels.iter_mut() { + for channel in &mut self.channels { channel.advance_envelopes(); for effect in channel.effects.iter().flatten() { use PatternEffect as E; + match *effect { // Tick effects E::Volume(Volume::Slide(Some(volume))) => { @@ -364,10 +361,8 @@ impl<'a> Player<'a> { E::Speed(..) | E::PatternBreak | E::PatternJump(..) - | E::Volume(Volume::Set(..)) - | E::Volume(Volume::Bump { .. }) - | E::Porta(Porta::Tone(..)) - | E::Porta(Porta::Bump { .. }) + | E::Volume(Volume::Set(..) | Volume::Bump { .. }) + | E::Porta(Porta::Tone(..) | Porta::Bump { .. }) | E::GlobalVolume(..) | E::SampleOffset(..) | E::PlaybackDirection(..) => {} @@ -445,9 +440,9 @@ impl<'a> Player<'a> { .enumerate() .filter_map(|(i, e)| e.map(|e| (i, e))) { - channel.change_effect(i, effect.clone()); - use PatternEffect as E; + + channel.change_effect(i, effect); match channel.effects[i].expect("`change_effect` sets the effect") { // Init effects E::Speed(Speed::Bpm(bpm)) => { @@ -485,7 +480,7 @@ impl<'a> Player<'a> { if let Some(sample) = &channel.sample && direction == PlaybackDirection::Backwards { - channel.pos_sample = sample.buffer.len_seconds() as f64 + channel.pos_sample = sample.buffer.len_seconds(); } } E::SampleOffset(Some(offset)) => { @@ -497,13 +492,11 @@ impl<'a> Player<'a> { } // Noops - no init E::Volume(Volume::Slide(..)) - | E::Porta(Porta::Tone(..)) - | E::Porta(Porta::Slide { .. }) + | E::Porta(Porta::Tone(..) | Porta::Slide { .. }) | E::RetriggerNote(..) => {} // Unreachable because memory has to be initialized E::Volume(Volume::Bump { volume: None, .. }) - | E::Porta(Porta::Tone(None)) - | E::Porta(Porta::Bump { finetune: None, .. }) + | E::Porta(Porta::Tone(None) | Porta::Bump { finetune: None, .. }) | E::SampleOffset(None) => { unreachable!("Effects should have their memory initialized") } diff --git a/engine/src/asset/sound/dat/pattern_effect.rs b/engine/src/asset/sound/dat/pattern_effect.rs index aa6c882..0c830de 100644 --- a/engine/src/asset/sound/dat/pattern_effect.rs +++ b/engine/src/asset/sound/dat/pattern_effect.rs @@ -115,15 +115,19 @@ impl PatternEffect { self.memory_key().is_some() } + #[rustfmt::skip] pub fn is_empty(&self) -> bool { + use PatternEffect as E; + matches!( self, - PatternEffect::Porta(Porta::Tone(None)) - | PatternEffect::Porta(Porta::Slide { finetune: None, .. }) - | PatternEffect::Porta(Porta::Bump { finetune: None, .. }) - | PatternEffect::Volume(Volume::Slide(None)) - | PatternEffect::Volume(Volume::Bump { volume: None, .. }) - | PatternEffect::SampleOffset(None) + E::Porta( + Porta::Tone(None) + | Porta::Slide { finetune: None, .. } + | Porta::Bump { finetune: None, .. } + ) + | E::Volume(Volume::Slide(None) | Volume::Bump { volume: None, .. }) + | E::SampleOffset(None) ) } } @@ -135,10 +139,10 @@ impl AssetParser for Option { fn parser(should_parse: Self::Context<'_>) -> impl Fn(Input) -> Result { move |input| { + use PatternEffect as E; + let (input, kind) = number::le_u8(input)?; let (input, value) = number::le_u8(input)?; - - use PatternEffect as E; Ok(( input, should_parse.then(|| match kind { diff --git a/engine/src/asset/sound/dat/pattern_event.rs b/engine/src/asset/sound/dat/pattern_event.rs index 0d93023..f159e3d 100644 --- a/engine/src/asset/sound/dat/pattern_event.rs +++ b/engine/src/asset/sound/dat/pattern_event.rs @@ -63,15 +63,15 @@ impl AssetParser for PatternEventFlags { Ok(( input, // TODO(nenikitov): Should be a `Result` - PatternEventFlags::from_bits(flags).expect(&format!( - "PatternEvent flags should be valid: received: {flags:b}" - )), + PatternEventFlags::from_bits(flags).unwrap_or_else(|| { + panic!("PatternEvent flags should be valid: received: {flags:b}") + }), )) } } } -impl AssetParser for Option>> { +impl AssetParser for Option { type Output = Self; type Context<'ctx> = (bool, &'ctx [Rc]); @@ -84,7 +84,10 @@ impl AssetParser for Option>> { Ok(( input, - should_parse.then(|| instruments.get(instrument as usize).map(Rc::clone)), + should_parse.then(|| match instruments.get(instrument as usize) { + Some(instrument) => PatternEventInstrument::Instrument(instrument.clone()), + None => PatternEventInstrument::Ghost, + }), )) } } @@ -125,10 +128,18 @@ impl AssetParser for Option { } } +#[derive(Default, Debug, Clone)] +pub enum PatternEventInstrument { + #[default] + Ghost, + Instrument(Rc), +} + #[derive(Default, Debug)] pub struct PatternEvent { pub note: Option, - pub instrument: Option>>, + // Option> + pub instrument: Option, pub volume: Option, pub effects: [Option; 2], } @@ -158,7 +169,7 @@ impl AssetParser for PatternEvent { flags.contains(PatternEventFlags::ChangeNote), )(input)?; - let (input, instrument) = >>>::parser(( + let (input, instrument) = >::parser(( (flags.contains(PatternEventFlags::ChangeInstrument)), instruments, ))(input)?; diff --git a/engine/src/asset/sound/dat/t_instrument.rs b/engine/src/asset/sound/dat/t_instrument.rs index f56e48f..eac484f 100644 --- a/engine/src/asset/sound/dat/t_instrument.rs +++ b/engine/src/asset/sound/dat/t_instrument.rs @@ -29,9 +29,9 @@ impl AssetParser for TInstrumentFlags { Ok(( input, // TODO(nenikitov): Should be a `Result` - TInstrumentFlags::from_bits(flags).expect(&format!( - "PatternEvent flags should be valid: received: {flags:b}" - )), + TInstrumentFlags::from_bits(flags).unwrap_or_else(|| { + panic!("PatternEvent flags should be valid: received: {flags:b}") + }), )) } } @@ -261,13 +261,12 @@ impl TSample { let position = Self::SAMPLE_RATE as f64 * position; let frac = position.fract() as f32; - let Some(prev) = self.normalize(position as usize) else { - return None; - }; - let next = self.normalize(position as usize + 1); + let prev = self.normalize(position as usize)?; let prev = self.buffer[prev] as f32; - let next = next.map(|next| self.buffer[next] as f32).unwrap_or(0.); + + let next = self.normalize(position as usize + 1); + let next = next.map_or(0., |next| self.buffer[next] as f32); Some((prev + frac * (next - prev)) as i16) } diff --git a/engine/src/asset/sound/dat/t_song.rs b/engine/src/asset/sound/dat/t_song.rs index c5f424c..1ab8de1 100644 --- a/engine/src/asset/sound/dat/t_song.rs +++ b/engine/src/asset/sound/dat/t_song.rs @@ -35,7 +35,7 @@ impl std::fmt::Debug for TSong { d.value_with(|f| { let mut d = f.debug_map(); for (r, row) in pattern.iter().enumerate() { - if !row.iter().any(|c| c.has_content()) { + if !row.iter().any(PatternEvent::has_content) { continue; } @@ -50,31 +50,30 @@ impl std::fmt::Debug for TSong { d.key(&format!("C 0x{e:X}")); d.value_with(|f| { let mut d = f.debug_struct("Event"); - event.note.map(|note| { - d.field_with("note", |f| { - f.write_fmt(format_args!("{:?}", note)) - }); - }); - event.volume.map(|volume| { - d.field_with("volume", |f| { - f.write_fmt(format_args!("{:?}", volume)) - }); - }); - event.instrument.as_ref().map(|instrument| { + if let Some(note) = event.note { + d.field_with("note", |f| write!(f, "{note:?}")); + } + if let Some(volume) = event.volume { + d.field_with("volume", |f| write!(f, "{volume:?}")); + } + if let Some(instrument) = &event.instrument { d.field_with("instrument", |f| match instrument { - None => f.write_fmt(format_args!("None")), - Some(instrument) => f.write_fmt(format_args!( - "Some({})", - self.instruments - .iter() - .position(|i| Rc::ptr_eq(i, instrument)) - .unwrap() - )), + PatternEventInstrument::Ghost => write!(f, "Ghost"), + PatternEventInstrument::Instrument(instrument) => { + write!( + f, + "Instrument({})", + self.instruments + .iter() + .position(|i| Rc::ptr_eq(i, instrument)) + .unwrap() + ) + } }); - }); + } if event.effects.iter().any(Option::is_some) { d.field_with("effects", |f| { - f.write_fmt(format_args!("{:?}", event.effects)) + write!(f, "{:?}", event.effects) }); } d.finish() diff --git a/engine/src/asset/sound/sample.rs b/engine/src/asset/sound/sample.rs index b2b9c79..1874940 100644 --- a/engine/src/asset/sound/sample.rs +++ b/engine/src/asset/sound/sample.rs @@ -18,7 +18,7 @@ impl AudioSamplePointFormat { pub trait AudioSamplePoint: Default + Clone + Copy { const SIZE_BYTES: usize = size_of::(); - fn into_normalized_f32(&self) -> f32; + fn into_normalized_f32(self) -> f32; fn from_normalized_f32(value: f32) -> Self; fn wave_format() -> AudioSamplePointFormat; @@ -26,11 +26,11 @@ pub trait AudioSamplePoint: Default + Clone + Copy { } impl AudioSamplePoint for i16 { - fn into_normalized_f32(&self) -> f32 { - if *self < 0 { - -(*self as f32 / Self::MIN as f32) + fn into_normalized_f32(self) -> f32 { + if self < 0 { + -(self as f32 / Self::MIN as f32) } else { - (*self as f32 / Self::MAX as f32) + (self as f32 / Self::MAX as f32) } } diff --git a/engine/src/lib.rs b/engine/src/lib.rs index d8d7057..a6f1160 100644 --- a/engine/src/lib.rs +++ b/engine/src/lib.rs @@ -9,6 +9,8 @@ clippy::cast_lossless, clippy::cast_possible_truncation, clippy::cast_precision_loss, + clippy::cast_sign_loss, + clippy::missing_fields_in_debug, clippy::unreadable_literal, incomplete_features,