Skip to content

feat(ahbg): presentation board — fixed occupancy offsets and viewBox bounds - #20

Merged
erinepshovel-code merged 1 commit into
mainfrom
repair/ahbg-presentation-replacement
Sep 1, 2026
Merged

erinepshovel-code merged 1 commit into
mainfrom
repair/ahbg-presentation-replacement

Conversation

@erinepshovel-code

Copy link
Copy Markdown
Contributor

Replacement for closed #17 (presentation-only board)

Built from current main. Recovers only the presentation surface from #17 and
repairs the two remaining renderer defects; it carries no engine, War, turn,
RNG, permissions, or agent-harness behavior.

Repairs

  • initial vs final multi-unit occupancy offsets: unit markers now compute
    their start offset from the initial occupancy group of the motion origin
    and their end offset from the final group of the presented tile. A unit
    moving off a crowded tile is spread apart at the start; a unit arriving on a
    crowded tile is spread apart at the end.
  • SVG viewBox bounds: viewBox is derived from seed circles plus every
    displaced unit marker extent at both motion endpoints, so offset markers can
    no longer clip at the board edge.
  • Regression tests added for both defects (mirrored offset math + source
    structure assertions).

Boundary preserved

  • UCNS remains geometry authority: the renderer only scales supplied
    x/y/source_slot; geometry.py never reconstructs centers.
  • Snapshot standing stays not-mechanics; projector copies only declared
    browser-facing fields.

Gates

  • python -m unittest discover -s ahbg/presentation/tests -p 'test*.py' — 11 OK
  • node --check ahbg/presentation/board.js — OK
  • python -m compileall -q ahbg/presentation — OK
  • msdmd MODULE_BUILD collection over ahbg/presentation — 5 modules collected

…ts and viewBox

Recovered the presentation-only board from closed #17 (built from current
main). Repairs the two remaining renderer defects:

- initial vs final multi-unit occupancy offsets: a unit now uses the initial
  occupancy group of its motion origin for the start marker and the final
  group of its presented tile for the end marker, instead of applying the
  final-tile offset to both endpoints.
- SVG viewBox bounds: viewBox now derives from seed circles plus every
  displaced unit marker extent at both motion endpoints, so offset markers
  cannot clip at the board edge.

Keeps UCNS as geometry authority (presentation only scales supplied centers).
Regression tests mirror the frozen offset math and assert the corrected
source structure.
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-01T23:20:29.369483Z 1ede527 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@erinepshovel-code
erinepshovel-code merged commit 04253ab into main Sep 1, 2026
6 checks passed
@erinepshovel-code
erinepshovel-code deleted the repair/ahbg-presentation-replacement branch September 1, 2026 23:17

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1ede527993

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +318 to +321
marker.setAttribute("cx", origin.x); marker.setAttribute("cy", origin.y); marker.setAttribute("r", UNIT_RADIUS); marker.setAttribute("class", "unit");
svg.appendChild(marker);
const label = document.createElementNS("http://www.w3.org/2000/svg", "text");
label.setAttribute("x", origin.x); label.setAttribute("y", origin.y + 4); label.setAttribute("class", "unit-label"); label.textContent = unit.label || unit.id;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Base moved markers at their final positions

When SVG animation is unavailable or suppressed, such as in a static SVG export or a reduced-motion stylesheet, the marker and label retain these origin coordinates because only the SMIL <animate fill="freeze"> elements move them. The snapshot and inspection panel nevertheless report the unit at its destination, producing a visibly stale board; use the destination as the base attributes and animate from the origin.

Useful? React with 👍 / 👎.


// viewBox covers seed circles and every displaced unit marker at both its
// motion origin and its presented destination, so offset markers never clip.
const markerExtent = UNIT_RADIUS + 8;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Include unit labels in the viewBox bounds

For a unit with a long permitted label on an outer tile, the fixed 19-pixel extent covers the circle but not the rendered text, so the label extends outside the computed viewBox and is clipped. Because validation places no length bound on labels, the bounds should account for the label's rendered width or the UI should constrain labels to the reserved extent.

Useful? React with 👍 / 👎.

label.setAttribute("x", origin.x); label.setAttribute("y", origin.y + 4); label.setAttribute("class", "unit-label"); label.textContent = unit.label || unit.id;
svg.appendChild(label);

if (motion) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Reposition stationary units when occupancy changes

When another unit enters or leaves a stationary unit's tile, that stationary unit can have different origin and dest offsets, but this guard only adds animation for units that themselves have a motion. It therefore remains at its initial offset; for example, when a mover joins a lone stationary unit, the stationary marker stays centered while the mover ends only one spread radius away, leaving the markers overlapping despite the computed final group. Reposition every placement whose endpoints differ, regardless of whether that unit owns the motion event.

Useful? React with 👍 / 👎.

// === END MODULE_BUILD ===

const UCNS_COMMIT = "1975fe70cf4e0826a8020c2da3047569e277af64";
const SQRT3_HALF = Math.sqrt(3) / 2;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Consume UCNS coordinates instead of reconstructing them

When loading sample_snapshot.json fails and the embedded fallback is used, this browser-local Math.sqrt(3) / 2 calculation supplies the ring coordinates, so the presentation package becomes a separately maintained implementation of UCNS geometry rather than merely scaling source-produced coordinates. This contradicts the geometry boundary documented in ahbg/README.md:13-17 and can let the fallback diverge from the canonical source while still claiming its commit; embed source-generated coordinate data or a canonical generated artifact instead of recomputing it here.

AGENTS.md reference: AGENTS.md:L13-L14

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant