Skip to content

fix(uniformbuilder): never request a device image for an award with no device - #245

Closed
SyniRon wants to merge 1 commit into
mainfrom
claude/triage-issue-244-c7b3bc
Closed

SyniRon wants to merge 1 commit into
mainfrom
claude/triage-issue-244-c7b3bc

Conversation

@SyniRon

@SyniRon SyniRon commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Closes #244

What changed

AwardRegistry.getMaxAwardCount now returns 0, not 1, for an award whose catalog entry names no attachment device. One line, plus a comment.

One new test case in awardNumerals.test.js: two rows of the Basic Assault Course Ribbon draw a plain ribbon. It runs through the existing GetCanvasObject seam with a stubbed fetch, next to the Meritorious Service Medal cluster case.

Why

Twelve catalog awards have no device. For those, the max count fell back to 1, so a second MILPAC row raised ribbonDisplayedAttachmentCount to 1 while ribbonAttachmentType stayed null. The canvas then requested attachments/null/1.png, got a 404, and logged "Error loading ribbon attachment" on every render. The ribbon still drew plain, so the uniform looked right.

With a max of 0, the existing increment guard never passes for a device-less award. The count stays 0 and the canvas requests nothing. The renderer, the valor path (oakClustersValor/0.png) and the catalog are untouched.

For the reviewer

  • The new test is red on main (1 !== 0) and green here. Reverting the one line reddens it again.
  • Every caller of getMaxAwardCount was checked. MedalWithValor overwrites the max to 14 after super(). All unit citations carry a device. The numeral awards clamp with Math.min against a table value, not the fallback.
  • The brief on Uniform builder requests a missing attachment image for some repeated ribbons #244 asked for 1, 2 and 3 rows across all twelve awards. The fix is keyed by device type, so no single mutation of the changed line tells those cases apart. One red-to-green case carries it. The control for a real device is already pinned by the Meritorious Service Medal test.
  • npm test exits 0. Prettier passes.

This was generated by AI

…o device

The registry's max device count fell back to 1 for the twelve catalog
awards that name no attachment type. A second MILPAC row of such an award
then raised the displayed count to 1 with the type still null, and the
canvas requested attachments/null/1.png. The request returned 404 and
logged "Error loading ribbon attachment" on every render. The ribbon still
drew plain, so the uniform looked right.

The fallback is now 0. The existing increment guard never passes for a
device-less award, so the count stays 0 and the canvas requests nothing.
The renderer and the valor path are unchanged.

Adds one red-to-green case through GetCanvasObject: two rows of the Basic
Assault Course Ribbon draw a plain ribbon.

Closes #244

> *This was generated by AI*
@SyniRon
SyniRon requested a review from Vercin-G as a code owner September 15, 2026 00:17
@SyniRon
SyniRon enabled auto-merge (squash) September 15, 2026 00:25
@SyniRon SyniRon closed this Sep 18, 2026
auto-merge was automatically disabled September 18, 2026 02:32

Pull request was closed

@SyniRon
SyniRon deleted the claude/triage-issue-244-c7b3bc branch September 18, 2026 02:32
@SyniRon

SyniRon commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator Author

This was generated by AI during triage.

This PR closed when its placeholder branch was renamed to fix/no-device-attachment-request. The same commit, 1f6dd2a, continues in #246 with main merged in.

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.

Uniform builder requests a missing attachment image for some repeated ribbons

2 participants