Skip to content

Commit c38878c

Browse files
author
DavidQ
committed
Fix Asset Manager V2 asset ID generation for type, role, and color assets - PR_26139_018-asset-manager-id-builder-fixes
1 parent 897302e commit c38878c

6 files changed

Lines changed: 129 additions & 56 deletions

File tree

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
# PR_26139_018 Asset Manager ID Builder Fixes Report
2+
3+
## Summary
4+
- Updated Asset Manager V2 color ID generation to use `assets.color.<role>.<usage-or-game>` and never append the color name.
5+
- Allowed color assets to validate and save without Usage; blank Usage now generates the `game` segment.
6+
- Preserved `assets.color.background.game` background color behavior.
7+
- Kept selected file state through Type changes so Asset ID regenerates immediately; incompatible file/type combinations now remain visibly blocked by validation.
8+
9+
## Files Changed
10+
- `tools/asset-manager-v2/js/assetManagerMetadata.js`
11+
- `tools/asset-manager-v2/js/controls/AssetFormControl.js`
12+
- `tools/asset-manager-v2/js/services/AssetSchemaValidator.js`
13+
- `tests/playwright/tools/AssetManagerV2.spec.mjs`
14+
- `tests/playwright/tools/WorkspaceManagerV2.spec.mjs`
15+
16+
## Targeted Validation Covered
17+
- Type change updates generated Asset ID.
18+
- Role change updates generated Asset ID.
19+
- Color save succeeds without Usage.
20+
- Color ID excludes color name.
21+
- Background color remains `assets.color.background.game`.
22+
- Workspace-launched Asset Manager uses the same role-based color ID contract.
23+
24+
## Validation Commands
25+
- PASS: `node --check tools/asset-manager-v2/js/assetManagerMetadata.js`
26+
- PASS: `node --check tools/asset-manager-v2/js/controls/AssetFormControl.js`
27+
- PASS: `node --check tools/asset-manager-v2/js/services/AssetSchemaValidator.js`
28+
- PASS: `node --check tests/playwright/tools/AssetManagerV2.spec.mjs`
29+
- PASS: `node --check tests/playwright/tools/WorkspaceManagerV2.spec.mjs`
30+
- PASS: `npx playwright test tests/playwright/tools/AssetManagerV2.spec.mjs --project=playwright --workers=1 --reporter=list --grep "launches Asset Manager V2 with temporary UAT context and schema-complete asset controls|launches Asset Manager V2 from Workspace Manager V2 with schema-valid context and workspace return nav"`
31+
- PASS: `npx playwright test tests/playwright/tools/WorkspaceManagerV2.spec.mjs --project=playwright --workers=1 --reporter=list --grep "uses header lifecycle controls and launches tools from fixed Workspace Manager V2 tiles"`
32+
- PASS: `npm run build:manifest`
33+
- PASS: `git diff --check`
34+
35+
## Notes
36+
- The first targeted Asset Manager Playwright run found stale test expectations for source path preservation and current Workspace Manager tile count. The assertions were corrected and the targeted validation was rerun successfully.
37+
- Full samples smoke test was not run because this PR is limited to Asset Manager V2 ID builder and validation behavior.
38+
- Playwright V8 coverage artifacts were refreshed by the targeted Playwright runs; coverage remains advisory.

tests/playwright/tools/AssetManagerV2.spec.mjs

