From f3aa45d317c09b9d2732961c2d404e8e2c4862de Mon Sep 17 00:00:00 2001 From: Eric J Date: Sun, 30 Aug 2026 13:06:31 -0700 Subject: [PATCH] Register a dialog when it is shown, not when it is constructed `Dialog`'s constructor pushed `this` onto the static `s_openDialogs`, and `close()` was the only thing that took an entry back out. `super()` runs before any subclass body, so a subclass constructor that threw afterwards left an entry behind for a dialog that was never shown. `Dialog.anyOpen()` then answered true with nothing on screen, and the `E` keybind - which reads exactly that to choose between closing the top dialog and opening the inventory - went to the wrong branch. Two fixes, and they are not alternatives. Narrow: `DisplayPanelEditor` draws three icons whose names come out of the blueprint's `control_behavior.parameters`, and `F.CreateIcon` ends in a bare `throw` for a name FD does not have. No planet prototype is exported at all, so `nauvis`, `vulcanus`, `fulgora` and `gleba` all reach it, and 19 icon references in the committed corpus use one (issue #231). Those three calls now go through `F.SafeIcon`, so a missing icon costs the icon and a warning naming it, not the whole dialog. `F.SafeIcon` is `ToolsPanel`'s own `safeIcon`, moved into `controls/functions.ts` beside the two throwing icon builders it guards. Moved rather than copied: two copies of a fallback are two things to keep in step, and the one nothing exercises is the one that rots. General: registration moved from the constructor to the `added` event, which is the point a subclass cannot skip and cannot reach early. A constructor that throws never returns the object to be added, so it never registers. All four construction sites are in `UIContainer` and add on the next line, so nothing can read the registry in between. The rejected alternative - a try/catch in `UIContainer.createEditor` - is measured in the spec header: it does not clear the registry, since `s_openDialogs` is protected, and it buys the silence of every future broken editor. One correction to the issue text, measured. The phantom is not permanent. `closeLast()` calls `close()` on it and `close()` filters the registry before it destroys anything, so the entry clears itself. The cost is one swallowed `E` or `Escape` per failed open, not a dead keybind for the session. `tests/dialog-registry-leak.spec.ts` covers both halves and carries the mutation record. The general half needs its own fixture, because once the narrow half is in, no display panel can reach the throw any more: it uses an assembling machine carrying a recipe name FD does not have, which reaches an unguarded `F.CreateIcon` through `Editor`'s `Recipe` slot. That editor is still broken and this does not fix it - it is the fixture precisely because it is broken. `openDialogCount()` reads the pixi child count, so it cannot see a phantom entry at all. The only thing that can is the keybind that branches on it, which is also the thing the user loses. Closes #280 --- packages/editor/src/UI/ToolsPanel.ts | 37 +-- packages/editor/src/UI/UIContainer.ts | 20 ++ packages/editor/src/UI/controls/Dialog.ts | 40 ++- packages/editor/src/UI/controls/functions.ts | 32 +++ .../src/UI/editors/DisplayPanelEditor.ts | 31 +- tests/dialog-registry-leak.spec.ts | 269 ++++++++++++++++++ 6 files changed, 399 insertions(+), 30 deletions(-) create mode 100644 tests/dialog-registry-leak.spec.ts diff --git a/packages/editor/src/UI/ToolsPanel.ts b/packages/editor/src/UI/ToolsPanel.ts index a0f11f40..22af1018 100644 --- a/packages/editor/src/UI/ToolsPanel.ts +++ b/packages/editor/src/UI/ToolsPanel.ts @@ -17,7 +17,7 @@ import { colors, styles } from './style' class WireSlot extends Slot { public constructor(wireName: string) { super(wireName) - this.content = safeIcon(wireName, () => F.CreateIcon(wireName)) + this.content = F.SafeIcon(wireName, () => F.CreateIcon(wireName)) this.on('pointerdown', e => { if (e.button !== 0) return @@ -135,24 +135,13 @@ const WIRES = ['copper-wire', 'red-wire', 'green-wire'] */ const ROWS = 2 -/** - * An icon that failed to build costs this one slot rather than the whole - * panel - `F.CreateIcon`/`F.CreateUtilitySpriteIcon` throw by design for a - * name FD does not have (see `need()` in `core/need.ts`), and nothing above - * `generateSlots()` catches. Falls back to an empty, childless `Container` - - * not a drawn blank square, which this doc comment used to claim (#242 - * review): it has no graphic of its own at all, so the slot still exists and - * is still clickable, but nothing is visible where the icon would have been. - * Named in a warning rather than silently swallowed either way. - */ -function safeIcon(name: string, build: () => Container): Container { - try { - return build() - } catch (error) { - G.logger({ text: `Could not build the "${name}" icon: ${String(error)}`, type: 'warning' }) - return new Container() - } -} +/* + `safeIcon` used to live here. It is `F.SafeIcon` now, in + `controls/functions.ts` beside the two throwing icon builders it guards, + because `DisplayPanelEditor` needs the same guard for an icon a blueprint + names rather than one this file hardcodes (issue #280). Moved rather than + copied - what it does is unchanged, and its doc comment moved with it. +*/ export class ToolsPanel extends Panel { private slotsContainer: Container @@ -224,33 +213,33 @@ export class ToolsPanel extends Panel { altSlot, new WireSlot(WIRES[0]), new ActionSlot( - safeIcon('import_slot', () => + F.SafeIcon('import_slot', () => F.CreateUtilitySpriteIcon(FD.utilitySprites.import_slot) ), () => G.UI.toggleImportDialog() ), new WireSlot(WIRES[1]), new ActionSlot( - safeIcon('export_slot', () => + F.SafeIcon('export_slot', () => F.CreateUtilitySpriteIcon(FD.utilitySprites.export_slot) ), () => G.UI.toggleExportDialog() ), new WireSlot(WIRES[2]), new ActionSlot( - safeIcon('signal-anticlockwise-circle-arrow', () => + F.SafeIcon('signal-anticlockwise-circle-arrow', () => F.CreateIcon('signal-anticlockwise-circle-arrow') ), () => G.bp.history.undo() ), new ActionSlot( - safeIcon('signal-clockwise-circle-arrow', () => + F.SafeIcon('signal-clockwise-circle-arrow', () => F.CreateIcon('signal-clockwise-circle-arrow') ), () => G.bp.history.redo() ), new ActionSlot( - safeIcon('downloading', () => + F.SafeIcon('downloading', () => F.CreateUtilitySpriteIcon(FD.utilitySprites.downloading) ), () => G.quickActions.exportImage() diff --git a/packages/editor/src/UI/UIContainer.ts b/packages/editor/src/UI/UIContainer.ts index ec764a96..69ba2982 100644 --- a/packages/editor/src/UI/UIContainer.ts +++ b/packages/editor/src/UI/UIContainer.ts @@ -54,6 +54,26 @@ export class UIContainer extends Container { this.debugContainer.visible = visible } + /* + Deliberately still a bare call with no try/catch, and that is a + narrower statement than it looks (issue #280). + + A throwing editor constructor no longer damages anything outside + itself: `Dialog` registers on `added` rather than in its constructor, + so an editor that dies part-built is never added and never counted as + open. See the comment there. + + What a catch here would add is swallowing the throw, and that is the + wrong trade. `createEditor` is the point where a dialog is asked for + because the user clicked an entity; an editor that cannot be built is a + bug in that editor, and it should say so loudly - the failure is + already visible as one entity with no dialog, and the console keeps the + stack that names which one. Catching it here would turn every future + broken editor into a click that does nothing. + + The fix for a specific editor is in that editor - `DisplayPanelEditor` + and `F.SafeIcon` are the worked example. + */ public createEditor(entity: Entity): void { const editor = createEditor(entity) if (editor) { diff --git a/packages/editor/src/UI/controls/Dialog.ts b/packages/editor/src/UI/controls/Dialog.ts index 1eaf09a0..cad4c96f 100644 --- a/packages/editor/src/UI/controls/Dialog.ts +++ b/packages/editor/src/UI/controls/Dialog.ts @@ -33,7 +33,45 @@ export abstract class Dialog extends Panel { this.addLabel(12, 10, title, styles.dialog.title) } - Dialog.s_openDialogs.push(this) + /* + Registered when the dialog joins the display tree, not here. + + This line used to be a bare `Dialog.s_openDialogs.push(this)`, and + a constructor is the one place a dialog cannot safely claim to be + open from: `super()` runs before any subclass body, and `close()` + is the only thing that takes an entry back out. So a subclass + constructor that throws after `super()` returns leaves an entry + behind for a dialog that was never shown and that nothing holds a + reference to (issue #280). + + That is reachable rather than theoretical. Icon names come out of + the blueprint, `F.CreateIcon` throws for a name FD does not have, + and `UIContainer.createEditor` is a bare call with no try/catch + above it - `DisplayPanelEditor` drawing a planet icon is the + measured path. A try/catch at that one call site would have fixed + that one call site; every later `new SomethingEditor()` would be + free to repeat it. + + `added` is the point a subclass cannot skip and cannot reach early. + Pixi emits it from `addChild`/`addChildAt` on the child, and every + dialog in this codebase is constructed and then added by its + creator - the four sites are `UIContainer`'s `createEditor`, + `toggleImportDialog`, `toggleExportDialog` and `createInventory`, + each of which adds to `dialogsContainer` on the next line. A + constructor that throws never returns the object to be added, so + it never registers. Nothing here adds a dialog to anything else, + and `Panel`'s own `addChild` of its background emits on the + background, not on `this`. + + `once`, not `on`: a dialog is added exactly once and then closed, + and `once` means a re-add cannot register a second entry for the + same object, which would take two `closeLast()` presses to clear. + + The window this opens - constructed but not yet added, and so not + yet `anyOpen()` - is closed by every call site being synchronous. + Nothing can read the registry between the two statements. + */ + this.once('added', () => Dialog.s_openDialogs.push(this)) } /** Closes last open dialog */ diff --git a/packages/editor/src/UI/controls/functions.ts b/packages/editor/src/UI/controls/functions.ts index a37791c7..db41fc58 100644 --- a/packages/editor/src/UI/controls/functions.ts +++ b/packages/editor/src/UI/controls/functions.ts @@ -419,6 +419,37 @@ function CreateUtilitySpriteIcon(data: SpriteData, maxSize = 32, setAnchor = tru return sprite } +/** + * An icon that failed to build costs the one slot it was going in rather than + * whatever was drawing it - `CreateIcon` and `CreateUtilitySpriteIcon` above + * both throw by design for a name FD does not have (see `need()` in + * `core/need.ts`), and a name can come straight out of a blueprint. Falls back + * to an empty, childless `Container`: no graphic of its own at all, so the slot + * or row still exists and is still laid out, but nothing is visible where the + * icon would have been. Named in a warning either way rather than silently + * swallowed - note that reaches the user as a toast, not a console line. + * + * Takes a builder rather than a name so it covers both icon functions. Lives + * here, next to the two calls it is guarding, rather than in whichever caller + * needed it first: `ToolsPanel` wanted it for a hardcoded name it could not + * spell wrong twice, `DisplayPanelEditor` wants it for a name a blueprint + * chose, and a second copy in the second caller is how the two drift. + * + * **This is not a substitute for a try/catch above a whole feature.** It is the + * right tool only where the icon is one piece of something bigger that should + * still be drawn without it. Where a throw would cost the caller everything - + * see `TileContainer.generateSprite` in CLAUDE.md - the question is what to + * draw instead, not how to skip one sprite. + */ +function SafeIcon(name: string, build: () => Container): Container { + try { + return build() + } catch (error) { + G.logger({ text: `Could not build the "${name}" icon: ${String(error)}`, type: 'warning' }) + return new Container() + } +} + export default { ShadeColor, DrawRectangle, @@ -427,6 +458,7 @@ export default { CreateIconWithAmount, CreateRecipe, CreateUtilitySpriteIcon, + SafeIcon, applyTint, colorAndAlphaToColorSource, rgbToColorSource, diff --git a/packages/editor/src/UI/editors/DisplayPanelEditor.ts b/packages/editor/src/UI/editors/DisplayPanelEditor.ts index 522ec781..c2172ba0 100644 --- a/packages/editor/src/UI/editors/DisplayPanelEditor.ts +++ b/packages/editor/src/UI/editors/DisplayPanelEditor.ts @@ -18,12 +18,29 @@ const COMPARATOR_COL_WIDTH = 18 const VALUE_COL_WIDTH = 40 const CONDITION_WIDTH = ICON_COL_WIDTH + COMPARATOR_COL_WIDTH + VALUE_COL_WIDTH +/* + Every icon this file draws is named by the blueprint, in + `control_behavior.parameters`, so every one of them is a name FD may not + have - and `F.CreateIcon` ends in a bare `throw` for such a name. There is + no try/catch anywhere above here: `UIContainer.createEditor` is a bare call, + so a throw costs the whole dialog rather than one icon, and it costs it + after `Dialog`'s constructor has already registered the dialog as open + (issue #280). `F.SafeIcon` is what keeps the cost to the icon. + + Not hypothetical. `data.json` exports no planet prototype at all, so + `nauvis`, `vulcanus`, `fulgora` and `gleba` are in none of the five + collections `CreateIcon` searches and all four reach that throw - 19 icon + references in the committed corpus use one (issue #231). A panel captioned + for a planet is exactly the panel someone writes. +*/ + /** Icon(s) + comparator + value, laid out in fixed-width columns to line up across rows */ function createConditionDisplay(condition: ICondition | undefined): Container { const container = new Container() if (!condition || !condition.first_signal?.name) return container - const icon = F.CreateIcon(condition.first_signal.name, CONDITION_ICON_SIZE) + const firstName = condition.first_signal.name + const icon = F.SafeIcon(firstName, () => F.CreateIcon(firstName, CONDITION_ICON_SIZE)) icon.position.set(ICON_COL_WIDTH / 2, CONDITION_ICON_SIZE / 2) container.addChild(icon) @@ -35,8 +52,11 @@ function createConditionDisplay(condition: ICondition | undefined): Container { container.addChild(comparatorLabel) const valueX = ICON_COL_WIDTH + COMPARATOR_COL_WIDTH - if (condition.second_signal?.name) { - const secondIcon = F.CreateIcon(condition.second_signal.name, CONDITION_ICON_SIZE) + const secondName = condition.second_signal?.name + if (secondName) { + const secondIcon = F.SafeIcon(secondName, () => + F.CreateIcon(secondName, CONDITION_ICON_SIZE) + ) secondIcon.position.set(valueX + CONDITION_ICON_SIZE / 2, CONDITION_ICON_SIZE / 2) container.addChild(secondIcon) } else { @@ -91,8 +111,9 @@ export class DisplayPanelEditor extends Editor { const row = new Container() row.position.set(12, 210 + i * ROW_HEIGHT) - if (param.icon?.name) { - const icon = F.CreateIcon(param.icon.name, 20) + const iconName = param.icon?.name + if (iconName) { + const icon = F.SafeIcon(iconName, () => F.CreateIcon(iconName, 20)) icon.position.set(10, 10) row.addChild(icon) } diff --git a/tests/dialog-registry-leak.spec.ts b/tests/dialog-registry-leak.spec.ts new file mode 100644 index 00000000..b1c29a77 --- /dev/null +++ b/tests/dialog-registry-leak.spec.ts @@ -0,0 +1,269 @@ +import { test, expect } from '@playwright/test' +import { encodeBlueprint as encode, packVersion as version } from './helpers/encode-blueprint' +import { suppressOverlays } from './helpers/overlays' + +/* + A dialog whose constructor throws, and what it leaves behind (issue #280). + + Two separate things, and they are not alternatives. + + The NARROW half is `DisplayPanelEditor`. Every icon it draws is named by the + blueprint - `control_behavior.parameters` carries an icon per row and a + signal at each end of that row's condition - and `F.CreateIcon` ends in a + bare `throw` for a name FD does not have. `data.json` exports no planet + prototype at all, so `nauvis`, `vulcanus`, `fulgora` and `gleba` all reach + it (issue #231), and 19 icon references in the committed corpus use one. The + throw cost the whole dialog: clicking the panel did nothing at all. The fix + is `F.SafeIcon`, so the cost is the one icon and a warning naming it. + + The GENERAL half is `Dialog`'s registry. `Dialog`'s constructor used to push + `this` onto the static `s_openDialogs`, and `super()` runs before any + subclass body, so a subclass that threw afterwards left an entry for a + dialog that was never shown. `Dialog.anyOpen()` then answered true with + nothing on screen, and `E` - which reads exactly that to decide between + "close the top dialog" and "open the inventory" - went to the wrong branch. + Registration moved to the `added` event, which a constructor that throws + never reaches. + + Why the general half needs its own case, and its own entity. Once the narrow + half is in, no display panel can reach the throw any more, so the registry + fix would have nothing to prove. The third test uses an assembling machine + carrying a recipe name FD does not have, which is the same class of input - + a name out of the blueprint, reaching an unguarded `F.CreateIcon`, this time + through `Editor`'s own `Recipe` slot. That editor is still broken and this + spec does not fix it; it is the fixture precisely because it is broken. + + What can and cannot be observed. `window.__fbe_test.openDialogCount()` reads + the pixi child count of `dialogsContainer`, so it cannot see a phantom + registry entry at all - a leaked entry and a clean registry both read 0. The + only thing that can see one is the keybind that branches on it, so the third + test presses `E` and asks whether the inventory opened. That is also the + thing the user actually loses. + + One correction to the issue text, measured. The phantom is not permanent: + `closeLast()` calls `close()` on it, and `close()` filters the registry + before it destroys anything, so the entry clears itself. The cost is one + swallowed `E` (or `Escape`) per failed open, not a dead keybind for the + session. Two failed opens cost two presses, which is why the third test + clicks twice. + + MUTATION RECORD - each half reverted in turn, against this spec. + + 1. `F.SafeIcon(...)` -> `F.CreateIcon(...)` at all three sites in + `DisplayPanelEditor.ts` (the narrow half reverted, registry fix kept): + - test 1 FAILS: `expect(received).toEqual(expected)` on the page-error + list, `["Error: No item, fluid, recipe, signal or inventory group + named nauvis"]` against `[]`; the dialog count assertion that follows + would have failed too, 0 against 1. + - test 2 FAILS the same way, naming `vulcanus`. + - test 3 PASSES - it never opens a display panel. + + 2. `this.once('added', () => Dialog.s_openDialogs.push(this))` -> + `Dialog.s_openDialogs.push(this)` in `Dialog.ts` (the general half + reverted, `F.SafeIcon` kept): + - test 3 FAILS: `expect(received).toBe(expected)` on the dialog count + after `E`, 0 against 1. Both phantoms are still queued, so the first + `E` closes one instead of opening the inventory. + - tests 1 and 2 PASS - nothing in them throws any more, so nothing + leaks. + + 3. Both reverted (the pre-fix code): + - all three FAIL, each with its own message above. + + So neither half covers for the other, and neither test can pass for the + wrong reason. + + 4. The alternative the issue offers, measured rather than argued: the + registry push left in the constructor, and a bare try/catch wrapped + around `UIContainer.createEditor` instead (`F.SafeIcon` kept). + - test 3 FAILS, and it fails at the CONTROL rather than at the keybind: + `expect(received).toHaveLength(expected)`, 2 against 0, on the + page-error filter that opens test 3. Swallowing the throw is all + that catch does. The phantom entries are still queued and the `E` + press after them still goes to the wrong branch; the test never + gets that far, because the constructor's failure has stopped being + visible at all. That is the whole objection to it, written down in + `UIContainer.createEditor`'s own comment: it does not clear the + registry - `s_openDialogs` is `protected`, so clearing it from there + would need a new public escape hatch on `Dialog` - and it buys the + silence of every future broken editor for nothing. + + Runs against the dev server like the rest of tests/ - see CLAUDE.md for the + two servers that have to be up. +*/ + +type Page = import('@playwright/test').Page + +/* + `connect_to_logistic_network` rather than a wire, because + `Entity.generateConnector` is what puts the editor down its read-only + conditions branch and either input satisfies it. A wire would need a second + entity to run to; this needs one panel. +*/ +function displayPanel(parameters: Record[]): string { + return encode({ + item: 'blueprint', + version: version(2, 0, 55), + icons: [{ index: 1, signal: { type: 'item', name: 'display-panel' } }], + entities: [ + { + entity_number: 1, + name: 'display-panel', + position: { x: 0.5, y: 0.5 }, + control_behavior: { connect_to_logistic_network: true, parameters }, + }, + ], + }) +} + +/** A planet as the row's own icon - `DisplayPanelEditor`'s parameter loop. */ +const PLANET_ROW_ICON = displayPanel([ + { + icon: { type: 'space-location', name: 'nauvis' }, + text: 'home', + condition: { + first_signal: { type: 'item', name: 'iron-plate' }, + comparator: '>', + constant: 5, + }, + }, +]) + +/* + A planet at each end of the condition instead - `createConditionDisplay`. + Kept apart from the row icon so that reverting either call site on its own + is caught: with both planets in one blueprint, whichever threw first would + mask the other. +*/ +const PLANET_CONDITION = displayPanel([ + { + icon: { type: 'item', name: 'iron-plate' }, + text: 'away', + condition: { + first_signal: { type: 'space-location', name: 'vulcanus' }, + comparator: '<', + second_signal: { type: 'space-location', name: 'gleba' }, + }, + }, +]) + +/* + An assembling machine naming a recipe FD does not have. Decodes and loads - + the schema's `recipeName` keyword only warns - and then `Editor`'s `Recipe` + slot calls `F.CreateIcon` on it with nothing catching, so the constructor + throws after `super()` has run. The fixture for the registry half. +*/ +const UNKNOWN_RECIPE = encode({ + item: 'blueprint', + version: version(2, 0, 55), + icons: [{ index: 1, signal: { type: 'item', name: 'assembling-machine-1' } }], + entities: [ + { + entity_number: 1, + name: 'assembling-machine-1', + position: { x: 1.5, y: 1.5 }, + recipe: 'totally-not-a-recipe', + }, + ], +}) + +async function load(page: Page, source: string): Promise { + const errors: string[] = [] + page.on('pageerror', e => errors.push(String(e))) + + await suppressOverlays(page) + await page.goto('/') + await page.waitForFunction(() => window.__fbe_test !== undefined, { timeout: 60_000 }) + await page.evaluate(async (src: string) => { + const t = window.__fbe_test + await t.loadBp(await t.getBlueprintOrBookFromSource(src)) + }, source) + return errors +} + +/* + Hover then click, which is `openEntityGUI`. Steps away first because + hovering is driven by GridData's `update32` and only fires when the pointer + crosses a tile boundary - moving to a point it already occupies emits + nothing. Same reason as display-panel-editor.spec.ts and chest-editor.spec.ts. +*/ +async function clickEntity(page: Page, entityNumber: number): Promise { + const at = await page.evaluate( + (n: number) => window.__fbe_test.entityScreenPosition(n), + entityNumber + ) + if (!at) throw new Error(`no entity ${entityNumber} in the loaded blueprint`) + + await page.mouse.move(at.x, at.y + 240) + await page.mouse.move(at.x, at.y) + expect(await page.evaluate(() => window.__fbe_test.editorMode())).toBe('EDIT') + + await page.mouse.down() + await page.mouse.up() +} + +const dialogCount = (page: Page): Promise => + page.evaluate(() => window.__fbe_test.openDialogCount()) + +test('a display panel row icon naming a planet costs the icon, not the dialog', async ({ + page, +}) => { + const errors = await load(page, PLANET_ROW_ICON) + expect(await dialogCount(page)).toBe(0) + + await clickEntity(page, 1) + expect(errors, `page errors: ${errors.join(' | ')}`).toEqual([]) + expect(await dialogCount(page)).toBe(1) + + /* + The warning names the icon. It reaches the user as a toast rather than a + console line - `G.logger` is wired to the website's toasts - so a probe + listening on `page.on('console')` would see nothing and read a working + guard as a silent skip. + */ + await expect(page.locator('.toasts-warning', { hasText: 'nauvis' })).toBeVisible() + + await page.keyboard.press('Escape') + expect(await dialogCount(page)).toBe(0) + expect(errors, `page errors: ${errors.join(' | ')}`).toEqual([]) +}) + +test('a condition signal naming a planet costs that icon, not the dialog', async ({ page }) => { + const errors = await load(page, PLANET_CONDITION) + + await clickEntity(page, 1) + expect(errors, `page errors: ${errors.join(' | ')}`).toEqual([]) + expect(await dialogCount(page)).toBe(1) + + // Both ends of the condition are guarded, not just the first. + await expect(page.locator('.toasts-warning', { hasText: 'vulcanus' })).toBeVisible() + await expect(page.locator('.toasts-warning', { hasText: 'gleba' })).toBeVisible() + + await page.keyboard.press('Escape') + expect(await dialogCount(page)).toBe(0) +}) + +test('an editor constructor that throws leaves the E keybind alone', async ({ page }) => { + const errors = await load(page, UNKNOWN_RECIPE) + + /* + Twice. One phantom entry costs one `E` press, so a single failed open + would still let the second press through and a spec pressing twice would + pass against the leak. Two failed opens against one press cannot. + */ + await clickEntity(page, 1) + await clickEntity(page, 1) + + /* + The control for this whole test: the constructor really did throw, twice. + Without it a change that quietly made the machine editor open would leave + the assertion below passing while measuring nothing about the registry. + */ + expect(errors.filter(e => e.includes('totally-not-a-recipe'))).toHaveLength(2) + expect(await dialogCount(page)).toBe(0) + + // Nothing is open, so E opens the inventory. A leaked entry sends it to + // `Dialog.closeLast()` instead and nothing appears. + await page.keyboard.press('KeyE') + expect(await dialogCount(page)).toBe(1) +})