Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide point types with coordinate system type-safety #32017

Merged
merged 14 commits into from
Jul 7, 2020
Merged

Provide point types with coordinate system type-safety #32017

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
273482e
Add Catch2 StringMaker definition for point
jbytheway Jun 28, 2019
c749e6b
Add Catch2 generators for point, tripoint
jbytheway Jun 28, 2019
cd62e73
Implement initial type-safe points and conversions
jbytheway Jun 30, 2019
f35eb51
Tests for the coord_point conversion functions
jbytheway Jun 30, 2019
11d587a
More coord_point features
jbytheway Jul 1, 2019
7edc6b9
Add constexpr_fatal macro for constexpr errors
jbytheway Jul 2, 2019
65bd438
Swap origin and scale everywhere
jbytheway Jul 2, 2019
c4d9521
Value-init hash object to suppress clang warning
jbytheway Jul 2, 2019
48c6d82
Document the point name mnemonics
jbytheway Jul 2, 2019
7051f4d
Add more operators to coord_point
jbytheway Jul 1, 2020
1050b99
Fix include guards
jbytheway Jul 2, 2020
7496683
Appease clang-tidy
jbytheway Jul 2, 2020
8cdd687
Mark point generators final
jbytheway Jul 2, 2020
9a0fdae
Suppress cata-use-point-apis in some tests
jbytheway Jul 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
369 changes: 369 additions & 0 deletions src/coordinates.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,375 @@
#include "enums.h"
#include "game_constants.h"
#include "point.h"
#include "debug.h"

namespace coords
{

enum class scale {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a tag type (not enumeration value) is better. It allows easier adding of new systems. All properties (like scaling/absolute vs relative) can be done withing that tag type.

E.g.

class sub_map_system;

class map_system {
// this would allow converting map_system to sub_map_system, but not to say vehicle coordinates
// instead of a debug message this would yield a compiler error (as there is no factor_to( vehicle_system) function)
    static int factor_to( const sub_map_system & ) {
        return 12;
    }
};

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the code is simpler with two independent features rather than a tag type that controls all aspects. Could use types rather than an enum. I chose the enum because more meta-programming logic can be written in "regular C++" constexpr functions, rather than type-specialization style, which I was hoping would be clearer and easier to follow.

I'll try to keep both approaches in mind, though, if this version gets unwieldy.

map_square,
submap,
overmap_terrain,
segment,
overmap,
vehicle
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vehicle scale should possibly be split into two scales: vehicle_mount (the unrotated nominal displacement from the 0,0,0 position) and vehicle_actual (the current projection of a vehicle_mount along a tileray). Perhaps those should be special origins.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I haven't delved too deeply into vehicle stuff yet. My thoughts were that the mount coords would be origin vehicle, scale vehicle, while the projected-into-world coordinates would be origin vehicle, scale map_square. But I'm not too tied to that idea yet.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not too particular about how they're laid out, I just hate:
void coord_translate( int dir, const point &pivot, const point &p, point &q ) because it's not immediately clear that p and q are actually supposed to be separate types and whether pivot is the same or a different type.

If that becomes
void coord_translate( int dir, const veh_veh_point &pivot, const veh_veh_point &p, veh_ms_point &q ) then I'm satisfied, aside from veh_veh_point looking kind of silly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scale veh should be scale mnt, so:
void coord_translate( int dir, const veh_mnt_point &pivot, const veh_mnt_point &p, veh_ms_point &q )
which doesn't look too bad.

};

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:
constexpr_fatal( 0, "Requested scale of %d", s );
}
}

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:
constexpr_fatal( origin::abs, "Requested origin for scale %d", s );
}
}

