Address code review.

CHANGELOG.md

@@ -15,9 +15,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Admin command to reload all chunks on the server
 - Furniture and waypoints in site2 towns
 - text input for trading
-- Themed Site CliffTown, hoodoo/arabic inspired stone structures inhabited by mountaineer NPCs.    
+- Themed Site CliffTown, hoodoo/arabic inspired stone structures inhabited by mountaineer NPCs.
 - NPCs now have rudimentary personalities
 - Added Belarusian translation
+- Add FOV check for agents scanning for targets they are hostile to
 
 ### Changed
 
@@ -81,7 +82,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Convert giant trees to site2
 - Add new upgraded travelers
 - Wallrunning
-- Add FOV check for agents scanning for targets they are hostile to
 
 ### Changed
 

common/src/util/spatial_grid.rs

@@ -49,7 +49,8 @@ impl SpatialGrid {
     }
 
     /// Get an iterator over the entities overlapping the provided axis aligned
-    /// bounding region. NOTE: for best optimization of the iterator use
+    /// bounding region.
+    /// NOTE: for best optimization of the iterator use
     /// `for_each` rather than a for loop.
     pub fn in_aabr<'a>(&'a self, aabr: Aabr<i32>) -> impl Iterator<Item = specs::Entity> + 'a {
         let iter = |max_entity_radius, grid: &'a hashbrown::HashMap<_, _>, lg2_cell_size| {

server/src/sys/agent.rs

@@ -13,10 +13,9 @@ use crate::{
         },
         data::{AgentData, AttackData, Path, ReadData, Tactic, TargetData},
         util::{
-            aim_projectile, are_our_owners_hostile, does_entity_see_other,
-            entity_looks_like_cultist, get_attacker_of_entity, get_entity_by_id, is_dead,
-            is_dead_or_invulnerable, is_entity_a_village_guard, is_invulnerable, is_villager,
-            stop_pursuing,
+            aim_projectile, are_our_owners_hostile, does_entity_see_other, get_attacker_of_entity,
+            get_entity_by_id, is_dead, is_dead_or_invulnerable, is_dressed_as_cultist,
+            is_invulnerable, is_village_guard, is_villager, stop_pursuing,
         },
     },
 };
