Deduplicate pvp-checks

common/src/cmd.rs

@@ -323,6 +323,7 @@ impl ChatCommand {
                  true for overwrite to alter an existing ban..",
                 Some(Moderator),
             ),
+            #[rustfmt::skip]
             ChatCommand::BattleMode => cmd(
                 vec![Enum(
                     "battle mode",

common/src/combat.rs

@@ -12,13 +12,13 @@ use crate::{
         },
         poise::PoiseChange,
         skills::SkillGroupKind,
-        Body, CharacterState, Combo, Energy, EnergyChange, EnergySource, Health, HealthChange,
+        Alignment, Body, CharacterState, Combo, Energy, EnergyChange, EnergySource, Health,
-        HealthSource, Inventory, Ori, Player, Poise, SkillSet, Stats,
+        HealthChange, HealthSource, Inventory, Ori, Player, Poise, SkillSet, Stats,
     },
     event::ServerEvent,
     outcome::Outcome,
     states::utils::StageSection,
-    uid::Uid,
+    uid::{Uid, UidAllocator},
     util::Dir,
 };
 
@@ -28,7 +28,7 @@ use rand::{thread_rng, Rng};
 use serde::{Deserialize, Serialize};
 
 #[cfg(not(target_arch = "wasm32"))]
-use specs::Entity as EcsEntity;
+use specs::{saveload::MarkerAllocator, Entity as EcsEntity, ReadStorage};
 #[cfg(not(target_arch = "wasm32"))]
 use std::{ops::MulAssign, time::Duration};
 #[cfg(not(target_arch = "wasm32"))] use vek::*;
@@ -469,14 +469,56 @@ impl Attack {
     }
 }
 
-/// Says if we should allow negative effects from one player to another
+/// Function that checks for unintentional PvP between players.
 ///
