* Fixed character load errors not being handled in Voxygen.

* Improved server error message for character load errors.
* Added server logging for item asset load errors during character load.
* Fixed character select error message dialog not supporting long messages.
This commit is contained in:
Ben Wallis 2020-12-30 14:50:17 +00:00
parent 6b8b5ea465
commit 659ca3e737
9 changed files with 57 additions and 28 deletions

View File

@ -493,13 +493,18 @@ impl Client {
{ {
const C_TYPE: ClientType = ClientType::Game; const C_TYPE: ClientType = ClientType::Game;
let verified = msg.verify(C_TYPE, self.registered, self.presence); let verified = msg.verify(C_TYPE, self.registered, self.presence);
assert!(
verified, // Due to the fact that character loading is performed asynchronously after
format!( // initial connect it is possible to receive messages after a character load
"c_type: {:?}, registered: {}, presence: {:?}, msg: {:?}", // error while in the wrong state.
C_TYPE, self.registered, self.presence, msg if !verified {
) warn!(
"Received ClientType::Game message when not in game (Registered: {} Presence: \
{:?}), dropping message: {:?} ",
self.registered, self.presence, msg
); );
return Ok(());
}
} }
match msg { match msg {
ClientMsg::Type(msg) => self.register_stream.send(msg), ClientMsg::Type(msg) => self.register_stream.send(msg),

View File

@ -212,7 +212,7 @@ pub fn convert_stats_to_database(
pub fn convert_inventory_from_database_items(database_items: &[Item]) -> Result<Inventory, Error> { pub fn convert_inventory_from_database_items(database_items: &[Item]) -> Result<Inventory, Error> {
let mut inventory = Inventory::new_empty(); let mut inventory = Inventory::new_empty();
for db_item in database_items.iter() { for db_item in database_items.iter() {
let mut item = common::comp::Item::new_from_asset(db_item.item_definition_id.as_str())?; let mut item = get_item_from_asset(db_item.item_definition_id.as_str())?;
// NOTE: Since this is freshly loaded, the atomic is *unique.* // NOTE: Since this is freshly loaded, the atomic is *unique.*
let comp = item.get_item_id_for_database(); let comp = item.get_item_id_for_database();
@ -269,7 +269,7 @@ pub fn convert_loadout_from_database_items(
) -> Result<Loadout, Error> { ) -> Result<Loadout, Error> {
let mut loadout = loadout_builder::LoadoutBuilder::new(); let mut loadout = loadout_builder::LoadoutBuilder::new();
for db_item in database_items.iter() { for db_item in database_items.iter() {
let item = common::comp::Item::new_from_asset(db_item.item_definition_id.as_str())?; let item = get_item_from_asset(db_item.item_definition_id.as_str())?;
// NOTE: item id is currently *unique*, so we can store the ID safely. // NOTE: item id is currently *unique*, so we can store the ID safely.
let comp = item.get_item_id_for_database(); let comp = item.get_item_id_for_database();
comp.store(Some(NonZeroU64::try_from(db_item.item_id as u64).map_err( comp.store(Some(NonZeroU64::try_from(db_item.item_id as u64).map_err(
@ -365,3 +365,13 @@ pub fn convert_stats_from_database(stats: &Stats, alias: String) -> common::comp
new_stats new_stats
} }
fn get_item_from_asset(item_definition_id: &str) -> Result<common::comp::Item, Error> {
common::comp::Item::new_from_asset(item_definition_id).map_err(|err| {
Error::AssetError(format!(
"Error loading item asset: {} - {}",
item_definition_id,
err.to_string()
))
})
}

View File

@ -115,11 +115,19 @@ impl CharacterLoader {
CharacterLoaderRequestKind::LoadCharacterData { CharacterLoaderRequestKind::LoadCharacterData {
player_uuid, player_uuid,
character_id, character_id,
} => CharacterLoaderResponseKind::CharacterData(Box::new( } => {
conn.transaction(|txn| { let result = conn.transaction(|txn| {
load_character_data(player_uuid, character_id, txn, &map) load_character_data(player_uuid, character_id, txn, &map)
}), });
)), if result.is_err() {
error!(
?result,
"Error loading character data for character_id: {}",
character_id
);
}
CharacterLoaderResponseKind::CharacterData(Box::new(result))
},
}, },
}) })
{ {

View File

@ -2,7 +2,6 @@
extern crate diesel; extern crate diesel;
use common::assets::Error as AssetsError;
use std::fmt; use std::fmt;
#[derive(Debug)] #[derive(Debug)]
@ -40,10 +39,6 @@ impl fmt::Display for Error {
} }
} }
impl From<AssetsError> for Error {
fn from(error: AssetsError) -> Error { Error::AssetError(error.to_string()) }
}
impl From<diesel::result::Error> for Error { impl From<diesel::result::Error> for Error {
fn from(error: diesel::result::Error) -> Error { Error::DatabaseError(error) } fn from(error: diesel::result::Error) -> Error { Error::DatabaseError(error) }
} }

View File

@ -55,6 +55,10 @@ pub struct GlobalState {
// TODO: redo this so that the watcher doesn't have to exist for reloading to occur // TODO: redo this so that the watcher doesn't have to exist for reloading to occur
pub i18n: AssetHandle<Localization>, pub i18n: AssetHandle<Localization>,
pub clipboard: Option<iced_winit::Clipboard>, pub clipboard: Option<iced_winit::Clipboard>,
// NOTE: This can be removed from GlobalState if client state behavior is refactored to not
// enter the game before confirmation of successful character load
/// An error returned by Client that needs to be displayed by the UI
pub client_error: Option<String>,
} }
impl GlobalState { impl GlobalState {

View File

@ -188,6 +188,7 @@ fn main() {
singleplayer: None, singleplayer: None,
i18n, i18n,
clipboard, clipboard,
client_error: None,
}; };
run::run(global_state, event_loop); run::run(global_state, event_loop);

View File

@ -11,7 +11,7 @@ use crate::{
use client::{self, Client}; use client::{self, Client};
use common::{comp, resources::DeltaTime, span}; use common::{comp, resources::DeltaTime, span};
use specs::WorldExt; use specs::WorldExt;
use std::{cell::RefCell, rc::Rc}; use std::{cell::RefCell, mem, rc::Rc};
use tracing::error; use tracing::error;
use ui::CharSelectionUi; use ui::CharSelectionUi;
@ -188,7 +188,7 @@ impl PlayState for CharSelectionState {
self.char_selection_ui.select_character(character_id); self.char_selection_ui.select_character(character_id);
}, },
client::Event::CharacterError(error) => { client::Event::CharacterError(error) => {
self.char_selection_ui.display_error(error); global_state.client_error = Some(error);
}, },
_ => {}, _ => {},
} }
@ -202,6 +202,10 @@ impl PlayState for CharSelectionState {
}, },
} }
if let Some(error) = mem::take(&mut global_state.client_error) {
self.char_selection_ui.display_error(error);
}
// TODO: make sure rendering is not relying on cleaned up stuff // TODO: make sure rendering is not relying on cleaned up stuff
self.client.borrow_mut().cleanup(); self.client.borrow_mut().cleanup();
@ -213,7 +217,7 @@ impl PlayState for CharSelectionState {
} }
} }
fn name(&self) -> &'static str { "Title" } fn name(&self) -> &'static str { "Character Selection" }
fn render(&mut self, renderer: &mut Renderer, _: &Settings) { fn render(&mut self, renderer: &mut Renderer, _: &Settings) {
let client = self.client.borrow(); let client = self.client.borrow();

View File

@ -656,13 +656,13 @@ impl Controls {
}, },
InfoContent::CharacterError(error) => Column::with_children(vec![ InfoContent::CharacterError(error) => Column::with_children(vec![
Text::new(error).size(fonts.cyri.scale(24)).into(), Text::new(error).size(fonts.cyri.scale(24)).into(),
Container::new(neat_button( Row::with_children(vec![neat_button(
no_button, no_button,
i18n.get("common.close"), i18n.get("common.close"),
FILL_FRAC_ONE, FILL_FRAC_ONE,
button_style, button_style,
Some(Message::ClearCharacterListError), Some(Message::ClearCharacterListError),
)) )])
.height(Length::Units(28)) .height(Length::Units(28))
.into(), .into(),
]) ])
@ -679,11 +679,11 @@ impl Controls {
(28, 28, 22, 255).into(), (28, 28, 22, 255).into(),
), ),
) )
.width(Length::Fill) .width(Length::Shrink)
.height(Length::Fill) .height(Length::Shrink)
.max_width(400) .max_width(400)
.max_height(130) .max_height(500)
.padding(16) .padding(24)
.center_x() .center_x()
.center_y(); .center_y();

View File

@ -181,7 +181,9 @@ impl SessionState {
}, },
client::Event::Outcome(outcome) => outcomes.push(outcome), client::Event::Outcome(outcome) => outcomes.push(outcome),
client::Event::CharacterCreated(_) => {}, client::Event::CharacterCreated(_) => {},
client::Event::CharacterError(_) => {}, client::Event::CharacterError(error) => {
global_state.client_error = Some(error);
},
} }
} }