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

@@ -52,12 +52,12 @@ fn complete_player(part: &str, client: &Client) -> Vec<String> {
 }
 
 fn complete_command(part: &str) -> Vec<String> {
-    CHAT_SHORTCUTS
+    let part = part.strip_prefix('/').unwrap_or(part);
-        .keys()
+
-        .map(ToString::to_string)
+    ChatCommand::iter_with_keywords()
-        .chain(CHAT_COMMANDS.iter().map(ToString::to_string))
+        .map(|(kwd, _)| kwd)
-        .filter(|kwd| kwd.starts_with(part) || format!("/{}", kwd).starts_with(part))
+        .filter(|kwd| kwd.starts_with(part))
-        .map(|c| format!("/{}", c))
+        .map(|kwd| format!("/{}", kwd))
         .collect()
 }
 

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
+                // TODO: consider alternatives since this leads to an ever growing list as
-                // in.
+                // 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 {

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<String, Vec<(String, u32)>>);
 impl assets::Asset for KitManifest {
@@ -189,15 +123,6 @@ impl assets::Asset for SkillPresetManifest {
 }
 
 lazy_static! {
-    pub static ref CHAT_SHORTCUTS: HashMap<char, ChatCommand> = [
-        ('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<String> = 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::<Vec<_>>()
             .join(" ")
     }
+
+    /// Produce an iterator over all the available commands
+    pub fn iter() -> impl Iterator<Item = Self> { <Self as strum::IntoEnumIterator>::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<Item = (&'static str, Self)> {
+        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<ChatCommand, ()> {
-        let kwd = if let Some(stripped) = keyword.strip_prefix('/') {
+        let keyword = keyword.strip_prefix('/').unwrap_or(keyword);
-            stripped
+
-        } else {
+        Self::iter_with_keywords()
-            &keyword
+            // Find command with matching string as keyword
-        };
+            .find_map(|(kwd, command)| (kwd == keyword).then(|| command))
-        if keyword.len() == 1 {
+            // Return error if not found
-            if let Some(c) = keyword
+            .ok_or(())
-                .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(())
     }
 }
 

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
+        ChatCommand::iter()
-            .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() {
+        ChatCommand::iter()
-            message += &format!(" /{} => /{}", k, v.keyword());
+            .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),