maniator / maniator/verticopolis

[P3] Menu undo is tower-scoped while the design docs justify themselves by protecting the rename field

Open
#722 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
TypeScript
Stars
5
Forks
1
PR merge metrics
No merged PRs in 30d

Description

Deferred from the /gds-code-review of the desktop command seam (PR #715), rounds 3 and 4.

src/game/hostCommands.ts routes undo and redo straight to app.ui.cb.onUndo() / onRedo(), with no equivalent of the ownsNativeUndo yield that src/game/inputKeys.ts:56 implements for the keyboard path. So with the tower-rename field focused, the menu's Edit > Undo rolls back a tower action instead of the text edit.

Why this is not simply a bug

hostCommands.ts documents the divergence as deliberate: a menu item is a global application affordance, a keystroke is field-local by convention, and the toolbar's undo arrow is tower-scoped too. That reasoning is sound on its own.

The problem is that three other documents justify the entire no-accelerator design by pointing at this exact case. desktop/shell/src/menu.ts, the shell README.md, and the story's Hazard 2 all say the Edit menu declares no CommandOrControl+Z specifically so the game can keep yielding Ctrl+Z to the tower-rename field. Clicking the menu item does the thing those documents call unacceptable.

So one of two things is true and the codebase should say which:

  1. Menu undo being tower-scoped is correct, and the no-accelerator rationale should stop citing the rename field as its reason (the accelerator-collision argument stands on its own without it).
  2. The rename field genuinely needs protecting from undo whatever the source, in which case dispatch needs the ownsNativeUndo check and OPENS_A_DIALOG is the wrong set to hang it on.
Also in scope

While deciding: undo and redo currently run during a live editor press, because the editor-busy guard only refuses OPENS_A_DIALOG commands. The stated reason for that guard is that a card must not move under a held pointer, and an undo can mutate or remove the facility the press is acting on, which is arguably worse than the stats refresh the frame loop already declines in that state.

Review layers flagged this in rounds 3 and 4. Not blocking #715: it requires a product decision, not a fix.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Read src/game/hostCommands.ts and src/game/inputKeys.ts:56 first, then compare the rationale in desktop/shell/src/menu.ts, the shell README.md, and the story's Hazard 2. Trace undo and redo during a focused rename field and live editor press. Done means the project makes one consistent decision about menu scope, editor-busy behavior, and the documentation that explains it.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
desktop, game-dev
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.