feat(uniformbuilder): generate award sprite sheets from upload folder in CI

[JensGryspeert]

What

Adds a CI pipeline so contributors can add a new medal/ribbon to the Uniform Builder without hand-splicing the binary sprite sheets in GIMP. The only human image work left is supplying the artwork and editing the registry.

The workflow:

1. Contributor edits AwardRegistry.jsx for placement, drops source PNG(s) into uniform-uploads/, adds a manifest.json entry, and pushes.
2. award_sprites CI validates the manifest against the registry, normalizes each source to its tile geometry, splices the tile into the ribbon/medal sheet(s) at the registry-derived row, removes the consumed sources, empties the manifest, and opens a PR with just the regenerated sheets + cleanup.

How

• generateAwardSprites.js — parses AwardRegistry.jsx as text (never executed), validates, normalizes with sharp, and splices.
  • awardType-gated sheet membership: ribbon sheet = {Medal, MedalTiered, MedalWithValor, Ribbon, RibbonDonationLogic} with awardPriority; medal sheet = {Medal, MedalTiered, MedalWithValor} with medalPriority ≥ 2. Tabs/weapon quals/unit citations/badges (which live in other assets) are rejected.
  • Splice via raw-byte concatenation so every non-inserted row stays byte-identical and each sheet's sub-tile trailing pixel survives. Multiple inserts interleave at their final registry rows; appending past the sheet end pads to the exact row.
  • Validates before any write: not-in-registry, neither-sheet, duplicate, and missing-source entries fail the run by name. Both sheets are computed before either is written (atomic).
