feat(gamestudio): AI-assisted game maker - real generation, editor + live preview, library, share (.taosapp)#1602
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Warning Review limit reached
Next review available in: 31 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (38)
📝 WalkthroughWalkthroughThis PR rewrites Game Studio from a scene/template-based create-play-share pipeline into an AI-assisted create-editor-library-share flow backed by a new persistent games API, adds four playable seed games, and separately fixes rkllama's default port, toggle styling, and provider lifecycle_state reporting. ChangesGame Studio Rewrite
Providers Fixes
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CreateView
participant GenerateGame
participant TaosAgent
participant GamesApi
User->>CreateView: enter prompt, click Generate
CreateView->>GenerateGame: generateGame(prompt, onProgress)
GenerateGame->>GamesApi: fetchSeedFiles(template)
GenerateGame->>TaosAgent: stream chat request
TaosAgent-->>GenerateGame: streamed file-block response
GenerateGame->>GenerateGame: parseFileBlocks, preserve vendor files
GenerateGame->>GamesApi: saveGame(id, files)
GamesApi-->>GenerateGame: GameRecord
GenerateGame-->>CreateView: result.game.id
CreateView->>User: onOpenGame -> Editor view
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
Adds /api/games CRUD (list/get/save/delete), a sandboxed preview endpoint
(/api/games/{id}/preview/{path}) with a strict no-allow-same-origin CSP,
and a package endpoint that builds a real .taosapp zip via a new
build_package() helper in userspace/package.py (the inverse of the
existing extract_package()). Mirrors routes/video.py's storage pattern:
data/games/{id}/ with a game.json sidecar, filename traversal guards,
and per-file/total size caps. Leaves /api/games/chess/* untouched.
Four starter templates ship as actual playable seed files under desktop/public/gamestudio-seeds/: platformer-lite and orbit-shooter on three.js (bundling a relative three.module.js, never a CDN), and top-down-collector and breakout on canvas. Create streams the taos-agent the same way App Studio's Build view does, using a file-block convention (### FILE: <name> + a fenced block per file, see file-blocks.ts) to parse the agent's response; on a parse failure the unmodified seed is saved instead and the user is told so, never a fake 'customized' result. Vendor files (three.module.js) are never sent to the agent and can never be overwritten by its response. CreateView now streams real progress from the actual request instead of a setTimeout-driven build log.
…chat
The core of Game Studio: a file list + textarea code editor (Coding
Studio's CodeMirror setup is a private, unexported component, so this is
the agreed v1 fallback -- monospace, synced line-number gutter,
Tab-to-indent), a live preview iframe pointed at the real
/api/games/{id}/preview/index.html with sandbox="allow-scripts" (no
allow-same-origin), and an AI chat sidebar that proposes file changes
using the same file-block convention as Create. Manual edits and applied
AI changes both save through debounced PUT /api/games/{id} and refresh
the preview. Play maximizes the preview to fullscreen with the same
mandatory Exit-to-taOS + Escape handling as the old PlayView, keeping
keyboard focus on the game iframe.
Library lists saved games (GET /api/games) with open-in-editor, rename,
and a backend-confirmed delete -- the row only drops after DELETE
succeeds and the list is refetched from the server, never optimistically.
Share replaces the stub 'coming soon' notice with two real actions built
on the same package: GET /api/games/{id}/package (added in the
persistence commit) returns a real .taosapp zip. 'Install on this taOS'
POSTs that exact file to the existing, unmodified
/api/userspace-apps/install endpoint -- the same pipeline
ImportAppButton already uses -- tagged provenance "ai-generated" (a
small optional param added to installUserspaceApp() for this). The
static security analyzer runs on install same as any other userspace
app; findings are previewed and shown honestly via App Studio's existing
analyze-source.ts/FindingsPanel, reused as-is. 'Export .taosapp'
downloads the identical package as a file. No public-store submission in
v1 -- that goes through the maintainer review flow.
… docs GameStudioApp's rail is now Create / Editor / Library / Share -- Editor absorbs the old Play view (a fullscreen Play mode lives inside it instead), Library is new. GameStudioApp.test.tsx adapts the 13 existing tests to the real flows (navigation, Create's prompt + template gallery, using a template round-trips through save -> open -> load against a small in-memory fake of the games backend) instead of the old fake build-log assertions. Updates the Game Studio blurb in the README's studios section to describe the real capability.
jaylfc
force-pushed
the
feat/gamestudio-ai-maker
branch
from
6589347 to
d8b471f
Compare
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| continue | ||
| try: | ||
| files[p.name] = p.read_text(encoding="utf-8") | ||
| except (UnicodeDecodeError, OSError): |
There was a problem hiding this comment.
CRITICAL: _list_game_files silently drops any file that is not valid UTF-8 text (binary assets, UTF-16, etc.) via the bare except (UnicodeDecodeError, OSError): continue. The PR description explicitly calls out "Generated art/audio assets" as a v1 follow-up, so binary uploads are on the roadmap. When that lands, any non-text asset will vanish from the GET response and from the generated .taosapp package with no error surfaced to the client. Consider returning an explicit "skipped binary file" entry (or rejecting non-text files at save time with a clear 415), and at minimum log a warning so a future bug report is debuggable.
| game_dir.mkdir(parents=True, exist_ok=True) | ||
|
|
||
| for p in game_dir.iterdir(): | ||
| if p.is_file() and p.name != "game.json" and p.name not in body.files: |
There was a problem hiding this comment.
WARNING: save_game is non-atomic: it first unlinks any existing files not in the new file map, then writes the new ones. A crash, OOM kill, or request timeout between the unlink loop (line 131-133) and the write loop (line 134-135) leaves the game in a partially-deleted state on disk, and the next GET will return a reduced file set. Write the new files to a staging directory (or write to *.tmp and rename) before unlinking, so the on-disk state is always a complete previous-or-new snapshot.
| async def list_games(request: Request): | ||
| """List all saved games' metadata, most recently updated first.""" | ||
| games_dir = _games_dir(request) | ||
| results = [] |
There was a problem hiding this comment.
WARNING: list_games reads every game's game.json synchronously and returns the full list with no pagination, sorting, or limit. With the 40-file × 2 MB per-game caps this is fine for tens of games but will degrade (and waste memory on the server) as the library grows. Consider at minimum a ?limit=&offset= query and a stable sort key, and cap the response with a sane default (e.g. 200).
|
|
||
|
|
||
| def _is_safe_filename(name: str) -> bool: | ||
| return bool(name) and name not in (".", "..") and "/" not in name and "\\" not in name and ".." not in name |
There was a problem hiding this comment.
WARNING: _is_safe_filename rejects ANY filename containing the substring .. (not just as a leading/trailing path component), so legitimate names like my..backup.js or data..v2.json are blocked. That matches the same overly-broad rule in file-blocks.ts:isSafeFilename, so the two stay in sync, but both should be tightened to not (name == "." or name == ".." or name.startswith("../") or name.startswith("./")) plus the separator check, so future filenames that legitimately contain .. are not silently rejected.
| buf = io.BytesIO() | ||
| with zipfile.ZipFile(buf, "w", zipfile.ZIP_DEFLATED) as zf: | ||
| zf.writestr("manifest.yaml", yaml.safe_dump(manifest, sort_keys=False)) | ||
| for name, content in files.items(): |
There was a problem hiding this comment.
WARNING: build_package iterates files.items() and passes content straight to zf.writestr(name, content). There is no validation that content is a str (a non-string would surface as a low-level TypeError from zipfile) and no validation that name is a flat filename without path separators (so a malicious or buggy caller can write members like ../escape.txt into the zip). The current call sites in routes/games.py already pre-validate names, so this is safe today, but build_package is a new public-ish helper that the PR description claims is the inverse of extract_package -- it should defend itself the same way extract_package does (reject traversals, assert string content) so future callers cannot accidentally produce an unsafe package.
| setSaveState("saving"); | ||
| setSaveError(null); | ||
| try { | ||
| await saveGame(gameId, { files: nextFiles }); |
There was a problem hiding this comment.
WARNING: The debounced save does not cancel in-flight writes. scheduleSave (line 131) only debounces the next save, not the current one. If the user types fast enough that a second flushSave fires before the first await saveGame(gameId, ...) on line 118 resolves, two PUTs race against each other. Both use full-replace semantics so the final state is the later write, but the first write still consumes a network round-trip and produces a transient intermediate server state that the next GET would see. Track an in-flight token (or AbortController on the fetch) and either skip overlapping saves or wait for the current one to finish before queuing the next.
| }; | ||
|
|
||
| const handleKeyDown = (e: React.KeyboardEvent<HTMLTextAreaElement>) => { | ||
| if (e.key !== "Tab") return; |
There was a problem hiding this comment.
SUGGESTION: The Tab handler only inserts 2 spaces at the caret/selection and has no Shift+Tab outdent, so selecting 5 lines and pressing Tab indents nothing. For a code editor this is the single most-expected feature and is also what every other editor in the desktop (Coding Studio's CodeView, App Studio's code editor) provides. Either route to a shared indent helper or at minimum split the selection on \n and prepend/remove 2 spaces to each line.
Code Review SummaryStatus: 7 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
SUGGESTION
Files Reviewed (38 files)
Fix these issues in Kilo Cloud Reviewed by minimax-m3 · Input: 101.3K · Output: 14.9K · Cached: 2.9M |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
desktop/src/apps/gamestudio/games-api.ts (1)
85-87: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winLow-entropy id suffix risks silent game overwrite on collision.
uniqueSuffix()produces only ~1.68M possible values (4 base36 chars). SincegenerateGame/createFromTemplatebuild ids asslugify(prompt-or-title) + "-" + uniqueSuffix(), and template-based creation always uses the same deterministic prefix (e.g. every "Use template" on Breakout yieldsbreakout-<suffix>), the effective collision space per template is just this suffix space. Because the backend'sPUTis an upsert that unlinks any existing file not in the new request (per the games.py snippet), a collision would silently delete/overwrite an unrelated game's files.
crypto.randomUUID()is a standard, well-supported browser API for this use case and trivially increases entropy.♻️ Proposed fix
export function uniqueSuffix(): string { - return Math.random().toString(36).slice(2, 6); + return crypto.randomUUID().replace(/-/g, "").slice(0, 8); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@desktop/src/apps/gamestudio/games-api.ts` around lines 85 - 87, The id suffix generation in uniqueSuffix() is too low-entropy and can collide, causing generateGame and createFromTemplate to reuse the same slugified prefix plus short suffix. Replace the Math.random().toString(36).slice(2, 6) approach with a stronger browser-native random id source such as crypto.randomUUID() (or an equivalent higher-entropy suffix generator), and keep the existing callers unchanged so game ids remain unique across repeated template creations.desktop/public/gamestudio-seeds/platformer-lite/game.js (1)
203-208: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winPer-frame allocations in the camera follow.
updateCameraallocates a.clone()plus anew THREE.Vector3(0, 0.5, 0)every single frame. Both can be hoisted/reused across calls.♻️ Proposed fix
+const LOOK_OFFSET = new THREE.Vector3(0, 0.5, 0); +const lookTarget = new THREE.Vector3(); + function updateCamera(dt) { cameraTarget.copy(player.position).add(cameraOffset); camera.position.lerp(cameraTarget, 1 - Math.pow(0.001, dt)); - const lookTarget = player.position.clone().add(new THREE.Vector3(0, 0.5, 0)); + lookTarget.copy(player.position).add(LOOK_OFFSET); camera.lookAt(lookTarget); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@desktop/public/gamestudio-seeds/platformer-lite/game.js` around lines 203 - 208, The updateCamera function is creating per-frame allocations by calling player.position.clone() and new THREE.Vector3(0, 0.5, 0) on every update. Hoist and reuse a persistent Vector3 for the look target offset, then update it in place inside updateCamera so camera.lookAt can use the reused vector without allocating each frame.desktop/public/gamestudio-seeds/orbit-shooter/game.js (1)
173-190: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winPer-frame
Vector3allocation in the enemy update loop.
enemy.mesh.position.lerpVectors(enemy.start, new THREE.Vector3(0, 0.6, 0), t)allocates a newVector3every enemy, every frame. Hoist it as a module-level constant target since it never changes.♻️ Proposed fix
+const ENEMY_TARGET = new THREE.Vector3(0, 0.6, 0); + function updateEnemies(dt) { for (let i = enemies.length - 1; i >= 0; i--) { const enemy = enemies[i]; enemy.elapsed += dt; const t = Math.min(enemy.elapsed / enemy.travelTime, 1); - enemy.mesh.position.lerpVectors(enemy.start, new THREE.Vector3(0, 0.6, 0), t); + enemy.mesh.position.lerpVectors(enemy.start, ENEMY_TARGET, t);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@desktop/public/gamestudio-seeds/orbit-shooter/game.js` around lines 173 - 190, The enemy update loop in updateEnemies is creating a new THREE.Vector3 for the fixed target on every frame, which causes unnecessary allocations. Hoist the constant target vector (0, 0.6, 0) to a module-level reusable symbol and pass that into enemy.mesh.position.lerpVectors so the loop reuses the same object instead of constructing one per enemy per tick.desktop/src/apps/gamestudio/EditorView.test.tsx (1)
77-105: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider a regression test for the debounce/apply save-ordering race.
This suite covers the happy-path apply flow well, but doesn't cover the scenario where a manual edit schedules a debounced save and an AI proposal is applied before that timer fires (see the
handleApplycomment inEditorView.tsx). Once that fix lands, a test asserting the last PUT reflects the applied AI content (not a stale debounced payload) would guard against regressing it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@desktop/src/apps/gamestudio/EditorView.test.tsx` around lines 77 - 105, Add a regression test in EditorView.test.tsx around the existing apply flow to cover the debounce/save-ordering race described in handleApply. Simulate a manual edit that schedules a debounced save, then apply the AI proposal before that timer fires, and assert through putCalls that the final PUT payload matches the applied AI content rather than a stale debounced draft. Use the existing EditorView, makeFetchMock, and apply-button flow to keep the test aligned with the current behavior.desktop/src/lib/userspace-apps.ts (1)
70-73: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winType
provenanceasAppProvenanceinstead ofstring.AppProvenanceis already defined in this module, and the current call sites only pass valid values, so this keeps the API aligned and catches bad tags at compile time.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@desktop/src/lib/userspace-apps.ts` around lines 70 - 73, The installUserspaceApp API currently accepts provenance as a plain string, which is too loose for the existing AppProvenance usage in this module. Update the installUserspaceApp function signature to type provenance as AppProvenance, and ensure any internal handling and call sites remain compatible so invalid tags are rejected at compile time. Use the installUserspaceApp and AppProvenance symbols to locate and adjust the affected types.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@desktop/public/gamestudio-seeds/breakout/game.js`:
- Around line 6-10: Preserve brick progress when resizing: resizeCanvas
currently triggers layoutBricks, and layoutBricks rebuilds all bricks as alive,
which resets destroyed bricks while score/lives remain unchanged. Update
layoutBricks to retain each brick’s existing alive state (for example by reusing
the prior bricks array/state when recalculating positions) and ensure
resizeCanvas only recomputes geometry via resizeCanvas and layoutBricks without
respawning already-broken bricks.
In `@desktop/src/apps/gamestudio/EditorView.tsx`:
- Around line 251-264: handleApply currently leaves the existing saveTimerRef
active and performs side effects inside the setFiles updater, which can race
with a later flush and duplicate the save request. In handleApply, clear any
pending debounce timer the same way handleManualRefresh does before applying
pendingEdit.files, then update state without calling flushSave from inside
setFiles. Move flushSave(next) out of the updater so the state change stays
pure, and only call setApplying(false) after the async flushSave completes so
the applying flag covers the full save lifecycle.
In `@desktop/src/apps/gamestudio/games-api.ts`:
- Around line 45-48: The renameGame flow is doing a read-modify-write on the
full GameRecord and re-sending the fetched files snapshot, which can overwrite
newer in-flight edits. Update renameGame to avoid touching files when only the
name changes, ideally by using a rename-only API or adding an optimistic
concurrency/version check in getGame/saveGame so the current
prompt/template/files state is not clobbered.
In `@desktop/src/apps/gamestudio/LibraryView.tsx`:
- Around line 30-87: The async handlers in LibraryView update React state after
awaiting requests without checking whether the component is still mounted, so
add the same mounted-ref/cancelled guard used in ShareView. Update LibraryView’s
load, handleRename, and handleDelete to skip setGames, setError, and setBusyId
in catch/finally when unmounted, using a shared ref or cleanup flag tied to the
component lifecycle.
---
Nitpick comments:
In `@desktop/public/gamestudio-seeds/orbit-shooter/game.js`:
- Around line 173-190: The enemy update loop in updateEnemies is creating a new
THREE.Vector3 for the fixed target on every frame, which causes unnecessary
allocations. Hoist the constant target vector (0, 0.6, 0) to a module-level
reusable symbol and pass that into enemy.mesh.position.lerpVectors so the loop
reuses the same object instead of constructing one per enemy per tick.
In `@desktop/public/gamestudio-seeds/platformer-lite/game.js`:
- Around line 203-208: The updateCamera function is creating per-frame
allocations by calling player.position.clone() and new THREE.Vector3(0, 0.5, 0)
on every update. Hoist and reuse a persistent Vector3 for the look target
offset, then update it in place inside updateCamera so camera.lookAt can use the
reused vector without allocating each frame.
In `@desktop/src/apps/gamestudio/EditorView.test.tsx`:
- Around line 77-105: Add a regression test in EditorView.test.tsx around the
existing apply flow to cover the debounce/save-ordering race described in
handleApply. Simulate a manual edit that schedules a debounced save, then apply
the AI proposal before that timer fires, and assert through putCalls that the
final PUT payload matches the applied AI content rather than a stale debounced
draft. Use the existing EditorView, makeFetchMock, and apply-button flow to keep
the test aligned with the current behavior.
In `@desktop/src/apps/gamestudio/games-api.ts`:
- Around line 85-87: The id suffix generation in uniqueSuffix() is too
low-entropy and can collide, causing generateGame and createFromTemplate to
reuse the same slugified prefix plus short suffix. Replace the
Math.random().toString(36).slice(2, 6) approach with a stronger browser-native
random id source such as crypto.randomUUID() (or an equivalent higher-entropy
suffix generator), and keep the existing callers unchanged so game ids remain
unique across repeated template creations.
In `@desktop/src/lib/userspace-apps.ts`:
- Around line 70-73: The installUserspaceApp API currently accepts provenance as
a plain string, which is too loose for the existing AppProvenance usage in this
module. Update the installUserspaceApp function signature to type provenance as
AppProvenance, and ensure any internal handling and call sites remain compatible
so invalid tags are rejected at compile time. Use the installUserspaceApp and
AppProvenance symbols to locate and adjust the affected types.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 556d8e68-ae84-49e0-8056-d62c4a394429
📒 Files selected for processing (42)
README.mddesktop/public/gamestudio-seeds/breakout/game.jsdesktop/public/gamestudio-seeds/breakout/index.htmldesktop/public/gamestudio-seeds/orbit-shooter/game.jsdesktop/public/gamestudio-seeds/orbit-shooter/index.htmldesktop/public/gamestudio-seeds/orbit-shooter/three.module.jsdesktop/public/gamestudio-seeds/platformer-lite/game.jsdesktop/public/gamestudio-seeds/platformer-lite/index.htmldesktop/public/gamestudio-seeds/platformer-lite/three.module.jsdesktop/public/gamestudio-seeds/top-down-collector/game.jsdesktop/public/gamestudio-seeds/top-down-collector/index.htmldesktop/src/apps/GameStudioApp.test.tsxdesktop/src/apps/GameStudioApp.tsxdesktop/src/apps/ProvidersApp.test.tsxdesktop/src/apps/ProvidersApp.tsxdesktop/src/apps/gamestudio/CreateView.tsxdesktop/src/apps/gamestudio/EditorView.test.tsxdesktop/src/apps/gamestudio/EditorView.tsxdesktop/src/apps/gamestudio/FileEditor.tsxdesktop/src/apps/gamestudio/LibraryView.test.tsxdesktop/src/apps/gamestudio/LibraryView.tsxdesktop/src/apps/gamestudio/PlayView.tsxdesktop/src/apps/gamestudio/ShareView.test.tsxdesktop/src/apps/gamestudio/ShareView.tsxdesktop/src/apps/gamestudio/create-game.tsdesktop/src/apps/gamestudio/file-blocks.test.tsdesktop/src/apps/gamestudio/file-blocks.tsdesktop/src/apps/gamestudio/game-authoring-prompt.tsdesktop/src/apps/gamestudio/game-state.tsdesktop/src/apps/gamestudio/games-api.tsdesktop/src/apps/gamestudio/generate-game.test.tsdesktop/src/apps/gamestudio/generate-game.tsdesktop/src/apps/gamestudio/match-template.tsdesktop/src/apps/gamestudio/templates.tsdesktop/src/apps/gamestudio/types.tsdesktop/src/apps/gamestudio/useGameScene.tsdesktop/src/lib/userspace-apps.tstests/test_routes_games.pytests/test_routes_providers.pytinyagentos/routes/games.pytinyagentos/routes/providers.pytinyagentos/userspace/package.py
💤 Files with no reviewable changes (4)
- desktop/src/apps/gamestudio/PlayView.tsx
- desktop/src/apps/gamestudio/game-state.ts
- desktop/src/apps/gamestudio/create-game.ts
- desktop/src/apps/gamestudio/useGameScene.ts
| function resizeCanvas() { | ||
| canvas.width = window.innerWidth; | ||
| canvas.height = window.innerHeight; | ||
| layoutBricks(); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Window resize wipes brick-break progress (score keeps counting).
resizeCanvas calls layoutBricks() on every resize event, and layoutBricks unconditionally rebuilds every brick with alive: true. Resizing the browser window (or the preview pane containing this canvas) mid-game respawns all previously-broken bricks while score/lives are untouched — the player can keep re-breaking the same bricks for unbounded score, and the "clear all bricks to win" condition becomes effectively unreachable if the panel is resized at all during play.
🐛 Proposed fix: preserve alive-state across relayout
function layoutBricks() {
+ const previousAlive =
+ bricks.length === ROWS * COLS ? bricks.map((b) => b.alive) : null;
bricks = [];
const totalGap = BRICK_GAP * (COLS - 1);
const brickWidth = (canvas.width - BRICK_SIDE_MARGIN * 2 - totalGap) / COLS;
+ let index = 0;
for (let row = 0; row < ROWS; row++) {
for (let col = 0; col < COLS; col++) {
bricks.push({
x: BRICK_SIDE_MARGIN + col * (brickWidth + BRICK_GAP),
y: BRICK_TOP_MARGIN + row * (BRICK_HEIGHT + BRICK_GAP),
w: brickWidth,
h: BRICK_HEIGHT,
color: ROW_COLORS[row % ROW_COLORS.length],
- alive: true,
+ alive: previousAlive ? previousAlive[index] : true,
});
+ index += 1;
}
}
}Also applies to: 39-55
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@desktop/public/gamestudio-seeds/breakout/game.js` around lines 6 - 10,
Preserve brick progress when resizing: resizeCanvas currently triggers
layoutBricks, and layoutBricks rebuilds all bricks as alive, which resets
destroyed bricks while score/lives remain unchanged. Update layoutBricks to
retain each brick’s existing alive state (for example by reusing the prior
bricks array/state when recalculating positions) and ensure resizeCanvas only
recomputes geometry via resizeCanvas and layoutBricks without respawning
already-broken bricks.
| const handleApply = useCallback(() => { | ||
| if (!pendingEdit) return; | ||
| setApplying(true); | ||
| setFiles((prev) => { | ||
| const next = { ...prev }; | ||
| for (const [path, f] of Object.entries(pendingEdit.files)) { | ||
| next[path] = f.content; | ||
| } | ||
| void flushSave(next); | ||
| return next; | ||
| }); | ||
| setPendingEdit(null); | ||
| setApplying(false); | ||
| }, [pendingEdit, flushSave]); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="desktop/src/apps/gamestudio/EditorView.tsx"
echo "== file size =="
wc -l "$FILE"
echo
echo "== AST outline =="
ast-grep outline "$FILE" --view expanded || true
echo
echo "== relevant symbols =="
rg -n "handleApply|handleManualRefresh|scheduleSave|flushSave|saveTimerRef|applying|filesRef|mountedRef|setApplying|setFiles|pendingEdit" "$FILE"
echo
echo "== surrounding lines around relevant functions =="
sed -n '120,320p' "$FILE"Repository: jaylfc/taOS
Length of output: 9023
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="desktop/src/apps/gamestudio/EditorView.tsx"
echo "== flushSave section =="
sed -n '108,160p' "$FILE"
echo
echo "== handleApply and nearby render state =="
sed -n '248,470p' "$FILE"Repository: jaylfc/taOS
Length of output: 10868
handleApply needs to clear the pending debounce and keep side effects out of setFiles.
- It can race with an existing
saveTimerRefand later save the pre-AI file set over the applied edit; clear the timer here the same wayhandleManualRefreshdoes. flushSave(next)runs inside thesetFilesupdater, which makes the updater impure and can duplicate the network request under Strict Mode.setApplying(false)runs immediately, sodisabled={applying}never covers the async save; reset it afterflushSavesettles.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@desktop/src/apps/gamestudio/EditorView.tsx` around lines 251 - 264,
handleApply currently leaves the existing saveTimerRef active and performs side
effects inside the setFiles updater, which can race with a later flush and
duplicate the save request. In handleApply, clear any pending debounce timer the
same way handleManualRefresh does before applying pendingEdit.files, then update
state without calling flushSave from inside setFiles. Move flushSave(next) out
of the updater so the state change stays pure, and only call setApplying(false)
after the async flushSave completes so the applying flag covers the full save
lifecycle.
| export async function renameGame(id: string, name: string): Promise<GameRecord> { | ||
| const current = await getGame(id); | ||
| return saveGame(id, { name, prompt: current.prompt, template: current.template, files: current.files }); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect relevant files and surrounding context
git ls-files 'desktop/src/apps/gamestudio/games-api.ts' 'desktop/src/apps/gamestudio/**/*' 'desktop/src/**/games.py' 'desktop/src/**/GameStudioApp*' 'desktop/src/**/EditorView*' | sed 's#^`#FILE` #'
printf '\n--- games-api.ts ---\n'
cat -n desktop/src/apps/gamestudio/games-api.ts | sed -n '1,220p'
printf '\n--- search for PUT /api/games semantics and rename usages ---\n'
rg -n "PUT /api/games|saveGame\(|renameGame\(|windowId|GameStudioApp|autosave|deboun" desktop/src -S
printf '\n--- backend game API file(s) ---\n'
fd -a 'games.py|game.*py|games-api.*|games.*ts' desktop/src | sed -n '1,200p'Repository: jaylfc/taOS
Length of output: 50367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map likely relevant files before reading targeted slices.
fd -a 'games-api.ts|games.py|GameStudioApp*|EditorView*' desktop/src | sed -n '1,200p'
printf '\n--- ast outline games-api.ts ---\n'
ast-grep outline desktop/src/apps/gamestudio/games-api.ts --view expanded
printf '\n--- ast outline likely app entry ---\n'
for f in $(fd -a 'GameStudioApp*' desktop/src); do
echo "FILE $f"
ast-grep outline "$f" --view expanded | sed -n '1,220p'
done
printf '\n--- ast outline backend games.py if found ---\n'
for f in $(fd -a 'games.py' desktop/src); do
echo "FILE $f"
ast-grep outline "$f" --view expanded | sed -n '1,260p'
doneRepository: jaylfc/taOS
Length of output: 1962
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read precise slices around the relevant functions.
printf '\n--- games-api.ts (1-220) ---\n'
cat -n desktop/src/apps/gamestudio/games-api.ts | sed -n '1,220p'
printf '\n--- search for editor autosave and multi-window clues ---\n'
rg -n "windowId|autosave|debounce|saveGame\(|renameGame\(|getGame\(|createFromTemplate|generateGame|GameStudioApp" desktop/src/apps/gamestudio desktop/src -S
printf '\n--- backend file(s) named games.py under desktop/src ---\n'
fd -a 'games.py' desktop/src | sed -n '1,200p'Repository: jaylfc/taOS
Length of output: 50367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Minimal targeted evidence collection.
printf 'FILES\n'
git ls-files 'desktop/src/apps/gamestudio/games-api.ts' 'desktop/src/apps/gamestudio/*' 'desktop/src/**/games.py' | sed -n '1,200p'
printf '\nGAMES API\n'
cat -n desktop/src/apps/gamestudio/games-api.ts | sed -n '1,220p'
printf '\nSEARCH\n'
rg -n "renameGame\(|saveGame\(|windowId|autosave|debounce|GameStudioApp" desktop/src/apps/gamestudio desktop/src -SRepository: jaylfc/taOS
Length of output: 50367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- GameStudioApp.tsx ---\n'
cat -n desktop/src/apps/GameStudioApp.tsx | sed -n '1,220p'
printf '\n--- LibraryView.tsx ---\n'
cat -n desktop/src/apps/gamestudio/LibraryView.tsx | sed -n '1,220p'
printf '\n--- EditorView.tsx ---\n'
cat -n desktop/src/apps/gamestudio/EditorView.tsx | sed -n '1,260p'
printf '\n--- GameStudioApp.test.tsx (relevant slices) ---\n'
cat -n desktop/src/apps/GameStudioApp.test.tsx | sed -n '1,220p'Repository: jaylfc/taOS
Length of output: 31911
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify whether rename and autosave can overlap within the app flow.
python3 - <<'PY'
from pathlib import Path
files = [
Path("desktop/src/apps/GameStudioApp.tsx"),
Path("desktop/src/apps/gamestudio/LibraryView.tsx"),
Path("desktop/src/apps/gamestudio/EditorView.tsx"),
Path("desktop/src/apps/GameStudioApp.test.tsx"),
]
for p in files:
print(f"\n--- {p} ---")
text = p.read_text()
for i, line in enumerate(text.splitlines(), 1):
print(f"{i:4d}: {line}")
PYRepository: jaylfc/taOS
Length of output: 42865
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- GameStudioApp.tsx (1-180) ---'
sed -n '1,180p' desktop/src/apps/GameStudioApp.tsx | nl -ba
echo '--- LibraryView.tsx (1-180) ---'
sed -n '1,180p' desktop/src/apps/gamestudio/LibraryView.tsx | nl -ba
echo '--- EditorView.tsx (1-220) ---'
sed -n '1,220p' desktop/src/apps/gamestudio/EditorView.tsx | nl -ba
echo '--- GameStudioApp.test.tsx (1-220) ---'
sed -n '1,220p' desktop/src/apps/GameStudioApp.test.tsx | nl -baRepository: jaylfc/taOS
Length of output: 223
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- GameStudioApp.tsx (1-180) ---'
sed -n '1,180p' desktop/src/apps/GameStudioApp.tsx | nl -ba | sed -n '1,180p'
echo '--- LibraryView.tsx (1-180) ---'
sed -n '1,180p' desktop/src/apps/gamestudio/LibraryView.tsx | nl -ba | sed -n '1,180p'
echo '--- EditorView.tsx (1-220) ---'
sed -n '1,220p' desktop/src/apps/gamestudio/EditorView.tsx | nl -ba | sed -n '1,220p'
echo '--- GameStudioApp.test.tsx (1-220) ---'
sed -n '1,220p' desktop/src/apps/GameStudioApp.test.tsx | nl -ba | sed -n '1,220p'Repository: jaylfc/taOS
Length of output: 223
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for local documentation of PUT semantics and game record shape.
rg -n "authoritative|full-replace|PUT /api/games|saveGame|GameRecord|files:" desktop/src/apps/gamestudio desktop/src -S
echo '--- types.ts ---'
sed -n '1,220p' desktop/src/apps/gamestudio/types.ts | nl -ba
echo '--- generate-game.ts (1-140) ---'
sed -n '1,140p' desktop/src/apps/gamestudio/generate-game.ts | nl -baRepository: jaylfc/taOS
Length of output: 12327
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- GameStudioApp.tsx (1-120) ---'
sed -n '1,120p' desktop/src/apps/GameStudioApp.tsx | nl -ba
echo '--- LibraryView.tsx (1-120) ---'
sed -n '1,120p' desktop/src/apps/gamestudio/LibraryView.tsx | nl -ba
echo '--- EditorView.tsx (1-220) ---'
sed -n '1,220p' desktop/src/apps/gamestudio/EditorView.tsx | nl -baRepository: jaylfc/taOS
Length of output: 223
renameGame can clobber in-flight file saves (desktop/src/apps/gamestudio/games-api.ts:45-47)
It fetches the full game record and PUTs back the fetched files snapshot. If an editor save is already in flight, a rename can overwrite newer file edits with stale data. A rename-only endpoint or optimistic concurrency check would avoid the read-modify-write race.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@desktop/src/apps/gamestudio/games-api.ts` around lines 45 - 48, The
renameGame flow is doing a read-modify-write on the full GameRecord and
re-sending the fetched files snapshot, which can overwrite newer in-flight
edits. Update renameGame to avoid touching files when only the name changes,
ideally by using a rename-only API or adding an optimistic concurrency/version
check in getGame/saveGame so the current prompt/template/files state is not
clobbered.
| export function LibraryView({ onOpenGame }: LibraryViewProps) { | ||
| const [games, setGames] = useState<GameMeta[]>([]); | ||
| const [loading, setLoading] = useState(true); | ||
| const [error, setError] = useState<string | null>(null); | ||
| const [busyId, setBusyId] = useState<string | null>(null); | ||
|
|
||
| const load = useCallback(async () => { | ||
| setLoading(true); | ||
| setError(null); | ||
| try { | ||
| setGames(await listGames()); | ||
| } catch (e) { | ||
| setError(e instanceof Error ? e.message : String(e)); | ||
| } finally { | ||
| setLoading(false); | ||
| } | ||
| }, []); | ||
|
|
||
| useEffect(() => { | ||
| void load(); | ||
| }, [load]); | ||
|
|
||
| const handleRename = useCallback( | ||
| async (game: GameMeta) => { | ||
| const next = window.prompt("Rename game:", game.name); | ||
| if (!next?.trim() || next.trim() === game.name) return; | ||
| setBusyId(game.id); | ||
| try { | ||
| await renameGame(game.id, next.trim()); | ||
| await load(); | ||
| } catch (e) { | ||
| setError(e instanceof Error ? e.message : String(e)); | ||
| } finally { | ||
| setBusyId(null); | ||
| } | ||
| }, | ||
| [load], | ||
| ); | ||
|
|
||
| const handleDelete = useCallback( | ||
| async (game: GameMeta) => { | ||
| const ok = window.confirm(`Delete "${game.name}"? This can't be undone.`); | ||
| if (!ok) return; | ||
| setBusyId(game.id); | ||
| try { | ||
| await deleteGame(game.id); | ||
| // Backend-confirmed: only refresh the list (from the server) after | ||
| // the delete call has actually succeeded -- never splice the row | ||
| // out of local state optimistically. | ||
| await load(); | ||
| } catch (e) { | ||
| setError(e instanceof Error ? e.message : String(e)); | ||
| } finally { | ||
| setBusyId(null); | ||
| } | ||
| }, | ||
| [load], | ||
| ); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Guard state updates after unmount.
load, handleRename, and handleDelete all update state after awaiting network calls with no unmount guard, unlike ShareView's cancelled pattern in this same PR. If the user navigates away from Library mid-request, these calls to setGames/setError/setBusyId fire on an unmounted component.
🛡️ Proposed fix (mounted-ref guard)
+import { useCallback, useEffect, useRef, useState } from "react";
-import { useCallback, useEffect, useState } from "react";
...
export function LibraryView({ onOpenGame }: LibraryViewProps) {
const [games, setGames] = useState<GameMeta[]>([]);
const [loading, setLoading] = useState(true);
const [error, setError] = useState<string | null>(null);
const [busyId, setBusyId] = useState<string | null>(null);
+ const mountedRef = useRef(true);
+ useEffect(() => {
+ mountedRef.current = true;
+ return () => {
+ mountedRef.current = false;
+ };
+ }, []);
const load = useCallback(async () => {
setLoading(true);
setError(null);
try {
- setGames(await listGames());
+ const games = await listGames();
+ if (mountedRef.current) setGames(games);
} catch (e) {
- setError(e instanceof Error ? e.message : String(e));
+ if (mountedRef.current) setError(e instanceof Error ? e.message : String(e));
} finally {
- setLoading(false);
+ if (mountedRef.current) setLoading(false);
}
}, []);Apply the same guard to handleRename/handleDelete's catch/finally blocks.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@desktop/src/apps/gamestudio/LibraryView.tsx` around lines 30 - 87, The async
handlers in LibraryView update React state after awaiting requests without
checking whether the component is still mounted, so add the same
mounted-ref/cancelled guard used in ShareView. Update LibraryView’s load,
handleRename, and handleDelete to skip setGames, setError, and setBusyId in
catch/finally when unmounted, using a shared ref or cleanup flag tied to the
component lifecycle.
1 participant
Summary
Rebuilds Game Studio from scaffolding (fake
delay()-driven build steps, a static template picker, a stub Share view, no persistence) into a real AI-assisted game maker.tinyagentos/routes/games.py, extends the existing chess-move file):GET/PUT/DELETE /api/games[/{id}], a sandboxed preview endpoint (GET /api/games/{id}/preview/{path}) with a strictsandbox allow-scripts(noallow-same-origin) CSP, and a package endpoint (GET /api/games/{id}/package) that builds a real.taosappzip via a newbuild_package()helper intinyagentos/userspace/package.py(the inverse of the existingextract_package()). Storage mirrorsroutes/video.py:data/games/{id}/with agame.jsonsidecar, flat-filename traversal guards, size/count caps./api/games/chess/*is untouched.desktop/public/gamestudio-seeds/: a 3D platformer and orbit shooter on three.js (bundling a relativethree.module.js, never a CDN), and a 2D top-down collector and breakout on canvas. Each is genuinely playable standalone.stream-chat.ts), asking it to return a complete customized file set using a file-block convention (### FILE: <name>+ a fenced code block per file — seedesktop/src/apps/gamestudio/file-blocks.ts). On a parse failure the unmodified seed is saved and the user is told so — never a fake "customized" result. Vendor files (three.module.js) are excluded from what's sent to the agent and can never be overwritten by its response.sandbox="allow-scripts", noallow-same-origin) with manual refresh + auto-refresh on save, and an AI chat sidebar using the same file-block convention — proposed changes show a per-file before/after line-count summary and require an explicit Apply. Play maximizes the preview to fullscreen with the same mandatory Exit-to-taOS + Escape handling the old PlayView had..taosappto the existing, unmodified/api/userspace-apps/installendpoint (the same pipelineImportAppButtonuses) taggedprovenance: "ai-generated"; the static security analyzer runs on install like any other userspace app, and findings are previewed/shown honestly via App Studio's existinganalyze-source.ts/FindingsPanel, reused as-is. "Export .taosapp" downloads the identical package. No public-Store submission in v1 — that goes through the maintainer review flow.AI file-set convention