Address comments

- make Behavior's capabilities and state private
 - Typo in a comment
 - add basic tests

Apply 1 suggestion(s) to 1 file(s)

Remove comments in agent.rs

common/src/comp/agent.rs

@@ -210,8 +210,6 @@ pub struct Agent {
     pub patrol_origin: Option<Vec3<f32>>,
     pub target: Option<Target>,
     pub chaser: Chaser,
-    /// Does the agent talk when e.g. hit by the player
-    // TODO move speech patterns into a Behavior component
     pub psyche: Psyche,
     pub inbox: VecDeque<AgentEvent>,
     pub action_timer: f32,

common/src/comp/behavior.rs

@@ -5,11 +5,16 @@ use crate::trade::SiteId;
 
 bitflags! {
     #[derive(Default)]
-    pub struct BehaviorFlag: u8 {
+    pub struct BehaviorCapability: u8 {
-        const CAN_SPEAK         = 0b00000001;
+        const SPEAK = 0b00000001;
-        const CAN_TRADE         = 0b00000010;
+        const TRADE = 0b00000010;
-        const IS_TRADING        = 0b00000100;
+    }
-        const IS_TRADING_ISSUER = 0b00001000;
+}
+bitflags! {
+    #[derive(Default)]
+    pub struct BehaviorState: u8 {
+        const TRADING        = 0b00000001;
+        const TRADING_ISSUER = 0b00000010;
     }
 }
 
@@ -20,27 +25,76 @@ bitflags! {
 /// state when needed
 #[derive(Default, Copy, Clone, Debug)]
 pub struct Behavior {
-    pub flags: BehaviorFlag,
+    capabilities: BehaviorCapability,
+    state: BehaviorState,
     pub trade_site: Option<SiteId>,
 }
 
-impl From<BehaviorFlag> for Behavior {
+impl From<BehaviorCapability> for Behavior {
-    fn from(flags: BehaviorFlag) -> Self {
+    fn from(capabilities: BehaviorCapability) -> Self {
         Behavior {
-            flags,
+            capabilities,
+            state: BehaviorState::default(),
             trade_site: None,
         }
     }
 }
 
 impl Behavior {
-    pub fn set(&mut self, flags: BehaviorFlag) { self.flags.set(flags, true) }
+    /// Set capabilities to the Behavior
+    pub fn allow(&mut self, capabilities: BehaviorCapability) {
+        self.capabilities.set(capabilities, true)
+    }
 
-    pub fn unset(&mut self, flags: BehaviorFlag) { self.flags.set(flags, false) }
+    /// Unset capabilities to the Behavior
+    pub fn deny(&mut self, capabilities: BehaviorCapability) {
+        self.capabilities.set(capabilities, false)
+    }
 
-    pub fn has(&self, flags: BehaviorFlag) -> bool { self.flags.contains(flags) }
+    /// Check if the Behavior is able to do something
+    pub fn can(&self, capabilities: BehaviorCapability) -> bool {
+        self.capabilities.contains(capabilities)
+    }
+
+    /// Set a state to the Behavior
+    pub fn set(&mut self, state: BehaviorState) { self.state.set(state, true) }
+
+    /// Unset a state to the Behavior
+    pub fn unset(&mut self, state: BehaviorState) { self.state.set(state, false) }
+
+    /// Check if the Behavior has a specific state
+    pub fn is(&self, state: BehaviorState) -> bool { self.state.contains(state) }
 }
 
 impl Component for Behavior {
     type Storage = IdvStorage<Self>;
 }
+
+#[cfg(test)]
+mod tests {
+    use super::{Behavior, BehaviorCapability, BehaviorState};
+
+    /// Test to verify that Behavior is working correctly at its most basic
+    /// usages
+    #[test]
+    pub fn basic() {
+        let mut b = Behavior::default();
+        // test capabilities
+        assert!(!b.can(BehaviorCapability::SPEAK));
+        b.allow(BehaviorCapability::SPEAK);
+        assert!(b.can(BehaviorCapability::SPEAK));
+        b.deny(BehaviorCapability::SPEAK);
+        assert!(!b.can(BehaviorCapability::SPEAK));
+        // test states
+        assert!(!b.is(BehaviorState::TRADING));
+        b.set(BehaviorState::TRADING);
+        assert!(b.is(BehaviorState::TRADING));
+        b.unset(BehaviorState::TRADING);
+        assert!(!b.is(BehaviorState::TRADING));
+        // test `from`
+        let b = Behavior::from(BehaviorCapability::SPEAK | BehaviorCapability::TRADE);
+        assert!(b.can(BehaviorCapability::SPEAK));
+        assert!(b.can(BehaviorCapability::TRADE));
+        assert!(b.can(BehaviorCapability::SPEAK | BehaviorCapability::TRADE));
+    }
+}

common/src/comp/mod.rs

@@ -49,7 +49,7 @@ pub use self::{
     agent::{Agent, Alignment},
     aura::{Aura, AuraChange, AuraKind, Auras},
     beam::{Beam, BeamSegment},
-    behavior::{Behavior, BehaviorFlag},
+    behavior::{Behavior, BehaviorCapability, BehaviorState},
     body::{
         biped_large, biped_small, bird_medium, bird_small, dragon, fish_medium, fish_small, golem,
         humanoid, object, quadruped_low, quadruped_medium, quadruped_small, ship, theropod,

common/src/states/basic_summon.rs

@@ -2,7 +2,7 @@ use crate::{
     comp::{
         self,
         inventory::loadout_builder::{LoadoutBuilder, LoadoutConfig},
-        Behavior, BehaviorFlag, CharacterState, StateUpdate,
+        Behavior, BehaviorCapability, CharacterState, StateUpdate,
     },
     event::{LocalEvent, ServerEvent},
     outcome::Outcome,
@@ -105,7 +105,7 @@ impl CharacterBehavior for Data {
                             loadout,
                             body,
                             agent: Some(comp::Agent::new(None, &body, true)),
-                            behavior: Some(Behavior::from(BehaviorFlag::CAN_SPEAK)),
+                            behavior: Some(Behavior::from(BehaviorCapability::SPEAK)),
                             alignment: comp::Alignment::Owned(*data.uid),
                             scale: self
                                 .static_data

server/src/cmd.rs

@@ -15,7 +15,7 @@ use common::{
         buff::{BuffCategory, BuffData, BuffKind, BuffSource},
         inventory::item::MaterialStatManifest,
         invite::InviteKind,
-        BehaviorFlag, ChatType, Inventory, Item, LightEmitter, WaypointArea,
+        BehaviorCapability, ChatType, Inventory, Item, LightEmitter, WaypointArea,
     },
     effect::Effect,
     event::{EventBus, ServerEvent},
@@ -1075,7 +1075,7 @@ fn handle_spawn_airship(
                 });
             if let Some(pos) = destination {
                 builder = builder.with(comp::Agent::with_destination(pos));
-                builder = builder.with(comp::Behavior::from(BehaviorFlag::CAN_SPEAK))
+                builder = builder.with(comp::Behavior::from(BehaviorCapability::SPEAK))
             }
             builder.build();
 

server/src/events/invite.rs

@@ -226,12 +226,13 @@ pub fn handle_invite_accept(server: &mut Server, entity: specs::Entity) {
                     }
                     let pricing = behaviors
                         .get(inviter)
-                        .and_then(|b| index.get_site_prices(b.trade_site))
+                        .and_then(|b| b.trade_site.map(|id| index.get_site_prices(id)))
                         .or_else(|| {
                             behaviors
                                 .get(entity)
-                                .and_then(|b| index.get_site_prices(b.trade_site))
+                                .and_then(|b| b.trade_site.map(|id| index.get_site_prices(id)))
-                        });
+                        })
+                        .flatten();
                     clients.get(inviter).map(|c| {
                         c.send(ServerGeneral::UpdatePendingTrade(
                             id,

server/src/events/trade.rs

@@ -35,14 +35,15 @@ fn notify_agent_prices(
     event: AgentEvent,
 ) {
     if let (Some(agent), Some(behavior)) = (agents.get_mut(entity), behaviors.get(entity)) {
-        let prices = index.get_site_prices(behavior.trade_site);
+        if let Some(site_id) = behavior.trade_site {
+            let prices = index.get_site_prices(site_id);
             if let AgentEvent::UpdatePendingTrade(boxval) = event {
                 // Box<(tid, pend, _, inventories)>) = event {
                 agent
                     .inbox
                     .push_front(AgentEvent::UpdatePendingTrade(Box::new((
-                    // Prefer using this Agent's price data, but use the counterparty's price data
+                        // Prefer using this Agent's price data, but use the counterparty's price
-                    // if we don't have price data
+                        // data if we don't have price data
                         boxval.0,
                         boxval.1,
                         prices.unwrap_or(boxval.2),
@@ -50,6 +51,7 @@ fn notify_agent_prices(
                     ))));
             }
         }
+    }
 }
 
 /// Invoked when the trade UI is up, handling item changes, accepts, etc
@@ -123,7 +125,10 @@ pub fn handle_process_trade_action(
                             prices = prices.or_else(|| {
                                 behaviors
                                     .get(e)
-                                    .and_then(|b| server.index.get_site_prices(b.trade_site))
+                                    .and_then(|b| {
+                                        b.trade_site.map(|id| server.index.get_site_prices(id))
+                                    })
+                                    .flatten()
                             });
                         }
                     }

server/src/rtsim/tick.rs

@@ -2,7 +2,7 @@
 
 use super::*;
 use common::{
-    comp::{self, inventory::loadout_builder::LoadoutBuilder, Behavior, BehaviorFlag},
+    comp::{self, inventory::loadout_builder::LoadoutBuilder, Behavior, BehaviorCapability},
     event::{EventBus, ServerEvent},
     resources::{DeltaTime, Time},
     terrain::TerrainGrid,
@@ -105,7 +105,7 @@ impl<'a> System<'a> for Sys {
             let pos = comp::Pos(spawn_pos);
             let agent = Some(comp::Agent::new(None, &body, false));
             let behavior = matches!(body, comp::Body::Humanoid(_))
-                .then(|| Behavior::from(BehaviorFlag::CAN_SPEAK));
+                .then(|| Behavior::from(BehaviorCapability::SPEAK));
             let rtsim_entity = Some(RtSimEntity(id));
             let event = match body {
                 comp::Body::Ship(ship) => ServerEvent::CreateShip {

server/src/sys/agent.rs

@@ -14,9 +14,9 @@ use common::{
             ItemDesc, ItemKind,
         },
         skills::{AxeSkill, BowSkill, HammerSkill, Skill, StaffSkill, SwordSkill},
-        Agent, Alignment, Behavior, BehaviorFlag, Body, CharacterState, ControlAction,
+        Agent, Alignment, Behavior, BehaviorCapability, BehaviorState, Body, CharacterState,
-        ControlEvent, Controller, Energy, Health, InputKind, Inventory, LightEmitter, MountState,
+        ControlAction, ControlEvent, Controller, Energy, Health, InputKind, Inventory,
-        Ori, PhysicsState, Pos, Scale, Stats, UnresolvedChatMsg, Vel,
+        LightEmitter, MountState, Ori, PhysicsState, Pos, Scale, Stats, UnresolvedChatMsg, Vel,
     },
     event::{Emitter, EventBus, ServerEvent},
     path::TraversalConfig,
@@ -577,7 +577,7 @@ impl<'a> AgentData<'a> {
         }
         if agent.action_timer > 0.0 {
             if agent.action_timer
-                < (if behavior.has(BehaviorFlag::IS_TRADING) {
+                < (if behavior.is(BehaviorState::TRADING) {
                     TRADE_INTERACTION_TIME
                 } else {
                     DEFAULT_INTERACTION_TIME
@@ -615,7 +615,7 @@ impl<'a> AgentData<'a> {
                 let dist_sqrd = self.pos.0.distance_squared(tgt_pos.0);
                 // Should the agent flee?
                 if 1.0 - agent.psyche.aggro > self.damage && self.flees {
-                    if agent.action_timer == 0.0 && behavior.has(BehaviorFlag::CAN_SPEAK) {
+                    if agent.action_timer == 0.0 && behavior.can(BehaviorCapability::SPEAK) {
                         let msg = "npc.speech.villager_under_attack".to_string();
                         event_emitter
                             .emit(ServerEvent::Chat(UnresolvedChatMsg::npc(*self.uid, msg)));
@@ -643,7 +643,7 @@ impl<'a> AgentData<'a> {
                         read_data.buffs.get(target),
                     ) {
                         agent.target = None;
-                        if behavior.has(BehaviorFlag::CAN_SPEAK) {
+                        if behavior.can(BehaviorCapability::SPEAK) {
                             let msg = "npc.speech.villager_enemy_killed".to_string();
                             event_emitter
                                 .emit(ServerEvent::Chat(UnresolvedChatMsg::npc(*self.uid, msg)));
@@ -907,7 +907,7 @@ impl<'a> AgentData<'a> {
         let msg = agent.inbox.pop_back();
         match msg {
             Some(AgentEvent::Talk(by, subject)) => {
-                if behavior.has(BehaviorFlag::CAN_SPEAK) {
+                if behavior.can(BehaviorCapability::SPEAK) {
                     if let Some(target) = read_data.uid_allocator.retrieve_entity_internal(by.id())
                     {
                         agent.target = Some(Target {
@@ -960,7 +960,7 @@ impl<'a> AgentData<'a> {
                                         event_emitter.emit(ServerEvent::Chat(
                                             UnresolvedChatMsg::npc(*self.uid, msg),
                                         ));
-                                    } else if behavior.has(BehaviorFlag::CAN_TRADE) {
+                                    } else if behavior.can(BehaviorCapability::TRADE) {
                                         let msg = "npc.speech.merchant_advertisement".to_string();
                                         event_emitter.emit(ServerEvent::Chat(
                                             UnresolvedChatMsg::npc(*self.uid, msg),
@@ -973,8 +973,8 @@ impl<'a> AgentData<'a> {
                                     }
                                 },
                                 Subject::Trade => {
-                                    if behavior.has(BehaviorFlag::CAN_TRADE) {
+                                    if behavior.can(BehaviorCapability::TRADE) {
-                                        if !behavior.has(BehaviorFlag::IS_TRADING) {
+                                        if !behavior.is(BehaviorState::TRADING) {
                                             controller.events.push(ControlEvent::InitiateInvite(
                                                 by,
                                                 InviteKind::Trade,
@@ -1122,8 +1122,8 @@ impl<'a> AgentData<'a> {
                 }
             },
             Some(AgentEvent::TradeInvite(with)) => {
-                if behavior.has(BehaviorFlag::CAN_TRADE) {
+                if behavior.can(BehaviorCapability::TRADE) {
-                    if !behavior.has(BehaviorFlag::IS_TRADING) {
+                    if !behavior.is(BehaviorState::TRADING) {
                         // stand still and looking towards the trading player
                         controller.actions.push(ControlAction::Stand);
                         controller.actions.push(ControlAction::Talk);
@@ -1139,13 +1139,13 @@ impl<'a> AgentData<'a> {
                         controller
                             .events
                             .push(ControlEvent::InviteResponse(InviteResponse::Accept));
-                        behavior.unset(BehaviorFlag::IS_TRADING_ISSUER);
+                        behavior.unset(BehaviorState::TRADING_ISSUER);
-                        behavior.set(BehaviorFlag::IS_TRADING);
+                        behavior.set(BehaviorState::TRADING);
                     } else {
                         controller
                             .events
                             .push(ControlEvent::InviteResponse(InviteResponse::Decline));
-                        if behavior.has(BehaviorFlag::CAN_SPEAK) {
+                        if behavior.can(BehaviorCapability::SPEAK) {
                             event_emitter.emit(ServerEvent::Chat(UnresolvedChatMsg::npc(
                                 *self.uid,
                                 "npc.speech.merchant_busy".to_string(),
@@ -1157,7 +1157,7 @@ impl<'a> AgentData<'a> {
                     controller
                         .events
                         .push(ControlEvent::InviteResponse(InviteResponse::Decline));
-                    if behavior.has(BehaviorFlag::CAN_SPEAK) {
+                    if behavior.can(BehaviorCapability::SPEAK) {
                         event_emitter.emit(ServerEvent::Chat(UnresolvedChatMsg::npc(
                             *self.uid,
                             "npc.speech.villager_decline_trade".to_string(),
@@ -1166,7 +1166,7 @@ impl<'a> AgentData<'a> {
                 }
             },
             Some(AgentEvent::TradeAccepted(with)) => {
-                if !behavior.has(BehaviorFlag::IS_TRADING) {
+                if !behavior.is(BehaviorState::TRADING) {
                     if let Some(target) =
                         read_data.uid_allocator.retrieve_entity_internal(with.id())
                     {
@@ -1176,12 +1176,12 @@ impl<'a> AgentData<'a> {
                             selected_at: read_data.time.0,
                         });
                     }
-                    behavior.set(BehaviorFlag::IS_TRADING);
+                    behavior.set(BehaviorState::TRADING);
-                    behavior.set(BehaviorFlag::IS_TRADING_ISSUER);
+                    behavior.set(BehaviorState::TRADING_ISSUER);
                 }
             },
             Some(AgentEvent::FinishedTrade(result)) => {
-                if behavior.has(BehaviorFlag::IS_TRADING) {
+                if behavior.is(BehaviorState::TRADING) {
                     match result {
                         TradeResult::Completed => {
                             event_emitter.emit(ServerEvent::Chat(UnresolvedChatMsg::npc(
@@ -1194,13 +1194,13 @@ impl<'a> AgentData<'a> {
                             "npc.speech.merchant_trade_declined".to_string(),
                         ))),
                     }
-                    behavior.unset(BehaviorFlag::IS_TRADING);
+                    behavior.unset(BehaviorState::TRADING);
                 }
             },
             Some(AgentEvent::UpdatePendingTrade(boxval)) => {
                 let (tradeid, pending, prices, inventories) = *boxval;
-                if behavior.has(BehaviorFlag::IS_TRADING) {
+                if behavior.is(BehaviorState::TRADING) {
-                    let who: usize = if behavior.has(BehaviorFlag::IS_TRADING_ISSUER) {
+                    let who: usize = if behavior.is(BehaviorState::TRADING_ISSUER) {
                         0
                     } else {
                         1
@@ -1234,7 +1234,7 @@ impl<'a> AgentData<'a> {
                         }
                         if pending.phase != TradePhase::Mutate {
                             // we got into the review phase but without balanced goods, decline
-                            behavior.unset(BehaviorFlag::IS_TRADING);
+                            behavior.unset(BehaviorState::TRADING);
                             event_emitter.emit(ServerEvent::ProcessTradeAction(
                                 *self.entity,
                                 tradeid,
@@ -1245,7 +1245,7 @@ impl<'a> AgentData<'a> {
                 }
             },
             None => {
-                if behavior.has(BehaviorFlag::CAN_SPEAK) {
+                if behavior.can(BehaviorCapability::SPEAK) {
                     // no new events, continue looking towards the last interacting player for some
                     // time
                     if let Some(Target { target, .. }) = &agent.target {
@@ -1369,7 +1369,7 @@ impl<'a> AgentData<'a> {
                         (
                             self.alignment.map_or(false, |alignment| {
                                 if matches!(alignment, Alignment::Npc) && e_inventory.equipped_items().filter(|item| item.tags().contains(&ItemTag::Cultist)).count() > 2 {
-                                    if behavior.has(BehaviorFlag::CAN_SPEAK) {
+                                    if behavior.can(BehaviorCapability::SPEAK) {
                                         if self.rtsim_entity.is_some() {
                                             agent.rtsim_controller.events.push(
                                                 RtSimEvent::AddMemory(Memory {

server/src/sys/terrain.rs

@@ -3,7 +3,8 @@ use crate::{
 };
 use common::{
     comp::{
-        self, bird_medium, inventory::loadout_builder::LoadoutConfig, Alignment, BehaviorFlag, Pos,
+        self, bird_medium, inventory::loadout_builder::LoadoutConfig, Alignment,
+        BehaviorCapability, Pos,
     },
     event::{EventBus, ServerEvent},
     generation::get_npc_name,
@@ -206,10 +207,10 @@ impl<'a> System<'a> for Sys {
                     behavior: if entity.has_agency {
                         let mut behavior = Behavior::default();
                         if can_speak {
-                            behavior.set(BehaviorFlag::CAN_SPEAK);
+                            behavior.allow(BehaviorCapability::SPEAK);
                         }
                         if trade_for_site.is_some() {
-                            behavior.set(BehaviorFlag::CAN_TRADE);
+                            behavior.allow(BehaviorCapability::TRADE);
                             behavior.trade_site = trade_for_site
                         }
                         Some(behavior)

world/src/index.rs

@@ -71,10 +71,9 @@ impl Index {
 
     pub fn colors(&self) -> AssetHandle<Arc<Colors>> { self.colors }
 
-    pub fn get_site_prices(&self, site_id: Option<SiteId>) -> Option<SitePrices> {
+    pub fn get_site_prices(&self, site_id: SiteId) -> Option<SitePrices> {
-        site_id
+        self.sites
-            .map(|i| self.sites.recreate_id(i))
+            .recreate_id(site_id)
-            .flatten()
             .map(|i| self.sites.get(i))
             .map(|s| s.economy.get_site_prices())
     }