@@ -1434,7 +1433,7 @@ impl<'a> AgentData<'a> {
             if is_villager(self.alignment) {
                 if self.remembers_fight_with(target, read_data) {
                     chat_villager_remembers_fighting();
-                } else if entity_looks_like_cultist(target, read_data) {
+                } else if is_dressed_as_cultist(target, read_data) {
                     chat("npc.speech.villager_cultist_alarm");
                 } else {
                     chat("npc.speech.menacing");
@@ -1569,7 +1568,7 @@ impl<'a> AgentData<'a> {
         let get_enemy = |entity: EcsEntity| {
             if self.is_entity_an_enemy(entity, read_data) {
                 Some(entity)
-            } else if self.defends_entity(entity, read_data) {
+            } else if self.should_defend(entity, read_data) {
                 if let Some(attacker) = get_attacker_of_entity(entity, read_data) {
                     if !is_alignment_passive_towards_entity(attacker) {
                         // aggro_on: attack immediately, do not warn/menace.
@@ -2398,10 +2397,10 @@ impl<'a> AgentData<'a> {
         (entity != *self.entity)
             && (are_our_owners_hostile(self.alignment, alignment, read_data)
                 || self.remembers_fight_with(entity, read_data)
-                || self.is_villager_and_entity_is_cultist(entity, read_data))
+                || self.is_villager_and_is_entity_dressed_as_cultist(entity, read_data))
     }
 
-    fn defends_entity(&self, entity: EcsEntity, read_data: &ReadData) -> bool {
+    fn should_defend(&self, entity: EcsEntity, read_data: &ReadData) -> bool {
         let entity_alignment = read_data.alignments.get(entity);
 
         let we_are_friendly = entity_alignment.map_or(false, |entity_alignment| {
@@ -2419,11 +2418,15 @@ impl<'a> AgentData<'a> {
             matches!(entity_alignment, Some(Alignment::Owned(ouid)) if *self.uid == *ouid);
 
         (we_are_friendly && we_share_species)
-            || (is_entity_a_village_guard(*self.entity, read_data) && is_villager(entity_alignment))
+            || (is_village_guard(*self.entity, read_data) && is_villager(entity_alignment))
             || self_owns_entity
     }
 
-    fn is_villager_and_entity_is_cultist(&self, entity: EcsEntity, read_data: &ReadData) -> bool {
-        is_villager(self.alignment) && entity_looks_like_cultist(entity, read_data)
+    fn is_villager_and_is_entity_dressed_as_cultist(
+        &self,
+        entity: EcsEntity,
+        read_data: &ReadData,
+    ) -> bool {
+        is_villager(self.alignment) && is_dressed_as_cultist(entity, read_data)
     }
 }

server/src/sys/agent/util.rs

@@ -132,7 +132,7 @@ pub fn is_villager(alignment: Option<&Alignment>) -> bool {
     alignment.map_or(false, |alignment| matches!(alignment, Alignment::Npc))
 }
 
-pub fn is_entity_a_village_guard(entity: EcsEntity, read_data: &ReadData) -> bool {
+pub fn is_village_guard(entity: EcsEntity, read_data: &ReadData) -> bool {
     read_data
         .stats
         .get(entity)
@@ -164,6 +164,19 @@ pub fn positions_have_line_of_sight(pos_a: &Pos, pos_b: &Pos, read_data: &ReadDa
         >= dist_sqrd
 }
 
+pub fn is_dressed_as_cultist(entity: EcsEntity, read_data: &ReadData) -> bool {
+    read_data
+        .inventories
+        .get(entity)
+        .map_or(false, |inventory| {
+            inventory
+                .equipped_items()
+                .filter(|item| item.tags().contains(&ItemTag::Cultist))
+                .count()
+                > 2
+        })
+}
+
 pub fn does_entity_see_other(
     agent: &Agent,
     entity: EcsEntity,
@@ -171,13 +184,13 @@ pub fn does_entity_see_other(
     controller: &Controller,
     read_data: &ReadData,
 ) -> bool {
-    let stealth_coefficient = {
-        let is_other_being_stealthy = read_data
+    let other_stealth_coefficient = {
+        let is_other_stealthy = read_data
             .char_states
             .get(other)
             .map_or(false, CharacterState::is_stealthy);
 
-        if is_other_being_stealthy {
+        if is_other_stealthy {
             // TODO: We shouldn't have to check CharacterState. This should be factored in
             // by the function (such as the one we're calling below) that supposedly
             // computes a coefficient given stealthy-ness.
@@ -191,18 +204,16 @@ pub fn does_entity_see_other(
         read_data.positions.get(entity),
         read_data.positions.get(other),
     ) {
-        let dist = other_pos.0 - pos.0;
         let dist_sqrd = other_pos.0.distance_squared(pos.0);
 
         let within_sight_dist = {
-            let sight_dist = agent.psyche.sight_dist / stealth_coefficient;
+            let sight_dist = agent.psyche.sight_dist / other_stealth_coefficient;
             dist_sqrd < sight_dist.powi(2)
         };
 
-        let within_fov = dist
-                .try_normalized()
-                // FIXME: Should this be map_or(false)?
-                .map_or(true, |v| v.dot(*controller.inputs.look_dir) > 0.15);
+        let within_fov = (other_pos.0 - pos.0)
+            .try_normalized()
+            .map_or(false, |v| v.dot(*controller.inputs.look_dir) > 0.15);
 
         within_sight_dist && positions_have_line_of_sight(pos, other_pos, read_data) && within_fov
     } else {