Do not process commands coming from ourselves (help text)

Quite a few changes involved. Mostly variable renames and a few changes to `await`s.

Not ready yet because bot cannot login due to some arcane error of `expected value at line 1 column 1`.

Co-authored-by: projectmoon <projectmoon@noreply.git.agnos.is>
Reviewed-on: projectmoon/tenebrous-dicebot#98

.drone.yml

@@ -3,7 +3,7 @@ name: build-and-test
 
 steps:
 - name: test
-  image: rust:1.51
+  image: rust:1.80
   commands:
   - apt-get update
   - apt-get install -y cmake

Dockerfile

@@ -1,16 +1,15 @@
 # Builder image with development dependencies.
-FROM bougyman/voidlinux:glibc as builder
+FROM ghcr.io/void-linux/void-linux:latest-mini-x86_64 as builder
+RUN xbps-install -S
+RUN xbps-install -yu xbps
 RUN xbps-install -Syu
-RUN xbps-install -Sy base-devel rustup cargo cmake wget gnupg
+RUN xbps-install -Sy base-devel rustup cmake wget gnupg
 RUN xbps-install -Sy openssl-devel libstdc++-devel
 RUN rustup-init -qy
 
 # Install tini for signal processing and zombie killing
 ENV TINI_VERSION v0.19.0
 ADD https://github.com/krallin/tini/releases/download/${TINI_VERSION}/tini /usr/local/bin/tini
-ADD https://github.com/krallin/tini/releases/download/${TINI_VERSION}/tini.asc /tini.asc
-RUN gpg --batch --keyserver hkp://keyserver.ubuntu.com --recv-keys 595E85A6B1B4779EA4DAAEC70B588DFF0527A9B7 \
- && gpg --batch --verify /tini.asc /usr/local/bin/tini
 RUN chmod +x /usr/local/bin/tini
 
 # Build dicebot
@@ -20,7 +19,10 @@ ADD . ./
 RUN . /root/.cargo/env && cargo build --release
 
 # Final image
-FROM bougyman/voidlinux:tiny
+FROM ghcr.io/void-linux/void-linux:latest-mini-x86_64
+RUN xbps-install -S
+RUN xbps-install -yu xbps
+RUN xbps-install -Syu
 RUN xbps-install -Sy ca-certificates libstdc++
 COPY --from=builder \
     /root/src/target/release/dicebot \

README.md

@@ -278,6 +278,7 @@ The most basic plans are:
 * Perhaps some sort of character sheet integration. But for that, we
   would need a sheet service.
 * Use environment variables instead of config file in Docker image.
+* Per-system game rules.
 
 ## Credits
 

dicebot/Cargo.toml

@@ -1,6 +1,7 @@
 [package]
 name = "tenebrous-dicebot"
-version = "0.13.1"
+version = "0.13.2"
+rust-version = "1.68"
 authors = ["projectmoon <projectmoon@agnos.is>", "Taylor C. Richberger <taywee@gmx.com>"]
 edition = "2018"
 license = 'AGPL-3.0-or-later'
@@ -15,7 +16,7 @@ tonic-build = "0.4"
 
 [dependencies]
 # indexmap version locked fixes a dependency cycle.
-indexmap = "=1.6.2"
+# indexmap = "=1.6.2"
 log = "0.4"
 tracing-subscriber = "0.2"
 toml = "0.5"
@@ -32,9 +33,10 @@ combine = "4.5"
 futures = "0.3"
 html2text = "0.2"
 phf = { version = "0.8", features = ["macros"] }
-matrix-sdk = { version = "0.3" }
-refinery = { version = "0.6", features = ["rusqlite"]}
-barrel = { version = "0.6", features = ["sqlite3"] }
+matrix-sdk = { version = "0.6" }
+refinery = { version = "0.8", features = ["rusqlite"]}
+barrel = { version = "0.7", features = ["sqlite3"] }
+strum = { version = "0.22", features = ["derive"] }
 tempfile = "3"
 substring = "1.4"
 fuse-rust = "0.2"
@@ -43,7 +45,7 @@ prost = "0.7"
 tenebrous-rpc = { path = "../rpc", version = "0.1.0" }
 
 [dependencies.sqlx]
-version = "0.5"
+version = "0.6"
 features = [ "offline", "sqlite", "runtime-tokio-native-tls" ]
 
 [dependencies.serde]

dicebot/Dockerfile

@@ -1,35 +0,0 @@
-# Builder image with development dependencies.
-FROM bougyman/voidlinux:glibc as builder
-RUN xbps-install -Syu
-RUN xbps-install -Sy base-devel rustup cargo cmake wget gnupg
-RUN xbps-install -Sy openssl-devel libstdc++-devel
-RUN rustup-init -qy
-RUN rustup component add rustfmt # Needed for protobuf building.
-
-# Install tini for signal processing and zombie killing
-ENV TINI_VERSION v0.19.0
-ADD https://github.com/krallin/tini/releases/download/${TINI_VERSION}/tini /usr/local/bin/tini
-ADD https://github.com/krallin/tini/releases/download/${TINI_VERSION}/tini.asc /tini.asc
-RUN gpg --batch --keyserver hkp://p80.pool.sks-keyservers.net:80 --recv-keys 595E85A6B1B4779EA4DAAEC70B588DFF0527A9B7 \
- && gpg --batch --verify /tini.asc /usr/local/bin/tini
-RUN chmod +x /usr/local/bin/tini
-
-# Build dicebot
-RUN mkdir -p /root/src
-WORKDIR /root/src
-ADD . ./
-RUN . /root/.cargo/env && cargo build --release
-
-# Final image
-FROM bougyman/voidlinux:tiny
-RUN xbps-install -Sy ca-certificates libstdc++
-COPY --from=builder \
-    /root/src/target/release/dicebot \
-    /usr/local/bin/
-COPY --from=builder \
-    /usr/local/bin/tini \
-    /usr/local/bin/
-
-ENV XDG_CACHE_HOME "/cache"
-ENV DATABASE_PATH "/cache/bot-db"
-ENTRYPOINT [ "/usr/local/bin/tini", "-v", "--", "/usr/local/bin/dicebot", "/config/dicebot-config.toml" ]

dicebot/src/basic/dice.rs

@@ -6,8 +6,9 @@
 use std::fmt;
 use std::ops::{Deref, DerefMut};
 
-//Old stuff, for regular dice rolling. To be moved elsewhere.
-
+/// A basic dice roll, in XdY notation, like "1d4" or "3d6".
+/// Optionally supports D&D advantage/disadvantge keep-or-drop
+/// functionality.
 #[derive(Debug, PartialEq, Eq, Clone, Copy)]
 pub struct Dice {
     pub(crate) count: u32,
@@ -15,26 +16,42 @@ pub struct Dice {
     pub(crate) keep_drop: KeepOrDrop,
 }
 
+/// Enum indicating how to handle bonuses or penalties using extra
+/// dice. If set to Keep, the roll will keep the highest X number of
+/// dice in the roll, and add those together. If set to Drop, the
+/// opposite is performed, and the lowest X number of dice are added
+/// instead. If set to None, then all dice in the roll are added up as
+/// normal.
+#[derive(Debug, PartialEq, Eq, Clone, Copy)]
+pub enum KeepOrDrop {
+    /// Keep only the X highest dice for adding up to the total.
+    Keep(u32),
+
+    /// Keep only the X lowest dice (i.e. drop the highest) for adding
+    /// up to the total.
+    Drop(u32),
+
+    /// Add up all dice in the roll for the total.
+    None,
+}
+
 impl fmt::Display for Dice {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self.keep_drop {
             KeepOrDrop::Keep(keep) => write!(f, "{}d{}k{}", self.count, self.sides, keep),
             KeepOrDrop::Drop(drop) => write!(f, "{}d{}dh{}", self.count, self.sides, drop),
-            KeepOrDrop::None       => write!(f, "{}d{}", self.count, self.sides),
+            KeepOrDrop::None => write!(f, "{}d{}", self.count, self.sides),
         }
     }
 }
 
-#[derive(Debug, PartialEq, Eq, Clone, Copy)]
-pub enum KeepOrDrop {
-    Keep (u32),
-    Drop (u32),
-    None,
-}
-
 impl Dice {
     pub fn new(count: u32, sides: u32, keep_drop: KeepOrDrop) -> Dice {
-        Dice { count, sides, keep_drop }
+        Dice {
+            count,
+            sides,
+            keep_drop,
+        }
     }
 }
 

dicebot/src/basic/parser.rs

@@ -4,6 +4,8 @@
  * project.
  */
 use nom::bytes::complete::take_while;
