Add a Blueprint Info editor: name, icons, description, and grid alignment

[koenigstag]

Re-submission of #222, recreated after the repo detach.

Summary

• New "Blueprint Info" dialog for the blueprint itself (not an entity), opened via a button in the top-left corner next to the FBE logo — the editor had no way to view or change a blueprint's own name/description/icons/grid-alignment before this, only round-tripping them silently on import/export.
• Name and Description — plain/multiline text fields, routed through history.updateValue so edits are undoable like everything else.
• Icons — up to 4 slots, left-click opens the icon picker (items/fluids/recipes/virtual signals), right-click clears a slot.
• Grid alignment — a new section covering snap-to-grid, absolute-snapping and position-relative-to-grid, the three blueprint-string fields the editor already decoded/re-encoded but never exposed:
  • "Snap to grid" checkbox, Grid size (Width/Height), Grid position (X/Y), and an Absolute/Relative choice.
  • The exact serialization rules (when each field is written vs. omitted, and what "enabling from off" defaults to) were verified by decoding several real exported blueprint strings by hand rather than guessed — see commit messages for the specifics. This caught and fixed a real bug where turning snapping back off could leave a stale absolute-snapping: true behind in the exported string.
  • New RadioButton control (circular) for the Absolute/Relative choice, since Checkbox draws a rounded square that reads as an independent toggle rather than one of two options.

Test plan

• vp check / vp test pass
• Manually verified end-to-end: dialog opens from the corner button; name/description/icons edit and persist; grid-size/position fields enable/disable correctly across Snap-to-grid/Absolute/Relative combinations; round-tripped several real blueprint strings (Absolute, Relative, grid off) through the app and confirmed the re-exported JSON matches the original byte-for-byte in the relevant keys; undo/redo works throughout.

[koenigstag]

