diff --git a/assets/voxygen/i18n/en/main.ron b/assets/voxygen/i18n/en/main.ron index 9b3069856f..ab22a8ac9d 100644 --- a/assets/voxygen/i18n/en/main.ron +++ b/assets/voxygen/i18n/en/main.ron @@ -41,6 +41,7 @@ You can create an account over at https://veloren.net/account/."#, "main.login.server_not_found": "Server not found", "main.login.authentication_error": "Auth error on server", + "main.login.internal_error": "Internal error on client (most likely, player character was deleted)", "main.login.failed_auth_server_url_invalid": "Failed to connect to auth server", "main.login.insecure_auth_scheme": "The auth Scheme HTTP is NOT supported. It's insecure! For development purposes, HTTP is allowed for 'localhost' or debug builds", "main.login.server_full": "Server is full", diff --git a/client/src/error.rs b/client/src/error.rs index 57de911e94..a9079d77d6 100644 --- a/client/src/error.rs +++ b/client/src/error.rs @@ -1,6 +1,7 @@ use authc::AuthClientError; pub use network::{InitProtocolError, NetworkConnectError, NetworkError}; use network::{ParticipantError, StreamError}; +use specs::error::Error as SpecsError; #[derive(Debug)] pub enum Error { @@ -22,6 +23,11 @@ pub enum Error { InvalidCharacter, //TODO: InvalidAlias, Other(String), + SpecsErr(SpecsError), +} + +impl From for Error { + fn from(err: SpecsError) -> Self { Self::SpecsErr(err) } } impl From for Error { diff --git a/client/src/lib.rs b/client/src/lib.rs index 2c9f44a113..5a4bf27060 100644 --- a/client/src/lib.rs +++ b/client/src/lib.rs @@ -852,8 +852,9 @@ impl Client { pub fn request_remove_character(&mut self) { self.send_msg(ClientGeneral::ExitInGame); } pub fn set_view_distance(&mut self, view_distance: u32) { - self.view_distance = Some(view_distance.max(1).min(65)); - self.send_msg(ClientGeneral::SetViewDistance(self.view_distance.unwrap())); + let view_distance = view_distance.max(1).min(65); + self.view_distance = Some(view_distance); + self.send_msg(ClientGeneral::SetViewDistance(view_distance)); } pub fn use_slot(&mut self, slot: Slot) { @@ -1381,13 +1382,13 @@ impl Client { if let Some(client_character_state) = ecs.read_storage::().get(entity) { - if last_character_states - .get(entity) - .map(|l| !client_character_state.same_variant(&l.0)) - .unwrap_or(true) + if let Some(l) = last_character_states + .entry(entity) + .ok() + .map(|l| l.or_insert_with(|| comp::Last(client_character_state.clone()))) + .filter(|l| !client_character_state.same_variant(&l.0)) { - let _ = last_character_states - .insert(entity, comp::Last(client_character_state.clone())); + *l = comp::Last(client_character_state.clone()); } } } @@ -1815,7 +1816,22 @@ impl Client { InventoryUpdateEvent::CollectFailed => {}, _ => { // Push the updated inventory component to the client - self.state.write_component(self.entity(), inventory); + // FIXME: Figure out whether this error can happen under normal gameplay, + // if not find a better way to handle it, if so maybe consider kicking the + // client back to login? + let entity = self.entity(); + if let Err(e) = self + .state + .ecs_mut() + .write_storage() + .insert(entity, inventory) + { + warn!( + ?e, + "Received an inventory update event for client entity, but this \ + entity was not found... this may be a bug." + ); + } }, } @@ -2353,14 +2369,12 @@ impl Drop for Client { trace!("no disconnect msg necessary as client wasn't registered") } - tokio::task::block_in_place(|| { - if let Err(e) = self - .runtime - .block_on(self.participant.take().unwrap().disconnect()) - { - warn!(?e, "error when disconnecting, couldn't send all data"); - } - }); + self.runtime.spawn( + self.participant + .take() + .expect("Only set to None in Drop") + .disconnect(), + ); } } diff --git a/common/ecs/src/system.rs b/common/ecs/src/system.rs index dc7bb03fbc..e061582260 100644 --- a/common/ecs/src/system.rs +++ b/common/ecs/src/system.rs @@ -77,7 +77,12 @@ impl CpuTimeline { fn end(&mut self) -> std::time::Duration { let end = Instant::now(); self.measures.push((end, ParMode::None)); - end.duration_since(self.measures.first().unwrap().0) + end.duration_since( + self.measures + .first() + .expect("We just pushed onto the vector.") + .0, + ) } fn get(&self, time: Instant) -> ParMode { @@ -181,9 +186,17 @@ pub fn gen_stats( // update ALL states for individual in individual_cores_wanted.iter() { let actual = (individual.1 as f32 / total_or_max) * physical_threads as f32; - let p = result.get_mut(individual.0).unwrap(); - if (p.measures.last().unwrap().1 - actual).abs() > 0.0001 { - p.measures.push((relative_time, actual)); + if let Some(p) = result.get_mut(individual.0) { + if p.measures + .last() + .map(|last| (last.1 - actual).abs()) + .unwrap_or(0.0) + > 0.0001 + { + p.measures.push((relative_time, actual)); + } + } else { + tracing::warn!("Invariant violation: keys in both hashmaps should be the same."); } } } diff --git a/common/net/src/sync/mod.rs b/common/net/src/sync/mod.rs index 7997437db5..f924459b07 100644 --- a/common/net/src/sync/mod.rs +++ b/common/net/src/sync/mod.rs @@ -10,7 +10,7 @@ pub use common::uid::{Uid, UidAllocator}; pub use packet::{ handle_insert, handle_interp_insert, handle_interp_modify, handle_interp_remove, handle_modify, handle_remove, CompPacket, CompSyncPackage, EntityPackage, EntitySyncPackage, - InterpolatableComponent, StatePackage, + InterpolatableComponent, }; pub use sync_ext::WorldSyncExt; pub use track::UpdateTracker; diff --git a/common/net/src/sync/packet.rs b/common/net/src/sync/packet.rs index 1b5164b8f0..87bfe0a0c3 100644 --- a/common/net/src/sync/packet.rs +++ b/common/net/src/sync/packet.rs @@ -104,36 +104,6 @@ pub struct EntityPackage { pub comps: Vec

, } -#[derive(Clone, Debug, Serialize, Deserialize)] -pub struct StatePackage { - pub entities: Vec>, -} - -impl Default for StatePackage

{ - fn default() -> Self { - Self { - entities: Vec::new(), - } - } -} - -impl StatePackage

{ - pub fn new() -> Self { Self::default() } - - pub fn with_entities( - mut self, - mut entities: Vec>, - ) -> Self { - self.entities.append(&mut entities); - self - } - - pub fn with_entity(mut self, entry: EntityPackage

) -> Self { - self.entities.push(entry); - self - } -} - #[derive(Clone, Debug, Serialize, Deserialize)] pub struct EntitySyncPackage { pub created_entities: Vec, diff --git a/common/net/src/sync/sync_ext.rs b/common/net/src/sync/sync_ext.rs index 90d09985e8..f2256bd3c9 100644 --- a/common/net/src/sync/sync_ext.rs +++ b/common/net/src/sync/sync_ext.rs @@ -1,7 +1,5 @@ use super::{ - packet::{ - CompPacket, CompSyncPackage, CompUpdateKind, EntityPackage, EntitySyncPackage, StatePackage, - }, + packet::{CompPacket, CompSyncPackage, CompUpdateKind, EntityPackage, EntitySyncPackage}, track::UpdateTracker, }; use common::{ @@ -31,7 +29,6 @@ pub trait WorldSyncExt { &mut self, entity_package: EntityPackage

