Skip to content

Commit

Permalink
Add custom clang-tidy check to improve use of point arithmetic operat…
Browse files Browse the repository at this point in the history 
…ors (#33161)

* Support int * point multiplication

Previously the point operand had to be on the LHS.

* Add point + tripoint operator

Previously you could only do tripoint + point.

* Initial version of UsePoingArithmetic check

This is a clang-tidy check intended to automatically refactor arithmetic
using point and tripoint coordinates to be higher-level point
arithmetic.

* Perform a refactoring pass

* Manually fix some coordinate transformations

These were highlighted by the new cata-use-point-arithmetic check.
  • Loading branch information
@jbytheway @ZhilkinSerg
jbytheway authored and ZhilkinSerg committed Aug 12, 2019
1 parent 4df9ad2 commit 6c8f543
Show file tree
Hide file tree
Showing 48 changed files with 1,108 additions and 242 deletions.
16 changes: 8 additions & 8 deletions src/activity_handlers.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1739,7 +1739,7 @@ void activity_handlers::pulp_do_turn( player_activity *act, player *p )
units::legacy_volume_factor ) ) { // Splatter some blood around
// Splatter a bit more randomly, so that it looks cooler
const int radius = mess_radius + x_in_y( pulp_power, 500 ) + x_in_y( pulp_power, 1000 );
const tripoint dest( pos.x + rng( -radius, radius ), pos.y + rng( -radius, radius ), pos.z );
const tripoint dest( pos + point( rng( -radius, radius ), rng( -radius, radius ) ) );
const field_type_id type_blood = ( mess_radius > 1 && x_in_y( pulp_power, 10000 ) ) ?
corpse.get_mtype()->gibType() :
corpse.get_mtype()->bloodType();
Expand Down Expand Up @@ -2162,9 +2162,9 @@ void activity_handlers::oxytorch_finish( player_activity *act, player *p )
g->m.spawn_item( pos, "spike", rng( 1, 19 ) );
g->m.spawn_item( pos, "scrap", rng( 1, 8 ) );
} else if( ter == t_bars ) {
if( g->m.ter( {pos.x + 1, pos.y, pos.z} ) == t_sewage || g->m.ter( {pos.x, pos.y + 1, pos.z} ) ==
if( g->m.ter( pos + point( 1, 0 ) ) == t_sewage || g->m.ter( pos + point( 0, 1 ) ) ==
t_sewage ||
g->m.ter( {pos.x - 1, pos.y, pos.z} ) == t_sewage || g->m.ter( {pos.x, pos.y - 1, pos.z} ) ==
g->m.ter( pos + point( -1, 0 ) ) == t_sewage || g->m.ter( pos + point( 0, -1 ) ) ==
t_sewage ) {
g->m.ter_set( pos, t_sewage );
g->m.spawn_item( p->pos(), "pipe", rng( 1, 2 ) );
Expand Down Expand Up @@ -2652,7 +2652,7 @@ void activity_handlers::travel_do_turn( player_activity *act, player *p )
return;
}
tripoint sm_tri = g->m.getlocal( sm_to_ms_copy( omt_to_sm_copy( p->omt_path.back() ) ) );
tripoint centre_sub = tripoint( sm_tri.x + SEEX, sm_tri.y + SEEY, sm_tri.z );
tripoint centre_sub = sm_tri + point( SEEX, SEEY );
if( !g->m.passable( centre_sub ) ) {
auto candidates = g->m.points_in_radius( centre_sub, 2 );
for( const auto &elem : candidates ) {
Expand Down Expand Up @@ -3417,9 +3417,9 @@ void activity_handlers::hacksaw_finish( player_activity *act, player *p )
g->m.spawn_item( p->pos(), "spike", 19 );
g->m.spawn_item( p->pos(), "scrap", 8 );
} else if( ter == t_bars ) {
if( g->m.ter( { pos.x + 1, pos.y, pos.z } ) == t_sewage || g->m.ter( { pos.x, pos.y + 1, pos.z } )
if( g->m.ter( pos + point( 1, 0 ) ) == t_sewage || g->m.ter( pos + point( 0, 1 ) )
== t_sewage ||
g->m.ter( { pos.x - 1, pos.y, pos.z } ) == t_sewage || g->m.ter( { pos.x, pos.y - 1, pos.z } ) ==
g->m.ter( pos + point( -1, 0 ) ) == t_sewage || g->m.ter( pos + point( 0, -1 ) ) ==
t_sewage ) {
g->m.ter_set( pos, t_sewage );
g->m.spawn_item( p->pos(), "pipe", 3 );
Expand Down Expand Up @@ -3464,7 +3464,7 @@ void activity_handlers::chop_tree_finish( player_activity *act, player *p )
// try again
}

const tripoint to = pos + point( 3 * direction.x + rng( -1, 1 ), 3 * direction.y + rng( -1, 1 ) );
const tripoint to = pos + 3 * direction.xy() + point( rng( -1, 1 ), rng( -1, 1 ) );
std::vector<tripoint> tree = line_to( pos, to, rng( 1, 8 ) );
for( auto &elem : tree ) {
g->m.destroy( elem );
Expand Down Expand Up @@ -4071,7 +4071,7 @@ void activity_handlers::tree_communion_do_turn( player_activity *act, player *p
}
for( int dx = -1; dx <= 1; dx++ ) {
for( int dy = -1; dy <= 1; dy++ ) {
tripoint neighbor = tripoint( tpt.x + dx, tpt.y + dy, tpt.z );
tripoint neighbor = tpt + point( dx, dy );
if( seen.find( neighbor ) != seen.end() ) {
continue;
}
Expand Down
20 changes: 9 additions & 11 deletions src/animation.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,7 @@ bool is_layer_visible( const std::map<tripoint, explosion_tile> &layer )
//! Get (x, y) relative to u's current position and view
tripoint relative_view_pos( const player &u, const int x, const int y, const int z ) noexcept
{
return tripoint { POSX + x - u.posx() - u.view_offset.x,
POSY + y - u.posy() - u.view_offset.y,
z - u.posz() - u.view_offset.z };
return -u.view_offset + tripoint( POSX + x - u.posx(), POSY + y - u.posy(), z - u.posz() );
}

tripoint relative_view_pos( const player &u, const tripoint &p ) noexcept
Expand Down Expand Up @@ -339,10 +337,10 @@ void explosion_handler::draw_custom_explosion( const tripoint &,
const tripoint &pt = pr.first;
explosion_neighbors &ngh = pr.second.neighborhood;

set_neighbors( tripoint( pt.x - 1, pt.y, pt.z ), ngh, N_WEST, N_EAST );
set_neighbors( tripoint( pt.x + 1, pt.y, pt.z ), ngh, N_EAST, N_WEST );
set_neighbors( tripoint( pt.x, pt.y - 1, pt.z ), ngh, N_NORTH, N_SOUTH );
set_neighbors( tripoint( pt.x, pt.y + 1, pt.z ), ngh, N_SOUTH, N_NORTH );
set_neighbors( pt + point( -1, 0 ), ngh, N_WEST, N_EAST );
set_neighbors( pt + point( 1, 0 ), ngh, N_EAST, N_WEST );
set_neighbors( pt + point( 0, -1 ), ngh, N_NORTH, N_SOUTH );
set_neighbors( pt + point( 0, 1 ), ngh, N_SOUTH, N_NORTH );
}

// We need to save the layers because we will draw them in reverse order
Expand All @@ -366,10 +364,10 @@ void explosion_handler::draw_custom_explosion( const tripoint &,
const tripoint &pt = pr.first;
const explosion_neighbors ngh = pr.second.neighborhood;

unset_neighbor( tripoint( pt.x - 1, pt.y, pt.z ), ngh, N_WEST, N_EAST );
unset_neighbor( tripoint( pt.x + 1, pt.y, pt.z ), ngh, N_EAST, N_WEST );
unset_neighbor( tripoint( pt.x, pt.y - 1, pt.z ), ngh, N_NORTH, N_SOUTH );
unset_neighbor( tripoint( pt.x, pt.y + 1, pt.z ), ngh, N_SOUTH, N_NORTH );
unset_neighbor( pt + point( -1, 0 ), ngh, N_WEST, N_EAST );
unset_neighbor( pt + point( 1, 0 ), ngh, N_EAST, N_WEST );
unset_neighbor( pt + point( 0, -1 ), ngh, N_NORTH, N_SOUTH );
unset_neighbor( pt + point( 0, 1 ), ngh, N_SOUTH, N_NORTH );
neighbors.erase( pr.first );
}

Expand Down
62 changes: 30 additions & 32 deletions src/cata_tiles.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ void tileset_loader::copy_surface_to_texture( const SDL_Surface_Ptr &surf, const
for( const SDL_Rect rect : input_range ) {
assert( offset.x % sprite_width == 0 );
assert( offset.y % sprite_height == 0 );
const point pos( offset.x + rect.x, offset.y + rect.y );
const point pos( offset + point( rect.x, rect.y ) );
assert( pos.x % sprite_width == 0 );
assert( pos.y % sprite_height == 0 );
const size_t index = this->offset + ( pos.x / sprite_width ) + ( pos.y / sprite_height ) *
Expand Down Expand Up @@ -1282,11 +1282,9 @@ void cata_tiles::draw( int destx, int desty, const tripoint &center, int width,
}
if( g->u.controlling_vehicle ) {
if( cata::optional<tripoint> indicator_offset = g->get_veh_dir_indicator_location( true ) ) {
draw_from_id_string( "cursor", C_NONE, empty_string, {
indicator_offset->x + g->u.posx(),
indicator_offset->y + g->u.posy(), center.z
},
0, 0, LL_LIT, false );
draw_from_id_string( "cursor", C_NONE, empty_string, indicator_offset->xy() + tripoint( g->u.posx(),
g->u.posy(), center.z ),
0, 0, LL_LIT, false );
}
}

Expand Down Expand Up @@ -2045,10 +2043,10 @@ bool cata_tiles::draw_furniture( const tripoint &p, lit_level ll, int &height_3d

// for rotation information
const int neighborhood[4] = {
static_cast<int>( g->m.furn( tripoint( p.x, p.y + 1, p.z ) ) ), // south
static_cast<int>( g->m.furn( tripoint( p.x + 1, p.y, p.z ) ) ), // east
static_cast<int>( g->m.furn( tripoint( p.x - 1, p.y, p.z ) ) ), // west
static_cast<int>( g->m.furn( tripoint( p.x, p.y - 1, p.z ) ) ) // north
static_cast<int>( g->m.furn( p + point( 0, 1 ) ) ), // south
static_cast<int>( g->m.furn( p + point( 1, 0 ) ) ), // east
static_cast<int>( g->m.furn( p + point( -1, 0 ) ) ), // west
static_cast<int>( g->m.furn( p + point( 0, -1 ) ) ) // north
};

int subtile = 0;
Expand All @@ -2074,10 +2072,10 @@ bool cata_tiles::draw_trap( const tripoint &p, lit_level ll, int &height_3d )
}

const int neighborhood[4] = {
static_cast<int>( g->m.tr_at( tripoint( p.x, p.y + 1, p.z ) ).loadid ), // south
static_cast<int>( g->m.tr_at( tripoint( p.x + 1, p.y, p.z ) ).loadid ), // east
static_cast<int>( g->m.tr_at( tripoint( p.x - 1, p.y, p.z ) ).loadid ), // west
static_cast<int>( g->m.tr_at( tripoint( p.x, p.y - 1, p.z ) ).loadid ) // north
static_cast<int>( g->m.tr_at( p + point( 0, 1 ) ).loadid ), // south
static_cast<int>( g->m.tr_at( p + point( 1, 0 ) ).loadid ), // east
static_cast<int>( g->m.tr_at( p + point( -1, 0 ) ).loadid ), // west
static_cast<int>( g->m.tr_at( p + point( 0, -1 ) ).loadid ) // north
};

int subtile = 0;
Expand Down Expand Up @@ -2110,10 +2108,10 @@ bool cata_tiles::draw_field_or_item( const tripoint &p, lit_level ll, int &heigh
if( fld_type.display_field ) {
// for rotation information
const int neighborhood[4] = {
static_cast<int>( g->m.field_at( tripoint( p.x, p.y + 1, p.z ) ).displayed_field_type() ), // south
static_cast<int>( g->m.field_at( tripoint( p.x + 1, p.y, p.z ) ).displayed_field_type() ), // east
static_cast<int>( g->m.field_at( tripoint( p.x - 1, p.y, p.z ) ).displayed_field_type() ), // west
static_cast<int>( g->m.field_at( tripoint( p.x, p.y - 1, p.z ) ).displayed_field_type() ) // north
static_cast<int>( g->m.field_at( p + point( 0, 1 ) ).displayed_field_type() ), // south
static_cast<int>( g->m.field_at( p + point( 1, 0 ) ).displayed_field_type() ), // east
static_cast<int>( g->m.field_at( p + point( -1, 0 ) ).displayed_field_type() ), // west
static_cast<int>( g->m.field_at( p + point( 0, -1 ) ).displayed_field_type() ) // north
};

int subtile = 0;
Expand Down Expand Up @@ -2545,27 +2543,27 @@ void cata_tiles::draw_explosion_frame()
subtile = corner;
rotation = 0;

draw_from_id_string( exp_name, { exp_pos.x - i, exp_pos.y - i, exp_pos.z },
draw_from_id_string( exp_name, exp_pos + point( -i, -i ),
subtile, rotation++, LL_LIT, nv_goggles_activated );
draw_from_id_string( exp_name, { exp_pos.x - i, exp_pos.y + i, exp_pos.z },
draw_from_id_string( exp_name, exp_pos + point( -i, i ),
subtile, rotation++, LL_LIT, nv_goggles_activated );
draw_from_id_string( exp_name, { exp_pos.x + i, exp_pos.y + i, exp_pos.z },
draw_from_id_string( exp_name, exp_pos + point( i, i ),
subtile, rotation++, LL_LIT, nv_goggles_activated );
draw_from_id_string( exp_name, { exp_pos.x + i, exp_pos.y - i, exp_pos.z },
draw_from_id_string( exp_name, exp_pos + point( i, -i ),
subtile, rotation++, LL_LIT, nv_goggles_activated );

subtile = edge;
for( int j = 1 - i; j < 0 + i; j++ ) {
rotation = 0;
draw_from_id_string( exp_name, { exp_pos.x + j, exp_pos.y - i, exp_pos.z },
draw_from_id_string( exp_name, exp_pos + point( j, -i ),
subtile, rotation, LL_LIT, nv_goggles_activated );
draw_from_id_string( exp_name, { exp_pos.x + j, exp_pos.y + i, exp_pos.z },
draw_from_id_string( exp_name, exp_pos + point( j, i ),
subtile, rotation, LL_LIT, nv_goggles_activated );

rotation = 1;
draw_from_id_string( exp_name, { exp_pos.x - i, exp_pos.y + j, exp_pos.z },
draw_from_id_string( exp_name, exp_pos + point( -i, j ),
subtile, rotation, LL_LIT, nv_goggles_activated );
draw_from_id_string( exp_name, { exp_pos.x + i, exp_pos.y + j, exp_pos.z },
draw_from_id_string( exp_name, exp_pos + point( i, j ),
subtile, rotation, LL_LIT, nv_goggles_activated );
}
}
Expand Down Expand Up @@ -2745,8 +2743,8 @@ void cata_tiles::draw_zones_frame()
for( int iY = zone_start.y; iY <= zone_end.y; ++ iY ) {
for( int iX = zone_start.x; iX <= zone_end.x; ++iX ) {
draw_from_id_string( "highlight", C_NONE, empty_string,
{ iX + zone_offset.x, iY + zone_offset.y, g->u.pos().z },
0, 0, LL_LIT, false );
zone_offset.xy() + tripoint( iX, iY, g->u.pos().z ),
0, 0, LL_LIT, false );
}
}

Expand Down Expand Up @@ -2777,10 +2775,10 @@ void cata_tiles::get_terrain_orientation( const tripoint &p, int &rota, int &sub

// get terrain neighborhood
const ter_id neighborhood[4] = {
g->m.ter( tripoint( p.x, p.y + 1, p.z ) ), // south
g->m.ter( tripoint( p.x + 1, p.y, p.z ) ), // east
g->m.ter( tripoint( p.x - 1, p.y, p.z ) ), // west
g->m.ter( tripoint( p.x, p.y - 1, p.z ) ) // north
g->m.ter( p + point( 0, 1 ) ), // south
g->m.ter( p + point( 1, 0 ) ), // east
g->m.ter( p + point( -1, 0 ) ), // west
g->m.ter( p + point( 0, -1 ) ) // north
};

bool connects[4];
Expand Down
8 changes: 4 additions & 4 deletions src/computer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1586,16 +1586,16 @@ void computer::activate_failure( computer_failure_type fail )
for( int i = 0; i < leak_size; i++ ) {
std::vector<point> next_move;
if( g->m.passable( p.x, p.y - 1 ) ) {
next_move.push_back( point( p.x, p.y - 1 ) );
next_move.push_back( p + point( 0, -1 ) );
}
if( g->m.passable( p.x + 1, p.y ) ) {
next_move.push_back( point( p.x + 1, p.y ) );
next_move.push_back( p + point( 1, 0 ) );
}
if( g->m.passable( p.x, p.y + 1 ) ) {
next_move.push_back( point( p.x, p.y + 1 ) );
next_move.push_back( p + point( 0, 1 ) );
}
if( g->m.passable( p.x - 1, p.y ) ) {
next_move.push_back( point( p.x - 1, p.y ) );
next_move.push_back( p + point( -1, 0 ) );
}

if( next_move.empty() ) {
Expand Down
8 changes: 4 additions & 4 deletions src/construction.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -947,10 +947,10 @@ bool construct::check_empty( const tripoint &p )
inline std::array<tripoint, 4> get_orthogonal_neighbors( const tripoint &p )
{
return {{
tripoint( p.x, p.y - 1, p.z ),
tripoint( p.x, p.y + 1, p.z ),
tripoint( p.x - 1, p.y, p.z ),
tripoint( p.x + 1, p.y, p.z )
p + point( 0, -1 ),
p + point( 0, 1 ),
p + point( -1, 0 ),
p + point( 1, 0 )
}};
}

Expand Down
5 changes: 5 additions & 0 deletions src/coordinate_conversions.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,11 @@ point sm_to_om_remain( int &x, int &y )
return point( divide( x, 2 * OMAPX, x ), divide( y, 2 * OMAPY, y ) );
}

point omt_to_ms_copy( const point &p )
{
return point( p.x * 2 * SEEX, p.x * 2 * SEEY );
}

point omt_to_sm_copy( int x, int y )
{
return point( x * 2, y * 2 );
Expand Down
2 changes: 2 additions & 0 deletions src/coordinate_conversions.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,8 @@ inline void omt_to_sm( tripoint &p )
{
omt_to_sm( p.x, p.y );
}
// overmap terrain to map square
point omt_to_ms_copy( const point &p );
// overmap to submap, basically: x *= 2 * OMAPX
point om_to_sm_copy( int x, int y );
inline point om_to_sm_copy( const point &p )
Expand Down
3 changes: 1 addition & 2 deletions src/debug_menu.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1292,8 +1292,7 @@ void debug()
#if defined(TILES)
// *INDENT-OFF*
const point offset{
POSX - u.posx() + u.view_offset.x,
POSY - u.posy() + u.view_offset.y
u.view_offset.xy() + point( POSX - u.posx(), POSY - u.posy() )
}; // *INDENT-ON*
g->draw_ter();
auto sounds_to_draw = sounds::get_monster_sounds();
Expand Down
4 changes: 2 additions & 2 deletions src/defense.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1424,12 +1424,12 @@ void defense_game::spawn_wave_monster( const mtype_id &type )
} else if( one_in( 2 ) ) {
pnt = point( rng( HALF_MAPSIZE_X, HALF_MAPSIZE_X + SEEX ), rng( 1, SEEY ) );
if( one_in( 2 ) ) {
pnt = point( pnt.x, MAPSIZE_Y - 1 - pnt.y );
pnt = point( pnt.x, -pnt.y ) + point( 0, MAPSIZE_Y - 1 );
}
} else {
pnt = point( rng( 1, SEEX ), rng( HALF_MAPSIZE_Y, HALF_MAPSIZE_Y + SEEY ) );
if( one_in( 2 ) ) {
pnt = point( MAPSIZE_X - 1 - pnt.x, pnt.y );
pnt = point( -pnt.x, pnt.y ) + point( MAPSIZE_X - 1, 0 );
}
}
if( g->is_empty( { pnt, g->get_levz() } ) ) {
Expand Down
Loading

0 comments on commit 6c8f543

Please sign in to comment.