diff --git a/crates/benilla-app/assets/ui/CraftFrame.xml b/crates/benilla-app/assets/ui/CraftFrame.xml index 7c689a16..8c3f8166 100644 --- a/crates/benilla-app/assets/ui/CraftFrame.xml +++ b/crates/benilla-app/assets/ui/CraftFrame.xml @@ -104,21 +104,24 @@ out of scope (`craft.rs`'s own note). 7. ENGINE API gaps (TradeSkillFrame.xml's own "no engine equivalent" posture, restated per substitution actually used here): - - `Button:SetTextColor`/`:LockHighlight`/`:UnlockHighlight` have no native method (grepped; - `benilla-ui/src/script/button.rs` ships texture-pairs/SetText/Enable/Disable/ - RegisterForClicks/SetButtonState/Click only) — row coloring routes through a named child - FontString (`$parentName`/`$parentSubText`) instead of the button's own inherited text - region (the ref's `CraftText`/`CraftSubText`, `ClassTrainerSkillButtonTemplate`'s - auto-created text object); the persistent selection glow is the separate - `BenillaCraftHighlightFrame` (TradeSkillFrame.xml's own selection-highlight precedent) - instead of Lock/UnlockHighlight — no independent hover glow beyond it, and - `CraftButtonTemplate` never overrides `OnEnter` for a tooltip either — this window's list - rows carry NO hover tooltip at all, faithfully. (An earlier revision of this line added - "TradeSkill's own row hover IS a real ref behavior; Craft's is not" — WRONG, and never - checked: `TradeSkillSkillButtonTemplate` overrides `OnClick` alone, and the shared base + - (CLOSED — the row name is the button's own label now.) An earlier revision recorded + `Button:SetTextColor`/`:LockHighlight`/`:UnlockHighlight` as having no engine method and + routed row colouring through a `$parentName` child FontString. The engine has shipped + all three for a long time; the note went stale and the workaround outlived it, costing + this window the white hover/selected label the real client has (decision 1605). The row + name is now a `` (the ref's `CraftText`) under the base's + `//` trio, and Update calls `SetTextColor` / + `LockHighlight` / `UnlockHighlight` where the ref does (l.190/234/236). `$parentSubText` + stays a child FontString, as it is in the ref, and is whitened by hand for the selected + row (l.232). NOT taken: the ref's per-row `craftButton.r/g/b` stash and the base's own + `OnEnter`/Craft's `OnLeave` subtext recolour dance (l.191-193, xml l.22-26) — with + Beast Training out of scope (deviation 6) `craftSubSpellName` is always `""`, so every + one of those paints an empty string. The row still carries NO hover tooltip: the ref + has none either. (An earlier revision of this line added "TradeSkill's own row hover IS + a real ref behavior; Craft's is not" — WRONG about *tooltips*, and never checked: + `TradeSkillSkillButtonTemplate` overrides `OnClick` alone, and the shared base `ClassTrainerSkillButtonTemplate` (`Interface\FrameXML\ClassTrainerFrameTemplates.xml`) - only recolours `$parentSubText` on hover. NEITHER window's rows tooltip. TradeSkill's - stray OnEnter is gone; see that file's row-template comment.) + only recolours `$parentSubText` on hover. NEITHER window's rows tooltip.) - `Button:GetID()`/`:SetID()` have no native method (same grep) — the ref's `craftButton:SetID(craftIndex)` + `this:GetID()` pair becomes a plain Lua field, `button.craftIndex`, set in the Update loop and read by the click handler @@ -357,12 +360,13 @@ button.craftIndex = craftIndex local color = CraftTypeColor[craftType] if color then - button.name:SetTextColor(color.r, color.g, color.b) + button:SetTextColor(color.r, color.g, color.b) -- ref l.190 + button.subtext:SetTextColor(color.r, color.g, color.b) -- ref l.195 end if (numAvailable or 0) == 0 then - button.name:SetText(" " .. craftName) + button:SetText(" " .. craftName) else - button.name:SetText(" " .. craftName .. " [" .. numAvailable .. "]") + button:SetText(" " .. craftName .. " [" .. numAvailable .. "]") end if craftSubSpellName and craftSubSpellName ~= "" then button.subtext:SetText(format(PARENS_TEMPLATE, craftSubSpellName)) @@ -370,9 +374,19 @@ button.subtext:SetText("") end button:Show() + -- TWO things paint a selected row, and the ref does both here (l.229-236): the + -- glow bar is BenillaCraftHighlightFrame, and the WHITE LABEL is the row's own + -- LockHighlight() — the template's `` + -- instance winning over the difficulty colour SetTextColor just wrote, which + -- reaches the NORMAL instance only. The subtext is whitened by hand because it is + -- a child FontString, not the button's label, exactly as the ref does it (l.232). if selection == craftIndex then highlight:SetPoint("TOPLEFT", button, "TOPLEFT", 0, 0) highlight:Show() + button:LockHighlight() + button.subtext:SetTextColor(1.0, 1.0, 1.0) + else + button:UnlockHighlight() end else button.craftIndex = nil @@ -567,19 +581,29 @@ - - - - + + + + + + + + - self.name = getglobal(self:GetName() .. "Name") self.subtext = getglobal(self:GetName() .. "SubText") BenillaCraftSkillButton_OnClick(self, arg1) diff --git a/crates/benilla-app/assets/ui/TradeSkillFrame.xml b/crates/benilla-app/assets/ui/TradeSkillFrame.xml index b2424378..271f2605 100644 --- a/crates/benilla-app/assets/ui/TradeSkillFrame.xml +++ b/crates/benilla-app/assets/ui/TradeSkillFrame.xml @@ -110,19 +110,22 @@ onto the ONE input-box instance in `BenillaTradeSkillFrame_OnLoad` (`function box:SetNumber(n) self:SetText(tostring(floor(n or 0))) end`), so every call site below still reads `TradeSkillInputBox:SetNumber(...)` exactly like the reference. - - `Button:SetTextColor`/`:LockHighlight`/`:UnlockHighlight` have no native method (grepped; - `benilla-ui/src/script/button.rs` ships texture-pairs/SetText/Enable/Disable/ - RegisterForClicks/SetButtonState/Click only) — row coloring routes through a named child - FontString (`$parentName`, TrainerFrame.xml's own row-template shape) instead of the - button itself; the persistent selection glow is the separate - `BenillaTradeSkillHighlightFrame` (TrainerFrame.xml's own selection-highlight precedent) - instead of Lock/UnlockHighlight — no independent hover glow is added beyond it - (TrainerFrame's own rows carry none either). **The Frame is the thing shown, hidden and + - (CLOSED — the row is the reference's shape now.) An earlier revision of this bullet + recorded `Button:SetTextColor`/`:LockHighlight`/`:UnlockHighlight` as having no engine + method, and routed row colouring through a child `$parentName` FontString instead of + the button's own label. The engine has shipped all three for a long time; the note went + stale and the workaround outlived it, which cost this window the white hover/selected + label the real client has (decision 1605). The row now carries a `` under + the base's `//` trio, Update calls + `button:SetTextColor` / `LockHighlight` / `UnlockHighlight` exactly where the ref does + (l.113/127/144/146), and the white text is the HighlightFont instance, not a repaint. + **The selection is still TWO things.** The orange bar is + `BenillaTradeSkillHighlightFrame` — and **the Frame is what is shown, hidden and positioned; `BenillaTradeSkillHighlight` is only its texture, and only ever gets - `SetVertexColor`** (ref l.99/142-143 vs l.200) — Update addressed the texture for all + `SetVertexColor`** (ref l.99/142-143 vs l.200). Update addressed the texture for all four until decision 1598, which left the frame `hidden="true"` for the window's whole - life and meant NO selection ever painted. CraftFrame.xml/TrainerFrame.xml already split - the two correctly; this window was the outlier. + life and meant NO selection ever painted. The white label is the row's own highlight + lock, and the two are independent: 1598 restored the bar and the row stayed unlit. - `SecondsToTime`/`BuildColoredListString` (ref `UIParent.lua`) have no engine equivalent (they're Lua-side FrameXML utilities, not C API) — transcribed as GUARDED GLOBALS (`if not X then function X(...) ... end end`), BuffFrame.xml's own @@ -441,16 +444,13 @@ button.skillIndex = skillIndex local color = TradeSkillTypeColor[skillType] if color then - button.name:SetTextColor(color.r, color.g, color.b) + button:SetTextColor(color.r, color.g, color.b) end -- Header row (ref l.118-127, wow-re `tradeskill` TU-B): the group name -- verbatim, no "[n]" suffix, the +/- fold icon + its hover glow swapped on - -- isExpanded. UnlockHighlight() (ref l.127) is dropped — no native - -- Lock/UnlockHighlight (this file's header comment, deviation 7); the - -- persistent selection glow below is the separate BenillaTradeSkillHighlight - -- texture, never this button's own highlight lock. + -- isExpanded. if skillType == "header" then - button.name:SetText(skillName) + button:SetText(skillName) if isExpanded then button:SetNormalTexture("Interface\\Buttons\\UI-MinusButton-Up") else @@ -461,17 +461,29 @@ button:SetNormalTexture("") getglobal(button:GetName() .. "Highlight"):SetTexture("") if (numAvailable or 0) == 0 then - button.name:SetText(" " .. skillName) + button:SetText(" " .. skillName) else - button.name:SetText(" " .. skillName .. " [" .. numAvailable .. "]") + button:SetText(" " .. skillName .. " [" .. numAvailable .. "]") end end button:Show() -- The selection glow only ever lands on a recipe row — SetSelection never -- selects a header (deviation 1), so `selection` can't be one's index. + -- + -- TWO separate things paint a selected row, and the ref does both here + -- (l.142-146): the orange bar is BenillaTradeSkillHighlightFrame, and the + -- WHITE LABEL is the row's own LockHighlight() — the base's + -- `` over the difficulty colour + -- SetTextColor just wrote. The lock reaches nothing else on a recipe row: the + -- branch above blanked that row's HighlightTexture. Unlocking every other row + -- covers both of the ref's own UnlockHighlight calls at once — l.146's for a + -- passed-over recipe and l.127's for a header, which can never be `selection`. if selection == skillIndex then highlight:SetPoint("TOPLEFT", button, "TOPLEFT", 0, 0) highlight:Show() + button:LockHighlight() + else + button:UnlockHighlight() end end else @@ -827,17 +839,16 @@ own row hover IS a real ref behavior") on the strength of it. Both are corrected: no OnEnter. Since `$parentSubText` is empty for every row of this window, the base's own hover recolour is invisible here too, so the row carries no hover script at all. + + **The row still lights up on hover, and no script does it.** That is the base's + `` — the button's own highlighted font instance, + swapped in by the engine while the cursor is on the row and held by `LockHighlight()` on the + selected one. Reading 1598's "bare on hover" as "nothing happens on hover" is the trap this + paragraph exists to close: what the reference has no trace of is a *tooltip*, not the white + label (decision 1605). --> diff --git a/crates/benilla-app/assets/ui/TrainerFrame.xml b/crates/benilla-app/assets/ui/TrainerFrame.xml index 4aff9813..e8b1bebd 100644 --- a/crates/benilla-app/assets/ui/TrainerFrame.xml +++ b/crates/benilla-app/assets/ui/TrainerFrame.xml @@ -276,9 +276,15 @@ if index <= numServices then local name, subText, serviceType, isExpanded = GetTrainerServiceInfo(index) if not name then name = UNKNOWN_LABEL end + -- Every row is unlocked here and the selected one re-locked below (ref l.183/190). + -- The lock is what paints a selected row's name white: the template's + -- `` instance wins over the state + -- colour SetTextColor writes, which reaches the NORMAL instance only. It reaches + -- nothing else on a service row — the branch below blanks that row's fold glow. + button:UnlockHighlight() if serviceType == "header" then - button.name:SetText(name) - button.name:SetTextColor(NORMAL_FONT_COLOR.r, NORMAL_FONT_COLOR.g, NORMAL_FONT_COLOR.b) + button:SetText(name) + button:SetTextColor(NORMAL_FONT_COLOR.r, NORMAL_FONT_COLOR.g, NORMAL_FONT_COLOR.b) button.subtext:SetText("") if isExpanded then button:SetNormalTexture("Interface\\Buttons\\UI-MinusButton-Up") @@ -289,20 +295,24 @@ else button:SetNormalTexture("") button.foldHighlight:SetTexture("") - button.name:SetText(" " .. name) + button:SetText(" " .. name) if subText and subText ~= "" then button.subtext:SetText(format(PARENS_TEMPLATE, subText)) else button.subtext:SetText("") end + -- The rank rides the NAME's right edge, wherever the name ended (ref l.157, + -- B253's flow row) — re-anchored every repaint because the name's width is + -- the thing that changes. + button.subtext:SetPoint("LEFT", getglobal(button:GetName() .. "Text"), "RIGHT", 10, 0) if serviceType == "available" then - button.name:SetTextColor(0, 1.0, 0) + button:SetTextColor(0, 1.0, 0) button.subR, button.subG, button.subB = 0, 0.6, 0 elseif serviceType == "used" then - button.name:SetTextColor(GRAY_FONT_COLOR.r, GRAY_FONT_COLOR.g, GRAY_FONT_COLOR.b) + button:SetTextColor(GRAY_FONT_COLOR.r, GRAY_FONT_COLOR.g, GRAY_FONT_COLOR.b) button.subR, button.subG, button.subB = 0.5, 0.5, 0.5 else - button.name:SetTextColor(0.9, 0, 0) + button:SetTextColor(0.9, 0, 0) button.subR, button.subG, button.subB = 0.6, 0, 0 end button.subtext:SetTextColor(button.subR, button.subG, button.subB) @@ -311,7 +321,8 @@ button:Show() else button.serviceIndex = nil - button.name:SetText("") + button:UnlockHighlight() + button:SetText("") button.subtext:SetText("") button:SetNormalTexture("") button.foldHighlight:SetTexture("") @@ -341,12 +352,15 @@ end hl:SetPoint("TOPLEFT", selButton, "TOPLEFT", 0, 0) hl:Show() - -- The selected row's text goes white (HIGHLIGHT) — a deliberate divergence from the ref, - -- which whitens only the subtext + LockHighlight and leaves the name state-coloured: the - -- director wants the whole selected row legible against its colour glow (a red/gray glow - -- swallows red/gray text). Runs AFTER the per-row state colouring above, so it wins for - -- the selected row; subR/subG/subB go white too so a hover-then-leave keeps it white. - selButton.name:SetTextColor(HIGHLIGHT_FONT_COLOR.r, HIGHLIGHT_FONT_COLOR.g, HIGHLIGHT_FONT_COLOR.b) + -- The whole selected row goes white, and it is the ref's own pair of moves (l.183-184): + -- LockHighlight for the NAME (the button's label, so the white comes from its + -- HighlightFont instance, not from a repaint that the next Update would undo) and an + -- explicit whiten for the SUBTEXT, which is a child FontString the lock cannot reach. + -- An earlier revision called the white name a deliberate divergence "the ref does not + -- do"; the ref does it, through the lock this window had no engine method for at the + -- time (decision 1605). subR/subG/subB go white too so a hover-then-leave keeps the + -- subtext white. + selButton:LockHighlight() selButton.subtext:SetTextColor(HIGHLIGHT_FONT_COLOR.r, HIGHLIGHT_FONT_COLOR.g, HIGHLIGHT_FONT_COLOR.b) selButton.subR, selButton.subG, selButton.subB = HIGHLIGHT_FONT_COLOR.r, HIGHLIGHT_FONT_COLOR.g, HIGHLIGHT_FONT_COLOR.b @@ -558,14 +572,33 @@ - - - - - - + + + + + + + + + @@ -578,7 +611,6 @@ - self.name = getglobal(self:GetName() .. "Name") self.subtext = getglobal(self:GetName() .. "SubText") self.foldHighlight = self:GetHighlightTexture() diff --git a/crates/benilla-app/src/lib.rs b/crates/benilla-app/src/lib.rs index 48496b02..00569479 100644 --- a/crates/benilla-app/src/lib.rs +++ b/crates/benilla-app/src/lib.rs @@ -158,6 +158,7 @@ mod ui_unit; mod ui_world_map; mod video; mod vplates; +mod world_backdrop; mod world_state; mod world_state_ui; @@ -224,6 +225,7 @@ use ui_trade::UiTradePlugin; use ui_tradeskill::UiTradeSkillPlugin; use ui_trainer::UiTrainerPlugin; use ui_unit::UiUnitPlugin; +use world_backdrop::WorldBackdropPlugin; // The `benilla` launcher shim (the bin package) is this library's only caller: it stamps the // build id at compile time and hands it into [`run`]. Re-exported so the shim needs no bevy @@ -532,6 +534,10 @@ pub fn run(build: BuildId) -> AppExit { // The player-UI quad pass (decision 0068 §2): its own composited-above-the-world, // below-the-egui-dev-overlays camera + sorted-quad renderer. `$WOW_UI_DEMO=1` seeds a proof scene. .add_plugins(PlayerUiPlugin) + // The world's frame, rendered off-screen and handed to the UI pass as its first quad — the + // seam that puts the UI-over-world blend back into gamma bytes (0161/0254's last piece). + // Registered AFTER the UI pass: it writes `UiQuads`, which that plugin owns. + .add_plugins(WorldBackdropPlugin) // The HUD minimap (decision 0203 phase 1): fills the `` widget's extracted hole with // the streamed tile window + mask + player arrow, and feeds the zone text. .add_plugins(minimap::MinimapPlugin) diff --git a/crates/benilla-app/src/ui_pass.rs b/crates/benilla-app/src/ui_pass.rs index 70e0799a..b013a9a2 100644 --- a/crates/benilla-app/src/ui_pass.rs +++ b/crates/benilla-app/src/ui_pass.rs @@ -276,6 +276,17 @@ impl Default for UiQuad { /// whole UI re-batched every frame (the 0365 live-city churn). #[derive(Resource, Default)] pub(crate) struct UiQuads { + /// The **world backdrop** ([`crate::world_backdrop`]): the frame the world camera rendered, + /// drawn before every other quad so the UI blends over it in the same gamma bytes it blends + /// over itself in. Not part of either lane and not sorted with them — it is not content + /// competing for a `z_key`, it is the ground. `None` whenever there is no world to paint (the + /// glue screens, the loading screen, a gated camera). + /// + /// It changes no batching decision beyond being first, and it flags [`Self::dirty`] only when + /// the QUAD changes — arrival, departure, a resize. Its image's contents change every frame and + /// deliberately do not flag anything: the batch holds the handle and the material samples + /// whatever the world camera just rendered into it. + pub backdrop: Option, pub quads: Vec, /// The append lane — see the struct doc. Compared by the rebuild, never flagged. pub overlays: Vec, @@ -566,13 +577,21 @@ fn spawn_ui_camera(mut commands: Commands) { Camera { order: UI_CAMERA_ORDER, output_mode: CameraOutputMode::Write { - // PREMULTIPLIED, not `ALPHA_BLENDING`: `ui_quad.wgsl` writes premultiplied colour - // (rgb·a), so a `SrcAlpha` factor here would weight it by alpha a SECOND time — - // translucent UI over the world came out `rgb·a²`, and a pure-additive quad (a = 0) - // over the world was multiplied clean away. Opaque panels (a = 1) hid it. - blend_state: Some( - bevy::render::render_resource::BlendState::PREMULTIPLIED_ALPHA_BLENDING, - ), + // **No blend at all** — this camera now carries the world too + // ([`crate::world_backdrop`]), so its target is a whole opaque frame and the blit + // is a copy. That is the point: the blend that used to happen HERE, against the + // sRGB swapchain view, was the frame's one linear composite, and it was the only + // one that mixed UI with world. Moving the world into the UI's own byte buffer + // moves that blend into `ui_quad.wgsl`'s gamma target, where every other UI blend + // already lives. + // + // It also retires the hazard 0254 patched around here: `ui_quad.wgsl` writes + // PREMULTIPLIED colour, so the original `ALPHA_BLENDING`'s `SrcAlpha` factor + // weighted it by alpha twice (`rgb·a²`), and a pure-additive quad (a = 0) over the + // world was multiplied clean away. `PREMULTIPLIED_ALPHA_BLENDING` fixed the + // arithmetic but kept the blend — and kept it in the wrong space. With nothing to + // blend against, neither factor can be wrong. + blend_state: None, clear_color: ClearColorConfig::None, }, // An overlay must composite ONLY its own pixels. `ClearColorConfig::None` made this @@ -884,23 +903,27 @@ fn rebuild_ui_mesh( let (meshes, materials) = (&mut stores.0, &mut stores.1); let q = quads.as_mut(); // TOGGLEUI hides at the *draw*, not at the producers: both lanes keep filling, so the UI comes - // back exactly as it was (see [`crate::ui_hide::UiHidden`]). Retire the batches on the toggle's - // down edge; on the way back up, force one full rebuild — while dark we swallow each frame's - // change flag and keep the append-lane mirror current, so neither lane can hand the rebuild a - // stale "nothing changed" the moment the UI returns. The edge is the resource's own change tick - // (`UiHidden` is written only by the binding and the world-exit reset), not a `Local` mirror. - if hidden.0 { - if hidden.is_changed() { - retire_batches(&mut pools, &mut commands); - } - q.dirty = false; - q.last_overlays.clone_from(&q.overlays); - return; - } + // back exactly as it was (see [`crate::ui_hide::UiHidden`]). + // + // **It hides the two LANES, never the backdrop.** The world reaches the screen as this pass's + // own first quad now ([`crate::world_backdrop`]), so the older "retire every batch while + // hidden" would black the screen — the exact inverse of a binding whose stated point is to + // leave "the world and nothing else". While dark we still swallow each frame's change flag and + // keep the append-lane mirror current, so neither lane can hand the rebuild a stale "nothing + // changed" the moment the UI returns. The edge is the resource's own change tick (`UiHidden` is + // written only by the binding and the world-exit reset), not a `Local` mirror. if hidden.is_changed() { q.dirty = true; } - if !q.dirty && q.overlays == q.last_overlays { + let lanes_hidden = hidden.0; + if lanes_hidden { + q.last_overlays.clone_from(&q.overlays); + // Nothing either lane produces can move a pixel while dark, so only the toggle edge and + // the backdrop's own arrival/departure (which sets `dirty`) reach the rebuild below. + if !q.dirty { + return; + } + } else if !q.dirty && q.overlays == q.last_overlays { return; } // `WOW_UI_DIFF=1` — WHO re-triggers the rebuild? Names the first differing overlay quad (or @@ -947,8 +970,11 @@ fn rebuild_ui_mesh( retire_batches(&mut pools, &mut commands); return; }; - if q.quads.is_empty() && q.overlays.is_empty() { + let lanes_empty = lanes_hidden || (q.quads.is_empty() && q.overlays.is_empty()); + if q.backdrop.is_none() && lanes_empty { retire_batches(&mut pools, &mut commands); + q.dirty = false; + q.last_overlays.clone_from(&q.overlays); return; } @@ -961,8 +987,19 @@ fn rebuild_ui_mesh( // Stable sort by z_key: the WoW-style total order. Stable so equal-z_key quads keep the producer's // original relative order (their own decl-order tiebreak, if any). Base lane first, append lane // after — the same relative order the one-Vec era produced. - let mut sorted: Vec<&UiQuad> = q.quads.iter().chain(q.overlays.iter()).collect(); + let mut sorted: Vec<&UiQuad> = if lanes_hidden { + Vec::new() + } else { + q.quads.iter().chain(q.overlays.iter()).collect() + }; sorted.sort_by_key(|q| q.z_key); + // The backdrop is PREPENDED, not sorted in. Giving it a `z_key` would mean picking a number + // below every other producer's and trusting all of them to stay above it — and the append + // lane's lowest band is already 0 (`overlay_z::WORLD_TEXT`), so there is no room under it + // without renumbering a total order that encodes fidelity facts. Position, not arithmetic. + if let Some(backdrop) = q.backdrop.as_ref() { + sorted.insert(0, backdrop); + } // Geometry probe (`WOW_UI_PROBE=1`): dump each textured quad's screen rect once — the // capture-harness companion for diagnosing extracted-vs-rendered geometry by data instead of @@ -1426,6 +1463,60 @@ mod tests { ); } + /// **TOGGLEUI hides the UI, not the world.** Since the world arrives as this pass's own + /// backdrop quad ([`crate::world_backdrop`]), the dark path can no longer mean "retire every + /// batch" — that would black the screen, which is the exact inverse of what the binding is + /// for. One batch survives while dark (the backdrop's), and the lanes come back on top of it. + /// + /// This is the regression the change itself created: every earlier version of the hidden path + /// asserted zero batches, and zero batches is now a black screen. + #[test] + fn toggleui_keeps_the_world_backdrop() { + let mut app = rebuild_app(); + let backdrop = UiQuad { + rect: Rect::from_corners(Vec2::ZERO, Vec2::new(800.0, 600.0)), + texture: None, + ..UiQuad::default() + }; + { + let mut q = app.world_mut().resource_mut::(); + q.backdrop = Some(backdrop); + q.dirty = true; + } + app.update(); + let lit = batches(&mut app); + assert!(lit >= 1, "content drawn to begin with"); + + set_hidden(&mut app, true); + app.update(); + assert_eq!( + batches(&mut app), + 1, + "dark ⇒ the backdrop alone; anything less is a black screen" + ); + + set_hidden(&mut app, false); + app.update(); + assert_eq!( + batches(&mut app), + lit, + "the UI comes back over the same world" + ); + } + + /// With no world to paint (the glue screens, the loading screen, a gated camera) the dark path + /// is the old one: nothing at all. The backdrop earns an exemption because it IS the world, + /// not because it is first in the list. + #[test] + fn toggleui_with_no_backdrop_still_retires_everything() { + let mut app = rebuild_app(); + app.update(); + assert_eq!(batches(&mut app), 1, "one batch drawn to begin with"); + set_hidden(&mut app, true); + app.update(); + assert_eq!(batches(&mut app), 0, "no world, no backdrop, nothing drawn"); + } + /// **The desaturation flag reaches the MATERIAL, and splits the run** (decision 1327). /// /// Everything upstream of here can be right — the Lua sets it, extract carries it, the quad diff --git a/crates/benilla-app/src/ui_script/trainer_tests.rs b/crates/benilla-app/src/ui_script/trainer_tests.rs index e25ea8af..cd55d74f 100644 --- a/crates/benilla-app/src/ui_script/trainer_tests.rs +++ b/crates/benilla-app/src/ui_script/trainer_tests.rs @@ -230,12 +230,12 @@ fn shipped_trainer_frame_drives_end_to_end() { // Row 1 renders the "Arms" header (its name, no indent); row 2 the first service. assert_eq!( - s.eval::("return BenillaTrainerService1Name:GetText()") + s.eval::("return BenillaTrainerService1Text:GetText()") .unwrap(), "Arms" ); assert_eq!( - s.eval::("return BenillaTrainerService2Name:GetText()") + s.eval::("return BenillaTrainerService2Text:GetText()") .unwrap(), " Heroic Strike" ); @@ -318,7 +318,7 @@ fn clicking_a_header_row_collapses_its_group() { // Full tree: 2 headers + 4 services = 6 rows; row 2 is Heroic Strike. assert_eq!(s.eval::("return GetNumTrainerServices()").unwrap(), 6); assert_eq!( - s.eval::("return BenillaTrainerService2Name:GetText()") + s.eval::("return BenillaTrainerService2Text:GetText()") .unwrap(), " Heroic Strike" ); @@ -329,7 +329,7 @@ fn clicking_a_header_row_collapses_its_group() { .unwrap(); assert_eq!(s.eval::("return GetNumTrainerServices()").unwrap(), 4); assert_eq!( - s.eval::("return BenillaTrainerService2Name:GetText()") + s.eval::("return BenillaTrainerService2Text:GetText()") .unwrap(), "Fury", "Arms folded; its header (row 1) now abuts the Fury header (row 2)" @@ -475,11 +475,11 @@ fn wheel_over_a_row_scrolls_the_list() { s.fire_event("TRAINER_SHOW", vec![ScriptValue::Str("Sana".into())]); assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); - let row1 = "return BenillaTrainerService1Name:GetText()"; + let row1 = "return BenillaTrainerService1Text:GetText()"; // At the top: row 1 is the "Arms" header, row 2 the first service. assert_eq!(s.eval::(row1).unwrap(), "Arms"); assert_eq!( - s.eval::("return BenillaTrainerService2Name:GetText()") + s.eval::("return BenillaTrainerService2Text:GetText()") .unwrap(), " Service 01" ); @@ -506,17 +506,35 @@ fn wheel_over_a_row_scrolls_the_list() { assert_eq!(s.eval::(row1).unwrap(), " Service 01"); } -/// The selected service row's text renders white (HIGHLIGHT), legible against its colour glow — a -/// deliberate divergence from the ref (which whitens only the subtext, keeping the name state-coloured). -/// Selecting the unavailable Cleave (red when unselected) whitens its name; selecting away restores it. +/// The selected service row's name renders white (HIGHLIGHT), legible against its colour glow — and +/// so does a HOVERED one. Both are the reference's own behaviour, through one mechanism: the row's +/// name is the button's `` under a ``, the +/// engine swaps that instance in while the cursor is on the row, and `LockHighlight()` pins it for +/// the selection (Blizzard_TrainerUI.lua l.183). `SetTextColor` writes the NORMAL instance only, so +/// the state colour cannot follow the label into either state — which is exactly why it goes white. +/// +/// An earlier revision of this test called the white name "a deliberate divergence from the ref +/// (which whitens only the subtext)". The ref whitens the subtext *by hand* precisely BECAUSE the +/// subtext is a child FontString the lock cannot reach; the name it leaves to the lock. Ours could +/// not, because the name was a child FontString too and the engine's highlighted label fell back to +/// the normal state's colour — so the white was hand-painted here and absent on hover entirely +/// (decision 1605). #[test] -fn selected_service_row_text_is_white() { +fn a_selected_or_hovered_service_row_paints_its_name_white() { let mut s = trainer_script(); s.set_money(50); s.set_trainer(Some(menu())); s.fire_event("TRAINER_SHOW", vec![ScriptValue::Str("Sana".into())]); assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); + // The name really is the button's own label now — a child FontString would leave + // GetFontString() nil and no per-state font could reach it. + assert!( + s.eval::("return BenillaTrainerService1:GetFontString() ~= nil") + .unwrap(), + "the row name is the Button's ButtonText, the only region per-state fonts reach" + ); + // Cleave (index 3) is unavailable → red when not selected. Select it → its name goes white. s.run("SelectTrainerService(3); BenillaTrainerFrame_Update()") .unwrap(); @@ -534,6 +552,31 @@ fn selected_service_row_text_is_white() { text_has_color(&s.extract(), "Cleave", [0.9, 0.0, 0.0]), "an unselected unavailable row is red again" ); + + // Now hover it, selecting nothing: the HighlightFont instance takes over and the row lights up. + let (x, y) = text_center(&s.extract(), "Cleave"); + s.mouse_move(x, y); + s.resolve(); + assert!( + text_has_color(&s.extract(), "Cleave", [1.0, 1.0, 1.0]), + "a hovered row's name renders white, with no script doing it" + ); + assert!( + text_has_color(&s.extract(), "Heroic Strike", [1.0, 1.0, 1.0]), + "and the SELECTED row stays white while another is hovered" + ); + + // Cursor off the list: the hovered row falls back to its state colour, the selected one holds. + s.mouse_move(1000.0, 20.0); + s.resolve(); + assert!( + text_has_color(&s.extract(), "Cleave", [0.9, 0.0, 0.0]), + "cursor away: red again" + ); + assert!( + text_has_color(&s.extract(), "Heroic Strike", [1.0, 1.0, 1.0]), + "the selection's white is the LOCK, not the hover" + ); } /// Scrolling is silent, faithfully: the mouse WHEEL plays no sound (the ref's @@ -767,7 +810,8 @@ fn recipe_menu() -> TrainerState { /// (`ClassTrainerFrameTemplates.xml`'s `` at width 0 + the per-row /// `SetPoint("LEFT", Text, "RIGHT", 10, 0)`), read off the player's chain. /// -/// The mutation check is the first assertion: put a `` back on `$parentName` and the request +/// The mutation check is the first assertion: put a `` back on the row's `` and +/// the request /// carries a wrap width, the long name measures two lines, and its height passes the row height. #[test] fn a_long_row_name_stays_on_one_line_and_carries_its_rank_along() { @@ -818,7 +862,7 @@ fn a_long_row_name_stays_on_one_line_and_carries_its_rank_along() { let row_of = |s: &mut UiScript, needle: &str| -> i64 { s.eval::(&format!( "for i = 1, 11 do local b = getglobal('BenillaTrainerService' .. i) \ - local t = b.name:GetText() if t and strfind(t, '{needle}', 1, 1) then return i end \ + local t = b:GetText() if t and strfind(t, '{needle}', 1, 1) then return i end \ end return 0" )) .unwrap() @@ -830,7 +874,8 @@ fn a_long_row_name_stays_on_one_line_and_carries_its_rank_along() { let geom = |s: &mut UiScript, row: i64| -> (f32, f32, f32, f32) { s.eval::<(f32, f32, f32, f32)>(&format!( "local b = getglobal('BenillaTrainerService{row}') \ - return b.name:GetHeight(), b.name:GetRight(), b.subtext:GetLeft(), b:GetHeight()" + local n = b:GetFontString() \ + return n:GetHeight(), n:GetRight(), b.subtext:GetLeft(), b:GetHeight()" )) .unwrap() }; diff --git a/crates/benilla-app/src/world_backdrop.rs b/crates/benilla-app/src/world_backdrop.rs new file mode 100644 index 00000000..8dfffb29 --- /dev/null +++ b/crates/benilla-app/src/world_backdrop.rs @@ -0,0 +1,244 @@ +//! The world as the UI's **backdrop quad** — the seam that put the UI-over-world blend back into +//! gamma bytes (the third and last piece of the composite lane 0161 and 0254 built). +//! +//! ## The seam that was left +//! +//! Both halves of the frame already composite the way the reference's fixed-function device does, +//! and each does it by the same trick. The world (0161): every world shader emits raw gamma bytes, +//! the blur runs on them, and the FFXGlow combine takes the frame's one `srgb_to_linear` so the +//! sRGB present-encode restores the byte. The UI (0254): `ui_quad.wgsl` emits gamma values into an +//! `Rgba8UnormSrgb` target, so the store encodes, the blend's destination read decodes, and every +//! hardware blend is therefore arithmetic on the gamma value — `alphaMode="ADD"` really is +//! `dst + texel·α`, clamped, exactly as EGxBlend 3 does it. +//! +//! Two correct byte lanes, and **the UI-over-world blend fell in the seam between them.** The UI +//! camera composited its finished image onto the swapchain through its output blit, and the +//! swapchain view is sRGB, so that one blend — the only one that mixes UI with world — ran in +//! linear. 0254 named it as a residual and described it as "a small deviation on antialiased UI +//! edges over the world". It is not an edge artefact: it is every translucent UI pixel over the +//! 3D world, at full area. +//! +//! Measured on the chat dock, which is the most translucent surface the client has. +//! `ChatFrameTab`'s body is black at α = 102/255, so a docked tab must leave the scene at **60 %** +//! of its brightness; the unselected tab (frame α 0.5) at 80 %. Against a bare-scene capture of +//! the same pixels (`ui-chat-tabhover` at `$WOW_TABHOVER=9`) they measured **77.5 %** and **89 %** — +//! and a linear composite predicts exactly that, to within a byte, at every point checked +//! (scene 93 → 72 vs 72.5 predicted; 70 → 54 vs 53.9; 95 → 74 vs 74.1). The tab had almost no +//! plate, so the hover glow — an ADD, at full strength — sat on nothing and read as a blue lozenge +//! floating on grass, louder than the tab that was actually selected. That is what the director +//! reported, and it was never a chat bug. +//! +//! ## The fix: put the world *inside* the UI's byte buffer +//! +//! Not a third lane. The world camera renders to an off-screen image instead of the swapchain, and +//! that image is drawn as the **first quad of the UI pass** — the ground everything else is painted +//! on. Every UI blend over the world is then the same blend as every UI blend over UI: the one +//! 0254 already verified, in the target it already verified it in. The output blit stops blending +//! entirely (it now carries an opaque frame), which retires the `rgb·a²` hazard 0254 had to patch +//! `PREMULTIPLIED_ALPHA_BLENDING` around. +//! +//! **The image is un-encoded float, and both halves of that are borrowed rather than invented** — +//! it is the portrait booths' own target format, chosen there for the same two reasons +//! ([`crate::portrait`]'s `new_target_image`). Un-encoded, because the UI arc composites in gamma +//! and takes its one decode at the end: a backdrop that pre-encoded would land a second encode in +//! that chain. `ui_quad.wgsl`'s ordinary arm re-encodes what it samples (`linear_to_srgb`), which +//! turns FFXGlow's linear output back into the client's byte — the same round trip a booth image +//! takes, and exact in f32. Float rather than `Rgba8Unorm`, because quantizing *un-encoded* values +//! to 8 bits is B126's banding collapse (decision 0804): below display byte 100 an un-encoded 8-bit +//! grid reaches ~25 levels where the gamma backbuffer has 100. +//! +//! Nothing about the world lane changes: FFXGlow keeps its decode, the frame still holds exactly +//! one, and an opaque world pixel must come out byte-identical to before. That identity is the +//! regression test, the same one 0161 used. + +use bevy::asset::RenderAssetUsages; +use bevy::camera::RenderTarget; +use bevy::prelude::*; +use bevy::render::render_resource::{Extent3d, TextureDimension, TextureFormat, TextureUsages}; +use bevy::window::PrimaryWindow; + +use crate::ui_pass::{UiQuad, UiQuads}; +use benilla_world::view::WorldCamera; + +/// The off-screen image the world camera renders into, and which the UI pass draws first. +/// +/// Sized in **physical** pixels to match the swapchain 1:1 — the backdrop quad covers the window +/// exactly, so the sample is an identity resample and opaque world pixels survive untouched. +#[derive(Resource)] +pub(crate) struct WorldBackdrop { + pub(crate) image: Handle, + /// The size the image was last built at, in physical px — the resize gate. + size: UVec2, +} + +/// A fresh backdrop image at `size` physical px. See the module doc for the format's two halves. +fn new_backdrop_image(size: UVec2) -> Image { + let mut image = Image::new_fill( + Extent3d { + width: size.x.max(1), + height: size.y.max(1), + depth_or_array_layers: 1, + }, + TextureDimension::D2, + &[0; 8], + TextureFormat::Rgba16Float, + RenderAssetUsages::default(), + ); + image.texture_descriptor.usage = + TextureUsages::TEXTURE_BINDING | TextureUsages::COPY_DST | TextureUsages::RENDER_ATTACHMENT; + image +} + +fn window_physical_size(window: &Window) -> UVec2 { + UVec2::new( + window.physical_width().max(1), + window.physical_height().max(1), + ) +} + +fn setup_backdrop( + mut commands: Commands, + mut images: ResMut>, + windows: Query<&Window, With>, +) { + let size = windows + .single() + .map(window_physical_size) + .unwrap_or(UVec2::new(1280, 720)); + let image = images.add(new_backdrop_image(size)); + commands.insert_resource(WorldBackdrop { image, size }); +} + +/// Keep the backdrop the window's size. A stale-sized backdrop would still *work* (the quad covers +/// the window either way) but the sample would stop being 1:1 and opaque world pixels would start +/// moving under a resample — the one property this whole lane is built to preserve. +fn track_window_size( + mut backdrop: ResMut, + mut images: ResMut>, + windows: Query<&Window, With>, +) { + let Ok(window) = windows.single() else { + return; + }; + let size = window_physical_size(window); + if size == backdrop.size { + return; + } + let handle = backdrop.image.clone(); + if let Some(image) = images.get_mut(&handle) { + *image = new_backdrop_image(size); + backdrop.size = size; + } +} + +/// Point the world camera at the backdrop instead of the swapchain. +/// +/// A system rather than a component on the spawn, because there are two spawn sites for the same +/// camera (the real one and `player::setup`'s no-client-data fallback) and neither should have to +/// know about the composite lane. `benilla-worldview` is unaffected — it links `benilla-world`, not +/// this crate, and has no UI camera to composite with, so its world camera keeps the swapchain. +/// +/// `Added` rather than a per-frame compare: `RenderTarget` carries no `PartialEq`, and nothing in +/// the tree rewrites a world camera's target once set (the only other `Camera` writer is the gate +/// that flips `is_active`). Retarget it when it appears, and leave it alone after. +fn retarget_world_camera( + backdrop: Res, + mut cameras: Query<&mut RenderTarget, Added>, +) { + for mut current in &mut cameras { + *current = RenderTarget::Image(backdrop.image.clone().into()); + } +} + +/// Publish the backdrop quad for this frame — full-window, opaque, ahead of every other quad. +/// +/// Emitted only while the world camera is actually drawing. With no world (the glue screens, the +/// loading screen, a gated camera) the image holds a stale or never-written frame, and painting it +/// would be worse than the transparent clear the UI pass falls back to. +fn emit_backdrop_quad( + backdrop: Res, + mut quads: ResMut, + cameras: Query<&Camera, With>, + windows: Query<&Window, With>, +) { + let drawing = cameras.iter().any(|c| c.is_active); + let next = windows.single().ok().filter(|_| drawing).map(|window| { + // Logical px, y-down from the top-left — `rebuild_ui_mesh`'s own rect space. + UiQuad { + rect: Rect::from_corners(Vec2::ZERO, Vec2::new(window.width(), window.height())), + texture: Some(backdrop.image.clone()), + ..UiQuad::default() + } + }); + // **Flag the rebuild only when the quad itself changes** — its arrival, its departure, a + // resize. Its CONTENTS change every frame and must not: the mesh batch holds the image handle + // and the material samples whatever the world camera just rendered into it, so a per-frame + // `dirty` here would drag every batch in the pass through a full rebuild for a picture that + // did not move — the 0365 live-city churn, re-introduced from the one producer that runs every + // single frame. + if quads.backdrop != next { + quads.backdrop = next; + quads.dirty = true; + } +} + +/// Owns the backdrop image, the world camera's target, and the quad. See the module doc. +pub(crate) struct WorldBackdropPlugin; + +impl Plugin for WorldBackdropPlugin { + fn build(&self, app: &mut App) { + app.add_systems(Startup, setup_backdrop).add_systems( + Update, + (track_window_size, retarget_world_camera, emit_backdrop_quad) + .chain() + .before(crate::ui_pass::UiQuadAppend), + ); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use bevy::image::TextureFormatPixelInfo as _; + + /// The backdrop is float and un-encoded — the two halves the module doc argues for, and the + /// pair a future "make it 8-bit, it's only a backdrop" edit would quietly break (an sRGB label + /// double-encodes through `ui_quad`; an 8-bit un-encoded grid is B126's banding). + #[test] + fn the_backdrop_is_unencoded_float() { + let image = new_backdrop_image(UVec2::new(320, 200)); + assert_eq!(image.texture_descriptor.format, TextureFormat::Rgba16Float); + assert!( + !image.texture_descriptor.format.is_srgb(), + "an sRGB label would land a second encode in a chain that decodes once, at the end" + ); + assert!( + !TextureFormat::Rgba16Float.is_srgb(), + "the swapchain's own view is sRGB — the backdrop deliberately is not" + ); + } + + /// It must be usable as a camera target AND samplable by the UI pass. Dropping either usage + /// bit fails at device level, far from here. + #[test] + fn the_backdrop_is_both_a_target_and_a_texture() { + let image = new_backdrop_image(UVec2::new(320, 200)); + let usage = image.texture_descriptor.usage; + assert!(usage.contains(TextureUsages::RENDER_ATTACHMENT)); + assert!(usage.contains(TextureUsages::TEXTURE_BINDING)); + } + + /// Physical, not logical: the backdrop matches the swapchain 1:1 so the UI pass's sample is an + /// identity resample. A zero-size window (minimised on some platforms) must still produce a + /// legal texture rather than a device error. + #[test] + fn a_degenerate_size_still_builds_a_legal_texture() { + let image = new_backdrop_image(UVec2::ZERO); + assert_eq!(image.texture_descriptor.size.width, 1); + assert_eq!(image.texture_descriptor.size.height, 1); + assert_eq!( + image.data.as_ref().map(Vec::len), + TextureFormat::Rgba16Float.pixel_size().ok() + ); + } +} diff --git a/crates/benilla-app/tests/tradeskill_frame.rs b/crates/benilla-app/tests/tradeskill_frame.rs index be13d983..af75d42a 100644 --- a/crates/benilla-app/tests/tradeskill_frame.rs +++ b/crates/benilla-app/tests/tradeskill_frame.rs @@ -11,7 +11,8 @@ //! text, a REAL menu-row click driving the exclusive filter, the "All" row restoring it). use benilla_ui::script::{ - TradeSkillDifficulty, TradeSkillReagent, TradeSkillRecipe, TradeSkillState, UiScript, + CraftRecipe, CraftState, CraftTooltip, TradeSkillDifficulty, TradeSkillReagent, + TradeSkillRecipe, TradeSkillState, UiScript, }; const UI_DIR: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/assets/ui"); @@ -467,3 +468,231 @@ fn a_row_click_shows_the_selection_glow_and_a_row_hover_shows_nothing() { ); } } + +/// **The recipe list lights its rows white — under the cursor, and on the selected one.** The +/// director put the real client's Blacksmithing window next to ours: there, the hovered row and the +/// selected row are both white, while every other recipe wears its difficulty colour. Ours wore the +/// difficulty colour everywhere, hover and selection included. +/// +/// One mechanism does both, and it is the row BUTTON's, not a script's. +/// `ClassTrainerSkillButtonTemplate` — the base under this window's rows, Craft's and the class +/// trainer's — declares `` (white) beside its +/// ``, and `TradeSkillFrame_Update` paints each row with +/// `skillButton:SetTextColor(difficulty)` and then calls `LockHighlight()` on the selected one +/// (Blizzard_TradeSkillUI.lua l.113/144). A `SetTextColor` writes the NORMAL font instance only, so +/// it cannot reach a highlighted label — which is exactly why the reference's rows still turn white. +/// +/// Ours could not, for two reasons that had to be fixed together (decision 1605): the row's label +/// was a child `$parentName` FontString rather than the button's own ``, so no per-state +/// font could reach it; and the engine's highlighted label fell back to the normal state's colour, +/// so even a ButtonText would have stayed orange. This test is the end-to-end pin — it reads the +/// colour off the extracted text quad, not off any Lua state, so it fails if either half regresses. +#[test] +fn a_hovered_or_selected_recipe_row_paints_its_label_white() { + let mut s = UiScript::new().unwrap(); + load_ui(&s); + s.set_screen_size(1024.0, 768.0); + s.set_trade_skill(Some(state())); + s.fire_event("TRADE_SKILL_SHOW", vec![]); + + // Row 1 is the "Mail" header; rows 2 and 3 are its two Medium recipes, and opening the window + // auto-selects the first of them. + assert_eq!( + s.eval::("return GetTradeSkillSelectionIndex()") + .unwrap(), + 2, + "the show-time auto-selection landed on the first recipe" + ); + + // The label really is the button's own ButtonText now — a child FontString would leave + // GetFontString() nil and take every per-state font with it. + assert!( + s.eval::("return BenillaTradeSkillSkill2:GetFontString() ~= nil") + .unwrap(), + "the row label is the Button's ButtonText, the only region per-state fonts reach" + ); + + let row_color = |s: &mut UiScript, n: i64| -> [f32; 4] { + let text = s + .eval::(&format!("return BenillaTradeSkillSkill{n}:GetText()")) + .unwrap(); + s.resolve(); + s.extract() + .into_iter() + .find_map(|q| match q.content { + benilla_ui::script::QuadContent::Text { + text: Some(t), + color, + .. + } if t == text => color, + _ => None, + }) + .unwrap_or_else(|| panic!("no text quad for row {n} (\"{text}\")")) + }; + let park_cursor_off_the_list = |s: &mut UiScript| { + s.resolve(); + s.mouse_move(1000.0, 20.0); + }; + let hover_row = |s: &mut UiScript, n: i64| { + s.resolve(); + let (x, y) = ( + s.eval::(&format!( + "return (BenillaTradeSkillSkill{n}:GetLeft() + BenillaTradeSkillSkill{n}:GetRight()) / 2" + )) + .unwrap(), + s.eval::(&format!( + "return (BenillaTradeSkillSkill{n}:GetTop() + BenillaTradeSkillSkill{n}:GetBottom()) / 2" + )) + .unwrap(), + ); + s.mouse_move(x as f32, y as f32); + }; + + const WHITE: [f32; 4] = [1.0, 1.0, 1.0, 1.0]; + const MEDIUM: [f32; 4] = [1.0, 1.0, 0.0, 1.0]; // TradeSkillTypeColor["medium"], the book's own + + park_cursor_off_the_list(&mut s); + assert_eq!( + row_color(&mut s, 2), + WHITE, + "the SELECTED row is white with the cursor nowhere near it — LockHighlight, not a repaint" + ); + assert_eq!( + row_color(&mut s, 3), + MEDIUM, + "an unselected, unhovered recipe wears its difficulty colour" + ); + + hover_row(&mut s, 3); + assert_eq!( + row_color(&mut s, 3), + WHITE, + "hovered: the HighlightFont instance is in force over SetTextColor's difficulty paint" + ); + assert_eq!( + row_color(&mut s, 2), + WHITE, + "and the selected row stays lit while another row is hovered" + ); + + // Click row 3: the white follows the selection, and row 2 falls back to its difficulty colour. + s.run("BenillaTradeSkillSkill3:Click()").unwrap(); + pump(&mut s); + park_cursor_off_the_list(&mut s); + assert_eq!( + s.eval::("return GetTradeSkillSelectionIndex()") + .unwrap(), + 3 + ); + assert_eq!(row_color(&mut s, 3), WHITE, "the new selection is white"); + assert_eq!( + row_color(&mut s, 2), + MEDIUM, + "the old selection is UNLOCKED back to its difficulty colour, not left lit" + ); +} + +/// **The craft list lights the same way, off the same base template.** `CraftButtonTemplate` and +/// `TradeSkillSkillButtonTemplate` both inherit `ClassTrainerSkillButtonTemplate`, so its +/// `` is one mechanism serving both windows — and +/// `Craft_Update` locks its selected row exactly as the tradeskill one does (Blizzard_CraftUI.lua +/// l.234). Ours had the same `$parentName`-FontString workaround in both files, so fixing only the +/// window the director was looking at would have left the enchanting book wrong beside it +/// (decision 1605). +#[test] +fn a_hovered_or_selected_craft_row_paints_its_label_white() { + let mut s = UiScript::new().unwrap(); + load_ui(&s); + s.set_screen_size(1024.0, 768.0); + + let recipe = |spell_id: u32, name: &str| CraftRecipe { + spell_id, + name: name.into(), + sub_name: String::new(), + difficulty: TradeSkillDifficulty::Medium, + num_available: 1, + icon: Some("Interface\\Icons\\Spell_Holy_Heal".into()), + description: None, + needs_item_target: false, + reagents: vec![], + tools: vec![], + tooltip: CraftTooltip::Spell(spell_id), + spell_level: 0, + }; + s.set_craft(Some(CraftState { + name: "Enchanting".into(), + rank: 100, + max_rank: 150, + craft_type: 3, + recipes: vec![ + recipe(7420, "Enchant Bracer - Minor Health"), + recipe(7426, "Enchant Chest - Minor Absorption"), + ], + })); + s.fire_event("CRAFT_SHOW", vec![]); + assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); + + assert!( + s.eval::("return BenillaCraft1:GetFontString() ~= nil") + .unwrap(), + "the craft row name is the Button's ButtonText" + ); + + let row_color = |s: &mut UiScript, n: i64| -> [f32; 4] { + let text = s + .eval::(&format!("return BenillaCraft{n}:GetText()")) + .unwrap(); + s.resolve(); + s.extract() + .into_iter() + .find_map(|q| match q.content { + benilla_ui::script::QuadContent::Text { + text: Some(t), + color, + .. + } if t == text => color, + _ => None, + }) + .unwrap_or_else(|| panic!("no text quad for craft row {n} (\"{text}\")")) + }; + + const WHITE: [f32; 4] = [1.0, 1.0, 1.0, 1.0]; + const MEDIUM: [f32; 4] = [1.0, 1.0, 0.0, 1.0]; // CraftTypeColor["medium"] + + // Row 1 is the show-time selection; row 2 is not. + s.run("SelectCraft(1); BenillaCraftFrame_Update()").unwrap(); + s.resolve(); + s.mouse_move(1000.0, 20.0); + assert_eq!( + row_color(&mut s, 1), + WHITE, + "the selected craft row is white" + ); + assert_eq!( + row_color(&mut s, 2), + MEDIUM, + "an unselected one wears its difficulty colour" + ); + + // Hover row 2. + s.resolve(); + let (x, y) = ( + s.eval::("return (BenillaCraft2:GetLeft() + BenillaCraft2:GetRight()) / 2") + .unwrap(), + s.eval::("return (BenillaCraft2:GetTop() + BenillaCraft2:GetBottom()) / 2") + .unwrap(), + ); + s.mouse_move(x as f32, y as f32); + assert_eq!(row_color(&mut s, 2), WHITE, "hovered: white"); + + // And the selection follows a click, releasing the old row's lock. + s.run("SelectCraft(2); BenillaCraftFrame_Update()").unwrap(); + s.resolve(); + s.mouse_move(1000.0, 20.0); + assert_eq!(row_color(&mut s, 2), WHITE); + assert_eq!( + row_color(&mut s, 1), + MEDIUM, + "the old selection is UNLOCKED, not left lit" + ); +} diff --git a/crates/benilla-ui/src/script/extract.rs b/crates/benilla-ui/src/script/extract.rs index e0acb6d7..b79d6599 100644 --- a/crates/benilla-ui/src/script/extract.rs +++ b/crates/benilla-ui/src/script/extract.rs @@ -210,9 +210,9 @@ impl UiScript { } // A Button's state textures show by interaction state (the texture-array // "current" pointer): a non-current state texture emits no quad this frame. - // The ButtonText additionally re-points to the current STATE's font object - // (disabled > hovered > normal; hover falls back to normal) — the client's - // per-state label font swap (UIPanelButtonTemplate's gold/white/gray trio). + // The ButtonText additionally re-points to the current STATE's font instance + // (disabled > highlighted > normal) — the client's per-state label font swap + // (UIPanelButtonTemplate's gold/white/gray trio). let mut state_font: Option<&FontObject> = None; let mut state_color: Option<[f32; 4]> = None; // `Button:SetFont` — the face/size/flags written on the button's own embedded @@ -230,24 +230,50 @@ impl UiScript { continue; } if bs.text == Some(rh) { - let name = if !bs.enabled { - bs.disabled_font.as_ref() - } else if hovered { - bs.highlight_font.as_ref().or(bs.normal_font.as_ref()) + // The label inherits ONE of the button's three embedded font instances + // WHOLE (normal `+0x33c` / highlight `+0x3b8` / disabled `+0x434`): the + // client swaps which instance the label reads, it does not merge axes + // across them. So the object and the colour are picked as a PAIR, and a + // state with no font object of its own is not in force at all — the + // label stays on the normal instance, face and colour together. + // + // The pairing is the load-bearing half, and it is what this used to get + // wrong (the colour fell back to `normal_color` while the font did not). + // `UIDropDownMenu.lua` sets every row colour through `SetTextColor` + // **and** `SetHighlightTextColor` with the same values (l.216-220, + // l.829-830) — a second call it would never need if the first reached + // the hovered state. The tradeskill list leans on the other direction: + // its rows are `SetTextColor`'d to the recipe's difficulty and still + // turn white under the cursor, because `` is the instance in force there + // (`ClassTrainerFrameTemplates.xml` l.74) and a normal-instance colour + // cannot reach it. + // + // `LockHighlight()` counts as highlighted HERE too, not only for the + // HighlightTexture in `region_visible`. `TradeSkillFrame_Update` blanks + // a recipe row's highlight texture to `""` and *then* locks the row it + // selected (Blizzard_TradeSkillUI.lua l.131/144) — with no texture left + // to pin, the lock's only possible effect on that row is this label + // swap, which is the white text on the selected recipe. Craft + // (l.234) and the class trainer (l.183) lock the same way. + // + // INFERRED, not byte-verified: that an unset state instance leaves the + // label on the normal one. A null slot in the *texture* array draws + // nothing (decision 0227) and a font instance with no object cannot + // work that way — a disabled button with no `` still + // shows its label. Every state-colour caller in our own UI ships the + // matching font object, so the two readings agree on all of them. + let highlighted = hovered || bs.locked_highlight; + let (name, color) = if !bs.enabled && bs.disabled_font.is_some() { + (bs.disabled_font.as_ref(), bs.disabled_color) + } else if bs.enabled && highlighted && bs.highlight_font.is_some() { + (bs.highlight_font.as_ref(), bs.highlight_color) } else { - bs.normal_font.as_ref() + (bs.normal_font.as_ref(), bs.normal_color) }; state_font = name.and_then(|n| model.font_object(n)); button_font = bs.font.as_ref(); - // The per-state color override (Button:Set*TextColor) — hover falls - // back to the normal color, mirroring the font fallback above. - state_color = if !bs.enabled { - bs.disabled_color - } else if hovered { - bs.highlight_color.or(bs.normal_color) - } else { - bs.normal_color - }; + state_color = color; } } // A TITLE REGION NEVER DRAWS. It is a hit rectangle, not a visual: wow-re diff --git a/crates/benilla-ui/src/script/tests/button.rs b/crates/benilla-ui/src/script/tests/button.rs index a1e7b119..74be4ef9 100644 --- a/crates/benilla-ui/src/script/tests/button.rs +++ b/crates/benilla-ui/src/script/tests/button.rs @@ -485,6 +485,117 @@ fn button_label_repaints_by_state_font_object() { assert!((c[0] - 0.1).abs() < 1e-6 && (c[1] - 0.2).abs() < 1e-6); } +/// **The highlighted label is its own font instance — and `LockHighlight` enters it.** The two +/// halves of the recipe-list look the director asked for: a row turns white under the cursor, and +/// the SELECTED row stays white with the cursor elsewhere. +/// +/// Both are one mechanism in the reference. `ClassTrainerSkillButtonTemplate` (the base under +/// TradeSkill's, Craft's and the class trainer's rows) declares +/// ``, and each window paints its rows by difficulty +/// with `skillButton:SetTextColor(...)` — orange, yellow, green, gray. If a `SetTextColor` reached +/// the highlighted state, hovering a row would leave it orange; it does not, because the highlight +/// instance is a different font instance and the normal one's colour cannot reach it. That is the +/// first assertion below. +/// +/// The second is `LockHighlight`. `TradeSkillFrame_Update` blanks a recipe row's highlight texture +/// (`getglobal("TradeSkillSkill"..i.."Highlight"):SetTexture("")`, Blizzard_TradeSkillUI.lua l.131) +/// and *then* locks the row it selected (l.144) — with no texture left to pin, the lock can only be +/// reaching the label. Craft (Blizzard_CraftUI.lua l.234) and the trainer +/// (Blizzard_TrainerUI.lua l.183) do the same. +#[test] +fn a_locked_or_hovered_button_wears_its_highlight_font_over_its_normal_color() { + let mut s = script(); + s.set_screen_size(800.0, 600.0); + s.register_font_object( + "RowNormal", + FontObject { + color: Some([1.0, 0.82, 0.0, 1.0]), + height: Some(12.0), + ..Default::default() + }, + ); + s.register_font_object( + "RowHighlight", + FontObject { + color: Some([1.0, 1.0, 1.0, 1.0]), + height: Some(12.0), + ..Default::default() + }, + ); + s.run( + r#" + b = CreateFrame("Button", "RowBtn") + b:SetPoint("BOTTOMLEFT", 100, 100); b:SetSize(100, 20) + b:SetText("Rough Copper Vest") + b:SetTextFontObject("RowNormal") + b:SetHighlightFontObject("RowHighlight") + -- The difficulty paint every one of the three list windows applies to every row. + b:SetTextColor(1.0, 0.5, 0.25) + "#, + ) + .unwrap(); + + let label_color = |s: &mut crate::script::UiScript| { + s.resolve(); + s.extract() + .into_iter() + .find_map(|q| match q.content { + QuadContent::Text { + text: Some(t), + color, + .. + } if t == "Rough Copper Vest" => Some(color), + _ => None, + }) + .expect("label text quad") + }; + + s.resolve(); + s.mouse_move(400.0, 300.0); // nowhere near the row + assert_eq!( + label_color(&mut s), + Some([1.0, 0.5, 0.25, 1.0]), + "at rest the row wears the difficulty colour SetTextColor gave it" + ); + + s.resolve(); + s.mouse_move(150.0, 110.0); // over the row + assert_eq!( + label_color(&mut s), + Some([1.0, 1.0, 1.0, 1.0]), + "hovered: the HIGHLIGHT instance is in force, and the normal colour cannot reach it" + ); + + // Cursor away again, but the row is the selected one. + s.resolve(); + s.mouse_move(400.0, 300.0); + s.run("b:LockHighlight()").unwrap(); + assert_eq!( + label_color(&mut s), + Some([1.0, 1.0, 1.0, 1.0]), + "locked: white with the cursor elsewhere — the selected recipe row" + ); + s.run("b:UnlockHighlight()").unwrap(); + assert_eq!( + label_color(&mut s), + Some([1.0, 0.5, 0.25, 1.0]), + "unlocked: back to the difficulty colour" + ); + + // A button with no highlight instance keeps the normal one whole, hover or not — the shape + // every plain labelled Button in the corpus has. + s.run(r#"b:SetHighlightFontObject(nil); b:SetText("Rough Copper Vest")"#) + .unwrap(); + s.resolve(); + s.mouse_move(150.0, 110.0); + assert_eq!( + label_color(&mut s), + Some([1.0, 0.5, 0.25, 1.0]), + "no → hover changes nothing" + ); + assert!(s.errors().is_empty(), "{:?}", s.errors()); +} + /// **`Button:GetTextWidth` / `GetTextHeight`** — the reference's own Button text-extent readers /// (`0x782290` / `0x782390`; wow-re `widget-api-batch-benilla.md` Q8 carves them present on Button /// and `GetStringWidth` **absent**). Both forward to the label FontString's own extent slots, which diff --git a/crates/benilla-ui/src/widget/kinds/mod.rs b/crates/benilla-ui/src/widget/kinds/mod.rs index f8d779e0..449c063c 100644 --- a/crates/benilla-ui/src/widget/kinds/mod.rs +++ b/crates/benilla-ui/src/widget/kinds/mod.rs @@ -441,7 +441,9 @@ pub struct ButtonState { /// object — the client's per-state CFontString font swap (UIPanelButtonTemplate's gold /// normal / white highlight / gray disabled label). `None` = keep the label's own paint. pub normal_font: Option, - /// See [`ButtonState::normal_font`] — the hovered state (falls back to `normal_font`). + /// See [`ButtonState::normal_font`] — the highlighted state (hovered **or** + /// [`locked_highlight`](ButtonState::locked_highlight)). `None` means the button has no + /// highlight instance at all and the label stays on the normal one, colour included. pub highlight_font: Option, /// See [`ButtonState::normal_font`] — the disabled state. pub disabled_font: Option, @@ -462,13 +464,18 @@ pub struct ButtonState { /// (`info.textR/G/B`, isTitle's NORMAL-yellow and notClickable's HIGHLIGHT-white recolors of /// a disabled row). `None` = the state font's paint. pub normal_color: Option<[f32; 4]>, - /// See [`ButtonState::normal_color`] — the hovered state (falls back to `normal_color`). + /// See [`ButtonState::normal_color`] — the highlighted state. It does **not** fall back to + /// `normal_color`: each state is its own font instance, so a `SetTextColor` cannot reach the + /// highlighted label (which is why `UIDropDownMenu.lua` always pairs the two setters). pub highlight_color: Option<[f32; 4]>, /// See [`ButtonState::normal_color`] — the disabled state. pub disabled_color: Option<[f32; 4]>, - /// `LockHighlight()` — the HighlightTexture draws regardless of hover until + /// `LockHighlight()` — the button reads as highlighted regardless of hover until /// `UnlockHighlight()` (ref `CButton::LockHighlight`; the dropdown kit keeps a checked row's - /// highlight lit). + /// highlight lit). That covers BOTH halves of the highlighted look: the HighlightTexture + /// ([`Self::region_visible`]) and the label's font instance (`script::extract`). The list + /// windows lean on the second alone — a tradeskill/craft/trainer recipe row blanks its + /// highlight texture to `""` and locks the selected row anyway, purely for the white label. pub locked_highlight: bool, }