feat(v0.9.2): flatten icon hierarchy + simplify resolver + audit script#94
Conversation
Per the v0.9.2 scoping plan: collapse the per-game icon directory structure into a single flat hierarchy (one drawing per item id, shared across every game that has the item) and replace the per-game manifest probing with a single top-level manifest. - git mv public/icons/acgcn/<category>/* → public/icons/<category>/* (118 files preserved through git history); delete public/icons/acgcn/. - Update generate-icon-manifest.ts to scan the flat layout and emit public/icons/manifest.json keyed by category → id → ext. - Simplify <ItemIcon> resolution: (category, id) → URL, with RENAME_OVERRIDES applied before manifest lookup (citrus-long-horned-beetle, sabretooth-*, pachycephalosaurus-* and similar cross-game spelling drifts). - Special-case fossils to a generic placeholder when no per-id art exists, sidestepping the ACNH per-piece vs. older-games per-skeleton schema drift. - Drop useGameHasIcons; replace with per-item useHasIcon and useIconChecker hooks. Consumers (CollectibleRow, ItemExpandPanel, HomeTab, GlobalSearchDropdown) now gate the categorical Glyph fallback per item. - Add scripts/audit-icon-coverage.ts + npm run audit:icons. Output committed to docs/v0.9.2-icon-coverage-audit.md (first run: ACGCN 100%, ACWW 54%, ACCF 68%, ACNL 32%, ACNH 23% — drives v0.9.4-v0.9.7 gap-fill scoping). - Tests cover resolver URL construction, override application, manifest cache (one fetch across consumers), and the fossil placeholder fallback. First hand-drawn icons (fish/sea-bass, fish/koi) ride along under the new flat layout.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR implements the v0.9.2 icon architecture shift: icons are no longer stored per-game, but instead as a single flat hierarchy keyed only by (category, id), with a top-level manifest and a simplified resolver that applies cross-game rename aliases.
Changes:
- Flattened icon storage to
public/icons/<category>/<id>.<ext>and introduced a singlepublic/icons/manifest.jsonwith{ category: { id: ext } }. - Reworked the client-side manifest cache + resolver to be global (single fetch per session) and added rename canonicalization + new hooks (
useHasIcon,useIconChecker). - Added an audit script and npm command to report per-game catalog coverage against the flat manifest.
Reviewed changes
Copilot reviewed 15 out of 135 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/components/search/GlobalSearchDropdown.tsx | Switches search dropdown icon gating to per-item checks via useIconChecker(). |
| src/components/itemIconUtils.ts | Introduces flat-manifest cache + resolver (resolveIconUrl), rename overrides, and new icon-checking hooks. |
| src/components/ItemIcon.tsx | Updates <ItemIcon> to use the flat manifest and resolver; keeps gameId as an optional no-op prop. |
| src/components/ItemIcon.test.tsx | Updates tests for flat manifest shape, resolver behavior, rename overrides, and cache semantics. |
| src/components/ItemExpandPanel.tsx | Changes expand-panel icon rendering to gate on useHasIcon(category, id). |
| src/components/HomeTab.tsx | Updates recent/shelf rendering to use useIconChecker() for per-item icon gating. |
| src/components/CollectibleRow.tsx | Updates row rendering to gate icon vs. glyph per item using useHasIcon(category, id). |
| scripts/README.md | Updates documentation for flat manifest generation and adds audit script usage. |
| scripts/generate-icon-manifest.ts | Rewrites manifest generator to emit a single top-level manifest from flat icon directories. |
| scripts/audit-icon-coverage.ts | Adds coverage audit script that compares per-game catalogs against the flat manifest (with rename overrides). |
| public/icons/manifest.json | Adds the new top-level icon manifest storing per-id extensions. |
| public/icons/fossils/trilobite.png | Adds/moves fossil icon asset into flat hierarchy. |
| public/icons/fossils/dinosaur-track.png | Adds/moves fossil icon asset into flat hierarchy. |
| public/icons/fossils/dinosaur-egg.png | Adds/moves fossil icon asset into flat hierarchy. |
| public/icons/fossils/ammonite.png | Adds/moves fossil icon asset into flat hierarchy. |
| public/icons/fossils/amber.png | Adds/moves fossil icon asset into flat hierarchy. |
| public/icons/fish/pale-chub.png | Adds/moves fish icon asset into flat hierarchy. |
| public/icons/fish/large-bass.png | Adds/moves fish icon asset into flat hierarchy. |
| public/icons/fish/killifish.png | Adds/moves fish icon asset into flat hierarchy. |
| public/icons/fish/guppy.jpg | Adds/moves fish icon asset into flat hierarchy. |
| public/icons/fish/giant-catfish.png | Adds/moves fish icon asset into flat hierarchy. |
| public/icons/bugs/tiger-swallowtail-butterfly.jpg | Adds/moves bug icon asset into flat hierarchy. |
| public/icons/bugs/snail.jpg | Adds/moves bug icon asset into flat hierarchy. |
| public/icons/bugs/seven-spotted-ladybug.png | Adds/moves bug icon asset into flat hierarchy. |
| public/icons/bugs/pill-bug.jpg | Adds/moves bug icon asset into flat hierarchy. |
| public/icons/bugs/mountain-stag-beetle.png | Adds/moves bug icon asset into flat hierarchy. |
| public/icons/bugs/mosquito.jpg | Adds/moves bug icon asset into flat hierarchy. |
| public/icons/bugs/mole-cricket.jpg | Adds/moves bug icon asset into flat hierarchy. |
| public/icons/bugs/mantis.jpg | Adds/moves bug icon asset into flat hierarchy. |
| public/icons/bugs/common-butterfly.jpg | Adds/moves bug icon asset into flat hierarchy. |
| public/icons/bugs/cockroach.png | Adds/moves bug icon asset into flat hierarchy. |
| public/icons/bugs/bagworm.jpg | Adds/moves bug icon asset into flat hierarchy. |
| public/icons/acgcn/manifest.json | Removes the old per-game manifest (ACGCN) after flattening. |
| package.json | Adds audit:icons npm script for the new audit tool. |
| docs/v0.9.2-icon-routing-plan.md | Adds the v0.9.2 scoping/design plan documenting the new flat icon model. |
Comments suppressed due to low confidence (1)
src/components/ItemExpandPanel.tsx:60
ItemIconalready has a built-in placeholder (monogram) when an entry is missing or the image fails to load. InItemExpandPanel, gating the entire icon block onuseHasIcon(category, item.id)means uncovered items render no icon at all (no glyph, no placeholder), which is a behavioral regression from rendering<ItemIcon>and letting it fall back. Consider rendering<ItemIcon>unconditionally (or adding an explicit fallback element) and usinguseHasIcononly when you truly want to hide the icon region entirely.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * Returns a stable predicate `(category, id) => boolean` keyed off the current | ||
| * manifest state. Use inside list renders where calling `useHasIcon` per row | ||
| * would violate the rules of hooks. | ||
| */ | ||
| export function useIconChecker(): ( | ||
| category: CategoryId, | ||
| id: string | ||
| ) => boolean { | ||
| const state = useManifestState(); | ||
| if (state.status !== 'present') return () => false; | ||
| const manifest = state.manifest; | ||
| return (category, id) => resolveIconUrl(manifest, category, id) !== null; | ||
| } |
There was a problem hiding this comment.
Tracked in #95 as a small follow-up. The current returned function is logically stable but reallocates per render; wrapping in useMemo keyed on the manifest fixes the referential stability. Out of scope for this architectural PR.
| function listGameDirs(): string[] { | ||
| if (!existsSync(DATA_ROOT)) return []; | ||
| return readdirSync(DATA_ROOT).filter(name => { | ||
| try { | ||
| return statSync(join(DATA_ROOT, name)).isDirectory(); | ||
| } catch { | ||
| return false; | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
Fixed in ba1f547. Added .sort() to match the pattern already used in scripts/audit-icon-coverage.ts. Manifest regenerated; no entry differences (order was incidentally already alphabetical).
| // id → ext. Note: parse('foo.png').ext = '.png'; strip the leading dot. | ||
| const extById = new Map<string, string>(); | ||
| for (const filename of files) { | ||
| const { name, ext } = parse(filename); | ||
| extById.set(name, ext.replace(/^\./, '')); | ||
| } |
There was a problem hiding this comment.
Real, but pre-existing across the icon set from the v0.9.1 wiki scrape — not introduced by this PR. Browsers sniff content type from magic bytes so rendering works today. Filed as #96 for a dedicated cleanup pass.
| for (const cat of CATEGORIES) { | ||
| const ids = readCatalogIds(gameId, cat); | ||
| for (const id of ids) { | ||
| total++; | ||
| const canonicalId = canonicalize(id); | ||
| if (manifest[cat]?.[canonicalId]) { | ||
| covered++; | ||
| } else { | ||
| uncovered.push({ category: cat, id, canonicalId }); | ||
| } |
There was a problem hiding this comment.
Audit numbers are accurate today since no fossils.placeholder entry exists in the manifest yet. Will sync the audit's fossil-handling logic to the resolver's placeholder fallback in the same sub-PR that introduces the placeholder asset (planned for v0.9.2 PR (b) or wherever the generic fossil drawing lands). Tracked in the v0.9.2 plan doc.
Summary
Architectural shift per the v0.9.2 scoping plan (
docs/v0.9.2-icon-routing-plan.md): one drawing per item id, shared across every game that has the item. Collapses the v0.9.1 per-game icon directories into a single flat hierarchy and rebuilds the resolver around a single top-level manifest.What changed
Layout:
public/icons/<gameId>/<category>/<id>.<ext>→public/icons/<category>/<id>.<ext>. 118 ACGCN icons moved viagit mv(history preserved); the now-emptypublic/icons/acgcn/is removed.Manifest: single
public/icons/manifest.jsonshaped{ category: { id: ext } }. Filenames are the invariant<id>.<ext>, so the manifest stores only the per-id extension.Resolver:
(category, id) → URL.RENAME_OVERRIDES(cross-game spelling drift likecitrus-long-horned-beetle↔citrus-longhorn-beetle,sabertooth-*↔sabretooth-*,pachycephalosaur-*↔pachycephalosaurus-*,peking-man↔peking-man-skull) is applied before manifest lookup.Fossils: special-cased to a generic
fossils/placeholder.<ext>fallback when no per-id art exists. Sidesteps the ACNH per-piece vs. older-games per-skeleton schema drift; the placeholder file lands later (v1.0 roadmap).Hooks:
useGameHasIcons(gameId)removed. Replaced withuseHasIcon(category, id)anduseIconChecker()(a stable predicate for use inside list renders without violating the rules of hooks). Consumers gate the categoricalGlyphfallback per item rather than per game.Audit script:
scripts/audit-icon-coverage.tsreads per-game catalogs, applies overrides, and writesdocs/v0.9.2-icon-coverage-audit.md. First run summary:These per-game uncovered lists drive scoping for v0.9.4-v0.9.7 gap-fill releases. Note: ACNL/ACNH coverage will lift sharply once
RENAME_OVERRIDESsettles in — the current numbers reflect the conservative initial alias set.Decisions
Glyphfallback instead of leaning on<ItemIcon>'s monogram placeholder. The categorical tint is meaningfully better UX than plain initials. AddinguseIconChecker()(vs. onlyuseHasIcon) was the cleanest way to gate it inside list renders.gameIdas a no-op prop on<ItemIcon>instead of churning every callsite. The resolver ignores it; existing callers compile unchanged.git mvand removals — the audit script lands cleanly alongside.What's not in this PR
CHANGELOG, version bump to
0.9.2-beta, andversion-history.htmlupdates land separately as PR (b) — release prep — to keep this PR focused on the architectural shift.Test plan
npm run lintcleannpm run buildclean (286 KB / 89 KB gzipped — no regression)npm test— 79 tests pass, including 16 covering the new resolver/cache surface