diff --git a/common/src/comp/chat.rs b/common/src/comp/chat.rs index f8989577da..51620b2c5e 100644 --- a/common/src/comp/chat.rs +++ b/common/src/comp/chat.rs @@ -18,7 +18,7 @@ pub enum ChatMode { /// Talk to players in your region of the world Region, /// Talk to your current group of players - Group(Group), + Group, /// Talk to your faction Faction(String), /// Talk to every player on the server @@ -31,19 +31,31 @@ impl Component for ChatMode { impl ChatMode { /// Create a plain message from your current chat mode and uuid. - pub fn to_plain_msg(&self, from: Uid, text: impl ToString) -> UnresolvedChatMsg { + pub fn to_plain_msg( + &self, + from: Uid, + text: impl ToString, + group: Option, + ) -> Result { let chat_type = match self { ChatMode::Tell(to) => ChatType::Tell(from, *to), ChatMode::Say => ChatType::Say(from), ChatMode::Region => ChatType::Region(from), - ChatMode::Group(group) => ChatType::Group(from, *group), + ChatMode::Group => ChatType::Group( + from, + group.ok_or( + "You tried sending a group message while not belonging to a group. Use /world \ + or /region to change your chat mode.", + )?, + ), ChatMode::Faction(faction) => ChatType::Faction(from, faction.clone()), ChatMode::World => ChatType::World(from), }; - UnresolvedChatMsg { + + Ok(UnresolvedChatMsg { chat_type, content: Content::Plain(text.to_string()), - } + }) } } @@ -346,10 +358,10 @@ impl GenericChatMsg { } } - pub fn get_group_mut_sender(&mut self) -> Option<(&mut G, Option<&Uid>)> { - match &mut self.chat_type { - ChatType::GroupMeta(g) => Some((g, None)), - ChatType::Group(sender, g) => Some((g, Some(sender))), + pub fn get_group(&self) -> Option<&G> { + match &self.chat_type { + ChatType::GroupMeta(g) => Some(g), + ChatType::Group(_, g) => Some(g), _ => None, } } diff --git a/server/src/cmd.rs b/server/src/cmd.rs index 6072e3cd3a..f9643126e5 100644 --- a/server/src/cmd.rs +++ b/server/src/cmd.rs @@ -2888,7 +2888,9 @@ fn handle_tell( } else { message_opt.join(" ") }; - server.state.send_chat(mode.to_plain_msg(target_uid, msg)); + server + .state + .send_chat(mode.to_plain_msg(target_uid, msg, None)?); server.notify_client(target, ServerGeneral::ChatMode(mode)); Ok(()) } else { @@ -2913,7 +2915,7 @@ fn handle_faction( let msg = args.join(" "); if !msg.is_empty() { if let Some(uid) = server.state.ecs().read_storage().get(target) { - server.state.send_chat(mode.to_plain_msg(*uid, msg)); + server.state.send_chat(mode.to_plain_msg(*uid, msg, None)?); } } server.notify_client(target, ServerGeneral::ChatMode(mode)); @@ -2933,14 +2935,16 @@ fn handle_group( no_sudo(client, target)?; let groups = server.state.ecs().read_storage::(); - if let Some(group) = groups.get(target) { - let mode = comp::ChatMode::Group(*group); + if let Some(group) = groups.get(target).copied() { + let mode = comp::ChatMode::Group; drop(groups); insert_or_replace_component(server, target, mode.clone(), "target")?; let msg = args.join(" "); if !msg.is_empty() { if let Some(uid) = server.state.ecs().read_storage().get(target) { - server.state.send_chat(mode.to_plain_msg(*uid, msg)); + server + .state + .send_chat(mode.to_plain_msg(*uid, msg, Some(group))?); } } server.notify_client(target, ServerGeneral::ChatMode(mode)); @@ -3064,7 +3068,7 @@ fn handle_region( let msg = args.join(" "); if !msg.is_empty() { if let Some(uid) = server.state.ecs().read_storage().get(target) { - server.state.send_chat(mode.to_plain_msg(*uid, msg)); + server.state.send_chat(mode.to_plain_msg(*uid, msg, None)?); } } server.notify_client(target, ServerGeneral::ChatMode(mode)); @@ -3085,7 +3089,7 @@ fn handle_say( let msg = args.join(" "); if !msg.is_empty() { if let Some(uid) = server.state.ecs().read_storage().get(target) { - server.state.send_chat(mode.to_plain_msg(*uid, msg)); + server.state.send_chat(mode.to_plain_msg(*uid, msg, None)?); } } server.notify_client(target, ServerGeneral::ChatMode(mode)); @@ -3106,7 +3110,7 @@ fn handle_world( let msg = args.join(" "); if !msg.is_empty() { if let Some(uid) = server.state.ecs().read_storage().get(target) { - server.state.send_chat(mode.to_plain_msg(*uid, msg)); + server.state.send_chat(mode.to_plain_msg(*uid, msg, None)?); } } server.notify_client(target, ServerGeneral::ChatMode(mode)); diff --git a/server/src/state_ext.rs b/server/src/state_ext.rs index 4562b0c84a..25672f1bd6 100644 --- a/server/src/state_ext.rs +++ b/server/src/state_ext.rs @@ -908,7 +908,7 @@ impl StateExt for State { /// Send the chat message to the proper players. Say and region are limited /// by location. Faction and group are limited by component. - fn send_chat(&self, mut msg: comp::UnresolvedChatMsg) { + fn send_chat(&self, msg: comp::UnresolvedChatMsg) { let ecs = self.ecs(); let is_within = |target, a: &comp::Pos, b: &comp::Pos| a.0.distance_squared(b.0) < target * target; @@ -918,31 +918,7 @@ impl StateExt for State { let group_manager = ecs.read_resource::(); - let group_info = msg.get_group_mut_sender().and_then(|(g, from)| { - // Check if the sender is in the group he is trying to send the message to. If - // it doesn't match, update the message's group accordingly. - // There currently is a bug that can cause group messages to appear in other - // groups. This commonly happens when a player has his [ChatMode] set - // to group and is kicked from his current group or joins another, - // any message sent afterwards appears in the previous group as long - // as his chatmode isn't updated. - // TODO: Find out why sometimes messages from players never previously in the - // group leak. - // FIXME: Another pontential fix could be updating the [ChatMode] as the player - // leaves and joins a new group. Possibly a better solution? - if let Some(from) = from { - let group = ecs - .read_storage::() - .get(entity_from_uid(*from)?) - .copied()?; - - if g != &group { - *g = group; - } - } - - group_manager.group_info(*g) - }); + let group_info = msg.get_group().and_then(|g| group_manager.group_info(*g)); let resolved_msg = msg .clone() @@ -1100,7 +1076,12 @@ impl StateExt for State { }, comp::ChatType::Group(from, g) => { if group_info.is_none() { - // group not found, reply with command error + // Group not found, reply with command error + // This should usually NEVER happen since now it is checked whether the + // sender is still in the group upon emitting the message (TODO: Can this be + // triggered if the message is sent in the same tick as the sender is + // removed from the group?) + let reply = comp::ChatType::CommandError.into_plain_msg( "You are using group chat but do not belong to a group. Use /world or \ /region to change chat.", diff --git a/server/src/sys/msg/general.rs b/server/src/sys/msg/general.rs index 17162b3227..3d3b6605be 100644 --- a/server/src/sys/msg/general.rs +++ b/server/src/sys/msg/general.rs @@ -1,12 +1,12 @@ use crate::client::Client; use common::{ - comp::{ChatMode, Player}, + comp::{ChatMode, ChatType, Group, Player}, event::{EventBus, ServerEvent}, resources::Time, uid::Uid, }; use common_ecs::{Job, Origin, Phase, System}; -use common_net::msg::ClientGeneral; +use common_net::msg::{ClientGeneral, ServerGeneral}; use rayon::prelude::*; use specs::{Entities, Join, ParJoin, Read, ReadStorage, WriteStorage}; use tracing::{debug, error, warn}; @@ -15,10 +15,11 @@ impl Sys { fn handle_general_msg( server_emitter: &mut common::event::Emitter<'_, ServerEvent>, entity: specs::Entity, - _client: &Client, + client: &Client, player: Option<&Player>, uids: &ReadStorage<'_, Uid>, chat_modes: &ReadStorage<'_, ChatMode>, + groups: &ReadStorage<'_, Group>, msg: ClientGeneral, ) -> Result<(), crate::error::Error> { match msg { @@ -27,8 +28,17 @@ impl Sys { if let Some(from) = uids.get(entity) { const CHAT_MODE_DEFAULT: &ChatMode = &ChatMode::default(); let mode = chat_modes.get(entity).unwrap_or(CHAT_MODE_DEFAULT); - // Send chat message - server_emitter.emit(ServerEvent::Chat(mode.to_plain_msg(*from, message))); + // Try sending the chat message + match mode.to_plain_msg(*from, message, groups.get(entity).copied()) { + Ok(message) => { + server_emitter.emit(ServerEvent::Chat(message)); + }, + Err(error) => { + client.send_fallible(ServerGeneral::ChatMsg( + ChatType::CommandError.into_plain_msg(error), + )); + }, + } } else { error!("Could not send message. Missing player uid"); } @@ -71,6 +81,7 @@ impl<'a> System<'a> for Sys { ReadStorage<'a, Uid>, ReadStorage<'a, ChatMode>, ReadStorage<'a, Player>, + ReadStorage<'a, Group>, WriteStorage<'a, Client>, ); @@ -80,7 +91,7 @@ impl<'a> System<'a> for Sys { fn run( _job: &mut Job, - (entities, server_event_bus, time, uids, chat_modes, players, mut clients): Self::SystemData, + (entities, server_event_bus, time, uids, chat_modes, players, groups, mut clients): Self::SystemData, ) { (&entities, &mut clients, players.maybe()) .par_join() @@ -95,6 +106,7 @@ impl<'a> System<'a> for Sys { player, &uids, &chat_modes, + &groups, msg, ) }); diff --git a/voxygen/src/hud/chat.rs b/voxygen/src/hud/chat.rs index 02e4479338..06286f0ed7 100644 --- a/voxygen/src/hud/chat.rs +++ b/voxygen/src/hud/chat.rs @@ -801,7 +801,7 @@ fn render_chat_mode(chat_mode: &ChatMode, imgs: &Imgs) -> (Color, conrod_core::i ChatMode::Say => (SAY_COLOR, imgs.chat_say_small), ChatMode::Region => (REGION_COLOR, imgs.chat_region_small), ChatMode::Faction(_) => (FACTION_COLOR, imgs.chat_faction_small), - ChatMode::Group(_) => (GROUP_COLOR, imgs.chat_group_small), + ChatMode::Group => (GROUP_COLOR, imgs.chat_group_small), ChatMode::Tell(_) => (TELL_COLOR, imgs.chat_tell_small), } }