+use nom::error::ErrorKind as NomErrorKind;
+use nom::Err as NomErr;
 use nom::{
     alt, bytes::complete::tag, character::complete::digit1, complete, many0, named,
     sequence::tuple, tag, IResult,
@@ -31,54 +33,74 @@ enum Sign {
     Minus,
 }
 
+/// Intermediate parsed value for a keep-drop expression to indicate
+/// which one it is.
+enum ParsedKeepOrDrop<'a> {
+    Keep(&'a str),
+    Drop(&'a str),
+    NotPresent,
+}
+
+macro_rules! too_big {
+    ($input: expr) => {
+        NomErr::Error(($input, NomErrorKind::TooLarge))
+    };
+}
+
 /// Parse a dice expression.  Does not eat whitespace
 fn parse_dice(input: &str) -> IResult<&str, Dice> {
-    // parse main dice expression
     let (input, (count, _, sides)) = tuple((digit1, tag("d"), digit1))(input)?;
+    let count: u32 = count.parse().map_err(|_| too_big!(count))?;
+    let sides = sides.parse().map_err(|_| too_big!(sides))?;
+    let (input, keep_drop) = parse_keep_or_drop(input, count)?;
+    Ok((input, Dice::new(count, sides, keep_drop)))
+}
 
-    // check for keep expression to keep highest dice (2d20k1)
-    let (keep, input) = match tuple::<&str, _, (_, _), _>((tag("k"), digit1))(input) {
-        // if ok, keep expression is present
-        Ok((rest, (_, keep_amount))) => (keep_amount, rest),
-        // otherwise absent and keep all dice
-        Err(_) => ("", input)
-    };
-
-    // check for drop expression to drop highest dice (2d20dh1)
-    let (drop, input) = match tuple::<&str, _, (_, _), _>((tag("dh"), digit1))(input) {
-        // if ok, keep expression is present
-        Ok((rest, (_, drop_amount))) => (drop_amount, rest),
-        // otherwise absent and keep all dice
-        Err(_) => ("", input)
-    };
-
-    let count: u32 = count.parse().unwrap();
-
-    // don't allow keep greater than number of dice, and don't allow keep zero
-    let keep_drop = match keep.parse::<u32>() {
-        // Ok, there's a keep value, check and create Keep
-        Ok(i) => match i {
-            _i if _i > count || _i == 0 => KeepOrDrop::None,
-            i => KeepOrDrop::Keep(i),
-        },
-        // Err, check if drop works
-        Err(_) => {
-            match drop.parse::<u32>() {
-                // Ok, there's a drop value, check and create Drop
-                Ok(i) => match i {
-                    _i if i >= count => KeepOrDrop::None,
-                    i => KeepOrDrop::Drop(i),
-                },
-                // Err, there's neither keep nor drop
-                Err(_) => KeepOrDrop::None,
-            }
+/// Extract keep/drop number as a string. Fails if the value is not a
+/// string.
+fn parse_keep_or_drop_text<'a>(
+    symbol: &'a str,
+    input: &'a str,
+) -> IResult<&'a str, ParsedKeepOrDrop<'a>> {
+    let (parsed_kd, input) = match tuple::<&str, _, (_, _), _>((tag(symbol), digit1))(input) {
+        // if ok, one of the expressions is present
+        Ok((rest, (_, kd_expr))) => match symbol {
+            "k" => (ParsedKeepOrDrop::Keep(kd_expr), rest),
+            "dh" => (ParsedKeepOrDrop::Drop(kd_expr), rest),
+            _ => panic!("Unrecogized keep-drop symbol: {}", symbol),
         },
+        // otherwise absent (attempt to keep all dice)
+        Err(_) => (ParsedKeepOrDrop::NotPresent, input),
     };
 
-    Ok((
-        input,
-        Dice::new(count, sides.parse().unwrap(), keep_drop),
-    ))
+    Ok((input, parsed_kd))
+}
+
+/// Parse keep/drop expression, which consits of "k" or "dh" following
+/// a dice expression. For example, "1d4h3" or "1d4dh2".
+fn parse_keep_or_drop<'a>(input: &'a str, count: u32) -> IResult<&'a str, KeepOrDrop> {
+    let (input, keep) = parse_keep_or_drop_text("k", input)?;
+    let (input, drop) = parse_keep_or_drop_text("dh", input)?;
+
+    use ParsedKeepOrDrop::*;
+    let keep_drop: KeepOrDrop = match (keep, drop) {
+        //Potential valid Keep expression.
+        (Keep(keep), NotPresent) => match keep.parse().map_err(|_| too_big!(input))? {
+            _i if _i > count || _i == 0 => Ok(KeepOrDrop::None),
+            i => Ok(KeepOrDrop::Keep(i)),
+        },
+        //Potential valid Drop expression.
+        (NotPresent, Drop(drop)) => match drop.parse().map_err(|_| too_big!(input))? {
+            _i if _i >= count => Ok(KeepOrDrop::None),
+            i => Ok(KeepOrDrop::Drop(i)),
+        },
+        //No Keep or Drop specified; regular behavior.
+        (NotPresent, NotPresent) => Ok(KeepOrDrop::None),
+        //Anything else is an error.
+        _ => Err(NomErr::Error((input, NomErrorKind::Many1))),
+    }?;
+
+    Ok((input, keep_drop))
 }
 
 // Parse a single digit expression.  Does not eat whitespace
@@ -149,18 +171,92 @@ mod tests {
     use super::*;
     #[test]
     fn dice_test() {
-        assert_eq!(parse_dice("2d4"), Ok(("", Dice::new(2, 4, KeepOrDrop::None))));
-        assert_eq!(parse_dice("20d40"), Ok(("", Dice::new(20, 40, KeepOrDrop::None))));
-        assert_eq!(parse_dice("8d7"), Ok(("", Dice::new(8, 7, KeepOrDrop::None))));
-        assert_eq!(parse_dice("2d20k1"), Ok(("", Dice::new(2, 20, KeepOrDrop::Keep(1)))));
-        assert_eq!(parse_dice("100d10k90"), Ok(("", Dice::new(100, 10, KeepOrDrop::Keep(90)))));
-        assert_eq!(parse_dice("11d10k10"), Ok(("", Dice::new(11, 10, KeepOrDrop::Keep(10)))));
-        assert_eq!(parse_dice("12d10k11"), Ok(("", Dice::new(12, 10, KeepOrDrop::Keep(11)))));
-        assert_eq!(parse_dice("12d10k13"), Ok(("", Dice::new(12, 10, KeepOrDrop::None))));
-        assert_eq!(parse_dice("12d10k0"), Ok(("", Dice::new(12, 10, KeepOrDrop::None))));
-        assert_eq!(parse_dice("20d40dh5"), Ok(("", Dice::new(20, 40, KeepOrDrop::Drop(5)))));
-        assert_eq!(parse_dice("8d7dh9"), Ok(("", Dice::new(8, 7, KeepOrDrop::None))));
-        assert_eq!(parse_dice("8d7dh8"), Ok(("", Dice::new(8, 7, KeepOrDrop::None))));
+        assert_eq!(
+            parse_dice("2d4"),
+            Ok(("", Dice::new(2, 4, KeepOrDrop::None)))
+        );
+        assert_eq!(
+            parse_dice("20d40"),
+            Ok(("", Dice::new(20, 40, KeepOrDrop::None)))
+        );
+        assert_eq!(
+            parse_dice("8d7"),
+            Ok(("", Dice::new(8, 7, KeepOrDrop::None)))
+        );
+        assert_eq!(
+            parse_dice("2d20k1"),
+            Ok(("", Dice::new(2, 20, KeepOrDrop::Keep(1))))
+        );
+        assert_eq!(
+            parse_dice("100d10k90"),
+            Ok(("", Dice::new(100, 10, KeepOrDrop::Keep(90))))
+        );
+        assert_eq!(
+            parse_dice("11d10k10"),
+            Ok(("", Dice::new(11, 10, KeepOrDrop::Keep(10))))
+        );
+        assert_eq!(
+            parse_dice("12d10k11"),
+            Ok(("", Dice::new(12, 10, KeepOrDrop::Keep(11))))
+        );
+        assert_eq!(
+            parse_dice("12d10k13"),
+            Ok(("", Dice::new(12, 10, KeepOrDrop::None)))
+        );
+        assert_eq!(
+            parse_dice("12d10k0"),
+            Ok(("", Dice::new(12, 10, KeepOrDrop::None)))
+        );
+        assert_eq!(
+            parse_dice("20d40dh5"),
+            Ok(("", Dice::new(20, 40, KeepOrDrop::Drop(5))))
+        );
+        assert_eq!(
+            parse_dice("8d7dh9"),
+            Ok(("", Dice::new(8, 7, KeepOrDrop::None)))
+        );
+        assert_eq!(
+            parse_dice("8d7dh8"),
+            Ok(("", Dice::new(8, 7, KeepOrDrop::None)))
+        );
+    }
+
+    #[test]
+    fn cant_have_both_keep_and_drop_test() {
+        let res = parse_dice("1d4k3dh2");
+        assert!(res.is_err());
+        match res {
+            Err(NomErr::Error((_, kind))) => {
+                assert_eq!(kind, NomErrorKind::Many1);
+            }
+            _ => panic!("Got success, expected error"),
+        }
+    }
+
+    #[test]
+    fn big_number_of_dice_doesnt_crash_test() {
+        let res = parse_dice("64378631476346123874527551481376547657868536d4");
+        assert!(res.is_err());
+        match res {
+            Err(NomErr::Error((input, kind))) => {
+                assert_eq!(kind, NomErrorKind::TooLarge);
+                assert_eq!(input, "64378631476346123874527551481376547657868536");
+            }
+            _ => panic!("Got success, expected error"),
+        }
+    }
+
+    #[test]
+    fn big_number_of_sides_doesnt_crash_test() {
+        let res = parse_dice("1d423562312587425472658956278456298376234876");
+        assert!(res.is_err());
+        match res {
+            Err(NomErr::Error((input, kind))) => {
+                assert_eq!(kind, NomErrorKind::TooLarge);
+                assert_eq!(input, "423562312587425472658956278456298376234876");
+            }
+            _ => panic!("Got success, expected error"),
+        }
     }
 
     #[test]
@@ -219,7 +315,9 @@ mod tests {
             Ok((
                 "",
                 ElementExpression(vec![SignedElement::Positive(Element::Dice(Dice::new(
-                    8, 4, KeepOrDrop::None
+                    8,
+                    4,
+                    KeepOrDrop::None
                 )))])
             ))
         );
@@ -238,7 +336,9 @@ mod tests {
             Ok((
                 " \n ",
                 ElementExpression(vec![SignedElement::Negative(Element::Dice(Dice::new(
-                    8, 4, KeepOrDrop::None
+                    8,
+                    4,
+                    KeepOrDrop::None
                 )))])
             ))
         );

dicebot/src/bin/dicebot-cmd.rs

@@ -1,4 +1,4 @@
-use matrix_sdk::identifiers::room_id;
+use matrix_sdk::ruma::room_id;
 use matrix_sdk::Client;
 use tenebrous_dicebot::commands;
 use tenebrous_dicebot::commands::ResponseExtractor;
@@ -29,7 +29,7 @@ async fn main() -> Result<(), BotError> {
     let context = Context {
         db,
         account: Account::default(),
-        matrix_client: Client::new(homeserver).expect("Could not create matrix client"),
+        matrix_client: Client::new(homeserver).await.expect("Could not create matrix client"),
         origin_room: RoomContext {
             id: &room_id!("!fakeroomid:example.com"),
             display_name: "fake room".to_owned(),

dicebot/src/bin/dicebot.rs

@@ -21,7 +21,7 @@ async fn init(config_path: &str) -> Result<(Arc<Config>, Database, Client), BotE
     let cfg = Arc::new(cfg);
     let sqlite_path = format!("{}/dicebot.sqlite", cfg.database_path());
     let db = Database::new(&sqlite_path).await?;
-    let client = tenebrous_dicebot::matrix::create_client(&cfg)?;
+    let client = tenebrous_dicebot::matrix::create_client(&cfg).await?;
     Ok((cfg, db, client))
 }
 
@@ -55,7 +55,7 @@ async fn run() -> Result<(), BotError> {
 
     match try_join!(bot, grpc) {
         Ok(_) => (),
-        Err(e) => error!("Error: {}", e),
+        Err(e) => error!("Error: {:?}", e),
     };
 
     Ok(())

dicebot/src/bot/command_execution.rs

@@ -8,12 +8,8 @@ use crate::{
     models::Account,
 };
 use futures::stream::{self, StreamExt};
-use matrix_sdk::{
-    self,
-    identifiers::{EventId, RoomId},
-    room::Joined,
-    Client,
-};
+use matrix_sdk::ruma::{OwnedEventId, RoomId};
+use matrix_sdk::{self, room::Joined, Client};
 use std::clone::Clone;
 use std::convert::TryFrom;
 
@@ -24,7 +20,7 @@ pub(super) async fn handle_single_result(
     cmd_result: &ExecutionResult,
     respond_to: &str,
     room: &Joined,
-    event_id: EventId,
+    event_id: OwnedEventId,
 ) {
     let html = cmd_result.message_html(respond_to);
     let plain = cmd_result.message_plain(respond_to);
@@ -112,9 +108,9 @@ fn get_account_active_room(client: &Client, account: &Account) -> Result<Option<
     let active_room = account
         .registered_user()
         .and_then(|u| u.active_room.as_deref())
-        .map(|room_id| RoomId::try_from(room_id))
+        .map(|room_id| <&RoomId>::try_from(room_id))
         .transpose()?
-        .and_then(|active_room_id| client.get_joined_room(&active_room_id));
+        .and_then(|active_room_id| client.get_joined_room(active_room_id));
 
     Ok(active_room)
 }

dicebot/src/bot/event_handlers.rs

@@ -2,32 +2,25 @@ use super::DiceBot;
 use crate::db::sqlite::Database;
 use crate::db::Rooms;
 use crate::error::BotError;
-use async_trait::async_trait;
 use log::{debug, error, info, warn};
-use matrix_sdk::{
-    self,
-    events::{
-        room::member::MemberEventContent,
-        room::message::{MessageEventContent, MessageType, TextMessageEventContent},
-        StrippedStateEvent, SyncMessageEvent,
-    },
-    room::Room,
-    EventHandler,
-};
+use matrix_sdk::ruma::events::room::member::RoomMemberEventContent;
+use matrix_sdk::ruma::events::{StrippedStateEvent, SyncMessageLikeEvent};
+use matrix_sdk::{self, room::Room, ruma::events::room::message::RoomMessageEventContent};
+use matrix_sdk::{Client, DisplayName};
 use std::ops::Sub;
+use std::time::UNIX_EPOCH;
 use std::time::{Duration, SystemTime};
-use std::{clone::Clone, time::UNIX_EPOCH};
 
 /// Check if a message is recent enough to actually process. If the
 /// message is within "oldest_message_age" seconds, this function
 /// returns true. If it's older than that, it returns false and logs a
 /// debug message.
 fn check_message_age(
-    event: &SyncMessageEvent<MessageEventContent>,
+    event: &SyncMessageLikeEvent<RoomMessageEventContent>,
     oldest_message_age: u64,
 ) -> bool {
     let sending_time = event
-        .origin_server_ts
+        .origin_server_ts()
         .to_system_time()
         .unwrap_or(UNIX_EPOCH);
 
@@ -54,7 +47,7 @@ fn check_message_age(
 /// the bot left and rejoined quickly.
 async fn should_process_message<'a>(
     bot: &DiceBot,
-    event: &SyncMessageEvent<MessageEventContent>,
+    event: &SyncMessageLikeEvent<RoomMessageEventContent>,
 ) -> Result<(String, String), BotError> {
     //Ignore messages that are older than configured duration.
     if !check_message_age(event, bot.config.oldest_message_age()) {
@@ -68,23 +61,29 @@ async fn should_process_message<'a>(
         return Err(BotError::ShouldNotProcessError);
     }
 
-    let (msg_body, sender_username) = if let SyncMessageEvent {
-        content:
-            MessageEventContent {
-                msgtype: MessageType::Text(TextMessageEventContent { body, .. }),
-                ..
-            },
-        sender,
-        ..
-    } = event
-    {
-        (
-            body.clone(),
-            format!("@{}:{}", sender.localpart(), sender.server_name()),
-        )
-    } else {
-        (String::new(), String::new())
-    };
+    let msg_body: String = event
+        .as_original()
+        .map(|e| e.content.body())
+        .map(str::to_string)
+        .unwrap_or_else(|| String::new());
+
+    let sender_username: String = format!(
+        "@{}:{}",
+        event.sender().localpart(),
+        event.sender().server_name()
+    );
+
+    // Do not process messages from the bot itself. Otherwise it might
+    // try to execute its own commands.
+    let bot_username = bot
+        .client
+        .user_id()
+        .map(|u| format!("@{}:{}", u.localpart(), u.server_name()))
+        .unwrap_or_default();
+
+    if sender_username == bot_username {
+        return Err(BotError::ShouldNotProcessError);
+    }
 
     Ok((msg_body, sender_username))
 }
@@ -101,58 +100,64 @@ async fn should_process_event(db: &Database, room_id: &str, event_id: &str) -> b
         })
 }
 
-/// This event emitter listens for messages with dice rolling commands.
-/// Originally adapted from the matrix-rust-sdk examples.
-#[async_trait]
-impl EventHandler for DiceBot {
-    async fn on_stripped_state_member(
-        &self,
-        room: Room,
-        event: &StrippedStateEvent<MemberEventContent>,
-        _: Option<MemberEventContent>,
-    ) {
-        let room = match room {
-            Room::Invited(invited_room) => invited_room,
-            _ => return,
-        };
+pub(super) async fn on_stripped_state_member(
+    event: StrippedStateEvent<RoomMemberEventContent>,
+    client: Client,
+    room: Room,
+) {
+    let room = match room {
+        Room::Invited(invited_room) => invited_room,
+        _ => return,
+    };
 
-        if room.own_user_id().as_str() != event.state_key {
-            return;
-        }
-
-        info!(
-            "Autojoining room {}",
-            room.display_name().await.ok().unwrap_or_default()
-        );
-
-        if let Err(e) = self.client.join_room_by_id(&room.room_id()).await {
-            warn!("Could not join room: {}", e.to_string())
-        }
+    if room.own_user_id().as_str() != event.state_key {
+        return;
     }
 
-    async fn on_room_message(&self, room: Room, event: &SyncMessageEvent<MessageEventContent>) {
-        let room = match room {
-            Room::Joined(joined_room) => joined_room,
-            _ => return,
-        };
+    info!(
+        "Autojoining room {}",
+        room.display_name()
+            .await
+            .ok()
+            .unwrap_or_else(|| DisplayName::Named("[error]".to_string()))
+    );
 
-        let room_id = room.room_id().as_str();
-        if !should_process_event(&self.db, room_id, event.event_id.as_str()).await {
-            return;
-        }
-
-        let (msg_body, sender_username) =
-            if let Ok((msg_body, sender_username)) = should_process_message(self, &event).await {
-                (msg_body, sender_username)
-            } else {
-                return;
-            };
-
-        let results = self
-            .execute_commands(&room, &sender_username, &msg_body)
-            .await;
-
-        self.handle_results(&room, &sender_username, event.event_id.clone(), results)
-            .await;
+    if let Err(e) = client.join_room_by_id(&room.room_id()).await {
+        warn!("Could not join room: {}", e.to_string())
     }
 }
+
+pub(super) async fn on_room_message(
+    event: SyncMessageLikeEvent<RoomMessageEventContent>,
+    room: Room,
+    bot: DiceBot,
+) {
+    let room = match room {
+        Room::Joined(joined_room) => joined_room,
+        _ => return,
+    };
+
+    let room_id = room.room_id().as_str();
+    if !should_process_event(&bot.db, room_id, event.event_id().as_str()).await {
+        return;
+    }
+
+    let (msg_body, sender_username) =
+        if let Ok((msg_body, sender_username)) = should_process_message(&bot, &event).await {
+            (msg_body, sender_username)
+        } else {
+            return;
+        };
+
+    let results = bot
+        .execute_commands(&room, &sender_username, &msg_body)
+        .await;
+
+    bot.handle_results(
+        &room,
+        &sender_username,
+        event.event_id().to_owned(),
+        results,
+    )
+    .await;
+}

dicebot/src/bot/mod.rs

@@ -5,7 +5,12 @@ use crate::db::DbState;
 use crate::error::BotError;
 use crate::state::DiceBotState;
 use log::info;
-use matrix_sdk::{self, identifiers::EventId, room::Joined, Client, SyncSettings};
+use matrix_sdk::room::Room;
+use matrix_sdk::ruma::events::room::message::RoomMessageEventContent;
+use matrix_sdk::ruma::events::SyncMessageLikeEvent;
+use matrix_sdk::ruma::OwnedEventId;
+use matrix_sdk::{self, room::Joined, Client};
+use matrix_sdk::config::SyncSettings;
 use std::clone::Clone;
 use std::sync::{Arc, RwLock};
 
@@ -18,6 +23,7 @@ const MAX_COMMANDS_PER_MESSAGE: usize = 50;
 
 /// The DiceBot struct represents an active dice bot. The bot is not
 /// connected to Matrix until its run() function is called.
+#[derive(Clone)]
 pub struct DiceBot {
     /// A reference to the configuration read in on application start.
     config: Arc<Config>,
@@ -63,12 +69,14 @@ impl DiceBot {
         let device_id: Option<String> = self.db.get_device_id().await?;
         let device_id: Option<&str> = device_id.as_deref();
 
-        client
-            .login(username, password, device_id, Some("matrix dice bot"))
-            .await?;
+        let no_device_ld_login = || client.login_username(username, password);
+        let device_id_login = |id| client.login_username(username, password).device_id(id);
+        let login = device_id.map_or_else(no_device_ld_login, device_id_login);
+
+        login.send().await?;
 
         if device_id.is_none() {
-            let device_id = client.device_id().await.ok_or(BotError::NoDeviceIdFound)?;
+            let device_id = client.device_id().ok_or(BotError::NoDeviceIdFound)?;
             self.db.set_device_id(device_id.as_str()).await?;
             info!("Recorded new device ID: {}", device_id.as_str());
         } else {
@@ -79,19 +87,35 @@ impl DiceBot {
         Ok(())
     }
 
+    async fn bind_events(&self) {
+        //on room message: need closure to pass bot ref in.
+        self.client
+            .add_event_handler({
+                let bot: DiceBot = self.clone();
+                move |event: SyncMessageLikeEvent<RoomMessageEventContent>, room: Room| {
+                    let bot = bot.clone();
+                    async move { event_handlers::on_room_message(event, room, bot).await }
+                }
+            });
+
+        //auto-join handler
+        self.client
+            .add_event_handler(event_handlers::on_stripped_state_member);
+    }
+
     /// Logs the bot in to Matrix and listens for events until program
     /// terminated, or a panic occurs. Originally adapted from the
     /// matrix-rust-sdk command bot example.
     pub async fn run(self) -> Result<(), BotError> {
         let client = self.client.clone();
         self.login(&client).await?;
+        self.bind_events().await;
 
-        client.set_event_handler(Box::new(self)).await;
         info!("Listening for commands");
 
         // TODO replace with sync_with_callback for cleaner shutdown
         // process.
-        client.sync(SyncSettings::default()).await;
+        client.sync(SyncSettings::default()).await?;
         Ok(())
     }
 
@@ -121,7 +145,7 @@ impl DiceBot {
         &self,
         room: &Joined,
         sender_username: &str,
-        event_id: EventId,
+        event_id: OwnedEventId,
         results: Vec<(String, ExecutionResult)>,
     ) {
         if results.len() >= 1 {

dicebot/src/cofd/dice.rs

@@ -332,7 +332,7 @@ mod tests {
     macro_rules! dummy_room {
         () => {
             crate::context::RoomContext {
-                id: &matrix_sdk::identifiers::room_id!("!fakeroomid:example.com"),
+                id: &matrix_sdk::ruma::room_id!("!fakeroomid:example.com"),
                 display_name: "displayname".to_owned(),
                 secure: false,
             }
@@ -485,7 +485,7 @@ mod tests {
         let ctx = Context {
             account: crate::models::Account::default(),
             db: db,
-            matrix_client: matrix_sdk::Client::new(homeserver).unwrap(),
+            matrix_client: matrix_sdk::Client::new(homeserver).await.unwrap(),
             origin_room: dummy_room!(),
             active_room: dummy_room!(),
             username: "username",
@@ -527,7 +527,7 @@ mod tests {
         let ctx = Context {
             account: crate::models::Account::default(),
             db: db,
-            matrix_client: matrix_sdk::Client::new(homeserver).unwrap(),
+            matrix_client: matrix_sdk::Client::new(homeserver).await.unwrap(),
             origin_room: dummy_room!(),
             active_room: dummy_room!(),
             username: "username",
@@ -566,7 +566,7 @@ mod tests {
         let ctx = Context {
             account: crate::models::Account::default(),
             db: db.clone(),
-            matrix_client: matrix_sdk::Client::new(homeserver).unwrap(),
+            matrix_client: matrix_sdk::Client::new(homeserver).await.unwrap(),
             origin_room: dummy_room!(),
             active_room: dummy_room!(),
             username: "username",

dicebot/src/cofd/parser.rs

@@ -45,13 +45,13 @@ pub fn parse_modifiers(input: &str) -> Result<DicePoolModifiers, DiceParsingErro
     let (result, rest) = parser.parse(input)?;
 
     if rest.len() == 0 {
-        convert_to_info(&result)
+        convert_to_modifiers(&result)
     } else {
         Err(DiceParsingError::UnconsumedInput)
     }
 }
 
-fn convert_to_info(parsed: &Vec<ParsedInfo>) -> Result<DicePoolModifiers, DiceParsingError> {
+fn convert_to_modifiers(parsed: &Vec<ParsedInfo>) -> Result<DicePoolModifiers, DiceParsingError> {
     use ParsedInfo::*;
     if parsed.len() == 0 {
         Ok(DicePoolModifiers::default())
@@ -79,19 +79,8 @@ fn convert_to_info(parsed: &Vec<ParsedInfo>) -> Result<DicePoolModifiers, DicePa
 }
 
 pub fn parse_dice_pool(input: &str) -> Result<DicePool, BotError> {
-    //The "modifiers:" part is optional. Assume amounts if no modifier
-    //section found.
-    let split = input.split(":").collect::<Vec<_>>();
-    let (modifiers_str, amounts_str) = (match split[..] {
-        [amounts] => Ok(("", amounts)),
-        [modifiers, amounts] => Ok((modifiers, amounts)),
-        _ => Err(BotError::DiceParsingError(
-            DiceParsingError::UnconsumedInput,
-        )),
-    })?;
-
+    let (amounts, modifiers_str) = parse_amounts(input)?;
     let modifiers = parse_modifiers(modifiers_str)?;
-    let amounts = parse_amounts(&amounts_str)?;
     Ok(DicePool::new(amounts, modifiers))
 }
 
@@ -175,7 +164,7 @@ mod tests {
 
     #[test]
     fn dice_pool_number_with_quality() {
-        let result = parse_dice_pool("n:8");
+        let result = parse_dice_pool("8 n");
         assert!(result.is_ok());
         assert_eq!(
             result.unwrap(),
@@ -186,7 +175,7 @@ mod tests {
     #[test]
     fn dice_pool_number_with_success_change() {
         let modifiers = DicePoolModifiers::custom_exceptional_on(3);
-        let result = parse_dice_pool("s3:8");
+        let result = parse_dice_pool("8 s3");
         assert!(result.is_ok());
         assert_eq!(result.unwrap(), DicePool::easy_with_modifiers(8, modifiers));
     }
@@ -194,7 +183,7 @@ mod tests {
     #[test]
     fn dice_pool_with_quality_and_success_change() {
         let modifiers = DicePoolModifiers::custom(DicePoolQuality::Rote, 3);
-        let result = parse_dice_pool("rs3:8");
+        let result = parse_dice_pool("8 rs3");
         assert!(result.is_ok());
         assert_eq!(result.unwrap(), DicePool::easy_with_modifiers(8, modifiers));
     }
@@ -224,20 +213,20 @@ mod tests {
 
         let expected = DicePool::new(amounts, modifiers);
 
-        let result = parse_dice_pool("rs3:8+10-2+varname");
+        let result = parse_dice_pool("8+10-2+varname rs3");
         assert!(result.is_ok());
         assert_eq!(result.unwrap(), expected);
 
-        let result = parse_dice_pool("rs3:8+10-   2 + varname");
+        let result = parse_dice_pool("8+10-   2 + varname      rs3");
         assert!(result.is_ok());
         assert_eq!(result.unwrap(), expected);
 
-        let result = parse_dice_pool("rs3  :  8+ 10 -2 + varname");
+        let result = parse_dice_pool("8+ 10 -2 + varname           rs3");
         assert!(result.is_ok());
         assert_eq!(result.unwrap(), expected);
 
         //This one has tabs in it.
-        let result = parse_dice_pool("  r	s3  :	8	+ 10 -2 + varname");
+        let result = parse_dice_pool(" 	8	+ 10 -2 + varname	r	s3");
         assert!(result.is_ok());
         assert_eq!(result.unwrap(), expected);
     }

dicebot/src/commands/mod.rs

@@ -162,11 +162,12 @@ mod tests {
     use super::*;
     use management::RegisterCommand;
     use url::Url;
+    use matrix_sdk::ruma::room_id;
 
     macro_rules! dummy_room {
         () => {
             crate::context::RoomContext {
-                id: &matrix_sdk::identifiers::room_id!("!fakeroomid:example.com"),
+                id: &room_id!("!fakeroomid:example.com"),
                 display_name: "displayname".to_owned(),
                 secure: false,
             }
@@ -176,7 +177,7 @@ mod tests {
     macro_rules! secure_room {
         () => {
             crate::context::RoomContext {
-                id: &matrix_sdk::identifiers::room_id!("!fakeroomid:example.com"),
+                id: &room_id!("!fakeroomid:example.com"),
                 display_name: "displayname".to_owned(),
                 secure: true,
             }
@@ -195,7 +196,7 @@ mod tests {
         let ctx = Context {
             account: crate::models::Account::default(),
             db: db,
-            matrix_client: matrix_sdk::Client::new(homeserver).unwrap(),
+            matrix_client: matrix_sdk::Client::new(homeserver).await.unwrap(),
             origin_room: secure_room!(),
             active_room: secure_room!(),
             username: "myusername",
@@ -218,7 +219,7 @@ mod tests {
         let ctx = Context {
             account: crate::models::Account::default(),
             db: db,
-            matrix_client: matrix_sdk::Client::new(homeserver).unwrap(),
+            matrix_client: matrix_sdk::Client::new(homeserver).await.unwrap(),
             origin_room: secure_room!(),
             active_room: secure_room!(),
             username: "myusername",
@@ -241,7 +242,7 @@ mod tests {
         let ctx = Context {
             account: crate::models::Account::default(),
             db: db,
-            matrix_client: matrix_sdk::Client::new(homeserver).unwrap(),
+            matrix_client: matrix_sdk::Client::new(homeserver).await.unwrap(),
             origin_room: dummy_room!(),
             active_room: dummy_room!(),
             username: "myusername",
@@ -264,7 +265,7 @@ mod tests {
         let ctx = Context {
             account: crate::models::Account::default(),
             db: db,
-            matrix_client: matrix_sdk::Client::new(homeserver).unwrap(),
+            matrix_client: matrix_sdk::Client::new(homeserver).await.unwrap(),
             origin_room: dummy_room!(),
             active_room: dummy_room!(),
             username: "myusername",
@@ -287,7 +288,7 @@ mod tests {
         let ctx = Context {
             account: crate::models::Account::default(),
             db: db,
-            matrix_client: matrix_sdk::Client::new(homeserver).unwrap(),
+            matrix_client: matrix_sdk::Client::new(homeserver).await.unwrap(),
             origin_room: dummy_room!(),
             active_room: dummy_room!(),
             username: "myusername",

dicebot/src/commands/parser.rs

@@ -221,9 +221,9 @@ mod tests {
 
     #[test]
     fn pool_whitespace_test() {
-        parse_command("!pool ns3:8   ").expect("was error");
-        parse_command("   !pool ns3:8").expect("was error");
-        parse_command("   !pool ns3:8   ").expect("was error");
+        parse_command("!pool 8   ns3    ").expect("was error");
+        parse_command("   !pool 8 ns3").expect("was error");
+        parse_command("   !pool 8   ns3    ").expect("was error");
     }
 
     #[test]

dicebot/src/commands/rooms.rs

@@ -6,7 +6,7 @@ use crate::matrix;
 use async_trait::async_trait;
 use fuse_rust::{Fuse, FuseProperty, Fuseable};
 use futures::stream::{self, StreamExt, TryStreamExt};
-use matrix_sdk::{identifiers::UserId, Client};
+use matrix_sdk::{ruma::OwnedUserId, Client};
 use std::convert::TryFrom;
 
 /// Holds matrix room ID and display name as strings, for use with
@@ -62,13 +62,13 @@ async fn get_rooms_for_user(
     client: &Client,
     user_id: &str,
 ) -> Result<Vec<RoomNameAndId>, BotError> {
-    let user_id = UserId::try_from(user_id)?;
+    let user_id = OwnedUserId::try_from(user_id)?;
     let rooms_for_user = matrix::get_rooms_for_user(client, &user_id).await?;
     let mut rooms_for_user: Vec<RoomNameAndId> = stream::iter(rooms_for_user)
         .filter_map(|room| async move {
             Some(room.display_name().await.map(|room_name| RoomNameAndId {
                 id: room.room_id().to_string(),
-                name: room_name,
+                name: room_name.to_string(),
             }))
         })
         .try_collect()

dicebot/src/context.rs

@@ -1,8 +1,8 @@
 use crate::db::sqlite::Database;
 use crate::error::BotError;
 use crate::models::Account;
-use matrix_sdk::identifiers::{RoomId, UserId};
 use matrix_sdk::room::Joined;
+use matrix_sdk::ruma::{RoomId, UserId};
 use matrix_sdk::Client;
 use std::convert::TryFrom;
 
@@ -48,15 +48,16 @@ impl RoomContext<'_> {
         // TODO is_direct is a hack; the bot should set eligible rooms
         // to Direct Message upon joining, if other contact has
         // requested it. Waiting on SDK support.
-        let display_name = room
+        let display_name =
+            room
             .display_name()
             .await
             .ok()
-            .unwrap_or_default()
-            .to_string();
+            .map(|d| d.to_string())
+            .unwrap_or_default();
 
-        let sending_user = UserId::try_from(sending_user)?;
-        let user_in_room = room.get_member(&sending_user).await.ok().is_some();
+        let sending_user = <&UserId>::try_from(sending_user)?;
+        let user_in_room = room.get_member(sending_user).await.ok().is_some();
         let is_direct = room.active_members().await?.len() == 2;
 
         Ok(RoomContext {

dicebot/src/cthulhu/dice.rs

@@ -270,7 +270,7 @@ macro_rules! is_variable {
                 element: Element::Variable(_),
                 ..
             }
-        );
+        )
     };
 }
 
@@ -427,11 +427,12 @@ mod tests {
     use crate::db::sqlite::Database;
     use crate::parser::dice::{Amount, Element, Operator};
     use url::Url;
+    use matrix_sdk::ruma::room_id;
 
     macro_rules! dummy_room {
         () => {
             crate::context::RoomContext {
-                id: &matrix_sdk::identifiers::room_id!("!fakeroomid:example.com"),
+                id: &room_id!("!fakeroomid:example.com"),
                 display_name: "displayname".to_owned(),
                 secure: false,
             }
@@ -511,7 +512,7 @@ mod tests {
         let ctx = Context {
             account: crate::models::Account::default(),
             db: db,
-            matrix_client: matrix_sdk::Client::new(homeserver).unwrap(),
+            matrix_client: matrix_sdk::Client::new(homeserver).await.unwrap(),
             origin_room: dummy_room!(),
             active_room: dummy_room!(),
             username: "username",
@@ -549,7 +550,7 @@ mod tests {
         let ctx = Context {
             account: crate::models::Account::default(),
             db: db,
-            matrix_client: matrix_sdk::Client::new(homeserver).unwrap(),
+            matrix_client: matrix_sdk::Client::new(homeserver).await.unwrap(),
             origin_room: dummy_room!(),
             active_room: dummy_room!(),
             username: "username",
@@ -587,7 +588,7 @@ mod tests {
         let ctx = Context {
             account: crate::models::Account::default(),
             db: db,
-            matrix_client: matrix_sdk::Client::new(homeserver).unwrap(),
+            matrix_client: matrix_sdk::Client::new(homeserver).await.unwrap(),
             origin_room: dummy_room!(),
             active_room: dummy_room!(),
             username: "username",

dicebot/src/cthulhu/parser.rs

@@ -4,16 +4,13 @@ use crate::parser::dice::DiceParsingError;
 //TOOD convert these to use parse_amounts from the common dice code.
 
 fn parse_modifier(input: &str) -> Result<DiceRollModifier, DiceParsingError> {
-    if input.ends_with("bb") {
-        Ok(DiceRollModifier::TwoBonus)
-    } else if input.ends_with("b") {
-        Ok(DiceRollModifier::OneBonus)
-    } else if input.ends_with("pp") {
-        Ok(DiceRollModifier::TwoPenalty)
-    } else if input.ends_with("p") {
-        Ok(DiceRollModifier::OnePenalty)
-    } else {
-        Ok(DiceRollModifier::Normal)
+    match input.trim() {
+        "bb" => Ok(DiceRollModifier::TwoBonus),
+        "b" => Ok(DiceRollModifier::OneBonus),
+        "pp" => Ok(DiceRollModifier::TwoPenalty),
+        "p" => Ok(DiceRollModifier::OnePenalty),
+        "" => Ok(DiceRollModifier::Normal),
+        _ => Err(DiceParsingError::InvalidModifiers),
     }
 }
 
@@ -21,32 +18,70 @@ fn parse_modifier(input: &str) -> Result<DiceRollModifier, DiceParsingError> {
 //Split based on :, send first part to parse_modifier.
 //Send second part to parse_amounts
 pub fn parse_regular_roll(input: &str) -> Result<DiceRoll, DiceParsingError> {
-    let input: Vec<&str> = input.trim().split(":").collect();
-
-    let (modifiers_str, amounts_str) = match input[..] {
-        [amounts] => Ok(("", amounts)),
-        [modifiers, amounts] => Ok((modifiers, amounts)),
-        _ => Err(DiceParsingError::UnconsumedInput),
-    }?;
-
+    let (amount, modifiers_str) = crate::parser::dice::parse_single_amount(input)?;
     let modifier = parse_modifier(modifiers_str)?;
-    let amount = crate::parser::dice::parse_single_amount(amounts_str)?;
     Ok(DiceRoll { modifier, amount })
 }
 
 pub fn parse_advancement_roll(input: &str) -> Result<AdvancementRoll, DiceParsingError> {
     let input = input.trim();
-    let amounts = crate::parser::dice::parse_single_amount(input)?;
+    let (amounts, unconsumed_input) = crate::parser::dice::parse_single_amount(input)?;
 
-    Ok(AdvancementRoll {
-        existing_skill: amounts,
-    })
+    if unconsumed_input.len() == 0 {
+        Ok(AdvancementRoll {
+            existing_skill: amounts,
+        })
+    } else {
+        Err(DiceParsingError::InvalidAmount)
+    }
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::parser::dice::{Amount, Element, Operator};
+    use crate::parser::dice::{Amount, DiceParsingError, Element, Operator};
+
+    #[test]
+    fn parse_modifier_rejects_bad_value() {
+        let modifier = parse_modifier("qqq");
+        assert!(matches!(modifier, Err(DiceParsingError::InvalidModifiers)))
+    }
+
+    #[test]
+    fn parse_modifier_accepts_one_bonus() {
+        let modifier = parse_modifier("b");
+        assert!(matches!(modifier, Ok(DiceRollModifier::OneBonus)))
+    }
+
+    #[test]
+    fn parse_modifier_accepts_two_bonus() {
+        let modifier = parse_modifier("bb");
+        assert!(matches!(modifier, Ok(DiceRollModifier::TwoBonus)))
+    }
+
+    #[test]
+    fn parse_modifier_accepts_two_penalty() {
+        let modifier = parse_modifier("pp");
+        assert!(matches!(modifier, Ok(DiceRollModifier::TwoPenalty)))
+    }
+
+    #[test]
+    fn parse_modifier_accepts_one_penalty() {
+        let modifier = parse_modifier("p");
+        assert!(matches!(modifier, Ok(DiceRollModifier::OnePenalty)))
+    }
+
+    #[test]
+    fn parse_modifier_accepts_normal() {
+        let modifier = parse_modifier("");
+        assert!(matches!(modifier, Ok(DiceRollModifier::Normal)))
+    }
+
+    #[test]
+    fn parse_modifier_accepts_normal_unaffected_by_whitespace() {
+        let modifier = parse_modifier("         ");
+        assert!(matches!(modifier, Ok(DiceRollModifier::Normal)))
+    }
 
     #[test]
     fn regular_roll_accepts_single_number() {
@@ -72,7 +107,7 @@ mod tests {
 
     #[test]
     fn regular_roll_accepts_two_bonus() {
-        let result = parse_regular_roll("bb:60");
+        let result = parse_regular_roll("60 bb");
         assert!(result.is_ok());
         assert_eq!(
             DiceRoll {
@@ -88,7 +123,7 @@ mod tests {
 
     #[test]
     fn regular_roll_accepts_one_bonus() {
-        let result = parse_regular_roll("b:60");
+        let result = parse_regular_roll("60 b");
         assert!(result.is_ok());
         assert_eq!(
             DiceRoll {
@@ -104,7 +139,7 @@ mod tests {
 
     #[test]
     fn regular_roll_accepts_two_penalty() {
-        let result = parse_regular_roll("pp:60");
+        let result = parse_regular_roll("60 pp");
         assert!(result.is_ok());
         assert_eq!(
             DiceRoll {
@@ -120,7 +155,7 @@ mod tests {
 
     #[test]
     fn regular_roll_accepts_one_penalty() {
-        let result = parse_regular_roll("p:60");
+        let result = parse_regular_roll("60 p");
         assert!(result.is_ok());
         assert_eq!(
             DiceRoll {
@@ -140,21 +175,21 @@ mod tests {
         assert!(parse_regular_roll("   60").is_ok());
         assert!(parse_regular_roll("   60    ").is_ok());
 
-        assert!(parse_regular_roll("bb:60     ").is_ok());
-        assert!(parse_regular_roll("   bb:60").is_ok());
-        assert!(parse_regular_roll("   bb:60    ").is_ok());
+        assert!(parse_regular_roll("60bb     ").is_ok());
+        assert!(parse_regular_roll("   60 bb").is_ok());
+        assert!(parse_regular_roll("   60   bb    ").is_ok());
 
-        assert!(parse_regular_roll("b:60     ").is_ok());
-        assert!(parse_regular_roll("   b:60").is_ok());
-        assert!(parse_regular_roll("   b:60    ").is_ok());
+        assert!(parse_regular_roll("60b     ").is_ok());
+        assert!(parse_regular_roll("   60 b").is_ok());
+        assert!(parse_regular_roll("   60  b    ").is_ok());
 
-        assert!(parse_regular_roll("pp:60     ").is_ok());
-        assert!(parse_regular_roll("   pp:60").is_ok());
-        assert!(parse_regular_roll("   pp:60    ").is_ok());
+        assert!(parse_regular_roll("60pp     ").is_ok());
+        assert!(parse_regular_roll("   60 pp").is_ok());
+        assert!(parse_regular_roll("   60 pp   ").is_ok());
 
-        assert!(parse_regular_roll("p:60     ").is_ok());
-        assert!(parse_regular_roll("   p:60").is_ok());
-        assert!(parse_regular_roll("   p:60    ").is_ok());
+        assert!(parse_regular_roll("60p     ").is_ok());
+        assert!(parse_regular_roll("   60p  ").is_ok());
+        assert!(parse_regular_roll("   60  p    ").is_ok());
     }
 
     #[test]

dicebot/src/db/sqlite/migrator/migrations/V10__add_game_system.rs

@@ -0,0 +1,22 @@
+use crate::systems::GameSystem;
+use barrel::backend::Sqlite;
+use barrel::{types, types::Type, Migration};
+use itertools::Itertools;
+use strum::IntoEnumIterator;
+
+fn primary_id() -> Type {
+    types::text().unique(true).primary(true).nullable(false)
+}
+
+pub fn migration() -> String {
+    let mut m = Migration::new();
+
+    //Normally we would add a CHECK clause here, but types::custom requires a 'static string.
+    //Which means we can't automagically generate one from the enum.
+    m.create_table("room_info", move |t| {
+        t.add_column("room_id", primary_id());
+        t.add_column("game_system", types::text().nullable(false));
+    });
+
+    m.make::<Sqlite>()
+}

dicebot/src/db/sqlite/migrator/migrations/mod.rs

@@ -1,2 +1 @@
-use refinery::include_migration_mods;
-include_migration_mods!("src/db/sqlite/migrator/migrations");
+

dicebot/src/db/sqlite/migrator/mod.rs

@@ -5,7 +5,7 @@ use sqlx::ConnectOptions;
 use std::str::FromStr;
 use thiserror::Error;
 
-pub mod migrations;
+//pub mod migrations;
 
 #[derive(Error, Debug)]
 pub enum MigrationError {
@@ -16,6 +16,11 @@ pub enum MigrationError {
     RefineryError(#[from] refinery::Error),
 }
 
+mod embedded {
+    use refinery::embed_migrations;
+    embed_migrations!("src/db/sqlite/migrator/migrations");
+}
+
 /// Run database migrations against the sqlite database.
 pub async fn migrate(db_path: &str) -> Result<(), MigrationError> {
     //Create database if missing.
@@ -28,6 +33,6 @@ pub async fn migrate(db_path: &str) -> Result<(), MigrationError> {
 
     let mut conn = Config::new(ConfigDbType::Sqlite).set_db_path(db_path);
     info!("Running migrations");
-    migrations::runner().run(&mut conn)?;
+    embedded::migrations::runner().run(&mut conn)?;
     Ok(())
 }

dicebot/src/error.rs

@@ -18,6 +18,12 @@ pub enum BotError {
     #[error("could not retrieve device id")]
     NoDeviceIdFound,
 
+    #[error("could not build client: {0}")]
+    ClientBuildError(#[from] matrix_sdk::ClientBuildError),
+
+    #[error("could not open matrix store: {0}")]
+    OpenStoreError(#[from] matrix_sdk::store::OpenStoreError),
+
     #[error("command error: {0}")]
     CommandError(#[from] CommandError),
 
@@ -33,15 +39,15 @@ pub enum BotError {
     #[error("could not parse URL")]
     UrlParseError(#[from] url::ParseError),
 
+    #[error("could not parse ID")]
+    IdParseError(#[from] matrix_sdk::ruma::IdParseError),
+
     #[error("error in matrix state store: {0}")]
     MatrixStateStoreError(#[from] matrix_sdk::StoreError),
 
     #[error("uncategorized matrix SDK error: {0}")]
     MatrixError(#[from] matrix_sdk::Error),
 
-    #[error("uncategorized matrix SDK base error: {0}")]
-    MatrixBaseError(#[from] matrix_sdk::BaseError),
-
     #[error("future canceled")]
     FutureCanceledError,
 
@@ -79,8 +85,8 @@ pub enum BotError {
     #[error("could not convert to proper integer type")]
     TryFromIntError(#[from] std::num::TryFromIntError),
 
-    #[error("identifier error: {0}")]
-    IdentifierError(#[from] matrix_sdk::identifiers::Error),
+    // #[error("identifier error: {0}")]
+    // IdentifierError(#[from] matrix_sdk::ruma::Error),
 
     #[error("password creation error: {0}")]
     PasswordCreationError(argon2::Error),

dicebot/src/help.rs

@@ -6,6 +6,9 @@ pub fn parse_help_topic(input: &str) -> Option<HelpTopic> {
         "dicepool" => Some(HelpTopic::DicePool),
         "dice" => Some(HelpTopic::RollingDice),
         "cthulhu" => Some(HelpTopic::Cthulhu),
+        "variables" => Some(HelpTopic::Variables),
+        "var" => Some(HelpTopic::Variables),
+        "variable" => Some(HelpTopic::Variables),
         "" => Some(HelpTopic::General),
         _ => None,
     }
@@ -16,6 +19,7 @@ pub enum HelpTopic {
     DicePool,
     Cthulhu,
     RollingDice,
+    Variables,
     General,
 }
 
@@ -101,6 +105,34 @@ Note: If !cthadv is given a variable, and the roll is successful, it will
 update the variable with the new skill.
 "};
 
+const VARIABLES_HELP: &'static str = indoc! {"
+Variables
+
+Commands: !get, !set, !variables
+
+Manage variables that can be substituted into roll commands.
+
+Examples: !get myvar, !set myvar 10
+
+!get <variable> = show variable of the given name
+!set <variable> <num> = set a variable to a number
+
+The !variables command will list all variables for the room. The
+variables command cna be used in a secure room to avoid spamming the
+actual room that the variable is set in.
+
+Variable names can be used in all types of dice rolls:
+
+!pool myvar + 3
+!roll myvar
+
+There are some limitations on variables: they cannot themselves be
+dice expressions (i.e. can only be numbers), and they must be uniquely
+parseable in an expression (i.e 'myvard6' does not work for the !roll
+command).
+
+"};
+
 const GENERAL_HELP: &'static str = indoc! {"
 General Help
 
@@ -117,6 +149,7 @@ impl HelpTopic {
             HelpTopic::DicePool => DICEPOOL_HELP,
             HelpTopic::Cthulhu => CTHULHU_HELP,
             HelpTopic::RollingDice => DICE_HELP,
+            HelpTopic::Variables => VARIABLES_HELP,
             HelpTopic::General => GENERAL_HELP,
         }
     }

dicebot/src/lib.rs

@@ -14,3 +14,4 @@ pub mod models;
 mod parser;
 pub mod rpc;
 pub mod state;
+pub mod systems;

dicebot/src/matrix.rs

@@ -2,15 +2,12 @@ use std::path::PathBuf;
 
 use futures::stream::{self, StreamExt, TryStreamExt};
 use log::error;
-use matrix_sdk::{events::room::message::NoticeMessageEventContent, room::Joined, ClientConfig};
-use matrix_sdk::{
-    events::room::message::{InReplyTo, Relation},
-    events::room::message::{MessageEventContent, MessageType},
-    events::AnyMessageEventContent,
-    identifiers::EventId,
-    Error as MatrixError,
-};
-use matrix_sdk::{identifiers::RoomId, identifiers::UserId, Client};
+use matrix_sdk::ruma::events::room::message::{InReplyTo, RoomMessageEventContent, Relation};
+use matrix_sdk::ruma::events::AnyMessageLikeEventContent;
+use matrix_sdk::ruma::{RoomId, OwnedEventId, OwnedUserId};
+use matrix_sdk::Client;
+use matrix_sdk::Error as MatrixError;
+use matrix_sdk::room::Joined;
 use url::Url;
 
 use crate::{config::Config, error::BotError};
@@ -32,12 +29,16 @@ fn extract_error_message(error: MatrixError) -> String {
 }
 
 /// Creates the matrix client.
-pub fn create_client(config: &Config) -> Result<Client, BotError> {
+pub async fn create_client(config: &Config) -> Result<Client, BotError> {
     let cache_dir = cache_dir()?;
-    let client_config = ClientConfig::new().store_path(cache_dir);
     let homeserver_url = Url::parse(&config.matrix_homeserver())?;
 
-    Ok(Client::new_with_config(homeserver_url, client_config)?)
+    let client = Client::builder()
+        .sled_store(cache_dir, None)?
+        .homeserver_url(homeserver_url).build()
+        .await?;
+
+    Ok(client)
 }
 
 /// Retrieve a list of users in a given room.
@@ -59,7 +60,7 @@ pub async fn get_users_in_room(
 
 pub async fn get_rooms_for_user(
     client: &Client,
-    user: &UserId,
+    user: &OwnedUserId,
 ) -> Result<Vec<Joined>, MatrixError> {
     // Carries errors through, in case we cannot load joined user IDs
     // from the room for some reason.
@@ -87,7 +88,7 @@ pub async fn send_message(
     client: &Client,
     room_id: &RoomId,
     message: (&str, &str),
-    reply_to: Option<EventId>,
+    reply_to: Option<OwnedEventId>,
 ) {
     let (html, plain) = message;
     let room = match client.get_joined_room(room_id) {
@@ -95,15 +96,13 @@ pub async fn send_message(
         _ => return,
     };
 
-    let mut content = MessageEventContent::new(MessageType::Notice(
-        NoticeMessageEventContent::html(plain.trim(), html),
-    ));
+    let mut content = RoomMessageEventContent::notice_html(plain.trim(), html);
 
     content.relates_to = reply_to.map(|event_id| Relation::Reply {
-        in_reply_to: InReplyTo::new(event_id),
+        in_reply_to: InReplyTo::new(event_id)
     });
 
-    let content = AnyMessageEventContent::RoomMessage(content);
+    let content = AnyMessageLikeEventContent::RoomMessage(content);
 
     let result = room.send(content, None).await;
 

dicebot/src/parser/dice.rs

@@ -151,8 +151,9 @@ where
 /// should not have an operator, but every one after that should.
 /// Accepts expressions like "8", "10 + variablename", "variablename -
 /// 3", etc. This function is currently common to systems that don't
-/// deal with XdY rolls. Support for that will be added later.
-pub fn parse_amounts(input: &str) -> ParseResult<Vec<Amount>> {
+/// deal with XdY rolls. Support for that will be added later. Returns
+/// parsed amounts and unconsumed input (e.g. roll modifiers).
+pub fn parse_amounts(input: &str) -> ParseResult<(Vec<Amount>, &str)> {
     let input = input.trim();
 
     let remaining_amounts = many(amount_parser()).map(|amounts: Vec<ParseResult<Amount>>| amounts);
@@ -169,31 +170,23 @@ pub fn parse_amounts(input: &str) -> ParseResult<Vec<Amount>> {
             (amounts, results.1)
         })?;
 
-    if rest.len() == 0 {
-        // Any ParseResult errors will short-circuit the collect.
-        results.into_iter().collect()
-    } else {
-        Err(DiceParsingError::UnconsumedInput)
-    }
+    // Any ParseResult errors will short-circuit the collect.
+    let results: Vec<Amount> = results.into_iter().collect::<ParseResult<_>>()?;
+    Ok((results, rest))
 }
 
 /// Parse an expression that expects a single number or variable. No
 /// operators are allowed. This function is common to systems that
 /// don't deal with XdY rolls. Currently. this function does not
-/// support parsing negative numbers.
-pub fn parse_single_amount(input: &str) -> ParseResult<Amount> {
+/// support parsing negative numbers. Returns the parsed amount and
+/// any unconsumed input (useful for dice roll modifiers).
+pub fn parse_single_amount(input: &str) -> ParseResult<(Amount, &str)> {
     // TODO add support for negative numbers, as technically they
     // should be allowed.
     let input = input.trim();
     let mut parser = first_amount_parser().map(|amount: ParseResult<Amount>| amount);
-
     let (result, rest) = parser.parse(input)?;
-
-    if rest.len() == 0 {
-        result
-    } else {
-        Err(DiceParsingError::UnconsumedInput)
-    }
+    Ok((result?, rest))
 }
 
 #[cfg(test)]
@@ -206,10 +199,13 @@ mod parse_single_amount_tests {
         assert!(result.is_ok());
         assert_eq!(
             result.unwrap(),
-            Amount {
-                operator: Operator::Plus,
-                element: Element::Variable("abc".to_string())
-            }
+            (
+                Amount {
+                    operator: Operator::Plus,
+                    element: Element::Variable("abc".to_string())
+                },
+                ""
+            )
         )
     }
 
@@ -233,24 +229,15 @@ mod parse_single_amount_tests {
         assert!(result.is_ok());
         assert_eq!(
             result.unwrap(),
-            Amount {
-                operator: Operator::Plus,
-                element: Element::Number(1)
-            }
+            (
+                Amount {
+                    operator: Operator::Plus,
+                    element: Element::Number(1)
+                },
+                ""
+            )
         )
     }
-
-    #[test]
-    fn parse_multiple_elements_test() {
-        let result = parse_single_amount("1+abc");
-        assert!(result.is_err());
-
-        let result = parse_single_amount("abc+1");
-        assert!(result.is_err());
-
-        let result = parse_single_amount("-1-abc");
-        assert!(result.is_err());
-    }
 }
 
 #[cfg(test)]
@@ -263,20 +250,26 @@ mod parse_many_amounts_tests {
         assert!(result.is_ok());
         assert_eq!(
             result.unwrap(),
-            vec![Amount {
-                operator: Operator::Plus,
-                element: Element::Number(1)
-            }]
+            (
+                vec![Amount {
+                    operator: Operator::Plus,
+                    element: Element::Number(1)
+                }],
+                ""
+            )
         );
 
         let result = parse_amounts("10");
         assert!(result.is_ok());
         assert_eq!(
             result.unwrap(),
-            vec![Amount {
-                operator: Operator::Plus,
-                element: Element::Number(10)
-            }]
+            (
+                vec![Amount {
+                    operator: Operator::Plus,
+                    element: Element::Number(10)
+                }],
+                ""
+            )
         );
     }
 
@@ -295,20 +288,26 @@ mod parse_many_amounts_tests {
         assert!(result.is_ok());
         assert_eq!(
             result.unwrap(),
-            vec![Amount {
-                operator: Operator::Plus,
-                element: Element::Variable("asdf".to_string())
-            }]
+            (
+                vec![Amount {
+                    operator: Operator::Plus,
+                    element: Element::Variable("asdf".to_string())
+                }],
+                ""
+            )
         );
 
         let result = parse_amounts("nosis");
         assert!(result.is_ok());
         assert_eq!(
             result.unwrap(),
-            vec![Amount {
-                operator: Operator::Plus,
-                element: Element::Variable("nosis".to_string())
-            }]
+            (
+                vec![Amount {
+                    operator: Operator::Plus,
+                    element: Element::Variable("nosis".to_string())
+                }],
+                ""
+            )
         );
     }
 

dicebot/src/rpc/service.rs

@@ -4,7 +4,8 @@ use crate::matrix;
 use crate::{config::Config, db::sqlite::Database};
 use futures::stream;
 use futures::{StreamExt, TryFutureExt, TryStreamExt};
-use matrix_sdk::{identifiers::UserId, room::Joined, Client};
+use matrix_sdk::ruma::OwnedUserId;
+use matrix_sdk::{room::Joined, Client};
 use std::convert::TryFrom;
 use std::sync::Arc;
 use tenebrous_rpc::protos::dicebot::{
@@ -84,7 +85,7 @@ impl Dicebot for DicebotRpcService {
         request: Request<UserIdRequest>,
     ) -> Result<Response<RoomsListReply>, Status> {
         let UserIdRequest { user_id } = request.into_inner();
-        let user_id = UserId::try_from(user_id).map_err(BotError::from)?;
+        let user_id = OwnedUserId::try_from(user_id).map_err(BotError::from)?;
 
         let rooms_for_user = matrix::get_rooms_for_user(&self.client, &user_id)
             .err_into::<BotError>()
@@ -94,7 +95,7 @@ impl Dicebot for DicebotRpcService {
             .filter_map(|room: Joined| async move {
                 let room: Result<Room, _> = room.display_name().await.map(|room_name| Room {
                     room_id: room.room_id().to_string(),
-                    display_name: room_name,
+                    display_name: room_name.to_string(),
                 });
 
                 Some(room)

dicebot/src/systems/mod.rs

@@ -0,0 +1,21 @@
+use strum::{AsRefStr, Display, EnumIter, EnumString};
+
+#[derive(EnumString, EnumIter, AsRefStr, Display)]
+pub(crate) enum GameSystem {
+    ChroniclesOfDarkness,
+    Changeling,
+    MageTheAwakening,
+    WerewolfTheForsaken,
+    DeviantTheRenegades,
+    MummyTheCurse,
+    PrometheanTheCreated,
+    CallOfCthulhu,
+    DungeonsAndDragons5e,
+    DungeonsAndDragons4e,
+    DungeonsAndDragons35e,
+    DungeonsAndDragons2e,
+    DungeonsAndDragons1e,
+    None,
+}
+
+impl GameSystem {}