[SMS/GG] clean up VDP/PSG version determination code

this pushes the determination logic down into the backend core, which also fixes a bug where the NTSC/PAL setting was ignored when loading from a .zip or .7z file containing a .sms file

backend/smsgg-core/src/api.rs

@@ -60,6 +60,20 @@ impl DerefMut for FrameBuffer {
     }
 }
 
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum SmsGgHardware {
+    MasterSystem,
+    GameGear,
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Default, EnumDisplay, EnumFromStr)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
+pub enum SmsModel {
+    Sms1,
+    #[default]
+    Sms2,
+}
+
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Default, Encode, Decode, EnumDisplay, EnumFromStr)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub enum SmsRegion {
@@ -70,8 +84,10 @@ pub enum SmsRegion {
 
 #[derive(Debug, Clone, Copy)]
 pub struct SmsGgEmulatorConfig {
-    pub vdp_version: VdpVersion,
-    pub psg_version: PsgVersion,
+    pub hardware: SmsGgHardware,
+    pub sms_timing_mode: TimingMode,
+    pub sms_model: SmsModel,
+    pub forced_psg_version: Option<PsgVersion>,
     pub pixel_aspect_ratio: Option<PixelAspectRatio>,
     pub remove_sprite_limit: bool,
     pub sms_region: SmsRegion,
@@ -114,9 +130,15 @@ impl SmsGgEmulator {
     ) -> Self {
         let cartridge_ram = save_writer.load_bytes("sav").ok();
 
+        let vdp_version = determine_vdp_version(&config);
+        let psg_version = determine_psg_version(&config);
+
+        log::info!("VDP version: {vdp_version:?}");
+        log::info!("PSG version: {psg_version:?}");
+
         let memory = Memory::new(rom, cartridge_ram);
-        let vdp = Vdp::new(config.vdp_version, config.remove_sprite_limit);
-        let psg = Psg::new(config.psg_version);
+        let vdp = Vdp::new(vdp_version, config.remove_sprite_limit);
+        let psg = Psg::new(psg_version);
         let input = InputState::new(config.sms_region);
 
         let mut z80 = Z80::new();
@@ -130,7 +152,7 @@ impl SmsGgEmulator {
             memory,
             z80,
             vdp,
-            vdp_version: config.vdp_version,
+            vdp_version,
             pixel_aspect_ratio: config.pixel_aspect_ratio,
             psg,
             ym2413,
@@ -147,6 +169,15 @@ impl SmsGgEmulator {
         }
     }
 
+    #[must_use]
+    pub fn hardware(&self) -> SmsGgHardware {
+        if self.vdp_version.is_master_system() {
+            SmsGgHardware::MasterSystem
+        } else {
+            SmsGgHardware::GameGear
+        }
+    }
+
     #[must_use]
     pub fn vdp_version(&self) -> VdpVersion {
         self.vdp_version
@@ -201,6 +232,31 @@ fn init_z80(z80: &mut Z80) {
     z80.set_interrupt_mode(InterruptMode::Mode1);
 }
 
+fn determine_vdp_version(config: &SmsGgEmulatorConfig) -> VdpVersion {
+    match (config.hardware, config.sms_timing_mode, config.sms_model) {
+        (SmsGgHardware::MasterSystem, TimingMode::Ntsc, SmsModel::Sms1) => {
+            VdpVersion::NtscMasterSystem1
+        }
+        (SmsGgHardware::MasterSystem, TimingMode::Ntsc, SmsModel::Sms2) => {
+            VdpVersion::NtscMasterSystem2
+        }
+        (SmsGgHardware::MasterSystem, TimingMode::Pal, SmsModel::Sms1) => {
+            VdpVersion::PalMasterSystem1
+        }
+        (SmsGgHardware::MasterSystem, TimingMode::Pal, SmsModel::Sms2) => {
+            VdpVersion::PalMasterSystem2
+        }
+        (SmsGgHardware::GameGear, _, _) => VdpVersion::GameGear,
+    }
+}
+
+fn determine_psg_version(config: &SmsGgEmulatorConfig) -> PsgVersion {
+    config.forced_psg_version.unwrap_or(match config.hardware {
+        SmsGgHardware::MasterSystem => PsgVersion::MasterSystem2,
+        SmsGgHardware::GameGear => PsgVersion::Standard,
+    })
+}
+
 impl EmulatorTrait for SmsGgEmulator {
     type Inputs = SmsGgInputs;
     type Config = SmsGgEmulatorConfig;
@@ -308,9 +364,11 @@ impl EmulatorTrait for SmsGgEmulator {
     }
 
     fn reload_config(&mut self, config: &Self::Config) {
-        self.vdp_version = config.vdp_version;
-        self.vdp.set_version(config.vdp_version);
-        self.psg.set_version(config.psg_version);
+        self.vdp_version = determine_vdp_version(config);
+        self.vdp.set_version(self.vdp_version);
+
+        self.psg.set_version(determine_psg_version(config));
+
         self.pixel_aspect_ratio = config.pixel_aspect_ratio;
         self.vdp.set_remove_sprite_limit(config.remove_sprite_limit);
         self.input.set_region(config.sms_region);

backend/smsgg-core/src/lib.rs

@@ -6,7 +6,9 @@ mod memory;
 pub mod psg;
 mod vdp;
 
-pub use api::{SmsGgEmulator, SmsGgEmulatorConfig, SmsGgError, SmsGgResult, SmsRegion};
+pub use api::{
+    SmsGgEmulator, SmsGgEmulatorConfig, SmsGgError, SmsGgHardware, SmsGgResult, SmsModel, SmsRegion,
+};
 pub use input::{SmsGgButton, SmsGgInputs, SmsGgJoypadState};
 pub use vdp::{VdpVersion, gg_color_to_rgb, sms_color_to_rgb};
 

frontend/jgenesis-cli/src/main.rs

@@ -6,7 +6,6 @@ use gb_core::api::{GbAspectRatio, GbPalette, GbcColorCorrection};
 use genesis_core::{GenesisAspectRatio, GenesisControllerType, GenesisRegion};
 use jgenesis_common::frontend::{EmulatorTrait, TimingMode};
 use jgenesis_native_config::AppConfig;
-use jgenesis_native_config::smsgg::SmsModel;
 use jgenesis_native_driver::config::input::{NesControllerType, SnesControllerType};
 use jgenesis_native_driver::config::{GgAspectRatio, SmsAspectRatio};
 use jgenesis_native_driver::input::MappableInputs;
@@ -17,8 +16,8 @@ use jgenesis_renderer::config::{
 };
 use nes_core::api::NesAspectRatio;
 use s32x_core::api::S32XVideoOut;
-use smsgg_core::SmsRegion;
 use smsgg_core::psg::PsgVersion;
+use smsgg_core::{SmsModel, SmsRegion};
 use snes_core::api::{AudioInterpolationMode, SnesAspectRatio};
 use std::ffi::OsStr;
 use std::fs;

frontend/jgenesis-gui/src/app/smsgg.rs

@@ -4,10 +4,9 @@ use crate::app::{App, OpenWindow};
 use crate::emuthread::EmuThreadStatus;
 use egui::{Context, Window};
 use jgenesis_common::frontend::TimingMode;
-use jgenesis_native_config::smsgg::SmsModel;
 use jgenesis_native_driver::config::{GgAspectRatio, SmsAspectRatio};
-use smsgg_core::SmsRegion;
 use smsgg_core::psg::PsgVersion;
+use smsgg_core::{SmsModel, SmsRegion};
 
 impl App {
     pub(super) fn render_smsgg_general_settings(&mut self, ctx: &Context) {

frontend/jgenesis-native-config/src/smsgg.rs

@@ -1,21 +1,9 @@
 use crate::AppConfig;
 use jgenesis_common::frontend::TimingMode;
 use jgenesis_native_driver::config::{GgAspectRatio, SmsAspectRatio, SmsGgConfig};
-use jgenesis_proc_macros::{EnumDisplay, EnumFromStr};
 use serde::{Deserialize, Serialize};
 use smsgg_core::psg::PsgVersion;
-use smsgg_core::{SmsRegion, VdpVersion};
-use std::ffi::OsStr;
-use std::path::Path;
-
-#[derive(
-    Debug, Clone, Copy, PartialEq, Eq, Default, Serialize, Deserialize, EnumDisplay, EnumFromStr,
-)]
-pub enum SmsModel {
-    Sms1,
-    #[default]
-    Sms2,
-}
+use smsgg_core::{SmsModel, SmsRegion};
 
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
 pub struct SmsGgAppConfig {
@@ -55,25 +43,15 @@ impl Default for SmsGgAppConfig {
 impl AppConfig {
     #[must_use]
     pub fn smsgg_config(&self, path: String) -> Box<SmsGgConfig> {
-        let vdp_version = if Path::new(&path).extension().and_then(OsStr::to_str) == Some("sms") {
-            match (self.smsgg.sms_timing_mode, self.smsgg.sms_model) {
-                (TimingMode::Ntsc, SmsModel::Sms2) => Some(VdpVersion::NtscMasterSystem2),
-                (TimingMode::Pal, SmsModel::Sms2) => Some(VdpVersion::PalMasterSystem2),
-                (TimingMode::Ntsc, SmsModel::Sms1) => Some(VdpVersion::NtscMasterSystem1),
-                (TimingMode::Pal, SmsModel::Sms1) => Some(VdpVersion::PalMasterSystem1),
-            }
-        } else {
-            None
-        };
-
         Box::new(SmsGgConfig {
             common: self.common_config(
                 path,
                 self.inputs.smsgg_keyboard.clone(),
                 self.inputs.smsgg_joystick.clone(),
             ),
-            vdp_version,
-            psg_version: self.smsgg.psg_version,
+            sms_timing_mode: self.smsgg.sms_timing_mode,
+            sms_model: self.smsgg.sms_model,
+            forced_psg_version: self.smsgg.psg_version,
             remove_sprite_limit: self.smsgg.remove_sprite_limit,
             sms_aspect_ratio: self.smsgg.sms_aspect_ratio,
             gg_aspect_ratio: self.smsgg.gg_aspect_ratio,

frontend/jgenesis-native-driver/src/config.rs

@@ -19,7 +19,7 @@ use s32x_core::api::{S32XVideoOut, Sega32XEmulatorConfig};
 use segacd_core::api::SegaCdEmulatorConfig;
 use serde::{Deserialize, Serialize};
 use smsgg_core::psg::PsgVersion;
-use smsgg_core::{SmsGgEmulatorConfig, SmsRegion, VdpVersion};
+use smsgg_core::{SmsGgEmulatorConfig, SmsGgHardware, SmsModel, SmsRegion};
 use snes_core::api::{
     AudioInterpolationMode, CoprocessorRomFn, CoprocessorRoms, SnesAspectRatio, SnesEmulatorConfig,
 };
@@ -145,8 +145,9 @@ impl<KeyboardConfig, JoystickConfig> CommonConfig<KeyboardConfig, JoystickConfig
 pub struct SmsGgConfig {
     #[indent_nested]
     pub common: CommonConfig<SmsGgInputConfig<KeyboardInput>, SmsGgInputConfig<JoystickInput>>,
-    pub vdp_version: Option<VdpVersion>,
-    pub psg_version: Option<PsgVersion>,
+    pub sms_timing_mode: TimingMode,
+    pub sms_model: SmsModel,
+    pub forced_psg_version: Option<PsgVersion>,
     pub remove_sprite_limit: bool,
     pub sms_aspect_ratio: SmsAspectRatio,
     pub gg_aspect_ratio: GgAspectRatio,
@@ -158,19 +159,16 @@ pub struct SmsGgConfig {
 }
 
 impl SmsGgConfig {
-    pub(crate) fn to_emulator_config(
-        &self,
-        vdp_version: VdpVersion,
-        psg_version: PsgVersion,
-    ) -> SmsGgEmulatorConfig {
-        let pixel_aspect_ratio = if vdp_version.is_master_system() {
-            self.sms_aspect_ratio.to_pixel_aspect_ratio()
-        } else {
-            self.gg_aspect_ratio.to_pixel_aspect_ratio()
+    pub(crate) fn to_emulator_config(&self, hardware: SmsGgHardware) -> SmsGgEmulatorConfig {
+        let pixel_aspect_ratio = match hardware {
+            SmsGgHardware::MasterSystem => self.sms_aspect_ratio.to_pixel_aspect_ratio(),
+            SmsGgHardware::GameGear => self.gg_aspect_ratio.to_pixel_aspect_ratio(),
         };
         SmsGgEmulatorConfig {
-            vdp_version,
-            psg_version,
+            hardware,
+            sms_timing_mode: self.sms_timing_mode,
+            sms_model: self.sms_model,
+            forced_psg_version: self.forced_psg_version,
             pixel_aspect_ratio,
             remove_sprite_limit: self.remove_sprite_limit,
             sms_region: self.sms_region,
@@ -182,33 +180,14 @@ impl SmsGgConfig {
     }
 }
 
-pub(crate) fn default_vdp_version_for_ext(file_ext: &str) -> VdpVersion {
-    match file_ext {
-        "sms" => VdpVersion::NtscMasterSystem2,
-        "gg" => VdpVersion::GameGear,
-        _ => {
-            log::warn!("Unknown file extension {file_ext}, defaulting to NTSC SMS VDP");
-            VdpVersion::NtscMasterSystem2
-        }
-    }
-}
-
-pub(crate) fn default_psg_version_for_ext(file_ext: &str) -> PsgVersion {
-    match file_ext {
-        "sms" => PsgVersion::MasterSystem2,
-        _ => PsgVersion::Standard,
-    }
-}
-
-pub(crate) fn default_smsgg_window_size(vdp_version: VdpVersion) -> WindowSize {
-    match vdp_version {
-        VdpVersion::NtscMasterSystem1 | VdpVersion::NtscMasterSystem2 => {
-            WindowSize { width: 940, height: 720 }
-        }
-        VdpVersion::PalMasterSystem1 | VdpVersion::PalMasterSystem2 => {
-            WindowSize { width: 1056, height: 720 }
-        }
-        VdpVersion::GameGear => WindowSize { width: 576, height: 432 },
+pub(crate) fn default_smsgg_window_size(
+    hardware: SmsGgHardware,
+    sms_timing_mode: TimingMode,
+) -> WindowSize {
+    match (hardware, sms_timing_mode) {
+        (SmsGgHardware::MasterSystem, TimingMode::Ntsc) => WindowSize { width: 940, height: 720 },
+        (SmsGgHardware::MasterSystem, TimingMode::Pal) => WindowSize { width: 1056, height: 720 },
+        (SmsGgHardware::GameGear, _) => WindowSize { width: 576, height: 432 },
     }
 }
 

frontend/jgenesis-native-driver/src/mainloop/smsgg.rs

@@ -6,8 +6,7 @@ use crate::{AudioError, NativeEmulator, NativeEmulatorResult, config};
 use jgenesis_common::frontend::EmulatorTrait;
 
 use crate::config::RomReadResult;
-use smsgg_core::psg::PsgVersion;
-use smsgg_core::{SmsGgButton, SmsGgEmulator, SmsGgEmulatorConfig, SmsGgInputs};
+use smsgg_core::{SmsGgButton, SmsGgEmulator, SmsGgEmulatorConfig, SmsGgHardware, SmsGgInputs};
 use std::path::Path;
 
 pub type NativeSmsGgEmulator =
@@ -24,16 +23,8 @@ impl NativeSmsGgEmulator {
 
         self.reload_common_config(&config.common)?;
 
-        let vdp_version = config.vdp_version.unwrap_or_else(|| self.emulator.vdp_version());
-        let psg_version = config.psg_version.unwrap_or_else(|| {
-            if vdp_version.is_master_system() {
-                PsgVersion::MasterSystem2
-            } else {
-                PsgVersion::Standard
-            }
-        });
-
-        let emulator_config = config.to_emulator_config(vdp_version, psg_version);
+        let hardware = self.emulator.hardware();
+        let emulator_config = config.to_emulator_config(hardware);
         self.emulator.reload_config(&emulator_config);
         self.config = emulator_config;
 
@@ -67,29 +58,34 @@ pub fn create_smsgg(config: Box<SmsGgConfig>) -> NativeEmulatorResult<NativeSmsG
     let save_path = rom_file_path.with_extension("sav");
     let mut save_writer = FsSaveWriter::new(save_path);
 
-    let vdp_version =
-        config.vdp_version.unwrap_or_else(|| config::default_vdp_version_for_ext(&extension));
-    let psg_version =
-        config.psg_version.unwrap_or_else(|| config::default_psg_version_for_ext(&extension));
-
-    log::info!("VDP version: {vdp_version:?}");
-    log::info!("PSG version: {psg_version:?}");
+    let hardware = hardware_for_ext(&extension);
 
     let rom_title = file_name_no_ext(rom_file_path)?;
     let window_title = format!("smsgg - {rom_title}");
 
-    let emulator_config = config.to_emulator_config(vdp_version, psg_version);
+    let emulator_config = config.to_emulator_config(hardware);
     let emulator = SmsGgEmulator::create(rom, emulator_config, &mut save_writer);
 
     NativeSmsGgEmulator::new(
         emulator,
         emulator_config,
         config.common,
-        config::default_smsgg_window_size(vdp_version),
+        config::default_smsgg_window_size(hardware, config.sms_timing_mode),
         &window_title,
         save_writer,
         save_state_path,
         basic_input_mapper_fn(&SmsGgButton::ALL),
         debug::smsgg::render_fn,
     )
 }
+
+fn hardware_for_ext(extension: &str) -> SmsGgHardware {
+    match extension.to_ascii_lowercase().as_str() {
+        "sms" => SmsGgHardware::MasterSystem,
+        "gg" => SmsGgHardware::GameGear,
+        _ => {
+            log::error!("Unrecognized file extension '{extension}', defaulting to SMS mode");
+            SmsGgHardware::MasterSystem
+        }
+    }
+}