• .github/workflows/award_sprites.yml — triggers on push to uniform-uploads/** (bot branch ignored, concurrency-guarded), runs npm run format:check as a self-gate, then opens the PR via peter-evans/create-pull-request into the pushed branch.
• uniform-uploads/ — tracked drop folder with manifest.json + a README documenting the flow.
• Adds sharp as a root devDependency and a sprites:generate script.

Verification

• generateAwardSprites.test.js — 34 assertions over synthetic sheets (parsing, membership gating, validation errors, interleaved multi-insert, padded append, full run idempotency).
• Verified against copies of the real sheets: a Foxhole-style insert grows ribbon 769→783 and medal 5281→5401, preserves RGBA, leaves rows below byte-identical, cleans the uploads, and empties the manifest.
• npm run format:check passes.

Notes

• Does not edit AwardRegistry.jsx or the .xcf GIMP sources (additions only; the human owns the registry).
• PRs opened with the default GITHUB_TOKEN don't trigger other workflows, which is why the workflow runs format:check itself before opening the PR.

[SyniRon]

Really nice work. I pulled this into a worktree and checked the parsing and splice math against the real registry and both actual sheets, and the geometry holds up: ribbon rows at awardPriority * 14, medal at (medalPriority - 2) * 120, both sheets RGBA at the sizes the code expects. The validate-then-normalize-then-write ordering is the right shape for something destructive running unattended, and the byte-concat splice genuinely keeps untouched rows identical. I verified the multi-insert interleaving and the padded-append case too. Good tests on the pure functions.

A few things I'd want sorted before this runs on real contributor pushes. Two are worth calling out up front because they're not visible in the diff:

1. The test suite never runs in CI. There's no test script in package.json and no workflow step runs generateAwardSprites.test.js. All 424 lines of it are protecting nothing right now. One line in package.json plus a step fixes it, and everything else here gets safer once it's in place.

2. parseAward can lose a field silently and that turns into data loss. If a registry entry is found but one field doesn't parse, it returns a partial object instead of failing. A medal whose medalPriority didn't parse just doesn't get placed, but its source PNG still gets deleted and the manifest still gets emptied, so the run reports success while the art is gone. Left a detailed inline note on this one.

The rest are inline: the atomicity comment promises more than the code delivers, the regex breaks on nested braces, the spritesheet-geometry.md reference points at a file that isn't in the repo, and a couple of smaller things.

One design question rather than a bug: this is insert-only. If someone wants to fix the art on an award that already has a tile, adding it to the manifest splices a duplicate and shifts everything below it. The README's "renumber when adding" instruction covers the new-award path, but nothing stops or warns about the fix-existing-art case. Worth either a guard or a loud note in the README.

[JensGryspeert]

Thanks for the thorough review — all of it is addressed in dd72a92.

The two up-front items:

1. Tests now run in CI. Added a test script (node generateAwardSprites.test.js) and wired it into both workflows: award_sprites runs it before generating (self-gate), and lint_format runs it on every PR to main so generator regressions are caught even when the change isn't under uniform-uploads/. Grew the suite from 34 to 41 assertions (brace-balanced parse, present-but-unparsed hard error, placed-vs-ignored source deletion).
2. Silent partial-parse data loss is closed. A registry field that's present but doesn't parse is now a distinct hard validation error that aborts before any normalize/write/delete, and source deletion only happens once a tile is actually placed — so the "medal vanishes with its art, run exits 0" path is gone from both directions.

The rest are answered inline.

[JensGryspeert]

Follow-up on the insert-only design question (b19b567): the generator now supports in-place replace, not just docs.

Set "replace": true on a manifest entry and CI overwrites the tile already at that award's registry row — no shift, no height change — so re-uploading art for an existing award fixes it instead of splicing a duplicate. New awards still default to insert. Replacing a row past the end of the sheet (no tile there yet) fails the run, so you can't accidentally "replace" something that should be an insert.

Covered by new tests (in-place overwrite with rows above/below untouched and height unchanged, plus the past-the-end rejection) and documented in uniform-uploads/README.md.

[SyniRon]

Came back through this in a fresh worktree at b19b567. First: you actually did the work, and it shows. Every one of the ten items from last round is fixed in the code, not just in the replies. The brace-balanced parse, the present-but-unparsed hard error before any write, the temp-then-rename ordering, the consumed.add move into the placement blocks, the workflow change-gate and the warnings in the PR body. I checked each against the source and the suite runs green (48 assertions). The partial-parse data-loss path we were worried about is genuinely closed.

The problem is the feature you added on top of those fixes. The replace: true path reopens the same class of silent data loss we just spent a round closing. When a manifest mixes a replace with a lower-priority insert, the insert grows the buffer before the replace lands, so the replace overwrites the wrong award while the intended one stays broken, and both sources still get deleted. The run exits 0. I left the mechanics and a reproduction inline on buildSplicedSheet, along with the doc comment that currently claims this case is safe.

That one blocks. Either reject mixing a replace and an insert on the same sheet, or apply replaces against the original offsets before any insert grows the buffer; it needs a mixed-batch test either way.

The rest is smaller and inline: the two-rename window isn't atomic and leaves a stale sheet plus an orphan tmp file if the second throws. One leftover from last round too, the awardType "covered by new tests" note doesn't match the suite. None of those block; the mixed replace does.

[JensGryspeert]

Thanks for catching the regression the replace feature introduced — all three of this round's items are addressed in 6943c81.

The blocker (mixed replace + insert): rejected, not reconciled. A replace and an insert can no longer run against the same sheet in one batch — guarded in buildSplicedSheet (per-sheet, fires before any byte is touched) and pre-flighted in validateManifest. The line 262 comment now describes the rejection instead of claiming the old false safety. Tested at both levels with a mixed batch, asserting the sheet stays byte-identical.

Non-blocking, also done:

• The two-rename window now has a try/finally that unlinks any orphan *.tmp.png, plus a comment that a partial-rename crash (EXDEV) leaves a half-applied state needing manual reconciliation — the renames still can't be made atomic across two destinations, so that's documented rather than papered over.
• The awardType "covered by new tests" gap is closed: a Typeless Medal fixture now reaches the distinct "no parseable awardType" branch and asserts it doesn't collapse into "does not belong".

Suite is at 53 assertions, Prettier-clean. Replies left inline on each thread.

[SyniRon]

This looks good overall. I found one last correctness issue before approval.

The replace guard on line 301 rejects the bottom-most ribbon award. The real ribbon sheet is 769px tall (54 * 14 + 13), so its last row is only 13px high.

The European/African/Middle Eastern Campaign Medal at awardPriority: 54 starts at y = 756. The current check does 756 + 14 = 770 > 769 and throws "no existing tile to overwrite", even though the tile is there in the remaining 13px. Re-uploading that award's art therefore fails. I reproduced it with a synthetic 43x769 sheet.

Checking the start of the row should handle it:

if (ins.y >= height) {

rawTile.copy already clamps to the available height, so it will copy 13px into that last row. A replace starting at or below height will still be rejected.

Please add a test with a partial final row too. The current test only covers a full last row. The medal sheet does not hit this case right now, only ribbon.

Once that's fixed, I'll approve.