Skip to content

Cuter sprites - #5

Merged
dvd90 merged 2 commits into
mainfrom
feat/cuter-sprites
Jul 11, 2026
Merged

Cuter sprites#5
dvd90 merged 2 commits into
mainfrom
feat/cuter-sprites

Conversation

@dvd90

@dvd90 dvd90 commented Jul 11, 2026

Copy link
Copy Markdown
Owner

The v0.1 creature read as boxy — this redesigns it to actually be cute (the pet is the UI).

Rounded body, wide-set expressive eyes per mood, little antennae + feet on the kid, a footless chibi baby, a speckled egg. Drawn with single-width box-drawing glyphs, so the terminal width math (line.length === display width) is unchanged and the ≤12×6 invariant still holds.

  • src/ui/sprites/byte.ts redesigned
  • Snapshots regenerated (sprites, App, card)
  • Repo share card regenerated with the new art

Preview of all moods + growth stages: rendered in the PR discussion / shared separately.

🤖 Generated with Claude Code

Redesign the creature with single-width box-drawing glyphs (width math
unchanged): rounded head, wide-set expressive eyes per mood, little
antennae and feet on the kid, a footless chibi baby, a speckled egg.
Regenerate the repo's share card with the new art; snapshots updated.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@claude

claude Bot commented Jul 11, 2026

Copy link
Copy Markdown

Claude finished @dvd90's task in 2m 53s —— View job


