From 4b692177aef1f7002e7bb06d5c347066ac2671d5 Mon Sep 17 00:00:00 2001 From: pkuzic Date: Fri, 25 Sep 2026 23:26:09 +0100 Subject: [PATCH] MONKEY (reviewfix): make skybox path lookup deterministic Co-Authored-By: GPT-5.6 Sol (Codex) --- crates/benilla-formats/src/light.rs | 5 +++ crates/benilla-formats/src/light/skybox.rs | 37 ++++++++++++++++++++-- 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/crates/benilla-formats/src/light.rs b/crates/benilla-formats/src/light.rs index 6ffd1159..c9a016f5 100644 --- a/crates/benilla-formats/src/light.rs +++ b/crates/benilla-formats/src/light.rs @@ -181,6 +181,9 @@ pub struct LightCatalog { /// other rows are MOSB skyboxes. MONKEY (skybox): flags and the celestial model ride along /// ([`SkyboxDef`]), read from either table layout. skyboxes: HashMap, + /// MONKEY (reviewfix): normalised model path в†’ lowest matching `LightSkybox` id. Some patched + /// tables alias a model with different flags; path lookup must not depend on `HashMap` order. + skybox_by_path: HashMap, } fn light_schema() -> Schema { @@ -328,6 +331,7 @@ impl LightCatalog { }; // MONKEY (skybox): both layouts, keyed by the header's field count. let skyboxes = skybox::load_skyboxes(chain, LIGHT_SKYBOX)?; + let skybox_by_path = skybox::index_paths(&skyboxes); Ok(LightCatalog { lights, int_bands, @@ -337,6 +341,7 @@ impl LightCatalog { light_params_water_alpha, light_params_skybox, skyboxes, + skybox_by_path, }) } diff --git a/crates/benilla-formats/src/light/skybox.rs b/crates/benilla-formats/src/light/skybox.rs index 70322687..ab6c319a 100644 --- a/crates/benilla-formats/src/light/skybox.rs +++ b/crates/benilla-formats/src/light/skybox.rs @@ -94,6 +94,19 @@ pub(super) fn load_skyboxes(chain: &mut Chain, file: &str) -> Result) -> HashMap { + let mut by_path = HashMap::with_capacity(skyboxes.len()); + for (&id, def) in skyboxes { + by_path + .entry(def.path.to_ascii_lowercase()) + .and_modify(|old: &mut u32| *old = (*old).min(id)) + .or_insert(id); + } + by_path +} + /// The modern collector's step: dedupe by model with the larger alpha, then scale every other /// entry by `1 − alpha` (`addSkyBox` steps 1 and 4). pub(super) fn collect(list: &mut Vec, id: u32, alpha: f32) { @@ -126,9 +139,10 @@ impl LightCatalog { /// The row whose model is `path`, for a MOSB skybox the table also names. pub fn skybox_def_by_path(&self, path: &str) -> Option<&SkyboxDef> { - self.skyboxes - .values() - .find(|d| d.path.eq_ignore_ascii_case(path)) + let path = crate::models::model_path(path).to_ascii_lowercase(); + self.skybox_by_path + .get(&path) + .and_then(|id| self.skyboxes.get(id)) } /// The skybox the slot's param names; a weather slot without one falls back to the clear @@ -234,6 +248,23 @@ mod tests { assert_eq!(l, vec![ZoneSkybox { id: 1, weight: 1.0 }]); } + #[test] + fn duplicate_paths_resolve_to_the_lowest_id() { + let def = |path: &str| SkyboxDef { + path: path.into(), + flags: 0, + celestial: None, + }; + let skyboxes = HashMap::from([ + (17, def(r"environments\stars\shared.m2")), + (3, def(r"ENVIRONMENTS\STARS\SHARED.M2")), + (9, def(r"environments\stars\other.m2")), + ]); + let paths = index_paths(&skyboxes); + assert_eq!(paths.get(r"environments\stars\shared.m2"), Some(&3)); + assert_eq!(paths.get(r"environments\stars\other.m2"), Some(&9)); + } + /// The installed chain loads in either layout. #[test] fn the_installed_table_loads() {