Skip to content

Commit

Permalink
Reach attack around corners (CleverRaven#75939)
Browse files Browse the repository at this point in the history
Fixes an issue where melee reach attacks would hit a wall before, even
if there were other possibilities to draw a straight line to the target.

The issue before was two issues:
* `map::sees` returned cached values even if `bresenham_slope` was
  changed
* `map::find_clear_path` did not try different values for
  `candidate_offset` if `start_sign` was 0

The first issue was that the algorithm being used in
`map::find_clear_path` tries to find a value for `bresenham_slope` (also
named `candidate_offset`) that creates a clear path to the target. But
in `map::sees` we create a cache key for an LRU cache using only the
from and to points, and the cache key specifically does not contain
`bresenham_slope`. This made it so that repeated calls to `map::sees`
using different values for `bresenham_slope` would always just return
the cached value from the first evaluation of the method.

The second issue was that in cases where `start_sign` was 0 in
`map::find_clear_path`, then the algorithm for finding a good value for
`bresenham_slope` always used `candidate_offset=0` since it incorrectly
multiplied the `horizontal_offset` with `0`. In effect, it previously
iterated several values for `horizontal_offset`, but all of them
resulted in `0` being tried for `bresenham_slope`.

This commit instead updates `map::sees` so that calls from
`map::find_clear_path` never allow cached results. The cache will still
be populated correctly once a path is chosen, but with this commit, it
will also allow calls to attempt using different values for
`bresenham_slope`.

The effect of this is that melee reach attacks will now correctly find a
straight path when being done adjacent to an obstacle. A side-effect is
also that ranged attacks that find a straight path from A to B should
now also find a straight path from B to A (they did not always do so
before).
  • Loading branch information
inogenous authored Aug 26, 2024
1 parent eeb3921 commit 04fcfa7
Show file tree
Hide file tree
Showing 3 changed files with 254 additions and 7 deletions.
15 changes: 9 additions & 6 deletions src/map.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7800,7 +7800,7 @@ bool map::sees( const tripoint &F, const tripoint &T, const int range,
}

bool map::sees( const tripoint_bub_ms &F, const tripoint_bub_ms &T, const int range,
int &bresenham_slope, bool with_fields ) const
int &bresenham_slope, bool with_fields, bool allow_cached ) const
{
bool ( map:: * f_transparent )( const tripoint & p ) const =
with_fields ? &map::is_transparent : &map::is_transparent_wo_fields;
Expand All @@ -7812,9 +7812,11 @@ bool map::sees( const tripoint_bub_ms &F, const tripoint_bub_ms &T, const int ra
return false; // Out of range!
}
const point key = sees_cache_key( F, T );
char cached = skew_cache.get( key, -1 );
if( cached >= 0 ) {
return cached > 0;
if( allow_cached ) {
char cached = skew_cache.get( key, -1 );
if( cached >= 0 ) {
return cached > 0;
}
}
bool visible = true;

Expand Down Expand Up @@ -8061,8 +8063,9 @@ std::vector<tripoint_bub_ms> map::find_clear_path( const tripoint_bub_ms &source
// Not totally sure of the derivation.
const int max_start_offset = std::abs( ideal_start_offset ) * 2 + 1;
for( int horizontal_offset = -1; horizontal_offset <= max_start_offset; ++horizontal_offset ) {
int candidate_offset = horizontal_offset * start_sign;
if( sees( source, destination, rl_dist( source, destination ), candidate_offset ) ) {
int candidate_offset = horizontal_offset * ( start_sign == 0 ? 1 : start_sign );
if( sees( source, destination, rl_dist( source, destination ),
candidate_offset, /*with_fields=*/true, /*allow_cached=*/false ) ) {
return line_to( source, destination, candidate_offset, 0 );
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/map.h
Original file line number Diff line number Diff line change
Expand Up @@ -701,7 +701,7 @@ class map
bool sees( const tripoint &F, const tripoint &T, int range, int &bresenham_slope,
bool with_fields = true ) const;
bool sees( const tripoint_bub_ms &F, const tripoint_bub_ms &T, int range, int &bresenham_slope,
bool with_fields = true ) const;
bool with_fields = true, bool allow_cached = true ) const;
point sees_cache_key( const tripoint_bub_ms &from, const tripoint_bub_ms &to ) const;
public:
/**
Expand Down
244 changes: 244 additions & 0 deletions tests/map_path_test.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,244 @@
#include "cata_catch.h"
#include "coordinates.h"
#include "coords_fwd.h"
#include "coordinate_constants.h"
#include "game.h"
#include "map.h"
#include "map_helpers.h"
#include "type_id.h"

static void place_obstacle( map &m, const std::vector<tripoint_bub_ms> &places )
{
const ter_id t_wall_metal( "t_wall_metal" );
for( const tripoint_bub_ms &p : places ) {
m.ter_set( p, t_wall_metal );
}
m.set_transparency_cache_dirty( 0 );
m.build_map_cache( 0 );
for( const tripoint_bub_ms &p : places ) {
REQUIRE( !m.is_transparent( p.raw() ) );
}
}

static void expect_path( const std::vector<tripoint_bub_ms> &expected,
const std::vector<tripoint_bub_ms> &actual )
{
THEN( "path avoids obstacle" ) {
CHECK( expected == actual );
}
}

static map &setup_map_without_obstacles()
{
const ter_id t_floor( "t_floor" );
map &here = get_map();
clear_map();
const tripoint_bub_ms topleft = tripoint_bub_ms_zero;
const tripoint_bub_ms bottomright { 20, 20, 0 };
for( const tripoint_bub_ms &p : here.points_in_rectangle( topleft, bottomright ) ) {
here.ter_set( p, t_floor );
}
return here;
}

TEST_CASE( "find_clear_path_with_adjacent_obstacles", "[map]" )
{
const tripoint_bub_ms p_center{ 10, 10, 0 };
// Adjacent mapsquares
const tripoint_bub_ms nw = p_center + tripoint_rel_ms_north_west;
const tripoint_bub_ms n = p_center + tripoint_rel_ms_north;
const tripoint_bub_ms ne = p_center + tripoint_rel_ms_north_east;
const tripoint_bub_ms e = p_center + tripoint_rel_ms_east;
const tripoint_bub_ms se = p_center + tripoint_rel_ms_south_east;
const tripoint_bub_ms s = p_center + tripoint_rel_ms_south;
const tripoint_bub_ms sw = p_center + tripoint_rel_ms_south_west;
const tripoint_bub_ms w = p_center + tripoint_rel_ms_west;
// Mapsquares further out
const tripoint_bub_ms n_nw = n + tripoint_rel_ms_north_west;
const tripoint_bub_ms n_n = n + tripoint_rel_ms_north;
const tripoint_bub_ms n_ne = n + tripoint_rel_ms_north_east;
const tripoint_bub_ms e_ne = e + tripoint_rel_ms_north_east;
const tripoint_bub_ms e_e = e + tripoint_rel_ms_east;
const tripoint_bub_ms e_se = e + tripoint_rel_ms_south_east;
const tripoint_bub_ms s_se = s + tripoint_rel_ms_south_east;
const tripoint_bub_ms s_s = s + tripoint_rel_ms_south;
const tripoint_bub_ms s_sw = s + tripoint_rel_ms_south_west;
const tripoint_bub_ms w_sw = w + tripoint_rel_ms_south_west;
const tripoint_bub_ms w_w = w + tripoint_rel_ms_west;
const tripoint_bub_ms w_nw = w + tripoint_rel_ms_north_west;

map &here = setup_map_without_obstacles();
GIVEN( "Map has adjacent obstacle directly nw, ne, se, sw" ) {
place_obstacle( here, { nw, ne, se, sw } );
WHEN( "map::find_clear_path to n_nw" ) {
expect_path( { n, n_nw }, here.find_clear_path( p_center, n_nw ) );
}
WHEN( "map::find_clear_path to n_n" ) {
expect_path( { n, n_n }, here.find_clear_path( p_center, n_n ) );
}
WHEN( "map::find_clear_path to n_ne" ) {
expect_path( { n, n_ne }, here.find_clear_path( p_center, n_ne ) );
}
WHEN( "map::find_clear_path to e_ne" ) {
expect_path( { e, e_ne }, here.find_clear_path( p_center, e_ne ) );
}
WHEN( "map::find_clear_path to e_e" ) {
expect_path( { e, e_e }, here.find_clear_path( p_center, e_e ) );
}
WHEN( "map::find_clear_path to e_se" ) {
expect_path( { e, e_se }, here.find_clear_path( p_center, e_se ) );
}
WHEN( "map::find_clear_path to s_se" ) {
expect_path( { s, s_se }, here.find_clear_path( p_center, s_se ) );
}
WHEN( "map::find_clear_path to s_s" ) {
expect_path( { s, s_s }, here.find_clear_path( p_center, s_s ) );
}
WHEN( "map::find_clear_path to s_sw" ) {
expect_path( { s, s_sw }, here.find_clear_path( p_center, s_sw ) );
}
WHEN( "map::find_clear_path to w_sw" ) {
expect_path( { w, w_sw }, here.find_clear_path( p_center, w_sw ) );
}
WHEN( "map::find_clear_path to w_w" ) {
expect_path( { w, w_w }, here.find_clear_path( p_center, w_w ) );
}
WHEN( "map::find_clear_path to w_nw" ) {
expect_path( { w, w_nw }, here.find_clear_path( p_center, w_nw ) );
}
}
GIVEN( "Map has adjacent obstacle directly n, e, s, w" ) {
place_obstacle( here, { n, e, s, w } );
WHEN( "map::find_clear_path to n_nw" ) {
expect_path( { nw, n_nw }, here.find_clear_path( p_center, n_nw ) );
}
WHEN( "map::find_clear_path to n_ne" ) {
expect_path( { ne, n_ne }, here.find_clear_path( p_center, n_ne ) );
}
WHEN( "map::find_clear_path to e_ne" ) {
expect_path( { ne, e_ne }, here.find_clear_path( p_center, e_ne ) );
}
WHEN( "map::find_clear_path to e_se" ) {
expect_path( { se, e_se }, here.find_clear_path( p_center, e_se ) );
}
WHEN( "map::find_clear_path to s_se" ) {
expect_path( { se, s_se }, here.find_clear_path( p_center, s_se ) );
}
WHEN( "map::find_clear_path to s_sw" ) {
expect_path( { sw, s_sw }, here.find_clear_path( p_center, s_sw ) );
}
WHEN( "map::find_clear_path to w_sw" ) {
expect_path( { sw, w_sw }, here.find_clear_path( p_center, w_sw ) );
}
WHEN( "map::find_clear_path to w_nw" ) {
expect_path( { nw, w_nw }, here.find_clear_path( p_center, w_nw ) );
}
}
clear_map();
}


TEST_CASE( "find_clear_path_5tiles_with_obstacles", "[map]" )
{
map &here = setup_map_without_obstacles();
const tripoint_bub_ms source{ 10, 10, 0 };
const tripoint_bub_ms target{ 5, 8, 0 };
GIVEN( "Map has obstacle southeast of target" ) {
/*
* Map layout: t=target s=source o=obstacle
* t . . . . . .
* . o . . . . .
* . . . . . s .
*/
const tripoint_bub_ms obstacle{ 6, 9, 0 };
place_obstacle( here, { obstacle } );
WHEN( "map::find_clear_path to target" ) {
const std::vector<tripoint_bub_ms> path = here.find_clear_path( source, target );
THEN( "path avoids obstacle" ) {
CHECK( path[0] == tripoint_bub_ms{ 9, 9, 0 } );
CHECK( path[1] == tripoint_bub_ms{ 8, 9, 0 } );
CHECK( path[2] == tripoint_bub_ms{ 7, 9, 0 } );
CHECK( path[3] == tripoint_bub_ms{ 6, 8, 0 } );
CHECK( path[4] == tripoint_bub_ms{ 5, 8, 0 } );
const bool path_contains_obstacle = std::find( path.begin(), path.end(), obstacle ) != path.end();
CHECK_FALSE( path_contains_obstacle );
}
CHECK( here.sees( source, target, -1 ) );
CHECK( here.sees( target, source, -1 ) );
}
}
GIVEN( "Map has obstacle east of target" ) {
/*
* Map layout: t=target s=source o=obstacle
* t o . . . . .
* . . . . . . .
* . . . . . s .
*/
const tripoint_bub_ms obstacle{ 6, 8, 0 };
place_obstacle( here, { obstacle } );
WHEN( "map::find_clear_path to target" ) {
const std::vector<tripoint_bub_ms> path = here.find_clear_path( source, target );
THEN( "path avoids obstacle" ) {
CHECK( path[0] == tripoint_bub_ms{ 9, 10, 0 } );
CHECK( path[1] == tripoint_bub_ms{ 8, 9, 0 } );
CHECK( path[2] == tripoint_bub_ms{ 7, 9, 0 } );
CHECK( path[3] == tripoint_bub_ms{ 6, 9, 0 } );
CHECK( path[4] == tripoint_bub_ms{ 5, 8, 0 } );
const bool path_contains_obstacle = std::find( path.begin(), path.end(), obstacle ) != path.end();
CHECK_FALSE( path_contains_obstacle );
}
CHECK( here.sees( source, target, -1 ) );
CHECK( here.sees( target, source, -1 ) );
}
}
GIVEN( "Map has obstacle northwest of source" ) {
/*
* Map layout: t=target s=source o=obstacle
* t . . . . . .
* . . . . o . .
* . . . . . s .
*/
const tripoint_bub_ms obstacle{ 9, 9, 0 };
place_obstacle( here, { obstacle } );
WHEN( "map::find_clear_path to target" ) {
const std::vector<tripoint_bub_ms> path = here.find_clear_path( source, target );
THEN( "path avoids obstacle" ) {
CHECK( path[0] == tripoint_bub_ms{ 9, 10, 0 } );
CHECK( path[1] == tripoint_bub_ms{ 8, 9, 0 } );
CHECK( path[2] == tripoint_bub_ms{ 7, 9, 0 } );
CHECK( path[3] == tripoint_bub_ms{ 6, 8, 0 } );
CHECK( path[4] == tripoint_bub_ms{ 5, 8, 0 } );
const bool path_contains_obstacle = std::find( path.begin(), path.end(), obstacle ) != path.end();
CHECK_FALSE( path_contains_obstacle );
}
CHECK( here.sees( source, target, -1 ) );
CHECK( here.sees( target, source, -1 ) );
}
}
GIVEN( "Map has obstacle west of source" ) {
/*
* Map layout: t=target s=source o=obstacle
* t . . . . . .
* . . . . . . .
* . . . . o s .
*/
const tripoint_bub_ms obstacle{ 9, 10, 0 };
place_obstacle( here, { obstacle } );
WHEN( "map::find_clear_path to target" ) {
const std::vector<tripoint_bub_ms> path = here.find_clear_path( source, target );
THEN( "path avoids obstacle" ) {
CHECK( path[0] == tripoint_bub_ms{ 9, 9, 0 } );
CHECK( path[1] == tripoint_bub_ms{ 8, 9, 0 } );
CHECK( path[2] == tripoint_bub_ms{ 7, 9, 0 } );
CHECK( path[3] == tripoint_bub_ms{ 6, 8, 0 } );
CHECK( path[4] == tripoint_bub_ms{ 5, 8, 0 } );
const bool path_contains_obstacle = std::find( path.begin(), path.end(), obstacle ) != path.end();
CHECK_FALSE( path_contains_obstacle );
}
CHECK( here.sees( source, target, -1 ) );
CHECK( here.sees( target, source, -1 ) );
}
}
clear_map();
}

0 comments on commit 04fcfa7

Please sign in to comment.