-
Notifications
You must be signed in to change notification settings - Fork 0
PR_26179_CHARLIE_035-sprites-frame-editing #272
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| const DEFAULT_GRID_SIZE = 16; | ||
| const DEFAULT_FRAME_DURATION_MS = 120; | ||
| const SUPPORTED_GRID_SIZES = Object.freeze([16, 32]); | ||
| const SUPPORTED_ZOOM_LEVELS = Object.freeze([1, 2, 4]); | ||
| const SHAPE_TOOLS = Object.freeze(["line", "rectangle", "circle"]); | ||
|
|
@@ -12,11 +13,22 @@ const EDITOR_COLOR_CSS_VARIABLES = Object.freeze({ | |
| orange: "--molten-orange", | ||
| }); | ||
|
|
||
| function createEditorFrame(frameNumber, paintedPixels = new Map()) { | ||
| return { | ||
| durationMs: DEFAULT_FRAME_DURATION_MS, | ||
| frameNumber, | ||
| paintedPixels: new Map(paintedPixels), | ||
| }; | ||
| } | ||
|
|
||
| const initialFrame = createEditorFrame(1); | ||
| const editorState = { | ||
| activeTool: "pencil", | ||
| activeColor: "ink", | ||
| activeFrameIndex: 0, | ||
| frames: [initialFrame], | ||
| gridSize: DEFAULT_GRID_SIZE, | ||
| paintedPixels: new Map(), | ||
| paintedPixels: initialFrame.paintedPixels, | ||
| shapeAnchor: null, | ||
| zoomLevel: 1, | ||
| }; | ||
|
|
@@ -48,8 +60,23 @@ function isInsideGrid(row, column) { | |
| return row >= 1 && row <= editorState.gridSize && column >= 1 && column <= editorState.gridSize; | ||
| } | ||
|
|
||
| function currentFrame() { | ||
| return editorState.frames[editorState.activeFrameIndex] || editorState.frames[0]; | ||
| } | ||
|
|
||
| function syncActiveFramePixels() { | ||
| editorState.paintedPixels = currentFrame().paintedPixels; | ||
| } | ||
|
|
||
| function renumberFrames() { | ||
| editorState.frames.forEach((frame, index) => { | ||
| frame.frameNumber = index + 1; | ||
| }); | ||
| } | ||
|
|
||
| function stateSnapshot() { | ||
| return { | ||
| activeFrameIndex: editorState.activeFrameIndex, | ||
| gridSize: editorState.gridSize, | ||
| paintedPixels: Array.from(editorState.paintedPixels.entries()), | ||
| }; | ||
|
|
@@ -144,7 +171,10 @@ function applyPaintedPixelsToGrid() { | |
|
|
||
| function applySnapshot(snapshot) { | ||
| setGridSize(snapshot.gridSize, { recordHistory: false }); | ||
| editorState.paintedPixels = new Map(snapshot.paintedPixels); | ||
| const targetIndex = Math.min(snapshot.activeFrameIndex ?? editorState.activeFrameIndex, editorState.frames.length - 1); | ||
| editorState.activeFrameIndex = Math.max(0, targetIndex); | ||
| currentFrame().paintedPixels = new Map(snapshot.paintedPixels); | ||
| syncActiveFramePixels(); | ||
| editorState.shapeAnchor = null; | ||
| applyPaintedPixelsToGrid(); | ||
| updateDraftStatus(); | ||
|
|
@@ -446,14 +476,86 @@ function renderPreview() { | |
| } | ||
|
|
||
| function renderFrameStrip() { | ||
| const thumbnail = document.querySelector("[data-sprites-frame-thumbnail='0']"); | ||
| const strip = document.querySelector("[data-sprites-frame-strip]"); | ||
| const status = document.querySelector("[data-sprites-frame-status]"); | ||
| if (thumbnail) { | ||
| renderPixelsToCanvas(thumbnail, editorState.paintedPixels, editorState.gridSize); | ||
| if (strip) { | ||
| strip.replaceChildren(); | ||
| editorState.frames.forEach((frame, index) => { | ||
| const button = document.createElement("button"); | ||
| button.className = `sprite-frame-card${index === editorState.activeFrameIndex ? " is-active" : ""}`; | ||
| button.type = "button"; | ||
| button.dataset.spritesFrameCard = String(index); | ||
| button.setAttribute("aria-pressed", String(index === editorState.activeFrameIndex)); | ||
| const title = document.createElement("span"); | ||
| title.className = "sprite-frame-card-title"; | ||
| title.textContent = `Frame ${frame.frameNumber}`; | ||
| const thumbnail = document.createElement("canvas"); | ||
| thumbnail.className = "sprite-frame-thumbnail"; | ||
| thumbnail.width = 48; | ||
| thumbnail.height = 48; | ||
| thumbnail.setAttribute("aria-label", `Frame ${frame.frameNumber} thumbnail`); | ||
| thumbnail.dataset.spritesFrameThumbnail = String(index); | ||
| const meta = document.createElement("span"); | ||
| meta.className = "sprite-frame-card-meta"; | ||
| meta.textContent = `${frame.paintedPixels.size} pixel${frame.paintedPixels.size === 1 ? "" : "s"}`; | ||
| button.append(title, thumbnail, meta); | ||
| button.addEventListener("click", () => selectFrame(index)); | ||
| strip.append(button); | ||
| renderPixelsToCanvas(thumbnail, frame.paintedPixels, editorState.gridSize); | ||
| }); | ||
| } | ||
| if (status) { | ||
| status.textContent = `Frame strip: 1 unsaved frame. Current editor draft has ${editorState.paintedPixels.size} painted pixel${editorState.paintedPixels.size === 1 ? "" : "s"}.`; | ||
| status.textContent = `Frame strip: ${editorState.frames.length} unsaved frame${editorState.frames.length === 1 ? "" : "s"}. Frame ${currentFrame().frameNumber} is selected with ${editorState.paintedPixels.size} painted pixel${editorState.paintedPixels.size === 1 ? "" : "s"}.`; | ||
| } | ||
| const deleteButton = document.querySelector("[data-sprites-delete-frame]"); | ||
| if (deleteButton) { | ||
| deleteButton.disabled = editorState.frames.length <= 1; | ||
| } | ||
| } | ||
|
|
||
| function selectFrame(index) { | ||
| if (index < 0 || index >= editorState.frames.length) { | ||
| return; | ||
| } | ||
| editorState.activeFrameIndex = index; | ||
| syncActiveFramePixels(); | ||
| editorState.shapeAnchor = null; | ||
| applyPaintedPixelsToGrid(); | ||
| updateDraftStatus(); | ||
| updateHistoryControls(); | ||
| } | ||
|
|
||
| function addFrame() { | ||
| editorState.frames.push(createEditorFrame(editorState.frames.length + 1)); | ||
| editorState.activeFrameIndex = editorState.frames.length - 1; | ||
| syncActiveFramePixels(); | ||
| editorState.shapeAnchor = null; | ||
| updateDraftStatus(); | ||
| updateHistoryControls(); | ||
| } | ||
|
|
||
| function duplicateFrame() { | ||
| const duplicate = createEditorFrame(editorState.frames.length + 1, editorState.paintedPixels); | ||
| editorState.frames.push(duplicate); | ||
| editorState.activeFrameIndex = editorState.frames.length - 1; | ||
| syncActiveFramePixels(); | ||
| editorState.shapeAnchor = null; | ||
| updateDraftStatus(); | ||
| updateHistoryControls(); | ||
| } | ||
|
|
||
| function deleteFrame() { | ||
| if (editorState.frames.length <= 1) { | ||
| return; | ||
| } | ||
| editorState.frames.splice(editorState.activeFrameIndex, 1); | ||
| renumberFrames(); | ||
| editorState.activeFrameIndex = Math.max(0, Math.min(editorState.activeFrameIndex, editorState.frames.length - 1)); | ||
| syncActiveFramePixels(); | ||
| editorState.shapeAnchor = null; | ||
| applyPaintedPixelsToGrid(); | ||
| updateDraftStatus(); | ||
| updateHistoryControls(); | ||
| } | ||
|
|
||
| function exportPreviewPng() { | ||
|
|
@@ -497,7 +599,10 @@ function setGridSize(size, options = {}) { | |
|
|
||
| grid.replaceChildren(); | ||
| editorState.gridSize = size; | ||
| editorState.paintedPixels.clear(); | ||
| editorState.frames.forEach((frame) => { | ||
| frame.paintedPixels.clear(); | ||
| }); | ||
|
Comment on lines
+602
to
+604
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When multiple frames exist, any Undo/Redo path calls Useful? React with 👍 / 👎. |
||
| syncActiveFramePixels(); | ||
| editorState.shapeAnchor = null; | ||
| grid.dataset.spritesGridSize = String(size); | ||
| grid.setAttribute("aria-label", gridLabel(size)); | ||
|
|
@@ -579,6 +684,23 @@ function wireCanvasActions() { | |
| } | ||
| } | ||
|
|
||
| function wireFrameActions() { | ||
| const addButton = document.querySelector("[data-sprites-add-frame]"); | ||
| if (addButton) { | ||
| addButton.addEventListener("click", addFrame); | ||
| } | ||
|
|
||
| const duplicateButton = document.querySelector("[data-sprites-duplicate-frame]"); | ||
| if (duplicateButton) { | ||
| duplicateButton.addEventListener("click", duplicateFrame); | ||
| } | ||
|
|
||
| const deleteButton = document.querySelector("[data-sprites-delete-frame]"); | ||
| if (deleteButton) { | ||
| deleteButton.addEventListener("click", deleteFrame); | ||
| } | ||
| } | ||
|
|
||
| function wireHistoryActions() { | ||
| const undoButton = document.querySelector("[data-sprites-undo]"); | ||
| if (undoButton) { | ||
|
|
@@ -618,6 +740,7 @@ wireGridControls(); | |
| wireDrawingTools(); | ||
| wirePaletteButtons(); | ||
| wireCanvasActions(); | ||
| wireFrameActions(); | ||
| wireHistoryActions(); | ||
| wireZoomControls(); | ||
| wireExportButton(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| # PR_26179_CHARLIE_035-sprites-frame-editing | ||
|
|
||
| Team: CHARLIE | ||
|
|
||
| Mode: Batch governance stacked feature workflow | ||
|
|
||
| Base branch: PR_26179_CHARLIE_034-sprites-frame-strip | ||
|
|
||
| Branch: PR_26179_CHARLIE_035-sprites-frame-editing | ||
|
|
||
| ## Summary | ||
|
|
||
| Added unsaved page-session frame editing for Sprite Creator. Creators can add, duplicate, select, and delete frames in the frame strip. Each frame keeps its own draft pixel map in page-session state only. No persistence, database, API, schema, or product-data source-of-truth behavior was introduced. | ||
|
|
||
| ## Branch Validation | ||
|
|
||
| PASS | ||
|
|
||
| - Built from `PR_26179_CHARLIE_034-sprites-frame-strip`. | ||
| - Scope stayed limited to unsaved frame editing. | ||
| - No DB/API/schema changes. | ||
| - No saved product data or authoritative browser-owned product data added. | ||
| - No start_of_day files changed. | ||
| - ZIP package created at the requested path. | ||
|
|
||
| ## Requirement Checklist | ||
|
|
||
| PASS - Add Frame control added. | ||
|
|
||
| PASS - Duplicate Frame control added. | ||
|
|
||
| PASS - Delete Frame control added and disabled when only one frame exists. | ||
|
|
||
| PASS - Frame selection swaps the center canvas and preview to the selected frame. | ||
|
|
||
| PASS - Each frame keeps separate unsaved page-session pixels. | ||
|
|
||
| PASS - No persistence added. | ||
|
|
||
| PASS - Targeted Playwright coverage updated. | ||
|
|
||
| ## Validation Lane | ||
|
|
||
| PASS - `node --check assets/toolbox/sprites/js/index.js` | ||
|
|
||
| PASS - `node --check dev/tests/playwright/tools/SpritesToolShell.spec.mjs` | ||
|
|
||
| PASS - `git diff --check -- toolbox/sprites/index.html assets/toolbox/sprites/js/index.js dev/tests/playwright/tools/SpritesToolShell.spec.mjs` | ||
|
|
||
| PASS - Runtime guard scan found no prohibited inline style/script/event-handler or stale placeholder text in touched runtime files. | ||
|
|
||
| PASS - `npx playwright test dev/tests/playwright/tools/SpritesToolShell.spec.mjs --workers=1 --reporter=list` | ||
|
|
||
| ## Manual Validation Notes | ||
|
|
||
| 1. Open `toolbox/sprites/index.html`. | ||
| 2. Draw on Frame 1. | ||
| 3. Click Add Frame and confirm Frame 2 is empty. | ||
| 4. Draw different pixels on Frame 2. | ||
| 5. Select Frame 1 and confirm its pixels return. | ||
| 6. Select Frame 2 and confirm its pixels return. | ||
| 7. Duplicate Frame 2 and confirm the duplicate keeps the current pixels. | ||
| 8. Delete the duplicate and confirm the strip returns to two unsaved frames. | ||
|
|
||
| ## ZIP | ||
|
|
||
| `dev/workspace/zip/PR_26179_CHARLIE_035-sprites-frame-editing_delta.zip` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,6 @@ | ||
| assets/theme-v2/css/gamefoundrystudio.css | ||
| assets/toolbox/sprites/js/index.js | ||
| dev/tests/playwright/tools/SpritesToolShell.spec.mjs | ||
| docs_build/dev/reports/PR_26179_CHARLIE_034-sprites-frame-strip.md | ||
| docs_build/dev/reports/PR_26179_CHARLIE_035-sprites-frame-editing.md | ||
| docs_build/dev/reports/codex_changed_files.txt | ||
| docs_build/dev/reports/codex_review.diff | ||
| toolbox/sprites/index.html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the active frame has edit history, deleting it leaves those snapshots in the global undo stack and keeps Undo enabled. If a user paints Frame 2, deletes Frame 2, then presses Undo,
applySnapshot()clamps that stale frame index to the remaining frame and overwrites Frame 1 with Frame 2's snapshot. Either record frame deletion in history or remove snapshots for the deleted frame before exposing Undo again.Useful? React with 👍 / 👎.