diff --git a/crates/benilla-app/assets/ui/UIParent.xml b/crates/benilla-app/assets/ui/UIParent.xml
index 60721c1e..a85d6c33 100644
--- a/crates/benilla-app/assets/ui/UIParent.xml
+++ b/crates/benilla-app/assets/ui/UIParent.xml
@@ -16,8 +16,18 @@
transcription of the whole routing table would be a list of no-ops that reads as coverage.
Known seam, stated: the real client PARENTS every FrameXML frame under UIParent (scale/
- visibility cascade); benilla's top-level frames still attach to the screen root. Same
- rect, so anchors agree - re-parenting is a separate arc if a cascade consumer appears.
+ visibility cascade). benilla is PART WAY there - 41 of our top-level frames declare
+ parent="UIParent" where the reference declares 104, and 32 frames the reference parents
+ (MainMenuBar, PlayerFrame, TargetFrame, the four MultiBars, UIErrorsFrame, the panels)
+ still attach to the screen root because the transcription dropped the attribute. Same
+ rect, so anchors agree.
+
+ The cascade consumer this note was waiting for HAS appeared (decision 1724): a cinematic
+ hides the HUD by doing `UIParent:Hide()` in the reference, and benilla substitutes an
+ engine-side draw-kill that also switches the mouse feed off - which is how the world came
+ to take every click of a race intro. Restoring the dropped parents is the fix; it is a
+ whole-UI arc (draw order, strata, ALT-Z, the world map's own full-screen row) and so is
+ proposed rather than taken.
-->
diff --git a/crates/benilla-app/assets/ui/UiPanels.xml b/crates/benilla-app/assets/ui/UiPanels.xml
index 8ee23c17..7bb79358 100644
--- a/crates/benilla-app/assets/ui/UiPanels.xml
+++ b/crates/benilla-app/assets/ui/UiPanels.xml
@@ -293,9 +293,17 @@
-- SetFullScreenFrame(frame) — UIParent.lua l.852-866. ONE line dropped: the reference also
-- does `UIParent:Hide()`/`:Show()` around the swap, hiding the whole game UI behind the
- -- fullscreen frame. benilla has no UIParent frame (the note on `slots` above), and the only
- -- full frame we ship — the world map — already paints its own full-screen blackout
- -- (WorldMapFrame.xml), which is what that Hide buys the reference.
+ -- fullscreen frame.
+ --
+ -- BOTH of the reasons this note used to give are now FALSE (decision 1724). A real
+ -- `UIParent` frame exists (UIParent.xml), and the world map is no longer the only `full`
+ -- row: `CinematicFrame` is a second one, and it paints nothing by design. So this dropped
+ -- line is exactly what makes a cinematic hide the HUD through `crate::ui_hide::UiHidden`
+ -- instead of through here — and `UiHidden` also means "the UI takes no mouse", which is
+ -- ALT-Z's meaning and the reason the world took every click of a fly-by.
+ --
+ -- Restoring it needs the HUD parented to UIParent first (the 32 dropped `parent="UIParent"`
+ -- declarations 1724 measured); hiding UIParent today would reach almost nothing.
function SetFullScreenFrame(frame)
local oldFrame = slots.fullscreen
slots.fullscreen = frame
diff --git a/crates/benilla-app/assets/ui/benilla.toc b/crates/benilla-app/assets/ui/benilla.toc
index 5f4f032b..093279c0 100644
--- a/crates/benilla-app/assets/ui/benilla.toc
+++ b/crates/benilla-app/assets/ui/benilla.toc
@@ -455,7 +455,9 @@ ScriptLogFrame.xml
# requirement rather than legibility is **after BuffFrame.xml**: the toast re-anchors
# TemporaryEnchantFrame out of its own way, and that frame is declared there.
HelpFrame.xml
-# The cinematic letterbox (the race-intro fly-by's two black bars, and the ESC that skips it).
+# The cinematic frame: the ESC that skips a fly-by, and the SCREENSHOT chord it hands back. It
+# draws NOTHING — the letterbox moved to Bevy UI nodes in `crate::cinematic` (decision 1699),
+# because the lane this frame paints into is dark for the whole shot.
# Needs UiPanels.xml far above for ShowUIPanel/HideUIPanel and its own UIPanelWindows row; needs
# nothing else, and nothing needs it — the engine reaches it purely by firing CINEMATIC_START.
# Seated late because it is an outermost, screen-owning layer, like the two windows below it.
diff --git a/crates/benilla-app/examples/addon_harness.rs b/crates/benilla-app/examples/addon_harness.rs
index 45599933..c41a009b 100644
--- a/crates/benilla-app/examples/addon_harness.rs
+++ b/crates/benilla-app/examples/addon_harness.rs
@@ -582,6 +582,71 @@ fn main() {
println!(" {label:>5} {n:>4} {}", "#".repeat(n.min(60)));
}
+ // **WHOSE package is incomplete** — printed immediately before the ranked blockers, because it
+ // is the line that stops a session hunting for a client bug that is not one. A `.toc` entry
+ // whose file the addon does not ship is the ADDON's defect, and the reference client's
+ // behaviour there is what ours already does: log `Couldn't open %s` and carry on
+ // (wow-re `ui/scratch/xml-toc-path-resolution.md` §4). Nothing is subtracted from the headline
+ // — 1213 — so both readings stay available.
+ let own: Vec<&addon_harness::AddonReport> = reports
+ .iter()
+ .filter(|r| !r.absent_own_files.is_empty())
+ .collect();
+ let foreign: Vec<&addon_harness::AddonReport> = reports
+ .iter()
+ .filter(|r| !r.absent_foreign_files.is_empty())
+ .collect();
+ // **"the WHOLE reason" is a claim, and it is checked before it is printed.** An addon can be
+ // short a file AND raise somewhere else; saying "this is why it fails" on the strength of
+ // `!loaded` alone would be the same overclaim this column exists to stop. It holds only when
+ // the absent entries account for every load error the addon has.
+ let sole_cause = |r: &addon_harness::AddonReport| {
+ !r.loaded && r.errors.len() == r.absent_own_files.len() + r.absent_foreign_files.len()
+ };
+ if !own.is_empty() || !foreign.is_empty() {
+ println!(
+ "\n MANIFEST ENTRIES WITH NO FILE — counted in the numbers above, listed here so"
+ );
+ println!(" the reader can tell a broken package from a broken client:");
+ for r in &own {
+ println!(
+ " {} — its own .toc lists {} file(s) the package does not contain{}",
+ r.name,
+ r.absent_own_files.len(),
+ if sole_cause(r) {
+ " [and that is its ONLY load error]"
+ } else {
+ ""
+ }
+ );
+ for f in r.absent_own_files.iter().take(4) {
+ println!(" missing: {f}");
+ }
+ }
+ for r in &foreign {
+ println!(
+ " {} — wants a folder that is not installed{}",
+ r.name,
+ if sole_cause(r) {
+ " [and that is its ONLY load error]"
+ } else {
+ ""
+ }
+ );
+ for f in r.absent_foreign_files.iter().take(4) {
+ println!(" missing: {f}");
+ }
+ }
+ let theirs = own.iter().filter(|r| sole_cause(r)).count();
+ let ours = foreign.iter().filter(|r| sole_cause(r)).count();
+ println!(
+ " ({} with an incomplete package of their own, {theirs} of which fail for that \
+ alone; {} wanting a neighbour, {ours} for that alone)",
+ own.len(),
+ foreign.len()
+ );
+ }
+
// What actually STOPPED them — the ranked first error. Read this before the demand list: a
// wall 60 addons hit is worth more than a verb 60 addons would like (decision 1193).
println!("\n what stopped them (addons whose FIRST load error was each):");
diff --git a/crates/benilla-app/src/addon_harness/mod.rs b/crates/benilla-app/src/addon_harness/mod.rs
index 96703d1a..f7815542 100644
--- a/crates/benilla-app/src/addon_harness/mod.rs
+++ b/crates/benilla-app/src/addon_harness/mod.rs
@@ -109,6 +109,30 @@ pub struct AddonReport {
pub loaded: bool,
/// Load errors, verbatim, tagged by file.
pub errors: Vec,
+ /// **Manifest entries the addon's own package does not contain.** Already counted in
+ /// [`Self::errors`], and deliberately not removed from it — 1213's rule for the fifth time:
+ /// a new question gets a new column, because silently shrinking `errors` would move `loaded`
+ /// and make every number in every past decision record incomparable.
+ ///
+ /// The question it asks that no other column can: **whose fault is this row?** Five of the
+ /// corpus's failures are one addon family shipping a `.toc` that lists files it does not
+ /// contain — `DPSMate_CureDisease.toc` names eight files and the folder holds six, because the
+ /// three `*Received*` ones were split into a sibling addon and the manifest was never updated.
+ /// The client is behaving correctly there (the reference logs `Couldn't open %s` and carries
+ /// on, which is what our loader does), and a headline that cannot say so invites a session to
+ /// go hunting for a bug that is not ours. The director's own AtlasLoot copy is the same shape.
+ pub absent_own_files: Vec,
+ /// **Manifest entries resolving OUTSIDE the addon's folder, into one that is not installed** —
+ /// resolved paths, not the raw entry, because the `..` collapse is the interesting half.
+ ///
+ /// A different question from [`Self::absent_own_files`] and kept apart from it: this addon's
+ /// package is fine, it wants a neighbour. The corpus's two are Auctioneer and BeanCounter
+ /// reaching `..\Blizzard_AuctionUI\Blizzard_AuctionUITemplates.xml` — which wow-re records as
+ /// **RESOLVING** in the real client (`ui/scratch/xml-toc-path-resolution.md` §5 case 2, by
+ /// name), because there the file layer can see `Blizzard_AuctionUI` inside `patch.MPQ`. Ours
+ /// reads the AddOns directory only, so it misses. That one IS ours, and the split is what
+ /// makes it visible instead of averaging with the row above.
+ pub absent_foreign_files: Vec,
/// Names it calls that the VM does not have — see the module doc on what this is worth.
pub missing_globals: Vec,
/// Dependencies named in its `.toc` that are not in the folder.
@@ -520,7 +544,7 @@ fn survey_one(
// consumer — the same rule `load_dependencies` applies to errors.
let baseline = RenderBaseline::of(&script);
- let errors = load_addon_files(&script, root, name, &toc);
+ let (errors, absent) = load_addon_files(&script, root, name, &toc);
let wants = missing_calls(root, name, &toc, &known, &dep_methods);
// AFTER the addon's files: a template it declares in its OWN XML is registered by then, so it
// is not missing. The check asks the VM's live registry, not a name list.
@@ -568,6 +592,8 @@ fn survey_one(
interface: toc.interface_versions(),
loaded: errors.is_empty(),
errors,
+ absent_own_files: absent.own,
+ absent_foreign_files: absent.foreign,
missing_globals: wants.missing_globals,
missing_deps,
missing_templates,
@@ -1635,15 +1661,47 @@ fn globals_of(script: &UiScript) -> BTreeSet {
.collect()
}
+/// What a manifest entry that does not resolve actually is — see [`AddonReport::absent_own_files`].
+#[derive(Debug, Default)]
+pub struct AbsentFiles {
+ /// Entries under the addon's **own** folder that the package does not contain.
+ pub own: Vec,
+ /// Entries resolving **outside** it, into a folder that is not installed.
+ pub foreign: Vec,
+}
+
/// Run the addon's manifest through the same two arms the real loader uses — `.lua` as a chunk,
/// anything else as FrameXML — with the AddOns root as the provider's path space (decision 1186).
-fn load_addon_files(script: &UiScript, root: &Path, name: &str, toc: &Toc) -> Vec {
+///
+/// Returns the errors **and**, beside them, the split of which entries did not resolve. The
+/// absent ones are still in `errors` — 1213's rule, for the fifth time: this asks a new question
+/// and gets a new column, it does not quietly shrink an old one.
+fn load_addon_files(
+ script: &UiScript,
+ root: &Path,
+ name: &str,
+ toc: &Toc,
+) -> (Vec, AbsentFiles) {
let provider = |req: &str| -> Option> { read_under(root, req) };
let mut errors = Vec::new();
+ let mut absent = AbsentFiles::default();
for file in &toc.files {
let path = benilla_ui::loader::join_ref(name, file);
let Some(bytes) = read_under(root, &path) else {
errors.push(format!("{file}: not found"));
+ // WHOSE package is incomplete. `join_ref` has already collapsed the `..`s the way the
+ // client does (wow-re `ui/scratch/xml-toc-path-resolution.md` §2), so the resolved
+ // path is what the real file layer would be handed — and an entry that still points
+ // inside the addon's own folder is the addon shipping a manifest it does not satisfy,
+ // while one pointing out of it wants a neighbour that is not installed.
+ let own = path
+ .strip_prefix(name)
+ .is_some_and(|rest| rest.starts_with('/'));
+ if own {
+ absent.own.push(file.clone());
+ } else {
+ absent.foreign.push(path.clone());
+ }
continue;
};
if is_lua(file) {
@@ -1665,7 +1723,7 @@ fn load_addon_files(script: &UiScript, root: &Path, name: &str, toc: &Toc) -> Ve
Err(e) => errors.push(format!("{file}: {e}")),
}
}
- errors
+ (errors, absent)
}
fn is_lua(entry: &str) -> bool {
@@ -3423,6 +3481,101 @@ mod dependency_tests {
let _ = std::fs::remove_dir_all(&tmp);
}
+ /// **A manifest entry with no file is split by WHOSE package is short** — the column that
+ /// stops a session hunting for a client bug that is not one.
+ ///
+ /// Both rows below are load failures and stay load failures: nothing is subtracted from
+ /// `loaded` or from `errors` (1213's rule, for the fifth time). What the split adds is the
+ /// attribution, and the two cases genuinely differ:
+ ///
+ /// - **`own`** — the addon ships a `.toc` listing a file it does not contain. Five corpus rows
+ /// are this, all one family: `DPSMate_CureDisease.toc` names eight files and the folder
+ /// holds six, because the three `*Received*` ones were split into a sibling addon and the
+ /// manifest was never updated. Our loader is behaving correctly (the reference logs
+ /// `Couldn't open %s` and carries on — wow-re `ui/scratch/xml-toc-path-resolution.md` §4).
+ /// - **`foreign`** — the entry escapes the addon's folder with `..`, which the client supports
+ /// and `join_ref` reproduces, into a folder that is not installed. The corpus's two are
+ /// Auctioneer and BeanCounter reaching `..\Blizzard_AuctionUI\...`, which wow-re records as
+ /// **RESOLVING** in the real client (§5 case 2, by name) because its file layer can see that
+ /// folder inside `patch.MPQ`. That one IS ours.
+ ///
+ /// The assertion that matters is that the two never merge: a single "files not found" count
+ /// would average a broken package with a real client gap and read as one number.
+ #[test]
+ fn an_absent_manifest_entry_is_attributed_to_whose_package_is_short() {
+ let tmp =
+ std::env::temp_dir().join(format!("benilla-harness-absent-{}", std::process::id()));
+ let _ = std::fs::remove_dir_all(&tmp);
+ let write = |name: &str, toc: &str, files: &[(&str, &str)]| {
+ let dir = tmp.join(name);
+ std::fs::create_dir_all(&dir).unwrap();
+ std::fs::write(dir.join(format!("{name}.toc")), toc).unwrap();
+ for (f, body) in files {
+ std::fs::write(dir.join(f), body).unwrap();
+ }
+ };
+ // Ships one of the two files its own manifest lists — DPSMate_CureDisease's exact shape.
+ write(
+ "ShortPackage",
+ "## Interface: 11200\nhere.lua\ngone.lua\n",
+ &[("here.lua", "ShortPackageRan = 1\n")],
+ );
+ // Reaches a neighbour that is not installed — Auctioneer's exact shape, `..` and all.
+ write(
+ "WantsNeighbour",
+ "## Interface: 11200\nown.lua\n..\\NotInstalled\\templates.xml\n",
+ &[("own.lua", "WantsNeighbourRan = 1\n")],
+ );
+
+ let reports = survey(&tmp);
+ let of = |n: &str| reports.iter().find(|r| r.name == n).unwrap();
+
+ let short = of("ShortPackage");
+ assert_eq!(
+ short.absent_own_files,
+ vec!["gone.lua".to_string()],
+ "the entry inside its own folder is the addon's own package being short"
+ );
+ assert!(
+ short.absent_foreign_files.is_empty(),
+ "and it is NOT a missing neighbour: {:?}",
+ short.absent_foreign_files
+ );
+
+ let wants = of("WantsNeighbour");
+ assert_eq!(
+ wants.absent_foreign_files,
+ vec!["NotInstalled/templates.xml".to_string()],
+ "`..` is collapsed the way the client collapses it, and the RESOLVED path is what is \
+ reported — the collapse is the interesting half"
+ );
+ assert!(
+ wants.absent_own_files.is_empty(),
+ "its own package is complete: {:?}",
+ wants.absent_own_files
+ );
+
+ // NOTHING is subtracted. Both are still load failures, still in `errors`, still counted
+ // in every headline — the split is a new column beside them, never a quieter old one.
+ for r in [short, wants] {
+ assert!(!r.loaded, "{}: still a load failure", r.name);
+ assert!(
+ r.errors.iter().any(|e| e.contains("not found")),
+ "{}: the error is still there verbatim: {:?}",
+ r.name,
+ r.errors
+ );
+ }
+ // ...and the file that DID exist ran, because a missing entry does not abort the manifest.
+ assert!(
+ !short.errors.iter().any(|e| e.contains("here.lua")),
+ "the surviving file loads: {:?}",
+ short.errors
+ );
+
+ let _ = std::fs::remove_dir_all(&tmp);
+ }
+
#[test]
fn a_sibling_addons_embedded_library_is_invisible() {
let tmp =
diff --git a/crates/benilla-app/src/addon_harness/probe.rs b/crates/benilla-app/src/addon_harness/probe.rs
index e6b87faa..cbec47e0 100644
--- a/crates/benilla-app/src/addon_harness/probe.rs
+++ b/crates/benilla-app/src/addon_harness/probe.rs
@@ -105,7 +105,7 @@ pub fn probe(root: &Path, name: &str, evals: &[String]) -> Option
&mut dep_order,
);
- let load_errors = super::load_addon_files(&script, root, name, &toc);
+ let (load_errors, _absent) = super::load_addon_files(&script, root, name, &toc);
let session_errors = super::drive_session_start(&mut script, name, &dep_order);
let answers = evals
diff --git a/crates/benilla-app/src/bindings.rs b/crates/benilla-app/src/bindings.rs
index 2a126920..fdb290ad 100644
--- a/crates/benilla-app/src/bindings.rs
+++ b/crates/benilla-app/src/bindings.rs
@@ -207,7 +207,7 @@ impl Plugin for BindingsPlugin {
.before(benilla_world::schedule::WorldStage::Input)
.run_if(in_state(ClientState::InWorld)),
load_character_bindings.run_if(in_state(ClientState::InWorld)),
- save_bindings.after(load_character_bindings),
+ drain_binding_requests.after(load_character_bindings),
),
);
}
@@ -294,12 +294,36 @@ fn load_character_bindings(
}
}
-/// Persist on the window's SaveBindings (Okay): write the set's diff; saving account while a
-/// character file exists deletes it — the confirmed permanent delete.
-fn save_bindings(script: Option>, files: Res) {
+/// Drain the VM's queued binding requests.
+///
+/// Two of them. `Save` persists on the window's SaveBindings (Okay): write the set's diff; saving
+/// account while a character file exists deletes it — the confirmed permanent delete. `Run` fires
+/// a named command's action outright, which is `RunBinding(name)` — the reference's own
+/// passthrough verb, whose one shipped caller is `CinematicFrame`'s `OnKeyDown` handing the
+/// SCREENSHOT chord back to its binding while it swallows every other key (decision 1724).
+fn drain_binding_requests(script: Option>, files: Res) {
let Some(mut script) = script else { return };
for req in script.take_keybind_requests() {
- let KeybindRequest::Save(which) = req;
+ // `RunBinding(name)` — fire the named command's action as if its chord had been pressed.
+ // Only the two Lua-bodied kinds can be run this way: a `Held`/`Host` command's action is a
+ // bit in the movement word, which a Lua call has no frame to assert for. The one shipped
+ // caller is `CinematicFrame`'s SCREENSHOT passthrough, and `SCREENSHOT` is `Kind::Edge`.
+ let which = match req {
+ KeybindRequest::Save(which) => which,
+ KeybindRequest::Run(name) => {
+ match SPECS.iter().find(|s| s.name == name).map(|s| &s.kind) {
+ Some(Kind::Edge(lua)) | Some(Kind::EdgeUpDown(lua, _)) => {
+ let lua = *lua;
+ if let Err(e) = script.run(lua) {
+ warn!("bindings(RunBinding {name}): {e}");
+ }
+ }
+ Some(_) => warn!("RunBinding({name}): a held/engine action has no body to run"),
+ None => warn!("RunBinding({name}): no such command"),
+ }
+ continue;
+ }
+ };
let snapshot = script.keybind_snapshot();
let text = store::to_diff(&snapshot);
let path = match which {
diff --git a/crates/benilla-app/src/cinematic.rs b/crates/benilla-app/src/cinematic.rs
index f0c0567b..8488d9a6 100644
--- a/crates/benilla-app/src/cinematic.rs
+++ b/crates/benilla-app/src/cinematic.rs
@@ -24,10 +24,15 @@
//! clamp, i.e. plays **once and freezes**; it does not loop. (`Scry_cam`, which is not a race
//! intro, is the one file authored to loop — a difference we inherit for free by ending on the
//! band rather than on the flag.)
-//! - **Between the shots of a multi-camera row the client sends `CMSG_NEXT_CINEMATIC_CAMERA`**
-//! (`0x48efe0`), and a camera id of `0` **ends** the cinematic rather than being skipped. No
-//! shipped row has a second camera, so this path is exercised only by a server with its own
-//! DBCs — which is exactly why it is written rather than assumed away.
+//! - **The client announces every shot with `CMSG_NEXT_CINEMATIC_CAMERA` — including the first.**
+//! The send sits inside the *shot arm* `0x48edf0`, not the shot advance: `0x48ef11 push 0xfb`
+//! → `0x418190` (the packet builder) → `0x48ef24 call 0x5ab630` (flush), immediately before
+//! `0x48ef29` starts that shot's narration and `0x48ef43` fades it in — the same builder/flush
+//! pair `0x48f154 push 0xfc` uses for the completion ack. Since every shot goes through the arm,
+//! a shipped single-camera race intro sends exactly one of these, at the start. benilla used to
+//! read this as "between the shots of a multi-camera row" and cite `0x48efe0`, which sent none
+//! at all on every shot the game actually ships. A camera id of `0` still **ends** the cinematic
+//! rather than being skipped (`0x48efe0`).
//! - **ESC is not an engine binding.** `StopCinematic` has zero native callers in the reference:
//! the only skip path is `CinematicFrame.xml`'s own `OnKeyDown`, which is why benilla's copy of
//! that frame (`assets/ui/CinematicFrame.xml`) carries the same handler and why the Lua binding
@@ -40,11 +45,17 @@
//!
//! # What we deliberately do differently
//!
-//! The reference defers both the start and the stop by **0.25 s** through a scheduled fade
-//! (`0x4c0d10`, the constant at `[0x804550]`). benilla plays the shot immediately and acks
-//! immediately: the fade is a transition effect we have not built, and faking its *delay* without
-//! its *picture* would only add latency. Noted rather than silently dropped — it is the one timing
-//! difference from the reference on this path.
+//! **Every cinematic boundary in the reference cuts through black, and benilla cuts hard.** The
+//! `0.25 s` at `[0x804550]` is not a scheduled delay — it is a screen fade, and the next step is
+//! its *completion callback*: `0x4c0d10` builds a fullscreen opaque-black quad (`0xff000000`) and
+//! latches what to run when it is fully up, so `CINEMATIC_START`, each shot advance and
+//! `EndCinematic` all execute at full black and `0x4c1280` fades back in. Six sites, both edges,
+//! always black, always 0.25 s (wow-re `ui/scratch/cinematic-camera-law.md` §3.7, a CORRECTION to
+//! the earlier "deferred by a delay" reading). There is **no audio fade anywhere on this path**.
+//!
+//! benilla plays and acks immediately. Building the picture is what would let the timing be
+//! faithful too, so the two go together and neither is faked without the other — decision 1724
+//! leaves both standing rather than adding latency with nothing on screen to justify it.
//!
//! # What playback takes over
//!
@@ -86,6 +97,12 @@ pub(crate) struct Cinematic {
/// One cinematic in flight.
struct Playing {
+ /// Which *run* this is, counted from process start. The `CinematicSequences.dbc` id alone is
+ /// not an identity: re-triggering the same sequence while it plays produces the same
+ /// `(sequence, shot)` pair, and the narration follower reads that pair to tell "still the same
+ /// shot" from "a new one" — so without this the picture restarted at t=0 while the voice kept
+ /// running from wherever it had got to.
+ run: u64,
/// The `CinematicSequences.dbc` id, for logging.
sequence_id: u32,
/// The row's shots, in order. Non-empty (a sequence that resolves to nothing is acked at the
@@ -109,12 +126,13 @@ impl Cinematic {
self.playing.is_some()
}
- /// The shot on screen: `(sequence id, shot index, narration sound id)`. The identity pair is
- /// what lets a follower tell "still the same shot" from "the next one", which is the question
- /// the narration channel actually asks.
- pub(crate) fn playing_shot(&self) -> Option<(u32, usize, u32)> {
+ /// The shot on screen: `(run, shot index, narration sound id)`. The identity pair is what lets
+ /// a follower tell "still the same shot" from "the next one", which is the question the
+ /// narration channel actually asks — and it is keyed on [`Playing::run`] rather than the
+ /// sequence id so that re-triggering the sequence already playing counts as a new shot.
+ pub(crate) fn playing_shot(&self) -> Option<(u64, usize, u32)> {
let play = self.playing.as_ref()?;
- Some((play.sequence_id, play.index, play.shot().sound_id))
+ Some((play.run, play.index, play.shot().sound_id))
}
}
@@ -130,6 +148,7 @@ impl Cinematic {
Self {
pending: None,
playing: Some(Playing {
+ run: 0,
sequence_id: 0,
shots: Vec::new(),
index: 0,
@@ -154,6 +173,14 @@ impl Cinematic {
#[derive(Component)]
struct LetterboxBar;
+/// What [`drive_letterbox`] switched off when the shot started, so it puts back only what it took.
+/// A player who had already pressed ALT-Z, or who was in a mouse-look session, keeps their state.
+#[derive(Default)]
+struct Takeover {
+ ui: bool,
+ cursor: bool,
+}
+
pub(crate) struct CinematicPlugin;
impl Plugin for CinematicPlugin {
@@ -255,7 +282,8 @@ fn drive_letterbox(
mut hidden: ResMut,
mut bars: Query<(&mut Node, &mut Visibility), With>,
windows: Query<&Window>,
- mut ours: Local,
+ mut cursor: Query<&mut bevy::window::CursorOptions, With>,
+ mut ours: Local,
mut logged: Local,
) {
let playing = cine.is_playing();
@@ -263,14 +291,37 @@ fn drive_letterbox(
// their choice when it ends.
if playing && !hidden.0 {
hidden.0 = true;
- *ours = true;
+ ours.ui = true;
info!("cinematic: HUD hidden for playback");
- } else if !playing && *ours {
+ } else if !playing && ours.ui {
hidden.0 = false;
- *ours = false;
+ ours.ui = false;
info!("cinematic: HUD restored");
}
+ // **The hardware cursor goes with it** — `0x58b590(0)` at StartCinematic, `(1)` at
+ // EndCinematic and at the leave-world teardown (wow-re `ui/scratch/cinematic-camera-law.md`
+ // §3.4, the complete 10-site census of the cinematic state cell). Nothing else in benilla
+ // would: this flag is otherwise written only by the mouse-look session in `player::camera`,
+ // and `control` skips that whole branch while the view is detached, so the pointer the player
+ // arrived at character-select with simply sat on top of the fly-by.
+ //
+ // Same restore-only-what-we-hid discipline as the HUD above, and the same **write only on a
+ // real change**: `CursorOptions` is what `bevy_winit`'s `changed_cursor_options` watches, and
+ // re-applying cursor state to AppKit every frame intermittently stalls the main thread
+ // (`player::control`'s shadow-copy note, the 0366 frame-tail hunt).
+ if let Ok(mut opts) = cursor.single_mut() {
+ if playing && opts.visible {
+ opts.visible = false;
+ ours.cursor = true;
+ } else if !playing && ours.cursor {
+ ours.cursor = false;
+ if !opts.visible {
+ opts.visible = true;
+ }
+ }
+ }
+
let height = playing
.then(|| windows.iter().next())
.flatten()
@@ -340,7 +391,31 @@ fn take_trigger(
ack(net.as_deref());
continue;
}
- cine.pending = Some(id);
+ // Last write wins, the reference's single-slot latch — but the id it displaces still owes
+ // the server its ack (see [`relinquish`]).
+ if let Some(dropped) = cine.pending.replace(id) {
+ warn!("cinematic: {dropped} displaced by {id} before it started — acking it");
+ ack(net.as_deref());
+ }
+ }
+}
+
+/// Give up whatever is in flight, paying its ack.
+///
+/// **The invariant this exists to keep: one `CMSG_COMPLETE_CINEMATIC` for every trigger we
+/// accepted.** Decision 0196 is why the direction matters — an unacked cinematic leaves vmangos
+/// anchoring object visibility to its own copy of the flying camera, and everything around the
+/// body stays despawned until relog. An *extra* ack is discarded by a server that is not watching
+/// one; a missing one is not recoverable in-session. So when two triggers are live at once, both
+/// are acked: they are two cinematics the server started, not one.
+fn relinquish(cine: &mut Cinematic, net: Option<&NetCommands>, why: &str) {
+ if let Some(play) = cine.playing.take() {
+ info!("cinematic: {} {why}", play.sequence_id);
+ ack(net);
+ }
+ if let Some(id) = cine.pending.take() {
+ info!("cinematic: {id} {why} before it started");
+ ack(net);
}
}
@@ -385,12 +460,34 @@ fn start_pending(
shots.len(),
shots.iter().map(|s| s.duration_ms).sum::()
);
+ // A trigger that lands on top of a running cinematic replaces it — and the one it replaces
+ // is acked here rather than dropped ([`relinquish`]'s invariant). No stock server does this;
+ // a GM `.debug play cinematic` during an intro does.
+ if let Some(old) = cine.playing.take() {
+ warn!(
+ "cinematic: {} replaced by {id} while playing — acking it",
+ old.sequence_id
+ );
+ ack(net.as_deref());
+ }
+ static RUN: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(1);
cine.playing = Some(Playing {
+ run: RUN.fetch_add(1, std::sync::atomic::Ordering::Relaxed),
sequence_id: id,
shots,
index: 0,
elapsed: Duration::ZERO,
});
+ announce_shot(net.as_deref());
+}
+
+/// `CMSG_NEXT_CINEMATIC_CAMERA` — sent as each shot is armed, the first one included (module doc:
+/// `0x48ef11`, inside the shot arm). It is how the server learns which camera is flying, which is
+/// the other half of decision 0196's visibility re-anchor.
+fn announce_shot(net: Option<&NetCommands>) {
+ if let Some(net) = net {
+ let _ = net.0.send(ClientCommand::NextCinematicCamera);
+ }
}
/// Advance the shot and seat the camera on it; hand over to the next shot, or end the cinematic.
@@ -433,9 +530,7 @@ fn drive(
}
play.index += 1;
play.elapsed = over;
- if let Some(net) = net.as_deref() {
- let _ = net.0.send(ClientCommand::NextCinematicCamera);
- }
+ announce_shot(net.as_deref());
}
let Ok(mut cam) = camera.single_mut() else {
@@ -453,6 +548,13 @@ fn drive(
let forward = (target - eye).normalize_or_zero();
let up = if forward == Vec3::ZERO {
Vec3::Y
+ } else if forward.cross(Vec3::Y).length_squared() < 1e-6 {
+ // Looking straight up or down: rotating `Y` about a `forward` that *is* `Y` is the
+ // identity, so the authored roll would vanish and `look_at` would fall back to an
+ // arbitrary (if deterministic) horizon. Roll the one basis vector that is guaranteed not
+ // to be parallel to the view instead — same angle, about the same axis, just a seed that
+ // survives. No shipped shot holds a vertical view, which is why this never showed.
+ Quat::from_axis_angle(forward, view.roll) * Vec3::Z
} else {
Quat::from_axis_angle(forward, view.roll) * Vec3::Y
};
@@ -468,14 +570,8 @@ fn drive(
/// skip is indistinguishable from a completion on the wire, which is what made decision 0196's
/// instant-ack legitimate in the first place.
pub(crate) fn stop(cine: &mut Cinematic, net: Option<&NetCommands>) {
- if let Some(play) = cine.playing.take() {
- info!("cinematic: {} stopped", play.sequence_id);
- ack(net);
- }
- // A trigger still waiting on the world is dropped too: the player has said "not this".
- if cine.pending.take().is_some() {
- ack(net);
- }
+ // A trigger still waiting on the world goes with it: the player has said "not this".
+ relinquish(cine, net, "stopped");
}
/// Leaving the world drops a cinematic without acking — the socket is going away with it.
@@ -509,6 +605,15 @@ fn feed_ui(
if *published == Some(playing) {
return;
}
+ // **A fresh VM is already not in a cinematic, so there is no edge to announce.** The memo
+ // starts `None`, and `None != Some(false)` — which fired a `CINEMATIC_STOP` into every login
+ // and every `/reload`, an event the reference never sends unedged. Seed the state instead of
+ // announcing it; the `/reload`-mid-cinematic case (`playing == true`) still falls through and
+ // re-fires `CINEMATIC_START` into the rebuilt tree, which is what the memo is here for.
+ if published.is_none() && !playing {
+ *published = Some(false);
+ return;
+ }
*published = Some(playing);
script.set_in_cinematic(playing);
script.fire_event(
@@ -584,3 +689,90 @@ mod tests {
}
}
}
+
+/// **The ack ledger.** Decision 0196's invariant is the one thing on this path that cannot be
+/// checked by looking at the screen: an unacked cinematic leaves vmangos anchoring object
+/// visibility to its own copy of the flying camera, and everything around the body stays
+/// despawned until relog. It had no test at all, and that is precisely why a second trigger could
+/// silently displace a playing cinematic and pay nothing — the bug these cases now pin.
+#[cfg(test)]
+mod ack_ledger {
+ use super::*;
+ use crate::net::ClientCommand;
+
+ fn wire() -> (NetCommands, crossbeam_channel::Receiver) {
+ let (tx, rx) = crossbeam_channel::unbounded();
+ (NetCommands(tx), rx)
+ }
+
+ fn acks(rx: &crossbeam_channel::Receiver) -> usize {
+ rx.try_iter()
+ .filter(|c| matches!(c, ClientCommand::CompleteCinematic))
+ .count()
+ }
+
+ /// One trigger in flight, one ack out — the ordinary ESC skip.
+ #[test]
+ fn stopping_a_playing_cinematic_acks_it_once() {
+ let (net, rx) = wire();
+ let mut cine = Cinematic::playing_for_test();
+ stop(&mut cine, Some(&net));
+ assert_eq!(acks(&rx), 1);
+ assert!(!cine.is_playing());
+ }
+
+ /// Two triggers accepted, two acks owed. Over-acking is discarded by a server that is not
+ /// watching a cinematic; under-acking is the failure that costs a relog, so this direction is
+ /// deliberate and not a duplicate to squash.
+ #[test]
+ fn a_playing_cinematic_and_a_latched_one_each_pay() {
+ let (net, rx) = wire();
+ let mut cine = Cinematic::playing_for_test();
+ cine.pending = Some(41);
+ stop(&mut cine, Some(&net));
+ assert_eq!(acks(&rx), 2);
+ assert!(cine.pending.is_none());
+ }
+
+ /// Nothing in flight owes nothing — ESC with no cinematic must not ack into the void.
+ #[test]
+ fn stopping_nothing_acks_nothing() {
+ let (net, rx) = wire();
+ let mut cine = Cinematic::default();
+ stop(&mut cine, Some(&net));
+ assert_eq!(acks(&rx), 0);
+ }
+
+ /// **The regression.** A trigger that displaces a latched one pays for the one it displaced —
+ /// the latch is last-write-wins (the reference's single slot), but the server still started
+ /// the cinematic it dropped.
+ #[test]
+ fn a_displaced_latch_is_acked() {
+ let (net, rx) = wire();
+ let mut cine = Cinematic {
+ pending: Some(41),
+ ..Default::default()
+ };
+ // What `take_trigger` does on the second message.
+ if let Some(_dropped) = cine.pending.replace(81) {
+ ack(Some(&net));
+ }
+ assert_eq!(acks(&rx), 1);
+ assert_eq!(cine.pending, Some(81));
+ }
+
+ /// Leaving the world is the one path that deliberately pays **nothing**: the socket is going
+ /// away with the cinematic, exactly as the reference's own teardown does (`0x490a80` clears
+ /// the flag and sends no `CMSG_COMPLETE_CINEMATIC`).
+ #[test]
+ fn abandoning_the_world_acks_nothing() {
+ let (net, rx) = wire();
+ let mut app = App::new();
+ app.insert_resource(Cinematic::playing_for_test());
+ app.insert_resource(net);
+ app.add_systems(Update, abandon_on_leaving_world);
+ app.update();
+ assert_eq!(acks(&rx), 0);
+ assert!(!app.world().resource::().is_playing());
+ }
+}
diff --git a/crates/benilla-app/src/cursor.rs b/crates/benilla-app/src/cursor.rs
index 191db3bc..5bef0af7 100644
--- a/crates/benilla-app/src/cursor.rs
+++ b/crates/benilla-app/src/cursor.rs
@@ -504,6 +504,7 @@ mod macos {
world_assets: Option>,
mode: Res,
rig: Res,
+ cinematic: Option>,
mut focus: MessageReader,
// None of the `Local`s below is a [`crate::ui_script::VmMemo`], deliberately (decision
// 1290's sweep): every one of them remembers something about the **OS** — what we last told
@@ -551,7 +552,15 @@ mod macos {
}
}
}
- let looking = rig.is_looking();
+ // **A cinematic hides the pointer too**, and on this platform it can only be done here:
+ // the reference's `0x58b590(0)` at StartCinematic / `(1)` at End (wow-re
+ // `ui/scratch/cinematic-camera-law.md` §3.4). `crate::cinematic` writes
+ // `CursorOptions.visible`, which is the right lever everywhere else and is *inert on
+ // macOS* for the reason this module exists — winit's cursor-rect route does not survive a
+ // continuously-redrawing Metal view. So the fly-by's pointer is hidden by joining the
+ // look session's own `NSCursor::hide()`/`unhide()` pair, which keeps AppKit's app-global
+ // hide counter balanced across both reasons instead of racing a second one against it.
+ let looking = rig.is_looking() || cinematic.is_some_and(|c| c.is_playing());
let stem = mode.0.stem();
// The held payload's cursor wins over the classified mode (falls back to the mode cursor
// when nothing is held, or its icon hasn't resolved/decoded this frame). The KEY names what
diff --git a/crates/benilla-app/src/net.rs b/crates/benilla-app/src/net.rs
index 47aba125..3ca0fb22 100644
--- a/crates/benilla-app/src/net.rs
+++ b/crates/benilla-app/src/net.rs
@@ -1694,9 +1694,11 @@ pub(crate) enum ClientCommand {
/// the flying cinematic camera (a first login's race intro) and every NPC around the body
/// despawns until relog (decision 0196).
CompleteCinematic,
- /// Advance a multi-camera cinematic to its next shot (`CMSG_NEXT_CINEMATIC_CAMERA`) — see
- /// [`benilla_protocol::WorldWriter::next_cinematic_camera`]. No shipped 1.12 sequence has a
- /// second camera, so this rides only a server with its own DBCs.
+ /// Announce the shot now being armed (`CMSG_NEXT_CINEMATIC_CAMERA`) — see
+ /// [`benilla_protocol::WorldWriter::next_cinematic_camera`]. Sent for **every** shot, the
+ /// first included: the reference's send is inside the shot arm (`0x48ef11`), not the shot
+ /// advance, so a shipped single-camera race intro sends exactly one. See
+ /// [`crate::cinematic`]'s module doc.
NextCinematicCamera,
/// **Acknowledge a granted mover mode** — root, water-walk, feather-fall or hover (decisions
/// 0308, 0866): the echoed `counter` + our live pose, on the opcode
diff --git a/crates/benilla-app/src/sound/cinematic.rs b/crates/benilla-app/src/sound/cinematic.rs
index f8e9a4ac..d54c3603 100644
--- a/crates/benilla-app/src/sound/cinematic.rs
+++ b/crates/benilla-app/src/sound/cinematic.rs
@@ -28,7 +28,7 @@ use benilla_assets::{LockRecover, WorldAssets};
use benilla_world::schedule::WorldStage;
use super::mixer::{self, StreamingSoundHandle};
-use super::{kit::SoundKits, SoundConfig, SoundOutput};
+use super::{kit::SoundKits, SoundOutput};
use crate::cinematic::Cinematic;
/// A cut-off narration is **cut**, with no fade at all — wow-re
@@ -43,9 +43,12 @@ const CUT_FADE_MS: u64 = 0;
/// The narration channel: which shot it belongs to, and its live handle.
#[derive(Default)]
pub(super) struct CinematicVoice {
- /// `(sequence id, shot index)` of the shot whose narration is playing — the identity that
- /// decides whether the current shot's audio is already running.
- shot: Option<(u32, usize)>,
+ /// `(run, shot index)` of the shot whose narration is playing — the identity that decides
+ /// whether the current shot's audio is already running. Keyed on the **run** rather than the
+ /// sequence id: re-triggering the sequence already on screen restarts the picture at `t = 0`,
+ /// and a sequence-keyed identity would have matched and left the voice running from wherever
+ /// it had reached.
+ shot: Option<(u64, usize)>,
handle: Option>,
}
@@ -70,20 +73,19 @@ fn drive_narration(
mut out: NonSendMut,
mut kits: ResMut,
assets: Option>,
- config: Res,
) {
let playing = cine.as_deref().and_then(Cinematic::playing_shot);
- let Some((sequence, index, sound_id)) = playing else {
+ let Some((run, index, sound_id)) = playing else {
// No cinematic (or it just ended/was skipped) — cut whatever was narrating.
voice.stop();
return;
};
- if voice.shot == Some((sequence, index)) {
+ if voice.shot == Some((run, index)) {
return;
}
// A new shot: the previous one's narration, if any, does not carry over.
voice.stop();
- voice.shot = Some((sequence, index));
+ voice.shot = Some((run, index));
if sound_id == 0 {
return;
}
@@ -122,7 +124,14 @@ fn drive_narration(
// The old reading — the Sfx category, "the same one every NPC voice line takes" — was a
// reasonable guess at where a player's expectation points, and it was wrong: this channel has
// no category.
- let amp = if config.enabled { kit_vol } else { 0.0 };
+ // **The kit volume only** — the `MasterSoundEffects` gate is carried by the master track, not
+ // baked in here. `apply_master_volume` drives master to 0 whenever `enabled` is off (or the
+ // dev mute is on) and back up when it returns, so folding `config.enabled` into a one-shot
+ // starting amp was redundant in the silencing direction and *wrong* in the other: a player who
+ // started a cinematic with sound off and turned it back on mid-narration got everything else
+ // back and a narration that stayed silent for the rest of the shot. The reference re-reads its
+ // gate (`0x7a529c`) every tick; the master track is where benilla re-reads it.
+ let amp = kit_vol;
match mixer_ref.play_stream(data.volume(mixer::amp_to_db(amp))) {
Ok(h) => {
info!("cinematic voice: {path}");
diff --git a/crates/benilla-app/src/sound/mod.rs b/crates/benilla-app/src/sound/mod.rs
index 6fd9cfe5..ca4fa5f9 100644
--- a/crates/benilla-app/src/sound/mod.rs
+++ b/crates/benilla-app/src/sound/mod.rs
@@ -211,9 +211,19 @@ fn feed_world_hold(
}
}
-/// Copy the cinematic's music stop into [`SoundConfig::music_suppressed`], once per frame in
-/// `PreUpdate` beside [`feed_world_hold`] — same shape, same reason: one answer per frame, read by
-/// every trigger system after it.
+/// Copy the cinematic's music stop into [`SoundConfig::music_suppressed`], once per frame.
+///
+/// **Ordered `WorldStage::Stream`, not `PreUpdate` beside [`feed_world_hold`]** — the two look
+/// alike and are not. The cover `feed_world_hold` reads is a state that lasts seconds, so reading
+/// last frame's answer costs nothing; the cinematic's music cut is a one-frame *edge*, and
+/// `crate::cinematic`'s driver asserts it in `WorldStage::Input` — after `PreUpdate` has already
+/// run. Read there, [`zone::zone_audio`] (`WorldStage::Present`) saw a stale `false` on exactly
+/// the frame a cinematic started: on a first-login race intro that is the first uncovered frame,
+/// where the zone's own area-change block starts a track — or, worse, the zone's *intro fanfare*,
+/// which is then stamped as played and does not come back for `MinDelayMinutes`. One frame later
+/// the suppression cut it again, so the symptom was a click and a consumed fanfare rather than
+/// anything you could hear as music. `Stream` sits between the two (`Net → Input → Stream →
+/// Present`), so the flag the pump reads is always this frame's.
fn feed_music_suppression(
mut config: ResMut,
cinematic: Option>,
@@ -418,7 +428,11 @@ impl Plugin for SoundPlugin {
)
// The cover's audio hold (see [`SoundConfig::world_hold`]): fed in PreUpdate so every
// trigger system this frame — whatever stage it runs in — reads one answer.
- .add_systems(PreUpdate, (feed_world_hold, feed_music_suppression))
+ .add_systems(PreUpdate, feed_world_hold)
+ .add_systems(
+ Update,
+ feed_music_suppression.in_set(benilla_world::schedule::WorldStage::Stream),
+ )
.add_systems(
Update,
(
diff --git a/crates/benilla-app/src/sound/zone.rs b/crates/benilla-app/src/sound/zone.rs
index 9fd86268..41ef0917 100644
--- a/crates/benilla-app/src/sound/zone.rs
+++ b/crates/benilla-app/src/sound/zone.rs
@@ -800,6 +800,11 @@ fn stop_world_soundscape(zone: &mut ZoneAudio, reason: &str) {
zone.area = None;
zone.interior = None;
zone.was_underwater = false;
+ // The cinematic latch is scheduler state like the rest of this list, and leaving it set was
+ // the one field that survived a world transition. It only ever self-corrected by luck of
+ // ordering: leave the world mid-cinematic and the next entry saw a falling edge that belonged
+ // to the *previous* session, logging a resume for a world that had had no cinematic in it.
+ zone.music_suppressed = false;
if had {
info!("sound: world soundscape stopped ({reason})");
}
diff --git a/crates/benilla-app/src/target/click.rs b/crates/benilla-app/src/target/click.rs
index 35bf738c..bb229218 100644
--- a/crates/benilla-app/src/target/click.rs
+++ b/crates/benilla-app/src/target/click.rs
@@ -117,6 +117,14 @@ pub(super) fn select_on_click(
// `deselectOnClick` (0961) gates the whole arm: "Sticky Targeting" checked = the CVar
// at 0 = an empty-world click keeps the target (1.12's own inverted checkbox).
_ => {
+ // A **corpse** is an object hit, not empty world (decision 1723): the reference's
+ // deselect lives in the terrain and nothing legs, and an object leg clears no
+ // selection — so a left-click on a body must leave the target exactly where it was.
+ // Without this the corpse would arrive here (its guid is in the other slot) and read
+ // as "clicked nothing", dropping the player's target every time they clicked a body.
+ if hovered.corpse.is_some() {
+ return;
+ }
if click_cfg.deselect_on_click && (!payload_held.0 || occlusion.distance.is_finite()) {
clear(&mut selection, &mut seam, engaged);
}
@@ -353,6 +361,69 @@ pub(super) fn act_on_right_click(
}
return;
}
+ // ── The corpse leg: `CGCorpse_C`'s own interact slot, `0x5d6bf0` (vtable `+0x60`) ───────────
+ // A corpse is not a unit and never routes through the unit block below — the reference gives
+ // its class a *different* function in the same slot the unit class fills with `0x60bea0`.
+ // Transcribed from the disassembly at `0x5d6bf0..0x5d6cdd`, which has exactly two legs:
+ //
+ // 1. `[player descriptor + UNIT_FIELD_MOUNTDISPLAYID] <= 0` (**not mounted**, `0x5d6c2a jg`)
+ // AND `0x5d6e20(corpse)` (**lootable** — `CORPSE_FIELD_DYNAMIC_FLAGS` bit 0) → a player-
+ // state check on the vtable `+0xa4` slot, which on failure raises message `0x85` and
+ // sends nothing; else `SetAutoLoot` (`0x5df460`) and then `0x5df130` — **`CMSG_LOOT`**.
+ // 2. else `CORPSE_FIELD_FLAGS` bit 5 (the PvP insignia) AND the `[0xb700e8]`
+ // SKIN_PLAYER_CORPSE learn latch AND `!0x6067d0(player, corpse)` → the skin cast
+ // (`0x5f05e0`). Unreachable without that latch, and nothing in 1.12.1 sets it.
+ //
+ // **Neither leg touches your own body**, which carries neither flag — the function runs and
+ // does nothing at all. Recovering your corpse is a separate route (decision 0308's
+ // `RECOVER_CORPSE` → `CMSG_RECLAIM_CORPSE`) whose click path is out for RE and is NOT
+ // guessed at here; the director reports a right-click on their own body raising the resurrect
+ // prompt, so a route exists and this function is not it.
+ if let (Some(entity), Some(guid)) = (hovered.corpse, hovered.corpse_guid) {
+ let store = stores.get(entity).ok().map(|(s, _)| s);
+ // The same one-line answer the GameObject leg gives (tag `use`): a corpse's click is three
+ // silent refusals deep — no flag, the range gray, the missing latch — and every one of them
+ // otherwise looks identical from the outside. One run says which.
+ if benilla_assets::trace::enabled_for("use") {
+ benilla_assets::trace::line(
+ "use",
+ &format!(
+ "right-click corpse guid={guid:#x} bones={} lootable={} insignia={} cursor={:?} unable={}",
+ store.is_some_and(|s| s.0.corpse_is_bones()),
+ store.is_some_and(|s| s.0.corpse_lootable()),
+ store.is_some_and(|s| s.0.corpse_pvp_insignia()),
+ cursor.kind,
+ cursor.unable
+ ),
+ );
+ }
+ if store.is_some_and(|s| s.0.corpse_lootable()) {
+ // Leg 1. Range rides the cursor's own gray, exactly as the unit loot branch does —
+ // the classifier and the click ask literally the same question, so the pouch can never
+ // be lit on something a click refuses.
+ if !cursor.unable {
+ debug!("right-click corpse loot: {guid:#x}");
+ let _ = seam.net.0.send(ClientCommand::Loot { guid });
+ // Same client-side prediction as the unit corpse (decision 0515): the reference's
+ // `CMSG_LOOT` sender arms `[player+0x1d28]` and kneels before any reply.
+ loot_latch.0 = Some(guid);
+ }
+ } else if store.is_some_and(|s| s.0.corpse_pvp_insignia()) {
+ // Leg 2, with its real precondition. `skin_player_corpse` is the mirror of `[0xb700e8]`
+ // and is `None` for every 1.12.1 player, so this is inert for the reference's own
+ // reason rather than by omission.
+ if let Some(spell_id) = learned.skin_player_corpse {
+ if !cursor.unable {
+ debug!("right-click corpse insignia: {guid:#x} (spell {spell_id})");
+ let _ = seam.net.0.send(ClientCommand::CastSpell {
+ spell_id,
+ target: Some(guid),
+ });
+ }
+ }
+ }
+ return;
+ }
let (Some(entity), Some(guid)) = (hovered.target, hovered.guid) else {
return;
};
diff --git a/crates/benilla-app/src/target/cursor_mode.rs b/crates/benilla-app/src/target/cursor_mode.rs
index ae80864d..2efd5365 100644
--- a/crates/benilla-app/src/target/cursor_mode.rs
+++ b/crates/benilla-app/src/target/cursor_mode.rs
@@ -186,6 +186,17 @@ const ATTACK_RANGE_SQ: f32 = 109.2025;
const MELEE_OFFSET: f32 = 1.333_33;
const MELEE_FLOOR: f32 = 5.0;
+/// A **corpse object**'s interact reach, squared — `25.0`, i.e. a flat **5.0 yd**.
+///
+/// The same `max(rA + rB + 1.333, 5.0)` shape as [`MELEE_FLOOR`] above, evaluated for a corpse:
+/// a corpse descriptor carries no `UNIT_FIELD_COMBATREACH` at all, so the sum can never clear the
+/// floor and the gate is the floor. Two independent byte facts agree on the number — the cursor
+/// leg's `CanLootNow 0x5ec110`, and the `CMSG_LOOT` sender `0x5df130`, whose owned bit-exact
+/// difftest (`loot_range_5df130_difftest`) compares `dist²` against a literal `25.0`. Written as
+/// the constant rather than the formula because the formula's other term does not exist here.
+/// Boundary-inclusive, center-to-center, like every other reach gate in this file.
+const CORPSE_INTERACT_RANGE_SQ: f32 = 25.0;
+
/// `GAMEOBJECT_TYPE_GENERIC` (vmangos `SharedDefines.h`) — world decoration whose highlightable
/// predicate is constant-false, so it never shows an interact cursor (wow-re cursor-system §4a).
pub(crate) const GO_TYPE_GENERIC: i32 = 5;
@@ -698,6 +709,58 @@ fn loot_cursor(auto_loot: bool, shift_held: bool) -> CursorKind {
}
}
+/// What a hovered **corpse object** classifies to — the reference's corpse classifier `0x482740`
+/// (wow-re `cursor-system.md` §3, VERIFIED), as a pure decision so it can be tested the way the
+/// service ladder is. `esi` = player, `edi` = corpse, `d2` = centre-to-centre distance².
+///
+/// Two legs, and there is **no third** — in particular no own-corpse leg. Your own body carries
+/// neither flag, fails both tests, and falls out to the reference's "else clear": the plain
+/// pointer. (Recovering it is decision 0308's `RECOVER_CORPSE` route, not a world cursor.)
+/// What the corpse classifier reads — one struct rather than seven positional bools, so the call
+/// sites (and the test) name each byte fact instead of counting commas.
+#[derive(Debug, Clone, Copy, Default)]
+struct CorpseFacts {
+ /// `0x5d6e20` — `CORPSE_FIELD_DYNAMIC_FLAGS` bit 0.
+ lootable: bool,
+ /// `CORPSE_FIELD_FLAGS` bit 5 — `CORPSE_FLAG_LOOTABLE`, the PvP insignia.
+ insignia: bool,
+ /// `[0xb700e8]` — the `SPELL_EFFECT_SKIN_PLAYER_CORPSE` learn-time latch.
+ skin_latch: bool,
+ /// `d2`, centre to centre.
+ dist_sq: f32,
+ /// The already-looting override: `[player+0x1d28/2c] == corpse GUID`.
+ looting_this: bool,
+ /// The effective auto-loot's two terms (0961): the knob…
+ auto_loot: bool,
+ /// …and the live key-0.
+ shift_held: bool,
+}
+
+fn corpse_cursor(f: CorpseFacts) -> Option<(CursorKind, bool)> {
+ // Leg 1 — lootable by me. Able-vs-Unable is `CanLootNow 0x5ec110(player, corpse)` — the same
+ // range+state gate as the unit loot leg — OR the already-looting override, which keeps the lit
+ // pouch on the object whose loot window is open even as you drift out of reach. The mode is
+ // Pickup(8)/LootAll(16) by the held key-0 (`0x41f8f0`, `0x4827f8`); both fail →
+ // UnablePickup(28)/UnableLootAll(36) at `0x482818`. Distance grays exactly as on units.
+ if f.lootable {
+ let able = f.dist_sq <= CORPSE_INTERACT_RANGE_SQ || f.looting_this;
+ return Some((loot_cursor(f.auto_loot, f.shift_held), !able));
+ }
+ // Leg 2 — the PvP-insignia / skin-player-corpse leg (`0x482875..0x4828a2`): the flag **and**
+ // the learn latch **and** `!0x6067d0(player, corpse)`, then the reach gate picks
+ // SkinAlliance(20)/SkinHorde(19) or their Unable twins.
+ //
+ // Modelled with its real precondition rather than omitted, so it stays inert for the
+ // reference's own reason: `[0xb700e8]` is latched only by learning a spell whose `Effect[0]`
+ // is `SPELL_EFFECT_SKIN_PLAYER_CORPSE`, and no 1.12.1 player holds one. Benilla ships one Skin
+ // cursor rather than the faction pair (`[0x80439c + 4×0x5efe00(player)]`) — a distinction with
+ // no consequence while the leg is unreachable.
+ if f.insignia && f.skin_latch {
+ return Some((CursorKind::Skin, f.dist_sq > CORPSE_INTERACT_RANGE_SQ));
+ }
+ None // "Else clear" — a plain body under the cursor is Point.
+}
+
/// Resolve this frame's [`WorldCursor`] from the hovered unit — the reference's classifier order:
/// interactable-NPC service ladder, else loot/skin/attack by state, each grayed by its own range
/// gate. No hover (or anything unresolvable) → Point.
@@ -729,6 +792,10 @@ pub(super) fn classify_cursor(
// The loot leg's Pickup/LootAll split (0965): the auto-loot knob (0961) + the live shift.
loot_cfg: Res,
keys: Res>,
+ // `[player+0x1d28/2c]` — the open loot session's object. The corpse leg's already-looting
+ // override reads it (the reference keeps the lit pouch on the corpse you are looting even
+ // once you have drifted past the range gate).
+ loot_latch: Res,
) {
// A **highlightable** GameObject shows its **data-driven** cursor (wow-re cursor-system §4): a
// mailbox's Mail, a plaque's Inspect, a vein's Mine / herb's GatherHerbs / picked lock's PickLock
@@ -859,8 +926,27 @@ pub(super) fn classify_cursor(
}
None // nothing matched → the reference's `0x4826cb` cursor-clear: Point
};
+ // The **corpse classifier** `0x482740`, whose whole decision is [`corpse_cursor`] — the store
+ // reads and the distance are all that happens here.
+ let resolve_corpse = || {
+ let (corpse_tf, store, _) = units.get(hovered.corpse?).ok()?;
+ let store = store?;
+ let (self_tf, _) = self_q.single().ok()?;
+ let shift = keys.pressed(KeyCode::ShiftLeft) || keys.pressed(KeyCode::ShiftRight);
+ corpse_cursor(CorpseFacts {
+ lootable: store.0.corpse_lootable(),
+ insignia: store.0.corpse_pvp_insignia(),
+ skin_latch: learned.skin_player_corpse.is_some(),
+ dist_sq: corpse_tf.translation.distance_squared(self_tf.translation),
+ looting_this: loot_latch.0.is_some() && loot_latch.0 == hovered.corpse_guid,
+ auto_loot: loot_cfg.auto_loot,
+ shift_held: shift,
+ })
+ };
let resolved = if go_is_nearest(&hovered, &hovered_object) {
resolve_go()
+ } else if hovered.corpse.is_some() {
+ resolve_corpse()
} else {
resolve_unit()
};
@@ -876,6 +962,84 @@ mod tests {
use super::*;
use benilla_protocol::messages::dialog_status;
+ /// The corpse classifier `0x482740` has two legs and no more — and neither of them is your
+ /// own body, which is what makes a corpse read as the plain pointer until something *else*
+ /// offers to recover it.
+ #[test]
+ fn the_corpse_classifier_has_two_legs_and_no_own_corpse_leg() {
+ let near = 25.0; // exactly the gate — boundary-inclusive, like every reach in this file
+ let far = 25.01;
+ let bones = |dist_sq| CorpseFacts {
+ lootable: true,
+ dist_sq,
+ ..CorpseFacts::default()
+ };
+ // Leg 1: lootable bones. Pickup vs LootAll by the effective auto-loot, grayed by range.
+ assert_eq!(
+ corpse_cursor(bones(near)),
+ Some((CursorKind::Pickup, false))
+ );
+ assert_eq!(
+ corpse_cursor(CorpseFacts {
+ auto_loot: true,
+ ..bones(near)
+ }),
+ Some((CursorKind::LootAll, false))
+ );
+ assert_eq!(corpse_cursor(bones(far)), Some((CursorKind::Pickup, true)));
+ // …and the already-looting override keeps it lit past the gate.
+ assert_eq!(
+ corpse_cursor(CorpseFacts {
+ looting_this: true,
+ ..bones(far)
+ }),
+ Some((CursorKind::Pickup, false))
+ );
+ // Leg 2 needs BOTH the insignia flag and the learn-time latch. The flag alone is nothing —
+ // which is why a battleground body shows no knife to a player who cannot skin one, and why
+ // this leg is inert in 1.12.1: nothing latches `[0xb700e8]`.
+ let insignia = |skin_latch, dist_sq| CorpseFacts {
+ insignia: true,
+ skin_latch,
+ dist_sq,
+ ..CorpseFacts::default()
+ };
+ assert_eq!(corpse_cursor(insignia(false, near)), None);
+ assert_eq!(
+ corpse_cursor(insignia(true, near)),
+ Some((CursorKind::Skin, false))
+ );
+ assert_eq!(
+ corpse_cursor(insignia(true, far)),
+ Some((CursorKind::Skin, true))
+ );
+ // Leg 1 wins outright when both flags are up — the binary tests lootable FIRST and returns.
+ assert_eq!(
+ corpse_cursor(CorpseFacts {
+ insignia: true,
+ skin_latch: true,
+ ..bones(near)
+ }),
+ Some((CursorKind::Pickup, false))
+ );
+ // **Your own corpse**: neither bit is set. Both legs miss, the classifier clears, and the
+ // cursor is the plain pointer at any range and with anything held.
+ let mine = |dist_sq| CorpseFacts {
+ skin_latch: true,
+ dist_sq,
+ ..CorpseFacts::default()
+ };
+ assert_eq!(corpse_cursor(mine(near)), None);
+ assert_eq!(
+ corpse_cursor(CorpseFacts {
+ auto_loot: true,
+ shift_held: true,
+ ..mine(far)
+ }),
+ None
+ );
+ }
+
/// The loot pouch follows the EFFECTIVE auto-loot (0961: setting XOR shift) — the cursor
/// and the click can never disagree about what a pick will do.
#[test]
diff --git a/crates/benilla-app/src/target/highlight.rs b/crates/benilla-app/src/target/highlight.rs
index bb57312a..e53adf93 100644
--- a/crates/benilla-app/src/target/highlight.rs
+++ b/crates/benilla-app/src/target/highlight.rs
@@ -38,9 +38,12 @@ pub(super) fn apply_highlight(
mut tags: Query<&mut MeshTag>,
mut was_lit: Local>,
) {
+ // [`Hovered::any`], not `target`: a hovered **corpse** is a pick too (decision 1723), so it
+ // holds the brighten off a farther GameObject exactly as a unit does. Whether the corpse body
+ // itself brightens is a separate byte question and is not assumed here.
let go = hovered_go
.target
- .filter(|_| hovered.target.is_none() || go_is_nearest(&hovered, &hovered_go));
+ .filter(|_| hovered.any().is_none() || go_is_nearest(&hovered, &hovered_go));
let unit_hover = hovered.target.filter(|_| go.is_none());
let mut want: Vec = Vec::new();
for root in [unit_hover, go, selection.target].into_iter().flatten() {
diff --git a/crates/benilla-app/src/target/hover.rs b/crates/benilla-app/src/target/hover.rs
index 9ce36424..b6a1a2b4 100644
--- a/crates/benilla-app/src/target/hover.rs
+++ b/crates/benilla-app/src/target/hover.rs
@@ -180,6 +180,8 @@ pub(super) fn update_hover(
) {
hovered.target = None;
hovered.guid = None;
+ hovered.corpse = None;
+ hovered.corpse_guid = None;
hovered.distance = f32::MAX;
if rig.is_looking() || pointer_over_ui.0 {
*last_pick = None;
@@ -217,7 +219,14 @@ pub(super) fn update_hover(
// even when the broad phase rejects them — the reference wouldn't click them there either.
let mut faithful: HashSet = HashSet::new();
for (entity, gt, net, anims, drv, store, children, mount_child, drawn, held) in &roots {
- if !matches!(net.kind, EntityKind::Unit | EntityKind::Player) {
+ // Units, players — and **corpses** (decision 1723). A corpse is a skinned character body
+ // like any other, so it belongs in this pass, not in a fourth picker duplicating it; the
+ // reference picks every CGObject in one trace and switches on type at the end. Which slot
+ // of [`Hovered`] the winner lands in is decided at the publish below, by kind.
+ if !matches!(
+ net.kind,
+ EntityKind::Unit | EntityKind::Player | EntityKind::Corpse
+ ) {
continue;
}
// Not drawn ⇒ not clickable (see the query's note). Deliberately BEFORE `faithful.insert`
@@ -280,7 +289,15 @@ pub(super) fn update_hover(
palettes.world_palette(rig.slot, rig.bones() as usize)
})
};
- let priority = if store.is_some_and(|s| s.0.unit_is_dead()) {
+ // The halo ladder (alive 3 / dead 2), with a **corpse below every unit** (1). Deliberately
+ // conservative and deliberately not read off `unit_is_dead()`: a corpse descriptor has no
+ // UNIT fields at all, so that accessor answers "alive" for one and would rank a body above
+ // a dead mob. Ranking it last means a corpse can never steal the generous-retry pick from
+ // a unit — the direction that cannot break an existing click. (Where `CGCorpse_C` actually
+ // sits in the reference's ladder is an open RE question, dispatched with 1723.)
+ let priority = if net.kind == EntityKind::Corpse {
+ 1
+ } else if store.is_some_and(|s| s.0.unit_is_dead()) {
2
} else {
3
@@ -335,16 +352,19 @@ pub(super) fn update_hover(
if faithful.contains(&parent) {
continue; // posed-mesh-tested above
}
- // Not a targetable unit (the self player; or — since 1706 — a **corpse**, which is a
- // streamed body with a guid, a character model and pickable parts, and yet is not a unit:
- // the reference's own interact slot for `CGCorpse_C` is `0x5d6bf0` (release/loot), never
- // `SetTarget`. The faithful path states this rule inline at its head; without it here the
- // corpse would be pass-2-eligible through the back door and a left click on one would send
- // a selection for a guid no unit owns.
+ // The same eligibility the faithful path states at its head — and **a bone pile lives
+ // here** (decision 1723): the skeletal corpse model ships without a skeleton, so it never
+ // reaches the posed-mesh pass and this box test is the only thing that can ever pick it.
+ // The self player is still excluded (its query filter), and so is everything that is not a
+ // body: a hit resolves to a `Guid` the click will act on, and the kind decides which slot
+ // of [`Hovered`] receives it.
let Ok((_, parent_net)) = units.get(parent) else {
continue;
};
- if !matches!(parent_net.kind, EntityKind::Unit | EntityKind::Player) {
+ if !matches!(
+ parent_net.kind,
+ EntityKind::Unit | EntityKind::Player | EntityKind::Corpse
+ ) {
continue;
}
if let Some(t) = ray_mesh_bounds(origin, dir, aabb, gt) {
@@ -384,9 +404,21 @@ pub(super) fn update_hover(
if best.is_some_and(|(t, _)| limit < t) {
best = None;
}
+ // **Switch on type at the end**, the reference's own shape: the one pick lands in the unit slot
+ // or the corpse slot, never both. Everything downstream that means "unit" reads `target` and so
+ // stays right by construction; the corpse legs read `corpse`.
if let Some((t, entity)) = best {
- hovered.target = Some(entity);
- hovered.guid = units.get(entity).ok().map(|(g, _)| g.0);
+ if let Ok((guid, net)) = units.get(entity) {
+ if net.kind == EntityKind::Corpse {
+ hovered.corpse = Some(entity);
+ hovered.corpse_guid = Some(guid.0);
+ } else {
+ hovered.target = Some(entity);
+ hovered.guid = Some(guid.0);
+ }
+ } else {
+ hovered.target = Some(entity);
+ }
hovered.distance = t;
}
*last_pick = best.map(|(_, e)| e);
diff --git a/crates/benilla-app/src/target/mod.rs b/crates/benilla-app/src/target/mod.rs
index 8cfe3051..4f2468e3 100644
--- a/crates/benilla-app/src/target/mod.rs
+++ b/crates/benilla-app/src/target/mod.rs
@@ -96,17 +96,45 @@ pub(crate) struct Selection {
pub(crate) guid: Option,
}
-/// The unit under the cursor this frame (the net entity + its guid), or `None`. Recomputed every frame
-/// by [`hover::update_hover`] (plate rects first, then the posed-mesh pick); a click selects it, and
-/// the mouseover highlight — the model emissive + the plate glow — reads it.
+/// The **character-model pick** this frame — the one ray pass over every skinned body in the scene,
+/// published by kind. Recomputed every frame by [`hover::update_hover`] (plate rects first, then the
+/// posed-mesh pick).
+///
+/// **Two slots, and at most one of them is ever set**, because there is one pick: [`Self::target`]
+/// when the winner is a unit or another player, [`Self::corpse`] when it is a corpse object. That
+/// split is the reference's own shape — it makes a single pick over all CGObjects and switches on
+/// **type** at the end (`0x480df0` → `0x7089c0`), and `CGCorpse_C` is a type that pick can land on
+/// while being neither selectable nor a GameObject. Keeping the corpse out of `target` is what
+/// keeps every existing consumer — selection, the plates, the unit tooltip, the mouseover
+/// highlight, the `mouseover` unit token — correct without knowing corpses exist: a corpse is not
+/// a unit and must never answer as one.
#[derive(Resource, Default, Clone, Copy)]
pub(crate) struct Hovered {
+ /// The hovered **unit or player** — the selectable one. `None` when the pick found a corpse,
+ /// or nothing.
pub(crate) target: Option,
pub(crate) guid: Option,
/// World-space ray distance to the picked mesh (a plate-rect hover is `0.0` — UI wins any tie).
- /// Meaningless when `target` is `None`. Lets the cursor/interact pick the **nearer** of a hovered
- /// unit vs. a hovered GameObject ([`go_is_nearest`]), the reference's single nearest-CGObject pick.
+ /// Meaningless when neither slot is set. Covers **whichever** slot won, so the cursor/interact
+ /// arbiter picks the nearer of the character pick vs. a hovered GameObject ([`go_is_nearest`]),
+ /// the reference's single nearest-CGObject pick.
pub(crate) distance: f32,
+ /// The hovered **corpse object** (decision 1723) — a streamed body with a guid and a character
+ /// model that is nonetheless not a unit: the reference's `CGCorpse_C` fills the interact slot
+ /// `+0x60` with its own `0x5d6bf0`, never `SetTarget`. Read by the cursor classifier's corpse
+ /// leg and the right-click corpse route; deliberately invisible to everything that means
+ /// "unit".
+ pub(crate) corpse: Option,
+ /// The hovered corpse's guid — `CMSG_LOOT` / `CMSG_RECLAIM_CORPSE` carry it.
+ pub(crate) corpse_guid: Option,
+}
+
+impl Hovered {
+ /// The picked entity whichever slot holds it — the "did the ray hit a character model at all?"
+ /// question the GameObject arbiter asks.
+ pub(crate) fn any(&self) -> Option {
+ self.target.or(self.corpse)
+ }
}
/// The **GameObject** under the cursor this frame — the interaction arc's separate hover authority
@@ -211,14 +239,20 @@ pub(crate) fn latch_press_pick(
};
}
-/// Which of the two hovered CGObjects — the unit [`Hovered`] or the [`HoveredObject`] — is the
-/// nearer thing under the cursor, and so the one a click acts on. The reference makes one pick over
-/// all CGObjects and switches on type; benilla runs the two picks separately, then chooses the
-/// nearer here. A GameObject wins only when it is strictly closer than a hovered unit (so a mailbox
-/// in front of a distant NPC takes the click; an NPC in front of a chest keeps it).
-pub(crate) fn go_is_nearest(unit: &Hovered, go: &HoveredObject) -> bool {
- match (unit.target, go.target) {
- (Some(_), Some(_)) => go.distance < unit.distance,
+/// Which of the two hovered CGObjects — the character-model [`Hovered`] or the [`HoveredObject`] —
+/// is the nearer thing under the cursor, and so the one a click acts on. The reference makes one
+/// pick over all CGObjects and switches on type; benilla runs the character and GameObject picks
+/// separately, then chooses the nearer here. A GameObject wins only when it is strictly closer
+/// than the character pick (so a mailbox in front of a distant NPC takes the click; an NPC in
+/// front of a chest keeps it).
+///
+/// **Either slot of [`Hovered`] counts** ([`Hovered::any`]): a hovered *corpse* holds the ground
+/// against a farther GameObject exactly as a unit does. Reading only `target` here — as this did
+/// before corpses could be picked — would have handed the click, the cursor, the tooltip and the
+/// highlight to any GameObject on screen the instant the ray landed on a body instead of a unit.
+pub(crate) fn go_is_nearest(character: &Hovered, go: &HoveredObject) -> bool {
+ match (character.any(), go.target) {
+ (Some(_), Some(_)) => go.distance < character.distance,
(None, Some(_)) => true,
_ => false,
}
@@ -377,8 +411,14 @@ mod tests {
fn gameobject_wins_the_click_only_when_the_nearer_object() {
let hov = |t: bool, d: f32| Hovered {
target: t.then_some(Entity::PLACEHOLDER),
- guid: None,
distance: d,
+ ..Hovered::default()
+ };
+ // The same hover, landed on a CORPSE instead of a unit — the other slot of the one pick.
+ let corpse = |t: bool, d: f32| Hovered {
+ corpse: t.then_some(Entity::PLACEHOLDER),
+ distance: d,
+ ..Hovered::default()
};
let obj = |t: bool, d: f32| HoveredObject {
target: t.then_some(Entity::PLACEHOLDER),
@@ -395,6 +435,13 @@ mod tests {
assert!(go_is_nearest(&hov(true, 10.0), &obj(true, 4.0)));
assert!(!go_is_nearest(&hov(true, 4.0), &obj(true, 10.0)));
assert!(!go_is_nearest(&hov(true, 5.0), &obj(true, 5.0)));
+ // A hovered CORPSE competes on exactly the same terms as a unit (decision 1723): it is the
+ // other slot of the same pick, so it keeps the click from a farther or tied GameObject and
+ // yields only to a strictly nearer one.
+ assert!(!go_is_nearest(&corpse(true, 5.0), &obj(false, 0.0)));
+ assert!(!go_is_nearest(&corpse(true, 4.0), &obj(true, 10.0)));
+ assert!(!go_is_nearest(&corpse(true, 5.0), &obj(true, 5.0)));
+ assert!(go_is_nearest(&corpse(true, 10.0), &obj(true, 4.0)));
}
/// The latch survives the hover being cleared — which is the whole reason it exists.
diff --git a/crates/benilla-app/src/ui_action/targeting/world.rs b/crates/benilla-app/src/ui_action/targeting/world.rs
index 9ee704e6..ef9783d0 100644
--- a/crates/benilla-app/src/ui_action/targeting/world.rs
+++ b/crates/benilla-app/src/ui_action/targeting/world.rs
@@ -284,6 +284,7 @@ mod tests {
target: Some(Entity::from_raw_u32(2).unwrap()),
guid: Some(0x1234),
distance: 4.0,
+ ..Hovered::default()
};
click(&mut world, id);
assert!(rx.try_recv().is_err(), "a unit in front of the chest wins");
diff --git a/crates/benilla-app/src/ui_script/cinematic_tests.rs b/crates/benilla-app/src/ui_script/cinematic_tests.rs
index ad31d017..fb79c0f7 100644
--- a/crates/benilla-app/src/ui_script/cinematic_tests.rs
+++ b/crates/benilla-app/src/ui_script/cinematic_tests.rs
@@ -127,3 +127,47 @@ fn the_host_has_a_reference_name_for_the_keys_it_now_delivers() {
);
}
}
+
+/// **The one key a cinematic gives back.** The module doc above says the reference's `OnKeyDown`
+/// calls `RunBinding("SCREENSHOT")` by hand *because* the walk's gate is existence — and benilla
+/// shipped that transcribed line against a `RunBinding` that did not exist in the whole codebase.
+/// Every press of the screenshot key during a fly-by raised `attempt to call global 'RunBinding'`
+/// and took no picture, and the three tests above stayed green through all of it because they only
+/// ever asked about keys the frame *swallows*.
+#[test]
+fn the_screenshot_key_is_handed_back_to_its_binding() {
+ let mut s = ui_with_the_cinematic_frame();
+ // The binding table the passthrough reads: `GetBindingKey("SCREENSHOT")` has to answer, or the
+ // arm is skipped and the test proves nothing.
+ s.register_bindings(&crate::bindings::registry_commands());
+ s.seed_binding_set(1, None);
+ s.load_binding_set(1);
+ let key = s
+ .keybind_snapshot()
+ .into_iter()
+ .find(|(name, _)| name == "SCREENSHOT")
+ .and_then(|(_, keys)| keys.into_iter().next())
+ .expect("SCREENSHOT ships a default chord");
+
+ start_cinematic(&mut s);
+ let _ = s.take_keybind_requests();
+
+ // Consumed like every other key — the frame is up, so the binding layer must not also see it.
+ assert!(
+ s.frame_key_input(&key),
+ "the frame consumes {key} like any other key"
+ );
+ // …and handed straight back to its command by hand. This is the whole point of the arm: the
+ // request is what the host runs, and an error here means no screenshot.
+ assert_eq!(
+ s.take_keybind_requests(),
+ vec![benilla_ui::script::keybind::KeybindRequest::Run(
+ "SCREENSHOT".into()
+ )],
+ "the screenshot key must reach its binding through RunBinding"
+ );
+
+ // ESCAPE is still the skip, and still queues nothing on the binding channel.
+ assert!(s.frame_key_input("ESCAPE"));
+ assert!(s.take_keybind_requests().is_empty());
+}
diff --git a/crates/benilla-app/src/ui_script/mod.rs b/crates/benilla-app/src/ui_script/mod.rs
index 218973e1..f71a9179 100644
--- a/crates/benilla-app/src/ui_script/mod.rs
+++ b/crates/benilla-app/src/ui_script/mod.rs
@@ -450,9 +450,26 @@ fn capture_ui_active(capture: Option>) -> bool
fn arbitrate_pointer_over_ui(
egui: Option>,
hover: Res,
+ cinematic: Option>,
mut over: ResMut,
) {
- over.0 = egui.is_some_and(|e| e.0) || hover.0.is_some();
+ // **A cinematic covers the screen with a frame, so the pointer is over the UI wherever it
+ // is.** That is a fact about the reference's own layout, not a special case: `CinematicFrame`
+ // is `setAllPoints` + `enableMouse="true"`, and while it is up it is the only mouse target the
+ // hit test can reach — because `UIParent:Hide()` has taken every other frame out of it.
+ //
+ // benilla cannot yet reach that answer through the hit test. `crate::cinematic` substitutes
+ // `UiHidden` for the reference's `UIParent:Hide()` (decision 1699, for want of a HUD parented
+ // to `UIParent`), and `UiHidden` also switches the whole mouse feed off — ALT-Z's meaning,
+ // where the world *should* take the mouse. So `feed_ui_input` never hit-tested
+ // `CinematicFrame` at all and the world took every click of a fly-by: right-drag turned the
+ // character, left-click picked targets through the flying camera, the wheel zoomed a rig
+ // nobody could see. **This line states the layout fact directly, and it is the interim, not
+ // the fix** — restoring the 32 dropped `parent="UIParent"` declarations lets the hit test
+ // reach the same answer on its own, and this argument (and this parameter) is then dead code
+ // to delete.
+ let in_cinematic = cinematic.is_some_and(|c| c.is_playing());
+ over.0 = in_cinematic || egui.is_some_and(|e| e.0) || hover.0.is_some();
}
/// The session lifecycle — the VM's birth, identity, death, and the reload. Every edge function
diff --git a/crates/benilla-app/src/ui_tooltip/mod.rs b/crates/benilla-app/src/ui_tooltip/mod.rs
index af43c027..990e3f19 100644
--- a/crates/benilla-app/src/ui_tooltip/mod.rs
+++ b/crates/benilla-app/src/ui_tooltip/mod.rs
@@ -639,10 +639,13 @@ fn drive_mouseover_tooltip(
// and an IN_USE object all show the gold name plate while showing NO interact cursor — 0466's
// "no cursor AND no tooltip" coupling was the regression. Transports never reach here — they
// are excluded from the pick set itself (0466's correct half).
- let go = hovered_go
- .target
- .zip(hovered_go.guid)
- .filter(|_| unit.is_none() || go_is_nearest(&hovered, &hovered_go));
+ // "Nothing else was picked, or the GameObject is the nearer pick." **A hovered corpse counts
+ // as something else picked** (decision 1723) — it has no tooltip of its own yet, and the
+ // `unit.is_none()` short-circuit would otherwise hand a farther GameObject the gold plate the
+ // moment the ray landed on a body instead of a unit.
+ let go = hovered_go.target.zip(hovered_go.guid).filter(|_| {
+ (unit.is_none() && hovered.corpse.is_none()) || go_is_nearest(&hovered, &hovered_go)
+ });
if let Some((guid, state)) = unit.filter(|_| go.is_none()) {
// Push first (the engine's builder + the recolor's UnitReaction read the token), then
diff --git a/crates/benilla-formats/src/cinematics.rs b/crates/benilla-formats/src/cinematics.rs
index cc8202d9..91b898a3 100644
--- a/crates/benilla-formats/src/cinematics.rs
+++ b/crates/benilla-formats/src/cinematics.rs
@@ -43,7 +43,7 @@
//! |---|---|---|---|---|---|---|
//! | 1 | PalantirOfAzora | 0.7854 | 14.9 s | 10 | const 0 | 205 yd |
//! | 2 | FlybyUndead | **1.5708** | 102.0 s | 22 | 18 keys | 523 yd |
-//! | 122 | FlybyNightElf | 0.7854 | 102.3 s | 32 | 19 keys | 813 yd |
+//! | 122 | FlybyNightElf | 0.7854 | 102.0 s | 32 | 19 keys | 813 yd |
//! | 142 | FlyByHuman | 0.7854 | 87.5 s | 20 | const 2π | 663 yd |
//! | 162 | FlyByGnome | 0.7854 | 76.7 s | 27 | const 0 | 705 yd |
//! | 182 | FlyByTroll | 0.7854 | 57.8 s | 21 | 12 keys | 39 yd |
@@ -281,8 +281,13 @@ pub struct CinematicPath {
pub near_clip: f32,
/// The authored far clip — see [`Self::near_clip`].
pub far_clip: f32,
- /// How long the shot runs, milliseconds: the sequence band the tracks are keyed inside.
+ /// How long the shot runs, milliseconds — the *width* of the sequence band the tracks are
+ /// keyed inside (`end − start`), not its end. See [`Self::sample`] for why the two differ.
pub duration_ms: u32,
+ /// Where that band begins on the model's global timeline; added to the sample time so a shot
+ /// whose first key is not at zero (`FlybyNightElf`, `Scry_cam`) starts on its first key
+ /// instead of holding it.
+ band_start: u32,
origin: [f32; 3],
facing_sin_cos: (f32, f32),
camera: M2Camera,
@@ -309,11 +314,15 @@ impl CinematicPath {
.ok_or_else(|| anyhow::anyhow!("model carries no camera record"))?;
// The shot's length is the model's own sequence band (stride 0x44 records at header
// 0x1c/0x20, band `[start, end]` at +0x04/+0x08 — the same walk the emitter-timing bake
- // uses). Every shipped fly-by carries exactly one sequence, `[0, end]`, whose end equals
- // the last key timestamp on its position and target tracks alike; the band is what the
- // file *says*, so that is what we read, with the last key as the fallback for a file that
- // authors no sequence at all.
- let duration_ms = sequence_band_end(bytes)
+ // uses). The reference arms this as an ordinary M2 animation on sequence 0, so the shot
+ // runs the band: it *plays* for `end − start` and its tracks are keyed at absolute
+ // global-timeline stamps inside `[start, end]`. Eight of the ten shipped cameras band at
+ // `[0, end]` and the distinction is invisible on them — but `FlybyNightElf` bands at
+ // `[333, 102333]` and `Scry_cam` at `[33, 3333]`, and reading `end` as the duration
+ // played those two 333 ms / 33 ms too long with the opening frames frozen on the first
+ // key (which is what sampling `t = 0` against a track that starts at 333 returns).
+ // The fallback, for a file that authors no sequence at all, is the last key.
+ let (band_start, band_end) = sequence_band(bytes)
.or_else(|| {
[
camera.positions.keys.last().map(|k| k.0),
@@ -322,8 +331,10 @@ impl CinematicPath {
.into_iter()
.flatten()
.max()
+ .map(|end| (0, end))
})
- .unwrap_or(0);
+ .unwrap_or((0, 0));
+ let duration_ms = band_end.saturating_sub(band_start);
Ok(Self {
camera_id: row.id,
sound_id: row.sound_id,
@@ -331,6 +342,7 @@ impl CinematicPath {
near_clip: camera.near_clip,
far_clip: camera.far_clip,
duration_ms,
+ band_start,
origin: row.origin,
facing_sin_cos: row.origin_facing.sin_cos(),
camera,
@@ -339,6 +351,12 @@ impl CinematicPath {
/// Sample the shot at `ms` from its start, **end-clamped** at both ends.
///
+ /// `ms` is measured from the *shot's* start; the tracks are keyed on the model's global
+ /// timeline, so it is offset by [`Self::band_start`] before it reaches them. On eight of the
+ /// ten shipped cameras that offset is zero and the two are the same number; on
+ /// `FlybyNightElf` (band `[333, 102333]`) and `Scry_cam` (`[33, 3333]`) it is the difference
+ /// between opening on the authored first key and holding it for a third of a second first.
+ ///
/// Two steps, in this order. The reference's publish pass composes each track against its base
/// (`eye = position_base + positions(t)`, `target = target_position_base + target(t)`); then
/// the local frame is planted in the world by the camera row's origin and facing:
@@ -354,6 +372,7 @@ impl CinematicPath {
/// the dwarf (41) and human (81) intros; the sign-flipped and axis-swapped alternatives are
/// off by an order of magnitude (module doc).
pub fn sample(&self, ms: u32) -> CinematicView {
+ let ms = self.band_start.saturating_add(ms);
let eye = self.to_world(sample_against(
&self.camera.positions,
self.camera.position_base,
@@ -387,9 +406,17 @@ fn sample_against(track: &M2Track>, base: [f32; 3], ms: u3
std::array::from_fn(|i| base[i] + d[i])
}
-/// The end of the model's **first** sequence band (header `0x1c` count / `0x20` offset, entry
-/// stride `0x44`, band `[start, end]` at `+0x04`/`+0x08`). `None` for a model with no sequences.
-fn sequence_band_end(b: &[u8]) -> Option {
+/// The model's **first** sequence band, `(start, end)` in global-timeline milliseconds (header
+/// `0x1c` count / `0x20` offset, entry stride `0x44`, band at `+0x04`/`+0x08`). `None` for a model
+/// with no sequences.
+///
+/// **`start` is not always zero, and the shipped corpus is its own proof of these offsets.** Eight
+/// of the ten cameras band at `[0, end]`, but `FlybyNightElf` bands at `[333, 102333]` and
+/// `Scry_cam` at `[33, 3333]` — and in both files the band's `start` is *exactly* the first
+/// timestamp on the position and target tracks, while `end` is exactly the last. Two fields that
+/// land on the first and last key of every file in the corpus are the first and last key, which
+/// is why this is read rather than assumed to be a duration.
+fn sequence_band(b: &[u8]) -> Option<(u32, u32)> {
let le_u32 = |o: usize| -> Option {
b.get(o..o + 4)
.map(|s| u32::from_le_bytes([s[0], s[1], s[2], s[3]]))
@@ -398,7 +425,7 @@ fn sequence_band_end(b: &[u8]) -> Option {
if n == 0 {
return None;
}
- le_u32(o + 0x08)
+ Some((le_u32(o + 0x04)?, le_u32(o + 0x08)?))
}
#[cfg(test)]
@@ -471,16 +498,26 @@ mod tests {
// for, and the one a step/linear-only sampler would silently mangle.
assert_eq!(path.camera.positions.interp, 2, "sequence {id} position");
assert_eq!(path.camera.target.interp, 2, "sequence {id} target");
- // **The end condition**: the authored sequence band ends exactly on the last key of
- // both vector tracks, on every shipped fly-by — so "the band" and "the last key" are
- // the same number, and reading either gives the same playback length.
+ // **The end condition, and the proof of the band offsets.** The authored sequence
+ // band brackets both vector tracks exactly: its `start` is the first key's timestamp
+ // and its `end` is the last one's, on every shipped fly-by. Two header fields that
+ // land on the first and last key of all eight files are the first and last key — and
+ // that is what licenses reading the playback length as `end − start` rather than as
+ // `end`, which is the bug this assertion now pins (`FlybyNightElf` bands at
+ // `[333, 102333]`, so the two readings differ by a third of a second and by whether
+ // the shot opens on its first key or holds it).
for (what, track) in [
("position", &path.camera.positions),
("target", &path.camera.target),
] {
+ assert_eq!(
+ track.keys.first().map(|k| k.0),
+ Some(path.band_start),
+ "sequence {id} {what} track starts on the band"
+ );
assert_eq!(
track.keys.last().map(|k| k.0),
- Some(path.duration_ms),
+ Some(path.band_start + path.duration_ms),
"sequence {id} {what} track ends on the band"
);
}
diff --git a/crates/benilla-protocol/src/messages/update_object/fields/mod.rs b/crates/benilla-protocol/src/messages/update_object/fields/mod.rs
index 0c4c3241..61fdb465 100644
--- a/crates/benilla-protocol/src/messages/update_object/fields/mod.rs
+++ b/crates/benilla-protocol/src/messages/update_object/fields/mod.rs
@@ -796,6 +796,20 @@ impl ObjectFields {
self.get_u32(36).unwrap_or(0) & 0x01 != 0
}
+ /// `CORPSE_FLAG_LOOTABLE` (`0x20`, [`Self::corpse_flags`] bit 5) — the **PvP insignia** flag,
+ /// the one the reference's corpse cursor classifier `0x482740` reads at
+ /// `[[corpse+0x110]+0x74] >> 5 & 1` for its second leg, and the corpse right-click `0x5d6bf0`
+ /// reads again at `0x5d6c9c` for the matching send.
+ ///
+ /// **It is not "reclaimable"** — `object-layer/scratch/w2a.md` calls it both things in one
+ /// file (line 264 "reclaimable", line 618 "PvP flag") and only the second survives the byte
+ /// read: both readers pair it with the `SPELL_EFFECT_SKIN_PLAYER_CORPSE` learn-time latch
+ /// `[0xb700e8]`, which is a skinning precondition and has nothing to do with recovering your
+ /// own body. Nothing in the reclaim path consults this bit.
+ pub fn corpse_pvp_insignia(&self) -> bool {
+ self.corpse_flags() & 0x20 != 0
+ }
+
/// `DYNAMICOBJECT_CASTER` (fields 6–7, `OBJECT_END + 0x0`, a 2-field guid — vmangos
/// `UpdateFields_1_12_1.h:325`): the unit whose ground cast anchored this object.
/// Index-twin of [`Self::corpse_owner`] — each TYPEID reuses `OBJECT_END`; the TypeId (or
diff --git a/crates/benilla-protocol/src/world/writer/lifecycle.rs b/crates/benilla-protocol/src/world/writer/lifecycle.rs
index a724bbcb..29db15b7 100644
--- a/crates/benilla-protocol/src/world/writer/lifecycle.rs
+++ b/crates/benilla-protocol/src/world/writer/lifecycle.rs
@@ -60,15 +60,17 @@ impl WorldWriter {
self.send(opcode::CMSG_COMPLETE_CINEMATIC, &[])
}
- /// Advance a multi-camera cinematic to its next shot (`CMSG_NEXT_CINEMATIC_CAMERA`, empty
- /// body) — see the opcode's own note. Sent *between* cameras; the run still ends with exactly
- /// one [`complete_cinematic`](Self::complete_cinematic).
+ /// Announce the cinematic shot being armed (`CMSG_NEXT_CINEMATIC_CAMERA`, empty body) — see
+ /// the opcode's own note. Sent once per shot, **including the first**, so a stock single-camera
+ /// race intro sends exactly one; the run still ends with exactly one
+ /// [`complete_cinematic`](Self::complete_cinematic).
///
- /// No shipped 1.12 `CinematicSequences` row carries a second camera, so this never fires
- /// against a stock server today. It exists because the sequencing is the *client's*, not the
- /// data's: a row with two cameras is one the client must play as a single continuous
- /// cinematic, and playing it silently — without the packet the reference sends at `0x48efe0`
- /// — is the kind of drift that only surfaces on a server shipping its own DBCs.
+ /// The reference's send is inside the shot arm `0x48edf0` (`0x48ef11 push 0xfb` → the
+ /// `0x418190` builder → `0x5ab630` flush), immediately before that shot's narration starts —
+ /// *not* inside the shot advance `0x48efe0`, where benilla first placed it. That misreading
+ /// mattered on the shipped path rather than only on a private server's: every 1.12
+ /// `CinematicSequences` row carries exactly one camera, so "between cameras" meant benilla
+ /// sent this packet **never**.
pub fn next_cinematic_camera(&mut self) -> Result<()> {
self.send(opcode::CMSG_NEXT_CINEMATIC_CAMERA, &[])
}
diff --git a/crates/benilla-ui/src/script/keybind.rs b/crates/benilla-ui/src/script/keybind.rs
index 1110f566..3e9ed776 100644
--- a/crates/benilla-ui/src/script/keybind.rs
+++ b/crates/benilla-ui/src/script/keybind.rs
@@ -70,10 +70,16 @@ pub struct AddonBindingBody {
/// A host request queued by Lua: persist the live table as set `1`/`2` (`Save`), on which the
/// app writes `benilla-config/bindings/…` — and, for `Save(1)` issued while the character set was
-/// active, deletes the character file (the reference's confirmed delete-on-switch).
-#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+/// active, deletes the character file (the reference's confirmed delete-on-switch); or fire a
+/// named binding's action outright (`Run`).
+#[derive(Clone, Debug, PartialEq, Eq)]
pub enum KeybindRequest {
Save(u32),
+ /// `RunBinding("SCREENSHOT")` — run the command's action as if its chord had been pressed.
+ /// It is a *request* rather than something the VM can do itself because a host command's
+ /// action is engine-side by construction (see this module's header): the VM holds the command
+ /// name and its chords, never its body.
+ Run(String),
}
/// The reference's key-string gate — the whole of `SetBinding`'s refusal (decision 1295), in its
@@ -605,6 +611,22 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> {
Ok(if ok { Some(1) } else { None })
})?,
)?;
+ // `RunBinding(command)` — the reference's own passthrough verb. Its live caller is
+ // `CinematicFrame`'s `OnKeyDown`, which hands the SCREENSHOT chord back to its binding while
+ // swallowing every other key, so a player can still photograph a fly-by. Anything the VM
+ // holds a body for it could run itself; a host command's action is engine-side, so this
+ // queues and the app fires it in the same frame.
+ lua.globals().set(
+ "RunBinding",
+ lua.create_function(|lua, command: String| {
+ let mut model = lua.app_data_mut::().expect("model app_data");
+ let name = command.to_ascii_uppercase();
+ if model.keybinds.by_name.contains_key(&name) {
+ model.keybinds.requests.push(KeybindRequest::Run(name));
+ }
+ Ok(())
+ })?,
+ )?;
lua.globals().set(
"GetBindingKey",
lua.create_function(|lua, command: String| {
diff --git a/crates/benilla-ui/src/script/modelframe.rs b/crates/benilla-ui/src/script/modelframe.rs
index c44b7c25..a17e499a 100644
--- a/crates/benilla-ui/src/script/modelframe.rs
+++ b/crates/benilla-ui/src/script/modelframe.rs
@@ -125,6 +125,35 @@ fn int(v: &Value) -> i32 {
num(v) as i32
}
+/// **Shape A's gate** (decision 1717's taxonomy, for a *string* position): the client's
+/// `lua_isstring` accepts tags 3|4 — a string **or a number** — and nothing else. A number is
+/// rendered to decimal text and used as the path, which is why this coerces rather than matching
+/// `Value::String` alone.
+fn string_arg(v: &Value) -> Option> {
+ match v {
+ Value::String(s) => s
+ .to_str()
+ .ok()
+ .map(|t| std::borrow::Cow::Owned(t.to_string())),
+ Value::Number(n) => Some(std::borrow::Cow::Owned(n.to_string())),
+ Value::Integer(i) => Some(std::borrow::Cow::Owned(i.to_string())),
+ _ => None,
+ }
+}
+
+/// The client's own `Usage:` error, with the receiver's name interpolated the way it interpolates
+/// it — `GetName()`, or `` when the widget has none (`0x84c7f0`).
+fn usage(lua: &Lua, this: &Table, call: &str) -> mlua::Error {
+ let name = this
+ .get::("GetName")
+ .and_then(|f| f.call::>(this.clone()))
+ .ok()
+ .flatten()
+ .unwrap_or_else(|| "".to_string());
+ let _ = lua;
+ mlua::Error::runtime(format!("Usage: {name}:{call}"))
+}
+
pub(super) fn install(lua: &Lua) -> mlua::Result<()> {
let m = lua.create_table()?;
@@ -135,17 +164,27 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> {
// cannot answer a stale path from three frames ago. Only the `SetModel` arm is a `Model` verb —
// the unit arm is `PlayerModel`'s (`playermodel_install` below), which is why the paper-doll
// and dress-up panes are ``s and every corpus `` drives the path arm.
+ // **`SetModel(nil)` RAISES. It is not a clear** — `ClearModel` is the clear, and it is a
+ // separate entry in the same table. This binding said the opposite until 2026-08-30 ("the
+ // documented clear, and how the corpus writes 'no model'"), which was invented: the byte read
+ // (`0x76d950`, shape A) gates the argument with `lua_isstring` and raises
+ // `Usage: %s:SetModel("file")` on nil, absent, boolean, table, function, userdata and thread.
+ // Measured before changing it — all 5 corpus call sites pass a string literal and our own
+ // FrameXML never calls it — so nothing was relying on the leniency.
+ //
+ // **What is NOT reproduced, deliberately:** the reference's *second* raise,
+ // `Invalid model file: %s` (`0x878b44`), for a path that does not resolve — its model load is
+ // synchronous, so it knows. We render no FrameXML models at all, so we cannot evaluate that
+ // condition, and a raise whose predicate we have to guess is worse than a named omission
+ // (1134 §4). A path that would be invalid is simply stored.
m.set(
"SetModel",
lua.create_function(|lua, (this, path): (Table, Value)| {
- let path = match &path {
- Value::String(s) => Some(s.to_str()?.to_string()),
- // `SetModel(nil)` is the documented clear, and reaching it through the same setter
- // is how the corpus writes "no model" — not everything calls ClearModel.
- _ => None,
- };
+ let path = string_arg(&path)
+ .ok_or_else(|| usage(lua, &this, "SetModel(\"file\")"))?
+ .to_string();
with_model(lua, &this, |m| {
- m.path = path;
+ m.path = Some(path);
m.unit = None;
})
})?,
@@ -290,6 +329,50 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> {
})?,
)?;
+ // ── The two verbs that touch no pane state ──────────────────────────────────────────────
+
+ // `AdvanceTime()` — `0x878948[14]` -> `0x76eca0`. **It does nothing, and that is the verified
+ // behaviour, not a stub.** The function takes no Lua argument (no `lua_gettop`, no `tonumber`,
+ // no `tostring` anywhere in `[0x76eca0, 0x76ed65)`), returns no value, and its one reachable
+ // call is `0x76cfb0`, whose entire body is `mov eax,1; ret`. It writes no `CSimpleModel` field
+ // and no global. So there is no clock to advance here and no omission to name: a receiver
+ // check and zero returns IS the reference.
+ //
+ // Present because the shipped chain calls it three times — `Cooldown.lua`, and the glue
+ // character screens' `` on ``, which inherits this very table — and
+ // an addon that hooks one of those will call it too.
+ m.set(
+ "AdvanceTime",
+ lua.create_function(|lua, this: Table| with_model(lua, &this, |_| ()))?,
+ )?;
+
+ // `ReplaceIconTexture(path)` — `0x878948[15]` -> `0x76ed70`. **A material swap, not a content
+ // setter**: `0x76cfe0(0xe, path)` -> `0x710ec0` replaces the refcounted handle in
+ // `[CM2Model+0xa4][i]` for every M2 texture whose `type == 14`, plus the same-typed slots on
+ // the ribbon (`0x7b7950`) and particle (`0x7b4d20`) emitters. It sets no pane content, clears
+ // no unit, and stores nothing on the `0x3dc`-byte widget — the override lives on the CM2Model
+ // and dies with it when `SetModel`/`ClearModel` releases the instance.
+ //
+ // **Which is why storing nothing here is faithful rather than lazy.** The reference's two
+ // "not ready" cases behave OPPOSITELY: with a `CM2Model` present but its data not resident the
+ // call is queued on `[cm2+0x3c]` and replayed; with **no CM2Model at all**
+ // (`[widget+0x318] == 0`) it is **dropped and never replayed**. This engine renders no
+ // FrameXML models, so our panes are permanently the second case — dropping it is what the
+ // client does in exactly our state.
+ //
+ // The argument gate is real and is shape A: `lua_isstring` (tags 3|4) then `lua_tostring`,
+ // raising `Usage: %s:ReplaceIconTexture("texture")` on anything else. It is kept because it is
+ // the whole observable behaviour left.
+ m.set(
+ "ReplaceIconTexture",
+ lua.create_function(|lua, (this, path): (Table, Value)| {
+ if string_arg(&path).is_none() {
+ return Err(usage(lua, &this, "ReplaceIconTexture(\"texture\")"));
+ }
+ with_model(lua, &this, |_| ())
+ })?,
+ )?;
+
lua.set_named_registry_value(REG_MODEL_METHODS, m)?;
playermodel_install(lua)
}
diff --git a/crates/benilla-ui/src/script/tests/modelframe.rs b/crates/benilla-ui/src/script/tests/modelframe.rs
index 343b1fe1..eb8dbd92 100644
--- a/crates/benilla-ui/src/script/tests/modelframe.rs
+++ b/crates/benilla-ui/src/script/tests/modelframe.rs
@@ -155,6 +155,79 @@ fn a_player_model_is_a_model_plus_three_and_the_chain_runs_one_way() {
);
}
+/// **`SetModel(nil)` raises; `ClearModel()` clears. They are not the same verb.**
+///
+/// This binding asserted the opposite until 2026-08-30 — "the documented clear, and how the corpus
+/// writes 'no model'" — which was invented, not read. `0x76d950` is shape A (decision 1717's
+/// taxonomy): `lua_isstring` gates the argument and `Usage: %s:SetModel("file")` is raised on
+/// anything that is not a string or a number. `ClearModel 0x76db20` is a separate table entry.
+///
+/// The same gate on `ReplaceIconTexture 0x76ed70`, whose *whole* observable behaviour it is: the
+/// swap itself lands on a `CM2Model` we do not have, and with no CM2Model the reference drops the
+/// call and never replays it.
+///
+/// A NUMBER is accepted by both — `lua_isstring` takes tags 3|4 and the client renders it to
+/// decimal text — which is the half a `Value::String`-only match would get wrong.
+#[test]
+fn the_string_setters_gate_their_argument_and_a_number_is_a_string() {
+ let mut s = script();
+ s.set_screen_size(800.0, 600.0);
+ s.run(r#"g = CreateFrame("Model", "GateM", UIParent)"#)
+ .unwrap();
+
+ s.run(r#"GateM:SetModel("Interface\\Buttons\\A.mdx")"#)
+ .unwrap();
+ for bad in ["nil", "{}", "true", "print", ""] {
+ assert!(
+ s.run(&format!("GateM:SetModel({bad})")).is_err(),
+ "SetModel({bad}) must raise — it is not the clear"
+ );
+ }
+ // ...and none of those raises disturbed the path that was already set.
+ assert_eq!(
+ s.eval::("return GateM:GetModel()").unwrap(),
+ r"Interface\Buttons\A.mdx",
+ "a raised setter leaves the pane alone"
+ );
+ // A number IS a string to `lua_isstring`, and the client renders it to decimal text.
+ s.run("GateM:SetModel(42)").unwrap();
+ assert_eq!(s.eval::("return GateM:GetModel()").unwrap(), "42");
+
+ // ClearModel is the clear, takes no argument, and empties the pane.
+ s.run("GateM:ClearModel()").unwrap();
+ assert_eq!(
+ s.eval::>("return GateM:GetModel()").unwrap(),
+ None
+ );
+
+ // `ReplaceIconTexture` — same gate, and that gate is all of it. A good argument is accepted
+ // and changes NOTHING observable: the swap targets the CM2Model's type-14 textures, and a
+ // pane with no CM2Model drops the call (the reference's own `[widget+0x318] == 0` leg).
+ for bad in ["nil", "{}", "true", ""] {
+ assert!(
+ s.run(&format!("GateM:ReplaceIconTexture({bad})")).is_err(),
+ "ReplaceIconTexture({bad}) must raise"
+ );
+ }
+ s.run(r#"GateM:SetModel("Interface\\Buttons\\A.mdx")"#)
+ .unwrap();
+ s.run(r#"GateM:ReplaceIconTexture("Interface\\Icons\\INV_Misc_QuestionMark")"#)
+ .unwrap();
+ assert_eq!(
+ s.eval::("return GateM:GetModel()").unwrap(),
+ r"Interface\Buttons\A.mdx",
+ "it is a MATERIAL swap, not a content setter — the pane's model is untouched"
+ );
+
+ // `AdvanceTime` takes nothing, returns nothing, and does nothing — verified, not stubbed.
+ assert_eq!(
+ s.eval::("return table.getn({ GateM:AdvanceTime() })")
+ .unwrap(),
+ 0,
+ "AdvanceTime pushes no return value"
+ );
+}
+
/// **The whole block, both directions** — decision 1718's rule applied to the two model tables.
///
/// The lists below are the reference's OWN enumerations, transcribed entry-for-entry from
@@ -203,10 +276,11 @@ fn the_two_model_tables_are_the_references_own() {
const PLAYERMODEL_3: [&str; 3] = ["SetUnit", "RefreshUnit", "SetRotation"];
/// The subset of [`MODEL_23`] this client does not build. Named, not stubbed (1134 §4) — their
/// bodies are uncarved and they have no corpus caller. Moving a name OUT of here means the
- /// verb was actually implemented.
- const UNBUILT: [&str; 7] = [
- "AdvanceTime",
- "ReplaceIconTexture",
+ /// verb was actually implemented — as `AdvanceTime` and `ReplaceIconTexture` were, once
+ /// wow-re carved them (`ui/scratch/model-advancetime-replaceicontexture.md`). The five left
+ /// are the fog near/far set, whose `ClearFog` semantics over the colour/near/far triple nobody
+ /// has read; guessing that clear would read as knowledge.
+ const UNBUILT: [&str; 5] = [
"SetFogNear",
"GetFogNear",
"SetFogFar",