refactor the lobby handler
authoralfadur
Tue, 01 Oct 2019 23:48:32 +0300
changeset 15444 a158ff8f84ef
parent 15443 e7c059ac6e54
child 15445 96e438b114f0
refactor the lobby handler
rust/hedgewars-server/src/core/indexslab.rs
rust/hedgewars-server/src/core/server.rs
rust/hedgewars-server/src/handlers.rs
rust/hedgewars-server/src/handlers/common.rs
rust/hedgewars-server/src/handlers/inlobby.rs
rust/hedgewars-server/src/handlers/inroom.rs
rust/hedgewars-server/src/protocol.rs
--- a/rust/hedgewars-server/src/core/indexslab.rs	Mon Sep 30 16:02:39 2019 +0200
+++ b/rust/hedgewars-server/src/core/indexslab.rs	Tue Oct 01 23:48:32 2019 +0300
@@ -41,7 +41,7 @@
         }
     }
 
-    pub fn iter(&self) -> impl Iterator<Item = (usize, &T)> {
+    pub fn iter(&self) -> impl Iterator<Item = (usize, &T)> + Clone {
         self.data
             .iter()
             .enumerate()
--- a/rust/hedgewars-server/src/core/server.rs	Mon Sep 30 16:02:39 2019 +0200
+++ b/rust/hedgewars-server/src/core/server.rs	Tue Oct 01 23:48:32 2019 +0300
@@ -2,17 +2,32 @@
     client::HwClient,
     indexslab::IndexSlab,
     room::HwRoom,
-    types::{ClientId, RoomId},
+    types::{ClientId, RoomId, ServerVar},
 };
 use crate::{protocol::messages::HwProtocolMessage::Greeting, utils};
 
+use crate::core::server::JoinRoomError::WrongProtocol;
 use bitflags::*;
 use log::*;
 use slab;
-use std::{borrow::BorrowMut, iter, num::NonZeroU16};
+use std::{borrow::BorrowMut, collections::HashSet, iter, num::NonZeroU16};
 
 type Slab<T> = slab::Slab<T>;
 
+pub enum CreateRoomError {
+    InvalidName,
+    AlreadyExists,
+}
+
+pub enum JoinRoomError {
+    DoesntExist,
+    WrongProtocol,
+    Full,
+    Restricted,
+}
+
+pub struct AccessError();
+
 pub struct HwAnteClient {
     pub nick: Option<String>,
     pub protocol_number: Option<NonZeroU16>,
@@ -43,7 +58,7 @@
     }
 
     pub fn remove_client(&mut self, client_id: ClientId) -> Option<HwAnteClient> {
-        let mut client = self.clients.remove(client_id);
+        let client = self.clients.remove(client_id);
         client
     }
 }
@@ -115,29 +130,123 @@
     }
 
     #[inline]
+    pub fn get_client_nick(&self, client_id: ClientId) -> &str {
+        &self.clients[client_id].nick
+    }
+
+    #[inline]
     pub fn create_room(
         &mut self,
         creator_id: ClientId,
         name: String,
         password: Option<String>,
-    ) -> RoomId {
-        create_room(
-            &mut self.clients[creator_id],
-            &mut self.rooms,
-            name,
-            password,
-        )
+    ) -> Result<(&HwClient, &HwRoom), CreateRoomError> {
+        use CreateRoomError::*;
+        if utils::is_name_illegal(&name) {
+            Err(InvalidName)
+        } else if self.has_room(&name) {
+            Err(AlreadyExists)
+        } else {
+            Ok(create_room(
+                &mut self.clients[creator_id],
+                &mut self.rooms,
+                name,
+                password,
+            ))
+        }
+    }
+
+    pub fn join_room(
+        &mut self,
+        client_id: ClientId,
+        room_id: RoomId,
+    ) -> Result<(&HwClient, &HwRoom, impl Iterator<Item = &HwClient> + Clone), JoinRoomError> {
+        use JoinRoomError::*;
+        let room = &mut self.rooms[room_id];
+        let client = &mut self.clients[client_id];
+
+        if client.protocol_number != room.protocol_number {
+            Err(WrongProtocol)
+        } else if room.is_join_restricted() {
+            Err(Restricted)
+        } else if room.players_number == u8::max_value() {
+            Err(Full)
+        } else {
+            move_to_room(client, room);
+            let room_id = room.id;
+            Ok((
+                &self.clients[client_id],
+                &self.rooms[room_id],
+                self.clients.iter().map(|(_, c)| c),
+            ))
+        }
     }
 
     #[inline]
