address review comments

This commit is contained in:
crabman 2024-03-25 19:56:53 +00:00
parent 89af8facbe
commit 4d67e808e1
No known key found for this signature in database
11 changed files with 55 additions and 63 deletions

View File

@ -1,10 +1,8 @@
use crate::{ use crate::{
assets::{self, AssetCombined, Concatenate}, assets::{self, AssetCombined, Concatenate},
combat::GroupTarget,
comp::{ comp::{
self, self, aura::AuraKindVariant, buff::BuffKind, inventory::item::try_all_item_defs,
aura::{AuraKindVariant, SimpleAuraTarget},
buff::BuffKind,
inventory::item::try_all_item_defs,
AdminRole as Role, Skill, AdminRole as Role, Skill,
}, },
generation::try_all_entity_configs, generation::try_all_entity_configs,
@ -443,7 +441,7 @@ impl ServerChatCommand {
Float("aura_radius", 10.0, Required), Float("aura_radius", 10.0, Required),
Float("aura_duration", 10.0, Optional), Float("aura_duration", 10.0, Optional),
Boolean("new_entity", "true".to_string(), Optional), Boolean("new_entity", "true".to_string(), Optional),
Enum("aura_target", SimpleAuraTarget::all_options(), Optional), Enum("aura_target", GroupTarget::all_options(), Optional),
Enum("aura_kind", AuraKindVariant::all_options(), Required), Enum("aura_kind", AuraKindVariant::all_options(), Required),
Any("aura spec", Optional), Any("aura spec", Optional),
], ],
@ -1335,13 +1333,12 @@ macro_rules! impl_from_to_str_cmd {
impl_from_to_str_cmd!(AuraKindVariant, ( impl_from_to_str_cmd!(AuraKindVariant, (
Buff => "buff", Buff => "buff",
FriendlyFire => "friendly_fire", FriendlyFire => "friendly_fire",
IgnorePvE => "ingore_pve" ForcePvP => "force_pvp"
)); ));
impl_from_to_str_cmd!(SimpleAuraTarget, ( impl_from_to_str_cmd!(GroupTarget, (
Group => "group", InGroup => "in_group",
OutOfGroup => "out_of_group", OutOfGroup => "out_of_group"
All => "all"
)); ));
/// Parse a series of command arguments into values, including collecting all /// Parse a series of command arguments into values, including collecting all

View File

@ -90,8 +90,8 @@ pub struct TargetInfo<'a> {
#[derive(Clone, Copy)] #[derive(Clone, Copy)]
pub struct AttackOptions { pub struct AttackOptions {
pub target_dodging: bool, pub target_dodging: bool,
/// Result of [`may_harm`] /// Result of [`permit_pvp`]
pub may_harm: bool, pub permit_pvp: bool,
pub target_group: GroupTarget, pub target_group: GroupTarget,
/// When set to `true`, entities in the same group or pets & pet owners may /// When set to `true`, entities in the same group or pets & pet owners may
/// hit eachother albeit the target_group being OutOfGroup /// hit eachother albeit the target_group being OutOfGroup
@ -275,7 +275,7 @@ impl Attack {
let AttackOptions { let AttackOptions {
target_dodging, target_dodging,
may_harm, permit_pvp,
allow_friendly_fire, allow_friendly_fire,
target_group, target_group,
precision_mult, precision_mult,
@ -288,11 +288,11 @@ impl Attack {
// it should avoid such "damage" or effect, unless friendly fire is enabled // it should avoid such "damage" or effect, unless friendly fire is enabled
let avoid_damage = |attack_damage: &AttackDamage| { let avoid_damage = |attack_damage: &AttackDamage| {
(matches!(attack_damage.target, Some(GroupTarget::OutOfGroup)) && !allow_friendly_fire) (matches!(attack_damage.target, Some(GroupTarget::OutOfGroup)) && !allow_friendly_fire)
&& (target_dodging || !may_harm) && (target_dodging || !permit_pvp)
}; };
let avoid_effect = |attack_effect: &AttackEffect| { let avoid_effect = |attack_effect: &AttackEffect| {
(matches!(attack_effect.target, Some(GroupTarget::OutOfGroup)) && !allow_friendly_fire) (matches!(attack_effect.target, Some(GroupTarget::OutOfGroup)) && !allow_friendly_fire)
&& (target_dodging || !may_harm) && (target_dodging || !permit_pvp)
}; };
let precision_mult = attacker let precision_mult = attacker
.and_then(|a| a.stats) .and_then(|a| a.stats)
@ -811,7 +811,7 @@ pub fn allow_friendly_fire(
/// If both players have PvP mode enabled, interact with NPC and /// If both players have PvP mode enabled, interact with NPC and
/// in any other case, this function will return `true` /// in any other case, this function will return `true`
// TODO: add parameter for doing self-harm? // TODO: add parameter for doing self-harm?
pub fn may_harm( pub fn permit_pvp(
alignments: &ReadStorage<Alignment>, alignments: &ReadStorage<Alignment>,
players: &ReadStorage<Player>, players: &ReadStorage<Player>,
entered_auras: &ReadStorage<EnteredAuras>, entered_auras: &ReadStorage<EnteredAuras>,
@ -849,16 +849,18 @@ pub fn may_harm(
entered_auras.get(target_owner), entered_auras.get(target_owner),
) && attacker_auras ) && attacker_auras
.auras .auras
.get(&AuraKindVariant::IgnorePvE) .get(&AuraKindVariant::ForcePvP)
.zip(target_auras.auras.get(&AuraKindVariant::IgnorePvE)) .zip(target_auras.auras.get(&AuraKindVariant::ForcePvP))
// Only allow forced pvp if both entities are affectd by the same FriendlyFire aura // Only allow forced pvp if both entities are affectd by the same FriendlyFire aura
.is_some_and(|(attacker, target)| attacker.intersection(target).next().is_some()) .is_some_and(|(attacker, target)| attacker.intersection(target).next().is_some())
{ {
return true; return true;
} }
// Prevent owners from attacking their pets and vice versa, unless friendly fire // Prevent PvP between pets, unless friendly fire is enabled
// is enabled //
// This code is NOT intended to prevent pet <-> owner combat,
// pets and their owners being in the same group should take care of that
if attacker_owner == target_owner { if attacker_owner == target_owner {
return allow_friendly_fire(entered_auras, attacker, target); return allow_friendly_fire(entered_auras, attacker, target);
} }

View File

@ -39,7 +39,7 @@ pub enum AuraKind {
pub enum AuraKindVariant { pub enum AuraKindVariant {
Buff, Buff,
FriendlyFire, FriendlyFire,
IgnorePvE, ForcePvP,
} }
/// Aura /// Aura
@ -90,13 +90,6 @@ pub enum AuraTarget {
All, All,
} }
// Only used for parsing in commands
pub enum SimpleAuraTarget {
Group,
OutOfGroup,
All,
}
#[derive(Copy, Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] #[derive(Copy, Clone, Debug, Serialize, Deserialize, PartialEq, Eq)]
pub enum Specifier { pub enum Specifier {
WardingAura, WardingAura,
@ -120,7 +113,7 @@ impl AsRef<AuraKindVariant> for AuraKind {
match self { match self {
AuraKind::Buff { .. } => &AuraKindVariant::Buff, AuraKind::Buff { .. } => &AuraKindVariant::Buff,
AuraKind::FriendlyFire => &AuraKindVariant::FriendlyFire, AuraKind::FriendlyFire => &AuraKindVariant::FriendlyFire,
AuraKind::ForcePvP => &AuraKindVariant::IgnorePvE, AuraKind::ForcePvP => &AuraKindVariant::ForcePvP,
} }
} }
} }

View File

@ -241,12 +241,12 @@ fn activate_aura(
// //
// We don't have this for now, but think about this // We don't have this for now, but think about this
// when we will add this. // when we will add this.
let may_harm = || { let permit_pvp = || {
let owner = match source { let owner = match source {
BuffSource::Character { by } => read_data.id_maps.uid_entity(by), BuffSource::Character { by } => read_data.id_maps.uid_entity(by),
_ => None, _ => None,
}; };
combat::may_harm( combat::permit_pvp(
&read_data.alignments, &read_data.alignments,
&read_data.players, &read_data.players,
&read_data.entered_auras, &read_data.entered_auras,
@ -256,7 +256,7 @@ fn activate_aura(
) )
}; };
conditions_held && (kind.is_buff() || allow_friendly_fire || may_harm()) conditions_held && (kind.is_buff() || allow_friendly_fire || permit_pvp())
}, },
AuraKind::FriendlyFire => true, AuraKind::FriendlyFire => true,
AuraKind::ForcePvP => { AuraKind::ForcePvP => {

View File

@ -251,7 +251,7 @@ impl<'a> System<'a> for Sys {
.and_then(|cs| cs.attack_immunities()) .and_then(|cs| cs.attack_immunities())
.map_or(false, |i| i.beams); .map_or(false, |i| i.beams);
// PvP check // PvP check
let may_harm = combat::may_harm( let permit_pvp = combat::permit_pvp(
&read_data.alignments, &read_data.alignments,
&read_data.players, &read_data.players,
&read_data.entered_auras, &read_data.entered_auras,
@ -284,7 +284,7 @@ impl<'a> System<'a> for Sys {
let attack_options = AttackOptions { let attack_options = AttackOptions {
target_dodging, target_dodging,
may_harm, permit_pvp,
allow_friendly_fire, allow_friendly_fire,
target_group, target_group,
precision_mult, precision_mult,

View File

@ -163,7 +163,7 @@ impl<'a> System<'a> for Sys {
if let Some((_, burning)) = buff_comp.iter_kind(BuffKind::Burning).next() { if let Some((_, burning)) = buff_comp.iter_kind(BuffKind::Burning).next() {
for t_entity in physics_state.touch_entities.keys().filter_map(|te_uid| { for t_entity in physics_state.touch_entities.keys().filter_map(|te_uid| {
read_data.id_maps.uid_entity(*te_uid).filter(|te| { read_data.id_maps.uid_entity(*te_uid).filter(|te| {
combat::may_harm( combat::permit_pvp(
&read_data.alignments, &read_data.alignments,
&read_data.players, &read_data.players,
&read_data.entered_auras, &read_data.entered_auras,

View File

@ -231,7 +231,7 @@ impl<'a> System<'a> for Sys {
}; };
// PvP check // PvP check
let may_harm = combat::may_harm( let permit_pvp = combat::permit_pvp(
&read_data.alignments, &read_data.alignments,
&read_data.players, &read_data.players,
&read_data.entered_auras, &read_data.entered_auras,
@ -269,7 +269,7 @@ impl<'a> System<'a> for Sys {
let attack_options = AttackOptions { let attack_options = AttackOptions {
target_dodging, target_dodging,
may_harm, permit_pvp,
allow_friendly_fire, allow_friendly_fire,
target_group, target_group,
precision_mult, precision_mult,

View File

@ -374,7 +374,7 @@ fn dispatch_hit(
}); });
// PvP check // PvP check
let may_harm = combat::may_harm( let permit_pvp = combat::permit_pvp(
&read_data.alignments, &read_data.alignments,
&read_data.players, &read_data.players,
&read_data.entered_auras, &read_data.entered_auras,
@ -467,7 +467,7 @@ fn dispatch_hit(
let attack_options = AttackOptions { let attack_options = AttackOptions {
target_dodging, target_dodging,
may_harm, permit_pvp,
allow_friendly_fire, allow_friendly_fire,
target_group: projectile_target_info.target_group, target_group: projectile_target_info.target_group,
precision_mult, precision_mult,

View File

@ -194,6 +194,12 @@ impl<'a> System<'a> for Sys {
}; };
// Check if it is a hit // Check if it is a hit
//
// TODO: Should the owner entity really be filtered out here? Unlike other
// attacks, explosions and shockwaves are rather "imprecise"
// attacks with which one shoud be easily able to hit oneself.
// Once we make shockwaves start out a little way out from the center, this can
// be removed.
let hit = entity != target let hit = entity != target
&& shockwave_owner.map_or(true, |owner| owner != target) && shockwave_owner.map_or(true, |owner| owner != target)
&& !health_b.is_dead && !health_b.is_dead
@ -252,7 +258,7 @@ impl<'a> System<'a> for Sys {
ShockwaveDodgeable::No => false, ShockwaveDodgeable::No => false,
}); });
// PvP check // PvP check
let may_harm = combat::may_harm( let permit_pvp = combat::permit_pvp(
&read_data.alignments, &read_data.alignments,
&read_data.players, &read_data.players,
&read_data.entered_auras, &read_data.entered_auras,
@ -264,7 +270,7 @@ impl<'a> System<'a> for Sys {
let precision_mult = None; let precision_mult = None;
let attack_options = AttackOptions { let attack_options = AttackOptions {
target_dodging, target_dodging,
may_harm, permit_pvp,
allow_friendly_fire, allow_friendly_fire,
target_group, target_group,
precision_mult, precision_mult,

View File

@ -28,7 +28,7 @@ use common::{
}, },
comp::{ comp::{
self, self,
aura::{AuraKindVariant, AuraTarget, SimpleAuraTarget}, aura::{AuraKindVariant, AuraTarget},
buff::{Buff, BuffData, BuffKind, BuffSource, MiscBuffData}, buff::{Buff, BuffData, BuffKind, BuffSource, MiscBuffData},
inventory::{ inventory::{
item::{all_items_expect, tool::AbilityMap, MaterialStatManifest, Quality}, item::{all_items_expect, tool::AbilityMap, MaterialStatManifest, Quality},
@ -57,7 +57,8 @@ use common::{
tether::Tethered, tether::Tethered,
uid::Uid, uid::Uid,
vol::ReadVol, vol::ReadVol,
weather, Damage, DamageKind, DamageSource, Explosion, LoadoutBuilder, RadiusEffect, weather, Damage, DamageKind, DamageSource, Explosion, GroupTarget, LoadoutBuilder,
RadiusEffect,
}; };
use common_net::{ use common_net::{
msg::{DisconnectReason, Notification, PlayerListUpdate, ServerGeneral}, msg::{DisconnectReason, Notification, PlayerListUpdate, ServerGeneral},
@ -3980,14 +3981,11 @@ fn handle_aura(
let target_uid = uid(server, target, "target")?; let target_uid = uid(server, target, "target")?;
let (Some(aura_radius), aura_duration, new_entity, aura_target, Some(aura_kind_variant), spec) = let (Some(aura_radius), aura_duration, new_entity, aura_target, Some(aura_kind_variant), spec) =
parse_cmd_args!(args, f32, f32, bool, SimpleAuraTarget, AuraKindVariant, ..Vec<String>) parse_cmd_args!(args, f32, f32, bool, GroupTarget, AuraKindVariant, ..Vec<String>)
else { else {
return Err(Content::Plain(action.help_string())); return Err(Content::Plain(action.help_string()));
}; };
let (new_entity, aura_target) = ( let new_entity = new_entity.unwrap_or(false);
new_entity.unwrap_or(false),
aura_target.unwrap_or(SimpleAuraTarget::OutOfGroup),
);
let aura_kind = match aura_kind_variant { let aura_kind = match aura_kind_variant {
AuraKindVariant::Buff => { AuraKindVariant::Buff => {
let (Some(buff), strength, duration, misc_data_spec) = let (Some(buff), strength, duration, misc_data_spec) =
@ -4026,15 +4024,15 @@ fn handle_aura(
} }
}, },
AuraKindVariant::FriendlyFire => AuraKind::FriendlyFire, AuraKindVariant::FriendlyFire => AuraKind::FriendlyFire,
AuraKindVariant::IgnorePvE => AuraKind::ForcePvP, AuraKindVariant::ForcePvP => AuraKind::ForcePvP,
}; };
let aura_target = server let aura_target = server
.state .state
.read_component_copied::<Uid>(target) .read_component_copied::<Uid>(target)
.map(|uid| match aura_target { .map(|uid| match aura_target {
SimpleAuraTarget::Group => AuraTarget::GroupOf(uid), Some(GroupTarget::InGroup) => AuraTarget::GroupOf(uid),
SimpleAuraTarget::OutOfGroup => AuraTarget::NotGroupOf(uid), Some(GroupTarget::OutOfGroup) => AuraTarget::NotGroupOf(uid),
SimpleAuraTarget::All => AuraTarget::All, None => AuraTarget::All,
}) })
.unwrap_or(AuraTarget::All); .unwrap_or(AuraTarget::All);

View File

@ -1203,9 +1203,7 @@ impl ServerEvent for ExplosionEvent {
), ),
) )
.join() .join()
.filter(|(e, _, h, _)| { .filter(|(_, _, h, _)| !h.is_dead)
!h.is_dead && owner_entity.map_or(true, |owner| owner != *e)
})
{ {
let dist_sqrd = ev.pos.distance_squared(pos_b.0); let dist_sqrd = ev.pos.distance_squared(pos_b.0);
@ -1289,7 +1287,7 @@ impl ServerEvent for ExplosionEvent {
) )
}); });
// PvP check // PvP check
let may_harm = combat::may_harm( let permit_pvp = combat::permit_pvp(
&alignments, &alignments,
&players, &players,
&entered_auras, &entered_auras,
@ -1299,7 +1297,7 @@ impl ServerEvent for ExplosionEvent {
); );
let attack_options = combat::AttackOptions { let attack_options = combat::AttackOptions {
target_dodging, target_dodging,
may_harm, permit_pvp,
allow_friendly_fire, allow_friendly_fire,
target_group, target_group,
precision_mult: None, precision_mult: None,
@ -1323,9 +1321,7 @@ impl ServerEvent for ExplosionEvent {
}, },
RadiusEffect::Entity(mut effect) => { RadiusEffect::Entity(mut effect) => {
for (entity_b, pos_b, body_b_maybe) in for (entity_b, pos_b, body_b_maybe) in
(&entities, &positions, bodies.maybe()) (&entities, &positions, bodies.maybe()).join()
.join()
.filter(|(e, _, _)| owner_entity.map_or(true, |owner| owner != *e))
{ {
let strength = if let Some(body) = body_b_maybe { let strength = if let Some(body) = body_b_maybe {
cylinder_sphere_strength( cylinder_sphere_strength(
@ -1350,8 +1346,8 @@ impl ServerEvent for ExplosionEvent {
// you want to harm yourself. // you want to harm yourself.
// //
// This can be changed later. // This can be changed later.
let may_harm = || { let permit_pvp = || {
combat::may_harm( combat::permit_pvp(
&alignments, &alignments,
&players, &players,
&entered_auras, &entered_auras,
@ -1365,7 +1361,7 @@ impl ServerEvent for ExplosionEvent {
if is_alive { if is_alive {
effect.modify_strength(strength); effect.modify_strength(strength);
if !effect.is_harm() || may_harm() { if !effect.is_harm() || permit_pvp() {
emit_effect_events( emit_effect_events(
&mut emitters, &mut emitters,
*time, *time,