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.

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

A tile constructor should behave as expected #522

Open
richelbilderbeek opened this issue Mar 23, 2019 · 1 comment
Open

A tile constructor should behave as expected #522

richelbilderbeek opened this issue Mar 23, 2019 · 1 comment
Assignees
Labels
high priority High priority, do this first

Comments

@richelbilderbeek
Copy link
Owner

richelbilderbeek commented Mar 23, 2019

Is your feature request related to a problem? Please describe.

When someone creates a tile with a certain x coordinat, this is a test most people expect to pass:

    const double x = 1.0;
    const tile a(x);
    assert(x == a.get_x());

Currently, however, this test fails, as the tile is rescaled during construction. I see why the tiles need to be rescaled, it just needs to be done elsewhere.

Describe the solution you'd like

Make the tile constructor behave as expected. Do the rescaling someplace else.

These tests should pass:

  //#define FIX_ISSUE_522
  #ifdef FIX_ISSUE_522
  //A tile behaves as expected
  {
    const double x{1.0};
    const double y{2.0};
    const double z{3.0};
    const double width{4.0};
    const double height{5.0};
    const double depth{6.0};
    const tile_type type{tile_type::arctic};
    const tile_id id{tile_id()};

    const tile a(x, y, z, width, height, depth, type, id);
    assert(std::abs(x - a.get_x()) < 0.001);
    assert(std::abs(y - a.get_y()) < 0.001);
    assert(std::abs(z - a.get_z()) < 0.001);
    assert(std::abs(width - a.get_width()) < 0.001);
    assert(std::abs(height - a.get_height()) < 0.001);
    assert(std::abs(depth - a.get_depth()) < 0.001);
    assert(type == a.get_type());
    assert(id.get() == a.get_id());
  }
  #endif // FIX_ISSUE_522

Describe alternatives you've considered

None.

Additional context

#463 depends on this.

@richelbilderbeek richelbilderbeek added the high priority High priority, do this first label Mar 23, 2019
richelbilderbeek pushed a commit that referenced this issue Mar 23, 2019
@DynCoder DynCoder self-assigned this Apr 2, 2019
@DynCoder
Copy link
Collaborator

DynCoder commented Apr 2, 2019

Assign myself as this is mega super ultra high priority

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
high priority High priority, do this first
Projects
None yet
Development

No branches or pull requests

2 participants