diff --git a/crates/workshop-rs/src/settings/schema.rs b/crates/workshop-rs/src/settings/schema.rs index 934cd50..880baef 100644 --- a/crates/workshop-rs/src/settings/schema.rs +++ b/crates/workshop-rs/src/settings/schema.rs @@ -1103,6 +1103,7 @@ fn canonical_concept(key: &str, path: &[PathPart<'_>]) -> Option { let key = key.trim_end_matches('%'); Some(match key { "health" => "health".to_string(), + "damageDealt" | "damageReceived" | "healingDealt" | "healingReceived" => key.to_string(), "passiveUltGen" => "ultimateGeneration.passive".to_string(), "combatUltGen" => "ultimateGeneration.combat".to_string(), "ultGen" => "ultimateGeneration".to_string(), @@ -1117,17 +1118,6 @@ fn canonical_concept(key: &str, path: &[PathPart<'_>]) -> Option { "enableScoping" => "primaryFire.scopingEnabled".to_string(), "enablePassiveUnlimitedFuel" => "passive.unlimitedFuelEnabled".to_string(), "enablePrimaryFireFreezeStack" => "primaryFire.freezeStackEnabled".to_string(), - key if key.ends_with("Cooldown") => "ability.cooldown".to_string(), - key if key.ends_with("RechargeRate") => "ability.rechargeRate".to_string(), - key if key.ends_with("EnergyChargeRate") => "ability.energyChargeRate".to_string(), - key if key.ends_with("MaximumTime") || key.ends_with("MaxTime") => { - "ability.maximumTime".to_string() - } - key if key.contains("EnemyKb") => "ability.knockback.enemy".to_string(), - key if key.contains("Kb") => "ability.knockback".to_string(), - key if key.contains("Damage") => "ability.damage".to_string(), - key if key.contains("Healing") => "ability.healing".to_string(), - key if key.contains("Resource") => "ability.resource".to_string(), "setValidControlPoints" | "firstActiveControlPoint" => path .iter() .filter_map(|part| match part { @@ -1146,8 +1136,27 @@ pub fn validate_catalog() -> Result<(), Vec> { use std::collections::{HashMap, HashSet}; let mut errors = Vec::new(); + let mut raw_paths: HashMap = HashMap::new(); + for entry in table::raw_entries() { + let path = table::path_string(entry.path); + if let Some(previous) = raw_paths.insert(path.clone(), *entry) { + let same_path = previous.path.len() == entry.path.len() + && previous + .path + .iter() + .zip(entry.path.iter()) + .all(|(left, right)| left == right); + if !same_path || !same_entry_shape(previous.kind, entry.kind) { + errors.push(format!( + "conflicting duplicate settings path between catalog projections: {path}" + )); + } + } + } let mut paths = HashSet::new(); - let mut concepts: HashMap<(String, SettingTargetKind), SettingValueDomain> = HashMap::new(); + let mut concepts: HashMap<(String, SettingTargetKind, String), SettingValueDomain> = + HashMap::new(); + let mut concept_keys: HashMap<(String, SettingTargetKind), String> = HashMap::new(); for definition in definitions() { if !paths.insert(definition.path.clone()) { @@ -1175,7 +1184,17 @@ pub fn validate_catalog() -> Result<(), Vec> { definition.path )); } - let key = (id.as_str().to_string(), definition.target_kind()); + let target_kind = definition.target_kind(); + let semantic_key = semantic_identity_key(definition.key); + let collision_key = (id.as_str().to_string(), target_kind.clone()); + if let Some(previous_key) = concept_keys.insert(collision_key, semantic_key.clone()) { + if previous_key != semantic_key { + errors.push(format!( + "conflicting settings concepts for {id}: {previous_key} vs {semantic_key}" + )); + } + } + let key = (id.as_str().to_string(), target_kind, semantic_key); if let Some(previous) = concepts.insert(key, definition.domain.clone()) { if previous != definition.domain { errors.push(format!("conflicting settings domains for {id}")); @@ -1189,6 +1208,27 @@ pub fn validate_catalog() -> Result<(), Vec> { } } +fn semantic_identity_key(key: &str) -> String { + match key { + "enableSecondaryFire" | "enableGenericSecondaryFire" => "enableSecondaryFire".to_string(), + _ => key.to_string(), + } +} + +fn same_entry_shape(left: KeyKind, right: KeyKind) -> bool { + matches!( + (left, right), + (KeyKind::Flag, KeyKind::Flag) + | (KeyKind::String, KeyKind::String) + | (KeyKind::Bool, KeyKind::Bool) + | (KeyKind::Number, KeyKind::Number) + | (KeyKind::Percent, KeyKind::Percent) + | (KeyKind::ListMap, KeyKind::ListMap) + | (KeyKind::ListHero, KeyKind::ListHero) + | (KeyKind::Enum(_), KeyKind::Enum(_)) + ) +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/workshop-rs/src/settings/table.rs b/crates/workshop-rs/src/settings/table.rs index e74d693..b661aa9 100644 --- a/crates/workshop-rs/src/settings/table.rs +++ b/crates/workshop-rs/src/settings/table.rs @@ -956,16 +956,26 @@ pub fn lookup(path: &[PathPart<'_>]) -> Option<&'static TableEntry> { }) } -/// Iterate the reviewed settings inventory with generated export entries -/// taking precedence over the legacy hand-written projection. Duplicate -/// paths are therefore represented once in the semantic catalog while the -/// parser and emitter continue to use the same lookup table. +/// Iterate the reviewed settings inventory with the hand-written projection +/// taking precedence over the generated export projection. Duplicate paths +/// are represented once in the semantic catalog while the parser and emitter +/// continue to use the same lookup table. pub fn entries() -> impl Iterator { + deduplicated_entries(ENTRIES.iter().chain(GENERATED_ENTRIES.iter())) +} + +/// Iterate both catalog projections without applying effective lookup +/// precedence. The semantic validator uses this to compare duplicate paths +/// instead of allowing `entries()` to hide stale or conflicting data. +pub(crate) fn raw_entries() -> impl Iterator { + ENTRIES.iter().chain(GENERATED_ENTRIES.iter()) +} + +fn deduplicated_entries( + entries: impl Iterator, +) -> impl Iterator { let mut paths = std::collections::HashSet::new(); - ENTRIES - .iter() - .chain(GENERATED_ENTRIES.iter()) - .filter(move |entry| paths.insert(path_string(entry.path))) + entries.filter(move |entry| paths.insert(path_string(entry.path))) } pub(crate) fn is_generated_entry(entry: &TableEntry) -> bool { diff --git a/crates/workshop-rs/tests/settings_pipeline.rs b/crates/workshop-rs/tests/settings_pipeline.rs index 9e38ea1..ae035df 100644 --- a/crates/workshop-rs/tests/settings_pipeline.rs +++ b/crates/workshop-rs/tests/settings_pipeline.rs @@ -335,6 +335,19 @@ fn settings_schema_projects_workshop_facts_without_display_names_in_ids() { ); } +#[test] +fn settings_schema_keeps_distinct_hero_modifier_identities() { + let damage_dealt = definitions_by_id(&SettingId::from("setting.hero.damageDealt")) + .find(|definition| definition.target_kind() == SettingTargetKind::Hero) + .expect("hero damage-dealt definition"); + let damage_received = definitions_by_id(&SettingId::from("setting.hero.damageReceived")) + .find(|definition| definition.target_kind() == SettingTargetKind::Hero) + .expect("hero damage-received definition"); + + assert_ne!(damage_dealt.id(), damage_received.id()); + assert_ne!(damage_dealt.path(), damage_received.path()); +} + #[test] fn settings_schema_exposes_normal_enum_and_list_domains() { let definitions: Vec<_> = definitions().collect(); diff --git a/docs/language-support/settings.md b/docs/language-support/settings.md index 188ff36..42d852b 100644 --- a/docs/language-support/settings.md +++ b/docs/language-support/settings.md @@ -50,7 +50,7 @@ assert_eq!( Applicability::Applicable ); -let ashe_only = definitions_by_id(&SettingId::from("setting.hero.ability.knockback.enemy")) +let ashe_only = definitions_by_id(&SettingId::from("setting.hero.ability1EnemyKb")) .find(|definition| definition.path().ends_with("ability1EnemyKb%")) .expect("exceptional hero setting"); let ana_ability = SettingTarget::HeroAbility {