Some cleanup, also prohibit applying the incorrect instrument type to a given channel

src/reskit/soundtrack/engines/echo/engine.rs

@@ -1,6 +1,6 @@
-use std::{collections::HashMap, error::Error, cmp::{min, max}};
+use std::{collections::{HashMap, HashSet}, error::Error, cmp::{min, max}};
 use linked_hash_set::LinkedHashSet;
-use crate::reskit::{soundtrack::types::{PatternRow, Note, Channel, OctaveFrequency, Effect, Instrument, NoiseType, DcsgChannelMode}, utility::print_warning};
+use crate::reskit::{soundtrack::types::{PatternRow, Note, Channel, OctaveFrequency, Effect, Instrument, NoiseType, DcsgChannelMode, InstrumentType}, utility::print_warning};
 
 // https://github.com/sikthehedgehog/Echo/blob/master/doc/esf.txt
 const ESF_NOTE_ON: u8 = 0x00;
@@ -59,6 +59,12 @@ const ESF_SEMITONE_A: u8        = 9;
 const ESF_SEMITONE_A_SHARP: u8  = 10;
 const ESF_SEMITONE_B: u8        = 11;
 
+struct InstrumentSet {
+    fm_ids: HashSet<u16>,
+    psg_ids: HashSet<u16>,
+    default_psg_id: Option<u16>
+}
+
 pub type EchoEvent = Vec<u8>;
 
 pub trait EchoFormat {
@@ -69,7 +75,7 @@ pub trait EchoFormat {
 
 }
 
-pub fn get_semitone( note: &Note ) -> Result<u8, Box<dyn Error>> {
+fn get_semitone( note: &Note ) -> Result<u8, Box<dyn Error>> {
     Ok( match note {
         Note::NoteOff => return Err( "internal error: attempted to get semitone for a Note::NoteOff" )?,
         Note::C(_) => ESF_SEMITONE_C,
@@ -90,7 +96,7 @@ pub fn get_semitone( note: &Note ) -> Result<u8, Box<dyn Error>> {
 /**
  * Notes are used to index PCM sample playback. C = 1, etc.
  */
-pub fn get_pcm_index( note: &Note ) -> Result<u8, Box<dyn Error>> {
+fn get_pcm_index( note: &Note ) -> Result<u8, Box<dyn Error>> {
     Ok( match note {
         Note::NoteOff => return Err( "internal error: attempted to get pcm index for a Note::NoteOff" )?,
         Note::C(_) => 0,
@@ -116,7 +122,7 @@ pub fn get_pcm_index( note: &Note ) -> Result<u8, Box<dyn Error>> {
     D  - 722 | F# - 910 | A# - 1146
     D# - 765 | G  - 964 | B  - 1214
 */
-pub fn get_fm_semitone_frequency( semitone: u8 ) -> Result<u16, Box<dyn Error>> {
+fn get_fm_semitone_frequency( semitone: u8 ) -> Result<u16, Box<dyn Error>> {
     Ok( match semitone {
         ESF_SEMITONE_C => 644,
         ESF_SEMITONE_C_SHARP => 681,
@@ -152,7 +158,7 @@ pub fn get_fm_semitone_frequency( semitone: u8 ) -> Result<u16, Box<dyn Error>>
     A# | 477 | 238 | 119 |  59 |  29 |  14
     B  | 450 | 225 | 112 |  56 |  28 |  14
  */
-pub fn get_psg_semitone_frequency( octave: u8, semitone: u8 ) -> Result<u16, Box<dyn Error>> {
+fn get_psg_semitone_frequency( octave: u8, semitone: u8 ) -> Result<u16, Box<dyn Error>> {
     Ok( match semitone {
         ESF_SEMITONE_C => match octave { 0 => 851, 1 => 425, 2 => 212, 3 => 106, 4 => 53, 5 | _ => 26 },
         ESF_SEMITONE_C_SHARP => match octave { 0 => 803, 1 => 401, 2 => 200, 3 => 100, 4 => 50, 5 | _ => 25 },
@@ -173,7 +179,7 @@ pub fn get_psg_semitone_frequency( octave: u8, semitone: u8 ) -> Result<u16, Box
 /**
  * Convenience method to get the octave of a given PSG frequency
  */
-pub fn get_psg_octave_from_frequency( frequency: i16 ) -> u8 {
+fn get_psg_octave_from_frequency( frequency: i16 ) -> u8 {
     if frequency > 425 {
         0
     } else if frequency > 212 && frequency <= 425 {
@@ -192,7 +198,7 @@ pub fn get_psg_octave_from_frequency( frequency: i16 ) -> u8 {
 /**
  * Get the setting for the noise generator based on the active noise mode effect.
  */
-pub fn get_noise_generator_setting( note: &Note, noise_mode: Option<&Effect> ) -> Result<Option<u8>, Box<dyn Error>> {
+fn get_noise_generator_setting( note: &Note, noise_mode: Option<&Effect> ) -> Result<Option<u8>, Box<dyn Error>> {
     Ok(
         if let Some( noise_mode ) = noise_mode {
             if let Effect::DcsgNoiseMode { mode, noise_type } = noise_mode {
@@ -230,15 +236,15 @@ pub fn get_noise_generator_setting( note: &Note, noise_mode: Option<&Effect> ) -
     )
 }
 
-pub fn get_fm_note_byte( octave: u8, semitone: u8 ) -> u8 {
+fn get_fm_note_byte( octave: u8, semitone: u8 ) -> u8 {
     32 * octave + 2 * semitone + 1
 }
 
-pub fn get_psg_note_byte( octave: u8, semitone: u8 ) -> u8 {
+fn get_psg_note_byte( octave: u8, semitone: u8 ) -> u8 {
     24 * octave + 2 * semitone
 }
 
-pub fn get_note( note: &Note, channel: u8 ) -> Result<u8, Box<dyn Error>> {
+fn get_note( note: &Note, channel: u8 ) -> Result<u8, Box<dyn Error>> {
     Ok( match channel {
         ESF_FM_1..=ESF_FM_3 | ESF_FM_4..=ESF_FM_6 => get_fm_note_byte( note.get_octave()?, get_semitone( note )? ),
         ESF_PSG_1..=ESF_PSG_4 => get_psg_note_byte( note.get_octave()?, get_semitone( note )? ),
@@ -246,7 +252,7 @@ pub fn get_note( note: &Note, channel: u8 ) -> Result<u8, Box<dyn Error>> {
     } )
 }
 
-pub fn get_volume( channel: u8, volume: u8 ) -> Result<u8, Box<dyn Error>> {
+fn get_volume( channel: u8, volume: u8 ) -> Result<u8, Box<dyn Error>> {
     Ok( match channel {
         ESF_FM_1..=ESF_FM_3 | ESF_FM_4..=ESF_FM_6 => 0x7F - volume,
         ESF_PSG_1..=ESF_PSG_4 => 0x0F - volume,
@@ -257,7 +263,7 @@ pub fn get_volume( channel: u8, volume: u8 ) -> Result<u8, Box<dyn Error>> {
 /**
  * Generate an echo delay event. "delay" is the amount to wait, in 1/60 of a second ticks.
  */
-pub fn get_delay( delay: u8 ) -> Result<EchoEvent, Box<dyn Error>> {
+fn get_delay( delay: u8 ) -> Result<EchoEvent, Box<dyn Error>> {
     let events: EchoEvent = if delay == 0 {
         // No delay to generate! Probably should be an error
         vec![]
@@ -274,7 +280,7 @@ pub fn get_delay( delay: u8 ) -> Result<EchoEvent, Box<dyn Error>> {
  * For a specific row and channel, get the events this row and channel contribute to the stream.
  * While doing so, update the state of the channel for future event generation.
  */
-pub fn get_events_for_channel( channels: &mut [Channel], active_channel: usize, row: &PatternRow, pcm_offset: u8, default_psg_instr_index: Option<u16> ) -> Result<Vec<EchoEvent>, Box<dyn Error>> {
+fn get_events_for_channel( channels: &mut [Channel], active_channel: usize, row: &PatternRow, pcm_offset: u8, instrument_set: &InstrumentSet ) -> Result<Vec<EchoEvent>, Box<dyn Error>> {
     let mut events: Vec<EchoEvent> = Vec::new();
 
     // Adjust volume
@@ -287,10 +293,25 @@ pub fn get_events_for_channel( channels: &mut [Channel], active_channel: usize,
 
     // Set the instrument
     if let Some( instrument ) = &row.instrument_index {
+        // Validate the instrument is applicable to the current channel
+        let correct_type = match channels[ active_channel ].id {
+            ESF_FM_1..=ESF_FM_3 | ESF_FM_4..=ESF_FM_6 => instrument_set.fm_ids.contains( instrument ),
+            ESF_FM_6_PCM => {
+                print_warning( "attempted to set instrument on FM6 when it is in pcm mode. this is valid, but it won't do anything until the next time it is set back to fm mode." );
+                instrument_set.fm_ids.contains( instrument )
+            },
+            ESF_PSG_1..=ESF_PSG_4 => instrument_set.psg_ids.contains( instrument ),
+            invalid_channel => return Err( format!( "internal error: get_events_for_channel: invalid channel {:#04X}", invalid_channel ) )?
+        };
+
         // Do not set the same instrument if it was already set before
         if &row.instrument_index != &channels[ active_channel ].active_instrument {
-            events.push( vec![ ESF_SET_INSTRUMENT | channels[ active_channel ].id, *instrument as u8 ] );
+            if correct_type {
-            channels[ active_channel ].active_instrument = Some( *instrument );
+                events.push( vec![ ESF_SET_INSTRUMENT | channels[ active_channel ].id, *instrument as u8 ] );
+                channels[ active_channel ].active_instrument = Some( *instrument );
+            } else {
+                print_warning( "attempted to set an fm instrument on a psg channel, or vice versa. the instrument was not set - your soundtrack may sound different than expected." );
+            }
         }
     } else {
         // This is SN1, SN2, SN3, or SN4 with no active instrument
@@ -298,7 +319,7 @@ pub fn get_events_for_channel( channels: &mut [Channel], active_channel: usize,
         if channels[ active_channel ].active_instrument.is_none() && ( ESF_PSG_1..=ESF_PSG_4 ).contains( &channels[ active_channel ].id ) {
             if row.note.is_some() {
                 // Set "__reskit_default_psg_instrument"
-                let instrument = default_psg_instr_index.ok_or( "internal error: no default psg instrument to apply" )?;
+                let instrument = instrument_set.default_psg_id.ok_or( "internal error: no default psg instrument to apply" )?;
 
                 events.push( vec![ ESF_SET_INSTRUMENT | channels[ active_channel ].id, instrument as u8 ] );
                 channels[ active_channel ].active_instrument = Some( instrument );
@@ -383,7 +404,7 @@ pub fn get_events_for_channel( channels: &mut [Channel], active_channel: usize,
  * Add effects from a given row to a particular channel and perform activities due on insert. This may generate Echo
  * events that should be applied to the stream.
  */
-pub fn apply_effects_to_channel( channel: &mut Channel, effects: &LinkedHashSet<Effect> ) -> Result<Vec<EchoEvent>, Box<dyn Error>> {
+fn apply_effects_to_channel( channel: &mut Channel, effects: &LinkedHashSet<Effect> ) -> Result<Vec<EchoEvent>, Box<dyn Error>> {
     let mut events: Vec<EchoEvent> = Vec::new();
 
     for effect in effects {
@@ -771,7 +792,9 @@ pub fn get_events_for_row( channels: &mut [Channel], instruments: &Vec<Instrumen
     let mut events: Vec<EchoEvent> = Vec::new();
 
     let mut index = 0;
-    let default_psg_instr_index: Option<u16> = instruments.iter().find_map( | item | {
+
+    // Find the index of the default psg instrument, if it exists
+    let default_psg_id: Option<u16> = instruments.iter().find_map( | item | {
         if item.name == "__reskit_default_psg_instrument" {
             Some( index )
         } else {
@@ -780,13 +803,44 @@ pub fn get_events_for_row( channels: &mut [Channel], instruments: &Vec<Instrumen
         }
     } );
 
+    // Generate InstrumentSet which is used to validate when instruments are set on channels
+    index = 0;
+    let fm_ids = instruments.iter()
+                            .filter_map( | instrument | if matches!( instrument.instrument_type, InstrumentType::Fm2612( _ ) ) {
+                                let result = index;
+                                index += 1;
+                                Some( result )
+                            } else {
+                                index += 1;
+                                None
+                            } )
+                            .collect();
+
+    index = 0;
+    let psg_ids = instruments.iter()
+                            .filter_map( | instrument | if matches!( instrument.instrument_type, InstrumentType::PsgDcsg( _ ) ) {
+                                let result = index;
+                                index += 1;
+                                Some( result )
+                            } else {
+                                index += 1;
+                                None
+                            } )
+                            .collect();
+
+    let instrument_set = InstrumentSet {
+        fm_ids,
+        psg_ids,
+        default_psg_id
+    };
+
     // Get events for each subrow (part of the total row for each channel)
     for i in 0..channels.len() {
         // Apply effects to channel's current state
         events.extend( apply_effects_to_channel( &mut channels[ i ], &subrows[ i ].effects )? );
 
         // Get the ESF events for this channel's part of the row
-        events.extend( get_events_for_channel( channels, i, subrows[ i ], pcm_offset, default_psg_instr_index )? );
+        events.extend( get_events_for_channel( channels, i, subrows[ i ], pcm_offset, &instrument_set )? );
     }
 
     Ok( get_delays( events, channels, ticks_to_wait )? )