Review: Cuter sprites (PR #5)

  • Gather context (diff, CLAUDE.md conventions)
  • Review src/ui/sprites/byte.ts changes (sprites-as-data, ≤12×6 invariant, display-width math)
  • Review snapshot updates (sprites, App, card)
  • Check TDD conventions (tests updated alongside sprite changes)
  • Post final review

Scope check: only src/ui/sprites/byte.ts, its snapshots, and the regenerated share-card SVG changed. No src/engine/** or src/collectors/** touched, so purity/never-throw rules aren't implicated here. Sprites stay as plain TS data objects (not inlined in components) — consistent with the "sprites are data" rule. No new deps, named exports only, commit is a proper feat:.

Main finding — the "display width == .length" claim needs verifying, not assuming

byte.ts:5-6 asserts the new glyphs are safe because "box-drawing and geometric glyphs are width 1." The box-drawing characters (╭╮┴│╰┬╵) are indeed universally narrow, but a few of the new eye/mouth glyphs live in Unicode ranges marked Ambiguous in EastAsianWidth.txt — notably (U+2022, General Punctuation) and parts of the Geometric Shapes / Miscellaneous Technical blocks that ◕ ◔ ⌣ are drawn from. "Ambiguous" characters render as 1 column in most Western-locale terminals but as 2 columns in many CJK-locale terminals and in apps configured with ambiguous-wide (a common tmux/iTerm2/Windows Terminal setting).

The existing invariant test (test/ui/sprites.test.ts:17-19) only checks frame.split('\n') line .length equality — that's UTF-16 code-unit count, not terminal display width, and every one of these glyphs is a single code unit, so the test passes regardless of how it actually renders. That means the exact scenario the PR body calls out ("terminal width math unchanged") isn't actually verified — it's asserted in a comment and happens to pass a test that can't distinguish it from the real risk. If any of these glyphs render 2-wide in a user's terminal, rows in the same frame padded with .length would visually misalign, breaking the ≤12×6 boxed look the redesign is going for — silently, and only for a subset of terminal configs, which makes it easy to miss in review/CI.

Suggested options (not necessarily blocking, given this is a low-severity visual-only risk and the ambiguous-width terminal configs are a minority case):

  • Swap , , , for glyphs confirmed Narrow/Neutral (many box-drawing/block-element alternates exist), or
  • Add a dev-time check using the same width logic Ink itself uses internally (string-width) against these specific glyphs, so a future glyph swap can't silently reintroduce this.

Fix this →

Everything else

  • frame() padding math checked by hand for kid (8×6), baby (6×5), egg (6×5) — all rows within a given frame are equal width and ≤12×6, matching the existing invariant test.
  • Egg's two frames (speckleRight true/false) are same width, so the shimmer animation won't jitter.
  • Snapshots (sprites, app, card) are consistent with the new glyphs and regenerated correctly — no stray diffs.
  • This is a data-only redesign already covered by the existing generalized invariant test (test/ui/sprites.test.ts:9), so no new tests were strictly required per TDD — the meta-test already exercises every (stage, mood) combo against the size/padding rules.
  • Note: I wasn't able to run npm test/typecheck/lint myself (no node_modules, and installing requires elevated approval not available here) — the hand-checked math above stands in for that, but please confirm CI is green before merge.

Overall: nice visual upgrade, keeps within the architectural rules. The one thing worth a second look before merging is the ambiguous-width glyph risk above — everything else looks good.

…book

- docs/demo.gif: rendered idle loop of Byte + HUD, embedded as the README hero
- README: expanded with install options, mood faces, life-stage table, a
  "How it works" data-flow section, and the share-card explainer
- docs/ARCHITECTURE.md: layer-by-layer tour + the v1 health formula
- CONTRIBUTING.md: setup, the non-negotiables, testing conventions
- docs/manual-tests.md: verified copy-paste scenarios for every mood and stage

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@claude

claude Bot commented Jul 11, 2026

Copy link
Copy Markdown

Claude finished @dvd90's task in 1m 18s —— View job


Review: Cuter sprites (PR #5) — update

  • Gather context (diff vs origin/main, CLAUDE.md conventions)
  • Review what changed in the new commit (4f3ae34)
  • Check TDD, engine purity, collectors-never-throw, product principles
  • Post final review

What's new since the last review: commit 4f3ae34 is docs-only — CONTRIBUTING.md, docs/ARCHITECTURE.md, docs/manual-tests.md, README rewrite, docs/demo.gif/demo.tape, and a regenerated docs/gitgotchi-card.svg (egg art). src/ui/sprites/byte.ts itself is byte-for-byte the same as it was for the first review — no code changed in this push.

Docs spot-check: the new architecture/README claims were cross-checked against the actual source and hold up — e.g. the life-stage table (README.md: baby @ 3 check-ins, kid @ 20 check-ins + coverage ≥ 50%, adult @ 100 + green streak ≥ 10, elder @ 365 days) matches src/engine/evolution.ts:19-22 exactly. ARCHITECTURE.md's data-flow/layer description matches the real module layout. No src/engine/** or src/collectors/** code was touched, so purity/never-throw rules aren't implicated by this push either — the new docs just describe those rules accurately, they don't loosen them.

Previous finding still open, unaddressed by this push: src/ui/sprites/byte.ts:8-24 uses , , , , which fall in Unicode's Ambiguous East Asian Width category — they render as 1 column in most Western-locale terminals but 2 columns under CJK locales or ambiguous-wide terminal settings (common in tmux/iTerm2/Windows Terminal). The invariant test (test/ui/sprites.test.ts:9-22) only checks .length (UTF-16 code units), which can't detect this, so the ≤12×6/equal-width guarantee the PR body claims isn't actually verified for the risky glyphs. This is low-severity and cosmetic-only (a minority of terminal configs), so not necessarily blocking, but worth a decision before merge rather than silently shipping:

  • swap the four glyphs for confirmed-Narrow alternatives, or
  • add a one-time dev check with string-width (already a transitive dep via Ink) asserting each glyph in EYES/MOUTH/BLINK is width 1.

Fix this →

Everything else in this push: the docs additions are well-scoped (contributing guide, architecture tour, manual-test playbook, README overhaul with a demo GIF) — none of it touches src/engine/** or src/collectors/**, so no new TDD/purity obligations are introduced. docs/gitgotchi-card.svg regeneration matches the new egg sprite. No new runtime deps added. Conventional commit messages used throughout.

Note: I still can't run npm test/typecheck/lint myself in this sandbox (no node_modules, no install permission) — please confirm CI is green before merge.

Overall: nice docs pass, no regressions introduced. The one open item is the ambiguous-width glyph risk from the original review, carried over unchanged.

@dvd90
dvd90 merged commit 60981c0 into main Jul 11, 2026
5 checks passed
@dvd90
dvd90 deleted the feat/cuter-sprites branch July 11, 2026 21:17
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