Lobby: Make player ordinals client controlled (#666)

- Thanks to uniqueness & upper bound constraint on the ordinal, it
  serves as a fully consistent maximum number of players constraint.

- It delegates ordinal (player number) assignment to the client who
  already receives such info from DE Connector.

Fixes #308.

crates/lobby/src/auth/db.rs

@@ -7,7 +7,7 @@ use thiserror::Error;
 use super::passwd::{DbPassword, MAX_PASS_HASH_LEN, MAX_PASS_SALT_LEN};
 use crate::{
     db::{FromRow, SQLITE_CONSTRAINT_PRIMARYKEY},
-    db_error,
+    db_error_code,
 };
 
 #[derive(Clone)]
@@ -48,7 +48,7 @@ impl Users {
             .execute(self.pool)
             .await;
 
-        db_error!(
+        db_error_code!(
             result,
             RegistrationError::UsernameTaken,
             SQLITE_CONSTRAINT_PRIMARYKEY

crates/lobby/src/db.rs

@@ -1,11 +1,10 @@
 use sqlx::sqlite::SqliteRow;
 
 pub const SQLITE_CONSTRAINT_PRIMARYKEY: &str = "1555";
-pub const SQLITE_CONSTRAINT_UNIQUE: &str = "2067";
 pub const SQLITE_CONSTRAINT_FOREIGNKEY: &str = "787";
 
 #[macro_export]
-macro_rules! db_error {
+macro_rules! db_error_code {
     ($result:expr, $error:expr, $code:expr) => {
         if let Err(sqlx::Error::Database(ref error)) = $result {
             if let Some(code) = error.code() {
@@ -17,6 +16,17 @@ macro_rules! db_error {
     };
 }
 
+#[macro_export]
+macro_rules! db_error_message {
+    ($result:expr, $error:expr, $message:expr) => {
+        if let Err(sqlx::Error::Database(ref error)) = $result {
+            if error.message() == $message {
+                return Err($error);
+            }
+        }
+    };
+}
+
 pub(crate) trait FromRow
 where
     Self: Sized,

crates/lobby/src/games/db.rs

@@ -2,20 +2,17 @@ use std::net::SocketAddr;
 
 use anyhow::{Context, Result};
 use de_lobby_model::{
-    Game, GameConfig, GameListing, GameMap, GamePartial, GameSetup, MAP_HASH_LEN,
-    MAX_GAME_NAME_LEN, MAX_MAP_NAME_LEN, MAX_USERNAME_LEN,
+    Game, GameConfig, GameListing, GameMap, GamePartial, GamePlayer, GamePlayerInfo, GameSetup,
+    MAP_HASH_LEN, MAX_GAME_NAME_LEN, MAX_MAP_NAME_LEN, MAX_USERNAME_LEN,
 };
 use futures_util::TryStreamExt;
 use log::info;
 use sqlx::{query, sqlite::SqliteRow, Pool, Row, Sqlite, SqliteExecutor};
 use thiserror::Error;
 
 use crate::{
-    db::{
-        FromRow, SQLITE_CONSTRAINT_FOREIGNKEY, SQLITE_CONSTRAINT_PRIMARYKEY,
-        SQLITE_CONSTRAINT_UNIQUE,
-    },
-    db_error,
+    db::{FromRow, SQLITE_CONSTRAINT_FOREIGNKEY, SQLITE_CONSTRAINT_PRIMARYKEY},
+    db_error_code, db_error_message,
 };
 
 // This should correspond to the longest valid socket address. IPv6 hast up to
@@ -85,7 +82,7 @@ impl Games {
         let setup = GameSetup::try_from_row(game_row)?;
 
         let mut players = Vec::new();
-        let mut player_rows = query("SELECT username FROM players WHERE game = ?;")
+        let mut player_rows = query("SELECT ordinal, username FROM players WHERE game = ?;")
             .bind(game)
             .fetch(self.pool);
 
@@ -94,8 +91,7 @@ impl Games {
             .await
             .context("Failed to retrieve game players from the DB")?
         {
-            let username: String = player_row.try_get("username")?;
-            players.push(username);
+            players.push(GamePlayer::try_from_row(player_row)?);
         }
 
         Ok(Some(Game::new(setup, players)))
@@ -117,7 +113,7 @@ impl Games {
                 .bind(game_setup.server().to_string())
                 .execute(&mut transaction)
                 .await;
-        db_error!(
+        db_error_code!(
             result,
             CreationError::NameTaken,
             SQLITE_CONSTRAINT_PRIMARYKEY
@@ -140,36 +136,50 @@ impl Games {
         Ok(())
     }
 
-    pub(super) async fn add_player(&self, username: &str, game: &str) -> Result<(), AdditionError> {
-        Self::add_player_inner(self.pool, false, username, game).await
+    pub(super) async fn add_player(
+        &self,
+        player: &GamePlayer,
+        game: &str,
+    ) -> Result<(), AdditionError> {
+        Self::add_player_inner(self.pool, false, player, game).await
     }
 
     async fn add_player_inner<'c, E>(
         executor: E,
         author: bool,
-        username: &str,
+        player: &GamePlayer,
         game: &str,
     ) -> Result<(), AdditionError>
     where
         E: SqliteExecutor<'c>,
     {
-        let result = query("INSERT INTO players (author, username, game) VALUES (?, ?, ?);")
-            .bind(author)
-            .bind(username)
-            .bind(game)
-            .execute(executor)
-            .await;
+        let result =
+            query("INSERT INTO players (ordinal, author, username, game) VALUES (?, ?, ?, ?);")
+                .bind(player.info().ordinal())
+                .bind(author)
+                .bind(player.username())
+                .bind(game)
+                .execute(executor)
+                .await;
 
-        db_error!(
+        db_error_code!(
             result,
             AdditionError::UserOrGameDoesNotExist,
             SQLITE_CONSTRAINT_FOREIGNKEY
         );
-        db_error!(
+
+        db_error_message!(
             result,
             AdditionError::AlreadyInAGame,
-            SQLITE_CONSTRAINT_UNIQUE
+            "UNIQUE constraint failed: players.username"
         );
+        db_error_message!(
+            result,
+            AdditionError::OrdinalConflict,
+            "UNIQUE constraint failed: players.game, players.ordinal"
+        );
+        db_error_message!(result, AdditionError::OrdinalTooLarge, "TOO-LARGE-ORDINAL");
+
         result.map_err(AdditionError::Database)?;
 
         Ok(())
@@ -267,6 +277,10 @@ pub(super) enum CreationError {
 pub(super) enum AdditionError {
     #[error("User is already in another game")]
     AlreadyInAGame,
+    #[error("Another player already joined the game with the same ordinal")]
+    OrdinalConflict,
+    #[error("Player ordinal is larger than maximum number of players in the game")]
+    OrdinalTooLarge,
     #[error("The user or the game does not exist")]
     UserOrGameDoesNotExist,
     #[error("A database error encountered")]
@@ -283,6 +297,16 @@ pub(super) enum RemovalError {
     Database(#[source] sqlx::Error),
 }
 
+impl FromRow for GamePlayer {
+    type Error = anyhow::Error;
+
+    fn try_from_row(row: SqliteRow) -> Result<Self, Self::Error> {
+        let username: String = row.try_get("username")?;
+        let ordinal: u8 = row.try_get("ordinal")?;
+        Ok(Self::new(username, GamePlayerInfo::new(ordinal)))
+    }
+}
+
 impl FromRow for GameSetup {
     type Error = anyhow::Error;
 

crates/lobby/src/games/endpoints.rs

@@ -1,5 +1,5 @@
 use actix_web::{get, post, put, web, HttpResponse, Responder};
-use de_lobby_model::{Game, GameSetup, Validatable};
+use de_lobby_model::{Game, GamePlayer, GamePlayerInfo, GameSetup, Validatable};
 use log::{error, warn};
 
 use super::db::{AdditionError, CreationError, Games, RemovalError};
@@ -76,15 +76,33 @@ async fn join(
     claims: web::ReqData<Claims>,
     games: web::Data<Games>,
     path: web::Path<String>,
+    player_info: web::Json<GamePlayerInfo>,
 ) -> impl Responder {
     let name = path.into_inner();
 
-    match games.add_player(claims.username(), name.as_str()).await {
+    let ordinal = player_info.ordinal();
+    if ordinal == 0 {
+        warn!("Game joining error: got ordinal equal to 0.");
+        return HttpResponse::BadRequest().json("Ordinals start with 0, got 1.");
+    }
+
+    let player = GamePlayer::new(claims.username().to_owned(), player_info.0);
+    match games.add_player(&player, name.as_str()).await {
         Ok(_) => HttpResponse::Ok().json(()),
         Err(AdditionError::AlreadyInAGame) => {
             warn!("Game joining error: a user is already in a different game.");
             HttpResponse::Forbidden().json("User is already in a different game.")
         }
+        Err(AdditionError::OrdinalConflict) => {
+            warn!("Game joining error: player ordinal conflict.");
+            HttpResponse::Conflict()
+                .json("Another player has already joined the game under the given ordinal.")
+        }
+        Err(AdditionError::OrdinalTooLarge) => {
+            warn!("Game joining error: too large ordinal: {ordinal}");
+            HttpResponse::Conflict()
+                .json("The given ordinal is larger than maximum number of players.")
+        }
         Err(AdditionError::UserOrGameDoesNotExist) => {
             warn!("Game joining error: the game or the user does not exist");
             HttpResponse::NotFound().json("Game not found.")

crates/lobby/src/games/init.sql

@@ -7,15 +7,29 @@ CREATE TABLE IF NOT EXISTS games (
 );
 
 CREATE TABLE IF NOT EXISTS players (
-    ordinal INTEGER PRIMARY KEY AUTOINCREMENT,
+    ordinal TINYINT NOT NULL,
     author BOOLEAN NOT NULL,
-    username CHARACTER({username_len}) NOT NULL UNIQUE,
+    username CHARACTER({username_len}) NOT NULL,
     game CHARACTER({game_name_len}) NOT NULL,
 
+    CONSTRAINT username UNIQUE (username),
+    CONSTRAINT ordinal UNIQUE (game, ordinal),
+
     FOREIGN KEY(username) REFERENCES users(username)
         ON UPDATE CASCADE
         ON DELETE CASCADE,
     FOREIGN KEY(game) REFERENCES games(name)
         ON UPDATE CASCADE
         ON DELETE CASCADE
 );
+
+
+CREATE TRIGGER IF NOT EXISTS check_ordinal
+BEFORE INSERT ON players
+FOR EACH ROW
+BEGIN
+    SELECT CASE
+        WHEN (SELECT max_players FROM games WHERE name = NEW.game) IS NOT NULL AND NEW.ordinal > (SELECT max_players FROM games WHERE name = NEW.game)
+        THEN RAISE(FAIL, 'TOO-LARGE-ORDINAL')
+    END;
+END;

crates/lobby_model/src/games.rs

@@ -13,28 +13,70 @@ const MAX_PLAYERS: u8 = 4;
 #[serde(rename_all = "camelCase")]
 pub struct Game {
     setup: GameSetup,
-    players: Vec<String>,
+    players: Vec<GamePlayer>,
 }
 
 impl Game {
-    /// Creates a new game with the author being the only player.
+    /// Creates a new game with the author having ordinal number of 1 and being
+    /// the only player.
     pub fn from_author(setup: GameSetup, author: String) -> Self {
-        Self::new(setup, vec![author])
+        Self::new(setup, vec![GamePlayer::new(author, GamePlayerInfo::new(1))])
     }
 
-    pub fn new(setup: GameSetup, players: Vec<String>) -> Self {
+    pub fn new(setup: GameSetup, players: Vec<GamePlayer>) -> Self {
         Self { setup, players }
     }
 
     pub fn setup(&self) -> &GameSetup {
         &self.setup
     }
 
-    pub fn players(&self) -> &[String] {
+    pub fn players(&self) -> &[GamePlayer] {
         self.players.as_slice()
     }
 }
 
+#[derive(Serialize, Deserialize)]
+#[serde(rename_all = "camelCase")]
+pub struct GamePlayer {
+    username: String,
+    info: GamePlayerInfo,
+}
+
+impl GamePlayer {
+    pub fn new(username: String, info: GamePlayerInfo) -> Self {
+        Self { username, info }
+    }
+
+    pub fn username(&self) -> &str {
+        self.username.as_str()
+    }
+
+    pub fn info(&self) -> &GamePlayerInfo {
+        &self.info
+    }
+}
+
+#[derive(Serialize, Deserialize)]
+#[serde(rename_all = "camelCase")]
+pub struct GamePlayerInfo {
+    ordinal: u8,
+}
+
+impl GamePlayerInfo {
+    /// # Panics
+    ///
+    /// Panics if ordinal equal to 0 is used.
+    pub fn new(ordinal: u8) -> Self {
+        assert!(ordinal > 0);
+        Self { ordinal }
+    }
+
+    pub fn ordinal(&self) -> u8 {
+        self.ordinal
+    }
+}
+
 #[derive(Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct GameListing(Vec<GamePartial>);

crates/lobby_model/src/lib.rs

@@ -3,8 +3,8 @@ pub use auth::{
     MIN_PASSWORD_LEN,
 };
 pub use games::{
-    Game, GameConfig, GameListing, GameMap, GamePartial, GameSetup, MAP_HASH_LEN,
-    MAX_GAME_NAME_LEN, MAX_MAP_NAME_LEN,
+    Game, GameConfig, GameListing, GameMap, GamePartial, GamePlayer, GamePlayerInfo, GameSetup,
+    MAP_HASH_LEN, MAX_GAME_NAME_LEN, MAX_MAP_NAME_LEN,
 };
 pub use validation::Validatable;