Skip to content

Genericize mixed-aperture naming (_ap34) ahead of adding aperture-7 sequences #55

Description

@gcol33

Background

Christian Carey emailed asking whether hexify could support a fictional
planet (used in tabletop RPGs) that needs mixed aperture sequences
including 4/7 and 7/4, not just the current 3/4 combinations. He also
asked, if that support were added, whether the existing _ap34-suffixed
functions should keep that name or become _ap347.

Current state

src/aperture_sequence.{h,cpp} implements mixed-sequence quantization
(hex_quantize_ap34, hex_center_ap34, hex_corners_ap34). It already
takes a generic ap_seq vector, but the internal dispatch
(is_0deg_orientation(), calc_cumulative_scale()) explicitly rejects
anything other than aperture 3 or 4 (throw std::runtime_error("... must be 3 or 4")), and the rotation-class model only knows about the two
"clean" classes used by 3/4 (0 deg / 30 deg, integer / sqrt(3) scale).

Aperture 7 uses a materially different rotation model: two additional
classes (Class III-A / III-B, ~19.1 deg / ~49.1 deg, the
kAp7RotDeg constant) with a sqrt(7) scale factor, not a multiple of 30
degrees. Folding aperture 7 into a mixed sequence means generalizing the
rotation-class state machine and corner generation, not just adding a
case to an if/else.

Proposal

Don't rename _ap34 to _ap347. That only defers the same problem to
whoever needs the next combination. Since the function signature already
takes an arbitrary ap_seq vector, genericize the name now
(hex_quantize_mixed / hex_center_mixed / hex_corners_mixed, or
similar) and let the internal aperture dispatch grow to whatever
apertures are actually supported, without another rename down the line.

Checklist

  • Generalize the rotation-class model in aperture_sequence.cpp to
    support aperture 7's non-30-degree classes (III-A/III-B) alongside
    the existing 0/30 degree classes
  • Extend calc_cumulative_scale() / substrate scale calculation for
    aperture-7 (sqrt(7) factor)
  • Rename hex_quantize_ap34/hex_center_ap34/hex_corners_ap34 to
    aperture-agnostic names, update R-level wrappers and call sites
  • Add mixed 4/7 and 7/4 sequence test coverage
  • Consider whether the R-level area-to-resolution helpers
    (EARTH_RADIUS_KM/EARTH_SURFACE_KM2 in R/constants.R) should
    also become parameterizable for non-Earth bodies, per Christian's
    original ask

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions