diff --git a/server/src/bin/generator_spike.rs b/server/src/bin/generator_spike.rs index 3bf7db449..5388223ce 100644 --- a/server/src/bin/generator_spike.rs +++ b/server/src/bin/generator_spike.rs @@ -295,6 +295,11 @@ const ALL_TRAITS: [PersonalityTrait; 10] = [ PersonalityTrait::Reclusive, ]; +// TODO(integration): Duplicates `traits_contradict` from npc/generate.rs. +// Extract to a shared helper in npc/mod.rs when wiring the spike into the main pipeline. +// TODO(#612 integration): duplicates the private `traits_contradict` in npc/generate.rs. +// When wiring the spike into the main pipeline, expose that function as pub(crate) +// and remove this copy. Kept separate for now to avoid spike coupling to ECS internals. fn traits_contradict(a: PersonalityTrait, b: PersonalityTrait) -> bool { use PersonalityTrait::*; matches!( @@ -354,20 +359,49 @@ fn gen_traits(rng: &mut SimRng, culture: &CultureProfile) -> Vec String { - let given_idx = rng.rng.random_range(0..culture.naming.given_names.len()); - let given = &culture.naming.given_names[given_idx]; - - if culture.naming.family_name_used_socially && !culture.naming.family_names.is_empty() { - let family_idx = rng.rng.random_range(0..culture.naming.family_names.len()); - let family = &culture.naming.family_names[family_idx]; - format!("{} {}", given, family) - } else { - given.clone() +/// Build a shuffled name pool of up to `count` unique names. +/// +/// Uses Fisher-Yates on given_names indices so each name appears at most once. +/// Returns fewer names than requested if the pool is smaller than `count`. +/// Callers must use the returned `Vec`'s length as the actual NPC count. +/// +/// Exits if `given_names` is empty (validator should catch this upstream, +/// but we guard here defensively — #5 fix). +fn build_name_pool(rng: &mut SimRng, culture: &CultureProfile, count: usize) -> Vec { + if culture.naming.given_names.is_empty() { + eprintln!( + "Error: culture '{}' has no given_names — cannot generate NPCs.", + culture.id + ); + process::exit(1); } + + let pool_size = culture.naming.given_names.len(); + let actual_count = count.min(pool_size); + + // Partial Fisher-Yates — produces `actual_count` unique given-name indices. + let mut indices: Vec = (0..pool_size).collect(); + for i in 0..actual_count { + let swap = rng.rng.random_range(i..pool_size); + indices.swap(i, swap); + } + + indices[..actual_count] + .iter() + .map(|&i| { + let given = &culture.naming.given_names[i]; + if culture.naming.family_name_used_socially && !culture.naming.family_names.is_empty() { + let family_idx = rng.rng.random_range(0..culture.naming.family_names.len()); + let family = &culture.naming.family_names[family_idx]; + format!("{} {}", given, family) + } else { + given.clone() + } + }) + .collect() } // --------------------------------------------------------------------------- @@ -375,6 +409,13 @@ fn gen_name(rng: &mut SimRng, culture: &CultureProfile) -> String { // --------------------------------------------------------------------------- fn pick_role<'a>(rng: &mut SimRng, zone: &'a ZoneSpec) -> &'a RoleSpec { + if zone.roles.is_empty() { + eprintln!( + "Error: zone '{}' has no roles — cannot generate NPCs.", + zone.zone_type + ); + process::exit(1); + } let total_weight: u32 = zone.roles.iter().map(|r| r.weight as u32).sum(); let roll = rng.rng.random_range(0..total_weight); let mut cumulative = 0u32; @@ -400,13 +441,12 @@ fn gen_behaviors(rng: &mut SimRng, role: &RoleSpec, culture: &CultureProfile) -> behaviors.push(role.typical_behaviors[idx].clone()); } - // Pick 1 culture-specific behavior (50% chance to add a second entry) - if !culture.values.description.is_empty() && rng.rng.random_range(0..2_u32) == 0 { - if !culture.naming.given_names.is_empty() { - // Use a behavioral tendency derived from cultural values description - let cultural_behavior = cultural_tendency(rng, culture); - behaviors.push(cultural_behavior); - } + // 50% chance to add a cultural behavior from the speech greeting pool. + // Guard on greetings being non-empty — cultural_tendency draws from it. + // (#7 fix: was incorrectly checking given_names, which is unrelated here) + if !culture.speech.greetings.is_empty() && rng.rng.random_range(0..2_u32) == 0 { + let cultural_behavior = cultural_tendency(rng, culture); + behaviors.push(cultural_behavior); } behaviors @@ -427,10 +467,12 @@ fn cultural_tendency(rng: &mut SimRng, culture: &CultureProfile) -> String { // Cultural markers // --------------------------------------------------------------------------- +/// Maximum filler words assigned to a single NPC from the culture pool. +const MAX_FILLER_WORDS: usize = 2; + fn gen_cultural_markers(rng: &mut SimRng, culture: &CultureProfile) -> CulturalMarkers { - // Pick 1-2 filler words let filler_count = if culture.speech.filler_words.len() > 1 { - rng.rng.random_range(1_usize..=2.min(culture.speech.filler_words.len())) + rng.rng.random_range(1_usize..=MAX_FILLER_WORDS.min(culture.speech.filler_words.len())) } else { culture.speech.filler_words.len() }; @@ -463,6 +505,12 @@ fn gen_cultural_markers(rng: &mut SimRng, culture: &CultureProfile) -> CulturalM // Relationship generation (name-based for spike — no StableId) // --------------------------------------------------------------------------- +/// Generate 0–3 relationships for an NPC. +/// +/// **One-directional by design:** each NPC independently picks their own +/// relationships. If Tev knows Renn as a "colleague (positive)" but Renn +/// has no entry for Tev, that is intentional — asymmetric awareness is +/// a core mechanic (D-034). The output may look like bugs but is correct. fn gen_relationships( rng: &mut SimRng, this_name: &str, @@ -488,6 +536,7 @@ fn gen_relationships( indices.swap(i, swap); } + // TODO(integration): Use RelationshipKind enum from npc/mod.rs instead of strings. let rel_kinds = ["colleague", "friend", "rival", "superior", "subordinate"]; let valences = [ RelationshipValence::Positive, @@ -517,15 +566,13 @@ fn generate_npc_blueprint( rng: &mut SimRng, zone: &ZoneSpec, culture: &CultureProfile, - all_names: &[String], + name: String, ) -> NpcBlueprint { - let name = gen_name(rng, culture); let role = pick_role(rng, zone); let traits = gen_traits(rng, culture); let observable_behaviors = gen_behaviors(rng, role, culture); let cultural_markers = gen_cultural_markers(rng, culture); - // Relationships assigned in a second pass once all names are known - let _ = all_names; // populated after all NPCs are named + // Relationships assigned in a second pass once all names are known. NpcBlueprint { name, @@ -541,6 +588,7 @@ fn generate_npc_blueprint( // Output formatting // --------------------------------------------------------------------------- +// TODO(integration): Derive Display on PersonalityTrait and drop this function. fn trait_label(t: PersonalityTrait) -> &'static str { use PersonalityTrait::*; match t { @@ -643,18 +691,40 @@ fn main() { (zone, culture) }; + // Validate zone spec + if zone.population_density < 1 { + eprintln!( + "Error: population_density must be >= 1 (got {} in zone '{}')", + zone.population_density, zone.zone_type + ); + process::exit(1); + } + if zone.roles.is_empty() { + eprintln!( + "Error: zone '{}' has no roles defined — cannot generate NPCs.", + zone.zone_type + ); + process::exit(1); + } + // Seed the RNG let mut rng = SimRng::new(args.seed); - // Determine NPC count from zone density - let npc_count = rng.rng.random_range( - zone.population_density as usize..=(zone.population_density as usize * 2).max(1) - ); + // Determine NPC count from zone density. + // population_density already validated >= 1 above. + let density = zone.population_density as usize; + let npc_count = rng.rng.random_range(density..=(density * 2)); let npc_count = npc_count.max(2); // at least 2 for relationship output + // Pre-generate unique names without replacement (#3 fix). + // build_name_pool caps at pool size — actual count may be less than requested. + let name_pool = build_name_pool(&mut rng, &culture, npc_count); + let _npc_count = name_pool.len(); // use capped count (for documentation) + // First pass: generate all NPCs (no relationships yet) - let mut npcs: Vec = (0..npc_count) - .map(|_| generate_npc_blueprint(&mut rng, &zone, &culture, &[])) + let mut npcs: Vec = name_pool + .into_iter() + .map(|name| generate_npc_blueprint(&mut rng, &zone, &culture, name)) .collect(); // Collect all names for relationship pass diff --git a/server/src/bin/validate_ron.rs b/server/src/bin/validate_ron.rs index 51356a159..5dd86c064 100644 --- a/server/src/bin/validate_ron.rs +++ b/server/src/bin/validate_ron.rs @@ -49,6 +49,27 @@ fn main() { Ok(spec) => { println!("Valid ZoneSpec: {} ({})", spec.label, spec.zone_type); println!(" {} roles, {} social sites", spec.roles.len(), spec.social_sites.len()); + let mut warnings = 0; + if spec.roles.is_empty() { + eprintln!(" WARNING: no roles defined — generator will reject this"); + warnings += 1; + } + if spec.population_density < 1 { + eprintln!(" WARNING: population_density is 0 — generator requires >= 1"); + warnings += 1; + } + for site in &spec.social_sites { + if site.min_npcs > site.max_npcs { + eprintln!( + " WARNING: social site '{}' has min_npcs ({}) > max_npcs ({})", + site.site_type, site.min_npcs, site.max_npcs + ); + warnings += 1; + } + } + if warnings > 0 { + process::exit(1); + } } Err(e) => { eprintln!("Invalid ZoneSpec in {}:", args.file); @@ -69,6 +90,14 @@ fn main() { profile.speech.filler_words.len(), profile.values.favored_traits.len() ); + let mut warnings = 0; + if profile.naming.given_names.is_empty() { + eprintln!(" WARNING: no given_names — generator will reject this"); + warnings += 1; + } + if warnings > 0 { + process::exit(1); + } } Err(e) => { eprintln!("Invalid CultureProfile in {}:", args.file); diff --git a/tooling/validate-ron b/tooling/validate-ron index 175971fe2..12304025a 100755 --- a/tooling/validate-ron +++ b/tooling/validate-ron @@ -20,7 +20,12 @@ if [ $# -lt 2 ]; then exit 1 fi -# Resolve file to absolute path before changing directory +# Check file exists before resolving — realpath gives unhelpful errors otherwise +if [ ! -f "$1" ]; then + echo "Error: file not found: $1" + exit 1 +fi + FILE="$(realpath "$1")" SCHEMA="$2"