, ) -> specs::Entity; - fn apply_state_package(&mut self, state_package: StatePackage

); fn apply_entity_sync_package(&mut self, package: EntitySyncPackage); fn apply_comp_sync_package(&mut self, package: CompSyncPackage

); } @@ -99,19 +96,6 @@ impl WorldSyncExt for specs::World { } } - fn apply_state_package(&mut self, state_package: StatePackage

) { - let StatePackage { entities } = state_package; - - // Apply state package entities - for entity_package in entities { - self.apply_entity_package(entity_package); - } - - // TODO: determine if this is needed - // Initialize entities - //self.maintain(); - } - fn apply_entity_sync_package(&mut self, package: EntitySyncPackage) { // Take ownership of the fields let EntitySyncPackage { diff --git a/common/src/character.rs b/common/src/character.rs index 112c4e5a81..c065941d21 100644 --- a/common/src/character.rs +++ b/common/src/character.rs @@ -18,7 +18,7 @@ pub struct Character { /// Data needed to render a single character item in the character list /// presented during character selection. -#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +#[derive(Clone, Debug, Serialize, Deserialize)] pub struct CharacterItem { pub character: Character, pub body: comp::Body, diff --git a/common/src/comp/chat.rs b/common/src/comp/chat.rs index d5ed529e1c..6d319c818d 100644 --- a/common/src/comp/chat.rs +++ b/common/src/comp/chat.rs @@ -44,8 +44,8 @@ impl ChatMode { } } -impl Default for ChatMode { - fn default() -> Self { Self::World } +impl ChatMode { + pub const fn default() -> Self { Self::World } } #[derive(Debug, Clone, Serialize, Deserialize)] diff --git a/common/src/comp/inputs.rs b/common/src/comp/inputs.rs index fddbe7d4e2..5d93e4247f 100644 --- a/common/src/comp/inputs.rs +++ b/common/src/comp/inputs.rs @@ -1,8 +1,8 @@ use crate::depot::Id; +use hashbrown::HashSet; use serde::{Deserialize, Serialize}; use specs::{Component, DerefFlaggedStorage}; use specs_idvs::IdvStorage; -use std::collections::HashSet; use vek::geom::Aabb; #[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize)] diff --git a/common/src/comp/inventory/item/mod.rs b/common/src/comp/inventory/item/mod.rs index 99cec6222b..1bcf4d9738 100644 --- a/common/src/comp/inventory/item/mod.rs +++ b/common/src/comp/inventory/item/mod.rs @@ -17,16 +17,17 @@ use crate::{ recipe::RecipeInput, terrain::{Block, SpriteKind}, }; -use core::mem; +use core::{ + convert::TryFrom, + mem, + num::{NonZeroU32, NonZeroU64}, + ops::Deref, +}; use crossbeam_utils::atomic::AtomicCell; use serde::{Deserialize, Serialize}; use specs::{Component, DerefFlaggedStorage}; use specs_idvs::IdvStorage; -use std::{ - num::{NonZeroU32, NonZeroU64}, - ops::Deref, - sync::Arc, -}; +use std::sync::Arc; use vek::Rgb; #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] @@ -185,6 +186,12 @@ pub struct CreateDatabaseItemId { item_id: Arc, }*/ +/// NOTE: Do not call `Item::clone` without consulting the core devs! It only +/// exists due to being required for message serialization at the moment, and +/// should not be used for any other purpose. +/// +/// FIXME: Turn on a Clippy lint forbidding the use of `Item::clone` using the +/// `disallowed_method` feature. #[derive(Clone, Debug, Serialize, Deserialize)] pub struct Item { /// item_id is hidden because it represents the persistent, storage entity @@ -242,21 +249,28 @@ pub struct ItemConfig { pub dodge_ability: Option, } -impl From<(&ItemKind, &[Item], &AbilityMap, &MaterialStatManifest)> for ItemConfig { - fn from( +#[derive(Debug)] +pub enum ItemConfigError { + BadItemKind, +} + +impl TryFrom<(&ItemKind, &[Item], &AbilityMap, &MaterialStatManifest)> for ItemConfig { + type Error = ItemConfigError; + + fn try_from( (item_kind, components, map, msm): (&ItemKind, &[Item], &AbilityMap, &MaterialStatManifest), - ) -> Self { + ) -> Result { if let ItemKind::Tool(tool) = item_kind { let abilities = tool.get_abilities(msm, components, map); - return ItemConfig { + Ok(ItemConfig { abilities, block_ability: None, dodge_ability: Some(CharacterAbility::default_roll()), - }; + }) + } else { + Err(ItemConfigError::BadItemKind) } - - unimplemented!("ItemConfig is currently only supported for Tools") } } @@ -314,6 +328,12 @@ impl ItemDef { } } +/// NOTE: This PartialEq instance is pretty broken! It doesn't check item +/// amount or any child items (and, arguably, doing so should be able to ignore +/// things like item order within the main inventory or within each bag, and +/// possibly even coalesce amounts, though these may be more controversial). +/// Until such time as we find an actual need for a proper PartialEq instance, +/// please don't rely on this for anything! impl PartialEq for Item { fn eq(&self, other: &Self) -> bool { self.item_def.item_definition_id == other.item_def.item_definition_id @@ -461,7 +481,20 @@ impl Item { /// Duplicates an item, creating an exact copy but with a new item ID pub fn duplicate(&self, msm: &MaterialStatManifest) -> Self { - Item::new_from_item_def(Arc::clone(&self.item_def), &self.components, msm) + let mut new_item = + Item::new_from_item_def(Arc::clone(&self.item_def), &self.components, msm); + new_item.set_amount(self.amount()).expect( + "`new_item` has the same `item_def` and as an invariant, \ + self.set_amount(self.amount()) should always succeed.", + ); + new_item.slots_mut().iter_mut().zip(self.slots()).for_each( + |(new_item_slot, old_item_slot)| { + *new_item_slot = old_item_slot + .as_ref() + .map(|old_item| old_item.duplicate(msm)); + }, + ); + new_item } /// FIXME: HACK: In order to set the entity ID asynchronously, we currently @@ -501,6 +534,7 @@ impl Item { let amount = u32::from(self.amount); self.amount = amount .checked_add(increase_by) + .filter(|&amount| amount <= self.max_amount()) .and_then(NonZeroU32::new) .ok_or(OperationFailure)?; Ok(()) @@ -516,7 +550,7 @@ impl Item { } pub fn set_amount(&mut self, give_amount: u32) -> Result<(), OperationFailure> { - if give_amount == 1 || self.item_def.is_stackable() { + if give_amount <= self.max_amount() { self.amount = NonZeroU32::new(give_amount).ok_or(OperationFailure)?; Ok(()) } else { @@ -534,16 +568,14 @@ impl Item { } fn update_item_config(&mut self, msm: &MaterialStatManifest) { - self.item_config = if let ItemKind::Tool(_) = self.kind() { - Some(Box::new(ItemConfig::from(( - self.kind(), - self.components(), - &self.item_def.ability_map, - msm, - )))) - } else { - None - }; + if let Ok(item_config) = ItemConfig::try_from(( + self.kind(), + self.components(), + &self.item_def.ability_map, + msm, + )) { + self.item_config = Some(Box::new(item_config)); + } } /// Returns an iterator that drains items contained within the item's slots @@ -572,6 +604,10 @@ impl Item { pub fn amount(&self) -> u32 { u32::from(self.amount) } + /// NOTE: invariant that amount() ≤ max_amount(), 1 ≤ max_amount(), + /// and if !self.is_stackable(), self.max_amount() = 1. + pub fn max_amount(&self) -> u32 { if self.is_stackable() { u32::MAX } else { 1 } } + pub fn quality(&self) -> Quality { self.item_def.quality } pub fn components(&self) -> &[Item] { &self.components } diff --git a/common/src/comp/inventory/loadout.rs b/common/src/comp/inventory/loadout.rs index b9318d01cb..e095e401e2 100644 --- a/common/src/comp/inventory/loadout.rs +++ b/common/src/comp/inventory/loadout.rs @@ -10,12 +10,13 @@ use serde::{Deserialize, Serialize}; use std::ops::Range; use tracing::warn; -#[derive(Clone, PartialEq, Debug, Serialize, Deserialize)] +#[derive(Clone, Debug, Serialize, Deserialize)] pub struct Loadout { slots: Vec, } -#[derive(Clone, PartialEq, Debug, Serialize, Deserialize)] +/// NOTE: Please don't derive a PartialEq Instance for this; that's broken! +#[derive(Clone, Debug, Serialize, Deserialize)] pub struct LoadoutSlot { /// The EquipSlot that this slot represents pub(super) equip_slot: EquipSlot, diff --git a/common/src/comp/inventory/loadout_builder.rs b/common/src/comp/inventory/loadout_builder.rs index 44d24ebaa8..ff995f1c88 100644 --- a/common/src/comp/inventory/loadout_builder.rs +++ b/common/src/comp/inventory/loadout_builder.rs @@ -599,7 +599,10 @@ impl LoadoutBuilder { let mut item_with_amount = |item_id: &str, amount: &mut f32| { if *amount > 0.0 { let mut item = Item::new_from_asset_expect(&item_id); - let n = rng.gen_range(1..(amount.min(100.0) as u32).max(2)); + // NOTE: Conversion to and from f32 works fine because we make sure the + // number we're converting is ≤ 100. + let max = amount.min(100.min(item.max_amount()) as f32) as u32; + let n = rng.gen_range(1..max.max(2)); *amount -= if item.set_amount(n).is_ok() { n as f32 } else { diff --git a/common/src/comp/inventory/mod.rs b/common/src/comp/inventory/mod.rs index 0727ac36d5..97c23314a6 100644 --- a/common/src/comp/inventory/mod.rs +++ b/common/src/comp/inventory/mod.rs @@ -31,7 +31,8 @@ pub mod trade_pricing; pub type InvSlot = Option; const DEFAULT_INVENTORY_SLOTS: usize = 18; -#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +/// NOTE: Do not add a PartialEq instance for Inventory; that's broken! +#[derive(Clone, Debug, Serialize, Deserialize)] pub struct Inventory { loadout: Loadout, /// The "built-in" slots belonging to the inventory itself, all other slots @@ -99,24 +100,61 @@ impl Inventory { } /// Adds a new item to the first fitting group of the inventory or starts a - /// new group. Returns the item again if no space was found. - pub fn push(&mut self, item: Item) -> Option { - if item.is_stackable() { - if let Some(slot_item) = self - .slots_mut() - .filter_map(Option::as_mut) - .find(|s| *s == &item) - { - return slot_item - .increase_amount(item.amount()) - .err() - .and(Some(item)); - } - } + /// new group. Returns the item in an error if no space was found, otherwise + /// returns the found slot. + pub fn push(&mut self, item: Item) -> Result<(), Item> { + // First, check to make sure there's enough room for all instances of the + // item (note that if we find any empty slots, we can guarantee this by + // just filling up the whole slot, but to be nice we won't use it if we + // can find enough space in any combination of existing slots, and + // that's what we check in the `is_stackable` case). - // No existing item to stack with or item not stackable, put the item in a new - // slot - self.insert(item) + if item.is_stackable() + && self + .slots() + .filter_map(Option::as_ref) + .filter(|s| *s == &item) + .try_fold(item.amount(), |remaining, current| { + remaining + .checked_sub(current.max_amount() - current.amount()) + .filter(|&remaining| remaining > 0) + }) + .is_none() + { + // We either exactly matched or had more than enough space for inserting the + // item into existing slots, so go do that! + assert!( + self.slots_mut() + .filter_map(Option::as_mut) + .filter(|s| *s == &item) + .try_fold(item.amount(), |remaining, current| { + // NOTE: Invariant that current.amount <= current.max_amount(), so the + // subtraction is safe. + let new_remaining = remaining + .checked_sub(current.max_amount() - current.amount()) + .filter(|&remaining| remaining > 0); + if new_remaining.is_some() { + // Not enough capacity left to hold all the remaining items, so we fill + // it as much as we can. + current + .set_amount(current.max_amount()) + .expect("max_amount() is always a valid amount."); + } else { + // Enough capacity to hold all the remaining items. + current + .increase_amount(remaining) + .expect("Already checked that there is enough room."); + } + new_remaining + }) + .is_none() + ); + Ok(()) + } else { + // No existing item to stack with or item not stackable, put the item in a new + // slot + self.insert(item) + } } /// Add a series of items to inventory, returning any which do not fit as an @@ -125,7 +163,7 @@ impl Inventory { // Vec doesn't allocate for zero elements so this should be cheap let mut leftovers = Vec::new(); for item in items { - if let Some(item) = self.push(item) { + if let Err(item) = self.push(item) { leftovers.push(item); } } @@ -149,7 +187,9 @@ impl Inventory { let mut leftovers = Vec::new(); for item in &mut items { if self.contains(&item).not() { - self.push(item).map(|overflow| leftovers.push(overflow)); + if let Err(overflow) = self.push(item) { + leftovers.push(overflow); + } } // else drop item if it was already in } if !leftovers.is_empty() { @@ -181,14 +221,17 @@ impl Inventory { } } if let Some(amount) = amount { - self.remove(src); let dstitem = self .get_mut(dst) .expect("self.get(dst) was Some right above this"); dstitem .increase_amount(amount) - .expect("already checked is_stackable"); - true + .map(|_| { + // Suceeded in adding the item, so remove it from `src`. + self.remove(src).expect("Already verified that src was populated."); + }) + // Can fail if we exceed `max_amount` + .is_ok() } else { false } @@ -318,9 +361,11 @@ impl Inventory { let mut return_item = item.duplicate(msm); let returning_amount = item.amount() / 2; item.decrease_amount(returning_amount).ok()?; - return_item - .set_amount(returning_amount) - .expect("Items duplicated from a stackable item must be stackable."); + return_item.set_amount(returning_amount).expect( + "return_item.amount() = item.amount() / 2 < item.amount() (since \ + item.amount() ≥ 1) ≤ item.max_amount() = return_item.max_amount(), since \ + return_item is a duplicate of item", + ); Some(return_item) } else { self.remove(inv_slot_id) @@ -367,6 +412,8 @@ impl Inventory { .filter(|item| item.matches_recipe_input(&*input)) { let claim = slot_claims.entry(inv_slot_id).or_insert(0); + // FIXME: Fishy, looks like it can underflow before min which can trigger an + // overflow check. let can_claim = (item.amount() - *claim).min(needed); *claim += can_claim; needed -= can_claim; @@ -387,14 +434,15 @@ impl Inventory { } /// Adds a new item to the first empty slot of the inventory. Returns the - /// item again if no free slot was found. - fn insert(&mut self, item: Item) -> Option { + /// item again in an Err if no free slot was found, otherwise returns a + /// reference to the item. + fn insert(&mut self, item: Item) -> Result<(), Item> { match self.slots_mut().find(|slot| slot.is_none()) { Some(slot) => { *slot = Some(item); - None + Ok(()) }, - None => Some(item), + None => Err(item), } } @@ -495,7 +543,7 @@ impl Inventory { /// currently supported. pub fn pickup_item(&mut self, mut item: Item) -> Result<(), Item> { if item.is_stackable() { - return self.push(item).map_or(Ok(()), Err); + return self.push(item); } if self.free_slots() < item.populated_slots() + 1 { @@ -506,11 +554,9 @@ impl Inventory { // itself into the inventory. We already know that there are enough free slots // so push will never give us an item back. item.drain().for_each(|item| { - self.push(item).unwrap_none(); + self.push(item).unwrap(); }); - self.push(item).unwrap_none(); - - Ok(()) + self.push(item) } /// Unequip an item from slot and place into inventory. Will leave the item @@ -528,7 +574,7 @@ impl Inventory { .and_then(|mut unequipped_item| { let unloaded_items: Vec = unequipped_item.drain().collect(); self.push(unequipped_item) - .expect_none("Failed to push item to inventory, precondition failed?"); + .expect("Failed to push item to inventory, precondition failed?"); // Unload any items that were inside the equipped item into the inventory, with // any that don't fit to be to be dropped on the floor by the caller @@ -626,7 +672,7 @@ impl Inventory { // inventory slot instead. if let Err(returned) = self.insert_at(inv_slot_id, from_equip) { self.push(returned) - .expect_none("Unable to push to inventory, no slots (bug in can_swap()?)"); + .expect("Unable to push to inventory, no slots (bug in can_swap()?)"); } items @@ -686,7 +732,7 @@ impl Component for Inventory { type Storage = DerefFlaggedStorage>; } -#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +#[derive(Clone, Debug, Serialize, Deserialize)] pub enum InventoryUpdateEvent { Init, Used, diff --git a/common/src/comp/inventory/test.rs b/common/src/comp/inventory/test.rs index 3b15c8dfee..5cc97a73d4 100644 --- a/common/src/comp/inventory/test.rs +++ b/common/src/comp/inventory/test.rs @@ -13,38 +13,47 @@ lazy_static! { /// Attempting to push into a full inventory should return the same item. #[test] fn push_full() { + let msm = &MaterialStatManifest::default(); let mut inv = Inventory { - slots: TEST_ITEMS.iter().map(|a| Some(a.clone())).collect(), + slots: TEST_ITEMS.iter().map(|a| Some(a.duplicate(msm))).collect(), loadout: LoadoutBuilder::new().build(), }; assert_eq!( - inv.push(TEST_ITEMS[0].clone()).unwrap(), - TEST_ITEMS[0].clone() + inv.push(TEST_ITEMS[0].duplicate(msm)).unwrap_err(), + TEST_ITEMS[0].duplicate(msm) ) } /// Attempting to push a series into a full inventory should return them all. #[test] fn push_all_full() { + let msm = &MaterialStatManifest::default(); let mut inv = Inventory { - slots: TEST_ITEMS.iter().map(|a| Some(a.clone())).collect(), + slots: TEST_ITEMS.iter().map(|a| Some(a.duplicate(msm))).collect(), loadout: LoadoutBuilder::new().build(), }; let Error::Full(leftovers) = inv - .push_all(TEST_ITEMS.iter().cloned()) + .push_all(TEST_ITEMS.iter().map(|item| item.duplicate(msm))) .expect_err("Pushing into a full inventory somehow worked!"); - assert_eq!(leftovers, TEST_ITEMS.clone()) + assert_eq!( + leftovers, + TEST_ITEMS + .iter() + .map(|item| item.duplicate(msm)) + .collect::>() + ) } /// Attempting to push uniquely into an inventory containing all the items /// should work fine. #[test] fn push_unique_all_full() { + let msm = &MaterialStatManifest::default(); let mut inv = Inventory { - slots: TEST_ITEMS.iter().map(|a| Some(a.clone())).collect(), + slots: TEST_ITEMS.iter().map(|a| Some(a.duplicate(msm))).collect(), loadout: LoadoutBuilder::new().build(), }; - inv.push_all_unique(TEST_ITEMS.iter().cloned()) + inv.push_all_unique(TEST_ITEMS.iter().map(|item| item.duplicate(msm))) .expect("Pushing unique items into an inventory that already contains them didn't work!"); } @@ -52,11 +61,12 @@ fn push_unique_all_full() { /// should work fine. #[test] fn push_all_empty() { + let msm = &MaterialStatManifest::default(); let mut inv = Inventory { slots: vec![None, None], loadout: LoadoutBuilder::new().build(), }; - inv.push_all(TEST_ITEMS.iter().cloned()) + inv.push_all(TEST_ITEMS.iter().map(|item| item.duplicate(msm))) .expect("Pushing items into an empty inventory didn't work!"); } @@ -64,22 +74,25 @@ fn push_all_empty() { /// should work fine. #[test] fn push_all_unique_empty() { + let msm = &MaterialStatManifest::default(); let mut inv = Inventory { slots: vec![None, None], loadout: LoadoutBuilder::new().build(), }; - inv.push_all_unique(TEST_ITEMS.iter().cloned()).expect( - "Pushing unique items into an empty inventory that didn't contain them didn't work!", - ); + inv.push_all_unique(TEST_ITEMS.iter().map(|item| item.duplicate(msm))) + .expect( + "Pushing unique items into an empty inventory that didn't contain them didn't work!", + ); } #[test] fn free_slots_minus_equipped_item_items_only_present_in_equipped_bag_slots() { + let msm = &MaterialStatManifest::default(); let mut inv = Inventory::new_empty(); let bag = get_test_bag(18); let bag1_slot = EquipSlot::Armor(ArmorSlot::Bag1); - inv.loadout.swap(bag1_slot, Some(bag.clone())); + inv.loadout.swap(bag1_slot, Some(bag.duplicate(msm))); inv.insert_at(InvSlotId::new(15, 0), bag) .unwrap() @@ -94,13 +107,14 @@ fn free_slots_minus_equipped_item_items_only_present_in_equipped_bag_slots() { #[test] fn free_slots_minus_equipped_item() { + let msm = &MaterialStatManifest::default(); let mut inv = Inventory::new_empty(); let bag = get_test_bag(18); let bag1_slot = EquipSlot::Armor(ArmorSlot::Bag1); - inv.loadout.swap(bag1_slot, Some(bag.clone())); + inv.loadout.swap(bag1_slot, Some(bag.duplicate(msm))); inv.loadout - .swap(EquipSlot::Armor(ArmorSlot::Bag2), Some(bag.clone())); + .swap(EquipSlot::Armor(ArmorSlot::Bag2), Some(bag.duplicate(msm))); inv.insert_at(InvSlotId::new(16, 0), bag) .unwrap() @@ -171,12 +185,13 @@ fn can_swap_equipped_bag_into_only_empty_slot_provided_by_itself_should_return_f #[test] fn unequip_items_both_hands() { + let msm = &MaterialStatManifest::default(); let mut inv = Inventory::new_empty(); let sword = Item::new_from_asset_expect("common.items.weapons.sword.steel-8"); - inv.replace_loadout_item(EquipSlot::Mainhand, Some(sword.clone())); - inv.replace_loadout_item(EquipSlot::Offhand, Some(sword.clone())); + inv.replace_loadout_item(EquipSlot::Mainhand, Some(sword.duplicate(msm))); + inv.replace_loadout_item(EquipSlot::Offhand, Some(sword.duplicate(msm))); // Fill all inventory slots except one fill_inv_slots(&mut inv, 17); @@ -200,6 +215,7 @@ fn unequip_items_both_hands() { #[test] fn equip_replace_already_equipped_item() { + let msm = &MaterialStatManifest::default(); let boots = Item::new_from_asset_expect("common.items.testing.test_boots"); let starting_sandles = Some(Item::new_from_asset_expect( @@ -207,8 +223,11 @@ fn equip_replace_already_equipped_item() { )); let mut inv = Inventory::new_empty(); - inv.push(boots.clone()); - inv.replace_loadout_item(EquipSlot::Armor(ArmorSlot::Feet), starting_sandles.clone()); + inv.push(boots.duplicate(msm)).unwrap(); + inv.replace_loadout_item( + EquipSlot::Armor(ArmorSlot::Feet), + starting_sandles.as_ref().map(|item| item.duplicate(msm)), + ); let _ = inv.equip(InvSlotId::new(0, 0)); @@ -219,10 +238,7 @@ fn equip_replace_already_equipped_item() { ); // Verify inventory - assert_eq!( - inv.slots[0].as_ref().unwrap().item_definition_id(), - starting_sandles.unwrap().item_definition_id() - ); + assert_eq!(&inv.slots[0], &starting_sandles,); assert_eq!(inv.populated_slots(), 1); } @@ -274,14 +290,15 @@ fn unequip_unequipping_bag_into_its_own_slot_with_no_other_free_slots() { #[test] fn equip_one_bag_equipped_equip_second_bag() { + let msm = &MaterialStatManifest::default(); let mut inv = Inventory::new_empty(); let bag = get_test_bag(9); inv.loadout - .swap(EquipSlot::Armor(ArmorSlot::Bag1), Some(bag.clone())) + .swap(EquipSlot::Armor(ArmorSlot::Bag1), Some(bag.duplicate(msm))) .unwrap_none(); - inv.push(bag); + inv.push(bag).unwrap(); let _ = inv.equip(InvSlotId::new(0, 0)); @@ -298,7 +315,7 @@ fn free_after_swap_equipped_item_has_more_slots() { .unwrap_none(); let small_bag = get_test_bag(9); - inv.push(small_bag); + inv.push(small_bag).unwrap(); // Fill all remaining slots fill_inv_slots(&mut inv, 35); @@ -319,10 +336,10 @@ fn free_after_swap_equipped_item_has_less_slots() { .unwrap_none(); let small_bag = get_test_bag(18); - inv.push(small_bag); + inv.push(small_bag).unwrap(); // Fill all slots except the last one - fill_inv_slots(&mut inv, 27); + fill_inv_slots(&mut inv, 26); let result = inv.free_after_swap(EquipSlot::Armor(ArmorSlot::Bag1), InvSlotId::new(0, 0)); @@ -352,7 +369,7 @@ fn free_after_swap_equipped_item_with_slots_swapped_with_empty_inv_slot() { fn free_after_swap_inv_item_with_slots_swapped_with_empty_equip_slot() { let mut inv = Inventory::new_empty(); - inv.push(get_test_bag(9)); + inv.push(get_test_bag(9)).unwrap(); // Add 5 items to the inventory fill_inv_slots(&mut inv, 5); @@ -368,7 +385,7 @@ fn free_after_swap_inv_item_without_slots_swapped_with_empty_equip_slot() { let mut inv = Inventory::new_empty(); let boots = Item::new_from_asset_expect("common.items.testing.test_boots"); - inv.push(boots); + inv.push(boots).unwrap(); // Add 5 items to the inventory fill_inv_slots(&mut inv, 5); @@ -380,8 +397,9 @@ fn free_after_swap_inv_item_without_slots_swapped_with_empty_equip_slot() { } fn fill_inv_slots(inv: &mut Inventory, items: u16) { + let msm = &MaterialStatManifest::default(); let boots = Item::new_from_asset_expect("common.items.testing.test_boots"); for _ in 0..items { - inv.push(boots.clone()); + inv.push(boots.duplicate(msm)).unwrap(); } } diff --git a/common/src/comp/inventory/trade_pricing.rs b/common/src/comp/inventory/trade_pricing.rs index 7fa8fd426d..18dd3c2ac7 100644 --- a/common/src/comp/inventory/trade_pricing.rs +++ b/common/src/comp/inventory/trade_pricing.rs @@ -10,13 +10,9 @@ use lazy_static::lazy_static; use serde::Deserialize; use tracing::{info, warn}; -#[derive(Debug)] -struct Entry { - probability: f32, - item: String, -} +type Entry = (String, f32); -type Entries = Vec<(String, f32)>; +type Entries = Vec; const PRICING_DEBUG: bool = false; #[derive(Default, Debug)] @@ -103,29 +99,29 @@ impl TradePricing { // add this much of a non-consumed crafting tool price - fn get_list(&self, good: Good) -> &Entries { + fn get_list(&self, good: Good) -> &[Entry] { match good { Good::Armor => &self.armor, Good::Tools => &self.tools, Good::Potions => &self.potions, Good::Food => &self.food, Good::Ingredients => &self.ingredients, - _ => panic!("invalid good"), + _ => &[], } } - fn get_list_mut(&mut self, good: Good) -> &mut Entries { + fn get_list_mut(&mut self, good: Good) -> &mut [Entry] { match good { Good::Armor => &mut self.armor, Good::Tools => &mut self.tools, Good::Potions => &mut self.potions, Good::Food => &mut self.food, Good::Ingredients => &mut self.ingredients, - _ => panic!("invalid good"), + _ => &mut [], } } - fn get_list_by_path(&self, name: &str) -> &Entries { + fn get_list_by_path(&self, name: &str) -> &[Entry] { match name { "common.items.crafting_ing.mindflayer_bag_damaged" => &self.armor, _ if name.starts_with("common.items.crafting_ing.") => &self.ingredients, @@ -186,12 +182,15 @@ impl TradePricing { entryvec.push((itemname.to_string(), probability)); } } - fn sort_and_normalize(entryvec: &mut Entries, scale: f32) { + fn sort_and_normalize(entryvec: &mut [Entry], scale: f32) { if !entryvec.is_empty() { entryvec.sort_by(|a, b| a.1.partial_cmp(&b.1).unwrap()); - let rescale = scale / entryvec.last().unwrap().1; - for i in entryvec.iter_mut() { - i.1 *= rescale; + if let Some((_, max_scale)) = entryvec.last() { + // most common item has frequency max_scale. avoid NaN + let rescale = scale / max_scale; + for i in entryvec.iter_mut() { + i.1 *= rescale; + } } } } @@ -228,16 +227,12 @@ impl TradePricing { input: r .inputs .iter() - .filter(|i| matches!(i.0, RecipeInput::Item(_))) - .map(|i| { - ( - if let RecipeInput::Item(it) = &i.0 { - it.id().into() - } else { - panic!("recipe logic broken"); - }, - i.1, - ) + .filter_map(|&(ref recipe_input, count)| { + if let RecipeInput::Item(it) = recipe_input { + Some((it.id().into(), count)) + } else { + None + } }) .collect(), }); @@ -262,16 +257,14 @@ impl TradePricing { // re-look up prices and sort the vector by ascending material cost, return // whether first cost is finite fn price_sort(s: &TradePricing, vec: &mut Vec) -> bool { - if !vec.is_empty() { - for e in vec.iter_mut() { - e.material_cost = calculate_material_cost(s, e); - } - vec.sort_by(|a, b| a.material_cost.partial_cmp(&b.material_cost).unwrap()); - //info!(?vec); - vec.first().unwrap().material_cost < TradePricing::UNAVAILABLE_PRICE - } else { - false + for e in vec.iter_mut() { + e.material_cost = calculate_material_cost(s, e); } + vec.sort_by(|a, b| a.material_cost.partial_cmp(&b.material_cost).unwrap()); + //info!(?vec); + vec.first() + .filter(|recipe| recipe.material_cost < TradePricing::UNAVAILABLE_PRICE) + .is_some() } // re-evaluate prices based on crafting tables // (start with cheap ones to avoid changing material prices after evaluation) diff --git a/common/src/depot.rs b/common/src/depot.rs index 358f357a1d..eed8bcf61c 100644 --- a/common/src/depot.rs +++ b/common/src/depot.rs @@ -3,7 +3,6 @@ use std::{ cmp::{Eq, Ord, Ordering, PartialEq, PartialOrd}, fmt, hash, marker::PhantomData, - ops::{Index, IndexMut}, }; /// Type safe index into Depot @@ -211,13 +210,3 @@ impl Depot { } } } - -impl Index> for Depot { - type Output = T; - - fn index(&self, id: Id) -> &Self::Output { self.get(id).unwrap() } -} - -impl IndexMut> for Depot { - fn index_mut(&mut self, id: Id) -> &mut Self::Output { self.get_mut(id).unwrap() } -} diff --git a/common/src/recipe.rs b/common/src/recipe.rs index b9fee6e6c2..0e67509059 100644 --- a/common/src/recipe.rs +++ b/common/src/recipe.rs @@ -46,7 +46,7 @@ impl Recipe { for i in 0..self.output.1 { let crafted_item = Item::new_from_item_def(Arc::clone(&self.output.0), &components, msm); - if let Some(item) = inv.push(crafted_item) { + if let Err(item) = inv.push(crafted_item) { return Ok(Some((item, self.output.1 - i))); } } diff --git a/common/src/region.rs b/common/src/region.rs index 1233ca5298..3db7fdaf54 100644 --- a/common/src/region.rs +++ b/common/src/region.rs @@ -108,14 +108,9 @@ impl RegionMap { span!(_guard, "tick", "Region::tick"); self.tick += 1; // Clear events within each region - for i in 0..self.regions.len() { - self.regions - .get_index_mut(i) - .map(|(_, v)| v) - .unwrap() - .events - .clear(); - } + self.regions.values_mut().for_each(|region| { + region.events.clear(); + }); // Add any untracked entities for (pos, id) in (&pos, &entities, !&self.tracked_entities) @@ -130,52 +125,55 @@ impl RegionMap { let mut regions_to_remove = Vec::new(); - for i in 0..self.regions.len() { - for (maybe_pos, _maybe_vel, id) in ( - pos.maybe(), - vel.maybe(), - &self.regions.get_index(i).map(|(_, v)| v).unwrap().bitset, - ) - .join() - { - match maybe_pos { - // Switch regions for entities which need switching - // TODO don't check every tick (use velocity) (and use id to stagger) - // Starting parameters at v = 0 check every 100 ticks - // tether_length^2 / vel^2 (with a max of every tick) - Some(pos) => { - let pos = pos.0.map(|e| e as i32); - let current_region = self.index_key(i).unwrap(); - let key = Self::pos_key(pos); - // Consider switching - // Calculate distance outside border - if key != current_region - && (Vec2::::from(pos) - Self::key_pos(current_region)) - .map(|e| e.abs() as u32) - .reduce_max() - > TETHER_LENGTH - { - // Switch - self.entities_to_move.push((i, id, pos)); - } - }, - // Remove any non-existant entities (or just ones that lost their position - // component) TODO: distribute this between ticks - None => { - // TODO: shouldn't there be a way to extract the bitset of entities with - // positions directly from specs? - self.entities_to_remove.push((i, id)); - }, + let RegionMap { + entities_to_move, + entities_to_remove, + regions, + .. + } = self; + regions + .iter() + .enumerate() + .for_each(|(i, (¤t_region, region_data))| { + for (maybe_pos, _maybe_vel, id) in + (pos.maybe(), vel.maybe(), ®ion_data.bitset).join() + { + match maybe_pos { + // Switch regions for entities which need switching + // TODO don't check every tick (use velocity) (and use id to stagger) + // Starting parameters at v = 0 check every 100 ticks + // tether_length^2 / vel^2 (with a max of every tick) + Some(pos) => { + let pos = pos.0.map(|e| e as i32); + let key = Self::pos_key(pos); + // Consider switching + // Calculate distance outside border + if key != current_region + && (Vec2::::from(pos) - Self::key_pos(current_region)) + .map(|e| e.abs() as u32) + .reduce_max() + > TETHER_LENGTH + { + // Switch + entities_to_move.push((i, id, pos)); + } + }, + // Remove any non-existant entities (or just ones that lost their position + // component) TODO: distribute this between ticks + None => { + // TODO: shouldn't there be a way to extract the bitset of entities with + // positions directly from specs? + entities_to_remove.push((i, id)); + }, + } } - } - // Remove region if it is empty - // TODO: distribute this between ticks - let (key, region) = self.regions.get_index(i).unwrap(); - if region.removable() { - regions_to_remove.push(*key); - } - } + // Remove region if it is empty + // TODO: distribute this between ticks + if region_data.removable() { + regions_to_remove.push(current_region); + } + }); // Mutate // Note entity moving is outside the whole loop so that the same entity is not @@ -268,10 +266,6 @@ impl RegionMap { self.regions.get_full(&key).map(|(i, _, _)| i) } - fn index_key(&self, index: usize) -> Option> { - self.regions.get_index(index).map(|(k, _)| k).copied() - } - /// Adds a new region /// Returns the index of the region in the index map fn insert(&mut self, key: Vec2) -> usize { diff --git a/common/sys/src/character_behavior.rs b/common/sys/src/character_behavior.rs index debc10c2b8..d3e8c8c13f 100644 --- a/common/sys/src/character_behavior.rs +++ b/common/sys/src/character_behavior.rs @@ -45,13 +45,16 @@ fn incorporate_update(join: &mut JoinStruct, mut state_update: StateUpdate) { if state_update.swap_equipped_weapons { let mut inventory = join.inventory.get_mut_unchecked(); let inventory = &mut *inventory; - inventory - .swap( - Slot::Equip(EquipSlot::Mainhand), - Slot::Equip(EquipSlot::Offhand), - ) - .first() - .unwrap_none(); // Swapping main and offhand never results in leftover items + assert!( + inventory + .swap( + Slot::Equip(EquipSlot::Mainhand), + Slot::Equip(EquipSlot::Offhand), + ) + .first() + .is_none(), + "Swapping main and offhand never results in leftover items", + ); } } diff --git a/common/sys/src/state.rs b/common/sys/src/state.rs index ca31775284..c30d1046b1 100644 --- a/common/sys/src/state.rs +++ b/common/sys/src/state.rs @@ -20,14 +20,16 @@ use common::{ use common_base::span; use common_ecs::{run_now, PhysicsMetrics, SysMetrics}; use common_net::sync::{interpolation as sync_interp, WorldSyncExt}; -use hashbrown::{HashMap, HashSet}; +use core::{convert::identity, time::Duration}; +use hashbrown::{hash_map, HashMap, HashSet}; use rayon::{ThreadPool, ThreadPoolBuilder}; use specs::{ + prelude::Resource, shred::{Fetch, FetchMut}, storage::{MaskedStorage as EcsMaskedStorage, Storage as EcsStorage}, Component, DispatcherBuilder, Entity as EcsEntity, WorldExt, }; -use std::{sync::Arc, time::Duration}; +use std::sync::Arc; use vek::*; /// How much faster should an in-game day be compared to a real day? @@ -41,18 +43,54 @@ const DAY_CYCLE_FACTOR: f64 = 24.0 * 2.0; /// avoid such a situation. const MAX_DELTA_TIME: f32 = 1.0; +/// NOTE: Please don't add `Deserialize` without checking to make sure we +/// can guarantee the invariant that every entry in `area_names` points to a +/// valid id in `areas`. #[derive(Default)] pub struct BuildAreas { - pub areas: Depot>, - pub area_names: HashMap>>, + areas: Depot>, + area_names: HashMap>>, } +pub enum BuildAreaError { + /// This build area name is reserved by the system. + Reserved, + /// The build area name was not found. + NotFound, +} + +/// Build area names that can only be inserted, not removed. +const RESERVED_BUILD_AREA_NAMES: &[&str] = &["world"]; + impl BuildAreas { - pub fn new() -> Self { - Self { - areas: Depot::default(), - area_names: HashMap::new(), + pub fn areas(&self) -> &Depot> { &self.areas } + + pub fn area_names(&self) -> &HashMap>> { &self.area_names } + + /// If the area_name is already in the map, returns Err(area_name). + pub fn insert(&mut self, area_name: String, area: Aabb) -> Result>, String> { + let area_name_entry = match self.area_names.entry(area_name) { + hash_map::Entry::Occupied(o) => return Err(o.replace_key()), + hash_map::Entry::Vacant(v) => v, + }; + let bb_id = self.areas.insert(area.made_valid()); + area_name_entry.insert(bb_id); + Ok(bb_id) + } + + pub fn remove(&mut self, area_name: &str) -> Result, BuildAreaError> { + if RESERVED_BUILD_AREA_NAMES.contains(&area_name) { + return Err(BuildAreaError::Reserved); } + let bb_id = self + .area_names + .remove(area_name) + .ok_or(BuildAreaError::NotFound)?; + let area = self.areas.remove(bb_id).expect( + "Entries in `areas` are added before entries in `area_names` in `insert`, and that is \ + the only exposed way to add elements to `area_names`.", + ); + Ok(area) } } @@ -221,7 +259,7 @@ impl State { ecs.insert(PlayerEntity(None)); ecs.insert(TerrainGrid::new().unwrap()); ecs.insert(BlockChange::default()); - ecs.insert(BuildAreas::new()); + ecs.insert(BuildAreas::default()); ecs.insert(TerrainChanges::default()); ecs.insert(EventBus::::default()); ecs.insert(game_mode); @@ -286,9 +324,24 @@ impl State { self } - /// Write a component attributed to a particular entity. - pub fn write_component(&mut self, entity: EcsEntity, comp: C) { - let _ = self.ecs.write_storage().insert(entity, comp); + /// Write a component attributed to a particular entity, ignoring errors. + /// + /// This should be used *only* when we can guarantee that the rest of the + /// code does not rely on the insert having succeeded (meaning the + /// entity is no longer alive!). + /// + /// Returns None if the entity was dead or there was no previous entry for + /// this component; otherwise, returns Some(old_component). + pub fn write_component_ignore_entity_dead( + &mut self, + entity: EcsEntity, + comp: C, + ) -> Option { + self.ecs + .write_storage() + .insert(entity, comp) + .ok() + .and_then(identity) } /// Delete a component attributed to a particular entity. @@ -306,6 +359,17 @@ impl State { self.ecs.read_storage().get(entity).copied() } + /// Given mutable access to the resource R, assuming the resource + /// component exists (this is already the behavior of functions like `fetch` + /// and `write_component_ignore_entity_dead`). Since all of our resources + /// are generated up front, any failure here is definitely a code bug. + pub fn mut_resource(&mut self) -> &mut R { + self.ecs.get_mut::().expect( + "Tried to fetch an invalid resource even though all our resources should be known at \ + compile time.", + ) + } + /// Get a read-only reference to the storage of a particular component type. pub fn read_storage(&self) -> EcsStorage>> { self.ecs.read_storage::() @@ -355,16 +419,16 @@ impl State { } /// Set a block in this state's terrain. - pub fn set_block(&mut self, pos: Vec3, block: Block) { + pub fn set_block(&self, pos: Vec3, block: Block) { self.ecs.write_resource::().set(pos, block); } /// Check if the block at given position `pos` has already been modified /// this tick. - pub fn can_set_block(&mut self, pos: Vec3) -> bool { + pub fn can_set_block(&self, pos: Vec3) -> bool { !self .ecs - .write_resource::() + .read_resource::() .blocks .contains_key(&pos) } diff --git a/server/src/character_creator.rs b/server/src/character_creator.rs index 9450bc9a8b..64e917b0d5 100644 --- a/server/src/character_creator.rs +++ b/server/src/character_creator.rs @@ -41,10 +41,14 @@ pub fn create_character( let mut inventory = Inventory::new_with_loadout(loadout); // Default items for new characters - inventory.push(Item::new_from_asset_expect( - "common.items.consumable.potion_minor", - )); - inventory.push(Item::new_from_asset_expect("common.items.food.cheese")); + inventory + .push(Item::new_from_asset_expect( + "common.items.consumable.potion_minor", + )) + .expect("Inventory has at least 2 slots left!"); + inventory + .push(Item::new_from_asset_expect("common.items.food.cheese")) + .expect("Inventory has at least 1 slot left!"); let waypoint = None; diff --git a/server/src/cmd.rs b/server/src/cmd.rs index d9a5a89bf2..85b355901d 100644 --- a/server/src/cmd.rs +++ b/server/src/cmd.rs @@ -6,6 +6,7 @@ use crate::{ settings::{BanRecord, EditableSetting}, Server, SpawnPoint, StateExt, }; +use authc::Uuid; use chrono::{NaiveTime, Timelike}; use common::{ cmd::{ChatCommand, CHAT_COMMANDS, CHAT_SHORTCUTS}, @@ -17,6 +18,7 @@ use common::{ invite::InviteKind, ChatType, Inventory, Item, LightEmitter, WaypointArea, }, + depot, effect::Effect, event::{EventBus, ServerEvent}, npc::{self, get_npc_name}, @@ -30,15 +32,11 @@ use common_net::{ msg::{DisconnectReason, Notification, PlayerListUpdate, ServerGeneral}, sync::WorldSyncExt, }; -use common_sys::state::BuildAreas; +use common_sys::state::{BuildAreaError, BuildAreas}; +use core::{convert::TryFrom, time::Duration}; +use hashbrown::HashSet; use rand::Rng; use specs::{Builder, Entity as EcsEntity, Join, WorldExt}; -use std::{ - collections::HashSet, - convert::TryFrom, - ops::{Deref, DerefMut}, - time::Duration, -}; use vek::*; use world::util::Sampler; @@ -60,13 +58,16 @@ impl ChatCommandExt for ChatCommand { format!("You don't have permission to use '/{}'.", self.keyword()), ), ); - return; - } else { - get_handler(self)(server, entity, entity, args, &self); + } else if let Err(err) = get_handler(self)(server, entity, entity, args, &self) { + server.notify_client( + entity, + ServerGeneral::server_msg(ChatType::CommandError, err), + ); } } } +type CmdResult = Result; /// Handler function called when the command is executed. /// # Arguments /// * `&mut Server` - the `Server` instance executing the command. @@ -79,7 +80,13 @@ impl ChatCommandExt for ChatCommand { /// * `&ChatCommand` - the command to execute with the above arguments. /// Handler functions must parse arguments from the the given `String` /// (`scan_fmt!` is included for this purpose). -type CommandHandler = fn(&mut Server, EcsEntity, EcsEntity, String, &ChatCommand); +/// +/// # Returns +/// +/// A `Result` that is `Ok` if the command went smoothly, and `Err` if it +/// failed; on failure, the string is sent to the client who initiated the +/// command. +type CommandHandler = fn(&mut Server, EcsEntity, EcsEntity, String, &ChatCommand) -> CmdResult<()>; fn get_handler(cmd: &ChatCommand) -> CommandHandler { match cmd { ChatCommand::Adminify => handle_adminify, @@ -142,33 +149,133 @@ fn get_handler(cmd: &ChatCommand) -> CommandHandler { } } -fn handle_drop_all( - server: &mut Server, - client: EcsEntity, - _target: EcsEntity, - _args: String, - _action: &ChatCommand, -) { - let pos = server +// Fallibly get position of entity with the given descriptor (used for error +// message). +fn position(server: &Server, entity: EcsEntity, descriptor: &str) -> CmdResult { + server .state .ecs() .read_storage::() - .get(client) - .cloned(); + .get(entity) + .copied() + .ok_or_else(|| format!("Cannot get position for {:?}!", descriptor)) +} + +fn position_mut( + server: &mut Server, + entity: EcsEntity, + descriptor: &str, + f: impl for<'a> FnOnce(&'a mut comp::Pos) -> T, +) -> CmdResult { + let mut pos_storage = server.state.ecs_mut().write_storage::(); + pos_storage + .get_mut(entity) + .map(f) + .ok_or_else(|| format!("Cannot get position for {:?}!", descriptor)) +} + +fn insert_or_replace_component( + server: &mut Server, + entity: EcsEntity, + component: C, + descriptor: &str, +) -> CmdResult<()> { + server + .state + .ecs_mut() + .write_storage() + .insert(entity, component) + .and(Ok(())) + .map_err(|_| format!("Entity {:?} is dead!", descriptor)) +} + +// Fallibly get uid of entity with the given descriptor (used for error +// message). +fn uid(server: &Server, target: EcsEntity, descriptor: &str) -> CmdResult { + server + .state + .ecs() + .read_storage::() + .get(target) + .copied() + .ok_or_else(|| format!("Cannot get uid for {:?}", descriptor)) +} + +fn area(server: &mut Server, area_name: &str) -> CmdResult>> { + server + .state + .mut_resource::() + .area_names() + .get(area_name) + .copied() + .ok_or_else(|| format!("Area name not found: {}", area_name)) +} + +// Prevent use through sudo. +fn no_sudo(client: EcsEntity, target: EcsEntity) -> CmdResult<()> { + if client == target { + Ok(()) + } else { + // This happens when [ab]using /sudo + Err("It's rude to impersonate people".into()) + } +} + +// Prevent application to hardcoded administrators. +fn verify_not_hardcoded_admin(server: &mut Server, uuid: Uuid, reason: &str) -> CmdResult<()> { + server + .editable_settings() + .admins + .contains(&uuid) + .then_some(()) + .ok_or_else(|| reason.into()) +} + +fn find_alias(ecs: &specs::World, alias: &str) -> CmdResult<(EcsEntity, Uuid)> { + (&ecs.entities(), &ecs.read_storage::()) + .join() + .find(|(_, player)| player.alias == alias) + .map(|(entity, player)| (entity, player.uuid())) + .ok_or_else(|| format!("Player {:?} not found!", alias)) +} + +fn find_uuid(ecs: &specs::World, uuid: Uuid) -> CmdResult { + (&ecs.entities(), &ecs.read_storage::()) + .join() + .find(|(_, player)| player.uuid() == uuid) + .map(|(entity, _)| entity) + .ok_or_else(|| format!("Player with UUID {:?} not found!", uuid)) +} + +fn find_username(server: &mut Server, username: &str) -> CmdResult { + server + .state + .mut_resource::() + .username_to_uuid(username) + .map_err(|_| format!("Unable to determine UUID for username \"{}\"", username)) +} + +fn handle_drop_all( + server: &mut Server, + _client: EcsEntity, + target: EcsEntity, + _args: String, + _action: &ChatCommand, +) -> CmdResult<()> { + let pos = position(server, target, "target")?; let mut items = Vec::new(); if let Some(mut inventory) = server .state .ecs() .write_storage::() - .get_mut(client) + .get_mut(target) { items = inventory.drain().collect(); } let mut rng = rand::thread_rng(); - let pos = pos.expect("expected pos for entity using dropall command"); for item in items { let vel = Vec3::new(rng.gen_range(-0.1..0.1), rng.gen_range(-0.1..0.1), 0.5); @@ -184,22 +291,25 @@ fn handle_drop_all( .with(comp::Vel(vel)) .build(); } + + Ok(()) } #[allow(clippy::useless_conversion)] // TODO: Pending review in #587 fn handle_give_item( server: &mut Server, - client: EcsEntity, + _client: EcsEntity, target: EcsEntity, args: String, action: &ChatCommand, -) { +) -> CmdResult<()> { if let (Some(item_name), give_amount_opt) = scan_fmt_some!(&args, &action.arg_fmt(), String, u32) { let give_amount = give_amount_opt.unwrap_or(1); if let Ok(item) = Item::new_from_asset(&item_name.replace('/', ".").replace("\\", ".")) { let mut item: Item = item; + let mut res = Ok(()); if let Ok(()) = item.set_amount(give_amount.min(2000)) { server .state @@ -207,17 +317,12 @@ fn handle_give_item( .write_storage::() .get_mut(target) .map(|mut inv| { - if inv.push(item).is_some() { - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandError, - format!( - "Player inventory full. Gave 0 of {} items.", - give_amount - ), - ), - ); + // NOTE: Deliberately ignores items that couldn't be pushed. + if inv.push(item).is_err() { + res = Err(format!( + "Player inventory full. Gave 0 of {} items.", + give_amount + )); } }); } else { @@ -230,129 +335,80 @@ fn handle_give_item( .get_mut(target) .map(|mut inv| { for i in 0..give_amount { - if inv.push(item.duplicate(&msm)).is_some() { - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandError, - format!( - "Player inventory full. Gave {} of {} items.", - i, give_amount - ), - ), - ); + // NOTE: Deliberately ignores items that couldn't be pushed. + if inv.push(item.duplicate(&msm)).is_err() { + res = Err(format!( + "Player inventory full. Gave {} of {} items.", + i, give_amount + )); break; } } }); } - let _ = server - .state - .ecs() - .write_storage::() - .insert( - target, - comp::InventoryUpdate::new(comp::InventoryUpdateEvent::Given), - ); + insert_or_replace_component( + server, + target, + comp::InventoryUpdate::new(comp::InventoryUpdateEvent::Given), + "target", + )?; + res } else { - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandError, - format!("Invalid item: {}", item_name), - ), - ); + Err(format!("Invalid item: {}", item_name)) } } else { - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, action.help_string()), - ); + Err(action.help_string()) } } fn handle_make_block( server: &mut Server, - client: EcsEntity, + _client: EcsEntity, target: EcsEntity, args: String, action: &ChatCommand, -) { +) -> CmdResult<()> { if let Some(block_name) = scan_fmt_some!(&args, &action.arg_fmt(), String) { if let Ok(bk) = BlockKind::try_from(block_name.as_str()) { - match server.state.read_component_copied::(target) { - Some(pos) => server.state.set_block( - pos.0.map(|e| e.floor() as i32), - Block::new(bk, Rgb::broadcast(255)), - ), - None => server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandError, - String::from("You have no position."), - ), - ), - } - } else { - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandError, - format!("Invalid block kind: {}", block_name), - ), + let pos = position(server, target, "target")?; + server.state.set_block( + pos.0.map(|e| e.floor() as i32), + Block::new(bk, Rgb::broadcast(255)), ); + Ok(()) + } else { + Err(format!("Invalid block kind: {}", block_name)) } } else { - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, action.help_string()), - ); + Err(action.help_string()) } } fn handle_make_sprite( server: &mut Server, - client: EcsEntity, + _client: EcsEntity, target: EcsEntity, args: String, action: &ChatCommand, -) { +) -> CmdResult<()> { if let Some(sprite_name) = scan_fmt_some!(&args, &action.arg_fmt(), String) { if let Ok(sk) = SpriteKind::try_from(sprite_name.as_str()) { - match server.state.read_component_copied::(target) { - Some(pos) => { - let pos = pos.0.map(|e| e.floor() as i32); - let new_block = server - .state - .get_block(pos) - // TODO: Make more principled. - .unwrap_or_else(|| Block::air(SpriteKind::Empty)) - .with_sprite(sk); - server.state.set_block(pos, new_block); - }, - None => server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandError, - String::from("You have no position."), - ), - ), - } + let pos = position(server, target, "target")?; + let pos = pos.0.map(|e| e.floor() as i32); + let new_block = server + .state + .get_block(pos) + // TODO: Make more principled. + .unwrap_or_else(|| Block::air(SpriteKind::Empty)) + .with_sprite(sk); + server.state.set_block(pos, new_block); + Ok(()) } else { - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandError, - format!("Invalid sprite kind: {}", sprite_name), - ), - ); + Err(format!("Invalid sprite kind: {}", sprite_name)) } } else { - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, action.help_string()), - ); + Err(action.help_string()) } } @@ -362,14 +418,15 @@ fn handle_motd( _target: EcsEntity, _args: String, _action: &ChatCommand, -) { +) -> CmdResult<()> { server.notify_client( client, ServerGeneral::server_msg( - ChatType::CommandError, + ChatType::CommandInfo, (*server.editable_settings().server_description).clone(), ), ); + Ok(()) } fn handle_set_motd( @@ -378,7 +435,7 @@ fn handle_set_motd( _target: EcsEntity, args: String, action: &ChatCommand, -) { +) -> CmdResult<()> { let data_dir = server.data_dir(); match scan_fmt!(&args, &action.arg_fmt(), String) { Ok(msg) => { @@ -389,7 +446,7 @@ fn handle_set_motd( server.notify_client( client, ServerGeneral::server_msg( - ChatType::CommandError, + ChatType::CommandInfo, format!("Server description set to \"{}\"", msg), ), ); @@ -402,142 +459,102 @@ fn handle_set_motd( server.notify_client( client, ServerGeneral::server_msg( - ChatType::CommandError, + ChatType::CommandInfo, "Removed server description".to_string(), ), ); }, } + Ok(()) } fn handle_jump( server: &mut Server, - client: EcsEntity, + _client: EcsEntity, target: EcsEntity, args: String, action: &ChatCommand, -) { +) -> CmdResult<()> { if let Ok((x, y, z)) = scan_fmt!(&args, &action.arg_fmt(), f32, f32, f32) { - match server.state.read_component_copied::(target) { - Some(current_pos) => { - server - .state - .write_component(target, comp::Pos(current_pos.0 + Vec3::new(x, y, z))); - server.state.write_component(target, comp::ForceUpdate); - }, - None => server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, "You have no position."), - ), - } + position_mut(server, target, "target", |current_pos| { + current_pos.0 += Vec3::new(x, y, z) + })?; + insert_or_replace_component(server, target, comp::ForceUpdate, "target") + } else { + Err(action.help_string()) } } fn handle_goto( server: &mut Server, - client: EcsEntity, + _client: EcsEntity, target: EcsEntity, args: String, action: &ChatCommand, -) { +) -> CmdResult<()> { if let Ok((x, y, z)) = scan_fmt!(&args, &action.arg_fmt(), f32, f32, f32) { - if server - .state - .read_component_copied::(target) - .is_some() - { - server - .state - .write_component(target, comp::Pos(Vec3::new(x, y, z))); - server.state.write_component(target, comp::ForceUpdate); - } else { - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, "You have no position."), - ); - } + position_mut(server, target, "target", |current_pos| { + current_pos.0 = Vec3::new(x, y, z) + })?; + insert_or_replace_component(server, target, comp::ForceUpdate, "target") } else { - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, action.help_string()), - ); + Err(action.help_string()) } } +/// TODO: Add autocompletion if possible (might require modifying enum to handle +/// dynamic values). fn handle_site( server: &mut Server, - client: EcsEntity, + _client: EcsEntity, target: EcsEntity, args: String, action: &ChatCommand, -) { +) -> CmdResult<()> { if let Ok(dest_name) = scan_fmt!(&args, &action.arg_fmt(), String) { - if server - .state - .read_component_copied::(target) - .is_some() - { - match server.world.civs().sites().find(|site| { + let site = server + .world + .civs() + .sites() + .find(|site| { site.site_tmp .map_or(false, |id| server.index.sites[id].name() == dest_name) - }) { - Some(site) => { - let pos = server - .world - .find_lowest_accessible_pos(server.index.as_index_ref(), site.center); - server.state.write_component(target, comp::Pos(pos)); - server.state.write_component(target, comp::ForceUpdate); - }, - None => { - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, "Site not found"), - ); - }, - }; - } else { - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, "You have no position."), - ); - } + }) + .ok_or_else(|| "Site not found".to_string())?; + + let site_pos = server + .world + .find_lowest_accessible_pos(server.index.as_index_ref(), site.center); + + position_mut(server, target, "target", |current_pos| { + current_pos.0 = site_pos + })?; + insert_or_replace_component(server, target, comp::ForceUpdate, "target") } else { - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, action.help_string()), - ); + Err(action.help_string()) } } fn handle_home( server: &mut Server, - client: EcsEntity, + _client: EcsEntity, target: EcsEntity, _args: String, _action: &ChatCommand, -) { - if server - .state - .read_component_copied::(target) - .is_some() - { - let home_pos = server.state.ecs().read_resource::().0; - let time = *server - .state - .ecs() - .read_resource::(); +) -> CmdResult<()> { + let home_pos = server.state.mut_resource::().0; + let time = *server.state.mut_resource::(); - server.state.write_component(target, comp::Pos(home_pos)); - server - .state - .write_component(target, comp::Waypoint::temp_new(home_pos, time)); - server.state.write_component(target, comp::ForceUpdate); - } else { - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, "You have no position."), - ); - } + position_mut(server, target, "target", |current_pos| { + current_pos.0 = home_pos + })?; + insert_or_replace_component( + server, + target, + comp::Waypoint::temp_new(home_pos, time), + "target", + )?; + insert_or_replace_component(server, target, comp::ForceUpdate, "target") } fn handle_kill( @@ -546,7 +563,7 @@ fn handle_kill( target: EcsEntity, _args: String, _action: &ChatCommand, -) { +) -> CmdResult<()> { let reason = if client == target { comp::HealthSource::Suicide } else if let Some(uid) = server.state.read_storage::().get(client) { @@ -563,6 +580,7 @@ fn handle_kill( .write_storage::() .get_mut(target) .map(|mut h| h.set_to(0, reason)); + Ok(()) } fn handle_time( @@ -571,10 +589,10 @@ fn handle_time( _target: EcsEntity, args: String, action: &ChatCommand, -) { +) -> CmdResult<()> { const DAY: u64 = 86400; - let time_in_seconds = server.state.ecs_mut().read_resource::().0; + let time_in_seconds = server.state.mut_resource::().0; let current_day = time_in_seconds as u64 / DAY; let day_start = (current_day * DAY) as f64; @@ -622,14 +640,7 @@ fn handle_time( // Absolute time (i.e: since in-game epoch) Some(n) => n as f64, None => { - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandError, - format!("'{}' is not a valid time.", n), - ), - ); - return; + return Err(format!("{:?} is not a valid time.", n)); }, }, }, @@ -687,11 +698,11 @@ fn handle_time( client, ServerGeneral::server_msg(ChatType::CommandInfo, msg), ); - return; + return Ok(()); }, }; - server.state.ecs_mut().write_resource::().0 = new_time; + server.state.mut_resource::().0 = new_time; if let Some(new_time) = NaiveTime::from_num_seconds_from_midnight_opt(((new_time as u64) % 86400) as u32, 0) @@ -704,15 +715,16 @@ fn handle_time( ), ); } + Ok(()) } fn handle_health( server: &mut Server, - client: EcsEntity, + _client: EcsEntity, target: EcsEntity, args: String, action: &ChatCommand, -) { +) -> CmdResult<()> { if let Ok(hp) = scan_fmt!(&args, &action.arg_fmt(), u32) { if let Some(mut health) = server .state @@ -721,17 +733,12 @@ fn handle_health( .get_mut(target) { health.set_to(hp * 10, comp::HealthSource::Command); + Ok(()) } else { - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, "You have no health."), - ); + Err("You have no health".into()) } } else { - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, "You must specify health amount!"), - ); + Err("You must specify health amount!".into()) } } @@ -741,24 +748,11 @@ fn handle_alias( target: EcsEntity, args: String, action: &ChatCommand, -) { - if client != target { - // Notify target that an admin changed the alias due to /sudo - server.notify_client( - target, - ServerGeneral::server_msg(ChatType::CommandInfo, "An admin changed your alias."), - ); - return; - } +) -> CmdResult<()> { if let Ok(alias) = scan_fmt!(&args, &action.arg_fmt(), String) { - if let Err(error) = comp::Player::alias_validate(&alias) { - // Prevent silly aliases - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, error.to_string()), - ); - return; - } + // Prevent silly aliases + comp::Player::alias_validate(&alias).map_err(|e| e.to_string())?; + let old_alias_optional = server .state .ecs_mut() @@ -787,11 +781,16 @@ fn handle_alias( )); } } + if client != target { + // Notify target that an admin changed the alias due to /sudo + server.notify_client( + target, + ServerGeneral::server_msg(ChatType::CommandInfo, "An admin changed your alias."), + ); + } + Ok(()) } else { - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, action.help_string()), - ); + Err(action.help_string()) } } @@ -801,56 +800,19 @@ fn handle_tp( target: EcsEntity, args: String, action: &ChatCommand, -) { - let opt_player = if let Some(alias) = scan_fmt_some!(&args, &action.arg_fmt(), String) { - let ecs = server.state.ecs(); - (&ecs.entities(), &ecs.read_storage::()) - .join() - .find(|(_, player)| player.alias == alias) - .map(|(entity, _)| entity) +) -> CmdResult<()> { + let player = if let Some(alias) = scan_fmt_some!(&args, &action.arg_fmt(), String) { + find_alias(server.state.ecs(), &alias)?.0 } else if client != target { - Some(client) + client } else { - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, "You must specify a player name"), - ); - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, action.help_string()), - ); - return; + return Err(action.help_string()); }; - if let Some(_pos) = server.state.read_component_copied::(target) { - if let Some(player) = opt_player { - if let Some(pos) = server.state.read_component_copied::(player) { - server.state.write_component(target, pos); - server.state.write_component(target, comp::ForceUpdate); - } else { - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandError, - "Unable to teleport to player!", - ), - ); - } - } else { - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, "Player not found!"), - ); - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, action.help_string()), - ); - } - } else { - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, "You have no position!"), - ); - } + let player_pos = position(server, player, "player")?; + position_mut(server, target, "target", |target_pos| { + *target_pos = player_pos + })?; + insert_or_replace_component(server, target, comp::ForceUpdate, "target") } fn handle_spawn( @@ -859,146 +821,111 @@ fn handle_spawn( target: EcsEntity, args: String, action: &ChatCommand, -) { - match scan_fmt_some!( - &args, - &action.arg_fmt(), - String, - npc::NpcBody, - String, - String - ) { +) -> CmdResult<()> { + match scan_fmt_some!(&args, &action.arg_fmt(), String, npc::NpcBody, u32, bool) { (Some(opt_align), Some(npc::NpcBody(id, mut body)), opt_amount, opt_ai) => { - let uid = server - .state - .read_component_copied(target) - .expect("Expected player to have a UID"); - if let Some(alignment) = parse_alignment(uid, &opt_align) { - let amount = opt_amount - .and_then(|a| a.parse().ok()) - .filter(|x| *x > 0) - .unwrap_or(1) - .min(50); + let uid = uid(server, target, "target")?; + let alignment = parse_alignment(uid, &opt_align)?; + let amount = opt_amount.filter(|x| *x > 0).unwrap_or(1).min(50); - let ai = opt_ai.unwrap_or_else(|| "true".to_string()); + let ai = opt_ai.unwrap_or(true); + let pos = position(server, target, "target")?; + let agent = if let comp::Alignment::Owned(_) | comp::Alignment::Npc = alignment { + comp::Agent::default() + } else { + comp::Agent::default().with_patrol_origin(pos.0) + }; - match server.state.read_component_copied::(target) { - Some(pos) => { - let agent = - if let comp::Alignment::Owned(_) | comp::Alignment::Npc = alignment { - comp::Agent::default() - } else { - comp::Agent::default().with_patrol_origin(pos.0) - }; + for _ in 0..amount { + let vel = Vec3::new( + rand::thread_rng().gen_range(-2.0..3.0), + rand::thread_rng().gen_range(-2.0..3.0), + 10.0, + ); - for _ in 0..amount { - let vel = Vec3::new( - rand::thread_rng().gen_range(-2.0..3.0), - rand::thread_rng().gen_range(-2.0..3.0), - 10.0, - ); + let body = body(); - let body = body(); + let loadout = LoadoutBuilder::build_loadout(body, None, None, None).build(); - let loadout = - LoadoutBuilder::build_loadout(body, None, None, None).build(); + let inventory = Inventory::new_with_loadout(loadout); - let inventory = Inventory::new_with_loadout(loadout); + let mut entity_base = server + .state + .create_npc( + pos, + comp::Stats::new(get_npc_name(id, npc::BodyType::from_body(body))), + comp::Health::new(body, 1), + comp::Poise::new(body), + inventory, + body, + ) + .with(comp::Vel(vel)) + .with(comp::MountState::Unmounted) + .with(alignment); - let mut entity_base = server - .state - .create_npc( - pos, - comp::Stats::new(get_npc_name( - id, - npc::BodyType::from_body(body), - )), - comp::Health::new(body, 1), - comp::Poise::new(body), - inventory, - body, - ) - .with(comp::Vel(vel)) - .with(comp::MountState::Unmounted) - .with(alignment); + if ai { + entity_base = entity_base.with(agent.clone()); + } - if ai == "true" { - entity_base = entity_base.with(agent.clone()); - } + let new_entity = entity_base.build(); - let new_entity = entity_base.build(); + // Add to group system if a pet + if matches!(alignment, comp::Alignment::Owned { .. }) { + let state = server.state(); + let clients = state.ecs().read_storage::(); + let uids = state.ecs().read_storage::(); + let mut group_manager = + state.ecs().write_resource::(); + group_manager.new_pet( + new_entity, + target, + &mut state.ecs().write_storage(), + &state.ecs().entities(), + &state.ecs().read_storage(), + &uids, + &mut |entity, group_change| { + clients + .get(entity) + .and_then(|c| { + group_change + .try_map(|e| uids.get(e).copied()) + .map(|g| (g, c)) + }) + .map(|(g, c)| { + c.send_fallible(ServerGeneral::GroupUpdate(g)); + }); + }, + ); + } else if let Some(group) = match alignment { + comp::Alignment::Wild => None, + comp::Alignment::Passive => None, + comp::Alignment::Enemy => Some(comp::group::ENEMY), + comp::Alignment::Npc | comp::Alignment::Tame => Some(comp::group::NPC), + comp::Alignment::Owned(_) => unreachable!(), + } { + insert_or_replace_component(server, new_entity, group, "new entity")?; + } - // Add to group system if a pet - if matches!(alignment, comp::Alignment::Owned { .. }) { - let state = server.state(); - let clients = state.ecs().read_storage::(); - let uids = state.ecs().read_storage::(); - let mut group_manager = - state.ecs().write_resource::(); - group_manager.new_pet( - new_entity, - target, - &mut state.ecs().write_storage(), - &state.ecs().entities(), - &state.ecs().read_storage(), - &uids, - &mut |entity, group_change| { - clients - .get(entity) - .and_then(|c| { - group_change - .try_map(|e| uids.get(e).copied()) - .map(|g| (g, c)) - }) - .map(|(g, c)| { - c.send_fallible(ServerGeneral::GroupUpdate(g)); - }); - }, - ); - } else if let Some(group) = match alignment { - comp::Alignment::Wild => None, - comp::Alignment::Passive => None, - comp::Alignment::Enemy => Some(comp::group::ENEMY), - comp::Alignment::Npc | comp::Alignment::Tame => { - Some(comp::group::NPC) - }, - comp::Alignment::Owned(_) => unreachable!(), - } { - let _ = - server.state.ecs().write_storage().insert(new_entity, group); - } - - if let Some(uid) = server.state.ecs().uid_from_entity(new_entity) { - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandInfo, - format!("Spawned entity with ID: {}", uid), - ), - ); - } - } - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandInfo, - format!("Spawned {} entities", amount), - ), - ); - }, - None => server.notify_client( + if let Some(uid) = server.state.ecs().uid_from_entity(new_entity) { + server.notify_client( client, - ServerGeneral::server_msg(ChatType::CommandError, "You have no position!"), - ), + ServerGeneral::server_msg( + ChatType::CommandInfo, + format!("Spawned entity with ID: {}", uid), + ), + ); } } - }, - _ => { server.notify_client( client, - ServerGeneral::server_msg(ChatType::CommandError, action.help_string()), + ServerGeneral::server_msg( + ChatType::CommandInfo, + format!("Spawned {} entities", amount), + ), ); + Ok(()) }, + _ => Err(action.help_string()), } } @@ -1008,39 +935,33 @@ fn handle_spawn_training_dummy( target: EcsEntity, _args: String, _action: &ChatCommand, -) { - match server.state.read_component_copied::(target) { - Some(pos) => { - let vel = Vec3::new( - rand::thread_rng().gen_range(-2.0..3.0), - rand::thread_rng().gen_range(-2.0..3.0), - 10.0, - ); +) -> CmdResult<()> { + let pos = position(server, target, "target")?; + let vel = Vec3::new( + rand::thread_rng().gen_range(-2.0..3.0), + rand::thread_rng().gen_range(-2.0..3.0), + 10.0, + ); - let body = comp::Body::Object(comp::object::Body::TrainingDummy); + let body = comp::Body::Object(comp::object::Body::TrainingDummy); - let stats = comp::Stats::new("Training Dummy".to_string()); + let stats = comp::Stats::new("Training Dummy".to_string()); - let health = comp::Health::new(body, 0); - let poise = comp::Poise::new(body); + let health = comp::Health::new(body, 0); + let poise = comp::Poise::new(body); - server - .state - .create_npc(pos, stats, health, poise, Inventory::new_empty(), body) - .with(comp::Vel(vel)) - .with(comp::MountState::Unmounted) - .build(); + server + .state + .create_npc(pos, stats, health, poise, Inventory::new_empty(), body) + .with(comp::Vel(vel)) + .with(comp::MountState::Unmounted) + .build(); - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandInfo, "Spawned a training dummy"), - ); - }, - None => server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, "You have no position!"), - ), - } + server.notify_client( + client, + ServerGeneral::server_msg(ChatType::CommandInfo, "Spawned a training dummy"), + ); + Ok(()) } fn handle_spawn_airship( @@ -1049,45 +970,39 @@ fn handle_spawn_airship( target: EcsEntity, args: String, action: &ChatCommand, -) { +) -> CmdResult<()> { let angle = scan_fmt!(&args, &action.arg_fmt(), f32).ok(); - match server.state.read_component_copied::(target) { - Some(mut pos) => { - pos.0.z += 50.0; - const DESTINATION_RADIUS: f32 = 2000.0; - let angle = angle.map(|a| a * std::f32::consts::PI / 180.0); - let destination = angle.map(|a| { - pos.0 - + Vec3::new( - DESTINATION_RADIUS * a.cos(), - DESTINATION_RADIUS * a.sin(), - 200.0, - ) - }); - let mut builder = server - .state - .create_ship(pos, comp::ship::Body::DefaultAirship, true) - .with(LightEmitter { - col: Rgb::new(1.0, 0.65, 0.2), - strength: 2.0, - flicker: 1.0, - animated: true, - }); - if let Some(pos) = destination { - builder = builder.with(comp::Agent::default().with_destination(pos)) - } - builder.build(); - - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandInfo, "Spawned an airship"), - ); - }, - None => server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, "You have no position!"), - ), + let mut pos = position(server, target, "target")?; + pos.0.z += 50.0; + const DESTINATION_RADIUS: f32 = 2000.0; + let angle = angle.map(|a| a * std::f32::consts::PI / 180.0); + let destination = angle.map(|a| { + pos.0 + + Vec3::new( + DESTINATION_RADIUS * a.cos(), + DESTINATION_RADIUS * a.sin(), + 200.0, + ) + }); + let mut builder = server + .state + .create_ship(pos, comp::ship::Body::DefaultAirship, true) + .with(LightEmitter { + col: Rgb::new(1.0, 0.65, 0.2), + strength: 2.0, + flicker: 1.0, + animated: true, + }); + if let Some(pos) = destination { + builder = builder.with(comp::Agent::default().with_destination(pos)) } + builder.build(); + + server.notify_client( + client, + ServerGeneral::server_msg(ChatType::CommandInfo, "Spawned an airship"), + ); + Ok(()) } fn handle_spawn_campfire( @@ -1096,42 +1011,36 @@ fn handle_spawn_campfire( target: EcsEntity, _args: String, _action: &ChatCommand, -) { - match server.state.read_component_copied::(target) { - Some(pos) => { - server - .state - .create_object(pos, comp::object::Body::CampfireLit) - .with(LightEmitter { - col: Rgb::new(1.0, 0.65, 0.2), - strength: 2.0, - flicker: 1.0, - animated: true, - }) - .with(WaypointArea::default()) - .with(comp::Auras::new(vec![Aura::new( - AuraKind::Buff { - kind: BuffKind::CampfireHeal, - data: BuffData::new(0.02, Some(Duration::from_secs(1))), - category: BuffCategory::Natural, - source: BuffSource::World, - }, - 5.0, - None, - AuraTarget::All, - )])) - .build(); +) -> CmdResult<()> { + let pos = position(server, target, "target")?; + server + .state + .create_object(pos, comp::object::Body::CampfireLit) + .with(LightEmitter { + col: Rgb::new(1.0, 0.65, 0.2), + strength: 2.0, + flicker: 1.0, + animated: true, + }) + .with(WaypointArea::default()) + .with(comp::Auras::new(vec![Aura::new( + AuraKind::Buff { + kind: BuffKind::CampfireHeal, + data: BuffData::new(0.02, Some(Duration::from_secs(1))), + category: BuffCategory::Natural, + source: BuffSource::World, + }, + 5.0, + None, + AuraTarget::All, + )])) + .build(); - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandInfo, "Spawned a campfire"), - ); - }, - None => server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, "You have no position!"), - ), - } + server.notify_client( + client, + ServerGeneral::server_msg(ChatType::CommandInfo, "Spawned a campfire"), + ); + Ok(()) } fn handle_safezone( @@ -1140,38 +1049,31 @@ fn handle_safezone( target: EcsEntity, args: String, action: &ChatCommand, -) { +) -> CmdResult<()> { let range = scan_fmt_some!(&args, &action.arg_fmt(), f32); + let pos = position(server, target, "target")?; + server + .state + .create_object(pos, comp::object::Body::BoltNature) + .with(comp::Mass(10_f32.powi(10))) + .with(comp::Auras::new(vec![Aura::new( + AuraKind::Buff { + kind: BuffKind::Invulnerability, + data: BuffData::new(1.0, Some(Duration::from_secs(1))), + category: BuffCategory::Natural, + source: BuffSource::World, + }, + range.unwrap_or(100.0), + None, + AuraTarget::All, + )])) + .build(); - match server.state.read_component_copied::(target) { - Some(pos) => { - server - .state - .create_object(pos, comp::object::Body::BoltNature) - .with(comp::Mass(10_f32.powi(10))) - .with(comp::Auras::new(vec![Aura::new( - AuraKind::Buff { - kind: BuffKind::Invulnerability, - data: BuffData::new(1.0, Some(Duration::from_secs(1))), - category: BuffCategory::Natural, - source: BuffSource::World, - }, - range.unwrap_or(100.0), - None, - AuraTarget::All, - )])) - .build(); - - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandInfo, "Spawned a safe zone"), - ); - }, - None => server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, "You have no position!"), - ), - } + server.notify_client( + client, + ServerGeneral::server_msg(ChatType::CommandInfo, "Spawned a safe zone"), + ); + Ok(()) } fn handle_permit_build( @@ -1180,45 +1082,38 @@ fn handle_permit_build( target: EcsEntity, args: String, action: &ChatCommand, -) { +) -> CmdResult<()> { if let Some(area_name) = scan_fmt_some!(&args, &action.arg_fmt(), String) { - let ecs = server.state.ecs(); - if server - .state - .read_storage::() - .get(target) - .is_none() - { - let _ = ecs - .write_storage::() - .insert(target, comp::CanBuild { - enabled: false, - build_areas: HashSet::new(), - }); + let bb_id = area(server, &area_name)?; + let mut can_build = server.state.ecs().write_storage::(); + let entry = can_build + .entry(target) + .map_err(|_| "Cannot find target entity!".to_string())?; + let mut comp_can_build = entry.or_insert(comp::CanBuild { + enabled: false, + build_areas: HashSet::new(), + }); + comp_can_build.build_areas.insert(bb_id); + drop(can_build); + if client != target { + server.notify_client( + target, + ServerGeneral::server_msg( + ChatType::CommandInfo, + format!("You are now permitted to build in {}", area_name), + ), + ); } - if let Some(bb_id) = ecs - .read_resource::() - .deref() - .area_names - .get(&area_name) - { - if let Some(mut comp_can_build) = ecs.write_storage::().get_mut(target) - { - comp_can_build.build_areas.insert(*bb_id); - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandInfo, - format!("Permission to build in {} granted", area_name), - ), - ); - } - } - } else { server.notify_client( client, - ServerGeneral::server_msg(ChatType::CommandError, action.help_string()), + ServerGeneral::server_msg( + ChatType::CommandInfo, + format!("Permission to build in {} granted", area_name), + ), ); + Ok(()) + } else { + Err(action.help_string()) } } @@ -1228,32 +1123,35 @@ fn handle_revoke_build( target: EcsEntity, args: String, action: &ChatCommand, -) { +) -> CmdResult<()> { if let Some(area_name) = scan_fmt_some!(&args, &action.arg_fmt(), String) { - let ecs = server.state.ecs(); - if let Some(bb_id) = ecs - .read_resource::() - .deref() - .area_names - .get(&area_name) - { - if let Some(mut comp_can_build) = ecs.write_storage::().get_mut(target) - { - comp_can_build.build_areas.retain(|&x| x != *bb_id); + let bb_id = area(server, &area_name)?; + let mut can_build = server.state.ecs_mut().write_storage::(); + if let Some(mut comp_can_build) = can_build.get_mut(target) { + comp_can_build.build_areas.retain(|&x| x != bb_id); + drop(can_build); + if client != target { server.notify_client( - client, + target, ServerGeneral::server_msg( ChatType::CommandInfo, - format!("Permission to build in {} revoked", area_name), + format!("Your permission to build in {} has been revoked", area_name), ), ); } + server.notify_client( + client, + ServerGeneral::server_msg( + ChatType::CommandInfo, + format!("Permission to build in {} revoked", area_name), + ), + ); + Ok(()) + } else { + Err("You do not have permission to build.".into()) } } else { - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, action.help_string()), - ); + Err(action.help_string()) } } @@ -1263,14 +1161,24 @@ fn handle_revoke_build_all( target: EcsEntity, _args: String, _action: &ChatCommand, -) { +) -> CmdResult<()> { let ecs = server.state.ecs(); ecs.write_storage::().remove(target); + if client != target { + server.notify_client( + target, + ServerGeneral::server_msg( + ChatType::CommandInfo, + "Your build permissions have been revoked.", + ), + ); + } server.notify_client( client, ServerGeneral::server_msg(ChatType::CommandInfo, "All build permissions revoked"), ); + Ok(()) } fn handle_players( @@ -1279,7 +1187,7 @@ fn handle_players( _target: EcsEntity, _args: String, _action: &ChatCommand, -) { +) -> CmdResult<()> { let ecs = server.state.ecs(); let entity_tuples = ( @@ -1298,6 +1206,7 @@ fn handle_players( ), ), ); + Ok(()) } fn handle_build( @@ -1306,34 +1215,26 @@ fn handle_build( target: EcsEntity, _args: String, _action: &ChatCommand, -) { +) -> CmdResult<()> { if let Some(mut can_build) = server .state .ecs() .write_storage::() .get_mut(target) { - if can_build.enabled { - can_build.enabled = false; - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandInfo, "Toggled off build mode!"), - ); - } else { - can_build.enabled = true; - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandInfo, "Toggled on build mode!"), - ); - } - } else { - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandInfo, - "You do not have permission to build.", - ), + let toggle_string = if can_build.enabled { "off" } else { "on" }; + let chat_msg = ServerGeneral::server_msg( + ChatType::CommandInfo, + format!("Toggled {:?} build mode!", toggle_string), ); + can_build.enabled ^= true; + if client != target { + server.notify_client(target, chat_msg.clone()); + } + server.notify_client(client, chat_msg); + Ok(()) + } else { + Err("You do not have permission to build.".into()) } } @@ -1343,7 +1244,7 @@ fn handle_build_area_add( _target: EcsEntity, args: String, action: &ChatCommand, -) { +) -> CmdResult<()> { if let (Some(area_name), Some(xlo), Some(xhi), Some(ylo), Some(yhi), Some(zlo), Some(zhi)) = scan_fmt_some!( &args, &action.arg_fmt(), @@ -1355,40 +1256,21 @@ fn handle_build_area_add( i32, i32 ) { - let ecs = server.state.ecs(); - if ecs - .read_resource::() - .deref() - .area_names - .contains_key(&area_name) - { - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandError, - format!("Build zone {} already exists!", area_name), - ), - ); - return; - } - let bb_id = ecs.write_resource::().deref_mut().areas.insert( - Aabb { + let build_areas = server.state.mut_resource::(); + let msg = ServerGeneral::server_msg( + ChatType::CommandInfo, + format!("Created build zone {}", area_name), + ); + build_areas + .insert(area_name, Aabb { min: Vec3::new(xlo, ylo, zlo), max: Vec3::new(xhi, yhi, zhi), - } - .made_valid(), - ); - ecs.write_resource::() - .deref_mut() - .area_names - .insert(area_name.clone(), bb_id); - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandInfo, - format!("Created build zone {}", area_name), - ), - ); + }) + .map_err(|area_name| format!("Build zone {} already exists!", area_name))?; + server.notify_client(client, msg); + Ok(()) + } else { + Err(action.help_string()) } } @@ -1398,26 +1280,24 @@ fn handle_build_area_list( _target: EcsEntity, _args: String, _action: &ChatCommand, -) { - let ecs = server.state.ecs(); - let build_areas = ecs.read_resource::(); - - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandInfo, - build_areas.area_names.iter().fold( - "Build Areas:".to_string(), - |acc, (area_name, bb_id)| { - if let Some(aabb) = build_areas.areas.get(*bb_id) { - format!("{}\n{}: {} to {}", acc, area_name, aabb.min, aabb.max) - } else { - acc - } - }, - ), +) -> CmdResult<()> { + let build_areas = server.state.mut_resource::(); + let msg = ServerGeneral::server_msg( + ChatType::CommandInfo, + build_areas.area_names().iter().fold( + "Build Areas:".to_string(), + |acc, (area_name, bb_id)| { + if let Some(aabb) = build_areas.areas().get(*bb_id) { + format!("{}\n{}: {} to {}", acc, area_name, aabb.min, aabb.max) + } else { + acc + } + }, ), ); + + server.notify_client(client, msg); + Ok(()) } fn handle_build_area_remove( @@ -1426,27 +1306,17 @@ fn handle_build_area_remove( _target: EcsEntity, args: String, action: &ChatCommand, -) { +) -> CmdResult<()> { if let Some(area_name) = scan_fmt_some!(&args, &action.arg_fmt(), String) { - let ecs = server.state.ecs(); - let mut build_areas = ecs.write_resource::(); + let build_areas = server.state.mut_resource::(); - let bb_id = match build_areas.area_names.get(&area_name) { - Some(x) => *x, - None => { - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandError, - format!("No such build area '{}'", area_name), - ), - ); - return; - }, - }; - - let _ = build_areas.area_names.remove(&area_name); - let _ = build_areas.areas.remove(bb_id); + build_areas.remove(&area_name).map_err(|err| match err { + BuildAreaError::Reserved => format!( + "Build area is reserved and cannot be removed: {}", + area_name + ), + BuildAreaError::NotFound => format!("No such build area {}", area_name), + })?; server.notify_client( client, ServerGeneral::server_msg( @@ -1454,6 +1324,9 @@ fn handle_build_area_remove( format!("Removed build zone {}", area_name), ), ); + Ok(()) + } else { + Err(action.help_string()) } } @@ -1463,12 +1336,12 @@ fn handle_help( _target: EcsEntity, args: String, action: &ChatCommand, -) { +) -> CmdResult<()> { if let Some(cmd) = scan_fmt_some!(&args, &action.arg_fmt(), ChatCommand) { server.notify_client( client, ServerGeneral::server_msg(ChatType::CommandInfo, cmd.help_string()), - ); + ) } else { let mut message = String::new(); for cmd in CHAT_COMMANDS.iter() { @@ -1484,17 +1357,18 @@ fn handle_help( server.notify_client( client, ServerGeneral::server_msg(ChatType::CommandInfo, message), - ); + ) } + Ok(()) } -fn parse_alignment(owner: Uid, alignment: &str) -> Option { +fn parse_alignment(owner: Uid, alignment: &str) -> CmdResult { match alignment { - "wild" => Some(comp::Alignment::Wild), - "enemy" => Some(comp::Alignment::Enemy), - "npc" => Some(comp::Alignment::Npc), - "pet" => Some(comp::Alignment::Owned(owner)), - _ => None, + "wild" => Ok(comp::Alignment::Wild), + "enemy" => Ok(comp::Alignment::Enemy), + "npc" => Ok(comp::Alignment::Npc), + "pet" => Ok(comp::Alignment::Owned(owner)), + _ => Err(format!("Invalid alignment: {:?}", alignment)), } } @@ -1504,7 +1378,7 @@ fn handle_kill_npcs( _target: EcsEntity, _args: String, _action: &ChatCommand, -) { +) -> CmdResult<()> { let ecs = server.state.ecs(); let mut healths = ecs.write_storage::(); let players = ecs.read_storage::(); @@ -1522,6 +1396,7 @@ fn handle_kill_npcs( client, ServerGeneral::server_msg(ChatType::CommandInfo, text), ); + Ok(()) } #[allow(clippy::float_cmp)] // TODO: Pending review in #587 @@ -1533,69 +1408,56 @@ fn handle_object( target: EcsEntity, args: String, action: &ChatCommand, -) { +) -> CmdResult<()> { let obj_type = scan_fmt!(&args, &action.arg_fmt(), String); - let pos = server - .state - .ecs() - .read_storage::() - .get(target) - .copied(); + let pos = position(server, target, "target")?; let ori = server .state .ecs() .read_storage::() .get(target) - .copied(); + .copied() + .ok_or_else(|| "Cannot get orientation for target".to_string())?; /*let builder = server.state .create_object(pos, ori, obj_type) .with(ori);*/ - if let (Some(pos), Some(ori)) = (pos, ori) { - let obj_str_res = obj_type.as_ref().map(String::as_str); - if let Some(obj_type) = comp::object::ALL_OBJECTS - .iter() - .find(|o| Ok(o.to_string()) == obj_str_res) - { - server - .state - .create_object(pos, *obj_type) - .with( - comp::Ori::from_unnormalized_vec( - // converts player orientation into a 90° rotation for the object by using - // the axis with the highest value - { - let look_dir = ori.look_dir(); - look_dir.map(|e| { - if e.abs() == look_dir.map(|e| e.abs()).reduce_partial_max() { - e - } else { - 0.0 - } - }) - }, - ) - .unwrap_or_default(), + let obj_str_res = obj_type.as_ref().map(String::as_str); + if let Some(obj_type) = comp::object::ALL_OBJECTS + .iter() + .find(|o| Ok(o.to_string()) == obj_str_res) + { + server + .state + .create_object(pos, *obj_type) + .with( + comp::Ori::from_unnormalized_vec( + // converts player orientation into a 90° rotation for the object by using + // the axis with the highest value + { + let look_dir = ori.look_dir(); + look_dir.map(|e| { + if e.abs() == look_dir.map(|e| e.abs()).reduce_partial_max() { + e + } else { + 0.0 + } + }) + }, ) - .build(); - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandInfo, - format!("Spawned: {}", obj_str_res.unwrap_or("")), - ), - ); - } else { - return server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, "Object not found!"), - ); - } - } else { + .unwrap_or_default(), + ) + .build(); server.notify_client( client, - ServerGeneral::server_msg(ChatType::CommandError, "You have no position!"), + ServerGeneral::server_msg( + ChatType::CommandInfo, + format!("Spawned: {}", obj_str_res.unwrap_or("")), + ), ); + Ok(()) + } else { + Err("Object not found!".into()) } } @@ -1606,7 +1468,7 @@ fn handle_light( target: EcsEntity, args: String, action: &ChatCommand, -) { +) -> CmdResult<()> { let (opt_r, opt_g, opt_b, opt_x, opt_y, opt_z, opt_s) = scan_fmt_some!(&args, &action.arg_fmt(), f32, f32, f32, f32, f32, f32, f32); @@ -1615,14 +1477,7 @@ fn handle_light( if let (Some(r), Some(g), Some(b)) = (opt_r, opt_g, opt_b) { if r < 0.0 || g < 0.0 || b < 0.0 { - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandError, - "cr, cg and cb values mustn't be negative.", - ), - ); - return; + return Err("cr, cg and cb values mustn't be negative.".into()); } let r = r.max(0.0).min(1.0); @@ -1640,35 +1495,24 @@ fn handle_light( if let Some(s) = opt_s { light_emitter.strength = s.max(0.0) }; - let pos = server + let pos = position(server, target, "target")?; + let builder = server .state - .ecs() - .read_storage::() - .get(target) - .copied(); - if let Some(pos) = pos { - let builder = server - .state - .ecs_mut() - .create_entity_synced() - .with(pos) - .with(comp::ForceUpdate) - .with(light_emitter); - if let Some(light_offset) = light_offset_opt { - builder.with(light_offset).build(); - } else { - builder.build(); - } - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandInfo, "Spawned object."), - ); + .ecs_mut() + .create_entity_synced() + .with(pos) + .with(comp::ForceUpdate) + .with(light_emitter); + if let Some(light_offset) = light_offset_opt { + builder.with(light_offset).build(); } else { - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, "You have no position!"), - ); + builder.build(); } + server.notify_client( + client, + ServerGeneral::server_msg(ChatType::CommandInfo, "Spawned object."), + ); + Ok(()) } #[allow(clippy::useless_conversion)] // TODO: Pending review in #587 @@ -1678,7 +1522,7 @@ fn handle_lantern( target: EcsEntity, args: String, action: &ChatCommand, -) { +) -> CmdResult<()> { if let (Some(s), r, g, b) = scan_fmt_some!(&args, &action.arg_fmt(), f32, f32, f32, f32) { if let Some(mut light) = server .state @@ -1700,7 +1544,7 @@ fn handle_lantern( ChatType::CommandInfo, "You adjusted flame strength and color.", ), - ); + ) } else { server.notify_client( client, @@ -1708,77 +1552,67 @@ fn handle_lantern( ChatType::CommandInfo, "You adjusted flame strength.", ), - ); + ) } + Ok(()) } else { - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, "Please equip a lantern first"), - ); + Err("Please equip a lantern first".into()) } } else { - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, action.help_string()), - ); + Err(action.help_string()) } } fn handle_explosion( server: &mut Server, - client: EcsEntity, + _client: EcsEntity, target: EcsEntity, args: String, action: &ChatCommand, -) { +) -> CmdResult<()> { let power = scan_fmt!(&args, &action.arg_fmt(), f32).unwrap_or(8.0); - if power > 512.0 { - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandError, - "Explosion power mustn't be more than 512.", - ), - ); - return; + const MIN_POWER: f32 = 0.0; + const MAX_POWER: f32 = 512.0; + + if power > MAX_POWER { + return Err(format!( + "Explosion power mustn't be more than {:?}.", + MAX_POWER + )); } else if power <= 0.0 { - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandError, - "Explosion power must be more than 0.", - ), - ); - return; + return Err(format!( + "Explosion power must be more than {:?}.", + MIN_POWER + )); } - let ecs = server.state.ecs(); - - match server.state.read_component_copied::(target) { - Some(pos) => { - ecs.read_resource::>() - .emit_now(ServerEvent::Explosion { - pos: pos.0, - explosion: Explosion { - effects: vec![ - RadiusEffect::Entity(Effect::Damage(Damage { - source: DamageSource::Explosion, - value: 100.0 * power, - })), - RadiusEffect::TerrainDestruction(power), - ], - radius: 3.0 * power, - reagent: None, - }, - owner: ecs.read_storage::().get(target).copied(), - }) - }, - None => server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, "You have no position!"), - ), - } + let pos = position(server, target, "target")?; + let owner = server + .state + .ecs() + .read_storage::() + .get(target) + .copied(); + server + .state + .mut_resource::>() + .emit_now(ServerEvent::Explosion { + pos: pos.0, + explosion: Explosion { + effects: vec![ + RadiusEffect::Entity(Effect::Damage(Damage { + source: DamageSource::Explosion, + value: 100.0 * power, + })), + RadiusEffect::TerrainDestruction(power), + ], + radius: 3.0 * power, + reagent: None, + }, + owner, + }); + Ok(()) } fn handle_waypoint( @@ -1787,81 +1621,67 @@ fn handle_waypoint( target: EcsEntity, _args: String, _action: &ChatCommand, -) { - match server.state.read_component_copied::(target) { - Some(pos) => { - let time = server.state.ecs().read_resource(); - let _ = server - .state - .ecs() - .write_storage::() - .insert(target, comp::Waypoint::temp_new(pos.0, *time)); - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandInfo, "Waypoint saved!"), - ); - server.notify_client( - client, - ServerGeneral::Notification(Notification::WaypointSaved), - ); - }, - None => server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, "You have no position!"), - ), - } +) -> CmdResult<()> { + let pos = position(server, target, "target")?; + let time = *server.state.mut_resource::(); + insert_or_replace_component( + server, + target, + comp::Waypoint::temp_new(pos.0, time), + "target", + )?; + server.notify_client( + client, + ServerGeneral::server_msg(ChatType::CommandInfo, "Waypoint saved!"), + ); + server.notify_client( + target, + ServerGeneral::Notification(Notification::WaypointSaved), + ); + Ok(()) } #[allow(clippy::useless_conversion)] // TODO: Pending review in #587 fn handle_adminify( server: &mut Server, - client: EcsEntity, + _client: EcsEntity, _target: EcsEntity, args: String, action: &ChatCommand, -) { +) -> CmdResult<()> { if let Ok(alias) = scan_fmt!(&args, &action.arg_fmt(), String) { - let ecs = server.state.ecs(); - let opt_player = (&ecs.entities(), &ecs.read_storage::()) - .join() - .find(|(_, player)| alias == player.alias) - .map(|(entity, _)| entity); - match opt_player { - Some(player) => { - let is_admin = if server - .state - .read_component_copied::(player) - .is_some() - { - ecs.write_storage::().remove(player); - false - } else { - ecs.write_storage().insert(player, comp::Admin).is_ok() - }; - // Update player list so the player shows up as admin in client chat. - let msg = ServerGeneral::PlayerListUpdate(PlayerListUpdate::Admin( - *ecs.read_storage::() - .get(player) - .expect("Player should have uid"), - is_admin, - )); - server.state.notify_players(msg); - }, - None => { - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandError, - format!("Player '{}' not found!", alias), - ), - ); - }, - } + let (player, uuid) = find_alias(server.state.ecs(), &alias)?; + let uid = uid(server, player, "player")?; + verify_not_hardcoded_admin( + server, + uuid, + "Admins specified in server configuration files cannot be de-adminified.", + )?; + let is_admin = if server + .state + .read_component_copied::(player) + .is_some() + { + server + .state + .ecs() + .write_storage::() + .remove(player); + false + } else { + server + .state + .ecs() + .write_storage() + .insert(player, comp::Admin) + .is_ok() + }; + // Update player list so the player shows up as admin in client chat. + let msg = ServerGeneral::PlayerListUpdate(PlayerListUpdate::Admin(uid, is_admin)); + server.state.notify_players(msg); + Ok(()) } else { - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, action.help_string()), - ); + Err(action.help_string()) } } @@ -1873,60 +1693,26 @@ fn handle_tell( target: EcsEntity, args: String, action: &ChatCommand, -) { - if client != target { - // This happens when [ab]using /sudo - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, "It's rude to impersonate people"), - ); - return; - } +) -> CmdResult<()> { + no_sudo(client, target)?; + if let (Some(alias), message_opt) = scan_fmt_some!(&args, &action.arg_fmt(), String, String) { let ecs = server.state.ecs(); - if let Some(player) = (&ecs.entities(), &ecs.read_storage::()) - .join() - .find(|(_, player)| player.alias == alias) - .map(|(entity, _)| entity) - { - if player == client { - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, "You can't /tell yourself."), - ); - return; - } - let client_uid = *ecs - .read_storage() - .get(client) - .expect("Player must have uid"); - let player_uid = *ecs - .read_storage() - .get(player) - .expect("Player must have uid"); - let mode = comp::ChatMode::Tell(player_uid); - let _ = server - .state - .ecs() - .write_storage() - .insert(client, mode.clone()); - let msg = message_opt.unwrap_or_else(|| format!("{} wants to talk to you.", alias)); - server.state.send_chat(mode.new_message(client_uid, msg)); - server.notify_client(client, ServerGeneral::ChatMode(mode)); - } else { - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandError, - format!("Player '{}' not found!", alias), - ), - ); + let player = find_alias(ecs, &alias)?.0; + + if player == target { + return Err("You can't /tell yourself.".into()); } + let target_uid = uid(server, target, "target")?; + let player_uid = uid(server, player, "player")?; + let mode = comp::ChatMode::Tell(player_uid); + insert_or_replace_component(server, target, mode.clone(), "target")?; + let msg = message_opt.unwrap_or_else(|| format!("{} wants to talk to you.", alias)); + server.state.send_chat(mode.new_message(target_uid, msg)); + server.notify_client(target, ServerGeneral::ChatMode(mode)); + Ok(()) } else { - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, action.help_string()), - ); + Err(action.help_string()) } } @@ -1936,33 +1722,23 @@ fn handle_faction( target: EcsEntity, msg: String, _action: &ChatCommand, -) { - if client != target { - // This happens when [ab]using /sudo - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, "It's rude to impersonate people"), - ); - return; - } - let ecs = server.state.ecs(); - if let Some(comp::Faction(faction)) = ecs.read_storage().get(client) { +) -> CmdResult<()> { + no_sudo(client, target)?; + + let factions = server.state.ecs().read_storage(); + if let Some(comp::Faction(faction)) = factions.get(target) { let mode = comp::ChatMode::Faction(faction.to_string()); - let _ = ecs.write_storage().insert(client, mode.clone()); + drop(factions); + insert_or_replace_component(server, target, mode.clone(), "target")?; if !msg.is_empty() { - if let Some(uid) = ecs.read_storage().get(client) { + if let Some(uid) = server.state.ecs().read_storage().get(target) { server.state.send_chat(mode.new_message(*uid, msg)); } } - server.notify_client(client, ServerGeneral::ChatMode(mode)); + server.notify_client(target, ServerGeneral::ChatMode(mode)); + Ok(()) } else { - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandError, - "Please join a faction with /join_faction", - ), - ); + Err("Please join a faction with /join_faction".into()) } } @@ -1972,174 +1748,123 @@ fn handle_group( target: EcsEntity, msg: String, _action: &ChatCommand, -) { - if client != target { - // This happens when [ab]using /sudo - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, "It's rude to impersonate people"), - ); - return; - } - let ecs = server.state.ecs(); - if let Some(group) = ecs.read_storage::().get(client) { +) -> CmdResult<()> { + no_sudo(client, target)?; + + let groups = server.state.ecs().read_storage::(); + if let Some(group) = groups.get(target) { let mode = comp::ChatMode::Group(*group); - let _ = ecs.write_storage().insert(client, mode.clone()); + drop(groups); + insert_or_replace_component(server, target, mode.clone(), "target")?; if !msg.is_empty() { - if let Some(uid) = ecs.read_storage().get(client) { + if let Some(uid) = server.state.ecs().read_storage().get(target) { server.state.send_chat(mode.new_message(*uid, msg)); } } - server.notify_client(client, ServerGeneral::ChatMode(mode)); + server.notify_client(target, ServerGeneral::ChatMode(mode)); + Ok(()) } else { - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, "Please create a group first"), - ); + Err("Please create a group first".into()) } } fn handle_group_invite( server: &mut Server, client: EcsEntity, - _target: EcsEntity, + target: EcsEntity, args: String, action: &ChatCommand, -) { +) -> CmdResult<()> { if let Some(target_alias) = scan_fmt_some!(&args, &action.arg_fmt(), String) { - let ecs = server.state.ecs(); - let target_player_opt = (&ecs.entities(), &ecs.read_storage::()) - .join() - .find(|(_, player)| player.alias == target_alias) - .map(|(entity, _)| entity); + let target_player = find_alias(server.state.ecs(), &target_alias)?.0; + let uid = uid(server, target_player, "player")?; - if let Some(target_player) = target_player_opt { - let uid = *ecs - .read_storage::() - .get(target_player) - .expect("Failed to get uid for player"); - - ecs.read_resource::>() - .emit_now(ServerEvent::InitiateInvite(client, uid, InviteKind::Group)); + server + .state + .mut_resource::>() + .emit_now(ServerEvent::InitiateInvite(target, uid, InviteKind::Group)); + if client != target { server.notify_client( - client, + target, ServerGeneral::server_msg( ChatType::CommandInfo, - format!("Invited {} to the group.", target_alias), + format!("{} has been invited to your group.", target_alias), ), ); - } else { - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandError, - format!("Player with alias {} not found", target_alias), - ), - ) } - } else { + server.notify_client( client, - ServerGeneral::server_msg(ChatType::CommandError, action.help_string()), + ServerGeneral::server_msg( + ChatType::CommandInfo, + format!("Invited {} to the group.", target_alias), + ), ); + Ok(()) + } else { + Err(action.help_string()) } } fn handle_group_kick( server: &mut Server, - client: EcsEntity, - _target: EcsEntity, + _client: EcsEntity, + target: EcsEntity, args: String, action: &ChatCommand, -) { +) -> CmdResult<()> { // Checking if leader is already done in group_manip if let Some(target_alias) = scan_fmt_some!(&args, &action.arg_fmt(), String) { - let ecs = server.state.ecs(); - let target_player_opt = (&ecs.entities(), &ecs.read_storage::()) - .join() - .find(|(_, player)| player.alias == target_alias) - .map(|(entity, _)| entity); + let target_player = find_alias(server.state.ecs(), &target_alias)?.0; + let uid = uid(server, target_player, "player")?; - if let Some(target_player) = target_player_opt { - let uid = *ecs - .read_storage::() - .get(target_player) - .expect("Failed to get uid for player"); - - ecs.read_resource::>() - .emit_now(ServerEvent::GroupManip(client, comp::GroupManip::Kick(uid))); - } else { - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandError, - format!("Player with alias {} not found", target_alias), - ), - ) - } + server + .state + .mut_resource::>() + .emit_now(ServerEvent::GroupManip(target, comp::GroupManip::Kick(uid))); + Ok(()) } else { - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, action.help_string()), - ); + Err(action.help_string()) } } fn handle_group_leave( server: &mut Server, - client: EcsEntity, - _target: EcsEntity, + _client: EcsEntity, + target: EcsEntity, _args: String, _action: &ChatCommand, -) { +) -> CmdResult<()> { server .state - .ecs() - .read_resource::>() - .emit_now(ServerEvent::GroupManip(client, comp::GroupManip::Leave)); + .mut_resource::>() + .emit_now(ServerEvent::GroupManip(target, comp::GroupManip::Leave)); + Ok(()) } fn handle_group_promote( server: &mut Server, - client: EcsEntity, - _target: EcsEntity, + _client: EcsEntity, + target: EcsEntity, args: String, action: &ChatCommand, -) { +) -> CmdResult<()> { // Checking if leader is already done in group_manip if let Some(target_alias) = scan_fmt_some!(&args, &action.arg_fmt(), String) { - let ecs = server.state.ecs(); - let target_player_opt = (&ecs.entities(), &ecs.read_storage::()) - .join() - .find(|(_, player)| player.alias == target_alias) - .map(|(entity, _)| entity); + let target_player = find_alias(server.state.ecs(), &target_alias)?.0; + let uid = uid(server, target_player, "player")?; - if let Some(target_player) = target_player_opt { - let uid = *ecs - .read_storage::() - .get(target_player) - .expect("Failed to get uid for player"); - - ecs.read_resource::>() - .emit_now(ServerEvent::GroupManip( - client, - comp::GroupManip::AssignLeader(uid), - )); - } else { - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandError, - format!("Player with alias {} not found", target_alias), - ), - ) - } + server + .state + .mut_resource::>() + .emit_now(ServerEvent::GroupManip( + target, + comp::GroupManip::AssignLeader(uid), + )); + Ok(()) } else { - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, action.help_string()), - ); + Err(action.help_string()) } } @@ -2149,27 +1874,18 @@ fn handle_region( target: EcsEntity, msg: String, _action: &ChatCommand, -) { - if client != target { - // This happens when [ab]using /sudo - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, "It's rude to impersonate people"), - ); - return; - } +) -> CmdResult<()> { + no_sudo(client, target)?; + let mode = comp::ChatMode::Region; - let _ = server - .state - .ecs() - .write_storage() - .insert(client, mode.clone()); + insert_or_replace_component(server, target, mode.clone(), "target")?; if !msg.is_empty() { - if let Some(uid) = server.state.ecs().read_storage().get(client) { + if let Some(uid) = server.state.ecs().read_storage().get(target) { server.state.send_chat(mode.new_message(*uid, msg)); } } - server.notify_client(client, ServerGeneral::ChatMode(mode)); + server.notify_client(target, ServerGeneral::ChatMode(mode)); + Ok(()) } fn handle_say( @@ -2178,27 +1894,18 @@ fn handle_say( target: EcsEntity, msg: String, _action: &ChatCommand, -) { - if client != target { - // This happens when [ab]using /sudo - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, "It's rude to impersonate people"), - ); - return; - } +) -> CmdResult<()> { + no_sudo(client, target)?; + let mode = comp::ChatMode::Say; - let _ = server - .state - .ecs() - .write_storage() - .insert(client, mode.clone()); + insert_or_replace_component(server, target, mode.clone(), "target")?; if !msg.is_empty() { - if let Some(uid) = server.state.ecs().read_storage().get(client) { + if let Some(uid) = server.state.ecs().read_storage().get(target) { server.state.send_chat(mode.new_message(*uid, msg)); } } - server.notify_client(client, ServerGeneral::ChatMode(mode)); + server.notify_client(target, ServerGeneral::ChatMode(mode)); + Ok(()) } fn handle_world( @@ -2207,64 +1914,39 @@ fn handle_world( target: EcsEntity, msg: String, _action: &ChatCommand, -) { - if client != target { - // This happens when [ab]using /sudo - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, "It's rude to impersonate people"), - ); - return; - } +) -> CmdResult<()> { + no_sudo(client, target)?; + let mode = comp::ChatMode::World; - let _ = server - .state - .ecs() - .write_storage() - .insert(client, mode.clone()); + insert_or_replace_component(server, target, mode.clone(), "target")?; if !msg.is_empty() { - if let Some(uid) = server.state.ecs().read_storage().get(client) { + if let Some(uid) = server.state.ecs().read_storage().get(target) { server.state.send_chat(mode.new_message(*uid, msg)); } } - server.notify_client(client, ServerGeneral::ChatMode(mode)); + server.notify_client(target, ServerGeneral::ChatMode(mode)); + Ok(()) } fn handle_join_faction( server: &mut Server, - client: EcsEntity, + _client: EcsEntity, target: EcsEntity, args: String, action: &ChatCommand, -) { - if client != target { - // This happens when [ab]using /sudo - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, "It's rude to impersonate people"), - ); - return; - } - if let Some(alias) = server - .state - .ecs() - .read_storage::() - .get(target) - .map(|player| player.alias.clone()) - { +) -> CmdResult<()> { + let players = server.state.ecs().read_storage::(); + if let Some(alias) = players.get(target).map(|player| player.alias.clone()) { + drop(players); let (faction_leave, mode) = if let Ok(faction) = scan_fmt!(&args, &action.arg_fmt(), String) { let mode = comp::ChatMode::Faction(faction.clone()); - let _ = server + insert_or_replace_component(server, target, mode.clone(), "target")?; + let faction_join = server .state .ecs() .write_storage() - .insert(client, mode.clone()); - let faction_leave = server - .state - .ecs() - .write_storage() - .insert(client, comp::Faction(faction.clone())) + .insert(target, comp::Faction(faction.clone())) .ok() .flatten() .map(|f| f.0); @@ -2272,19 +1954,15 @@ fn handle_join_faction( ChatType::FactionMeta(faction.clone()) .chat_msg(format!("[{}] joined faction ({})", alias, faction)), ); - (faction_leave, mode) + (faction_join, mode) } else { let mode = comp::ChatMode::default(); - let _ = server - .state - .ecs() - .write_storage() - .insert(client, mode.clone()); + insert_or_replace_component(server, target, mode.clone(), "target")?; let faction_leave = server .state .ecs() .write_storage() - .remove(client) + .remove(target) .map(|comp::Faction(f)| f); (faction_leave, mode) }; @@ -2294,12 +1972,10 @@ fn handle_join_faction( .chat_msg(format!("[{}] left faction ({})", alias, faction)), ); } - server.notify_client(client, ServerGeneral::ChatMode(mode)); + server.notify_client(target, ServerGeneral::ChatMode(mode)); + Ok(()) } else { - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, "Could not find your player alias"), - ); + Err("Could not find your player alias".into()) } } @@ -2310,14 +1986,8 @@ fn handle_debug_column( target: EcsEntity, _args: String, _action: &ChatCommand, -) { - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandError, - "Unsupported without worldgen enabled", - ), - ); +) -> CmdResult<()> { + Err("Unsupported without worldgen enabled".into()) } #[cfg(feature = "worldgen")] @@ -2327,24 +1997,16 @@ fn handle_debug_column( target: EcsEntity, args: String, action: &ChatCommand, -) { +) -> CmdResult<()> { let sim = server.world.sim(); let sampler = server.world.sample_columns(); - let mut wpos = Vec2::new(0, 0); - if let Ok((x, y)) = scan_fmt!(&args, &action.arg_fmt(), i32, i32) { - wpos = Vec2::new(x, y); + let wpos = if let Ok((x, y)) = scan_fmt!(&args, &action.arg_fmt(), i32, i32) { + Vec2::new(x, y) } else { - match server.state.read_component_copied::(target) { - Some(pos) => wpos = pos.0.xy().map(|x| x as i32), - None => server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandError, - String::from("You have no position."), - ), - ), - } - } + let pos = position(server, target, "target")?; + // FIXME: Deal with overflow, if needed. + pos.0.xy().map(|x| x as i32) + }; let msg_generator = || { let alt = sim.get_interpolated(wpos, |chunk| chunk.alt)?; let basement = sim.get_interpolated(wpos, |chunk| chunk.basement)?; @@ -2398,123 +2060,83 @@ spawn_rate {:?} "#, }; if let Some(s) = msg_generator() { server.notify_client(client, ServerGeneral::server_msg(ChatType::CommandInfo, s)); + Ok(()) } else { - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, "Not a pregenerated chunk."), - ); - } -} - -fn find_target( - ecs: &specs::World, - opt_alias: Option, - fallback: EcsEntity, -) -> Result { - if let Some(alias) = opt_alias { - (&ecs.entities(), &ecs.read_storage::()) - .join() - .find(|(_, player)| player.alias == alias) - .map(|(entity, _)| entity) - .ok_or_else(|| { - ServerGeneral::server_msg( - ChatType::CommandError, - format!("Player '{}' not found!", alias), - ) - }) - } else { - Ok(fallback) + Err("Not a pregenerated chunk.".into()) } } fn handle_skill_point( server: &mut Server, - client: EcsEntity, + _client: EcsEntity, target: EcsEntity, args: String, action: &ChatCommand, -) { - let (a_skill_tree, a_sp, a_alias) = - scan_fmt_some!(&args, &action.arg_fmt(), String, u16, String); +) -> CmdResult<()> { + if let (Some(a_skill_tree), Some(sp), a_alias) = + scan_fmt_some!(&args, &action.arg_fmt(), String, u16, String) + { + let skill_tree = parse_skill_tree(&a_skill_tree)?; + let player = a_alias + .map(|alias| find_alias(server.state.ecs(), &alias).map(|(target, _)| target)) + .unwrap_or(Ok(target))?; - if let (Some(skill_tree), Some(sp)) = (a_skill_tree, a_sp) { - let target = find_target(&server.state.ecs(), a_alias, target); - - let mut error_msg = None; - - match target { - Ok(player) => { - if let Some(skill_tree) = parse_skill_tree(&skill_tree) { - if let Some(mut stats) = server - .state - .ecs_mut() - .write_storage::() - .get_mut(player) - { - stats.skill_set.add_skill_points(skill_tree, sp); - } else { - error_msg = Some(ServerGeneral::server_msg( - ChatType::CommandError, - "Player has no stats!", - )); - } - } - }, - Err(e) => { - error_msg = Some(e); - }, - } - - if let Some(msg) = error_msg { - server.notify_client(client, msg); + if let Some(mut stats) = server + .state + .ecs_mut() + .write_storage::() + .get_mut(player) + { + stats.skill_set.add_skill_points(skill_tree, sp); + Ok(()) + } else { + Err("Player has no stats!".into()) } + } else { + Err(action.help_string()) } } -fn parse_skill_tree(skill_tree: &str) -> Option { +fn parse_skill_tree(skill_tree: &str) -> CmdResult { use comp::{item::tool::ToolKind, skills::SkillGroupKind}; match skill_tree { - "general" => Some(SkillGroupKind::General), - "sword" => Some(SkillGroupKind::Weapon(ToolKind::Sword)), - "axe" => Some(SkillGroupKind::Weapon(ToolKind::Axe)), - "hammer" => Some(SkillGroupKind::Weapon(ToolKind::Hammer)), - "bow" => Some(SkillGroupKind::Weapon(ToolKind::Bow)), - "staff" => Some(SkillGroupKind::Weapon(ToolKind::Staff)), - "sceptre" => Some(SkillGroupKind::Weapon(ToolKind::Sceptre)), - _ => None, + "general" => Ok(SkillGroupKind::General), + "sword" => Ok(SkillGroupKind::Weapon(ToolKind::Sword)), + "axe" => Ok(SkillGroupKind::Weapon(ToolKind::Axe)), + "hammer" => Ok(SkillGroupKind::Weapon(ToolKind::Hammer)), + "bow" => Ok(SkillGroupKind::Weapon(ToolKind::Bow)), + "staff" => Ok(SkillGroupKind::Weapon(ToolKind::Staff)), + "sceptre" => Ok(SkillGroupKind::Weapon(ToolKind::Sceptre)), + _ => Err(format!("{} is not a skill group!", skill_tree)), } } fn handle_debug( server: &mut Server, - client: EcsEntity, + _client: EcsEntity, target: EcsEntity, _args: String, _action: &ChatCommand, -) { +) -> CmdResult<()> { if let Ok(items) = comp::Item::new_from_asset_glob("common.items.debug.*") { server .state() .ecs() .write_storage::() .get_mut(target) - .map(|mut inv| inv.push_all_unique(items.into_iter())); - let _ = server - .state - .ecs() - .write_storage::() - .insert( - target, - comp::InventoryUpdate::new(comp::InventoryUpdateEvent::Debug), - ); + .ok_or("Cannot get inventory for target")? + .push_all_unique(items.into_iter()) + // Deliberately swallow the error if not enough debug items could be added--though it + // might be nice to let the admin know, it's better than dropping them on the ground. + .ok(); + insert_or_replace_component( + server, + target, + comp::InventoryUpdate::new(comp::InventoryUpdateEvent::Debug), + "target", + ) } else { - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandError, - "Debug items not found? Something is very broken.", - ), - ); + Err("Debug items not found? Something is very broken.".into()) } } @@ -2524,35 +2146,27 @@ fn handle_remove_lights( target: EcsEntity, args: String, action: &ChatCommand, -) { +) -> CmdResult<()> { let opt_radius = scan_fmt_some!(&args, &action.arg_fmt(), f32); - let opt_player_pos = server.state.read_component_copied::(target); + let player_pos = position(server, target, "target")?; let mut to_delete = vec![]; - match opt_player_pos { - Some(player_pos) => { - let ecs = server.state.ecs(); - for (entity, pos, _, _, _) in ( - &ecs.entities(), - &ecs.read_storage::(), - &ecs.read_storage::(), - !&ecs.read_storage::(), - !&ecs.read_storage::(), - ) - .join() - { - if opt_radius - .map(|r| pos.0.distance(player_pos.0) < r) - .unwrap_or(true) - { - to_delete.push(entity); - } - } - }, - None => server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, "You have no position."), - ), + let ecs = server.state.ecs(); + for (entity, pos, _, _, _) in ( + &ecs.entities(), + &ecs.read_storage::(), + &ecs.read_storage::(), + !&ecs.read_storage::(), + !&ecs.read_storage::(), + ) + .join() + { + if opt_radius + .map(|r| pos.0.distance(player_pos.0) < r) + .unwrap_or(true) + { + to_delete.push(entity); + } } let size = to_delete.len(); @@ -2565,8 +2179,9 @@ fn handle_remove_lights( server.notify_client( client, - ServerGeneral::server_msg(ChatType::CommandError, format!("Removed {} lights!", size)), + ServerGeneral::server_msg(ChatType::CommandInfo, format!("Removed {} lights!", size)), ); + Ok(()) } fn handle_sudo( @@ -2575,39 +2190,25 @@ fn handle_sudo( _target: EcsEntity, args: String, action: &ChatCommand, -) { +) -> CmdResult<()> { if let (Some(player_alias), Some(cmd), cmd_args) = scan_fmt_some!(&args, &action.arg_fmt(), String, String, String) { let cmd_args = cmd_args.unwrap_or_else(|| String::from("")); if let Ok(action) = cmd.parse() { let ecs = server.state.ecs(); - let entity_opt = (&ecs.entities(), &ecs.read_storage::()) - .join() - .find(|(_, player)| player.alias == player_alias) - .map(|(entity, _)| entity); - if let Some(entity) = entity_opt { - get_handler(&action)(server, client, entity, cmd_args, &action); - } else { - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, "Could not find that player"), - ); - } + let (entity, uuid) = find_alias(ecs, &player_alias)?; + verify_not_hardcoded_admin( + server, + uuid, + "Cannot sudo admins specified in server configuration files.", + )?; + get_handler(&action)(server, client, entity, cmd_args, &action) } else { - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandError, - format!("Unknown command: /{}", cmd), - ), - ); + Err(format!("Unknown command: /{}", cmd)) } } else { - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, action.help_string()), - ); + Err(action.help_string()) } } @@ -2617,7 +2218,7 @@ fn handle_version( _target: EcsEntity, _args: String, _action: &ChatCommand, -) { +) -> CmdResult<()> { server.notify_client( client, ServerGeneral::server_msg( @@ -2629,6 +2230,7 @@ fn handle_version( ), ), ); + Ok(()) } fn handle_whitelist( @@ -2637,78 +2239,63 @@ fn handle_whitelist( _target: EcsEntity, args: String, action: &ChatCommand, -) { +) -> CmdResult<()> { if let Ok((whitelist_action, username)) = scan_fmt!(&args, &action.arg_fmt(), String, String) { - let lookup_uuid = || { - server - .state - .ecs() - .read_resource::() - .username_to_uuid(&username) - .map_err(|_| { - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandError, - format!("Unable to determine UUID for username \"{}\"", &username), - ), - ) - }) - .ok() - }; - if whitelist_action.eq_ignore_ascii_case("add") { - if let Some(uuid) = lookup_uuid() { - server - .editable_settings_mut() - .whitelist - .edit(server.data_dir().as_ref(), |w| w.insert(uuid)); - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandInfo, - format!("\"{}\" added to whitelist", username), - ), - ); - } - } else if whitelist_action.eq_ignore_ascii_case("remove") { - if let Some(uuid) = lookup_uuid() { - server - .editable_settings_mut() - .whitelist - .edit(server.data_dir().as_ref(), |w| w.remove(&uuid)); - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandInfo, - format!("\"{}\" removed from whitelist", username), - ), - ); - } - } else { + let uuid = find_username(server, &username)?; + server + .editable_settings_mut() + .whitelist + .edit(server.data_dir().as_ref(), |w| w.insert(uuid)); server.notify_client( client, - ServerGeneral::server_msg(ChatType::CommandError, action.help_string()), + ServerGeneral::server_msg( + ChatType::CommandInfo, + format!("\"{}\" added to whitelist", username), + ), ); + Ok(()) + } else if whitelist_action.eq_ignore_ascii_case("remove") { + let uuid = find_username(server, &username)?; + server + .editable_settings_mut() + .whitelist + .edit(server.data_dir().as_ref(), |w| w.remove(&uuid)); + server.notify_client( + client, + ServerGeneral::server_msg( + ChatType::CommandInfo, + format!("\"{}\" removed from whitelist", username), + ), + ); + Ok(()) + } else { + Err(action.help_string()) } } else { - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, action.help_string()), - ); + Err(action.help_string()) } } -fn kick_player(server: &mut Server, target_player: EcsEntity, reason: &str) { - server - .state - .ecs() - .read_resource::>() - .emit_now(ServerEvent::ClientDisconnect(target_player)); +fn kick_player( + server: &mut Server, + (target_player, uuid): (EcsEntity, Uuid), + reason: &str, +) -> CmdResult<()> { + verify_not_hardcoded_admin( + server, + uuid, + "Cannot kick admins specified in server configuration files.", + )?; server.notify_client( target_player, ServerGeneral::Disconnect(DisconnectReason::Kicked(reason.to_string())), ); + server + .state + .mut_resource::>() + .emit_now(ServerEvent::ClientDisconnect(target_player)); + Ok(()) } fn handle_kick( @@ -2717,43 +2304,28 @@ fn handle_kick( _target: EcsEntity, args: String, action: &ChatCommand, -) { +) -> CmdResult<()> { if let (Some(target_alias), reason_opt) = scan_fmt_some!(&args, &action.arg_fmt(), String, String) { let reason = reason_opt.unwrap_or_default(); let ecs = server.state.ecs(); - let target_player_opt = (&ecs.entities(), &ecs.read_storage::()) - .join() - .find(|(_, player)| player.alias == target_alias) - .map(|(entity, _)| entity); + let target_player = find_alias(ecs, &target_alias)?; - if let Some(target_player) = target_player_opt { - kick_player(server, target_player, &reason); - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandInfo, - format!( - "Kicked {} from the server with reason: {}", - target_alias, reason - ), - ), - ); - } else { - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandError, - format!("Player with alias {} not found", target_alias), - ), - ) - } - } else { + kick_player(server, target_player, &reason)?; server.notify_client( client, - ServerGeneral::server_msg(ChatType::CommandError, action.help_string()), + ServerGeneral::server_msg( + ChatType::CommandInfo, + format!( + "Kicked {} from the server with reason: {}", + target_alias, reason + ), + ), ); + Ok(()) + } else { + Err(action.help_string()) } } @@ -2763,71 +2335,42 @@ fn handle_ban( _target: EcsEntity, args: String, action: &ChatCommand, -) { - if let (Some(target_alias), reason_opt) = - scan_fmt_some!(&args, &action.arg_fmt(), String, String) - { +) -> CmdResult<()> { + if let (Some(username), reason_opt) = scan_fmt_some!(&args, &action.arg_fmt(), String, String) { let reason = reason_opt.unwrap_or_default(); - let uuid_result = server - .state - .ecs() - .read_resource::() - .username_to_uuid(&target_alias); + let uuid = find_username(server, &username)?; - if let Ok(uuid) = uuid_result { - if server.editable_settings().banlist.contains_key(&uuid) { - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandError, - format!("{} is already on the banlist", target_alias), - ), - ) - } else { - server - .editable_settings_mut() - .banlist - .edit(server.data_dir().as_ref(), |b| { - b.insert(uuid, BanRecord { - username_when_banned: target_alias.clone(), - reason: reason.clone(), - }); - }); - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandInfo, - format!( - "Added {} to the banlist with reason: {}", - target_alias, reason - ), - ), - ); - - // If the player is online kick them - let ecs = server.state.ecs(); - let target_player_opt = (&ecs.entities(), &ecs.read_storage::()) - .join() - .find(|(_, player)| player.alias == target_alias) - .map(|(entity, _)| entity); - if let Some(target_player) = target_player_opt { - kick_player(server, target_player, &reason); - } - } + if server.editable_settings().banlist.contains_key(&uuid) { + Err(format!("{} is already on the banlist", username)) } else { + server + .editable_settings_mut() + .banlist + .edit(server.data_dir().as_ref(), |b| { + b.insert(uuid, BanRecord { + username_when_banned: username.clone(), + reason: reason.clone(), + }); + }); server.notify_client( client, ServerGeneral::server_msg( - ChatType::CommandError, - format!("Unable to determine UUID for username \"{}\"", target_alias), + ChatType::CommandInfo, + format!("Added {} to the banlist with reason: {}", username, reason), ), - ) + ); + + // If the player is online kick them (this may fail if the player is a hardcoded + // admin; we don't care about that case because hardcoded admins can log on even + // if they're on the ban list). + let ecs = server.state.ecs(); + if let Ok(target_player) = find_uuid(ecs, uuid) { + let _ = kick_player(server, (target_player, uuid), &reason); + } + Ok(()) } } else { - server.notify_client( - client, - ServerGeneral::server_msg(ChatType::CommandError, action.help_string()), - ); + Err(action.help_string()) } } @@ -2837,41 +2380,25 @@ fn handle_unban( _target: EcsEntity, args: String, action: &ChatCommand, -) { +) -> CmdResult<()> { if let Ok(username) = scan_fmt!(&args, &action.arg_fmt(), String) { - let uuid_result = server - .state - .ecs() - .read_resource::() - .username_to_uuid(&username); + let uuid = find_username(server, &username)?; - if let Ok(uuid) = uuid_result { - server - .editable_settings_mut() - .banlist - .edit(server.data_dir().as_ref(), |b| { - b.remove(&uuid); - }); - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandInfo, - format!("{} was successfully unbanned", username), - ), - ); - } else { - server.notify_client( - client, - ServerGeneral::server_msg( - ChatType::CommandError, - format!("Unable to determine UUID for username \"{}\"", username), - ), - ) - } - } else { + server + .editable_settings_mut() + .banlist + .edit(server.data_dir().as_ref(), |b| { + b.remove(&uuid); + }); server.notify_client( client, - ServerGeneral::server_msg(ChatType::CommandError, action.help_string()), + ServerGeneral::server_msg( + ChatType::CommandInfo, + format!("{} was successfully unbanned", username), + ), ); + Ok(()) + } else { + Err(action.help_string()) } } diff --git a/server/src/connection_handler.rs b/server/src/connection_handler.rs index 44b975ec04..7f0f11a198 100644 --- a/server/src/connection_handler.rs +++ b/server/src/connection_handler.rs @@ -142,9 +142,16 @@ impl ConnectionHandler { impl Drop for ConnectionHandler { fn drop(&mut self) { - let _ = self.stop_sender.take().unwrap().send(()); + let _ = self + .stop_sender + .take() + .expect("`stop_sender` is private, initialized as `Some`, and only updated in Drop") + .send(()); trace!("aborting ConnectionHandler"); - self.thread_handle.take().unwrap().abort(); + self.thread_handle + .take() + .expect("`thread_handle` is private, initialized as `Some`, and only updated in Drop") + .abort(); trace!("aborted ConnectionHandler!"); } } diff --git a/server/src/events/information.rs b/server/src/events/information.rs index 2125f61d02..b6e650c578 100644 --- a/server/src/events/information.rs +++ b/server/src/events/information.rs @@ -35,15 +35,13 @@ pub fn handle_site_info(server: &Server, entity: EcsEntity, id: u64) { .economy .labor_values .iter() - .filter(|a| a.1.is_some()) - .map(|(g, a)| (g, a.unwrap())) + .filter_map(|(g, a)| a.map(|a| (g, a))) .collect(), values: site .economy .values .iter() - .filter(|a| a.1.is_some()) - .map(|(g, a)| (g, a.unwrap())) + .filter_map(|(g, a)| a.map(|a| (g, a))) .collect(), labors: site.economy.labors.iter().map(|(_, a)| (*a)).collect(), last_exports: site diff --git a/server/src/events/interaction.rs b/server/src/events/interaction.rs index 0f40ddd4dd..c559fc96cf 100644 --- a/server/src/events/interaction.rs +++ b/server/src/events/interaction.rs @@ -77,6 +77,7 @@ pub fn handle_npc_interaction(server: &mut Server, interactor: EcsEntity, npc_en } } +/// FIXME: Make mounting more robust, avoid bidirectional links. pub fn handle_mount(server: &mut Server, mounter: EcsEntity, mountee: EcsEntity) { let state = server.state_mut(); @@ -101,8 +102,14 @@ pub fn handle_mount(server: &mut Server, mounter: EcsEntity, mountee: EcsEntity) state.ecs().uid_from_entity(mounter), state.ecs().uid_from_entity(mountee), ) { - state.write_component(mountee, comp::MountState::MountedBy(mounter_uid)); - state.write_component(mounter, comp::Mounting(mountee_uid)); + // We know the entities must exist to be able to look up their UIDs, so these + // are guaranteed to work; hence we can ignore possible errors + // here. + state.write_component_ignore_entity_dead( + mountee, + comp::MountState::MountedBy(mounter_uid), + ); + state.write_component_ignore_entity_dead(mounter, comp::Mounting(mountee_uid)); } } } @@ -126,8 +133,11 @@ pub fn handle_unmount(server: &mut Server, mounter: EcsEntity) { } #[allow(clippy::nonminimal_bool)] // TODO: Pending review in #587 -pub fn handle_possess(server: &Server, possessor_uid: Uid, possesse_uid: Uid) { - let ecs = &server.state.ecs(); +/// FIXME: This code is dangerous and needs to be refactored. We can't just +/// comment it out, but it needs to be fixed for a variety of reasons. Get rid +/// of this ASAP! +pub fn handle_possess(server: &mut Server, possessor_uid: Uid, possesse_uid: Uid) { + let ecs = server.state.ecs(); if let (Some(possessor), Some(possesse)) = ( ecs.entity_from_uid(possessor_uid.into()), ecs.entity_from_uid(possesse_uid.into()), @@ -231,18 +241,21 @@ pub fn handle_possess(server: &Server, possessor_uid: Uid, possesse_uid: Uid) { let mut inventories = ecs.write_storage::(); let mut inventory = inventories .entry(possesse) - .expect("Could not read inventory component while possessing") + .expect("Nobody has &mut World, so there's no way to delete an entity.") .or_insert(Inventory::new_empty()); let debug_item = comp::Item::new_from_asset_expect("common.items.debug.admin_stick"); if let item::ItemKind::Tool(_) = debug_item.kind() { - inventory - .swap( - Slot::Equip(EquipSlot::Mainhand), - Slot::Equip(EquipSlot::Offhand), - ) - .first() - .unwrap_none(); // Swapping main and offhand never results in leftover items + assert!( + inventory + .swap( + Slot::Equip(EquipSlot::Mainhand), + Slot::Equip(EquipSlot::Offhand), + ) + .first() + .is_none(), + "Swapping main and offhand never results in leftover items", + ); inventory.replace_loadout_item(EquipSlot::Mainhand, Some(debug_item)); } diff --git a/server/src/events/inventory_manip.rs b/server/src/events/inventory_manip.rs index d922628c07..39c81fb490 100644 --- a/server/src/events/inventory_manip.rs +++ b/server/src/events/inventory_manip.rs @@ -80,78 +80,101 @@ pub fn handle_inventory(server: &mut Server, entity: EcsEntity, manip: comp::Inv }) }; + let mut inventories = state.ecs().write_storage::(); + let mut inventory = if let Some(inventory) = inventories.get_mut(entity) { + inventory + } else { + error!( + ?entity, + "Can't manipulate inventory, entity doesn't have one" + ); + return; + }; + match manip { comp::InventoryManip::Pickup(uid) => { - let picked_up_item: Option; - let item_entity = if let (Some((item, item_entity)), Some(mut inv)) = ( - state - .ecs() - .entity_from_uid(uid.into()) - .and_then(|item_entity| { - state - .ecs() - .write_storage::() - .get_mut(item_entity) - .map(|item| (item.clone(), item_entity)) - }), - state - .ecs() - .write_storage::() - .get_mut(entity), - ) { - picked_up_item = Some(item.clone()); - - let entity_cylinder = get_cylinder(state, entity); - if !within_pickup_range(entity_cylinder, || get_cylinder(state, item_entity)) { - debug!( - ?entity_cylinder, - "Failed to pick up item as not within range, Uid: {}", uid - ); - return; - }; - - // Grab the health from the entity and check if the entity is dead. - let healths = state.ecs().read_storage::(); - if let Some(entity_health) = healths.get(entity) { - if entity_health.is_dead { - debug!("Failed to pick up item as the entity is dead"); - return; // If dead, don't continue - } - } - - // First try to equip the picked up item - if let Err(returned_item) = inv.try_equip(item) { - // If we couldn't equip it (no empty slot for it or unequippable) then attempt - // to add the item to the entity's inventory - match inv.pickup_item(returned_item) { - Ok(_) => Some(item_entity), - Err(_) => None, // Inventory was full - } - } else { - Some(item_entity) - } + let item_entity = if let Some(item_entity) = state.ecs().entity_from_uid(uid.into()) { + item_entity } else { - // Item entity/component could not be found - most likely because the entity + // Item entity could not be found - most likely because the entity // attempted to pick up the same item very quickly before its deletion of the // world from the first pickup attempt was processed. - debug!("Failed to get entity/component for item Uid: {}", uid); + debug!("Failed to get entity for item Uid: {}", uid); + return; + }; + let entity_cylinder = get_cylinder(state, entity); + + // FIXME: Raycast so we can't pick up items through walls. + if !within_pickup_range(entity_cylinder, || get_cylinder(state, item_entity)) { + debug!( + ?entity_cylinder, + "Failed to pick up item as not within range, Uid: {}", uid + ); + return; + } + + // Grab the health from the entity and check if the entity is dead. + let healths = state.ecs().read_storage::(); + if let Some(entity_health) = healths.get(entity) { + if entity_health.is_dead { + debug!("Failed to pick up item as the entity is dead"); + return; // If dead, don't continue + } + } + drop(healths); + + // First, we remove the item, assuming picking it up will succeed (we do this to + // avoid cloning the item, as we should not call Item::clone and it + // may be removed!). + let mut item_storage = state.ecs().write_storage::(); + let item = if let Some(item) = item_storage.remove(item_entity) { + item + } else { + // Item component could not be found - most likely because the entity + // attempted to pick up the same item very quickly before its deletion of the + // world from the first pickup attempt was processed. + debug!("Failed to delete item component for entity, Uid: {}", uid); return; }; - let event = if let Some(item_entity) = item_entity { - if let Err(err) = state.delete_entity_recorded(item_entity) { - // If this occurs it means the item was duped as it's been pushed to the - // entity's inventory but also left on the ground - panic!("Failed to delete picked up item entity: {:?}", err); - } - comp::InventoryUpdate::new(comp::InventoryUpdateEvent::Collected( - picked_up_item.unwrap(), - )) - } else { - comp::InventoryUpdate::new(comp::InventoryUpdateEvent::CollectFailed) + // NOTE: We dup the item for message purposes. + let item_msg = item.duplicate(&state.ecs().read_resource::()); + + // Next, we try to equip the picked up item + let event = match inventory.try_equip(item).or_else(|returned_item| { + // If we couldn't equip it (no empty slot for it or unequippable) then attempt + // to add the item to the entity's inventory + inventory.pickup_item(returned_item) + }) { + Err(returned_item) => { + // Inventory was full, so we need to put back the item (note that we know there + // was no old item component for this entity). + item_storage.insert(entity, returned_item).expect( + "We know item_entity exists since we just successfully removed its Item \ + component.", + ); + drop(item_storage); + drop(inventories); + comp::InventoryUpdate::new(comp::InventoryUpdateEvent::CollectFailed) + }, + Ok(_) => { + // We succeeded in picking up the item, so we may now delete its old entity + // entirely. + drop(item_storage); + drop(inventories); + state.delete_entity_recorded(item_entity).expect( + "We knew item_entity existed since we just successfully removed its Item \ + component.", + ); + comp::InventoryUpdate::new(comp::InventoryUpdateEvent::Collected(item_msg)) + }, }; - state.write_component(entity, event); + state + .ecs() + .write_storage() + .insert(entity, event) + .expect("We know entity exists since we got its inventory."); }, comp::InventoryManip::Collect(pos) => { let block = state.terrain().get(pos).ok().copied(); @@ -174,34 +197,31 @@ pub fn handle_inventory(server: &mut Server, entity: EcsEntity, manip: comp::Inv }; if let Some(item) = comp::Item::try_reclaim_from_block(block) { - let (event, item_was_added) = if let Some(mut inv) = state - .ecs() - .write_storage::() - .get_mut(entity) - { - match inv.push(item.clone()) { - None => ( - Some(comp::InventoryUpdate::new( - comp::InventoryUpdateEvent::Collected(item), - )), - true, - ), - Some(_) => ( - Some(comp::InventoryUpdate::new( - comp::InventoryUpdateEvent::CollectFailed, - )), - false, - ), - } - } else { - debug!( - "Can't add item to inventory: entity has no inventory ({:?})", - entity - ); - (None, false) + // NOTE: We dup the item for message purposes. + let item_msg = + item.duplicate(&state.ecs().read_resource::()); + let (event, item_was_added) = match inventory.push(item) { + Ok(_) => ( + Some(comp::InventoryUpdate::new( + comp::InventoryUpdateEvent::Collected(item_msg), + )), + true, + ), + // The item we created was in some sense "fake" so it's safe to + // drop it. + Err(_) => ( + Some(comp::InventoryUpdate::new( + comp::InventoryUpdateEvent::CollectFailed, + )), + false, + ), }; if let Some(event) = event { - state.write_component(entity, event); + state + .ecs() + .write_storage() + .insert(entity, event) + .expect("We know entity exists since we got its inventory."); if item_was_added { // we made sure earlier the block was not already modified this tick state.set_block(pos, block.into_vacant()) @@ -222,19 +242,9 @@ pub fn handle_inventory(server: &mut Server, entity: EcsEntity, manip: comp::Inv ); } } + drop(inventories); }, comp::InventoryManip::Use(slot) => { - let mut inventories = state.ecs().write_storage::(); - let mut inventory = if let Some(inventory) = inventories.get_mut(entity) { - inventory - } else { - error!( - ?entity, - "Can't manipulate inventory, entity doesn't have one" - ); - return; - }; - let mut maybe_effect = None; let event = match slot { @@ -380,7 +390,9 @@ pub fn handle_inventory(server: &mut Server, entity: EcsEntity, manip: comp::Inv Some(comp::InventoryUpdateEvent::Used) }, _ => { - inventory.insert_or_stack_at(slot, item).unwrap(); + inventory.insert_or_stack_at(slot, item).expect( + "slot was just vacated of item, so it definitely fits there.", + ); None }, } @@ -420,55 +432,51 @@ pub fn handle_inventory(server: &mut Server, entity: EcsEntity, manip: comp::Inv } } if let Some(event) = event { - state.write_component(entity, comp::InventoryUpdate::new(event)); + state + .ecs() + .write_storage() + .insert(entity, comp::InventoryUpdate::new(event)) + .expect("We know entity exists since we got its inventory."); } }, comp::InventoryManip::Swap(a, b) => { let ecs = state.ecs(); if let Some(pos) = ecs.read_storage::().get(entity) { - if let Some(mut inventory) = ecs.write_storage::().get_mut(entity) - { - let mut merged_stacks = false; + let mut merged_stacks = false; - // If both slots have items and we're attemping to drag from one stack - // into another, stack the items. - if let (Slot::Inventory(slot_a), Slot::Inventory(slot_b)) = (a, b) { - merged_stacks |= inventory.merge_stack_into(slot_a, slot_b); - } + // If both slots have items and we're attemping to drag from one stack + // into another, stack the items. + if let (Slot::Inventory(slot_a), Slot::Inventory(slot_b)) = (a, b) { + merged_stacks |= inventory.merge_stack_into(slot_a, slot_b); + } - // If the stacks weren't mergable carry out a swap. - if !merged_stacks { - dropped_items.extend(inventory.swap(a, b).into_iter().map(|x| { - ( - *pos, - state - .read_component_copied::(entity) - .unwrap_or_default(), - x, - ) - })); - } + // If the stacks weren't mergable carry out a swap. + if !merged_stacks { + dropped_items.extend(inventory.swap(a, b).into_iter().map(|x| { + ( + *pos, + state + .read_component_copied::(entity) + .unwrap_or_default(), + x, + ) + })); } } + drop(inventories); - state.write_component( - entity, - comp::InventoryUpdate::new(comp::InventoryUpdateEvent::Swapped), - ); + state + .ecs() + .write_storage() + .insert( + entity, + comp::InventoryUpdate::new(comp::InventoryUpdateEvent::Swapped), + ) + .expect("We know entity exists since we got its inventory."); }, comp::InventoryManip::SplitSwap(slot, target) => { let msm = state.ecs().read_resource::(); - let mut inventories = state.ecs().write_storage::(); - let mut inventory = if let Some(inventory) = inventories.get_mut(entity) { - inventory - } else { - error!( - ?entity, - "Can't manipulate inventory, entity doesn't have one" - ); - return; - }; // If both slots have items and we're attemping to split from one stack // into another, ensure that they are the same type of item. If they are @@ -496,27 +504,22 @@ pub fn handle_inventory(server: &mut Server, entity: EcsEntity, manip: comp::Inv inventory.insert_or_stack_at(target, item).ok(); } } - drop(inventory); - drop(inventories); drop(msm); - state.write_component( - entity, - comp::InventoryUpdate::new(comp::InventoryUpdateEvent::Swapped), - ); + state + .ecs() + .write_storage() + .insert( + entity, + comp::InventoryUpdate::new(comp::InventoryUpdateEvent::Swapped), + ) + .expect("We know entity exists since we got its inventory."); + drop(inventories); }, comp::InventoryManip::Drop(slot) => { let item = match slot { - Slot::Inventory(slot) => state - .ecs() - .write_storage::() - .get_mut(entity) - .and_then(|mut inv| inv.remove(slot)), - Slot::Equip(slot) => state - .ecs() - .write_storage::() - .get_mut(entity) - .and_then(|mut inv| inv.replace_loadout_item(slot, None)), + Slot::Inventory(slot) => inventory.remove(slot), + Slot::Equip(slot) => inventory.replace_loadout_item(slot, None), }; // FIXME: We should really require the drop and write to be atomic! @@ -532,19 +535,20 @@ pub fn handle_inventory(server: &mut Server, entity: EcsEntity, manip: comp::Inv item, )); } - state.write_component( - entity, - comp::InventoryUpdate::new(comp::InventoryUpdateEvent::Dropped), - ); + state + .ecs() + .write_storage() + .insert( + entity, + comp::InventoryUpdate::new(comp::InventoryUpdateEvent::Dropped), + ) + .expect("We know entity exists since we got its inventory."); + drop(inventories); }, comp::InventoryManip::SplitDrop(slot) => { let msm = state.ecs().read_resource::(); let item = match slot { - Slot::Inventory(slot) => state - .ecs() - .write_storage::() - .get_mut(entity) - .and_then(|mut inv| inv.take_half(slot, &msm)), + Slot::Inventory(slot) => inventory.take_half(slot, &msm), Slot::Equip(_) => None, }; @@ -562,47 +566,48 @@ pub fn handle_inventory(server: &mut Server, entity: EcsEntity, manip: comp::Inv )); } drop(msm); - state.write_component( - entity, - comp::InventoryUpdate::new(comp::InventoryUpdateEvent::Dropped), - ); + state + .ecs() + .write_storage() + .insert( + entity, + comp::InventoryUpdate::new(comp::InventoryUpdateEvent::Dropped), + ) + .expect("We know entity exists since we got its inventory."); + drop(inventories); }, comp::InventoryManip::CraftRecipe(recipe) => { - if let Some(mut inv) = state - .ecs() - .write_storage::() - .get_mut(entity) - { - let recipe_book = default_recipe_book().read(); - let craft_result = recipe_book.get(&recipe).and_then(|r| { - r.perform( - &mut inv, - &state.ecs().read_resource::(), - ) - .ok() - }); + let recipe_book = default_recipe_book().read(); + let craft_result = recipe_book.get(&recipe).and_then(|r| { + r.perform( + &mut inventory, + &state.ecs().read_resource::(), + ) + .ok() + }); + drop(inventories); - // FIXME: We should really require the drop and write to be atomic! - if craft_result.is_some() { - let _ = state.ecs().write_storage().insert( - entity, - comp::InventoryUpdate::new(comp::InventoryUpdateEvent::Craft), - ); - } + // FIXME: We should really require the drop and write to be atomic! + if craft_result.is_some() { + let _ = state.ecs().write_storage().insert( + entity, + comp::InventoryUpdate::new(comp::InventoryUpdateEvent::Craft), + ); + } - // Drop the item if there wasn't enough space - if let Some(Some((item, amount))) = craft_result { - for _ in 0..amount { - dropped_items.push(( - state - .read_component_copied::(entity) - .unwrap_or_default(), - state - .read_component_copied::(entity) - .unwrap_or_default(), - item.clone(), - )); - } + // Drop the item if there wasn't enough space + if let Some(Some((item, amount))) = craft_result { + let msm = state.ecs().read_resource::(); + for _ in 0..amount { + dropped_items.push(( + state + .read_component_copied::(entity) + .unwrap_or_default(), + state + .read_component_copied::(entity) + .unwrap_or_default(), + item.duplicate(&msm), + )); } } }, diff --git a/server/src/events/mod.rs b/server/src/events/mod.rs index 348b0043ca..59c5d0c26a 100644 --- a/server/src/events/mod.rs +++ b/server/src/events/mod.rs @@ -124,7 +124,7 @@ impl Server { ServerEvent::Mount(mounter, mountee) => handle_mount(self, mounter, mountee), ServerEvent::Unmount(mounter) => handle_unmount(self, mounter), ServerEvent::Possess(possessor_uid, possesse_uid) => { - handle_possess(&self, possessor_uid, possesse_uid) + handle_possess(self, possessor_uid, possesse_uid) }, ServerEvent::InitCharacterData { entity, diff --git a/server/src/events/player.rs b/server/src/events/player.rs index 5ff85b3d91..9311284892 100644 --- a/server/src/events/player.rs +++ b/server/src/events/player.rs @@ -101,28 +101,37 @@ pub fn handle_client_disconnect(server: &mut Server, entity: EcsEntity) -> Event .write_storage::() .get_mut(entity) { - let participant = client.participant.take().unwrap(); - let pid = participant.remote_pid(); - server.runtime.spawn( - async { - let now = std::time::Instant::now(); - debug!("Start handle disconnect of client"); - if let Err(e) = participant.disconnect().await { - debug!( - ?e, - "Error when disconnecting client, maybe the pipe already broke" - ); - }; - trace!("finished disconnect"); - let elapsed = now.elapsed(); - if elapsed.as_millis() > 100 { - warn!(?elapsed, "disconnecting took quite long"); - } else { - debug!(?elapsed, "disconnecting took"); + // NOTE: There are not and likely willl not be a way to safeguard against + // receiving multiple `ServerEvent::ClientDisconnect` messages in a tick + // intended for the same player, so the `None` case here is *not* a bug + // and we should not log it as a potential issue. + if let Some(participant) = client.participant.take() { + let pid = participant.remote_pid(); + server.runtime.spawn( + async { + let now = std::time::Instant::now(); + debug!("Start handle disconnect of client"); + if let Err(e) = participant.disconnect().await { + debug!( + ?e, + "Error when disconnecting client, maybe the pipe already broke" + ); + }; + trace!("finished disconnect"); + let elapsed = now.elapsed(); + if elapsed.as_millis() > 100 { + warn!(?elapsed, "disconnecting took quite long"); + } else { + debug!(?elapsed, "disconnecting took"); + } } - } - .instrument(tracing::debug_span!("client_disconnect", ?pid, ?entity)), - ); + .instrument(tracing::debug_span!( + "client_disconnect", + ?pid, + ?entity + )), + ); + } } let state = server.state_mut(); diff --git a/server/src/events/trade.rs b/server/src/events/trade.rs index 657602673b..bda4a32a4c 100644 --- a/server/src/events/trade.rs +++ b/server/src/events/trade.rs @@ -166,7 +166,7 @@ fn commit_trade(ecs: &specs::World, trade: &PendingTrade) -> TradeResult { None => return TradeResult::Declined, } } - let mut inventories = ecs.write_component::(); + let mut inventories = ecs.write_storage::(); for entity in entities.iter() { if inventories.get_mut(*entity).is_none() { return TradeResult::Declined; diff --git a/server/src/lib.rs b/server/src/lib.rs index ca08779b6d..c598e6baa7 100644 --- a/server/src/lib.rs +++ b/server/src/lib.rs @@ -86,7 +86,6 @@ use persistence::{ character_loader::{CharacterLoader, CharacterLoaderResponseKind}, character_updater::CharacterUpdater, }; -use plugin_api::Uid; use prometheus::Registry; use prometheus_hyper::Server as PrometheusServer; use specs::{join::Join, Builder, Entity as EcsEntity, SystemData, WorldExt}; @@ -337,10 +336,9 @@ impl Server { max: Vec3::new(world_size.x, world_size.y, 32767), } .made_valid(); - let world_aabb_id = build_areas.areas.insert(world_aabb); build_areas - .area_names - .insert("world".to_string(), world_aabb_id); + .insert("world".to_string(), world_aabb) + .expect("The initial insert should always work."); } // Insert the world into the ECS (todo: Maybe not an Arc?) @@ -748,11 +746,18 @@ impl Server { .ecs() .read_storage::() .get(entity) - .unwrap() + .expect( + "We just created this entity with a Client component using build(), and we have \ + &mut access to the ecs so it can't have been deleted yet.", + ) .send(ServerInit::GameSync { // Send client their entity entity_package: TrackedComps::fetch(&self.state.ecs()) - .create_entity_package(entity, None, None, None), + .create_entity_package(entity, None, None, None) + .expect( + "We just created this entity as marked() (using create_entity_synced) so \ + it definitely has a uid", + ), time_of_day: *self.state.ecs().read_resource(), max_group_size: self.settings().max_player_group_size, client_timeout: self.settings().client_timeout, @@ -840,22 +845,24 @@ impl Server { uid_allocator: &self.state.ecs().read_resource::().into(), player: self.state.ecs().read_component().into(), }; + let uid = if let Some(uid) = ecs_world.uid.get(entity).copied() { + uid + } else { + self.notify_client( + entity, + ServerGeneral::server_msg( + comp::ChatType::CommandError, + "Can't get player UUID (player may be disconnected?)", + ), + ); + return; + }; let rs = plugin_manager.execute_event( &ecs_world, &plugin_api::event::ChatCommandEvent { command: kwd.clone(), command_args: args.split(' ').map(|x| x.to_owned()).collect(), - player: plugin_api::event::Player { - id: plugin_api::Uid( - (self - .state - .ecs() - .read_storage::() - .get(entity) - .expect("Can't get player UUID [This should never appen]")) - .0, - ), - }, + player: plugin_api::event::Player { id: uid }, }, ); match rs { @@ -976,8 +983,7 @@ impl Server { .map(|(e, _)| e) }) { // Remove admin component if the player is ingame - let _ = self - .state + self.state .ecs() .write_storage::() .remove(entity); diff --git a/server/src/login_provider.rs b/server/src/login_provider.rs index 3cc37b0229..c75bae7d10 100644 --- a/server/src/login_provider.rs +++ b/server/src/login_provider.rs @@ -21,7 +21,7 @@ fn derive_uuid(username: &str) -> Uuid { state = state.wrapping_mul(309485009821345068724781371); } - Uuid::from_slice(&state.to_be_bytes()).unwrap() + Uuid::from_u128(state) } /// derive Uuid for "singleplayer" is a pub fn @@ -107,26 +107,35 @@ impl LoginProvider { match pending.pending_r.try_recv() { Ok(Err(e)) => Some(Err(e)), Ok(Ok((username, uuid))) => { - if let Some(ban_record) = banlist.get(&uuid) { - // Pull reason string out of ban record and send a copy of it - return Some(Err(RegisterError::Banned(ban_record.reason.clone()))); + // Hardcoded admins can always log in. + let is_admin = admins.contains(&uuid); + if !is_admin { + if let Some(ban_record) = banlist.get(&uuid) { + // Pull reason string out of ban record and send a copy of it + return Some(Err(RegisterError::Banned(ban_record.reason.clone()))); + } + + // non-admins can only join if the whitelist is empty (everyone can join) + // or his name is in the whitelist + if !whitelist.is_empty() && !whitelist.contains(&uuid) { + return Some(Err(RegisterError::NotOnWhitelist)); + } } - // user can only join if he is admin, the whitelist is empty (everyone can join) - // or his name is in the whitelist - if !whitelist.is_empty() && !whitelist.contains(&uuid) && !admins.contains(&uuid) { - return Some(Err(RegisterError::NotOnWhitelist)); - } #[cfg(feature = "plugins")] { + // Plugin player join hooks execute for all players, but are only allowed to + // filter non-admins. match plugin_manager.execute_event(&world, &PlayerJoinEvent { player_name: username.clone(), player_id: *uuid.as_bytes(), }) { Ok(e) => { - for i in e.into_iter() { - if let PlayerJoinResult::Kick(a) = i { - return Some(Err(RegisterError::Kicked(a))); + if !is_admin { + for i in e.into_iter() { + if let PlayerJoinResult::Kick(a) = i { + return Some(Err(RegisterError::Kicked(a))); + } } } }, diff --git a/server/src/persistence/character/conversions.rs b/server/src/persistence/character/conversions.rs index a747a5a6fd..975cba5723 100644 --- a/server/src/persistence/character/conversions.rs +++ b/server/src/persistence/character/conversions.rs @@ -253,6 +253,9 @@ pub fn convert_inventory_from_database_items( // Stack Size if db_item.stack_size == 1 || item.is_stackable() { + // FIXME: On failure, collect the set of items that don't fit and return them + // (to be dropped next to the player) as this could be the result of + // a change in the max amount for that item. item.set_amount(u32::try_from(db_item.stack_size).map_err(|_| { Error::ConversionError(format!( "Invalid item stack size for stackable={}: {}", @@ -280,6 +283,11 @@ pub fn convert_inventory_from_database_items( let insert_res = inventory.insert_at(slot, item).map_err(|_| { // If this happens there were too many items in the database for the current // inventory size + // + // FIXME: On failure, collect the set of items that don't fit and return them + // (to be dropped next to the player) as this could be the + // result of a change in the slot capacity for an equipped bag + // (or a change in the inventory size). Error::ConversionError(format!( "Error inserting item into inventory, position: {:?}", slot diff --git a/server/src/state_ext.rs b/server/src/state_ext.rs index da4116e2d6..a83a300436 100644 --- a/server/src/state_ext.rs +++ b/server/src/state_ext.rs @@ -361,37 +361,40 @@ impl StateExt for State { fn initialize_character_data(&mut self, entity: EcsEntity, character_id: CharacterId) { let spawn_point = self.ecs().read_resource::().0; - self.write_component(entity, comp::Controller::default()); - self.write_component(entity, comp::Pos(spawn_point)); - self.write_component(entity, comp::Vel(Vec3::zero())); - self.write_component(entity, comp::Ori::default()); - self.write_component(entity, comp::Collider::Box { - radius: 0.4, - z_min: 0.0, - z_max: 1.75, - }); - self.write_component(entity, comp::Gravity(1.0)); - self.write_component(entity, comp::CharacterState::default()); - self.write_component( - entity, - comp::Alignment::Owned(self.read_component_copied(entity).unwrap()), - ); - self.write_component(entity, comp::Buffs::default()); - self.write_component(entity, comp::Auras::default()); - self.write_component(entity, comp::Combo::default()); + if let Some(player_uid) = self.read_component_copied::(entity) { + // NOTE: By fetching the player_uid, we validated that the entity exists, and we + // call nothing that can delete it in any of the subsequent + // commands, so we can assume that all of these calls succeed, + // justifying ignoring the result of insertion. + self.write_component_ignore_entity_dead(entity, comp::Controller::default()); + self.write_component_ignore_entity_dead(entity, comp::Pos(spawn_point)); + self.write_component_ignore_entity_dead(entity, comp::Vel(Vec3::zero())); + self.write_component_ignore_entity_dead(entity, comp::Ori::default()); + self.write_component_ignore_entity_dead(entity, comp::Collider::Box { + radius: 0.4, + z_min: 0.0, + z_max: 1.75, + }); + self.write_component_ignore_entity_dead(entity, comp::Gravity(1.0)); + self.write_component_ignore_entity_dead(entity, comp::CharacterState::default()); + self.write_component_ignore_entity_dead(entity, comp::Alignment::Owned(player_uid)); + self.write_component_ignore_entity_dead(entity, comp::Buffs::default()); + self.write_component_ignore_entity_dead(entity, comp::Auras::default()); + self.write_component_ignore_entity_dead(entity, comp::Combo::default()); - // Make sure physics components are updated - self.write_component(entity, comp::ForceUpdate); + // Make sure physics components are updated + self.write_component_ignore_entity_dead(entity, comp::ForceUpdate); - const INITIAL_VD: u32 = 5; //will be changed after login - self.write_component( - entity, - Presence::new(INITIAL_VD, PresenceKind::Character(character_id)), - ); + const INITIAL_VD: u32 = 5; //will be changed after login + self.write_component_ignore_entity_dead( + entity, + Presence::new(INITIAL_VD, PresenceKind::Character(character_id)), + ); - // Tell the client its request was successful. - if let Some(client) = self.ecs().read_storage::().get(entity) { - client.send_fallible(ServerGeneral::CharacterSuccess); + // Tell the client its request was successful. + if let Some(client) = self.ecs().read_storage::().get(entity) { + client.send_fallible(ServerGeneral::CharacterSuccess); + } } } @@ -406,12 +409,16 @@ impl StateExt for State { }), )); - self.write_component(entity, comp::Collider::Box { + // NOTE: By fetching the player_uid, we validated that the entity exists, and we + // call nothing that can delete it in any of the subsequent + // commands, so we can assume that all of these calls succeed, + // justifying ignoring the result of insertion. + self.write_component_ignore_entity_dead(entity, comp::Collider::Box { radius: body.radius(), z_min: 0.0, z_max: body.height(), }); - self.write_component(entity, body); + self.write_component_ignore_entity_dead(entity, body); let (health_level, energy_level) = ( stats .skill_set @@ -424,21 +431,21 @@ impl StateExt for State { .unwrap_or(None) .unwrap_or(0), ); - self.write_component(entity, comp::Health::new(body, health_level)); - self.write_component(entity, comp::Energy::new(body, energy_level)); - self.write_component(entity, comp::Poise::new(body)); - self.write_component(entity, stats); - self.write_component(entity, inventory); - self.write_component( + self.write_component_ignore_entity_dead(entity, comp::Health::new(body, health_level)); + self.write_component_ignore_entity_dead(entity, comp::Energy::new(body, energy_level)); + self.write_component_ignore_entity_dead(entity, comp::Poise::new(body)); + self.write_component_ignore_entity_dead(entity, stats); + self.write_component_ignore_entity_dead(entity, inventory); + self.write_component_ignore_entity_dead( entity, comp::InventoryUpdate::new(comp::InventoryUpdateEvent::default()), ); if let Some(waypoint) = waypoint { - self.write_component(entity, waypoint); - self.write_component(entity, comp::Pos(waypoint.get_pos())); - self.write_component(entity, comp::Vel(Vec3::zero())); - self.write_component(entity, comp::ForceUpdate); + self.write_component_ignore_entity_dead(entity, waypoint); + self.write_component_ignore_entity_dead(entity, comp::Pos(waypoint.get_pos())); + self.write_component_ignore_entity_dead(entity, comp::Vel(Vec3::zero())); + self.write_component_ignore_entity_dead(entity, comp::ForceUpdate); } } } @@ -585,8 +592,8 @@ impl StateExt for State { ) .join() { - if lazy_msg.is_none() { - lazy_msg = Some(client.prepare(msg.take().unwrap())); + if let Some(msg) = msg.take() { + lazy_msg = Some(client.prepare(msg)); } lazy_msg.as_ref().map(|ref msg| client.send_prepared(&msg)); } @@ -602,8 +609,8 @@ impl StateExt for State { ) .join() { - if lazy_msg.is_none() { - lazy_msg = Some(client.prepare(msg.take().unwrap())); + if let Some(msg) = msg.take() { + lazy_msg = Some(client.prepare(msg)); } lazy_msg.as_ref().map(|ref msg| client.send_prepared(&msg)); } diff --git a/server/src/sys/entity_sync.rs b/server/src/sys/entity_sync.rs index 8be7888009..0ba441468b 100644 --- a/server/src/sys/entity_sync.rs +++ b/server/src/sys/entity_sync.rs @@ -15,6 +15,7 @@ use common::{ }; use common_ecs::{Job, Origin, Phase, System}; use common_net::{msg::ServerGeneral, sync::CompSyncPackage}; +use itertools::Either; use specs::{Entities, Join, Read, ReadExpect, ReadStorage, Write, WriteStorage}; use vek::*; @@ -127,18 +128,19 @@ impl<'a> System<'a> for Sys { continue; } let entity = entities.entity(*id); - if let Some((_uid, pos, vel, ori)) = uids.get(entity).and_then(|uid| { - positions.get(entity).map(|pos| { - (uid, pos, velocities.get(entity), orientations.get(entity)) - }) - }) { - let create_msg = - ServerGeneral::CreateEntity(tracked_comps.create_entity_package( + if let Some(pkg) = positions + .get(entity) + .map(|pos| (pos, velocities.get(entity), orientations.get(entity))) + .and_then(|(pos, vel, ori)| { + tracked_comps.create_entity_package( entity, Some(*pos), vel.copied(), ori.copied(), - )); + ) + }) + { + let create_msg = ServerGeneral::CreateEntity(pkg); for (client, regions, client_entity, _) in &mut subscribers { if maybe_key .as_ref() @@ -178,38 +180,35 @@ impl<'a> System<'a> for Sys { .take_deleted_in_region(key) .unwrap_or_default(), ); - let mut entity_sync_package = Some(entity_sync_package); - let mut comp_sync_package = Some(comp_sync_package); - let mut entity_sync_lazymsg = None; - let mut comp_sync_lazymsg = None; - subscribers.iter_mut().for_each(move |(client, _, _, _)| { - if entity_sync_lazymsg.is_none() { - entity_sync_lazymsg = Some(client.prepare(ServerGeneral::EntitySync( - entity_sync_package.take().unwrap(), - ))); - comp_sync_lazymsg = Some( - client.prepare(ServerGeneral::CompSync(comp_sync_package.take().unwrap())), - ); - } - entity_sync_lazymsg - .as_ref() - .map(|msg| client.send_prepared(&msg)); - comp_sync_lazymsg - .as_ref() - .map(|msg| client.send_prepared(&msg)); - }); + // We lazily initializethe the synchronization messages in case there are no + // clients. + let mut entity_comp_sync = Either::Left((entity_sync_package, comp_sync_package)); + for (client, _, _, _) in &mut subscribers { + let msg = + entity_comp_sync.right_or_else(|(entity_sync_package, comp_sync_package)| { + ( + client.prepare(ServerGeneral::EntitySync(entity_sync_package)), + client.prepare(ServerGeneral::CompSync(comp_sync_package)), + ) + }); + // We don't care much about stream errors here since they could just represent + // network disconnection, which is handled elsewhere. + let _ = client.send_prepared(&msg.0); + let _ = client.send_prepared(&msg.1); + entity_comp_sync = Either::Right(msg); + } for (client, _, client_entity, client_pos) in &mut subscribers { let mut comp_sync_package = CompSyncPackage::new(); - for (_, entity, &uid, &pos, vel, ori, force_update, collider) in ( + for (_, entity, &uid, (&pos, last_pos), vel, ori, force_update, collider) in ( region.entities(), &entities, &uids, - &positions, - velocities.maybe(), - orientations.maybe(), - force_updates.maybe(), + (&positions, last_pos.mask().maybe()), + (&velocities, last_vel.mask().maybe()).maybe(), + (&orientations, last_vel.mask().maybe()).maybe(), + force_updates.mask().maybe(), colliders.maybe(), ) .join() @@ -246,45 +245,48 @@ impl<'a> System<'a> for Sys { } }; - if last_pos.get(entity).is_none() { + if last_pos.is_none() { comp_sync_package.comp_inserted(uid, pos); } else if send_now { comp_sync_package.comp_modified(uid, pos); } - vel.map(|v| { - if last_vel.get(entity).is_none() { + if let Some((v, last_vel)) = vel { + if last_vel.is_none() { comp_sync_package.comp_inserted(uid, *v); } else if send_now { comp_sync_package.comp_modified(uid, *v); } - }); + } - ori.map(|o| { - if last_ori.get(entity).is_none() { + if let Some((o, last_ori)) = ori { + if last_ori.is_none() { comp_sync_package.comp_inserted(uid, *o); } else if send_now { comp_sync_package.comp_modified(uid, *o); } - }); + } } client.send_fallible(ServerGeneral::CompSync(comp_sync_package)); } // Update the last physics components for each entity - for (_, entity, &pos, vel, ori) in ( + for (_, _, &pos, vel, ori, last_pos, last_vel, last_ori) in ( region.entities(), &entities, &positions, velocities.maybe(), orientations.maybe(), + last_pos.entries(), + last_vel.entries(), + last_ori.entries(), ) .join() { - let _ = last_pos.insert(entity, Last(pos)); - vel.map(|v| last_vel.insert(entity, Last(*v))); - ori.map(|o| last_ori.insert(entity, Last(*o))); + last_pos.replace(Last(pos)); + vel.and_then(|&v| last_vel.replace(Last(v))); + ori.and_then(|&o| last_ori.replace(Last(o))); } } @@ -347,10 +349,12 @@ impl<'a> System<'a> for Sys { // system?) let mut tof_lazymsg = None; for client in (&clients).join() { - if tof_lazymsg.is_none() { - tof_lazymsg = Some(client.prepare(ServerGeneral::TimeOfDay(*time_of_day))); - } - tof_lazymsg.as_ref().map(|msg| client.send_prepared(&msg)); + let msg = tof_lazymsg + .unwrap_or_else(|| client.prepare(ServerGeneral::TimeOfDay(*time_of_day))); + // We don't care much about stream errors here since they could just represent + // network disconnection, which is handled elsewhere. + let _ = client.send_prepared(&msg); + tof_lazymsg = Some(msg); } } } diff --git a/server/src/sys/msg/character_screen.rs b/server/src/sys/msg/character_screen.rs index 8e5a4c51a0..19ed7d2f9d 100644 --- a/server/src/sys/msg/character_screen.rs +++ b/server/src/sys/msg/character_screen.rs @@ -17,7 +17,6 @@ impl Sys { #[allow(clippy::too_many_arguments)] fn handle_client_character_screen_msg( server_emitter: &mut common::event::Emitter<'_, ServerEvent>, - new_chat_msgs: &mut Vec<(Option, UnresolvedChatMsg)>, entity: specs::Entity, client: &Client, character_loader: &ReadExpect<'_, CharacterLoader>, @@ -68,7 +67,7 @@ impl Sys { if !client.login_msg_sent.load(Ordering::Relaxed) { if let Some(player_uid) = uids.get(entity) { - new_chat_msgs.push((None, UnresolvedChatMsg { + server_emitter.emit(ServerEvent::Chat(UnresolvedChatMsg { chat_type: ChatType::Online(*player_uid), message: "".to_string(), })); @@ -155,13 +154,11 @@ impl<'a> System<'a> for Sys { ): Self::SystemData, ) { let mut server_emitter = server_event_bus.emitter(); - let mut new_chat_msgs = Vec::new(); for (entity, client) in (&entities, &clients).join() { let _ = super::try_recv_all(client, 1, |client, msg| { Self::handle_client_character_screen_msg( &mut server_emitter, - &mut new_chat_msgs, entity, client, &character_loader, @@ -174,19 +171,5 @@ impl<'a> System<'a> for Sys { ) }); } - - // Handle new chat messages. - for (entity, msg) in new_chat_msgs { - // Handle chat commands. - if msg.message.starts_with('/') { - if let (Some(entity), true) = (entity, msg.message.len() > 1) { - let argv = String::from(&msg.message[1..]); - server_emitter.emit(ServerEvent::ChatCmd(entity, argv)); - } - } else { - // Send chat message - server_emitter.emit(ServerEvent::Chat(msg)); - } - } } } diff --git a/server/src/sys/msg/general.rs b/server/src/sys/msg/general.rs index 5e91e50e51..ca65c29577 100644 --- a/server/src/sys/msg/general.rs +++ b/server/src/sys/msg/general.rs @@ -1,6 +1,6 @@ use crate::{client::Client, metrics::PlayerMetrics}; use common::{ - comp::{ChatMode, Player, UnresolvedChatMsg}, + comp::{ChatMode, Player}, event::{EventBus, ServerEvent}, resources::Time, uid::Uid, @@ -18,7 +18,6 @@ impl Sys { #[allow(clippy::unnecessary_wraps)] fn handle_general_msg( server_emitter: &mut common::event::Emitter<'_, ServerEvent>, - new_chat_msgs: &mut Vec<(Option, UnresolvedChatMsg)>, entity: specs::Entity, client: &Client, player: Option<&Player>, @@ -32,10 +31,17 @@ impl Sys { if player.is_some() { match validate_chat_msg(&message) { Ok(()) => { - if let Some(from) = uids.get(entity) { - let mode = chat_modes.get(entity).cloned().unwrap_or_default(); - let msg = mode.new_message(*from, message); - new_chat_msgs.push((Some(entity), msg)); + if let Some(message) = message.strip_prefix('/') { + if !message.is_empty() { + let argv = String::from(message); + server_emitter.emit(ServerEvent::ChatCmd(entity, argv)); + } + } else if let Some(from) = uids.get(entity) { + const CHAT_MODE_DEFAULT: &ChatMode = &ChatMode::default(); + let mode = chat_modes.get(entity).unwrap_or(CHAT_MODE_DEFAULT); + // Send chat message + server_emitter + .emit(ServerEvent::Chat(mode.new_message(*from, message))); } else { error!("Could not send message. Missing player uid"); } @@ -49,7 +55,7 @@ impl Sys { } }, ClientGeneral::Terminate => { - debug!(?entity, "Client send message to termitate session"); + debug!(?entity, "Client send message to terminate session"); player_metrics .clients_disconnected .with_label_values(&["gracefully"]) @@ -97,13 +103,11 @@ impl<'a> System<'a> for Sys { ): Self::SystemData, ) { let mut server_emitter = server_event_bus.emitter(); - let mut new_chat_msgs = Vec::new(); for (entity, client, player) in (&entities, &clients, (&players).maybe()).join() { let res = super::try_recv_all(client, 3, |client, msg| { Self::handle_general_msg( &mut server_emitter, - &mut new_chat_msgs, entity, client, player, @@ -119,19 +123,5 @@ impl<'a> System<'a> for Sys { *client.last_ping.lock().unwrap() = time.0 } } - - // Handle new chat messages. - for (entity, msg) in new_chat_msgs { - // Handle chat commands. - if msg.message.starts_with('/') { - if let (Some(entity), true) = (entity, msg.message.len() > 1) { - let argv = String::from(&msg.message[1..]); - server_emitter.emit(ServerEvent::ChatCmd(entity, argv)); - } - } else { - // Send chat message - server_emitter.emit(ServerEvent::Chat(msg)); - } - } } } diff --git a/server/src/sys/msg/in_game.rs b/server/src/sys/msg/in_game.rs index ff8fdc3b69..38f49e1d9d 100644 --- a/server/src/sys/msg/in_game.rs +++ b/server/src/sys/msg/in_game.rs @@ -107,7 +107,7 @@ impl Sys { if comp_can_build.enabled { for area in comp_can_build.build_areas.iter() { if let Some(block) = build_areas - .areas + .areas() .get(*area) // TODO: Make this an exclusive check on the upper bound of the AABB // Vek defaults to inclusive which is not optimal @@ -125,7 +125,7 @@ impl Sys { if comp_can_build.enabled { for area in comp_can_build.build_areas.iter() { if build_areas - .areas + .areas() .get(*area) // TODO: Make this an exclusive check on the upper bound of the AABB // Vek defaults to inclusive which is not optimal diff --git a/server/src/sys/sentinel.rs b/server/src/sys/sentinel.rs index 778892724c..4806a27ee9 100644 --- a/server/src/sys/sentinel.rs +++ b/server/src/sys/sentinel.rs @@ -1,8 +1,8 @@ use common::{ comp::{ - Auras, BeamSegment, Body, Buffs, CanBuild, CharacterState, Collider, Combo, Energy, - Gravity, Group, Health, Inventory, Item, LightEmitter, Mass, MountState, Mounting, Ori, - Player, Poise, Pos, Scale, Shockwave, Stats, Sticky, Vel, + item::MaterialStatManifest, Auras, BeamSegment, Body, Buffs, CanBuild, CharacterState, + Collider, Combo, Energy, Gravity, Group, Health, Inventory, Item, LightEmitter, Mass, + MountState, Mounting, Ori, Player, Poise, Pos, Scale, Shockwave, Stats, Sticky, Vel, }, uid::Uid, }; @@ -63,6 +63,7 @@ pub struct TrackedComps<'a> { pub character_state: ReadStorage<'a, CharacterState>, pub shockwave: ReadStorage<'a, Shockwave>, pub beam_segment: ReadStorage<'a, BeamSegment>, + pub msm: ReadExpect<'a, MaterialStatManifest>, } impl<'a> TrackedComps<'a> { pub fn create_entity_package( @@ -71,13 +72,8 @@ impl<'a> TrackedComps<'a> { pos: Option, vel: Option, ori: Option, - ) -> EntityPackage { - let uid = self - .uid - .get(entity) - .copied() - .expect("No uid to create an entity package") - .0; + ) -> Option> { + let uid = self.uid.get(entity).copied()?.0; let mut comps = Vec::new(); self.body.get(entity).copied().map(|c| comps.push(c.into())); self.player @@ -120,7 +116,10 @@ impl<'a> TrackedComps<'a> { .get(entity) .copied() .map(|c| comps.push(c.into())); - self.item.get(entity).cloned().map(|c| comps.push(c.into())); + self.item + .get(entity) + .map(|item| item.duplicate(&self.msm)) + .map(|c| comps.push(c.into())); self.scale .get(entity) .copied() @@ -171,7 +170,7 @@ impl<'a> TrackedComps<'a> { vel.map(|c| comps.push(c.into())); ori.map(|c| comps.push(c.into())); - EntityPackage { uid, comps } + Some(EntityPackage { uid, comps }) } } #[derive(SystemData)] diff --git a/server/src/sys/subscription.rs b/server/src/sys/subscription.rs index 3e132d86e6..9b83dfede7 100644 --- a/server/src/sys/subscription.rs +++ b/server/src/sys/subscription.rs @@ -179,7 +179,7 @@ impl<'a> System<'a> for Sys { // already within the set of subscribed regions if subscription.regions.insert(key) { if let Some(region) = region_map.get(key) { - for (pos, vel, ori, _, entity) in ( + ( &positions, velocities.maybe(), orientations.maybe(), @@ -188,18 +188,19 @@ impl<'a> System<'a> for Sys { ) .join() .filter(|(_, _, _, _, e)| *e != client_entity) - { - // Send message to create entity and tracked components and physics - // components - client.send_fallible(ServerGeneral::CreateEntity( + .filter_map(|(pos, vel, ori, _, entity)| { tracked_comps.create_entity_package( entity, Some(*pos), vel.copied(), ori.copied(), - ), - )); - } + ) + }) + .for_each(|msg| { + // Send message to create entity and tracked components and + // physics components + client.send_fallible(ServerGeneral::CreateEntity(msg)); + }) } } } @@ -228,39 +229,40 @@ pub fn initialize_region_subscription(world: &World, entity: specs::Entity) { let tracked_comps = TrackedComps::fetch(world); for key in ®ions { if let Some(region) = region_map.get(*key) { - for (pos, vel, ori, _, entity) in ( + ( &world.read_storage::(), // We assume all these entities have a position world.read_storage::().maybe(), world.read_storage::().maybe(), region.entities(), &world.entities(), ) - .join() - // Don't send client its own components because we do that below - .filter(|t| t.4 != entity) - { + .join() + // Don't send client its own components because we do that below + .filter(|t| t.4 != entity) + .filter_map(|(pos, vel, ori, _, entity)| + tracked_comps.create_entity_package( + entity, + Some(*pos), + vel.copied(), + ori.copied(), + ) + ) + .for_each(|msg| { // Send message to create entity and tracked components and physics components - client.send_fallible(ServerGeneral::CreateEntity( - tracked_comps.create_entity_package( - entity, - Some(*pos), - vel.copied(), - ori.copied(), - ), - )); - } + client.send_fallible(ServerGeneral::CreateEntity(msg)); + }); } } // If client position was modified it might not be updated in the region system // so we send its components here - client.send_fallible(ServerGeneral::CreateEntity( - tracked_comps.create_entity_package( - entity, - Some(*client_pos), - world.read_storage().get(entity).copied(), - world.read_storage().get(entity).copied(), - ), - )); + if let Some(pkg) = tracked_comps.create_entity_package( + entity, + Some(*client_pos), + world.read_storage().get(entity).copied(), + world.read_storage().get(entity).copied(), + ) { + client.send_fallible(ServerGeneral::CreateEntity(pkg)); + } if let Err(e) = world.write_storage().insert(entity, RegionSubscription { fuzzy_chunk, diff --git a/voxygen/src/menu/main/mod.rs b/voxygen/src/menu/main/mod.rs index a8186acb7e..5e747e8f2f 100644 --- a/voxygen/src/menu/main/mod.rs +++ b/voxygen/src/menu/main/mod.rs @@ -126,6 +126,11 @@ impl PlayState for MainMenuState { localized_strings.get("main.login.server_not_found").into() }, InitError::ClientError(err) => match err { + client::Error::SpecsErr(e) => format!( + "{}: {}", + localized_strings.get("main.login.internal_error"), + e + ), client::Error::AuthErr(e) => format!( "{}: {}", localized_strings.get("main.login.authentication_error"), diff --git a/voxygen/src/mesh/greedy.rs b/voxygen/src/mesh/greedy.rs index 90d1b3e764..f39094fb7a 100644 --- a/voxygen/src/mesh/greedy.rs +++ b/voxygen/src/mesh/greedy.rs @@ -11,7 +11,7 @@ type TodoRect = ( Vec3, ); -pub struct GreedyConfig { +pub struct GreedyConfig { pub data: D, /// The minimum position to mesh, in the coordinate system used /// for queries against the volume. @@ -39,9 +39,6 @@ pub struct GreedyConfig { /// Given a position, return the glow information for the voxel at that /// position (i.e: additional non-sun light). pub get_glow: FG, - /// Given a position, return the color information for the voxel at that - /// position. - pub get_color: FC, /// Given a position, return the opacity information for the voxel at that /// position. Currently, we don't support real translucent lighting, so the /// value should either be `false` (for opaque blocks) or `true` @@ -63,8 +60,8 @@ pub struct GreedyConfig { /// world space, the normal facing out frmo the rectangle in world /// space, and meta information common to every voxel in this rectangle. pub push_quad: FP, - /// Create a texel (in the texture atlas) that corresponds to a face with - /// the given properties. + /// Given a position and the lighting information for a face at that + /// position, return the texel for the face at that position. pub make_face_texel: FT, } @@ -146,17 +143,16 @@ impl<'a> GreedyMesh<'a> { /// Returns an estimate of the bounds of the current meshed model. /// /// For more information on the config parameter, see [GreedyConfig]. - pub fn push( + pub fn push( &mut self, - config: GreedyConfig, + config: GreedyConfig, ) where FL: for<'r> FnMut(&'r mut D, Vec3) -> f32 + 'a, FG: for<'r> FnMut(&'r mut D, Vec3) -> f32 + 'a, - FC: for<'r> FnMut(&'r mut D, Vec3) -> Rgb + 'a, FO: for<'r> FnMut(&'r mut D, Vec3) -> bool + 'a, FS: for<'r> FnMut(&'r mut D, Vec3, Vec3, Vec2>) -> Option<(bool, M)>, FP: FnMut(Vec2, Vec2>, Vec3, Vec2>, Vec3, &M), - FT: for<'r> FnMut(&'r mut D, Vec3, u8, u8, Rgb) -> <::Surface as gfx::format::SurfaceTyped>::DataType + 'a, + FT: for<'r> FnMut(&'r mut D, Vec3, u8, u8) -> <::Surface as gfx::format::SurfaceTyped>::DataType + 'a, { span!(_guard, "push", "GreedyMesh::push"); let cont = greedy_mesh( @@ -194,7 +190,7 @@ impl<'a> GreedyMesh<'a> { pub fn max_size(&self) -> guillotiere::Size { self.max_size } } -fn greedy_mesh<'a, M: PartialEq, D: 'a, FL, FG, FC, FO, FS, FP, FT>( +fn greedy_mesh<'a, M: PartialEq, D: 'a, FL, FG, FO, FS, FP, FT>( atlas: &mut guillotiere::SimpleAtlasAllocator, col_lights_size: &mut Vec2, max_size: guillotiere::Size, @@ -205,21 +201,19 @@ fn greedy_mesh<'a, M: PartialEq, D: 'a, FL, FG, FC, FO, FS, FP, FT>( greedy_size_cross, get_light, get_glow, - get_color, get_opacity, mut should_draw, mut push_quad, make_face_texel, - }: GreedyConfig, + }: GreedyConfig, ) -> Box> where FL: for<'r> FnMut(&'r mut D, Vec3) -> f32 + 'a, FG: for<'r> FnMut(&'r mut D, Vec3) -> f32 + 'a, - FC: for<'r> FnMut(&'r mut D, Vec3) -> Rgb + 'a, FO: for<'r> FnMut(&'r mut D, Vec3) -> bool + 'a, FS: for<'r> FnMut(&'r mut D, Vec3, Vec3, Vec2>) -> Option<(bool, M)>, FP: FnMut(Vec2, Vec2>, Vec3, Vec2>, Vec3, &M), - FT: for<'r> FnMut(&'r mut D, Vec3, u8, u8, Rgb) -> <::Surface as gfx::format::SurfaceTyped>::DataType + 'a, + FT: for<'r> FnMut(&'r mut D, Vec3, u8, u8) -> <::Surface as gfx::format::SurfaceTyped>::DataType + 'a, { span!(_guard, "greedy_mesh"); // TODO: Collect information to see if we can choose a good value here. @@ -357,7 +351,6 @@ where draw_delta, get_light, get_glow, - get_color, get_opacity, make_face_texel, ); @@ -513,9 +506,8 @@ fn draw_col_lights( draw_delta: Vec3, mut get_light: impl FnMut(&mut D, Vec3) -> f32, mut get_glow: impl FnMut(&mut D, Vec3) -> f32, - mut get_color: impl FnMut(&mut D, Vec3) -> Rgb, mut get_opacity: impl FnMut(&mut D, Vec3) -> bool, - mut make_face_texel: impl FnMut(&mut D, Vec3, u8, u8, Rgb) -> <::Surface as gfx::format::SurfaceTyped>::DataType, + mut make_face_texel: impl FnMut(&mut D, Vec3, u8, u8) -> <::Surface as gfx::format::SurfaceTyped>::DataType, ) { todo_rects.into_iter().for_each(|(pos, uv, rect, delta)| { // NOTE: Conversions are safe because width, height, and offset must be @@ -589,10 +581,9 @@ fn draw_col_lights( 0.0 }) / 4.0; - let col = get_color(data, pos); let light = (darkness * 31.5) as u8; let glow = (glowiness * 31.5) as u8; - *col_light = make_face_texel(data, pos, light, glow, col); + *col_light = make_face_texel(data, pos, light, glow); }); }); }); diff --git a/voxygen/src/mesh/segment.rs b/voxygen/src/mesh/segment.rs index e48a4ea756..2ad0ffac24 100644 --- a/voxygen/src/mesh/segment.rs +++ b/voxygen/src/mesh/segment.rs @@ -78,12 +78,6 @@ where } }; let get_glow = |_vol: &mut V, _pos: Vec3| 0.0; - let get_color = |vol: &mut V, pos: Vec3| { - vol.get(pos) - .ok() - .and_then(|vox| vox.get_color()) - .unwrap_or(Rgb::zero()) - }; let get_opacity = |vol: &mut V, pos: Vec3| vol.get(pos).map(|vox| vox.is_empty()).unwrap_or(true); let should_draw = |vol: &mut V, pos: Vec3, delta: Vec3, uv| { @@ -102,7 +96,6 @@ where greedy_size_cross, get_light, get_glow, - get_color, get_opacity, should_draw, push_quad: |atlas_origin, dim, origin, draw_dim, norm, meta: &()| { @@ -116,11 +109,12 @@ where |atlas_pos, pos, norm, &_meta| create_opaque(atlas_pos, pos, norm), )); }, - make_face_texel: |vol: &mut V, pos, light, _, col| { - let (glowy, shiny) = vol - .get(pos) + make_face_texel: |vol: &mut V, pos, light, _| { + let cell = vol.get(pos).ok(); + let (glowy, shiny) = cell .map(|c| (c.is_glowy(), c.is_shiny())) .unwrap_or_default(); + let col = cell.and_then(|vox| vox.get_color()).unwrap_or(Rgb::zero()); TerrainVertex::make_col_light_figure(light, glowy, shiny, col) }, }); @@ -212,7 +206,6 @@ where greedy_size_cross, get_light, get_glow, - get_color, get_opacity, should_draw, push_quad: |atlas_origin, dim, origin, draw_dim, norm, meta: &bool| { @@ -226,8 +219,8 @@ where |atlas_pos, pos, norm, &meta| create_opaque(atlas_pos, pos, norm, meta), )); }, - make_face_texel: |_: &mut V, _, light, glow, col| { - TerrainVertex::make_col_light(light, glow, col) + make_face_texel: move |vol: &mut V, pos, light, glow| { + TerrainVertex::make_col_light(light, glow, get_color(vol, pos)) }, }); @@ -311,7 +304,6 @@ where greedy_size_cross, get_light, get_glow, - get_color, get_opacity, should_draw, push_quad: |atlas_origin, dim, origin, draw_dim, norm, meta: &()| { @@ -325,8 +317,8 @@ where |atlas_pos, pos, norm, &_meta| create_opaque(atlas_pos, pos, norm), )); }, - make_face_texel: |_: &mut V, _, light, glow, col| { - TerrainVertex::make_col_light(light, glow, col) + make_face_texel: move |vol: &mut V, pos, light, glow| { + TerrainVertex::make_col_light(light, glow, get_color(vol, pos)) }, }); diff --git a/voxygen/src/mesh/terrain.rs b/voxygen/src/mesh/terrain.rs index 177bd58e18..fbf1b54750 100644 --- a/voxygen/src/mesh/terrain.rs +++ b/voxygen/src/mesh/terrain.rs @@ -412,7 +412,6 @@ impl<'a, V: RectRasterableVol + ReadVol + Debug + 'static> greedy_size_cross, get_light, get_glow, - get_color, get_opacity, should_draw, push_quad: |atlas_origin, dim, origin, draw_dim, norm, meta: &FaceKind| match meta { @@ -439,8 +438,8 @@ impl<'a, V: RectRasterableVol + ReadVol + Debug + 'static> )); }, }, - make_face_texel: |_: &mut (), _, light, glow, col| { - TerrainVertex::make_col_light(light, glow, col) + make_face_texel: |data: &mut (), pos, light, glow| { + TerrainVertex::make_col_light(light, glow, get_color(data, pos)) }, });