From 0b92c7209643d99076d92650a1e585e70a101ffa Mon Sep 17 00:00:00 2001 From: Imbris Date: Sun, 6 Jun 2021 18:36:25 -0400 Subject: [PATCH] Make handling of shortcut keywords for commands clearer and revise a TODO related to how chat messages are stored and renamed with alias changes --- client/src/cmd.rs | 12 ++--- client/src/lib.rs | 8 ++- common/src/cmd.rs | 133 ++++++++++++---------------------------------- server/src/cmd.rs | 14 ++--- 4 files changed, 55 insertions(+), 112 deletions(-) diff --git a/client/src/cmd.rs b/client/src/cmd.rs index 7c5502ba8b..aafacd4347 100644 --- a/client/src/cmd.rs +++ b/client/src/cmd.rs @@ -52,12 +52,12 @@ fn complete_player(part: &str, client: &Client) -> Vec { } fn complete_command(part: &str) -> Vec { - CHAT_SHORTCUTS - .keys() - .map(ToString::to_string) - .chain(CHAT_COMMANDS.iter().map(ToString::to_string)) - .filter(|kwd| kwd.starts_with(part) || format!("/{}", kwd).starts_with(part)) - .map(|c| format!("/{}", c)) + let part = part.strip_prefix('/').unwrap_or(part); + + ChatCommand::iter_with_keywords() + .map(|(kwd, _)| kwd) + .filter(|kwd| kwd.starts_with(part)) + .map(|kwd| format!("/{}", kwd)) .collect() } diff --git a/client/src/lib.rs b/client/src/lib.rs index df1c802071..661c2ad5f7 100644 --- a/client/src/lib.rs +++ b/client/src/lib.rs @@ -1712,8 +1712,12 @@ impl Client { // Instead of removing players, mark them as offline because we need to // remember the names of disconnected players in chat. // - // TODO the server should re-use uids of players that log out and log back - // in. + // TODO: consider alternatives since this leads to an ever growing list as + // players log out and in. Keep in mind we might only want to + // keep only so many messages in chat the history. We could + // potentially use an ID that's more persistent than the Uid. + // One of the reasons we don't just store the string of the player name + // into the message is to make alias changes reflected in older messages. if let Some(player_info) = self.player_list.get_mut(&uid) { if player_info.is_online { diff --git a/common/src/cmd.rs b/common/src/cmd.rs index 5ec88c379d..e8f9054605 100644 --- a/common/src/cmd.rs +++ b/common/src/cmd.rs @@ -40,7 +40,7 @@ impl ChatCommandData { } // Please keep this sorted alphabetically :-) -#[derive(Copy, Clone)] +#[derive(Copy, Clone, strum_macros::EnumIter)] pub enum ChatCommand { Adminify, Airship, @@ -106,72 +106,6 @@ pub enum ChatCommand { World, } -// Thank you for keeping this sorted alphabetically :-) -pub static CHAT_COMMANDS: &[ChatCommand] = &[ - ChatCommand::Adminify, - ChatCommand::Airship, - ChatCommand::Alias, - ChatCommand::ApplyBuff, - ChatCommand::Ban, - ChatCommand::Build, - ChatCommand::BuildAreaAdd, - ChatCommand::BuildAreaList, - ChatCommand::BuildAreaRemove, - ChatCommand::Campfire, - ChatCommand::DebugColumn, - ChatCommand::DisconnectAllPlayers, - ChatCommand::DropAll, - ChatCommand::Dummy, - ChatCommand::Explosion, - ChatCommand::Faction, - ChatCommand::GiveItem, - ChatCommand::Goto, - ChatCommand::Group, - ChatCommand::GroupInvite, - ChatCommand::GroupKick, - ChatCommand::GroupLeave, - ChatCommand::GroupPromote, - ChatCommand::Health, - ChatCommand::Help, - ChatCommand::Home, - ChatCommand::JoinFaction, - ChatCommand::Jump, - ChatCommand::Kick, - ChatCommand::Kill, - ChatCommand::KillNpcs, - ChatCommand::Kit, - ChatCommand::Lantern, - ChatCommand::Light, - ChatCommand::MakeBlock, - ChatCommand::MakeSprite, - ChatCommand::Motd, - ChatCommand::Object, - ChatCommand::PermitBuild, - ChatCommand::Players, - ChatCommand::Region, - ChatCommand::RemoveLights, - ChatCommand::RevokeBuild, - ChatCommand::RevokeBuildAll, - ChatCommand::Safezone, - ChatCommand::Say, - ChatCommand::ServerPhysics, - ChatCommand::SetMotd, - ChatCommand::Site, - ChatCommand::SkillPoint, - ChatCommand::SkillPreset, - ChatCommand::Spawn, - ChatCommand::Sudo, - ChatCommand::Tell, - ChatCommand::Time, - ChatCommand::Tp, - ChatCommand::Unban, - ChatCommand::Version, - ChatCommand::Waypoint, - ChatCommand::Whitelist, - ChatCommand::Wiring, - ChatCommand::World, -]; - #[derive(Debug, PartialEq, Serialize, Deserialize, Clone)] pub struct KitManifest(pub HashMap>); impl assets::Asset for KitManifest { @@ -189,15 +123,6 @@ impl assets::Asset for SkillPresetManifest { } lazy_static! { - pub static ref CHAT_SHORTCUTS: HashMap = [ - ('f', ChatCommand::Faction), - ('g', ChatCommand::Group), - ('r', ChatCommand::Region), - ('s', ChatCommand::Say), - ('t', ChatCommand::Tell), - ('w', ChatCommand::World), - ].iter().cloned().collect(); - static ref ALIGNMENTS: Vec = vec!["wild", "enemy", "npc", "pet"] .iter() .map(|s| s.to_string()) @@ -740,6 +665,20 @@ impl ChatCommand { } } + /// The short keyword used to invoke the command, omitting the leading '/'. + /// Returns None if the command doesn't have a short keyword + pub fn short_keyword(&self) -> Option<&'static str> { + Some(match self { + ChatCommand::Faction => "f", + ChatCommand::Group => "g", + ChatCommand::Region => "r", + ChatCommand::Say => "s", + ChatCommand::Tell => "t", + ChatCommand::World => "w", + _ => return None, + }) + } + /// A message that explains what the command does pub fn help_string(&self) -> String { let data = self.data(); @@ -773,6 +712,19 @@ impl ChatCommand { .collect::>() .join(" ") } + + /// Produce an iterator over all the available commands + pub fn iter() -> impl Iterator { ::iter() } + + /// Produce an iterator that first goes over all the short keywords + /// and their associated commands and then iterates over all the normal + /// keywords with their associated commands + pub fn iter_with_keywords() -> impl Iterator { + Self::iter() + // Go through all the shortcuts first + .filter_map(|c| c.short_keyword().map(|s| (s, c))) + .chain(Self::iter().map(|c| (c.keyword(), c))) + } } impl Display for ChatCommand { @@ -785,28 +737,13 @@ impl FromStr for ChatCommand { type Err = (); fn from_str(keyword: &str) -> Result { - let kwd = if let Some(stripped) = keyword.strip_prefix('/') { - stripped - } else { - &keyword - }; - if keyword.len() == 1 { - if let Some(c) = keyword - .chars() - .next() - .as_ref() - .and_then(|k| CHAT_SHORTCUTS.get(k)) - { - return Ok(*c); - } - } else { - for c in CHAT_COMMANDS { - if kwd == c.keyword() { - return Ok(*c); - } - } - } - Err(()) + let keyword = keyword.strip_prefix('/').unwrap_or(keyword); + + Self::iter_with_keywords() + // Find command with matching string as keyword + .find_map(|(kwd, command)| (kwd == keyword).then(|| command)) + // Return error if not found + .ok_or(()) } } diff --git a/server/src/cmd.rs b/server/src/cmd.rs index c62e27a1a9..a8f747e5b5 100644 --- a/server/src/cmd.rs +++ b/server/src/cmd.rs @@ -14,7 +14,7 @@ use authc::Uuid; use chrono::{NaiveTime, Timelike, Utc}; use common::{ assets, - cmd::{ChatCommand, BUFF_PACK, BUFF_PARSER, CHAT_COMMANDS, CHAT_SHORTCUTS}, + cmd::{ChatCommand, BUFF_PACK, BUFF_PARSER}, comp::{ self, aura::{Aura, AuraKind, AuraTarget}, @@ -1526,17 +1526,19 @@ fn handle_help( let entity_role = server.entity_admin_role(client); // Iterate through all commands you have permission to use. - CHAT_COMMANDS - .iter() + ChatCommand::iter() .filter(|cmd| cmd.needs_role() <= entity_role) .for_each(|cmd| { message += &cmd.help_string(); message += "\n"; }); message += "Additionally, you can use the following shortcuts:"; - for (k, v) in CHAT_SHORTCUTS.iter() { - message += &format!(" /{} => /{}", k, v.keyword()); - } + ChatCommand::iter() + .filter_map(|cmd| cmd.short_keyword().map(|k| (k, cmd))) + .for_each(|(k, cmd)| { + message += &format!(" /{} => /{}", k, cmd.keyword()); + }); + server.notify_client( client, ServerGeneral::server_msg(ChatType::CommandInfo, message),