From fe9ad3fa198827899c9eaced2aa9cbf186449e68 Mon Sep 17 00:00:00 2001 From: Shane Handley Date: Thu, 17 Oct 2019 13:09:01 +0900 Subject: [PATCH 01/18] Network timeout updates - Bugfix: Check whether the server response (pong) is greater than the timeout period, rather than the ping (which will always fire regardless of connection status) This was causing the timeout error event to never fire. - Feature: Send the player notifications to the chat window that they will be kicked due to disconnection for 6 seconds before kicking them back to the main menu. --- client/src/lib.rs | 24 +++++++++++++++++++++++- voxygen/src/session.rs | 13 +++++++++++-- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/client/src/lib.rs b/client/src/lib.rs index 3f1b611f55..b3c49ebfb5 100644 --- a/client/src/lib.rs +++ b/client/src/lib.rs @@ -30,14 +30,19 @@ use std::{ use uvth::{ThreadPool, ThreadPoolBuilder}; use vek::*; +// The duration of network inactivity until the player is kicked to the main menu const SERVER_TIMEOUT: Duration = Duration::from_secs(20); +// After this duration has elapsed, the user will begin getting kick warnings in their chat window +const SERVER_TIMEOUT_GRACE_PERIOD: Duration = Duration::from_secs(14); + pub enum Event { Chat { chat_type: ChatType, message: String, }, Disconnect, + DisconnectionNotification(u64), } pub struct Client { @@ -49,6 +54,7 @@ pub struct Client { postbox: PostBox, last_server_ping: Instant, + last_server_pong: Instant, last_ping_delta: f64, tick: u64, @@ -134,6 +140,7 @@ impl Client { postbox, last_server_ping: Instant::now(), + last_server_pong: Instant::now(), last_ping_delta: 0.0, tick: 0, @@ -494,6 +501,19 @@ impl Client { fn handle_new_messages(&mut self) -> Result, Error> { let mut frontend_events = Vec::new(); + // Check that we have an valid connection. + // Limit this to the ping time (1s), since that's the max resolution we have + if Instant::now().duration_since(self.last_server_ping) > Duration::from_secs(1) { + let duration_since_last_pong = Instant::now().duration_since(self.last_server_pong); + + // Dispatch a notification to the HUD warning they will be kicked in {n} seconds + if duration_since_last_pong.as_secs_f64() > SERVER_TIMEOUT_GRACE_PERIOD.as_secs_f64() { + frontend_events.push(Event::DisconnectionNotification( + SERVER_TIMEOUT.as_secs() - duration_since_last_pong.as_secs(), + )); + } + } + let new_msgs = self.postbox.new_messages(); if new_msgs.len() > 0 { @@ -508,6 +528,8 @@ impl Client { ServerMsg::InitialSync { .. } => return Err(Error::ServerWentMad), ServerMsg::Ping => self.postbox.send_message(ClientMsg::Pong), ServerMsg::Pong => { + self.last_server_pong = Instant::now(); + self.last_ping_delta = Instant::now() .duration_since(self.last_server_ping) .as_secs_f64() @@ -591,7 +613,7 @@ impl Client { } else if let Some(err) = self.postbox.error() { return Err(err.into()); // We regularily ping in the tick method - } else if Instant::now().duration_since(self.last_server_ping) > SERVER_TIMEOUT { + } else if Instant::now().duration_since(self.last_server_pong) > SERVER_TIMEOUT { return Err(Error::ServerTimeout); } Ok(frontend_events) diff --git a/voxygen/src/session.rs b/voxygen/src/session.rs index 0e89fd2907..1b1d560b9f 100644 --- a/voxygen/src/session.rs +++ b/voxygen/src/session.rs @@ -6,8 +6,9 @@ use crate::{ window::{Event, GameInput}, Direction, Error, GlobalState, PlayState, PlayStateResult, }; -use client::{self, Client}; +use client::{self, Client, Event::Chat}; use common::{ + ChatType, clock::Clock, comp, comp::{Pos, Vel}, @@ -55,13 +56,21 @@ impl SessionState { fn tick(&mut self, dt: Duration) -> Result<(), Error> { for event in self.client.borrow_mut().tick(self.inputs.clone(), dt)? { match event { - client::Event::Chat { + Chat { chat_type: _, message: _, } => { self.hud.new_message(event); } client::Event::Disconnect => {} // TODO + client::Event::DisconnectionNotification(time) => { + log::warn!("{}", format!("{:#?}", time)); + + self.hud.new_message(Chat { + chat_type: ChatType::Meta, + message: format!("Connection lost. Kicking in {} seconds", time), + }); + } } } From ef6f8b509aafe225c78e792190aafda798602368 Mon Sep 17 00:00:00 2001 From: Shane Handley Date: Thu, 17 Oct 2019 13:22:26 +0900 Subject: [PATCH 02/18] Eeek, remove logging. --- voxygen/src/session.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/voxygen/src/session.rs b/voxygen/src/session.rs index 1b1d560b9f..c62d0b0773 100644 --- a/voxygen/src/session.rs +++ b/voxygen/src/session.rs @@ -64,8 +64,6 @@ impl SessionState { } client::Event::Disconnect => {} // TODO client::Event::DisconnectionNotification(time) => { - log::warn!("{}", format!("{:#?}", time)); - self.hud.new_message(Chat { chat_type: ChatType::Meta, message: format!("Connection lost. Kicking in {} seconds", time), From 833c1d06b2b16cef0dbadf94301a4a959328c05b Mon Sep 17 00:00:00 2001 From: Shane Handley Date: Thu, 17 Oct 2019 13:51:26 +0900 Subject: [PATCH 03/18] Fix chat-cli to account for new case. --- chat-cli/src/main.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/chat-cli/src/main.rs b/chat-cli/src/main.rs index 975dec4e2b..ad7e7e65ce 100644 --- a/chat-cli/src/main.rs +++ b/chat-cli/src/main.rs @@ -77,6 +77,9 @@ fn main() { match event { Event::Chat { message, .. } => println!("{}", message), Event::Disconnect => {} // TODO + Event::DisconnectionNotification(time) => { + println!("{}", format!("Connection lost. Kicking in {} seconds", time)) + } } } // Clean up the server after a tick. From 3b24af76ab1814e168d861443e5e45ad22d79085 Mon Sep 17 00:00:00 2001 From: Shane Handley Date: Thu, 17 Oct 2019 14:14:20 +0900 Subject: [PATCH 04/18] Formatting --- chat-cli/src/main.rs | 7 ++++--- voxygen/src/session.rs | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/chat-cli/src/main.rs b/chat-cli/src/main.rs index ad7e7e65ce..f8caff4de7 100644 --- a/chat-cli/src/main.rs +++ b/chat-cli/src/main.rs @@ -77,9 +77,10 @@ fn main() { match event { Event::Chat { message, .. } => println!("{}", message), Event::Disconnect => {} // TODO - Event::DisconnectionNotification(time) => { - println!("{}", format!("Connection lost. Kicking in {} seconds", time)) - } + Event::DisconnectionNotification(time) => println!( + "{}", + format!("Connection lost. Kicking in {} seconds", time) + ), } } // Clean up the server after a tick. diff --git a/voxygen/src/session.rs b/voxygen/src/session.rs index c62d0b0773..388621c57a 100644 --- a/voxygen/src/session.rs +++ b/voxygen/src/session.rs @@ -8,13 +8,13 @@ use crate::{ }; use client::{self, Client, Event::Chat}; use common::{ - ChatType, clock::Clock, comp, comp::{Pos, Vel}, msg::ClientState, terrain::{Block, BlockKind}, vol::ReadVol, + ChatType, }; use log::error; use specs::Join; From 21f126acd4192cae244a0070df78b59829cf1b09 Mon Sep 17 00:00:00 2001 From: timokoesters Date: Fri, 18 Oct 2019 13:24:18 +0200 Subject: [PATCH 05/18] feat: show errors in main menu when client fails Instead of `[ERROR] Failed to tick the scene: Network(Bincode(Io(Custom { kind: UnexpectedEof, error: "failed to fill whole buffer" })))` --- voxygen/src/main.rs | 2 ++ voxygen/src/menu/char_selection/mod.rs | 4 ++++ voxygen/src/menu/main/mod.rs | 12 ++++++++---- voxygen/src/menu/main/ui.rs | 10 +++++----- voxygen/src/session.rs | 4 ++++ 5 files changed, 23 insertions(+), 9 deletions(-) diff --git a/voxygen/src/main.rs b/voxygen/src/main.rs index 8018da180d..381a3bd643 100644 --- a/voxygen/src/main.rs +++ b/voxygen/src/main.rs @@ -43,6 +43,7 @@ pub struct GlobalState { settings: Settings, window: Window, audio: AudioFrontend, + error_message: Option, } impl GlobalState { @@ -116,6 +117,7 @@ fn main() { audio, window: Window::new(&settings).expect("Failed to create window!"), settings, + error_message: None, }; let settings = &global_state.settings; diff --git a/voxygen/src/menu/char_selection/mod.rs b/voxygen/src/menu/char_selection/mod.rs index 941d94cd5f..b5d7397abb 100644 --- a/voxygen/src/menu/char_selection/mod.rs +++ b/voxygen/src/menu/char_selection/mod.rs @@ -114,6 +114,10 @@ impl PlayState for CharSelectionState { .tick(comp::ControllerInputs::default(), clock.get_last_delta()) { error!("Failed to tick the scene: {:?}", err); + global_state.error_message = Some( + "Connection lost!\nDid the server restart?\nIs the client up to date?" + .to_owned(), + ); return PlayStateResult::Pop; } self.client.borrow_mut().cleanup(); diff --git a/voxygen/src/menu/main/mod.rs b/voxygen/src/menu/main/mod.rs index 11ec61317f..3f0dcc7b97 100644 --- a/voxygen/src/menu/main/mod.rs +++ b/voxygen/src/menu/main/mod.rs @@ -75,11 +75,11 @@ impl PlayState for MainMenuState { } Some(Err(err)) => { client_init = None; - self.main_menu_ui.login_error( + global_state.error_message = Some( match err { InitError::BadAddress(_) | InitError::NoAddress => "Server not found", InitError::InvalidAuth => "Invalid credentials", - InitError::ServerIsFull => "Server is Full!", + InitError::ServerIsFull => "Server is full", InitError::ConnectionFailed(_) => "Connection failed", InitError::ClientCrashed => "Client crashed", } @@ -129,8 +129,8 @@ impl PlayState for MainMenuState { false, ))); } else { - self.main_menu_ui - .login_error("Invalid username or password".to_string()); + global_state.error_message = + Some("Invalid username or password".to_string()); } } MainMenuEvent::CancelLoginAttempt => { @@ -152,6 +152,10 @@ impl PlayState for MainMenuState { } } + if let Some(error) = global_state.error_message.take() { + self.main_menu_ui.show_error(error); + } + // Draw the UI to the screen. self.main_menu_ui.render(global_state.window.renderer_mut()); diff --git a/voxygen/src/menu/main/ui.rs b/voxygen/src/menu/main/ui.rs index eed12ae3a6..26b003d633 100644 --- a/voxygen/src/menu/main/ui.rs +++ b/voxygen/src/menu/main/ui.rs @@ -425,20 +425,20 @@ impl MainMenuUi { .rgba(1.0, 1.0, 1.0, 1.0) .font_size(25) .font_id(self.fonts.cyri); - Rectangle::fill_with([65.0 * 6.0, 100.0], color::TRANSPARENT) + Rectangle::fill_with([65.0 * 6.0, 140.0], color::TRANSPARENT) .rgba(0.1, 0.1, 0.1, 1.0) .parent(ui_widgets.window) - .up_from(self.ids.banner_top, 20.0) + .up_from(self.ids.banner_top, 15.0) .set(self.ids.login_error_bg, ui_widgets); Image::new(self.imgs.info_frame) - .w_h(65.0 * 6.0, 100.0) + .w_h(65.0 * 6.0, 140.0) .middle_of(self.ids.login_error_bg) .set(self.ids.error_frame, ui_widgets); text.mid_top_with_margin_on(self.ids.error_frame, 10.0) .set(self.ids.login_error, ui_widgets); if Button::image(self.imgs.button) .w_h(100.0, 30.0) - .mid_bottom_with_margin_on(self.ids.login_error_bg, 5.0) + .mid_bottom_with_margin_on(self.ids.login_error_bg, 10.0) .hover_image(self.imgs.button_hover) .press_image(self.imgs.button_press) .label_y(Relative::Scalar(2.0)) @@ -650,7 +650,7 @@ impl MainMenuUi { events } - pub fn login_error(&mut self, msg: String) { + pub fn show_error(&mut self, msg: String) { self.popup = Some(PopupData { msg, button_text: "Okay".to_string(), diff --git a/voxygen/src/session.rs b/voxygen/src/session.rs index 388621c57a..f4a99eff14 100644 --- a/voxygen/src/session.rs +++ b/voxygen/src/session.rs @@ -333,6 +333,10 @@ impl PlayState for SessionState { // Perform an in-game tick. if let Err(err) = self.tick(clock.get_avg_delta()) { error!("Failed to tick the scene: {:?}", err); + global_state.error_message = Some( + "Connection lost!\nDid the server restart?\nIs the client up to date?" + .to_owned(), + ); return PlayStateResult::Pop; } From c733c9571836d60053cac9502b9674f49fb75b7c Mon Sep 17 00:00:00 2001 From: timokoesters Date: Fri, 18 Oct 2019 22:05:37 +0200 Subject: [PATCH 06/18] improvement: UI for connecting to singleplayer servers + threading fixes --- server-cli/src/main.rs | 2 + voxygen/src/main.rs | 4 +- voxygen/src/menu/char_selection/mod.rs | 2 +- voxygen/src/menu/main/client_init.rs | 95 ++++++++++------- voxygen/src/menu/main/mod.rs | 111 +++++++++++++------- voxygen/src/menu/main/start_singleplayer.rs | 79 -------------- voxygen/src/menu/main/ui.rs | 11 +- voxygen/src/session.rs | 2 +- voxygen/src/singleplayer.rs | 14 ++- 9 files changed, 152 insertions(+), 168 deletions(-) delete mode 100644 voxygen/src/menu/main/start_singleplayer.rs diff --git a/server-cli/src/main.rs b/server-cli/src/main.rs index f19022a709..b7a26a4b2c 100644 --- a/server-cli/src/main.rs +++ b/server-cli/src/main.rs @@ -22,6 +22,8 @@ fn main() { // Create server let mut server = Server::new(settings).expect("Failed to create server instance!"); + println!("Server is ready to accept connections"); + loop { let events = server .tick(Input::default(), clock.get_last_delta()) diff --git a/voxygen/src/main.rs b/voxygen/src/main.rs index 381a3bd643..2ed9a04b8b 100644 --- a/voxygen/src/main.rs +++ b/voxygen/src/main.rs @@ -43,7 +43,7 @@ pub struct GlobalState { settings: Settings, window: Window, audio: AudioFrontend, - error_message: Option, + info_message: Option, } impl GlobalState { @@ -117,7 +117,7 @@ fn main() { audio, window: Window::new(&settings).expect("Failed to create window!"), settings, - error_message: None, + info_message: None, }; let settings = &global_state.settings; diff --git a/voxygen/src/menu/char_selection/mod.rs b/voxygen/src/menu/char_selection/mod.rs index b5d7397abb..3995e241f5 100644 --- a/voxygen/src/menu/char_selection/mod.rs +++ b/voxygen/src/menu/char_selection/mod.rs @@ -114,7 +114,7 @@ impl PlayState for CharSelectionState { .tick(comp::ControllerInputs::default(), clock.get_last_delta()) { error!("Failed to tick the scene: {:?}", err); - global_state.error_message = Some( + global_state.info_message = Some( "Connection lost!\nDid the server restart?\nIs the client up to date?" .to_owned(), ); diff --git a/voxygen/src/menu/main/client_init.rs b/voxygen/src/menu/main/client_init.rs index 02937fd78d..e288bd88e7 100644 --- a/voxygen/src/menu/main/client_init.rs +++ b/voxygen/src/menu/main/client_init.rs @@ -2,6 +2,8 @@ use client::{error::Error as ClientError, Client}; use common::comp; use crossbeam::channel::{unbounded, Receiver, TryRecvError}; use log::info; +use std::sync::atomic::{Ordering, AtomicBool}; +use std::sync::Arc; use std::{net::ToSocketAddrs, thread, time::Duration}; #[cfg(feature = "discord")] @@ -24,6 +26,7 @@ pub enum Error { // and create the client (which involves establishing a connection to the server). pub struct ClientInit { rx: Receiver>, + cancel: Arc, } impl ClientInit { pub fn new( @@ -35,6 +38,8 @@ impl ClientInit { let (server_address, default_port, prefer_ipv6) = connection_args; let (tx, rx) = unbounded(); + let cancel = Arc::new(AtomicBool::new(false)); + let cancel2 = Arc::clone(&cancel); thread::spawn(move || { // Sleep the thread to wait for the single-player server to start up. @@ -55,51 +60,58 @@ impl ClientInit { let mut last_err = None; - for socket_addr in first_addrs.into_iter().chain(second_addrs) { - match Client::new(socket_addr, player.view_distance) { - Ok(mut client) => { - if let Err(ClientError::InvalidAuth) = - client.register(player, password) - { - last_err = Some(Error::InvalidAuth); - break; - } - //client.register(player, password); - let _ = tx.send(Ok(client)); - - #[cfg(feature = "discord")] - { - if !server_address.eq("127.0.0.1") { - discord::send_all(vec![ - DiscordUpdate::Details(server_address), - DiscordUpdate::State("Playing...".into()), - ]); - } - } - - return; - } - Err(err) => { - match err { - // Assume the connection failed and try next address. - ClientError::Network(_) => { - last_err = Some(Error::ConnectionFailed(err)) - } - ClientError::TooManyPlayers => { - last_err = Some(Error::ServerIsFull); + 'tries: for _ in 0..=12 { // 1 Minute + if cancel2.load(Ordering::Relaxed) { + break; + } + for socket_addr in + first_addrs.clone().into_iter().chain(second_addrs.clone()) + { + match Client::new(socket_addr, player.view_distance) { + Ok(mut client) => { + if let Err(ClientError::InvalidAuth) = + client.register(player.clone(), password.clone()) + { + last_err = Some(Error::InvalidAuth); break; } - ClientError::InvalidAuth => { - last_err = Some(Error::InvalidAuth); + //client.register(player, password); + let _ = tx.send(Ok(client)); + + #[cfg(feature = "discord")] + { + if !server_address.eq("127.0.0.1") { + discord::send_all(vec![ + DiscordUpdate::Details(server_address), + DiscordUpdate::State("Playing...".into()), + ]); + } } - // TODO: Handle errors? - _ => panic!( + + return; + } + Err(err) => { + match err { + // Assume the connection failed and try again soon + ClientError::Network(_) => {} + ClientError::TooManyPlayers => { + last_err = Some(Error::ServerIsFull); + break 'tries; + } + ClientError::InvalidAuth => { + last_err = Some(Error::InvalidAuth); + break 'tries; + } + // TODO: Handle errors? + _ => panic!( "Unexpected non-network error when creating client: {:?}", err ), + } } } } + thread::sleep(Duration::from_secs(5)); } // Parsing/host name resolution successful but no connection succeeded. let _ = tx.send(Err(last_err.unwrap_or(Error::NoAddress))); @@ -111,7 +123,7 @@ impl ClientInit { } }); - ClientInit { rx } + ClientInit { rx, cancel, } } /// Poll if the thread is complete. /// Returns None if the thread is still running, otherwise returns the Result of client creation. @@ -122,4 +134,13 @@ impl ClientInit { Err(TryRecvError::Disconnected) => Some(Err(Error::ClientCrashed)), } } + pub fn cancel(&mut self) { + self.cancel.store(true, Ordering::Relaxed); + } +} + +impl Drop for ClientInit { + fn drop(&mut self) { + self.cancel(); + } } diff --git a/voxygen/src/menu/main/mod.rs b/voxygen/src/menu/main/mod.rs index 3f0dcc7b97..d6b0c94185 100644 --- a/voxygen/src/menu/main/mod.rs +++ b/voxygen/src/menu/main/mod.rs @@ -1,22 +1,23 @@ mod client_init; #[cfg(feature = "singleplayer")] -mod start_singleplayer; mod ui; use super::char_selection::CharSelectionState; -use crate::{window::Event, Direction, GlobalState, PlayState, PlayStateResult}; +use crate::{ + singleplayer::Singleplayer, window::Event, Direction, GlobalState, PlayState, PlayStateResult, +}; use argon2::{self, Config}; use client_init::{ClientInit, Error as InitError}; use common::{clock::Clock, comp}; use log::warn; #[cfg(feature = "singleplayer")] -use start_singleplayer::StartSingleplayerState; use std::time::Duration; use ui::{Event as MainMenuEvent, MainMenuUi}; pub struct MainMenuState { main_menu_ui: MainMenuUi, title_music_channel: Option, + singleplayer: Option, } impl MainMenuState { @@ -25,6 +26,7 @@ impl MainMenuState { Self { main_menu_ui: MainMenuUi::new(global_state), title_music_channel: None, + singleplayer: None, } } } @@ -48,6 +50,9 @@ impl PlayState for MainMenuState { ) } + // Reset singleplayer server if it was running already + self.singleplayer = None; + loop { // Handle window events. for event in global_state.window.fetch_events() { @@ -75,7 +80,7 @@ impl PlayState for MainMenuState { } Some(Err(err)) => { client_init = None; - global_state.error_message = Some( + global_state.info_message = Some( match err { InitError::BadAddress(_) | InitError::NoAddress => "Server not found", InitError::InvalidAuth => "Invalid credentials", @@ -100,49 +105,37 @@ impl PlayState for MainMenuState { password, server_address, } => { - let mut net_settings = &mut global_state.settings.networking; - net_settings.username = username.clone(); - net_settings.password = password.clone(); - if !net_settings.servers.contains(&server_address) { - net_settings.servers.push(server_address.clone()); - } - if let Err(err) = global_state.settings.save_to_file() { - warn!("Failed to save settings: {:?}", err); - } - - let player = comp::Player::new( - username.clone(), - Some(global_state.settings.graphics.view_distance), + attempt_login( + global_state, + username, + password, + server_address, + DEFAULT_PORT, + &mut client_init, ); - - if player.is_valid() { - // Don't try to connect if there is already a connection in progress. - client_init = client_init.or(Some(ClientInit::new( - (server_address, DEFAULT_PORT, false), - player, - { - let salt = b"staticsalt_zTuGkGvybZIjZbNUDtw15"; - let config = Config::default(); - argon2::hash_encoded(password.as_bytes(), salt, &config) - .unwrap() - }, - false, - ))); - } else { - global_state.error_message = - Some("Invalid username or password".to_string()); - } } MainMenuEvent::CancelLoginAttempt => { // client_init contains Some(ClientInit), which spawns a thread which contains a TcpStream::connect() call // This call is blocking // TODO fix when the network rework happens + self.singleplayer = None; client_init = None; self.main_menu_ui.cancel_connection(); } #[cfg(feature = "singleplayer")] MainMenuEvent::StartSingleplayer => { - return PlayStateResult::Push(Box::new(StartSingleplayerState::new())); + let (singleplayer, server_settings) = Singleplayer::new(None); // TODO: Make client and server use the same thread pool + + self.singleplayer = Some(singleplayer); + + attempt_login( + global_state, + "singleplayer".to_owned(), + "".to_owned(), + server_settings.gameserver_address.ip().to_string(), + server_settings.gameserver_address.port(), + &mut client_init, + ); } MainMenuEvent::Settings => {} // TODO MainMenuEvent::Quit => return PlayStateResult::Shutdown, @@ -152,8 +145,8 @@ impl PlayState for MainMenuState { } } - if let Some(error) = global_state.error_message.take() { - self.main_menu_ui.show_error(error); + if let Some(info) = global_state.info_message.take() { + self.main_menu_ui.show_info(info); } // Draw the UI to the screen. @@ -177,3 +170,45 @@ impl PlayState for MainMenuState { "Title" } } + +fn attempt_login( + global_state: &mut GlobalState, + username: String, + password: String, + server_address: String, + server_port: u16, + client_init: &mut Option, +) { + let mut net_settings = &mut global_state.settings.networking; + net_settings.username = username.clone(); + net_settings.password = password.clone(); + if !net_settings.servers.contains(&server_address) { + net_settings.servers.push(server_address.clone()); + } + if let Err(err) = global_state.settings.save_to_file() { + warn!("Failed to save settings: {:?}", err); + } + + let player = comp::Player::new( + username.clone(), + Some(global_state.settings.graphics.view_distance), + ); + + if player.is_valid() { + // Don't try to connect if there is already a connection in progress. + if client_init.is_none() { + *client_init = Some(ClientInit::new( + (server_address, server_port, false), + player, + { + let salt = b"staticsalt_zTuGkGvybZIjZbNUDtw15"; + let config = Config::default(); + argon2::hash_encoded(password.as_bytes(), salt, &config).unwrap() + }, + false, + )); + } + } else { + global_state.info_message = Some("Invalid username or password".to_string()); + } +} diff --git a/voxygen/src/menu/main/start_singleplayer.rs b/voxygen/src/menu/main/start_singleplayer.rs deleted file mode 100644 index b10b48ab3d..0000000000 --- a/voxygen/src/menu/main/start_singleplayer.rs +++ /dev/null @@ -1,79 +0,0 @@ -use super::client_init::ClientInit; -use crate::{ - menu::char_selection::CharSelectionState, singleplayer::Singleplayer, Direction, GlobalState, - PlayState, PlayStateResult, -}; -use common::comp; -use log::{info, warn}; -use server::settings::ServerSettings; - -pub struct StartSingleplayerState { - // Necessary to keep singleplayer working - _singleplayer: Singleplayer, - server_settings: ServerSettings, -} - -impl StartSingleplayerState { - /// Create a new `MainMenuState`. - pub fn new() -> Self { - let (_singleplayer, server_settings) = Singleplayer::new(None); // TODO: Make client and server use the same thread pool - - Self { - _singleplayer, - server_settings, - } - } -} - -impl PlayState for StartSingleplayerState { - fn play(&mut self, direction: Direction, global_state: &mut GlobalState) -> PlayStateResult { - match direction { - Direction::Forwards => { - let username = "singleplayer".to_owned(); - - let client_init = ClientInit::new( - //TODO: check why we are converting out IP:Port to String instead of parsing it directly as SockAddr - ( - self.server_settings.gameserver_address.ip().to_string(), - self.server_settings.gameserver_address.port(), - true, - ), - comp::Player::new( - username.clone(), - Some(global_state.settings.graphics.view_distance), - ), - String::default(), - true, - ); - - // Create the client. - let client = loop { - match client_init.poll() { - Some(Ok(client)) => break client, - Some(Err(err)) => { - warn!("Failed to start single-player server: {:?}", err); - return PlayStateResult::Pop; - } - _ => {} - } - }; - - // Print the metrics port - info!( - "Metrics port: {}", - self.server_settings.metrics_address.port() - ); - - PlayStateResult::Push(Box::new(CharSelectionState::new( - global_state, - std::rc::Rc::new(std::cell::RefCell::new(client)), - ))) - } - Direction::Backwards => PlayStateResult::Pop, - } - } - - fn name(&self) -> &'static str { - "Starting Single-Player" - } -} diff --git a/voxygen/src/menu/main/ui.rs b/voxygen/src/menu/main/ui.rs index 26b003d633..1e3a0dbd46 100644 --- a/voxygen/src/menu/main/ui.rs +++ b/voxygen/src/menu/main/ui.rs @@ -353,10 +353,11 @@ impl MainMenuUi { macro_rules! singleplayer { () => { events.push(Event::StartSingleplayer); - events.push(Event::LoginAttempt { - username: "singleplayer".to_string(), - password: String::default(), - server_address: "localhost".to_string(), + self.connecting = Some(std::time::Instant::now()); + self.popup = Some(PopupData { + msg: "Connecting...".to_string(), + button_text: "Cancel".to_string(), + popup_type: PopupType::ConnectionInfo, }); }; } @@ -650,7 +651,7 @@ impl MainMenuUi { events } - pub fn show_error(&mut self, msg: String) { + pub fn show_info(&mut self, msg: String) { self.popup = Some(PopupData { msg, button_text: "Okay".to_string(), diff --git a/voxygen/src/session.rs b/voxygen/src/session.rs index f4a99eff14..48a4ee5b62 100644 --- a/voxygen/src/session.rs +++ b/voxygen/src/session.rs @@ -333,7 +333,7 @@ impl PlayState for SessionState { // Perform an in-game tick. if let Err(err) = self.tick(clock.get_avg_delta()) { error!("Failed to tick the scene: {:?}", err); - global_state.error_message = Some( + global_state.info_message = Some( "Connection lost!\nDid the server restart?\nIs the client up to date?" .to_owned(), ); diff --git a/voxygen/src/singleplayer.rs b/voxygen/src/singleplayer.rs index 620ab34b54..b9978bee64 100644 --- a/voxygen/src/singleplayer.rs +++ b/voxygen/src/singleplayer.rs @@ -30,14 +30,18 @@ impl Singleplayer { // Create server let settings = ServerSettings::singleplayer(); - let server = Server::new(settings.clone()).expect("Failed to create server instance!"); - let server = match client { - Some(client) => server.with_thread_pool(client.thread_pool().clone()), - None => server, - }; + let thread_pool = client.map(|c| c.thread_pool().clone()); + let settings2 = settings.clone(); let thread = thread::spawn(move || { + let server = Server::new(settings2).expect("Failed to create server instance!"); + + let server = match thread_pool { + Some(pool) => server.with_thread_pool(pool), + None => server, + }; + run_server(server, receiver); }); From 26acd8b4275511dd3bb3e8fe983d142dfdd688f7 Mon Sep 17 00:00:00 2001 From: Shane Handley Date: Sat, 19 Oct 2019 16:14:05 +0900 Subject: [PATCH 07/18] Bump up the timeout tie for booting the single player server --- voxygen/src/menu/main/client_init.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/voxygen/src/menu/main/client_init.rs b/voxygen/src/menu/main/client_init.rs index e288bd88e7..91d7baf8ed 100644 --- a/voxygen/src/menu/main/client_init.rs +++ b/voxygen/src/menu/main/client_init.rs @@ -2,7 +2,7 @@ use client::{error::Error as ClientError, Client}; use common::comp; use crossbeam::channel::{unbounded, Receiver, TryRecvError}; use log::info; -use std::sync::atomic::{Ordering, AtomicBool}; +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; use std::{net::ToSocketAddrs, thread, time::Duration}; @@ -60,7 +60,8 @@ impl ClientInit { let mut last_err = None; - 'tries: for _ in 0..=12 { // 1 Minute + 'tries: for _ in 0..=60 { + // 300 Seconds if cancel2.load(Ordering::Relaxed) { break; } @@ -123,7 +124,7 @@ impl ClientInit { } }); - ClientInit { rx, cancel, } + ClientInit { rx, cancel } } /// Poll if the thread is complete. /// Returns None if the thread is still running, otherwise returns the Result of client creation. From 4a65cddd15df2c988a45b6208c10c03d6953a038 Mon Sep 17 00:00:00 2001 From: timokoesters Date: Sat, 19 Oct 2019 09:18:35 +0200 Subject: [PATCH 08/18] improvement: remove `wait` because it now always retries --- voxygen/src/menu/main/client_init.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/voxygen/src/menu/main/client_init.rs b/voxygen/src/menu/main/client_init.rs index 91d7baf8ed..7714543529 100644 --- a/voxygen/src/menu/main/client_init.rs +++ b/voxygen/src/menu/main/client_init.rs @@ -33,7 +33,6 @@ impl ClientInit { connection_args: (String, u16, bool), player: comp::Player, password: String, - wait: bool, ) -> Self { let (server_address, default_port, prefer_ipv6) = connection_args; @@ -42,11 +41,6 @@ impl ClientInit { let cancel2 = Arc::clone(&cancel); thread::spawn(move || { - // Sleep the thread to wait for the single-player server to start up. - if wait { - info!("Waiting for server to come up..."); - thread::sleep(Duration::from_millis(500)); - } // Parse ip address or resolves hostname. // Note: if you use an ipv6 address, the number after the last colon will be used // as the port unless you use [] around the address. From 81f41e278eb79b1a62570bb1bdf17910f4173f54 Mon Sep 17 00:00:00 2001 From: Shane Handley Date: Sun, 20 Oct 2019 13:36:38 +0900 Subject: [PATCH 09/18] Remove unused warn and fix dangling parameter. --- voxygen/src/menu/main/client_init.rs | 1 - voxygen/src/menu/main/mod.rs | 1 - 2 files changed, 2 deletions(-) diff --git a/voxygen/src/menu/main/client_init.rs b/voxygen/src/menu/main/client_init.rs index 7714543529..e59dfeeff7 100644 --- a/voxygen/src/menu/main/client_init.rs +++ b/voxygen/src/menu/main/client_init.rs @@ -1,7 +1,6 @@ use client::{error::Error as ClientError, Client}; use common::comp; use crossbeam::channel::{unbounded, Receiver, TryRecvError}; -use log::info; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; use std::{net::ToSocketAddrs, thread, time::Duration}; diff --git a/voxygen/src/menu/main/mod.rs b/voxygen/src/menu/main/mod.rs index d6b0c94185..a824be15bb 100644 --- a/voxygen/src/menu/main/mod.rs +++ b/voxygen/src/menu/main/mod.rs @@ -205,7 +205,6 @@ fn attempt_login( let config = Config::default(); argon2::hash_encoded(password.as_bytes(), salt, &config).unwrap() }, - false, )); } } else { From 69b008e0e653dbb4734642c5498d461bc2a1cb4a Mon Sep 17 00:00:00 2001 From: Shane Handley Date: Sun, 20 Oct 2019 14:42:54 +0900 Subject: [PATCH 10/18] Fix issue with the timeout error showing when there is an intentional logout from the game. --- voxygen/src/menu/char_selection/mod.rs | 5 +---- voxygen/src/session.rs | 21 +++++++++++++++------ 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/voxygen/src/menu/char_selection/mod.rs b/voxygen/src/menu/char_selection/mod.rs index 3995e241f5..15ccecd63b 100644 --- a/voxygen/src/menu/char_selection/mod.rs +++ b/voxygen/src/menu/char_selection/mod.rs @@ -114,12 +114,9 @@ impl PlayState for CharSelectionState { .tick(comp::ControllerInputs::default(), clock.get_last_delta()) { error!("Failed to tick the scene: {:?}", err); - global_state.info_message = Some( - "Connection lost!\nDid the server restart?\nIs the client up to date?" - .to_owned(), - ); return PlayStateResult::Pop; } + self.client.borrow_mut().cleanup(); // Finish the frame. diff --git a/voxygen/src/session.rs b/voxygen/src/session.rs index 48a4ee5b62..9df3824b23 100644 --- a/voxygen/src/session.rs +++ b/voxygen/src/session.rs @@ -6,7 +6,7 @@ use crate::{ window::{Event, GameInput}, Direction, Error, GlobalState, PlayState, PlayStateResult, }; -use client::{self, Client, Event::Chat}; +use client::{self, error::Error as ClientError, Client, Event::Chat}; use common::{ clock::Clock, comp, @@ -332,11 +332,20 @@ impl PlayState for SessionState { // Perform an in-game tick. if let Err(err) = self.tick(clock.get_avg_delta()) { - error!("Failed to tick the scene: {:?}", err); - global_state.info_message = Some( - "Connection lost!\nDid the server restart?\nIs the client up to date?" - .to_owned(), - ); + match err { + Error::ClientError(ClientError::ServerTimeout) => { + global_state.info_message = Some( + "Connection lost!\nDid the server restart?\nIs the client up to date?" + .to_owned(), + ); + + error!("[session] ServerTimeout: {:?}", err); + } + _ => { + error!("[session] Failed to tick the scene: {:?}", err); + } + } + return PlayStateResult::Pop; } From b15c107f0be058fb50e07395dcfb365216083b80 Mon Sep 17 00:00:00 2001 From: Shane Handley Date: Sun, 20 Oct 2019 16:24:36 +0900 Subject: [PATCH 11/18] Re-add the handling of error when the player loses connection on the character select screen. --- voxygen/src/menu/char_selection/mod.rs | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/voxygen/src/menu/char_selection/mod.rs b/voxygen/src/menu/char_selection/mod.rs index 15ccecd63b..3e138833e0 100644 --- a/voxygen/src/menu/char_selection/mod.rs +++ b/voxygen/src/menu/char_selection/mod.rs @@ -5,7 +5,7 @@ use crate::{ session::SessionState, window::Event as WinEvent, Direction, GlobalState, PlayState, PlayStateResult, }; -use client::{self, Client}; +use client::{self, Client, Error as ClientError}; use common::{assets, clock::Clock, comp, msg::ClientState}; use log::error; use scene::Scene; @@ -113,7 +113,20 @@ impl PlayState for CharSelectionState { .borrow_mut() .tick(comp::ControllerInputs::default(), clock.get_last_delta()) { - error!("Failed to tick the scene: {:?}", err); + match err { + ClientError::ServerTimeout => { + global_state.info_message = Some( + "Connection lost!\nDid the server restart?\nIs the client up to date?" + .to_owned(), + ); + + error!("[session] ServerTimeout: {:?}", err); + } + _ => { + error!("[session] Failed to tick the scene: {:?}", err); + } + } + return PlayStateResult::Pop; } From 15c725bfdea2bf34886588da253289f8d7a3b202 Mon Sep 17 00:00:00 2001 From: Shane Handley Date: Tue, 22 Oct 2019 12:46:12 +0900 Subject: [PATCH 12/18] Address code review points: - Clarify caffeine fueled comment - Be better at comparing Instant's, and catch the 0 seconds case to say Goodbye to the user - Switch println for 'info!' --- chat-cli/src/main.rs | 13 +++++++++---- client/src/lib.rs | 14 +++++++++----- server-cli/src/main.rs | 2 +- voxygen/src/session.rs | 7 ++++++- 4 files changed, 25 insertions(+), 11 deletions(-) diff --git a/chat-cli/src/main.rs b/chat-cli/src/main.rs index f8caff4de7..2f288c6500 100644 --- a/chat-cli/src/main.rs +++ b/chat-cli/src/main.rs @@ -77,12 +77,17 @@ fn main() { match event { Event::Chat { message, .. } => println!("{}", message), Event::Disconnect => {} // TODO - Event::DisconnectionNotification(time) => println!( - "{}", - format!("Connection lost. Kicking in {} seconds", time) - ), + Event::DisconnectionNotification(time) => { + let message = match time { + 0 => String::from("Goodbye!"), + _ => format!("Connection lost. Kicking in {} seconds", time), + }; + + println!("{}", message) + } } } + // Clean up the server after a tick. client.cleanup(); diff --git a/client/src/lib.rs b/client/src/lib.rs index b3c49ebfb5..460c19e170 100644 --- a/client/src/lib.rs +++ b/client/src/lib.rs @@ -502,15 +502,19 @@ impl Client { let mut frontend_events = Vec::new(); // Check that we have an valid connection. - // Limit this to the ping time (1s), since that's the max resolution we have + // Use the last ping time as a 1s rate limiter, we only notify the user once per second if Instant::now().duration_since(self.last_server_ping) > Duration::from_secs(1) { let duration_since_last_pong = Instant::now().duration_since(self.last_server_pong); // Dispatch a notification to the HUD warning they will be kicked in {n} seconds - if duration_since_last_pong.as_secs_f64() > SERVER_TIMEOUT_GRACE_PERIOD.as_secs_f64() { - frontend_events.push(Event::DisconnectionNotification( - SERVER_TIMEOUT.as_secs() - duration_since_last_pong.as_secs(), - )); + if duration_since_last_pong.as_secs() >= SERVER_TIMEOUT_GRACE_PERIOD.as_secs() { + if let Some(seconds_until_kick) = + SERVER_TIMEOUT.checked_sub(duration_since_last_pong) + { + frontend_events.push(Event::DisconnectionNotification( + seconds_until_kick.as_secs(), + )); + } } } diff --git a/server-cli/src/main.rs b/server-cli/src/main.rs index b7a26a4b2c..30f6b2f202 100644 --- a/server-cli/src/main.rs +++ b/server-cli/src/main.rs @@ -22,7 +22,7 @@ fn main() { // Create server let mut server = Server::new(settings).expect("Failed to create server instance!"); - println!("Server is ready to accept connections"); + info!("Server is ready to accept connections"); loop { let events = server diff --git a/voxygen/src/session.rs b/voxygen/src/session.rs index 9df3824b23..2dc73aad92 100644 --- a/voxygen/src/session.rs +++ b/voxygen/src/session.rs @@ -64,9 +64,14 @@ impl SessionState { } client::Event::Disconnect => {} // TODO client::Event::DisconnectionNotification(time) => { + let message = match time { + 0 => String::from("Goodbye!"), + _ => format!("Connection lost. Kicking in {} seconds", time), + }; + self.hud.new_message(Chat { chat_type: ChatType::Meta, - message: format!("Connection lost. Kicking in {} seconds", time), + message, }); } } From fb3350c605d7e2436799232dfb03d8d2adf9848d Mon Sep 17 00:00:00 2001 From: Shane Handley Date: Thu, 24 Oct 2019 10:15:58 +0900 Subject: [PATCH 13/18] Removed error which is now handled by the ServerTimeout case --- voxygen/src/menu/main/client_init.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/voxygen/src/menu/main/client_init.rs b/voxygen/src/menu/main/client_init.rs index e59dfeeff7..6c5ff9db49 100644 --- a/voxygen/src/menu/main/client_init.rs +++ b/voxygen/src/menu/main/client_init.rs @@ -14,8 +14,6 @@ pub enum Error { BadAddress(std::io::Error), // Parsing yielded an empty iterator (specifically to_socket_addrs()). NoAddress, - // Parsing/host name resolution successful but could not connect. - ConnectionFailed(ClientError), InvalidAuth, ClientCrashed, ServerIsFull, From 4330b33ce02a91cd0db8cb5fda3ec36916c297b9 Mon Sep 17 00:00:00 2001 From: Shane Handley Date: Thu, 24 Oct 2019 11:48:28 +0900 Subject: [PATCH 14/18] Hmm, alright have it your way rust. Not my change, so leaving it alone. --- voxygen/src/menu/main/client_init.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/voxygen/src/menu/main/client_init.rs b/voxygen/src/menu/main/client_init.rs index 6c5ff9db49..a53bfd3976 100644 --- a/voxygen/src/menu/main/client_init.rs +++ b/voxygen/src/menu/main/client_init.rs @@ -12,6 +12,8 @@ use crate::{discord, discord::DiscordUpdate}; pub enum Error { // Error parsing input string or error resolving host name. BadAddress(std::io::Error), + // Parsing/host name resolution successful but could not connect. + ConnectionFailed(ClientError), // Parsing yielded an empty iterator (specifically to_socket_addrs()). NoAddress, InvalidAuth, From 0fabe70960336721e45975c2917775d7d8ee5c41 Mon Sep 17 00:00:00 2001 From: Shane Handley Date: Sat, 26 Oct 2019 08:50:52 +0900 Subject: [PATCH 15/18] Ignore warning about variant never being constructed. It's being used elsewhere. Not sure why that isn't being picked up. --- voxygen/src/menu/main/client_init.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/voxygen/src/menu/main/client_init.rs b/voxygen/src/menu/main/client_init.rs index a53bfd3976..ff53f27006 100644 --- a/voxygen/src/menu/main/client_init.rs +++ b/voxygen/src/menu/main/client_init.rs @@ -13,6 +13,7 @@ pub enum Error { // Error parsing input string or error resolving host name. BadAddress(std::io::Error), // Parsing/host name resolution successful but could not connect. + #[allow(dead_code)] ConnectionFailed(ClientError), // Parsing yielded an empty iterator (specifically to_socket_addrs()). NoAddress, From f7cfcc53949555e4797d4fbc05ee6c8b2a5356ca Mon Sep 17 00:00:00 2001 From: Shane Handley Date: Sat, 26 Oct 2019 09:05:57 +0900 Subject: [PATCH 16/18] Re-add the metrics port logging which was lost in the merge. --- server-cli/src/main.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/server-cli/src/main.rs b/server-cli/src/main.rs index 30f6b2f202..466d5d9b79 100644 --- a/server-cli/src/main.rs +++ b/server-cli/src/main.rs @@ -18,11 +18,13 @@ fn main() { // Load settings let settings = ServerSettings::load(); + let metrics_port = &settings.metrics_address.port(); // Create server let mut server = Server::new(settings).expect("Failed to create server instance!"); info!("Server is ready to accept connections"); + info!("Metrics port: {}", metrics_port); loop { let events = server From 4df7e9533135387ef0055dbe72f2510317de5afe Mon Sep 17 00:00:00 2001 From: Shane Handley Date: Sun, 27 Oct 2019 04:42:28 +0900 Subject: [PATCH 17/18] Add a TODO regarding the future plan for the connection timeout setting. --- client/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/client/src/lib.rs b/client/src/lib.rs index 460c19e170..170a23c42b 100644 --- a/client/src/lib.rs +++ b/client/src/lib.rs @@ -31,6 +31,7 @@ use uvth::{ThreadPool, ThreadPoolBuilder}; use vek::*; // The duration of network inactivity until the player is kicked to the main menu +// @TODO in the future, this should be configurable on the server, and be provided to the client const SERVER_TIMEOUT: Duration = Duration::from_secs(20); // After this duration has elapsed, the user will begin getting kick warnings in their chat window From b6552556e73b4d08df8997fb26eb020f7ace2652 Mon Sep 17 00:00:00 2001 From: timokoesters Date: Wed, 6 Nov 2019 22:36:39 +0100 Subject: [PATCH 18/18] fix: show error on client failure again --- voxygen/src/menu/char_selection/mod.rs | 20 ++++++-------------- voxygen/src/session.rs | 20 ++++++-------------- 2 files changed, 12 insertions(+), 28 deletions(-) diff --git a/voxygen/src/menu/char_selection/mod.rs b/voxygen/src/menu/char_selection/mod.rs index 3e138833e0..a80b8acbde 100644 --- a/voxygen/src/menu/char_selection/mod.rs +++ b/voxygen/src/menu/char_selection/mod.rs @@ -5,7 +5,7 @@ use crate::{ session::SessionState, window::Event as WinEvent, Direction, GlobalState, PlayState, PlayStateResult, }; -use client::{self, Client, Error as ClientError}; +use client::{self, Client}; use common::{assets, clock::Clock, comp, msg::ClientState}; use log::error; use scene::Scene; @@ -113,19 +113,11 @@ impl PlayState for CharSelectionState { .borrow_mut() .tick(comp::ControllerInputs::default(), clock.get_last_delta()) { - match err { - ClientError::ServerTimeout => { - global_state.info_message = Some( - "Connection lost!\nDid the server restart?\nIs the client up to date?" - .to_owned(), - ); - - error!("[session] ServerTimeout: {:?}", err); - } - _ => { - error!("[session] Failed to tick the scene: {:?}", err); - } - } + global_state.info_message = Some( + "Connection lost!\nDid the server restart?\nIs the client up to date?" + .to_owned(), + ); + error!("[session] Failed to tick the scene: {:?}", err); return PlayStateResult::Pop; } diff --git a/voxygen/src/session.rs b/voxygen/src/session.rs index 2dc73aad92..8b91ed78f8 100644 --- a/voxygen/src/session.rs +++ b/voxygen/src/session.rs @@ -6,7 +6,7 @@ use crate::{ window::{Event, GameInput}, Direction, Error, GlobalState, PlayState, PlayStateResult, }; -use client::{self, error::Error as ClientError, Client, Event::Chat}; +use client::{self, Client, Event::Chat}; use common::{ clock::Clock, comp, @@ -337,19 +337,11 @@ impl PlayState for SessionState { // Perform an in-game tick. if let Err(err) = self.tick(clock.get_avg_delta()) { - match err { - Error::ClientError(ClientError::ServerTimeout) => { - global_state.info_message = Some( - "Connection lost!\nDid the server restart?\nIs the client up to date?" - .to_owned(), - ); - - error!("[session] ServerTimeout: {:?}", err); - } - _ => { - error!("[session] Failed to tick the scene: {:?}", err); - } - } + global_state.info_message = Some( + "Connection lost!\nDid the server restart?\nIs the client up to date?" + .to_owned(), + ); + error!("[session] Failed to tick the scene: {:?}", err); return PlayStateResult::Pop; }