Merge branch 'imbris/fix-apple-bug' into 'master'

Fix issues with not being able to grab highlighted apples by unifying distance checking

Closes #820

See merge request veloren/veloren!1532

common/src/util/find_dist.rs

@@ -0,0 +1,218 @@
+/// Calculate the shortest distance between the surfaces of two shapes
+use vek::*;
+
+pub trait FindDist<T> {
+    /// Compute roughly whether the other shape is out of range
+    /// Meant to be a cheap method for initial filtering
+    /// Must return true if the shape could be within the supplied distance but
+    /// is allowed to return true if the shape is actually just out of
+    /// range
+    fn approx_in_range(self, other: T, range: f32) -> bool;
+    /// Find the smallest distance between the two shapes
+    fn min_distance(self, other: T) -> f32;
+}
+
+/// A z-axis aligned cylinder
+#[derive(Clone, Copy, Debug)]
+pub struct Cylinder {
+    /// Center of the cylinder
+    pub center: Vec3<f32>,
+    /// Radius of the cylinder
+    pub radius: f32,
+    /// Height of the cylinder
+    pub height: f32,
+}
+
+impl Cylinder {
+    fn aabb(&self) -> Aabb<f32> {
+        Aabb {
+            min: self.center - Vec3::new(self.radius, self.radius, self.height / 2.0),
+            max: self.center + Vec3::new(self.radius, self.radius, self.height / 2.0),
+        }
+    }
+
+    #[inline]
+    pub fn from_components(
+        pos: Vec3<f32>,
+        scale: Option<crate::comp::Scale>,
+        collider: Option<crate::comp::Collider>,
+        char_state: Option<&crate::comp::CharacterState>,
+    ) -> Self {
+        let scale = scale.map_or(1.0, |s| s.0);
+        let radius = collider.map_or(0.5, |c| c.get_radius()) * scale;
+        let z_limit_modifier = char_state
+            .filter(|char_state| char_state.is_dodge())
+            .map_or(1.0, |_| 0.5)
+            * scale;
+        let (z_bottom, z_top) = collider
+            .map(|c| c.get_z_limits(z_limit_modifier))
+            .unwrap_or((-0.5 * z_limit_modifier, 0.5 * z_limit_modifier));
+
+        Self {
+            center: pos + Vec3::unit_z() * (z_top + z_bottom) / 2.0,
+            radius,
+            height: z_top - z_bottom,
+        }
+    }
+}
+
+/// An axis aligned cube
+#[derive(Clone, Copy, Debug)]
+pub struct Cube {
+    /// The position of min corner of the cube
+    pub min: Vec3<f32>,
+    /// The side length of the cube
+    pub side_length: f32,
+}
+
+impl FindDist<Cylinder> for Cube {
+    #[inline]
+    fn approx_in_range(self, other: Cylinder, range: f32) -> bool {
+        let cube_plus_range_aabb = Aabb {
+            min: self.min - range,
+            max: self.min + self.side_length + range,
+        };
+        let cylinder_aabb = other.aabb();
+
+        cube_plus_range_aabb.collides_with_aabb(cylinder_aabb)
+    }
+
+    #[inline]
+    fn min_distance(self, other: Cylinder) -> f32 {
+        // Distance between centers along the z-axis
+        let z_center_dist = (self.min.z + self.side_length / 2.0 - other.center.z).abs();
+        // Distance between surfaces projected onto the z-axis
+        let z_dist = (z_center_dist - (self.side_length + other.height) / 2.0).max(0.0);
+        // Distance between shapes projected onto the xy plane as a square/circle
+        let square_aabr = Aabr {
+            min: self.min.xy(),
+            max: self.min.xy() + self.side_length,
+        };
+        let xy_dist = (square_aabr.distance_to_point(other.center.xy()) - other.radius).max(0.0);
+        // Overall distance by pythagoras
+        (z_dist.powi(2) + xy_dist.powi(2)).sqrt()
+    }
+}
+
+impl FindDist<Cube> for Cylinder {
+    #[inline]
+    fn approx_in_range(self, other: Cube, range: f32) -> bool { other.approx_in_range(self, range) }
+
+    #[inline]
+    fn min_distance(self, other: Cube) -> f32 { other.min_distance(self) }
+}
+
+impl FindDist<Cylinder> for Cylinder {
+    #[inline]
+    fn approx_in_range(self, other: Cylinder, range: f32) -> bool {
+        let mut aabb = self.aabb();
+        aabb.min -= range;
+        aabb.max += range;
+
+        aabb.collides_with_aabb(other.aabb())
+    }
+
+    #[inline]
+    fn min_distance(self, other: Cylinder) -> f32 {
+        // Distance between centers along the z-axis
+        let z_center_dist = (self.center.z - other.center.z).abs();
+        // Distance between surfaces projected onto the z-axis
+        let z_dist = (z_center_dist - (self.height + other.height) / 2.0).max(0.0);
+        // Distance between shapes projected onto the xy plane as a circles
+        let xy_dist =
+            (self.center.xy().distance(other.center.xy()) - self.radius - other.radius).max(0.0);
+        // Overall distance by pythagoras
+        (z_dist.powi(2) + xy_dist.powi(2)).sqrt()
+    }
+}
+
+impl FindDist<Vec3<f32>> for Cylinder {
+    #[inline]
+    fn approx_in_range(self, other: Vec3<f32>, range: f32) -> bool {
+        let mut aabb = self.aabb();
+        aabb.min -= range;
+        aabb.max += range;
+
+        aabb.contains_point(other)
+    }
+
+    #[inline]
+    fn min_distance(self, other: Vec3<f32>) -> f32 {
+        // Distance between center and point along the z-axis
+        let z_center_dist = (self.center.z - other.z).abs();
+        // Distance between surface and point projected onto the z-axis
+        let z_dist = (z_center_dist - self.height / 2.0).max(0.0);
+        // Distance between shapes projected onto the xy plane
+        let xy_dist = (self.center.xy().distance(other.xy()) - self.radius).max(0.0);
+        // Overall distance by pythagoras
+        (z_dist.powi(2) + xy_dist.powi(2)).sqrt()
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn cylinder_vs_cube() {
+        //let offset = Vec3::new(1213.323, 5424.0, -231.0);
+        let offset = Vec3::zero();
+        let cylinder = Cylinder {
+            center: Vec3::new(0.0, 0.0, 0.0) + offset,
+            radius: 2.0,
+            height: 4.0,
+        };
+
+        let cube = Cube {
+            min: Vec3::new(-0.5, -0.5, -0.5) + offset,
+            side_length: 1.0,
+        };
+
+        assert!(cube.approx_in_range(cylinder, 0.0));
+        assert!(cube.min_distance(cylinder).abs() < f32::EPSILON);
+        assert!((cube.min_distance(cylinder) - cylinder.min_distance(cube)).abs() < 0.001);
+
+        let cube = Cube {
+            min: cube.min + Vec3::unit_x() * 50.0,
+            side_length: 1.0,
+        };
+
+        assert!(!cube.approx_in_range(cylinder, 5.0)); // Note: technically it is not breaking any promises if this returns true but this will be useful as a warning if the filtering is not tight as we were expecting
+        assert!(cube.approx_in_range(cylinder, 47.51));
+        assert!((cube.min_distance(cylinder) - 47.5).abs() < 0.001);
+        assert!((cube.min_distance(cylinder) - cylinder.min_distance(cube)).abs() < 0.001);
+    }
+
+    #[test]
+    fn zero_size_cylinder() {
+        let cylinder = Cylinder {
+            center: Vec3::new(1.0, 2.0, 3.0),
+            radius: 0.0,
+            height: 0.0,
+        };
+
+        let point = Vec3::new(1.0, 2.5, 3.5);
+
+        assert!(cylinder.approx_in_range(point, 0.71));
+        assert!(cylinder.min_distance(point) < 0.71);
+        assert!(cylinder.min_distance(point) > 0.70);
+
+        let cube = Cube {
+            min: Vec3::new(0.5, 1.9, 2.1),
+            side_length: 1.0,
+        };
+
+        assert!(cylinder.approx_in_range(cube, 0.0));
+        assert!(cylinder.min_distance(cube) < f32::EPSILON);
+
+        let cube = Cube {
+            min: Vec3::new(1.0, 2.0, 4.5),
+            side_length: 1.0,
+        };
+
+        assert!(cylinder.approx_in_range(cube, 1.51));
+        assert!(cylinder.approx_in_range(cube, 100.51));
+        assert!(cylinder.min_distance(cube) < 1.501);
+        assert!(cylinder.min_distance(cube) > 1.499);
+    }
+}

common/src/util/mod.rs

@@ -1,5 +1,6 @@
 mod color;
 pub mod dir;
+pub mod find_dist;
 mod option;
 pub mod userdata_dir;
 

server/src/events/inventory_manip.rs

@@ -3,12 +3,12 @@ use common::{
     comp::{
         self, item,
         slot::{self, Slot},
-        Pos,
     },
     consts::MAX_PICKUP_RANGE,
     msg::ServerGeneral,
     recipe::default_recipe_book,
     sync::{Uid, WorldSyncExt},
+    util::find_dist::{self, FindDist},
     vol::ReadVol,
 };
 use comp::LightEmitter;
@@ -39,6 +39,23 @@ pub fn handle_inventory(server: &mut Server, entity: EcsEntity, manip: comp::Inv
     let mut dropped_items = Vec::new();
     let mut thrown_items = Vec::new();
 
+    let get_cylinder = |state: &common::state::State, entity| {
+        let ecs = state.ecs();
+        let positions = ecs.read_storage::<comp::Pos>();
+        let scales = ecs.read_storage::<comp::Scale>();
+        let colliders = ecs.read_storage::<comp::Collider>();
+        let char_states = ecs.read_storage::<comp::CharacterState>();
+
+        positions.get(entity).map(|p| {
+            find_dist::Cylinder::from_components(
+                p.0,
+                scales.get(entity).copied(),
+                colliders.get(entity).copied(),
+                char_states.get(entity),
+            )
+        })
+    };
+
     match manip {
         comp::InventoryManip::Pickup(uid) => {
             let picked_up_item: Option<comp::Item>;
@@ -59,30 +76,32 @@ pub fn handle_inventory(server: &mut Server, entity: EcsEntity, manip: comp::Inv
                     .get_mut(entity),
             ) {
                 picked_up_item = Some(item.clone());
-                if !within_pickup_range(
-                    state.ecs().read_storage::<comp::Pos>().get(entity),
-                    state.ecs().read_storage::<comp::Pos>().get(item_entity),
-                ) {
-                    debug!("Failed to pick up item as not within range, Uid: {}", uid);
+
+                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 player and check if the player is dead.
+                // Grab the health from the entity and check if the entity is dead.
                 let healths = state.ecs().read_storage::<comp::Health>();
                 if let Some(entity_health) = healths.get(entity) {
                     if entity_health.is_dead {
-                        debug!("Failed to pick up item as the player is dead");
+                        debug!("Failed to pick up item as the entity is dead");
                         return; // If dead, don't continue
                     }
                 }
 
-                // Attempt to add the item to the player's inventory
+                // Attempt to add the item to the entity's inventory
                 match inv.push(item) {
                     None => Some(item_entity),
                     Some(_) => None, // Inventory was full
                 }
             } else {
-                // Item entity/component could not be found - most likely because the player
+                // Item entity/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 get entity/component for item Uid: {}", uid);
@@ -92,7 +111,7 @@ pub fn handle_inventory(server: &mut Server, entity: EcsEntity, manip: comp::Inv
             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
-                    // player's inventory but also left on the ground
+                    // entity's inventory but also left on the ground
                     panic!("Failed to delete picked up item entity: {:?}", err);
                 }
                 comp::InventoryUpdate::new(comp::InventoryUpdateEvent::Collected(
@@ -111,12 +130,17 @@ pub fn handle_inventory(server: &mut Server, entity: EcsEntity, manip: comp::Inv
             if let Some(block) = block {
                 if block.is_collectible() && state.can_set_block(pos) {
                     // Check if the block is within pickup range
-                    if !within_pickup_range(
-                        state.ecs().read_storage::<comp::Pos>().get(entity),
-                        // We convert the Vec<i32> pos into a Vec<f32>, adding 0.5 to get the
-                        // center of the block
-                        Some(&Pos(pos.map(|e| e as f32 + 0.5))),
-                    ) {
+                    let entity_cylinder = get_cylinder(state, entity);
+                    if !within_pickup_range(entity_cylinder, || {
+                        Some(find_dist::Cube {
+                            min: pos.as_(),
+                            side_length: 1.0,
+                        })
+                    }) {
+                        debug!(
+                            ?entity_cylinder,
+                            "Failed to pick up block as not within range, block pos: {}", pos
+                        );
                         return;
                     };
 
@@ -241,7 +265,7 @@ pub fn handle_inventory(server: &mut Server, entity: EcsEntity, manip: comp::Inv
                                 {
                                     let uid = state
                                         .read_component_copied(entity)
-                                        .expect("Expected player to have a UID");
+                                        .expect("Expected entity to have a UID");
                                     if (
                                         &state.read_storage::<comp::Alignment>(),
                                         &state.read_storage::<comp::Agent>(),
@@ -529,37 +553,47 @@ pub fn handle_inventory(server: &mut Server, entity: EcsEntity, manip: comp::Inv
     }
 }
 
-fn within_pickup_range(player_position: Option<&Pos>, item_position: Option<&Pos>) -> bool {
-    match (player_position, item_position) {
-        (Some(ppos), Some(ipos)) => ppos.0.distance_squared(ipos.0) < MAX_PICKUP_RANGE.powi(2),
-        _ => false,
-    }
+fn within_pickup_range<S: FindDist<find_dist::Cylinder>>(
+    entity_cylinder: Option<find_dist::Cylinder>,
+    shape_fn: impl FnOnce() -> Option<S>,
+) -> bool {
+    entity_cylinder
+        .and_then(|entity_cylinder| {
+            shape_fn().map(|shape| dbg!(shape.min_distance(entity_cylinder)) < MAX_PICKUP_RANGE)
+        })
+        .unwrap_or(false)
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
     use common::comp::Pos;
+    use find_dist::*;
     use vek::Vec3;
 
+    // Helper function
+    fn test_cylinder(pos: comp::Pos) -> Option<Cylinder> {
+        Some(Cylinder::from_components(pos.0, None, None, None))
+    }
+
     #[test]
     fn pickup_distance_within_range() {
-        let player_position = Pos(Vec3::zero());
+        let position = Pos(Vec3::zero());
         let item_position = Pos(Vec3::one());
 
         assert_eq!(
-            within_pickup_range(Some(&player_position), Some(&item_position)),
+            within_pickup_range(test_cylinder(position), || test_cylinder(item_position),),
             true
         );
     }
 
     #[test]
     fn pickup_distance_not_within_range() {
-        let player_position = Pos(Vec3::zero());
+        let position = Pos(Vec3::zero());
         let item_position = Pos(Vec3::one() * 500.0);
 
         assert_eq!(
-            within_pickup_range(Some(&player_position), Some(&item_position)),
+            within_pickup_range(test_cylinder(position), || test_cylinder(item_position),),
             false
         );
     }

voxygen/src/session.rs

@@ -21,7 +21,10 @@ use common::{
     outcome::Outcome,
     span,
     terrain::{Block, BlockKind},
-    util::Dir,
+    util::{
+        find_dist::{Cube, Cylinder, FindDist},
+        Dir,
+    },
     vol::ReadVol,
 };
 use ordered_float::OrderedFloat;
@@ -54,6 +57,7 @@ pub struct SessionState {
     is_aiming: bool,
     target_entity: Option<specs::Entity>,
     selected_entity: Option<(specs::Entity, std::time::Instant)>,
+    interactable: Option<Interactable>,
 }
 
 /// Represents an active game session (i.e., the one being played).
@@ -94,6 +98,7 @@ impl SessionState {
             is_aiming: false,
             target_entity: None,
             selected_entity: None,
+            interactable: None,
         }
     }
 
@@ -273,9 +278,9 @@ impl PlayState for SessionState {
                 .get(self.client.borrow().entity())
                 .is_some();
 
-            let interactable = select_interactable(
+            self.interactable = select_interactable(
                 &self.client.borrow(),
-                self.target_entity,
+                target_entity,
                 select_pos,
                 &self.scene,
             );
@@ -283,14 +288,12 @@ impl PlayState for SessionState {
             // Only highlight interactables
             // unless in build mode where select_pos highlighted
             self.scene
-                .set_select_pos(
-                    select_pos
-                        .filter(|_| can_build)
-                        .or_else(|| match interactable {
-                            Some(Interactable::Block(_, block_pos)) => Some(block_pos),
-                            _ => None,
-                        }),
-                );
+                .set_select_pos(select_pos.filter(|_| can_build).or_else(
+                    || match self.interactable {
+                        Some(Interactable::Block(_, block_pos)) => Some(block_pos),
+                        _ => None,
+                    },
+                ));
 
             // Handle window events.
             for event in events {
@@ -496,7 +499,7 @@ impl PlayState for SessionState {
                         self.key_state.collect = state;
 
                         if state {
-                            if let Some(interactable) = interactable {
+                            if let Some(interactable) = self.interactable {
                                 let mut client = self.client.borrow_mut();
                                 match interactable {
                                     Interactable::Block(block, pos) => {
@@ -1051,7 +1054,8 @@ impl PlayState for SessionState {
                 let scene_data = SceneData {
                     state: client.state(),
                     player_entity: client.entity(),
-                    target_entity: self.target_entity,
+                    // Only highlight if interactable
+                    target_entity: self.interactable.and_then(Interactable::entity),
                     loaded_distance: client.loaded_distance(),
                     view_distance: client.view_distance().unwrap_or(1),
                     tick: client.get_tick(),
@@ -1116,7 +1120,8 @@ impl PlayState for SessionState {
             let scene_data = SceneData {
                 state: client.state(),
                 player_entity: client.entity(),
-                target_entity: self.target_entity,
+                // Only highlight if interactable
+                target_entity: self.interactable.and_then(Interactable::entity),
                 loaded_distance: client.loaded_distance(),
                 view_distance: client.view_distance().unwrap_or(1),
                 tick: client.get_tick(),
@@ -1158,14 +1163,22 @@ fn under_cursor(
     span!(_guard, "under_cursor");
     // Choose a spot above the player's head for item distance checks
     let player_entity = client.entity();
-    let player_pos = match client
-        .state()
-        .read_storage::<comp::Pos>()
-        .get(player_entity)
-    {
-        Some(pos) => pos.0 + (Vec3::unit_z() * 2.0),
-        _ => cam_pos, // Should never happen, but a safe fallback
+    let ecs = client.state().ecs();
+    let positions = ecs.read_storage::<comp::Pos>();
+    let player_pos = match positions.get(player_entity) {
+        Some(pos) => pos.0,
+        None => cam_pos, // Should never happen, but a safe fallback
     };
+    let scales = ecs.read_storage();
+    let colliders = ecs.read_storage();
+    let char_states = ecs.read_storage();
+    // Get the player's cylinder
+    let player_cylinder = Cylinder::from_components(
+        player_pos,
+        scales.get(player_entity).copied(),
+        colliders.get(player_entity).copied(),
+        char_states.get(player_entity),
+    );
     let terrain = client.state().terrain();
 
     let cam_ray = terrain
@@ -1177,8 +1190,8 @@ fn under_cursor(
 
     // The ray hit something, is it within range?
     let (build_pos, select_pos) = if matches!(cam_ray.1, Ok(Some(_)) if
-        player_pos.distance_squared(cam_pos + cam_dir * cam_dist)
-        <= MAX_PICKUP_RANGE.powi(2))
+        player_cylinder.min_distance(cam_pos + cam_dir * (cam_dist + 0.01))
+        <= MAX_PICKUP_RANGE)
     {
         (
             Some((cam_pos + cam_dir * (cam_dist - 0.01)).map(|e| e.floor() as i32)),
@@ -1190,7 +1203,6 @@ fn under_cursor(
 
     // See if ray hits entities
     // Currently treated as spheres
-    let ecs = client.state().ecs();
     // Don't cast through blocks
     // Could check for intersection with entity from last frame to narrow this down
     let cast_dist = if let Ok(Some(_)) = cam_ray.1 {
@@ -1204,14 +1216,15 @@ fn under_cursor(
     // on final result)
     let mut nearby = (
         &ecs.entities(),
-        &ecs.read_storage::<comp::Pos>(),
-        ecs.read_storage::<comp::Scale>().maybe(),
+        &positions,
+        scales.maybe(),
         &ecs.read_storage::<comp::Body>()
     )
         .join()
         .filter(|(e, _, _, _)| *e != player_entity)
         .map(|(e, p, s, b)| {
             const RADIUS_SCALE: f32 = 3.0;
+            // TODO: use collider radius instead of body radius?
             let radius = s.map_or(1.0, |s| s.0) * b.radius() * RADIUS_SCALE;
             // Move position up from the feet
             let pos = Vec3::new(p.0.x, p.0.y, p.0.z + radius);
@@ -1239,9 +1252,17 @@ fn under_cursor(
         .map(|(e, p, r, _)| (e, *p, r))
         // Find first one that intersects the ray segment
         .find(|(_, p, r)| seg_ray.projected_point(*p).distance_squared(*p) < r.powi(2))
-        .and_then(|(e, p, r)| {
-            let dist_to_player = p.distance(player_pos);
-            (dist_to_player - r < MAX_TARGET_RANGE).then_some((*e, dist_to_player))
+        .and_then(|(e, p, _)| {
+            // Get the entity's cylinder
+            let target_cylinder = Cylinder::from_components(
+                p,
+                scales.get(*e).copied(),
+                colliders.get(*e).copied(),
+                char_states.get(*e),
+            );
+
+            let dist_to_player = player_cylinder.min_distance(target_cylinder);
+            (dist_to_player < MAX_TARGET_RANGE).then_some((*e, dist_to_player))
         });
 
     // TODO: consider setting build/select to None when targeting an entity
@@ -1254,6 +1275,15 @@ enum Interactable {
     Entity(specs::Entity),
 }
 
+impl Interactable {
+    fn entity(self) -> Option<specs::Entity> {
+        match self {
+            Self::Entity(e) => Some(e),
+            Self::Block(_, _) => None,
+        }
+    }
+}
+
 /// Select interactable to hightlight, display interaction text for, and to
 /// interact with if the interact key is pressed
 /// Selected in the following order
@@ -1262,13 +1292,17 @@ enum Interactable {
 /// 3) Closest of nearest interactable entity/block
 fn select_interactable(
     client: &Client,
-    target_entity: Option<specs::Entity>,
+    target_entity: Option<(specs::Entity, f32)>,
     selected_pos: Option<Vec3<i32>>,
     scene: &Scene,
 ) -> Option<Interactable> {
     span!(_guard, "select_interactable");
+    // TODO: once there are multiple distances for different types of interactions
+    // this code will need to be revamped to cull things by varying distances
+    // based on the types of interactions available for those things
     use common::{spiral::Spiral2d, terrain::TerrainChunk, vol::RectRasterableVol};
-    target_entity.map(Interactable::Entity)
+    target_entity
+        .and_then(|(e, dist_to_player)| (dist_to_player < MAX_PICKUP_RANGE).then_some(Interactable::Entity(e)))
         .or_else(|| selected_pos.and_then(|sp|
                 client.state().terrain().get(sp).ok().copied()
                     .filter(Block::is_collectible).map(|b| Interactable::Block(b, sp))
@@ -1276,80 +1310,84 @@ fn select_interactable(
         .or_else(|| {
             let ecs = client.state().ecs();
             let player_entity = client.entity();
-            ecs
-                .read_storage::<comp::Pos>()
-                .get(player_entity).and_then(|player_pos| {
-                    let closest_interactable_entity = (
-                        &ecs.entities(),
-                        &ecs.read_storage::<comp::Pos>(),
-                        ecs.read_storage::<comp::Scale>().maybe(),
-                        &ecs.read_storage::<comp::Body>(),
-                        // Must have this comp to be interactable (for now)
-                        &ecs.read_storage::<comp::Item>(),
-                    )
-                        .join()
-                        .filter(|(e, _, _, _, _)| *e != player_entity)
-                        .map(|(e, p, s, b, _)| {
-                            let radius = s.map_or(1.0, |s| s.0) * b.radius();
-                            // Distance squared from player to the entity
-                            // Note: the position of entities is currently at their feet so this
-                            // distance is between their feet positions
-                            let dist_sqr = p.0.distance_squared(player_pos.0);
-                            (e, radius, dist_sqr)
-                        })
-                        // Roughly filter out entities farther than interaction distance
-                        .filter(|(_, r, d_sqr)| *d_sqr <= MAX_PICKUP_RANGE.powi(2) + 2.0 * MAX_PICKUP_RANGE * r + r.powi(2))
-                        // Note: entities are approximated as spheres here
-                        // to determine which is closer
-                        // Substract sphere radius from distance to the player
-                        .map(|(e, r, d_sqr)| (e, d_sqr.sqrt() - r))
-                        .min_by_key(|(_, dist)| OrderedFloat(*dist));
+            let positions = ecs.read_storage::<comp::Pos>();
+            let player_pos = positions.get(player_entity)?.0;
 
-                    // Only search as far as closest interactable entity
-                    let search_dist = closest_interactable_entity
-                        .map_or(MAX_PICKUP_RANGE, |(_, dist)| dist);
-                    let player_chunk = player_pos.0.xy().map2(TerrainChunk::RECT_SIZE, |e, sz| {
-                        (e.floor() as i32).div_euclid(sz as i32)
-                    });
-                    let terrain = scene.terrain();
+            let scales = ecs.read_storage::<comp::Scale>();
+            let colliders = ecs.read_storage::<comp::Collider>();
+            let char_states = ecs.read_storage::<comp::CharacterState>();
 
-                    // Find closest interactable block
-                    // TODO: consider doing this one first?
-                    let closest_interactable_block_pos = Spiral2d::new()
-                        // TODO: this formula for the number to take was guessed
-                        // Note: assume RECT_SIZE.x == RECT_SIZE.y
-                        .take(((search_dist / TerrainChunk::RECT_SIZE.x as f32).ceil() as usize * 2 + 1).pow(2))
-                        .flat_map(|offset| {
-                            let chunk_pos = player_chunk + offset;
-                            let chunk_voxel_pos =
-                                    Vec3::<i32>::from(chunk_pos * TerrainChunk::RECT_SIZE.map(|e| e as i32));
-                            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
-                        .flat_map(|(chunk_data, chunk_pos)| {
-                            chunk_data
-                                .blocks_of_interest
-                                .interactables
-                                .iter()
-                                .map(move |block_offset| chunk_pos + block_offset)
-                        })
-                        // TODO: confirm that adding 0.5 here is correct
-                        .map(|block_pos| (
-                                block_pos,
-                                block_pos.map(|e| e as f32 + 0.5)
-                                    .distance_squared(player_pos.0)
-                        ))
-                        .min_by_key(|(_, dist_sqr)| OrderedFloat(*dist_sqr));
+            let player_cylinder = Cylinder::from_components(
+                player_pos,
+                scales.get(player_entity).copied(),
+                colliders.get(player_entity).copied(),
+                char_states.get(player_entity),
+            );
 
-                    // Pick closer one if they exist
-                    closest_interactable_block_pos
-                        .filter(|(_, dist_sqr)| search_dist.powi(2) > *dist_sqr)
-                        .and_then(|(block_pos, _)|
-                            client.state().terrain().get(block_pos).ok().copied()
-                                .map(|b| Interactable::Block(b, block_pos))
-                        )
-                        .or_else(|| closest_interactable_entity.map(|(e, _)| Interactable::Entity(e)))
+            let closest_interactable_entity = (
+                &ecs.entities(),
+                &positions,
+                scales.maybe(),
+                colliders.maybe(),
+                char_states.maybe(),
+                // Must have this comp to be interactable (for now)
+                &ecs.read_storage::<comp::Item>(),
+            )
+                .join()
+                .filter(|(e, _, _, _, _, _)| *e != player_entity)
+                .map(|(e, p, s, c, cs, _)| {
+                    let cylinder = Cylinder::from_components(p.0, s.copied(), c.copied(), cs);
+                    (e, cylinder)
                 })
+                // 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
+            let search_dist = closest_interactable_entity
+                .map_or(MAX_PICKUP_RANGE, |(_, dist)| dist);
+            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();
+
+            // Find closest interactable block
+            // TODO: consider doing this one first?
+            let closest_interactable_block_pos = Spiral2d::new()
+                // TODO: this formula for the number to take was guessed
+                // Note: assume RECT_SIZE.x == RECT_SIZE.y
+                .take(((search_dist / TerrainChunk::RECT_SIZE.x as f32).ceil() as usize * 2 + 1).pow(2))
+                .flat_map(|offset| {
+                    let chunk_pos = player_chunk + offset;
+                    let chunk_voxel_pos =
+                            Vec3::<i32>::from(chunk_pos * TerrainChunk::RECT_SIZE.map(|e| e as i32));
+                    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
+                .flat_map(|(chunk_data, chunk_pos)| {
+                    chunk_data
+                        .blocks_of_interest
+                        .interactables
+                        .iter()
+                        .map(move |block_offset| chunk_pos + block_offset)
+                })
+                .map(|block_pos| (
+                        block_pos,
+                        block_pos.map(|e| e as f32 + 0.5)
+                            .distance_squared(player_pos)
+                ))
+                .min_by_key(|(_, dist_sqr)| OrderedFloat(*dist_sqr))
+                .map(|(block_pos, _)| block_pos);
+
+            // Pick closer one if they exist
+            closest_interactable_block_pos
+                .filter(|block_pos|  player_cylinder.min_distance(Cube { min: block_pos.as_(), side_length: 1.0}) < search_dist)
+                .and_then(|block_pos|
+                    client.state().terrain().get(block_pos).ok().copied()
+                        .map(|b| Interactable::Block(b, block_pos))
+                )
+                .or_else(|| closest_interactable_entity.map(|(e, _)| Interactable::Entity(e)))
         })
 }