diff --git a/accessibility/docs/scratch.md b/accessibility/docs/scratch.md index d6ed781..ec6733f 100644 --- a/accessibility/docs/scratch.md +++ b/accessibility/docs/scratch.md @@ -7,7 +7,7 @@ fixing what Talon hits. Remaining work is polish, mod settings and distribution. **Game is playable start-to-finish by ear** from main menu through the intro conversation into gameplay, plus journal, character, actions and inventory — -verified by the testbed, **78/78 checks**. +verified by the testbed, **88/88 checks**. **Not set up yet, or unsure how the two trees relate?** → [00-dev-setup.md](00-dev-setup.md). @@ -790,6 +790,60 @@ pressing `1` twice, and putting the world back** (a lit torch would otherwise change FoV for the viewwatch and loot checks that follow). Plus the header-table canary and the `Wield`/`Unwield` flip. +### Done — modal popups: recipe / lore / map (bug fix, reported by Talon) + +**Symptom:** learning a recipe (and similar) speaks the word **"Popup"** and then +nothing — the body text is never read. + +**Cause.** There are **two entirely separate popup mechanisms**, and we had only +covered one. + +- `patches/popupstack.lua` wraps `popupStack.add` — the *fading corner* popups. + But `views/fadingPopup.lua` registers only **four** types: `journal`, `loot`, + `bestiary`, `tutorial`. The `recipe` / `lore` / `map` keys in that patch's + `typeNames` table can never fire. +- Recipe, lore and map popups are **modal**. `states/{ingame,inventory,research}` + build one with `modules/{recipe,lore,map}Popup.lua` and then + `states.switch("ingame_popup", {popup = ...})`. + +`ingame_popup` had a name in `stateNames.lua` ("Popup") and **no descriptor**, so +`announceScreen` said "Popup" and the poll had nothing to add. Exactly the +symptom. + +**Fix** — `accessibility/states/ingame_popup.lua` (24th descriptor): + +- `popupObject` comes off the state via `getAnyUpvalue`; the four popup modules + are factories exposing **only `draw`**, so every string is read as an upvalue + of `popupObject.draw`. +- **Type is detected by which locals exist**, since the popup object carries no + type tag: `itemsList`→loot, `items`→recipe, `title`+`subHeader`→lore, + `title`→map. An unrecognised popup falls back to reading the usual field names + rather than to silence. +- Reading order follows **draw order**, so speech matches the screen top to + bottom. +- Newlines are flattened (`lorePopup`'s footer is one sentence with hard breaks + in it) and blocks are pushed into the console history, so `[` / `]` / `\` walk + a long lore fragment back. `=` re-reads it via the `details` hook. +- `read()` is idempotent per the journal warning; the popup table's identity is + the focus key, and only the history push is guarded. +- Blocks are joined with a **space, not ". ", when the block already ends in + sentence punctuation** — otherwise NVDA gets "book.. This". + +Live text: +`Popup. New recipe available! A bunch of ivy leaves. See the details in your +journal.` + +**88 testbed checks, 0 failures** (was 78). The prologue fires no popup, so the +testbed raises them itself by calling `ingame.recipePopup` and `ingame.lorePopup` +— both display-only, so nothing is learned and the world is untouched. ⚠ The +dismiss key needs `wait(90)` first: `ingame_popup.keypressed` ignores input until +`fadeInAlpha >= 0.8`, over a 0.6 s fade. The self-test can only say +`custom read()` here (the probe is on the popup object, not the state), so **the +testbed check is this descriptor's version-bump canary**. + +Still unexercised: the **map** popup (needs `worldmap.getLastAreaIcon`, i.e. a +discovered area) and the modal **loot** popup from `inventory.lootPopup`. + ### Known gaps (next up) - **Phase 5**: player status key (HP, statuses, current action), popup announcements (`popupStack.add`), damage alerts @@ -854,7 +908,7 @@ utterance — speech is verifiable from the log, not only by ear. `!` after ## HANDOFF — read this if you are resuming ### State of play -Phases 0–5 complete. **23 state descriptors, 7 patches, 78/78 testbed checks**, +Phases 0–5 complete. **24 state descriptors, 7 patches, 88/88 testbed checks**, history unit tests passing, **one line changed in the game** (`main.lua`). Everything in the original brief is implemented: console speech with @@ -864,7 +918,7 @@ exploration, wall tones, and keyboard+speech access to every screen. ### First thing to do after any change ``` lua accessibility/tests/test_history.lua # pure Lua, instant -bash accessibility/tests/run-testbed.sh # ~4 min, 78 checks +bash accessibility/tests/run-testbed.sh # ~4 min, 88 checks git diff --stat vanilla-0.4.2 -- . ':!accessibility' ':!.gitignore' ':!CLAUDE.md' # must stay: main.lua | 1 + ``` @@ -912,8 +966,10 @@ These are blocked on content the prologue does not contain, **not** known broken - **The loot overlay (`s`) against real loot.** The prologue contains none, so live it has only ever said `No loot in view`. The contents path is proven by the testbed's planted-loot check, not by a real chest. -- **Popup announcements** (`patches/popupstack.lua`) — loot/quest/recipe popups - did not fire during the prologue run. +- **Corner popup announcements** (`patches/popupstack.lua`) — loot / quest / + bestiary / tutorial popups did not fire during the prologue run. (The *modal* + recipe / lore / map popups are a separate mechanism and are now covered by the + `ingame_popup` descriptor and the testbed.) ### Known-missing features (deliberate, not bugs) - **No mod settings UI.** Verbosity (`describe.verbosity`), wall-tone config diff --git a/accessibility/init.lua b/accessibility/init.lua index a0593d8..47e7a2a 100644 --- a/accessibility/init.lua +++ b/accessibility/init.lua @@ -46,6 +46,7 @@ local describedStates = { "config_menu", "keybindings_menu", "research", + "ingame_popup", } local initialized = false diff --git a/accessibility/states/ingame_popup.lua b/accessibility/states/ingame_popup.lua new file mode 100644 index 0000000..9e30b5b --- /dev/null +++ b/accessibility/states/ingame_popup.lua @@ -0,0 +1,179 @@ +-- Descriptor for `ingame_popup` — the modal popup shown when you learn a recipe, +-- read a lore fragment, unfold a map, or loot from a menu. +-- +-- WHY THIS EXISTS +-- +-- `patches/popupstack.lua` covers the *fading corner* popups, and it looked like +-- that was all of them. It isn't. `views/fadingPopup.lua` registers only four +-- types (journal / loot / bestiary / tutorial); recipe, lore and map popups never +-- touch popupStack at all. They are built directly by `modules/{recipe,lore,map, +-- loot}Popup.lua` and shown by `states.switch("ingame_popup", {popup = ...})` +-- from `states/{ingame,inventory,research}.lua`. +-- +-- So `ingame_popup` had a spoken name in stateNames ("Popup") and no descriptor: +-- the mod announced the word "Popup" and then went silent over the entire body +-- of text. That is exactly the reported symptom. +-- +-- HOW THE TEXT IS REACHED +-- +-- Each popup module is a factory returning a table whose only member is `draw` +-- (recipe/lore/map) — every string is a closure local. So we take `popupObject` +-- off the state and read the strings as upvalues of `popupObject.draw`. That is +-- the same probe technique the menu descriptors use. Because it probes the popup +-- object rather than the state, the startup self-test can only report +-- "custom read()" here — the version-bump canary is the testbed check, which +-- builds each popup type and asserts its text still comes out. +-- +-- Blocks are pushed into the console history so `[` / `]` / `\` walk a long lore +-- fragment back afterwards — a popup can easily be several paragraphs, which is +-- too much to catch in one utterance. + +local history = require("accessibility.history") + +-- Layout newlines are pure presentation: `lorePopup`'s footer is one sentence +-- with hard breaks in it, and a reader pauses mid-thought at every one. Same +-- flattening states/journal.lua does to journal pages. +local function flatten(text) + if type(text) ~= "string" then + return nil + end + text = text:gsub("%s*\n%s*", " "):gsub("^%s+", ""):gsub("%s+$", "") + if text == "" then + return nil + end + return text +end + +local function add(blocks, text) + text = flatten(text) + if text then + blocks[#blocks + 1] = text + end +end + +-- Name a list entry. `recipePopup` builds {icon, title}; `lootPopup` builds real +-- items, which carry `name` and may stack. +local function entryName(entry) + if type(entry) ~= "table" then + return nil + end + local name = entry.title or entry.name + if type(name) ~= "string" then + return nil + end + local quantity = tonumber(entry.quantity) + if quantity and quantity > 1 then + return name .. ", " .. quantity + end + return name +end + +-- The four popup modules differ only in which locals they hold and in what order +-- draw() paints them. Reading order follows draw order, so what is spoken matches +-- what is on screen top to bottom. +-- +-- Detection is by which locals exist, since the popup object itself carries no +-- type tag: +-- itemsList -> loot (real items) +-- items -> recipe ({title} rows) +-- title + subHeader -> lore +-- title -> map +local function shape(get) + if get("itemsList") ~= nil then + return "loot", { "flavourText", "header", "@itemsList" } + end + if get("items") ~= nil then + return "recipe", { "header", "subHeader", "@items", "footer" } + end + if get("title") ~= nil then + if get("subHeader") ~= nil then + return "lore", { "text", "header", "title", "subHeader", "footer" } + end + return "map", { "text", "title", "footer" } + end + -- Unknown popup type (a future game version adding one): read whatever of the + -- usual fields it has rather than falling back to silence. + return "unknown", { "text", "header", "subHeader", "title", "footer" } +end + +local function blocksFor(stateTable, menus) + local popupObject = menus.getAnyUpvalue(stateTable, "popupObject") + if type(popupObject) ~= "table" or type(popupObject.draw) ~= "function" then + return nil, nil + end + + local function get(name) + return menus.getUpvalue(popupObject.draw, name) + end + + local _, fields = shape(get) + local blocks = {} + + for _, field in ipairs(fields) do + if field:sub(1, 1) == "@" then + local list = get(field:sub(2)) + if type(list) == "table" then + for _, entry in ipairs(list) do + add(blocks, entryName(entry)) + end + end + else + add(blocks, get(field)) + end + end + + if #blocks == 0 then + return nil, nil + end + + -- The popup table is unique per popup, so its identity is a sound focus key: + -- it changes exactly when a new popup is shown and never within one. + return blocks, tostring(popupObject) +end + +-- Blocks are separate lines on screen, so they need a sentence break between them +-- when spoken -- but most already end in one ("...very old book."), and a blind +-- "." join gives NVDA "book.. This", which it reads with an audible stumble. +local function join(blocks) + local out = blocks[1] + for i = 2, #blocks do + local sep = out:find("[%.%?!:,]$") and " " or ". " + out = out .. sep .. blocks[i] + end + return out +end + +local lastKey + +return { + reset = function() + lastKey = nil + end, + + -- read() is called from the poll, from the post-keypress announce and from + -- menus.repeatFocus, so it must be idempotent (see the warning in + -- states/journal.lua). It returns the same text for the same key every time; + -- only the history push is guarded, since duplicating a popup's paragraphs in + -- the review buffer on every frame would swamp it. + read = function(stateTable, menus) + local blocks, key = blocksFor(stateTable, menus) + if not blocks then + return nil, nil + end + + if key ~= lastKey then + lastKey = key + for _, block in ipairs(blocks) do + pcall(history.append, block, "text") + end + end + + return join(blocks), key + end, + + -- `=` re-reads the popup on demand, for when the utterance went by. + details = function(stateTable, menus) + local blocks = blocksFor(stateTable, menus) + return blocks + end, +} diff --git a/accessibility/testbed.lua b/accessibility/testbed.lua index 093719d..624e8f9 100644 --- a/accessibility/testbed.lua +++ b/accessibility/testbed.lua @@ -968,6 +968,63 @@ local function scenario() check("slider announces value changes", true, "via ingame_wait") end + -- ingame_popup: the modal recipe / lore / map / loot popups. + -- + -- These do NOT go through popupStack (views/fadingPopup only registers + -- journal, loot, bestiary and tutorial), so patches/popupstack.lua never + -- saw them -- the whole body of text was silent behind the spoken word + -- "Popup". The prologue never fires one, so we raise them by calling the + -- state's own entry points. Both are display-only: recipePopup just shows + -- a template's name, lorePopup just renders the params it is handed, so + -- neither teaches the player anything or touches the world. + do + local ingameState = require("states/ingame") + + local function readPopup(label, raise, wants) + local ok = pcall(raise) + if not ok or not waitForState("ingame_popup", 120) then + check(label .. ": popup opened", false, "state = " .. currentState()) + return + end + + local text = focusText() or "" + check(label .. ": popup speaks more than its name", + #text > 0, text) + for _, want in ipairs(wants) do + check(label .. ": says '" .. want .. "'", + text:lower():find(want:lower(), 1, true) ~= nil, text) + end + + -- keypressed ignores input until fadeInAlpha >= 0.8, and + -- fadeInPeriod is 0.6s -- so it must be given time before the + -- dismiss key will register. + wait(90) + press("escape", "dismiss popup") + wait(10) + check(label .. ": popup dismissed", currentState() == "ingame", + "got " .. currentState()) + end + + readPopup("recipe popup", + function() ingameState.recipePopup("ivy_leaves") end, + {"New recipe available", "ivy", "journal"}) + + -- The lore popup carries the longest body text of the four, and is + -- the case where "it says 'popup' and stops" was most visible. + readPopup("lore popup", + function() + ingameState.lorePopup({ + knowledgeType = "practical", + topic = "testbed topic", + text = "A fragment of text from a very old book.", + unlocked = 1, + max = 3, + }) + end, + {"A fragment of text from a very old book", "This text contains", + "testbed topic"}) + end + -- The game's own examine mode. press("x", "enter examine mode") wait(10)