From 273482e5510c6fee4f13e726546d9301aa3d7bc0 Mon Sep 17 00:00:00 2001 From: John Bytheway Date: Fri, 28 Jun 2019 23:12:23 +0100 Subject: [PATCH 01/14] Add Catch2 StringMaker definition for point --- tests/stringmaker.h | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/stringmaker.h b/tests/stringmaker.h index f65d7fc3c2450..bf8cf7521a5dd 100644 --- a/tests/stringmaker.h +++ b/tests/stringmaker.h @@ -26,6 +26,13 @@ struct StringMaker { } }; +template<> +struct StringMaker { + static std::string convert( const point &p ) { + return string_format( "point( %d, %d )", p.x, p.y ); + } +}; + template<> struct StringMaker { static std::string convert( const rectangle &r ) { From c749e6b08d619dc8cd69d22c403dfcf6f8856343 Mon Sep 17 00:00:00 2001 From: John Bytheway Date: Fri, 28 Jun 2019 23:14:05 +0100 Subject: [PATCH 02/14] Add Catch2 generators for point, tripoint --- tests/cata_generators.cpp | 67 +++++++++++++++++++++++++++++++++++++++ tests/cata_generators.h | 18 +++++++++++ 2 files changed, 85 insertions(+) create mode 100644 tests/cata_generators.cpp create mode 100644 tests/cata_generators.h diff --git a/tests/cata_generators.cpp b/tests/cata_generators.cpp new file mode 100644 index 0000000000000..143359361aab6 --- /dev/null +++ b/tests/cata_generators.cpp @@ -0,0 +1,67 @@ +#include "cata_generators.h" + +#include "point.h" +#include "rng.h" + +class RandomPointGenerator : + public Catch::Generators::IGenerator +{ + public: + RandomPointGenerator( int low, int high ) : + engine( rng_get_engine() ), + dist( low, high ) { + this->next(); + } + + const point &get() const override { + return current_point; + } + + bool next() override { + current_point = point( dist( engine ), dist( engine ) ); + return true; + } + protected: + cata_default_random_engine &engine; + std::uniform_int_distribution<> dist; + point current_point; +}; + +class RandomTripointGenerator : + public Catch::Generators::IGenerator +{ + public: + RandomTripointGenerator( int low, int high, int zlow, int zhigh ) : + engine( rng_get_engine() ), + xy_dist( low, high ), + z_dist( zlow, zhigh ) { + this->next(); + } + + const tripoint &get() const override { + return current_point; + } + + bool next() override { + current_point = tripoint( xy_dist( engine ), xy_dist( engine ), z_dist( engine ) ); + return true; + } + protected: + cata_default_random_engine &engine; + std::uniform_int_distribution<> xy_dist; + std::uniform_int_distribution<> z_dist; + tripoint current_point; +}; + +Catch::Generators::GeneratorWrapper random_points( int low, int high ) +{ + return Catch::Generators::GeneratorWrapper( + std::make_unique( low, high ) ); +} + +Catch::Generators::GeneratorWrapper random_tripoints( + int low, int high, int zlow, int zhigh ) +{ + return Catch::Generators::GeneratorWrapper( + std::make_unique( low, high, zlow, zhigh ) ); +} diff --git a/tests/cata_generators.h b/tests/cata_generators.h new file mode 100644 index 0000000000000..8be27ce8b9136 --- /dev/null +++ b/tests/cata_generators.h @@ -0,0 +1,18 @@ +#pragma once +#ifndef CATA_GENERATORS_H +#define CATA_GENERATORS_H + +// Some Catch2 Generators for generating our data types + +#include "catch/catch.hpp" +#include "game_constants.h" + +struct point; +struct tripoint; + +Catch::Generators::GeneratorWrapper random_points( int low = -1000, int high = 1000 ); + +Catch::Generators::GeneratorWrapper random_tripoints( + int low = -1000, int high = 1000, int zlow = -OVERMAP_DEPTH, int zhigh = OVERMAP_HEIGHT ); + +#endif // CATA_GENERATORS_H From cd62e735b93747c204dd884272fa72cdb8cec3bf Mon Sep 17 00:00:00 2001 From: John Bytheway Date: Sun, 30 Jun 2019 21:54:14 +0100 Subject: [PATCH 03/14] Implement initial type-safe points and conversions This takes the form of a wrapper class around point and/or tripoint which adds type parameters for the coordinate system in which the point has meaning. Added some helper functions to point.h needed to implement the conversions. --- src/coordinates.h | 171 ++++++++++++++++++++++++++++++++++++++++++++++ src/point.h | 31 +++++++++ 2 files changed, 202 insertions(+) diff --git a/src/coordinates.h b/src/coordinates.h index 3aaff85dc9e5d..2d216cc24c058 100644 --- a/src/coordinates.h +++ b/src/coordinates.h @@ -8,6 +8,177 @@ #include "enums.h" #include "game_constants.h" #include "point.h" +#include "debug.h" + +namespace coords +{ + +enum class scale { + map_square, + submap, + overmap_terrain, + segment, + overmap, + vehicle +}; + +constexpr scale ms = scale::map_square; +constexpr scale sm = scale::submap; +constexpr scale omt = scale::overmap_terrain; +constexpr scale seg = scale::segment; +constexpr scale om = scale::overmap; + +constexpr int map_squares_per( scale s ) +{ + static_assert( SEEX == SEEY, "we assume submaps are square" ); + static_assert( OMAPX == OMAPY, "we assume overmaps are square" ); + + switch( s ) { + case scale::map_square: + return 1; + case scale::submap: + return SEEX; + case scale::overmap_terrain: + return SEEX * 2; + case scale::segment: + return SEG_SIZE * map_squares_per( scale::overmap_terrain ); + case scale::overmap: + return OMAPX * map_squares_per( scale::overmap_terrain ); + default: + debugmsg( "Requested scale of %d", s ); + abort(); + } +} + +enum class origin { + relative, // this is a special origin that can be added to any other + abs, // the global absolute origin for the entire game + submap, // from corner of submap + overmap_terrain, // from corner of overmap_terrain + overmap, // from corner of overmap +}; + +constexpr origin origin_from_scale( scale s ) +{ + switch( s ) { + case scale::submap: + return origin::submap; + case scale::overmap_terrain: + return origin::overmap_terrain; + case scale::overmap: + return origin::overmap; + default: + debugmsg( "Requested origin for of %d", s ); + abort(); + } +} + +// A generic coordinate-type-safe point. +// Point should be the underlying representation type (either point or +// tripoint). +// Scale and Origin define the coordinate system for the point. +template +class coord_point +{ + public: + coord_point() = default; + explicit coord_point( const Point &p ) : + raw_( p ) + {} + + Point &raw() { + return raw_; + } + const Point &raw() const { + return raw_; + } + private: + Point raw_; +}; + +template +struct project_to_impl; + +template +struct project_to_impl { + template + coord_point operator()( + const coord_point &src ) { + return coord_point( multiply_xy( src.raw(), ScaleUp ) ); + } +}; + +template +struct project_to_impl<0, ScaleDown, ResultScale> { + template + coord_point operator()( + const coord_point &src ) { + return coord_point( + divide_xy_round_to_minus_infinity( src.raw(), ScaleDown ) ); + } +}; + +template +inline coord_point project_to( + const coord_point &src ) +{ + constexpr int scale_down = map_squares_per( ResultScale ) / map_squares_per( SourceScale ); + constexpr int scale_up = map_squares_per( SourceScale ) / map_squares_per( ResultScale ); + return project_to_impl()( src ); +} + +template +struct quotient_remainder { + constexpr static origin RemainderOrigin = origin_from_scale( CoarseScale ); + using quotient_type = coord_point; + quotient_type quotient; + using remainder_type = coord_point; + remainder_type remainder; + + // For assigning to std::tie( q, r ); + operator std::tuple() { + return std::tie( quotient, remainder ); + } +}; + +template +inline quotient_remainder project_remain( + const coord_point &src ) +{ + constexpr int ScaleDown = map_squares_per( ResultScale ) / map_squares_per( SourceScale ); + static_assert( ScaleDown > 0, "You can only project to coarser coordinate systems" ); + constexpr static origin RemainderOrigin = origin_from_scale( ResultScale ); + coord_point quotient( + divide_xy_round_to_minus_infinity( src.raw(), ScaleDown ) ); + coord_point remainder( + src.raw() - quotient.raw() * ScaleDown ); + + return { quotient, remainder }; +} + +} // namespace coords + +using point_ms_abs = coords::coord_point; +using point_ms_sm = coords::coord_point; +using point_ms_omt = coords::coord_point; +using point_sm_abs = coords::coord_point; +using point_sm_omt = coords::coord_point; +using point_sm_om = coords::coord_point; +using point_omt_abs = coords::coord_point; +using point_omt_om = coords::coord_point; +using point_seg_abs = coords::coord_point; +using point_om_abs = coords::coord_point; + +using tripoint_ms_abs = coords::coord_point; +using tripoint_ms_sm = coords::coord_point; +using tripoint_ms_omt = coords::coord_point; +using tripoint_sm_abs = coords::coord_point; +using tripoint_omt_abs = coords::coord_point; +using tripoint_seg_abs = coords::coord_point; +using tripoint_om_abs = coords::coord_point; + +using coords::project_to; +using coords::project_remain; /* find appropriate subdivided coordinates for absolute tile coordinate. * This is less obvious than one might think, for negative coordinates, so this diff --git a/src/point.h b/src/point.h index 4649d6b1fdfee..55bb2f68201d4 100644 --- a/src/point.h +++ b/src/point.h @@ -123,6 +123,25 @@ struct point { void serialize( const point &p, JsonOut &jsout ); void deserialize( point &p, JsonIn &jsin ); +inline int divide_round_to_minus_infinity( int n, int d ) +{ + if( n >= 0 ) { + return n / d; + } + return ( n - d + 1 ) / d; +} + +inline point multiply_xy( const point &p, int f ) +{ + return point( p.x * f, p.y * f ); +} + +inline point divide_xy_round_to_minus_infinity( const point &p, int d ) +{ + return point( divide_round_to_minus_infinity( p.x, d ), + divide_round_to_minus_infinity( p.y, d ) ); +} + // NOLINTNEXTLINE(cata-xy) struct tripoint { int x = 0; @@ -228,6 +247,18 @@ struct tripoint { } }; +inline tripoint multiply_xy( const tripoint &p, int f ) +{ + return tripoint( p.x * f, p.y * f, p.z ); +} + +inline tripoint divide_xy_round_to_minus_infinity( const tripoint &p, int d ) +{ + return tripoint( divide_round_to_minus_infinity( p.x, d ), + divide_round_to_minus_infinity( p.y, d ), + p.z ); +} + struct rectangle { point p_min; point p_max; From f35eb51e31915ec2963750c168663f5ef0e08daa Mon Sep 17 00:00:00 2001 From: John Bytheway Date: Sun, 30 Jun 2019 21:56:11 +0100 Subject: [PATCH 04/14] Tests for the coord_point conversion functions This tests project_to and project_remain. --- tests/coordinate_test.cpp | 135 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 135 insertions(+) create mode 100644 tests/coordinate_test.cpp diff --git a/tests/coordinate_test.cpp b/tests/coordinate_test.cpp new file mode 100644 index 0000000000000..74c8cf1545d37 --- /dev/null +++ b/tests/coordinate_test.cpp @@ -0,0 +1,135 @@ +#include "catch/catch.hpp" + +#include "coordinates.h" +#include "coordinate_conversions.h" +#include "cata_generators.h" +#include "stringmaker.h" + +constexpr int num_trials = 5; + +TEST_CASE( "coordinate_conversion_consistency", "[coords]" ) +{ + SECTION( "omt_to_om_point" ) { + point p = GENERATE( take( num_trials, random_points() ) ); + CAPTURE( p ); + point_om_abs new_conversion = project_to( point_omt_abs( p ) ); + point old_conversion = omt_to_om_copy( p ); + CHECK( old_conversion == new_conversion.raw() ); + } + + SECTION( "omt_to_om_tripoint" ) { + tripoint p = GENERATE( take( num_trials, random_tripoints() ) ); + CAPTURE( p ); + tripoint_om_abs new_conversion = project_to( tripoint_omt_abs( p ) ); + tripoint old_conversion = omt_to_om_copy( p ); + CHECK( old_conversion == new_conversion.raw() ); + } + + SECTION( "omt_to_om_remain_point" ) { + point p = GENERATE( take( num_trials, random_points() ) ); + CAPTURE( p ); + point_om_abs new_conversion; + point_omt_om remainder; + std::tie( new_conversion, remainder ) = project_remain( point_omt_abs( p ) ); + point old_conversion = omt_to_om_remain( p ); + CHECK( old_conversion == new_conversion.raw() ); + CHECK( p == remainder.raw() ); + } + + SECTION( "sm_to_omt_point" ) { + point p = GENERATE( take( num_trials, random_points() ) ); + CAPTURE( p ); + point_omt_abs new_conversion = project_to( point_sm_abs( p ) ); + point old_conversion = sm_to_omt_copy( p ); + CHECK( old_conversion == new_conversion.raw() ); + } + + SECTION( "sm_to_omt_remain_point" ) { + point p = GENERATE( take( num_trials, random_points() ) ); + CAPTURE( p ); + point_omt_abs new_conversion; + point_sm_omt remainder; + std::tie( new_conversion, remainder ) = project_remain( point_sm_abs( p ) ); + point old_conversion = sm_to_omt_remain( p ); + CHECK( old_conversion == new_conversion.raw() ); + CHECK( p == remainder.raw() ); + } + + SECTION( "sm_to_om_point" ) { + point p = GENERATE( take( num_trials, random_points() ) ); + CAPTURE( p ); + point_om_abs new_conversion = project_to( point_sm_abs( p ) ); + point old_conversion = sm_to_om_copy( p ); + CHECK( old_conversion == new_conversion.raw() ); + } + + SECTION( "sm_to_om_remain_point" ) { + point p = GENERATE( take( num_trials, random_points() ) ); + CAPTURE( p ); + point_om_abs new_conversion; + point_sm_om remainder; + std::tie( new_conversion, remainder ) = project_remain( point_sm_abs( p ) ); + point old_conversion = sm_to_om_remain( p ); + CHECK( old_conversion == new_conversion.raw() ); + CHECK( p == remainder.raw() ); + } + + SECTION( "omt_to_sm_point" ) { + point p = GENERATE( take( num_trials, random_points() ) ); + CAPTURE( p ); + point_sm_abs new_conversion = project_to( point_omt_abs( p ) ); + point old_conversion = omt_to_sm_copy( p ); + CHECK( old_conversion == new_conversion.raw() ); + } + + SECTION( "om_to_sm_point" ) { + point p = GENERATE( take( num_trials, random_points() ) ); + CAPTURE( p ); + point_sm_abs new_conversion = project_to( point_om_abs( p ) ); + point old_conversion = om_to_sm_copy( p ); + CHECK( old_conversion == new_conversion.raw() ); + } + + SECTION( "ms_to_sm_point" ) { + point p = GENERATE( take( num_trials, random_points() ) ); + CAPTURE( p ); + point_sm_abs new_conversion = project_to( point_ms_abs( p ) ); + point old_conversion = ms_to_sm_copy( p ); + CHECK( old_conversion == new_conversion.raw() ); + } + + SECTION( "sm_to_ms_point" ) { + point p = GENERATE( take( num_trials, random_points() ) ); + CAPTURE( p ); + point_ms_abs new_conversion = project_to( point_sm_abs( p ) ); + point old_conversion = sm_to_ms_copy( p ); + CHECK( old_conversion == new_conversion.raw() ); + } + + SECTION( "ms_to_omt_point" ) { + point p = GENERATE( take( num_trials, random_points() ) ); + CAPTURE( p ); + point_omt_abs new_conversion = project_to( point_ms_abs( p ) ); + point old_conversion = ms_to_omt_copy( p ); + CHECK( old_conversion == new_conversion.raw() ); + } + + SECTION( "ms_to_omt_remain_point" ) { + point p = GENERATE( take( num_trials, random_points() ) ); + CAPTURE( p ); + point_omt_abs new_conversion; + point_ms_omt remainder; + std::tie( new_conversion, remainder ) = project_remain( point_ms_abs( p ) ); + point old_conversion = ms_to_omt_remain( p ); + CHECK( old_conversion == new_conversion.raw() ); + CHECK( p == remainder.raw() ); + } + + SECTION( "omt_to_seg_tripoint" ) { + tripoint p = GENERATE( take( num_trials, random_tripoints() ) ); + CAPTURE( p ); + tripoint_seg_abs new_conversion = project_to( tripoint_omt_abs( p ) ); + tripoint old_conversion = omt_to_seg_copy( p ); + CHECK( old_conversion == new_conversion.raw() ); + } +} From 11d587ac7b1c0c8cd843d3ea1950087215830a9d Mon Sep 17 00:00:00 2001 From: John Bytheway Date: Mon, 1 Jul 2019 23:36:35 +0100 Subject: [PATCH 05/14] More coord_point features Add arithmetic and comparison operator overloads, constructors, hashing. --- src/coordinates.h | 101 ++++++++++++++++++++++++++-- tests/coordinate_test.cpp | 135 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 232 insertions(+), 4 deletions(-) diff --git a/src/coordinates.h b/src/coordinates.h index 2d216cc24c058..3d7ed695fe0c1 100644 --- a/src/coordinates.h +++ b/src/coordinates.h @@ -81,21 +81,99 @@ template class coord_point { public: - coord_point() = default; - explicit coord_point( const Point &p ) : + constexpr coord_point() = default; + explicit constexpr coord_point( const Point &p ) : raw_( p ) {} + template + constexpr coord_point( T x, T y ) : raw_( x, y ) {} + template + constexpr coord_point( T x, T y, T z ) : raw_( x, y, z ) {} - Point &raw() { + constexpr Point &raw() { return raw_; } - const Point &raw() const { + constexpr const Point &raw() const { return raw_; } + + constexpr auto x() const { + return raw_.x; + } + constexpr auto y() const { + return raw_.y; + } + constexpr auto z() const { + return raw_.z; + } + + coord_point &operator+=( const coord_point &r ) { + raw_ += r.raw(); + return *this; + } + + coord_point &operator-=( const coord_point &r ) { + raw_ -= r.raw(); + return *this; + } private: Point raw_; }; +template +constexpr inline bool operator==( const coord_point &l, + const coord_point &r ) +{ + return l.raw() == r.raw(); +} + +template +constexpr inline bool operator!=( const coord_point &l, + const coord_point &r ) +{ + return l.raw() != r.raw(); +} + +template +constexpr inline bool operator<( const coord_point &l, + const coord_point &r ) +{ + return l.raw() < r.raw(); +} + +template +constexpr inline coord_point operator+( + const coord_point &l, + const coord_point &r ) +{ + return coord_point( l.raw() + r.raw() ); +} + +template < typename Point, scale Scale, origin OriginR, + // enable_if to prevent ambiguity with above when both args are + // relative + typename = std::enable_if_t < OriginR != origin::relative >> +constexpr inline coord_point operator+( + const coord_point &l, + const coord_point &r ) +{ + return coord_point( l.raw() + r.raw() ); +} + +template +constexpr inline coord_point operator-( + const coord_point &l, + const coord_point &r ) +{ + return coord_point( l.raw() - r.raw() ); +} + +template +inline std::ostream &operator<<( std::ostream &os, const coord_point &p ) +{ + return os << p.raw(); +} + template struct project_to_impl; @@ -158,6 +236,20 @@ inline quotient_remainder project_remai } // namespace coords +namespace std +{ + +template +struct hash> { + std::size_t operator()( const coords::coord_point &p ) const { + const hash h; + return h( p.raw() ); + } +}; + +} // namespace std + +using point_ms_rel = coords::coord_point; using point_ms_abs = coords::coord_point; using point_ms_sm = coords::coord_point; using point_ms_omt = coords::coord_point; @@ -169,6 +261,7 @@ using point_omt_om = coords::coord_point; using point_om_abs = coords::coord_point; +using tripoint_ms_rel = coords::coord_point; using tripoint_ms_abs = coords::coord_point; using tripoint_ms_sm = coords::coord_point; using tripoint_ms_omt = coords::coord_point; diff --git a/tests/coordinate_test.cpp b/tests/coordinate_test.cpp index 74c8cf1545d37..dc63d1b83e786 100644 --- a/tests/coordinate_test.cpp +++ b/tests/coordinate_test.cpp @@ -7,8 +7,143 @@ constexpr int num_trials = 5; +TEST_CASE( "coordinate_operations", "[coords]" ) +{ + SECTION( "construct_from_raw_point" ) { + point p = GENERATE( take( num_trials, random_points() ) ); + point_ms_abs cp( p ); + CHECK( cp.x() == p.x ); + CHECK( cp.y() == p.y ); + } + + SECTION( "construct_from_raw_tripoint" ) { + tripoint p = GENERATE( take( num_trials, random_tripoints() ) ); + tripoint_ms_abs cp( p ); + CHECK( cp.x() == p.x ); + CHECK( cp.y() == p.y ); + CHECK( cp.z() == p.z ); + } + + SECTION( "construct_from_values" ) { + tripoint p = GENERATE( take( num_trials, random_tripoints() ) ); + { + point_ms_abs cp( p.x, p.y ); + CHECK( cp.x() == p.x ); + CHECK( cp.y() == p.y ); + } + { + tripoint_ms_abs cp( p.x, p.y, p.z ); + CHECK( cp.x() == p.x ); + CHECK( cp.y() == p.y ); + CHECK( cp.z() == p.z ); + } + } + + SECTION( "addition" ) { + point p0 = GENERATE( take( num_trials, random_points() ) ); + point p1 = GENERATE( take( num_trials, random_points() ) ); + CAPTURE( p0, p1 ); + point_ms_abs abs0( p0 ); + point_ms_rel rel0( p0 ); + point_ms_rel rel1( p1 ); + SECTION( "rel + rel -> rel" ) { + point_ms_rel sum = rel0 + rel1; + CHECK( sum.raw() == p0 + p1 ); + } + SECTION( "abs + rel -> abs" ) { + point_ms_abs sum = abs0 + rel1; + CHECK( sum.raw() == p0 + p1 ); + } + SECTION( "rel + abs -> abs" ) { + point_ms_abs sum = rel1 + abs0; + CHECK( sum.raw() == p0 + p1 ); + } + SECTION( "rel += rel" ) { + rel0 += rel1; + CHECK( rel0.raw() == p0 + p1 ); + } + SECTION( "abs += rel" ) { + abs0 += rel1; + CHECK( abs0.raw() == p0 + p1 ); + } + } + + SECTION( "subtraction" ) { + point p0 = GENERATE( take( num_trials, random_points() ) ); + point p1 = GENERATE( take( num_trials, random_points() ) ); + CAPTURE( p0, p1 ); + point_ms_abs abs0( p0 ); + point_ms_rel rel0( p0 ); + point_ms_rel rel1( p1 ); + SECTION( "rel - rel -> rel" ) { + point_ms_rel diff = rel0 - rel1; + CHECK( diff.raw() == p0 - p1 ); + } + SECTION( "abs - rel -> abs" ) { + point_ms_abs diff = abs0 - rel1; + CHECK( diff.raw() == p0 - p1 ); + } + SECTION( "rel -= rel" ) { + rel0 -= rel1; + CHECK( rel0.raw() == p0 - p1 ); + } + SECTION( "abs -= rel" ) { + abs0 -= rel1; + CHECK( abs0.raw() == p0 - p1 ); + } + } +} + +TEST_CASE( "coordinate_comparison", "[coords]" ) +{ + SECTION( "compare_points" ) { + point p0 = GENERATE( take( num_trials, random_points() ) ); + point p1 = GENERATE( take( num_trials, random_points() ) ); + CAPTURE( p0, p1 ); + point_ms_rel cp0( p0 ); + point_ms_rel cp1( p1 ); + CAPTURE( cp0, cp1 ); + + CHECK( ( p0 < p1 ) == ( cp0 < cp1 ) ); + CHECK( ( p0 == p1 ) == ( cp0 == cp1 ) ); + CHECK( cp0 == cp0 ); + CHECK( !( cp0 != cp0 ) ); + } + + SECTION( "compare_tripoints" ) { + tripoint p0 = GENERATE( take( num_trials, random_tripoints() ) ); + tripoint p1 = GENERATE( take( num_trials, random_tripoints() ) ); + CAPTURE( p0, p1 ); + tripoint_ms_rel cp0( p0 ); + tripoint_ms_rel cp1( p1 ); + CAPTURE( cp0, cp1 ); + + CHECK( ( p0 < p1 ) == ( cp0 < cp1 ) ); + CHECK( ( p0 == p1 ) == ( cp0 == cp1 ) ); + CHECK( cp0 == cp0 ); + CHECK( !( cp0 != cp0 ) ); + } +} + +TEST_CASE( "coordinate_hash", "[coords]" ) +{ + SECTION( "point_hash" ) { + point p = GENERATE( take( num_trials, random_points() ) ); + point_ms_abs cp( p ); + CHECK( std::hash()( cp ) == std::hash()( p ) ); + } + + SECTION( "tripoint_hash" ) { + tripoint p = GENERATE( take( num_trials, random_tripoints() ) ); + tripoint_ms_abs cp( p ); + CHECK( std::hash()( cp ) == std::hash()( p ) ); + } +} + TEST_CASE( "coordinate_conversion_consistency", "[coords]" ) { + // Verifies that the new coord_point-based conversions yield the same + // results as the legacy conversion functions. SECTION( "omt_to_om_point" ) { point p = GENERATE( take( num_trials, random_points() ) ); CAPTURE( p ); From 7edc6b9a49fd3112307317b1fe49c7cf9483e78a Mon Sep 17 00:00:00 2001 From: John Bytheway Date: Tue, 2 Jul 2019 10:08:47 +0100 Subject: [PATCH 06/14] Add constexpr_fatal macro for constexpr errors We need a complicated way to report constepxr errors in older gcc versions because gcc 5.3 didn't quite implement all of C++14 constexpr properly. Add a macro that performs the workaround solution on gcc < 6, but uses a neater solution (including an error message) on other compilers. --- src/coordinates.h | 6 ++---- src/debug.h | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/coordinates.h b/src/coordinates.h index 3d7ed695fe0c1..c9c0e1686e79d 100644 --- a/src/coordinates.h +++ b/src/coordinates.h @@ -45,8 +45,7 @@ constexpr int map_squares_per( scale s ) case scale::overmap: return OMAPX * map_squares_per( scale::overmap_terrain ); default: - debugmsg( "Requested scale of %d", s ); - abort(); + constexpr_fatal( 0, "Requested scale of %d", s ); } } @@ -68,8 +67,7 @@ constexpr origin origin_from_scale( scale s ) case scale::overmap: return origin::overmap; default: - debugmsg( "Requested origin for of %d", s ); - abort(); + constexpr_fatal( origin::abs, "Requested origin for scale %d", s ); } } diff --git a/src/debug.h b/src/debug.h index 4f8ab2ffc385a..f6d4834ed7199 100644 --- a/src/debug.h +++ b/src/debug.h @@ -80,6 +80,21 @@ inline void realDebugmsg( const char *const filename, const char *const line, std::forward( args )... ) ); } +// A fatal error for use in constexpr functions +// This exists for compatibility reasons. On gcc 5.3 we need a +// different implementation that is messier. +// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67371 +// Pass a placeholder return value to be used on gcc 5.3 (it won't +// actually be returned, it's just needed for the type), and then +// args as if to debugmsg for the remaining args. +#if defined(__GNUC__) && __GNUC__ < 6 +#define constexpr_fatal(ret, ...) \ + do { return false ? ( ret ) : ( abort(), ( ret ) ); } while(false) +#else +#define constexpr_fatal(ret, ...) \ + do { debugmsg(__VA_ARGS__); abort(); } while(false) +#endif + /** * Used to generate game report information. */ From 65bd43858f0c6b0a3186046243b833eab858d0c1 Mon Sep 17 00:00:00 2001 From: John Bytheway Date: Tue, 2 Jul 2019 22:20:37 +0100 Subject: [PATCH 07/14] Swap origin and scale everywhere After reading this code more, I've decided that the notation and mnemonics I'm adding are easier to understand if you put the origin before scale. e.g. point_sm_ms is a "submap map square", and point_rel_omt is a "relative overmap terrain". --- src/coordinates.h | 134 +++++++++++++++++++------------------- tests/coordinate_test.cpp | 90 ++++++++++++------------- 2 files changed, 112 insertions(+), 112 deletions(-) diff --git a/src/coordinates.h b/src/coordinates.h index c9c0e1686e79d..afd9ced1008bd 100644 --- a/src/coordinates.h +++ b/src/coordinates.h @@ -75,7 +75,7 @@ constexpr origin origin_from_scale( scale s ) // Point should be the underlying representation type (either point or // tripoint). // Scale and Origin define the coordinate system for the point. -template +template class coord_point { public: @@ -105,12 +105,12 @@ class coord_point return raw_.z; } - coord_point &operator+=( const coord_point &r ) { + coord_point &operator+=( const coord_point &r ) { raw_ += r.raw(); return *this; } - coord_point &operator-=( const coord_point &r ) { + coord_point &operator-=( const coord_point &r ) { raw_ -= r.raw(); return *this; } @@ -118,56 +118,56 @@ class coord_point Point raw_; }; -template -constexpr inline bool operator==( const coord_point &l, - const coord_point &r ) +template +constexpr inline bool operator==( const coord_point &l, + const coord_point &r ) { return l.raw() == r.raw(); } -template -constexpr inline bool operator!=( const coord_point &l, - const coord_point &r ) +template +constexpr inline bool operator!=( const coord_point &l, + const coord_point &r ) { return l.raw() != r.raw(); } -template -constexpr inline bool operator<( const coord_point &l, - const coord_point &r ) +template +constexpr inline bool operator<( const coord_point &l, + const coord_point &r ) { return l.raw() < r.raw(); } -template -constexpr inline coord_point operator+( - const coord_point &l, - const coord_point &r ) +template +constexpr inline coord_point operator+( + const coord_point &l, + const coord_point &r ) { - return coord_point( l.raw() + r.raw() ); + return coord_point( l.raw() + r.raw() ); } -template < typename Point, scale Scale, origin OriginR, +template < typename Point, origin OriginR, scale Scale, // enable_if to prevent ambiguity with above when both args are // relative typename = std::enable_if_t < OriginR != origin::relative >> -constexpr inline coord_point operator+( - const coord_point &l, - const coord_point &r ) +constexpr inline coord_point operator+( + const coord_point &l, + const coord_point &r ) { - return coord_point( l.raw() + r.raw() ); + return coord_point( l.raw() + r.raw() ); } -template -constexpr inline coord_point operator-( - const coord_point &l, - const coord_point &r ) +template +constexpr inline coord_point operator-( + const coord_point &l, + const coord_point &r ) { - return coord_point( l.raw() - r.raw() ); + return coord_point( l.raw() - r.raw() ); } -template -inline std::ostream &operator<<( std::ostream &os, const coord_point &p ) +template +inline std::ostream &operator<<( std::ostream &os, const coord_point &p ) { return os << p.raw(); } @@ -178,37 +178,37 @@ struct project_to_impl; template struct project_to_impl { template - coord_point operator()( - const coord_point &src ) { - return coord_point( multiply_xy( src.raw(), ScaleUp ) ); + coord_point operator()( + const coord_point &src ) { + return coord_point( multiply_xy( src.raw(), ScaleUp ) ); } }; template struct project_to_impl<0, ScaleDown, ResultScale> { template - coord_point operator()( - const coord_point &src ) { - return coord_point( + coord_point operator()( + const coord_point &src ) { + return coord_point( divide_xy_round_to_minus_infinity( src.raw(), ScaleDown ) ); } }; template -inline coord_point project_to( - const coord_point &src ) +inline coord_point project_to( + const coord_point &src ) { constexpr int scale_down = map_squares_per( ResultScale ) / map_squares_per( SourceScale ); constexpr int scale_up = map_squares_per( SourceScale ) / map_squares_per( ResultScale ); return project_to_impl()( src ); } -template +template struct quotient_remainder { constexpr static origin RemainderOrigin = origin_from_scale( CoarseScale ); - using quotient_type = coord_point; + using quotient_type = coord_point; quotient_type quotient; - using remainder_type = coord_point; + using remainder_type = coord_point; remainder_type remainder; // For assigning to std::tie( q, r ); @@ -218,15 +218,15 @@ struct quotient_remainder { }; template -inline quotient_remainder project_remain( - const coord_point &src ) +inline quotient_remainder project_remain( + const coord_point &src ) { constexpr int ScaleDown = map_squares_per( ResultScale ) / map_squares_per( SourceScale ); static_assert( ScaleDown > 0, "You can only project to coarser coordinate systems" ); constexpr static origin RemainderOrigin = origin_from_scale( ResultScale ); - coord_point quotient( + coord_point quotient( divide_xy_round_to_minus_infinity( src.raw(), ScaleDown ) ); - coord_point remainder( + coord_point remainder( src.raw() - quotient.raw() * ScaleDown ); return { quotient, remainder }; @@ -237,9 +237,9 @@ inline quotient_remainder project_remai namespace std { -template -struct hash> { - std::size_t operator()( const coords::coord_point &p ) const { +template +struct hash> { + std::size_t operator()( const coords::coord_point &p ) const { const hash h; return h( p.raw() ); } @@ -247,26 +247,26 @@ struct hash> { } // namespace std -using point_ms_rel = coords::coord_point; -using point_ms_abs = coords::coord_point; -using point_ms_sm = coords::coord_point; -using point_ms_omt = coords::coord_point; -using point_sm_abs = coords::coord_point; -using point_sm_omt = coords::coord_point; -using point_sm_om = coords::coord_point; -using point_omt_abs = coords::coord_point; -using point_omt_om = coords::coord_point; -using point_seg_abs = coords::coord_point; -using point_om_abs = coords::coord_point; - -using tripoint_ms_rel = coords::coord_point; -using tripoint_ms_abs = coords::coord_point; -using tripoint_ms_sm = coords::coord_point; -using tripoint_ms_omt = coords::coord_point; -using tripoint_sm_abs = coords::coord_point; -using tripoint_omt_abs = coords::coord_point; -using tripoint_seg_abs = coords::coord_point; -using tripoint_om_abs = coords::coord_point; +using point_rel_ms = coords::coord_point; +using point_abs_ms = coords::coord_point; +using point_sm_ms = coords::coord_point; +using point_omt_ms = coords::coord_point; +using point_abs_sm = coords::coord_point; +using point_omt_sm = coords::coord_point; +using point_om_sm = coords::coord_point; +using point_abs_omt = coords::coord_point; +using point_om_omt = coords::coord_point; +using point_abs_seg = coords::coord_point; +using point_abs_om = coords::coord_point; + +using tripoint_rel_ms = coords::coord_point; +using tripoint_abs_ms = coords::coord_point; +using tripoint_sm_ms = coords::coord_point; +using tripoint_omt_ms = coords::coord_point; +using tripoint_abs_sm = coords::coord_point; +using tripoint_abs_omt = coords::coord_point; +using tripoint_abs_seg = coords::coord_point; +using tripoint_abs_om = coords::coord_point; using coords::project_to; using coords::project_remain; diff --git a/tests/coordinate_test.cpp b/tests/coordinate_test.cpp index dc63d1b83e786..b2825cd130648 100644 --- a/tests/coordinate_test.cpp +++ b/tests/coordinate_test.cpp @@ -11,14 +11,14 @@ TEST_CASE( "coordinate_operations", "[coords]" ) { SECTION( "construct_from_raw_point" ) { point p = GENERATE( take( num_trials, random_points() ) ); - point_ms_abs cp( p ); + point_abs_ms cp( p ); CHECK( cp.x() == p.x ); CHECK( cp.y() == p.y ); } SECTION( "construct_from_raw_tripoint" ) { tripoint p = GENERATE( take( num_trials, random_tripoints() ) ); - tripoint_ms_abs cp( p ); + tripoint_abs_ms cp( p ); CHECK( cp.x() == p.x ); CHECK( cp.y() == p.y ); CHECK( cp.z() == p.z ); @@ -27,12 +27,12 @@ TEST_CASE( "coordinate_operations", "[coords]" ) SECTION( "construct_from_values" ) { tripoint p = GENERATE( take( num_trials, random_tripoints() ) ); { - point_ms_abs cp( p.x, p.y ); + point_abs_ms cp( p.x, p.y ); CHECK( cp.x() == p.x ); CHECK( cp.y() == p.y ); } { - tripoint_ms_abs cp( p.x, p.y, p.z ); + tripoint_abs_ms cp( p.x, p.y, p.z ); CHECK( cp.x() == p.x ); CHECK( cp.y() == p.y ); CHECK( cp.z() == p.z ); @@ -43,19 +43,19 @@ TEST_CASE( "coordinate_operations", "[coords]" ) point p0 = GENERATE( take( num_trials, random_points() ) ); point p1 = GENERATE( take( num_trials, random_points() ) ); CAPTURE( p0, p1 ); - point_ms_abs abs0( p0 ); - point_ms_rel rel0( p0 ); - point_ms_rel rel1( p1 ); + point_abs_ms abs0( p0 ); + point_rel_ms rel0( p0 ); + point_rel_ms rel1( p1 ); SECTION( "rel + rel -> rel" ) { - point_ms_rel sum = rel0 + rel1; + point_rel_ms sum = rel0 + rel1; CHECK( sum.raw() == p0 + p1 ); } SECTION( "abs + rel -> abs" ) { - point_ms_abs sum = abs0 + rel1; + point_abs_ms sum = abs0 + rel1; CHECK( sum.raw() == p0 + p1 ); } SECTION( "rel + abs -> abs" ) { - point_ms_abs sum = rel1 + abs0; + point_abs_ms sum = rel1 + abs0; CHECK( sum.raw() == p0 + p1 ); } SECTION( "rel += rel" ) { @@ -72,15 +72,15 @@ TEST_CASE( "coordinate_operations", "[coords]" ) point p0 = GENERATE( take( num_trials, random_points() ) ); point p1 = GENERATE( take( num_trials, random_points() ) ); CAPTURE( p0, p1 ); - point_ms_abs abs0( p0 ); - point_ms_rel rel0( p0 ); - point_ms_rel rel1( p1 ); + point_abs_ms abs0( p0 ); + point_rel_ms rel0( p0 ); + point_rel_ms rel1( p1 ); SECTION( "rel - rel -> rel" ) { - point_ms_rel diff = rel0 - rel1; + point_rel_ms diff = rel0 - rel1; CHECK( diff.raw() == p0 - p1 ); } SECTION( "abs - rel -> abs" ) { - point_ms_abs diff = abs0 - rel1; + point_abs_ms diff = abs0 - rel1; CHECK( diff.raw() == p0 - p1 ); } SECTION( "rel -= rel" ) { @@ -100,8 +100,8 @@ TEST_CASE( "coordinate_comparison", "[coords]" ) point p0 = GENERATE( take( num_trials, random_points() ) ); point p1 = GENERATE( take( num_trials, random_points() ) ); CAPTURE( p0, p1 ); - point_ms_rel cp0( p0 ); - point_ms_rel cp1( p1 ); + point_rel_ms cp0( p0 ); + point_rel_ms cp1( p1 ); CAPTURE( cp0, cp1 ); CHECK( ( p0 < p1 ) == ( cp0 < cp1 ) ); @@ -114,8 +114,8 @@ TEST_CASE( "coordinate_comparison", "[coords]" ) tripoint p0 = GENERATE( take( num_trials, random_tripoints() ) ); tripoint p1 = GENERATE( take( num_trials, random_tripoints() ) ); CAPTURE( p0, p1 ); - tripoint_ms_rel cp0( p0 ); - tripoint_ms_rel cp1( p1 ); + tripoint_rel_ms cp0( p0 ); + tripoint_rel_ms cp1( p1 ); CAPTURE( cp0, cp1 ); CHECK( ( p0 < p1 ) == ( cp0 < cp1 ) ); @@ -129,14 +129,14 @@ TEST_CASE( "coordinate_hash", "[coords]" ) { SECTION( "point_hash" ) { point p = GENERATE( take( num_trials, random_points() ) ); - point_ms_abs cp( p ); - CHECK( std::hash()( cp ) == std::hash()( p ) ); + point_abs_ms cp( p ); + CHECK( std::hash()( cp ) == std::hash()( p ) ); } SECTION( "tripoint_hash" ) { tripoint p = GENERATE( take( num_trials, random_tripoints() ) ); - tripoint_ms_abs cp( p ); - CHECK( std::hash()( cp ) == std::hash()( p ) ); + tripoint_abs_ms cp( p ); + CHECK( std::hash()( cp ) == std::hash()( p ) ); } } @@ -147,7 +147,7 @@ TEST_CASE( "coordinate_conversion_consistency", "[coords]" ) SECTION( "omt_to_om_point" ) { point p = GENERATE( take( num_trials, random_points() ) ); CAPTURE( p ); - point_om_abs new_conversion = project_to( point_omt_abs( p ) ); + point_abs_om new_conversion = project_to( point_abs_omt( p ) ); point old_conversion = omt_to_om_copy( p ); CHECK( old_conversion == new_conversion.raw() ); } @@ -155,7 +155,7 @@ TEST_CASE( "coordinate_conversion_consistency", "[coords]" ) SECTION( "omt_to_om_tripoint" ) { tripoint p = GENERATE( take( num_trials, random_tripoints() ) ); CAPTURE( p ); - tripoint_om_abs new_conversion = project_to( tripoint_omt_abs( p ) ); + tripoint_abs_om new_conversion = project_to( tripoint_abs_omt( p ) ); tripoint old_conversion = omt_to_om_copy( p ); CHECK( old_conversion == new_conversion.raw() ); } @@ -163,9 +163,9 @@ TEST_CASE( "coordinate_conversion_consistency", "[coords]" ) SECTION( "omt_to_om_remain_point" ) { point p = GENERATE( take( num_trials, random_points() ) ); CAPTURE( p ); - point_om_abs new_conversion; - point_omt_om remainder; - std::tie( new_conversion, remainder ) = project_remain( point_omt_abs( p ) ); + point_abs_om new_conversion; + point_om_omt remainder; + std::tie( new_conversion, remainder ) = project_remain( point_abs_omt( p ) ); point old_conversion = omt_to_om_remain( p ); CHECK( old_conversion == new_conversion.raw() ); CHECK( p == remainder.raw() ); @@ -174,7 +174,7 @@ TEST_CASE( "coordinate_conversion_consistency", "[coords]" ) SECTION( "sm_to_omt_point" ) { point p = GENERATE( take( num_trials, random_points() ) ); CAPTURE( p ); - point_omt_abs new_conversion = project_to( point_sm_abs( p ) ); + point_abs_omt new_conversion = project_to( point_abs_sm( p ) ); point old_conversion = sm_to_omt_copy( p ); CHECK( old_conversion == new_conversion.raw() ); } @@ -182,9 +182,9 @@ TEST_CASE( "coordinate_conversion_consistency", "[coords]" ) SECTION( "sm_to_omt_remain_point" ) { point p = GENERATE( take( num_trials, random_points() ) ); CAPTURE( p ); - point_omt_abs new_conversion; - point_sm_omt remainder; - std::tie( new_conversion, remainder ) = project_remain( point_sm_abs( p ) ); + point_abs_omt new_conversion; + point_omt_sm remainder; + std::tie( new_conversion, remainder ) = project_remain( point_abs_sm( p ) ); point old_conversion = sm_to_omt_remain( p ); CHECK( old_conversion == new_conversion.raw() ); CHECK( p == remainder.raw() ); @@ -193,7 +193,7 @@ TEST_CASE( "coordinate_conversion_consistency", "[coords]" ) SECTION( "sm_to_om_point" ) { point p = GENERATE( take( num_trials, random_points() ) ); CAPTURE( p ); - point_om_abs new_conversion = project_to( point_sm_abs( p ) ); + point_abs_om new_conversion = project_to( point_abs_sm( p ) ); point old_conversion = sm_to_om_copy( p ); CHECK( old_conversion == new_conversion.raw() ); } @@ -201,9 +201,9 @@ TEST_CASE( "coordinate_conversion_consistency", "[coords]" ) SECTION( "sm_to_om_remain_point" ) { point p = GENERATE( take( num_trials, random_points() ) ); CAPTURE( p ); - point_om_abs new_conversion; - point_sm_om remainder; - std::tie( new_conversion, remainder ) = project_remain( point_sm_abs( p ) ); + point_abs_om new_conversion; + point_om_sm remainder; + std::tie( new_conversion, remainder ) = project_remain( point_abs_sm( p ) ); point old_conversion = sm_to_om_remain( p ); CHECK( old_conversion == new_conversion.raw() ); CHECK( p == remainder.raw() ); @@ -212,7 +212,7 @@ TEST_CASE( "coordinate_conversion_consistency", "[coords]" ) SECTION( "omt_to_sm_point" ) { point p = GENERATE( take( num_trials, random_points() ) ); CAPTURE( p ); - point_sm_abs new_conversion = project_to( point_omt_abs( p ) ); + point_abs_sm new_conversion = project_to( point_abs_omt( p ) ); point old_conversion = omt_to_sm_copy( p ); CHECK( old_conversion == new_conversion.raw() ); } @@ -220,7 +220,7 @@ TEST_CASE( "coordinate_conversion_consistency", "[coords]" ) SECTION( "om_to_sm_point" ) { point p = GENERATE( take( num_trials, random_points() ) ); CAPTURE( p ); - point_sm_abs new_conversion = project_to( point_om_abs( p ) ); + point_abs_sm new_conversion = project_to( point_abs_om( p ) ); point old_conversion = om_to_sm_copy( p ); CHECK( old_conversion == new_conversion.raw() ); } @@ -228,7 +228,7 @@ TEST_CASE( "coordinate_conversion_consistency", "[coords]" ) SECTION( "ms_to_sm_point" ) { point p = GENERATE( take( num_trials, random_points() ) ); CAPTURE( p ); - point_sm_abs new_conversion = project_to( point_ms_abs( p ) ); + point_abs_sm new_conversion = project_to( point_abs_ms( p ) ); point old_conversion = ms_to_sm_copy( p ); CHECK( old_conversion == new_conversion.raw() ); } @@ -236,7 +236,7 @@ TEST_CASE( "coordinate_conversion_consistency", "[coords]" ) SECTION( "sm_to_ms_point" ) { point p = GENERATE( take( num_trials, random_points() ) ); CAPTURE( p ); - point_ms_abs new_conversion = project_to( point_sm_abs( p ) ); + point_abs_ms new_conversion = project_to( point_abs_sm( p ) ); point old_conversion = sm_to_ms_copy( p ); CHECK( old_conversion == new_conversion.raw() ); } @@ -244,7 +244,7 @@ TEST_CASE( "coordinate_conversion_consistency", "[coords]" ) SECTION( "ms_to_omt_point" ) { point p = GENERATE( take( num_trials, random_points() ) ); CAPTURE( p ); - point_omt_abs new_conversion = project_to( point_ms_abs( p ) ); + point_abs_omt new_conversion = project_to( point_abs_ms( p ) ); point old_conversion = ms_to_omt_copy( p ); CHECK( old_conversion == new_conversion.raw() ); } @@ -252,9 +252,9 @@ TEST_CASE( "coordinate_conversion_consistency", "[coords]" ) SECTION( "ms_to_omt_remain_point" ) { point p = GENERATE( take( num_trials, random_points() ) ); CAPTURE( p ); - point_omt_abs new_conversion; - point_ms_omt remainder; - std::tie( new_conversion, remainder ) = project_remain( point_ms_abs( p ) ); + point_abs_omt new_conversion; + point_omt_ms remainder; + std::tie( new_conversion, remainder ) = project_remain( point_abs_ms( p ) ); point old_conversion = ms_to_omt_remain( p ); CHECK( old_conversion == new_conversion.raw() ); CHECK( p == remainder.raw() ); @@ -263,7 +263,7 @@ TEST_CASE( "coordinate_conversion_consistency", "[coords]" ) SECTION( "omt_to_seg_tripoint" ) { tripoint p = GENERATE( take( num_trials, random_tripoints() ) ); CAPTURE( p ); - tripoint_seg_abs new_conversion = project_to( tripoint_omt_abs( p ) ); + tripoint_abs_seg new_conversion = project_to( tripoint_abs_omt( p ) ); tripoint old_conversion = omt_to_seg_copy( p ); CHECK( old_conversion == new_conversion.raw() ); } From c4d9521e5fa482dfac05cf5d034c5f8de20402f5 Mon Sep 17 00:00:00 2001 From: John Bytheway Date: Tue, 2 Jul 2019 22:24:47 +0100 Subject: [PATCH 08/14] Value-init hash object to suppress clang warning --- src/coordinates.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coordinates.h b/src/coordinates.h index afd9ced1008bd..1ba00863f78b0 100644 --- a/src/coordinates.h +++ b/src/coordinates.h @@ -240,7 +240,7 @@ namespace std template struct hash> { std::size_t operator()( const coords::coord_point &p ) const { - const hash h; + const hash h{}; return h( p.raw() ); } }; From 48c6d8219e2f0055cc72ff2dcbac3e60a3b63874 Mon Sep 17 00:00:00 2001 From: John Bytheway Date: Tue, 2 Jul 2019 22:29:34 +0100 Subject: [PATCH 09/14] Document the point name mnemonics --- src/coordinates.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/coordinates.h b/src/coordinates.h index 1ba00863f78b0..d8e965ad0e305 100644 --- a/src/coordinates.h +++ b/src/coordinates.h @@ -247,6 +247,17 @@ struct hash> { } // namespace std +/** Typedefs for point types with coordinate mnemonics. + * + * Each name is of the form (tri)point__ where tells you the + * context in which the point has meaning, and tells you what one unit + * of the point means. + * + * For example: + * point_omt_ms is the position of a map square within an overmap terrain. + * tripoint_rel_sm is a relative tripoint submap offset. + */ +/*@{*/ using point_rel_ms = coords::coord_point; using point_abs_ms = coords::coord_point; using point_sm_ms = coords::coord_point; @@ -267,6 +278,7 @@ using tripoint_abs_sm = coords::coord_point; using tripoint_abs_seg = coords::coord_point; using tripoint_abs_om = coords::coord_point; +/*@}*/ using coords::project_to; using coords::project_remain; From 7051f4d0412893a7d8d31cc61187e369ba3aaabc Mon Sep 17 00:00:00 2001 From: John Bytheway Date: Tue, 30 Jun 2020 21:37:45 -0400 Subject: [PATCH 10/14] Add more operators to coord_point Using these points in a larger proof of concept revealed the need for more operators. Add them. * Allow adding or subtracting raw points from coord_points (so that e.g. point_north, etc. can be used easily). * Allow more mixed-dimension arithmetic. * Allow subtracting two similarly-typed points to create a relative point. * Allow non-const access to individual coordinates. --- src/coordinates.h | 125 +++++++++++++++++++++++++++++++++----- src/point.h | 4 ++ tests/coordinate_test.cpp | 49 ++++++++++++++- 3 files changed, 161 insertions(+), 17 deletions(-) diff --git a/src/coordinates.h b/src/coordinates.h index d8e965ad0e305..59003e62c602c 100644 --- a/src/coordinates.h +++ b/src/coordinates.h @@ -79,6 +79,8 @@ template class coord_point { public: + static constexpr int dimension = Point::dimension; + constexpr coord_point() = default; explicit constexpr coord_point( const Point &p ) : raw_( p ) @@ -87,6 +89,10 @@ class coord_point constexpr coord_point( T x, T y ) : raw_( x, y ) {} template constexpr coord_point( T x, T y, T z ) : raw_( x, y, z ) {} + template + constexpr coord_point( const coord_point &xy, T z ) : + raw_( xy.raw(), z ) + {} constexpr Point &raw() { return raw_; @@ -95,16 +101,39 @@ class coord_point return raw_; } + constexpr auto &x() { + return raw_.x; + } constexpr auto x() const { return raw_.x; } + constexpr auto &y() { + return raw_.y; + } constexpr auto y() const { return raw_.y; } + constexpr auto xy() const { + return coord_point( raw_.xy() ); + } + constexpr auto &z() { + return raw_.z; + } constexpr auto z() const { return raw_.z; } + std::string to_string() const { + return raw_.to_string(); + } + + void serialize( JsonOut &jsout ) const { + raw().serialize( jsout ); + } + void deserialize( JsonIn &jsin ) { + raw().deserialize( jsin ); + } + coord_point &operator+=( const coord_point &r ) { raw_ += r.raw(); return *this; @@ -114,6 +143,42 @@ class coord_point raw_ -= r.raw(); return *this; } + + coord_point &operator+=( const point &r ) { + raw_ += r; + return *this; + } + + coord_point &operator-=( const point &r ) { + raw_ -= r; + return *this; + } + + coord_point &operator+=( const tripoint &r ) { + raw_ += r; + return *this; + } + + coord_point &operator-=( const tripoint &r ) { + raw_ -= r; + return *this; + } + + friend inline coord_point operator+( const coord_point &l, const point &r ) { + return coord_point( l.raw() + r ); + } + + friend inline coord_point operator+( const coord_point &l, const tripoint &r ) { + return coord_point( l.raw() + r ); + } + + friend inline coord_point operator-( const coord_point &l, const point &r ) { + return coord_point( l.raw() - r ); + } + + friend inline coord_point operator-( const coord_point &l, const tripoint &r ) { + return coord_point( l.raw() - r ); + } private: Point raw_; }; @@ -139,31 +204,61 @@ constexpr inline bool operator<( const coord_point &l, return l.raw() < r.raw(); } -template -constexpr inline coord_point operator+( - const coord_point &l, - const coord_point &r ) +template +constexpr inline auto operator+( + const coord_point &l, + const coord_point &r ) { - return coord_point( l.raw() + r.raw() ); + using PointResult = decltype( PointL() + PointR() ); + return coord_point( l.raw() + r.raw() ); } -template < typename Point, origin OriginR, scale Scale, +template < typename PointL, typename PointR, origin OriginR, scale Scale, // enable_if to prevent ambiguity with above when both args are // relative typename = std::enable_if_t < OriginR != origin::relative >> -constexpr inline coord_point operator+( - const coord_point &l, - const coord_point &r ) +constexpr inline auto operator+( + const coord_point &l, + const coord_point &r ) +{ + using PointResult = decltype( PointL() + PointR() ); + return coord_point( l.raw() + r.raw() ); +} + +template +constexpr inline auto operator-( + const coord_point &l, + const coord_point &r ) +{ + using PointResult = decltype( PointL() + PointR() ); + return coord_point( l.raw() - r.raw() ); +} + +template < typename PointL, typename PointR, origin Origin, scale Scale, + // enable_if to prevent ambiguity with above when both args are + // relative + typename = std::enable_if_t < Origin != origin::relative >> +constexpr inline auto operator-( + const coord_point &l, + const coord_point &r ) +{ + using PointResult = decltype( PointL() + PointR() ); + return coord_point( l.raw() - r.raw() ); +} + +// Only relative points can be multiplied by a constant +template +constexpr inline coord_point operator*( + int l, const coord_point &r ) { - return coord_point( l.raw() + r.raw() ); + return coord_point( l * r.raw() ); } -template -constexpr inline coord_point operator-( - const coord_point &l, - const coord_point &r ) +template +constexpr inline coord_point operator*( + const coord_point &r, int l ) { - return coord_point( l.raw() - r.raw() ); + return coord_point( r.raw() * l ); } template diff --git a/src/point.h b/src/point.h index 55bb2f68201d4..2c8fe42fc5728 100644 --- a/src/point.h +++ b/src/point.h @@ -33,6 +33,8 @@ class JsonOut; // NOLINTNEXTLINE(cata-xy) struct point { + static constexpr int dimension = 2; + int x = 0; int y = 0; constexpr point() = default; @@ -144,6 +146,8 @@ inline point divide_xy_round_to_minus_infinity( const point &p, int d ) // NOLINTNEXTLINE(cata-xy) struct tripoint { + static constexpr int dimension = 3; + int x = 0; int y = 0; int z = 0; diff --git a/tests/coordinate_test.cpp b/tests/coordinate_test.cpp index b2825cd130648..8c7e7a1859271 100644 --- a/tests/coordinate_test.cpp +++ b/tests/coordinate_test.cpp @@ -7,6 +7,22 @@ constexpr int num_trials = 5; +static_assert( point::dimension == 2, "" ); +static_assert( tripoint::dimension == 3, "" ); +static_assert( point_abs_omt::dimension == 2, "" ); +static_assert( tripoint_abs_omt::dimension == 3, "" ); + +TEST_CASE( "coordinate_strings", "[coords]" ) +{ + CHECK( point_abs_omt( 3, 4 ).to_string() == "(3,4)" ); + + SECTION( "coord_point_matches_point" ) { + point p = GENERATE( take( num_trials, random_points() ) ); + point_abs_ms cp( p ); + CHECK( p.to_string() == cp.to_string() ); + } +} + TEST_CASE( "coordinate_operations", "[coords]" ) { SECTION( "construct_from_raw_point" ) { @@ -40,9 +56,11 @@ TEST_CASE( "coordinate_operations", "[coords]" ) } SECTION( "addition" ) { - point p0 = GENERATE( take( num_trials, random_points() ) ); + tripoint t0 = GENERATE( take( num_trials, random_tripoints() ) ); + point p0 = t0.xy(); point p1 = GENERATE( take( num_trials, random_points() ) ); CAPTURE( p0, p1 ); + tripoint_abs_ms abst0( t0 ); point_abs_ms abs0( p0 ); point_rel_ms rel0( p0 ); point_rel_ms rel1( p1 ); @@ -53,6 +71,12 @@ TEST_CASE( "coordinate_operations", "[coords]" ) SECTION( "abs + rel -> abs" ) { point_abs_ms sum = abs0 + rel1; CHECK( sum.raw() == p0 + p1 ); + tripoint_abs_ms sum_t = abst0 + rel1; + CHECK( sum_t.raw() == t0 + p1 ); + } + SECTION( "abs + raw -> abs" ) { + point_abs_ms sum = abs0 + p1; + CHECK( sum.raw() == p0 + p1 ); } SECTION( "rel + abs -> abs" ) { point_abs_ms sum = rel1 + abs0; @@ -66,13 +90,20 @@ TEST_CASE( "coordinate_operations", "[coords]" ) abs0 += rel1; CHECK( abs0.raw() == p0 + p1 ); } + SECTION( "abs += raw" ) { + abs0 += p1; + CHECK( abs0.raw() == p0 + p1 ); + } } SECTION( "subtraction" ) { - point p0 = GENERATE( take( num_trials, random_points() ) ); + tripoint t0 = GENERATE( take( num_trials, random_tripoints() ) ); + point p0 = t0.xy(); point p1 = GENERATE( take( num_trials, random_points() ) ); CAPTURE( p0, p1 ); + tripoint_abs_ms abst0( t0 ); point_abs_ms abs0( p0 ); + point_abs_ms abs1( p1 ); point_rel_ms rel0( p0 ); point_rel_ms rel1( p1 ); SECTION( "rel - rel -> rel" ) { @@ -83,6 +114,16 @@ TEST_CASE( "coordinate_operations", "[coords]" ) point_abs_ms diff = abs0 - rel1; CHECK( diff.raw() == p0 - p1 ); } + SECTION( "abs - raw -> abs" ) { + point_abs_ms diff = abs0 - p1; + CHECK( diff.raw() == p0 - p1 ); + } + SECTION( "abs - abs -> rel" ) { + point_rel_ms diff0 = abs0 - abs1; + CHECK( diff0.raw() == p0 - p1 ); + tripoint_rel_ms diff1 = abst0 - abs1; + CHECK( diff1.raw() == t0 - p1 ); + } SECTION( "rel -= rel" ) { rel0 -= rel1; CHECK( rel0.raw() == p0 - p1 ); @@ -91,6 +132,10 @@ TEST_CASE( "coordinate_operations", "[coords]" ) abs0 -= rel1; CHECK( abs0.raw() == p0 - p1 ); } + SECTION( "abs -= raw" ) { + abs0 -= p1; + CHECK( abs0.raw() == p0 - p1 ); + } } } From 1050b99e925ebd1246a53b8f46085f5c862e5e9a Mon Sep 17 00:00:00 2001 From: John Bytheway Date: Wed, 1 Jul 2020 21:49:11 -0400 Subject: [PATCH 11/14] Fix include guards --- tests/cata_generators.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/cata_generators.h b/tests/cata_generators.h index 8be27ce8b9136..85dbc1c6a0e9f 100644 --- a/tests/cata_generators.h +++ b/tests/cata_generators.h @@ -1,6 +1,6 @@ #pragma once -#ifndef CATA_GENERATORS_H -#define CATA_GENERATORS_H +#ifndef CATA_TESTS_CATA_GENERATORS_H +#define CATA_TESTS_CATA_GENERATORS_H // Some Catch2 Generators for generating our data types @@ -15,4 +15,4 @@ Catch::Generators::GeneratorWrapper random_points( int low = -1000, int h Catch::Generators::GeneratorWrapper random_tripoints( int low = -1000, int high = 1000, int zlow = -OVERMAP_DEPTH, int zhigh = OVERMAP_HEIGHT ); -#endif // CATA_GENERATORS_H +#endif // CATA_TESTS_CATA_GENERATORS_H From 7496683b0edbc04256aa46485948df5952d3f8b1 Mon Sep 17 00:00:00 2001 From: John Bytheway Date: Wed, 1 Jul 2020 21:49:42 -0400 Subject: [PATCH 12/14] Appease clang-tidy --- tests/coordinate_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/coordinate_test.cpp b/tests/coordinate_test.cpp index 8c7e7a1859271..116269fcaf545 100644 --- a/tests/coordinate_test.cpp +++ b/tests/coordinate_test.cpp @@ -14,7 +14,7 @@ static_assert( tripoint_abs_omt::dimension == 3, "" ); TEST_CASE( "coordinate_strings", "[coords]" ) { - CHECK( point_abs_omt( 3, 4 ).to_string() == "(3,4)" ); + CHECK( point_abs_omt( point( 3, 4 ) ).to_string() == "(3,4)" ); SECTION( "coord_point_matches_point" ) { point p = GENERATE( take( num_trials, random_points() ) ); From 8cdd687cc513581a7fb83cd930059672c69a868e Mon Sep 17 00:00:00 2001 From: John Bytheway Date: Wed, 1 Jul 2020 22:13:12 -0400 Subject: [PATCH 13/14] Mark point generators final They call a virtual function from the constructor. Marking them final suppresses the clang warning about this. --- tests/cata_generators.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/cata_generators.cpp b/tests/cata_generators.cpp index 143359361aab6..ede6a7838c2b6 100644 --- a/tests/cata_generators.cpp +++ b/tests/cata_generators.cpp @@ -3,7 +3,7 @@ #include "point.h" #include "rng.h" -class RandomPointGenerator : +class RandomPointGenerator final : public Catch::Generators::IGenerator { public: @@ -27,7 +27,7 @@ class RandomPointGenerator : point current_point; }; -class RandomTripointGenerator : +class RandomTripointGenerator final : public Catch::Generators::IGenerator { public: From 9a0fdae34a2f758da57c26e822d0cb0b0aee4778 Mon Sep 17 00:00:00 2001 From: John Bytheway Date: Wed, 1 Jul 2020 22:14:36 -0400 Subject: [PATCH 14/14] Suppress cata-use-point-apis in some tests We are specifically testing the APIs that this check would refactor the code away from. --- tests/coordinate_test.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/coordinate_test.cpp b/tests/coordinate_test.cpp index 116269fcaf545..a8798b41095d8 100644 --- a/tests/coordinate_test.cpp +++ b/tests/coordinate_test.cpp @@ -43,11 +43,13 @@ TEST_CASE( "coordinate_operations", "[coords]" ) SECTION( "construct_from_values" ) { tripoint p = GENERATE( take( num_trials, random_tripoints() ) ); { + //NOLINTNEXTLINE(cata-use-point-apis) point_abs_ms cp( p.x, p.y ); CHECK( cp.x() == p.x ); CHECK( cp.y() == p.y ); } { + //NOLINTNEXTLINE(cata-use-point-apis) tripoint_abs_ms cp( p.x, p.y, p.z ); CHECK( cp.x() == p.x ); CHECK( cp.y() == p.y );