Address fourth round of Gadfly findings on #76
Build image / build-and-push (push) Successful in 6s
Build image / build-and-push (push) Successful in 6s
- Split hexCenters' doc: the count/limit contract had run straight on from the #75 anti-regression paragraph with no separator, so its opening "It" read as referring to the wrong thing. - Write the stagger as pitch/2 rather than radius. Same value, but the intent is "half a pitch" and only incidentally "one radius". - fitAxis's step<=0 guard is unreachable from its only caller. Kept, and now says so: a helper this small shouldn't need its caller read to be shown safe, and the failure mode without it is ±Inf into an int conversion. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01H3zbym8Doka2d7D48maSgZ
This commit is contained in:
+14
-5
@@ -214,10 +214,13 @@ type localPoint struct{ x, y float64 }
|
||||
// Do not "simplify" this back to anchoring at the region's min corner. That is
|
||||
// what #75 was: staggered rows start a full pitch in, and the leftover all lands
|
||||
// on the far edge, where clumps hang outside a bed that nothing clips them to.
|
||||
// It reports the total alongside the points, and works that total out BEFORE
|
||||
// building anything: a fill large enough to be refused shouldn't allocate its
|
||||
// whole lattice first just to be counted and thrown away. Over `limit` it
|
||||
// returns (nil, total) so the caller can refuse with the real number.
|
||||
//
|
||||
// # Counting before building
|
||||
//
|
||||
// hexCenters returns the total alongside the points, and works that total out
|
||||
// BEFORE building anything: a fill large enough to be refused shouldn't allocate
|
||||
// its whole lattice first just to be counted and thrown away. Over `limit` it
|
||||
// returns (nil, total), so the caller can still refuse with the real number.
|
||||
func hexCenters(r Region, radius, spacing float64, limit int) ([]localPoint, int) {
|
||||
if radius <= 0 {
|
||||
return nil, 0
|
||||
@@ -259,7 +262,7 @@ func hexCenters(r Region, radius, spacing float64, limit int) ([]localPoint, int
|
||||
// and centering THAT run puts it exactly half a pitch off its neighbours.
|
||||
// A single-column region has nothing to stagger against.
|
||||
if row%2 == 1 && cols > 1 {
|
||||
n, x = staggered, r.MinX+x0+radius
|
||||
n, x = staggered, r.MinX+x0+pitch/2
|
||||
}
|
||||
for i := 0; i < n; i++ {
|
||||
pts = append(pts, localPoint{x + float64(i)*pitch, y})
|
||||
@@ -276,6 +279,12 @@ func hexCenters(r Region, radius, spacing float64, limit int) ([]localPoint, int
|
||||
// A span too small to hold even one position at that inset still gets one, in the
|
||||
// middle: filling a bed narrower than a single plop with one plop is a better
|
||||
// answer than refusing to plant it.
|
||||
//
|
||||
// The step<=0 half of that guard is currently unreachable — hexCenters, the only
|
||||
// caller, returns early unless radius > 0, which makes both steps it passes
|
||||
// positive. It stays because dividing by a non-positive step yields ±Inf and then
|
||||
// a garbage int conversion, and a helper this small should not require reading
|
||||
// its caller to know it is safe. Deliberate, not an oversight.
|
||||
func fitAxis(length, step, inset float64) (n int, start float64) {
|
||||
if step <= 0 || length < 2*inset {
|
||||
return 1, length / 2
|
||||
|
||||
Reference in New Issue
Block a user