// 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<typename Point, origin Origin, scale Scale>
class coord_point
{
public:
static constexpr int dimension = Point::dimension;

constexpr coord_point() = default;
explicit constexpr coord_point( const Point &p ) :
raw_( p )
{}
template<typename T>
constexpr coord_point( T x, T y ) : raw_( x, y ) {}
template<typename T>
constexpr coord_point( T x, T y, T z ) : raw_( x, y, z ) {}
template<typename T>
constexpr coord_point( const coord_point<point, Origin, Scale> &xy, T z ) :
raw_( xy.raw(), z )
{}

constexpr Point &raw() {
return raw_;
}
constexpr const Point &raw() const {
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<point, Origin, Scale>( 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<Point, origin::relative, Scale> &r ) {
raw_ += r.raw();
return *this;
}

coord_point &operator-=( const coord_point<Point, origin::relative, Scale> &r ) {
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_;
};

template<typename Point, origin Origin, scale Scale>
constexpr inline bool operator==( const coord_point<Point, Origin, Scale> &l,
const coord_point<Point, Origin, Scale> &r )
{
return l.raw() == r.raw();
}

template<typename Point, origin Origin, scale Scale>
constexpr inline bool operator!=( const coord_point<Point, Origin, Scale> &l,
const coord_point<Point, Origin, Scale> &r )
{
return l.raw() != r.raw();
}

template<typename Point, origin Origin, scale Scale>
constexpr inline bool operator<( const coord_point<Point, Origin, Scale> &l,
const coord_point<Point, Origin, Scale> &r )
{
return l.raw() < r.raw();
}

template<typename PointL, typename PointR, origin OriginL, scale Scale>
constexpr inline auto operator+(
const coord_point<PointL, OriginL, Scale> &l,
const coord_point<PointR, origin::relative, Scale> &r )
{
using PointResult = decltype( PointL() + PointR() );
return coord_point<PointResult, OriginL, Scale>( l.raw() + r.raw() );
}

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 auto operator+(
const coord_point<PointL, origin::relative, Scale> &l,
const coord_point<PointR, OriginR, Scale> &r )
{
using PointResult = decltype( PointL() + PointR() );
return coord_point<PointResult, OriginR, Scale>( l.raw() + r.raw() );
}

template<typename PointL, typename PointR, origin OriginL, scale Scale>
constexpr inline auto operator-(
const coord_point<PointL, OriginL, Scale> &l,
const coord_point<PointR, origin::relative, Scale> &r )
{
using PointResult = decltype( PointL() + PointR() );
return coord_point<PointResult, OriginL, Scale>( 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<PointL, Origin, Scale> &l,
const coord_point<PointR, Origin, Scale> &r )
{
using PointResult = decltype( PointL() + PointR() );
return coord_point<PointResult, origin::relative, Scale>( l.raw() - r.raw() );
}

// Only relative points can be multiplied by a constant
template<typename Point, scale Scale>
constexpr inline coord_point<Point, origin::relative, Scale> operator*(
int l, const coord_point<Point, origin::relative, Scale> &r )
{
return coord_point<Point, origin::relative, Scale>( l * r.raw() );
}

template<typename Point, scale Scale>
constexpr inline coord_point<Point, origin::relative, Scale> operator*(
const coord_point<Point, origin::relative, Scale> &r, int l )
{
return coord_point<Point, origin::relative, Scale>( r.raw() * l );
}

template<typename Point, origin Origin, scale Scale>
inline std::ostream &operator<<( std::ostream &os, const coord_point<Point, Origin, Scale> &p )
{
return os << p.raw();
}

template<int ScaleUp, int ScaleDown, scale ResultScale>
struct project_to_impl;

template<int ScaleUp, scale ResultScale>
struct project_to_impl<ScaleUp, 0, ResultScale> {
template<typename Point, origin Origin, scale SourceScale>
coord_point<Point, Origin, ResultScale> operator()(
const coord_point<Point, Origin, SourceScale> &src ) {
return coord_point<Point, Origin, ResultScale>( multiply_xy( src.raw(), ScaleUp ) );
}
};

template<int ScaleDown, scale ResultScale>
struct project_to_impl<0, ScaleDown, ResultScale> {
template<typename Point, origin Origin, scale SourceScale>
coord_point<Point, Origin, ResultScale> operator()(
const coord_point<Point, Origin, SourceScale> &src ) {
return coord_point<Point, Origin, ResultScale>(
divide_xy_round_to_minus_infinity( src.raw(), ScaleDown ) );
}
};

template<scale ResultScale, typename Point, origin Origin, scale SourceScale>
inline coord_point<Point, Origin, ResultScale> project_to(
const coord_point<Point, Origin, SourceScale> &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<scale_up, scale_down, ResultScale>()( src );
}

template<typename Point, origin Origin, scale CoarseScale, scale FineScale>
struct quotient_remainder {
constexpr static origin RemainderOrigin = origin_from_scale( CoarseScale );
using quotient_type = coord_point<Point, Origin, CoarseScale>;
quotient_type quotient;
using remainder_type = coord_point<Point, RemainderOrigin, FineScale>;
remainder_type remainder;

// For assigning to std::tie( q, r );
operator std::tuple<quotient_type &, remainder_type &>() {
return std::tie( quotient, remainder );
}
};

template<scale ResultScale, typename Point, origin Origin, scale SourceScale>
inline quotient_remainder<Point, Origin, ResultScale, SourceScale> project_remain(
const coord_point<Point, Origin, SourceScale> &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<Point, Origin, ResultScale> quotient(
divide_xy_round_to_minus_infinity( src.raw(), ScaleDown ) );
coord_point<Point, RemainderOrigin, SourceScale> remainder(
src.raw() - quotient.raw() * ScaleDown );

return { quotient, remainder };
}

} // namespace coords

namespace std
{

template<typename Point, coords::origin Origin, coords::scale Scale>
struct hash<coords::coord_point<Point, Origin, Scale>> {
std::size_t operator()( const coords::coord_point<Point, Origin, Scale> &p ) const {
const hash<Point> h{};
return h( p.raw() );
}
};

} // namespace std

/** Typedefs for point types with coordinate mnemonics.
*
* Each name is of the form (tri)point_<origin>_<scale> where <origin> tells you the
* context in which the point has meaning, and <scale> 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<point, coords::origin::relative, coords::ms>;
using point_abs_ms = coords::coord_point<point, coords::origin::abs, coords::ms>;
using point_sm_ms = coords::coord_point<point, coords::origin::submap, coords::ms>;
using point_omt_ms = coords::coord_point<point, coords::origin::overmap_terrain, coords::ms>;
using point_abs_sm = coords::coord_point<point, coords::origin::abs, coords::sm>;
using point_omt_sm = coords::coord_point<point, coords::origin::overmap_terrain, coords::sm>;
using point_om_sm = coords::coord_point<point, coords::origin::overmap, coords::sm>;
using point_abs_omt = coords::coord_point<point, coords::origin::abs, coords::omt>;
using point_om_omt = coords::coord_point<point, coords::origin::overmap, coords::omt>;
using point_abs_seg = coords::coord_point<point, coords::origin::abs, coords::seg>;
using point_abs_om = coords::coord_point<point, coords::origin::abs, coords::om>;

using tripoint_rel_ms = coords::coord_point<tripoint, coords::origin::relative, coords::ms>;
using tripoint_abs_ms = coords::coord_point<tripoint, coords::origin::abs, coords::ms>;
using tripoint_sm_ms = coords::coord_point<tripoint, coords::origin::submap, coords::ms>;
using tripoint_omt_ms = coords::coord_point<tripoint, coords::origin::overmap_terrain, coords::ms>;
using tripoint_abs_sm = coords::coord_point<tripoint, coords::origin::abs, coords::sm>;
using tripoint_abs_omt = coords::coord_point<tripoint, coords::origin::abs, coords::omt>;
using tripoint_abs_seg = coords::coord_point<tripoint, coords::origin::abs, coords::seg>;
using tripoint_abs_om = coords::coord_point<tripoint, coords::origin::abs, coords::om>;
/*@}*/

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
Expand Down
15 changes: 15 additions & 0 deletions src/debug.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,21 @@ inline void realDebugmsg( const char *const filename, const char *const line,
std::forward<Args>( 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.
*/
Expand Down
Loading