Lines changed: 39 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -569,6 +569,24 @@ test.describe("Asset Manager V2", () => {
569569
await expect(page.locator("#assetStretchOverrideInput")).toHaveValue("0");
570570
await expect(page.locator("#statusLog")).toHaveValue(/OK Selected file nebula-background\.png validated as type image, kind png, role background\./);
571571
await expect(page.locator("#addAssetButton")).toBeEnabled();
572+
await page.locator("#assetRoleSelect").selectOption("preview");
573+
await expect(page.locator("#assetIdInput")).toHaveValue("assets.image.preview.nebula-background");
574+
await expect(page.locator("#assetStretchOverrideField")).toBeHidden();
575+
await page.locator("#assetRoleSelect").selectOption("background");
576+
await expect(page.locator("#assetIdInput")).toHaveValue("assets.image.background.nebula-background");
577+
await expect(page.locator("#assetStretchOverrideField")).toBeVisible();
578+
await page.locator("#assetKindAudio").check();
579+
await expect(page.locator("#assetRoleSelect")).toHaveValue("sound");
580+
await expect(page.locator("#assetIdInput")).toHaveValue("assets.audio.sound.nebula-background");
581+
await expect(page.locator("#assetPathInput")).toHaveValue("assets/images/nebula-background.png");
582+
await expect(page.locator("#statusLog")).toHaveValue(/FAIL Selected file validation failed: File nebula-background\.png is not accepted for Audio assets\./);
583+
await expect(page.locator("#addAssetButton")).toBeDisabled();
584+
await page.locator("#assetKindImage").check();
585+
await expect(page.locator("#assetRoleSelect")).toHaveValue("background");
586+
await expect(page.locator("#assetIdInput")).toHaveValue("assets.image.background.nebula-background");
587+
await expect(page.locator("#assetPathInput")).toHaveValue("assets/images/nebula-background.png");
588+
await expect(page.locator("#statusLog")).toHaveValue(/OK Selected file nebula-background\.png validated as type image, kind png, role background\./);
589+
await expect(page.locator("#addAssetButton")).toBeEnabled();
572590
await page.locator("#addAssetButton").click();
573591
await expect(page.locator("#statusLog")).toHaveValue(/OK Added assets\.image\.background\.nebula-background\./);
574592
await expect(page.locator("#assetList")).toContainText("assets.image.background.nebula-background");
@@ -1230,29 +1248,28 @@ test.describe("Asset Manager V2", () => {
12301248
isSelected: true,
12311249
swatchBorderColor: "rgb(255, 255, 255)"
12321250
});
1233-
await expect(page.locator("#assetIdInput")).toHaveValue("");
1251+
await expect(page.locator("#assetIdInput")).toHaveValue("assets.color.hud.game");
12341252
await expect(page.locator("#assetPathInput")).toHaveValue("palette://workspace/signal-violet");
1235-
await expect(page.locator("#statusLog")).toHaveValue(/FAIL Selected color validation failed: Color usage is required for color assets\./);
1236-
await expect(page.locator("#addAssetButton")).toBeDisabled();
1253+
await expect(page.locator("#statusLog")).toHaveValue(/OK Selected color Signal Violet! validated as type color, kind hex, role hud\./);
1254+
await expect(page.locator("#addAssetButton")).toBeEnabled();
12371255
await page.locator("#assetRoleSelect").selectOption("background");
1238-
await page.locator("#assetUsageInput").fill("game");
12391256
await expect(page.locator("#assetIdInput")).toHaveValue("assets.color.background.game");
12401257
await expect(page.locator("#assetUsageField")).toHaveCount(1);
12411258
await page.locator("#assetRoleSelect").selectOption("hud");
12421259
await page.locator("#assetUsageInput").fill("Menu Highlight");
1243-
await expect(page.locator("#assetIdInput")).toHaveValue("assets.color.hud.menu-highlight.signal-violet");
1260+
await expect(page.locator("#assetIdInput")).toHaveValue("assets.color.hud.menu-highlight");
12441261
await expect(page.locator("#assetPathInput")).toHaveValue("palette://workspace/signal-violet");
12451262
await expect(page.locator("#statusLog")).toHaveValue(/OK Selected color Signal Violet! validated as type color, kind hex, role hud\./);
12461263
await page.locator("#addAssetButton").click();
1247-
await expect(page.locator("#assetList")).toContainText("assets.color.hud.menu-highlight.signal-violet");
1264+
await expect(page.locator("#assetList")).toContainText("assets.color.hud.menu-highlight");
12481265
await expect(page.locator("#assetList")).toContainText("assets.font.ui.vector-battle");
12491266
await expect(page.locator("#assetList")).toContainText("assets.image.preview.uat-preview");
12501267
await expect(page.locator("#assetList")).toContainText("assets.video.cutscene.8-mile");
12511268
await expect(page.locator("#selectedAssetDetails")).not.toContainText("Final ID");
1252-
await expect(page.locator("#selectedAssetDetails")).toContainText("assets.color.hud.menu-highlight.signal-violet");
1269+
await expect(page.locator("#selectedAssetDetails")).toContainText("assets.color.hud.menu-highlight");
12531270
const output = JSON.parse(await page.locator("#inspectorOutput").textContent());
12541271
expect(output.assets[0]).toEqual({
1255-
id: "assets.color.hud.menu-highlight.signal-violet",
1272+
id: "assets.color.hud.menu-highlight",
12561273
type: "color",
12571274
kind: "hex",
12581275
role: "hud",
@@ -1394,7 +1411,7 @@ test.describe("Asset Manager V2", () => {
13941411
});
13951412

13961413
try {
1397-
await expect(page.locator("#workspaceToolTiles [data-workspace-tool-id]")).toHaveCount(7);
1414+
await expect(page.locator("#workspaceToolTiles [data-workspace-tool-id]")).toHaveCount(8);
13981415
await expect(page.locator('[data-workspace-tool-id="workspace-manager-v2"]')).toHaveCount(0);
13991416
await selectFakeWorkspaceRepo(page);
14001417
await page.locator("#activeGameSelect").selectOption("Asteroids");
@@ -1503,22 +1520,25 @@ test.describe("Asset Manager V2", () => {
15031520
expect(paletteTitles.some((title) => title.includes("name: HUD Blue"))).toBe(true);
15041521
expect(paletteTitles.some((title) => title.includes("name: Vector White"))).toBe(true);
15051522
await page.locator('#assetColorSwatchList button[title*="HUD Blue"]').click();
1506-
await expect(page.locator("#assetIdInput")).toHaveValue("");
1523+
await expect(page.locator("#assetIdInput")).toHaveValue("assets.color.hud.game");
15071524
await expect(page.locator("#assetPathInput")).toHaveValue("palette://workspace/hud-blue");
1508-
await expect(page.locator("#statusLog")).toHaveValue(/FAIL Selected color validation failed: Color usage is required for color assets\./);
1509-
await expect(page.locator("#addAssetButton")).toBeDisabled();
1525+
await expect(page.locator("#statusLog")).toHaveValue(/OK Selected color HUD Blue validated as type color, kind hex, role hud\./);
1526+
await expect(page.locator("#addAssetButton")).toBeEnabled();
1527+
await page.locator("#assetRoleSelect").selectOption("background");
1528+
await expect(page.locator("#assetIdInput")).toHaveValue("assets.color.background.game");
1529+
await page.locator("#assetRoleSelect").selectOption("hud");
15101530
await page.locator("#assetUsageInput").fill("Primary HUD");
1511-
await expect(page.locator("#assetIdInput")).toHaveValue("assets.color.hud.primary-hud.hud-blue");
1531+
await expect(page.locator("#assetIdInput")).toHaveValue("assets.color.hud.primary-hud");
15121532
await expect(page.locator("#assetPathInput")).toHaveValue("palette://workspace/hud-blue");
15131533
await expect(page.locator("#statusLog")).toHaveValue(/OK Selected color HUD Blue validated as type color, kind hex, role hud\./);
15141534
await page.locator("#assetRoleSelect").selectOption("accent");
1515-
await expect(page.locator("#assetIdInput")).toHaveValue("assets.color.accent.primary-hud.hud-blue");
1535+
await expect(page.locator("#assetIdInput")).toHaveValue("assets.color.accent.primary-hud");
15161536
await expect(page.locator("#statusLog")).toHaveValue(/OK Selected color HUD Blue validated as type color, kind hex, role accent\./);
15171537
await page.locator("#assetRoleSelect").selectOption("hud");
1518-
await expect(page.locator("#assetIdInput")).toHaveValue("assets.color.hud.primary-hud.hud-blue");
1538+
await expect(page.locator("#assetIdInput")).toHaveValue("assets.color.hud.primary-hud");
15191539
await expect(page.locator("#addAssetButton")).toBeEnabled();
15201540
await page.locator("#addAssetButton").click();
1521-
await expect(page.locator("#assetList")).toContainText("assets.color.hud.primary-hud.hud-blue");
1541+
await expect(page.locator("#assetList")).toContainText("assets.color.hud.primary-hud");
15221542
await expect(page.locator('#assetPreview [data-preview-type="color"][data-preview-kind="hex"]')).toBeVisible();
15231543
await expect(page.locator(".asset-manager-v2__preview-color span")).toHaveCSS("background-color", "rgb(120, 183, 255)");
15241544
await expect(page.locator("#inspectorOutput")).toContainText("\"type\": \"color\"");
@@ -1579,7 +1599,7 @@ test.describe("Asset Manager V2", () => {
15791599
role: "preview",
15801600
source: "asset-manager-v2"
15811601
});
1582-
expect(storedAssetSession.data.assets["assets.color.hud.primary-hud.hud-blue"]).toEqual({
1602+
expect(storedAssetSession.data.assets["assets.color.hud.primary-hud"]).toEqual({
15831603
path: "palette://workspace/hud-blue",
15841604
type: "color",
15851605
kind: "hex",
@@ -1596,7 +1616,7 @@ test.describe("Asset Manager V2", () => {
15961616
expect(storedContext.tools["palette-manager-v2"].swatches.length).toBeGreaterThan(0);
15971617
expect(storedContext.tools["object-vector-studio-v2"].objects.map((object) => object.id)).toContain("object.asteroids.ship");
15981618
expect(storedContext.tools["workspace-v2"]).toBeUndefined();
1599-
expect(Object.keys(storedContext.tools).sort()).toEqual(["asset-manager-v2", "object-vector-studio-v2", "palette-manager-v2", "text2speech-V2"]);
1619+
expect(Object.keys(storedContext.tools).sort()).toEqual(["asset-manager-v2", "object-vector-studio-v2", "palette-manager-v2"]);
16001620
await page.locator("#returnToWorkspaceButton").click();
16011621
await expect(page).toHaveURL(/workspace-manager-v2\/index\.html\?hostContextId=workspace-manager-v2-/);
16021622
await expect(page.locator("#activeGameSelect")).toHaveValue("Asteroids");
@@ -1614,7 +1634,7 @@ test.describe("Asset Manager V2", () => {
16141634
expect(savedManifest.tools["asset-manager-v2"].previewImagePath).toBeUndefined();
16151635
expect(savedManifest.tools["asset-manager-v2"].assets["assets.color.background.game"]).toEqual(storedContext.tools["asset-manager-v2"].assets["assets.color.background.game"]);
16161636
expect(savedManifest.tools["asset-manager-v2"].assets["assets.audio.sound.laser"]).toEqual(storedAssetSession.data.assets["assets.audio.sound.laser"]);
1617-
expect(savedManifest.tools["asset-manager-v2"].assets["assets.color.hud.primary-hud.hud-blue"]).toEqual(storedAssetSession.data.assets["assets.color.hud.primary-hud.hud-blue"]);
1637+
expect(savedManifest.tools["asset-manager-v2"].assets["assets.color.hud.primary-hud"]).toEqual(storedAssetSession.data.assets["assets.color.hud.primary-hud"]);
16181638
expect(savedManifest.tools["object-vector-studio-v2"].objects.map((object) => object.id)).toContain("object.asteroids.ship");
16191639

16201640
expect(pageErrors).toEqual([]);

tests/playwright/tools/WorkspaceManagerV2.spec.mjs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10675,13 +10675,13 @@ test.describe("Workspace Manager V2 bootstrap", () => {
1067510675
await expect(sessionPurpleSwatch).toBeVisible();
1067610676
await sessionPurpleSwatch.click();
1067710677
await page.locator("#assetUsageInput").fill("session");
10678-
await expect(page.locator("#assetIdInput")).toHaveValue("assets.color.hud.session.workspace-session-purple");
10678+
await expect(page.locator("#assetIdInput")).toHaveValue("assets.color.hud.session");
1067910679
await page.locator("#addAssetButton").click();
10680-
await expect(page.locator("#statusLog")).toHaveValue(/OK Added assets\.color\.hud\.session\.workspace-session-purple\./);
10680+
await expect(page.locator("#statusLog")).toHaveValue(/OK Added assets\.color\.hud\.session\./);
1068110681
await expect(page.locator("#statusLog")).toHaveValue(/OK workspace\.tools\.asset-manager-v2 now has 16 validated assets\./);
1068210682
const editedAssetSession = await page.evaluate(() => JSON.parse(sessionStorage.getItem("workspace.tools.asset-manager-v2")));
1068310683
expect(Object.keys(editedAssetSession.data.assets)).toHaveLength(16);
10684-
expect(editedAssetSession.data.assets["assets.color.hud.session.workspace-session-purple"]).toMatchObject({
10684+
expect(editedAssetSession.data.assets["assets.color.hud.session"]).toMatchObject({
1068510685
color: {
1068610686
hex: "#123456",
1068710687
name: "Workspace Session Purple",
@@ -10700,7 +10700,7 @@ test.describe("Workspace Manager V2 bootstrap", () => {
1070010700
expect(Date.parse(editedAssetSession.dirty.changedAt)).not.toBeNaN();
1070110701
expect(editedAssetSession.dirty.changedKeys).toEqual(expect.arrayContaining([
1070210702
"data.assets",
10703-
'data.assets["assets.color.hud.session.workspace-session-purple"]'
10703+
'data.assets["assets.color.hud.session"]'
1070410704
]));
1070510705
await page.locator("#returnToWorkspaceButton").click();
1070610706
await expect(page).toHaveURL(/workspace-manager-v2\/index\.html\?hostContextId=workspace-manager-v2-/);

tools/asset-manager-v2/js/assetManagerMetadata.js

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -235,21 +235,13 @@ export function assetIdForFile(type, fileName, role) {
235235
return `assets.${normalizedType}.${normalizedRole}.${filenamePartForAssetId(fileName)}`;
236236
}
237237

238-
export function assetIdForColor(type, role, usage, colorName) {
238+
export function assetIdForColor(type, role, usage) {
239239
const normalizedType = dotPathIdSegment(type);
240240
const normalizedRole = dotPathIdSegment(role);
241-
const normalizedUsage = dotPathIdSegment(usage);
242-
const normalizedColorName = dotPathIdSegment(colorName);
243-
if (!normalizedType || !normalizedRole || !normalizedUsage) {
244-
return "";
245-
}
246-
if (normalizedType === "color" && normalizedRole === "background" && normalizedUsage === "game") {
247-
return "assets.color.background.game";
248-
}
249-
if (!normalizedColorName) {
241+
if (!normalizedType || !normalizedRole) {
250242
return "";
251243
}
252-
return `assets.${normalizedType}.${normalizedRole}.${normalizedUsage}.${normalizedColorName}`;
244+
return `assets.${normalizedType}.${normalizedRole}.${dotPathIdSegment(usage) || "game"}`;
253245
}
254246

255247
export function suggestedRoleForFile(type, fileName) {

tools/asset-manager-v2/js/controls/AssetFormControl.js

Lines changed: 42 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ export class AssetFormControl {
206206
input.addEventListener("change", () => {
207207
this.updateFileAccept();
208208
this.updateRoleOptions({ preserveCurrentRole: false });
209-
this.clearSelectionFields();
209+
this.applyTypeChange({ onFileSelected });
210210
this.updatePickerMode();
211211
onChange();
212212
});
@@ -217,7 +217,7 @@ export class AssetFormControl {
217217
this.applyDerivedFileValues();
218218
onFileSelected(this.readValue(), this.selectedFileInfo);
219219
} else if (this.selectedColorInfo) {
220-
this.applyDerivedAssetId(this.selectedColorInfo.name);
220+
this.applyDerivedAssetId();
221221
this.onColorSelected?.(this.readValue(), this.selectedColorInfo);
222222
} else {
223223
this.applyDerivedAssetId();
@@ -288,7 +288,6 @@ export class AssetFormControl {
288288
&& value.kind
289289
&& value.path
290290
&& value.role
291-
&& (value.type !== "color" || value.usage)
292291
&& !this.selectedFileError);
293292
}
294293

@@ -450,7 +449,7 @@ export class AssetFormControl {
450449
this.selectedFileError = "";
451450
this.kindValue = "hex";
452451
this.pathInput.value = colorAssetPath(this.selectedColorInfo.name);
453-
this.applyDerivedAssetId(this.selectedColorInfo.name);
452+
this.applyDerivedAssetId();
454453
this.renderColorSwatches();
455454
}
456455

@@ -468,10 +467,48 @@ export class AssetFormControl {
468467
const fallbackFromId = this.assetIdInput.value.split(".").slice(3).join(".");
469468
const name = fileName || fallbackFromPath || fallbackFromId;
470469
this.assetIdInput.value = this.selectedKind() === "color"
471-
? assetIdForColor(this.selectedKind(), this.selectedRole(), this.usageInput.value, name)
470+
? assetIdForColor(this.selectedKind(), this.selectedRole(), this.usageInput.value)
472471
: assetIdForFile(this.selectedKind(), name, this.selectedRole());
473472
}
474473

474+
applyTypeChange({ onFileSelected } = {}) {
475+
const type = this.selectedKind();
476+
if (type === "color") {
477+
this.selectedFileInfo = null;
478+
this.selectedFileError = "";
479+
this.kindValue = this.selectedColorInfo ? "hex" : "";
480+
this.pathInput.value = this.selectedColorInfo ? colorAssetPath(this.selectedColorInfo.name) : "";
481+
this.applyDerivedAssetId();
482+
if (this.selectedColorInfo) {
483+
this.onColorSelected?.(this.readValue(), this.selectedColorInfo);
484+
}
485+
return;
486+
}
487+
if (!this.selectedFileInfo) {
488+
this.clearSelectionFields();
489+
return;
490+
}
491+
this.selectedColorInfo = null;
492+
this.selectedFileInfo = {
493+
...this.selectedFileInfo,
494+
type
495+
};
496+
const suggestedRole = suggestedRoleForFile(type, this.selectedFileInfo.name);
497+
if (suggestedRole && [...this.roleSelect.options].some((option) => option.value === suggestedRole)) {
498+
this.roleSelect.value = suggestedRole;
499+
}
500+
this.updateStretchControl();
501+
this.selectedFileError = fileMatchesAccept(type, {
502+
name: this.selectedFileInfo.name,
503+
type: this.selectedFileInfo.mimeType
504+
})
505+
? ""
506+
: `File ${this.selectedFileInfo.name} is not accepted for ${labelForKind(type)} assets.`;
507+
this.kindValue = this.selectedFileInfo.kind;
508+
this.applyDerivedFileValues();
509+
onFileSelected?.(this.readValue(), this.selectedFileInfo);
510+
}
511+
475512
updateStretchControl({ preserveValue = false } = {}) {
476513
const role = this.selectedRole();
477514
const supportsStretch = roleSupportsStretchOverride(this.selectedKind(), role);

0 commit comments

Comments
 (0)