diff --git a/crates/benilla-app/assets/ui/BagFrame.xml b/crates/benilla-app/assets/ui/BagFrame.xml index 64331b06..e94aa7fc 100644 --- a/crates/benilla-app/assets/ui/BagFrame.xml +++ b/crates/benilla-app/assets/ui/BagFrame.xml @@ -1033,20 +1033,33 @@ -- starts moving, so the drop reads as the icon being drawn into the bag rather than as a -- solid tile sliding. Those numbers are the file's, not ours. -- - -- THE ONE AUTHORED NUMBER is the model-unit → pixel scale, because a frame's 3D - -- projection is not RE-recorded (the ref's ItemAnim frame has no at all, so the rect - -- cannot be it) — see decision 0887's named gap. 1200 px/unit is the sibling calibration: - -- CooldownFrameTemplate is the same kind of degenerate UI model, its four quads span 0.04 - -- units, and at its scale="0.75" that sweep covers a 36px action button ⇒ 0.04·K·0.75 = 36. - -- At K = 1200 this model's own numbers land on 36 px of icon (exactly a bag-bar button), - -- a 48 px fall and a 12 px sideways drift — which is why the sizes below are stated as the - -- model's ratios times the icon, not as free pixels: correct the scale in one place and the - -- whole animation stays in proportion. - BENILLA_ITEM_PUSH_PX_PER_UNIT = 1200 + -- THE MODEL-UNIT → PIXEL SCALE is no longer ours to calibrate: the widget's + -- projection is RE-recorded now (wow-re `system/ui/scratch/modelframe-animation-clock.md`, + -- dispatched over the pet bar's B228 and folded back with it). The ortho leg gives + -- + -- px per model unit = (5/3) * modelScale * layoutScale * screenHeightPx + -- + -- which in FrameXML units is **1280 * modelScale * layoutScale**, aspect- and + -- resolution-independent, with the model ORIGIN at the frame rect's bottom-left. The ref's + -- `ItemAnimTemplate` (MainMenuBarBagButtons.xml:4) carries **no `scale` attribute**, so + -- modelScale is 1 and the constant is a flat 1280. Decision 0887's named gap is closed. + -- + -- This REPLACES a back-fit, and the back-fit is worth naming because it looked like + -- evidence. The old 1200 came from assuming CooldownFrameTemplate's 0.04-unit sweep + -- *exactly* covers a 36 px action button at its scale="0.75" (⇒ K = 1200). The assumption + -- is the error: at the real projection that sweep is 0.04 · 1280 · 0.75 = **38.4 px**, so + -- it overhangs its button by 6.7% rather than fitting it — exactly as the autocast shine's + -- 30.72 px square overhangs its 30 px button. Solving against a wrong premise produced a + -- number that then "cross-checked" against a second model derived the same wrong way, and + -- both files cited the agreement as the reason to trust it. + -- + -- Everything below is the model's own ratios times this constant, so the correction lands + -- in one place and the animation stays in proportion. + BENILLA_ITEM_PUSH_PX_PER_UNIT = 1280 BENILLA_ITEM_PUSH_DURATION = 1.0 - BENILLA_ITEM_PUSH_SIZE = 0.03 * BENILLA_ITEM_PUSH_PX_PER_UNIT -- 36 - BENILLA_ITEM_PUSH_FALL = 0.04 * BENILLA_ITEM_PUSH_PX_PER_UNIT -- 48 - BENILLA_ITEM_PUSH_DRIFT = 0.01 * BENILLA_ITEM_PUSH_PX_PER_UNIT -- 12 + BENILLA_ITEM_PUSH_SIZE = 0.03 * BENILLA_ITEM_PUSH_PX_PER_UNIT -- 38.4 + BENILLA_ITEM_PUSH_FALL = 0.04 * BENILLA_ITEM_PUSH_PX_PER_UNIT -- 51.2 + BENILLA_ITEM_PUSH_DRIFT = 0.01 * BENILLA_ITEM_PUSH_PX_PER_UNIT -- 12.8 -- {time, value} keyframes, linear between (M2 track interpolation 1) — the file's own keys. BENILLA_ITEM_PUSH_ALPHA = { {0.000, 0.0}, {0.133, 1.0}, {0.267, 1.0}, {1.000, 0.0} } BENILLA_ITEM_PUSH_SCALE = { {0.000, 1.0}, {0.133, 1.2}, {0.267, 1.0}, {1.000, 0.014} } diff --git a/crates/benilla-app/assets/ui/PetActionBar.xml b/crates/benilla-app/assets/ui/PetActionBar.xml index acf81494..e228836d 100644 --- a/crates/benilla-app/assets/ui/PetActionBar.xml +++ b/crates/benilla-app/assets/ui/PetActionBar.xml @@ -111,12 +111,23 @@ -- and shrinks away there. The effect is therefore four comet trails running round the -- button's rim, not a rotating ring. -- - -- Model units -> pixels is the same 1200 px/unit calibration BagFrame.xml derived from the - -- Cooldown model (its own header carries the derivation), and it CROSS-CHECKS here: the - -- lap square is 0.02 units, the ref's Model carries scale="1.2", and 0.02 * 1200 * 1.2 = - -- 28.8 px — the rim of a 30 px button, which is where the sparkles visibly run. That - -- agreement between two independently-authored UI models is the reason to trust the - -- number rather than eyeball a size. + -- MODEL UNITS -> PIXELS is the widget's own projection, not a calibration + -- (wow-re `system/ui/scratch/modelframe-animation-clock.md`, §1317's fold-back). There is + -- no fixed px/unit: the ortho leg gives + -- + -- px per model unit = (5/3) * modelScale * layoutScale * screenHeightPx + -- + -- which in FrameXML units collapses to **1280 * modelScale * layoutScale**, independent of + -- aspect and resolution. `scale="1.2"` on the ref's is SetModelScale, NOT + -- Frame:SetScale (`0x76cb36`, stored at `+0x3a0`), so 1 model unit = 1536 FrameXML units + -- and the 0.02 path is **30.72** across — 1.024x a 30-unit button, overhanging its top and + -- right by 2.4%. The model ORIGIN is the frame rect's BOTTOM-LEFT, not its centre, which is + -- why a square authored 0..0.02 lands across the whole button instead of in one quadrant. + -- + -- This corrects the 1200 px/unit figure this header carried until then, which was the right + -- formula evaluated at a 720 px screen **with the 1.2 left out** — two errors that nearly + -- cancelled (28.8 vs 30.72) and made the wrong number look like it cross-checked. BagFrame's + -- Cooldown calibration is the same coincidence and is flagged there. -- -- OUR ONE DEPARTURE is the particle COUNT: 300 live per emitter is 12 000 quads across a -- full bar. We draw a SAMPLE of the same trail — each sample sits where the bone was `age` @@ -125,20 +136,19 @@ -- -- **How many samples is DERIVED, not chosen** (B228). It was 8, evenly spaced in age, on -- the claim that the trail "reads identically at a fraction" of 300. Nobody had checked - -- that claim, and the arithmetic says it is false. The rim is 4 x 28.8 = 115.2 px round and - -- the lap is 2.0 s, so the bone travels 57.6 px/s and a particle's 1.0 s life spans 57.6 px - -- of rim — two whole edges. Eight even samples therefore sit 7.2 px apart, while the stars - -- behind the head are 4.3 px and then 2.9 px across: the back half of every trail was a row - -- of SEPARATED DOTS where the file draws one continuous streak. Marching dots are legible - -- motion in a way a smooth streak is not, which is the shape of thing a reporter reads as - -- "too fast" (nazriel, B228) even at the right period — and the period IS right, measured - -- through this file's own OnUpdate in `pet_bar_tests`. + -- that claim, and the arithmetic says it is false. The rim is 4 x 30.72 = 122.9 px round + -- and the lap is 2.0 s, so the bone travels 61.4 px/s and a particle's 1.0 s life spans + -- 61.4 px of rim — two whole edges. Eight even samples therefore sit 7.7 px apart, while + -- the stars behind the head are 4.6 px and then 3.1 px across: the back half of every trail + -- was a row of SEPARATED DOTS where the file draws one continuous streak. Marching dots are + -- legible motion in a way a smooth streak is not, so it read as speed on a loop that runs + -- at the authored rate (nazriel, B228). -- -- So the samples are spaced by the file's own SIZE ramp rather than evenly: each step -- advances one star-width / OVERLAP of arc. That spends samples in the tail where the - -- stars are small and saves them at the head where a 14.4 px star covers everything — + -- stars are small and saves them at the head where a 15 px star covers everything — -- continuous everywhere, and cheaper than a uniform spacing fine enough for the tail. - BENILLA_PET_AUTOCAST_PX_PER_UNIT = 1200 + BENILLA_PET_AUTOCAST_PX_PER_UNIT = 1280 -- the ortho leg's BENILLA_PET_AUTOCAST_SCALE = 1.2 -- the ref Model's BENILLA_PET_AUTOCAST_PERIOD = 2.0 -- seq0 loop, seconds BENILLA_PET_AUTOCAST_LIFE = 1.0 -- particle lifespan @@ -166,20 +176,55 @@ {1.000, 1.000, 1.000, 0.0}, } - -- Where an emitter sits at lap fraction `f` (0..1), as an offset from the button's CENTER - -- in pixels. The bone tracks walk the square anticlockwise from its bottom-left corner, - -- one edge per quarter-lap; this is that walk, re-centred (the model's square runs 0..0.02 - -- in both axes, so its centre is at 0.01). + -- ── The widget's CLOCK, truncation and all ──────────────────────────────────────────── + -- + -- The reference does NOT play this loop at wall-clock rate, and that is the whole of what + -- was left of B228. `CSimpleModel` overrides `CSimpleFrame`'s per-frame OnUpdate slot + -- (vtable `0x81c608` idx14 -> `0x76d7f0`) and advances its private `CM2Scene` clock by + -- `__ftol(elapsed * 1000.0)` — **truncated, with no fractional carry**. The WORLD's driver + -- does the same conversion with a `+0.5` first (`0x48366b`); the widget path omits it + -- (`0x76d846`). So the widget throws away `frac(elapsed * 1000)` every single frame, and a + -- 2000 ms band takes `2000 * T / floor(T)` ms at frame time T ms: + -- + -- 30 fps -> 2020 ms · 60 fps -> 2083 ms · 75 fps -> 2051 · 144 fps -> 2315 + -- + -- It is **never faster than 2.000 s**. Our old exact-2.000 s clock was therefore faster + -- than any real 1.12 client at any frame rate — the floor of the range rather than a point + -- in it — which is exactly nazriel's "a bit too fast". (wow-re + -- `system/ui/scratch/modelframe-animation-clock.md`, §5 cross-checked.) + -- + -- So the clock is an integer MILLISECOND counter here too, advanced the reference's way. + -- This is the mechanism, not a tuned constant: it tracks the player's own frame rate the + -- way theirs did. Two knowing limits. The reference's `elapsed` is an **f32**, so + -- `f32(0.01) * 1000 = 9.99999978` truncates to 9 — a whole millisecond lost at a nominally + -- clean frame rate, and the exact hazard the world path's `+0.5` cancels; we do not model + -- f32 rounding, so we lose marginally less than it does. And above 1000 fps the reference + -- freezes outright (every frame truncates to 0 ms) — reproduced rather than guarded, + -- because a guard would be inventing behaviour, but no 2006 client ever reached it and a + -- frozen sparkle at four-figure frame rates is this line's fault, not a new bug. + BENILLA_PET_AUTOCAST_PERIOD_MS = 2000 + + function BenillaPetAutocast_Advance(frame, elapsed) + local ms = (frame.clock or 0) + math.floor(elapsed * 1000) + frame.clock = math.mod(ms, BENILLA_PET_AUTOCAST_PERIOD_MS) + return frame.clock + end + + -- Where an emitter sits at lap fraction `f` (0..1), as an offset from the button's + -- BOTTOMLEFT in pixels — the model's own origin (see the header), so this is the authored + -- square placed exactly where the widget places it: 0..30.72 across a 30 px button, its + -- corner on the button's corner, overhanging the top and right by 0.72 px. The bone tracks + -- walk it clockwise on screen from that bottom-left corner, one edge per quarter-lap. function BenillaPetAutocast_Point(f) - local h = BENILLA_PET_AUTOCAST_SIZE / 2 + local s = BENILLA_PET_AUTOCAST_SIZE f = f - math.floor(f) -- wrap into 0..1 local edge = f * 4 local leg = math.floor(edge) local t = edge - leg -- 0..1 along this edge - if leg == 0 then return -h, -h + t * 2 * h -- bottom-left -> top-left - elseif leg == 1 then return -h + t * 2 * h, h -- top-left -> top-right - elseif leg == 2 then return h, h - t * 2 * h -- top-right -> bottom-right - else return h - t * 2 * h, -h -- bottom-right -> bottom-left + if leg == 0 then return 0, t * s -- bottom-left -> top-left + elseif leg == 1 then return t * s, s -- top-left -> top-right + elseif leg == 2 then return s, s - t * s -- top-right -> bottom-right + else return s - t * s, 0 -- bottom-right -> bottom-left end end @@ -259,7 +304,7 @@ -- Show and hide are EDGE work, not per-frame work: this runs for all ten buttons every -- frame and all but one or two of them are dark, so the old unconditional Hide loop was -- spending most of the bar's frame budget re-hiding what was already hidden. - function BenillaPetAutocast_Update(button, clock) + function BenillaPetAutocast_Update(button, clockMs) if not button.autocasting then if button.sparksLit then for i = 1, table.getn(button.sparks) do button.sparks[i]:Hide() end @@ -269,12 +314,13 @@ end if not button.sparks then BenillaPetAutocast_Create(button) end local sparks = button.sparks - local lap = clock / BENILLA_PET_AUTOCAST_PERIOD + local lap = clockMs / BENILLA_PET_AUTOCAST_PERIOD_MS for i = 1, table.getn(sparks) do local spark = sparks[i] - -- SetPoint on the SAME point replaces it, so no ClearAllPoints is needed. + -- SetPoint on the SAME point replaces it, so no ClearAllPoints is needed. The + -- anchor is the button's BOTTOMLEFT because that is the model's own origin. local x, y = BenillaPetAutocast_Point(lap + spark.phase) - spark:SetPoint("CENTER", button, "CENTER", x, y) + spark:SetPoint("CENTER", button, "BOTTOMLEFT", x, y) end if not button.sparksLit then for i = 1, table.getn(sparks) do sparks[i]:Show() end @@ -522,12 +568,9 @@ -- The shared sparkle clock. One OnUpdate for the whole bar rather than ten: the four -- emitters are one looping model, so every autocasting button is in step by construction. function BenillaPetActionBar_OnUpdate(frame, elapsed) - frame.clock = (frame.clock or 0) + elapsed - if frame.clock > BENILLA_PET_AUTOCAST_PERIOD then - frame.clock = frame.clock - BENILLA_PET_AUTOCAST_PERIOD - end + local clockMs = BenillaPetAutocast_Advance(frame, elapsed) for i = 1, NUM_PET_ACTION_SLOTS do - BenillaPetAutocast_Update(getglobal("PetActionButton" .. i), frame.clock) + BenillaPetAutocast_Update(getglobal("PetActionButton" .. i), clockMs) end end diff --git a/crates/benilla-app/assets/ui/SpellBookFrame.xml b/crates/benilla-app/assets/ui/SpellBookFrame.xml index 03e4967b..134c95ae 100644 --- a/crates/benilla-app/assets/ui/SpellBookFrame.xml +++ b/crates/benilla-app/assets/ui/SpellBookFrame.xml @@ -242,12 +242,13 @@ -- while the window is shown, so it costs nothing the rest of the time. function BenillaSpellBookFrame_OnUpdate(frame, elapsed) if not BenillaPetAutocast_Update then return end - frame.clock = (frame.clock or 0) + elapsed - if frame.clock > BENILLA_PET_AUTOCAST_PERIOD then - frame.clock = frame.clock - BENILLA_PET_AUTOCAST_PERIOD - end + -- The clock advance is the pet bar's too, and SHARED rather than copied: it is the + -- reference Model widget's own truncating millisecond accumulator (that file's header + -- carries the bytes), and a second hand-rolled copy is exactly how the two surfaces + -- would drift apart the next time it is corrected. + local clockMs = BenillaPetAutocast_Advance(frame, elapsed) for i = 1, SPELLS_PER_PAGE do - BenillaPetAutocast_Update(getglobal("SpellButton" .. i), frame.clock) + BenillaPetAutocast_Update(getglobal("SpellButton" .. i), clockMs) end end diff --git a/crates/benilla-app/src/ui_cast.rs b/crates/benilla-app/src/ui_cast.rs index d573123e..1f148af2 100644 --- a/crates/benilla-app/src/ui_cast.rs +++ b/crates/benilla-app/src/ui_cast.rs @@ -518,22 +518,33 @@ fn local_self_cancel( } } -/// The casting bar's **label** for a spell — the `name` argument of `SPELLCAST_START` / -/// `SPELLCAST_CHANNEL_START`, which the reference `CastingBarFrame` puts straight into -/// `CastingBarText:SetText(...)`. +/// `SPELLCAST_START`'s **name argument** — what the reference `CastingBarFrame` puts straight into +/// `CastingBarText:SetText(...)` for a *cast*. Byte-verified at `0x6e7a2d`–`0x6e7a47` (wow-re +/// `spell/scratch/wave-cast.md`, "The casting-bar TEXT law", §5 cross-checked): the handler tests +/// `AttributesEx3 & 0x4` and pushes **the empty string `0x882748`** when set, `Name[locale]` when +/// clear — the event still fires either way, carrying its duration, so the bar is a normal +/// full-length *untitled* bar rather than a hidden one. /// -/// Almost always the spell's `Spell.dbc` name, with two ways to come back empty: +/// So two ways to come back empty: /// -/// - **`AttributesEx3 & 0x4`** (`SPELL_ATTR_EX3_NO_CASTING_BAR_TEXT`, -/// [`SpellDisplay::no_casting_bar_text`]) — the attribute that exists precisely so an internal +/// - **`AttributesEx3 & 0x4`** ([`SpellDisplay::no_casting_bar_text`], the emulators' +/// `SPELL_ATTR_EX3_NO_CASTING_BAR_TEXT`) — the attribute that exists precisely so an internal /// spell with no player-facing name never puts one on the bar. Three shipped rows carry it, and -/// one of them is *named after the bit*: **22810 "Opening - No Text"**, the opener the client -/// casts at `LockType 13` ground containers. Printing the name unconditionally is what put that -/// placeholder over a gathered Hyacinth Mushroom (B247, decision 1312). +/// one is *named after the bit*: **22810 "Opening - No Text"**, the opener the client casts at +/// `LockType 13` ground containers. Printing the name regardless is what put that placeholder +/// over a gathered Hyacinth Mushroom (B247, decision 1312). /// - **no catalog row** (an unknown id, or no client data) — the empty string this always /// returned in that case. /// -/// The bar still shows, fills and sweeps either way; only the text is suppressed. +/// **This is the CAST bar's law and it does not generalise to the channel bar** — which is why +/// this is `SPELLCAST_START`'s label and not a shared helper. `0x6e7a2d` is the only +/// `SpellRec+0x24 & 4` test in the whole image (censused twice), and the channel handler +/// `0x6e7550` gates on entirely different bits: `AttributesEx3 & 0x2000` fires **no event at all**, +/// and the name is `Name[locale]` only when `AttributesEx & 0x2000_0000` is set, otherwise the +/// literal `GetText("CHANNELING")`. Neither is modelled yet — [`CastBarEdge::ChannelStart`] still +/// passes the plain name — so a channel bar can read a spell name where the reference reads +/// "Channeling". Named residual, not an oversight: it is a visible change to every channelled +/// spell and wants the director's eye, not a silent fold-in. fn cast_bar_label(spells: Option<&crate::ui_action::Spells>, id: u32) -> String { spells .and_then(|s| s.catalog.get(id)) @@ -585,7 +596,15 @@ fn feed_cast_bar( script.set_casting( auto_repeat.0.is_some() || inflight(&pending, &queued_melee, started, now).is_some(), ); - let name = |id: u32| cast_bar_label(spells.as_deref(), id); + // The two bars take their names from DIFFERENT laws — see [`cast_bar_label`]. The plain + // `Spell.dbc` name is the channel bar's (incomplete) one; the cast bar's is attribute-gated. + let channel_name = |id: u32| -> String { + spells + .as_ref() + .and_then(|s| s.catalog.get(id)) + .map(|d| d.name.clone()) + .unwrap_or_default() + }; for edge in feed.0.drain(..) { let (event, args): (&str, Vec) = match edge { CastBarEdge::Start { @@ -594,7 +613,7 @@ fn feed_cast_bar( } => ( "SPELLCAST_START", vec![ - ScriptValue::Str(name(spell_id)), + ScriptValue::Str(cast_bar_label(spells.as_deref(), spell_id)), ScriptValue::Int(i64::from(cast_time_ms)), ], ), @@ -612,7 +631,7 @@ fn feed_cast_bar( "SPELLCAST_CHANNEL_START", vec![ ScriptValue::Int(i64::from(duration_ms)), - ScriptValue::Str(name(spell_id)), + ScriptValue::Str(channel_name(spell_id)), ], ), CastBarEdge::ChannelUpdate { remaining_ms: 0 } => ("SPELLCAST_CHANNEL_STOP", vec![]), @@ -680,7 +699,8 @@ mod tests { assert_eq!( cast_bar_label(Some(&spells), 22810), "", - "the bar fills and sweeps, but shows no name" + "the event still fires carrying its duration — a full-length UNTITLED bar, \ + not a hidden one (0x6e7a33 pushes the empty string, then fires as usual)" ); // The pre-existing empty cases are untouched: an unknown id, and no client data at all. assert_eq!(cast_bar_label(Some(&spells), 133), ""); diff --git a/crates/benilla-app/src/ui_script/addons.rs b/crates/benilla-app/src/ui_script/addons.rs index 003dad04..1ba72853 100644 --- a/crates/benilla-app/src/ui_script/addons.rs +++ b/crates/benilla-app/src/ui_script/addons.rs @@ -293,7 +293,12 @@ fn read_under(root: &Path, rel: &str) -> Option> { /// install, it does not live in one. Resolved through [`crate::local_state`], the only module /// allowed to compute that path (0954) — which is already `None` under `$WOW_CAPTURE`, so a /// deterministic baseline cannot depend on what somebody has installed (0008). -fn root() -> Option { +/// +/// `pub(crate)` since 1322: the texture side needs the same folder — the sprite decoder's +/// `Interface\AddOns\` loose-file resolve and the `SetTexture` probe both map that virtual prefix +/// onto this root (`ui_script::lifecycle::install_texture_resolvers`), so there is exactly one +/// answer to "where do addons live" however the question is asked. +pub(crate) fn root() -> Option { root_from(crate::local_state::home()) } diff --git a/crates/benilla-app/src/ui_script/bag_tests.rs b/crates/benilla-app/src/ui_script/bag_tests.rs index d21b6e40..2f17dffc 100644 --- a/crates/benilla-app/src/ui_script/bag_tests.rs +++ b/crates/benilla-app/src/ui_script/bag_tests.rs @@ -1800,10 +1800,15 @@ fn an_item_push_drops_its_icon_into_the_bag_that_took_it() { // t=0: invisible, full size, held a whole fall above the button (and a drift to its left) — // the .m2's alpha key (0.000, 0.0), scale key (0.000, 1.0), translation (0.000, (0,0)). + // + // The pixel figures below are the model's own ratios at the widget's REAL projection + // (1280 px per model unit at this template's modelScale of 1 — BagFrame.xml's header carries + // the derivation and why the 1200 that produced 36/48/12 was a back-fit). Same animation, + // 6.7% larger: a 38.4 px card, a 51.2 px fall, a 12.8 px drift. let (dx, dy) = offset(&s, "CharacterBag1Slot"); assert!( - (dy - 48.0).abs() < 0.5 && (dx + 12.0).abs() < 0.5, - "starts one fall (48px) above and one drift (12px) left of the button: got ({dx}, {dy})" + (dy - 51.2).abs() < 0.5 && (dx + 12.8).abs() < 0.5, + "starts one fall (51.2px) above and one drift (12.8px) left: got ({dx}, {dy})" ); assert!( alpha(&s, "CharacterBag1Slot") < 0.01, @@ -1818,19 +1823,19 @@ fn an_item_push_drops_its_icon_into_the_bag_that_took_it() { "opaque by the alpha track's second key" ); assert!( - (size(&s, "CharacterBag1Slot") - 36.0 * 1.2).abs() < 0.5, - "swollen to 1.2x the 36px icon at the scale track's peak" + (size(&s, "CharacterBag1Slot") - 38.4 * 1.2).abs() < 0.5, + "swollen to 1.2x the 38.4px card at the scale track's peak" ); // t=0.267: back to the icon's own size, still opaque, still parked. s.tick(0.134); s.resolve(); assert!( - (size(&s, "CharacterBag1Slot") - 36.0).abs() < 0.5, - "settled back to the 36px icon at the scale track's third key" + (size(&s, "CharacterBag1Slot") - 38.4).abs() < 0.5, + "settled back to the 38.4px card at the scale track's third key" ); let (_, dy) = offset(&s, "CharacterBag1Slot"); - assert!((dy - 48.0).abs() < 0.5, "has not started falling: got {dy}"); + assert!((dy - 51.2).abs() < 0.5, "has not started falling: got {dy}"); // t=0.5: STILL PARKED — the translation track's second key is (0,0), so the whole first half // second is a hang above the bar. The other two tracks are already running down, though: the @@ -1840,11 +1845,11 @@ fn an_item_push_drops_its_icon_into_the_bag_that_took_it() { s.resolve(); let (_, dy) = offset(&s, "CharacterBag1Slot"); assert!( - (dy - 48.0).abs() < 0.5, + (dy - 51.2).abs() < 0.5, "has not moved yet at the half second: got {dy}" ); assert!( - (size(&s, "CharacterBag1Slot") - 36.0 * 0.686).abs() < 0.5, + (size(&s, "CharacterBag1Slot") - 38.4 * 0.686).abs() < 0.5, "but is already dwindling: got {}", size(&s, "CharacterBag1Slot") ); @@ -1859,7 +1864,7 @@ fn an_item_push_drops_its_icon_into_the_bag_that_took_it() { s.resolve(); let (_, dy) = offset(&s, "CharacterBag1Slot"); assert!( - (dy - 24.0).abs() < 0.5, + (dy - 25.6).abs() < 0.5, "half the fall travelled by t=0.75: got {dy}" ); diff --git a/crates/benilla-app/src/ui_script/lifecycle.rs b/crates/benilla-app/src/ui_script/lifecycle.rs index 2292e199..022d66c1 100644 --- a/crates/benilla-app/src/ui_script/lifecycle.rs +++ b/crates/benilla-app/src/ui_script/lifecycle.rs @@ -48,7 +48,7 @@ pub(crate) fn setup_script(world: &mut World) { /// dropped the VM and skipped index 0 on the in-game load would fail exactly there — verified in /// wow-re's §5 for this change (1291). fn install_boot_vm(world: &mut World) { - let script = match UiScript::new() { + let mut script = match UiScript::new() { Ok(s) => s, Err(e) => { error!("ui_script: VM init failed: {e}"); @@ -56,6 +56,7 @@ fn install_boot_vm(world: &mut World) { return; } }; + install_texture_resolvers(world, &mut script); load_global_strings(world, &script); load_emote_tokens(world, &script); if ui_wanted(world) { @@ -66,6 +67,33 @@ fn install_boot_vm(world: &mut World) { world.insert_non_send_resource(script); } +/// Wire up the two halves of `Interface\AddOns\` texture art (decision 1322): the sprite +/// decoder's **loose-file root** (so addon-shipped BLP/TGA files render at all — the store's +/// [`benilla_assets::WorldAssets::set_loose_sprite_root`]) and the VM's **texture probe** (so the +/// path form of `SetTexture` can answer the reference's 1|nil load verdict — Atlas picks its map +/// art by that return). Both resolve the same one folder ([`addons::root`], hermetic-`None` under +/// `$WOW_CAPTURE`), and the probe walks the same [`benilla_assets::sprite_candidates`] the +/// renderer decodes with, so the verdict the Lua caller gets is the verdict the screen shows. +/// +/// No `WorldAssets` (no client data) means no backend: nothing to install, and the VM's path form +/// keeps answering nil — the engine-less truth. +fn install_texture_resolvers(world: &mut World, script: &mut UiScript) { + let root = addons::root(); + let Some(mut assets) = world.get_resource_mut::() else { + return; + }; + assets.set_loose_sprite_root(root.clone()); + let chain = assets.chain.clone(); + script.set_texture_probe(Box::new(move |path| { + benilla_assets::sprite_candidates(path).iter().any(|c| { + chain.lock_recover().contains(c) + || root + .as_deref() + .is_some_and(|r| benilla_assets::loose_sprite_file(r, c).is_some()) + }) + })); +} + /// Does this run want the player UI at all? Captures stay pristine — their baselines regression-test /// the WORLD render — unless `WOW_CAPTURE_UI=1` opts the UI in. fn ui_wanted(world: &World) -> bool { diff --git a/crates/benilla-app/src/ui_script/pet_bar_tests.rs b/crates/benilla-app/src/ui_script/pet_bar_tests.rs index dbed812c..a0fbacf4 100644 --- a/crates/benilla-app/src/ui_script/pet_bar_tests.rs +++ b/crates/benilla-app/src/ui_script/pet_bar_tests.rs @@ -647,20 +647,23 @@ fn the_keybind_pair_pushes_and_casts_without_the_clicks_forks() { /// **B228 — the autocast trail's motion, MEASURED off the shipped handler.** /// -/// nazriel reads the shine as "a bit too fast". The health pass answered that from the constants — -/// the .m2 authors a 2.000 s loop and `BENILLA_PET_AUTOCAST_PERIOD` is 2.0 — which is a read, not a -/// measurement: it says nothing about whether the OnUpdate wiring turns wall-clock seconds into -/// that period, and an `elapsed` in the wrong unit would be invisible to it. So this drives the -/// real `` with real seconds and reads the head spark's own SetPoint back. +/// nazriel reads the shine as "a bit too fast", and the answer turned out to be in the widget's +/// CLOCK, which no constants read could have reached. `CSimpleModel`'s per-frame OnUpdate advances +/// its private `CM2Scene` by `__ftol(elapsed * 1000.0)` — **truncated, no fractional carry**, where +/// the world's driver adds 0.5 first (wow-re `modelframe-animation-clock.md`, §5 cross-checked). +/// So the reference's 2000 ms band actually takes `2000 * T / floor(T)` ms at frame time T, and is +/// **never faster than 2.000 s**. An exact 2.000 s emulation — which is what a measurement of our +/// old wiring confirmed we had — was therefore faster than any real client at any frame rate. /// -/// What is pinned, all of it off the file (`m2bones` on `Interface\Buttons\UI-AutoCastButton.m2`): -/// a 28.8 px square (0.02 units × 1200 px/unit × the ref Model's 1.2), one lap per 2.000 s, a -/// corner every 0.500 s, LINEAR between them (`interp == 1` at the bone record's translation -/// M2Track — so the half-edge sample sits at the half-time, no easing), and the four emitters one -/// quarter-lap apart. If the reported overspeed is in the period, it fails here; if this holds and -/// the report stands, the period is exonerated and the gap is in the trail's SHAPE. +/// This drives the real `` with real seconds at real frame times and reads the head +/// spark's own SetPoint back. What is pinned: the lap comes out SLOW by exactly the truncation's +/// amount at 60 and 144 fps; the geometry is the 30.72 px square the widget's ortho projection puts +/// on a 30 px button (0.02 units × 1280 × the ref Model's `SetModelScale` 1.2), origin at the +/// button's BOTTOM-LEFT because that is the model's own origin; a corner every quarter lap, LINEAR +/// between them (`interp == 1` at the bone record's translation M2Track, so the half-edge sample +/// sits at the half-time); and the four emitters one quarter-lap apart. #[test] -fn the_autocast_trail_laps_the_rim_once_every_two_seconds() { +fn the_autocast_trail_laps_the_rim_the_reference_widgets_slow_way() { let mut s = UiScript::new().unwrap(); s.set_screen_size(1024.0, 768.0); load_pet_bar(&s); @@ -692,57 +695,76 @@ fn the_autocast_trail_laps_the_rim_once_every_two_seconds() { ); }; - // 0.02 units × 1200 px/unit × 1.2 = 28.8 px across, so the corners sit ±14.4 from centre. - const H: f32 = 14.4; - let (bl, tl, tr, br) = ((-H, -H), (-H, H), (H, H), (H, -H)); + // 0.02 units × 1280 px/unit × 1.2 = 30.72 px across, laid from the button's BOTTOMLEFT — so + // the walk runs 0..30.72 in both axes, not ±half from the centre. + const S: f32 = 30.72; + let (bl, tl, tr, br) = ((0.0, 0.0), (0.0, S), (S, S), (S, 0.0)); - // ── The clock WIRING: four half-second ticks of the shipped OnUpdate, one corner each. This - // is the half a constants read cannot reach — `arg1` really is seconds, and the bar's clock - // advances 1:1 with it. - let corner = |s: &mut UiScript, dt: f32, want: (f32, f32), what: &str| { - s.tick(dt); - near(head(s, 0), want, what); + // ── The CLOCK, at a real frame time. 60 fps ⇒ dt = 16.667 ms ⇒ only **16** banked per frame, + // so the 2000 ms band needs 125 frames = **2.083 s of wall clock**. At 2.000 s (120 frames) + // the clock has banked 1920 ms = 0.96 of a lap: the head is still on the last edge, running + // right-to-left toward home and 0.04 × 122.88 = 4.92 px short of it. That shortfall IS the + // bug — it is how much slower than our old exact-2.000 s clock the reference has always been. + let frames = |s: &mut UiScript, n: usize, dt: f32| { + for _ in 0..n { + s.tick(dt); + } }; - corner( - &mut s, - 0.5, - tl, - "0.5 s in: one edge, at the top-left corner", + const DT60: f32 = 1.0 / 60.0; + frames(&mut s, 120, DT60); // 2.000 s of wall clock + near( + head(&s, 0), + (4.9152, 0.0), + "2.000 s at 60 fps: NOT home yet — the reference is slower than the authored band", ); - corner(&mut s, 0.5, tr, "1.0 s: half a lap"); - corner(&mut s, 0.5, br, "1.5 s: three quarters"); - corner( - &mut s, - 0.5, + frames(&mut s, 5, DT60); // 2.083 s + near( + head(&s, 0), bl, - "2.0 s: exactly one lap, back where it started", + "2.083 s at 60 fps: one lap, the reference's own 2000*T/floor(T)", ); - // ── The position LAW, driven at exact clocks so no tick accumulation can blur it. The - // mid-edge sample is the `interp == 1` pin: linear means the half-time is the half-edge, and - // a spline-eased track would sit short of it. - let at = |s: &UiScript, clock: f32| -> (f32, f32) { + // 144 fps truncates harder — 6.944 ms banked as 6 — so the same lap takes 2.315 s there. A + // frame-rate-independent clock would land both of these at 2.000 s and fail here. + let mut fast = UiScript::new().unwrap(); + fast.set_screen_size(1024.0, 768.0); + load_pet_bar(&fast); + declare_token_strings(&fast); + fast.set_pet_actions(true, true, true, hunter_slots()); + fast.fire_event("PET_BAR_UPDATE", vec![]); + fast.resolve(); + frames(&mut fast, 333, 1.0 / 144.0); // 2.313 s of wall clock ⇒ 1998 ms banked + let (fx, fy) = head(&fast, 0); + assert!( + fy == 0.0 && fx > 0.0 && fx < 1.0, + "2.313 s at 144 fps: still 2 ms short of the lap, at {fx:?},{fy:?}" + ); + + // ── The position LAW, driven at exact MILLISECOND clocks so no tick accumulation can blur it. + // The mid-edge sample is the `interp == 1` pin: linear means the half-time is the half-edge, + // and a spline-eased track would sit short of it. + let at = |s: &UiScript, ms: u32| -> (f32, f32) { s.run(&format!( - "BenillaPetAutocast_Update(PetActionButton4, {clock})" + "BenillaPetAutocast_Update(PetActionButton4, {ms})" )) .unwrap(); head(s, 0) }; - near(at(&s, 0.0), bl, "clock 0"); + near(at(&s, 0), bl, "clock 0"); near( - at(&s, 0.25), - (-H, 0.0), + at(&s, 250), + (0.0, S / 2.0), "quarter edge in: the LEFT edge's midpoint", ); - near(at(&s, 0.5), tl, "clock 0.5"); - near(at(&s, 1.0), tr, "clock 1.0"); - near(at(&s, 1.5), br, "clock 1.5"); - near(at(&s, 2.0), bl, "clock 2.0 wraps onto clock 0"); + near(at(&s, 500), tl, "clock 500 ms"); + near(at(&s, 1000), tr, "clock 1000 ms"); + near(at(&s, 1500), br, "clock 1500 ms"); + near(at(&s, 2000), bl, "clock 2000 ms wraps onto clock 0"); // ── The trail is CONTINUOUS, which is what deriving the sample count buys (B228). No two // neighbours in a trail sit further apart than the smaller of their two stars is wide, so the // streak has no seam at any age. The 8-even-samples version failed this from mid-life back — - // 7.2 px apart with 4.3 px stars — and a row of separated dots is what reads as marching. + // 7.7 px apart with 4.6 px stars — and a row of separated dots is what reads as marching. let worst_gap: f32 = s .eval( "local worst = 0 \ @@ -765,7 +787,7 @@ fn the_autocast_trail_laps_the_rim_once_every_two_seconds() { // ── The four emitters are ONE LAP, a quarter apart — the file's four bones at the four // corners, chasing each other (m2bones: pivots at (-0.001,0), (0.019,0), (0.019,0.02), // (-0.001,0.02), same track, one corner of phase between neighbours). - s.run("BenillaPetAutocast_Update(PetActionButton4, 0.0)") + s.run("BenillaPetAutocast_Update(PetActionButton4, 0)") .unwrap(); for (e, want, name) in [ (0, bl, "emitter 0"), diff --git a/crates/benilla-assets/src/world_assets/mod.rs b/crates/benilla-assets/src/world_assets/mod.rs index 659fabcb..cc80c2b5 100644 --- a/crates/benilla-assets/src/world_assets/mod.rs +++ b/crates/benilla-assets/src/world_assets/mod.rs @@ -17,6 +17,7 @@ //! a texture+blend share a material handle — which is what lets Bevy batch draws. use std::collections::HashMap; +use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex, MutexGuard}; use bevy::image::Image; @@ -25,7 +26,7 @@ use bevy::render::render_resource::{Buffer, Face}; use crate::materials::{WowModelExt, WowModelMaterial, VANILLA_ALPHA_KEY_REF}; use crate::SpatialCache; -use benilla_formats::{read_texture_mip_chain, read_texture_rgba, Chain, ModelBlend}; +use benilla_formats::{blp_to_rgba, read_texture_mip_chain, tga_to_rgba, Chain, ModelBlend}; /// BLP/RGBA -> Bevy `Image` texture helpers (+ solid/cursor/liquid-frame textures). Split out for /// size; re-exported so callers keep using `benilla_assets::{solid_layer_chain, …}`. @@ -69,6 +70,16 @@ pub struct WorldAssets { /// (decision 0203). Its own cache for the same reason as the others: the format is baked into /// the GPU image. masks: HashMap>>, + /// The **loose-file root for `Interface\AddOns\` sprite paths** — the AddOns folder, installed + /// by the app ([`Self::set_loose_sprite_root`]), `None` until then and in every capture run + /// (the app resolves it through `local_state`, which is hermetic under `$WOW_CAPTURE`). + /// + /// Addon art never lives in an MPQ: the reference's Storm open reads loose files from the game + /// directory, which is how `` works at all. + /// Our equivalent maps that virtual prefix onto the one addon root (decision 1185's single + /// folder; decision 1322). Only the UI-sprite decoders consult it — world art has no business + /// under `AddOns\`. + loose_root: Option, /// Materials deduped by their identity (texture path + blend, or the untextured fallback). pub model_materials: SpatialCache>, /// The shared global-light storage buffer (`lighting::global_light`). Held here so `model_material` @@ -165,24 +176,92 @@ pub fn sprite_candidates(path: &str) -> [String; 2] { /// runtime from a DBC. /// /// Walks [`sprite_candidates`] in order and takes the first that both reads and decodes; only when -/// **both** fail is it a miss. A candidate that reads but won't decode falls through exactly like +/// **all** fail is it a miss. A candidate that reads but won't decode falls through exactly like /// one that isn't there — the reference's two loaders both fail silently, so a bad first candidate /// can't poison the second. -fn decode_sprite(chain: &Mutex, path: &str) -> Option<(u32, u32, Vec)> { +/// +/// Each candidate is asked of **two stores**: the patch chain, then — for `Interface\AddOns\` +/// paths — the loose addon folder ([`loose_sprite_file`], decision 1322). The chain never holds an +/// `AddOns\` path (Blizzard's own `Blizzard_*` stubs aside, addon art only exists on disk), so the +/// order between the stores is unobservable; chain-first keeps every non-addon path on exactly the +/// code it always ran. +fn decode_sprite( + chain: &Mutex, + loose_root: Option<&Path>, + path: &str, +) -> Option<(u32, u32, Vec)> { let candidates = sprite_candidates(path); let mut chain = chain.lock_recover(); for candidate in &candidates { - if let Ok(decoded) = read_texture_rgba(&mut chain, candidate) { - return Some(decoded); + if let Ok(bytes) = chain.read_file(candidate) { + if let Ok(decoded) = decode_sprite_bytes(&bytes) { + return Some(decoded); + } + } + if let Some(file) = loose_root.and_then(|root| loose_sprite_file(root, candidate)) { + if let Ok(decoded) = std::fs::read(&file) + .map_err(anyhow::Error::from) + .and_then(|bytes| decode_sprite_bytes(&bytes)) + { + return Some(decoded); + } } } warn!( - "texture miss: '{path}' does not resolve in the patch chain (tried {})", + "texture miss: '{path}' does not resolve in the patch chain{} (tried {})", + if loose_root.is_some() { + " or the AddOns folder" + } else { + "" + }, candidates.join(", ") ); None } +/// Decode one candidate's bytes to RGBA8, picking the format by content: a `BLP2` magic is a BLP, +/// anything else is offered to the TGA decoder (TGA has no magic; its header validation is the +/// gate). Content, not extension, because the ecosystem mislabels freely — a BLP named `.tga` and +/// the reverse both ship in real addon folders, and the pixel bytes always know what they are. +fn decode_sprite_bytes(bytes: &[u8]) -> anyhow::Result<(u32, u32, Vec)> { + if bytes.starts_with(b"BLP2") { + blp_to_rgba(bytes) + } else { + tga_to_rgba(bytes) + } +} + +/// Map a **normalized** sprite candidate (`interface\addons\\<…>.blp`, lowercase, +/// backslashed — [`normalize_path`] has run) onto a file under the loose addon root, or `None` if +/// it is not an `Interface\AddOns\` path or nothing is there. +/// +/// The walk matches each component **case-insensitively** (exact join first — free on the +/// case-insensitive filesystems macOS installs default to — then a `read_dir` scan): the reference +/// is a Windows client and addons reference their own art in arbitrary case. The candidate cannot +/// escape the root — `normalize_path` leaves no `/`, and any dot-component is refused before a +/// filesystem call happens (same lexical posture as `ui_script::addons::read_under`). +pub fn loose_sprite_file(root: &Path, candidate: &str) -> Option { + let rel = candidate.strip_prefix("interface\\addons\\")?; + let mut at = root.to_path_buf(); + for comp in rel.split('\\') { + if comp.is_empty() || comp.starts_with('.') { + return None; + } + let direct = at.join(comp); + if direct.exists() { + at = direct; + continue; + } + let found = std::fs::read_dir(&at).ok()?.flatten().find(|e| { + e.file_name() + .to_str() + .is_some_and(|n| n.eq_ignore_ascii_case(comp)) + })?; + at = found.path(); + } + at.is_file().then_some(at) +} + /// Mutate an asset only when `differs` says its current state doesn't already match. /// /// `Assets::get_mut` alone marks the asset Modified — a uniform re-upload and (on the Metal @@ -256,11 +335,27 @@ impl WorldAssets { tiled_sprites: HashMap::new(), portraits: HashMap::new(), masks: HashMap::new(), + loose_root: None, model_materials: SpatialCache::default(), shared_light, } } + /// Install (or clear) the loose-file root for `Interface\AddOns\` sprite paths — see the + /// [`Self::loose_root`] field doc. Called by the app once it knows the AddOns folder; evicts + /// cached **misses** so a path asked before the root existed gets a second look (a cached hit + /// can only have come from the chain and stays right). + pub fn set_loose_sprite_root(&mut self, root: Option) { + if self.loose_root == root { + return; + } + self.loose_root = root; + self.sprites.retain(|_, v| v.is_some()); + self.tiled_sprites.retain(|_, v| v.is_some()); + self.portraits.retain(|_, v| v.is_some()); + self.masks.retain(|_, v| v.is_some()); + } + /// Decode + upload a BLP once per path; later requests for the same texture share the handle. /// `None` if the texture is missing or fails to decode. pub fn texture( @@ -300,7 +395,7 @@ impl WorldAssets { if let Some(cached) = self.sprites.get(&key) { return cached.clone(); } - let loaded = decode_sprite(&self.chain, path) + let loaded = decode_sprite(&self.chain, self.loose_root.as_deref(), path) .map(|(w, h, rgba)| images.add(sprite_image(w, h, rgba))); self.sprites.insert(key, loaded.clone()); loaded @@ -311,7 +406,7 @@ impl WorldAssets { /// which resizes the 128×32 frame to the plate's exact size). Same extensionless→`.blp` resolve /// as [`Self::sprite_texture`]; not cached (a one-shot decode at load, not a per-frame ask). pub fn decode_rgba(&mut self, path: &str) -> Option<(u32, u32, Vec)> { - decode_sprite(&self.chain, path) + decode_sprite(&self.chain, self.loose_root.as_deref(), path) } /// A UI sprite decoded with **repeat** (wrap) addressing — the frame `Backdrop` tiled pieces @@ -328,7 +423,7 @@ impl WorldAssets { if let Some(cached) = self.tiled_sprites.get(&key) { return cached.clone(); } - let loaded = decode_sprite(&self.chain, path) + let loaded = decode_sprite(&self.chain, self.loose_root.as_deref(), path) .map(|(w, h, rgba)| images.add(sprite_image_tiled(w, h, rgba))); self.tiled_sprites.insert(key, loaded.clone()); loaded @@ -347,7 +442,7 @@ impl WorldAssets { if let Some(cached) = self.portraits.get(&key) { return cached.clone(); } - let loaded = decode_sprite(&self.chain, path) + let loaded = decode_sprite(&self.chain, self.loose_root.as_deref(), path) .map(|(w, h, rgba)| images.add(portrait_image(w, h, rgba))); self.portraits.insert(key, loaded.clone()); loaded @@ -365,8 +460,8 @@ impl WorldAssets { if let Some(cached) = self.masks.get(&key) { return cached.clone(); } - let loaded = - decode_sprite(&self.chain, path).map(|(w, h, rgba)| images.add(mask_image(w, h, rgba))); + let loaded = decode_sprite(&self.chain, self.loose_root.as_deref(), path) + .map(|(w, h, rgba)| images.add(mask_image(w, h, rgba))); self.masks.insert(key, loaded.clone()); loaded } @@ -381,8 +476,11 @@ impl WorldAssets { path: &str, images: &mut Assets, ) -> Option> { - let (w, h, rgba) = - read_texture_rgba(&mut self.chain.lock_recover(), &normalize_path(path)).ok()?; + let (w, h, rgba) = benilla_formats::read_texture_rgba( + &mut self.chain.lock_recover(), + &normalize_path(path), + ) + .ok()?; Some(images.add(cursor_texture(w, h, rgba))) } @@ -519,3 +617,47 @@ pub struct RenderConfig { pub enum AssetSet { Open, } + +#[cfg(test)] +mod tests { + use super::*; + + /// The disk tree spells its names in mixed case, the candidate arrives normalized-lowercase + /// (as [`sprite_candidates`] always hands it over) — the walk must land on the file anyway, + /// because the reference is a Windows client and addon authors never matched their own case. + #[test] + fn loose_sprite_file_maps_addon_paths_case_insensitively() { + let root = + std::env::temp_dir().join(format!("benilla-loose-sprite-test-{}", std::process::id())); + let dir = root.join("Atlas").join("Images").join("Maps"); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("BlackrockDepths.blp"), b"x").unwrap(); + + // The returned SPELLING is filesystem-dependent (a case-insensitive filesystem answers + // the exact-join fast path with the candidate's own casing), so the claim is that the + // path opens the right file, not how it is spelt. + let hit = loose_sprite_file( + &root, + "interface\\addons\\atlas\\images\\maps\\blackrockdepths.blp", + ) + .expect("mixed-case tree resolves a lowercase candidate"); + assert_eq!(std::fs::read(&hit).unwrap(), b"x"); + + // Not an AddOns path → not this store's question. + assert_eq!(loose_sprite_file(&root, "interface\\icons\\foo.blp"), None); + // A directory is not a file, and a missing file is a miss, not an error. + assert_eq!(loose_sprite_file(&root, "interface\\addons\\atlas"), None); + assert_eq!( + loose_sprite_file(&root, "interface\\addons\\atlas\\images\\maps\\nope.blp"), + None + ); + // Dot-components never reach the filesystem (the read_under posture; `normalize_path` + // already forbids `/`, so this is the one lexical escape left to refuse). + assert_eq!( + loose_sprite_file(&root, "interface\\addons\\..\\..\\etc\\passwd"), + None + ); + + std::fs::remove_dir_all(&root).ok(); + } +} diff --git a/crates/benilla-formats/src/lib.rs b/crates/benilla-formats/src/lib.rs index 091f5933..1f5fa129 100644 --- a/crates/benilla-formats/src/lib.rs +++ b/crates/benilla-formats/src/lib.rs @@ -16,6 +16,9 @@ use benilla_dbc::{DbcParser, FieldType, Schema, SchemaField}; mod chain; pub use chain::{Chain, ChainEntry}; +/// The UI texture table's other half — loose `.tga` art (addon folders; decision 1322). +mod tga; +pub use tga::tga_to_rgba; /// Where the WoW install is — the one resolver (decision 1175). Paired with [`Chain`]: this says /// *where*, that opens it. mod install; diff --git a/crates/benilla-formats/src/tga.rs b/crates/benilla-formats/src/tga.rs new file mode 100644 index 00000000..b5ba2c65 --- /dev/null +++ b/crates/benilla-formats/src/tga.rs @@ -0,0 +1,213 @@ +//! TGA (Targa) decoder — the OTHER half of the reference's UI texture table. +//! +//! `TextureCreate`'s extension table is `{".tga", ".blp"}` (`0x835248`; see +//! `world_assets::sprite_candidates` in benilla-assets), so every UI sprite reference is two +//! candidate files and one of them is a TGA. Inside the MPQs that arm is a near-no-op — 1.12 ships +//! its UI art as BLP — but **addon folders are where TGAs actually live**: BLP tooling was rare in +//! 2006 and the ecosystem shipped loose `.tga` art constantly, so the loose-file resolve +//! (decision 1322) is what finally makes this decoder load-bearing. +//! +//! The slice implemented is the slice that art actually uses: image types 2/10 (truecolor, +//! raw/RLE) and 3/11 (grayscale, raw/RLE), 8/24/32-bit pixels, both vertical origins and the +//! (vanishingly rare) right-origin mirror. Color-mapped TGAs (types 1/9) are refused — none have +//! surfaced in the corpus, and a silent palette guess is worse than a loud miss. +//! +//! [`tga_to_rgba`] validates the header before touching pixels, so it is safe to use as a +//! *fallback* decoder on bytes of uncertain format (TGA has no magic; the caller sniffs `BLP2` +//! first and only then tries this — a non-TGA almost always fails the type/depth/size checks). + +use anyhow::{bail, Result}; + +/// Decode a TGA to RGBA8: `(width, height, pixels)` — top-down row order, like +/// [`crate::blp_to_rgba`]. +pub fn tga_to_rgba(bytes: &[u8]) -> Result<(u32, u32, Vec)> { + let Some(header) = bytes.get(..18) else { + bail!("TGA: truncated header ({} bytes)", bytes.len()); + }; + let id_length = header[0] as usize; + let color_map_type = header[1]; + let image_type = header[2]; + let color_map_len = u16::from_le_bytes([header[5], header[6]]) as usize; + let color_map_entry_bits = header[7] as usize; + let width = u16::from_le_bytes([header[12], header[13]]) as u32; + let height = u16::from_le_bytes([header[14], header[15]]) as u32; + let bpp = header[16]; + let descriptor = header[17]; + + let rle = match image_type { + 2 | 3 => false, + 10 | 11 => true, + 1 | 9 => bail!("TGA: color-mapped image (type {image_type}) not supported"), + t => bail!("TGA: unknown image type {t}"), + }; + let grayscale = matches!(image_type, 3 | 11); + let bytes_per_pixel = match (bpp, grayscale) { + (24, false) => 3, + (32, false) => 4, + (8, true) => 1, + _ => bail!("TGA: unsupported depth {bpp} for image type {image_type}"), + }; + if width == 0 || height == 0 || width > 8192 || height > 8192 { + bail!("TGA: implausible dimensions {width}x{height}"); + } + + // Skip the id field and (for a truecolor image that still carries one) the color map. + let map_bytes = if color_map_type == 1 { + color_map_len * color_map_entry_bits.div_ceil(8) + } else { + 0 + }; + let data_start = 18 + id_length + map_bytes; + let Some(data) = bytes.get(data_start..) else { + bail!("TGA: truncated before pixel data"); + }; + + let pixel_count = (width as usize) * (height as usize); + let mut pixels = Vec::with_capacity(pixel_count * bytes_per_pixel); + if rle { + // RLE packets: header bit 7 = run (one pixel, repeated count times), else raw + // (count pixels follow); count = low 7 bits + 1. Runs may NOT cross the image end. + let mut at = 0usize; + while pixels.len() < pixel_count * bytes_per_pixel { + let Some(&packet) = data.get(at) else { + bail!("TGA: RLE stream truncated"); + }; + at += 1; + let count = ((packet & 0x7f) as usize) + 1; + if packet & 0x80 != 0 { + let Some(px) = data.get(at..at + bytes_per_pixel) else { + bail!("TGA: RLE run truncated"); + }; + at += bytes_per_pixel; + for _ in 0..count { + pixels.extend_from_slice(px); + } + } else { + let n = count * bytes_per_pixel; + let Some(px) = data.get(at..at + n) else { + bail!("TGA: RLE raw packet truncated"); + }; + at += n; + pixels.extend_from_slice(px); + } + } + pixels.truncate(pixel_count * bytes_per_pixel); + } else { + let n = pixel_count * bytes_per_pixel; + let Some(px) = data.get(..n) else { + bail!("TGA: pixel data truncated"); + }; + pixels.extend_from_slice(px); + } + + // BGR(A)/gray → RGBA, honouring the descriptor's origin bits (bit 5: top-origin, bit 4: + // right-origin — TGA's default is bottom-left, our output is top-down left-to-right). + let top_down = descriptor & 0x20 != 0; + let right_first = descriptor & 0x10 != 0; + let mut rgba = vec![0u8; pixel_count * 4]; + for y in 0..height as usize { + let src_y = if top_down { y } else { height as usize - 1 - y }; + for x in 0..width as usize { + let src_x = if right_first { + width as usize - 1 - x + } else { + x + }; + let s = (src_y * width as usize + src_x) * bytes_per_pixel; + let d = (y * width as usize + x) * 4; + match bytes_per_pixel { + 1 => { + let g = pixels[s]; + rgba[d..d + 4].copy_from_slice(&[g, g, g, 0xff]); + } + 3 => { + rgba[d..d + 4].copy_from_slice(&[pixels[s + 2], pixels[s + 1], pixels[s], 0xff]) + } + _ => { + rgba[d..d + 4].copy_from_slice(&[ + pixels[s + 2], + pixels[s + 1], + pixels[s], + pixels[s + 3], + ]); + } + } + } + } + Ok((width, height, rgba)) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// A minimal TGA: 18-byte header + pixels. + fn tga(image_type: u8, w: u16, h: u16, bpp: u8, descriptor: u8, data: &[u8]) -> Vec { + let mut out = vec![0u8; 18]; + out[2] = image_type; + out[12..14].copy_from_slice(&w.to_le_bytes()); + out[14..16].copy_from_slice(&h.to_le_bytes()); + out[16] = bpp; + out[17] = descriptor; + out.extend_from_slice(data); + out + } + + #[test] + fn uncompressed_32bit_bottom_origin_flips_and_swizzles() { + // 2 wide × 2 tall, bottom-up rows: file row 0 is the image's BOTTOM row. + // Bottom row: blue, green; top row: red, half-alpha white — as BGRA. + let px = [ + 255, 0, 0, 255, /* blue */ 0, 255, 0, 255, /* green */ + 0, 0, 255, 255, /* red */ 255, 255, 255, 128, /* white a=128 */ + ]; + let (w, h, rgba) = tga_to_rgba(&tga(2, 2, 2, 32, 0, &px)).unwrap(); + assert_eq!((w, h), (2, 2)); + // Top-down output: row 0 = red, white; row 1 = blue, green — as RGBA. + assert_eq!( + rgba, + vec![ + 255, 0, 0, 255, 255, 255, 255, 128, // + 0, 0, 255, 255, 0, 255, 0, 255, + ] + ); + } + + #[test] + fn uncompressed_24bit_top_origin_gets_opaque_alpha() { + let px = [255, 0, 0, /* blue */ 0, 0, 255 /* red */]; + let (w, h, rgba) = tga_to_rgba(&tga(2, 2, 1, 24, 0x20, &px)).unwrap(); + assert_eq!((w, h), (2, 1)); + assert_eq!(rgba, vec![0, 0, 255, 255, 255, 0, 0, 255]); + } + + #[test] + fn rle_run_and_raw_packets_expand() { + // 4×1 top-origin: a run of 3 red + a raw packet of 1 green (BGR, 24-bit). + let data = [0x82, 0, 0, 255, 0x00, 0, 255, 0]; + let (w, _, rgba) = tga_to_rgba(&tga(10, 4, 1, 24, 0x20, &data)).unwrap(); + assert_eq!(w, 4); + assert_eq!( + rgba, + vec![ + 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, // + 0, 255, 0, 255, + ] + ); + } + + #[test] + fn grayscale_replicates_channels() { + let (_, _, rgba) = tga_to_rgba(&tga(3, 1, 1, 8, 0x20, &[7])).unwrap(); + assert_eq!(rgba, vec![7, 7, 7, 255]); + } + + #[test] + fn garbage_and_unsupported_types_are_loud_misses() { + assert!(tga_to_rgba(b"BLP2xxxxxxxxxxxxxxxxxx").is_err()); // a BLP is not a TGA + assert!(tga_to_rgba(&[0u8; 4]).is_err()); // truncated header + assert!(tga_to_rgba(&tga(1, 2, 2, 8, 0, &[0; 32])).is_err()); // color-mapped + assert!(tga_to_rgba(&tga(2, 2, 2, 16, 0, &[0; 32])).is_err()); // 16-bit + assert!(tga_to_rgba(&tga(2, 2, 2, 32, 0, &[0; 4])).is_err()); // truncated pixels + } +} diff --git a/crates/benilla-ui/src/script/mod.rs b/crates/benilla-ui/src/script/mod.rs index 18a9672f..1cc74506 100644 --- a/crates/benilla-ui/src/script/mod.rs +++ b/crates/benilla-ui/src/script/mod.rs @@ -154,6 +154,7 @@ pub use mail::{MailInboxRow, MailSendRequest, MailState}; pub use measure::TextMeasure; pub use merchant::{ItemStatsHead, MerchantItem, MerchantState}; pub(crate) use model::Model; +pub use model::TextureProbe; pub use party::{PartyMemberInfo, PartyRequest, PartyState, RaidMemberInfo}; pub use pet::{PetActionView, PetStats}; pub use quest::{QuestAction, QuestItemView, QuestPanel, QuestSelect, QuestState}; diff --git a/crates/benilla-ui/src/script/model.rs b/crates/benilla-ui/src/script/model.rs index a6d09c24..1a192bba 100644 --- a/crates/benilla-ui/src/script/model.rs +++ b/crates/benilla-ui/src/script/model.rs @@ -15,6 +15,9 @@ use super::{ // The Rust-side model (plain data; lives in lua.app_data) // ───────────────────────────────────────────────────────────────────────────────────────────── +/// The host's texture-path resolvability oracle — see [`Model::texture_probe`]. +pub type TextureProbe = Box bool>; + /// The Rust-side model behind the Lua VM — the arena, the layout inputs + resolved rects, the /// id↔handle bijection, region visuals, and the event/script registrations. Held in `lua.app_data` /// (interior-mutable) so callbacks reach it; contains **no** mlua handles (the MAXCSTACK discipline). @@ -28,6 +31,13 @@ pub(crate) struct Model { /// answer inside the Lua call that asked, instead of a frame later. `None` in every VM without /// a font atlas, which is the measure round-trip's own world and behaves exactly as it did. pub(crate) measurer: Option>, + /// The host's texture-path oracle ([`super::UiScript::set_texture_probe`]): does this sprite + /// reference resolve to a file — patch chain or loose addon folder? What lets the path form of + /// `SetTexture` return the reference's **1 | nil** load verdict inline (wow-re + /// `widget-api-batch-benilla.md` Q1 — Atlas branches on it to pick its map art). `None` in an + /// engine-less VM (tests, the addon harness), where the path form keeps answering nil: no + /// backend, nothing loads — which is also exactly what those tests always saw. + pub(crate) texture_probe: Option, /// Where per-addon saved variables live: the account-scoped folder, then this character's. /// Both are directories holding one `.lua` per declaring addon. pub(crate) addons_saved_account: Option, @@ -1040,6 +1050,7 @@ impl Model { addons: Vec::new(), addons_root: None, measurer: None, + texture_probe: None, addons_saved_account: None, addons_saved_character: None, framexml_templates: Default::default(), diff --git a/crates/benilla-ui/src/script/object/frame_state.rs b/crates/benilla-ui/src/script/object/frame_state.rs index 86bac911..c01f6c02 100644 --- a/crates/benilla-ui/src/script/object/frame_state.rs +++ b/crates/benilla-ui/src/script/object/frame_state.rs @@ -198,34 +198,99 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { } })?, )?; + // SetParent — the runtime reparent, per the byte-verified law (wow-re + // `ui/scratch/setparent-runtime-strata-level.md`; decision 1323). The binding half + // (`0x7a1550`): the parent argument is a frame table, a NAME string (`0x76c760`), or an + // explicit nil — an ABSENT argument is NOT the nil path (`0x6f3400` returns −1) and raises + // like an unresolvable name; the cycle guard is the binding's own inline ancestor walk and + // RAISES (`0x87cb14`), it does not silently no-op. The arena half is + // `reparent_begin`/`reparent_finish`, split so `OnHide` fires between them the way the + // reference's hide→show round-trip does — `fire_visibility_changes` reads each frame's live + // state for direction, so hide and show must be fired from their own phase, never one + // combined list. m.set( "SetParent", - lua.create_function(|lua, (this, parent): (Table, Value)| { + lua.create_function(|lua, args: mlua::MultiValue| { + let mut it = args.into_iter(); + let this = match it.next() { + Some(Value::Table(t)) => t, + _ => return Err(mlua::Error::runtime("expected a frame")), + }; let h = frame_handle_of(lua, &this)?; + let who = { + let model = lua.app_data_ref::().expect("model"); + model + .arena + .frame(h) + .and_then(|f| f.name.clone()) + .unwrap_or_else(|| "".to_string()) + }; + let parent_arg = it.next(); let new_parent = { let model = lua.app_data_ref::().expect("model"); - match &parent { - Value::Table(t) => decode_id(t) - .ok() - .and_then(|id| model.id_to_frame.get(&id).copied()), - Value::String(s) => { - s.to_str().ok().and_then(|n| model.arena.lookup(n.as_ref())) + match &parent_arg { + // Explicit nil reparents to the screen root (the strata/level RESET path). + Some(Value::Nil) => None, + Some(Value::Table(t)) => Some( + decode_id(t) + .ok() + .and_then(|id| model.id_to_frame.get(&id).copied()) + .ok_or_else(|| { + mlua::Error::runtime(format!( + "{who}:SetParent(): Couldn't find region named ''" + )) + })?, + ), + Some(Value::String(s)) => { + let n = s.to_str()?.to_string(); + Some(model.arena.lookup(&n).ok_or_else(|| { + mlua::Error::runtime(format!( + "{who}:SetParent(): Couldn't find region named '{n}'" + )) + })?) + } + // Absent argument, or any other type: the reference never reaches the + // reparent slot (`0x87cb48` raise). + _ => { + return Err(mlua::Error::runtime(format!( + "{who}:SetParent(): Couldn't find region named ''" + ))) } - _ => None, } }; - let changed = { + // The cycle guard raises — `"%s:SetParent(): Would create a loop parenting to %s"` + // (`0x7a177f`'s inline `+0x9c` walk, `newParent == self` included). + if let Some(np) = new_parent { + let model = lua.app_data_ref::().expect("model"); + if np == h || model.arena.is_ancestor(h, np) { + let pname = model + .arena + .frame(np) + .and_then(|f| f.name.clone()) + .unwrap_or_else(|| "".to_string()); + return Err(mlua::Error::runtime(format!( + "{who}:SetParent(): Would create a loop parenting to {pname}" + ))); + } + } + let hidden = { let mut model = lua.app_data_mut::().expect("model"); - let before = model.arena.frame(h).and_then(|f| f.parent); - let changed = model.arena.set_parent(h, new_parent); - // A real reparent can move the subtree's effective scale — a layout-gate input - // (the arena rejects same-parent/cycle calls, so `before` moving is the test). - if model.arena.frame(h).and_then(|f| f.parent) != before { - model.touch_layout(); - } - changed + model.arena.reparent_begin(h, new_parent) }; - event::fire_visibility_changes(lua, changed); + // None = the verified total no-op (same parent, `0x76ab20`): nothing runs, not even + // the layout touch. + let Some(hidden) = hidden else { return Ok(()) }; + let was_visible = !hidden.is_empty(); + event::fire_visibility_changes(lua, hidden); + let shown = { + let mut model = lua.app_data_mut::().expect("model"); + let shown = model.arena.reparent_finish(h, new_parent, was_visible); + // A real reparent moves the subtree's effective scale and anchors — a + // layout-gate input. + model.touch_layout(); + shown + }; + event::fire_visibility_changes(lua, shown); Ok(()) })?, )?; diff --git a/crates/benilla-ui/src/script/region/paint.rs b/crates/benilla-ui/src/script/region/paint.rs index ef9221d7..4ac6c5e1 100644 --- a/crates/benilla-ui/src/script/region/paint.rs +++ b/crates/benilla-ui/src/script/region/paint.rs @@ -136,29 +136,48 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { // reached us only once the survey started seating the addon registry — the whole point of // that instrument fix. The stray `true` is meaningless in 1.12 and the addon author // presumably meant a later client's second parameter; either way the client shrugs. + // The RETURN is part of the contract (wow-re `widget-api-batch-benilla.md` Q1, VERIFIED): + // the path form answers **1 | nil — nil meaning the file did not load** — and the colour + // and clear forms answer 1. Atlas is the load-bearing caller: `Atlas_Refresh` does + // `local builtIn = AtlasMap:SetTexture("…\Images\Maps\"..zoneID)` and walks its plugin + // fallback chain on nil, so a binding that returns nothing draws no map at all whatever + // the file resolver does. The load verdict comes from the host's [`Model::texture_probe`] + // (existence across patch chain + loose addon folder — the same candidate walk the + // renderer resolves with); a VM with no probe installed has no backend, so its path form + // stays nil, which is both literally true and what every engine-less test always observed. lua.create_function( |lua, (this, arg, g, b, a): (Table, Value, Value, Value, Value)| { let rh = region_handle_of(lua, &this)?; let mut model = lua.app_data_mut::().expect("model"); - let data = model.region_data.entry(rh).or_default(); // A plain SetTexture makes the region ordinary again — drop any portrait circular mask // and any live-unit-portrait binding. + let data = model.region_data.entry(rh).or_default(); data.circular = false; data.portrait_unit = None; // Both forms write the SAME `+0xcc` texture slot — the path form loads a file // there (`0x770200`), the colour form generates an 8×8 solid into it // (`0x770360`) — so each clears the other. NEITHER touches the vertex colour at // `+0xb8`: a tint outlives the art it was tinting. - match &arg { + let loaded = match &arg { // SetTexture("") clears, same as SetTexture(nil) — the ref lua blanks state - // art with the empty string (QuestLogFrame.lua:165-166). + // art with the empty string (QuestLogFrame.lua:165-166). The return for "" + // is read as the path form's failure (nothing loads from an empty name); + // INFERRED — no corpus caller reads it. Value::String(s) if s.to_str()?.is_empty() => { data.texture = None; data.fill = None; + false } Value::String(s) => { - data.texture = Some(s.to_str()?.to_string()); + let path = s.to_str()?.to_string(); + data.texture = Some(path.clone()); data.fill = None; + drop(model); + let model = lua.app_data_ref::().expect("model"); + model + .texture_probe + .as_ref() + .is_some_and(|probe| probe(&path)) } // The colour form, and the ONLY branch that looks at the trailing three. A // non-numeric there takes the same default a missing one does, which is what @@ -176,16 +195,22 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { data.fill = Some([as_f32(&arg), chan(&g, 0.0), chan(&b, 0.0), chan(&a, 1.0)]); data.texture = None; + true } // SetTexture(nil) clears (the live API's blank-the-region form); a cleared - // texture region draws nothing. + // texture region draws nothing. Returns 1 (Q1's `SetTexture(nil) / ()` row). Value::Nil => { data.texture = None; data.fill = None; + true } - _ => {} - } - Ok(()) + _ => false, + }; + Ok(if loaded { + Value::Number(1.0) + } else { + Value::Nil + }) }, )?, )?; @@ -375,3 +400,13 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { )?; Ok(()) } + +impl crate::script::UiScript { + /// Install the host's texture-path oracle — the resolver behind the path form of + /// `SetTexture`'s **1 | nil** return ([`Model::texture_probe`]). The host hands in existence + /// over its real stores (patch chain + loose addon folder); a VM that never gets one keeps + /// answering nil for every path, the engine-less truth. + pub fn set_texture_probe(&mut self, probe: crate::script::TextureProbe) { + self.model_mut().texture_probe = Some(probe); + } +} diff --git a/crates/benilla-ui/src/script/tests/events.rs b/crates/benilla-ui/src/script/tests/events.rs index c3a28608..16e9984a 100644 --- a/crates/benilla-ui/src/script/tests/events.rs +++ b/crates/benilla-ui/src/script/tests/events.rs @@ -172,3 +172,96 @@ fn has_script_reports_the_kind_is_supported_not_that_one_is_set() { "over-permissive by design today — see the comment at the binding" ); } + +/// **The walk steps by a next saved BEFORE the handler runs** (`0x703ee8`; decision 1324): a +/// handler that unregisters ITSELF mid-dispatch cannot rob its successor. This is AceEvent-2.0's +/// fire-once idiom for `PLAYER_LOGIN`/`VARIABLES_LOADED` — its frame unregisters inside the +/// handler, and the index-walk this replaces skipped whichever addon registered right after it +/// (Bagnon_Forever's DB never initialized; the director's SaveBagData error dialogs). +#[test] +fn a_self_unregistering_handler_does_not_rob_its_successor() { + let mut s = script(); + s.run( + r#" + log = {} + for _, n in ipairs({"WalkA", "WalkB", "WalkC"}) do + local f = CreateFrame("Frame", n) + f:RegisterEvent("E") + f:SetScript("OnEvent", function() + table.insert(log, n) + if n == "WalkB" then WalkB:UnregisterEvent("E") end + end) + end + "#, + ) + .unwrap(); + s.fire_event("E", vec![]); + assert!(s.errors().is_empty(), "{:?}", s.errors()); + let log: Vec = s.eval("return log").unwrap(); + assert_eq!( + log, + vec!["WalkA", "WalkB", "WalkC"], + "the once-idiom's self-removal must not skip the next listener" + ); + // The removal held: a second fire reaches only A and C. + s.fire_event("E", vec![]); + let log: Vec = s.eval("return log").unwrap(); + assert_eq!(log, vec!["WalkA", "WalkB", "WalkC", "WalkA", "WalkC"]); +} + +/// A handler that unregisters the walk's SAVED next ends the dispatch there — the reference frees +/// that node and walks into zeroed links (an accident we render as a deterministic stop) — while a +/// frame registered mid-dispatch tail-appends and is still visited. +#[test] +fn mid_dispatch_removal_of_the_next_stops_and_append_is_visited() { + let mut s = script(); + s.run( + r#" + log = {} + local function reg(n, body) + local f = CreateFrame("Frame", n) + f:RegisterEvent("E2") + f:SetScript("OnEvent", function() table.insert(log, n); if body then body() end end) + return f + end + reg("NxA", function() + NxB:UnregisterEvent("E2") -- kill the walk's saved next + end) + reg("NxB") + reg("NxC") + "#, + ) + .unwrap(); + s.fire_event("E2", vec![]); + let log: Vec = s.eval("return log").unwrap(); + assert_eq!( + log, + vec!["NxA"], + "removing the saved next ends the dispatch" + ); + + s.run( + r#" + log = {} + local function reg(n, body) + local f = CreateFrame("Frame", n) + f:RegisterEvent("E3") + f:SetScript("OnEvent", function() table.insert(log, n); if body then body() end end) + end + reg("ApA", function() + local f = CreateFrame("Frame", "ApLate") + f:RegisterEvent("E3") + f:SetScript("OnEvent", function() table.insert(log, "ApLate") end) + end) + reg("ApB") + "#, + ) + .unwrap(); + s.fire_event("E3", vec![]); + let log: Vec = s.eval("return log").unwrap(); + assert_eq!( + log, + vec!["ApA", "ApB", "ApLate"], + "a tail-append during dispatch is still visited this dispatch" + ); +} diff --git a/crates/benilla-ui/src/script/tests/mod.rs b/crates/benilla-ui/src/script/tests/mod.rs index 1e6623de..1e81f6c5 100644 --- a/crates/benilla-ui/src/script/tests/mod.rs +++ b/crates/benilla-ui/src/script/tests/mod.rs @@ -26,6 +26,7 @@ mod object_model; mod reference_surface; mod regions; mod scrollframe; +mod setparent; mod size_changed; mod slider; mod statusbar; diff --git a/crates/benilla-ui/src/script/tests/setparent.rs b/crates/benilla-ui/src/script/tests/setparent.rs new file mode 100644 index 00000000..15802774 --- /dev/null +++ b/crates/benilla-ui/src/script/tests/setparent.rs @@ -0,0 +1,205 @@ +//! The runtime `SetParent` law — strata/level re-assignment, the hide→show round-trip, and the +//! binding's error surface, per wow-re `ui/scratch/setparent-runtime-strata-level.md` (decision +//! 1323). Every test drives the production Lua binding; the arena split +//! (`reparent_begin`/`reparent_finish`) is exercised through it. + +use super::common::script; + +/// `strata := parent.strata`, `level := parent.level + 1` (`0x76ab5a`/`0x76ab65`) — and the +/// subtree is NOT re-based (`propagate = 0`): an existing child keeps its absolute level, landing +/// below its own parent, exactly as shipped. +#[test] +fn reparent_relevels_the_moved_frame_only() { + let s = script(); + s.run( + r#" + High = CreateFrame("Frame", "High") + High:SetFrameStrata("DIALOG") + High:SetFrameLevel(6) + Panel = CreateFrame("Frame", "Panel") -- MEDIUM 0 + Child = CreateFrame("Frame", "Child", Panel) -- MEDIUM 1 (creation inherit) + Panel:SetParent(High) + "#, + ) + .unwrap(); + assert_eq!( + s.eval::("return Panel:GetFrameStrata()").unwrap(), + "DIALOG", + "strata := parent's, forced onto the moved frame" + ); + assert_eq!( + s.eval::("return Panel:GetFrameLevel()").unwrap(), + 7, + "level := parent.level + 1" + ); + assert_eq!( + s.eval::("return Child:GetFrameStrata()").unwrap(), + "DIALOG", + "the strata force recurses over the subtree" + ); + assert_eq!( + s.eval::("return Child:GetFrameLevel()").unwrap(), + 1, + "the child keeps its absolute level — BELOW its parent's new 7; the client ships that" + ); +} + +/// `SetParent(nil)` is a RESET — strata MEDIUM, level 0 (`0x76aba3`/`0x76abac`) — never "keep +/// current". +#[test] +fn reparent_to_nil_resets_strata_and_level() { + let s = script(); + s.run( + r#" + High = CreateFrame("Frame", "SPHigh") + High:SetFrameStrata("TOOLTIP") + High:SetFrameLevel(9) + F = CreateFrame("Frame", "SPF", High) + F:SetParent(nil) + "#, + ) + .unwrap(); + assert_eq!( + s.eval::("return F:GetFrameStrata()").unwrap(), + "MEDIUM" + ); + assert_eq!(s.eval::("return F:GetFrameLevel()").unwrap(), 0); +} + +/// A reparent of an effectively-visible frame is a hide→show round-trip: `OnHide` fires (under the +/// OLD parent), then `OnShow` refires down the subtree — and an `OnShow` doing +/// `SetFrameLevel(GetParent():GetFrameLevel()+1)` lands the child back above its parent. That +/// hand-repair is AtlasLoot's own idiom and the reason its browse panel works on the reference — +/// the round-trip is what makes it run. +#[test] +fn a_visible_reparent_refires_onhide_then_onshow() { + let s = script(); + s.run( + r#" + log = {} + Old = CreateFrame("Frame", "RTOld") + New = CreateFrame("Frame", "RTNew") + New:SetFrameLevel(4) + Mover = CreateFrame("Frame", "RTMover", Old) + Kid = CreateFrame("Frame", "RTKid", Mover) + Mover:SetScript("OnHide", function() + table.insert(log, "hide:" .. Mover:GetParent():GetName()) + end) + Mover:SetScript("OnShow", function() + table.insert(log, "show:" .. Mover:GetParent():GetName()) + end) + Kid:SetScript("OnShow", function() + Kid:SetFrameLevel(Kid:GetParent():GetFrameLevel() + 1) + table.insert(log, "kidshow") + end) + Mover:SetParent(New) + "#, + ) + .unwrap(); + let log: Vec = s.eval("return log").unwrap(); + assert_eq!( + log, + vec!["hide:RTOld", "show:RTNew", "kidshow"], + "OnHide observes the OLD parent, OnShow the new, and the refire walks the subtree" + ); + assert_eq!( + s.eval::("return RTMover:GetFrameLevel()").unwrap(), + 5, + "the moved frame sits at parent+1" + ); + assert_eq!( + s.eval::("return RTKid:GetFrameLevel()").unwrap(), + 6, + "the child's own OnShow hand-repaired it above its parent — the propagate-0 law's \ + period-correct workaround" + ); +} + +/// A reparent of a hidden frame fires neither event, and does not recompute effective visibility: +/// the `+0xd4` cascade only runs inside the show half (`0x76abfd` gates on the captured `ebx`), so +/// a shown-but-invisible frame moved under a visible parent STAYS effectively invisible until +/// something shows it. Shipped behaviour, byte-verified — not an oversight. +#[test] +fn a_hidden_reparent_fires_nothing_and_leaves_visibility_stale() { + let s = script(); + s.run( + r#" + fired = 0 + Hidden = CreateFrame("Frame", "STHidden") + Hidden:Hide() + Vis = CreateFrame("Frame", "STVis") + F = CreateFrame("Frame", "STF", Hidden) -- shown bit true, chain hidden + F:SetScript("OnShow", function() fired = fired + 1 end) + F:SetScript("OnHide", function() fired = fired + 1 end) + F:SetParent(Vis) + "#, + ) + .unwrap(); + assert_eq!( + s.eval::("return fired").unwrap(), + 0, + "neither event fired" + ); + assert!( + s.eval::("return STF:IsShown() and not STF:IsVisible()") + .unwrap(), + "shown bit intact, effective visibility stale-false under the visible new parent" + ); + // An explicit Show is what recomputes it — but Show() no-ops while the bit is already set, so + // a real transition needs the toggle. The Hide half fires no OnHide (the frame was already + // effectively invisible — transition-gated); only the Show half's false→true fires. + s.run("STF:Hide(); STF:Show()").unwrap(); + assert!(s.eval::("return STF:IsVisible()").unwrap()); + assert_eq!( + s.eval::("return fired").unwrap(), + 1, + "the toggle's OnShow alone" + ); +} + +/// The binding's error surface: a cycle RAISES (`0x87cb14` — including `newParent == self`), an +/// unresolvable name raises `Couldn't find region named`, and an ABSENT argument is the same raise +/// — not the nil path. The same-parent call is a total no-op that fires nothing. +#[test] +fn the_binding_raises_on_cycle_bad_name_and_absent_argument() { + let s = script(); + s.run( + r#" + A = CreateFrame("Frame", "ErrA") + B = CreateFrame("Frame", "ErrB", A) + "#, + ) + .unwrap(); + let cycle = s.run("A:SetParent(B)").unwrap_err().to_string(); + assert!( + cycle.contains("Would create a loop parenting to ErrB"), + "cycle raises: {cycle}" + ); + let self_cycle = s.run("A:SetParent(A)").unwrap_err().to_string(); + assert!(self_cycle.contains("Would create a loop"), "{self_cycle}"); + let bad = s.run("A:SetParent('NoSuchFrame')").unwrap_err().to_string(); + assert!( + bad.contains("Couldn't find region named 'NoSuchFrame'"), + "{bad}" + ); + let absent = s.run("A:SetParent()").unwrap_err().to_string(); + assert!(absent.contains("Couldn't find region named"), "{absent}"); + + // Same parent: the total no-op — no events, no level rewrite. + s.run( + r#" + B:SetFrameLevel(9) + n = 0 + B:SetScript("OnHide", function() n = n + 1 end) + B:SetScript("OnShow", function() n = n + 1 end) + B:SetParent(A) + "#, + ) + .unwrap(); + assert_eq!(s.eval::("return n").unwrap(), 0); + assert_eq!( + s.eval::("return B:GetFrameLevel()").unwrap(), + 9, + "0x76ab20 skips everything — the level is not re-derived" + ); +} diff --git a/crates/benilla-ui/src/script/tests/toplevel.rs b/crates/benilla-ui/src/script/tests/toplevel.rs index 061e094b..f9013cc8 100644 --- a/crates/benilla-ui/src/script/tests/toplevel.rs +++ b/crates/benilla-ui/src/script/tests/toplevel.rs @@ -210,18 +210,21 @@ fn the_trigger_is_the_effective_visibility_transition_not_the_show_call() { .unwrap(); assert_eq!( level(&mut s, "Dialog"), - 0, - "a Show that moves no effective visibility raises nothing" + 1, + "a Show that moves no effective visibility raises nothing — the 1 is not a raise, it is \ + SetParent's own level := Holder(0)+1 (decision 1323's re-level law)" ); assert_eq!(level(&mut s, "Board"), 5, "and compacts nothing"); s.run("Holder:Show()").unwrap(); + // Occupied visible MEDIUM levels at the raise: Holder 0, Dialog 1, Board 5 → compaction + // renumbers to {0, 1, 2} (Board → 2), count 3, and the raise writes Dialog := 3. assert_eq!( level(&mut s, "Dialog"), - 2, + 3, "Dialog became effective-visible through its parent's Show and raised itself" ); - assert_eq!(level(&mut s, "Board"), 1, "the compaction ran with it"); + assert_eq!(level(&mut s, "Board"), 2, "the compaction ran with it"); } // ── The gate ──────────────────────────────────────────────────────────────────────────────────── diff --git a/crates/benilla-ui/src/script/tick.rs b/crates/benilla-ui/src/script/tick.rs index 9a7a725e..e4659352 100644 --- a/crates/benilla-ui/src/script/tick.rs +++ b/crates/benilla-ui/src/script/tick.rs @@ -13,26 +13,50 @@ impl UiScript { /// arguments. Events reach registered frames regardless of visibility, **in registration /// order** — the client's `SignalEvent 0x703e50` walks the per-event listener list head-first /// (tail-append insert `0x7052d0`, wow-re `event-dispatch-order.md`: FIFO, first-registered - /// fires first), re-reading the next node per step, so a frame registered mid-dispatch is - /// still visited this dispatch. Cross-frame order is a LAW consumers depend on: both - /// ZoneText frames write `PVPInfoTextString` on one event — the last writer decides. Handler - /// errors are collected into [`UiScript::errors`], never panicking. + /// fires first). Cross-frame order is a LAW consumers depend on: both ZoneText frames write + /// `PVPInfoTextString` on one event — the last writer decides. Handler errors are collected + /// into [`UiScript::errors`], never panicking. + /// + /// **The walk steps by a next-listener SAVED BEFORE the handler runs** (`0x703ee8: mov + /// edi,[eax+0x4]` — "save the next node before firing"; decision 1324), never by list index. + /// That is what makes a handler that UNREGISTERS ITSELF mid-dispatch unable to rob its + /// successor — and it is load-bearing in the wild: AceEvent-2.0 treats `PLAYER_LOGIN` / + /// `VARIABLES_LOADED` as fire-once events and unregisters its frame *inside* the handler, so + /// an index walk skipped whichever addon registered right after AceEvent (Bagnon_Forever's + /// saved-bag DB never initialized — the director's `SaveBagData:76` error dialogs). A frame + /// registered mid-dispatch tail-appends and is still visited; a handler that unregisters the + /// walk's SAVED next stops the dispatch there (the reference frees the node and walks into + /// its zeroed links — memory-accident territory we render as a deterministic stop, a knowing + /// divergence from an accident, not from a mechanism). pub fn fire_event(&mut self, event: &str, args: Vec) { - // Walk by index, re-reading the live list each step (the client's fresh next-node read) — - // one short borrow per step, none held across the handler call. - let mut i = 0; - loop { - let id = { - let mut model = self.model_mut(); - let Some(&h) = model.event_to_frames.get(event).and_then(|l| l.get(i)) else { - break; - }; - model.frame_id(h) + let mut at = { + let model = self.model_mut(); + model + .event_to_frames + .get(event) + .and_then(|l| l.first().copied()) + }; + while let Some(h) = at { + let mut model = self.model_mut(); + // The saved handle must still be registered — its removal (by the previous handler) + // ends the walk, per the doc above. + let Some(pos) = model + .event_to_frames + .get(event) + .and_then(|l| l.iter().position(|&x| x == h)) + else { + break; }; + let next = model + .event_to_frames + .get(event) + .and_then(|l| l.get(pos + 1).copied()); + let id = model.frame_id(h); + drop(model); if let Err(e) = event::fire_event_handler(&self.lua, id, event, &args) { self.push_error(e); } - i += 1; + at = next; } } diff --git a/crates/benilla-ui/src/widget/propagation.rs b/crates/benilla-ui/src/widget/propagation.rs index 7721c0b2..8650ddb9 100644 --- a/crates/benilla-ui/src/widget/propagation.rs +++ b/crates/benilla-ui/src/widget/propagation.rs @@ -241,38 +241,88 @@ impl WidgetArena { } } - // ── Reparenting (set_parent) ───────────────────────────────────────────────────────────────── + // ── Reparenting (reparent_begin / reparent_finish) ─────────────────────────────────────────── - /// Reparent `h` under `new_parent` (or `None` for top-level), then propagate effective - /// visibility, scale, and alpha down `h`'s subtree — `SetParent` re-inherits the parent chain's - /// `effectiveVisible` (`frame-model.md`), and effective scale/alpha likewise depend on the - /// chain. Returns the frames whose `effective_visible` changed (as [`set_shown`](Self::set_shown) - /// does), so the caller can fire `OnShow`/`OnHide`. + /// The runtime reparent, phase 1 of 2 — the guards and the **hide half** of + /// `CSimpleFrame::SetParent 0x76ab10`'s byte-verified sequence (wow-re + /// `ui/scratch/setparent-runtime-strata-level.md` §2; decision 1323, which corrects this + /// module's previous "strata/level are NOT changed by reparenting" — refuted at `0x76ab65`). /// - /// A reparent that would create a cycle (making `h` its own ancestor) is rejected as a no-op — - /// the client guards this with `is_descendant 0x767010` (`propagation.md`). Strata/level are NOT - /// changed by reparenting (a frame keeps them until explicitly set). - pub fn set_parent( + /// Returns `None` for the byte-verified TOTAL no-op — `newParent == currentParent` (`0x76ab20` + /// skips *everything*) — and for a dead handle or a cycle (the Lua binding raises on a cycle + /// *before* calling here, per the binding's own inline `+0x9c` walk at `0x7a177f`; a direct + /// caller gets the silent reject as a backstop). Otherwise `Some(hidden)`: every frame whose + /// effective visibility just dropped, in pre-order — empty when the frame was not effectively + /// visible ("a reparent of a hidden frame fires neither" event). The caller fires `OnHide` for + /// those and then calls [`Self::reparent_finish`]; the hide runs while the frame still hangs + /// under its OLD parent, as the reference's `0x76ab41` does before the `+0x9c` store, so an + /// `OnHide` handler observes the old parent. (One knowing divergence: the reference unlinks + /// from the old parent's child list *before* the hide, so an `OnHide` calling + /// `oldParent:GetChildren()` no longer sees the frame there; ours still does — the relink is + /// phase 2's, so a handler that itself reparents can't leave the lists half-spliced.) + pub fn reparent_begin( &mut self, h: FrameHandle, new_parent: Option, - ) -> Vec { - let mut changed = Vec::new(); - if self.frame(h).is_none() { - return changed; - } + ) -> Option> { + self.frame(h)?; // Keep only a live new parent; reject a cycle (new_parent == h or below h). let new_parent = new_parent.filter(|&p| self.frame(p).is_some()); if let Some(np) = new_parent { if np == h || self.is_ancestor(h, np) { - return changed; + return None; } } - - let old_parent = self.frame(h).unwrap().parent; - if old_parent == new_parent { - return changed; + if self.frame(h).unwrap().parent == new_parent { + return None; } + let mut hidden = Vec::new(); + if self.frame(h).unwrap().effective_visible { + self.propagate_visible(h, false, &mut hidden); + } + Some(hidden) + } + + /// Phase 2 — the relink and the rest of `0x76ab10`'s verified sequence: **strata := parent's** + /// (`0x76ab5a` → the value-gated subtree force of [`Self::set_frame_strata`]; MEDIUM for nil), + /// **level := parent.level + 1 with `propagate = 0`** (`0x76ab65`; 0 for nil), scale + /// re-inherit, then the **show half**. Returns the frames whose effective visibility came back + /// (fire `OnShow` for those, in order). + /// + /// Load-bearing consequences, each verified in the note: + /// + /// - **Existing children keep their absolute levels** (`propagate = 0`; `level_compact + /// 0x764eb0`'s one caller is the toplevel Raise, unreachable from here). A child created when + /// the parent sat lower can land BELOW its own parent — the client ships that and nothing + /// repairs it but the child's own `SetFrameLevel` (which is exactly what AtlasLoot's + /// button templates do in `OnShow`, and why its browse panel works on the reference). + /// - **The show half is gated on `was_visible`** (`0x76abfd`: `ebx && (parent==0 || + /// parent->+0xd4)`) — the `+0xd4` cascade only runs inside `0x76ae10`, so a reparent of a + /// hidden (or hidden-chained) frame does NOT recompute effective visibility: moved under a + /// visible parent it stays effectively invisible until something shows it. That staleness is + /// the shipped behaviour, not an oversight to "fix" here. + /// - The `OnShow` refire down the re-shown subtree is what lets an addon hand-repair the + /// propagate-0 level law (`this:SetFrameLevel(GetParent():GetFrameLevel()+1)` in `OnShow` — + /// the reference fires it via the `+0x84`/`+0x88` hide→show round-trip on any visible + /// reparent). + /// + /// `was_visible` is phase 1's `!hidden.is_empty()` — the reference's `ebx`, captured at + /// `0x76ab2b` before anything runs. Alpha stays untouched throughout: the client only pushes + /// alpha at `SetAlpha` time (module alpha section). + pub fn reparent_finish( + &mut self, + h: FrameHandle, + new_parent: Option, + was_visible: bool, + ) -> Vec { + let mut shown = Vec::new(); + if self.frame(h).is_none() { + return shown; // died inside an OnHide — nothing left to move + } + let new_parent = new_parent.filter(|&p| self.frame(p).is_some()); + // Relink from the CURRENT parent (an OnHide handler may itself have reparented; the + // reference's unconditional `+0x9c` store at `0x76ab49` means the outer call wins). + let old_parent = self.frame(h).unwrap().parent; if let Some(op) = old_parent { if let Some(of) = self.frame_mut(op) { of.children.retain(|&c| c != h); @@ -283,13 +333,41 @@ impl WidgetArena { self.frame_mut(np).unwrap().children.push(h); } - let pv = self.parent_visible(h); - self.propagate_visible(h, pv, &mut changed); + let (pstrata, plevel) = match new_parent { + Some(np) => { + let pf = self.frame(np).expect("live new parent"); + (pf.strata, pf.level.saturating_add(1)) + } + // SetParent(nil) is a RESET — strata 3 (MEDIUM), level 0 (`0x76aba3`/`0x76abac`) — + // not "keep current". + None => (Strata::default(), 0), + }; + self.set_frame_strata(h, pstrata); + self.set_frame_level(h, plevel, false); let ps = self.parent_effective_scale(h); self.propagate_scale(h, ps); - // Alpha: untouched by a reparent — the client only pushes alpha at SetAlpha time - // (module alpha section), so the moved subtree keeps its last-written values. - changed + + if was_visible && self.parent_visible(h) { + self.propagate_visible(h, true, &mut shown); + } + shown + } + + /// [`Self::reparent_begin`] + [`Self::reparent_finish`] in one call, for callers with no Lua + /// to fire between the halves (tests, direct arena use). Returns both phases' visibility + /// changes concatenated — hide half first. The Lua binding does NOT use this: it must fire + /// `OnHide` between the phases (the events' direction is read from live state at fire time). + pub fn set_parent( + &mut self, + h: FrameHandle, + new_parent: Option, + ) -> Vec { + let Some(hidden) = self.reparent_begin(h, new_parent) else { + return Vec::new(); + }; + let was_visible = !hidden.is_empty(); + let shown = self.reparent_finish(h, new_parent, was_visible); + [hidden, shown].concat() } /// Re-link a **region leaf** (Texture/FontString) to a new owner frame — the arena half of