From 2eb7d366d973f9636327cb7b65d27244deb1d445 Mon Sep 17 00:00:00 2001 From: Imbris Date: Thu, 9 Mar 2023 22:37:45 -0500 Subject: [PATCH 01/13] Always try to remove a block if it is bonked, even if we can't reclaim an item from it! --- server/src/events/entity_manipulation.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/server/src/events/entity_manipulation.rs b/server/src/events/entity_manipulation.rs index 74354f083c..f7c4bc4b47 100644 --- a/server/src/events/entity_manipulation.rs +++ b/server/src/events/entity_manipulation.rs @@ -1039,13 +1039,13 @@ pub fn handle_bonk(server: &mut Server, pos: Vec3, owner: Option, targ use common::terrain::SpriteKind; let pos = pos.map(|e| e.floor() as i32); if let Some(block) = terrain.get(pos).ok().copied().filter(|b| b.is_bonkable()) { - if let Some(item) = comp::Item::try_reclaim_from_block(block) { - if block_change - .try_set(pos, block.with_sprite(SpriteKind::Empty)) - .is_some() - { - drop(terrain); - drop(block_change); + if block_change + .try_set(pos, block.with_sprite(SpriteKind::Empty)) + .is_some() + { + drop(terrain); + drop(block_change); + if let Some(item) = comp::Item::try_reclaim_from_block(block) { server .state .create_object(Default::default(), match block.get_sprite() { @@ -1064,7 +1064,7 @@ pub fn handle_bonk(server: &mut Server, pos: Vec3, owner: Option, targ }) .build(); } - }; + } } } } From fe38b9b92b1af5c4c95e28490cdeab025d78e703 Mon Sep 17 00:00:00 2001 From: Imbris Date: Thu, 9 Mar 2023 23:01:37 -0500 Subject: [PATCH 02/13] Add documentation to bool field --- common/src/comp/controller.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/common/src/comp/controller.rs b/common/src/comp/controller.rs index 2a2758f15b..9c049ac97f 100644 --- a/common/src/comp/controller.rs +++ b/common/src/comp/controller.rs @@ -45,6 +45,7 @@ pub enum InventoryManip { Pickup(Uid), Collect { sprite_pos: Vec3, + /// If second field is `true`, item will be consumed on collection. required_item: Option<(InvSlotId, bool)>, }, Use(Slot), From 349d1726a69fc70a112833e9c70f34da2ebead59 Mon Sep 17 00:00:00 2001 From: Imbris Date: Thu, 9 Mar 2023 23:30:06 -0500 Subject: [PATCH 03/13] Only emit SpriteUnlocked outcome if the sprite interaction kind is Unlock. Also rustfmt decided to make a bunch of changes. --- common/src/states/sprite_interact.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/common/src/states/sprite_interact.rs b/common/src/states/sprite_interact.rs index e2c39c94d9..0151687741 100644 --- a/common/src/states/sprite_interact.rs +++ b/common/src/states/sprite_interact.rs @@ -129,11 +129,13 @@ impl CharacterBehavior for Data { }; output_events .emit_server(ServerEvent::InventoryManip(data.entity, inv_manip)); - output_events.emit_local(LocalEvent::CreateOutcome( - Outcome::SpriteUnlocked { - pos: self.static_data.sprite_pos, - }, - )); + if matches!(self.static_data.sprite_kind, SpriteInteractKind::Unlock) { + output_events.emit_local(LocalEvent::CreateOutcome( + Outcome::SpriteUnlocked { + pos: self.static_data.sprite_pos, + }, + )); + } } // Done end_ability(data, &mut update); From 93eab4791d5f255312e6e624795da5147f467b89 Mon Sep 17 00:00:00 2001 From: Imbris Date: Thu, 9 Mar 2023 23:46:24 -0500 Subject: [PATCH 04/13] Remove ComponentKey TODO in Inventory::get_slot_of_item since item_definition_id contains component IDs! --- common/src/comp/inventory/mod.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/common/src/comp/inventory/mod.rs b/common/src/comp/inventory/mod.rs index 6eed4ace70..f3bb929f4d 100644 --- a/common/src/comp/inventory/mod.rs +++ b/common/src/comp/inventory/mod.rs @@ -400,11 +400,6 @@ impl Inventory { self.slots_with_id() .find(|&(_, it)| { if let Some(it) = it { - if it.components().len() == item.components().len() { - // TODO: add a ComponentKey struct to compare components, see issue #1226 - debug_assert!(it.components().is_empty()); - debug_assert!(item.components().is_empty()); - } it.item_definition_id() == item.item_definition_id() } else { false From 0d8aa16d89cfcc3844d9906d78552b4a827e9de4 Mon Sep 17 00:00:00 2001 From: Imbris Date: Fri, 10 Mar 2023 01:33:20 -0500 Subject: [PATCH 05/13] Avoid duplicate searches in the inventory for required items when interacting with sprites and rustfmt decides to format a bunch of stuff... * Add PartialEq impls between ItemDefinitionId<'_> and ItemDefinitionIdOwned. * Remove unused Serialize and Deserialize derives from ItemDefinitionId<'_> * Add Inventory::get_slot_of_item_by_def_id which acts like Inventory::get_slot_of_item but accepts a ItemDefinitionIdOwned reference instead of an Item reference. * Add some TODOs for some potential optimizations * Rustfmt decided now was the time to format some random stuff I didn't touch. Maybe I fixed something it was getting stuck on???? But some files I didn't make any changes (although might have inadvertantly saved them when viewing in editor (with fmt on save)). * InvSlotId passed to SpriteInteract character state instead of refinding the item in the inventory (if it moved we simply give up on the state as if the requirements weren't met). (overall in this change 3 searches for the item in the inventory are reduced to a single one) --- common/src/comp/inventory/item/mod.rs | 37 ++++++++++++++++- common/src/comp/inventory/mod.rs | 15 +++++++ common/src/comp/inventory/trade_pricing.rs | 5 ++- common/src/states/sprite_interact.rs | 48 +++++++++------------- common/src/states/utils.rs | 34 ++++++++------- 5 files changed, 93 insertions(+), 46 deletions(-) diff --git a/common/src/comp/inventory/item/mod.rs b/common/src/comp/inventory/item/mod.rs index 3766512560..f9a5ee7b72 100644 --- a/common/src/comp/inventory/item/mod.rs +++ b/common/src/comp/inventory/item/mod.rs @@ -462,7 +462,10 @@ impl ItemBase { } } -#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq, Hash)] +// TODO: could this theorectically hold a ref to the actual components and +// lazily get their IDs for hash/partialeq/debug/to_owned/etc? (i.e. eliminating +// `Vec`s) +#[derive(Clone, Debug, PartialEq, Eq, Hash)] pub enum ItemDefinitionId<'a> { Simple(&'a str), Modular { @@ -1291,6 +1294,38 @@ pub fn try_all_item_defs() -> Result, Error> { Ok(defs.ids().map(|id| id.to_string()).collect()) } +impl PartialEq> for ItemDefinitionIdOwned { + fn eq(&self, other: &ItemDefinitionId<'_>) -> bool { + use ItemDefinitionId as DefId; + match self { + Self::Simple(simple) => { + matches!(other, DefId::Simple(other_simple) if simple == other_simple) + }, + Self::Modular { + pseudo_base, + components, + } => matches!( + other, + DefId::Modular { pseudo_base: other_base, components: other_comps } + if pseudo_base == other_base && components == other_comps + ), + Self::Compound { + simple_base, + components, + } => matches!( + other, + DefId::Compound { simple_base: other_base, components: other_comps } + if simple_base == other_base && components == other_comps + ), + } + } +} + +impl PartialEq for ItemDefinitionId<'_> { + #[inline] + fn eq(&self, other: &ItemDefinitionIdOwned) -> bool { other == self } +} + #[cfg(test)] mod tests { use super::*; diff --git a/common/src/comp/inventory/mod.rs b/common/src/comp/inventory/mod.rs index f3bb929f4d..4eff57e20c 100644 --- a/common/src/comp/inventory/mod.rs +++ b/common/src/comp/inventory/mod.rs @@ -408,6 +408,21 @@ impl Inventory { .map(|(slot, _)| slot) } + pub fn get_slot_of_item_by_def_id( + &self, + item_def_id: &item::ItemDefinitionIdOwned, + ) -> Option { + self.slots_with_id() + .find(|&(_, it)| { + if let Some(it) = it { + it.item_definition_id() == *item_def_id + } else { + false + } + }) + .map(|(slot, _)| slot) + } + /// Get content of a slot pub fn get(&self, inv_slot_id: InvSlotId) -> Option<&Item> { self.slot(inv_slot_id).and_then(Option::as_ref) diff --git a/common/src/comp/inventory/trade_pricing.rs b/common/src/comp/inventory/trade_pricing.rs index 58024af6b9..a98ec5ca96 100644 --- a/common/src/comp/inventory/trade_pricing.rs +++ b/common/src/comp/inventory/trade_pricing.rs @@ -456,6 +456,7 @@ struct EqualitySet { impl EqualitySet { fn canonical<'a>(&'a self, item_name: &'a ItemDefinitionIdOwned) -> &'a ItemDefinitionIdOwned { + // TODO: use hashbrown Equivalent trait to avoid needing owned item def here let canonical_itemname = self .equivalence_class .get(item_name) @@ -996,7 +997,7 @@ impl TradePricing { result } - fn get_materials_impl(&self, item: &ItemDefinitionId) -> Option { + fn get_materials_impl(&self, item: &ItemDefinitionId<'_>) -> Option { self.price_lookup(&item.to_owned()).cloned() } @@ -1012,7 +1013,7 @@ impl TradePricing { } #[must_use] - pub fn get_materials(item: &ItemDefinitionId) -> Option { + pub fn get_materials(item: &ItemDefinitionId<'_>) -> Option { TRADE_PRICING.get_materials_impl(item) } diff --git a/common/src/states/sprite_interact.rs b/common/src/states/sprite_interact.rs index 0151687741..78e7673b65 100644 --- a/common/src/states/sprite_interact.rs +++ b/common/src/states/sprite_interact.rs @@ -1,8 +1,7 @@ use super::utils::*; use crate::{ comp::{ - character_state::OutputEvents, - item::{Item, ItemDefinitionIdOwned}, + character_state::OutputEvents, inventory::slot::InvSlotId, item::ItemDefinitionIdOwned, CharacterState, InventoryManip, StateUpdate, }, event::{LocalEvent, ServerEvent}, @@ -33,8 +32,13 @@ pub struct StaticData { /// Was sneaking pub was_sneak: bool, /// The item required to interact with the sprite, if one was required - // If second field is true, item should be consumed on collection - pub required_item: Option<(ItemDefinitionIdOwned, bool)>, + /// + /// The second field is the slot that the required item was in when this + /// state was created. If it isn't in this slot anymore the interaction will + /// fail. + /// + /// If third field is true, item should be consumed on collection + pub required_item: Option<(ItemDefinitionIdOwned, InvSlotId, bool)>, /// Miscellaneous information about the ability pub ability_info: AbilityInfo, } @@ -97,32 +101,20 @@ impl CharacterBehavior for Data { } } else { // Create inventory manipulation event - let required_item = - self.static_data - .required_item - .as_ref() - .and_then(|(i, consume)| { - Some(( - Item::new_from_item_definition_id( - i.as_ref(), - data.ability_map, - data.msm, - ) - .ok()?, - *consume, - )) - }); - let has_required_item = - required_item.as_ref().map_or(true, |(item, _consume)| { - data.inventory.map_or(false, |inv| inv.contains(item)) + let (has_required_item, inv_slot) = self + .static_data + .required_item + .as_ref() + .map_or((true, None), |&(ref item_def_id, slot, consume)| { + // Check that required item is still in expected slot + let has_item = data + .inventory + .and_then(|inv| inv.get(slot)) + .map_or(false, |item| item.item_definition_id() == *item_def_id); + + (has_item, has_item.then_some((slot, consume))) }); if has_required_item { - let inv_slot = required_item.and_then(|(item, consume)| { - Some(( - data.inventory.and_then(|inv| inv.get_slot_of_item(&item))?, - consume, - )) - }); let inv_manip = InventoryManip::Collect { sprite_pos: self.static_data.sprite_pos, required_item: inv_slot, diff --git a/common/src/states/utils.rs b/common/src/states/utils.rs index 9732bf92d8..50996c256c 100644 --- a/common/src/states/utils.rs +++ b/common/src/states/utils.rs @@ -7,7 +7,7 @@ use crate::{ character_state::OutputEvents, controller::InventoryManip, inventory::slot::{ArmorSlot, EquipSlot, Slot}, - item::{armor::Friction, tool::AbilityContext, Hands, Item, ItemKind, ToolKind}, + item::{armor::Friction, tool::AbilityContext, Hands, ItemKind, ToolKind}, quadruped_low, quadruped_medium, quadruped_small, skills::{Skill, SwimSkill, SKILL_MODIFIERS}, theropod, Body, CharacterAbility, CharacterState, Density, InputAttr, InputKind, @@ -918,24 +918,28 @@ pub fn handle_manipulate_loadout( UnlockKind::Requires(item) => Some((item, false)), UnlockKind::Consumes(item) => Some((item, true)), }); - let has_required_items = required_item - .as_ref() - .and_then(|(i, _consume)| { - Item::new_from_item_definition_id( - i.as_ref(), - data.ability_map, - data.msm, - ) - .ok() - }) - .map_or(true, |i| { - data.inventory.map_or(false, |inv| inv.contains(&i)) - }); + + // None: An required items exist but no available + // Some(None): No required items + // Some(Some(_)): Required items satisfied, contains info about them + let has_required_items = match required_item { + Some((item_id, consume)) => { + if let Some(slot) = data + .inventory + .and_then(|inv| inv.get_slot_of_item_by_def_id(&item_id)) + { + Some(Some((item_id, slot, consume))) + } else { + None + } + }, + None => Some(None), + }; // If path can be found between entity interacting with sprite and entity, start // interaction with sprite if not_blocked_by_terrain { - if has_required_items { + if let Some(required_item) = has_required_items { // If the sprite is collectible, enter the sprite interaction character // state TODO: Handle cases for sprite being // interactible, but not collectible (none currently From 2d66145620c1d4ea95a26f96d9316f72bbe372ab Mon Sep 17 00:00:00 2001 From: Imbris Date: Fri, 10 Mar 2023 02:24:09 -0500 Subject: [PATCH 06/13] Add some documentation to pos_key/key_pos/pos_chunk methods which are easy to mix up --- common/src/volumes/vol_grid_2d.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/common/src/volumes/vol_grid_2d.rs b/common/src/volumes/vol_grid_2d.rs index b77ac13250..70e4c2459a 100644 --- a/common/src/volumes/vol_grid_2d.rs +++ b/common/src/volumes/vol_grid_2d.rs @@ -221,12 +221,19 @@ impl VolGrid2d { pub fn remove(&mut self, key: Vec2) -> Option> { self.chunks.remove(&key) } + /// Converts a chunk key (i.e. coordinates in terms of chunks) into a + /// position in the world (aka "wpos"). + /// + /// The returned position will be in the corner of the chunk. #[inline(always)] pub fn key_pos(&self, key: Vec2) -> Vec2 { Self::key_chunk(key) } + /// Converts a position in the world into a chunk key (i.e. coordinates in + /// terms of chunks). #[inline(always)] pub fn pos_key(&self, pos: Vec3) -> Vec2 { Self::chunk_key(pos) } + /// Gets the chunk that contains the provided world position. #[inline(always)] pub fn pos_chunk(&self, pos: Vec3) -> Option<&V> { self.get_key(self.pos_key(pos)) } From dd6e6ccf919d8f1ffeba4657f01936e982926707 Mon Sep 17 00:00:00 2001 From: Imbris Date: Fri, 10 Mar 2023 02:50:08 -0500 Subject: [PATCH 07/13] Add Vec3::one * 0.5 to sprite item drop position instead of Vec3::unit_z, this should put the new enity in the center of the voxel that the sprite occupied --- server/src/events/inventory_manip.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/events/inventory_manip.rs b/server/src/events/inventory_manip.rs index f830877de8..770f2eb2b7 100644 --- a/server/src/events/inventory_manip.rs +++ b/server/src/events/inventory_manip.rs @@ -370,7 +370,7 @@ pub fn handle_inventory(server: &mut Server, entity: EcsEntity, manip: comp::Inv sprite_pos.x as f32, sprite_pos.y as f32, sprite_pos.z as f32, - ) + Vec3::unit_z(), + ) + Vec3::one() * 0.5, )) .with(comp::Vel(Vec3::zero())) .build(); From 475ae65d16f40ee2af569bdad683812cee365513 Mon Sep 17 00:00:00 2001 From: Imbris Date: Fri, 10 Mar 2023 19:24:09 -0500 Subject: [PATCH 08/13] Avoid extra StructureBlock clones in layer/tree.rs --- world/src/layer/tree.rs | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/world/src/layer/tree.rs b/world/src/layer/tree.rs index 75c9639050..1fbe4ea8b4 100644 --- a/world/src/layer/tree.rs +++ b/world/src/layer/tree.rs @@ -271,19 +271,24 @@ pub fn apply_trees_to( }) .sum(), ) + Vec3::unit_z() * (wpos.z - tree.pos.z); + let sblock; block_from_structure( info.index(), - &if let Some(block) = match &tree.model { - TreeModel::Structure(s) => s.get(model_pos).ok().cloned(), + if let Some(block) = match &tree.model { + TreeModel::Structure(s) => s.get(model_pos).ok(), TreeModel::Procedural(t, leaf_block) => Some( match t.is_branch_or_leaves_at(model_pos.map(|e| e as f32 + 0.5)) { (_, _, true, _) => { - StructureBlock::Filled(BlockKind::Wood, Rgb::new(150, 98, 41)) + sblock = StructureBlock::Filled( + BlockKind::Wood, + Rgb::new(150, 98, 41), + ); + &sblock }, - (_, _, _, true) => StructureBlock::None, - (true, _, _, _) => t.config.trunk_block.clone(), - (_, true, _, _) => leaf_block.clone(), - _ => StructureBlock::None, + (_, _, _, true) => &StructureBlock::None, + (true, _, _, _) => &t.config.trunk_block, + (_, true, _, _) => &leaf_block, + _ => &StructureBlock::None, }, ), } { From 3ad929762385f1a277321488f4a3c450aabf7b70 Mon Sep 17 00:00:00 2001 From: Imbris Date: Fri, 10 Mar 2023 21:31:22 -0500 Subject: [PATCH 09/13] Ensure outcome event bus is cleared on the client since those are unused, and add a TODO about either using them or not generating them --- client/src/lib.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/client/src/lib.rs b/client/src/lib.rs index 34812b897b..a646260a75 100644 --- a/client/src/lib.rs +++ b/client/src/lib.rs @@ -1794,6 +1794,13 @@ impl Client { .ecs() .fetch::>() .recv_all(); + // TODO: avoid emitting these in the first place OR actually use outcomes + // generated locally on the client (if they can be deduplicated from + // ones that the server generates or if the client can reliably generate + // them (e.g. syncing skipping character states past certain + // stages might skip points where outcomes are generated, however we might not + // care about this?) and the server doesn't need to send them) + let _ = self.state.ecs().fetch::>().recv_all(); // 5) Terrain self.tick_terrain()?; From 619f62cb638c52ec92f5fcbc0beafcf8ca0f6dfb Mon Sep 17 00:00:00 2001 From: Imbris Date: Fri, 10 Mar 2023 21:52:14 -0500 Subject: [PATCH 10/13] Use try_set for door destruction to avoid accidentally overwriting any other changes that already occured that tick. Also get terrain/block change resources from the ecs once to avoid the overhead of fetching them from the ecs (and aquiring/releasing runtime borrow). --- common/state/src/state.rs | 10 ++++----- server/src/events/inventory_manip.rs | 33 +++++++++++++++------------- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/common/state/src/state.rs b/common/state/src/state.rs index 96ac72f74e..79b82ca378 100644 --- a/common/state/src/state.rs +++ b/common/state/src/state.rs @@ -68,6 +68,10 @@ impl BlockChange { } } + /// Check if the block at given position `pos` has already been modified + /// this tick. + pub fn can_set_block(&self, pos: Vec3) -> bool { !self.blocks.contains_key(&pos) } + pub fn clear(&mut self) { self.blocks.clear(); } } @@ -461,11 +465,7 @@ impl State { /// Check if the block at given position `pos` has already been modified /// this tick. pub fn can_set_block(&self, pos: Vec3) -> bool { - !self - .ecs - .read_resource::() - .blocks - .contains_key(&pos) + self.ecs.read_resource::().can_set_block(pos) } /// Removes every chunk of the terrain. diff --git a/server/src/events/inventory_manip.rs b/server/src/events/inventory_manip.rs index 770f2eb2b7..cf96baee5d 100644 --- a/server/src/events/inventory_manip.rs +++ b/server/src/events/inventory_manip.rs @@ -252,17 +252,21 @@ pub fn handle_inventory(server: &mut Server, entity: EcsEntity, manip: comp::Inv sprite_pos, required_item, } => { - let block = state.terrain().get(sprite_pos).ok().copied(); + let ecs = state.ecs(); + let terrain = ecs.read_resource::(); + let mut block_change = ecs.write_resource::(); + + let block = terrain.get(sprite_pos).ok().copied(); let mut drop_item = None; if let Some(block) = block { - if block.is_collectible() && state.can_set_block(sprite_pos) { + if block.is_collectible() && block_change.can_set_block(sprite_pos) { // If an item was required to collect the sprite, consume it now if let Some((inv_slot, true)) = required_item { inventory.take( inv_slot, - &state.ecs().read_resource::(), - &state.ecs().read_resource::(), + &ecs.read_resource::(), + &ecs.read_resource::(), ); } @@ -270,20 +274,19 @@ pub fn handle_inventory(server: &mut Server, entity: EcsEntity, manip: comp::Inv if let Some(item) = comp::Item::try_reclaim_from_block(block) { // NOTE: We dup the item for message purposes. let item_msg = item.duplicate( - &state.ecs().read_resource::(), - &state.ecs().read_resource::(), + &ecs.read_resource::(), + &ecs.read_resource::(), ); let event = match inventory.push(item) { Ok(_) => { - let ecs = state.ecs(); if let Some(group_id) = ecs.read_storage::().get(entity) { announce_loot_to_group( group_id, ecs, entity, item_msg.duplicate( - &state.ecs().read_resource::(), - &state.ecs().read_resource::(), + &ecs.read_resource::(), + &ecs.read_resource::(), ), ); } @@ -303,15 +306,13 @@ pub fn handle_inventory(server: &mut Server, entity: EcsEntity, manip: comp::Inv ) }, }; - state - .ecs() - .write_storage() + ecs.write_storage() .insert(entity, event) .expect("We know entity exists since we got its inventory."); } // We made sure earlier the block was not already modified this tick - state.set_block(sprite_pos, block.into_vacant()); + block_change.set(sprite_pos, block.into_vacant()); // If the block was a keyhole, remove nearby door blocks // TODO: Abstract this code into a generalised way to do block updates? @@ -339,11 +340,11 @@ pub fn handle_inventory(server: &mut Server, entity: EcsEntity, manip: comp::Inv pending.remove(&pos); if !destroyed.contains(&pos) && matches!( - state.terrain().get(pos).ok().and_then(|b| b.get_sprite()), + terrain.get(pos).ok().and_then(|b| b.get_sprite()), Some(SpriteKind::KeyDoor) ) { - state.set_block(pos, Block::empty()); + block_change.try_set(pos, Block::empty()); destroyed.insert(pos); pending.extend(dirs.into_iter().map(|dir| pos + dir)); @@ -362,6 +363,8 @@ pub fn handle_inventory(server: &mut Server, entity: EcsEntity, manip: comp::Inv } } drop(inventories); + drop(terrain); + drop(block_change); if let Some(item) = drop_item { state .create_item_drop(Default::default(), item) From 19b5ed348746bd55cc54f6defdaada65cd8e5bf9 Mon Sep 17 00:00:00 2001 From: Imbris Date: Sat, 11 Mar 2023 11:06:08 -0500 Subject: [PATCH 11/13] Appease clippy --- common/src/states/utils.rs | 15 +++++---------- world/src/layer/tree.rs | 2 +- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/common/src/states/utils.rs b/common/src/states/utils.rs index 50996c256c..3ff9b0340b 100644 --- a/common/src/states/utils.rs +++ b/common/src/states/utils.rs @@ -923,16 +923,11 @@ pub fn handle_manipulate_loadout( // Some(None): No required items // Some(Some(_)): Required items satisfied, contains info about them let has_required_items = match required_item { - Some((item_id, consume)) => { - if let Some(slot) = data - .inventory - .and_then(|inv| inv.get_slot_of_item_by_def_id(&item_id)) - { - Some(Some((item_id, slot, consume))) - } else { - None - } - }, + // Produces `None` if we can't find the item or `Some(Some(_))` if we can + Some((item_id, consume)) => data + .inventory + .and_then(|inv| inv.get_slot_of_item_by_def_id(&item_id)) + .map(|slot| Some((item_id, slot, consume))), None => Some(None), }; diff --git a/world/src/layer/tree.rs b/world/src/layer/tree.rs index 1fbe4ea8b4..836be68044 100644 --- a/world/src/layer/tree.rs +++ b/world/src/layer/tree.rs @@ -287,7 +287,7 @@ pub fn apply_trees_to( }, (_, _, _, true) => &StructureBlock::None, (true, _, _, _) => &t.config.trunk_block, - (_, true, _, _) => &leaf_block, + (_, true, _, _) => leaf_block, _ => &StructureBlock::None, }, ), From 6b8e22d6cc55a303559fba0eed5f3729ee35c699 Mon Sep 17 00:00:00 2001 From: Imbris Date: Sat, 11 Mar 2023 21:07:10 -0500 Subject: [PATCH 12/13] Improvements and fixes for interacting/collecting * Inert entities like arrows no longer block interactions like picking up items! Logic looking for the closest entity will skip them. * When pickaxe is not equipped and wielded we now show "Needs Pickaxe" as the hint text for mineable blocks. * Mineable blocks that aren't pointed at now show the mining text hint instead of the text hint used for regular collectible blocks. * Fixed recent bug where all interactables were showing the open text hint. * Split `BlockInteraction` out of the `Interaction` enum in voxygen since we were using this enum for two different things. --- CHANGELOG.md | 2 + assets/voxygen/i18n/en/hud/misc.ftl | 1 + common/src/states/utils.rs | 9 +- voxygen/src/hud/mod.rs | 84 ++++++++++------ voxygen/src/hud/overitem.rs | 28 +++--- voxygen/src/scene/terrain/watcher.rs | 20 +--- voxygen/src/session/interactable.rs | 137 ++++++++++++++++++++------- voxygen/src/session/mod.rs | 13 +-- voxygen/src/session/target.rs | 3 +- 9 files changed, 189 insertions(+), 108 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d59b08a17..ca830cfa94 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Lamps, embers and campfires use glowing indices - Non-potion drinks no longer heal as much as potions. - Added SFX to the new sword abilities +- Fixed various issues with showing the correct text hint for interactable blocks. +- Intert entities like arrows no longer obstruct interacting with nearby entities/blocks. ## [0.14.0] - 2023-01-07 diff --git a/assets/voxygen/i18n/en/hud/misc.ftl b/assets/voxygen/i18n/en/hud/misc.ftl index 54c4a4612a..bf765653c9 100644 --- a/assets/voxygen/i18n/en/hud/misc.ftl +++ b/assets/voxygen/i18n/en/hud/misc.ftl @@ -45,6 +45,7 @@ hud-use = Use hud-unlock-requires = Open with { $item } hud-unlock-consumes = Use { $item } to open hud-mine = Mine +hud-needs_pickaxe = Needs Pickaxe hud-talk = Talk hud-trade = Trade hud-mount = Mount diff --git a/common/src/states/utils.rs b/common/src/states/utils.rs index 3ff9b0340b..4df839538b 100644 --- a/common/src/states/utils.rs +++ b/common/src/states/utils.rs @@ -17,9 +17,9 @@ use crate::{ event::{LocalEvent, ServerEvent}, outcome::Outcome, states::{behavior::JoinData, utils::CharacterState::Idle, *}, - terrain::{TerrainChunkSize, UnlockKind}, + terrain::{TerrainGrid, UnlockKind}, util::Dir, - vol::{ReadVol, RectVolSize}, + vol::ReadVol, }; use core::hash::BuildHasherDefault; use fxhash::FxHasher64; @@ -844,10 +844,7 @@ pub fn handle_manipulate_loadout( if close_to_sprite { // First, get sprite data for position, if there is a sprite use sprite_interact::SpriteInteractKind; - let sprite_chunk_pos = sprite_pos - .xy() - .map2(TerrainChunkSize::RECT_SIZE, |e, sz| e.rem_euclid(sz as i32)) - .with_z(sprite_pos.z); + let sprite_chunk_pos = TerrainGrid::chunk_offs(sprite_pos); let sprite_cfg = data .terrain .pos_chunk(sprite_pos) diff --git a/voxygen/src/hud/mod.rs b/voxygen/src/hud/mod.rs index 159fb4867c..935c222d0c 100644 --- a/voxygen/src/hud/mod.rs +++ b/voxygen/src/hud/mod.rs @@ -65,12 +65,9 @@ use crate::{ game_input::GameInput, hud::{img_ids::ImgsRot, prompt_dialog::DialogOutcomeEvent}, render::UiDrawer, - scene::{ - camera::{self, Camera}, - terrain::Interaction, - }, + scene::camera::{self, Camera}, session::{ - interactable::Interactable, + interactable::{BlockInteraction, Interactable}, settings_change::{ Audio, Chat as ChatChange, Interface as InterfaceChange, SettingsChange, }, @@ -673,6 +670,7 @@ pub struct DebugInfo { pub struct HudInfo { pub is_aiming: bool, + pub is_mining: bool, pub is_first_person: bool, pub viewpoint_entity: specs::Entity, pub mutable_viewpoint: bool, @@ -2019,7 +2017,10 @@ impl Hud { pickup_failed_pulse: self.failed_entity_pickups.get(&entity).cloned(), }, &self.fonts, - vec![(GameInput::Interact, i18n.get_msg("hud-pick_up").to_string())], + vec![( + Some(GameInput::Interact), + i18n.get_msg("hud-pick_up").to_string(), + )], ) .set(overitem_id, ui_widgets); } @@ -2039,31 +2040,46 @@ impl Hud { let over_pos = pos + Vec3::unit_z() * 0.7; let interaction_text = || match interaction { - Interaction::Collect => { - vec![(GameInput::Interact, i18n.get_msg("hud-collect").to_string())] + BlockInteraction::Collect => { + vec![( + Some(GameInput::Interact), + i18n.get_msg("hud-collect").to_string(), + )] }, - Interaction::Craft(_) => { - vec![(GameInput::Interact, i18n.get_msg("hud-use").to_string())] + BlockInteraction::Craft(_) => { + vec![( + Some(GameInput::Interact), + i18n.get_msg("hud-use").to_string(), + )] }, - Interaction::Unlock(kind) => vec![(GameInput::Interact, match kind { - UnlockKind::Free => i18n.get_msg("hud-open").to_string(), - UnlockKind::Requires(item) => i18n - .get_msg_ctx("hud-unlock-requires", &i18n::fluent_args! { - "item" => item.as_ref().itemdef_id() - .map(|id| Item::new_from_asset_expect(id).describe()) - .unwrap_or_else(|| "modular item".to_string()), - }) - .to_string(), - UnlockKind::Consumes(item) => i18n - .get_msg_ctx("hud-unlock-requires", &i18n::fluent_args! { - "item" => item.as_ref().itemdef_id() - .map(|id| Item::new_from_asset_expect(id).describe()) - .unwrap_or_else(|| "modular item".to_string()), - }) - .to_string(), - })], - Interaction::Mine => { - vec![(GameInput::Primary, i18n.get_msg("hud-mine").to_string())] + BlockInteraction::Unlock(kind) => { + vec![(Some(GameInput::Interact), match kind { + UnlockKind::Free => i18n.get_msg("hud-open").to_string(), + UnlockKind::Requires(item) => i18n + .get_msg_ctx("hud-unlock-requires", &i18n::fluent_args! { + "item" => item.as_ref().itemdef_id() + .map(|id| Item::new_from_asset_expect(id).describe()) + .unwrap_or_else(|| "modular item".to_string()), + }) + .to_string(), + UnlockKind::Consumes(item) => i18n + .get_msg_ctx("hud-unlock-requires", &i18n::fluent_args! { + "item" => item.as_ref().itemdef_id() + .map(|id| Item::new_from_asset_expect(id).describe()) + .unwrap_or_else(|| "modular item".to_string()), + }) + .to_string(), + })] + }, + BlockInteraction::Mine => { + if info.is_mining { + vec![( + Some(GameInput::Primary), + i18n.get_msg("hud-mine").to_string(), + )] + } else { + vec![(None, i18n.get_msg("hud-needs_pickaxe").to_string())] + } }, }; @@ -2083,7 +2099,10 @@ impl Hud { overitem_properties, self.pulse, &global_state.window.key_layout, - vec![(GameInput::Interact, i18n.get_msg("hud-open").to_string())], + vec![( + Some(GameInput::Interact), + i18n.get_msg("hud-open").to_string(), + )], ) .x_y(0.0, 100.0) .position_ingame(over_pos) @@ -2152,7 +2171,10 @@ impl Hud { overitem_properties, self.pulse, &global_state.window.key_layout, - vec![(GameInput::Interact, i18n.get_msg("hud-sit").to_string())], + vec![( + Some(GameInput::Interact), + i18n.get_msg("hud-sit").to_string(), + )], ) .x_y(0.0, 100.0) .position_ingame(over_pos) diff --git a/voxygen/src/hud/overitem.rs b/voxygen/src/hud/overitem.rs index 00112ab29b..e12e71f697 100644 --- a/voxygen/src/hud/overitem.rs +++ b/voxygen/src/hud/overitem.rs @@ -46,7 +46,8 @@ pub struct Overitem<'a> { properties: OveritemProperties, pulse: f32, key_layout: &'a Option, - interaction_options: Vec<(GameInput, String)>, + // GameInput optional so we can just show stuff like "needs pickaxe" + interaction_options: Vec<(Option, String)>, } impl<'a> Overitem<'a> { @@ -60,7 +61,7 @@ impl<'a> Overitem<'a> { properties: OveritemProperties, pulse: f32, key_layout: &'a Option, - interaction_options: Vec<(GameInput, String)>, + interaction_options: Vec<(Option, String)>, ) -> Self { Self { name, @@ -177,19 +178,20 @@ impl<'a> Widget for Overitem<'a> { .interaction_options .iter() .filter_map(|(input, action)| { - Some(( - self.controls - .get_binding(*input) - .filter(|_| self.properties.active)?, - action, - )) + let binding = if let Some(input) = input { + Some(self.controls.get_binding(*input)?) + } else { + None + }; + Some((binding, action)) }) .map(|(input, action)| { - format!( - "{} {}", - input.display_string(self.key_layout).as_str(), - action - ) + if let Some(input) = input { + let input = input.display_string(self.key_layout); + format!("{} {action}", input.as_str()) + } else { + action.to_string() + } }) .collect::>() .join("\n"); diff --git a/voxygen/src/scene/terrain/watcher.rs b/voxygen/src/scene/terrain/watcher.rs index 0edc35c696..b783cb07cc 100644 --- a/voxygen/src/scene/terrain/watcher.rs +++ b/voxygen/src/scene/terrain/watcher.rs @@ -1,16 +1,16 @@ use crate::hud::CraftingTab; -use common::terrain::{BlockKind, SpriteKind, TerrainChunk, UnlockKind}; +use common::terrain::{BlockKind, SpriteKind, TerrainChunk}; use common_base::span; use rand::prelude::*; use rand_chacha::ChaCha8Rng; use vek::*; -#[derive(Clone, Debug)] +#[derive(Clone, Copy, Debug)] pub enum Interaction { + /// This covers mining, unlocking, and regular collectable things (e.g. + /// twigs). Collect, - Unlock(UnlockKind), Craft(CraftingTab), - Mine, } pub enum FireplaceType { @@ -169,17 +169,7 @@ impl BlocksOfInterest { }, } if block.collectible_id().is_some() { - interactables.push(( - pos, - block - .get_sprite() - .map(|s| { - Interaction::Unlock( - s.unlock_condition(chunk.meta().sprite_cfg_at(pos).cloned()), - ) - }) - .unwrap_or(Interaction::Collect), - )); + interactables.push((pos, Interaction::Collect)); } if let Some(glow) = block.get_glow() { // Currently, we count filled blocks as 'minor' lights, and sprites as diff --git a/voxygen/src/session/interactable.rs b/voxygen/src/session/interactable.rs index 5dc6e27c21..a78a6563c0 100644 --- a/voxygen/src/session/interactable.rs +++ b/voxygen/src/session/interactable.rs @@ -12,19 +12,30 @@ use common::{ consts::MAX_PICKUP_RANGE, link::Is, mounting::Mount, - terrain::Block, + terrain::{Block, TerrainGrid, UnlockKind}, util::find_dist::{Cube, Cylinder, FindDist}, vol::ReadVol, }; use common_base::span; -use crate::scene::{terrain::Interaction, Scene}; +use crate::{ + hud::CraftingTab, + scene::{terrain::Interaction, Scene}, +}; + +#[derive(Clone, Debug)] +pub enum BlockInteraction { + Collect, + Unlock(UnlockKind), + Craft(CraftingTab), + // TODO: mining blocks don't use the interaction key, so it might not be the best abstraction + // to have them here, will see how things turn out + Mine, +} -// TODO: extract mining blocks (the None case in the Block variant) from this -// enum since they don't use the interaction key #[derive(Clone, Debug)] pub enum Interactable { - Block(Block, Vec3, Interaction), + Block(Block, Vec3, BlockInteraction), Entity(specs::Entity), } @@ -35,6 +46,44 @@ impl Interactable { Self::Block(_, _, _) => None, } } + + fn from_block_pos( + terrain: &TerrainGrid, + pos: Vec3, + interaction: Interaction, + ) -> Option { + let Ok(&block) = terrain.get(pos) else { return None }; + let block_interaction = match interaction { + Interaction::Collect => { + // Check if this is an unlockable sprite + let unlock = block.get_sprite().and_then(|sprite| { + let Some(chunk) = terrain.pos_chunk(pos) else { return None }; + let sprite_chunk_pos = TerrainGrid::chunk_offs(pos); + let sprite_cfg = chunk.meta().sprite_cfg_at(sprite_chunk_pos); + let unlock_condition = sprite.unlock_condition(sprite_cfg.cloned()); + // HACK: No other way to distinguish between things that should be unlockable + // and regular sprites with the current unlock_condition method so we hack + // around that by saying that it is a regular collectible sprite if + // `unlock_condition` returns UnlockKind::Free and the cfg was `None`. + if sprite_cfg.is_some() || !matches!(&unlock_condition, UnlockKind::Free) { + Some(unlock_condition) + } else { + None + } + }); + + if let Some(unlock) = unlock { + BlockInteraction::Unlock(unlock) + } else if block.mine_tool().is_some() { + BlockInteraction::Mine + } else { + BlockInteraction::Collect + } + }, + Interaction::Craft(tab) => BlockInteraction::Craft(tab), + }; + Some(Self::Block(block, pos, block_interaction)) + } } /// Select interactable to highlight, display interaction text for, and to @@ -62,13 +111,10 @@ pub(super) fn select_interactable( collect_target.map(|t| t.distance), ]); - fn get_block(client: &Client, target: Target) -> Option { - client - .state() - .terrain() - .get(target.position_int()) - .ok() - .copied() + let terrain = client.state().terrain(); + + fn get_block(terrain: &common::terrain::TerrainGrid, target: Target) -> Option { + terrain.get(target.position_int()).ok().copied() } if let Some(interactable) = entity_target @@ -83,8 +129,9 @@ pub(super) fn select_interactable( .or_else(|| { collect_target.and_then(|t| { if Some(t.distance) == nearest_dist { - get_block(client, t) - .map(|b| Interactable::Block(b, t.position_int(), Interaction::Collect)) + get_block(&terrain, t).map(|b| { + Interactable::Block(b, t.position_int(), BlockInteraction::Collect) + }) } else { None } @@ -93,14 +140,18 @@ pub(super) fn select_interactable( .or_else(|| { mine_target.and_then(|t| { if Some(t.distance) == nearest_dist { - get_block(client, t).and_then(|b| { + get_block(&terrain, t).and_then(|b| { // Handling edge detection. sometimes the casting (in Target mod) returns a // position which is actually empty, which we do not want labeled as an // interactable. We are only returning the mineable air // elements (e.g. minerals). The mineable weakrock are used // in the terrain selected_pos, but is not an interactable. if b.mine_tool().is_some() && b.is_air() { - Some(Interactable::Block(b, t.position_int(), Interaction::Mine)) + Some(Interactable::Block( + b, + t.position_int(), + BlockInteraction::Mine, + )) } else { None } @@ -124,6 +175,9 @@ pub(super) fn select_interactable( let colliders = ecs.read_storage::(); let char_states = ecs.read_storage::(); let is_mount = ecs.read_storage::>(); + let bodies = ecs.read_storage::(); + let items = ecs.read_storage::(); + let stats = ecs.read_storage::(); let player_cylinder = Cylinder::from_components( player_pos, @@ -135,20 +189,39 @@ pub(super) fn select_interactable( let closest_interactable_entity = ( &ecs.entities(), &positions, + &bodies, scales.maybe(), colliders.maybe(), char_states.maybe(), !&is_mount, + (stats.mask() | items.mask()).maybe(), ) .join() - .filter(|(e, _, _, _, _, _)| *e != player_entity) - .map(|(e, p, s, c, cs, _)| { + .filter_map(|(e, p, b, s, c, cs, _, has_stats_or_item)| { + // Note, if this becomes expensive to compute do it after the distance check! + // + // The entities that can be interacted with: + // * Sitting at campfires (Body::is_campfire) + // * Talking/trading with npcs (note hud code uses Alignment but I can't bring + // myself to write more code on that depends on having this on the client so + // we just check for presence of Stats component for now, it is okay to have + // some false positives here as long as it doesn't frequently prevent us from + // interacting with actual interactable entities that are closer by) + // * Dropped items that can be picked up (Item component) + let is_interactable = b.is_campfire() || has_stats_or_item.is_some(); + + if e == player_entity || !is_interactable { + return None; + }; + let cylinder = Cylinder::from_components(p.0, s.copied(), c, cs); - (e, cylinder) + // Roughly filter out entities farther than interaction distance + if player_cylinder.approx_in_range(cylinder, MAX_PICKUP_RANGE) { + Some((e, player_cylinder.min_distance(cylinder))) + } else { + None + } }) - // Roughly filter out entities farther than interaction distance - .filter(|(_, cylinder)| player_cylinder.approx_in_range(*cylinder, MAX_PICKUP_RANGE)) - .map(|(e, cylinder)| (e, player_cylinder.min_distance(cylinder))) .min_by_key(|(_, dist)| OrderedFloat(*dist)); // Only search as far as closest interactable entity @@ -156,7 +229,7 @@ pub(super) fn select_interactable( let player_chunk = player_pos.xy().map2(TerrainChunk::RECT_SIZE, |e, sz| { (e.floor() as i32).div_euclid(sz as i32) }); - let terrain = scene.terrain(); + let scene_terrain = scene.terrain(); // Find closest interactable block // TODO: consider doing this one first? @@ -168,7 +241,7 @@ pub(super) fn select_interactable( let chunk_pos = player_chunk + offset; let chunk_voxel_pos = Vec3::::from(chunk_pos * TerrainChunk::RECT_SIZE.map(|e| e as i32)); - terrain.get(chunk_pos).map(|data| (data, chunk_voxel_pos)) + scene_terrain.get(chunk_pos).map(|data| (data, chunk_voxel_pos)) }) // TODO: maybe we could make this more efficient by putting the // interactables is some sort of spatial structure @@ -180,10 +253,10 @@ pub(super) fn select_interactable( .map(move |(block_offset, interaction)| (chunk_pos + block_offset, interaction)) }) .map(|(block_pos, interaction)| ( - block_pos, - block_pos.map(|e| e as f32 + 0.5) - .distance_squared(player_pos), - interaction, + block_pos, + block_pos.map(|e| e as f32 + 0.5) + .distance_squared(player_pos), + interaction, )) .min_by_key(|(_, dist_sqr, _)| OrderedFloat(*dist_sqr)) .map(|(block_pos, _, interaction)| (block_pos, interaction)); @@ -197,13 +270,7 @@ pub(super) fn select_interactable( }) < search_dist }) .and_then(|(block_pos, interaction)| { - client - .state() - .terrain() - .get(block_pos) - .ok() - .copied() - .map(|b| Interactable::Block(b, block_pos, interaction.clone())) + Interactable::from_block_pos(&terrain, block_pos, *interaction) }) .or_else(|| closest_interactable_entity.map(|(e, _)| Interactable::Entity(e))) } diff --git a/voxygen/src/session/mod.rs b/voxygen/src/session/mod.rs index 073728b03b..f2c541ea5f 100644 --- a/voxygen/src/session/mod.rs +++ b/voxygen/src/session/mod.rs @@ -49,13 +49,13 @@ use crate::{ key_state::KeyState, menu::char_selection::CharSelectionState, render::{Drawer, GlobalsBindGroup}, - scene::{camera, terrain::Interaction, CameraMode, DebugShapeId, Scene, SceneData}, + scene::{camera, CameraMode, DebugShapeId, Scene, SceneData}, settings::Settings, window::{AnalogGameInput, Event}, Direction, GlobalState, PlayState, PlayStateResult, }; use hashbrown::HashMap; -use interactable::{select_interactable, Interactable}; +use interactable::{select_interactable, BlockInteraction, Interactable}; use settings_change::Language::ChangeLanguage; use target::targets_under_cursor; #[cfg(feature = "egui-ui")] @@ -930,19 +930,19 @@ impl PlayState for SessionState { match interactable { Interactable::Block(block, pos, interaction) => { match interaction { - Interaction::Collect - | Interaction::Unlock(_) => { + BlockInteraction::Collect + | BlockInteraction::Unlock(_) => { if block.is_collectible() { client.collect_block(*pos); } }, - Interaction::Craft(tab) => { + BlockInteraction::Craft(tab) => { self.hud.show.open_crafting_tab( *tab, block.get_sprite().map(|s| (*pos, s)), ) }, - Interaction::Mine => {}, + BlockInteraction::Mine => {}, } }, Interactable::Entity(entity) => { @@ -1366,6 +1366,7 @@ impl PlayState for SessionState { global_state.clock.get_stable_dt(), HudInfo { is_aiming, + is_mining, is_first_person: matches!( self.scene.camera().get_mode(), camera::CameraMode::FirstPerson diff --git a/voxygen/src/session/target.rs b/voxygen/src/session/target.rs index bb904916e4..d19d990f28 100644 --- a/voxygen/src/session/target.rs +++ b/voxygen/src/session/target.rs @@ -100,8 +100,7 @@ pub(super) fn targets_under_cursor( } }; - let (collect_pos, _, collect_cam_ray) = - find_pos(|b: Block| matches!(b.collectible_id(), Some(Some(_)))); + let (collect_pos, _, collect_cam_ray) = find_pos(|b: Block| b.is_collectible()); let (mine_pos, _, mine_cam_ray) = is_mining .then(|| find_pos(|b: Block| b.mine_tool().is_some())) .unwrap_or((None, None, None)); From 94dd8c2b706f50336aba7cdd7638ac9dca5eb317 Mon Sep 17 00:00:00 2001 From: Imbris Date: Fri, 17 Mar 2023 20:17:46 -0400 Subject: [PATCH 13/13] Address review on 3817 --- assets/voxygen/i18n/en/hud/misc.ftl | 3 ++- server/src/events/inventory_manip.rs | 2 +- voxygen/src/hud/mod.rs | 16 ++++++++++++++-- voxygen/src/session/interactable.rs | 23 ++++++++++------------- voxygen/src/session/mod.rs | 2 +- 5 files changed, 28 insertions(+), 18 deletions(-) diff --git a/assets/voxygen/i18n/en/hud/misc.ftl b/assets/voxygen/i18n/en/hud/misc.ftl index bf765653c9..f3ed55dfd4 100644 --- a/assets/voxygen/i18n/en/hud/misc.ftl +++ b/assets/voxygen/i18n/en/hud/misc.ftl @@ -45,7 +45,8 @@ hud-use = Use hud-unlock-requires = Open with { $item } hud-unlock-consumes = Use { $item } to open hud-mine = Mine -hud-needs_pickaxe = Needs Pickaxe +hud-mine-needs_pickaxe = Needs Pickaxe +hud-mine-needs_unhandled_case = Needs ??? hud-talk = Talk hud-trade = Trade hud-mount = Mount diff --git a/server/src/events/inventory_manip.rs b/server/src/events/inventory_manip.rs index cf96baee5d..81b183253c 100644 --- a/server/src/events/inventory_manip.rs +++ b/server/src/events/inventory_manip.rs @@ -373,7 +373,7 @@ pub fn handle_inventory(server: &mut Server, entity: EcsEntity, manip: comp::Inv sprite_pos.x as f32, sprite_pos.y as f32, sprite_pos.z as f32, - ) + Vec3::one() * 0.5, + ) + Vec3::one().with_z(0.0) * 0.5, )) .with(comp::Vel(Vec3::zero())) .build(); diff --git a/voxygen/src/hud/mod.rs b/voxygen/src/hud/mod.rs index 935c222d0c..c4362f9e72 100644 --- a/voxygen/src/hud/mod.rs +++ b/voxygen/src/hud/mod.rs @@ -2071,14 +2071,26 @@ impl Hud { .to_string(), })] }, - BlockInteraction::Mine => { + BlockInteraction::Mine(mine_tool) => { if info.is_mining { vec![( Some(GameInput::Primary), i18n.get_msg("hud-mine").to_string(), )] } else { - vec![(None, i18n.get_msg("hud-needs_pickaxe").to_string())] + match mine_tool { + ToolKind::Pick => { + vec![(None, i18n.get_msg("hud-mine-needs_pickaxe").to_string())] + }, + // TODO: The required tool for mining something may not always be a + // pickaxe! + _ => { + vec![( + None, + i18n.get_msg("hud-mine-needs_unhandled_case").to_string(), + )] + }, + } } }, }; diff --git a/voxygen/src/session/interactable.rs b/voxygen/src/session/interactable.rs index a78a6563c0..a660af5300 100644 --- a/voxygen/src/session/interactable.rs +++ b/voxygen/src/session/interactable.rs @@ -9,6 +9,7 @@ use super::{ use client::Client; use common::{ comp, + comp::tool::ToolKind, consts::MAX_PICKUP_RANGE, link::Is, mounting::Mount, @@ -30,7 +31,7 @@ pub enum BlockInteraction { Craft(CraftingTab), // TODO: mining blocks don't use the interaction key, so it might not be the best abstraction // to have them here, will see how things turn out - Mine, + Mine(ToolKind), } #[derive(Clone, Debug)] @@ -74,8 +75,8 @@ impl Interactable { if let Some(unlock) = unlock { BlockInteraction::Unlock(unlock) - } else if block.mine_tool().is_some() { - BlockInteraction::Mine + } else if let Some(mine_tool) = block.mine_tool() { + BlockInteraction::Mine(mine_tool) } else { BlockInteraction::Collect } @@ -113,10 +114,6 @@ pub(super) fn select_interactable( let terrain = client.state().terrain(); - fn get_block(terrain: &common::terrain::TerrainGrid, target: Target) -> Option { - terrain.get(target.position_int()).ok().copied() - } - if let Some(interactable) = entity_target .and_then(|t| { if t.distance < MAX_TARGET_RANGE && Some(t.distance) == nearest_dist { @@ -129,7 +126,7 @@ pub(super) fn select_interactable( .or_else(|| { collect_target.and_then(|t| { if Some(t.distance) == nearest_dist { - get_block(&terrain, t).map(|b| { + terrain.get(t.position_int()).ok().map(|&b| { Interactable::Block(b, t.position_int(), BlockInteraction::Collect) }) } else { @@ -140,17 +137,17 @@ pub(super) fn select_interactable( .or_else(|| { mine_target.and_then(|t| { if Some(t.distance) == nearest_dist { - get_block(&terrain, t).and_then(|b| { + terrain.get(t.position_int()).ok().and_then(|&b| { // Handling edge detection. sometimes the casting (in Target mod) returns a // position which is actually empty, which we do not want labeled as an // interactable. We are only returning the mineable air // elements (e.g. minerals). The mineable weakrock are used // in the terrain selected_pos, but is not an interactable. - if b.mine_tool().is_some() && b.is_air() { + if let Some(mine_tool) = b.mine_tool() && b.is_air() { Some(Interactable::Block( b, t.position_int(), - BlockInteraction::Mine, + BlockInteraction::Mine(mine_tool), )) } else { None @@ -197,6 +194,7 @@ pub(super) fn select_interactable( (stats.mask() | items.mask()).maybe(), ) .join() + .filter(|&(e, _, _, _, _, _, _, _)| e != player_entity) // skip the player's entity .filter_map(|(e, p, b, s, c, cs, _, has_stats_or_item)| { // Note, if this becomes expensive to compute do it after the distance check! // @@ -209,8 +207,7 @@ pub(super) fn select_interactable( // interacting with actual interactable entities that are closer by) // * Dropped items that can be picked up (Item component) let is_interactable = b.is_campfire() || has_stats_or_item.is_some(); - - if e == player_entity || !is_interactable { + if !is_interactable { return None; }; diff --git a/voxygen/src/session/mod.rs b/voxygen/src/session/mod.rs index f2c541ea5f..be080a9a79 100644 --- a/voxygen/src/session/mod.rs +++ b/voxygen/src/session/mod.rs @@ -942,7 +942,7 @@ impl PlayState for SessionState { block.get_sprite().map(|s| (*pos, s)), ) }, - BlockInteraction::Mine => {}, + BlockInteraction::Mine(_) => {}, } }, Interactable::Entity(entity) => {