Improve overmapbuffer searching routines (#37482)

* Add simple unit tests for `closest_points` functions

* Minor simplification and optimization of `closest_points` functions

* Allow specifying minimal distance in `closest_points` functions

* Use `closest_points` functions for overmap buffer search

src/overmapbuffer.cpp

@@ -930,7 +930,7 @@ tripoint overmapbuffer::find_closest( const tripoint &origin, const std::string
 tripoint overmapbuffer::find_closest( const tripoint &origin, const omt_find_params &params )
 {
     // Check the origin before searching adjacent tiles!
-    if( params.min_distance == 0 &&  is_findable_location( origin, params ) ) {
+    if( params.min_distance == 0 && is_findable_location( origin, params ) ) {
         return origin;
     }
 
@@ -949,60 +949,54 @@ tripoint overmapbuffer::find_closest( const tripoint &origin, const omt_find_par
     // See overmap::place_specials for how we attempt to insure specials are placed within this
     // range.  The actual number is 5 because 1 covers the current overmap,
     // and each additional one expends the search to the next concentric circle of overmaps.
-    int max = params.search_range ? params.search_range : OMAPX * 5;
-    const int min_distance = std::max( 0, params.min_distance );
-    // expanding box
-    for( int dist = min_distance; dist <= max; dist++ ) {
-        // each edge length is 2*dist-2, because corners belong to one edge
-        // south is +y, north is -y
-        for( int i = min_distance * 2; i < dist * 2; i++ ) {
-            for( int z = -OVERMAP_DEPTH; z <= OVERMAP_HEIGHT; z++ ) {
-                //start at northwest, scan north edge
-                const tripoint n_loc( origin.x - dist + i, origin.y - dist, z );
-                if( is_findable_location( n_loc, params ) ) {
-                    return n_loc;
-                }
+    const int min_dist = params.min_distance;
+    const int max_dist = params.search_range ? params.search_range : OMAPX * 5;
 
-                //start at southeast, scan south
-                const tripoint s_loc( origin.x + dist - i, origin.y + dist, z );
-                if( is_findable_location( s_loc, params ) ) {
-                    return s_loc;
-                }
+    std::vector<tripoint> result;
+    cata::optional<int> found_dist;
 
-                //start at southwest, scan west
-                const tripoint w_loc( origin.x - dist, origin.y + dist - i, z );
-                if( is_findable_location( w_loc, params ) ) {
-                    return w_loc;
-                }
+    for( const point &loc_xy : closest_points_first( origin.xy(), min_dist, max_dist ) ) {
+        const int dist_xy = square_dist( origin.xy(), loc_xy );
 
-                //start at northeast, scan east
-                const tripoint e_loc( origin.x + dist, origin.y - dist + i, z );
-                if( is_findable_location( e_loc, params ) ) {
-                    return e_loc;
-                }
+        if( found_dist && *found_dist < dist_xy ) {
+            break;
+        }
+
+        for( int z = -OVERMAP_DEPTH; z <= OVERMAP_HEIGHT; z++ ) {
+            const tripoint loc = { loc_xy, z };
+            const int dist = square_dist( origin, loc );
+
+            if( found_dist && *found_dist < dist ) {
+                continue;
+            }
+
+            if( is_findable_location( loc, params ) ) {
+                found_dist = dist;
+                result.push_back( loc );
             }
         }
     }
-    return overmap::invalid_tripoint;
+
+    return random_entry( result, overmap::invalid_tripoint );
 }
 
 std::vector<tripoint> overmapbuffer::find_all( const tripoint &origin,
         const omt_find_params &params )
 {
     std::vector<tripoint> result;
     // dist == 0 means search a whole overmap diameter.
-    const int dist = params.search_range ? params.search_range : OMAPX;
-    const int min_distance = std::max( 0, params.min_distance );
-    for( const tripoint &search_loc : points_in_radius( origin, dist ) ) {
-        if( square_dist( origin, search_loc ) < min_distance ) {
-            continue;
-        }
-        if( is_findable_location( search_loc, params ) ) {
-            result.push_back( search_loc );
+    const int min_dist = params.min_distance;
+    const int max_dist = params.search_range ? params.search_range : OMAPX;
+
+    for( const tripoint &loc : closest_tripoints_first( origin, min_dist, max_dist ) ) {
+        if( is_findable_location( loc, params ) ) {
+            result.push_back( loc );
         }
     }
+
     return result;
 }
+
 std::vector<tripoint> overmapbuffer::find_all( const tripoint &origin, const std::string &type,
         int dist, bool must_be_seen, ot_match_type match_type,
         bool existing_overmaps_only,

src/point.cpp

@@ -38,38 +38,69 @@ point clamp_inclusive( const point &p, const rectangle &r )
     return point( clamp( p.x, r.p_min.x, r.p_max.x ), clamp( p.y, r.p_min.y, r.p_max.y ) );
 }
 
-std::vector<tripoint> closest_tripoints_first( const tripoint &center, size_t radius )
+std::vector<tripoint> closest_tripoints_first( const tripoint &center, int max_dist )
 {
-    std::vector<tripoint> points;
-    int X = radius * 2 + 1;
-    int Y = radius * 2 + 1;
-    int x = 0;
-    int y = 0;
-    int dx = 0;
-    int dy = -1;
-    int t = std::max( X, Y );
-    int maxI = t * t;
-    for( int i = 0; i < maxI; i++ ) {
-        if( -X / 2 <= x && x <= X / 2 && -Y / 2 <= y && y <= Y / 2 ) {
-            points.push_back( center + point( x, y ) );
-        }
+    return closest_tripoints_first( center, 0, max_dist );
+}
+
+std::vector<tripoint> closest_tripoints_first( const tripoint &center, int min_dist, int max_dist )
+{
+    const std::vector<point> points = closest_points_first( center.xy(), min_dist, max_dist );
+
+    std::vector<tripoint> result;
+    result.reserve( points.size() );
+
+    for( const point &p : points ) {
+        result.emplace_back( p, center.z );
+    }
+
+    return result;
+}
+
+std::vector<point> closest_points_first( const point &center, int max_dist )
+{
+    return closest_points_first( center, 0, max_dist );
+}
+
+std::vector<point> closest_points_first( const point &center, int min_dist, int max_dist )
+{
+    min_dist = std::max( min_dist, 0 );
+    max_dist = std::max( max_dist, 0 );
+
+    if( min_dist > max_dist ) {
+        return {};
+    }
+
+    const int min_edge = min_dist * 2 + 1;
+    const int max_edge = max_dist * 2 + 1;
+
+    const int n = max_edge * max_edge - ( min_edge - 2 ) * ( min_edge - 2 );
+    const bool is_center_included = min_dist == 0;
+
+    std::vector<point> result;
+    result.reserve( n + ( is_center_included ? 1 : 0 ) );
+
+    if( is_center_included ) {
+        result.push_back( center );
+    }
+
+    int x = std::max( min_dist, 1 );
+    int y = 1 - x;
+
+    int dx = 1;
+    int dy = 0;
+
+    for( int i = 0; i < n; i++ ) {
+        result.push_back( center + point{ x, y } );
+
         if( x == y || ( x < 0 && x == -y ) || ( x > 0 && x == 1 - y ) ) {
-            t = dx;
-            dx = -dy;
-            dy = t;
+            std::swap( dx, dy );
+            dx = -dx;
         }
+
         x += dx;
         y += dy;
     }
-    return points;
-}
 
-std::vector<point> closest_points_first( const point &center, size_t radius )
-{
-    const std::vector<tripoint> tripoints = closest_tripoints_first( tripoint( center, 0 ), radius );
-    std::vector<point> points;
-    for( const tripoint &p : tripoints ) {
-        points.push_back( p.xy() );
-    }
-    return points;
+    return result;
 }

src/point.h

@@ -303,8 +303,12 @@ struct sphere {
  * Following functions return points in a spiral pattern starting at center_x/center_y until it hits the radius. Clockwise fashion.
  * Credit to Tom J Nowell; http://stackoverflow.com/a/1555236/1269969
  */
-std::vector<tripoint> closest_tripoints_first( const tripoint &center, size_t radius );
-std::vector<point> closest_points_first( const point &center, size_t radius );
+std::vector<tripoint> closest_tripoints_first( const tripoint &center, int max_dist );
+std::vector<tripoint> closest_tripoints_first( const tripoint &center, int min_dist, int max_dist );
+
+std::vector<point> closest_points_first( const point &center, int max_dist );
+std::vector<point> closest_points_first( const point &center, int min_dist, int max_dist );
+
 
 inline point abs( const point &p )
 {

tests/point_test.cpp

@@ -49,3 +49,59 @@ TEST_CASE( "tripoint_xy", "[point]" )
     tripoint p( 1, 2, 3 );
     CHECK( p.xy() == point( 1, 2 ) );
 }
+
+TEST_CASE( "closest_tripoints_first", "[point]" )
+{
+    const tripoint center = { 1, -1, 2 };
+
+    GIVEN( "min_dist > max_dist" ) {
+        const std::vector<tripoint> result = closest_tripoints_first( center, 1, 0 );
+
+        CHECK( result.empty() );
+    }
+
+    GIVEN( "min_dist = max_dist = 0" ) {
+        const std::vector<tripoint> result = closest_tripoints_first( center, 0, 0 );
+
+        CHECK( result.size() == 1 );
+        CHECK( result[0] == tripoint{ 1, -1, 2 } );
+    }
+
+    GIVEN( "min_dist = 0, max_dist = 1" ) {
+        const std::vector<tripoint> result = closest_tripoints_first( center, 0, 1 );
+
+        CHECK( result.size() == 9 );
+        CHECK( result[0] == tripoint{  1, -1, 2 } );
+        CHECK( result[1] == tripoint{  2, -1, 2 } );
+        CHECK( result[2] == tripoint{  2,  0, 2 } );
+        CHECK( result[3] == tripoint{  1,  0, 2 } );
+        CHECK( result[4] == tripoint{  0,  0, 2 } );
+        CHECK( result[5] == tripoint{  0, -1, 2 } );
+        CHECK( result[6] == tripoint{  0, -2, 2 } );
+        CHECK( result[7] == tripoint{  1, -2, 2 } );
+        CHECK( result[8] == tripoint{  2, -2, 2 } );
+    }
+
+    GIVEN( "min_dist = 2, max_dist = 2" ) {
+        const std::vector<tripoint> result = closest_tripoints_first( center, 2, 2 );
+
+        CHECK( result.size() == 16 );
+
+        CHECK( result[0]  == tripoint{  3, -2, 2 } );
+        CHECK( result[1]  == tripoint{  3, -1, 2 } );
+        CHECK( result[2]  == tripoint{  3,  0, 2 } );
+        CHECK( result[3]  == tripoint{  3,  1, 2 } );
+        CHECK( result[4]  == tripoint{  2,  1, 2 } );
+        CHECK( result[5]  == tripoint{  1,  1, 2 } );
+        CHECK( result[6]  == tripoint{  0,  1, 2 } );
+        CHECK( result[7]  == tripoint{ -1,  1, 2 } );
+        CHECK( result[8]  == tripoint{ -1,  0, 2 } );
+        CHECK( result[9]  == tripoint{ -1, -1, 2 } );
+        CHECK( result[10] == tripoint{ -1, -2, 2 } );
+        CHECK( result[11] == tripoint{ -1, -3, 2 } );
+        CHECK( result[12] == tripoint{  0, -3, 2 } );
+        CHECK( result[13] == tripoint{  1, -3, 2 } );
+        CHECK( result[14] == tripoint{  2, -3, 2 } );
+        CHECK( result[15] == tripoint{  3, -3, 2 } );
+    }
+}