From 453cf15ba969d8815e4346fabf03801ecb56f1bb Mon Sep 17 00:00:00 2001 From: pythonlearner1025 Date: Wed, 16 Sep 2026 17:26:21 -0700 Subject: [PATCH 1/2] fix(editor): Play runs the scene tab that is open The owner had assets/weapons-lab.scene.gltf open in a tab, pressed Play, and the main scene ran. Pass 1a pinned Play to the main scene by the plan's own decision. A scene on screen is the scene you expect to play, so the rule is now the active document. - startRunMode takes store.active when it is a SceneDocument, else store.mainScene. - beforeRun names the scene the run borrowed, and stopRunMode restores that one. - The plan's 4.4 Play paragraph records the new rule and why. Proved headless on a copy of the terminator project: the weapons lab tab now runs the weapons lab, the main scene tab and an object or texture tab run the main scene, and Stop puts the scene that ran back with its name, its saved hash and its unsaved edit. typecheck, lint, kite3d (9 tests) and test:scripts pass. --- docs/documents-and-viewport-plan.md | 2 +- .../editor/src/documents/DocumentStore.ts | 2 +- .../editor/src/documents/EditorDocument.ts | 4 +- packages/editor/src/utils/PlayModeHelper.ts | 36 +++++----- packages/kite3d/test/editor-open-tab.test.ts | 67 +++++++++++++++++-- 5 files changed, 83 insertions(+), 28 deletions(-) diff --git a/docs/documents-and-viewport-plan.md b/docs/documents-and-viewport-plan.md index 9c0bf1d6..c48127e3 100644 --- a/docs/documents-and-viewport-plan.md +++ b/docs/documents-and-viewport-plan.md @@ -214,7 +214,7 @@ External change: the session's listener (`onProjectEvent`, `:418-437`) keeps its A 412 on save keeps today's three answers (cancel, reload, overwrite, `writeResolvingConflict`, `:1639-1661`), and "reload" applies to the document being saved, not to "the open file". -Play: `startRunMode` already refuses anything but a scene (`PlayModeHelper.ts:34`). Play from any tab activates the main scene document first, then runs as today: the in-memory snapshot (`exportRunningScene`, `:789-801`), `startGame` on the viewport (`PlayModeHelper.ts:120`), pause, inspect, stop and restore (`:146-197`). While Play runs, the tab strip is disabled; switching would detach the running scene. Decision 5. +Play: `startRunMode` runs the scene tab that is open, and the main scene from a tab that holds no scene of its own. Pass 1a pinned Play to the main scene and the owner reported it as a bug: a scene on screen is the scene you expect to play. The rest is unchanged: the document goes on the viewport, then the in-memory snapshot (`exportRunningScene`, `:789-801`), `startGame` on the viewport (`PlayModeHelper.ts:120`), pause, inspect, stop and restore (`:146-197`). `startGame` never reads the `mainScene` setting; only the published runtime's `createGame` does, to load its own scene. Stop puts back the scene that ran, which the `beforeRun` record names. While Play runs, the tab strip is disabled; switching would detach the running scene. Decision 5. Screenshot: `captureScreenshot` (`:695`) captures the viewport, which shows the active document; `kite3d screenshot` still finds one canvas (`packages/kite3d/src/screenshot.ts:92`). diff --git a/packages/editor/src/documents/DocumentStore.ts b/packages/editor/src/documents/DocumentStore.ts index 41dca7f6..70cd19a4 100644 --- a/packages/editor/src/documents/DocumentStore.ts +++ b/packages/editor/src/documents/DocumentStore.ts @@ -38,7 +38,7 @@ export class DocumentStore extends EventDispatcher<{change: object}> { return this.documents.find(d => d.path === this.activeId) } - /** The main scene, which Play, the screenshot and the settings reload all act on. */ + /** The main scene document, which Play runs when the tab on screen holds no scene of its own. */ get mainScene(): SceneDocument | undefined { const doc = this.documents.find(d => d.path === this.mainScenePath) return doc instanceof SceneDocument ? doc : undefined diff --git a/packages/editor/src/documents/EditorDocument.ts b/packages/editor/src/documents/EditorDocument.ts index 7ec54614..8a41d5fc 100644 --- a/packages/editor/src/documents/EditorDocument.ts +++ b/packages/editor/src/documents/EditorDocument.ts @@ -72,8 +72,8 @@ export abstract class EditorDocument extends EventDispatcher<{change: object}> { private _dirty = false /** - * Play borrows the main scene, reloads a snapshot into it and puts the scene back at Stop. Those - * loads are not the user's edits, so nothing is dirty while a game runs. + * Play borrows the scene it runs, reloads a snapshot into it and puts the scene back at Stop. + * Those loads are not the user's edits, so nothing is dirty while a game runs. */ get dirty() { if (this.session.playMode.isRunningMode) return false diff --git a/packages/editor/src/utils/PlayModeHelper.ts b/packages/editor/src/utils/PlayModeHelper.ts index 6da946dd..71a93902 100644 --- a/packages/editor/src/utils/PlayModeHelper.ts +++ b/packages/editor/src/utils/PlayModeHelper.ts @@ -4,6 +4,7 @@ import {settingsKey} from "./project.ts"; import {ViewerInstanceManager} from "./ViewerInstanceManager.ts"; import {isPackageProject} from "./projectUtils.ts"; import {EditModePlugin} from "./EditModePlugin.ts"; +import {SceneDocument} from "../documents/SceneDocument.ts"; export class PlayModeHelper extends EventDispatcher<{ runModePauseChange: {}, @@ -19,8 +20,8 @@ export class PlayModeHelper extends EventDispatcher<{ // The run on the edit viewer: the project's scripts, plugins, clock, components, physics and main(). private running: RunningGame | null = null - // Play borrows the main scene and gives it back at Stop: the dirty flag and the name the file carries. - private beforeRun: {dirty: boolean, savedHash: string | null, sceneName: string | null} | null = null + // The scene this run borrowed, and what it looked like before. Stop gives both back. + private beforeRun: {scene: SceneDocument, dirty: boolean, savedHash: string | null, sceneName: string | null} | null = null constructor(private manager: ViewerInstanceManager) { super() @@ -32,8 +33,11 @@ export class PlayModeHelper extends EventDispatcher<{ // save current scene to running.glb // load running.glb in play mode const store = manager.store - const scene = store?.mainScene - if (!store || !scene) return false + if (!store) return false + // Play runs the scene tab that is open. An object, material or texture tab holds no scene, + // so the main scene runs from those. + const scene = store.active instanceof SceneDocument ? store.active : store.mainScene + if (!scene) return false if (manager.savingScene) return false if (this.isRunningMode) { @@ -48,9 +52,9 @@ export class PlayModeHelper extends EventDispatcher<{ const isPackage = isPackageProject(project) if (!project || (isPackage && !project.handle)) return false - // Play runs the main scene, whatever tab was showing. Switching while it runs is refused. + // A scene runs on the viewport, so it goes there first. Switching while it runs is refused. await store.activateForPlay(scene.path) - this.beforeRun = {dirty: scene.dirty, savedHash: scene.savedHash, sceneName: scene.sceneName} + this.beforeRun = {scene, dirty: scene.dirty, savedHash: scene.savedHash, sceneName: scene.sceneName} // Play runs the scene as it is, not as an isolated view shows it. manager.get().getPlugin(EditModePlugin)?.exitIsolate() @@ -157,8 +161,9 @@ export class PlayModeHelper extends EventDispatcher<{ if (!project || (isPackage && !project.handle)) return false const store = manager.store - const scene = store?.mainScene - if (!store || !scene) return + const before = this.beforeRun + if (!store || !before) return + const scene = before.scene const v = manager.get() const picking = v.getPlugin(PickingPlugin) @@ -192,15 +197,12 @@ export class PlayModeHelper extends EventDispatcher<{ } } - if (this.beforeRun) { - scene.savedHash = this.beforeRun.savedHash - scene.dirty = this.beforeRun.dirty - // The run's snapshot is threepipe's raw export, and it names the model root 'Scene'. - // Importing that snapshot back would write its name over the one the scene file carries. - scene.sceneName = this.beforeRun.sceneName - this.beforeRun = null - } - + scene.savedHash = before.savedHash + scene.dirty = before.dirty + // The run's snapshot is threepipe's raw export, and it names the model root 'Scene'. + // Importing that snapshot back would write its name over the one the scene file carries. + scene.sceneName = before.sceneName + this.beforeRun = null } } diff --git a/packages/kite3d/test/editor-open-tab.test.ts b/packages/kite3d/test/editor-open-tab.test.ts index 1858d1d0..219f3034 100644 --- a/packages/kite3d/test/editor-open-tab.test.ts +++ b/packages/kite3d/test/editor-open-tab.test.ts @@ -43,6 +43,39 @@ it('leaves the viewer canvas mounted in the tab an open adds', async (context) = expect(await viewportOnScreen(page)).toEqual({mounted: true, drawn: true}) }) +// Guards the owner's report: "i have assets/weapons-lab.scene.gltf open in a tab and press Play, +// and the main scene runs". Play runs the scene tab that is on screen. +it('runs the scene tab that is open, not the main scene', async (context) => { + const chromium = await browserLauncher() + if (!chromium) { + context.skip('Playwright is not installed. Run npx playwright install chromium.') + return + } + const root = await temporaryProject() + const server = await createDevServer({projectRoot: root, port: 0}) + cleanup.push(() => server.close()) + const browser = await chromium.launch({headless: true}) + cleanup.push(() => browser.close()) + + const page = await browser.newPage({viewport: {width: 1280, height: 800}}) + await page.goto(server.url, {waitUntil: 'domcontentloaded', timeout: 60_000}) + await page.waitForFunction('window.kite3dProjectLoaded === true', undefined, {timeout: 60_000}) + await page.waitForSelector('.editorCanvasContainer canvas', {timeout: 30_000}) + + await page.dblclick('button.file-item-button[title="second.scene.gltf"]', {timeout: 30_000}) + await page.waitForSelector('[role="tab"][aria-selected="true"] .document-tab-name:text-is("second.scene.gltf")', { + timeout: 60_000, + }) + + // Pause turns on once the whole run has started, snapshot reload included. + await page.click('[aria-label="Run"]') + await page.waitForSelector('[aria-label="Pause"]:not([disabled])', {timeout: 60_000}) + + const running = await sceneNodeNames(page) as string[] + expect(running).toContain('SecondSceneTriangle') + expect(running).not.toContain('MainSceneTriangle') +}) + /** * Whether the viewer's canvas is in the container the page shows, and has a size. A query finds * mounted elements alone, so a canvas left behind in the panel that closed answers false. @@ -55,6 +88,15 @@ function viewportOnScreen(page: {evaluate: (script: string) => Promise} })()`) } +/** Every named node under the model root: the scene the viewport is showing. */ +function sceneNodeNames(page: {evaluate: (script: string) => Promise}): Promise { + return page.evaluate(`(() => { + const names = [] + window.viewer.scene.modelRoot.traverse((o) => { if (o.name) names.push(o.name) }) + return names + })()`) +} + async function browserLauncher() { try { const {chromium} = await import('playwright') @@ -78,20 +120,31 @@ async function temporaryProject(): Promise { // material document fetches the preview environment before its first show settles. await writeFile(resolve(root, 'pixel.png'), Buffer.from(ONE_PIXEL_PNG, 'base64')) await mkdir(resolve(root, 'assets'), {recursive: true}) - await writeFile(resolve(root, 'assets/main.scene.gltf'), `${JSON.stringify({ + await writeFile(resolve(root, 'assets/main.scene.gltf'), sceneGltf('MainSceneTriangle')) + // A second scene, so a Play can start from a tab that is not the main scene. It sits at the + // project root because the Files panel opens there and the folders need a navigation click. + await writeFile(resolve(root, 'second.scene.gltf'), sceneGltf('SecondSceneTriangle')) + await mkdir(resolve(root, 'node_modules/@kite3d/engine/dist'), {recursive: true}) + await writeFile(resolve(root, 'node_modules/@kite3d/engine/package.json'), JSON.stringify({version: KITE3D_VERSION})) + await writeFile(resolve(root, 'node_modules/@kite3d/engine/dist/runtime.js'), 'installed runtime') + return root +} + +/** + * One triangle under a node the test can name, so the tree says which scene is on the viewport. + * The name carries no space: the run's snapshot is glTF, and glTF writes a space as an underscore. + */ +function sceneGltf(nodeName: string): string { + return `${JSON.stringify({ asset: {version: '2.0'}, scene: 0, scenes: [{nodes: [0]}], - nodes: [{name: 'Authored triangle', mesh: 0}], + nodes: [{name: nodeName, mesh: 0}], meshes: [{primitives: [{attributes: {POSITION: 0}}]}], accessors: [{bufferView: 0, componentType: 5126, count: 3, type: 'VEC3', min: [-1, -1, 0], max: [1, 1, 0]}], bufferViews: [{buffer: 0, byteOffset: 0, byteLength: 36, target: 34962}], buffers: [{byteLength: 36, uri: 'data:application/octet-stream;base64,AAAAAAAAgD8AAAAAAAAAAAAAAIA/AAAAAAAAAAAAAAAAAACAPwAAAAA='}], - })}\n`) - await mkdir(resolve(root, 'node_modules/@kite3d/engine/dist'), {recursive: true}) - await writeFile(resolve(root, 'node_modules/@kite3d/engine/package.json'), JSON.stringify({version: KITE3D_VERSION})) - await writeFile(resolve(root, 'node_modules/@kite3d/engine/dist/runtime.js'), 'installed runtime') - return root + })}\n` } const ONE_PIXEL_PNG = 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==' From 6f3ede313b64860f7672bc37ec26d34781bfecbd Mon Sep 17 00:00:00 2001 From: pythonlearner1025 Date: Wed, 16 Sep 2026 17:43:28 -0700 Subject: [PATCH 2/2] fix(editor): Stop returns to the tab Play was pressed on Play from an object, material or texture tab has to switch to the main scene to run it. Stop left the editor on that scene. The tab the user pressed Play on is the tab they expect back. - beforeRun records store.activeId before the activate that Play does. - stopRunMode activates it again once the scene is restored. - activateForPlay already no-ops on a tab that is gone or already on screen, so a scene tab stays where it is and a closed tab leaves the scene on screen. Proved headless on a copy of the terminator project: from an object tab and from a texture tab the main scene runs and Stop brings the tab back with its own tree on the viewport; from the weapons lab tab and from the main scene tab Stop stays put. One new test guards it. typecheck, lint, kite3d (10 tests) and test:scripts pass. --- docs/documents-and-viewport-plan.md | 2 +- .../editor/src/documents/DocumentStore.ts | 2 +- packages/editor/src/utils/PlayModeHelper.ts | 17 ++++++-- packages/kite3d/test/editor-open-tab.test.ts | 39 +++++++++++++++++++ 4 files changed, 55 insertions(+), 5 deletions(-) diff --git a/docs/documents-and-viewport-plan.md b/docs/documents-and-viewport-plan.md index c48127e3..8339fd94 100644 --- a/docs/documents-and-viewport-plan.md +++ b/docs/documents-and-viewport-plan.md @@ -214,7 +214,7 @@ External change: the session's listener (`onProjectEvent`, `:418-437`) keeps its A 412 on save keeps today's three answers (cancel, reload, overwrite, `writeResolvingConflict`, `:1639-1661`), and "reload" applies to the document being saved, not to "the open file". -Play: `startRunMode` runs the scene tab that is open, and the main scene from a tab that holds no scene of its own. Pass 1a pinned Play to the main scene and the owner reported it as a bug: a scene on screen is the scene you expect to play. The rest is unchanged: the document goes on the viewport, then the in-memory snapshot (`exportRunningScene`, `:789-801`), `startGame` on the viewport (`PlayModeHelper.ts:120`), pause, inspect, stop and restore (`:146-197`). `startGame` never reads the `mainScene` setting; only the published runtime's `createGame` does, to load its own scene. Stop puts back the scene that ran, which the `beforeRun` record names. While Play runs, the tab strip is disabled; switching would detach the running scene. Decision 5. +Play: `startRunMode` runs the scene tab that is open, and the main scene from a tab that holds no scene of its own. Pass 1a pinned Play to the main scene and the owner reported it as a bug: a scene on screen is the scene you expect to play. The rest is unchanged: the document goes on the viewport, then the in-memory snapshot (`exportRunningScene`, `:789-801`), `startGame` on the viewport (`PlayModeHelper.ts:120`), pause, inspect, stop and restore (`:146-197`). `startGame` never reads the `mainScene` setting; only the published runtime's `createGame` does, to load its own scene. Stop puts back the scene that ran and then the tab Play was pressed on, both named by the `beforeRun` record; a tab closed meanwhile leaves the scene on screen. While Play runs, the tab strip is disabled; switching would detach the running scene. Decision 5. Screenshot: `captureScreenshot` (`:695`) captures the viewport, which shows the active document; `kite3d screenshot` still finds one canvas (`packages/kite3d/src/screenshot.ts:92`). diff --git a/packages/editor/src/documents/DocumentStore.ts b/packages/editor/src/documents/DocumentStore.ts index 70cd19a4..62686159 100644 --- a/packages/editor/src/documents/DocumentStore.ts +++ b/packages/editor/src/documents/DocumentStore.ts @@ -96,7 +96,7 @@ export class DocumentStore extends EventDispatcher<{change: object}> { await this.activateForPlay(path) } - /** Activates whatever the caller names, Play included. */ + /** Activates whatever the caller names, past the gate above. Play and Stop come through here. */ async activateForPlay(path: string) { const doc = this.find(path) if (!doc || this.activeId === path) return diff --git a/packages/editor/src/utils/PlayModeHelper.ts b/packages/editor/src/utils/PlayModeHelper.ts index 71a93902..7302ed68 100644 --- a/packages/editor/src/utils/PlayModeHelper.ts +++ b/packages/editor/src/utils/PlayModeHelper.ts @@ -20,8 +20,12 @@ export class PlayModeHelper extends EventDispatcher<{ // The run on the edit viewer: the project's scripts, plugins, clock, components, physics and main(). private running: RunningGame | null = null - // The scene this run borrowed, and what it looked like before. Stop gives both back. - private beforeRun: {scene: SceneDocument, dirty: boolean, savedHash: string | null, sceneName: string | null} | null = null + // The scene this run borrowed, what it looked like before, and the tab Play was pressed on. + // Stop gives all three back. + private beforeRun: { + scene: SceneDocument, activeId: string | null, + dirty: boolean, savedHash: string | null, sceneName: string | null, + } | null = null constructor(private manager: ViewerInstanceManager) { super() @@ -52,9 +56,12 @@ export class PlayModeHelper extends EventDispatcher<{ const isPackage = isPackageProject(project) if (!project || (isPackage && !project.handle)) return false + // The tab Play was pressed on. The activate below overwrites it, and a cold scene reads its + // file in that same activate, so the rest of the record is taken after it. + const activeId = store.activeId // A scene runs on the viewport, so it goes there first. Switching while it runs is refused. await store.activateForPlay(scene.path) - this.beforeRun = {scene, dirty: scene.dirty, savedHash: scene.savedHash, sceneName: scene.sceneName} + this.beforeRun = {scene, activeId, dirty: scene.dirty, savedHash: scene.savedHash, sceneName: scene.sceneName} // Play runs the scene as it is, not as an isolated view shows it. manager.get().getPlugin(EditModePlugin)?.exitIsolate() @@ -203,6 +210,10 @@ export class PlayModeHelper extends EventDispatcher<{ // Importing that snapshot back would write its name over the one the scene file carries. scene.sceneName = before.sceneName this.beforeRun = null + + // The tab Play was pressed on comes back. activateForPlay does nothing when that tab is the + // scene that just ran, or when it is no longer open, and the scene stays on the viewport. + if (before.activeId) await store.activateForPlay(before.activeId) } } diff --git a/packages/kite3d/test/editor-open-tab.test.ts b/packages/kite3d/test/editor-open-tab.test.ts index 219f3034..b9c2e32d 100644 --- a/packages/kite3d/test/editor-open-tab.test.ts +++ b/packages/kite3d/test/editor-open-tab.test.ts @@ -76,6 +76,45 @@ it('runs the scene tab that is open, not the main scene', async (context) => { expect(running).not.toContain('MainSceneTriangle') }) +// Guards the owner's second report on the same press: Play from a tab that holds no scene has to +// switch to the main scene, and Stop has to give the tab back. +it('returns to the tab Play was pressed on', async (context) => { + const chromium = await browserLauncher() + if (!chromium) { + context.skip('Playwright is not installed. Run npx playwright install chromium.') + return + } + const root = await temporaryProject() + const server = await createDevServer({projectRoot: root, port: 0}) + cleanup.push(() => server.close()) + const browser = await chromium.launch({headless: true}) + cleanup.push(() => browser.close()) + + const page = await browser.newPage({viewport: {width: 1280, height: 800}}) + await page.goto(server.url, {waitUntil: 'domcontentloaded', timeout: 60_000}) + await page.waitForFunction('window.kite3dProjectLoaded === true', undefined, {timeout: 60_000}) + await page.waitForSelector('.editorCanvasContainer canvas', {timeout: 30_000}) + + await page.dblclick('button.file-item-button[title="pixel.png"]', {timeout: 30_000}) + await page.waitForSelector('[role="tab"][aria-selected="true"] .document-tab-name:text-is("pixel.png")', { + timeout: 60_000, + }) + + await page.click('[aria-label="Run"]') + await page.waitForSelector('[aria-label="Pause"]:not([disabled])', {timeout: 60_000}) + // A texture holds no scene, so the main scene runs and its tab is the one on screen. + expect(await sceneNodeNames(page)).toContain('MainSceneTriangle') + await page.waitForSelector('[role="tab"][aria-selected="true"] .document-tab-name:text-is("main.scene.gltf")', { + timeout: 30_000, + }) + + await page.click('[aria-label="Edit"]') + await page.waitForSelector('[role="tab"][aria-selected="true"] .document-tab-name:text-is("pixel.png")', { + timeout: 60_000, + }) + expect(await sceneNodeNames(page)).not.toContain('MainSceneTriangle') +}) + /** * Whether the viewer's canvas is in the container the page shows, and has a size. A query finds * mounted elements alone, so a canvas left behind in the panel that closed answers false.