From ac857ada365a3167f77d0c7c7a7349bc0b31e7d2 Mon Sep 17 00:00:00 2001 From: grom Date: Sun, 25 Oct 2026 12:43:08 -0600 Subject: [PATCH] superwow: CanLootUnit, GetSpeed and UnitNameplate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three of the DLL's own manifest, and two of them needed no new feed at all — which is the point worth recording, because the row that called this family "todo, blocked: each needs new app-side state" was wrong about two of the three. `CanLootUnit` is the loot sparkle's test read through the token map: `UnitState` already carries `dead` and raw `dynamic_flags`, so a corpse that sparkles answers 1 and there is one source of truth for "has loot for me". The bit is per-viewer on the server, so "can" means can. `UnitNameplate` goes through the SAME `plate_frame` wrapper as ClassicAPI's `C_NamePlate.GetNamePlateForUnit`, welded by a test that asserts both names hand back one object — an addon may feature-detect either, and two different frames for one plate is the kind of bug that only shows up as an anchor in the wrong place. `GetSpeed` is the one that needed a field: the posture snapshot gained `speed`, filled from `net::current_speed`, the reference's own `CMovement::GetCurrentSpeed 0x7c4c90`. What could not be settled is written down rather than guessed over — upstream publishes no prose and nothing in the corpus calls it, so "current" is a reading, taken because it is the more informative of the two and because it is the only function in the client named after speed that a hook would return. Standing still it is 0, as that getter is. `SelfMovement` loses its `Eq` (an f32 field); nothing compared two snapshots — the feed assigns a fresh one each tick. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn --- crates/benilla-app/src/ui_unit.rs | 14 +- .../src/script/compat/classic_api/mod.rs | 3 +- .../script/compat/classic_api/nameplate.rs | 29 +++- .../src/script/compat/classic_api/state.rs | 1 + .../benilla-ui/src/script/compat/superwow.rs | 127 ++++++++++++++++++ crates/benilla-ui/src/script/model.rs | 8 +- .../src/script/tests/reference_surface.rs | 10 ++ docs/ADDON_COMPAT.md | 5 +- 8 files changed, 189 insertions(+), 8 deletions(-) diff --git a/crates/benilla-app/src/ui_unit.rs b/crates/benilla-app/src/ui_unit.rs index c6ec2b70..ae87fe3a 100644 --- a/crates/benilla-app/src/ui_unit.rs +++ b/crates/benilla-app/src/ui_unit.rs @@ -759,7 +759,10 @@ fn feed_self_movement( // `Option`, like `player` and the queries: a UI-only harness has no world at all, and the // indoor bit is the one field that needs one. Everything else still feeds without it. world: Option, - self_q: Query<&ObjectStore, With>, + // The speed block is `Option` on its own account, not just for a harness: it arrives with the + // create block's LIVING half, so there is a window at world entry where the player exists and + // its speeds do not. + self_q: Query<(&ObjectStore, Option<&crate::net::UnitSpeeds>), With>, // The camera rig, for `IsMouselooking()`. `Option` for the same reason `world` is: a UI-only // harness builds no camera, and every other field still feeds without one. rig: Option>, @@ -769,7 +772,10 @@ fn feed_self_movement( }; use crate::creature_anim::move_flags; let flags = player.as_deref().map_or(0, |p| p.move_flags()); - let store = self_q.iter().next(); + let (store, speeds) = match self_q.iter().next() { + Some((store, speeds)) => (Some(store), speeds), + None => (None, None), + }; script.set_self_movement(benilla_ui::script::SelfMovement { swimming: flags & move_flags::SWIMMING != 0, mounted: store.is_some_and(|s| s.0.unit_mount_display_id() > 0), @@ -785,6 +791,10 @@ fn feed_self_movement( | move_flags::TURN_RIGHT) != 0, shapeshift_form: store.map_or(0, |s| s.0.unit_shapeshift_form()), + // SuperWoW's `GetSpeed()`: the reference's own `CMovement::GetCurrentSpeed` walk over our + // speed block and the same move flags every field above reads, so the number cannot + // disagree with `swimming` or `moving` about what the player is doing this tick. + speed: speeds.map_or(0.0, |s| crate::net::current_speed(&s.0, flags)), mouselooking: rig.is_some_and(|r| r.mouselooking()), }); } diff --git a/crates/benilla-ui/src/script/compat/classic_api/mod.rs b/crates/benilla-ui/src/script/compat/classic_api/mod.rs index de39f42d..6316fffd 100644 --- a/crates/benilla-ui/src/script/compat/classic_api/mod.rs +++ b/crates/benilla-ui/src/script/compat/classic_api/mod.rs @@ -55,7 +55,8 @@ mod map; mod math_ext; mod merchant_frame; mod misc; -mod nameplate; +// `pub(in …compat)` for its `plate_frame`, which SuperWoW's `UnitNameplate` shares. +pub(in crate::script::compat) mod nameplate; mod new_items; mod player_cache; mod player_info; diff --git a/crates/benilla-ui/src/script/compat/classic_api/nameplate.rs b/crates/benilla-ui/src/script/compat/classic_api/nameplate.rs index 5acf1b4b..1f175132 100644 --- a/crates/benilla-ui/src/script/compat/classic_api/nameplate.rs +++ b/crates/benilla-ui/src/script/compat/classic_api/nameplate.rs @@ -28,8 +28,13 @@ use crate::script::object::frame_wrapper; use crate::script::unit::{guid_token, token_guid}; use crate::script::Model; -/// A plate's Lua frame value, by the arena id its handle maps to. -fn plate_frame(lua: &Lua, frame: crate::widget::FrameHandle) -> mlua::Result { +/// A plate's Lua frame value, by the arena id its handle maps to. Shared with SuperWoW's +/// `UnitNameplate` (`compat/superwow.rs`), which is the same lookup under the older name — one +/// wrapper, so the two cannot hand back different objects for one plate. +pub(in crate::script::compat) fn plate_frame( + lua: &Lua, + frame: crate::widget::FrameHandle, +) -> mlua::Result
{ let id = lua .app_data_mut::() .expect("model app_data") @@ -271,6 +276,26 @@ mod tests { ); } + /// SuperWoW's `UnitNameplate("unit")` is this same lookup under the older name, and hands + /// back the **same object** — an addon that feature-detects one and falls back to the other + /// must not get two different frames for one plate. + #[test] + fn unit_nameplate_is_the_same_frame_as_the_namespaced_getter() { + let s = script(); + assert!(s + .eval::( + r#"return UnitNameplate("target") ~= nil + and UnitNameplate("target") == C_NamePlate.GetNamePlateForUnit("target")"# + ) + .unwrap()); + assert!( + s.eval::(r#"return UnitNameplate("pet") == nil"#) + .unwrap(), + "a unit with no plate up is nil, not a raise" + ); + assert!(s.eval::("return UnitNameplate() == nil").unwrap()); + } + #[test] fn retiring_the_plates_empties_the_lists() { let mut s = script(); diff --git a/crates/benilla-ui/src/script/compat/classic_api/state.rs b/crates/benilla-ui/src/script/compat/classic_api/state.rs index b4e1301e..fb330ace 100644 --- a/crates/benilla-ui/src/script/compat/classic_api/state.rs +++ b/crates/benilla-ui/src/script/compat/classic_api/state.rs @@ -113,6 +113,7 @@ mod tests { rooted: false, moving: false, shapeshift_form: 3, + speed: 0.0, }); assert_eq!(s.eval::("return IsSwimming()").unwrap(), 1); diff --git a/crates/benilla-ui/src/script/compat/superwow.rs b/crates/benilla-ui/src/script/compat/superwow.rs index f994589c..704b9d51 100644 --- a/crates/benilla-ui/src/script/compat/superwow.rs +++ b/crates/benilla-ui/src/script/compat/superwow.rs @@ -218,6 +218,77 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { // is dropped by the same rule every routed line follows, which is upstream's behaviour too — // the function appends to an open file. // + // CanLootUnit("unit"): whether that unit's corpse holds loot for us. Needs no new feed — + // [`UnitState`] already carries `dynamic_flags` raw and `dead`, so this is the same two-part + // test the loot sparkle makes on the engine side (`creature_anim/spell_visual.rs`'s + // `shows_loot_sparkle`: `unit_is_dead() && unit_lootable()`), read through the token map + // instead of the ECS. One source of truth: a corpse that sparkles answers 1 here. + // + // `UNIT_DYNFLAG_LOOTABLE` (bit `0x1` of `UNIT_DYNAMIC_FLAGS`) is **per viewer** on the server + // — vmangos raises it only for players who may actually loot — so permission is already in + // the bit, and "can" means can, not merely "has loot". A unit with no descriptor (an + // out-of-range party member, a token naming nothing) answers nil rather than 0. + // + // 1 or nil, the shape every 1.12 predicate answers in, which an addon tests with `== 1`. + g.set( + "CanLootUnit", + lua.create_function(|lua, token: Option| { + let model = lua.app_data_ref::().expect("model app_data"); + let u = token.as_deref().and_then(|t| model.unit(t)); + Ok(crate::script::binding_abi::flag( + u.is_some_and(|u| u.dead && u.dynamic_flags & 0x1 != 0), + )) + })?, + )?; + + // UnitNameplate("unit"): that unit's nameplate frame, nil when it has none up. The same + // lookup ClassicAPI's `C_NamePlate.GetNamePlateForUnit` makes, through the same + // `plate_frame` wrapper, so the two names cannot hand back different objects for one plate — + // which matters here, since an addon may well feature-detect one and fall back to the other. + // + // benilla's plates are engine frames with no FrameXML container, so this is the plate itself + // (see `compat/classic_api/nameplate.rs`'s header for the same note about modern's wrapper). + g.set( + "UnitNameplate", + lua.create_function(|lua, token: Option| { + let frame = { + let model = lua.app_data_ref::().expect("model app_data"); + token + .as_deref() + .and_then(|t| model.unit(t)) + .map(|u| u.guid) + .filter(|g| *g != 0) + .and_then(|g| model.nameplates.frame_for_guid(g)) + }; + match frame { + Some(frame) => Ok(Some(super::classic_api::nameplate::plate_frame( + lua, frame, + )?)), + None => Ok(None), + } + })?, + )?; + + // GetSpeed(): the player's own current movement speed in yards a second, from the per-tick + // posture snapshot — the engine side computes it with `net.rs`'s `current_speed`, which is + // the reference's own `CMovement::GetCurrentSpeed 0x7c4c90` walk (swimming first, then walk + // mode as `min(walk, run)`, then run, each backward leg a `min` with its forward field). + // + // **What could not be settled**: upstream publishes no usage string for this one (it takes no + // argument) and no addon in grom's corpus calls it, so whether its number is the *current* + // speed or the unit's *run* speed is not knowable from here. Current is the reading taken, + // because it is the one function in the client named after speed that a hook would return, + // and because it is the strictly more informative of the two — a snare or a mount shows in + // it. The consequence to know: standing still, this is 0, exactly as the reference's own + // getter is (`flags & ANY_MOVE == 0`). + g.set( + "GetSpeed", + lua.create_function(|lua, ()| { + let model = lua.app_data_ref::().expect("model app_data"); + Ok(f64::from(model.self_movement.speed)) + })?, + )?; + // The second argument is the DLL's own: its usage string reads `CombatLogAdd("text" [, // addToRawLog])`, printed beside the `Logs\WoWCombatLog.txt` literal it writes to. Truthy, // the line is ALSO fired as `RAW_COMBATLOG`, so an addon listening there sees it beside the @@ -693,6 +764,62 @@ mod tests { ); } + /// `CanLootUnit("unit")` is the loot sparkle's own two-part test read through the token map: + /// dead, and `UNIT_DYNAMIC_FLAGS` bit `0x1`. A unit with no descriptor answers nil, not 0. + #[test] + fn can_loot_unit_is_dead_plus_the_lootable_bit() { + let mut s = UiScript::new().unwrap(); + let unit = |dead: bool, flags: u32| { + Some(UnitState { + exists: true, + has_object: true, + dead, + dynamic_flags: flags, + ..Default::default() + }) + }; + s.set_unit("target", unit(true, 0x1)); + s.set_unit("mouseover", unit(true, 0x4)); // tapped by someone else, no loot for us + s.set_unit("focus", unit(false, 0x1)); // alive and flagged: not a corpse yet + + assert_eq!(s.eval::(r#"return CanLootUnit("target")"#).unwrap(), 1); + assert_eq!( + s.eval::>(r#"return CanLootUnit("mouseover")"#) + .unwrap(), + None, + "nil, not 0 — the 1.12 predicate shape" + ); + assert_eq!( + s.eval::>(r#"return CanLootUnit("focus")"#) + .unwrap(), + None, + "the bit alone is not enough: the sparkle tests dead as well" + ); + assert_eq!( + s.eval::>(r#"return CanLootUnit("party1")"#) + .unwrap(), + None, + "no snapshot at all" + ); + assert_eq!(s.eval::>("return CanLootUnit()").unwrap(), None); + } + + /// `GetSpeed()` reads the posture snapshot's own number, and is 0 before anything feeds one. + #[test] + fn get_speed_reads_the_fed_snapshot() { + let mut s = UiScript::new().unwrap(); + assert_eq!( + s.eval::("return GetSpeed()").unwrap(), + 0.0, + "standing still — and before the first feed, which read the same" + ); + s.set_self_movement(crate::script::SelfMovement { + speed: 14.0, // epic mount + ..Default::default() + }); + assert_eq!(s.eval::("return GetSpeed()").unwrap(), 14.0); + } + /// `CombatLogAdd(text)` queues one line per call, in order, for the app to append to /// `WoWCombatLog.txt`; the drain empties it. /// diff --git a/crates/benilla-ui/src/script/model.rs b/crates/benilla-ui/src/script/model.rs index 099c4a57..86a74ab3 100644 --- a/crates/benilla-ui/src/script/model.rs +++ b/crates/benilla-ui/src/script/model.rs @@ -370,7 +370,9 @@ pub struct PlayerCacheEntry { /// Lua has no path to any of these at all (no mounted/swim/indoor getter exists on the client), /// so there is no reference default to match — `Default` (everything `false`/`0`) is only ever /// observed for the one frame before the feed system's first run. -#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +// No `Eq`: [`SelfMovement::speed`] is an `f32`. Nothing compares two snapshots for equality — +// the feed assigns a fresh one each tick — so the bound was never load-bearing. +#[derive(Debug, Clone, Copy, Default, PartialEq)] pub struct SelfMovement { /// `MOVEFLAG_SWIMMING`. pub swimming: bool, @@ -390,6 +392,10 @@ pub struct SelfMovement { pub moving: bool, /// `UNIT_FIELD_BYTES_2` byte 3, `0` not shapeshifted (`ShapeshiftForm.dbc` row otherwise). pub shapeshift_form: u8, + /// SuperWoW's `GetSpeed()`: the player's current speed in yd/s, `benilla-app`'s + /// `net::current_speed` over this unit's own speed block and move flags — the reference's + /// `CMovement::GetCurrentSpeed 0x7c4c90`. 0 standing still, as that getter is. + pub speed: f32, /// `IsMouselooking()`: the camera rig's own mouse-look flag, the reference's `[cam+0x90] & 1` /// (`CameraControl::mouselooking`) — a right-held turn or a both-button run, not a /// left-button orbit. This one *is* 1.12 data; it is here rather than behind its own feed diff --git a/crates/benilla-ui/src/script/tests/reference_surface.rs b/crates/benilla-ui/src/script/tests/reference_surface.rs index ef113890..084b7d2b 100644 --- a/crates/benilla-ui/src/script/tests/reference_surface.rs +++ b/crates/benilla-ui/src/script/tests/reference_surface.rs @@ -274,6 +274,16 @@ fn allowed_beyond_1_12() -> HashSet<&'static str> { "Clickthrough", "SetMouseoverUnit", "SpellInfo", + // SuperWoW's `CanLootUnit`, `GetSpeed` and `UnitNameplate` (script/compat/superwow.rs), + // three facts 1.12's Lua cannot reach: whether a corpse holds loot for us (the dyn-flag + // bit the loot sparkle reads, which is per-viewer on the server so it means *may* loot), + // the player's own speed in yd/s (the reference computes it C-side for the movement code + // and publishes it nowhere), and a unit's nameplate frame (1.12's only route is scanning + // `WorldFrame:GetChildren()` and reading regions positionally). `UnitNameplate` is the + // same lookup as `C_NamePlate.GetNamePlateForUnit` through one shared wrapper. + "CanLootUnit", + "GetSpeed", + "UnitNameplate", // SuperWoW's `CombatLogAdd(text)` (script/compat/superwow.rs): one line appended to // `WoWCombatLog.txt`, which 1.12 writes C-side off its own chat router and gives Lua no // way into. It is also a detection name in its own right — BigWigs' Spell Requests gates diff --git a/docs/ADDON_COMPAT.md b/docs/ADDON_COMPAT.md index 8bfb4ea8..bedc7a95 100644 --- a/docs/ADDON_COMPAT.md +++ b/docs/ADDON_COMPAT.md @@ -80,10 +80,11 @@ The usage strings, verbatim, for what is left: | `TrackUnit("unit")` / `UntrackUnit("unit")` | todo — pfUI calls `TrackUnit` for every party and raid member under `C.unitframes.track_group` (`modules/superwow.lua:96-127`, inside `pcall`), to put the group on the minimap | | `UnitPosition("unit")` | done | | `GetPlayerBuffID(buffIndex)` | done | -| `UnitNameplate("unit")` | todo — the plate frame for a unit; `NamePlates::guid_for_frame` already keys the other direction | -| `CanlootUnit("unit")` | todo. **Note the lowercase `l`**: the *usage* string says `CanlootUnit` where the registered name is `CanLootUnit`. The registered name is what an addon calls; the typo is in the error message only | +| `UnitNameplate("unit")` | **done** — `compat/superwow.rs`, over `NamePlates::frame_for_guid`, through the **same** `plate_frame` wrapper ClassicAPI's `C_NamePlate.GetNamePlateForUnit` uses, so the two names cannot hand back different objects for one plate. Welded by a test that asserts exactly that, because an addon may feature-detect either | +| `CanlootUnit("unit")` | **done** — needed no new feed: `UnitState` already carries `dead` and raw `dynamic_flags`, so it is the loot sparkle's own two-part test (`shows_loot_sparkle`: `unit_is_dead() && unit_lootable()`) read through the token map instead of the ECS, and a corpse that sparkles answers 1. Bit `0x1` is **per viewer** on the server — vmangos raises it only for players who may loot — so "can" really means can. **Note the lowercase `l`**: the *usage* string says `CanlootUnit` where the registered name is `CanLootUnit`; the registered name is what an addon calls, and the typo is in the error message only | | `GetMapPositionWorldLoc(continentIndex, zoneIndex, mapX, mapY)` | todo — map fraction to world XY, which `benilla-world`'s own map maths already does for the world map | | `GetMapBoundaries(continentIndex, zoneIndex)` | todo — the zone's world extent, the same table that conversion reads | +| `GetSpeed()` (no usage string: it takes no argument) | **done** — one new field on the posture snapshot, filled by `ui_unit.rs`'s `feed_self_movement` from `net.rs`'s `current_speed`, the reference's own `CMovement::GetCurrentSpeed 0x7c4c90` walk over our speed block and the same move flags every other field of that snapshot reads. **What could not be settled**: upstream publishes no prose and no addon in the corpus calls it, so whether its number is the *current* speed or the unit's *run* speed is unknowable from here. Current is the reading taken — it is the one client function named after speed that a hook would return, and the strictly more informative of the two (a snare or a mount shows in it). The consequence to know: standing still it is 0, exactly as that getter is | | `GetInboxItemLink(messageIndex)` | done (built for ClassicAPI's `Mail`) | | `GetSoundDriverName(driverIndex)` | todo — with `GetSoundDevices`, `GetNumSoundDrivers`, `GetLoadedSoundDriver`: four getters over the audio backend. benilla has one backend and no enumeration, so these answer a one-row list honestly or not at all; no addon in the corpus calls any of them | | `GetQuestIDForLogIndex(questIndex)` / `GetQuestLinkForLogIndex(questIndex)` | done |