-/// NOTE: this function doesn't handle pets or friendly-fire, you will need to
+/// Returns `false` if attack will create unintentional conflict,
-/// figure it out on call-side.
+/// e.g. if player with PvE mode will harm pets of other players
-pub fn may_harm(attacker: Option<&Player>, target: Option<&Player>) -> bool {
+/// or other players will do the same to such player.
-    attacker
+///
-        .zip(target)
+/// If both players have PvP mode enabled, interact with NPC and
-        .map_or(true, |(attacker, target)| attacker.may_harm(target))
+/// in any other case, this function will return `true`
+// TODO: add parameter for doing self-harm?
+pub fn may_harm(
+    alignments: &ReadStorage<Alignment>,
+    players: &ReadStorage<Player>,
+    uid_allocator: &UidAllocator,
+    attacker: Option<EcsEntity>,
+    target: EcsEntity,
+) -> bool {
+    // Return owner entity if pet,
+    // or just return entity back otherwise
+    let owner_if_pet = |entity| {
+        let alignment = alignments.get(entity).copied();
+        if let Some(Alignment::Owned(uid)) = alignment {
+            // return original entity
+            // if can't get owner
+            uid_allocator
+                .retrieve_entity_internal(uid.into())
+                .unwrap_or(entity)
+        } else {
+            entity
+        }
+    };
+
+    // Just return ok if attacker is unknown, it's probably
+    // environment or command.
+    let attacker = match attacker {
+        Some(attacker) => attacker,
+        None => return true,
+    };
+
+    // "Dereference" to owner if this is a pet.
+    let attacker = owner_if_pet(attacker);
+    let target = owner_if_pet(target);
+
+    // Get player components
+    let attacker_info = players.get(attacker);
+    let target_info = players.get(target);
+
+    // Return `true` if not players.
+    attacker_info
+        .zip(target_info)
+        .map_or(true, |(a, t)| a.may_harm(t))
 }
 
 #[cfg(not(target_arch = "wasm32"))]

common/src/comp/agent.rs

@@ -3,10 +3,10 @@ use crate::{
     path::Chaser,
     rtsim::RtSimController,
     trade::{PendingTrade, ReducedInventory, SiteId, SitePrices, TradeId, TradeResult},
-    uid::{Uid, UidAllocator},
+    uid::Uid,
 };
 use serde::Deserialize;
-use specs::{saveload::MarkerAllocator, Component, Entity as EcsEntity};
+use specs::{Component, Entity as EcsEntity};
 use specs_idvs::IdvStorage;
 use std::{collections::VecDeque, fmt};
 use strum::IntoEnumIterator;
@@ -34,15 +34,6 @@ pub enum Alignment {
     Passive,
 }
 
-// Helper function to get owner
-pub fn owner_of(alignment: Option<Alignment>, uid_allocator: &UidAllocator) -> Option<EcsEntity> {
-    if let Some(Alignment::Owned(uid)) = alignment {
-        uid_allocator.retrieve_entity_internal(uid.into())
-    } else {
-        None
-    }
-}
-
 #[derive(Copy, Clone, Debug, PartialEq)]
 pub enum Mark {
     Merchant,

common/systems/src/aura.rs

@@ -1,7 +1,6 @@
 use common::{
     combat,
     comp::{
-        agent::owner_of,
         aura::{AuraChange, AuraKey, AuraKind, AuraTarget},
         buff::{Buff, BuffCategory, BuffChange, BuffSource},
         group::Group,
@@ -178,7 +177,7 @@ fn activate_aura(
             // Which means that you can't apply debuffs on you and your group
             // even if it's intented mechanic.
             //
-            // Not that we have this for now, but think about this
+            // We don't have this for now, but think about this
             // when we will add this.
             let may_harm = || {
                 let owner = match source {
@@ -187,18 +186,12 @@ fn activate_aura(
                     },
                     _ => None,
                 };
-                let owner_if_pet = |entity| {
-                    // Return owner entity if pet,
-                    // or just return entity back otherwise
-                    owner_of(
-                        read_data.alignments.get(entity).copied(),
-                        &read_data.uid_allocator,
-                    )
-                    .unwrap_or(entity)
-                };
                 combat::may_harm(
-                    owner.and_then(|owner| read_data.players.get(owner_if_pet(owner))),
+                    &read_data.alignments,
-                    read_data.players.get(owner_if_pet(target)),
+                    &read_data.players,
+                    &read_data.uid_allocator,
+                    owner,
+                    target,
                 )
             };
 

common/systems/src/beam.rs

@@ -1,7 +1,7 @@
 use common::{
     combat::{self, AttackOptions, AttackSource, AttackerInfo, TargetInfo},
     comp::{
-        agent::{owner_of, Sound, SoundKind},
+        agent::{Sound, SoundKind},
         Alignment, Beam, BeamSegment, Body, CharacterState, Combo, Energy, Group, Health,
         HealthSource, Inventory, Ori, Player, Pos, Scale, Stats,
     },
@@ -216,18 +216,12 @@ impl<'a> System<'a> for Sys {
 
 
                     // PvP check
-                    let owner_if_pet = |entity| {
-                        // Return owner entity if pet,
-                        // or just return entity back otherwise
-                        owner_of(
-                            read_data.alignments.get(entity).copied(),
-                            &read_data.uid_allocator,
-                        )
-                        .unwrap_or(entity)
-                    };
                     let may_harm = combat::may_harm(
-                        beam_owner.and_then(|owner| read_data.players.get(owner_if_pet(owner))),
+                        &read_data.alignments,
-                        read_data.players.get(owner_if_pet(target)),
+                        &read_data.players,
+                        &read_data.uid_allocator,
+                        beam_owner,
+                        target,
                     );
                     let attack_options = AttackOptions {
                         // No luck with dodging beams

common/systems/src/melee.rs

@@ -1,7 +1,7 @@
 use common::{
     combat::{self, AttackOptions, AttackSource, AttackerInfo, TargetInfo},
     comp::{
-        agent::{owner_of, Sound, SoundKind},
+        agent::{Sound, SoundKind},
         Alignment, Body, CharacterState, Combo, Energy, Group, Health, Inventory, Melee, Ori,
         Player, Pos, Scale, Stats,
     },
@@ -165,18 +165,12 @@ impl<'a> System<'a> for Sys {
                     };
 
                     // PvP check
-                    let owner_if_pet = |entity| {
-                        // Return owner entity if pet,
-                        // or just return entity back otherwise
-                        owner_of(
-                            read_data.alignments.get(entity).copied(),
-                            &read_data.uid_allocator,
-                        )
-                        .unwrap_or(entity)
-                    };
                     let may_harm = combat::may_harm(
-                        read_data.players.get(owner_if_pet(attacker)),
+                        &read_data.alignments,
-                        read_data.players.get(owner_if_pet(target)),
+                        &read_data.players,
+                        &read_data.uid_allocator,
+                        Some(attacker),
+                        target,
                     );
 
                     let attack_options = AttackOptions {

common/systems/src/projectile.rs

@@ -1,7 +1,7 @@
 use common::{
     combat::{self, AttackOptions, AttackSource, AttackerInfo, TargetInfo},
     comp::{
-        agent::{owner_of, Sound, SoundKind},
+        agent::{Sound, SoundKind},
         projectile, Alignment, Body, CharacterState, Combo, Energy, Group, Health, HealthSource,
         Inventory, Ori, PhysicsState, Player, Pos, Projectile, Stats, Vel,
     },
@@ -301,18 +301,12 @@ fn dispatch_hit(
             }
 
             // PvP check
-            let owner_if_pet = |entity| {
-                // Return owner entity if pet,
-                // or just return entity back otherwise
-                owner_of(
-                    read_data.alignments.get(entity).copied(),
-                    &read_data.uid_allocator,
-                )
-                .unwrap_or(entity)
-            };
             let may_harm = combat::may_harm(
-                owner.and_then(|owner| read_data.players.get(owner_if_pet(owner))),
+                &read_data.alignments,
-                read_data.players.get(owner_if_pet(target)),
+                &read_data.players,
+                &read_data.uid_allocator,
+                owner,
+                target,
             );
 
             let attack_options = AttackOptions {

common/systems/src/shockwave.rs

@@ -1,7 +1,7 @@
 use common::{
     combat::{self, AttackOptions, AttackSource, AttackerInfo, TargetInfo},
     comp::{
-        agent::{owner_of, Sound, SoundKind},
+        agent::{Sound, SoundKind},
         Alignment, Body, CharacterState, Combo, Energy, Group, Health, HealthSource, Inventory,
         Ori, PhysicsState, Player, Pos, Scale, Shockwave, ShockwaveHitEntities, Stats,
     },
@@ -211,19 +211,12 @@ impl<'a> System<'a> for Sys {
                     };
 
                     // PvP check
-                    let owner_if_pet = |entity| {
-                        // Return owner entity if pet,
-                        // or just return entity back otherwise
-                        owner_of(
-                            read_data.alignments.get(entity).copied(),
-                            &read_data.uid_allocator,
-                        )
-                        .unwrap_or(entity)
-                    };
                     let may_harm = combat::may_harm(
-                        shockwave_owner
+                        &read_data.alignments,
-                            .and_then(|owner| read_data.players.get(owner_if_pet(owner))),
+                        &read_data.players,
-                        read_data.players.get(owner_if_pet(target)),
+                        &read_data.uid_allocator,
+                        shockwave_owner,
+                        target,
                     );
                     let attack_options = AttackOptions {
                         // Trying roll during earthquake isn't the best idea

server/src/cmd.rs

@@ -31,7 +31,7 @@ use common::{
     event::{EventBus, ServerEvent},
     generation::EntityInfo,
     npc::{self, get_npc_name},
-    resources::{PlayerPhysicsSettings, TimeOfDay, BattleMode},
+    resources::{BattleMode, PlayerPhysicsSettings, TimeOfDay},
     terrain::{Block, BlockKind, SpriteKind, TerrainChunkSize},
     uid::Uid,
     vol::RectVolSize,

server/src/events/entity_manipulation.rs

@@ -1,7 +1,7 @@
 use crate::{
     client::Client,
     comp::{
-        agent::{owner_of, Agent, AgentEvent, Sound, SoundKind},
+        agent::{Agent, AgentEvent, Sound, SoundKind},
         biped_large, bird_large, quadruped_low, quadruped_medium, quadruped_small,
         skills::SkillGroupKind,
         theropod, BuffKind, BuffSource, PhysicsState,
@@ -945,15 +945,12 @@ pub fn handle_explosion(server: &Server, pos: Vec3<f32>, explosion: Explosion, o
                         };
 
                         // PvP check
-                        let owner_if_pet = |entity| {
-                            // Return owner entity if pet,
-                            // or just return entity back otherwise
-                            owner_of(alignments.get(entity).copied(), uid_allocator)
-                                .unwrap_or(entity)
-                        };
                         let may_harm = combat::may_harm(
-                            owner_entity.and_then(|owner| players.get(owner_if_pet(owner))),
+                            alignments,
-                            players.get(owner_if_pet(entity_b)),
+                            players,
+                            uid_allocator,
+                            owner_entity,
+                            entity_b,
                         );
                         let attack_options = combat::AttackOptions {
                             // cool guyz maybe don't look at explosions
@@ -994,27 +991,22 @@ pub fn handle_explosion(server: &Server, pos: Vec3<f32>, explosion: Explosion, o
                         1.0 - distance_squared / explosion.radius.powi(2)
                     };
 
-                    // Player check only accounts for PvP/PvE flag.
+                    // Player check only accounts for PvP/PvE flag, but bombs
+                    // are intented to do friendly fire.
                     //
-                    // But bombs are intented to do
+                    // What exactly is friendly fire is subject to discussion.
-                    // friendly fire.
-                    //
-                    // What exactly friendly fire is subject to discussion.
                     // As we probably want to minimize possibility of being dick
                     // even to your group members, the only exception is when
                     // you want to harm yourself.
                     //
                     // This can be changed later.
-                    // PvP check
-                    let owner_if_pet = |entity| {
-                        // Return owner entity if pet,
-                        // or just return entity back otherwise
-                        owner_of(alignments.get(entity).copied(), uid_allocator).unwrap_or(entity)
-                    };
                     let may_harm = || {
                         combat::may_harm(
-                            owner_entity.and_then(|owner| players.get(owner_if_pet(owner))),
+                            alignments,
-                            players.get(owner_if_pet(entity_b)),
+                            players,
+                            uid_allocator,
+                            owner_entity,
+                            entity_b,
                         ) || owner_entity.map_or(true, |entity_a| entity_a == entity_b)
                     };
                     if strength > 0.0 {