PR recreated after the repo detach (was #222). Ready for review — waiting on any fixes/comments.

[wormeyman]

Review of #243

I read the full diff at 735292c. The doc comments on gridPositionOffset and commitGridPosition made the design easy to follow, and one open question in them sent me off to measure the real game, so this is longer than a normal review.

Your central argument is right, and I have measured proof of it. The premise in 8529c2f, that blueprint-grid-position.json measured a different field, holds up. I built an interactive probe against Factorio 2.0.77 and found the game's blueprint panel carries three X/Y pairs, two of which were on screen at once holding different values. "Grid position" writes no key into the export at all; it translates the coordinates. blueprint_position_relative_to_grid is the neighbour. The fixture and probe are on branch probe-grid-position-gui.

The same measurement turned up one thing the formula gets wrong, which is finding 15 below. It is numbered last because I measured it last, not because it matters least.

Findings 1-4 change the exported blueprint without telling the user, 5-8 are worth fixing before merge, and 9-14 are minor. Two are marked PLAUSIBLE where I could not fully confirm reachability.

---

Blocking - these change the export silently

1. gridPositionOffset survives snapping being turned off - Blueprint.ts:1126

serialize() applies gridPositionOffset unconditionally, but refreshEnabled() (BlueprintAlignment.ts:370) disables the Grid position fields once snapToGrid is undefined.

Tick Snap to grid, type 3/4 into Grid position, untick Snap to grid. All three snapping keys leave the export, but gridPositionOffsetStore still holds the solved offset, so every exported position stays shifted for the rest of the session with no field left to zero it. Undo is the only way back. tests/blueprint-grid-position.spec.ts records this in a comment without asserting it is wanted.

2. refreshFromBlueprint leaves m_PositionDirty set - BlueprintAlignment.ts:342

The flag is set on 'changed' (line 279) and cleared only in commitPosition (line 335).

Take a blueprint with snap-to-grid and no position-relative-to-grid. Type "12" into Absolute X, then click the Relative radio. Pixi's pointerdown runs before the DOM blur, so the radio handler calls refreshFromBlueprint(), which rewrites X to "0" and leaves the flag set; blur then fires and commitPosition() writes {x:0, y:0}. The typed value is lost, and the blueprint gains an explicit origin position where it had none, which serialize() treats as significant via positionIsUnset (Blueprint.ts:1177). Unticking Snap to grid is the same sequence.

3. Grid position hides an offset on an empty blueprint - BlueprintAlignment.ts:322

The editor opens on an empty blueprint, where getGridPositionDisplay() early-returns {0,0} (Blueprint.ts:1087).

Open Blueprint Info, tick Snap to grid, type 5 into Grid position X, Tab. current is {0,0}, so delta.x is -5 and the offset becomes {x:-5,y:0}. The event fires refreshFromBlueprint, which reads {0,0} again and resets the box to "0". The user watches the value vanish, and every entity placed afterwards exports shifted by -5 with nothing on screen saying why.

4. First rejected keystroke wipes a restricted box - TextInput.ts:384

_restrict_value is only written inside _applyRestriction, never by the text setter. This one predates the PR, but nothing committed an emptied box to the model before the blur wiring existed, so the PR is what makes it reachable.

Load a blueprint with snap-to-grid {x:16,y:16} and click into Width. It shows "16", but _restrict_value is still '', because the constructor sets .value programmatically and fires no input event. Type a non-digit first: _applyRestriction rejects "16x" and assigns this.text = this._restrict_value, which is ''. Tab out and commitSize() runs parseGridSize('') = 1, so the grid becomes 1x1. The same path on Absolute X over a loaded {x:-5} gives {x:0}. tests/text-input.spec.ts:195 cannot catch it because it types a valid character first, which seeds _restrict_value.

---

Worth fixing before merge

5. Corner button closes a covered dialog - UIContainer.ts:103

toggleBlueprintInfoEditor closes the info dialog even when it is not the top one. The button is added at line 42, after dialogsContainer at line 34, so it draws above every dialog and stays clickable at (152,6), outside the centred ones.

Open Blueprint Info, click an icon slot to open the picker, then click the corner button. close() destroys the dialog underneath and orphans the picker; blueprintInfoEditor is then undefined, so the next click stacks a fresh Blueprint Info dialog under the still-open picker.

6. Ticking snap-to-grid needs two Ctrl+Z - BlueprintAlignment.ts:240

snapToGrid = {...} commits one transaction and absoluteSnapping = true commits another, since each setter ends in history.updateValue(...).commit(). One undo leaves snap-to-grid set with absolute-snapping back to false, a state the user never chose. History.startTransaction()/commitTransaction() already exists, and Blueprint's own load path uses it to group writes like these.

7. Checkbox handler uses the unfloored parser - BlueprintAlignment.ts:243

The enable-snapping branch parses Width/Height with parseGridValue rather than parseGridSize. That function's own comment (lines 40-48) explains that writing a 0 straight through produced a snap-to-grid {"x":0,"y":4} the game will not accept back, and commitSize (line 296) uses the floored parser for exactly that reason. This handler is the one writer left that can still produce it, and finding 4 makes it reachable today.

8. Name and description commit per keystroke - BlueprintInfoEditor.ts:110 and :137

Both use 'changed', which is one DOM input event per keystroke, so a 20-character name is 20 separate Change blueprint name transactions and undoing a rename takes 20 presses. This is the pattern your own BlueprintAlignment.ts:93-102 documents as the reason those fields moved to 'blur'.

---

Minor

9. Grid position goes stale - BlueprintAlignment.ts:287 (PLAUSIBLE)

The dialog subscribes to the four alignment events, but getGridPositionDisplay() derives from every entity and tile position. Place or delete an entity that extends the minimum corner while the dialog is open and the box keeps its old value, and blurring it then commits that stale value as a fresh target.

10. Math.min spread over every coordinate - Blueprint.ts:1102

Math.min(...xs) throws RangeError somewhere around 100k entities plus tiles, taking the dialog constructor with it. getCenter() at line 744 does the same min/max as data.reduce((min, d) => Math.min(min, ...), Infinity), with neither that limit nor the two full-length array allocations per call.

11. package-lock.json carries a 156-line libc strip

156 deletions and one addition: 52 libc fields removed, 32 ["glibc"] and 20 ["musl"], from optional platform binaries. npm reads that field to skip a binary built for the wrong C library. It is the signature of regenerating under npm 11 while the root devEngines pins ^12. Nothing here needs a lockfile change, so reverting the file is the fix.

12. Hardcoded (170,24) instead of the hook this PR adds - tests/text-input.spec.ts:279

BlueprintInfoButton sets position.set(152, 6) on a 36x36 button and the spec encodes the derived centre by hand. This PR already moved that button once, in 6826096d. window.__fbe_test.openBlueprintInfoEditor() was added in this same PR (tests/helpers/fbe-test-api.ts:49) for this purpose, and blueprint-grid-position.spec.ts:88 uses it.

13. Doc comment reverses the real ordering - BlueprintInfoEditor.ts:94

The comment says Editor.loadBlueprint calls Dialog.closeAll() before swapping G.bp. In Editor.ts, line 326 assigns G.bp = bp, lines 328-329 construct and initBP() a new BlueprintContainer, and line 330 calls Dialog.closeAll() last. That ordering is the whole justification given for not guarding against the dialog outliving its blueprint, so it is worth correcting even if the guard turns out to be unnecessary.

14. serialize() mutates icons outside history - Blueprint.ts:1109 (PLAUSIBLE)

serialize() calls generateIcons(), which writes into this.icons without History and without emitting 'icon'. Pre-existing, but newly reachable now that icons are user-editable and observed by BlueprintIconSlot: clear all four slots, then Ctrl+C. The icons repopulate, the slots keep drawing empty, and Ctrl+Z cannot remove them.

---

15. The corner reads entity centres, the game reads edges - Blueprint.ts:1102

Measured on Factorio 2.0.77, the version this editor targets. The probe has to be interactive because no scripting API reaches the field. The result is survivingReadings: ["entityEdgesAndTiles"], with all three controls passing and the three rival readings killed on every scored row.

getGridPositionDisplay() builds its minimum from e.position.x, the entity centre:

const xs = [...this.entities.valuesArray().map(e => e.position.x), ...tiles]

The game takes the entity's edge. On the measured row where Grid position is 3, your formula gives 2. The probe layout was built to separate exactly this: a 3x3 assembling machine decides the x minimum, where the centre floors to 1 and the edge to 0, and a lone tile decides y. The y axis agreed at 5 under both readings, because a tile set that minimum and a tile has no footprint. That is why every earlier case agreed, and it matches your doc comment already flagging the footprint half as untested.

So the error is floor(size/2) on whichever axis a multi-tile entity sets the minimum. A blueprint edged by belts agrees; one edged by an assembler is a tile out.

What hides it is that the editor stays self-consistent. commitGridPosition solves for the offset that makes getGridPositionDisplay() read the target back, using the same formula on both sides, so the box always shows what was typed and no round-trip inside the editor can expose the difference. It shows up only against the game: export with Grid position 3 and Factorio's own panel reads 4.

One caveat decides the fix. For assembling-machine-1 the tile footprint edge (9.0) and the collision-box edge (about 9.1) floor to the same integer, so this measurement cannot say which one the game uses. Separating them needs an entity whose collision box is inset by more than the fractional part. That is worth settling before choosing an implementation, and tile_width is not the field to reach for: it is a centring parity rather than a size (issue #142).

---

Out of scope, but adjacent

serialize()'s getItemTypeForBp has no space-location arm. That is on the base branch and predates this PR, so a planet-name icon round-trips as type: 'item'. Making icons editable makes the existing loss easier to hit.

On CI

Everything is green except claude-review, which fails on Could not fetch an OIDC token. That is the standard restriction on fork PRs, not a problem with your code. All four Playwright shards pass, and I confirmed blueprint-grid-position.spec.ts, blueprint-snapping.spec.ts and text-input.spec.ts actually ran rather than being skipped.

[wormeyman]

Two corrections to finding 15 above. Neither changes it, but both were stated as measurements.

The assembler's collision edge is 9.3, not "about 9.1". Its collision_box is [[-1.2,-1.2],[1.2,1.2]] and its centre sits at 10.5. Both still floor to 9, so the point stands: that entity cannot tell a tile-footprint edge from a collision-box edge.

survivingReadings is now two entries, not one. I quoted ["entityEdgesAndTiles"]. The analyzer only carried four candidate rules at the time, and none of them was "read the collision box edge". Adding it as a fifth makes the same run report both entityEdgesAndTiles and entityCollisionEdgesAndTiles, which is the honest answer: the corner is read from an edge, and that layout could not say which edge. The prose caveat I posted was right; the machine-readable result was overstated.

What this does not change: getGridPositionDisplay() reads e.position, the centre, and the game does not. On the measured row where Grid position is 3, the formula gives 2.

A second layout to settle which edge is on #249. It uses a half-diagonal-rail, which is close to the only entity that can: the editor derives a footprint by ceiling the collision box, so a box escapes its own footprint only where a declared tile_width overrides that - five of the 155 entities in data.json - and it is the only one splitting all three readings apart on a single axis.

[wormeyman]

Finding 15 is now settled. The open half was "which edge", and a second measured run answers it: the game reads each entity's tile footprint edge, and counts tiles.

Two scored rows, on a layout built so that one entity splits all three candidate rules apart on the y axis while tiles decide x:

reading	Grid position 2,6	Grid position 8,10
entity centres + tiles (what ships today)	2,5	8,9
entity footprint edges + tiles	2,6	8,10
entity collision-box edges + tiles	2,7	8,11
footprint edges, tiles ignored	-5,6	1,10

Each wrong reading misses by exactly one tile on y, in opposite directions, and ignoring tiles misses by seven on x. Four controls passed. Fixture and probe are on #249.

So the change to getGridPositionDisplay() is to build the minimum from each entity's footprint edge rather than e.position:

const xs = [...this.entities.valuesArray().map(e => e.position.x), ...tiles]

becomes the same minimum taken over position.x - width / 2, with width from the entity's tile footprint. Tiles stay as they are, since counting them is confirmed.

Two things worth knowing before writing it.

getEntitySize in factorioData.ts already returns the footprint you want, so this does not need new geometry. It reads a declared tile_width/tile_height when there is one and otherwise ceilings the collision box.

Do not reach for collision_box directly. Besides being the reading this run just refuted, data.json disagrees with the running game about it for every rail prototype and nothing else - 16 of 155 entities, worst case 1.45 tiles on legacy-curved-rail, while the other 139 agree to within Factorio's 1/256 position quantum. That is filed separately as #251. The footprint route avoids it, because a declared tile_height is used as-is and ceiling absorbs the rest.

One unrelated thing the run turned up, filed as its own issue: the game refused Grid position 3,5 on this layout with "Grid position and blueprint grid position coordinates need to be either all even or all odd", having accepted the same 3,5 on the previous layout. So the rule depends on the blueprint's own content, and BlueprintAlignment.ts validates none of the three rules the locale defines. The editor can currently write a grid position that the game will not take back.

[wormeyman]

Reviewed c4db8c0c against the fifteen findings from my last pass. Fourteen are fixed, and I checked each one against the code rather than against the commit message. The remaining one is fixed for the case I measured, with the gap it cannot close written down at the site.

Finding 15 is implemented exactly as measured

getGridPositionDisplay() now reads the corner through minCorner(footprintData()), the same reading getCenter() uses, and getEntitySize supplies the footprint so no new geometry was needed.

Two things I checked rather than assumed:

getCenter()'s arithmetic came through the extraction byte for byte, so the round-trip fixture could not have moved. git diff shows the min/max reduces unchanged, only relocated.

The new test at tests/blueprint-grid-position.spec.ts:316 pins the case that actually separates the two readings, and the numbers work out. A assembling-machine-1 at (10.5, 10.5) has its footprint edge at 9 and its centre at 10.5, and getCenter() rounds the 9..12 box to 11, so the edge reading gives -floor(9 - 11) = 2 where a centre reading gives -floor(10.5 - 11) = 1. That is the difference the game settled, and the test cites the fixture.

Finding 10 dropped the Math.min(...) spread for a reduce, so the 100k-entity RangeError is gone along with the two array allocations per call.

Finding 11 is fully reverted. git diff base...HEAD -- package-lock.json is empty, so the 156-line libc strip is gone rather than partially undone.

Finding 9, and the half it does not reach

create-entity, remove-entity and create-tile are hooked now, and the comment at BlueprintAlignment.ts:316 says plainly that create-tile has no remove-tile counterpart to hook. That is the right way to leave a gap.

One more case falls in the same hole and is not mentioned: Entity emits 'position' (Entity.ts:300), so moving an entity that sets the minimum corner leaves the box showing its old value, exactly like deleting one used to. Blurring it afterwards then commits the stale reading as a fresh target, which is the same failure the three new hooks close. Worth either hooking or naming alongside remove-tile.

Three small things

Closes #243 in the commit body points at this PR, not at an issue. Since the merge squashes the body into history, it is worth correcting before it lands.

Finding 5's fix reads dialogsContainer.children[len - 1] to decide what is topmost, while Dialog.closeLast() reads s_openDialogs[len - 1]. Two answers to one question, and they agree only as long as nothing else is ever added to dialogsContainer. This repo's own rule for that is in CLAUDE.md under Asking the real game: a bound written down twice is one you have to keep in sync, and the one that loses is the one nothing tests. A Dialog.isTopmost(this) beside the existing isOpen would leave one copy.

tests/text-input.spec.ts reaches the hook through (window as any).__fbe_test, which steps around the single declare global in tests/helpers/fbe-test-api.ts. blueprint-grid-position.spec.ts:88 uses the typed one.

One consequence worth knowing

Routing generateIcons() through setIcon inside a transaction means serialize() can now write to history. On a blueprint with no icons yet, the first Ctrl+C fills them in and spends one undo step doing it. That is the fix working as asked, since the whole complaint in finding 14 was that those icons could not be undone. It is just worth knowing that copying is no longer read-only against the model.

Adjacent, and not this PR's problem

The guard at the top of generateIcons() is correct, and I checked the reason given for it. History.commitTransaction() returns false on an empty transaction before it reaches this.activeTransaction = undefined, and startTransaction() only builds a new Transaction when activeTransaction is undefined. So an empty transaction anywhere leaves the stale one open, and the next unrelated startTransaction adopts it, keeps its label, and returns false.

That is on the base branch, not in this PR, and this PR is the only place I have seen it written down. It deserves its own issue.

CI

Green on c4db8c0c. claude-review fails on the OIDC token again, which is the fork-PR restriction rather than anything in the code. I confirmed blueprint-info-editor.spec.ts and blueprint-grid-position.spec.ts both actually ran in shard 1, and text-input.spec.ts in shard 4, rather than being collected and skipped.

[wormeyman]

A second pass over c4db8c0c turned up six things my first review missed, and one of them is bad enough that I am switching to request changes. Everything below I traced through the code myself.

Sorry for the two-comment review. The first pass checked the fix against my old findings, which is exactly the pass that could not see any of this.

Blocking: a failed icon generation kills undo for the whole session

generateIcons() opens a transaction at Blueprint.ts:989 and commits it at :1032. In between, at :1016, iconPairs[0][0] is dereferenced without a guard.

getIconPairs skips every entity whose Entity.getItemName is undefined, so a blueprint whose entities are all non-minable gives back [], and iconPairs[0][0] throws a TypeError. The throw escapes generateIcons before commitTransaction() runs, so transactionCount is stuck at 1 and activeTransaction is never cleared.

encode() catches it into a toast, so the app carries on and nothing points at History. But the damage is already done. updateValue() calls startTransaction() and Action.commit() calls commitTransaction(), so from that moment every edit runs 1 -> 2 -> 1 and commitTransaction returns at the transactionCount === 0 check without pushing anything to transactionHistory. Edits still apply. Nothing is ever recorded. Undo and redo are dead for the rest of the session, silently.

This is reachable by hand, not just in theory. 20 of the 155 entities in data.json have no minable.result, and two of them can be placed straight from the inventory:

captive-biter-spawner  <- item "captive-biter-spawner"
space-platform-hub     <- item "space-platform-hub"

So: new blueprint, place a space-platform-hub, press Ctrl+C. That is a space platform blueprint at its most basic, which is squarely what this fork is for.

Before this PR the same throw was harmless, because generateIcons wrote into a plain Map and touched no history at all. Routing it through setIcon is what turned a caught error into a permanent one.

The same root: Ctrl+C spends the user's next Ctrl+Z

serialize() calls generateIcons(), so on a blueprint with no icons yet the first copy writes to history. Build something from scratch, place a chest, press Ctrl+C, then press Ctrl+Z expecting the chest back. You get the auto-generated icon removed instead, and the chest stays.

Your own new spec works around this at blueprint-info-editor.spec.ts:197:

Settles the icon-autogeneration transaction (finding #14, below) out of the way first [...] it would otherwise be the transaction the first undo below reverts instead of the checkbox's.

That was the right call for the test. It is worth reading as a signal about the feature, though, because a user gets no encodeLoaded() call to settle it for them.

My finding 14 asked for those icons to be undoable, and this is a fair reading of it. But serialize() is the wrong place to write from, and moving generation out of it fixes this and the blocking item above together, since a throw outside a transaction goes back to costing nothing.

A rejected keystroke writes a grid position

TextInput._onInputInput (TextInput.ts:307) runs _applyRestriction and then emits 'changed' unconditionally, so a character the restriction just threw away still reports a change.

Open Blueprint Info on a blueprint that has never carried position-relative-to-grid. Tick Snap to grid, click Absolute X showing 0, type a. The restriction rejects it and puts 0 back, so nothing changed, but 'changed' still fires and BlueprintAlignment.ts:303 sets m_PositionDirty. Tab away and commitPosition() writes {x: 0, y: 0}.

It reaches the export. positionIsUnset at Blueprint.ts:1247 is positionRelativeToGrid === undefined, and the key is emitted whenever that is false, so the blueprint now carries "position-relative-to-grid":{"x":0,"y":0} where it previously had no such key. That is the finding-2 phantom origin arriving by a path the dirty flag does not cover.

An untouched blur pushes an undo entry

The comment at BlueprintAlignment.ts:356 says a zero delta is a no-op because gridPositionOffset's own pointsEqual guard catches it. That holds once the store has a value, and not before.

pointsEqual(undefined, {x: 0, y: 0}) returns false (Blueprint.ts:105). The getter hands back ?? {x: 0, y: 0} while the setter's guard compares the raw store field, so on a fresh blueprint the two differ, the write goes through, and a Change blueprint grid position offset transaction lands on the stack. Click into Grid position X and straight back out without typing, and your next Ctrl+Z undoes that instead of your last real edit.

The new entity subscriptions discard a field being typed into

This one is a consequence of what I asked for in finding 9, so it is on me as much as on you.

refreshFromBlueprint() overwrites m_XInput.text at :395 and clears m_PositionDirty at :410. It is now subscribed to create-entity and remove-entity at :327.

Open Blueprint Info, type 12 into Absolute X without blurring, then place or delete an entity on the canvas around the dialog. Ctrl+Z does it too, since undo re-creates entities. The field is rewritten from the model and the flag is cleared, so the 12 is gone and the later blur commits nothing. That is the same edit-destroying refresh you removed from the 'changed' path, coming back through the entity events.

Control+z is invisible to the #208 guard

Five new call sites use Control+z (blueprint-info-editor.spec.ts:210, :281, :312, and blueprint-grid-position.spec.ts:302, :311).

TEXT_EDIT_KEYS is ['A', 'C', 'V', 'X', 'Z'] and the CHORD regex at spec-modifier-keys.test.ts:104 carries only the g flag, so a lowercase z never matches. The guard reports zero violations for these files and no allowlist entry explains them, which means the check silently covers nothing here. The convention it can see is Control+KeyZ, as at chest-filters.spec.ts:390.

These presses go to the app rather than to a focused input, so they work today. The problem is that the guard stops watching the file.

Everything from my first review still stands

The fourteen fixes I verified are all still good, and the finding-15 work in particular is exactly right. None of the above touches it.

[wormeyman]

Splitting out the parts of my last comment that are not yours, so it is clearer what this PR actually has to carry.

Two of them predate this branch and are now tracked separately:

• generateIcons throws on a blueprint whose entities are all non-minable, so it cannot be exported #254 is the unguarded iconPairs[0][0]. It is on the base branch and throws there too, so a blueprint holding only a space-platform-hub already fails to export today.
• History leaves activeTransaction set on an empty commit, and any throw inside a transaction silently disables undo for the session #255 is the History weakness that turns that throw into a dead undo stack once the write goes through a transaction.

So the blocking item on this PR is narrower than I made it sound: it is that routing generateIcons() through History escalates an existing caught error into a session-wide one. Guarding the empty list here would close it, and #254 is where the same guard belongs for the base branch. Whichever lands first, the other becomes a no-op.

• The #208 modifier-key guard only matches uppercase, so a lowercase Control+z is invisible to it #256 is the modifier-key guard only matching uppercase. Your five Control+z call sites are correct as written and I am not asking you to change them. They reach the app's undo keybind rather than a focused input, which is the case where Control is right. The guard simply cannot see them, and that is the guard's problem to fix.

That leaves four things on this PR from my last comment: the icon transaction escalation, Ctrl+C spending the next Ctrl+Z, the rejected keystroke committing {0, 0}, and refreshFromBlueprint discarding a field being typed into. The pointsEqual(undefined, ...) one is yours too, though it is minor.

[wormeyman]

Re-reviewed after the merges. Two of the six items from the last round are closed, though not by changes here. The base branch moved underneath the PR:

• 549b2a96 (#256) made the modifier guard case-insensitive, which closes the Control+z item outright.
• 83a23595 (#258) guards the empty icon-pair list, which removes the trigger for the session-wide undo kill. The mechanism is still there. generateIcons hand-rolls startTransaction and commitTransaction, and History.transaction(text, fn) with try/finally now exists on the base branch. Any throw between those two lines still leaves the transaction open and kills undo for the rest of the session, silently.

The other four are untouched. The last author commit is c4db8c0c (08-22), which predates the review; everything since is a merge.

One new blocker

It comes out of finding 14's own fix. Routing icon generation through History was the right call for the bypass, but serialize() calls generateIcons(), and History.commitTransaction truncates the redo stack before pushing. So on a blueprint carrying no icons, Ctrl+C destroys redo:

Load a blueprint with no icons. Make three edits. Undo three times, so redo holds three entries. Press Ctrl+C. All three are gone.

Same path for the share URL and the export dialog. blueprint-info-editor.spec.ts:285 pins the current behaviour, so that test has to move too. The fix is not to revert finding 14. It is to stop a read (serialize) from triggering a write.

Five changes close all of it

1. Move generateIcons() out of serialize().
2. Wrap what is left of generateIcons in history.transaction(...). One line.
3. Gate 'changed' in TextInput._onInputInput on the value actually having changed. A rejected keystroke currently writes position-relative-to-grid: {x:0, y:0} into a blueprint that never carried the key.
4. Stop refreshFromBlueprint clobbering a focused field, and cache or debounce the entity-event refresh. Typing into Absolute X and then placing an entity loses what you typed. Separately, with the dialog open, every remove-entity rebuilds two arrays sized to the whole blueprint.
5. Compare against the store's getter in the grid-position setter. pointsEqual(undefined, {x:0, y:0}) is false, so tabbing through Grid position without typing pushes an undo entry.

The measured half is right

The -floor(min corner) rule over footprint edges and tiles matches what the game does, and the snapToGrid && absoluteSnapping gate matches the fixture. Finding 15 is implemented exactly as measured, and the new specs each fail against the pre-fix code.