-    pub fn move_to_room(&mut self, client_id: ClientId, room_id: RoomId) {
-        move_to_room(&mut self.clients[client_id], &mut self.rooms[room_id])
+    pub fn join_room_by_name(
+        &mut self,
+        client_id: ClientId,
+        room_name: &str,
+    ) -> Result<(&HwClient, &HwRoom, impl Iterator<Item = &HwClient> + Clone), JoinRoomError> {
+        use JoinRoomError::*;
+        let room = self.rooms.iter().find(|(_, r)| r.name == room_name);
+        if let Some((_, room)) = room {
+            let room_id = room.id;
+            self.join_room(client_id, room_id)
+        } else {
+            Err(DoesntExist)
+        }
+    }
+
+    #[inline]
+    pub fn set_var(&mut self, client_id: ClientId, var: ServerVar) -> Result<(), AccessError> {
+        if self.clients[client_id].is_admin() {
+            match var {
+                ServerVar::MOTDNew(msg) => self.greetings.for_latest_protocol = msg,
+                ServerVar::MOTDOld(msg) => self.greetings.for_old_protocols = msg,
+                ServerVar::LatestProto(n) => self.latest_protocol = n,
+            }
+            Ok(())
+        } else {
+            Err(AccessError())
+        }
     }
 
+    #[inline]
+    pub fn get_vars(&self, client_id: ClientId) -> Result<[ServerVar; 3], AccessError> {
+        if self.clients[client_id].is_admin() {
+            Ok([
+                ServerVar::MOTDNew(self.greetings.for_latest_protocol.clone()),
+                ServerVar::MOTDOld(self.greetings.for_old_protocols.clone()),
+                ServerVar::LatestProto(self.latest_protocol),
+            ])
+        } else {
+            Err(AccessError())
+        }
+    }
+
+    pub fn get_used_protocols(&self, client_id: ClientId) -> Result<Vec<u16>, AccessError> {
+        if self.clients[client_id].is_admin() {
+            let mut protocols: HashSet<_> = self
+                .clients
+                .iter()
+                .map(|(_, c)| c.protocol_number)
+                .chain(self.rooms.iter().map(|(_, r)| r.protocol_number))
+                .collect();
+            let mut protocols: Vec<_> = protocols.drain().collect();
+            protocols.sort();
+            Ok(protocols)
+        } else {
+            Err(AccessError())
+        }
+    }
+
+    #[inline]
     pub fn has_room(&self, name: &str) -> bool {
         self.find_room(name).is_some()
     }
 
+    #[inline]
     pub fn find_room(&self, name: &str) -> Option<&HwRoom> {
         self.rooms
             .iter()
@@ -234,12 +343,12 @@
     entry.insert(room)
 }
 
-fn create_room(
-    client: &mut HwClient,
-    rooms: &mut Slab<HwRoom>,
+fn create_room<'a, 'b>(
+    client: &'a mut HwClient,
+    rooms: &'b mut Slab<HwRoom>,
     name: String,
     password: Option<String>,
-) -> RoomId {
+) -> (&'a HwClient, &'b HwRoom) {
     let room = allocate_room(rooms);
 
     room.master_id = Some(client.id);
@@ -255,7 +364,7 @@
     client.set_is_ready(true);
     client.set_is_joined_mid_game(false);
 
-    room.id
+    (client, room)
 }
 
 fn move_to_room(client: &mut HwClient, room: &mut HwRoom) {
--- a/rust/hedgewars-server/src/handlers.rs	Mon Sep 30 16:02:39 2019 +0200
+++ b/rust/hedgewars-server/src/handlers.rs	Tue Oct 01 23:48:32 2019 +0300
@@ -32,6 +32,7 @@
 mod inanteroom;
 mod inlobby;
 mod inroom;
+mod strings;
 
 #[derive(PartialEq, Debug)]
 pub struct Sha1Digest([u8; 20]);
@@ -160,6 +161,11 @@
     }
 
     #[inline]
+    pub fn warn(&mut self, message: &str) {
+        self.add(Warning(message.to_string()).send_self());
+    }
+
+    #[inline]
     pub fn request_io(&mut self, task: IoTask) {
         self.io_tasks.push(task)
     }
--- a/rust/hedgewars-server/src/handlers/common.rs	Mon Sep 30 16:02:39 2019 +0200
+++ b/rust/hedgewars-server/src/handlers/common.rs	Tue Oct 01 23:48:32 2019 +0300
@@ -2,7 +2,7 @@
     core::{
         client::HwClient,
         room::HwRoom,
-        server::HwServer,
+        server::{HwServer, JoinRoomError},
         types::{ClientId, GameCfg, RoomId, TeamInfo, Vote, VoteType},
     },
     protocol::messages::{
@@ -227,32 +227,43 @@
     );
 }
 
-pub fn enter_room(
-    server: &mut HwServer,
-    client_id: ClientId,
-    room_id: RoomId,
+pub fn get_room_join_data<'a, I: Iterator<Item = &'a HwClient> + Clone>(
+    client: &HwClient,
+    room: &HwRoom,
+    room_clients: I,
     response: &mut Response,
 ) {
-    let nick = server.clients[client_id].nick.clone();
-    server.move_to_room(client_id, room_id);
+    #[inline]
+    fn collect_nicks<'a, I, F>(clients: I, f: F) -> Vec<String>
+    where
+        I: Iterator<Item = &'a HwClient>,
+        F: Fn(&&'a HwClient) -> bool,
+    {
+        clients.filter(f).map(|c| &c.nick).cloned().collect()
+    }
 
-    response.add(RoomJoined(vec![nick.clone()]).send_all().in_room(room_id));
+    let nick = client.nick.clone();
+    response.add(RoomJoined(vec![nick.clone()]).send_all().in_room(room.id));
     response.add(ClientFlags(add_flags(&[Flags::InRoom]), vec![nick]).send_all());
-    let nicks = server.collect_nicks(|(_, c)| c.room_id == Some(room_id));
+    let nicks = collect_nicks(room_clients.clone(), |c| c.room_id == Some(room.id));
     response.add(RoomJoined(nicks).send_self());
 
-    get_room_teams(server, room_id, client_id, response);
-
-    let room = &server.rooms[room_id];
-    get_room_config(room, client_id, response);
+    get_room_teams(room, client.id, response);
+    get_room_config(room, client.id, response);
 
     let mut flag_selectors = [
         (
             Flags::RoomMaster,
-            server.collect_nicks(|(_, c)| c.is_master()),
+            collect_nicks(room_clients.clone(), |c| c.is_master()),
         ),
-        (Flags::Ready, server.collect_nicks(|(_, c)| c.is_ready())),
-        (Flags::InGame, server.collect_nicks(|(_, c)| c.is_in_game())),
+        (
+            Flags::Ready,
+            collect_nicks(room_clients.clone(), |c| c.is_ready()),
+        ),
+        (
+            Flags::InGame,
+            collect_nicks(room_clients.clone(), |c| c.is_in_game()),
+        ),
     ];
 
     for (flag, nicks) in &mut flag_selectors {
@@ -270,6 +281,16 @@
     }
 }
 
+pub fn get_room_join_error(error: JoinRoomError, response: &mut Response) {
+    use super::strings::*;
+    match error {
+        JoinRoomError::DoesntExist => response.warn(NO_ROOM),
+        JoinRoomError::WrongProtocol => response.warn(WRONG_PROTOCOL),
+        JoinRoomError::Full => response.warn(ROOM_FULL),
+        JoinRoomError::Restricted => response.warn(ROOM_JOIN_RESTRICTED),
+    }
+}
+
 pub fn exit_room(server: &mut HwServer, client_id: ClientId, response: &mut Response, msg: &str) {
     let client = &mut server.clients[client_id];
 
@@ -354,13 +375,7 @@
     }
 }
 
-pub fn get_room_teams(
-    server: &HwServer,
-    room_id: RoomId,
-    to_client: ClientId,
-    response: &mut Response,
-) {
-    let room = &server.rooms[room_id];
+pub fn get_room_teams(room: &HwRoom, to_client: ClientId, response: &mut Response) {
     let current_teams = match room.game_info {
         Some(ref info) => &info.teams_at_start,
         None => &room.teams,
--- a/rust/hedgewars-server/src/handlers/inlobby.rs	Mon Sep 30 16:02:39 2019 +0200
+++ b/rust/hedgewars-server/src/handlers/inlobby.rs	Tue Oct 01 23:48:32 2019 +0300
@@ -1,10 +1,10 @@
 use mio;
 
-use super::common::rnd_reply;
+use super::{common::rnd_reply, strings::*};
 use crate::{
     core::{
         client::HwClient,
-        server::HwServer,
+        server::{AccessError, CreateRoomError, HwServer, JoinRoomError},
         types::{ClientId, ServerVar},
     },
     protocol::messages::{
@@ -23,41 +23,34 @@
     message: HwProtocolMessage,
 ) {
     use crate::protocol::messages::HwProtocolMessage::*;
+
     match message {
-        CreateRoom(name, password) => {
-            if is_name_illegal(&name) {
-                response.add(Warning("Illegal room name! A room name must be between 1-40 characters long, must not have a trailing or leading space and must not have any of these characters: $()*+?[]^{|}".to_string()).send_self());
-            } else if server.has_room(&name) {
-                response.add(
-                    Warning("A room with the same name already exists.".to_string()).send_self(),
-                );
-            } else {
-                let flags_msg = ClientFlags(
-                    add_flags(&[Flags::RoomMaster, Flags::Ready]),
-                    vec![server.clients[client_id].nick.clone()],
-                );
-
-                let room_id = server.create_room(client_id, name, password);
-                let room = &server.rooms[room_id];
-                let client = &server.clients[client_id];
-
+        CreateRoom(name, password) => match server.create_room(client_id, name, password) {
+            Err(CreateRoomError::InvalidName) => response.warn(ILLEGAL_ROOM_NAME),
+            Err(CreateRoomError::AlreadyExists) => response.warn(ROOM_EXISTS),
+            Ok((client, room)) => {
                 response.add(
                     RoomAdd(room.info(Some(&client)))
                         .send_all()
                         .with_protocol(room.protocol_number),
                 );
                 response.add(RoomJoined(vec![client.nick.clone()]).send_self());
-                response.add(flags_msg.send_self());
-
+                response.add(
+                    ClientFlags(
+                        add_flags(&[Flags::RoomMaster, Flags::Ready]),
+                        vec![client.nick.clone()],
+                    )
+                    .send_self(),
+                );
                 response.add(
                     ClientFlags(add_flags(&[Flags::InRoom]), vec![client.nick.clone()]).send_self(),
                 );
-            };
-        }
+            }
+        },
         Chat(msg) => {
             response.add(
                 ChatMsg {
-                    nick: server.clients[client_id].nick.clone(),
+                    nick: server.get_client_nick(client_id).to_string(),
                     msg,
                 }
                 .send_all()
@@ -65,99 +58,62 @@
                 .but_self(),
             );
         }
-        JoinRoom(name, _password) => {
-            let room = server.rooms.iter().find(|(_, r)| r.name == name);
-            let room_id = room.map(|(_, r)| r.id);
-
-            let client = &mut server.clients[client_id];
-
-            if let Some((_, room)) = room {
-                if client.protocol_number != room.protocol_number {
-                    response.add(
-                        Warning("Room version incompatible to your Hedgewars version!".to_string())
-                            .send_self(),
-                    );
-                } else if room.is_join_restricted() {
-                    response.add(
-                        Warning(
-                            "Access denied. This room currently doesn't allow joining.".to_string(),
-                        )
-                        .send_self(),
-                    );
-                } else if room.players_number == u8::max_value() {
-                    response.add(Warning("This room is already full".to_string()).send_self());
-                } else if let Some(room_id) = room_id {
-                    super::common::enter_room(server, client_id, room_id, response);
+        JoinRoom(name, _password) => match server.join_room_by_name(client_id, &name) {
+            Err(error) => super::common::get_room_join_error(error, response),
+            Ok((client, room, room_clients)) => {
+                super::common::get_room_join_data(client, room, room_clients, response)
+            }
+        },
+        Follow(nick) => {
+            if let Some(client) = server.find_client(&nick) {
+                if let Some(room_id) = client.room_id {
+                    match server.join_room(client_id, room_id) {
+                        Err(error) => super::common::get_room_join_error(error, response),
+                        Ok((client, room, room_clients)) => {
+                            super::common::get_room_join_data(client, room, room_clients, response)
+                        }
+                    }
+                } else {
+                    response.warn(NO_ROOM);
                 }
             } else {
-                response.add(Warning("No such room.".to_string()).send_self());
+                response.warn(NO_USER);
             }
         }
-        Follow(nick) => {
-            if let Some(HwClient {
-                room_id: Some(room_id),
-                ..
-            }) = server.find_client(&nick)
-            {
-                let room = &server.rooms[*room_id];
-                response.add(Joining(room.name.clone()).send_self());
-                super::common::enter_room(server, client_id, *room_id, response);
+        SetServerVar(var) => match server.set_var(client_id, var) {
+            Err(AccessError()) => response.warn(ACCESS_DENIED),
+            Ok(()) => response.add(server_chat(VARIABLE_UPDATED.to_string()).send_self()),
+        },
+        GetServerVar => match server.get_vars(client_id) {
+            Err(AccessError()) => response.warn(ACCESS_DENIED),
+            Ok(vars) => {
+                response.add(
+                    ServerVars(vars.iter().flat_map(|v| v.to_protocol()).collect()).send_self(),
+                );
             }
-        }
-        SetServerVar(var) => {
-            if !server.clients[client_id].is_admin() {
-                response.add(Warning("Access denied.".to_string()).send_self());
-            } else {
-                match var {
-                    ServerVar::MOTDNew(msg) => server.greetings.for_latest_protocol = msg,
-                    ServerVar::MOTDOld(msg) => server.greetings.for_old_protocols = msg,
-                    ServerVar::LatestProto(n) => server.latest_protocol = n,
-                }
-            }
-        }
-        GetServerVar => {
-            if !server.clients[client_id].is_admin() {
-                response.add(Warning("Access denied.".to_string()).send_self());
-            } else {
-                let vars: Vec<_> = [
-                    ServerVar::MOTDNew(server.greetings.for_latest_protocol.clone()),
-                    ServerVar::MOTDOld(server.greetings.for_old_protocols.clone()),
-                    ServerVar::LatestProto(server.latest_protocol),
-                ]
-                .iter()
-                .flat_map(|v| v.to_protocol())
-                .collect();
-                response.add(ServerVars(vars).send_self());
-            }
-        }
+        },
         Rnd(v) => {
             response.add(rnd_reply(&v).send_self());
         }
-        Stats => {
-            let mut protocols: HashSet<_> = server
-                .clients
-                .iter()
-                .map(|(_, c)| c.protocol_number)
-                .chain(server.rooms.iter().map(|(_, r)| r.protocol_number))
-                .collect();
-            let mut protocols: Vec<_> = protocols.drain().collect();
-            protocols.sort();
-
-            let mut html = Vec::with_capacity(protocols.len() + 2);
+        Stats => match server.get_used_protocols(client_id) {
+            Err(AccessError()) => response.warn(ACCESS_DENIED),
+            Ok(protocols) => {
+                let mut html = Vec::with_capacity(protocols.len() + 2);
 
-            html.push("<table>".to_string());
-            for protocol in protocols {
-                html.push(format!(
-                    "<tr><td>{}</td><td>{}</td><td>{}</td></tr>",
-                    super::utils::protocol_version_string(protocol),
-                    server.protocol_clients(protocol).count(),
-                    server.protocol_rooms(protocol).count()
-                ));
+                html.push("<table>".to_string());
+                for protocol in protocols {
+                    html.push(format!(
+                        "<tr><td>{}</td><td>{}</td><td>{}</td></tr>",
+                        super::utils::protocol_version_string(protocol),
+                        server.protocol_clients(protocol).count(),
+                        server.protocol_rooms(protocol).count()
+                    ));
+                }
+                html.push("</table>".to_string());
+
+                response.add(Warning(html.join("")).send_self());
             }
-            html.push("</table>".to_string());
-
-            response.add(Warning(html.join("")).send_self());
-        }
+        },
         List => warn!("Deprecated LIST message received"),
         _ => warn!("Incorrect command in lobby state"),
     }
--- a/rust/hedgewars-server/src/handlers/inroom.rs	Mon Sep 30 16:02:39 2019 +0200
+++ b/rust/hedgewars-server/src/handlers/inroom.rs	Tue Oct 01 23:48:32 2019 +0300
@@ -52,7 +52,7 @@
         [size, typ, body..MAX] => {
             VALID_MESSAGES.contains(typ)
                 && match body {
-                    [1...MAX_HEDGEHOGS_PER_TEAM, team, ..] if *typ == b'h' => {
+                    [1..=MAX_HEDGEHOGS_PER_TEAM, team, ..] if *typ == b'h' => {
                         team_indices.contains(team)
                     }
                     _ => *typ != b'h',
@@ -272,12 +272,8 @@
             Some((_, name)) => {
                 client.teams_in_game -= 1;
                 client.clan = room.find_team_color(client.id);
-                super::common::remove_teams(
-                    room,
-                    vec![name.to_string()],
-                    client.is_in_game(),
-                    response,
-                );
+                let names = vec![name.to_string()];
+                super::common::remove_teams(room, names, client.is_in_game(), response);
 
                 match room.game_info {
                     Some(ref info) if info.teams_in_game == 0 => {
@@ -438,7 +434,7 @@
                 }
                 VoteType::NewSeed => None,
                 VoteType::HedgehogsPerTeam(number) => match number {
-                    1...MAX_HEDGEHOGS_PER_TEAM => None,
+                    1..=MAX_HEDGEHOGS_PER_TEAM => None,
                     _ => Some("/callvote hedgehogs: Specify number from 1 to 8.".to_string()),
                 },
             };
--- a/rust/hedgewars-server/src/protocol.rs	Mon Sep 30 16:02:39 2019 +0200
+++ b/rust/hedgewars-server/src/protocol.rs	Tue Oct 01 23:48:32 2019 +0300
@@ -24,7 +24,8 @@
     fn recover(&mut self) -> bool {
         self.is_recovering = match parser::malformed_message(&self.buf[..]) {
             Ok((tail, ())) => {
-                self.buf.consume(self.buf.len() - tail.len());
+                let length = tail.len();
+                self.buf.consume(self.buf.len() - length);
                 false
             }
             _ => {
@@ -50,7 +51,8 @@
                 match parser::message(&self.buf[..]) {
                     Ok((tail, message)) => {
                         messages.push(message);
-                        self.buf.consume(self.buf.len() - tail.len());
+                        let length = tail.len();
+                        self.buf.consume(self.buf.len() - length);
                     }
                     Err(nom::Err::Incomplete(_)) => break,
                     Err(nom::Err::Failure(e)) | Err(nom::Err::Error(e)) => {